axum 0.8.0-rc.1: optional query parameters

[yanns]

I'm trying out https://github.com/tokio-rs/axum/releases/tag/axum-v0.8.0-rc.1 on some internal services, and I have an issue with the change about optional extractors: #2475

My code:

#[derive(Deserialize, Debug)]
struct ConsistencyParams {
    #[serde(deserialize_with = "deserialize_untagged_enum_case_insensitive")]
    consistency: Consistency,
}

#[debug_handler]
async fn expand(
   ...
    consistency: Option<Query<ConsistencyParams>>,
   ...
) -> Response {

When I call the HTTP endpoint without the consistency query parameter, the request is rejected with:

2024-12-18T08:43:16.504317Z TRACE http-request{correlation_id="reference-535743ab-319d-46ae-8241-d4b49d370ee5"}: axum::rejection: rejecting request status=400 body="Failed to deserialize query string: missing field `consistency`" rejection_type="axum::extract::rejection::FailedToDeserializeQueryString"
2024-12-18T08:43:16.504393Z  INFO http-request{correlation_id="reference-535743ab-319d-46ae-8241-d4b49d370ee5"}: reference_service::telemetry::logging::access_logs: POST /{project_key}/reference-expansion 400 http.method="POST" http.status_code=400 http.received_bytes=96 http.sent_bytes=63 ctp.api_endpoint="POST /{project_key}/reference-expansion" duration=0.00019 type="reference-http-access"
thread 'component_tests::eventual_consistency::expanded_obj_are_strong_consistent_on_user_session_by_default' panicked at tests/reference/component_tests/component_tests_helpers.rs:329:9:
assertion `left == right` failed: body: Ok("Failed to deserialize query string: missing field `consistency`")
  left: 400
 right: 200

The URL is looking like this:

/project-a/reference-expansion?expand=mainProduct&expand=order

When I look at the description of #2475, I can read:

/// # `Option<Query<T>>` behavior
///
/// If `Query<T>` itself is used as an extractor and there is no query string in
/// the request URL, `T`'s `Deserialize` implementation is called on an empty
/// string instead.
///
/// You can avoid this by using `Option<Query<T>>`, which gives you `None` in
/// the case that there is no query string in the request URL.
///
/// Note that an empty query string is not the same as no query string, that is
/// `https://example.org/` and `https://example.org/?` are not treated the same
/// in this case.

I can observe the last behavior described:

• a request to /project-a/reference-expansion is accepted
• a request to /project-a/reference-expansion? is not accepted

I'm not sure about this new behavior.
A handler can define several query parameters, and maybe all of them are optional.
And it needs to be able to accept a query with:

• none of them set (already possible)
• all of them set (already possible)
• some of them set (not possible)

[jplatte]

Right, I could see us simply remove that OptionalFromRequestParts impl, it's likely more confusing than useful.

That said, what were you doing before?

[yanns]

The code I've put is the one running with axum 0.7.

[Turbo87]

shouldn't it be consistency: Option<Consistency> in the struct if the field is optional?

[yanns]

I'll try.

[yanns]

shouldn't it be consistency: Option<Consistency> in the struct if the field is optional?

I've tried that, and I observe the same behavior.

[Turbo87]

can you share a reproduction? I would expect this to work 😅

[yanns]

can you share a reproduction? I would expect this to work 😅

It's an internal application.
I can take time to create a new application, but not in the very near future.

[yanns]

oh wait, I'm using a custom deserializer. The issue can come from this.

[yanns]

You're right, it's working without custom deserializer.

[yanns]

At the end, I could make it work with:

#[debug_handler]
async fn expand(
   ...
    consistency: Option<Query<ConsistencyParams>>,
   ...
) -> Response {

#[derive(Deserialize, Debug)]
struct ConsistencyParams {
    #[serde(
        default,
        deserialize_with = "deserialize_untagged_enum_case_insensitive"
    )]
    consistency: Option<Consistency>,
}

fn deserialize_untagged_enum_case_insensitive<'de, T, D>(deserializer: D) -> Result<T, D::Error>
where
    T: Deserialize<'de>,
    D: Deserializer<'de>,
{
    use serde::de::Error;
    use serde_json::Value;
    T::deserialize(Value::String(
        String::deserialize(deserializer)?.to_lowercase(),
    ))
    .map_err(Error::custom)
}

[yanns]

I guess I can close this issue now, unless there's something to learn from it and to change code-wise or documentation-wise.

[yanns]

What could maybe be improved: I had to use an Option<Query that itself has consistency: Option<Consistency>.
At then, I have 2 options, that I flatten with .and_then. Not the end of the world, but I'm wondering if we can have something easier to use.

[Turbo87]

why did you need the Option<Query<_>>? I think this should work with just regular Query<_> too if the Option<_> is now inside the ConsistencyParams struct 🤔

[jplatte]

I think what could be improved is removing the OptionalFromRequestParts impl. As I was writing its docs, I was already wondering when that would ever be useful. I think this issue just shows very clearly that it's a footgun (as was the previous behavior of Option<Query<_>>, by the way).

[Turbo87]

yep, I think I agree with that

[yanns]

why did you need the Option<Query<_>>? I think this should work with just regular Query<_> too if the Option<_> is now inside the ConsistencyParams struct 🤔

Yes you're right, I can use Query<_> directly.
When I tried before, it was failing because of the custom deserializer.