Skip to content

Fix invalid expression for total count on inverse relationship filter from resource definition #1711

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Apr 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 17 additions & 9 deletions src/JsonApiDotNetCore/Queries/QueryLayerComposer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,25 @@ public QueryLayerComposer(IEnumerable<IQueryConstraintProvider> constraintProvid
// @formatter:wrap_chained_method_calls restore

FilterExpression? primaryFilter = GetFilter(Array.Empty<QueryExpression>(), hasManyRelationship.LeftType);
FilterExpression? secondaryFilter = GetFilter(filtersInSecondaryScope, hasManyRelationship.RightType);

FilterExpression inverseFilter = GetInverseRelationshipFilter(primaryId, hasManyRelationship, inverseRelationship);
if (primaryFilter != null && inverseRelationship is HasOneAttribute)
{
// We can't lift the field chains in a primary filter, because there's no way for a custom filter expression to express
// the scope of its chains. See https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/1671.
return null;
}

FilterExpression? secondaryFilter = GetFilter(filtersInSecondaryScope, hasManyRelationship.RightType);
FilterExpression inverseFilter = GetInverseRelationshipFilter(primaryId, primaryFilter, hasManyRelationship, inverseRelationship);

return LogicalExpression.Compose(LogicalOperator.And, inverseFilter, primaryFilter, secondaryFilter);
return LogicalExpression.Compose(LogicalOperator.And, inverseFilter, secondaryFilter);
}

private static FilterExpression GetInverseRelationshipFilter<TId>([DisallowNull] TId primaryId, HasManyAttribute relationship,
RelationshipAttribute inverseRelationship)
private static FilterExpression GetInverseRelationshipFilter<TId>([DisallowNull] TId primaryId, FilterExpression? primaryFilter,
HasManyAttribute relationship, RelationshipAttribute inverseRelationship)
{
return inverseRelationship is HasManyAttribute hasManyInverseRelationship
? GetInverseHasManyRelationshipFilter(primaryId, relationship, hasManyInverseRelationship)
? GetInverseHasManyRelationshipFilter(primaryId, primaryFilter, relationship, hasManyInverseRelationship)
: GetInverseHasOneRelationshipFilter(primaryId, relationship, (HasOneAttribute)inverseRelationship);
}

Expand All @@ -120,14 +127,15 @@ private static ComparisonExpression GetInverseHasOneRelationshipFilter<TId>([Dis
return new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId));
}

private static HasExpression GetInverseHasManyRelationshipFilter<TId>([DisallowNull] TId primaryId, HasManyAttribute relationship,
HasManyAttribute inverseRelationship)
private static HasExpression GetInverseHasManyRelationshipFilter<TId>([DisallowNull] TId primaryId, FilterExpression? primaryFilter,
HasManyAttribute relationship, HasManyAttribute inverseRelationship)
{
AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(idAttribute));
var idComparison = new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId));

return new HasExpression(new ResourceFieldChainExpression(inverseRelationship), idComparison);
FilterExpression filter = LogicalExpression.Compose(LogicalOperator.And, idComparison, primaryFilter)!;
return new HasExpression(new ResourceFieldChainExpression(inverseRelationship), filter);
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System.ComponentModel.DataAnnotations;
using JetBrains.Annotations;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Resources.Annotations;

namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceDefinitions.Reading;

[UsedImplicitly(ImplicitUseTargetFlags.Members)]
[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.ResourceDefinitions.Reading")]
public sealed class Constellation : Identifiable<int>
{
[Attr]
public string Name { get; set; } = null!;

[Attr]
[Required]
public Season? VisibleDuring { get; set; }

[HasMany]
public ISet<Star> Stars { get; set; } = new HashSet<Star>();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using JetBrains.Annotations;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Queries.Expressions;
using JsonApiDotNetCore.Resources.Annotations;

namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceDefinitions.Reading;

[UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)]
public sealed class ConstellationDefinition(
IResourceGraph resourceGraph, IClientSettingsProvider clientSettingsProvider, ResourceDefinitionHitCounter hitCounter)
: HitCountingResourceDefinition<Constellation, int>(resourceGraph, hitCounter)
{
private readonly IClientSettingsProvider _clientSettingsProvider = clientSettingsProvider;

protected override ResourceDefinitionExtensibilityPoints ExtensibilityPointsToTrack => ResourceDefinitionExtensibilityPoints.Reading;

public override FilterExpression? OnApplyFilter(FilterExpression? existingFilter)
{
FilterExpression? baseFilter = base.OnApplyFilter(existingFilter);

if (_clientSettingsProvider.AreConstellationsVisibleDuringWinterHidden)
{
AttrAttribute visibleDuringAttribute = ResourceType.GetAttributeByPropertyName(nameof(Constellation.VisibleDuring));
var visibleDuringChain = new ResourceFieldChainExpression(visibleDuringAttribute);
var visibleDuringComparison = new ComparisonExpression(ComparisonOperator.Equals, visibleDuringChain, new LiteralConstantExpression(Season.Winter));
var notVisibleDuringComparison = new NotExpression(visibleDuringComparison);

return LogicalExpression.Compose(LogicalOperator.And, baseFilter, notVisibleDuringComparison);
}

return baseFilter;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceDefinitions.Reading;

public interface IClientSettingsProvider
{
bool AreVeryLargeStarsHidden { get; }
bool AreConstellationsVisibleDuringWinterHidden { get; }
bool IsIncludePlanetMoonsBlocked { get; }
bool ArePlanetsWithPrivateNameHidden { get; }
bool IsStarGivingLightToMoonAutoIncluded { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ public ResourceDefinitionReadTests(IntegrationTestContext<TestableStartup<Univer
{
_testContext = testContext;

testContext.UseController<ConstellationsController>();
testContext.UseController<StarsController>();
testContext.UseController<PlanetsController>();
testContext.UseController<MoonsController>();

testContext.ConfigureServices(services =>
{
services.AddResourceDefinition<ConstellationDefinition>();
services.AddResourceDefinition<StarDefinition>();
services.AddResourceDefinition<PlanetDefinition>();
services.AddResourceDefinition<MoonDefinition>();
Expand Down Expand Up @@ -323,7 +325,6 @@ public async Task Filter_from_resource_definition_is_applied_at_secondary_endpoi

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
await dbContext.ClearTableAsync<Planet>();
dbContext.Stars.Add(star);
await dbContext.SaveChangesAsync();
});
Expand Down Expand Up @@ -375,7 +376,6 @@ public async Task Filter_from_resource_definition_is_applied_at_relationship_end

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
await dbContext.ClearTableAsync<Planet>();
dbContext.Stars.Add(star);
await dbContext.SaveChangesAsync();
});
Expand Down Expand Up @@ -409,6 +409,198 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
}, options => options.WithStrictOrdering());
}

[Fact]
public async Task No_total_when_resource_definition_has_filter_on_inverse_ManyToOne_at_secondary_endpoint()
{
// Arrange
var hitCounter = _testContext.Factory.Services.GetRequiredService<ResourceDefinitionHitCounter>();

var settingsProvider = (TestClientSettingsProvider)_testContext.Factory.Services.GetRequiredService<IClientSettingsProvider>();
settingsProvider.HideVeryLargeStars();

Star star = _fakers.Star.GenerateOne();
star.Planets = _fakers.Planet.GenerateSet(1);

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
dbContext.Stars.Add(star);
await dbContext.SaveChangesAsync();
});

string route = $"/stars/{star.StringId}/planets";

// Act
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);

// Assert
httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK);

responseDocument.Data.ManyValue.Should().HaveCount(1);
responseDocument.Data.ManyValue[0].Id.Should().Be(star.Planets.ElementAt(0).StringId);

responseDocument.Meta.Should().BeNull();

hitCounter.HitExtensibilityPoints.Should().BeEquivalentTo(new[]
{
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplyPagination),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplySort),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplyIncludes),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.GetMeta)
}, options => options.WithStrictOrdering());
}

[Fact]
public async Task Has_total_when_resource_definition_has_filter_on_inverse_ManyToMany_at_secondary_endpoint()
{
// Arrange
var hitCounter = _testContext.Factory.Services.GetRequiredService<ResourceDefinitionHitCounter>();

var settingsProvider = (TestClientSettingsProvider)_testContext.Factory.Services.GetRequiredService<IClientSettingsProvider>();
settingsProvider.HideConstellationsVisibleDuringWinter();

Constellation constellation = _fakers.Constellation.GenerateOne();
constellation.VisibleDuring = Season.Winter;
constellation.Stars = _fakers.Star.GenerateSet(1);

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
dbContext.Constellations.Add(constellation);
await dbContext.SaveChangesAsync();
});

string route = $"/constellations/{constellation.StringId}/stars";

// Act
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);

// Assert
httpResponse.ShouldHaveStatusCode(HttpStatusCode.NotFound);

responseDocument.Errors.Should().HaveCount(1);

ErrorObject error = responseDocument.Errors[0];
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
error.Title.Should().Be("The requested resource does not exist.");
error.Detail.Should().Be($"Resource of type 'constellations' with ID '{constellation.StringId}' does not exist.");

responseDocument.Meta.Should().ContainTotal(0);

hitCounter.HitExtensibilityPoints.Should().BeEquivalentTo(new[]
{
(typeof(Constellation), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyPagination),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySort),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyIncludes),
(typeof(Constellation), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Constellation), ResourceDefinitionExtensibilityPoints.OnApplyFilter)
}, options => options.WithStrictOrdering());
}

[Fact]
public async Task No_total_when_resource_definition_has_filter_on_inverse_ManyToOne_at_relationship_endpoint()
{
// Arrange
var hitCounter = _testContext.Factory.Services.GetRequiredService<ResourceDefinitionHitCounter>();

var settingsProvider = (TestClientSettingsProvider)_testContext.Factory.Services.GetRequiredService<IClientSettingsProvider>();
settingsProvider.HideVeryLargeStars();

Star star = _fakers.Star.GenerateOne();
star.Planets = _fakers.Planet.GenerateSet(1);

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
dbContext.Stars.Add(star);
await dbContext.SaveChangesAsync();
});

string route = $"/stars/{star.StringId}/relationships/planets";

// Act
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);

// Assert
httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK);

responseDocument.Data.ManyValue.Should().HaveCount(1);
responseDocument.Data.ManyValue[0].Id.Should().Be(star.Planets.ElementAt(0).StringId);

responseDocument.Meta.Should().BeNull();

hitCounter.HitExtensibilityPoints.Should().BeEquivalentTo(new[]
{
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplyPagination),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplySort),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplyIncludes),
(typeof(Planet), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter)
}, options => options.WithStrictOrdering());
}

[Fact]
public async Task Has_total_when_resource_definition_has_filter_on_inverse_ManyToMany_at_relationship_endpoint()
{
// Arrange
var hitCounter = _testContext.Factory.Services.GetRequiredService<ResourceDefinitionHitCounter>();

var settingsProvider = (TestClientSettingsProvider)_testContext.Factory.Services.GetRequiredService<IClientSettingsProvider>();
settingsProvider.HideConstellationsVisibleDuringWinter();

Constellation constellation = _fakers.Constellation.GenerateOne();
constellation.VisibleDuring = Season.Winter;
constellation.Stars = _fakers.Star.GenerateSet(1);

await _testContext.RunOnDatabaseAsync(async dbContext =>
{
dbContext.Constellations.Add(constellation);
await dbContext.SaveChangesAsync();
});

string route = $"/constellations/{constellation.StringId}/relationships/stars";

// Act
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);

// Assert
httpResponse.ShouldHaveStatusCode(HttpStatusCode.NotFound);

responseDocument.Errors.Should().HaveCount(1);

ErrorObject error = responseDocument.Errors[0];
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
error.Title.Should().Be("The requested resource does not exist.");
error.Detail.Should().Be($"Resource of type 'constellations' with ID '{constellation.StringId}' does not exist.");

responseDocument.Meta.Should().ContainTotal(0);

hitCounter.HitExtensibilityPoints.Should().BeEquivalentTo(new[]
{
(typeof(Constellation), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyPagination),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyFilter),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySort),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplyIncludes),
(typeof(Star), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Constellation), ResourceDefinitionExtensibilityPoints.OnApplySparseFieldSet),
(typeof(Constellation), ResourceDefinitionExtensibilityPoints.OnApplyFilter)
}, options => options.WithStrictOrdering());
}

[Fact]
public async Task Sort_from_resource_definition_is_applied()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using JetBrains.Annotations;

namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceDefinitions.Reading;

[UsedImplicitly(ImplicitUseTargetFlags.Members)]
public enum Season
{
Winter,
Spring,
Summer,
Fall
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,7 @@ public sealed class Star : Identifiable<int>

[HasMany]
public ISet<Planet> Planets { get; set; } = new HashSet<Planet>();

[HasMany]
public ISet<Constellation> IsPartOf { get; set; } = new HashSet<Constellation>();
}
Loading
Loading