From 942d804a0c99361f65b70b0f7b56ae0d1a5c1e3b Mon Sep 17 00:00:00 2001 From: John Viegas Date: Mon, 28 Apr 2025 12:46:05 -0700 Subject: [PATCH 1/2] Test Coverage Checklist for Enhanced Dynamo DB PRs --- .../enhanced-client-testing.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md diff --git a/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md b/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md new file mode 100644 index 000000000000..a47387f4f82d --- /dev/null +++ b/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md @@ -0,0 +1,59 @@ +# DynamoDB Enhanced Client Test Checklist + +Below is the checklist required for merging DynamoDB Enhanced Client test changes. +Please complete this checklist when making any changes to Enhanced DDB. + +**Instructions:** +- Mark completed tests with [x] (e.g., [x] instead of [ ]) +- For tests not completed, provide a reason in the "Comments" column +- If your code changes are covered by existing tests, reference the test class and method name in the Comments column (e.g., `TableSchemaTest.testFromBean()`) +- Please copy the markdown into your PR and update it in Testing section +- [Reference for CRUD Operations test](https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/BasicCrudTest.java) +- [Reference for Data Types and Null Handling](https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/document/EnhancedDocumentTestData.java) + +```md +## Test Coverage Checklist + +| Scenario | Done | Comments if Not Done | +|---------|:----:|---------------------| +| **1. Different TableSchema Creation Methods** | | | +| a. TableSchema.fromBean(Customer.class) | [ ] | | +| b. TableSchema.fromImmutableClass(Customer.class) for immutable classes | [ ] | | +| c. TableSchema.documentSchemaBuilder().build() | [ ] | | +| d. StaticTableSchema.builder(Customer.class) | [ ] | | +| **2. Nesting of Different TableSchema Types** | | | +| a. @DynamoDbBean with nested @DynamoDbBean as NonNull | [ ] | | +| b. @DynamoDbBean with nested @DynamoDbImmutable as NonNull | [ ] | | +| c. @DynamoDbImmutable with nested @DynamoDbBean as NonNull | [ ] | | +| d. @DynamoDbBean with nested @DynamoDbBean as Null | [ ] | | +| e. @DynamoDbBean with nested @DynamoDbImmutable as Null| [ ] | | +| f. @DynamoDbImmutable with nested @DynamoDbBean as Null | [ ] | | +| **3. CRUD Operations** | | | +| a. scan() | [ ] | | +| b. query() | [ ] | | +| c. updateItem() | [ ] | | +| d. putItem() | [ ] | | +| e. getItem() | [ ] | | +| f. deleteItem() | [ ] | | +| g. batchGetItem() | [ ] | | +| h. batchWriteItem() | [ ] | | +| i. transactGetItems() | [ ] | | +| j. transactWriteItems() | [ ] | | +| **4. Data Types and Null Handling** | | | +| a. top-level null attributes | [ ] | | +| b. collections with null elements | [ ] | | +| c. maps with null values | [ ] | | +| d. conversion between null Java values and AttributeValue | [ ] | | +| e. full serialization/deserialization cycle with null values | [ ] | | +| **5. AsyncTable and SyncTable** | | | +| a. DynamoDbAsyncTable Testing | [ ] | | +| b. DynamoDbTable Testing | [ ] | | +| **6. New/Modification in Extensions** | | | +| a. Tables with Scenario in ScenarioSl No.1 (All table schemas are Must) | [ ] | | +| b. Test with Default Values in Annotations | [ ] | | +| c. Combination of Annotation and Builder passes extension | [ ] | | +| **7. New/Modification in Converters** | | | +| a. Tables with Scenario in ScenarioSl No.1 (All table schemas are Must) | [ ] | | +| b. Test with Default Values in Annotations | [ ] | | +| c. Test All Scenarios from 1 to 5 | [ ] | | +``` \ No newline at end of file From e7c3ac37372cac263cde3d8fa6fbc19bd8c84885 Mon Sep 17 00:00:00 2001 From: John Viegas Date: Mon, 5 May 2025 15:40:10 -0700 Subject: [PATCH 2/2] Updated the review comments --- .../enhanced-client-testing.md | 44 +++++++++++++------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md b/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md index a47387f4f82d..4742ef420e96 100644 --- a/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md +++ b/docs/testing/services-custom/dynamodb-enhanced/enhanced-client-testing.md @@ -8,8 +8,8 @@ Please complete this checklist when making any changes to Enhanced DDB. - For tests not completed, provide a reason in the "Comments" column - If your code changes are covered by existing tests, reference the test class and method name in the Comments column (e.g., `TableSchemaTest.testFromBean()`) - Please copy the markdown into your PR and update it in Testing section -- [Reference for CRUD Operations test](https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/BasicCrudTest.java) -- [Reference for Data Types and Null Handling](https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/document/EnhancedDocumentTestData.java) + + ```md ## Test Coverage Checklist @@ -21,14 +21,14 @@ Please complete this checklist when making any changes to Enhanced DDB. | b. TableSchema.fromImmutableClass(Customer.class) for immutable classes | [ ] | | | c. TableSchema.documentSchemaBuilder().build() | [ ] | | | d. StaticTableSchema.builder(Customer.class) | [ ] | | -| **2. Nesting of Different TableSchema Types** | | | -| a. @DynamoDbBean with nested @DynamoDbBean as NonNull | [ ] | | -| b. @DynamoDbBean with nested @DynamoDbImmutable as NonNull | [ ] | | -| c. @DynamoDbImmutable with nested @DynamoDbBean as NonNull | [ ] | | -| d. @DynamoDbBean with nested @DynamoDbBean as Null | [ ] | | -| e. @DynamoDbBean with nested @DynamoDbImmutable as Null| [ ] | | -| f. @DynamoDbImmutable with nested @DynamoDbBean as Null | [ ] | | -| **3. CRUD Operations** | | | +| **2. Nesting of Different TableSchema Types ** (Ref-1)| | | +| a. @DynamoDbBean with non-null nested @DynamoDbBean attribute | [ ] | | +| b. @DynamoDbBean with non-null nested @DynamoDbImmutable attribute | [ ] | | +| c. @DynamoDbImmutable with non-null nested @DynamoDbBean attribute | [ ] | | +| d. @DynamoDbBean with null nested @DynamoDbBean attribute | [ ] | | +| e. @DynamoDbBean with null nested @DynamoDbImmutable attribute | [ ] | | +| f. @DynamoDbImmutable with null nested @DynamoDbBean attribute | [ ] | | +| **3. CRUD Operations** (Ref-2) | | | | a. scan() | [ ] | | | b. query() | [ ] | | | c. updateItem() | [ ] | | @@ -39,12 +39,11 @@ Please complete this checklist when making any changes to Enhanced DDB. | h. batchWriteItem() | [ ] | | | i. transactGetItems() | [ ] | | | j. transactWriteItems() | [ ] | | -| **4. Data Types and Null Handling** | | | +| **4. Null Handling for Different Attribute types (Ref-3)** | | | | a. top-level null attributes | [ ] | | | b. collections with null elements | [ ] | | | c. maps with null values | [ ] | | -| d. conversion between null Java values and AttributeValue | [ ] | | -| e. full serialization/deserialization cycle with null values | [ ] | | +| d. full serialization/deserialization cycle with null values | [ ] | | | **5. AsyncTable and SyncTable** | | | | a. DynamoDbAsyncTable Testing | [ ] | | | b. DynamoDbTable Testing | [ ] | | @@ -56,4 +55,21 @@ Please complete this checklist when making any changes to Enhanced DDB. | a. Tables with Scenario in ScenarioSl No.1 (All table schemas are Must) | [ ] | | | b. Test with Default Values in Annotations | [ ] | | | c. Test All Scenarios from 1 to 5 | [ ] | | -``` \ No newline at end of file +``` + +## Reference + +[Ref-1] Issue were assigning Nested attribute as Null value resulted in NPE [#6037](https://github.com/aws/aws-sdk-java-v2/pull/6037) + +[Ref-2] [Reference for CRUD Operations test](https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/BasicCrudTest.java) + +[Ref-3] Example for Null Handling for Different Attribute types + +| Case | Bean Representation | Item Representation (DynamoDB JSON) | +|-------------------------------------------------|---------------------|-------------------------------------| +| **a. Top-level null attributes** | 
Customer c = new Customer();
c.setAccountId("123");
c.setName(null);
| 
{
  "accountId": {"S": "123"},
  "name": {"NULL": true}
}
| +| **b. Collections with null elements** | 
Customer c = new Customer();
c.setAccountId("123");
c.setEmails(Arrays.asList(
  "a@example.com", null));
| 
{
  "accountId": {"S": "123"},
  "emails": {
    "L": [
      {"S": "a@example.com"},
      {"NULL": true}
    ]
  }
}
| +| **c. Maps with null values** | 
Customer c = new Customer();
c.setAccountId("123");
Map attrs = new HashMap<>();
attrs.put("tier", "gold");
attrs.put("status", null);
c.setAttributes(attrs);
| 
{
  "accountId": {"S": "123"},
  "attributes": {
    "M": {
      "tier": {"S": "gold"},
      "status": {"NULL": true}
    }
  }
}
| +| **d. Full serialization/deserialization cycle** | 
// Create with nulls
Customer c = new Customer();
c.setAccountId("123");
c.setName(null);
c.setEmails(Arrays.asList("a@example.com", null));

// Save to DynamoDB
table.putItem(c);

// Retrieve
Customer retrieved = table.getItem(key);

// Nulls preserved
assert retrieved.getName() == null;
assert retrieved.getEmails().get(1) == null;
| 
{
  "accountId": {"S": "123"},
  "name": {"NULL": true},
  "emails": {
    "L": [
      {"S": "a@example.com"},
      {"NULL": true}
    ]
  }
}
| +- [Reference for Null Handling for Different Attribute types](https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/document/EnhancedDocumentTestData.java) +