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

[release/7.0] Scaffolding: Fix code for EF6 many-to-many pattern #29793

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

bricelam
Copy link
Contributor

@bricelam bricelam commented Dec 7, 2022

Certain patterns of many-to-many join tables that don't follow the EF Core conventions--most notably ones generated by EF6--result in an incomplete model when scaffolding. This fix adds additional handling for these patterns.

Fixes #29544, fixes #29634

Customer impact

Multiple users have reported issues in this area. Some issues can be worked around by applying the fix to custom scaffolding templates. Others require manually updating the code after it's scaffolded. The incorrect models result in runtime exceptions about missing tables and columns.

Regression

Yes. Correct models were scaffolded in EF Core 6.

Testing

Added automated tests covering more unconventional many-to-many join tables.

Risk

Low. Adds back code that I neglected to port as part of the custom templates feature.

@bricelam bricelam requested a review from a team December 7, 2022 18:59
@@ -803,7 +823,7 @@ public System.IFormatProvider FormatProvider
{
get
{
return this.formatProviderField;
return this.formatProviderField ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

Copy link
Contributor Author

@bricelam bricelam Dec 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you expect anything less from a VS tool that generates code. 😉 (The resx one drives me crazy with all its extra whitespace.) I wasn't gonna fight it...

@wtgodbe
Copy link
Member

wtgodbe commented Jan 4, 2023

@bricelam could you resolve the merge conflict?

@mckelvj
Copy link

mckelvj commented Nov 8, 2023

I see this issue fixed and closed Jan 31 but I am experiencing this issue where it is not reverse engineering two many-to-many tables. It is defining the one entity the DbContext on one table, but not the other and the Class for the entity is not created and the public virtual DbSet class {get; set;} is not defined. No errors are identified in the response to the "dotnet ef dbcontext scaffold" command. Everything else in the database is scaffold correctly.

There are two tables in the database that are not scaffolded. They are both many-to-many. Everything else is correct. I'm using version 7.0.13

A scripted table definition of one table is a shown below:

USE [########]
GO

/****** Object: Table [dbo].[AspNetUserRoles] Script Date: 11/8/2023 3:27:42 PM ******/
SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE TABLE [dbo].[AspNetUserRoles](
[UserId] nvarchar NOT NULL,
[RoleId] nvarchar NOT NULL,
CONSTRAINT [PK_dbo.AspNetUserRoles] PRIMARY KEY CLUSTERED
(
[UserId] ASC,
[RoleId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]
GO

ALTER TABLE [dbo].[AspNetUserRoles] WITH CHECK ADD CONSTRAINT [FK_dbo.AspNetUserRoles_dbo.AspNetRoles_RoleId] FOREIGN KEY([RoleId])
REFERENCES [dbo].[AspNetRoles] ([Id])
ON DELETE CASCADE
GO

ALTER TABLE [dbo].[AspNetUserRoles] CHECK CONSTRAINT [FK_dbo.AspNetUserRoles_dbo.AspNetRoles_RoleId]
GO

ALTER TABLE [dbo].[AspNetUserRoles] WITH CHECK ADD CONSTRAINT [FK_dbo.AspNetUserRoles_dbo.AspNetUsers_UserId] FOREIGN KEY([UserId])
REFERENCES [dbo].[AspNetUsers] ([Id])
ON DELETE CASCADE
GO

ALTER TABLE [dbo].[AspNetUserRoles] CHECK CONSTRAINT [FK_dbo.AspNetUserRoles_dbo.AspNetUsers_UserId]
GO

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

Successfully merging this pull request may close these issues.

None yet

4 participants