Resolve PR feedback

sdk/search/Azure.Search.Documents/src/Indexes/FieldBuilder.cs

@@ -226,7 +226,7 @@ static bool IsNullableType(Type type) =>
             {
                 return DataTypeInfo.Simple(searchFieldDataType);
             }
-            else if (SpatialProxyFactory.CanCreate(propertyType))
+            else if (SpatialProxyFactory.IsSupportedPoint(propertyType))
             {
                 return DataTypeInfo.Simple(SearchFieldDataType.GeographyPoint);
             }
@@ -370,7 +370,7 @@ public static bool TryGet(Type type, IMemberNameConverter nameProvider, out Obje
                     !type.IsEnum &&
                     !s_unsupportedTypes.Contains(type) &&
                     !s_primitiveTypeMap.ContainsKey(type) &&
-                    !SpatialProxyFactory.CanCreate(type) &&
+                    !SpatialProxyFactory.IsSupportedPoint(type) &&
                     !typeof(IEnumerable).IsAssignableFrom(type))
                 {
                     const BindingFlags bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic;

sdk/search/Azure.Search.Documents/src/Spatial/SpatialProxyFactory.cs

@@ -13,15 +13,16 @@ namespace Azure.Search.Documents
     internal static class SpatialProxyFactory
     {
         private const string Name = "Microsoft.Spatial";
+        private const string GeographyPointTypeName = "Microsoft.Spatial.GeographyPoint";
 
         // TODO: When GeoJson is built into Azure.Core, change these proxies to adapter for GeoJson types.
         // https://github.com/Azure/azure-sdk-for-net/issues/13319
 
         private static readonly IReadOnlyDictionary<string, Func<object, GeographyProxy>> s_types =
-            new Dictionary<string, Func<object, GeographyProxy>>(StringComparer.OrdinalIgnoreCase)
+            new Dictionary<string, Func<object, GeographyProxy>>(StringComparer.Ordinal)
             {
+                [GeographyPointTypeName] = value => new GeographyPointProxy(value),
                 ["Microsoft.Spatial.GeographyLineString"] = value => new GeographyLineStringProxy(value),
-                ["Microsoft.Spatial.GeographyPoint"] = value => new GeographyPointProxy(value),
                 ["Microsoft.Spatial.GeographyPolygon"] = value => new GeographyPolygonProxy(value),
             };
 
@@ -32,44 +33,22 @@ internal static class SpatialProxyFactory
         /// <returns>A value indicating whether <paramref name="type"/> can be created by this factory.</returns>
         public static bool CanCreate(Type type)
         {
-            if (type is null)
+            if (type == null)
             {
                 return false;
             }
 
             return TryGetFactory(type, out _);
         }
 
-        /// <summary>
-        /// Creates a <see cref="GeographyProxy"/> from the given <paramref name="value"/> if supported.
-        /// <seealso cref="CanCreate(Type)"/>
-        /// </summary>
-        /// <param name="value">The value to proxy.</param>
-        /// <returns>A <see cref="GeographyProxy"/> from the given <paramref name="value"/> if supported, or null if <paramref name="value"/> is null.</returns>
-        /// <exception cref="NotSupportedException">The <paramref name="value"/> type is not supported.</exception>
-        public static GeographyProxy Create(object value)
-        {
-            if (value is null)
-            {
-                return null;
-            }
-
-            if (TryCreate(value, out GeographyProxy proxy))
-            {
-                return proxy;
-            }
-
-            throw new NotSupportedException($"Type {value.GetType()} is not supported");
-        }
-
         /// <summary>
         /// Attempts to creates a <see cref="GeographyProxy"/> from the given <paramref name="value"/> if supported.
         /// <seealso cref="CanCreate(Type)"/>
         /// </summary>
         /// <param name="value">The value to proxy.</param>
         /// <param name="proxy">The proxied value if supported.</param>
         /// <returns>True if the <paramref name="value"/> could be proxied; otherwise, false.</returns>
-        internal static bool TryCreate(object value, out GeographyProxy proxy)
+        public static bool TryCreate(object value, out GeographyProxy proxy)
         {
             if (value is { })
             {
@@ -85,10 +64,23 @@ internal static bool TryCreate(object value, out GeographyProxy proxy)
             return false;
         }
 
-        private static bool TryGetFactory(Type type, out Func<object, GeographyProxy> factory)
+        /// <summary>
+        /// Gets a value indicating whether the given <paramref name="type"/> represents a supported spatial point.
+        /// </summary>
+        /// <param name="type">The type to check.</param>
+        /// <returns>A value indicating whether the given <paramref name="type"/> represents a supported spatial point.</returns>
+        public static bool IsSupportedPoint(Type type) =>
+            type != null && IsSupportedAssembly(type) && GeographyPointTypeName.Equals(type.FullName, StringComparison.Ordinal);
+
+        private static bool IsSupportedAssembly(Type type)
         {
             AssemblyName assemblyName = type.Assembly.GetName();
-            if (string.Equals(assemblyName.Name, Name, StringComparison.OrdinalIgnoreCase))
+            return string.Equals(assemblyName.Name, Name, StringComparison.OrdinalIgnoreCase);
+        }
+
+        private static bool TryGetFactory(Type type, out Func<object, GeographyProxy> factory)
+        {
+            if (IsSupportedAssembly(type))
             {
                 while (type != typeof(object))
                 {

sdk/search/Azure.Search.Documents/tests/FieldBuilderTests.cs

@@ -7,6 +7,7 @@
 using System.Text.Json.Serialization;
 using Azure.Search.Documents.Indexes;
 using Azure.Search.Documents.Indexes.Models;
+using Microsoft.Spatial;
 using NUnit.Framework;
 using KeyFieldAttribute = System.ComponentModel.DataAnnotations.KeyAttribute;
 
@@ -447,6 +448,29 @@ public void FieldBuilderFailsWithHelpfulErrorMessageOnUnsupportedTypes(Type mode
             Assert.AreEqual(expectedErrorMessage, e.Message);
         }
 
+        [Test]
+        public void SupportsSpecificSpatialTypes()
+        {
+            IList<SearchField> fields = new FieldBuilder().Build(typeof(ModelWithSpatialProperties));
+            foreach (SearchField field in fields)
+            {
+                switch (field.Name)
+                {
+                    case nameof(ModelWithSpatialProperties.ID):
+                        Assert.AreEqual(SearchFieldDataType.String, field.Type);
+                        break;
+
+                    case nameof(ModelWithSpatialProperties.GeographyPoint):
+                        Assert.AreEqual(SearchFieldDataType.GeographyPoint, field.Type);
+                        break;
+
+                    default:
+                        Assert.AreEqual(SearchFieldDataType.Complex, field.Type, $"Unexpected type for field '{field.Name}'");
+                        break;
+                }
+            }
+        }
+
         private static IEnumerable<(Type, SearchFieldDataType, string)> CombineTestData(
             IEnumerable<Type> modelTypes,
             IEnumerable<(SearchFieldDataType dataType, string fieldName)> testData) =>
@@ -597,5 +621,23 @@ private class ModelWithIgnoredProperties
 
             public InnerModelWithIgnoredProperties[] Inner { get; set; }
         }
+
+        private class ModelWithSpatialProperties
+        {
+            [SimpleField(IsKey = true)]
+            public string ID { get; set; }
+
+            public GeographyPoint GeographyPoint { get; set; }
+
+            public GeographyLineString GeographyLineString { get; set; }
+
+            public GeographyPolygon GeographyPolygon { get; set; }
+
+            public GeometryPoint GeometryPoint { get; set; }
+
+            public GeometryLineString GeometryLineString { get; set; }
+
+            public GeometryPolygon GeometryPolygon { get; set; }
+        }
     }
 }

sdk/search/Azure.Search.Documents/tests/Spatial/SpatialProxyFactoryTests.cs

@@ -25,8 +25,11 @@ public void CanCreate(Type type, bool expected) =>
             Assert.AreEqual(expected, SpatialProxyFactory.CanCreate(type));
 
         [Test]
-        public void CreateNull() =>
-            Assert.IsNull(SpatialProxyFactory.Create(null));
+        public void CreateNull()
+        {
+            Assert.IsFalse(SpatialProxyFactory.TryCreate(null, out GeographyProxy proxy));
+            Assert.IsNull(proxy);
+        }
 
         [Test]
         public void CreateGeographyPoint()
@@ -89,11 +92,28 @@ public void CreateGeographyLineString()
             }
         }
 
-        [TestCaseSource(nameof(CreateThrowsData))]
-        public void CreateThrows(object value) =>
-            Assert.Throws<NotSupportedException>(() => SpatialProxyFactory.Create(value));
+        [TestCaseSource(nameof(CreateFailsData))]
+        public void CreateFails(object value)
+        {
+            Assert.IsFalse(SpatialProxyFactory.TryCreate(value, out GeographyProxy proxy));
+            Assert.IsNull(proxy);
+        }
+
+        [TestCase(null, false)]
+        [TestCase(typeof(object), false)]
+        [TestCase(typeof(int), false)]
+        [TestCase(typeof(GeographyPoint), true)]
+        [TestCase(typeof(GeometryPoint), false)]
+        [TestCase(typeof(GeographyPosition), false)]
+        [TestCase(typeof(GeometryPosition), false)]
+        [TestCase(typeof(GeographyPolygon), false)]
+        [TestCase(typeof(GeometryPolygon), false)]
+        [TestCase(typeof(GeographyLineString), false)]
+        [TestCase(typeof(GeometryLineString), false)]
+        public void IsSupportedPoint(Type type, bool expected) =>
+            Assert.AreEqual(expected, SpatialProxyFactory.IsSupportedPoint(type));
 
-        private static IEnumerable CreateThrowsData => new[]
+        private static IEnumerable CreateFailsData => new[]
         {
             new TestCaseData(new object()),
             new TestCaseData(1),