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

export handler::Handler #1339

Closed
wants to merge 4 commits into from
Closed

export handler::Handler #1339

wants to merge 4 commits into from

Conversation

sergeysova
Copy link

@sergeysova sergeysova commented Feb 4, 2020

I want to implement a custom Factory to strictly define arguments for handlers

impl<F, B, T, R, O> Handler<F, T, R, O>
where
    F: BoundFactory<B, T, R, O>,
    R: Future<Output = O>,
    O: Responder,
{
    pub fn new(hnd: F) -> Self {
        Handler {
            hnd,
            _t: PhantomData,
        }
    }
}


pub trait BoundFactory<B, T, R, O>: Clone + 'static
where
    R: std::future::Future<Output = O>,
    O: actix_web::Responder,
{
    fn call(&self, bound: B, param: T) -> R;
}

impl<Func, B0, R, O> BoundFactory<(B0,), (), R, O> for Func
where
    Func: Fn(B0) -> R + Clone + 'static,
    R: std::future::Future<Output = O>,
    O: actix_web::Responder,
{
    fn call(&self, bound: (B0,), _: ()) -> R {
        (self)(bound.0)
    }
}
impl<Func, B0, A0, R, O> BoundFactory<(B0,), (A0,), R, O> for Func
where
    Func: Fn(B0, A0) -> R + Clone + 'static,
    R: std::future::Future<Output = O>,
    O: actix_web::Responder,
{
    fn call(&self, bound: (B0,), args: (A0,)) -> R {
        (self)(bound.0, args.0)
    }
}

impl<Func, B0, B1, R, O> BoundFactory<(B0, B1), (), R, O> for Func
where
    Func: Fn(B0, B1) -> R + Clone + 'static,
    R: std::future::Future<Output = O>,
    O: actix_web::Responder,
{
    fn call(&self, bound: (B0, B1), _: ()) -> R {
        (self)(bound.0, bound.1)
    }
}

impl<Func, B0, B1, A0, A1, R, O> BoundFactory<(B0, B1), (A0, A1), R, O> for Func
where
    Func: Fn(B0, B1, A0, A1) -> R + Clone + 'static,
    R: std::future::Future<Output = O>,
    O: actix_web::Responder,
{
    fn call(&self, bound: (B0, B1), args: (A0, A1)) -> R {
        (self)(bound.0, bound.1, args.0, args.1)
    }
}

I want to implement a custom Factory to strictly define arguments for handlers
@robjtede robjtede requested a review from a team February 4, 2020 10:06
@cetra3
Copy link
Contributor

cetra3 commented Feb 4, 2020

I think Handler is an internal struct and should remain that way.

You can use Service instead to accomplish what you can do. I had a PR to simplify this down a bit, but was closed.

@sergeysova
Copy link
Author

@cetra3 Can you show me example with Service please?

@cetra3
Copy link
Contributor

cetra3 commented Feb 5, 2020

@sergeysova see my PR.

@sergeysova
Copy link
Author

@JohnTitor what do you think?

I tries to implement actix-swagger(cargo-swagg) code generator. I need a factory with required arguments.

@cdbattags
Copy link
Member

I think Handler is an internal struct and should remain that way.

You can use Service instead to accomplish what you can do. I had a PR to simplify this down a bit, but was closed.

I agree with this sentiment I believe this can already be done

@sergeysova
Copy link
Author

Can you show me an example, I want to define Factory with required arguments and types and some optional rest.

For example:

fn my_handler(body: web::Json<Type>, another: Another) {}

fn handler2(body: web::Json<Type>, app: web::Data<App>) {}

fn handler3(body: web::Json<Type>) {}

fn handler_wr(body: web::Json<Another>) {}

I want to have some method, that receives any function that have first argument of specified type, and rest arguments just impl FromRequst.

// ok
bind_handler(my_handler);
bind_handler(handler2);
bind_handler(handler3);

// fail
bind_handler(handler_wr);

@robjtede
Copy link
Member

@sergeysova is this still something you need access to?

@sergeysova
Copy link
Author

@robjtede yes. It should be useful

@robjtede robjtede requested a review from a team June 10, 2020 16:49
@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2020

Codecov Report

Merging #1339 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1339      +/-   ##
==========================================
- Coverage   53.49%   53.48%   -0.02%     
==========================================
  Files         128      131       +3     
  Lines       11979    11982       +3     
==========================================
  Hits         6408     6408              
- Misses       5571     5574       +3     
Impacted Files Coverage Δ
src/lib.rs 68.42% <ø> (ø)
actix-http/src/header/common/if_match.rs
actix-http/src/header/common/etag.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/expires.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/if_modified_since.rs 0.00% <0.00%> (ø)
actix-http/src/header/common/last_modified.rs 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2c71cc...e66ca71. Read the comment docs.

@robjtede
Copy link
Member

@sergeysova Ok, what's the reason that the exposed Factory trait isnt enough? Seems weird that a struct would be useful to you in this.

@robjtede robjtede added the A-web project: actix-web label Sep 14, 2020
@robjtede robjtede mentioned this pull request Dec 22, 2020
2 tasks
@robjtede robjtede closed this Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web B-semver-patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants