JsonDeserializer extractor for zero-copy deserialization

[future-highway]

Motivation

The current JSON extractor (Json) is restricted to types that implement serde::de::DeserializedOwned, which prevents deserializing types that use #[serde(borrow)] to facilitate zero-copy deserialization.

Solution

Introduce an alternate JSON extractor (JsonDeserializer) that takes ownership of the request's body (using the Bytes extractor), and can deserialize to any type that implements serde::Deserialized<'de> on demand.

WARNING

Two things I feel I need to draw extra attention to:

1. serde_json will fail to parse perfectly valid JSON string data into &str if it contains escaped characters (e.g., { "foo": "\"I'm in quotes\"" }) and serde implicitly borrows when the field is of type &str, which makes it a little easy to reject valid JSON. Using Cow<'a, str> and Cow<'a, [u8]> instead, allows serde_json to fall back to String and Vec<u8>. This is documented on the extractor.

2. Serde's docs for deserializer lifetimes "note that <T> where T: Deserialize<'static> is never what you want. Also Deserialize<'de> + 'static is never what you want. Generally writing 'static anywhere near Deserialize is a sign of being on the wrong track." I think my usage is okay.

[jplatte]

I like this idea, but I think the new extractor should live in axum-extra.

[davidpdrsn]

I like this idea, but I think the new extractor should live in axum-extra.

I agree. I also think it should be called something else than JsonDeserializer but not really sure what 🤔

[future-highway]

Having it in axum-extra does seem to align better with the project organization; I will move it. There will probably be some code duplication as that seems like a better alternative to making things pub.

I didn't really like the name either, so a better one would be great. I considered JsonParser, JsonReader, and a couple others as well, but JsonDeserializer seemed the most accurate.

[future-highway]

Thought I could move it with just duplicating a small amount to code, but ended up seeming to need duplicates of almost everything the Json extractor used, so I decided to go a different route...

I introduced an optional dependency on the visibility crate to toggle the code I needed to pub when using the __private feature flag. Hopefully that is acceptable.

[davidpdrsn]

I don’t think that’s acceptable. I’d rather have duplication than a bigger public api with lots of hidden things.

[future-highway]

Sorry about the noise; I can't get the axum-extra tests to run locally.

[jplatte]

I can't get the axum-extra tests to run locally.

What command are you running? I'd do cargo test -p axum-extra --all-features.

[future-highway]

Hmm, that's what I was running too, but I get a few errors. Running the tests for the entire workspace worked though.

axum % cargo test -p axum-extra --all-features
Compiling axum-extra v0.9.0 (/OpenSource/axum/axum-extra)
error[E0432]: unresolved import `axum_macros::__private_axum_test`
--> axum-extra/src/lib.rs:115:5
|
115 | use axum_macros::__private_axum_test as test;
|     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `__private_axum_test` in the root
|
note: found an item that was configured out
--> /OpenSource/axum/axum-macros/src/lib.rs:587:8
|
587 | pub fn __private_axum_test(_attr: TokenStream, input: TokenStream) -> TokenStream {
    |        ^^^^^^^^^^^^^^^^^^^
    = note: the item is gated behind the `__private` feature

    error: cannot determine resolution for the attribute macro `crate::test`
    --> axum-extra/src/extract/optional_path.rs:71:7
        |
        71 |     #[crate::test]
        |       ^^^^^^^^^^^
        |
        = note: import resolution is stuck, try simplifying macro imports

    error[E0433]: failed to resolve: could not find `debug_handler` in `axum`
    --> axum-extra/src/protobuf.rs:271:17
        |
        271 |         #[axum::debug_handler]
        |                 ^^^^^^^^^^^^^ could not find `debug_handler` in `axum`
    |
    note: found an item that was configured out
        --> /OpenSource/axum/axum/src/lib.rs:467:22
        |
        467 | pub use axum_macros::debug_handler;
    |                      ^^^^^^^^^^^^^
    = note: the item is gated behind the `macros` feature

    Some errors have detailed explanations: E0432, E0433.
        For more information about an error, try `rustc --explain E0432`.
    error: could not compile `axum-extra` (lib test) due to 3 previous errors

[davidpdrsn]

I think we can merge this as is. We can always rename things later 😊

Do you wanna add a note to axum-extra's changelog?

[yanns]

This link does not seem to work. See https://docs.rs/axum-extra/latest/axum_extra/extract/struct.JsonDeserializer.html

[davidpdrsn]

I think we just forgot a reference for it like https://docs.rs/axum-extra/latest/src/axum_extra/extract/json_deserializer.rs.html#39. Do you wanna submit a PR perhaps?

[yanns]

Yes I can do it.
I'm wondering why it is working for serde::de::DeserializeOwned. I'm not sure I understand how all of this work.

[davidpdrsn]

Because that symbol has been imported.

[yanns]

Because that symbol has been imported.

I only see use serde::Deserialize;. I don't see that serde::de::DeserializeOwned is imported.

[davidpdrsn]

Huh yeah thats true. No idea 😅

[jplatte]

I think it might be the <'de> that's confusing rustdoc. How about:

Suggested change

	/// Deserialize request bodies into some type that implements [`serde::Deserialize<'de>`].
	/// Deserialize request bodies into some type that implements [`serde::Deserialize<'de>`][serde::Deserialize].

[yanns]

I confirm this is working.
#2498

[yanns]

Maybe [serde::de::Deserialize<'de>] would work.