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

Reverse Engineer - Virtual property and collection named badly when field name ends in GUID #26990

Closed
TimesliceTechnologies opened this issue Dec 13, 2021 · 12 comments · Fixed by #29105
Assignees
Labels
area-scaffolding closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@TimesliceTechnologies
Copy link

We are using Reverse Engineer to migrate from EF6 .Net Framework with EDMX and all its good stuff to EF6 Core.
I have some very large databases that have primary key fields and FK field names that end in "GUID". EG: UserGUID, OrderGUID etc. We have many large applications that rely on the existing model / entity names.

When Power Tools runs reverse engineer, which relies on EF Core, it applies unexpected names to the virtual properties.
According to ErikEJ at EFtools the problematic code in EFcore is

Basically that code see "id" at the end of a foreign key or something and strips "id", "ID", or "Id" and leaves the "GU". The end result is creation of virtual members with ridiculous names like "UserGU" instead of "User".

In our case we are trying to migrate a very large EF6 set of applications to EFcore. the EF6 models were generated using EF6 Database first which works exceptionally well. Ideally EFcore reverse engineer should generate a model that is identical to EF6. Without that capability to generate a DB first model that mirros what EF6 generates is a nightmare.

Simply fixing the naming logic to resolve table names for virtual properites would be a great start.

Example tables:
Users and UserLogins

Primary key in Users is UserGUID
Primary key in UserLogins is UserLoginGUID
UserLogins relates to Users on UserLogins.UserGUID=Users.UserGUID
The generated UserLogin class includes virtual property for User named as UserGU instead of User
EG: public virtual User UserGU { get; set; }

The same issue applies to unusal names for one to many where the collection will be name something like:
public virtual ICollection TrusteeOwnerUserGUs { get; set; }
Instead of TrusteeOwners. See the sample attached.

There is considerable difference between what EF6 Framework generates when using database first to generate the EDMX and classes.
That's a really tough migration issue in any large application. This is especially true with respect to the collections. EF6 set a standard for Database first that EFcore should follow so that migration from EF6 to EFcore is at least a reasonably effortless path.

Further technical details
EF Core version in use: EF Core 6
EF Core Power Tools version: 2.5.832
Database engine: SQL Server 2014
Visual Studio version: Visual Studio 2022 17.0.1

Files included:
I am including a VS2022 solution that has both a EF6 core class library and an EF6 Framework class library along with a SQL database creation script in the hope you can investigate and resolve the problem with foreign keys that may or may not end in "ID" and in the longer term arrive at reverse engineer that will replicates the naming that EF6 Framework generates (even if that is a new option). The alternative is a huge hill to climb migrating from EF6 to EFcore.

EFtoolsTestSolution.zip
EFtoolsDBcreate.zip

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 13, 2021

It is still unclear to me what you actually think is a correct approach - ErikEJ/EFCorePowerTools#1205 (comment)

@TimesliceTechnologies
Copy link
Author

Sorry if I have not been clear enough on this.
Maybe looking at it from a different perspetive might help:
IE: "Why do we end up with a virtual property named "UserGU" when the relationship clearly references a table named Users"?

I believe the correct approach is to determine the name of the target table the foreign key / relationship references. I my attached sample code example the target table in the relationship is "Users" (singular form "User". Definitely not "UserGU",

It appears to me the existing EFcore code you referred to infers the name of the target by finding "id" at the end of the foreign key, and just assumes the target table name should be whatever it resolves by just stripping the last 2 characters "id" off the foreign key field name. Removing the last two characters "id" leaves (in my example) "UserGU". I have not dug into the EFcore code, but I am assuming the virtual names are created based on field name from the foreign key rather than the actual target table name.

My expectation is that the virtual property name should be the singular and pluralized name of the entity in the target table based on the name of the target table / relationship.

Any clearer? I know that what EF6 generates is entirely logical and reliable based on the default templates in EF6 for DB first.
My attached example provides both EFcore and EF6 example projects.

@TimesliceTechnologies
Copy link
Author

TimesliceTechnologies commented Dec 13, 2021

And a subsquent question to clarify:

Why does EFcore reverse engineer generate different entity names than EF6 generates?
It would seem reasonable to expect they both generate exactly the same model names assuming both EFcore and EF6 use default settings (IE: Nothing other than default T4 templates were used in EF6).

@ajcvickers
Copy link
Member

@TimesliceTechnologies Using the table names for relationships can work well (depending on the table names) for tables with a single relationship between them. However, it falls down when there are multiple relationships between two tables, which is quite common. This is why EF Core instead uses foreign key column names, which often have semantically closer meaning to the relationship than the table names, and also work for multiple relationships between two tables. Templating for scaffolding, which is planned for EF7 will allow different strategies for navigation naming, allowing you to use table names as EF6 does.

That being said, we do think that the detection of "GUID" as a name ending in "ID" is something that we can improve.

@ajcvickers ajcvickers added area-scaffolding priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release labels Dec 15, 2021
@ajcvickers ajcvickers added this to the 7.0.0 milestone Dec 15, 2021
@TimesliceTechnologies
Copy link
Author

Thanks for the quick response Arthur.
Agreed on the multiple relationship issue.

I am hopeful we might see the GUID problem addressed in EFcore relatively soon. That seems like a relatively minor adjustment to the current code and would resolve a large part of the problem in migrating our model from EF6 to EFcore.

In the bigger picture some support to completely replicate old EF6 (framework) naming strategy would be extremely helpful for those of use that relied on the original EF6 DB first for many years and have huge systems with multiple applications that reference the model where a change in entity and navigation naming will be very challenging to resolve during a migration to EFcore.

@smitpatel smitpatel added type-bug and removed priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release labels Dec 15, 2021
@adoconnection
Copy link

+1 for this fix. I rely on guids a lot. Every time I run scaffold command it ruins all my reference names

image

Navigation properties will have names like CategoryGu and ProductGu

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 2, 2022

@adoconnection EF Core Power Tools allows you to easily rename properties - would that help?

@adoconnection
Copy link

adoconnection commented Feb 2, 2022

@adoconnection EF Core Power Tools allows you to easily rename properties - would that help?

Hello Erik! Thank you for fast reply. Not really. The Power Tools will generate navigation properties the same way with "Gu'" on the end.

I prefer design database in SSMS, so I have to refresh context quite ofter. If I fix names once, it will be overwriten by next update 😭

@TimesliceTechnologies
Copy link
Author

TimesliceTechnologies commented Feb 2, 2022 via email

@adoconnection
Copy link

adoconnection commented Feb 2, 2022

"Id" seems to be magic constant scattered across EFcore sources

private const string KeySuffix = "Id";

var property = TryGetProperty(
dependentEntityType,
baseName, "Id");

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 3, 2022

@adoconnection not magic, by convention.

@smitpatel
Copy link
Contributor

smitpatel commented Aug 13, 2022

Blocked by #28682

@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 15, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-rc2 Sep 15, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-scaffolding closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants