Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

BUG: Using the --jsonschema_opt=enforce_oneof flag makes all "optional" fields in the .proto files required in the jsonschema #161

Open
mansoor2016 opened this issue Jul 17, 2023 · 3 comments
Assignees
Labels
awaiting_feedback Waiting for the reporter to respond

Comments

@mansoor2016
Copy link

mansoor2016 commented Jul 17, 2023

Using the following CL options:

protoc --plugin=${HOME}/go/bin/protoc-gen-jsonschema --jsonschema_opt=file_extension=schema.json --jsonschema_opt=disallow_additional_properties --jsonschema_out=schemas --proto_path=$CI_PROJECT_DIR/protobuf $CI_PROJECT_DIR/protobuf/*.proto --experimental_allow_proto3_optional

converts this proto:

message FusionConfig
{
  oneof fusion_config{
    BasicFusionConfig basic_fusion = 1;
  }

  message BasicFusionConfig
  {
    uint64 detections_to_start_fuse = 2;
    optional double track_termination_delay = 3;
    FusionReducer fusion_reducer = 4;

    enum FusionReducer
    {
      MEAN = 0;
      MEDIAN = 1;
    }
  }
}

into this schema:

{
    "$schema": "http://json-schema.org/draft-04/schema#",
    "$ref": "#/definitions/FusionConfig",
    "definitions": {
        "FusionConfig": {
            "properties": {
                "basic_fusion": {
                    "$ref": "#/definitions/pem.FusionConfig.BasicFusionConfig",
                    "additionalProperties": false
                }
            },
            "additionalProperties": false,
            "type": "object",
            "oneOf": [
                {
                    "required": [
                        "basic_fusion"
                    ]
                }
            ],
            "title": "Fusion Config"
        },
        "pem.FusionConfig.BasicFusionConfig": {
            "properties": {
                "detections_to_start_fuse": {
                    "type": "string",
                    "description": "How many consecutive detections do we need to add sensor detection to fusion."
                },
                "track_termination_delay": {
                    "type": "number",
                    "description": "How long (seconds) until we stop including this track in the fusion. Currently unused. Optional."
                },
                "fusion_reducer": {
                    "enum": [
                        "MEAN",
                        0,
                        "MEDIAN",
                        1
                    ],
                    "oneOf": [
                        {
                            "type": "string"
                        },
                        {
                            "type": "integer"
                        }
                    ],
                    "title": "Fusion Reducer"
                }
            },
            "additionalProperties": false,
            "type": "object",
            "oneOf": [
                {
                    "required": [
                        "track_termination_delay"
                    ]
                }
            ],
            "title": "Basic Fusion Config",
            "description": "Nested types. Frame-wise, system-wide fusion, no temporal correlation, simplest model."
        }
    }
}

without this flag, it produces the following jsonschema:

{
    "$schema": "http://json-schema.org/draft-04/schema#",
    "$ref": "#/definitions/FusionConfig",
    "definitions": {
        "FusionConfig": {
            "properties": {
                "basic_fusion": {
                    "$ref": "#/definitions/pem.FusionConfig.BasicFusionConfig",
                    "additionalProperties": false
                }
            },
            "additionalProperties": false,
            "type": "object",
            "title": "Fusion Config"
        },
        "pem.FusionConfig.BasicFusionConfig": {
            "properties": {
                "detections_to_start_fuse": {
                    "type": "string",
                    "description": "How many consecutive detections do we need to add sensor detection to fusion."
                },
                "track_termination_delay": {
                    "type": "number",
                    "description": "How long (seconds) until we stop including this track in the fusion. Currently unused. Optional."
                },
                "fusion_reducer": {
                    "enum": [
                        "MEAN",
                        0,
                        "MEDIAN",
                        1
                    ],
                    "oneOf": [
                        {
                            "type": "string"
                        },
                        {
                            "type": "integer"
                        }
                    ],
                    "title": "Fusion Reducer"
                }
            },
            "additionalProperties": false,
            "type": "object",
            "title": "Basic Fusion Config",
            "description": "Nested types. Frame-wise, system-wide fusion, no temporal correlation, simplest model."
        }
    }
}
@mansoor2016 mansoor2016 changed the title Using the --jsonschema_opt=enforce_oneof flag makes all "optional" fields in the .proto files required in the jsonschema BUG: Using the --jsonschema_opt=enforce_oneof flag makes all "optional" fields in the .proto files required in the jsonschema Jul 18, 2023
@chrusty
Copy link
Owner

chrusty commented Aug 6, 2023

Hello @mansoor2016. Agreed, that generated schema looks a bit odd. Can I ask though, why are you using a OneOf with only one option? Isn't that just the same as having a single basic_fusion attribute?

@chrusty chrusty self-assigned this Aug 6, 2023
@chrusty chrusty added the awaiting_feedback Waiting for the reporter to respond label Aug 6, 2023
@mansoor2016
Copy link
Author

Hello @mansoor2016. Agreed, that generated schema looks a bit odd. Can I ask though, why are you using a OneOf with only one option? Isn't that just the same as having a single basic_fusion attribute?

future-proofing the schema, you are right that we could have a single attribute for now

@sghosh-discovery
Copy link

We are running into the same issue.
If we have a proto line like optional wbd.protobuf.metadata.common.v4.Identifier edit_id = 4, the generated json schema with enforce_oneff makes it look like this:

"oneOf": [
                {
                    "required": [
                        "edit_id"
                    ]
                }
            ],

This breaks the JSON schema check.

chrusty pushed a commit that referenced this issue Feb 12, 2024
* Fix one of and optional behaviour on proto3

* Update test data and result file
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting_feedback Waiting for the reporter to respond
Projects
None yet
Development

No branches or pull requests

3 participants