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

Confusing migrations after upgrade from 6 to 7 #28962

Closed
MoazAlkharfan opened this issue Sep 2, 2022 · 2 comments · Fixed by #28969
Closed

Confusing migrations after upgrade from 6 to 7 #28962

MoazAlkharfan opened this issue Sep 2, 2022 · 2 comments · Fixed by #28969
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release regression type-bug
Milestone

Comments

@MoazAlkharfan
Copy link

MoazAlkharfan commented Sep 2, 2022

So I've been trying to upgrade ef core to 7 preview to test out the new features
after upgrading the packages I generate a new migration so any changes will be applied
What we're seeing here is multiple problems:

  • The most obvious problem is it's generating non user friendly column names that can't be used for someone querying the database directly ex: ForeignKeyToAnotherTable changing to ForeignKey1 which is a big no no
  • The second problem is what you see below is it's dropping foreign keys, renaming columns, renaming indexes, then adding the foreign key to a DIFFERENT table than the one it's supposed to!
    • The column Activity<User>Id was renamed to ActivityEvent_ActivityId1
    • The foreign key FK_Events_UserActivities_Activity<User>Id was renamed to FK_Events_CompanyActivities_ActivityEvent_ActivityId1
    • The same foreign key was pointing before to table UserActivities now it points to CompanyActivities

Include your code

No full reproduction, request if needed, it will take at least 3 days before i can provide a trimmed down working reproduction

This is a trimmed down version of the entities so it becomes easier to understand the structure being used

public abstract class Entity
{
    public string Id { get; set; }
}

public class Event<TTarget> : Entity
    where TTarget : class
{
    public virtual TTarget Target { get; set; }

    protected Event() { }

    public Event(TTarget target)
    {
        Target = target;
    }
}

public abstract class Activity<T> : Entity
    where T : class
{
    public virtual T Target { get; set; }

    public virtual List<ActivityEvent<T>> Events { get; private set; }
}

public abstract class ActivityEvent<TTarget> : Event<Activity<TTarget>>
    where TTarget : class
{
    protected ActivityEvent() { }

    protected ActivityEvent(Activity<TTarget> target)
        : base(target)
    {
    }
}

public class Company : Entity
{
    public virtual List<Activity<Company>> Activities { get; set; }
}

public class User : Entity
{
    public virtual List<Activity<User>> Activities { get; set; }
}


public class CompanyActivityConfiguration : IEntityTypeConfiguration<Activity<Company>>
{
    public void Configure(EntityTypeBuilder<Activity<Company>> builder)
    {
        builder.HasMany(x => x.Events)
            .WithOne(x => x.Target)
            .OnDelete(DeleteBehavior.Restrict);

        builder.ToTable(x => x.IsTemporal());
    }
}

public class UserActivityConfiguration : IEntityTypeConfiguration<Activity<User>>
{
    public void Configure(EntityTypeBuilder<Activity<User>> builder)
    {
        builder.HasMany(x => x.Events)
            .WithOne(x => x.Target)
            .OnDelete(DeleteBehavior.Restrict);

        builder.ToTable(x => x.IsTemporal());
    }
}


public class AppDbContext : DbContext
{
    public DbSet<Activity<User>> UserActivities { get; private set; } = null!;

    public DbSet<Activity<Company>> CompanyActivities { get; private set; } = null!;
    
    public DbSet<Event> Events { get; private set; } = null!;
    
    public AppDbContext(DbContextOptions<AppDbContext> options)
        : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        builder.ApplyConfigurationsFromAssembly(typeof(AppDbContext).Assembly);
    }
}

Initial migration that was generated with ef core 6

namespace project.Migrations
{
    public partial class Initial : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
           
            // ommited
            // ...
           
            migrationBuilder.AddForeignKey(
                name: "FK_Events_UserActivities_Activity<User>Id",
                schema: "app",
                table: "Events",
                column: "Activity<User>Id",
                principalSchema: "app",
                principalTable: "UserActivities",
                principalColumn: "Id");

            // ommited
            // ...
        }

        // ommited
       // ...
    }
}

Migration generated directly after upgrading from ef core 6 to ef core 7-preview

namespace project.Migrations
{
    /// <inheritdoc />
    public partial class Ef7Update : Migration
    {
        /// <inheritdoc />
        protected override void Up(MigrationBuilder migrationBuilder)
        {
           
            // ommited
            // ...
           
            migrationBuilder.DropForeignKey(
                name: "FK_Events_UserActivities_Activity<User>Id",
                schema: "app",
                table: "Events");

            // ommited
            // ...
           
            migrationBuilder.RenameColumn(
                name: "Activity<User>Id",
                schema: "app",
                table: "Events",
                newName: "ActivityEvent_ActivityId1")
                .Annotation("SqlServer:IsTemporal", true)
                .Annotation("SqlServer:TemporalHistoryTableName", "EventsHistory")
                .Annotation("SqlServer:TemporalHistoryTableSchema", "app")
                .Annotation("SqlServer:TemporalPeriodEndColumnName", "PeriodEnd")
                .Annotation("SqlServer:TemporalPeriodStartColumnName", "PeriodStart");

            // omitted code
            // ...

            migrationBuilder.RenameIndex(
                name: "IX_Events_Activity<User>Id",
                schema: "app",
                table: "Events",
                newName: "IX_Events_ActivityEvent_ActivityId1");

            migrationBuilder.AddForeignKey(
                name: "FK_Events_CompanyActivities_ActivityEvent_ActivityId1",
                schema: "app",
                table: "Events",
                column: "ActivityEvent_ActivityId1",
                principalSchema: "app",
                principalTable: "CompanyActivities",
                principalColumn: "Id");

            // omitted code
            // ...

        }

        // omitted code
        // ...
    }
}

Include provider and version information

EF Core version: 7.0.0-rc.2.22452.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 7.0

@ajcvickers ajcvickers added the priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release label Sep 2, 2022
@ajcvickers ajcvickers added this to the 7.0.0 milestone Sep 2, 2022
@ajcvickers
Copy link
Member

Note from triage: revert the naming breaking change.

@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Sep 2, 2022
@AndriySvyryd AndriySvyryd removed their assignment Sep 2, 2022
@AndriySvyryd
Copy link
Member

@MoazAlkharfan I've reverted the naming change, but for the other issues I'd need a complete repro. Try the next daily build or the public release of rc2 when it comes out and open a new issue if you are experiencing problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release regression type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants