Implement the `Header` trait

yoshuawuyts commented 3 years ago

Closes #286

This PR implements the Header trait, creating a unified interface for the convention we had of TypedHeader::{value, name, apply}. This instead enables an interface of:

pub trait Header {
    fn header_name(&self) -> HeaderName;
    fn header_value(&self) -> HeaderValue;
    fn apply_header<H: AsMut<Headers>>(&self, mut headers: H);
}

Because header_name and header_value have to be implemented, apply_header has a default implementation. This should make it easier to keep up to date.

Breaking changes

This patch doesn't break anything, but it does set us up for the 3.0.0 release. When we're ready to implement the 3.0.0 changes we should remove the name, value and apply methods in favor of keeping the trait only. This patch also explicitly chose to keep the value method implementation where it is in order to not create a massive merge conflict with #313.

Bug fixes

This patch also fixes two bugs: a wrong header name in TE and in TransferEncoding. These have been adjusted and now use the right header name.

Prior work

This patch serves as an alternative to https://github.com/http-rs/http-types/pull/285, implementing the design I referenced in https://github.com/http-rs/http-types/pull/285#issuecomment-750891612.

Future directions

We should consider taking a look at the headers submodule in general. A few changes I think would be worthwhile:

Move the header name constants from the crate root to be associated constants on HeaderName instead.
Rename Headers to HeaderMap, matching the hyperium/hyper naming convention
Consider changing the return type of ToHeaderValues from an associated iterator to be an instance of HeaderValues instead. The trait precedes the concrete type, and having a trait and a type with similar names be completely unrelated feels awkward.
Do a pass on the typed headers to ensure the right AsRef and AsMut implementations are provided.
Do a pass on the typed headers to clean up the examples, making them practical.
Do a pass on the typed headers to link each header to their respective MDN documentation.
Implement per-method examples on the typed headers.
Create http_types::headers crate-level docs with an explainer of the different types in the crate.

Fishrock123 commented 3 years ago

Can (&str, &str) implement this Header trait?

I ask because I think it's still the best way to solve the Surf .header(header) issue.

yoshuawuyts commented 3 years ago

@Fishrock123 good question; will have to try. I suspect there may actually be limits on what we can do because of how we're using generics.

yoshuawuyts commented 3 years ago

Oh woah, the impl actually worked! Added a test for it too:

#[cfg(test)]
mod test {
    use super::*;

    #[test]
    fn header_from_strings() {
        let strings = ("Content-Length", "12");
        assert_eq!(strings.header_name(), "Content-Length");
        assert_eq!(strings.header_value(), "12");
    }
}

yoshuawuyts commented 3 years ago

It's been a while. Assuming this is good -- going to merge it.

Fishrock123 commented 3 years ago

Sorry. I think this should probably be able to deal with borrows if possible, see #336/#335

http-rs / http-types