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..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,11 +14,20 @@ 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; - _requestResource = currentRequest.GetRequestResource(); + if (currentRequest.RequestRelationship != null) + { + _requestResource= resourceGraph.GetResourceContext(currentRequest.RequestRelationship.RightType); + } + else + { + _requestResource = _mainRequestResource; + } } protected QueryParameterService() { } @@ -70,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/src/JsonApiDotNetCore/Services/DefaultResourceService.cs b/src/JsonApiDotNetCore/Services/DefaultResourceService.cs index 39efc5a72c..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; } - + public virtual async Task> GetAsync() { _hookExecutor?.BeforeRead(ResourcePipeline.Get); @@ -134,10 +134,15 @@ 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 entityQuery = ApplyInclude(_repository.Get(id), chainPrefix: new List { relationship }); var entity = await _repository.FirstOrDefaultAsync(entityQuery); - if (entity == null) // this does not make sense. If the parent entity is not found, this error is thrown? + 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. @@ -251,12 +256,34 @@ 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) + { + if (chainPrefix != null) + { + inclusionChain.InsertRange(0, chainPrefix); + } + entities = _repository.Include(entities, inclusionChain.ToArray()); + } + } return entities; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs new file mode 100644 index 0000000000..e023590da5 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/NestedResourceTests.cs @@ -0,0 +1,72 @@ +using System.Linq; +using System.Net; +using System.Threading.Tasks; +using Bogus; +using JsonApiDotNetCoreExample.Models; +using JsonApiDotNetCoreExampleTests.Helpers.Models; +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 NestedResourceRoute_RequestWithIncludeQueryParam_ReturnsRequestedRelationships() + { + // 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); + } + + [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); + } + } +} diff --git a/test/UnitTests/Services/EntityResourceService_Tests.cs b/test/UnitTests/Services/EntityResourceService_Tests.cs index f18a88772c..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,7 +99,13 @@ public async Task GetRelationshipAsync_Returns_Relationship_Value() private DefaultResourceService GetService() { - return new DefaultResourceService(new List(), 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); } } }