From d7cfd929b2fd4e3065d81d1a728dc8c028fc6204 Mon Sep 17 00:00:00 2001 From: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com> Date: Mon, 14 Apr 2025 08:56:41 -0700 Subject: [PATCH] Updated Nullable handling in VectorStoreRecordModelBuilder --- dotnet/SK-dotnet.sln | 9 ++ .../VectorStoreRecordModelBuilder.cs | 21 ++-- .../VectorData.UnitTests/.editorconfig | 6 ++ .../VectorStoreRecordModelBuilderTests.cs | 99 +++++++++++++++++++ .../VectorData.UnitTests.csproj | 34 +++++++ 5 files changed, 163 insertions(+), 6 deletions(-) create mode 100644 dotnet/src/Connectors/VectorData.UnitTests/.editorconfig create mode 100644 dotnet/src/Connectors/VectorData.UnitTests/ConnectorSupport/VectorStoreRecordModelBuilderTests.cs create mode 100644 dotnet/src/Connectors/VectorData.UnitTests/VectorData.UnitTests.csproj diff --git a/dotnet/SK-dotnet.sln b/dotnet/SK-dotnet.sln index c12066190a47..73d01abb4ae3 100644 --- a/dotnet/SK-dotnet.sln +++ b/dotnet/SK-dotnet.sln @@ -532,6 +532,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ProcessWithCloudEvents.Proc EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ProcessWithCloudEvents.Grpc", "samples\Demos\ProcessWithCloudEvents\ProcessWithCloudEvents.Grpc\ProcessWithCloudEvents.Grpc.csproj", "{08D84994-794A-760F-95FD-4EFA8998A16D}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "VectorData.UnitTests", "src\Connectors\VectorData.UnitTests\VectorData.UnitTests.csproj", "{FB55A80D-BD5C-4252-8E66-59BB1712E621}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -1451,6 +1453,12 @@ Global {08D84994-794A-760F-95FD-4EFA8998A16D}.Publish|Any CPU.Build.0 = Release|Any CPU {08D84994-794A-760F-95FD-4EFA8998A16D}.Release|Any CPU.ActiveCfg = Release|Any CPU {08D84994-794A-760F-95FD-4EFA8998A16D}.Release|Any CPU.Build.0 = Release|Any CPU + {FB55A80D-BD5C-4252-8E66-59BB1712E621}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {FB55A80D-BD5C-4252-8E66-59BB1712E621}.Debug|Any CPU.Build.0 = Debug|Any CPU + {FB55A80D-BD5C-4252-8E66-59BB1712E621}.Publish|Any CPU.ActiveCfg = Publish|Any CPU + {FB55A80D-BD5C-4252-8E66-59BB1712E621}.Publish|Any CPU.Build.0 = Publish|Any CPU + {FB55A80D-BD5C-4252-8E66-59BB1712E621}.Release|Any CPU.ActiveCfg = Release|Any CPU + {FB55A80D-BD5C-4252-8E66-59BB1712E621}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1648,6 +1656,7 @@ Global {7C092DD9-9985-4D18-A817-15317D984149} = {5D4C0700-BBB5-418F-A7B2-F392B9A18263} {31F6608A-FD36-F529-A5FC-C954A0B5E29E} = {7C092DD9-9985-4D18-A817-15317D984149} {08D84994-794A-760F-95FD-4EFA8998A16D} = {7C092DD9-9985-4D18-A817-15317D984149} + {FB55A80D-BD5C-4252-8E66-59BB1712E621} = {5A7028A7-4DDF-4E4F-84A9-37CE8F8D7E89} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {FBDC56A3-86AD-4323-AA0F-201E59123B83} diff --git a/dotnet/src/Connectors/VectorData.Abstractions/ConnectorSupport/VectorStoreRecordModelBuilder.cs b/dotnet/src/Connectors/VectorData.Abstractions/ConnectorSupport/VectorStoreRecordModelBuilder.cs index 1cac1ace9576..4337d1b69165 100644 --- a/dotnet/src/Connectors/VectorData.Abstractions/ConnectorSupport/VectorStoreRecordModelBuilder.cs +++ b/dotnet/src/Connectors/VectorData.Abstractions/ConnectorSupport/VectorStoreRecordModelBuilder.cs @@ -357,10 +357,7 @@ protected virtual void ValidateProperty(VectorStoreRecordPropertyModel propertyM { var type = propertyModel.Type; - if (type.IsGenericType && Nullable.GetUnderlyingType(type) is Type underlyingType) - { - type = underlyingType; - } + type = GetNullableUnderlyingTypeOrDefault(type); switch (propertyModel) { @@ -396,10 +393,20 @@ protected virtual void ValidateProperty(VectorStoreRecordPropertyModel propertyM } } + private static Type GetNullableUnderlyingTypeOrDefault(Type type) + { + return type.IsGenericType && Nullable.GetUnderlyingType(type) is Type underlyingType ? underlyingType : type; + } + + private static Type GetNullableTypeOrDefault(Type type) + { + return type.IsValueType && !type.IsGenericTypeDefinition ? typeof(Nullable<>).MakeGenericType(type) : type; + } + private static void ValidatePropertyType(string propertyName, Type propertyType, string propertyCategoryDescription, HashSet supportedTypes, HashSet? supportedEnumerableElementTypes = null) { // Add shortcut before testing all the more expensive scenarios. - if (supportedTypes.Contains(propertyType)) + if (supportedTypes.Contains(propertyType) || supportedTypes.Contains(GetNullableTypeOrDefault(propertyType))) { return; } @@ -409,7 +416,9 @@ private static void ValidatePropertyType(string propertyName, Type propertyType, { var typeToCheck = GetCollectionElementType(propertyType); - if (!supportedEnumerableElementTypes.Contains(typeToCheck)) + typeToCheck = GetNullableUnderlyingTypeOrDefault(typeToCheck); + + if (!supportedEnumerableElementTypes.Contains(typeToCheck) && !supportedEnumerableElementTypes.Contains(GetNullableTypeOrDefault(typeToCheck))) { var supportedEnumerableElementTypesString = string.Join(", ", supportedEnumerableElementTypes!.Select(t => t.FullName)); throw new NotSupportedException($"Enumerable {propertyCategoryDescription} properties must have one of the supported element types: {supportedEnumerableElementTypesString}. Element type of the property '{propertyName}' is {typeToCheck.FullName}."); diff --git a/dotnet/src/Connectors/VectorData.UnitTests/.editorconfig b/dotnet/src/Connectors/VectorData.UnitTests/.editorconfig new file mode 100644 index 000000000000..394eef685f21 --- /dev/null +++ b/dotnet/src/Connectors/VectorData.UnitTests/.editorconfig @@ -0,0 +1,6 @@ +# Suppressing errors for Test projects under dotnet folder +[*.cs] +dotnet_diagnostic.CA2007.severity = none # Do not directly await a Task +dotnet_diagnostic.VSTHRD111.severity = none # Use .ConfigureAwait(bool) is hidden by default, set to none to prevent IDE from changing on autosave +dotnet_diagnostic.CS1591.severity = none # Missing XML comment for publicly visible type or member +dotnet_diagnostic.IDE1006.severity = warning # Naming rule violations diff --git a/dotnet/src/Connectors/VectorData.UnitTests/ConnectorSupport/VectorStoreRecordModelBuilderTests.cs b/dotnet/src/Connectors/VectorData.UnitTests/ConnectorSupport/VectorStoreRecordModelBuilderTests.cs new file mode 100644 index 000000000000..1479723122ad --- /dev/null +++ b/dotnet/src/Connectors/VectorData.UnitTests/ConnectorSupport/VectorStoreRecordModelBuilderTests.cs @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using Microsoft.Extensions.VectorData; +using Microsoft.Extensions.VectorData.ConnectorSupport; +using Xunit; + +namespace VectorData.UnitTests; + +public class VectorStoreRecordModelBuilderTests +{ + [Theory] + [InlineData(typeof(bool), typeof(RecordWithSimpleDataProperties))] + [InlineData(typeof(bool?), typeof(RecordWithSimpleDataProperties))] + [InlineData(typeof(bool), typeof(RecordWithNullableDataProperties))] + [InlineData(typeof(bool?), typeof(RecordWithNullableDataProperties))] + [InlineData(typeof(bool), typeof(RecordWithEnumerableDataProperties))] + [InlineData(typeof(bool?), typeof(RecordWithEnumerableDataProperties))] + [InlineData(typeof(bool), typeof(RecordWithEnumerableNullableDataProperties))] + [InlineData(typeof(bool?), typeof(RecordWithEnumerableNullableDataProperties))] + public void BuildWithDifferentDataPropertyTypesReturnsVectorStoreRecordModel(Type dataPropertyType, Type recordType) + { + // Arrange + var options = new VectorStoreRecordModelBuildingOptions() + { + SupportsMultipleKeys = false, + SupportsMultipleVectors = true, + RequiresAtLeastOneVector = false, + + SupportedKeyPropertyTypes = [typeof(string)], + SupportedDataPropertyTypes = [dataPropertyType], + SupportedEnumerableDataPropertyElementTypes = [dataPropertyType], + SupportedVectorPropertyTypes = [typeof(ReadOnlyMemory)] + }; + + var builder = new VectorStoreRecordModelBuilder(options); + + // Act + var model = builder.Build(recordType, vectorStoreRecordDefinition: null); + + // Assert + Assert.NotNull(model); + } + + #region private + +#pragma warning disable CA1812 + private sealed class RecordWithSimpleDataProperties + { + [VectorStoreRecordKey] + public string Key { get; set; } + + [VectorStoreRecordData] + public bool Flag { get; set; } + + [VectorStoreRecordVector(Dimensions: 4)] + public ReadOnlyMemory Embedding { get; set; } + } + + private sealed class RecordWithNullableDataProperties + { + [VectorStoreRecordKey] + public string Key { get; set; } + + [VectorStoreRecordData] + public bool? Flag { get; set; } + + [VectorStoreRecordVector(Dimensions: 4)] + public ReadOnlyMemory Embedding { get; set; } + } + + private sealed class RecordWithEnumerableDataProperties + { + [VectorStoreRecordKey] + public string Key { get; set; } + + [VectorStoreRecordData] + public List Flags { get; set; } + + [VectorStoreRecordVector(Dimensions: 4)] + public ReadOnlyMemory Embedding { get; set; } + } + + private sealed class RecordWithEnumerableNullableDataProperties + { + [VectorStoreRecordKey] + public string Key { get; set; } + + [VectorStoreRecordData] + public List Flags { get; set; } + + [VectorStoreRecordVector(Dimensions: 4)] + public ReadOnlyMemory Embedding { get; set; } + } +#pragma warning restore CA1812 + + #endregion +} diff --git a/dotnet/src/Connectors/VectorData.UnitTests/VectorData.UnitTests.csproj b/dotnet/src/Connectors/VectorData.UnitTests/VectorData.UnitTests.csproj new file mode 100644 index 000000000000..0aac8e22c559 --- /dev/null +++ b/dotnet/src/Connectors/VectorData.UnitTests/VectorData.UnitTests.csproj @@ -0,0 +1,34 @@ + + + + VectorData.UnitTests + VectorData.UnitTests + net8.0 + true + enable + disable + false + $(NoWarn);VSTHRD111,CA2007,CS8618 + $(NoWarn);MEVD9001 + + + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + + \ No newline at end of file