OData Client supports nullable generic types for enums

[WanjohiSammy]

Related to OData/odata.net/issues/#2964

The OData client currently supports nullable generic types for enums.

For instance, consider the following Employee class where the EmpType is a generic enum key that can be nullable. This implementation does not result in an exception:

[Key("EmpNumber", "EmpType")]
public class Employee
{
    public int EmpNumber { get; set; }

    public EmployeeType? EmpType { get; set; }
}

public enum EmployeeType
{
    Hybrid = 1,
    FullTime = 2,
    PartTime = 3
}

However, based on this spec #secKey, this should throw an exception because:

Key properties MUST NOT be nullable

Assemblies affected

Microsoft.OData.Client 6.x

Reproduce steps

1. Clone OData repo:

git clone https://github.com/OData/odata.net

2. Open the project using either Visual Studio IDE

3. Navigate to test/FunctionalTests/Microsoft.OData.Client.Tests/Metadata/ClientTypeUtilTests.cs within the Microsoft.OData.Client.Tests project.

4. Execute the tests named:

void IFTypeProperty_HasMultipleKeyAttributes_GetKeyPropertiesOnType_DoesNotThrowException() {}
void IFTypeProperty_HasEnumTypeKeyAttribute_GetKeyPropertiesOnType_DoesNotThrowException() {}
void IFTypeProperty_HasNullableGenericTypeKeyAttribute_OfTypeEnum_GetKeyPropertiesOnType_DoesNotThrowException() {}

Expected result

An InvalidOperation exception with the following Error message should be thrown.

The key property '{0}' on for type '{1}' is of type '{2}' is nullable. Key properties MUST NOT be nullable.

Actual result

No error is thrown, and the tests passed.

Additional detail

In src/Microsoft.OData.Client/Metadata/ODataTypeInfo.cs GetKeyProperties(), check if the property type is a nullable generic type first and then throw the expected exception.

Here is the suggested change:

if (key.PropertyType == typeof(System.Nullable<>) || (key.PropertyType.IsGenericType() && key.PropertyType.GetGenericTypeDefinition() == typeof(System.Nullable<>)))
{
    throw c.Error.InvalidOperation(c.Strings.ClientType_KeysMustNotBeNullable(key.Name, typeName, key.PropertyType.FullName));
}

if (!PrimitiveType.IsKnownType(key.PropertyType) && !key.PropertyType.IsEnum())
{
    throw c.Error.InvalidOperation(c.Strings.ClientType_KeysMustBeSimpleTypes(key.Name, typeName, key.PropertyType.FullName));
}

[corranrogue9]

Great catch! I don't think this would be a source or binary breaking change, so I don't think we have to wait for a major release to do this if we introduce a behavior flag.