New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Containment navigation properties shouldn't have NavigationPropertyBindings to non-containment sets #1138

Closed
mikepizzo opened this Issue Mar 30, 2018 · 1 comment

Comments

Projects
None yet
1 participant
@mikepizzo
Member

mikepizzo commented Mar 30, 2018

By protocol, and by intuition, it should not be possible to specify a NavigationPropertyBinding for a containment navigation property.

Currently we support setting this, which could lead to unexpected behavior, including writing out NavigationPropertyBindings for containment navigation properties in CSDL.

Assemblies affected

OData Library 7.x

Reproduce steps

Call EdmNavigationSource.AddNavigationTarget(navProp,entitySet)
where navProp is a containment navigation proper and entitySet is an entity set defined within the container.

Expected result

An error should be thrown, or validation exception raised when validating the CSDL. Writing CSDL should never write out navigation property bindings for containment navigation properties.

Actual result

The call succeeds and writing the CSDL writes out the invalid navigation property binding for the contained navigation property.

Additional detail

In 7.4.3 you can also get this behavior if you call FindNavigationTarget for a containment navigation property, as that implicitly creates a target EdmContainedEntitySet. Subsequently writing out the CSDL writes out this navigation property binding (specifying itself as the target).

We can't actually throw an error because that would be a breaking change. We could/should make it a no-op and make sure that we never write out navigation property bindings for containment navigation properties.

@mikepizzo

This comment has been minimized.

Member

mikepizzo commented Mar 30, 2018

fixed in PR #1133.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment