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
Query: Detect parent identifier correctly when parent has owned navs

Fixes #16454
Part of #23211
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Nov 10, 2020
1 parent 2771595 commit 0ba7d14
Show file tree
Hide file tree
Showing 11 changed files with 724 additions and 126 deletions.
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1394,15 +1394,15 @@ public CollectionShaperExpression AddCollectionProjection(
var identifierFromParent = _identifier;
if (innerSelectExpression.Tables.LastOrDefault(e => e is InnerJoinExpression) is InnerJoinExpression
collectionInnerJoinExpression
&& collectionInnerJoinExpression.Table is SelectExpression collectionInnerSelectExpression)
&& collectionInnerJoinExpression.Table is TableExpressionBase collectionTableExpression)
{
// This computes true parent identifier count for correlation.
// The last inner joined table in innerSelectExpression brings collection data.
// Further tables load additional data on the collection (hence uses outer pattern)
// So identifier not coming from there (which would be at the start only) are for correlation with parent.
// Parent can have additional identifier if a owned reference was expanded.
var actualParentIdentifierCount = innerSelectExpression._identifier
.TakeWhile(e => !ReferenceEquals(e.Column.Table, collectionInnerSelectExpression))
.TakeWhile(e => !ReferenceEquals(e.Column.Table, collectionTableExpression))
.Count();
identifierFromParent = _identifier.Take(actualParentIdentifierCount).ToList();
}
Expand Down
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
121 changes: 66 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,74 @@ 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 valueGenerationStrategyAnnotation = ((IConventionProperty)property)
.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy);
var duplicateValueGenerationStrategyAnnotation = ((IConventionProperty)duplicateProperty)
.FindAnnotation(SqlServerAnnotationNames.ValueGenerationStrategy);

switch (propertyStrategy)
if (valueGenerationStrategyAnnotation?.GetConfigurationSource() == ConfigurationSource.Explicit
|| duplicateValueGenerationStrategyAnnotation?.GetConfigurationSource() == ConfigurationSource.Explicit)
{
throw new InvalidOperationException(
SqlServerStrings.DuplicateColumnNameValueGenerationStrategyMismatch(
duplicateProperty.DeclaringEntityType.DisplayName(),
duplicateProperty.Name,
property.DeclaringEntityType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}
}
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
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.Equal(0, result.Count);
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.Equal(0, result.Count);
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 0ba7d14

Please sign in to comment.