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

AppEntry should be exported #780

Closed
otavio opened this issue Apr 15, 2019 · 6 comments
Closed

AppEntry should be exported #780

otavio opened this issue Apr 15, 2019 · 6 comments

Comments

@otavio
Copy link
Contributor

otavio commented Apr 15, 2019

When splitting applications, it is expected to have an app or similar function which constructs the routes.

For example:

pub fn app(addr: Addr<actor::Machine>) -> App<AppEntry, Body> {
    App::new()
        .data(API::new(addr))
        .route("/info", web::get().to(API::info))
        .route("/log", web::get().to(API::probe))
        .route("/probe", web::post().to(API::probe))
        .route(
            "/update/download/abort",
            web::post().to(API::download_abort),
        )
}

is expected to work. However, ApiEntry is not exposed.

It is used at:

actix-web/src/app.rs

Lines 41 to 56 in 7a28b32

impl App<AppEntry, Body> {
/// Create application builder. Application can be configured with a builder-like pattern.
pub fn new() -> Self {
let fref = Rc::new(RefCell::new(None));
App {
endpoint: AppEntry::new(fref.clone()),
data: Vec::new(),
services: Vec::new(),
default: None,
factory_ref: fref,
config: AppConfigInner::default(),
external: Vec::new(),
_t: PhantomData,
}
}
}

So it'd make sense to have it exported.

@fafhrd91
Copy link
Member

App is not designed for this kind of usage. Would App::configure() help?

@otavio
Copy link
Contributor Author

otavio commented Apr 15, 2019

Possibly. I will try to adapt our code for it.

A mention of this would be good, on the migration document.

@otavio
Copy link
Contributor Author

otavio commented Apr 15, 2019

Yes, I went with App::configure to define the routes and like. It worked well.

Bellow is the code change:

pub fn configure(cfg: &mut web::RouterConfig, addr: Addr<actor::Machine>) {
    cfg.data(API::new(addr))
        .route("/info", web::get().to(API::info))
        .route("/log", web::get().to(API::log))
        .route("/probe", web::post().to(API::probe))
        .route(
            "/update/download/abort",
            web::post().to(API::download_abort),
        );
}

@otavio otavio closed this as completed Apr 15, 2019
@LukeMathWalker
Copy link
Contributor

I just ran in this exact same issue - it should probably be highlighted in App's documentation.

@winston0410
Copy link

Exact same issue.

@robjtede
Copy link
Member

robjtede commented Aug 7, 2021

App is a complex type and not intended to be returned from a function directly. The App::configure method provides a way to move routing setup to a shared function. In rare cases, a simple macro can be used for App setup. References: #780 #1005 #1156 #2039 #2073 #2082 #2301

@actix actix locked as resolved and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants