Interface for navigation source segments

[lisicase]

Consider creating an INavigationSourceSegment and have EntitySetSegment, NavigationPropertySegment, and SingletonSegment implement it so that there is a consistent way to retrieve the "navigation source" from a segment. (Per this suggestion.)

[gathogojr]

Hi @corranrogue9. This issue was created following your review comment here https://github.com/OData/AspNetCoreOData/pull/710#discussion_r986274966

I'd like to know if what you had in mind is

1. an interface containing properties that are common to EntitySetSegment and SingletonSegment and NavigationPropertySegment, or
2. a marker interface (e.g. INavigationSourceSegment) that EntitySetSegment and SingletonSegment and NavigationPropertySegment implement.

In respect to option (1), what would the common properties across EntitySetSegment and SingletonSegment and NavigationPropertySegment be? The 3 types implement ODataPathSegment that contains, a public IEdmType EdmType property, an internal IEdmNavigationSource TargetEdmNavigationSource property, and an internal IEdmType TargetEdmType property. The TargetEdmNavigationSource property is initialized to,

• an IEdmEntitySet argument from the EntitySetSegment constructor,
• an IEdmSingleton argument from the SingletonSegment constructor, and
• an IEdmNavigationSource argument from the NavigationPropertySegment constructor.

Would making TargetEdmNavigationSource property public serve the purpose you had in mind?

This - from LinkGenerationHelpers class:

IEdmNavigationSource currentNavigationSource = null;

var entitySetPathSegment = pathSegment as EntitySetSegment;
if (entitySetPathSegment != null)
{
    currentNavigationSource = entitySetPathSegment.EntitySet;
}

var navigationPathSegment = pathSegment as NavigationPropertySegment;
if (navigationPathSegment != null)
{
     currentNavigationSource = navigationPathSegment.NavigationSource;
 }

var singletonPathSegment = pathSegment as SingletonSegment;
if (singletonPathSegment != null)
{
    currentNavigationSource = singletonPathSegment.Singleton;
}

would become:

IEdmNavigationSource currentNavigationSource = null;

var navigationSourceSegment = pathSegment as INavigationSourceSegment;
if (pathSegment is EntitySetSegment || pathSegment is SingletonSegment || pathSegment is NavigationPropertySegment) 
{
    currentNavigationSource = pathSegment.TargetEdmNavigationSource;
}

Alternatively, given that,

• EntitySetSegment has an IEdmEntitySet EntitySet property that returns the navigation source,
• SingletonSegment has an IEdmSingleton Singleton property that returns the navigation source, and
• NavigationPropertySegment has an IEdmNavigationSource NavigationSource property that returns the navigation source.

Would it be desirable to have an IEdmNavigationSource NavigationSource in the new interface such that all 3 types implement that property to return the navigation source? Would that serve the objective you had in mind?

This:

IEdmNavigationSource currentNavigationSource = null;

var entitySetPathSegment = pathSegment as EntitySetSegment;
if (entitySetPathSegment != null)
{
    currentNavigationSource = entitySetPathSegment.EntitySet;
}

var navigationPathSegment = pathSegment as NavigationPropertySegment;
if (navigationPathSegment != null)
{
     currentNavigationSource = navigationPathSegment.NavigationSource;
 }

var singletonPathSegment = pathSegment as SingletonSegment;
if (singletonPathSegment != null)
{
    currentNavigationSource = singletonPathSegment.Singleton;
}

would become:

IEdmNavigationSource currentNavigationSource = null;

var navigationSourceSegment = pathSegment as INavigationSourceSegment;
if (navigationSourceSegment != null)
{
    currentNavigationSource = navigationSourceSegment.NavigationSource;
}

In respect to option (2), what purpose would a marker interface serve, for example in the context of the logic in the LinkGenerationHelpers class in ASP.NET Core OData.

Other potential areas where such an interface could have some use are: https://github.com/OData/AspNetCoreOData/blob/8d9a955be12a3846374b3693e1df7fdfc20293da/src/Microsoft.AspNetCore.OData/Routing/ODataPathExtensions.cs#L73 https://github.com/OData/AspNetCoreOData/blob/8d9a955be12a3846374b3693e1df7fdfc20293da/src/Microsoft.AspNetCore.OData/Edm/BindingPathHelper.cs#L65

Kindly share your thoughts on this. Altogether, let's consider whether this change is necessary and justifiable. cc. @habbes

[corranrogue9]

I'm thinking of having an interface like this:

public interface INavigationSourceSegment
{
	IEdmNavigationSource NavigationSource { get; }
}

and updating the 3 segment classes like this:

public sealed class EntitySetSegment : ODataPathSegment, INavigationSourceSegment
{
	...
	public IEdmNavigationSource NavigationSource => this.EntitySet;
	...
}

public sealed class NavigationPropertySegment : ODataPathSegment, INavigationSourceSegment
{
	// no changes needed because the property is already called NavigationSource
}

public sealed class SingletonSegment : ODataPathSegment, INavigationSourceSegment
{
	...
	public IEdmNavigationSource NavigationSource => this.Singleton;
	...
}

so that the LinkGenerationHelpers can be updated to do this:

private static void GenerateBaseODataPathSegments(...)
{
	...
	if (pathSegment is INavigationSourceSegment navigationSourceSegment)
	{
		currentNavigationSource = navigationSourceSegment.NavigationSource;
	}
	...
}

The idea being that we reduce the existing 3 casts down to 1, and we don't need to introduce any additional casts if we ever need to add this functionality to more segment types. Those segment types can just implement INavigationSourceSegment and they will "automatically" be picked up by the helper function.;