aws / aws/aws-sdk-net

DynamoDB Object persistence Model does not respect `required` keyword

Open
#3,276 5 comments 0 reactions 0 assignees View on GitHub
dynamodb feature-request p2 queued
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.