hyperium / http

Rust HTTP types
Apache License 2.0
1.12k stars 283 forks source link

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

Open djc opened 1 year ago

djc commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.