odata.net icon indicating copy to clipboard operation
odata.net copied to clipboard

Consider adding an `EntityType` property to `IEdmNavigationSource`

Open habbes opened this issue 1 year ago • 1 comments

This issue mentions the perf overhead of the IEdmNavigationSource.EntityType() extension method. I made slight improvements to that method but I think it would be easier if IEdmNavigationSource had an EntityType property that could be easily accessed without casting. Each implementation of IEdmNavigationSource could store the entity type in the property so that it can be accessed without having to recompute it every time (for example calling AsElementType() for entity sets).

Generally, I think we should revisit other EDM interfaces where we are doing recurring computations and casting that could be simplified and made more performant with simpler/unified interfaces with the right properties, e.g. see: https://github.com/OData/odata.net/issues/2528

habbes avatar Dec 01 '23 17:12 habbes

The IEdmNavigationSource interface already supports an IEdmType property named Type.

The Type property is initialized to IEdmCollectionType where the navigation source is an entity set and the EntitySetTypeMustBeCollectionOfEntityType validation rule verifies that.

In the case of a singleton, the Type property is initialized to IEdmEntityType and the SingletonTypeMustBeEntityType validation rule verifies that.

See also AmbiguousEntitySetBinding, AmbiguousSingletonBinding, and BadEntitySet.

The Type property in the context of IEdmNavigationSource represents more than just the entity type that the collection comprises of.

I think introducing another property IEdmEntityType EntityType while it can be derived from the Type property might be a poor design.

Currently, we evaluate the entity type that the collection comprises of (in the case of an entity set) as follows:

<IEdmNavigationSource>.Type.AsElementType() as IEdmEntityType

Where AsElementType extension method is implemented as follows:

public static IEdmType AsElementType(this IEdmType type)
{
    if (type == null)
	{
	    return type;
	}
	
	return (type.TypeKind == EdmTypeKind.Collection) ? (type as IEdmCollectionType).ElementType.Definition : type;
}

We need sufficient justification for adding the EntityType property - in the form of hard cogs assessment data, verification it's executed on hot paths, etc.

gathogojr avatar Mar 05 '24 11:03 gathogojr