From 4fa0f5cf9fd7178fb9dc2a541b20358dced36f23 Mon Sep 17 00:00:00 2001 From: John CSA <103165870+jluocsa@users.noreply.github.com> Date: Sat, 23 May 2026 11:59:15 -0700 Subject: [PATCH 1/3] .Net: Fix RedisJsonCollection upsert persisting unannotated POCO properties (#14021) RedisJsonMapper.MapFromDataToStorageModel previously serialized the entire record via JsonSerializer.SerializeToNode and only stripped the key, so any public POCO property without a [VectorStoreKey]/[VectorStoreData]/[VectorStoreVector] attribute (and not declared in the collection definition) was persisted to Redis. Build the JSON payload explicitly from model.DataProperties and model.VectorProperties, mirroring RedisJsonDynamicMapper. Storage keys still come from property.StorageName (already populated by CollectionJsonModelBuilder from JsonPropertyName / PropertyNamingPolicy / the .NET property name), so the produced JSON for schema-annotated properties matches the previous behavior. Updates RedisJsonCollectionTests upsert expectations (drops NotAnnotated:null fragments + TODOs) and adds an explicit assertion in RedisJsonMapperTests that unannotated properties are absent from the serialized JSON. --- .../src/VectorData/Redis/RedisJsonMapper.cs | 100 +++++++++--------- .../RedisJsonCollectionTests.cs | 12 +-- .../Redis.UnitTests/RedisJsonMapperTests.cs | 2 + 3 files changed, 56 insertions(+), 58 deletions(-) diff --git a/dotnet/src/VectorData/Redis/RedisJsonMapper.cs b/dotnet/src/VectorData/Redis/RedisJsonMapper.cs index 7755866b1c16..b2d3cb0a4ff1 100644 --- a/dotnet/src/VectorData/Redis/RedisJsonMapper.cs +++ b/dotnet/src/VectorData/Redis/RedisJsonMapper.cs @@ -27,72 +27,70 @@ internal sealed class RedisJsonMapper( /// public (string Key, JsonNode Node) MapFromDataToStorageModel(TConsumerDataModel dataModel, int recordIndex, IReadOnlyList?[]? generatedEmbeddings) { - // Convert the provided record into a JsonNode object and try to get the key field for it. - // Since we already checked that the key field is a string in the constructor, and that it exists on the model, - // the only edge case we have to be concerned about is if the key field is null. - var jsonNode = JsonSerializer.SerializeToNode(dataModel, jsonSerializerOptions)!.AsObject(); + // Extract the key. The constructor has already validated that the key property is a string. + var keyValue = model.KeyProperty.GetValueAsObject(dataModel) as string + ?? throw new InvalidOperationException($"Missing key field '{this._keyPropertyStorageName}' on provided record of type {typeof(TConsumerDataModel).FullName}."); - if (!(jsonNode.TryGetPropertyValue(this._keyPropertyStorageName, out var keyField) && keyField is JsonValue jsonValue)) + // Build the JSON payload from the model's data properties only, so that properties on the POCO that are not + // part of the vector-store schema (no [VectorStoreData]/[VectorStoreVector]/[VectorStoreKey] attribute and not + // in the collection definition) are not persisted in Redis. + var jsonNode = new JsonObject(); + + foreach (var dataProperty in model.DataProperties) { - throw new InvalidOperationException($"Missing key field '{this._keyPropertyStorageName}' on provided record of type {typeof(TConsumerDataModel).FullName}."); + var value = dataProperty.GetValueAsObject(dataModel); + jsonNode.Add( + dataProperty.StorageName, + value is null + ? null + : JsonSerializer.SerializeToNode(value, dataProperty.Type, jsonSerializerOptions)); } - // Remove the key field from the JSON object since we don't want to store it in the redis payload. - var keyValue = jsonValue.ToString(); - jsonNode.Remove(this._keyPropertyStorageName); - - // Go over the vector properties; inject any generated embeddings to overwrite the JSON serialized above. - // Also, for Embedding properties we also need to overwrite with a simple array (since Embedding gets serialized as a complex object). for (var i = 0; i < model.VectorProperties.Count; i++) { var property = model.VectorProperties[i]; - Embedding? embedding = generatedEmbeddings?[i]?[recordIndex] is Embedding ge ? ge : null; + var vector = generatedEmbeddings?[i]?[recordIndex] is Embedding ge + ? (object)ge + : property.GetValueAsObject(dataModel); - if (embedding is null) + if (vector is null) { - switch (Nullable.GetUnderlyingType(property.Type) ?? property.Type) - { - case var t when t == typeof(ReadOnlyMemory): - case var t2 when t2 == typeof(float[]): - case var t3 when t3 == typeof(ReadOnlyMemory): - case var t4 when t4 == typeof(double[]): - // The .NET vector property is a ReadOnlyMemory or T[] (not an Embedding), which means that JsonSerializer - // already serialized it correctly above. - // In addition, there's no generated embedding (which would be an Embedding which we'd need to handle manually). - // So there's nothing for us to do. - continue; - - case var t when t == typeof(Embedding): - case var t1 when t1 == typeof(Embedding): - embedding = (Embedding)property.GetValueAsObject(dataModel)!; - break; - - default: - throw new UnreachableException(); - } + jsonNode[property.StorageName] = null; + continue; } var jsonArray = new JsonArray(); - switch (embedding) + if (vector switch { - case Embedding e: - foreach (var item in e.Vector.Span) - { - jsonArray.Add(JsonValue.Create(item)); - } - break; - - case Embedding e: - foreach (var item in e.Vector.Span) - { - jsonArray.Add(JsonValue.Create(item)); - } - break; - - default: - throw new UnreachableException(); + ReadOnlyMemory m => m, + Embedding e => e.Vector, + float[] a => new ReadOnlyMemory(a), + _ => (ReadOnlyMemory?)null + } is ReadOnlyMemory floatMemory) + { + foreach (var item in floatMemory.Span) + { + jsonArray.Add(JsonValue.Create(item)); + } + } + else if (vector switch + { + ReadOnlyMemory m => m, + Embedding e => e.Vector, + double[] a => new ReadOnlyMemory(a), + _ => null + } is ReadOnlyMemory doubleMemory) + { + foreach (var item in doubleMemory.Span) + { + jsonArray.Add(JsonValue.Create(item)); + } + } + else + { + throw new UnreachableException(); } jsonNode[property.StorageName] = jsonArray; diff --git a/dotnet/test/VectorData/Redis.UnitTests/RedisJsonCollectionTests.cs b/dotnet/test/VectorData/Redis.UnitTests/RedisJsonCollectionTests.cs index 6441e0ff894b..a74d01a4b22b 100644 --- a/dotnet/test/VectorData/Redis.UnitTests/RedisJsonCollectionTests.cs +++ b/dotnet/test/VectorData/Redis.UnitTests/RedisJsonCollectionTests.cs @@ -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]}""")] public async Task CanUpsertRecordAsync(bool useDefinition, bool useCustomJsonSerializerOptions, string expectedUpsertedJson) { // Arrange @@ -320,7 +320,6 @@ public async Task CanUpsertRecordAsync(bool useDefinition, bool useCustomJsonSer await sut.UpsertAsync(model); // Assert - // TODO: Fix issue where NotAnnotated is being included in the JSON. var expectedArgs = new object[] { TestRecordKey1, "$", expectedUpsertedJson }; this._redisDatabaseMock .Verify( @@ -346,8 +345,7 @@ 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}""" }; + 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( diff --git a/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs b/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs index 3a6f157ec8f7..7255b6dadcc6 100644 --- a/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs +++ b/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs @@ -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().ToArray()); Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject?["Vector2"]?.AsArray().GetValues().ToArray()); + Assert.False(jsonObject!.ContainsKey("NotAnnotated")); } [Fact] @@ -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().ToArray()); Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject?["vector2"]?.AsArray().GetValues().ToArray()); + Assert.False(jsonObject!.ContainsKey("notAnnotated")); } [Fact] From 32e2b99859097841260820bdd7eae72b41a8fca5 Mon Sep 17 00:00:00 2001 From: John CSA <103165870+jluocsa@users.noreply.github.com> Date: Sun, 24 May 2026 07:45:58 -0700 Subject: [PATCH 2/3] .Net: address Copilot review on RedisJsonMapper tests Per Copilot's review on #14030: replace the null-forgiving operator on the mapped JsonObject with an explicit Assert.NotNull, then drop the '?' / '!' operators on subsequent index access and ContainsKey. This keeps test failures diagnostic (clean assertion failure instead of NullReferenceException) and reads as a single contract: 'the mapper must return a JsonObject; here is what it should contain'. --- .../Redis.UnitTests/RedisJsonMapperTests.cs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs b/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs index 7255b6dadcc6..8b581a3aad02 100644 --- a/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs +++ b/dotnet/test/VectorData/Redis.UnitTests/RedisJsonMapperTests.cs @@ -30,11 +30,12 @@ public void MapsAllFieldsFromDataToStorageModel() Assert.NotNull(actual.Node); Assert.Equal("test key", actual.Key); var jsonObject = actual.Node.AsObject(); - Assert.Equal("data 1", jsonObject?["Data1"]?.ToString()); - Assert.Equal("data 2", jsonObject?["Data2"]?.ToString()); - Assert.Equal(new float[] { 1, 2, 3, 4 }, jsonObject?["Vector1"]?.AsArray().GetValues().ToArray()); - Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject?["Vector2"]?.AsArray().GetValues().ToArray()); - Assert.False(jsonObject!.ContainsKey("NotAnnotated")); + Assert.NotNull(jsonObject); + Assert.Equal("data 1", jsonObject["Data1"]?.ToString()); + Assert.Equal("data 2", jsonObject["Data2"]?.ToString()); + Assert.Equal(new float[] { 1, 2, 3, 4 }, jsonObject["Vector1"]?.AsArray().GetValues().ToArray()); + Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject["Vector2"]?.AsArray().GetValues().ToArray()); + Assert.False(jsonObject.ContainsKey("NotAnnotated")); } [Fact] @@ -53,11 +54,12 @@ public void MapsAllFieldsFromDataToStorageModelWithCustomSerializerOptions() Assert.NotNull(actual.Node); Assert.Equal("test key", actual.Key); var jsonObject = actual.Node.AsObject(); - Assert.Equal("data 1", jsonObject?["data1"]?.ToString()); - Assert.Equal("data 2", jsonObject?["data2"]?.ToString()); - Assert.Equal(new float[] { 1, 2, 3, 4 }, jsonObject?["vector1"]?.AsArray().GetValues().ToArray()); - Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject?["vector2"]?.AsArray().GetValues().ToArray()); - Assert.False(jsonObject!.ContainsKey("notAnnotated")); + Assert.NotNull(jsonObject); + Assert.Equal("data 1", jsonObject["data1"]?.ToString()); + Assert.Equal("data 2", jsonObject["data2"]?.ToString()); + Assert.Equal(new float[] { 1, 2, 3, 4 }, jsonObject["vector1"]?.AsArray().GetValues().ToArray()); + Assert.Equal(new float[] { 5, 6, 7, 8 }, jsonObject["vector2"]?.AsArray().GetValues().ToArray()); + Assert.False(jsonObject.ContainsKey("notAnnotated")); } [Fact] From 379d2fe56d06712d91967f4499bfe31e0e19e3f5 Mon Sep 17 00:00:00 2001 From: John CSA <103165870+jluocsa@users.noreply.github.com> Date: Mon, 25 May 2026 12:05:05 -0700 Subject: [PATCH 3/3] .Net: deduplicate RedisJsonMapper float/double vector serialization Per Copilot's review on #14030: collapse the two near-identical switch-expression + foreach-Span blocks for float and double vector payloads into a single type-switch that funnels every supported shape (ReadOnlyMemory, Embedding, T[]) through a shared AppendVector(JsonArray, ReadOnlySpan) helper. Behavior is unchanged: same set of accepted vector shapes, same ordering, same UnreachableException on an unsupported runtime type. --- .../src/VectorData/Redis/RedisJsonMapper.cs | 57 ++++++++++--------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/dotnet/src/VectorData/Redis/RedisJsonMapper.cs b/dotnet/src/VectorData/Redis/RedisJsonMapper.cs index b2d3cb0a4ff1..fcf7583d2009 100644 --- a/dotnet/src/VectorData/Redis/RedisJsonMapper.cs +++ b/dotnet/src/VectorData/Redis/RedisJsonMapper.cs @@ -62,35 +62,28 @@ value is null var jsonArray = new JsonArray(); - if (vector switch + switch (vector) { - ReadOnlyMemory m => m, - Embedding e => e.Vector, - float[] a => new ReadOnlyMemory(a), - _ => (ReadOnlyMemory?)null - } is ReadOnlyMemory floatMemory) - { - foreach (var item in floatMemory.Span) - { - jsonArray.Add(JsonValue.Create(item)); - } - } - else if (vector switch - { - ReadOnlyMemory m => m, - Embedding e => e.Vector, - double[] a => new ReadOnlyMemory(a), - _ => null - } is ReadOnlyMemory doubleMemory) - { - foreach (var item in doubleMemory.Span) - { - jsonArray.Add(JsonValue.Create(item)); - } - } - else - { - throw new UnreachableException(); + case ReadOnlyMemory m: + AppendVector(jsonArray, m.Span); + break; + case Embedding e: + AppendVector(jsonArray, e.Vector.Span); + break; + case float[] a: + AppendVector(jsonArray, a.AsSpan()); + break; + case ReadOnlyMemory m: + AppendVector(jsonArray, m.Span); + break; + case Embedding e: + AppendVector(jsonArray, e.Vector.Span); + break; + case double[] a: + AppendVector(jsonArray, a.AsSpan()); + break; + default: + throw new UnreachableException(); } jsonNode[property.StorageName] = jsonArray; @@ -156,4 +149,12 @@ public TConsumerDataModel MapFromStorageToDataModel((string Key, JsonNode Node) return JsonSerializer.Deserialize(jsonObject, jsonSerializerOptions)!; } + + private static void AppendVector(JsonArray jsonArray, ReadOnlySpan span) + { + foreach (var item in span) + { + jsonArray.Add(JsonValue.Create(item)); + } + } }