murar8 / axum_typed_multipart

Type safe multipart/form-data handling for axum.
84 stars 14 forks source link

Make rejection field in BaseMultipart pub #55

Closed gengteng closed 1 year ago

gengteng commented 1 year ago

Hi, I'm working on axum-valid and trying to integrate axum with validify. I need to extract the data from a BaseMultipart initially parsed from the request as the payload. Then validate this payload, construct the actual data using the validated payload, and repackage it into a new BaseMultipart.

I would like to request making the rejection field in the BaseMultipart struct pub. Having it as pub would allow me to do this more easily. Here is a minimal reproduction of what I'm trying to do:

// file `typed_multipart.rs`
#[cfg(feature = "validify")]
impl<T: validify::Validify, R> crate::HasValidify for BaseMultipart<T, R> {
    type Validify = T;
    type PayloadExtractor = BaseMultipart<T::Payload, R>;

    fn from_validified(data: Self::Validify) -> Self {
        BaseMultipart {
            data,
            rejection: Default::default(), // ❌: need rejection to be pub
        }
    }
}

// file `validify.rs`
/// Trait for types that can supply a reference that can be modified and validated using `validify`.
///
/// Extractor types `T` that implement this trait can be used with `Validified`.
///
pub trait HasValidify: Sized {
    /// Inner type that can be modified and validated using `validify`.
    type Validify: Validify;
    ///
    type PayloadExtractor: PayloadExtractor<Payload = <Self::Validify as Validify>::Payload>;
    ///
    fn from_validified(v: Self::Validify) -> Self;
}

#[async_trait]
impl<State, Extractor> FromRequestParts<State> for Validified<Extractor>
where
    State: Send + Sync,
    Extractor: HasValidify,
    Extractor::Validify: Validify,
    Extractor::PayloadExtractor: FromRequestParts<State>,
{
    type Rejection =
        ValidifyRejection<<Extractor::PayloadExtractor as FromRequestParts<State>>::Rejection>;

    async fn from_request_parts(parts: &mut Parts, state: &State) -> Result<Self, Self::Rejection> {
        let payload = Extractor::PayloadExtractor::from_request_parts(parts, state)
            .await
            .map_err(ValidifyRejection::Inner)?;
        Ok(Validified(Extractor::from_validified(
            Extractor::Validify::validify(payload.get_payload())?,
        )))
    }
}
// and impl FromRequest for Validified ...

Please let me know if this change would break any existing interfaces or backwards compatibility. If so, I'm happy to discuss potential solutions. I'm open to your feedback and can amend this request accordingly.

Thanks for your consideration! Let me know if you would like any clarification or have any other questions.

murar8 commented 1 year ago

Hi, at the moment I am a bit busy, I will take a look during the weekend and update you.

murar8 commented 1 year ago

But technically yes, we could either have the field public or provide a new function that populates the error field with the default implementation like you are doing, I want to take a deeper look though to see if we can come up with a better pattern. Can I see how this would be used in a handler? Just to make sure I understand your objective.

gengteng commented 1 year ago

Thank you for your reply. I introduced an extractor called Validified, and here is an example of using Validified combined with BaseMultipart in a handler (using with other extractors like Json, WithRejection would be similar):

async fn handler(Validified(BaseMultipart { data, .. }): Validified<BaseMultipart<Data, Rejection>>) {
    // data has been constructed, modified and validated using validify
}

#[derive(TryFromMultipart, Validify)]
pub struct Data {
    v0: String,
}

Here the Data needs to derive the Validify trait. The validify library will generate an associated type DataPayload for it. My implementation in FromRequest / FromRequestParts needs to extract a BaseMultipart<DataPayload, Rejection> from the request, then take out the DataPayload, call the modification and validation functions provided by validify to construct a Data instance, and then reconstruct a BaseMultipart<Data, Rejection> with it.

Thank you again for your attention to this issue. Please let me know if any additional information is needed.

gengteng commented 1 year ago

I reviewed validify's implementation again and realized Validified cannot directly support axum_typed_multipart currently, since validify requires the payload to implement Deserialize. However, constructing data without relying on the payload is still possible. I greatly appreciate your response to my issue.

And I still believe making the rejection pub, similar to axum-extra's WithRejection, would improve the crate's ergonomics without breaking changes.