Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: Fix issue with required navigation and query filters #28858

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Query: Fix issue with required navigation and query filters #28858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions src/EFCore/Infrastructure/ModelValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Blog>().HasMany(x => x.PicturePosts).WithOne(x => x.Blog).IsRequired();
modelBuilder.Entity<Blog>(e => e.HasQueryFilter(b => b.IsDeleted == false));
modelBuilder.Entity<Post>(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<Post>(e => e.HasQueryFilter(p => p.IsDeleted == false).Ignore(e => e.Blog));
modelBuilder.Entity<PicturePost>().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<Blog>(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()
{
Expand Down
38 changes: 38 additions & 0 deletions test/EFCore.Tests/Infrastructure/ModelValidatorTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<PicturePost> PicturePosts { get; set; }
public List<BlogOwnedEntity> 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<Picture> 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);
Expand Down