Skip to content

Commit

Permalink
Create GraphUpdatesTests for owned entities
Browse files Browse the repository at this point in the history 
Disable owned entity reparenting if it leads to key modification
Fix default SQL Server value generation strategy for shared columns

Fixes #16454
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Nov 25, 2020
1 parent 30c1344 commit f481b97
Show file tree
Hide file tree
Showing 11 changed files with 734 additions and 151 deletions.
1 change: 1 addition & 0 deletions src/EFCore.SqlServer/Extensions/SqlServerPropertyExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ public static SqlServerValueGenerationStrategy GetValueGenerationStrategy(
{
return sharedTableRootProperty.GetValueGenerationStrategy(storeObject)
== SqlServerValueGenerationStrategy.IdentityColumn
&& !property.GetContainingForeignKeys().Any(fk => !fk.IsBaseLinking())
? SqlServerValueGenerationStrategy.IdentityColumn
: SqlServerValueGenerationStrategy.None;
}
Expand Down
124 changes: 69 additions & 55 deletions src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,63 +291,77 @@ protected override void ValidateCompatible(
var duplicatePropertyStrategy = duplicateProperty.GetValueGenerationStrategy(storeObject);
if (propertyStrategy != duplicatePropertyStrategy)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnNameValueGenerationStrategyMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
var isConflicting = ((IConventionProperty)property)
.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy)
?.GetConfigurationSource() == ConfigurationSource.Explicit
|| propertyStrategy != SqlServerValueGenerationStrategy.None;
var isDuplicateConflicting = ((IConventionProperty)duplicateProperty)
.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy)
?.GetConfigurationSource() == ConfigurationSource.Explicit
|| duplicatePropertyStrategy != SqlServerValueGenerationStrategy.None;

if (isConflicting && isDuplicateConflicting)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnNameValueGenerationStrategyMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}
}

switch (propertyStrategy)
else
{
case SqlServerValueGenerationStrategy.IdentityColumn:
var increment = property.GetIdentityIncrement(storeObject);
var duplicateIncrement = duplicateProperty.GetIdentityIncrement(storeObject);
if (increment != duplicateIncrement)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnIdentityIncrementMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}

var seed = property.GetIdentitySeed(storeObject);
var duplicateSeed = duplicateProperty.GetIdentitySeed(storeObject);
if (seed != duplicateSeed)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnIdentitySeedMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}

break;
case SqlServerValueGenerationStrategy.SequenceHiLo:
if (property.GetHiLoSequenceName(storeObject) != duplicateProperty.GetHiLoSequenceName(storeObject)
|| property.GetHiLoSequenceSchema(storeObject) != duplicateProperty.GetHiLoSequenceSchema(storeObject))
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnSequenceMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}

break;
switch (propertyStrategy)
{
case SqlServerValueGenerationStrategy.IdentityColumn:
var increment = property.GetIdentityIncrement(storeObject);
var duplicateIncrement = duplicateProperty.GetIdentityIncrement(storeObject);
if (increment != duplicateIncrement)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnIdentityIncrementMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}

var seed = property.GetIdentitySeed(storeObject);
var duplicateSeed = duplicateProperty.GetIdentitySeed(storeObject);
if (seed != duplicateSeed)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnIdentitySeedMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}

break;
case SqlServerValueGenerationStrategy.SequenceHiLo:
if (property.GetHiLoSequenceName(storeObject) != duplicateProperty.GetHiLoSequenceName(storeObject)
|| property.GetHiLoSequenceSchema(storeObject) != duplicateProperty.GetHiLoSequenceSchema(storeObject))
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnSequenceMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}

break;
}
}
}

Expand Down
20 changes: 8 additions & 12 deletions src/EFCore/ChangeTracking/Internal/ChangeDetector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,20 +370,16 @@ private void DetectNavigationChange(InternalEntityEntry entry, INavigationBase n
Check.DebugAssert(navigationBase is INavigation, "Issue #21673. Non-collection skip navigations not supported.");

var navigation = (INavigation)navigationBase;
if (!navigation.ForeignKey.IsOwnership
|| !navigation.IsOnDependent)
if (_loggingOptions.IsSensitiveDataLoggingEnabled)
{
if (_loggingOptions.IsSensitiveDataLoggingEnabled)
{
_logger.ReferenceChangeDetectedSensitive(entry, navigation, snapshotValue, currentValue);
}
else
{
_logger.ReferenceChangeDetected(entry, navigation, snapshotValue, currentValue);
}

stateManager.InternalEntityEntryNotifier.NavigationReferenceChanged(entry, navigation, snapshotValue, currentValue);
_logger.ReferenceChangeDetectedSensitive(entry, navigation, snapshotValue, currentValue);
}
else
{
_logger.ReferenceChangeDetected(entry, navigation, snapshotValue, currentValue);
}

stateManager.InternalEntityEntryNotifier.NavigationReferenceChanged(entry, navigation, snapshotValue, currentValue);
}
}
}
Expand Down
36 changes: 9 additions & 27 deletions test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1575,9 +1575,7 @@ public virtual void Shared_columns_are_stored_in_the_snapshot()
modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", b =>
{
b.Property<int>(""Id"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b.Property<int>(""AlternateId"")
.ValueGeneratedOnUpdateSometimes()
Expand Down Expand Up @@ -1838,9 +1836,7 @@ public virtual void Owned_types_are_stored_in_snapshot()
b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", ""EntityWithTwoProperties"", b1 =>
{
b1.Property<int>(""AlternateId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b1.Property<string>(""EntityWithStringKeyId"")
.HasColumnType(""nvarchar(450)"");
Expand Down Expand Up @@ -2063,9 +2059,7 @@ public virtual void Owned_types_are_stored_in_snapshot_when_excluded()
b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+EntityWithTwoProperties"", ""EntityWithTwoProperties"", b1 =>
{
b1.Property<int>(""AlternateId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b1.Property<string>(""EntityWithStringKeyId"")
.HasColumnType(""nvarchar(450)"");
Expand Down Expand Up @@ -2236,9 +2230,7 @@ public virtual void Weak_owned_types_are_stored_in_snapshot()
b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderBillingDetails"", b1 =>
{
b1.Property<int>(""OrderId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b1.HasKey(""OrderId"");
Expand All @@ -2250,9 +2242,7 @@ public virtual void Weak_owned_types_are_stored_in_snapshot()
b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 =>
{
b2.Property<int>(""OrderDetailsOrderId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b2.Property<string>(""City"")
.HasColumnType(""nvarchar(max)"");
Expand All @@ -2271,9 +2261,7 @@ public virtual void Weak_owned_types_are_stored_in_snapshot()
b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderDetails"", ""OrderShippingDetails"", b1 =>
{
b1.Property<int>(""OrderId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b1.HasKey(""OrderId"");
Expand All @@ -2285,9 +2273,7 @@ public virtual void Weak_owned_types_are_stored_in_snapshot()
b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 =>
{
b2.Property<int>(""OrderDetailsOrderId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b2.Property<string>(""City"")
.HasColumnType(""nvarchar(max)"");
Expand All @@ -2306,9 +2292,7 @@ public virtual void Weak_owned_types_are_stored_in_snapshot()
b.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+OrderInfo"", ""OrderInfo"", b1 =>
{
b1.Property<int>(""OrderId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b1.HasKey(""OrderId"");
Expand All @@ -2320,9 +2304,7 @@ public virtual void Weak_owned_types_are_stored_in_snapshot()
b1.OwnsOne(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+StreetAddress"", ""StreetAddress"", b2 =>
{
b2.Property<int>(""OrderInfoOrderId"")
.ValueGeneratedOnAdd()
.HasColumnType(""int"")
.UseIdentityColumn();
.HasColumnType(""int"");
b2.Property<string>(""City"")
.HasColumnType(""nvarchar(max)"");
Expand Down
8 changes: 4 additions & 4 deletions test/EFCore.Relational.Specification.Tests/Query/NullSemanticsQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -723,9 +723,9 @@ public virtual void Where_contains_on_parameter_empty_array_with_relational_null
var names = new string[0];
var result = context.Entities1
.Where(e => names.Contains(e.NullableStringA))
.Select(e => e.NullableStringA).ToList();
.Select(e => e.NullableStringA).ToList().Count;

Assert.Empty(result);
Assert.Equal(0, result);
}

[ConditionalFact]
Expand All @@ -735,9 +735,9 @@ public virtual void Where_contains_on_parameter_array_with_just_null_with_relati
var names = new string[] { null };
var result = context.Entities1
.Where(e => names.Contains(e.NullableStringA))
.Select(e => e.NullableStringA).ToList();
.Select(e => e.NullableStringA).ToList().Count;

Assert.Empty(result);
Assert.Equal(0, result);
}

[ConditionalTheory]
Expand Down
14 changes: 14 additions & 0 deletions test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,7 @@ public override int GetHashCode()
protected class RequiredSingle1 : NotifyingEntity
{
private int _id;
private bool _bool;
private Root _root;
private RequiredSingle2 _single;

Expand All @@ -1422,6 +1423,12 @@ public int Id
set => SetWithNotify(value, ref _id);
}

public bool Bool
{
get => _bool;
set => SetWithNotify(value, ref _bool);
}

public Root Root
{
get => _root;
Expand All @@ -1447,6 +1454,7 @@ public override int GetHashCode()
protected class RequiredSingle2 : NotifyingEntity
{
private int _id;
private bool _bool;
private RequiredSingle1 _back;

public int Id
Expand All @@ -1455,6 +1463,12 @@ public int Id
set => SetWithNotify(value, ref _id);
}

public bool Bool
{
get => _bool;
set => SetWithNotify(value, ref _bool);
}

public RequiredSingle1 Back
{
get => _back;
Expand Down
Loading

0 comments on commit f481b97

Please sign in to comment.