[BUG] Azure Search FieldBuilder does not handle enum conversion exception gracefully

[mpalumbo7]

Describe the bug
The FieldBuilder class does not support building a Field object from a class with an Enum property. The exception is obscure and lost me a lot of time

Exception or Stack Trace

An exception of type 'System.InvalidCastException' occurred in XXXXX.dll 
but was not handled in user code: 'Unable to cast object of type 

'Newtonsoft.Json.Serialization.JsonPrimitiveContract'
to
type 'Newtonsoft.Json.Serialization.JsonObjectContract'.'

To Reproduce
Have class with an Enum property, attempt to use FieldBuilder on it.

using Microsoft.Azure.Search;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;

namespace Search.Test
{
    public class FieldBuilderTest
    {
        enum TestType
        {
            Person, Place, Thing
        }
        class Test
        {
            [JsonConverter(typeof(StringEnumConverter))]
            public TestType Type { get; set; }
        }

        public void GenerateFields()
        {
            var fields = FieldBuilder.BuildForType<Test>();
        }
    }
}

Code Snippet
The FieldBuilder internally has a dictionary it uses to interpret types into Azure Search supported types.

private static readonly IReadOnlyDictionary<Type, DataType> PrimitiveTypeMap =
            new ReadOnlyDictionary<Type, DataType>(
                new Dictionary<Type, DataType>()
                {
                    [typeof(string)] = DataType.String,
                    [typeof(int)] = DataType.Int32,
                    [typeof(long)] = DataType.Int64,
                    [typeof(double)] = DataType.Double,
                    [typeof(bool)] = DataType.Boolean,
                    [typeof(DateTime)] = DataType.DateTimeOffset,
                    [typeof(DateTimeOffset)] = DataType.DateTimeOffset,
                    [typeof(GeographyPoint)] = DataType.GeographyPoint
                });

If a property does not match one of these types, it assumes it is a complex type and recursively tries to interpret the property (an Enum) as a complex type. See the GetDataTypeInfo method at the else condition.

private static DataTypeInfo GetDataTypeInfo(Type propertyType)
        {
            bool IsNullableType(Type type) =>
                type.IsConstructedGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>);

            if (PrimitiveTypeMap.TryGetValue(propertyType, out DataType dataType))
            {
                return new DataTypeInfo(dataType, propertyType);
            }
            else if (IsNullableType(propertyType))
            {
                return GetDataTypeInfo(propertyType.GenericTypeArguments[0]);
            }
            else if (TryGetEnumerableElementType(propertyType, out Type elementType))
            {
                DataTypeInfo elementTypeInfo = GetDataTypeInfo(elementType);
                return new DataTypeInfo(DataType.Collection(elementTypeInfo.DataType), elementTypeInfo.UnderlyingClrType);
            }
            else
            {
                return new DataTypeInfo(DataType.Complex, propertyType);
            }
        }

Further, if you have a custom JsonConverter to translate your nested class into a primitive type (i.e. convert an Address object into a simple string) the FieldBuilder will incorrectly interpret this property as a complex type - which will not match the serialized JSON (a string) that the SearchIndexClient will see.

Expected behavior
The FieldBuilder recognizes an enum, and does not try to interpret it as complex type. It should throw an enum not supported exception or something.

Even better, there could be a TypeAttribute that FieldBuilder uses in the same way it uses IsSearchable or other attributes. Or, it could accept a custom dictionary that is used to interpret custom data types to primitive types.

Setup (please complete the following information):

• OS: MacOS w/ .NET Core 2.2
• Lib: Azure Search v9.0.1

Additional context
Add any other context about the problem here.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

• Bug Description Added
• Repro Steps Added
• Setup information Added

[mpalumbo7]

I'll open a PR with the custom type map dictionary if I can get it to work...

[brjohnstmsft]

@mpalumbo7 Thanks for reporting this.

It seems like having FieldBuilder take a custom dictionary would expand its surface area too much and make it harder to reason about without understanding the implementation. We should consider other ways to support enum properties that keep the interface to FieldBuilder simple. When I have some time, I'll go back and take a look at existing patterns for how enums were supported before complex types; It's been a while since I've looked at that and until I get back up to speed I don't think I'm in a position to comment on your other proposed fixes.

[jupacaza]

My team sees repro of this issue.

[mpalumbo7]

@brjohnstmsft I tested the custom dictionary and then abandoned it. It worked, but one of my nested classes has the key annotation on it for a different index, which FieldBuilder interprets as duplicate keys.

Maybe FieldBuilder just isn’t going to work for me. I’ll just have to accept explicitly defining my fields and then trying to keep them in sync with my class JSON attributes.

Anyways, a check for an enum property should still be added with a detailed exception.

[brjohnstmsft]

@mpalumbo7 Thanks for poking at this problem. I'm optimistic that FieldBuilder can be made flexible enough to deal with these situations. I'll see what I can come up with.

[Rene-Sackers]

Running into this problem too, additionally, the same error occurs if you have a field of type decimal, for example:

[IsFilterable]
[IsSortable]
[IsFacetable]
public decimal ExampleField { get; set; }

Same error:

Unable to cast object of type 'Newtonsoft.Json.Serialization.JsonPrimitiveContract' to type 'Newtonsoft.Json.Serialization.JsonObjectContract'.

Edit: Also for collections, for example ICollection<string> should become DataType.Collection(DataType.String). I'm trying to get some Unit Tests set up for this on a fork, but having issues setting up the solution.

[brjohnstmsft]

@Rene-Sackers For the decimal case, what's the ideal behavior you would expect, given that Azure Search doesn't support Edm.Decimal? Are you using a custom JsonCnverter that would convert between the decimal property and one of the supported numeric types during indexing and retrieval?

Also, what sort of issues are you having setting up the solution? Maybe I can help.

[brjohnstmsft]

@Rene-Sackers I'm adding test coverage for ICollection to the FieldBuilder tests, and so far I'm unable to reproduce any problems. Can you describe the issue you encountered in more detail?

[brjohnstmsft]

Please see the description of this PR for details on where we landed in addressing each of the issues mentioned in this thread: #6833

I'll close this issue once the new version of the SDK is released. There are some other fixes we're working on, so the ETA is at least a few weeks.

[arv100kri]

We have a new nuget package released (v 9.1.0) that should contain the fixes from the aforementioned PR: https://www.nuget.org/packages/Microsoft.Azure.Search/