From a3cfd3a095f12f33cee647c9fc11f4560f02531e Mon Sep 17 00:00:00 2001 From: AndriySvyryd Date: Fri, 2 Feb 2018 17:46:36 -0800 Subject: [PATCH] Improve SaveChanges circular dependency message Update transient error message Add sensitive data to the conceptual null exception Don't validate mapping for shadow properties created by convention Improve incompatible principal entity during fixup exception Correct property is already a navigation exception Fixes #8363 Fixes #8365 Fixes #9696 Fixes #9817 Fixes #10135 Fixes #10856 --- .../Storage/Internal/InMemoryTable.cs | 2 +- .../Query/InheritanceRelationalTestBase.cs | 1 + .../Update/Internal/CommandBatchPreparer.cs | 178 +++++++++++++++++- .../GraphUpdatesTestBase.cs | 134 ++++++------- .../Query/InheritanceTestBase.cs | 27 +++ .../Properties/SqlServerStrings.Designer.cs | 2 +- .../Properties/SqlServerStrings.resx | 2 +- .../Internal/InternalEntityEntry.cs | 61 ++---- .../Internal/NavigationFixer.cs | 32 +++- .../ChangeTracking/Internal/StateManager.cs | 44 ++++- src/EFCore/Internal/Multigraph.cs | 87 ++++----- .../PropertyMappingValidationConvention.cs | 4 +- src/EFCore/Properties/CoreStrings.Designer.cs | 40 +++- src/EFCore/Properties/CoreStrings.resx | 20 +- .../Update/Internal/UpdateEntryExtensions.cs | 4 +- .../Query/InheritanceInMemoryTest.cs | 6 +- ...tesInMemoryWithSensitiveDataLoggingTest.cs | 2 +- .../Update/CommandBatchPreparerTest.cs | 85 +++++---- .../Internal/InternalClrEntityEntryTest.cs | 1 + .../Internal/InternalEntityEntryTestBase.cs | 61 ++++-- .../Internal/StateManagerTest.cs | 8 +- ...PropertyMappingValidationConventionTest.cs | 15 +- .../Metadata/MetadataBuilderTest.cs | 4 + .../ModelBuilding/ModelBuilderGenericTest.cs | 1 - 24 files changed, 574 insertions(+), 247 deletions(-) diff --git a/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs b/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs index e86a70c5e49..3ee82111a3c 100644 --- a/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs +++ b/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs @@ -145,7 +145,7 @@ protected virtual void ThrowUpdateConcurrencyException([NotNull] IUpdateEntry en entry.EntityType.DisplayName(), entry.BuildCurrentValuesString(entry.EntityType.FindPrimaryKey().Properties), entry.BuildOriginalValuesString(concurrencyConflicts.Keys), - string.Join(", ", concurrencyConflicts.Select(c => c.Key.Name + ":" + c.Value))), + "{" + string.Join(", ", concurrencyConflicts.Select(c => c.Key.Name + ": " + c.Value)) + "}"), new[] { entry }); } diff --git a/src/EFCore.Relational.Specification.Tests/Query/InheritanceRelationalTestBase.cs b/src/EFCore.Relational.Specification.Tests/Query/InheritanceRelationalTestBase.cs index 18567d87ad6..b64a3a19fd1 100644 --- a/src/EFCore.Relational.Specification.Tests/Query/InheritanceRelationalTestBase.cs +++ b/src/EFCore.Relational.Specification.Tests/Query/InheritanceRelationalTestBase.cs @@ -5,6 +5,7 @@ using Microsoft.EntityFrameworkCore.TestModels.Inheritance; using Xunit; +// ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.Query { public abstract class InheritanceRelationalTestBase : InheritanceTestBase diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 4d0bb988962..354e666d2b0 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; @@ -292,10 +293,171 @@ protected virtual IReadOnlyList> TopologicalSort([NotN AddUniqueValueEdges(modificationCommandGraph); - var sortedCommands - = modificationCommandGraph.BatchingTopologicalSort(data => { return string.Join(", ", data.Select(d => d.Item3.First())); }); + return modificationCommandGraph.BatchingTopologicalSort(FormatCycle); + } + + private string FormatCycle(IReadOnlyList>> data) + { + var builder = new StringBuilder(); + for (var i = 0; i < data.Count; i++) + { + var edge = data[i]; + Format(edge.Item1, builder); + + switch (edge.Item3.First()) + { + case IForeignKey foreignKey: + Format(foreignKey, edge.Item1, edge.Item2, builder); + break; + case IIndex index: + Format(index, edge.Item1, edge.Item2, builder); + break; + } + + if (i == data.Count - 1) + { + Format(edge.Item2, builder); + } + } + + return builder.ToString(); + } + + private void Format(ModificationCommand command, StringBuilder builder) + { + var entry = command.Entries.First(); + var entityType = entry.EntityType; + builder.Append(entityType.DisplayName()); + if (_sensitiveLoggingEnabled) + { + builder.Append(" { "); + var properties = entityType.FindPrimaryKey().Properties; + for (var i = 0; i < properties.Count; i++) + { + var keyProperty = properties[i]; + builder.Append("'"); + builder.Append(keyProperty.Name); + builder.Append("': "); + builder.Append(entry.GetCurrentValue(keyProperty)); + + if (i != properties.Count - 1) + { + builder.Append(", "); + } + } + builder.Append(" } "); + } + else + { + builder.Append(" "); + } + + builder.Append("["); + builder.Append(entry.EntityState); + builder.Append("]"); + } + + private void Format(IForeignKey foreignKey, ModificationCommand source, ModificationCommand target, StringBuilder builder) + { + var reverseDependency = !source.Entries.Any(e => foreignKey.DeclaringEntityType.IsAssignableFrom(e.EntityType)); + if (reverseDependency) + { + builder.Append(" <-"); + } + + builder.Append(" "); + if (foreignKey.DependentToPrincipal != null + || foreignKey.PrincipalToDependent != null) + { + if (!reverseDependency + && foreignKey.DependentToPrincipal != null) + { + builder.Append(foreignKey.DependentToPrincipal.Name); + builder.Append(" "); + } + + if (foreignKey.PrincipalToDependent != null) + { + builder.Append(foreignKey.PrincipalToDependent.Name); + builder.Append(" "); + } + + if (reverseDependency + && foreignKey.DependentToPrincipal != null) + { + builder.Append(foreignKey.DependentToPrincipal.Name); + builder.Append(" "); + } + } + else + { + builder.Append("ForeignKey "); + } + + var dependentCommand = reverseDependency ? target : source; + var dependentEntry = dependentCommand.Entries.First(e => foreignKey.DeclaringEntityType.IsAssignableFrom(e.EntityType)); + builder.Append("{ "); + for (var i = 0; i < foreignKey.Properties.Count; i++) + { + var property = foreignKey.Properties[i]; + builder.Append("'"); + builder.Append(property.Name); + builder.Append("'"); + if (_sensitiveLoggingEnabled) + { + builder.Append(": "); + builder.Append(dependentEntry.GetCurrentValue(property)); + } - return sortedCommands; + if (i != foreignKey.Properties.Count - 1) + { + builder.Append(", "); + } + } + builder.Append(" } "); + + if (!reverseDependency) + { + builder.Append("<- "); + } + } + + private void Format(IIndex index, ModificationCommand source, ModificationCommand target, StringBuilder builder) + { + var reverseDependency = source.EntityState != EntityState.Deleted; + if (reverseDependency) + { + builder.Append(" <-"); + } + + builder.Append(" Index "); + + var dependentCommand = reverseDependency ? target : source; + var dependentEntry = dependentCommand.Entries.First(e => index.DeclaringEntityType.IsAssignableFrom(e.EntityType)); + builder.Append("{ "); + for (var i = 0; i < index.Properties.Count; i++) + { + var property = index.Properties[i]; + builder.Append("'"); + builder.Append(property.Name); + builder.Append("'"); + if (_sensitiveLoggingEnabled) + { + builder.Append(": "); + builder.Append(dependentEntry.GetCurrentValue(property)); + } + + if (i != index.Properties.Count - 1) + { + builder.Append(", "); + } + } + builder.Append(" } "); + + if (!reverseDependency) + { + builder.Append("<- "); + } } // Builds a map from foreign key values to list of modification commands, with an entry for every command @@ -452,9 +614,8 @@ private void AddUniqueValueEdges(Multigraph c { var indexColumnModifications = command.ColumnModifications.Where( - cm => - index.Properties.Contains(cm.Property) - && (cm.IsWrite || cm.IsRead)); + cm => index.Properties.Contains(cm.Property) + && (cm.IsWrite || cm.IsRead)); if (command.EntityState == EntityState.Deleted || indexColumnModifications.Any()) @@ -492,9 +653,8 @@ private void AddUniqueValueEdges(Multigraph c { var indexColumnModifications = command.ColumnModifications.Where( - cm => - index.Properties.Contains(cm.Property) - && cm.IsWrite); + cm => index.Properties.Contains(cm.Property) + && cm.IsWrite); if (command.EntityState == EntityState.Added || indexColumnModifications.Any()) diff --git a/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs b/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs index 61e6512c604..eb5de26cf11 100644 --- a/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs +++ b/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs @@ -413,7 +413,7 @@ public virtual void Save_removed_optional_many_to_one_dependents(ChangeMechanism && (changeMechanism & ChangeMechanism.Fk) == 0) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Optional1), nameof(Optional2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Optional1), nameof(Optional2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -497,7 +497,7 @@ public virtual void Save_removed_required_many_to_one_dependents(ChangeMechanism if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Required1), nameof(Required2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Required1), nameof(Required2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -617,7 +617,7 @@ public virtual void Save_changed_optional_one_to_one(ChangeMechanism changeMecha if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(OptionalSingle1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(OptionalSingle1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -731,7 +731,7 @@ public virtual void Save_required_one_to_one_changed_by_reference(ChangeMechanis if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingle1), nameof(RequiredSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingle1), nameof(RequiredSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -896,7 +896,7 @@ public virtual void Save_required_non_PK_one_to_one_changed_by_reference(ChangeM if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(RequiredNonPkSingle1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(RequiredNonPkSingle1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -995,7 +995,7 @@ public virtual void Sever_optional_one_to_one(ChangeMechanism changeMechanism) && (changeMechanism & ChangeMechanism.Fk) == 0) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(OptionalSingle1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(OptionalSingle1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -1070,7 +1070,7 @@ public virtual void Sever_required_one_to_one(ChangeMechanism changeMechanism) if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingle1), nameof(RequiredSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingle1), nameof(RequiredSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -1138,7 +1138,7 @@ public virtual void Sever_required_non_PK_one_to_one(ChangeMechanism changeMecha if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(RequiredNonPkSingle1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(RequiredNonPkSingle1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -1484,7 +1484,7 @@ public virtual void Reparent_to_different_one_to_many(ChangeMechanism changeMech && (changeMechanism & ChangeMechanism.Fk) == 0) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalAk1), nameof(OptionalComposite2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalAk1), nameof(OptionalComposite2), "{Id: 3}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2005,11 +2005,12 @@ public virtual void Save_removed_optional_many_to_one_dependents_with_alternate_ { root = LoadOptionalAkGraph(context); - var childCollection = root.OptionalChildrenAk.First().Children; - var childCompositeCollection = root.OptionalChildrenAk.First().CompositeChildren; - var removed2 = childCollection.First(); - var removed1 = root.OptionalChildrenAk.Skip(1).First(); - var removed2c = childCompositeCollection.First(); + var firstChild = root.OptionalChildrenAk.OrderByDescending(c => c.Id).First(); + var childCollection = firstChild.Children; + var childCompositeCollection = firstChild.CompositeChildren; + var removed2 = childCollection.OrderByDescending(c => c.Id).First(); + var removed1 = root.OptionalChildrenAk.OrderByDescending(c => c.Id).Skip(1).First(); + var removed2c = childCompositeCollection.OrderByDescending(c => c.Id).First(); if ((changeMechanism & ChangeMechanism.Principal) != 0) { @@ -2037,8 +2038,9 @@ public virtual void Save_removed_optional_many_to_one_dependents_with_alternate_ if (Fixture.ForceRestrict && (changeMechanism & ChangeMechanism.Fk) == 0) { + Add(root.OptionalChildrenAk, removed1); Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalAk1), nameof(OptionalAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalAk1), nameof(OptionalAk2), "{Id: 4}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2071,7 +2073,7 @@ public virtual void Save_removed_optional_many_to_one_dependents_with_alternate_ AssertNavigations(loadedRoot); Assert.Equal(1, loadedRoot.OptionalChildrenAk.Count()); - Assert.Equal(1, loadedRoot.OptionalChildrenAk.First().Children.Count()); + Assert.Equal(1, loadedRoot.OptionalChildrenAk.OrderBy(c => c.Id).First().Children.Count()); } }); } @@ -2092,11 +2094,12 @@ public virtual void Save_removed_required_many_to_one_dependents_with_alternate_ { root = LoadRequiredAkGraph(context); - var childCollection = root.RequiredChildrenAk.First().Children; - var childCompositeCollection = root.RequiredChildrenAk.First().CompositeChildren; - removed2 = childCollection.First(); - removed2c = childCompositeCollection.First(); - removed1 = root.RequiredChildrenAk.Skip(1).First(); + var firstChild = root.RequiredChildrenAk.OrderByDescending(c => c.Id).First(); + var childCollection = firstChild.Children; + var childCompositeCollection = firstChild.CompositeChildren; + removed2 = childCollection.OrderBy(c => c.Id).First(); + removed2c = childCompositeCollection.OrderBy(c => c.Id).First(); + removed1 = root.RequiredChildrenAk.OrderByDescending(c => c.Id).Skip(1).First(); if ((changeMechanism & ChangeMechanism.Principal) != 0) { @@ -2121,8 +2124,9 @@ public virtual void Save_removed_required_many_to_one_dependents_with_alternate_ if (Fixture.ForceRestrict) { + Add(root.RequiredChildrenAk, removed1); Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredAk1), nameof(RequiredAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredAk1), nameof(RequiredAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2154,8 +2158,8 @@ public virtual void Save_removed_required_many_to_one_dependents_with_alternate_ Assert.False(context.Set().Any(e => e.Id == removed2c.Id)); Assert.Equal(1, loadedRoot.RequiredChildrenAk.Count()); - Assert.Equal(1, loadedRoot.RequiredChildrenAk.First().Children.Count()); - Assert.Equal(1, loadedRoot.RequiredChildrenAk.First().CompositeChildren.Count()); + Assert.Equal(1, loadedRoot.RequiredChildrenAk.OrderBy(c => c.Id).First().Children.Count()); + Assert.Equal(1, loadedRoot.RequiredChildrenAk.OrderBy(c => c.Id).First().CompositeChildren.Count()); } }); } @@ -2256,7 +2260,7 @@ public virtual void Save_changed_optional_one_to_one_with_alternate_key(ChangeMe if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(OptionalSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(OptionalSingleAk1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2384,7 +2388,7 @@ public virtual void Save_changed_optional_one_to_one_with_alternate_key_in_store if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(OptionalSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(OptionalSingleAk1), "{Id: 1}"), Assert.Throws(() => context2.SaveChanges()).Message); } else @@ -2578,7 +2582,7 @@ public virtual void Save_required_one_to_one_changed_by_reference_with_alternate if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(RequiredSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(RequiredSingleAk1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2726,7 +2730,7 @@ public virtual void Save_required_non_PK_one_to_one_changed_by_reference_with_al if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(RequiredNonPkSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(RequiredNonPkSingleAk1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2827,7 +2831,7 @@ public virtual void Sever_optional_one_to_one_with_alternate_key(ChangeMechanism && (changeMechanism & ChangeMechanism.Fk) == 0) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(OptionalSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(OptionalSingleAk1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2911,7 +2915,7 @@ public virtual void Sever_required_one_to_one_with_alternate_key(ChangeMechanism if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(RequiredSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(RequiredSingleAk1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -2985,7 +2989,7 @@ public virtual void Sever_required_non_PK_one_to_one_with_alternate_key(ChangeMe if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Root), nameof(RequiredNonPkSingleAk1)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Root), nameof(RequiredNonPkSingleAk1), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3318,7 +3322,7 @@ public virtual void Required_many_to_one_dependents_are_cascade_deleted() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Required1), nameof(Required2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Required1), nameof(Required2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3383,7 +3387,7 @@ public virtual void Optional_many_to_one_dependents_are_orphaned() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Optional1), nameof(Optional2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Optional1), nameof(Optional2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3444,7 +3448,7 @@ public virtual void Optional_one_to_one_are_orphaned() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalSingle1), nameof(OptionalSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalSingle1), nameof(OptionalSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3503,7 +3507,7 @@ public virtual void Required_one_to_one_are_cascade_deleted() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingle1), nameof(RequiredSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingle1), nameof(RequiredSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3562,7 +3566,7 @@ public virtual void Required_non_PK_one_to_one_are_cascade_deleted() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredNonPkSingle1), nameof(RequiredNonPkSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredNonPkSingle1), nameof(RequiredNonPkSingle2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3610,7 +3614,7 @@ public virtual void Optional_many_to_one_dependents_with_alternate_key_are_orpha Assert.Equal(2, root.OptionalChildrenAk.Count()); - var removed = root.OptionalChildrenAk.First(); + var removed = root.OptionalChildrenAk.OrderBy(c => c.Id).First(); removedId = removed.Id; var orphaned = removed.Children.ToList(); @@ -3625,7 +3629,7 @@ public virtual void Optional_many_to_one_dependents_with_alternate_key_are_orpha if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalAk1), nameof(OptionalAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalAk1), nameof(OptionalAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3676,7 +3680,7 @@ public virtual void Required_many_to_one_dependents_with_alternate_key_are_casca Assert.Equal(2, root.RequiredChildrenAk.Count()); - var removed = root.RequiredChildrenAk.First(); + var removed = root.RequiredChildrenAk.OrderBy(c => c.Id).First(); removedId = removed.Id; var cascadeRemoved = removed.Children.ToList(); @@ -3694,7 +3698,7 @@ public virtual void Required_many_to_one_dependents_with_alternate_key_are_casca if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredAk1), nameof(RequiredAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredAk1), nameof(RequiredAk2), "{Id: 3}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3760,7 +3764,7 @@ public virtual void Optional_one_to_one_with_alternate_key_are_orphaned() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalSingleAk1), nameof(OptionalSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalSingleAk1), nameof(OptionalSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3825,7 +3829,7 @@ public virtual void Required_one_to_one_with_alternate_key_are_cascade_deleted() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingleAk1), nameof(RequiredSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingleAk1), nameof(RequiredSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -3887,7 +3891,7 @@ public virtual void Required_non_PK_one_to_one_with_alternate_key_are_cascade_de if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredNonPkSingleAk1), nameof(RequiredNonPkSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredNonPkSingleAk1), nameof(RequiredNonPkSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4440,7 +4444,7 @@ public virtual void Optional_many_to_one_dependents_with_alternate_key_are_orpha ExecuteWithStrategyInTransaction( context => { - var removed = LoadOptionalAkGraph(context).OptionalChildrenAk.First(); + var removed = LoadOptionalAkGraph(context).OptionalChildrenAk.OrderBy(c => c.Id).First(); removedId = removed.Id; orphanedIds = removed.Children.Select(e => e.Id).ToList(); @@ -4467,7 +4471,7 @@ public virtual void Optional_many_to_one_dependents_with_alternate_key_are_orpha if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalAk1), nameof(OptionalComposite2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalAk1), nameof(OptionalComposite2), "{Id: 3}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4551,7 +4555,7 @@ public virtual void Optional_one_to_one_with_alternate_key_are_orphaned_in_store if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalSingleAk1), nameof(OptionalSingleComposite2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalSingleAk1), nameof(OptionalSingleComposite2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4621,7 +4625,7 @@ public virtual void Required_many_to_one_dependents_are_cascade_deleted_starting if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Required1), nameof(Required2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Required1), nameof(Required2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4686,7 +4690,7 @@ public virtual void Optional_many_to_one_dependents_are_orphaned_starting_detach if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Optional1), nameof(Optional2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Optional1), nameof(Optional2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4744,7 +4748,7 @@ public virtual void Optional_one_to_one_are_orphaned_starting_detached() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalSingle1), nameof(OptionalSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalSingle1), nameof(OptionalSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4801,7 +4805,7 @@ public virtual void Required_one_to_one_are_cascade_deleted_starting_detached() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingle1), nameof(RequiredSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingle1), nameof(RequiredSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4857,7 +4861,7 @@ public virtual void Required_non_PK_one_to_one_are_cascade_deleted_starting_deta if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredNonPkSingle1), nameof(RequiredNonPkSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredNonPkSingle1), nameof(RequiredNonPkSingle2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4904,7 +4908,7 @@ public virtual void Optional_many_to_one_dependents_with_alternate_key_are_orpha }, context => { - var removed = root.OptionalChildrenAk.First(); + var removed = root.OptionalChildrenAk.OrderBy(c => c.Id).First(); removedId = removed.Id; var orphaned = removed.Children.ToList(); @@ -4926,7 +4930,7 @@ public virtual void Optional_many_to_one_dependents_with_alternate_key_are_orpha if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalAk1), nameof(OptionalAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalAk1), nameof(OptionalAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -4976,7 +4980,7 @@ public virtual void Required_many_to_one_dependents_with_alternate_key_are_casca }, context => { - var removed = root.RequiredChildrenAk.First(); + var removed = root.RequiredChildrenAk.OrderBy(c => c.Id).First(); removedId = removed.Id; var cascadeRemoved = removed.Children.ToList(); @@ -4997,7 +5001,7 @@ public virtual void Required_many_to_one_dependents_with_alternate_key_are_casca if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredAk1), nameof(RequiredAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredAk1), nameof(RequiredAk2), "{Id: 3}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5061,7 +5065,7 @@ public virtual void Optional_one_to_one_with_alternate_key_are_orphaned_starting if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(OptionalSingleAk1), nameof(OptionalSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(OptionalSingleAk1), nameof(OptionalSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5124,7 +5128,7 @@ public virtual void Required_one_to_one_with_alternate_key_are_cascade_deleted_s if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingleAk1), nameof(RequiredSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingleAk1), nameof(RequiredSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5183,7 +5187,7 @@ public virtual void Required_non_PK_one_to_one_with_alternate_key_are_cascade_de if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredNonPkSingleAk1), nameof(RequiredNonPkSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredNonPkSingleAk1), nameof(RequiredNonPkSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5257,7 +5261,7 @@ public virtual void Required_many_to_one_dependents_are_cascade_detached_when_Ad if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(Required1), nameof(Required2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(Required1), nameof(Required2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5321,7 +5325,7 @@ public virtual void Required_one_to_one_are_cascade_detached_when_Added() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingle1), nameof(RequiredSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingle1), nameof(RequiredSingle2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5383,7 +5387,7 @@ public virtual void Required_non_PK_one_to_one_are_cascade_detached_when_Added() if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredNonPkSingle1), nameof(RequiredNonPkSingle2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredNonPkSingle1), nameof(RequiredNonPkSingle2), "{Id: 2}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5427,7 +5431,7 @@ public virtual void Required_many_to_one_dependents_with_alternate_key_are_casca Assert.Equal(2, root.RequiredChildrenAk.Count()); - var removed = root.RequiredChildrenAk.First(); + var removed = root.RequiredChildrenAk.OrderBy(c => c.Id).First(); removedId = removed.Id; var cascadeRemoved = removed.Children.ToList(); @@ -5467,7 +5471,7 @@ public virtual void Required_many_to_one_dependents_with_alternate_key_are_casca if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredAk1), nameof(RequiredAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredAk1), nameof(RequiredAk2), "{Id: 3}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5540,7 +5544,7 @@ public virtual void Required_one_to_one_with_alternate_key_are_cascade_detached_ if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredSingleAk1), nameof(RequiredSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredSingleAk1), nameof(RequiredSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else @@ -5604,7 +5608,7 @@ public virtual void Required_non_PK_one_to_one_with_alternate_key_are_cascade_de if (Fixture.ForceRestrict) { Assert.Equal( - CoreStrings.RelationshipConceptualNull(nameof(RequiredNonPkSingleAk1), nameof(RequiredNonPkSingleAk2)), + CoreStrings.RelationshipConceptualNullSensitive(nameof(RequiredNonPkSingleAk1), nameof(RequiredNonPkSingleAk2), "{Id: 1}"), Assert.Throws(() => context.SaveChanges()).Message); } else diff --git a/src/EFCore.Specification.Tests/Query/InheritanceTestBase.cs b/src/EFCore.Specification.Tests/Query/InheritanceTestBase.cs index 470b8a24bce..b6cf98c0a92 100644 --- a/src/EFCore.Specification.Tests/Query/InheritanceTestBase.cs +++ b/src/EFCore.Specification.Tests/Query/InheritanceTestBase.cs @@ -1,8 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Linq; using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.TestModels.Inheritance; using Microsoft.EntityFrameworkCore.TestUtilities; @@ -524,6 +526,31 @@ public virtual void Can_union_kiwis_and_eagles_as_birds() } } + [Fact] + public virtual void Setting_foreign_key_to_a_different_type_throws() + { + using (var context = CreateContext()) + { + var kiwi = context.Set().Single(); + + var eagle = new Eagle + { + Species = "Haliaeetus leucocephalus", + Name = "Bald eagle", + Group = EagleGroup.Booted, + EagleId = kiwi.Species + }; + + Assert.Equal(CoreStrings.IncompatiblePrincipalEntrySensitive( + "{EagleId: Apteryx haastii}", + nameof(Eagle), + "{Species: Haliaeetus leucocephalus}", + nameof(Kiwi), + nameof(Eagle)), + Assert.Throws(() => context.Add(eagle)).Message); + } + } + protected InheritanceContext CreateContext() => Fixture.CreateContext(); protected virtual void ClearLog() diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs index 216f9d08a07..b3b246acead 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs @@ -61,7 +61,7 @@ public static string AlterIdentityColumn => GetString("AlterIdentityColumn"); /// - /// An exception has been raised that is likely due to a transient failure. If you are connecting to a SQL Azure database consider using SqlAzureExecutionStrategy. + /// An exception has been raised that is likely due to a transient failure. Consider enabling transient error resiliency by adding 'EnableRetryOnFailure()' to the 'UseSqlServer' call. /// public static string TransientExceptionDetected => GetString("TransientExceptionDetected"); diff --git a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx index dbd9c3ccbd9..e1764eaf41c 100644 --- a/src/EFCore.SqlServer/Properties/SqlServerStrings.resx +++ b/src/EFCore.SqlServer/Properties/SqlServerStrings.resx @@ -136,7 +136,7 @@ To change the IDENTITY property of a column, the column needs to be dropped and recreated. - An exception has been raised that is likely due to a transient failure. If you are connecting to a SQL Azure database consider using SqlAzureExecutionStrategy. + An exception has been raised that is likely due to a transient failure. Consider enabling transient error resiliency by adding 'EnableRetryOnFailure()' to the 'UseSqlServer' call. No type was specified for the decimal column '{property}' on entity type '{entityType}'. This will cause values to be silently truncated if they do not fit in the default precision and scale. Explicitly specify the SQL server column type that can accommodate all the values using 'ForHasColumnType()'. diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs index 5e75577861b..199a0416684 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -15,6 +15,7 @@ using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Update; +using Microsoft.EntityFrameworkCore.Update.Internal; namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal { @@ -918,7 +919,7 @@ public virtual InternalEntityEntry PrepareToSave() /// This API supports the Entity Framework Core infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. /// - public virtual void HandleConceptualNulls() + public virtual void HandleConceptualNulls(bool sensitiveLoggingEnabled) { var fks = new List(); foreach (var foreignKey in EntityType.GetForeignKeys()) @@ -964,6 +965,15 @@ public virtual void HandleConceptualNulls() } else if (fks.Any()) { + if (sensitiveLoggingEnabled) + { + throw new InvalidOperationException( + CoreStrings.RelationshipConceptualNullSensitive( + fks.First().PrincipalEntityType.DisplayName(), + EntityType.DisplayName(), + this.BuildOriginalValuesString(EntityType.FindPrimaryKey().Properties))); + } + throw new InvalidOperationException( CoreStrings.RelationshipConceptualNull( fks.First().PrincipalEntityType.DisplayName(), @@ -978,6 +988,15 @@ public virtual void HandleConceptualNulls() if (property != null) { + if (sensitiveLoggingEnabled) + { + throw new InvalidOperationException( + CoreStrings.PropertyConceptualNullSensitive( + property.Name, + EntityType.DisplayName(), + this.BuildOriginalValuesString(EntityType.FindPrimaryKey().Properties))); + } + throw new InvalidOperationException( CoreStrings.PropertyConceptualNull( property.Name, @@ -986,46 +1005,6 @@ public virtual void HandleConceptualNulls() } } - /// - /// This API supports the Entity Framework Core infrastructure and is not intended to be used - /// directly from your code. This API may change or be removed in future releases. - /// - public virtual void CascadeDelete() - { - foreach (var fk in EntityType.GetReferencingForeignKeys()) - { - foreach (var dependent in (StateManager.GetDependentsFromNavigation(this, fk) - ?? StateManager.GetDependents(this, fk)).ToList()) - { - if ((dependent.EntityState != EntityState.Deleted) - && (dependent.EntityState != EntityState.Detached)) - { - if (fk.DeleteBehavior == DeleteBehavior.Cascade) - { - dependent.SetEntityState( - dependent.EntityState == EntityState.Added - ? EntityState.Detached - : EntityState.Deleted); - - dependent.CascadeDelete(); - } - else - { - foreach (var dependentProperty in fk.Properties) - { - dependent[dependentProperty] = null; - } - - if (dependent.HasConceptualNull) - { - dependent.HandleConceptualNulls(); - } - } - } - } - } - } - /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index eb8c5fb0feb..9bba70e8db3 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -1,12 +1,17 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Linq; using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; +using Microsoft.EntityFrameworkCore.Update.Internal; namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal { @@ -18,6 +23,7 @@ public class NavigationFixer : INavigationFixer { private readonly IChangeDetector _changeDetector; private readonly IEntityGraphAttacher _attacher; + private readonly bool _sensitiveLoggingEnabled; private bool _inFixup; /// @@ -26,10 +32,15 @@ public class NavigationFixer : INavigationFixer /// public NavigationFixer( [NotNull] IChangeDetector changeDetector, - [NotNull] IEntityGraphAttacher attacher) + [NotNull] IEntityGraphAttacher attacher, + [NotNull] ILoggingOptions loggingOptions) { _changeDetector = changeDetector; _attacher = attacher; + if (loggingOptions.IsSensitiveDataLoggingEnabled) + { + _sensitiveLoggingEnabled = true; + } } /// @@ -521,6 +532,25 @@ private void InitialFixup( var principalEntry = stateManager.GetPrincipal(entry, foreignKey); if (principalEntry != null) { + if (!foreignKey.PrincipalEntityType.IsAssignableFrom(principalEntry.EntityType)) + { + if (_sensitiveLoggingEnabled) + { + throw new InvalidOperationException(CoreStrings.IncompatiblePrincipalEntrySensitive( + entry.BuildCurrentValuesString(foreignKey.Properties), + entityType.DisplayName(), + entry.BuildOriginalValuesString(entityType.FindPrimaryKey().Properties), + principalEntry.EntityType.DisplayName(), + foreignKey.PrincipalEntityType.DisplayName())); + } + + throw new InvalidOperationException(CoreStrings.IncompatiblePrincipalEntry( + Property.Format(foreignKey.Properties), + entityType.DisplayName(), + principalEntry.EntityType.DisplayName(), + foreignKey.PrincipalEntityType.DisplayName())); + } + // Set navigation to principal based on FK properties SetNavigation(entry, foreignKey.DependentToPrincipal, principalEntry); diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 2e884d71155..af1509220a9 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -790,12 +790,12 @@ public virtual IReadOnlyList GetInternalEntriesToSave() || e.EntityState == EntityState.Added) && e.HasConceptualNull).ToList()) { - entry.HandleConceptualNulls(); + entry.HandleConceptualNulls(_sensitiveLoggingEnabled); } foreach (var entry in Entries.Where(e => e.EntityState == EntityState.Deleted).ToList()) { - entry.CascadeDelete(); + CascadeDelete(entry); } return Entries @@ -807,6 +807,46 @@ public virtual IReadOnlyList GetInternalEntriesToSave() .ToList(); } + /// + /// This API supports the Entity Framework Core infrastructure and is not intended to be used + /// directly from your code. This API may change or be removed in future releases. + /// + private void CascadeDelete(InternalEntityEntry entry) + { + foreach (var fk in entry.EntityType.GetReferencingForeignKeys()) + { + foreach (var dependent in (GetDependentsFromNavigation(entry, fk) + ?? GetDependents(entry, fk)).ToList()) + { + if ((dependent.EntityState != EntityState.Deleted) + && (dependent.EntityState != EntityState.Detached)) + { + if (fk.DeleteBehavior == DeleteBehavior.Cascade) + { + dependent.SetEntityState( + dependent.EntityState == EntityState.Added + ? EntityState.Detached + : EntityState.Deleted); + + CascadeDelete(dependent); + } + else + { + foreach (var dependentProperty in fk.Properties) + { + dependent[dependentProperty] = null; + } + + if (dependent.HasConceptualNull) + { + dependent.HandleConceptualNulls(_sensitiveLoggingEnabled); + } + } + } + } + } + } + /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. diff --git a/src/EFCore/Internal/Multigraph.cs b/src/EFCore/Internal/Multigraph.cs index 9a748c9b9bb..d987e73892a 100644 --- a/src/EFCore/Internal/Multigraph.cs +++ b/src/EFCore/Internal/Multigraph.cs @@ -15,7 +15,8 @@ namespace Microsoft.EntityFrameworkCore.Internal public class Multigraph : Graph { private readonly HashSet _vertices = new HashSet(); - private readonly Dictionary>> _successorMap = new Dictionary>>(); + private readonly Dictionary>> _successorMap = + new Dictionary>>(); /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used @@ -125,7 +126,7 @@ public virtual IReadOnlyList TopologicalSort( /// public virtual IReadOnlyList TopologicalSort( [CanBeNull] Func, bool> canBreakEdge, - [CanBeNull] Func>>, string> formatCycle) + [CanBeNull] Func>>, string> formatCycle) { var sortedQueue = new List(); var predecessorCounts = new Dictionary(); @@ -234,31 +235,36 @@ public virtual IReadOnlyList TopologicalSort( } cycle.Reverse(); - // Throw an exception - if (formatCycle == null) - { - throw new InvalidOperationException( - CoreStrings.CircularDependency( - cycle.Select(ToString).Join(" -> "))); - } - // Build the cycle message data - currentCycleVertex = cycle.First(); - var cycleData = new List>>(); - - foreach (var vertex in cycle.Skip(1)) - { - cycleData.Add(Tuple.Create(currentCycleVertex, vertex, GetEdges(currentCycleVertex, vertex))); - currentCycleVertex = vertex; - } - throw new InvalidOperationException( - CoreStrings.CircularDependency( - formatCycle(cycleData))); + ThrowCycle(cycle, formatCycle); } } } return sortedQueue; } + private void ThrowCycle(List cycle, Func>>, string> formatCycle) + { + string cycleString; + if (formatCycle == null) + { + cycleString = cycle.Select(ToString).Join(" -> "); + } + else + { + var currentCycleVertex = cycle.First(); + var cycleData = new List>>(); + + foreach (var vertex in cycle.Skip(1)) + { + cycleData.Add(Tuple.Create(currentCycleVertex, vertex, GetEdges(currentCycleVertex, vertex))); + currentCycleVertex = vertex; + } + cycleString = formatCycle(cycleData); + } + + throw new InvalidOperationException(CoreStrings.CircularDependency(cycleString)); + } + /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. @@ -271,7 +277,7 @@ public virtual IReadOnlyList> BatchingTopologicalSort() /// directly from your code. This API may change or be removed in future releases. /// public virtual IReadOnlyList> BatchingTopologicalSort( - [CanBeNull] Func>>, string> formatCycle) + [CanBeNull] Func>>, string> formatCycle) { var currentRootsQueue = new List(); var predecessorCounts = new Dictionary(); @@ -337,14 +343,17 @@ public virtual IReadOnlyList> BatchingTopologicalSort( { var currentCycleVertex = _vertices.First( v => + { + if (predecessorCounts.TryGetValue(v, out var predecessorNumber)) { - if (predecessorCounts.TryGetValue(v, out var predecessorNumber)) - { - return predecessorNumber != 0; - } - return false; - }); - var cyclicWalk = new List { currentCycleVertex }; + return predecessorNumber != 0; + } + return false; + }); + var cyclicWalk = new List + { + currentCycleVertex + }; var finished = false; while (!finished) { @@ -384,25 +393,7 @@ public virtual IReadOnlyList> BatchingTopologicalSort( } cycle.Add(startingVertex); - // Throw an exception - if (formatCycle == null) - { - throw new InvalidOperationException( - CoreStrings.CircularDependency( - cycle.Select(ToString).Join(" -> "))); - } - // Build the cycle message data - currentCycleVertex = cycle.First(); - var cycleData = new List>>(); - - foreach (var vertex in cycle.Skip(1)) - { - cycleData.Add(Tuple.Create(currentCycleVertex, vertex, GetEdges(currentCycleVertex, vertex))); - currentCycleVertex = vertex; - } - throw new InvalidOperationException( - CoreStrings.CircularDependency( - formatCycle(cycleData))); + ThrowCycle(cycle, formatCycle); } return result; diff --git a/src/EFCore/Metadata/Conventions/Internal/PropertyMappingValidationConvention.cs b/src/EFCore/Metadata/Conventions/Internal/PropertyMappingValidationConvention.cs index cd581e4804d..1e7d1f734dc 100644 --- a/src/EFCore/Metadata/Conventions/Internal/PropertyMappingValidationConvention.cs +++ b/src/EFCore/Metadata/Conventions/Internal/PropertyMappingValidationConvention.cs @@ -47,7 +47,9 @@ public virtual InternalModelBuilder Apply(InternalModelBuilder modelBuilder) foreach (var entityType in modelBuilder.Metadata.GetEntityTypes()) { - var unmappedProperty = entityType.GetProperties().FirstOrDefault(p => !IsMappedPrimitiveProperty(p)); + var unmappedProperty = entityType.GetProperties().FirstOrDefault(p => + (!ConfigurationSource.Convention.Overrides(p.GetConfigurationSource()) || !p.IsShadowProperty) + && !IsMappedPrimitiveProperty(p)); if (unmappedProperty != null) { diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 5a360db82c8..07f28fa758c 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -693,7 +693,7 @@ public static string KeyPropertyMustBeReadOnly([CanBeNull] object property, [Can property, entityType); /// - /// The association between entity types '{firstType}' and '{secondType}' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes. + /// The association between entity types '{firstType}' and '{secondType}' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. /// public static string RelationshipConceptualNull([CanBeNull] object firstType, [CanBeNull] object secondType) => string.Format( @@ -701,7 +701,7 @@ public static string RelationshipConceptualNull([CanBeNull] object firstType, [C firstType, secondType); /// - /// The property '{property}' on entity type '{entityType}' is marked as null, but this cannot be saved because the property is marked as required. + /// The property '{property}' on entity type '{entityType}' is marked as null, but this cannot be saved because the property is marked as required. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. /// public static string PropertyConceptualNull([CanBeNull] object property, [CanBeNull] object entityType) => string.Format( @@ -1577,7 +1577,7 @@ public static string ExecutionStrategyExistingTransaction([CanBeNull] object str strategy, getExecutionStrategyMethod); /// - /// Cannot call Property for the property '{property}' on entity type '{entityType}' because it is configured as a navigation property. Property can only be used to configure scalar properties. + /// '{property}' cannot be used as a property on entity type '{entityType}' because it is configured as a navigation. /// public static string PropertyCalledOnNavigation([CanBeNull] object property, [CanBeNull] object entityType) => string.Format( @@ -1911,7 +1911,7 @@ public static string SeedDatumIncompatibleValueSensitive([CanBeNull] object enti entityType, value, property, type); /// - /// The seed entity for entity type '{entityType}' cannot be added because the was no value provided for the required property '{property}'. + /// The seed entity for entity type '{entityType}' cannot be added because the was no value provided for the required property '{property}'. /// public static string SeedDatumMissingValue([CanBeNull] object entityType, [CanBeNull] object property) => string.Format( @@ -2256,6 +2256,38 @@ public static readonly EventDefinition L CoreEventId.NonOwnershipInverseNavigation, _resourceManager.GetString("LogNonOwnershipInverseNavigation"))); + /// + /// The property '{property}' is marked as null on entity '{entityType}' with the key value '{keyValue}', but this cannot be saved because the property is marked as required. + /// + public static string PropertyConceptualNullSensitive([CanBeNull] object property, [CanBeNull] object entityType, [CanBeNull] object keyValue) + => string.Format( + GetString("PropertyConceptualNullSensitive", nameof(property), nameof(entityType), nameof(keyValue)), + property, entityType, keyValue); + + /// + /// The association between entities '{firstType}' and '{secondType}' with the key value '{secondKeyValue}' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes. + /// + public static string RelationshipConceptualNullSensitive([CanBeNull] object firstType, [CanBeNull] object secondType, [CanBeNull] object secondKeyValue) + => string.Format( + GetString("RelationshipConceptualNullSensitive", nameof(firstType), nameof(secondType), nameof(secondKeyValue)), + firstType, secondType, secondKeyValue); + + /// + /// The foreign key {foreignKey} set on '{dependentEntityType}' matches an entity of type '{foundPrincipalEntityType}', however the principal entity type should be assignable to '{principalEntityType}'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. + /// + public static string IncompatiblePrincipalEntry([CanBeNull] object foreignKey, [CanBeNull] object dependentEntityType, [CanBeNull] object foundPrincipalEntityType, [CanBeNull] object principalEntityType) + => string.Format( + GetString("IncompatiblePrincipalEntry", nameof(foreignKey), nameof(dependentEntityType), nameof(foundPrincipalEntityType), nameof(principalEntityType)), + foreignKey, dependentEntityType, foundPrincipalEntityType, principalEntityType); + + /// + /// The foreign key '{foreignKeyValues}' set on '{dependentEntityType}' with the key value '{keyValue}' matches an entity of type '{foundPrincipalEntityType}', however the principal entity type should be assignable to '{principalEntityType}'. + /// + public static string IncompatiblePrincipalEntrySensitive([CanBeNull] object foreignKeyValues, [CanBeNull] object dependentEntityType, [CanBeNull] object keyValue, [CanBeNull] object foundPrincipalEntityType, [CanBeNull] object principalEntityType) + => string.Format( + GetString("IncompatiblePrincipalEntrySensitive", nameof(foreignKeyValues), nameof(dependentEntityType), nameof(keyValue), nameof(foundPrincipalEntityType), nameof(principalEntityType)), + foreignKeyValues, dependentEntityType, keyValue, foundPrincipalEntityType, principalEntityType); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index dc00114e36e..17e074d231b 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -372,10 +372,10 @@ The property '{property}' on entity type '{entityType}' must be marked as read-only after it has been saved because it is part of a key. Key properties are always read-only once an entity has been saved for the first time. - The association between entity types '{firstType}' and '{secondType}' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes. + The association between entity types '{firstType}' and '{secondType}' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. - The property '{property}' on entity type '{entityType}' is marked as null, but this cannot be saved because the property is marked as required. + The property '{property}' on entity type '{entityType}' is marked as null, but this cannot be saved because the property is marked as required. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. The foreign key {foreignKey} cannot be added to the entity type '{entityType}' because a foreign key on the same properties already exists on entity type '{duplicateEntityType}' and also targets the key {key} on '{principalType}'. @@ -708,7 +708,7 @@ The configured execution strategy '{strategy}' does not support user initiated transactions. Use the execution strategy returned by '{getExecutionStrategyMethod}' to execute all the operations in the transaction as a retriable unit. - Cannot call Property for the property '{property}' on entity type '{entityType}' because it is configured as a navigation property. Property can only be used to configure scalar properties. + '{property}' cannot be used as a property on entity type '{entityType}' because it is configured as a navigation. Query: '{queryModel}' uses a row limiting operation (Skip/Take) without OrderBy which may lead to unpredictable results. @@ -833,7 +833,7 @@ The seed entity for entity type '{entityType}' cannot be added because the value '{value}' provided for the property '{property}' is not of the type '{type}'. - The seed entity for entity type '{entityType}' cannot be added because the was no value provided for the required property '{property}'. + The seed entity for entity type '{entityType}' cannot be added because the was no value provided for the required property '{property}'. The seed entity for entity type '{entityType}' cannot be added because it has the navigation '{navigation}' set. To seed relationships you need to add the related entity seed to '{relatedEntityType}' and specify the foreign key values {foreignKeyProperties}. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the involved property values. @@ -954,4 +954,16 @@ The navigation '{targetEntityType}.{inverseNavigation}' cannot be used as the inverse of '{ownedEntityType}.{navigation}' because it's not the ownership navigation '{ownershipNavigation}' Warning CoreEventId.NonOwnershipInverseNavigation string string string string string + + The property '{property}' is marked as null on entity '{entityType}' with the key value '{keyValue}', but this cannot be saved because the property is marked as required. + + + The association between entities '{firstType}' and '{secondType}' with the key value '{secondKeyValue}' has been severed but the foreign key for this relationship cannot be set to null. If the dependent entity should be deleted, then setup the relationship to use cascade deletes. + + + The foreign key {foreignKey} set on '{dependentEntityType}' matches an entity of type '{foundPrincipalEntityType}', however the principal entity type should be assignable to '{principalEntityType}'. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values. + + + The foreign key '{foreignKeyValues}' set on '{dependentEntityType}' with the key value '{keyValue}' matches an entity of type '{foundPrincipalEntityType}', however the principal entity type should be assignable to '{principalEntityType}'. + \ No newline at end of file diff --git a/src/EFCore/Update/Internal/UpdateEntryExtensions.cs b/src/EFCore/Update/Internal/UpdateEntryExtensions.cs index 4ff13676054..9505c5bb5eb 100644 --- a/src/EFCore/Update/Internal/UpdateEntryExtensions.cs +++ b/src/EFCore/Update/Internal/UpdateEntryExtensions.cs @@ -19,13 +19,13 @@ public static class UpdateEntryExtensions /// directly from your code. This API may change or be removed in future releases. /// public static string BuildCurrentValuesString([NotNull] this IUpdateEntry entry, [NotNull] IEnumerable properties) - => string.Join(", ", properties.Select(p => p.Name + ":" + entry.GetCurrentValue(p))); + => "{" + string.Join(", ", properties.Select(p => p.Name + ": " + entry.GetCurrentValue(p))) + "}"; /// /// This API supports the Entity Framework Core infrastructure and is not intended to be used /// directly from your code. This API may change or be removed in future releases. /// public static string BuildOriginalValuesString([NotNull] this IUpdateEntry entry, [NotNull] IEnumerable properties) - => string.Join(", ", properties.Select(p => p.Name + ":" + entry.GetOriginalValue(p))); + => "{" + string.Join(", ", properties.Select(p => p.Name + ": " + entry.GetOriginalValue(p))) + "}"; } } diff --git a/test/EFCore.InMemory.FunctionalTests/Query/InheritanceInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/InheritanceInMemoryTest.cs index 8acfd8c110e..1f8ae7ddf69 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/InheritanceInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/InheritanceInMemoryTest.cs @@ -21,8 +21,7 @@ public override void Discriminator_used_when_projection_over_derived_type2() Assert.Equal( CoreStrings.PropertyNotFound(property: "Discriminator", entityType: "Bird"), Assert.Throws( - () => - base.Discriminator_used_when_projection_over_derived_type2()).Message); + () => base.Discriminator_used_when_projection_over_derived_type2()).Message); } public override void Discriminator_with_cast_in_shadow_property() @@ -30,8 +29,7 @@ public override void Discriminator_with_cast_in_shadow_property() Assert.Equal( CoreStrings.PropertyNotFound(property: "Discriminator", entityType: "Animal"), Assert.Throws( - () => - base.Discriminator_with_cast_in_shadow_property()).Message); + () => base.Discriminator_with_cast_in_shadow_property()).Message); } } } diff --git a/test/EFCore.InMemory.FunctionalTests/UpdatesInMemoryWithSensitiveDataLoggingTest.cs b/test/EFCore.InMemory.FunctionalTests/UpdatesInMemoryWithSensitiveDataLoggingTest.cs index 2918d0c9c8f..5ebc39f7bb5 100644 --- a/test/EFCore.InMemory.FunctionalTests/UpdatesInMemoryWithSensitiveDataLoggingTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/UpdatesInMemoryWithSensitiveDataLoggingTest.cs @@ -14,7 +14,7 @@ public UpdatesInMemoryWithSensitiveDataLoggingTest(UpdatesInMemoryWithSensitiveD #if !Test20 protected override string UpdateConcurrencyTokenMessage - => InMemoryStrings.UpdateConcurrencyTokenExceptionSensitive("Product", "Id:984ade3c-2f7b-4651-a351-642e92ab7146", "Price:3.49", "Price:1.49"); + => InMemoryStrings.UpdateConcurrencyTokenExceptionSensitive("Product", "{Id: 984ade3c-2f7b-4651-a351-642e92ab7146}", "{Price: 3.49}", "{Price: 1.49}"); #endif } } diff --git a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs index 98ad66d4d97..e43f3d2be2e 100644 --- a/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs +++ b/test/EFCore.Relational.Tests/Update/CommandBatchPreparerTest.cs @@ -332,8 +332,10 @@ public void BatchCommands_throws_on_non_store_generated_temporary_values() Assert.Throws(() => CreateCommandBatchPreparer().BatchCommands(new[] { entry }).ToList()).Message); } - [Fact] - public void Batch_command_throws_on_commands_with_circular_dependencies() + [InlineData(true)] + [InlineData(false)] + [Theory] + public void Batch_command_throws_on_commands_with_circular_dependencies(bool sensitiveLogging) { var model = CreateCyclicFKModel(); var configuration = CreateContextServices(model); @@ -345,18 +347,21 @@ public void Batch_command_throws_on_commands_with_circular_dependencies() var relatedFakeEntry = stateManager.GetOrCreateEntry(new RelatedFakeEntity { Id = 1, RelatedId = 42 }); relatedFakeEntry.SetEntityState(EntityState.Added); + var expectedCycle = sensitiveLogging + ? "FakeEntity { 'Id': 42 } [Added] <- ForeignKey { 'RelatedId': 42 } RelatedFakeEntity { 'Id': 1 } [Added] <- ForeignKey { 'RelatedId': 1 } FakeEntity { 'Id': 42 } [Added]" + : "FakeEntity [Added] <- ForeignKey { 'RelatedId' } RelatedFakeEntity [Added] <- ForeignKey { 'RelatedId' } FakeEntity [Added]"; + Assert.Equal( - CoreStrings.CircularDependency( - string.Join( - ", ", - model.FindEntityType(typeof(RelatedFakeEntity)).GetForeignKeys().First(), - model.FindEntityType(typeof(FakeEntity)).GetForeignKeys().First())), + CoreStrings.CircularDependency(expectedCycle), Assert.Throws( - () => CreateCommandBatchPreparer().BatchCommands(new[] { fakeEntry, relatedFakeEntry }).ToArray()).Message); + () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: sensitiveLogging) + .BatchCommands(new[] { fakeEntry, relatedFakeEntry }).ToArray()).Message); } - [Fact] - public void Batch_command_throws_on_commands_with_circular_dependencies_including_indexes() + [InlineData(true)] + [InlineData(false)] + [Theory] + public void Batch_command_throws_on_commands_with_circular_dependencies_including_indexes(bool sensitiveLogging) { var model = CreateCyclicFKModel(); var configuration = CreateContextServices(model); @@ -373,41 +378,43 @@ public void Batch_command_throws_on_commands_with_circular_dependencies_includin fakeEntry2.SetOriginalValue(fakeEntry2.EntityType.FindProperty(nameof(FakeEntity.UniqueValue)), "Test"); fakeEntry2.SetPropertyModified(fakeEntry2.EntityType.FindPrimaryKey().Properties.Single(), isModified: false); + var expectedCycle = sensitiveLogging + ? "FakeEntity { 'Id': 42 } [Added] <- ForeignKey { 'RelatedId': 42 } RelatedFakeEntity { 'Id': 1 } [Added] <- ForeignKey { 'RelatedId': 1 } FakeEntity { 'Id': 2 } [Modified] <- Index { 'UniqueValue': Test } FakeEntity { 'Id': 42 } [Added]" + : "FakeEntity [Added] <- ForeignKey { 'RelatedId' } RelatedFakeEntity [Added] <- ForeignKey { 'RelatedId' } FakeEntity [Modified] <- Index { 'UniqueValue' } FakeEntity [Added]"; + Assert.Equal( - CoreStrings.CircularDependency( - string.Join( - ", ", - model.FindEntityType(typeof(RelatedFakeEntity)).GetForeignKeys().Single(), - model.FindEntityType(typeof(FakeEntity)).GetForeignKeys().Single(), - model.FindEntityType(typeof(FakeEntity)).GetIndexes().Single(i => i.Properties.Any(p => p.Name == nameof(FakeEntity.UniqueValue))))), + CoreStrings.CircularDependency(expectedCycle), Assert.Throws( - () => CreateCommandBatchPreparer().BatchCommands(new[] { fakeEntry, relatedFakeEntry, fakeEntry2 }).ToArray()).Message); + () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: sensitiveLogging) + .BatchCommands(new[] { fakeEntry, relatedFakeEntry, fakeEntry2 }).ToArray()).Message); } - [Fact] - public void Batch_command_shows_correct_cycle_when_circular_dependencies() + [InlineData(true)] + [InlineData(false)] + [Theory] + public void Batch_command_throws_on_delete_commands_with_circular_dependencies(bool sensitiveLogging) { var model = CreateCyclicFkWithTailModel(); var configuration = CreateContextServices(model); var stateManager = configuration.GetRequiredService(); var fakeEntry = stateManager.GetOrCreateEntry(new FakeEntity { Id = 1, RelatedId = 2 }); - fakeEntry.SetEntityState(EntityState.Added); + fakeEntry.SetEntityState(EntityState.Deleted); var relatedFakeEntry = stateManager.GetOrCreateEntry(new RelatedFakeEntity { Id = 2, RelatedId = 1 }); - relatedFakeEntry.SetEntityState(EntityState.Added); + relatedFakeEntry.SetEntityState(EntityState.Deleted); var anotherFakeEntry = stateManager.GetOrCreateEntry(new AnotherFakeEntity { Id = 3, AnotherId = 2 }); - anotherFakeEntry.SetEntityState(EntityState.Added); + anotherFakeEntry.SetEntityState(EntityState.Deleted); + + var expectedCycle = sensitiveLogging + ? "FakeEntity { 'Id': 1 } [Deleted] ForeignKey { 'RelatedId': 2 } <- RelatedFakeEntity { 'Id': 2 } [Deleted] ForeignKey { 'RelatedId': 1 } <- FakeEntity { 'Id': 1 } [Deleted]" + : "FakeEntity [Deleted] ForeignKey { 'RelatedId' } <- RelatedFakeEntity [Deleted] ForeignKey { 'RelatedId' } <- FakeEntity [Deleted]"; Assert.Equal( - CoreStrings.CircularDependency( - string.Join( - ", ", - model.FindEntityType(typeof(FakeEntity)).GetForeignKeys().First(), - model.FindEntityType(typeof(RelatedFakeEntity)).GetForeignKeys().First())), + CoreStrings.CircularDependency(expectedCycle), Assert.Throws( - () => CreateCommandBatchPreparer().BatchCommands( + () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: sensitiveLogging).BatchCommands( // Order is important for this test. Entry which is not part of cycle but tail should come first. new[] { anotherFakeEntry, fakeEntry, relatedFakeEntry }).ToArray()).Message); } @@ -578,8 +585,8 @@ public void BatchCommands_throws_on_conflicting_updates_for_shared_table_added_e { Assert.Equal( RelationalStrings.ConflictingRowUpdateTypesSensitive( - nameof(RelatedFakeEntity), "Id:42", EntityState.Deleted, - nameof(FakeEntity), "Id:42", EntityState.Added), + nameof(RelatedFakeEntity), "{Id: 42}", EntityState.Deleted, + nameof(FakeEntity), "{Id: 42}", EntityState.Added), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: true) .BatchCommands(new[] { firstEntry, secondEntry }).ToArray()).Message); @@ -632,8 +639,8 @@ public void BatchCommands_throws_on_conflicting_values_for_shared_table_added_en { Assert.Equal( RelationalStrings.ConflictingRowValuesSensitive( - nameof(RelatedFakeEntity), nameof(FakeEntity), "Id:42", - "RelatedId:2", "RelatedId:1", "{'RelatedId'}"), + nameof(RelatedFakeEntity), nameof(FakeEntity), "{Id: 42}", + "{RelatedId: 2}", "{RelatedId: 1}", "{'RelatedId'}"), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: true) .BatchCommands(new[] { firstEntry, secondEntry }).ToArray()).Message); @@ -655,8 +662,8 @@ public void BatchCommands_throws_on_conflicting_values_for_shared_table_added_en { Assert.Equal( RelationalStrings.ConflictingOriginalRowValuesSensitive( - nameof(RelatedFakeEntity), nameof(FakeEntity), "Id:42", - "RelatedId:2", "RelatedId:1", "{'RelatedId'}"), + nameof(RelatedFakeEntity), nameof(FakeEntity), "{Id: 42}", + "{RelatedId: 2}", "{RelatedId: 1}", "{'RelatedId'}"), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: true) .BatchCommands(new[] { firstEntry, secondEntry }).ToArray()).Message); @@ -684,7 +691,7 @@ public void BatchCommands_throws_on_incomplete_updates_for_shared_table_no_princ var currentDbContext = CreateContextServices(CreateSharedTableModel()).GetRequiredService(); var stateManager = currentDbContext.GetDependencies().StateManager; - var first = new RelatedFakeEntity { Id = 42 }; + var first = new DerivedRelatedFakeEntity { Id = 42 }; var firstEntry = stateManager.GetOrCreateEntry(first); firstEntry.SetEntityState(state); @@ -696,7 +703,7 @@ public void BatchCommands_throws_on_incomplete_updates_for_shared_table_no_princ { Assert.Equal( RelationalStrings.SharedRowEntryCountMismatchSensitive( - nameof(RelatedFakeEntity), nameof(FakeEntity), nameof(FakeEntity), "Id:42", state), + nameof(DerivedRelatedFakeEntity), nameof(FakeEntity), nameof(FakeEntity), "{Id: 42}", state), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: true) .BatchCommands(new[] { firstEntry }).ToArray()).Message); @@ -705,7 +712,7 @@ public void BatchCommands_throws_on_incomplete_updates_for_shared_table_no_princ { Assert.Equal( RelationalStrings.SharedRowEntryCountMismatch( - nameof(RelatedFakeEntity), nameof(FakeEntity), nameof(FakeEntity), state), + nameof(DerivedRelatedFakeEntity), nameof(FakeEntity), nameof(FakeEntity), state), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: false) .BatchCommands(new[] { firstEntry, secondEntry }).ToArray()).Message); @@ -734,7 +741,7 @@ public void BatchCommands_throws_on_incomplete_updates_for_shared_table_no_leaf_ { Assert.Equal( RelationalStrings.SharedRowEntryCountMismatchSensitive( - nameof(DerivedRelatedFakeEntity), nameof(FakeEntity), nameof(AnotherFakeEntity), "Id:42", state), + nameof(DerivedRelatedFakeEntity), nameof(FakeEntity), nameof(AnotherFakeEntity), "{Id: 42}", state), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: true) .BatchCommands(new[] { firstEntry, secondEntry }).ToArray()).Message); @@ -772,7 +779,7 @@ public void BatchCommands_throws_on_incomplete_updates_for_shared_table_no_middl { Assert.Equal( RelationalStrings.SharedRowEntryCountMismatchSensitive( - nameof(FakeEntity), nameof(FakeEntity), nameof(DerivedRelatedFakeEntity), "Id:42", state), + nameof(FakeEntity), nameof(FakeEntity), nameof(DerivedRelatedFakeEntity), "{Id: 42}", state), Assert.Throws( () => CreateCommandBatchPreparer(stateManager: stateManager, sensitiveLogging: true) diff --git a/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs index a431c236ddf..b5643db4b24 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/InternalClrEntityEntryTest.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.DependencyInjection; using Xunit; +// ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal { public class InternalClrEntityEntryTest : InternalEntityEntryTestBase diff --git a/test/EFCore.Tests/ChangeTracking/Internal/InternalEntityEntryTestBase.cs b/test/EFCore.Tests/ChangeTracking/Internal/InternalEntityEntryTestBase.cs index 3137852c0b3..37665faa145 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/InternalEntityEntryTestBase.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/InternalEntityEntryTestBase.cs @@ -1182,7 +1182,7 @@ public void Unchanged_entity_with_conceptually_null_FK_with_cascade_delete_is_ma entry.SetEntityState(EntityState.Unchanged); entry[fkProperty] = null; - entry.HandleConceptualNulls(); + entry.HandleConceptualNulls(false); Assert.Equal(EntityState.Deleted, entry.EntityState); } @@ -1200,7 +1200,7 @@ public void Added_entity_with_conceptually_null_FK_with_cascade_delete_is_detach entry.SetEntityState(EntityState.Added); entry[fkProperty] = null; - entry.HandleConceptualNulls(); + entry.HandleConceptualNulls(false); Assert.Equal(EntityState.Detached, entry.EntityState); } @@ -1222,7 +1222,7 @@ public void Entity_with_partially_null_composite_FK_with_cascade_delete_is_marke entry.SetEntityState(EntityState.Unchanged); entry[fkProperty1] = null; - entry.HandleConceptualNulls(); + entry.HandleConceptualNulls(false); Assert.Equal(EntityState.Deleted, entry.EntityState); } @@ -1244,7 +1244,7 @@ public void Entity_with_partially_null_composite_FK_without_cascade_delete_is_or entry.SetEntityState(EntityState.Unchanged); entry[fkProperty1] = null; - entry.HandleConceptualNulls(); + entry.HandleConceptualNulls(false); Assert.Equal(EntityState.Modified, entry.EntityState); @@ -1252,8 +1252,10 @@ public void Entity_with_partially_null_composite_FK_without_cascade_delete_is_or Assert.Null(entry[fkProperty2]); } - [Fact] - public void Unchanged_entity_with_conceptually_null_FK_without_cascade_delete_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Unchanged_entity_with_conceptually_null_FK_without_cascade_delete_throws(bool sensitiveLoggingEnabled) { var model = BuildModel(); var entityType = model.FindEntityType(typeof(SomeDependentEntity).FullName); @@ -1262,6 +1264,8 @@ public void Unchanged_entity_with_conceptually_null_FK_without_cascade_delete_th var configuration = InMemoryTestHelpers.Instance.CreateContextServices(model); var entry = CreateInternalEntry(configuration, entityType, new SomeDependentEntity()); + entry.SetOriginalValue(keyProperties[0], 77); + entry.SetOriginalValue(keyProperties[1], "ReadySalted"); entry[keyProperties[0]] = 77; entry[keyProperties[1]] = "ReadySalted"; entry[fkProperty] = 99; @@ -1269,15 +1273,28 @@ public void Unchanged_entity_with_conceptually_null_FK_without_cascade_delete_th entry.SetEntityState(EntityState.Unchanged); entry[fkProperty] = null; - Assert.Equal( - CoreStrings.RelationshipConceptualNull( - model.FindEntityType(typeof(SomeEntity).FullName).DisplayName(), - entityType.DisplayName()), - Assert.Throws(() => entry.HandleConceptualNulls()).Message); + var exception = Assert.Throws(() => entry.HandleConceptualNulls(sensitiveLoggingEnabled)).Message; + if (sensitiveLoggingEnabled) + { + Assert.Equal( + CoreStrings.RelationshipConceptualNullSensitive( + model.FindEntityType(typeof(SomeEntity).FullName).DisplayName(), + entityType.DisplayName(), + "{Id1: 77, Id2: ReadySalted}"), exception); + } + else + { + Assert.Equal( + CoreStrings.RelationshipConceptualNull( + model.FindEntityType(typeof(SomeEntity).FullName).DisplayName(), + entityType.DisplayName()), exception); + } } - [Fact] - public void Unchanged_entity_with_conceptually_null_non_FK_property_throws() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void Unchanged_entity_with_conceptually_null_non_FK_property_throws(bool sensitiveLoggingEnabled) { var model = BuildModel(); var entityType = model.FindEntityType(typeof(SomeDependentEntity).FullName); @@ -1286,6 +1303,8 @@ public void Unchanged_entity_with_conceptually_null_non_FK_property_throws() var configuration = InMemoryTestHelpers.Instance.CreateContextServices(model); var entry = CreateInternalEntry(configuration, entityType, new SomeDependentEntity()); + entry.SetOriginalValue(keyProperties[0], 77); + entry.SetOriginalValue(keyProperties[1], "ReadySalted"); entry[keyProperties[0]] = 77; entry[keyProperties[1]] = "ReadySalted"; entry[property] = 99; @@ -1293,9 +1312,19 @@ public void Unchanged_entity_with_conceptually_null_non_FK_property_throws() entry.SetEntityState(EntityState.Unchanged); entry[property] = null; - Assert.Equal( - CoreStrings.PropertyConceptualNull("JustAProperty", entityType.DisplayName()), - Assert.Throws(() => entry.HandleConceptualNulls()).Message); + var exception = Assert.Throws(() => entry.HandleConceptualNulls(sensitiveLoggingEnabled)).Message; + if (sensitiveLoggingEnabled) + { + Assert.Equal( + CoreStrings.PropertyConceptualNullSensitive("JustAProperty", entityType.DisplayName(), "{Id1: 77, Id2: ReadySalted}"), + exception); + } + else + { + Assert.Equal( + CoreStrings.PropertyConceptualNull("JustAProperty", entityType.DisplayName()), + exception); + } } // ReSharper disable once ClassNeverInstantiated.Local diff --git a/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs index ec226696fcd..df06d4a1963 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs @@ -100,7 +100,7 @@ public void Identity_conflict_throws_for_primary_key_values_logged() context.Attach(new SingleKey { Id = 77, AlternateId = 66 }); Assert.Equal( - CoreStrings.IdentityConflictSensitive("SingleKey", "Id:77"), + CoreStrings.IdentityConflictSensitive("SingleKey", "{Id: 77}"), Assert.Throws( () => context.Attach(new SingleKey { Id = 77, AlternateId = 67 })).Message); } @@ -114,7 +114,7 @@ public void Identity_conflict_throws_for_alternate_key_values_logged() context.Attach(new SingleKey { Id = 77, AlternateId = 66 }); Assert.Equal( - CoreStrings.IdentityConflictSensitive("SingleKey", "AlternateId:66"), + CoreStrings.IdentityConflictSensitive("SingleKey", "{AlternateId: 66}"), Assert.Throws( () => context.Attach(new SingleKey { Id = 78, AlternateId = 66 })).Message); } @@ -128,7 +128,7 @@ public void Identity_conflict_throws_for_composite_primary_key_values_logged() context.Attach(new CompositeKey { Id1 = 77, Id2 = 78, AlternateId1 = 66, AlternateId2 = 67 }); Assert.Equal( - CoreStrings.IdentityConflictSensitive("CompositeKey", "Id1:77, Id2:78"), + CoreStrings.IdentityConflictSensitive("CompositeKey", "{Id1: 77, Id2: 78}"), Assert.Throws( () => context.Attach( new CompositeKey { Id1 = 77, Id2 = 78, AlternateId1 = 66, AlternateId2 = 68 })).Message); @@ -143,7 +143,7 @@ public void Identity_conflict_throws_for_composite_alternate_key_values_logged() context.Attach(new CompositeKey { Id1 = 77, Id2 = 78, AlternateId1 = 66, AlternateId2 = 67 }); Assert.Equal( - CoreStrings.IdentityConflictSensitive("CompositeKey", "AlternateId1:66, AlternateId2:67"), + CoreStrings.IdentityConflictSensitive("CompositeKey", "{AlternateId1: 66, AlternateId2: 67}"), Assert.Throws( () => context.Attach( new CompositeKey { Id1 = 77, Id2 = 79, AlternateId1 = 66, AlternateId2 = 67 })).Message); diff --git a/test/EFCore.Tests/Metadata/Conventions/Internal/PropertyMappingValidationConventionTest.cs b/test/EFCore.Tests/Metadata/Conventions/Internal/PropertyMappingValidationConventionTest.cs index 5a1c9f52dd3..ff51ca3af23 100644 --- a/test/EFCore.Tests/Metadata/Conventions/Internal/PropertyMappingValidationConventionTest.cs +++ b/test/EFCore.Tests/Metadata/Conventions/Internal/PropertyMappingValidationConventionTest.cs @@ -20,16 +20,27 @@ public virtual void Throws_when_added_property_is_not_of_primitive_type() { var modelBuilder = new InternalModelBuilder(new Model()); var entityTypeBuilder = modelBuilder.Entity(typeof(NonPrimitiveAsPropertyEntity), ConfigurationSource.Convention); - entityTypeBuilder.Property("Property", typeof(NavigationAsProperty), ConfigurationSource.Convention); + entityTypeBuilder.Property(nameof(NonPrimitiveAsPropertyEntity.Property), typeof(NavigationAsProperty), ConfigurationSource.Convention); Assert.Equal( CoreStrings.PropertyNotMapped( typeof(NonPrimitiveAsPropertyEntity).ShortDisplayName(), - "Property", + nameof(NonPrimitiveAsPropertyEntity.Property), typeof(NavigationAsProperty).ShortDisplayName()), Assert.Throws(() => CreateConvention().Apply(modelBuilder)).Message); } + [Fact] + public virtual void Does_not_throw_when_added_shadow_property_by_convention_is_not_of_primitive_type() + { + var modelBuilder = new InternalModelBuilder(new Model()); + var entityTypeBuilder = modelBuilder.Entity(typeof(NonPrimitiveAsPropertyEntity), ConfigurationSource.Convention); + entityTypeBuilder.Property("ShadowProperty", typeof(NavigationAsProperty), ConfigurationSource.Convention); + entityTypeBuilder.Ignore(nameof(NonPrimitiveAsPropertyEntity.Property), ConfigurationSource.Explicit); + + CreateConvention().Apply(modelBuilder); + } + [Fact] public virtual void Throws_when_primitive_type_property_is_not_added_or_ignored() { diff --git a/test/EFCore.Tests/Metadata/MetadataBuilderTest.cs b/test/EFCore.Tests/Metadata/MetadataBuilderTest.cs index 3266aef77e6..f51ef0f0c5d 100644 --- a/test/EFCore.Tests/Metadata/MetadataBuilderTest.cs +++ b/test/EFCore.Tests/Metadata/MetadataBuilderTest.cs @@ -8,6 +8,10 @@ using Microsoft.EntityFrameworkCore.TestUtilities; using Xunit; +// ReSharper disable MemberCanBePrivate.Local +// ReSharper disable CollectionNeverUpdated.Local +// ReSharper disable UnusedAutoPropertyAccessor.Local +// ReSharper disable InconsistentNaming namespace Microsoft.EntityFrameworkCore.Metadata { public class MetadataBuilderTest diff --git a/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs b/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs index fdecdc64035..5d30ad0dadf 100644 --- a/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs +++ b/test/EFCore.Tests/ModelBuilding/ModelBuilderGenericTest.cs @@ -6,7 +6,6 @@ using System.Linq; using System.Linq.Expressions; using Microsoft.EntityFrameworkCore.Infrastructure; -using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Conventions;