Skip to content

Fixed including extra resources from OnApplyIncudes #1011

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.

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

Merged
merged 4 commits into from
Jun 14, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ public void ConfigureServiceContainer(ICollection<Type> dbContextTypes)
_services.AddScoped<IGenericServiceFactory, GenericServiceFactory>();
_services.AddScoped(typeof(IResourceChangeTracker<>), typeof(ResourceChangeTracker<>));
_services.AddScoped<IPaginationContext, PaginationContext>();
_services.AddScoped<IEvaluatedIncludeCache, EvaluatedIncludeCache>();
_services.AddScoped<IQueryLayerComposer, QueryLayerComposer>();
_services.AddScoped<IInverseNavigationResolver, InverseNavigationResolver>();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using JsonApiDotNetCore.Resources.Annotations;
Expand Down Expand Up @@ -32,6 +33,11 @@ public IReadOnlyCollection<ResourceFieldChainExpression> GetRelationshipChains(I
{
ArgumentGuard.NotNull(include, nameof(include));

if (!include.Elements.Any())
{
return Array.Empty<ResourceFieldChainExpression>();
}

var converter = new IncludeToChainsConverter();
converter.Visit(include, null);

Expand Down
22 changes: 22 additions & 0 deletions src/JsonApiDotNetCore/Queries/Internal/EvaluatedIncludeCache.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
using JsonApiDotNetCore.Queries.Expressions;

namespace JsonApiDotNetCore.Queries.Internal
{
/// <inheritdoc />
internal sealed class EvaluatedIncludeCache : IEvaluatedIncludeCache
{
private IncludeExpression _include;

/// <inheritdoc />
public void Set(IncludeExpression include)
{
_include = include;
}

/// <inheritdoc />
public IncludeExpression Get()
{
return _include;
}
}
}
23 changes: 23 additions & 0 deletions src/JsonApiDotNetCore/Queries/Internal/IEvaluatedIncludeCache.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using JsonApiDotNetCore.Queries.Expressions;
using JsonApiDotNetCore.Resources;

namespace JsonApiDotNetCore.Queries.Internal
{
/// <summary>
/// Provides in-memory storage for the evaluated inclusion tree within a request. This tree is produced from query string and resource definition
/// callbacks. The cache enables the serialization layer to take changes from <see cref="IResourceDefinition{TResource,TId}.OnApplyIncludes" /> into
/// account.
/// </summary>
public interface IEvaluatedIncludeCache
{
/// <summary>
/// Stores the evaluated inclusion tree for later usage.
/// </summary>
void Set(IncludeExpression include);

/// <summary>
/// Gets the evaluated inclusion tree that was stored earlier.
/// </summary>
IncludeExpression Get();
}
}
16 changes: 12 additions & 4 deletions src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,28 @@ public class QueryLayerComposer : IQueryLayerComposer
private readonly IJsonApiOptions _options;
private readonly IPaginationContext _paginationContext;
private readonly ITargetedFields _targetedFields;
private readonly IEvaluatedIncludeCache _evaluatedIncludeCache;
private readonly SparseFieldSetCache _sparseFieldSetCache;

public QueryLayerComposer(IEnumerable<IQueryConstraintProvider> constraintProviders, IResourceContextProvider resourceContextProvider,
IResourceDefinitionAccessor resourceDefinitionAccessor, IJsonApiOptions options, IPaginationContext paginationContext,
ITargetedFields targetedFields)
ITargetedFields targetedFields, IEvaluatedIncludeCache evaluatedIncludeCache)
{
ArgumentGuard.NotNull(constraintProviders, nameof(constraintProviders));
ArgumentGuard.NotNull(resourceContextProvider, nameof(resourceContextProvider));
ArgumentGuard.NotNull(resourceDefinitionAccessor, nameof(resourceDefinitionAccessor));
ArgumentGuard.NotNull(options, nameof(options));
ArgumentGuard.NotNull(paginationContext, nameof(paginationContext));
ArgumentGuard.NotNull(targetedFields, nameof(targetedFields));
ArgumentGuard.NotNull(evaluatedIncludeCache, nameof(evaluatedIncludeCache));

_constraintProviders = constraintProviders;
_resourceContextProvider = resourceContextProvider;
_resourceDefinitionAccessor = resourceDefinitionAccessor;
_options = options;
_paginationContext = paginationContext;
_targetedFields = targetedFields;
_evaluatedIncludeCache = evaluatedIncludeCache;
_sparseFieldSetCache = new SparseFieldSetCache(_constraintProviders, resourceDefinitionAccessor);
}

Expand Down Expand Up @@ -72,6 +75,8 @@ public QueryLayer ComposeFromConstraints(ResourceContext requestResource)
QueryLayer topLayer = ComposeTopLayer(constraints, requestResource);
topLayer.Include = ComposeChildren(topLayer, constraints);

_evaluatedIncludeCache.Set(topLayer.Include);

return topLayer;
}

Expand Down Expand Up @@ -159,12 +164,15 @@ private IReadOnlyCollection<IncludeElementExpression> ProcessIncludeSet(IReadOnl
// @formatter:wrap_chained_method_calls restore

ResourceContext resourceContext = _resourceContextProvider.GetResourceContext(includeElement.Relationship.RightType);
bool isToManyRelationship = includeElement.Relationship is HasManyAttribute;

var child = new QueryLayer(resourceContext)
{
Filter = GetFilter(expressionsInCurrentScope, resourceContext),
Sort = GetSort(expressionsInCurrentScope, resourceContext),
Pagination = ((JsonApiOptions)_options).DisableChildrenPagination ? null : GetPagination(expressionsInCurrentScope, resourceContext),
Filter = isToManyRelationship ? GetFilter(expressionsInCurrentScope, resourceContext) : null,
Sort = isToManyRelationship ? GetSort(expressionsInCurrentScope, resourceContext) : null,
Pagination = isToManyRelationship
? ((JsonApiOptions)_options).DisableChildrenPagination ? null : GetPagination(expressionsInCurrentScope, resourceContext)
: null,
Projection = GetProjectionForSparseAttributeSet(resourceContext)
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public virtual void Read(string parameterName, StringValues parameterValue)

private object GetQueryableHandler(string parameterName)
{
Type resourceType = _request.PrimaryResource.ResourceType;
Type resourceType = (_request.SecondaryResource ?? _request.PrimaryResource).ResourceType;
object handler = _resourceDefinitionAccessor.GetQueryableHandlerForQueryStringParameter(resourceType, parameterName);

if (handler != null && _request.Kind != EndpointKind.Primary)
Expand Down
6 changes: 6 additions & 0 deletions src/JsonApiDotNetCore/Resources/JsonApiResourceDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,17 @@ public class JsonApiResourceDefinition<TResource, TId> : IResourceDefinition<TRe
{
protected IResourceGraph ResourceGraph { get; }

/// <summary>
/// Provides metadata for the resource type <typeparamref name="TResource" />.
/// </summary>
protected ResourceContext ResourceContext { get; }

public JsonApiResourceDefinition(IResourceGraph resourceGraph)
{
ArgumentGuard.NotNull(resourceGraph, nameof(resourceGraph));

ResourceGraph = resourceGraph;
ResourceContext = resourceGraph.GetResourceContext<TResource>();
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using JetBrains.Annotations;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Middleware;
using JsonApiDotNetCore.Queries.Internal;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Resources.Annotations;
using JsonApiDotNetCore.Serialization.Building;
Expand All @@ -21,27 +22,31 @@ public sealed class AtomicOperationsResponseSerializer : BaseSerializer, IJsonAp
private readonly ILinkBuilder _linkBuilder;
private readonly IFieldsToSerialize _fieldsToSerialize;
private readonly IResourceDefinitionAccessor _resourceDefinitionAccessor;
private readonly IEvaluatedIncludeCache _evaluatedIncludeCache;
private readonly IJsonApiRequest _request;
private readonly IJsonApiOptions _options;

/// <inheritdoc />
public string ContentType { get; } = HeaderConstants.AtomicOperationsMediaType;

public AtomicOperationsResponseSerializer(IResourceObjectBuilder resourceObjectBuilder, IMetaBuilder metaBuilder, ILinkBuilder linkBuilder,
IFieldsToSerialize fieldsToSerialize, IResourceDefinitionAccessor resourceDefinitionAccessor, IJsonApiRequest request, IJsonApiOptions options)
IFieldsToSerialize fieldsToSerialize, IResourceDefinitionAccessor resourceDefinitionAccessor, IEvaluatedIncludeCache evaluatedIncludeCache,
IJsonApiRequest request, IJsonApiOptions options)
: base(resourceObjectBuilder)
{
ArgumentGuard.NotNull(metaBuilder, nameof(metaBuilder));
ArgumentGuard.NotNull(linkBuilder, nameof(linkBuilder));
ArgumentGuard.NotNull(fieldsToSerialize, nameof(fieldsToSerialize));
ArgumentGuard.NotNull(resourceDefinitionAccessor, nameof(resourceDefinitionAccessor));
ArgumentGuard.NotNull(evaluatedIncludeCache, nameof(evaluatedIncludeCache));
ArgumentGuard.NotNull(request, nameof(request));
ArgumentGuard.NotNull(options, nameof(options));

_metaBuilder = metaBuilder;
_linkBuilder = linkBuilder;
_fieldsToSerialize = fieldsToSerialize;
_resourceDefinitionAccessor = resourceDefinitionAccessor;
_evaluatedIncludeCache = evaluatedIncludeCache;
_request = request;
_options = options;
}
Expand Down Expand Up @@ -93,6 +98,7 @@ private AtomicResultObject SerializeOperation(OperationContainer operation)
{
_request.CopyFrom(operation.Request);
_fieldsToSerialize.ResetCache();
_evaluatedIncludeCache.Set(null);

_resourceDefinitionAccessor.OnSerialize(operation.Resource);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using JsonApiDotNetCore.Queries;
using JsonApiDotNetCore.Queries.Expressions;
using JsonApiDotNetCore.Queries.Internal;
using JsonApiDotNetCore.QueryStrings;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Resources.Annotations;
using JsonApiDotNetCore.Serialization.Objects;
Expand All @@ -17,28 +16,31 @@ public class ResponseResourceObjectBuilder : ResourceObjectBuilder
{
private static readonly IncludeChainConverter IncludeChainConverter = new IncludeChainConverter();

private readonly ILinkBuilder _linkBuilder;
private readonly IIncludedResourceObjectBuilder _includedBuilder;
private readonly IEnumerable<IQueryConstraintProvider> _constraintProviders;
private readonly IResourceDefinitionAccessor _resourceDefinitionAccessor;
private readonly ILinkBuilder _linkBuilder;
private readonly IEvaluatedIncludeCache _evaluatedIncludeCache;
private readonly SparseFieldSetCache _sparseFieldSetCache;

private RelationshipAttribute _requestRelationship;

public ResponseResourceObjectBuilder(ILinkBuilder linkBuilder, IIncludedResourceObjectBuilder includedBuilder,
IEnumerable<IQueryConstraintProvider> constraintProviders, IResourceContextProvider resourceContextProvider,
IResourceDefinitionAccessor resourceDefinitionAccessor, IResourceObjectBuilderSettingsProvider settingsProvider)
IResourceDefinitionAccessor resourceDefinitionAccessor, IResourceObjectBuilderSettingsProvider settingsProvider,
IEvaluatedIncludeCache evaluatedIncludeCache)
: base(resourceContextProvider, settingsProvider.Get())
{
ArgumentGuard.NotNull(linkBuilder, nameof(linkBuilder));
ArgumentGuard.NotNull(includedBuilder, nameof(includedBuilder));
ArgumentGuard.NotNull(constraintProviders, nameof(constraintProviders));
ArgumentGuard.NotNull(resourceDefinitionAccessor, nameof(resourceDefinitionAccessor));
ArgumentGuard.NotNull(evaluatedIncludeCache, nameof(evaluatedIncludeCache));

_linkBuilder = linkBuilder;
_includedBuilder = includedBuilder;
_constraintProviders = constraintProviders;
_resourceDefinitionAccessor = resourceDefinitionAccessor;
_sparseFieldSetCache = new SparseFieldSetCache(_constraintProviders, resourceDefinitionAccessor);
_evaluatedIncludeCache = evaluatedIncludeCache;
_sparseFieldSetCache = new SparseFieldSetCache(constraintProviders, resourceDefinitionAccessor);
}

public RelationshipEntry Build(IIdentifiable resource, RelationshipAttribute requestRelationship)
Expand Down Expand Up @@ -72,7 +74,7 @@ protected override RelationshipEntry GetRelationshipData(RelationshipAttribute r
ArgumentGuard.NotNull(resource, nameof(resource));

RelationshipEntry relationshipEntry = null;
IReadOnlyCollection<IReadOnlyCollection<RelationshipAttribute>> relationshipChains = GetInclusionChain(relationship);
IReadOnlyCollection<IReadOnlyCollection<RelationshipAttribute>> relationshipChains = GetInclusionChainsStartingWith(relationship);

if (Equals(relationship, _requestRelationship) || relationshipChains.Any())
{
Expand Down Expand Up @@ -116,35 +118,24 @@ private bool IsRelationshipInSparseFieldSet(RelationshipAttribute relationship)
}

/// <summary>
/// Inspects the included relationship chains (see <see cref="IIncludeQueryStringParameterReader" /> to see if <paramref name="relationship" /> should be
/// included or not.
/// Inspects the included relationship chains and selects the ones that starts with the specified relationship.
/// </summary>
private IReadOnlyCollection<IReadOnlyCollection<RelationshipAttribute>> GetInclusionChain(RelationshipAttribute relationship)
private IReadOnlyCollection<IReadOnlyCollection<RelationshipAttribute>> GetInclusionChainsStartingWith(RelationshipAttribute relationship)
{
// @formatter:wrap_chained_method_calls chop_always
// @formatter:keep_existing_linebreaks true

ResourceFieldChainExpression[] chains = _constraintProviders
.SelectMany(provider => provider.GetConstraints())
.Select(expressionInScope => expressionInScope.Expression)
.OfType<IncludeExpression>()
.SelectMany(IncludeChainConverter.GetRelationshipChains)
.ToArray();

// @formatter:keep_existing_linebreaks restore
// @formatter:wrap_chained_method_calls restore
IncludeExpression include = _evaluatedIncludeCache.Get() ?? IncludeExpression.Empty;
IReadOnlyCollection<ResourceFieldChainExpression> chains = IncludeChainConverter.GetRelationshipChains(include);

var inclusionChain = new List<IReadOnlyCollection<RelationshipAttribute>>();
var inclusionChains = new List<IReadOnlyCollection<RelationshipAttribute>>();

foreach (ResourceFieldChainExpression chain in chains)
{
if (chain.Fields.First().Equals(relationship))
{
inclusionChain.Add(chain.Fields.Cast<RelationshipAttribute>().ToArray());
inclusionChains.Add(chain.Fields.Cast<RelationshipAttribute>().ToArray());
}
}

return inclusionChain;
return inclusionChains;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public sealed class TelevisionBroadcastDefinition : JsonApiResourceDefinition<Te
private readonly TelevisionDbContext _dbContext;
private readonly IJsonApiRequest _request;
private readonly IEnumerable<IQueryConstraintProvider> _constraintProviders;
private readonly ResourceContext _broadcastContext;

private DateTimeOffset? _storedArchivedAt;

Expand All @@ -35,7 +34,6 @@ public TelevisionBroadcastDefinition(IResourceGraph resourceGraph, TelevisionDbC
_dbContext = dbContext;
_request = request;
_constraintProviders = constraintProviders;
_broadcastContext = resourceGraph.GetResourceContext<TelevisionBroadcast>();
}

public override FilterExpression OnApplyFilter(FilterExpression existingFilter)
Expand All @@ -46,8 +44,7 @@ public override FilterExpression OnApplyFilter(FilterExpression existingFilter)

if (IsReturningCollectionOfTelevisionBroadcasts() && !HasFilterOnArchivedAt(existingFilter))
{
AttrAttribute archivedAtAttribute =
_broadcastContext.Attributes.Single(attr => attr.Property.Name == nameof(TelevisionBroadcast.ArchivedAt));
AttrAttribute archivedAtAttribute = ResourceContext.Attributes.Single(attr => attr.Property.Name == nameof(TelevisionBroadcast.ArchivedAt));

var archivedAtChain = new ResourceFieldChainExpression(archivedAtAttribute);

Expand All @@ -71,7 +68,7 @@ private bool IsRequestingCollectionOfTelevisionBroadcasts()
{
if (_request.IsCollection)
{
if (_request.PrimaryResource == _broadcastContext || _request.SecondaryResource == _broadcastContext)
if (_request.PrimaryResource == ResourceContext || _request.SecondaryResource == ResourceContext)
{
return true;
}
Expand All @@ -97,7 +94,7 @@ private bool IsIncludingCollectionOfTelevisionBroadcasts()

foreach (IncludeElementExpression includeElement in includeElements)
{
if (includeElement.Relationship is HasManyAttribute && includeElement.Relationship.RightType == _broadcastContext.ResourceType)
if (includeElement.Relationship is HasManyAttribute && includeElement.Relationship.RightType == ResourceContext.ResourceType)
{
return true;
}
Expand Down
Loading