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

Owned entity on genericly typed entity generates unwanted steps in new migrations #32763

Closed
jmvlangen opened this issue Jan 10, 2024 · 1 comment · Fixed by #32937
Closed
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@jmvlangen
Copy link

After upgrading our code from .NET 6 to .NET 8 we noticed some strange lines showing up in newly generated migration scripts. Even when there are no changes to the model, add-migration creates a migration which wants to drop some non-existent foreign keys and corresponding columns. Upon further inspection this column corresponds to the primary key of an owned entity of an entity whose class is genericly typed.

This is a minimal example that reproduces the problem:

using Microsoft.EntityFrameworkCore;

public class Parent<TChild>
{
    public int Id { get; set; }
    public string Name { get; set; }
    public TChild Child { get; set; }
}

public class Boy
{
    public string Name { get; set; }
}

public class TestContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer();

        base.OnConfiguring(optionsBuilder);
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        var parentBuilder = modelBuilder.Entity<Parent<Boy>>();

        parentBuilder.HasKey(p => p.Id);
        parentBuilder.OwnsOne(p => p.Child);

        base.OnModelCreating(modelBuilder);
    }
}

If you run Add-Migration from the EF core tools on this model, the first generated migration is fine. Every migration generated thereafter (even if no changes were made to the code) will contain the following two lines (besides all actual changes):

            migrationBuilder.DropForeignKey(
                name: "FK_Parent<Boy>_Parent<Boy>_Parent<Boy>Id",
                table: "Parent<Boy>");

            migrationBuilder.DropColumn(
                name: "Parent<Boy>Id",
                table: "Parent<Boy>");

In the TestContextModelSnapshot.cs we find:

            modelBuilder.Entity(".Parent<Boy>", b =>
                {
                    b.OwnsOne("Boy", "Child", b1 =>
                        {
                            b1.Property<int>("ParentId")
                                .HasColumnType("int")
                                .HasColumnName("Parent<Boy>Id");

                            b1.Property<string>("Name")
                                .IsRequired()
                                .HasColumnType("nvarchar(max)");

                            b1.HasKey("ParentId");

                            b1.ToTable("Parent<Boy>");

                            b1.WithOwner()
                                .HasForeignKey("ParentId");
                        });

                    b.Navigation("Child");
                });

The line containing

.HasColumnName("Parent<Boy>Id")

appears to be new. It did not appear in snapshots generated with the .NET 6 version of EF core. Since the property name seems to have changed in .NET 8, the column name seems to be added to stay consistent with previous version, even though this shadow property is never persisted to the database. Removing this line before generating a migration will make the migration omit the drop statements, but it does reinsert this line into the snapshot making it necessary to repeat this before every migration thereafter.

System information

EF Core version: 8.0.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer 8.0.1
Target framework: .NET 8
Operating system: Windows
IDE: Visual Studio Enterprise 2022 17.8.3

@ajcvickers
Copy link
Contributor

Note from triage: Still repros on the latest bits; regression from 7.

@ajcvickers ajcvickers self-assigned this Jan 16, 2024
@ajcvickers ajcvickers added this to the 8.0.x milestone Jan 16, 2024
ajcvickers added a commit that referenced this issue Jan 27, 2024
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 27, 2024
ajcvickers added a commit that referenced this issue Jan 30, 2024
@ajcvickers ajcvickers reopened this Jan 30, 2024
ajcvickers added a commit that referenced this issue Jan 30, 2024
@ajcvickers ajcvickers modified the milestones: 8.0.x, 8.0.3 Feb 2, 2024
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
2 participants