dynamodb-onetable icon indicating copy to clipboard operation
dynamodb-onetable copied to clipboard

Updating one property inside of an object empties the whole object

Open AXSJ opened this issue 3 years ago • 11 comments

Describe the bug

When updating one property in an object the whole object is emptied and replaced with the one property that was given in the update. For example, I have a field in my model called LotAddress which is an object that contains street, city, and state as properties. Imagine these fields already hold values, but the state is incorrect. If I were to update the state to the correct state, the whole LotAddress object would be emptied and hold only the updated state property.

To Reproduce

Steps to reproduce the behavior:

  1. Create a model. Ensure the entity has an attribute that is of type object, and the object attribute has more than two properties
  2. Create an item and ensure the properties of the aforementioned object hold values.
  3. Update only one field in the aforementioned object attribute.

Then you will notice the object will be emptied and only contain the field given in the update.

Cut/Paste

import { Table, Entity } from 'dynamodb-onetable'
import * as DynamoDB from 'aws-sdk/clients/dynamodb'

const client = new DynamoDB.DocumentClient({ region: process.env.REGION! })

const ProductSchema = {
  version: '0.0.1',
  client,
  name: 'ProductSchema',
  models: {
    Product: {
      pk: { type: String, value: '${_type}#${id}' },
      sk: { type: String, value: '${_type}#' },
      id: {
        type: String,
        generate: 'ulid',
        validate: Match.ulid,
      },
      address: {
        type: Object,
        required: true,
        schema: {
          street: { type: String },
          city: { type: String },
          postcode: { type: String },
          state: { type: String },
        },
      },

    },
  } as const,
  params: {
    isoDates: true,
    timestamps: true,
  },
}

const table = new Table({
  client,
  name: process.env.PRODUCT_TABLE!,
  schema: ProductSchema,
  logger: true,
  timestamps: true,
})

type ProductType = Entity<typeof ProductSchema.models.Product>

export const ProductModel = table.getModel<ProductType>('Product')


const { id, product: productUpdate, version } = event.arguments.input

const product = await ProductModel.update(
  {
    id,
    ...productUpdate,
    version: version + 1,
  },
  {
    where: '${version} = @{version}',
    substitutions: {
      version,
    },
    return: 'get',
  }
)

info OneTable "update" "Product" { 
    "trace": { 
        "model": "Product", 
        "cmd": { 
            "ConditionExpression": "(attribute_exists(#_0)) and (attribute_exists(#_1)) and (#_2 = :_0)", 
            "ExpressionAttributeNames": { 
                "#_5": "address", 
                "#_6": "id", 
            }, 
            "ExpressionAttributeValues": { 
                ":_0": 5, 
                ":_1": { 
                    "size": 1.2, 
                    "frontage": 1.3, 
                    "depth": 1.4 
                }, 
                ":_2": { 
                    "price": 444.444 
                }, 
                ":_3": { 
                    "street": "123 New Street", 
                    "city": "New City", 
                    "postcode": "4212", 
                    "state": "NSW" 
                }, 
                ":_4": "", 
                ":_5": "new updated stage", 
                ":_6": "Lot 0.07554870447006756", 
                ":_7": 6, 
                ":_8": "Product#", 
                ":_9": "2022-07-13T03:38:39.490Z" 
            }, 
            "TableName": TABLE, 
            "ReturnValues": "ALL_NEW", 
            "UpdateExpression": "set #_3 = :_1, #_4 = :_2, #_5 = :_3, #_6 = :_4, #_7 = :_5, #_8 = :_6, #_2 = :_7, #_9 = :_8, #_10 = :_8, #_11 = :_9", 
        }, 
        "op": "update", 
        "properties": { 
            "measurements": { 
                "size": 1.2, 
                "frontage": 1.3, 
                "depth": 1.4 
            }, 
            "price": { 
                "price": 444.444 
            }, 
            "address": { 
                "street": "123 New Street", 
                "city": "New City", 
                "postcode": "4212", 
                "state": "NSW" 
            }, 
            "id": "", 
        } 
    } 
} 
 
info OneTable result for "update" "Product" { 
    "cmd": { 
        "ConditionExpression": "(attribute_exists(#_0)) and (attribute_exists(#_1)) and (#_2 = :_0)", 
        "ExpressionAttributeNames": { 
            "#_2": "version", 
            "#_5": "address", 
            "#_6": "id",
        }, 
        "ExpressionAttributeValues": { 
            ":_0": 5, 
            ":_1": { 
                "size": 1.2, 
                "frontage": 1.3, 
                "depth": 1.4 
            }, 
            ":_2": { 
                "price": 444.444 
            }, 
            ":_3": { 
                "street": "123 New Street", 
                "city": "New City", 
                "postcode": "4212", 
                "state": "NSW" 
            }, 
            ":_4": "", 
            ":_5": "new updated stage", 
            ":_6": "Lot 0.07554870447006756", 
            ":_7": 6, 
            ":_8": "", 
            ":_9": "2022-07-13T03:38:39.490Z" 
        }, 
        "TableName": "", 
        "ReturnValues": "ALL_NEW", 
        "UpdateExpression": "set #_3 = :_1, #_4 = :_2, #_5 = :_3, #_6 = :_4, #_7 = :_5, #_8 = :_6, #_2 = :_7, #_9 = :_8, #_10 = :_8, #_11 = :_9", 
    "items": [ 
        { 
            "id": "",
            "address": { 
                "street": "123 New Street", 
                "city": "New City", 
                "postcode": "4212", 
                "state": "NSW" 
            }, 
            "created": "2022-07-13T03:24:11.894Z", 
            "updated": "2022-07-13T03:38:39.490Z" 
        } 
    ], 
    "op": "update", 
    "properties": { 
        "address": { 
            "street": "123 New Street", 
            "city": "New City", 
            "postcode": "4212", 
            "state": "NSW" 
        }, 
    }, 
    "params": { 
        "exists": true, 
        "parse": true, 
        "high": true, 
        "where": "${version} = @{version}", 
        "substitutions": { 
            "version": 5 
        }, 
        "return": "get", 
        "log": true, 
        "checked": true 
    } 
} 

Expected behavior When updating one property inside an object only that property should be updated, all other fields should not be altered or removed.

Environment (please complete the following information):

  • Node 14
  • OneTable 2.3.8
  • Serverless 3.15.2

AXSJ avatar Jul 13 '22 03:07 AXSJ

Unless I'm misreading, I can't see your actual update code statement.

Can you post the actual code with the issue?

mobsense avatar Jul 13 '22 04:07 mobsense

Done. Also, the payload/input can be seen from the screenshot. Thanks @mobsense

AXSJ avatar Jul 13 '22 06:07 AXSJ

Thank you for the detailed report and input. Really appreciated.

Let me discuss. Please read all the following closely and give me your input.

Current behavior is that when you update an object property, it replaces the prior value. What you are observing is the current design and intention. But this is clearly sub-optimal for your use case.

i.e.

Given a schema:

        User: {
            address:    { type: Object, schema: {
                street:  { type: String },
                city:    { type: String },
                mail:    { type: String },
            }},

and call:

User.update({address: {city: 'new-city}})

This will set address to {city: 'new-city'}

Now you could argue that this should only update the city and leave street unchanged. But that is not how it currently works. If we changed this, it would certainly break folks code. There are users and existing code that expects to be able to replace the "address" in this example with a new partial address and NOT preserve prior sub-fields that were not supplied.

The current workaround is to use "set"

User.update({}, {set: {city: 'new-city'}})

However, we are considering changing this behind a schema or Table flag.

If we did change, the question is how would you replace the prior "address" if you did not supply ALL the sub fields? i.e. how would you stop inheriting prior address.mail values if you did not supply mail in the update?

mobsense avatar Jul 15 '22 02:07 mobsense

One more thought. If we were to change this via an option flag, the following might be a direction worth pursuing.

If updating portions of a nested object, you could set fields to null (if Table.nulls is false (default)), to indicate that those properties should be removed if present.

ie.

User.update({address: {street, 'street', mail: null}})

In this alternate world, address.street would be updated. address.mail would be removed and address.city would be preserved.

What are we overlooking?

mobsense avatar Jul 15 '22 04:07 mobsense

I like your flag approach. A flag is a great option especially if you don't want to break code for existing projects. Maybe you can be able to set the flag in the schema, and override that flag in the update method with the same flag. And that flag determines if you want to preserve all nested fields you have not catered for in your update or remove those fields you have to not provded in the update. The default could be the latter. And if the latter is chosen then those errors produced in issue #359 would make sense as in the schema you marked the nested field as required but with the update (that removes unmentioned fields) those fields will be emptied and removed. However, if you chose the former option you are not removing any fields just updating existing ones and therefore your update can take any form as you are only changing the values.

The problem with giving a null is that null is a value in itself.

Also with your workaround I couldn't figure out how to use set for nested fields in an object, the documentation isn't clear and your example above doesn't show how to use set with a nested object.

image

The method we used in the above example still replaced the measurements object.

AXSJ avatar Jul 15 '22 08:07 AXSJ

Use:

set: { 'measurements.size': 10 }

Handling of nulls is determined by the Table params.nulls. This determines whether nulls are written to the database. If set to false, (the default), then setting a value to null means remove it. If set to true, then nulls are treated as values and are written to the database.

mobsense avatar Jul 15 '22 09:07 mobsense

Thanks @mobsense. We got it to work. However, this presents another issue for us, we must now add logic to put any updates for nested fields inside the set object. We have a lot of models so doing this isn't a sustainable solution

AXSJ avatar Jul 25 '22 09:07 AXSJ

Are you saying the proposed flag solution as I outlined would work okay for you?

mobsense avatar Jul 27 '22 22:07 mobsense

Yes please

AXSJ avatar Jul 29 '22 04:07 AXSJ

Stay tuned.

mobsense avatar Jul 29 '22 23:07 mobsense

See: #384

mobsense avatar Aug 09 '22 01:08 mobsense