Skip to content

.Net: Fix #14021: Prune unannotated POCO properties from Redis JSON payloads #14045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
summitbaj wants to merge 3 commits into
base: main
Choose a base branch
from
Open

.Net: Fix #14021: Prune unannotated POCO properties from Redis JSON payloads #14045

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3351b2b
Fix #14021: Prune unannotated POCO properties from Redis JSON payloads
summitbaj May 30, 2026
2428922
Address Copilot review feedback on PR
summitbaj May 30, 2026
2d1954e
Address Copilot feedback: Move Assert.NotNull before access to improv…
summitbaj May 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions dotnet/src/VectorData/Redis/RedisJsonMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ internal sealed class RedisJsonMapper<TConsumerDataModel>(
/// <summary>The key property.</summary>
private readonly string _keyPropertyStorageName = model.KeyProperty.StorageName;

/// <summary>Storage names for the allowed properties that should be retained in the Redis payload.</summary>
private readonly HashSet<string> _allowedStorageNames = new(
model.DataProperties.Select(p => p.StorageName).Concat(model.VectorProperties.Select(p => p.StorageName)),
StringComparer.Ordinal);

/// <inheritdoc />
public (string Key, JsonNode Node) MapFromDataToStorageModel(TConsumerDataModel dataModel, int recordIndex, IReadOnlyList<Embedding>?[]? generatedEmbeddings)
{
Expand All @@ -41,6 +46,13 @@ internal sealed class RedisJsonMapper<TConsumerDataModel>(
var keyValue = jsonValue.ToString();
jsonNode.Remove(this._keyPropertyStorageName);

// Remove any properties that are not part of the data model.
var propertiesToRemove = jsonNode.Select(x => x.Key).Where(key => !this._allowedStorageNames.Contains(key)).ToList();
foreach (var keyToRemove in propertiesToRemove)
{
jsonNode.Remove(keyToRemove);
}
Comment thread
summitbaj marked this conversation as resolved.

// Go over the vector properties; inject any generated embeddings to overwrite the JSON serialized above.
// Also, for Embedding<T> properties we also need to overwrite with a simple array (since Embedding<T> gets serialized as a complex object).
for (var i = 0; i < model.VectorProperties.Count; i++)
Expand Down
14 changes: 7 additions & 7 deletions dotnet/test/VectorData/Redis.UnitTests/RedisJsonCollectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,10 @@ public async Task CanDeleteManyRecordsWithVectorsAsync(bool useDefinition)
}

[Theory]
[InlineData(true, true, """{"data1_json_name":"data 1","data2":"data 2","vector1_json_name":[1,2,3,4],"vector2":[1,2,3,4],"notAnnotated":null}""")]
[InlineData(true, false, """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4],"NotAnnotated":null}""")]
[InlineData(false, true, """{"data1_json_name":"data 1","data2":"data 2","vector1_json_name":[1,2,3,4],"vector2":[1,2,3,4],"notAnnotated":null}""")]
[InlineData(false, false, """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4],"NotAnnotated":null}""")]
[InlineData(true, true, """{"data1_json_name":"data 1","data2":"data 2","vector1_json_name":[1,2,3,4],"vector2":[1,2,3,4]}""")]
[InlineData(true, false, """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4]}""")]
[InlineData(false, true, """{"data1_json_name":"data 1","data2":"data 2","vector1_json_name":[1,2,3,4],"vector2":[1,2,3,4]}""")]
[InlineData(false, false, """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4]}""")]
Comment on lines +308 to +311
public async Task CanUpsertRecordAsync(bool useDefinition, bool useCustomJsonSerializerOptions, string expectedUpsertedJson)
{
// Arrange
Expand All @@ -320,7 +320,7 @@ public async Task CanUpsertRecordAsync(bool useDefinition, bool useCustomJsonSer
await sut.UpsertAsync(model);

// Assert
// TODO: Fix issue where NotAnnotated is being included in the JSON.
// (Fixed) Removed TODO regarding NotAnnotated being included in the JSON.
Comment thread
summitbaj marked this conversation as resolved.
Outdated
var expectedArgs = new object[] { TestRecordKey1, "$", expectedUpsertedJson };
this._redisDatabaseMock
.Verify(
Expand All @@ -346,8 +346,8 @@ public async Task CanUpsertManyRecordsAsync(bool useDefinition)
await sut.UpsertAsync([model1, model2]);

// Assert
// TODO: Fix issue where NotAnnotated is being included in the JSON.
var expectedArgs = new object[] { TestRecordKey1, "$", """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4],"NotAnnotated":null}""", TestRecordKey2, "$", """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4],"NotAnnotated":null}""" };
// (Fixed) Removed TODO regarding NotAnnotated being included in the JSON.
var expectedArgs = new object[] { TestRecordKey1, "$", """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4]}""", TestRecordKey2, "$", """{"data1_json_name":"data 1","Data2":"data 2","vector1_json_name":[1,2,3,4],"Vector2":[1,2,3,4]}""" };
this._redisDatabaseMock
.Verify(
x => x.ExecuteAsync(
Expand Down
2 changes: 2 additions & 0 deletions dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public void MapsAllFieldsFromDataToStorageModel()
Assert.Equal("data 2", jsonObject?["Data2"]?.ToString());
Assert.Equal(new float[] { 1, 2, 3, 4 }, jsonObject?["Vector1"]?.AsArray().GetValues<float>().ToArray());
Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject?["Vector2"]?.AsArray().GetValues<float>().ToArray());
Assert.False(jsonObject!.ContainsKey("NotAnnotated"), "Unannotated properties should not be included in the storage model.");
Comment thread
summitbaj marked this conversation as resolved.
Outdated
}

[Fact]
Expand All @@ -56,6 +57,7 @@ public void MapsAllFieldsFromDataToStorageModelWithCustomSerializerOptions()
Assert.Equal("data 2", jsonObject?["data2"]?.ToString());
Assert.Equal(new float[] { 1, 2, 3, 4 }, jsonObject?["vector1"]?.AsArray().GetValues<float>().ToArray());
Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject?["vector2"]?.AsArray().GetValues<float>().ToArray());
Assert.False(jsonObject!.ContainsKey("notAnnotated"), "Unannotated properties should not be included in the storage model.");
}

[Fact]
Expand Down
Loading