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

Empty array as default for "set" type shouldn't pass schema validation #427

Open
raryanpur opened this issue Sep 12, 2024 · 2 comments
Open

Comments

@raryanpur
Copy link

raryanpur commented Sep 12, 2024

Describe the bug
An empty array as the default value for the "set" type shouldn't pass schema validation, since DynamoDB doesn't allow empty sets ("Sets" section on this page: https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html).

Having a little trouble testing but I think the DocumentClient will throw an error in this case. It's possible to pass options.convertEmptyValues=true to the DocumentClient, but since that's not the default could be a bit confusing if the ElectroDB schema passes validation but the DocumentClient throws an error.

ElectroDB Version
2.14.3

ElectroDB Playground Link
(if possible) Use the ElectroDB Playground to recreate your issue and supply the link here to help with troubleshooting.

Entity/Service Definitions
Include your entity model (or a model that sufficiently recreates your issue) to help troubleshoot.

{
    model: {
        entity: "my_entity",
        service: "my_service",
        version: "my_version"
    },
    attributes: {
        prop1: {
            type: "string"
        },
        prop2: {
            type: "string"
        },
        tags: {
            type: "set",
            items: "string",
            default: []
      },
    },
    indexes: {
        record: {
            pk: {
                field: "pk",
                composite: ["prop1"],
            },
            sk: {
                field: "sk",
                composite: ["prop2"],
            },
        }
    }
}

Expected behavior

Schema shouldn't pass validation if the default value of a set is [].

@raryanpur raryanpur changed the title Empty array as default for "set" type shouldn't pass schema valiation Empty array as default for "set" type shouldn't pass schema validation Sep 12, 2024
@tywalch
Copy link
Owner

tywalch commented Sep 12, 2024

Hi @raryanpur 👋

Thanks for posting! The philosophy of validation in the library has generally been to focus on verifying Electro specific conventions and then, for everything else, get out of the user's the way. The main reason for this is because limits, defaults, and validations in DynamoDB are all subject to change. To date, this boundary has been very helpful in maintaining the library's stability.

@raryanpur
Copy link
Author

raryanpur commented Sep 12, 2024

@tywalch thanks for the reply, and that definitely makes sense. Agree that's the right approach.

I think it's helpful for the schema validation in Electro to generally track the modeling boundaries of Dynamo, since the library is tightly coupled to Dynamo. Specific Dynamo limitations on values of data types (i.e. in this case, not allowing empty sets) feels close enough to schema modeling that it could be considered an Electro convention as well (and enforced by Electro), but there's definitely some grey area.

To your point of not including this as an Electro validation - since the user can set a flag on the DocumentClient to "handle" empty sets, not having Electro validate provides for more flexibility. I'm not sure if there would be any special considerations for setting this flag on the DocumentClient, but at least the user has the optionality as things are now.

Could see it making sense either way.

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

No branches or pull requests

2 participants