diff --git a/src/EFCore/EFCore.baseline.json b/src/EFCore/EFCore.baseline.json index 7c9812b71e2..1551ad9a333 100644 --- a/src/EFCore/EFCore.baseline.json +++ b/src/EFCore/EFCore.baseline.json @@ -4487,6 +4487,12 @@ { "Member": "static string OwnerlessOwnedType(object? ownedType);" }, + { + "Member": "static string OwnershipNotCascadeDelete(object? principalEntityType, object? dependentEntityType, object? deleteBehavior, object? cascadeBehavior);" + }, + { + "Member": "static string OwnershipNotRequired(object? principalEntityType, object? dependentEntityType);" + }, { "Member": "static string OwnershipToDependent(object? navigation, object? principalEntityType, object? dependentEntityType);" }, diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index e0a7e8f743c..fef5376a91e 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -1082,6 +1082,24 @@ protected virtual void ValidateOwnership( throw new InvalidOperationException(CoreStrings.OwnedDerivedType(entityType.DisplayName())); } + if (ownership.DeleteBehavior != DeleteBehavior.Cascade) + { + throw new InvalidOperationException( + CoreStrings.OwnershipNotCascadeDelete( + ownership.PrincipalEntityType.DisplayName(), + ownership.DeclaringEntityType.DisplayName(), + ownership.DeleteBehavior, + DeleteBehavior.Cascade)); + } + + if (!ownership.IsRequired) + { + throw new InvalidOperationException( + CoreStrings.OwnershipNotRequired( + ownership.PrincipalEntityType.DisplayName(), + ownership.DeclaringEntityType.DisplayName())); + } + foreach (var referencingFk in entityType.GetReferencingForeignKeys().Where(fk => !fk.IsOwnership && (fk.PrincipalEntityType != fk.DeclaringEntityType || !fk.Properties.SequenceEqual(entityType.FindPrimaryKey()!.Properties)) diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 186af2df532..ab3324990b4 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -2749,6 +2749,22 @@ public static string OwnerlessOwnedType(object? ownedType) GetString("OwnerlessOwnedType", nameof(ownedType)), ownedType); + /// + /// The ownership relationship from '{principalEntityType}' to '{dependentEntityType}' is configured with '{deleteBehavior}' delete behavior. Ownership relationships must use '{cascadeBehavior}' delete behavior. Either remove the explicit delete behavior configuration or don't configure this relationship as an ownership. + /// + public static string OwnershipNotCascadeDelete(object? principalEntityType, object? dependentEntityType, object? deleteBehavior, object? cascadeBehavior) + => string.Format( + GetString("OwnershipNotCascadeDelete", nameof(principalEntityType), nameof(dependentEntityType), nameof(deleteBehavior), nameof(cascadeBehavior)), + principalEntityType, dependentEntityType, deleteBehavior, cascadeBehavior); + + /// + /// The ownership relationship from '{principalEntityType}' to '{dependentEntityType}' is configured as optional. Ownership relationships must be required. Either remove the optional configuration or don't configure this relationship as an ownership. + /// + public static string OwnershipNotRequired(object? principalEntityType, object? dependentEntityType) + => string.Format( + GetString("OwnershipNotRequired", nameof(principalEntityType), nameof(dependentEntityType)), + principalEntityType, dependentEntityType); + /// /// The navigation '{navigation}' cannot be changed, because the foreign key between '{principalEntityType}' and '{dependentEntityType}' is an ownership. To change the navigation to the owned entity type remove the ownership. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 09ee563d2d1..99cfd004172 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -1510,6 +1510,12 @@ A tracking query is attempting to project an owned entity without a corresponding owner in its result, but owned entities cannot be tracked without their owner. Either include the owner entity in the result or make the query non-tracking using 'AsNoTracking'. + + The ownership relationship from '{principalEntityType}' to '{dependentEntityType}' is configured with '{deleteBehavior}' delete behavior. Ownership relationships must use '{cascadeBehavior}' delete behavior. Either remove the explicit delete behavior configuration or don't configure this relationship as an ownership. + + + The ownership relationship from '{principalEntityType}' to '{dependentEntityType}' is configured as optional. Ownership relationships must be required. Either remove the optional configuration or don't configure this relationship as an ownership. + The entity type '{ownedType}' has been marked as owned and must be referenced from another entity type via a navigation. Add a navigation to an entity type that points at '{ownedType}' or don't configure it as owned. diff --git a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs index 8e0eaccea41..c7488148b48 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs @@ -369,25 +369,11 @@ public virtual async Task Delete_principal_with_shadow_key_owned_collection_thro context.Attach(owner); context.Remove(owner); - if (Fixture.ForceClientNoAction) - { - if (async) - { - await Assert.ThrowsAsync(async () => await context.SaveChangesAsync()); - } - else - { - Assert.Throws(() => context.SaveChanges()); - } - } - else - { - Assert.Equal( - CoreStrings.UnknownShadowKeyValue("Owner.OwnedCollection#Owned", "Id"), - (async - ? await Assert.ThrowsAsync(async () => await context.SaveChangesAsync()) - : Assert.Throws(() => context.SaveChanges())).Message); - } + Assert.Equal( + CoreStrings.UnknownShadowKeyValue("Owner.OwnedCollection#Owned", "Id"), + (async + ? await Assert.ThrowsAsync(async () => await context.SaveChangesAsync()) + : Assert.Throws(() => context.SaveChanges())).Message); }); [Theory, InlineData(false, false, false), InlineData(false, false, true), InlineData(false, true, false), @@ -453,30 +439,17 @@ public virtual async Task Saving_unknown_key_value_marks_it_as_unmodified(bool a owner.Owned.Remove(owner.Owned.Single()); owner.Owned.Add(new NonCompositeOwnedCollection { Foo = "Rome" }); - if (Fixture.ForceClientNoAction) - { - await Assert.ThrowsAsync(async () => - _ = async - ? await context.SaveChangesAsync() - : context.SaveChanges()); - } - else - { - _ = async - ? await context.SaveChangesAsync() - : context.SaveChanges(); - } + _ = async + ? await context.SaveChangesAsync() + : context.SaveChanges(); }, async context => { - if (!Fixture.ForceClientNoAction) - { - var owner = async - ? await context.Set().SingleAsync() - : context.Set().Single(); + var owner = async + ? await context.Set().SingleAsync() + : context.Set().Single(); - Assert.Equal("Rome", owner.Owned.Single().Foo); - } + Assert.Equal("Rome", owner.Owned.Single().Foo); }); [Theory, InlineData(false), InlineData(true)] // Issue #19856 @@ -563,39 +536,19 @@ public virtual async Task Delete_principal_with_CLR_key_owned_collection(bool as context.Attach(owner); context.Remove(owner); - if (Fixture.ForceClientNoAction) + if (async) { - if (async) - { - await Assert.ThrowsAsync(async () => await context.SaveChangesAsync()); - } - else - { - Assert.Throws(() => context.SaveChanges()); - } + await context.SaveChangesAsync(); } else { - if (async) - { - await context.SaveChangesAsync(); - } - else - { - context.SaveChanges(); - } + context.SaveChanges(); } }, - async context => - { - if (!Fixture.ForceClientNoAction) - { - Assert.False( - async - ? await context.Set().AnyAsync() - : context.Set().Any()); - } - }); + async context => Assert.False( + async + ? await context.Set().AnyAsync() + : context.Set().Any())); [Theory, InlineData(false, false, false), InlineData(false, false, true), InlineData(false, true, false), InlineData(false, true, true), InlineData(true, false, false), InlineData(true, false, true), InlineData(true, true, false), diff --git a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientCascadeTest.cs b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientCascadeTest.cs index d7396dd6b44..548d1fcf94c 100644 --- a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientCascadeTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientCascadeTest.cs @@ -27,7 +27,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con foreach (var foreignKey in modelBuilder.Model .GetEntityTypes() .SelectMany(e => e.GetDeclaredForeignKeys()) - .Where(e => e.DeleteBehavior == DeleteBehavior.Cascade)) + .Where(e => e is { IsOwnership: false, DeleteBehavior: DeleteBehavior.Cascade })) { foreignKey.DeleteBehavior = DeleteBehavior.ClientCascade; } diff --git a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientNoActionTest.cs b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientNoActionTest.cs index 71c99728a8e..8864ab7143a 100644 --- a/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientNoActionTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/GraphUpdates/GraphUpdatesSqlServerClientNoActionTest.cs @@ -33,7 +33,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con foreach (var foreignKey in modelBuilder.Model .GetEntityTypes() - .SelectMany(e => e.GetDeclaredForeignKeys())) + .SelectMany(e => e.GetDeclaredForeignKeys()) + .Where(e => !e.IsOwnership)) { foreignKey.DeleteBehavior = DeleteBehavior.ClientNoAction; } diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index 275a48afaea..992bc920bbe 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -1570,6 +1570,61 @@ public virtual void Detects_entity_type_with_multiple_ownerships() builder); } + [Fact] + public virtual void Detects_ownership_with_non_cascade_delete_behavior() + { + var builder = CreateConventionlessModelBuilder(); + var modelBuilder = (InternalModelBuilder)builder.GetInfrastructure(); + var entityTypeBuilder = modelBuilder.Entity(typeof(SampleEntity), ConfigurationSource.Convention); + entityTypeBuilder.PrimaryKey([nameof(SampleEntity.Id)], ConfigurationSource.Convention); + entityTypeBuilder.Ignore(nameof(SampleEntity.Name), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.Number), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.OtherSamples), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.AnotherReferencedEntity), ConfigurationSource.Explicit); + + var ownershipBuilder = entityTypeBuilder.HasOwnership( + typeof(ReferencedEntity), nameof(SampleEntity.ReferencedEntity), ConfigurationSource.Convention); + + var ownedTypeBuilder = ownershipBuilder.Metadata.DeclaringEntityType.Builder; + ownedTypeBuilder.PrimaryKey(ownershipBuilder.Metadata.Properties.Select(p => p.Name).ToList(), ConfigurationSource.Convention); + ownedTypeBuilder.Ignore(nameof(ReferencedEntity.Id), ConfigurationSource.Explicit); + ownedTypeBuilder.Ignore(nameof(ReferencedEntity.SampleEntityId), ConfigurationSource.Explicit); + + ownershipBuilder.Metadata.DeleteBehavior = DeleteBehavior.Restrict; + + VerifyError( + CoreStrings.OwnershipNotCascadeDelete( + nameof(SampleEntity), nameof(ReferencedEntity), DeleteBehavior.Restrict, DeleteBehavior.Cascade), + builder); + } + + [Fact] + public virtual void Detects_optional_ownership() + { + var builder = CreateConventionlessModelBuilder(); + var modelBuilder = (InternalModelBuilder)builder.GetInfrastructure(); + var entityTypeBuilder = modelBuilder.Entity(typeof(SampleEntity), ConfigurationSource.Convention); + entityTypeBuilder.PrimaryKey([nameof(SampleEntity.Id)], ConfigurationSource.Convention); + entityTypeBuilder.Ignore(nameof(SampleEntity.Name), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.Number), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.OtherSamples), ConfigurationSource.Explicit); + entityTypeBuilder.Ignore(nameof(SampleEntity.AnotherReferencedEntity), ConfigurationSource.Explicit); + + var ownershipBuilder = entityTypeBuilder.HasOwnership( + typeof(ReferencedEntity), nameof(SampleEntity.ReferencedEntity), ConfigurationSource.Convention); + + var ownedTypeBuilder = ownershipBuilder.Metadata.DeclaringEntityType.Builder; + ownedTypeBuilder.PrimaryKey(ownershipBuilder.Metadata.Properties.Select(p => p.Name).ToList(), ConfigurationSource.Convention); + ownedTypeBuilder.Ignore(nameof(ReferencedEntity.Id), ConfigurationSource.Explicit); + ownedTypeBuilder.Ignore(nameof(ReferencedEntity.SampleEntityId), ConfigurationSource.Explicit); + + ownershipBuilder.Metadata.IsRequired = false; + + VerifyError( + CoreStrings.OwnershipNotRequired(nameof(SampleEntity), nameof(ReferencedEntity)), + builder); + } + [Fact] public virtual void Detects_principal_owned_entity_type() {