aws / aws-sdk-java-v2

The official AWS SDK for Java - Version 2
Apache License 2.0
2.16k stars 835 forks source link

DynamoDbTable.updateItem clears data from table, then only sets non-null field values #4711

Open ipsi-apant opened 10 months ago

ipsi-apant commented 10 months ago

Describe the bug

When executing DynamoDbTable.updateItem for a few fields, the execution clear the existing data and only sets new field values from input object. My understanding, for an existing record, when a Java object populated with primary key and fields to be changed, this operation should affect only the fields to be changed for the primary key record. Curious to know, how is this updateItem supposed to work?

Expected Behavior

Current Behavior

Overwriting existing contents of a record for updateItem operation.

I am using spring-cloud-aws-starter-dynamodb using DynamoDbTemplate AWS Spring DynamoDb, however, it fails when using below code snippet


/*
Existing record:
 Car(vin=1234567890, color=white, year=2020, model=Ford Mustang)
*/

final Car car = Car
    .builder()
    .vin("1234567890")
    .color("black") // existing record has white color
    .build();

Car updatedCar = dynamoDbEnhancedClient.table("Car", TableSchema.fromBean(Car.class))
                                    .updateItem(car);

assertEquals("black", updatedCar.getColor()); // PASS
assertEquals("2020", updatedCar.getYear()); // FAIL
assertEquals("Ford Mustang", updatedCar.getModel()); // FAIL

Reproduction Steps

  1. Create a Car table with fields vin (primary key), year, model, color
  2. Create Java class named as Car
  3. Add class level annotation @DynamoDbBean
  4. Add @DynamoDbPartitionKey annotation to a getter method of vin
  5. Add a new record in Car table as mentioned in Current Behavior
  6. Create an instance of dynamoDbEnhancedClient
  7. Execute code snippet as mentioned in Current Behavior

Possible Solution

Additional Information/Context

AWS Java SDK version used

2.20.153

JDK version used

21

Operating System and version

Windows 11 Pro x64

debora-ito commented 7 months ago

Hi @ipsi-apant apologies for the long silence.

Does it work if you set @DynamoDbUpdateBehavior to WRITE_IF_NOT_EXISTS?

https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/ddb-en-client-adv-features-upd-behavior.html

ipsi-apant commented 7 months ago

Hi @debora-ito , thank you for reviewing and your time.

I will give it a try for this option. This is a field level annotation, would like to learn more on, what are the challenges for not having a class level annotation (for all fields)?

I am wondering, for the above example, if this annotation added to the color property, would it still be able to update to 'black' color ?

My understanding from this java doc, if a new value for color is provided, it will be a 'a new record is being written` scenario, right ?

Write the new value if there is no existing value in the persisted record or a new record is being written, otherwise leave the existing value.

ipsi-apant commented 5 months ago

Hi @debora-ito, this is not working as expected.

AWS SDK version 2.24.1

It fails during update car operation. This line car.getModel().equals("Ford Mustang"). As per stacktrace it has null.

During debug I have captured screenshots after inserting a record and after update the record by vin number. Attached for review here. It did update color, however removed model and year.

Also tried using @DynamoDbIgnoreNulls. The result is same. https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/enhanced/dynamodb/mapper/annotations/DynamoDbIgnoreNulls.html

Car (bean)

@Data
@NoArgsConstructor
@AllArgsConstructor
@Builder
@DynamoDbBean
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_NULL)
public class Car {

  private String vin;
  private String model;
  private String color;
  private String year;

  @DynamoDbPartitionKey
  public String getVin() {
    return vin;
  }

  @DynamoDbUpdateBehavior(UpdateBehavior.WRITE_IF_NOT_EXISTS)
  public String getModel() {
    return model;
  }

  @DynamoDbUpdateBehavior(UpdateBehavior.WRITE_IF_NOT_EXISTS)
  public String getColor() {
    return color;
  }

  @DynamoDbUpdateBehavior(UpdateBehavior.WRITE_IF_NOT_EXISTS)
  public String getYear() {
    return year;
  }
}

Unit test (We are using Springboot Webflux)

@Test
  void testCarWithoutUpdateBehaviour() {
    // create new
    Mono<Car> saved = save(VIN_10, "Ford Mustang", "White", "2020");
    StepVerifier.create(saved)
                .expectNextMatches(car -> car.getVin()
                                             .equals(VIN_10)
                    && car.getModel()
                          .equals("Ford Mustang")
                    && car.getColor()
                          .equals("White")
                    && car.getYear()
                          .equals("2020")
                )
                .verifyComplete();

    // get car
    Mono<Car> gotCar = get(VIN_10);
    StepVerifier.create(gotCar)
                .expectNextMatches(car -> car.getVin()
                                             .equals(VIN_10)
                    && car.getModel()
                          .equals("Ford Mustang")
                    && car.getColor()
                          .equals("White")
                    && car.getYear()
                          .equals("2020")
                )
                .verifyComplete();

    // update car
    Mono<Car> updatedCar = update(VIN_10, "Black");
    StepVerifier.create(updatedCar)
                .expectNextMatches(car -> car.getVin()
                                             .equals(VIN_10)
                    && car.getModel()
                          .equals("Ford Mustang")
                    && car.getColor()
                          .equals("Black")
                    && car.getYear()
                          .equals("2020")
                )
                .verifyComplete();

  }

  private Mono<Car> get(String vin) {
    return carService.get(vin);
  }

  private Mono<Car> save(String vin, String model, String color, String year) {
    return carService.save(Car.builder()
                              .vin(vin)
                              .model(model)
                              .color(color)
                              .year(year)
                              .build());
  }

  private Mono<Car> update(String vin, String color) {
    return carService.update(Car.builder()
                                .vin(vin)
                                .color(color)
                                .build());
  }

Stacktrace

2024-03-25T10:59:59.058+11:00  INFO 55272 --- [token-manager] [    Test worker] CarService  : Saving car: Car(vin=VIN10, model=Ford Mustang, color=White, year=2020)
2024-03-25T10:59:59.072+11:00 DEBUG 55272 --- [token-manager] [    Test worker] AnnotationBasedDynamoDbTableNameResolver : Class: Car, DynamoDb table name: Car
2024-03-25T10:59:59.189+11:00  INFO 55272 --- [token-manager] [    Test worker] CarService  : Getting car with vin: VIN10
2024-03-25T10:59:59.190+11:00 DEBUG 55272 --- [token-manager] [    Test worker] AnnotationBasedDynamoDbTableNameResolver : Class: Car, DynamoDb table name: Car
2024-03-25T11:12:32.642+11:00  INFO 55272 --- [token-manager] [    Test worker] CarService  : Updating car: Car(vin=VIN10, model=null, color=Black, year=null)
2024-03-25T11:12:34.074+11:00 DEBUG 55272 --- [token-manager] [    Test worker] AnnotationBasedDynamoDbTableNameResolver : Class: Car, DynamoDb table name: Car

java.lang.AssertionError: expectation failed (failed running expectation on signal [onNext(Car(vin=VIN10, model=null, color=Black, year=null))] with [java.lang.NullPointerException]:
Cannot invoke "String.equals(Object)" because the return value of "Car.getModel()" is null)
    at reactor.test.MessageFormatter.assertionError(MessageFormatter.java:115)
    at reactor.test.MessageFormatter.failPrefix(MessageFormatter.java:104)
    at reactor.test.MessageFormatter.fail(MessageFormatter.java:73)
    at reactor.test.MessageFormatter.failOptional(MessageFormatter.java:88)
    at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.onExpectation(DefaultStepVerifierBuilder.java:1511)
    at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.onNext(DefaultStepVerifierBuilder.java:1146)
    at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onNext(FluxOnAssembly.java:539)
    at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onNext(MonoPeekTerminal.java:180)
    at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onNext(FluxOnAssembly.java:539)
    at reactor.core.publisher.MonoCallable$MonoCallableSubscription.request(MonoCallable.java:156)
    at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.request(FluxOnAssembly.java:649)
    at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.request(MonoPeekTerminal.java:139)
    at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.request(FluxOnAssembly.java:649)
    at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.onSubscribe(DefaultStepVerifierBuilder.java:1161)
    at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onSubscribe(FluxOnAssembly.java:633)
    at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onSubscribe(MonoPeekTerminal.java:152)
    at reactor.core.publisher.FluxOnAssembly$OnAssemblySubscriber.onSubscribe(FluxOnAssembly.java:633)
    at reactor.core.publisher.MonoCallable.subscribe(MonoCallable.java:48)
    at reactor.core.publisher.Mono.subscribe(Mono.java:4512)
    at reactor.test.DefaultStepVerifierBuilder$DefaultStepVerifier.toVerifierAndSubscribe(DefaultStepVerifierBuilder.java:891)
    at reactor.test.DefaultStepVerifierBuilder$DefaultStepVerifier.verify(DefaultStepVerifierBuilder.java:831)
    at reactor.test.DefaultStepVerifierBuilder$DefaultStepVerifier.verify(DefaultStepVerifierBuilder.java:823)
    at reactor.test.DefaultStepVerifierBuilder.verifyComplete(DefaultStepVerifierBuilder.java:690)
    at CarTests.testCarWithoutUpdateBehaviour(CarTests.java:79)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    Suppressed: java.lang.NullPointerException: Cannot invoke "String.equals(Object)" because the return value of "Car.getModel()" is null
        at CarTests.lambda$testCarWithoutUpdateBehaviour$3(CarTests.java:73)
        at reactor.test.DefaultStepVerifierBuilder.lambda$expectNextMatches$11(DefaultStepVerifierBuilder.java:558)
        at reactor.test.DefaultStepVerifierBuilder$SignalEvent.test(DefaultStepVerifierBuilder.java:2289)
        at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.onSignal(DefaultStepVerifierBuilder.java:1529)
        at reactor.test.DefaultStepVerifierBuilder$DefaultVerifySubscriber.onExpectation(DefaultStepVerifierBuilder.java:1477)
        ... 22 more

Screenshot 1 (after insert record)

image

Screenshot 1 (after update record)

image