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

Update discriminator columns when PK-to-PK dependent type is changed #29876

Merged
merged 2 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/EFCore.Relational/Update/ModificationCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,10 @@ private List<IColumnModification> GenerateColumnModifications()

foreach (var entry in _entries.Where(x => !x.EntityType.IsMappedToJson()))
{
var nonMainEntry = !_mainEntryAdded || entry != _entries[0];
var nonMainEntry = (!_mainEntryAdded || entry != _entries[0])
|| (updating
&& (entry.EntityState == EntityState.Deleted
|| entry.EntityState == EntityState.Added));
Copy link
Member

Choose a reason for hiding this comment

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

Rather than changing this add to the condition on 548 || entry.SharedIdentityEntry != null

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to work. Thanks!


var optionalDependentWithAllNull = false;

Expand Down
27 changes: 23 additions & 4 deletions test/EFCore.InMemory.FunctionalTests/UpdatesInMemoryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,36 @@ protected override void ExecuteWithStrategyInTransaction(
Action<UpdatesContext> nestedTestOperation1 = null,
Action<UpdatesContext> nestedTestOperation2 = null)
{
base.ExecuteWithStrategyInTransaction(testOperation, nestedTestOperation1, nestedTestOperation2);
Fixture.Reseed();
try
{
base.ExecuteWithStrategyInTransaction(testOperation, nestedTestOperation1, nestedTestOperation2);
}
finally
{
Fixture.Reseed();
}
}

protected override async Task ExecuteWithStrategyInTransactionAsync(
Func<UpdatesContext, Task> testOperation,
Func<UpdatesContext, Task> nestedTestOperation1 = null,
Func<UpdatesContext, Task> nestedTestOperation2 = null)
{
await base.ExecuteWithStrategyInTransactionAsync(testOperation, nestedTestOperation1, nestedTestOperation2);
Fixture.Reseed();
try
{
await base.ExecuteWithStrategyInTransactionAsync(testOperation, nestedTestOperation1, nestedTestOperation2);
}
finally
{
Fixture.Reseed();
}
}

// Issue #29875
public override Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
{
return Assert.ThrowsAsync<DbUpdateConcurrencyException>(
() => base.Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(async));
}

public abstract class UpdatesInMemoryFixtureBase : UpdatesFixtureBase
Expand Down
30 changes: 30 additions & 0 deletions test/EFCore.Specification.Tests/TestModels/UpdatesModel/Gift.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable enable

namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;

public class Gift
{
public int Id { get; set; }
public string? Recipient { get; set; }

public GiftObscurer? Obscurer { get; set; }
}

public abstract class GiftObscurer
{
public int Id { get; set; }
public string? Pattern { get; set; }
}

public class GiftBag : GiftObscurer
{
public int Size { get; set; }
}

public class GiftPaper : GiftObscurer
{
public int Thickness { get; set; }
}
32 changes: 32 additions & 0 deletions test/EFCore.Specification.Tests/TestModels/UpdatesModel/Lift.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable enable

namespace Microsoft.EntityFrameworkCore.TestModels.UpdatesModel;

public class Lift
{
public int Id { get; set; }
public string? Recipient { get; set; }

public LiftObscurer Obscurer { get; set; } = null!;
}

public abstract class LiftObscurer
{
public int Id { get; set; }

public int LiftId { get; set; }
public string? Pattern { get; set; }
}

public class LiftBag : LiftObscurer
{
public int Size { get; set; }
}

public class LiftPaper : LiftObscurer
{
public int Thickness { get; set; }
}
64 changes: 64 additions & 0 deletions test/EFCore.Specification.Tests/UpdatesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,60 @@ public virtual async Task Can_delete_and_add_for_same_key(bool async)
Assert.Equal(EntityState.Detached, context.Entry(rodney1).State);
});

[ConditionalTheory] // Issue #29789
[InlineData(false)]
[InlineData(true)]
public virtual async Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
=> await ExecuteWithStrategyInTransactionAsync(
async context =>
{
var gift = new Gift { Recipient = "Alice", Obscurer = new GiftPaper { Pattern = "Stripes" } };
await context.AddAsync(gift);
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
},
async context =>
{
var gift = await context.Set<Gift>().Include(e => e.Obscurer).SingleAsync();
var bag = new GiftBag { Pattern = "Gold stars" };
gift.Obscurer = bag;
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
},
async context =>
{
var gift = await context.Set<Gift>().Include(e => e.Obscurer).SingleAsync();

Assert.IsType<GiftBag>(gift.Obscurer);
Assert.Equal(gift.Id, gift.Obscurer.Id);
Assert.Single(context.Set<GiftObscurer>());
});

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public virtual async Task Can_change_type_of__dependent_by_replacing_with_new_dependent(bool async)
=> await ExecuteWithStrategyInTransactionAsync(
async context =>
{
var lift = new Lift { Recipient = "Alice", Obscurer = new LiftPaper { Pattern = "Stripes" } };
await context.AddAsync(lift);
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
},
async context =>
{
var lift = await context.Set<Lift>().Include(e => e.Obscurer).SingleAsync();
var bag = new LiftBag { Pattern = "Gold stars" };
lift.Obscurer = bag;
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
},
async context =>
{
var lift = await context.Set<Lift>().Include(e => e.Obscurer).SingleAsync();

Assert.IsType<LiftBag>(lift.Obscurer);
Assert.Equal(lift.Id, lift.Obscurer.LiftId);
Assert.Single(context.Set<LiftObscurer>());
});

[ConditionalFact]
public virtual void Mutation_of_tracked_values_does_not_mutate_values_in_store()
{
Expand Down Expand Up @@ -772,6 +826,16 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
.WithOne(l => l.Profile)
.IsRequired();
});

modelBuilder.Entity<Gift>();
modelBuilder.Entity<GiftObscurer>().HasOne<Gift>().WithOne(x => x.Obscurer).HasForeignKey<GiftObscurer>(e => e.Id);
modelBuilder.Entity<GiftBag>();
modelBuilder.Entity<GiftPaper>();

modelBuilder.Entity<Lift>();
modelBuilder.Entity<LiftObscurer>().HasOne<Lift>().WithOne(x => x.Obscurer).HasForeignKey<LiftObscurer>(e => e.LiftId);
modelBuilder.Entity<LiftBag>();
modelBuilder.Entity<LiftPaper>();
}

protected override void Seed(UpdatesContext context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
{
base.OnModelCreating(modelBuilder, context);

modelBuilder.Entity<Category>()
.UseTpcMappingStrategy();
modelBuilder.Entity<Category>().UseTpcMappingStrategy();
// modelBuilder.Entity<GiftObscurer>().UseTpcMappingStrategy(); Issue #29874
modelBuilder.Entity<LiftObscurer>().UseTpcMappingStrategy();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ public UpdatesSqlServerTPTTest(UpdatesSqlServerTPTFixture fixture, ITestOutputHe
{
}

[ConditionalTheory(Skip = "Issue #29874. Skipped because the database is in a bad state, but the test may or may not fail.")]
public override Task Can_change_type_of_pk_to_pk_dependent_by_replacing_with_new_dependent(bool async)
=> Task.CompletedTask;

public override void Save_with_shared_foreign_key()
{
base.Save_with_shared_foreign_key();
Expand Down Expand Up @@ -97,8 +101,9 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
{
base.OnModelCreating(modelBuilder, context);

modelBuilder.Entity<Category>()
.UseTptMappingStrategy();
modelBuilder.Entity<Category>().UseTptMappingStrategy();
modelBuilder.Entity<GiftObscurer>().UseTptMappingStrategy();
modelBuilder.Entity<LiftObscurer>().UseTptMappingStrategy();
}
}
}