tamasfe/aide

[Feature Request] exposing the static `SchemaContext`

y-haidar opened this issue · 4 comments

I am talking about this:

thread_local! {
static CONTEXT: RefCell<SchemaContext> = RefCell::new(SchemaContext::new());
}

Use case

Extending axum-jsonschema crate. For example, the code below will compile the schema validation on every request. Making my own context seems redundant, as the type could already exist in axum-jsonschema's SchemaContext.

pub mod path {
  use crate::errors::AppError;
  use aide::OperationIo;
  use axum::{extract::FromRequestParts, http::request::Parts};
  use jsonschema::output::BasicOutput;
  use schemars::schema_for;
  use serde::de::DeserializeOwned;
  use serde_json::{Map, Value};
  // use axum_macros::FromRequestParts;

  #[derive(OperationIo)]
  // #[from_request(via(axum::extract::Path), rejection(AppError))]
  #[aide(input_with = "axum::extract::Path<T>", json_schema)]
  pub struct Path<T>(pub T);

  #[axum::async_trait]
  impl<T, S> FromRequestParts<S> for Path<T>
  where
    T: DeserializeOwned + schemars::JsonSchema + Send,
    S: Send + Sync,
  {
    type Rejection = AppError;

    async fn from_request_parts(parts: &mut Parts, state: &S) -> Result<Self, Self::Rejection> {
      let raw_parrams = match axum::extract::RawPathParams::from_request_parts(parts, state).await {
        Ok(p) => p,
        Err(e) => return Err(e.into()),
      };

      let value = raw_parrams
        .into_iter()
        .map(|p| (p.0.to_owned(), Value::String(p.1.to_owned())))
        .collect::<Map<_, _>>();
      let value = Value::Object(value);

      let validation_result = {
        let schema =
          match jsonschema::JSONSchema::compile(&serde_json::to_value(schema_for!(T)).unwrap()) {
            Ok(s) => s,
            Err(error) => {
              tracing::error!(
                  %error,
                  type_name = std::any::type_name::<T>(),
                  "invalid JSON schema for type"
              );
              jsonschema::JSONSchema::compile(&Value::Object(Map::default())).unwrap()
            }
          };

        let out = schema.apply(&value).basic();

        match out {
          BasicOutput::Valid(_) => Ok(()),
          BasicOutput::Invalid(v) => Err(v),
        }
      };

      if let Err(errors) = validation_result {
        return Err(axum_jsonschema::JsonSchemaRejection::Schema(errors).into());
      }

      T::deserialize(value)
        .map_err(|e| {
          AppError::new("invalid request")
            .with_details(serde_json::json!({"path_serde": &format!("{e:#?}")}))
        })
        .map(Self)
    }
  }
}

Never mind. To be honest, I didn't know what thread_local does. So, you are already being redundant by having copies for each thread. I am assuming the price of have extra copies beats Mutex. If that is the case then, having even more copies per thread per extractor shouldn't be that big of a deal anyway. Closing

I am assuming the price of have extra copies beats Mutex.

Feel free not to assume, it was only an assumption on my part as well, it was never benchmarked. Servers are typically long-lived so the current approach introduces the tradeoff of having to compile and store the schemas for each thread once in order to avoid any locking mechanisms.

The thread-local context can still be exposed, or since it's just mostly a hashmap, you can easily replicate it for your own purposes. I'm also open to PRs extending validation to path/query parameters, I'm not sure how common it is to have JSON there, but the error format definitely beats serde's.

Just a side note, there are no JSON here. I am simply using jsonschema for validations that was already specified for schemars. Like:

#[derive(Deserialize, JsonSchema)]
struct TestPath {
  #[validate(length(equal = 3))]
  input1: String,
  #[validate(regex(pattern = "[A-Z][a-z]+"))]
  input2: String,
}

The axum::extract::RawPathParams does not use serde_json or even serde. They are using percent_encoding crate, which doesn't use serde. For the axum::extract::Path, however, the axum PathDeserializer is used(which is private), and it does implement serde::de::Deserializer. That being said, implementing Path/Query extractors this way does align with axum's Path/Query extractors usage, and it would, as it should, do the validations specified. I would be happy to work on this. Maybe even include other axum extractors, for completeness sake?

Can you check this? I have added a Path extractor