DynamoDB Object persistence Model does not respect `required` keyword
- Dominant language
- C#
- Stars
- 140
- Forks
- 891
- Avg merge
- 21h 51m
- Merged PRs (30d)
- 10
Description
### Describe the bug
C# 11 introduced the [required](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/required) keyword to signal to the compiler that class members must be initialized using an object initializer.
This is also used by `System.Text.Json.JsonSerializer` to perform data validation at runtime.
```csharp
using System;
using System.Text.Json;
var json = """{"Email": "onefish@example.com", "DisplayName": "Fish One"}""";
// This will fail if Email or DisplayName are missing above
var user = JsonSerializer.Deserialize(json);
if (user is null) {
return;
}
// No complaints about dereferencing a possible null-reference
Console.WriteLine(user.DisplayName.ToUpper());
// Class has two properties, Email and DisplayName, that can not be null.
class User {
public required string Email {get; set;}
public required string DisplayName {get; set;}
}
```
When retrieving a POCO from DynamoDB via `DynamoDBContext` this modifier is ignored, making it possible to create invalid objects accidentally.
Take the following DynamoDB table storing users with their email and a display name.
| Email (pk, string) | DisplayName (string) |
| ------------------- | -------------------- |
| onefish@example.com | Fish One |
| twofish@example.com | _null_ |
| redfish@example.com | _empty_ |
```csharp
var client = new AmazonDynamoDBClient();
var context = new DynamoDBContext(client);
var users = await context.ScanAsync([]).GetRemainingAsync();
// It *should* be safe to assume that DisplayName is not null and .ToUpper() will not fail with a NullReferenceException
Console.WriteLine(string.Join('\n', users.Select(user => user.DisplayName.ToUpper()));)
// Same as in previous example
class User {
[DynamoDBHashKey]
public required string Email {get; set;}
public required string DisplayName {get; set;}
}
```
Creating a `User` instance via `DynamoDBContext.ScanAsync([])` can create an invalid object if a property marked `required` does not exist on the corresponding item in DynamoDB, or is null, thus breaking expected guarantees.
### Expected Behavior
In the code snippet above I would expect the DynamoDB SDK to fail fast when encountering invalid values, instead of passing them along, which can cause the kind of unexpected behavior that `required` / `#nullable` is supposed to guard against.
### Current Behavior
The SDK creates invalid objects at runtime.
### Reproduction Steps
1. Create the following DynamoDB Table:
| Email (string, pk) | DisplayName (string) |
| ------------------- | -------------------- |
| onefish@example.com | Fish One |
| twofish@example.com | _null_ |
| redfish@example.com | _no value_ |
2. Apply the following patch to the main branch using `git apply`:
```diff
diff --git a/testapp/Program.cs b/testapp/Program.cs
new file mode 100644
index 00000000000..c12fd3b4a30
--- /dev/null
+++ b/testapp/Program.cs
@@ -0,0 +1,16 @@
+using Amazon.DynamoDBv2;
+using Amazon.DynamoDBv2.DataModel;
+
+var client = new AmazonDynamoDBClient();
+var context = new DynamoDBContext(client);
+
+var users = await context.ScanAsync([], new() {OverrideTableName = Environment.GetEnvironmentVariable("DDB_TABLE")}).GetRemainingAsync();
+
+Console.WriteLine(string.Join('\n', users.Select(user => user.DisplayName.ToUpper())));
+
+// Class has two properties, Email and DisplayName, that should never be null.
+class User {
+ [DynamoDBHashKey]
+ public required string Email {get; set;}
+ public required string DisplayName {get; set;}
+}
\ No newline at end of file
diff --git a/testapp/testapp.csproj b/testapp/testapp.csproj
new file mode 100644
index 00000000000..db7d421377c
--- /dev/null
+++ b/testapp/testapp.csproj
@@ -0,0 +1,20 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+ Exe
+ net8.0
+ enable
+ enable
+
+
+
```
3. Run the example
```bash
cd ./testapp
DDB_TABLE="" dotnet run
```
It will fail when trying to invoke `ToUpper()` on a null value.
### Possible Solution
#3277
### Additional Information/Context
I consider this a bug, since `System.Text.Json.JsonSerializer` upholds these guarantees at runtime, whereas `Amazon.DynamoDBv2.DataModel.DynamoDBContext` does not.
### AWS .NET SDK and/or Package version used
AWSSDK.DynamoDBv2
Git commit 23b74540be451eda8bb789980b4863bd8cc4241f (HEAD at time of writing)
### Targeted .NET Platform
.NET 8
### Operating System and version
Windows 11 22H2
Contributor guide
Research direction
Start with the DynamoDBContext.ScanAsync entry point and the reproduction files testapp/Program.cs and testapp/testapp.csproj. Review the possible solution in #3277 and trace how required properties are handled when DynamoDB values are missing or null. Done means invalid objects fail fast rather than causing a later NullReferenceException.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100