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

schema_for! on HashMap and BTreeMap no longer generates field information for values #2309

Open
snoyberg opened this issue Dec 10, 2024 · 3 comments · May be fixed by #2310
Open

schema_for! on HashMap and BTreeMap no longer generates field information for values #2309

snoyberg opened this issue Dec 10, 2024 · 3 comments · May be fixed by #2310
Assignees

Comments

@snoyberg
Copy link
Contributor

Sorry for the confusing title, allow me to demonstrate. I bisected this issue down to the commit:

e470075

You can see the issue with this code:

use cosmwasm_schema::cw_serde;
use std::collections::BTreeMap;

#[cw_serde]
struct HasMap {
    map_field: BTreeMap<String, u32>,
    normal_field: u32,
}

fn main() {
    let schema = cosmwasm_schema::schema_for!(HasMap);
    println!("{schema:#?}");
}

With cosmwasm-schema version 2.0.2, the output I receive is:

RootSchema {
    meta_schema: Some(
        "http://json-schema.org/draft-07/schema#",
    ),
    schema: SchemaObject {
        metadata: Some(
            Metadata {
                id: None,
                title: Some(
                    "HasMap",
                ),
                description: None,
                default: None,
                deprecated: false,
                read_only: false,
                write_only: false,
                examples: [],
            },
        ),
        instance_type: Some(
            Single(
                Object,
            ),
        ),
        format: None,
        enum_values: None,
        const_value: None,
        subschemas: None,
        number: None,
        string: None,
        array: None,
        object: Some(
            ObjectValidation {
                max_properties: None,
                min_properties: None,
                required: {
                    "map_field",
                    "normal_field",
                },
                properties: {
                    "map_field": Object(
                        SchemaObject {
                            metadata: None,
                            instance_type: Some(
                                Single(
                                    Object,
                                ),
                            ),
                            format: None,
                            enum_values: None,
                            const_value: None,
                            subschemas: None,
                            number: None,
                            string: None,
                            array: None,
                            object: Some(
                                ObjectValidation {
                                    max_properties: None,
                                    min_properties: None,
                                    required: {},
                                    properties: {},
                                    pattern_properties: {},
                                    additional_properties: Some(
                                        Object(
                                            SchemaObject {
                                                metadata: None,
                                                instance_type: Some(
                                                    Single(
                                                        Integer,
                                                    ),
                                                ),
                                                format: Some(
                                                    "uint32",
                                                ),
                                                enum_values: None,
                                                const_value: None,
                                                subschemas: None,
                                                number: Some(
                                                    NumberValidation {
                                                        multiple_of: None,
                                                        maximum: None,
                                                        exclusive_maximum: None,
                                                        minimum: Some(
                                                            0.0,
                                                        ),
                                                        exclusive_minimum: None,
                                                    },
                                                ),
                                                string: None,
                                                array: None,
                                                object: None,
                                                reference: None,
                                                extensions: {},
                                            },
                                        ),
                                    ),
                                    property_names: None,
                                },
                            ),
                            reference: None,
                            extensions: {},
                        },
                    ),
                    "normal_field": Object(
                        SchemaObject {
                            metadata: None,
                            instance_type: Some(
                                Single(
                                    Integer,
                                ),
                            ),
                            format: Some(
                                "uint32",
                            ),
                            enum_values: None,
                            const_value: None,
                            subschemas: None,
                            number: Some(
                                NumberValidation {
                                    multiple_of: None,
                                    maximum: None,
                                    exclusive_maximum: None,
                                    minimum: Some(
                                        0.0,
                                    ),
                                    exclusive_minimum: None,
                                },
                            ),
                            string: None,
                            array: None,
                            object: None,
                            reference: None,
                            extensions: {},
                        },
                    ),
                },
                pattern_properties: {},
                additional_properties: Some(
                    Bool(
                        false,
                    ),
                ),
                property_names: None,
            },
        ),
        reference: None,
        extensions: {},
    },
    definitions: {},
}

But with version 2.1.2, instead I get:

RootSchema {
    meta_schema: Some(
        "http://json-schema.org/draft-07/schema#",
    ),
    schema: SchemaObject {
        metadata: Some(
            Metadata {
                id: None,
                title: Some(
                    "HasMap",
                ),
                description: None,
                default: None,
                deprecated: false,
                read_only: false,
                write_only: false,
                examples: [],
            },
        ),
        instance_type: Some(
            Single(
                Object,
            ),
        ),
        format: None,
        enum_values: None,
        const_value: None,
        subschemas: None,
        number: None,
        string: None,
        array: None,
        object: Some(
            ObjectValidation {
                max_properties: None,
                min_properties: None,
                required: {
                    "map_field",
                    "normal_field",
                },
                properties: {
                    "map_field": Object(
                        SchemaObject {
                            metadata: None,
                            instance_type: Some(
                                Single(
                                    Object,
                                ),
                            ),
                            format: None,
                            enum_values: None,
                            const_value: None,
                            subschemas: None,
                            number: None,
                            string: None,
                            array: None,
                            object: Some(
                                ObjectValidation {
                                    max_properties: None,
                                    min_properties: None,
                                    required: {},
                                    properties: {},
                                    pattern_properties: {},
                                    additional_properties: Some(
                                        Bool(
                                            false,
                                        ),
                                    ),
                                    property_names: None,
                                },
                            ),
                            reference: None,
                            extensions: {},
                        },
                    ),
                    "normal_field": Object(
                        SchemaObject {
                            metadata: None,
                            instance_type: Some(
                                Single(
                                    Integer,
                                ),
                            ),
                            format: Some(
                                "uint32",
                            ),
                            enum_values: None,
                            const_value: None,
                            subschemas: None,
                            number: Some(
                                NumberValidation {
                                    multiple_of: None,
                                    maximum: None,
                                    exclusive_maximum: None,
                                    minimum: Some(
                                        0.0,
                                    ),
                                    exclusive_minimum: None,
                                },
                            ),
                            string: None,
                            array: None,
                            object: None,
                            reference: None,
                            extensions: {},
                        },
                    ),
                },
                pattern_properties: {},
                additional_properties: Some(
                    Bool(
                        false,
                    ),
                ),
                property_names: None,
            },
        ),
        reference: None,
        extensions: {},
    },
    definitions: {},
}

Since that's a lot of content, here's the diff between the two:

<                                         Object(
<                                             SchemaObject {
<                                                 metadata: None,
<                                                 instance_type: Some(
<                                                     Single(
<                                                         Integer,
<                                                     ),
<                                                 ),
<                                                 format: Some(
<                                                     "uint32",
<                                                 ),
<                                                 enum_values: None,
<                                                 const_value: None,
<                                                 subschemas: None,
<                                                 number: Some(
<                                                     NumberValidation {
<                                                         multiple_of: None,
<                                                         maximum: None,
<                                                         exclusive_maximum: None,
<                                                         minimum: Some(
<                                                             0.0,
<                                                         ),
<                                                         exclusive_minimum: None,
<                                                     },
<                                                 ),
<                                                 string: None,
<                                                 array: None,
<                                                 object: None,
<                                                 reference: None,
<                                                 extensions: {},
<                                             },
---
>                                         Bool(
>                                             false,

Note that the old version correctly generated schema information indicating that the contents of the field are an Object with uint32s as values. Without this, the generated schemas are missing that information, which ultimately (in our use case) leads to incorrectly generated TypeScript types.

snoyberg added a commit to Levana-Protocol/cosmwasm that referenced this issue Dec 10, 2024
This reverts commit e470075.

See: CosmWasm#2309
@aumetra
Copy link
Member

aumetra commented Dec 12, 2024

That is.. an interesting effect of that visitor. It definitely shouldn't have that kind of effect. The reason for the visitor was to prevent the TypeScript type generator to add a field to capture potential additional properties that weren't covered by the schema without adding #[serde(deny_unknown_fields)] (which had undesired issues for users; see #2019)

This might be a weird interation between map types and the additional_properties field? Not sure. I'll check that..

@aumetra
Copy link
Member

aumetra commented Dec 12, 2024

Actually, I have a potential solution. I'll just need to inspect generated TypeScript types to check for regressions.

The type of the value of the map is encoded in the additionalProperties, so if we simply set it to false if it's unset, it shouldn't complain?

And yeah, this points at some lack of coverage in our tests. After creating such a patch and regenerating all schemas, there is no difference.

@aumetra aumetra linked a pull request Dec 12, 2024 that will close this issue
@snoyberg
Copy link
Contributor Author

I don't know much about the JSON schema standard, so I'm not sure of the right outcome here. I appreciate you looking into it. I'd be happy to test out any changes against our codebase if it would be helpful.

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 a pull request may close this issue.

2 participants