Skip to content
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

Documentation Does not State when it is ok to NOT Explicitly Add FK Shadow Properties Before Using Them #2807

Closed
pha3z opened this issue Oct 25, 2020 · 4 comments · Fixed by #4269

Comments

@pha3z
Copy link

pha3z commented Oct 25, 2020

From this page: https://docs.microsoft.com/en-us/ef/core/modeling/relationships?tabs=fluent-api%2Cfluent-api-simple-key%2Csimple-key#shadow-foreign-key

"We recommend explicitly adding the shadow property to the model before using it as a foreign key (as shown below)."

WHY is it recommended (no explanation on the page)? What will happen if I don't? Do I have to follow this rule across the board?

I am trying to reduce volume of code. There's no explanation of what would happen if you call the .HasForeignKey(""BlogForeignKey") method without having explicitly added the Shadow Property beforehand. How do I know whether or not I can omit the explicit property addition?

I don't want to rely on convention, but I don't want to have heavily redundant code either. I just want to call .HasForeignKey() and pass the column name for the foreign key and then know that the model will be complete.

@pha3z
Copy link
Author

pha3z commented Oct 26, 2020

After playing around with other pieces of the frameworking, I'm guessing this might throw an exception if "BlogForeignKey" doesn't exist on the entity and hasn't been added a shadow property. But I'm only guessing. Would be helpful if the behavior was noted in the documentation.

@ajcvickers
Copy link
Member

@pha3z The recommendation is there because it allows the type of the shadow FK property to be explicitly specified. This avoids getting a type you don't expect, especially with regards to nullability. Putting this issue on the backlog to make this clearer in the docs.

I don't want to rely on convention, but I don't want to have heavily redundant code either.

I think you'll find it hard to not rely on any conventions when building the model. Also, the more conventions you don't use, then, by definition, the more redundant code you will have.

@ajcvickers ajcvickers added this to the Backlog milestone Oct 27, 2020
@pha3z
Copy link
Author

pha3z commented Oct 28, 2020

I don't want to rely on convention, but I don't want to have heavily redundant code either.

My statement was misleading as I didn't have a full appreciation of the problem space yet. My apologies. The real problem for me turned out to be that EF doesn't encourage you to configure its conventions (or even offer you a way to do so). I was able to work around this elegantly by adding extension methods for ReferenceBuilder, ReferenceCollectionBuilder, etc. so that I could employ my own conventions. It greatly cleaned up my models and enhanced both maintainability and readability.

I wonder if it would be an improvement to make conventions configurable or if it would make the system too complicated to understand and maintain. At the very least, maybe documenting an example pattern for someone to employ their own conventions with extension methods might be helpful. I wrestled for a long time with getting EF to do what I wanted until I realized I just needed to setup my own conventions.

I think you'll find it hard to not rely on any conventions when building the model. Also, the more conventions you don't use, then, by definition, the more redundant code you will have.

Also, good tip! Makes perfect sense! Thank you, sir!

@ajcvickers
Copy link
Member

@pha3z The conventions system was initially internal, but we have been working on making it public over the past few releases. It's now possible for database providers and extensions to introduce public conventions. The final part of this (applications changing/introducing conventions) is likely going to be in 6.0. See #214.

That being said, conventions are not trivial to implement, especially where there are interactions between different conventions. So changing the conventions is likely to be a fairly advanced thing to do. In general, extension methods (probably like the ones you mention) or bulk configuration of the underlying model are usually a more pragmatic choice for app developers.

@ajcvickers ajcvickers self-assigned this Feb 23, 2023
@ajcvickers ajcvickers modified the milestones: Backlog, 8.0.0 Feb 23, 2023
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Feb 25, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Mar 19, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Mar 28, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
ajcvickers added a commit that referenced this issue Mar 30, 2023
Fixes #4145
Fixes #4053
Fixes #3900
Fixes #3810
Fixes #3668
Fixes #3648
Fixes #3282
Fixes #3282
Fixes #2924
Fixes #2807
Fixes #2494
Fixes #2321
Fixes #2266

Part of #3996
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants