Fix for 10508. Create Association Entity automatically by convention.#21484
Merged
Fix for 10508. Create Association Entity automatically by convention.#21484
Conversation
Contributor
|
@lajones - It passed CLA check now. Probably intermittent error in the server. |
AndriySvyryd
reviewed
Jul 1, 2020
AndriySvyryd
reviewed
Jul 1, 2020
AndriySvyryd
reviewed
Jul 1, 2020
src/EFCore/Metadata/Conventions/Internal/ConventionDispatcher.ImmediateConventionScope.cs
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 1, 2020
AndriySvyryd
reviewed
Jul 1, 2020
| // do not create an automatic many-to-many association entity type | ||
| // for a self-referencing skip navigation, or for one that | ||
| // is already "in use" (i.e. has its Foreign Key assigned). | ||
| return; |
Member
There was a problem hiding this comment.
Why not doing it for self-ref?
Contributor
Author
There was a problem hiding this comment.
If I have 2 collection navigations pointing to an Entity Type from that same Entity Type, my expectation would be two 1:N navigations to self rather than one N:N. I guess I thought that it was ambiguous, and in ambiguous cases we do nothing and let the user decide.
Member
There was a problem hiding this comment.
We should discuss with the team
Contributor
There was a problem hiding this comment.
Any m2m navigation pair is potentially 2 different 1:N relationships.
Contributor
Author
There was a problem hiding this comment.
Agree we should discuss with the team.
AndriySvyryd
reviewed
Jul 1, 2020
smitpatel
reviewed
Jul 1, 2020
AndriySvyryd
reviewed
Jul 1, 2020
src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 1, 2020
src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 1, 2020
src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 1, 2020
src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 1, 2020
AndriySvyryd
reviewed
Jul 2, 2020
AndriySvyryd
reviewed
Jul 2, 2020
| } | ||
|
|
||
| private const string AssociationEntityTypeNameTemplate = "Join_{0}_{1}"; | ||
| private const string AssociationPropertyNameTemplate = "{0}_{1}"; |
Member
There was a problem hiding this comment.
We should discuss both of these with the team
Member
There was a problem hiding this comment.
What was the team decision WRT to FK property naming?
Contributor
Author
Contributor
There was a problem hiding this comment.
@AndriySvyryd Is there any reason we need special rules for FKs? We decided in the design meeting to just use the current rules.
Member
There was a problem hiding this comment.
No. That would be my preference as well.
AndriySvyryd
reviewed
Jul 2, 2020
AndriySvyryd
reviewed
Jul 2, 2020
AndriySvyryd
reviewed
Jul 2, 2020
AndriySvyryd
reviewed
Jul 2, 2020
AndriySvyryd
reviewed
Jul 2, 2020
AndriySvyryd
reviewed
Jul 2, 2020
fe3f06d to
31d8567
Compare
AndriySvyryd
reviewed
Jul 3, 2020
src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 3, 2020
src/EFCore/Metadata/Conventions/RelationshipDiscoveryConvention.cs
Outdated
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 3, 2020
AndriySvyryd
reviewed
Jul 3, 2020
AndriySvyryd
reviewed
Jul 3, 2020
AndriySvyryd
reviewed
Jul 3, 2020
AndriySvyryd
reviewed
Jul 3, 2020
smitpatel
reviewed
Jul 3, 2020
smitpatel
reviewed
Jul 3, 2020
AndriySvyryd
reviewed
Jul 6, 2020
AndriySvyryd
reviewed
Jul 7, 2020
AndriySvyryd
reviewed
Jul 7, 2020
src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 7, 2020
src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs
Outdated
Show resolved
Hide resolved
smitpatel
reviewed
Jul 7, 2020
src/EFCore/Metadata/Conventions/ManyToManyAssociationEntityTypeConvention.cs
Show resolved
Hide resolved
AndriySvyryd
reviewed
Jul 7, 2020
smitpatel
reviewed
Jul 7, 2020
AndriySvyryd
approved these changes
Jul 7, 2020
565e044 to
eb03576
Compare
…t convention-time and through HasMany().WithMany() for fluent API.
eb03576 to
61f6cb9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10508
Create Association Entity automatically by convention. Note: this is only the model building part - also need #19003 for the ChangeTracking part.