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

Implement common trait(s) to expose common traits of HTTP message (header) types #592

Open
djc opened this issue Feb 27, 2023 · 3 comments

Comments

@djc
Copy link

djc commented Feb 27, 2023

All HTTP messages in the http crate contain a bunch of shared data: the version, the headers and the extensions. Both the Request and the Response also allow callers to separate the headers part of a message from the body, resulting in two Parts types that both carry all three of these fields, so there are 4 types in total that share the same underlying data. It would be nice if there was a way to abstract over this data.

For my purposes, I mostly care about the HeaderMap<HeaderValue> -- my direct use case is that I have a function that makes it easier to extract the header's value as a &str and would like to apply this easily to both a request::Parts (which our framework uses internally to carry around request state) and a Response -- but it might make sense to take care of all three in one interface.

We could just have four impl AsRef<HeaderMap<HeaderValue>> for all four of these types as a minimal solution, or we could implement a custom trait, maybe like this:

trait HttpMessage {
     fn version(&self) -> Version;
     fn headers(&self) -> &HeaderMap<HeaderValue>;
     fn extensions(&self) -> &Extensions;
}
@neoeinstein
Copy link

If we opt to implement AsRef on headers and extensions, could we also implement AsMuttoo? That would allow abstracting over any structure where you could modify those particular attributes. I can see some value in having a singular HttpMessage, as then you can use method names rather than relying on implied or explicit types on as_ref/as_mut. Though in such a case, I would also want headers_mut(&mut self) and extensions_mut(&mut self) to be present as well. Doing both is also an option.

@seanmonstar
Copy link
Member

It'd probably be helpful to know if there's much other prior art of impl AsRef<_> and such for this kind of thing. As well as the contrary, to make a complete decision: what downsides would there be in doing so?

I think a larger trait gets complicated quickly, and people could rightly ask why they return concrete types instead of associated types, which doing it with associated types gets gnarly in its own way.

@djc
Copy link
Author

djc commented Mar 8, 2023

What kind of prior art are you looking for? For one thing, the http-types crate (which the async-std community started at some point, I think?) has AsRef<Headers> implementations for a bunch of types.

I can't think of many downsides. I guess it forces all implementers to share the same types internally, which they do already and this seems pretty fundamental to their operation.

I think a larger trait gets complicated quickly, and people could rightly ask why they return concrete types instead of associated types, which doing it with associated types gets gnarly in its own way.

I don't follow how associated types would be useful here -- also don't see much complexity in the larger trait in general, the definition I presented above seems pretty minimal and robust to changes. But I'd be happy with the most minimal take that only implements AsRef<HeaderMap<HeaderValue>> across types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants