tamasfe / aide

An API documentation library
Apache License 2.0
393 stars 62 forks source link

Feat extra extractors #102

Closed y-haidar closed 8 months ago

y-haidar commented 8 months ago

This is not done, but would like to request feedback on this approach.

Also, I got a question; the axum::Json::from_request already does deserialize so the code below would never return an Err(in theory?).

https://github.com/tamasfe/aide/blob/be3b6d77c21d94e67d45428f534db75c6be3bd98/crates/axum-jsonschema/src/lib.rs#L96-L99

Maybe do a trace::error and do something else? Not sure what, but it feel like the error shouldn't be exposed to crate users.

y-haidar commented 8 months ago

hmmm do I need master branch? Sorry, didn't notice the workflow; I am new to this. I'll make a new PR when ready, but I will keep this up to get your feedback.

tamasfe commented 8 months ago

hmmm do I need master branch? Sorry, didn't notice the workflow; I am new to this. I'll make a new PR when ready

No need, this looks good to me, thank you.

I currently have very tight deadlines at work, I'll do a review on the weekened, but please feel free to ping me in case I forget.

y-haidar commented 8 months ago

Sorry I know this is long, you can read this later.

... I'm not sure how common it is to have JSON there, but the error format definitely beats serde's.

I miss understood your comment. Indeed, while testing my work, I was getting schema validation error ... ,"error":"\"true\" is not of type \"boolean\"" ....

The problem is:

// Incorrect, Value is: `String("024fe6c1-a33e-45a5-8157-2e48f5ff6da9")`
let value: Value = axum::extract::Path::from_request_parts(parts, state)
   ...
// Correct, but jsonschema doesn't support validating `T` only `serde_json::Value`
let value: T = axum::extract::Path::from_request_parts(parts, state)
  ...

The RawPathParams that I started with, in a75d25891f6f76d1663f8fc0437d95e52c154356, gave me wrong impressions by working with uuid(as it kept the key), but ofc it will not work with a bool value. Quick fix is to require Serialize, serialize T from axum::extract::Path::from_request_parts, then validate, then return the same T. Is this okay? Maybe worth opening an issue on jsonschema-rs?

y-haidar commented 8 months ago

Closing PR. Using validator along with schemars can achieve what I originally wanted. I was hoping jsonschema to do this all by itself to reduce dependencies that do similar things. Below is what I am satisfied with:

const REGEX_STR: &'static str = "[A-Z][a-z]+";

lazy_static::lazy_static! {
  static ref RE_TWO_CHARS: Regex = Regex::new(REGEX_STR).unwrap();
}

#[derive(Deserialize, JsonSchema, Validate)]
struct TestPath {
  #[validate(length(equal = 3))]
  input1: String,
  #[validate(regex = "RE_TWO_CHARS")]
  input2: String,
  input3: bool,
}

Sorry for the time wasted. Also noticed axum-valid, which supports aide.

Wicpar commented 8 months ago

You also have garde which can validate regex from strings and other safer features like explicit opt-out from validation.