GREsau / okapi

OpenAPI (AKA Swagger) document generation for Rust projects
MIT License
631 stars 112 forks source link

Enforce strict key values for BTreeMap with enum key #128

Open MicaiahReid opened 1 year ago

MicaiahReid commented 1 year ago

Whenever a schema is generated for a BTreeMap with an enum value as the key, the result allows any string for the key. For example:

#[derive(serde::Serialize, JsonSchema, Debug, PartialEq, Eq, Clone, PartialOrd, Ord)]
enum Thing {
    Option1,
    Option2,
}
#[derive(serde::Serialize, JsonSchema)]
struct Response {
    reply: BTreeMap<Thing, String>,
}

generates:

 "components": {
    "schemas": {
      "Response": {
        "type": "object",
        "required": [
          "reply"
        ],
        "properties": {
          "reply": {
            "type": "object",
            "additionalProperties": {
              "type": "string"
            }
          }
        }
      }
    }
  }

This allows the reply object to have any key, when it should really be limited to the enum values. Something like:

 "components": {
    "schemas": {
      "Response": {
        "type": "object",
        "required": [
          "reply"
        ],
        "properties": {
          "reply": {
            "oneOf": [
              {
                "type": "object",
                "required": [
                  "Option1"
                ],
                "properties": {
                  "Option1": {
                    "type": "string"
                  }
                }
              },
              {
                "type": "object",
                "required": [
                  "Option2"
                ],
                "properties": {
                  "Option2": {
                    "type": "string"
                  }
                },
                "maxProperties": 1
              }
            ]
          }
        }
      }
    }
  }

I don't know if there's any other way to limit the possible key values for an object, other than using the oneOf property, but being able to limit potential keys of a spec using a BTreeMap would be really helpful

ralpha commented 1 year ago

Would have to look more into this. But in any case the IndexMap or more specifically the Schemars::Map should be preferred. This is because BTreeMap does not guarantee order or objects and can be different each time it is compiled. (What would make build result less reliable)

I also think this part is handled by Schemars and not by Okapi. As everything under "schemas": is created there. So also considering opening an issue there, or look if there already is an existing issue.

ralpha commented 1 year ago

If you want to have a look at the issue the BTreeMap implementation is here: https://github.com/GREsau/schemars/blob/master/schemars/src/json_schema_impls/maps.rs IndexMap uses the same implementation as HashMap and BTreeMap for sub component generation. (So will have same problem for all, but also 1 fix for all) For the internal Enum you need to look here: https://github.com/GREsau/schemars/blob/master/schemars_derive/src/schema_exprs.rs#L123

MicaiahReid commented 1 year ago

Great, thanks for the info, @ralpha! I'll take a look to see if I can get a PR in for a fix.

MicaiahReid commented 1 year ago

I've started a PR, but I don't think I'll have the time to properly grok this to get it working properly. If anyone wants to pick up where I left off, that would be rad, but I understand there are other priorities.

MicaiahReid commented 1 year ago

@ralpha I've opened a PR for this, I'd appreciate a review! https://github.com/GREsau/schemars/pull/231