Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trying to improve the documentation for App::data() #1736

Closed
wants to merge 9 commits into from
28 changes: 18 additions & 10 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,24 @@ where
InitError = (),
>,
{
/// Set application data. Application data could be accessed
/// by using `Data<T>` extractor where `T` is data type.
///
/// **Note**: http server accepts an application factory rather than
/// an application instance. Http server constructs an application
/// instance for each thread, thus application data must be constructed
/// multiple times. If you want to share data between different
/// threads, a shared object should be used, e.g. `Arc`. Internally `Data` type
/// uses `Arc` so data could be created outside of app factory and clones could
/// be stored via `App::app_data()` method.
/// Adds application data.
/// Application data can be accessed by using a `Data<T>` extractor where `T` is the data type.
///
/// The state is managed on a per-type basis and as such there can be
/// at most one value for any given type.
/// This means that only the first invocation of this function per type will have an effect,
/// all later ones will be ignored.
Comment on lines +80 to +81
Copy link
Member

@robjtede robjtede Jan 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true, or is it the last invocation is used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did look at the code back then and I thought this is what it does but I can take another look.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to be honest: I don't understand (anymore) how this is used. If one of you has any insights I'll happily add it otherwise I'd just remove that sentence.

It would be good to document the behavior though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know what you're right, and that's surprising behavior actually because .app_data works differently; a bug, even.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity (because I couldn't trace it anymore but I know I did in the past): Can you point me at the location where this happens?

///
/// Internally the data will be wrapped in an `Arc`.
/// If your data is already wrapped in an `Arc`
/// you can instead store it directly using the `App::app_data()` function.
robjtede marked this conversation as resolved.
Show resolved Hide resolved
///
/// **Note**: `HttpServer` accepts an application factory rather than
/// an application instance (`App`).
/// `HttpServer` constructs an application instance for each thread,
/// thus application data must be constructed multiple times.
/// If you want to share data between different threads,
/// a shared object should be used, e.g. `Arc`.
///
/// ```rust
/// use std::cell::Cell;
Expand Down