Wrong TypeKind values for Types of Terms

chrisspre commented 1 year ago

When reading official Capability annotation model, the types of the declared terms empty TypeKind

Assemblies affected

"Microsoft.OData.Core" Version="7.12.5"

Reproduce steps


var settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Ignore };
using var reader = XmlReader.Create("https://raw.githubusercontent.com/oasis-tcs/odata-vocabularies/main/vocabularies/Org.OData.Capabilities.V1.xml", settings);
if (!CsdlReader.TryParse(reader, out var model, out var errors))
{
    Console.WriteLine(string. Join("\r\n", errors));
    Environment.Exit(1);
}

IEdmType x = model.FindDeclaredType("Org.OData.Capabilities.V1.UpdateRestrictionsType");
Console.WriteLine("via FindDeclaredType: {0} {1}", x.FullTypeName(), x.TypeKind);
Console.WriteLine();

foreach (var term in model.SchemaElements.OfType<IEdmTerm>())
{
    var y = term.Type.Definition;
    if (y.FullTypeName().Contains("Update"))
    {
        Console.WriteLine("via IEdmTerm.Type.Definition: {0} {1}", y.FullTypeName(), y.TypeKind);
    }
}

Expected result

The output should show that the TypeKind of the types of the annotation terms are complex types.

via FindDeclaredType: Org.OData.Capabilities.V1.UpdateRestrictionsType Complex

via IEdmTerm.Type.Definition: Org.OData.Capabilities.V1.UpdateRestrictionsType Complex
via IEdmTerm.Type.Definition: Org.OData.Capabilities.V1.DeepUpdateSupportType Complex

Actual result

Output is the following, showing that the TypeKind for the type of a Term is None which is different from the TypeKind returned when finding the type directly in the model

via FindDeclaredType: Org.OData.Capabilities.V1.UpdateRestrictionsType Complex

via IEdmTerm.Type.Definition: Org.OData.Capabilities.V1.UpdateRestrictionsType None
via IEdmTerm.Type.Definition: Org.OData.Capabilities.V1.DeepUpdateSupportType None

gathogojr commented 1 year ago

Hi @chrisspre. I think the code below will yield what you were expecting:

foreach (var schemaType in model.SchemaElements.OfType<IEdmSchemaType>())
{
    x = schemaType.AsElementType();
    if (x.FullTypeName().Contains("Update"))
    {
        Console.WriteLine("{0} {1}", x.FullTypeName(), x.TypeKind);
    }
}

Here's how we derive the schema types and the terms https://github.com/OData/odata.net/blob/370cd96aa2a78aecff1b7f9ebe9bebaa7c25ab81/src/Microsoft.OData.Edm/Csdl/Semantics/CsdlSemanticsModel.cs#L132

chrisspre commented 1 year ago

@gathogojr
This does unfortunately, as far as I understand, not address the bug. The property (the one accessed through ITerm.Type.Definition ) still returns the wrong object. It is good to have a workaround but neither is it discoverable not does it help with the buggy behavior.

gathogojr commented 1 year ago

@chrisspre Turns out that there's no bug here, at least not in the manner described. The TryParse method of CsdlReader that is being called here implicitly loads all built-in vocabularies - including Org.Data.Capabilities.V1. Then we again explicitly load the Org.Data.Capabilities.V1 vocabulary. The consequence of that is that each element gets registered twice. Which is why term.Type.Definition resolves into Microsoft.OData.Edm.AmbiguousTypeBinding instead of the expected Org.OData.Capabilities.V1.UpdateRestrictionsType complex type:

If you however call the overload of TryParse that accepts includeDefaultVocabularies parameter and pass false as the value, everything works as expected:

if (!CsdlReader.TryParse(
    reader:  reader,
    references: Enumerable.Empty<IEdmModel>(),
    includeDefaultVocabularies: false,
    model: out var model,
    errors: out var errors))
{
    Console.WriteLine(string.Join("\r\n", errors));
    Environment.Exit(1);
}

The only question then is whether we should add logic to ensure that a default vocabulary that was already been loaded implicitly isn't loaded again explicitly resulting into the above scenario - and what priority to attach to that work. What are your thoughts?

OData / odata.net