Skip to content

fix section schema #75

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix section schema #75

wants to merge 1 commit into from

Conversation

koonpeng
Copy link
Collaborator

Bug fix

Fixed bug

schemars generates schemas with additionalProperties: false for enums.
When the enum is flatten, that additionalProperties: false is inherited by the parent
struct, which leads to schemas that can never be valid.

Example:

{
  "description": "Connect the request to a registered section.\n\n``` # bevy_impulse::Diagram::from_json_str(r#\" { \"version\": \"0.1.0\", \"start\": \"section_op\", \"ops\": { \"section_op\": { \"type\": \"section\", \"builder\": \"my_section_builder\", \"connect\": { \"my_section_output\": { \"builtin\": \"terminate\" } } } } } # \"#)?; # Ok::<_, serde_json::Error>(()) ```\n\nCustom sections can also be created via templates ``` # bevy_impulse::Diagram::from_json_str(r#\" { \"version\": \"0.1.0\", \"templates\": { \"my_template\": { \"inputs\": [\"section_input\"], \"outputs\": [\"section_output\"], \"buffers\": [], \"ops\": { \"section_input\": { \"type\": \"node\", \"builder\": \"my_node\", \"next\": \"section_output\" } } } }, \"start\": \"section_op\", \"ops\": { \"section_op\": { \"type\": \"section\", \"template\": \"my_template\", \"connect\": { \"section_output\": { \"builtin\": \"terminate\" } } } } } # \"#)?; # Ok::<_, serde_json::Error>(()) ```",
  "type": "object",
  "oneOf": [
    {
      "type": "object",
      "required": [
        "builder"
      ],
      "properties": {
        "builder": {
          "type": "string"
        }
      },
      "additionalProperties": false
    },
    {
      "type": "object",
      "required": [
        "template"
      ],
      "properties": {
        "template": {
          "type": "string"
        }
      },
      "additionalProperties": false
    }
  ],
  "required": [
    "type"
  ],
  "properties": {
    "config": {
      "default": null
    },
    "connect": {
      "default": {},
      "type": "object",
      "additionalProperties": {
        "$ref": "#/definitions/NextOperation"
      }
    },
    "type": {
      "type": "string",
      "enum": [
        "section"
      ]
    }
  }
},

Here the section schema needs to have a builder or template with no additional properties.
Which includes other properties like type, config etc, but type is also required which
breaks the schema.

Fix applied

Remove additionalProperties from the flattened field. The new unevaluatedProperties option in new json schema drafts addresses this issue, but schemars uses draft-07 so we can't use it. We could update the schema to draft-2020-12 but it's better not to make potentially breaking change.

If your pull request is a bug fix, delete this line and all the below lines.

If your pull request is a feature implementation, delete this line and all the above lines.

New feature implementation

Implemented feature

Implementation description

Signed-off-by: Teo Koon Peng <[email protected]>
@koonpeng koonpeng requested a review from mxgrey May 16, 2025 09:55
@mxgrey mxgrey added this to PMC Board May 16, 2025
@github-project-automation github-project-automation bot moved this to Inbox in PMC Board May 16, 2025
@mxgrey mxgrey moved this from Inbox to In Review in PMC Board May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

1 participant