Skip to content

Commit

Permalink
Set Cascade OnDelete for ownerships without conventions to avoid fals…
Browse files Browse the repository at this point in the history 
…e model diffs.

Fixes #18045
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Oct 24, 2019
1 parent 54961bc commit 752fd06
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 66 deletions.
22 changes: 7 additions & 15 deletions src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,8 @@ protected virtual void TrackData(
var entry = _sourceUpdateAdapter
.CreateEntry(sourceSeed, sourceEntityType);

// Mark as added first to generate missing values
// Issue #15289
entry.EntityState = EntityState.Added;
entry.EntityState = EntityState.Unchanged;
}
Expand Down Expand Up @@ -1754,21 +1756,11 @@ protected virtual void DiffData(
{
var (sourceProperty, sourceConverter, targetConverter) = keyPropertiesMap[i];
var sourceValue = sourceEntry.GetCurrentValue(sourceProperty);
if (targetKey.Properties[i].ClrType != sourceProperty.ClrType)
{
if (sourceConverter != null)
{
targetKeyValues[i] = sourceConverter.ConvertToProvider(sourceValue);
}
else
{
targetKeyValues[i] = targetConverter.ConvertFromProvider(sourceValue);
}
}
else
{
targetKeyValues[i] = sourceValue;
}
targetKeyValues[i] = targetKey.Properties[i].ClrType != sourceProperty.ClrType
? sourceConverter != null
? sourceConverter.ConvertToProvider(sourceValue)
: targetConverter.ConvertFromProvider(sourceValue)
: sourceValue;
}

var entry = _targetUpdateAdapter.TryGetEntry(targetKey, targetKeyValues);
Expand Down
13 changes: 13 additions & 0 deletions src/EFCore/Metadata/Internal/InternalRelationshipBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,12 @@ public virtual InternalRelationshipBuilder IsOwnership(bool? ownership, Configur
}

newRelationshipBuilder = Metadata.SetIsOwnership(ownership: true, configurationSource)?.Builder;
newRelationshipBuilder = newRelationshipBuilder?.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention);

if (newRelationshipBuilder == null)
{
return null;
}

foreach (var otherOwnership in otherOwnerships)
{
Expand Down Expand Up @@ -1074,6 +1080,12 @@ public virtual InternalRelationshipBuilder IsOwnership(bool? ownership, Configur
}

newRelationshipBuilder = Metadata.SetIsOwnership(ownership: true, configurationSource)?.Builder;
newRelationshipBuilder = newRelationshipBuilder?.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention);

if (newRelationshipBuilder == null)
{
return null;
}

using (var batch = ModelBuilder.Metadata.ConventionDispatcher.DelayConventions())
{
Expand Down Expand Up @@ -1113,6 +1125,7 @@ public virtual InternalRelationshipBuilder IsOwnership(bool? ownership, Configur
}

newRelationshipBuilder = Metadata.SetIsOwnership(ownership: true, configurationSource)?.Builder;
newRelationshipBuilder = newRelationshipBuilder?.OnDelete(DeleteBehavior.Cascade, ConfigurationSource.Convention);

if (newRelationshipBuilder == null
&& Metadata.PrincipalEntityType.Builder != null
Expand Down
123 changes: 90 additions & 33 deletions test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,20 @@ public void Model_differ_does_not_detect_views()

[ConditionalFact]
public void Model_differ_does_not_detect_views_with_owned_types()
{
Execute(
_ => { },
target => target.Entity<Order>(
x =>
{
x.ToView("Orders");
x.OwnsOne(y => y.Shipping);
}),
upOperations => Assert.Equal(0, upOperations.Count));
}

[ConditionalFact]
public void Model_differ_does_not_detect_views_with_weak_types()
{
Execute(
_ => { },
Expand Down Expand Up @@ -1678,10 +1692,8 @@ public void Rename_property_and_column()
[ConditionalFact]
public void Rename_property_and_column_when_snapshot()
{
// NB: No conventions to simulate the snapshot.
var sourceModelBuilder = new ModelBuilder(new ConventionSet());
sourceModelBuilder.Entity(
// ReSharper disable once AssignNullToNotNullAttribute
Execute(
source => source.Entity(
typeof(Crab).FullName,
x =>
{
Expand All @@ -1690,23 +1702,18 @@ public void Rename_property_and_column_when_snapshot()
x.Property<string>("CrabId");
x.HasKey("CrabId");
});

var targetModelBuilder = CreateModelBuilder();
targetModelBuilder.Entity<Crab>();

targetModelBuilder.FinalizeModel();

var modelDiffer = RelationalTestHelpers.Instance.CreateContextServices()
.GetRequiredService<IMigrationsModelDiffer>();
var operations = modelDiffer.GetDifferences(sourceModelBuilder.Model, targetModelBuilder.Model);

Assert.Equal(1, operations.Count);
}),
target => target.Entity<Crab>(),
operations =>
{
Assert.Equal(1, operations.Count);
var operation = Assert.IsType<RenameColumnOperation>(operations[0]);
Assert.Equal("Crab", operation.Table);
Assert.Equal("CrabId", operation.Name);
Assert.Equal("Id", operation.NewName);
var operation = Assert.IsType<RenameColumnOperation>(operations[0]);
Assert.Equal("Crab", operation.Table);
Assert.Equal("CrabId", operation.Name);
Assert.Equal("Id", operation.NewName);
},
skipSourceConventions: true);
}

private class Crab
Expand Down Expand Up @@ -2561,7 +2568,8 @@ public void Add_primary_key()
Assert.Equal("dbo", operation.Schema);
Assert.Equal("Puffin", operation.Table);
Assert.Equal("PK_Puffin", operation.Name);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -2630,7 +2638,8 @@ public void Alter_primary_key_columns()
Assert.Equal("Raven", addOperation.Table);
Assert.Equal("PK_Raven", addOperation.Name);
Assert.Equal(new[] { "RavenId" }, addOperation.Columns);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -2667,7 +2676,8 @@ public void Alter_primary_key_column_order_with_seed_data()
Assert.Equal("Raven", addOperation.Table);
Assert.Equal("PK_Raven", addOperation.Name);
Assert.Equal(new[] { "RavenId", "Id" }, addOperation.Columns);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -3523,7 +3533,8 @@ public void Alter_sequence_increment_by()
Assert.Equal(1, operation.MinValue);
Assert.Equal(4, operation.MaxValue);
Assert.True(operation.IsCyclic);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -3553,7 +3564,8 @@ public void Alter_sequence_max_value()
Assert.Equal(1, operation.MinValue);
Assert.Equal(5, operation.MaxValue);
Assert.True(operation.IsCyclic);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -3613,7 +3625,8 @@ public void Alter_sequence_cycle()
Assert.Equal(1, operation.MinValue);
Assert.Equal(4, operation.MaxValue);
Assert.False(operation.IsCyclic);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -3649,7 +3662,8 @@ public void Alter_sequence_type()
Assert.Equal(1, createOperation.MinValue);
Assert.Equal(4, createOperation.MaxValue);
Assert.True(createOperation.IsCyclic);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -3677,7 +3691,8 @@ public void Alter_sequence_start()
Assert.Equal("dbo", operation.Schema);
Assert.Equal("Golf", operation.Name);
Assert.Equal(5, operation.StartValue);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand All @@ -3699,7 +3714,8 @@ public void Restart_altered_sequence()
operations => Assert.Collection(
operations,
o => Assert.IsType<AlterSequenceOperation>(o),
o => Assert.IsType<RestartSequenceOperation>(o)));
o => Assert.IsType<RestartSequenceOperation>(o)),
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -5856,7 +5872,8 @@ public void Alter_renamed_sequence()
var alterSequenceOperation = Assert.IsType<AlterSequenceOperation>(operations[2]);
Assert.Equal("new", alterSequenceOperation.Schema);
Assert.Equal("Sequence", alterSequenceOperation.Name);
});
},
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -7429,6 +7446,7 @@ private class Customer
public int Id { get; set; }

public Address Mailing { get; set; }
public ICollection<Order> Orders { get; set; }
}

private class Address
Expand Down Expand Up @@ -7507,15 +7525,19 @@ public void Add_type_with_additional_ownership()
{
Execute(
source => source
.Entity<Customer>().OwnsOne(y => y.Mailing),
.Entity<Customer>()
.Ignore(c => c.Orders)
.OwnsOne(y => y.Mailing),
target => target
.Entity<Order>(
x =>
{
x.OwnsOne(y => y.Billing);
x.OwnsOne(y => y.Shipping);
})
.Entity<Customer>().OwnsOne(y => y.Mailing),
.Entity<Customer>()
.Ignore(c => c.Orders)
.OwnsOne(y => y.Mailing),
operations =>
{
var operation = Assert.IsType<CreateTableOperation>(Assert.Single(operations));
Expand Down Expand Up @@ -7567,7 +7589,8 @@ public void Add_type_with_ownership_SeedData()
{
var m = Assert.IsType<DropTableOperation>(o);
Assert.Equal("Order", m.Name);
}));
}),
skipSourceConventions: true);
}

[ConditionalFact]
Expand Down Expand Up @@ -7595,6 +7618,40 @@ public void SeedData_type_with_ownership_no_changes()
Assert.Empty);
}

[ConditionalFact]
public void SeedData_type_with_owned_collection_no_changes()
{
Execute(
common =>
{
common.Entity<Customer>(
c =>
{
c.Ignore(x => x.Mailing);
c.HasKey(x => x.Id);
c.HasData(new Customer { Id = 1 });
c.OwnsMany(y => y.Orders, x =>
{
x.Ignore(o => o.Billing);
x.Ignore(o => o.Shipping);
x.WithOwner()
.HasForeignKey("CustomerId");
x.HasKey("CustomerId", "Id");
x.HasData(new { Id = 2, CustomerId = 1 });
});
});
},
_ => { },
_ => { },
Assert.Empty,
Assert.Empty,
skipSourceConventions: true);
}

[ConditionalFact]
public void Old_style_ownership_to_new_style()
{
Expand Down
Loading

0 comments on commit 752fd06

Please sign in to comment.