sensedeep / dynamodb-onetable

DynamoDB access and management for one table designs with NodeJS
https://doc.onetable.io/
MIT License
689 stars 109 forks source link

Improve: Prohibit the removal of nested object required fields. #502

Closed ATaiIsHere closed 6 months ago

ATaiIsHere commented 1 year ago

version: 2.7.1 schema:

{
  version: '0.0.1',
  indexes: {
    primary: { hash: 'pk', sort: 'sk' },
  },
  models: {
    PERSON: {
      pk: { type: String, value: 'PERSON#${id}' },
      sk: { type: String, value: 'PERSON#${id}' },
      id: { type: String, generate: 'ulid', required: true },

      name: {
        type: Object,
        schema: {
          first: { type: String, required: true },
          last: { type: String },
        },
      },
    },
  },
  params: {
    timestamps: true,
    isoDates: true,
  },
}

I can remove name.first field by following command.

let result = await oneTable.getModel('PERSON').update({
    id: '01HAKE2N9KH0CWD5MHN2X9A197',
    name: {
      first: undefined,
    }
  })

https://github.com/sensedeep/dynamodb-onetable/blob/v2.7.1/src/Model.js#L1721C16-L1721C16 I think that should be edited to following condition:

(
  (op == 'put' && properties[field.name] == null) ||
  (this.getPartial() && op == 'update' && properties[field.name] === null) ||
  (!this.getPartial() && op == 'update' && properties[field.name] == null)
)

we need to make different judgments based on the partial parameter.

mobsense commented 9 months ago

Did a quick add, and all the tests fail. So that isn't quite right.

The existing code says you cannot set a property explicitly to null (i.e. you want to remove it) if it is required. This is the case for both partial and non-partial settings.

To remove you should be setting name.first to null. Setting to undefined will remove if partial is false, because you are saying you want to set "name" to {}. This is removing first correctly.