-
Notifications
You must be signed in to change notification settings - Fork 352
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
service trait takes request type parameter #232
Conversation
I'll wait on #229 so this PR is smaller. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
f: F, | ||
) -> FnServiceConfig<F, Fut, Cfg, Srv, Err> | ||
) -> FnServiceConfig<F, Fut, Cfg, Srv, Req, Err> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe FnServiceConfig<F, Fut, Cfg, Req>
would be enough type. Srv and Err type are actuall associate types for Fut so they can be unconstraint.
pub struct FnServiceConfig<F, Fut, Cfg, Srv, Req, Err> | ||
where | ||
F: Fn(Cfg) -> Fut, | ||
Fut: Future<Output = Result<Srv, Err>>, | ||
Srv: Service, | ||
Srv: Service<Req>, | ||
{ | ||
f: F, | ||
_t: PhantomData<(Fut, Cfg, Srv, Err)>, | ||
_t: PhantomData<(Fut, Cfg, Req, Srv, Err)>, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like previous comment.
pub struct FnServiceConfig<F, Fut, Cfg, Req>
where
F: Fn(Cfg) -> Fut,
Fut: Future,
{
f: F,
_t: PhantomData<(Fut, Cfg, Req)>,
}
} | ||
|
||
impl<F, Fut, Cfg, Srv, Err> FnServiceConfig<F, Fut, Cfg, Srv, Err> | ||
impl<F, Fut, Cfg, Srv, Req, Err> FnServiceConfig<F, Fut, Cfg, Srv, Req, Err> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
impl<F, Fut, Cfg, Srv, Req, Err> FnServiceConfig<F, Fut, Cfg, Req>
{ | ||
fn new(f: F) -> Self { | ||
FnServiceConfig { f, _t: PhantomData } | ||
} | ||
} | ||
|
||
impl<F, Fut, Cfg, Srv, Err> Clone for FnServiceConfig<F, Fut, Cfg, Srv, Err> | ||
impl<F, Fut, Cfg, Srv, Req, Err> Clone for FnServiceConfig<F, Fut, Cfg, Srv, Req, Err> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
impl<F, Fut, Cfg, Srv, Req, Err> Clone for FnServiceConfig<F, Fut, Cfg, Req>
@@ -275,13 +275,13 @@ where | |||
} | |||
} | |||
|
|||
impl<F, Fut, Cfg, Srv, Err> ServiceFactory for FnServiceConfig<F, Fut, Cfg, Srv, Err> | |||
impl<F, Fut, Cfg, Srv, Req, Err> ServiceFactory<Req> | |||
for FnServiceConfig<F, Fut, Cfg, Srv, Req, Err> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
impl<F, Fut, Cfg, Srv, Req, Err> ServiceFactory
for FnServiceConfig<F, Fut, Cfg, Req>
PR Type
Improvement
PR Checklist
Overview
It's advantageous to accept multiple request types for a given service. I believe this will simplify some of the complex types and handling in actix-web http1/http2 request dispatching.
Obviously massive breaking change to actix-service. Other breaking changes include:
actix-connect:
actix-server: