Allow more complex ConditionExpressions using UpdateItemCodableInput

[Andrea-Scuderi]

Description

The current implementation of DynamoDB.UpdateItemCodableInput doesn't not allow to implement optimistic locking. In fact, ConditionExpressions in that struct can compare the current value of a single attribute with a known value. However, many applications require more complex condition expressions that involve multiple attributes and operators. For example, consider the following case:

struct Item: Codable {
   let key: String
   let name: String
   let createdAt: String
   let updatedAt: String
}

with the current implementation we can update the DynamoDB table in this way:

let item = Item(
   key: "9D1DC963-411A-486F-BE26-ADF473F00B80",
   name: "MyItem"
   createdAt: "2023-05-08T18:17:34.972Z",
   updatedAt: "2023-05-11T12:18:41.699Z"
)
let item = try DynamoDB.UpdateItemCodableInput(
            conditionExpression: "attribute_exists(#createdAt)",
            key: [keyName],
            tableName: tableName,
            updateItem: item
        ).createUpdateItemInput()
let _ = try await db.updateItem(input)

The previous command has a problem though, there is no way to add a condition on the key and, in a concurrent scenario there is no way to prevent an update if updatedAt field doesn't match with the current value on the DynamoDB table.

Having additionalAttributeNames and additionalAttributeValues as parameter of the DynamoDB.UpdateItemCodableInput will allow to write more complex conditionExpression without loosing the flexibility introduced by the generated expressionAttributeValues and expressionAttributeNames.

let item = Item(
   key: "9D1DC963-411A-486F-BE26-ADF473F00B80",
   name: "MyItem"
   createdAt: "2023-05-08T18:17:34.972Z",
   updatedAt: "2023-05-12T12:18:41.699Z"
)
let oldUpdatedAt = "2023-05-11T12:18:41.699Z"
let item = try DynamoDB.UpdateItemCodableInput(
            additionalAttributeNames: ["#keyName": keyName],
            additionalAttributeValues: [":oldUpdatedAt" : .s(oldUpdatedAt)],
            conditionExpression: "attribute_exists(#keyName) AND #updatedAt = :oldUpdatedAt",
            key: [keyName],
            tableName: tableName,
            updateItem: item
        )
let _ = try await db.updateItem(input)

The previous update will update the table only if the key exists and the updateAt value in the table match the value oldUpdatedAt of the request, while the new value is set to Item.updatedAt.

In the previous example the additionalAttributeNames dictionary will be merged to expressionAttributeNames and additionalAttributeValues to expressionAttributeValues.

Implementation

• The implementation introduces a non breaking change
• Add additionalAttributeNames and additionalAttributeValues as nullable in the init method of DynamoDB.ConditionalUpdateItemCodableInput.
• In case expressionAttributeNames or updateExpression are nil and additionalAttributeNames is not nil, the content of additionalAttributeNames will be merged to expressionAttributeNames overriding existing keys.
• If additionalAttributeValues is not nil, the content of additionalAttributeValues will be merged to expressionAttributeValues overriding existing keys.

[adam-fowler]

Hi Andrea, sorry I haven't got back to you earlier on this. I was on holiday when you posted it and then proceeded to forget about it 🤭 .

I'm still trying to get my head around the exact need for these changes. It's been a while since I looked at the DynamoDB code. Once I have a better idea what is happening here I'll get back.

Your changes are breaking because they change the function signature. I would suggest you create a new version of the function with these additional parameters and functionality. Do not give the new parameters default values and then the compiler will chose one of the two based on whether you use these parameters.

[adam-fowler]

Does this basically allow you to add conditions on fields that aren't included in the Codable type?

[Andrea-Scuderi]

Does this basically allow you to add conditions on fields that aren't included in the Codable type?

Yes, correct.

[Andrea-Scuderi]

@adam-fowler Not sure why the PR fails on S3 tests though, they are not involved in the change.

[adam-fowler]

@adam-fowler Not sure why the PR fails on S3 tests though, they are not involved in the change.

localstack updated and broke something. I have a local fix, once that is in I'll tell you and you can merge main back into this branch

[adam-fowler]

If you merge main in now it should work.

[Andrea-Scuderi]

If you merge main in now it should work.

I've merged could you give it a go?

[adam-fowler]

Hi Andrea, I have been looking at this again and was wondering the API seems an odd mix of Attributes and Codable types. Do you think you could replace the additional names and values with a Codable additionalAttributes. eg

init<AdditionalAttributes: Decodable>(additionAttributes: AdditionalAttributes, conditionExpression: ...) {
    var item = try DynamoDBEncoder().encode(additionAttributes)
    ...

I don't know if this makes sense, what do you think?

[Andrea-Scuderi]

Hi Andrea, I have been looking at this again and was wondering the API seems an odd mix of Attributes and Codable types. Do you think you could replace the additional names and values with a Codable additionalAttributes. eg

init<AdditionalAttributes: Decodable>(additionAttributes: AdditionalAttributes, conditionExpression: ...) {
    var item = try DynamoDBEncoder().encode(additionAttributes)
    ...

I don't know if this makes sense, what do you think?

Nice one! I fixed it in https://github.com/soto-project/soto/pull/671/commits/07fb3cd3937794044dfbf56fa7fb25ef952dfc7b

Could you run the CI?

[Andrea-Scuderi]

This looks good. Wish I had done the initialisers for all the Codable stuff in a similar manner, instead of adding new Codable request types. Might add them later and deprecate the current separate type with conversion function. What's your thoughts?

Yes, it seems a good idea, but it's required to break compatibility. I realised this solution after I tried to add the Encodable in the struct declaration. I'd suggest to merge this and then iterate to improve that class. Happy to help.

[adam-fowler]

I'd suggest to merge this and then iterate to improve that class.

Oh yes I agree there