Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing OptionalFromRequestParts implementation for the Host extractor from the axum-extra crate #3170

Open
1 task done
dnlsndr opened this issue Jan 12, 2025 · 17 comments · May be fixed by #3177
Open
1 task done

Comments

@dnlsndr
Copy link

dnlsndr commented Jan 12, 2025

  • I have looked for existing issues (including closed) about this

Bug Report

Version

v0.8.1

Platform

UNIX

Crates

axum
axum-extra

Description

Now that the Host extractor was moved to the axum-extra crate and with the new changes to how Option extractors work, it's not possible to do something like this anymore:

use axum_extra::extract::Host;

async fn handler(host: Option<Host>){}

as the compiler complains with this error:

the trait bound `axum_extra::extract::Host: OptionalFromRequestParts<Arc<AppState>>` is not satisfied
you can use `cargo tree` to explore your dependency tree
required for `std::option::Option<axum_extra::extract::Host>` to implement `FromRequestParts<Arc<AppState>>`
@dnlsndr dnlsndr changed the title With the move of the Host extractor to the axum-extra crate, we've lost the OptionalFromRequestParts implementation for it Missing OptionalFromRequestParts implementation for the Host extractor from the axum-extra crate Jan 12, 2025
@taladar
Copy link

taladar commented Jan 14, 2025

The Query extractor is still missing one of those too.

@taladar
Copy link

taladar commented Jan 14, 2025

Specifically I meant the one from axum but axum_extra seems to have one too that is also affected.

@dnlsndr
Copy link
Author

dnlsndr commented Jan 14, 2025

@taladar Check this out: #3079 although their reasoning does not apply to extractors such as Host

@taladar
Copy link

taladar commented Jan 14, 2025

I see, so in case someone comes across this OptionalQuery from axum_extra looks like the correct replacement for the situation where the whole query value is optional, not just individual fields.

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 14, 2025

@taladar OptionalQuery is close to being deprecated too. I would recommend to wrap all your query fields with Option instead, since that best reflects reality of how query parameters work.

@taladar
Copy link

taladar commented Jan 14, 2025

Semantically 99% of the time when I even want several query parameters stored in the same value I want to know if all of them have been specified though, not a mess of several Option values, some of them potentially None that I have to check manually again anyway.

@jplatte
Copy link
Member

jplatte commented Jan 14, 2025

Could you give an example? I have the exact opposite experience, I have never seen a set of query parameters that are optional as a group, rather than individually.

@jplatte
Copy link
Member

jplatte commented Jan 14, 2025

@dnlsndr Want to send a PR for Host?

@taladar
Copy link

taladar commented Jan 14, 2025

Usually that occurs when you have related information that is useless if you only have some of it, e.g. you have a map API with x, y and z parameters or a sort by and sort direction parameter.

@dnlsndr
Copy link
Author

dnlsndr commented Jan 14, 2025

I'd be happy to! PR incoming.

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 14, 2025

you have a map API with x, y and z parameters

let (x, y, z) = match (x, y, z) {
  (Some(x), Some(y), Some(z)) => (x, y, z),
  _ => return Err(...),
}

I think something like this should work?

a sort by and sort direction parameter.

you can use #[serde(default = ...)] in this case without having to use Option inside the struct

@taladar
Copy link

taladar commented Jan 14, 2025

That is the kind of code that I would consider "manually checking". Obviously it is not impossible but there definitely is a use-case for not having that check inside the handler.

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 14, 2025

I haven't tried it, but another alternative could be to use something like this if you always need these params to be present together:

struct QueryParams {
  #[serde(flatten)]
  xyz: Option<XYZ>
}

struct XYZ {
  x: f32,
  y: f32,
  z: f32,
}

@Turbo87
Copy link
Collaborator

Turbo87 commented Jan 14, 2025

yet another alternative: you can use query: Result<Query, QueryRejection> and then let query = query.ok(); to explicitly ignore any deserialization errors. this essentially matches the previous implementation of Option<Query>.

@taladar
Copy link

taladar commented Jan 14, 2025

That last one does sound like an interesting option. In any case it would probably be useful to have something that works similar to Option mentioned in the docs somewhere.

@dnlsndr
Copy link
Author

dnlsndr commented Jan 15, 2025

@taladar I just stumbled across this in the source code:

Screenshot 2025-01-15 at 07 56 55

This seems to be doing what you want, dunno why it's separated out into a different extractor though. It's in the axum-extrac crate under the query.rs.

@taladar
Copy link

taladar commented Jan 15, 2025

Yes, that is the one I found too and that @Turbo87 said would soon be deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants