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

Allow interger key in flattened Map #1545

Closed
killercup opened this issue Jun 1, 2019 · 4 comments
Closed

Allow interger key in flattened Map #1545

killercup opened this issue Jun 1, 2019 · 4 comments

Comments

@killercup
Copy link
Contributor

I'm running into a weird problem right now, and I don't think I understand enough of serde to solve this by myself.

I'm trying to patch the openapiv3 crate so that its Responses type is a bit more consistent in what it expects. In the currently latest version 0.2.0 this type has two fields, default (Option<_Unimportant>), and responses (a BTreeMap<String, _Unimportant>). The latter is tagged with #[serde(flatten)] (and #[serde(default)]).

This responses type I have changed so it uses a struct StatusCode { … } for its keys. It has a custom Deserialize implementation that validates its format. Now, I would like to make it a bit more lenient in which types it expects.

The backstory is this: Writing OpenAPI definitions in YAML can easily become a verbose affair with, so authors like to specify the valid return types as concise as possible:

# ...
paths:
  /:
    get:
      description: Foobar
      responses:
        200:
          description: All is well

See that 200? That's an integer. It could also be a "200", and that is what the openapiv3 crate currently requires. I want to make this work with integers, too.

How can I do that?


Here's what I've discovered so far.

The StatusCodeVisitor I implemented (current state in glademiller/openapiv3#6) has a visit_i64 method, but it seems that it is never called in this case. The error I get is this (using the verbose-debug feature from #1544):

paths: data did not match any variant of untagged enum `ReferenceOr`
        - attempted to deserialize `Reference` but failed with: missing field `$ref`
        - attempted to deserialize `Item` but failed with: invalid type: integer `200`, expected field identifier at line 2 column 4

Looking at the serde_derive code I see that "field identifier" comes from deserialize_identifier and is the error text set for the expecting method this code generates.

I assume this code is included when deserializing fields? Does it get called because we are in a flatten context? That is where my knowledge ends. Any help is greatly appreciated!

@dtolnay
Copy link
Member

dtolnay commented Jun 22, 2019

I don't immediately know why this would be happening but I can take a look. It would help to have a minimal runnable example that reproduces the "invalid type: integer `200`, expected field identifier" error message.

@killercup
Copy link
Contributor Author

I've spent some time trying to make a minimal runnable example. git clone https://gist.github.com/db9a5d227fddbdd69b29947b0ff29bc5.git serde-issue-1545 && cd serde-issue-1545 && cargo test will show you an error similar to the one I quoted above.

The gist contains three test files, all of them trying to deserialize the same inputs. Two have a simplified Responses struct as a field of a root struct, once with String as map keys, once with the custom StatusCode. They both pass.

The failing test uses an #[serde(untagged)] enum ReferenceOr<T> as a wrapper around the nested responses field -- while still using String keys. So it seems I misread the error message: It's not (just) about flatten, but about untagged.

@dtolnay
Copy link
Member

dtolnay commented Jun 23, 2019

The interaction with untagged is a consequence of #1183. The implicit equivalence between integers and strings is a yaml-specific behavior so we don't get that when deserializing from the untagged enum's internal buffer data format.

I was able to get your tests in the gist working with this patch:

diff --git a/Cargo.toml b/Cargo.toml
@@ -21,6 +21,6 @@ name = "test_reference_enum"
 path = "test_reference_enum.rs"
 
 [dependencies]
-serde = { version = "1.0.92", features = ["derive"] }
+serde = { version = "1.0.93", features = ["derive"] }
 serde_yaml = "0.8.9"
 unindent = "0.1.3"
diff --git a/status_code.rs b/status_code.rs
@@ -34,6 +34,17 @@ impl<'de> Deserialize<'de> for StatusCode {
                 Ok(StatusCode::Code(value as u16))
             }
 
+            fn visit_u64<E>(self, value: u64) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                if value >= 100 && value < 1000 {
+                    Ok(StatusCode::Code(value as u16))
+                } else {
+                    Err(E::invalid_value(Unexpected::Unsigned(value), &self))
+                }
+            }
+
             fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
             where
                 E: de::Error,
@@ -64,6 +75,6 @@ impl<'de> Deserialize<'de> for StatusCode {
             }
         }
 
-        deserializer.deserialize_str(StatusCodeVisitor)
+        deserializer.deserialize_any(StatusCodeVisitor)
     }
 }
diff --git a/test_reference_enum.rs b/test_reference_enum.rs
@@ -1,5 +1,7 @@
 #![allow(unused)]
 
+mod status_code;
+use status_code::StatusCode;
 use std::collections::BTreeMap;
 
 #[derive(serde::Deserialize, Clone, Debug, PartialEq)]
@@ -22,7 +24,7 @@ struct Responses {
     default: Option<ReferenceOr<Unimportant>>,
     #[serde(flatten)]
     #[serde(default)]
-    values: BTreeMap<String, Unimportant>,
+    values: BTreeMap<StatusCode, Unimportant>,
 }
 
 #[derive(Debug, serde::Deserialize)]

@killercup
Copy link
Contributor Author

Thanks a lot for the quick update, David! This works perfectly :)

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

No branches or pull requests

2 participants