From 42caa548ef242530e40029d909710c9fed09697e Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 23 Aug 2022 16:24:13 -0700 Subject: [PATCH] Query: Fix issue with required navigation and query filters Resolves #26216 Resolves #26900 --- src/EFCore/Infrastructure/ModelValidator.cs | 29 +++++++----- .../Infrastructure/ModelValidatorTest.cs | 44 +++++++++++++++++++ .../Infrastructure/ModelValidatorTestBase.cs | 38 ++++++++++++++++ 3 files changed, 99 insertions(+), 12 deletions(-) diff --git a/src/EFCore/Infrastructure/ModelValidator.cs b/src/EFCore/Infrastructure/ModelValidator.cs index ee24105ede5..87325fa3ef2 100644 --- a/src/EFCore/Infrastructure/ModelValidator.cs +++ b/src/EFCore/Infrastructure/ModelValidator.cs @@ -927,19 +927,24 @@ protected virtual void ValidateQueryFilters( } } - var requiredNavigationWithQueryFilter = entityType - .GetNavigations() - .FirstOrDefault( - n => !n.IsCollection - && n.ForeignKey.IsRequired - && n.IsOnDependent - && n.ForeignKey.PrincipalEntityType.GetQueryFilter() != null - && n.ForeignKey.DeclaringEntityType.GetQueryFilter() == null); - - if (requiredNavigationWithQueryFilter != null) + if (!entityType.IsOwned()) { - logger.PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning( - requiredNavigationWithQueryFilter.ForeignKey); + // Owned type doesn't allow to define query filter + // So we don't check navigations there. We assume the owner will propagate filtering + var requiredNavigationWithQueryFilter = entityType + .GetNavigations() + .FirstOrDefault( + n => !n.IsCollection + && n.ForeignKey.IsRequired + && n.IsOnDependent + && n.ForeignKey.PrincipalEntityType.GetRootType().GetQueryFilter() != null + && n.ForeignKey.DeclaringEntityType.GetRootType().GetQueryFilter() == null); + + if (requiredNavigationWithQueryFilter != null) + { + logger.PossibleIncorrectRequiredNavigationWithQueryFilterInteractionWarning( + requiredNavigationWithQueryFilter.ForeignKey); + } } } } diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs index f3f63e1b31b..a4eab9911ba 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs @@ -1548,6 +1548,50 @@ public virtual void Required_navigation_with_query_filter_on_both_sides_doesnt_i VerifyLogDoesNotContain(message, modelBuilder); } + [ConditionalFact] + public virtual void Required_navigation_on_derived_type_with_query_filter_on_both_sides_doesnt_issue_a_warning() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity().HasMany(x => x.PicturePosts).WithOne(x => x.Blog).IsRequired(); + modelBuilder.Entity(e => e.HasQueryFilter(b => b.IsDeleted == false)); + modelBuilder.Entity(e => e.HasQueryFilter(p => p.IsDeleted == false)); + + var message = CoreResources.LogPossibleIncorrectRequiredNavigationWithQueryFilterInteraction( + CreateValidationLogger()).GenerateMessage(nameof(Blog), nameof(PicturePost)); + + VerifyLogDoesNotContain(message, modelBuilder); + } + + [ConditionalFact] + public virtual void Required_navigation_targeting_derived_type_with_no_query_filter_issues_a_warning() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity(e => e.HasQueryFilter(p => p.IsDeleted == false).Ignore(e => e.Blog)); + modelBuilder.Entity().HasMany(e => e.Pictures).WithOne(e => e.PicturePost).IsRequired(); + + var message = CoreResources.LogPossibleIncorrectRequiredNavigationWithQueryFilterInteraction( + CreateValidationLogger()).GenerateMessage(nameof(PicturePost), nameof(Picture)); + + VerifyWarning(message, modelBuilder); + } + + [ConditionalFact] + public virtual void Required_navigation_on_owned_type_with_query_filter_on_owner_doesnt_issue_a_warning() + { + var modelBuilder = CreateConventionModelBuilder(); + modelBuilder.Entity(e => + { + e.Ignore(i => i.PicturePosts); + e.HasQueryFilter(b => b.IsDeleted == false); + e.OwnsMany(i => i.BlogOwnedEntities); + }); + + var message = CoreResources.LogPossibleIncorrectRequiredNavigationWithQueryFilterInteraction( + CreateValidationLogger()).GenerateMessage(nameof(Blog), nameof(BlogOwnedEntity)); + + VerifyLogDoesNotContain(message, modelBuilder); + } + [ConditionalFact] public virtual void Shared_type_inheritance_throws() { diff --git a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs index 46ea3e9a6a1..ea0eb63e51a 100644 --- a/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs +++ b/test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs @@ -324,6 +324,44 @@ protected class DependentFour public PrincipalFour PrincipalFour { get; set; } } + public class Blog + { + public int BlogId { get; set; } + public bool IsDeleted { get; set; } + public ICollection PicturePosts { get; set; } + public List BlogOwnedEntities { get; set; } + } + + public class BlogOwnedEntity + { + public int BlogOwnedEntityId { get; set; } + public int BlogId { get; set; } + public Blog Blog { get; set; } + } + + public class Post + { + public int PostId { get; set; } + public int BlogId { get; set; } + public string Content { get; set; } + public bool IsDeleted { get; set; } + public Blog Blog { get; set; } + } + + public class PicturePost : Post + { + public string PictureUrl { get; set; } + public List Pictures { get; set; } + } + + public class Picture + { + public int PictureId { get; set; } + public bool IsDeleted { get; set; } + public int PicturePostId { get; set; } + public PicturePost PicturePost { get; set; } + } + protected ModelValidatorTestBase() { LoggerFactory = new ListLoggerFactory(l => l == DbLoggerCategory.Model.Validation.Name || l == DbLoggerCategory.Model.Name);