[WIP] Integrate hyperium/http crate

thomaseizinger commented 5 years ago

Depending on the http crate allows for removal of the StatusCode enum and instead use the definition from the http crate.

Unfortunately, the http crate does not (yet) support serde. I've opened a downstream PR for this (https://github.com/hyperium/http/pull/274) and patched the dependency used in this PR in order to keep developing.

The change is basically done, what is left is waiting for the downstream PR to be merged and a new version of http to be released.

Please have a look if you are ok with the changes :)

The changes are unsurprisingly breaking:

Using integers for status codes no longer works because the StatusCode from http doesn't have a From implementation for it. However, I re-exported the StatusCode definition from them, so users should be able to easily transition to using the constants defined in there.
The From implementations to the statuscode implementations of the other libraries are gone. StatusCode is now a foreign crate, so the orphan rule prevents us from providing such an implementation. This is not bad IMO, hopefully more crates will end up using the http crate anyway so this conversion will eventually be obsolete anyway.

Resolves #3.

chridou commented 5 years ago

I very much like the changes since the crate now takes the same road as the ecosystem does regarding HTTP.

I hope that HTTP will serialize to an integer since there is the field status in the HttpApiProblem` which is effectively a status code. If it will not I think we will have to use serdes field attribute "with" to do custom serialization(a lot of work...).

thomaseizinger commented 5 years ago

In the changes I proposed, the statuscode serializes to an integer yes :)

The serde with thing wouldn't be too much work actually but the fact that status is an Option makes it much harder. If we'd change it to be initialized to 500 by default, custom serialization would be much easier.

thomaseizinger commented 5 years ago

Having made all these changes I think some serialization tests would be good :D I am thinking of adding some using serde_test!

Also the integration to the other frameworks should be tested in my opinion :)

chridou commented 5 years ago

Hi!

I somehow lost hope that your PR on http crate regarding serialization for StatusCode will be merged soon.

I added some custom serialization to your code which you can find here: https://github.com/chridou/http-api-problem/tree/add-custom-serialization

What do you think?

I'm thinking of adding a trait like IntoStatusCode to make integration with the web frameworks easier...

thomaseizinger commented 5 years ago

What do you think?

Looks good. It gets us going and if it is ever merged, we can always fall back to a simpler way :)

I'm thinking of adding a trait like IntoStatusCode to make integration with the web frameworks easier...

Not sure if that is worth it. I'd rather push the web frameworks to use the http crate.

thomaseizinger commented 5 years ago

I'll close this then in favor of #6.

chridou / http-api-problem

[WIP] Integrate hyperium/http crate #4