serde icon indicating copy to clipboard operation
serde copied to clipboard

Document that flatten + deny_unknown_fields works at least in simple cases

Open safinaskar opened this issue 2 years ago • 4 comments
trafficstars

Hi. I did some experiments and I see that flatten + deny_unknown_fields combination works in some simple cases, such as this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=770a45f208ac8cbef83e968a87ca4f31 , despite docs say that this doesn't work: https://serde.rs/field-attrs.html#flatten .

On the other hand complex cases, such as this: https://github.com/serde-rs/serde/issues/2283 seem to not be supported.

So, please, document cases when this combination is supported, I want to rely on this feature in my code

safinaskar avatar Feb 25 '23 19:02 safinaskar

This note is from https://github.com/serde-rs/serde-rs.github.io/pull/120 -- @jonasbb could you take a look?

dtolnay avatar Mar 06 '23 16:03 dtolnay

Yes it can work in some situations. In the linked issue you see the reasons why I added the note in the first place. Combining the attributes is fragile. If you add this to Bar it breaks.

#[serde(flatten)]
v: serde_json::Value

If you add another layer around Foo, with flatten and deny_unknown_fieldsbit breaks.

What I cannot answer is if this is sufficiently supported to call it out in the documentation.

jonasbb avatar Mar 06 '23 17:03 jonasbb

I believe flatten + deny_unknown_fields is a footgun and probably should be warned against.

JohnDowson avatar Dec 12 '23 10:12 JohnDowson

It'd be nice if the deny_unknown_fields and flatten could be combined. I have a use case that I'm having trouble implementing and working around, AFAIK, precisely b/c these don't work together. Perhaps there's a better way to achieve the same thing and I'm just not aware of it? If so, please let me know what you'd suggest.

My use-case boils down to this:

  1. I have an endpoint (using rocket) that takes an optional "color": <value> JSON entry that's part of a larger structure;
  2. The <value> can be a name (str) or RGBW values ([f32; 4]). (An example entry within the larger structure can be "color": "red" or "color": [1, 0, 0, 0]; client's choice);

I'm using an enum for the name, a struct with a transparent field for the RGBW values, and another enum with associated data (this one gives clients the ability to choose either one; if there's a better way, I'm open to suggestions):

The following excerpts should help clarify. Some stuff has been omitted for brevity:

#[derive(..., Serialize, Deserialize)]
#[serde(rename_all = "lowercase" /* ,  deny_unknown_fields */ )]  // <-- see other comments
pub enum LedColorNameSchema {
    Red,
    Green,
    Blue,
    White,
    // setting `custom` is not allowed; `deny_unknown_fields` would help
    // cause an error instead of silently ignoring client inputs here,
    // but that conflicts with `flatten`
    #[serde(skip_deserializing)]
    Custom,
}

#[derive(..., Serialize, Deserialize)]
#[serde(transparent)]
pub struct LedColorRgbwSchema {
    rgbw: [f32; 4],
}

// This one allows clients to choose how to set the color
#[derive(..., Serialize, Deserialize)]
#[serde(rename_all = "lowercase", untagged)]
pub enum LedColorParamSchema {
    Name(LedColorNameSchema),
    Rgbw(LedColorRgbwSchema),
}

// This struct exists as a workaround for `enum`s not being allowed to be `Validate`;
// I still need to validate value ranges within the `[f32; 4]` RGBW array, though..
#[derive(..., Deserialize, Validate)]
pub struct LedColorUpdateSchema {
    #[validate(custom = "schemas::validate_color")]  // impl omitted
    pub color: LedColorParamSchema,
}

I can't use deny_unknown_fields b/c some of these types are flattened inside other "larger" types:

#[derive(..., Deserialize, Validate)]
#[serde(remote = "Self")]
pub struct StatusLedUpdateSchema {
    // ... other fields that also use `flatten` omitted
    //
    #[serde(flatten)]  // want this, but conflicts with `deny_unknown_fields`
    #[validate("...")]
    pub maybe_color: Option<LedColorUpdateSchema>,
    // ...
}

impl<'de> Deserialize<'de> for StatusLedUpdateSchema { /* ... */ }

So, I end up with mutually exclusive choices:

  1. use flatten and drop deny_unknown_fields, which preserves the public JSON API structure clients see, but at the expense of lying to clients[^1] with silent failures that look like "successes"; or
  2. use deny_unknown_fields and drop flatten, which preserves my ability to report errors to clients when they send invalid name values, but at the expense of screwing up the public JSON API structures due to nesting.

All of this just to try and explain my use-case and that, due to the incompatibility of these two attributes, it doesn't seem like I can get the behavior I need without screwing myself over somehow(?)

That said, and supposing it's impossible to make them compatible, what's a good approach to implement this kind of use-case? I'm assuming that I may be missing something that looks more obvious to other people, so maybe someone can point it out.

Thanks in advance.

[^1]: The lie is that the client thinks everything was set as intended in its PUT request, b/c it gets an HTTP-204, but the fact is the server-side completely (and silently) ignored the erroneous attempt to set the color. It shows up as None and the Deserialize impl doesn't even seem to get called (absent log messages).

ghost-in-the-zsh avatar Jun 11 '24 11:06 ghost-in-the-zsh