From d787ff69e6b12426c357a95b2cc2c8594096cd27 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 10:06:01 +0100 Subject: [PATCH 1/7] fix: support include on nested resources route --- .../Models/TodoItem.cs | 6 ++ .../Controllers/JsonApiController.cs | 1 - .../Common/QueryParameterService.cs | 9 ++- .../Services/DefaultResourceService.cs | 33 +++++++++-- .../Acceptance/Spec/CreatingDataTests.cs | 1 + .../Acceptance/Spec/NestedResourceTests.cs | 58 +++++++++++++++++++ 6 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs index a8a88c0cda..815bc52675 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs @@ -38,7 +38,9 @@ public TodoItem() public DateTimeOffset? OffsetDate { get; set; } public int? OwnerId { get; set; } + public int? AssigneeId { get; set; } + public Guid? CollectionId { get; set; } [HasOne] @@ -49,6 +51,7 @@ public TodoItem() [HasOne] public virtual Person OneToOnePerson { get; set; } + public virtual int? OneToOnePersonId { get; set; } [HasMany] @@ -59,13 +62,16 @@ public TodoItem() // cyclical to-one structure public virtual int? DependentOnTodoId { get; set; } + [HasOne] public virtual TodoItem DependentOnTodo { get; set; } // cyclical to-many structure public virtual int? ParentTodoId {get; set;} + [HasOne] public virtual TodoItem ParentTodo { get; set; } + [HasMany] public virtual List ChildrenTodos { get; set; } } diff --git a/src/JsonApiDotNetCore/Controllers/JsonApiController.cs b/src/JsonApiDotNetCore/Controllers/JsonApiController.cs index 6ed4ccca04..b61284ac0b 100644 --- a/src/JsonApiDotNetCore/Controllers/JsonApiController.cs +++ b/src/JsonApiDotNetCore/Controllers/JsonApiController.cs @@ -9,7 +9,6 @@ namespace JsonApiDotNetCore.Controllers { public class JsonApiController : BaseJsonApiController where T : class, IIdentifiable { - /// /// /// diff --git a/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs b/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs index cadd91f6fd..39681288b0 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs @@ -20,7 +20,14 @@ public abstract class QueryParameterService protected QueryParameterService(IResourceGraph resourceGraph, ICurrentRequest currentRequest) { _resourceGraph = resourceGraph; - _requestResource = currentRequest.GetRequestResource(); + if (currentRequest.RequestRelationship != null) + { + _requestResource= resourceGraph.GetResourceContext(currentRequest.RequestRelationship.RightType); + } + else + { + _requestResource = currentRequest.GetRequestResource(); + } } protected QueryParameterService() { } diff --git a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs index 39efc5a72c..4f59db0d9e 100644 --- a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs +++ b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs @@ -134,8 +134,10 @@ public virtual async Task GetRelationshipsAsync(TId id, string relati // TODO: it would be better if we could distinguish whether or not the relationship was not found, // vs the relationship not being set on the instance of T - var entityQuery = _repository.Include(_repository.Get(id), new RelationshipAttribute[] { relationship }); - var entity = await _repository.FirstOrDefaultAsync(entityQuery); + + var query = _repository.Get(id); + query = ApplyInclude(query, chainPrefix: new List { relationship }); + var entity = await _repository.FirstOrDefaultAsync(query); if (entity == null) // this does not make sense. If the parent entity is not found, this error is thrown? throw new JsonApiException(404, $"Relationship '{relationshipName}' not found."); @@ -251,12 +253,31 @@ protected virtual IQueryable ApplyFilter(IQueryable entiti /// /// /// - protected virtual IQueryable ApplyInclude(IQueryable entities) + protected virtual IQueryable ApplyInclude(IQueryable entities, IEnumerable chainPrefix = null) { var chains = _includeService.Get(); - if (chains != null && chains.Any()) - foreach (var r in chains) - entities = _repository.Include(entities, r.ToArray()); + bool hasInclusionChain = chains.Any(); + + if (chains == null) + { + throw new Exception(); + } + + if (chainPrefix != null && !hasInclusionChain) + { + hasInclusionChain = true; + chains.Add(new List()); + } + + + if (hasInclusionChain) + { + foreach (var inclusionChain in chains) + { + inclusionChain.InsertRange(0, chainPrefix); + entities = _repository.Include(entities, inclusionChain.ToArray()); + } + } return entities; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs index 51f10bf1d8..d362e99673 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs @@ -14,6 +14,7 @@ namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec { + public class CreatingDataTests : FunctionalTestCollection { private readonly Faker _todoItemFaker; diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs new file mode 100644 index 0000000000..7e443f7d5b --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs @@ -0,0 +1,58 @@ +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Threading.Tasks; +using Bogus; +using JsonApiDotNetCoreExample.Models; +using JsonApiDotNetCoreExampleTests.Helpers.Models; +using Microsoft.EntityFrameworkCore; +using Xunit; +using Person = JsonApiDotNetCoreExample.Models.Person; + +namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec +{ + public class NestedResourceTests : FunctionalTestCollection + { + private readonly Faker _todoItemFaker; + private readonly Faker _personFaker; + private readonly Faker _passportFaker; + + public NestedResourceTests(StandardApplicationFactory factory) : base(factory) + { + _todoItemFaker = new Faker() + .RuleFor(t => t.Description, f => f.Lorem.Sentence()) + .RuleFor(t => t.Ordinal, f => f.Random.Number()) + .RuleFor(t => t.CreatedDate, f => f.Date.Past()); + _personFaker = new Faker() + .RuleFor(t => t.FirstName, f => f.Name.FirstName()) + .RuleFor(t => t.LastName, f => f.Name.LastName()); + _passportFaker = new Faker() + .RuleFor(t => t.SocialSecurityNumber, f => f.Random.Number()); + } + + [Fact] + public async Task Include_NestedResource_CanInclude() + { + // Arrange + var assignee = _dbContext.Add(_personFaker.Generate()).Entity; + var todo = _dbContext.Add(_todoItemFaker.Generate()).Entity; + var owner = _dbContext.Add(_personFaker.Generate()).Entity; + var passport = _dbContext.Add(_passportFaker.Generate()).Entity; + _dbContext.SaveChanges(); + todo.AssigneeId = assignee.Id; + todo.OwnerId = owner.Id; + owner.PassportId = passport.Id; + _dbContext.SaveChanges(); + + // Act + var (body, response) = await Get($"/api/v1/people/{assignee.Id}/assignedTodoItems?include=owner.passport"); + + // Assert + AssertEqualStatusCode(HttpStatusCode.OK, response); + var resultTodoItem = _deserializer.DeserializeList(body).Data.SingleOrDefault(); + Assert.Equal(todo.Id, resultTodoItem.Id); + Assert.Equal(todo.Owner.Id, resultTodoItem.Owner.Id); + Assert.Equal(todo.Owner.Passport.Id, resultTodoItem.Owner.Passport.Id); + } + } +} From f530d374640cd303aa6f1fa76e4e2a1e0a677ec1 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 10:13:31 +0100 Subject: [PATCH 2/7] fix: related unit test --- src/JsonApiDotNetCore/Services/DefaultResourceService.cs | 5 ++++- test/UnitTests/Services/EntityResourceService_Tests.cs | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs index 4f59db0d9e..509c5b55b5 100644 --- a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs +++ b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs @@ -274,7 +274,10 @@ protected virtual IQueryable ApplyInclude(IQueryable entit { foreach (var inclusionChain in chains) { - inclusionChain.InsertRange(0, chainPrefix); + if (chainPrefix != null) + { + inclusionChain.InsertRange(0, chainPrefix); + } entities = _repository.Include(entities, inclusionChain.ToArray()); } } diff --git a/test/UnitTests/Services/EntityResourceService_Tests.cs b/test/UnitTests/Services/EntityResourceService_Tests.cs index f18a88772c..0b78f501bd 100644 --- a/test/UnitTests/Services/EntityResourceService_Tests.cs +++ b/test/UnitTests/Services/EntityResourceService_Tests.cs @@ -87,7 +87,9 @@ public async Task GetRelationshipAsync_Returns_Relationship_Value() private DefaultResourceService GetService() { - return new DefaultResourceService(new List(), new JsonApiOptions(), _repositoryMock.Object, _resourceGraph); + var includeService = new Mock(); + includeService.Setup(m => m.Get()).Returns(new List>()); + return new DefaultResourceService(new List() { includeService.Object }, new JsonApiOptions(), _repositoryMock.Object, _resourceGraph); } } } From daab069179a2c7c15b4e98393ee4400aafc34d76 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 12:08:36 +0100 Subject: [PATCH 3/7] test: add nested resource filter test --- .../Services/DefaultResourceService.cs | 17 +++++++++--- .../Acceptance/Spec/NestedResourceTests.cs | 27 ++++++++++++++++--- .../Services/EntityResourceService_Tests.cs | 22 ++++++++++++--- 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs index 509c5b55b5..8463307b33 100644 --- a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs +++ b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs @@ -135,11 +135,20 @@ public virtual async Task GetRelationshipsAsync(TId id, string relati // TODO: it would be better if we could distinguish whether or not the relationship was not found, // vs the relationship not being set on the instance of T - var query = _repository.Get(id); - query = ApplyInclude(query, chainPrefix: new List { relationship }); - var entity = await _repository.FirstOrDefaultAsync(query); - if (entity == null) // this does not make sense. If the parent entity is not found, this error is thrown? + var entityQuery = _repository.Get(id); + + entityQuery = ApplyFilter(entityQuery); + entityQuery = ApplySort(entityQuery); + entityQuery = ApplyInclude(entityQuery, chainPrefix: new List { relationship }); + entityQuery = ApplySelect(entityQuery); + + var entity = await _repository.FirstOrDefaultAsync(entityQuery); + if (entity == null) + { + /// TODO: this does not make sense. If the **parent** entity is not found, this error is thrown? + /// this error should be thrown when the relationship is not found. throw new JsonApiException(404, $"Relationship '{relationshipName}' not found."); + } if (!IsNull(_hookExecutor, entity)) { // AfterRead and OnReturn resource hook execution. diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs index 7e443f7d5b..b242c68242 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs @@ -1,11 +1,9 @@ -using System.Collections.Generic; -using System.Linq; +using System.Linq; using System.Net; using System.Threading.Tasks; using Bogus; using JsonApiDotNetCoreExample.Models; using JsonApiDotNetCoreExampleTests.Helpers.Models; -using Microsoft.EntityFrameworkCore; using Xunit; using Person = JsonApiDotNetCoreExample.Models.Person; @@ -31,7 +29,7 @@ public NestedResourceTests(StandardApplicationFactory factory) : base(factory) } [Fact] - public async Task Include_NestedResource_CanInclude() + public async Task NestedResourceRoute_IncludeDeeplyNestedRelationship_ReturnsRequestedRelationships() { // Arrange var assignee = _dbContext.Add(_personFaker.Generate()).Entity; @@ -54,5 +52,26 @@ public async Task Include_NestedResource_CanInclude() Assert.Equal(todo.Owner.Id, resultTodoItem.Owner.Id); Assert.Equal(todo.Owner.Passport.Id, resultTodoItem.Owner.Passport.Id); } + + [Fact] + public async Task NestedResourceRoute_Filter_ReturnsFilteredResources() + { + // Arrange + var assignee = _personFaker.Generate(); + //var assignee = _dbContext.Add(_personFaker.Generate()).Entity; + //var todos = _todoItemFaker.Generate(10).ToList(); + assignee.AssignedTodoItems = _todoItemFaker.Generate(10).ToList(); + //assignee.AssignedTodoItems = todos; + _dbContext.Add(assignee); + var ordinal = assignee.AssignedTodoItems.First().Ordinal; + + // Act + var (body, response) = await Get($"/api/v1/people/{assignee.Id}/assignedTodoItems?filter[ordinal]={ordinal}"); + + // Assert + AssertEqualStatusCode(HttpStatusCode.OK, response); + var resultTodoItem = _deserializer.DeserializeList(body).Data.SingleOrDefault(); + Assert.Equal(assignee.AssignedTodoItems.First().Id, resultTodoItem.Id); + } } } diff --git a/test/UnitTests/Services/EntityResourceService_Tests.cs b/test/UnitTests/Services/EntityResourceService_Tests.cs index 0b78f501bd..4fb8565927 100644 --- a/test/UnitTests/Services/EntityResourceService_Tests.cs +++ b/test/UnitTests/Services/EntityResourceService_Tests.cs @@ -19,9 +19,21 @@ public class EntityResourceService_Tests { private readonly Mock> _repositoryMock = new Mock>(); private readonly IResourceGraph _resourceGraph; + private readonly Mock _includeService; + private readonly Mock _sparseFieldsService; + private readonly Mock _pageService; + + public Mock _sortService { get; } + public Mock _filterService { get; } public EntityResourceService_Tests() { + _includeService = new Mock(); + _includeService.Setup(m => m.Get()).Returns(new List>()); + _sparseFieldsService = new Mock(); + _pageService = new Mock(); + _sortService = new Mock(); + _filterService = new Mock(); _resourceGraph = new ResourceGraphBuilder() .AddResource() .AddResource() @@ -87,9 +99,13 @@ public async Task GetRelationshipAsync_Returns_Relationship_Value() private DefaultResourceService GetService() { - var includeService = new Mock(); - includeService.Setup(m => m.Get()).Returns(new List>()); - return new DefaultResourceService(new List() { includeService.Object }, new JsonApiOptions(), _repositoryMock.Object, _resourceGraph); + var queryParamServices = new List + { + _includeService.Object, _pageService.Object, _filterService.Object, + _sortService.Object, _sparseFieldsService.Object + }; + + return new DefaultResourceService(queryParamServices, new JsonApiOptions(), _repositoryMock.Object, _resourceGraph); } } } From 5a33e72cde9df20246de829c7020e0817974563c Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 13:07:43 +0100 Subject: [PATCH 4/7] chore: abort attempt to fix filter/sort/select/page --- .../Services/DefaultResourceService.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs index 8463307b33..0d92389874 100644 --- a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs +++ b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs @@ -80,7 +80,7 @@ public virtual async Task DeleteAsync(TId id) if (!IsNull(_hookExecutor, entity)) _hookExecutor.AfterDelete(AsList(entity), ResourcePipeline.Delete, succeeded); return succeeded; } - + g public virtual async Task> GetAsync() { _hookExecutor?.BeforeRead(ResourcePipeline.Get); @@ -135,13 +135,7 @@ public virtual async Task GetRelationshipsAsync(TId id, string relati // TODO: it would be better if we could distinguish whether or not the relationship was not found, // vs the relationship not being set on the instance of T - var entityQuery = _repository.Get(id); - - entityQuery = ApplyFilter(entityQuery); - entityQuery = ApplySort(entityQuery); - entityQuery = ApplyInclude(entityQuery, chainPrefix: new List { relationship }); - entityQuery = ApplySelect(entityQuery); - + var entityQuery = ApplyInclude(_repository.Get(id), chainPrefix: new List { relationship }); var entity = await _repository.FirstOrDefaultAsync(entityQuery); if (entity == null) { From 2082d1b970371094f67313f0a3df154ae5762aae Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 13:09:17 +0100 Subject: [PATCH 5/7] fix: rogue character --- .../Services/DefaultResourceService.cs | 2 +- .../Acceptance/Spec/NestedResourceTests.cs | 21 ------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs index 0d92389874..86b5a2d976 100644 --- a/src/JsonApiDotNetCore/Services/DefaultResourceService.cs +++ b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs @@ -80,7 +80,7 @@ public virtual async Task DeleteAsync(TId id) if (!IsNull(_hookExecutor, entity)) _hookExecutor.AfterDelete(AsList(entity), ResourcePipeline.Delete, succeeded); return succeeded; } - g + public virtual async Task> GetAsync() { _hookExecutor?.BeforeRead(ResourcePipeline.Get); diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs index b242c68242..266d7679d2 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs @@ -52,26 +52,5 @@ public async Task NestedResourceRoute_IncludeDeeplyNestedRelationship_ReturnsReq Assert.Equal(todo.Owner.Id, resultTodoItem.Owner.Id); Assert.Equal(todo.Owner.Passport.Id, resultTodoItem.Owner.Passport.Id); } - - [Fact] - public async Task NestedResourceRoute_Filter_ReturnsFilteredResources() - { - // Arrange - var assignee = _personFaker.Generate(); - //var assignee = _dbContext.Add(_personFaker.Generate()).Entity; - //var todos = _todoItemFaker.Generate(10).ToList(); - assignee.AssignedTodoItems = _todoItemFaker.Generate(10).ToList(); - //assignee.AssignedTodoItems = todos; - _dbContext.Add(assignee); - var ordinal = assignee.AssignedTodoItems.First().Ordinal; - - // Act - var (body, response) = await Get($"/api/v1/people/{assignee.Id}/assignedTodoItems?filter[ordinal]={ordinal}"); - - // Assert - AssertEqualStatusCode(HttpStatusCode.OK, response); - var resultTodoItem = _deserializer.DeserializeList(body).Data.SingleOrDefault(); - Assert.Equal(assignee.AssignedTodoItems.First().Id, resultTodoItem.Id); - } } } From 5f920a22722a85f6f3a6854d2303ec8109efe396 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 13:28:21 +0100 Subject: [PATCH 6/7] feat: 400 on unsupported nested resources query params --- .../Common/QueryParameterService.cs | 19 +++++++++++++++---- .../QueryParameterServices/FilterService.cs | 1 + .../QueryParameterServices/PageService.cs | 14 +++++++++++++- .../QueryParameterServices/SortService.cs | 1 + .../SparseFieldsService.cs | 1 + .../Acceptance/Spec/NestedResourceTests.cs | 18 +++++++++++++++++- 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs b/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs index 39681288b0..577040a24e 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterService.cs @@ -1,11 +1,9 @@ -using System.Collections.Generic; -using System.Linq; +using System.Linq; using System.Text.RegularExpressions; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Internal.Contracts; using JsonApiDotNetCore.Managers.Contracts; using JsonApiDotNetCore.Models; -using Microsoft.Extensions.Primitives; namespace JsonApiDotNetCore.Query { @@ -16,9 +14,11 @@ public abstract class QueryParameterService { protected readonly IResourceGraph _resourceGraph; protected readonly ResourceContext _requestResource; + private readonly ResourceContext _mainRequestResource; protected QueryParameterService(IResourceGraph resourceGraph, ICurrentRequest currentRequest) { + _mainRequestResource = currentRequest.GetRequestResource(); _resourceGraph = resourceGraph; if (currentRequest.RequestRelationship != null) { @@ -26,7 +26,7 @@ protected QueryParameterService(IResourceGraph resourceGraph, ICurrentRequest cu } else { - _requestResource = currentRequest.GetRequestResource(); + _requestResource = _mainRequestResource; } } @@ -77,5 +77,16 @@ protected RelationshipAttribute GetRelationship(string propertyName) return relationship; } + + /// + /// Throw an exception if query parameters are requested that are unsupported on nested resource routes. + /// + protected void EnsureNoNestedResourceRoute() + { + if (_requestResource != _mainRequestResource) + { + throw new JsonApiException(400, $"Query parameter {Name} is currently not supported on nested resource endpoints (i.e. of the form '/article/1/author?{Name}=...'"); + } + } } } diff --git a/src/JsonApiDotNetCore/QueryParameterServices/FilterService.cs b/src/JsonApiDotNetCore/QueryParameterServices/FilterService.cs index feed8c49fe..bf921383ef 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/FilterService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/FilterService.cs @@ -31,6 +31,7 @@ public List Get() /// public virtual void Parse(KeyValuePair queryParameter) { + EnsureNoNestedResourceRoute(); var queries = GetFilterQueries(queryParameter); _filters.AddRange(queries.Select(GetQueryContexts)); } diff --git a/src/JsonApiDotNetCore/QueryParameterServices/PageService.cs b/src/JsonApiDotNetCore/QueryParameterServices/PageService.cs index 2827eaa2d9..b0b099058d 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/PageService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/PageService.cs @@ -2,7 +2,9 @@ using System.Collections.Generic; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Internal; +using JsonApiDotNetCore.Internal.Contracts; using JsonApiDotNetCore.Internal.Query; +using JsonApiDotNetCore.Managers.Contracts; using Microsoft.Extensions.Primitives; namespace JsonApiDotNetCore.Query @@ -12,7 +14,16 @@ public class PageService : QueryParameterService, IPageService { private readonly IJsonApiOptions _options; - public PageService(IJsonApiOptions options) + public PageService(IJsonApiOptions options, IResourceGraph resourceGraph, ICurrentRequest currentRequest) : base(resourceGraph, currentRequest) + { + _options = options; + PageSize = _options.DefaultPageSize; + } + + /// + /// constructor used for unit testing + /// + internal PageService(IJsonApiOptions options) { _options = options; PageSize = _options.DefaultPageSize; @@ -39,6 +50,7 @@ public PageService(IJsonApiOptions options) /// public virtual void Parse(KeyValuePair queryParameter) { + EnsureNoNestedResourceRoute(); // expected input = page[size]= // page[number]= 0> var propertyName = queryParameter.Key.Split(QueryConstants.OPEN_BRACKET, QueryConstants.CLOSE_BRACKET)[1]; diff --git a/src/JsonApiDotNetCore/QueryParameterServices/SortService.cs b/src/JsonApiDotNetCore/QueryParameterServices/SortService.cs index cd5a283e07..1212a9b486 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/SortService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/SortService.cs @@ -28,6 +28,7 @@ public SortService(IResourceDefinitionProvider resourceDefinitionProvider, /// public virtual void Parse(KeyValuePair queryParameter) { + EnsureNoNestedResourceRoute(); CheckIfProcessed(); // disallow multiple sort parameters. var queries = BuildQueries(queryParameter.Value); diff --git a/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs b/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs index 28b9312962..20b52a079e 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs @@ -45,6 +45,7 @@ public virtual void Parse(KeyValuePair queryParameter) { // expected: articles?fields=prop1,prop2 // articles?fields[articles]=prop1,prop2 <-- this form in invalid UNLESS "articles" is actually a relationship on Article // articles?fields[relationship]=prop1,prop2 + EnsureNoNestedResourceRoute(); var fields = new List { nameof(Identifiable.Id) }; fields.AddRange(((string)queryParameter.Value).Split(QueryConstants.COMMA)); diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs index 266d7679d2..e023590da5 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs @@ -29,7 +29,7 @@ public NestedResourceTests(StandardApplicationFactory factory) : base(factory) } [Fact] - public async Task NestedResourceRoute_IncludeDeeplyNestedRelationship_ReturnsRequestedRelationships() + public async Task NestedResourceRoute_RequestWithIncludeQueryParam_ReturnsRequestedRelationships() { // Arrange var assignee = _dbContext.Add(_personFaker.Generate()).Entity; @@ -52,5 +52,21 @@ public async Task NestedResourceRoute_IncludeDeeplyNestedRelationship_ReturnsReq Assert.Equal(todo.Owner.Id, resultTodoItem.Owner.Id); Assert.Equal(todo.Owner.Passport.Id, resultTodoItem.Owner.Passport.Id); } + + [Theory] + [InlineData("filter[ordinal]=1")] + [InlineData("fields=ordinal")] + [InlineData("sort=ordinal")] + [InlineData("page[number]=1")] + [InlineData("page[size]=10")] + public async Task NestedResourceRoute_RequestWithUnsupportedQueryParam_ReturnsBadRequest(string queryParam) + { + // Act + var (body, response) = await Get($"/api/v1/people/1/assignedTodoItems?{queryParam}"); + + // Assert + AssertEqualStatusCode(HttpStatusCode.BadRequest, response); + Assert.Contains("currently not supported", body); + } } } From e65a02eaa67f851c8f7f3d635b7a327297c7489f Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Fri, 22 Nov 2019 13:29:29 +0100 Subject: [PATCH 7/7] fix: redundant spacing --- .../Acceptance/Spec/CreatingDataTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs index d362e99673..51f10bf1d8 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs @@ -14,7 +14,6 @@ namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec { - public class CreatingDataTests : FunctionalTestCollection { private readonly Faker _todoItemFaker;