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

feat: manage map types with additionalProperties #98

Merged
merged 7 commits into from
Jun 28, 2024

Conversation

bdeneux
Copy link
Contributor

@bdeneux bdeneux commented Jun 25, 2024

This Pull Request introduces improvements to the JSON schema generation process, specifically for Map types. These improvements are in line with the JSON schema draft documentation and the output from the schemars tool.

When a Map is defined, it generates a JSON schema definition where additionalProperties is set to accept the type required by the map value type.

Consider the following Rust code:

#[cw_serde]
#[derive(QueryResponses)]
pub enum QueryMsg {
    /// # MapString
    #[returns(BarResponse)]
    MapString { foo: BTreeMap<String, String> },
    #[returns(BarResponse)]
    MapWithValue { foo: BTreeMap<String, Value> },
}

This code generates the following JSON schemas:

"map_string": {
    "type": "object",
    "required": ["foo"],
    "properties": {
        "foo": {
            "type": "object",
            "additionalProperties": {
                "type": "string"
            }
        }
    },
    "additionalProperties": false
}
"map_with_value": {
    "type": "object",
    "required": ["foo"],
    "properties": {
        "foo": {
            "type": "object",
            "additionalProperties": {
                "$ref": "#/definitions/Value"
            }
        }
    },
    "additionalProperties": false
}

In Go, this should generate a foo property of type map[string]string for the first schema and map[string]Value for the second one.

type QueryMsg_MapString struct {
	Foo map[string]string `json:"foo"`
}
type QueryMsg_MapWithValue struct {
	Foo map[string]Value `json:"foo"`
}

This PR includes modifications to the handling of the additionalProperties field, allowing it to accept either a boolean or a JSONSchema, in line with the JSON schema specification.

Furthermore, when additionalProperties is set to true (which is the default value in JSON Schema), it implies that the object can accept any property with any value. While this does not adhere strictly to the JSON schema specification (because we can set both properties and addionalProperties), it provides a more flexible approach to handling additionalProperties.

@bdeneux
Copy link
Contributor Author

bdeneux commented Jun 26, 2024

The schema in question can be found at: https://github.com/axone-protocol/axone-contract-schema/blob/refactor/go-codegen/schema/axone-cognitarium.json.

Please note that this schema has not been added to the test data due to the presence of another issue that is unrelated to additionalProperties but affects the generation process. A separate issue will be created to track and resolve this secondary problem. 🙂

Copy link
Owner

@srdtrk srdtrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the contribution once again. Looks pretty good, I've left some comments, let's address them and get this merged.

integration_test/integration_test.go Outdated Show resolved Hide resolved
pkg/codegen/properties.go Show resolved Hide resolved
Copy link
Owner

@srdtrk srdtrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, left one suggestion, will merge once that's resolved.

pkg/codegen/properties.go Show resolved Hide resolved
@srdtrk srdtrk merged commit ed67358 into srdtrk:main Jun 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants