diff --git a/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs b/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs index e6518bc7e2..e7ea772582 100644 --- a/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs +++ b/src/JsonApiDotNetCore/QueryParameterServices/SparseFieldsService.cs @@ -39,41 +39,71 @@ public List Get(RelationshipAttribute relationship = null) _selectedRelationshipFields.TryGetValue(relationship, out var fields); return fields; } - + /// public virtual void Parse(KeyValuePair queryParameter) - { - // expected: fields[TYPE]=prop1,prop2 - var typeName = queryParameter.Key.Split(QueryConstants.OPEN_BRACKET, QueryConstants.CLOSE_BRACKET)[1]; + { // 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 var fields = new List { nameof(Identifiable.Id) }; + fields.AddRange(((string)queryParameter.Value).Split(QueryConstants.COMMA)); - var relationship = _requestResource.Relationships.SingleOrDefault(a => a.Is(typeName)); - if (relationship == null && string.Equals(typeName, _requestResource.EntityName, StringComparison.OrdinalIgnoreCase) == false) - throw new JsonApiException(400, $"fields[{typeName}] is invalid"); + var keySplitted = queryParameter.Key.Split(QueryConstants.OPEN_BRACKET, QueryConstants.CLOSE_BRACKET); - fields.AddRange(((string)queryParameter.Value).Split(QueryConstants.COMMA)); - foreach (var field in fields) - { - if (relationship != default) - { - var relationProperty = _contextEntityProvider.GetContextEntity(relationship.DependentType); - var attr = relationProperty.Attributes.SingleOrDefault(a => a.Is(field)); - if (attr == null) - throw new JsonApiException(400, $"'{relationship.DependentType.Name}' does not contain '{field}'."); + if (keySplitted.Count() == 1) + { // input format: fields=prop1,prop2 + foreach (var field in fields) + RegisterRequestResourceField(field); + } + else + { // input format: fields[articles]=prop1,prop2 + string navigation = keySplitted[1]; + // it is possible that the request resource has a relationship + // that is equal to the resource name, like with self-referering data types (eg directory structures) + // if not, no longer support this type of sparse field selection. + if (navigation == _requestResource.EntityName && !_requestResource.Relationships.Any(a => a.Is(navigation))) + throw new JsonApiException(400, $"Use \"?fields=...\" instead of \"fields[{navigation}]\":" + + $" the square bracket navigations is now reserved " + + $"for relationships only. See https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/555#issuecomment-543100865"); - if (!_selectedRelationshipFields.TryGetValue(relationship, out var registeredFields)) - _selectedRelationshipFields.Add(relationship, registeredFields = new List()); - registeredFields.Add(attr); - } - else - { - var attr = _requestResource.Attributes.SingleOrDefault(a => a.Is(field)); - if (attr == null) - throw new JsonApiException(400, $"'{_requestResource.EntityName}' does not contain '{field}'."); + if (navigation.Contains(QueryConstants.DOT)) + throw new JsonApiException(400, $"fields[{navigation}] is not valid: deeply nested sparse field selection is not yet supported."); + + var relationship = _requestResource.Relationships.SingleOrDefault(a => a.Is(navigation)); + if (relationship == null) + throw new JsonApiException(400, $"\"{navigation}\" in \"fields[{navigation}]\" is not a valid relationship of {_requestResource.EntityName}"); + + foreach (var field in fields) + RegisterRelatedResourceField(field, relationship); - (_selectedFields = _selectedFields ?? new List()).Add(attr); - } } } + + /// + /// Registers field selection queries of the form articles?fields[author]=first-name + /// + private void RegisterRelatedResourceField(string field, RelationshipAttribute relationship) + { + var relationProperty = _contextEntityProvider.GetContextEntity(relationship.DependentType); + var attr = relationProperty.Attributes.SingleOrDefault(a => a.Is(field)); + if (attr == null) + throw new JsonApiException(400, $"'{relationship.DependentType.Name}' does not contain '{field}'."); + + if (!_selectedRelationshipFields.TryGetValue(relationship, out var registeredFields)) + _selectedRelationshipFields.Add(relationship, registeredFields = new List()); + registeredFields.Add(attr); + } + + /// + /// Registers field selection queries of the form articles?fields=title + /// + private void RegisterRequestResourceField(string field) + { + var attr = _requestResource.Attributes.SingleOrDefault(a => a.Is(field)); + if (attr == null) + throw new JsonApiException(400, $"'{_requestResource.EntityName}' does not contain '{field}'."); + + (_selectedFields = _selectedFields ?? new List()).Add(attr); + } } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs index feba7e7ce5..ea230a8261 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs @@ -104,7 +104,7 @@ public async Task Fields_Query_Selects_Sparse_Field_Sets() var server = new TestServer(builder); var client = server.CreateClient(); - var route = $"/api/v1/todo-items/{todoItem.Id}?fields[todo-items]=description,created-date"; + var route = $"/api/v1/todo-items/{todoItem.Id}?fields=description,created-date"; var request = new HttpRequestMessage(httpMethod, route); // act @@ -119,6 +119,36 @@ public async Task Fields_Query_Selects_Sparse_Field_Sets() Assert.Equal(todoItem.CreatedDate.ToString("G"), ((DateTime)deserializeBody.SingleData.Attributes["created-date"]).ToString("G")); } + [Fact] + public async Task Fields_Query_Selects_Sparse_Field_Sets_With_Type_As_Navigation() + { + // arrange + var todoItem = new TodoItem + { + Description = "description", + Ordinal = 1, + CreatedDate = DateTime.Now + }; + _dbContext.TodoItems.Add(todoItem); + await _dbContext.SaveChangesAsync(); + + var builder = new WebHostBuilder() + .UseStartup(); + var httpMethod = new HttpMethod("GET"); + var server = new TestServer(builder); + var client = server.CreateClient(); + var route = $"/api/v1/todo-items/{todoItem.Id}?fields[todo-items]=description,created-date"; + var request = new HttpRequestMessage(httpMethod, route); + + // act + var response = await client.SendAsync(request); + var body = await response.Content.ReadAsStringAsync(); + + // assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Contains("relationships only", body); + } + [Fact] public async Task Fields_Query_Selects_All_Fieldset_With_HasOne() { diff --git a/test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs b/test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs index eeb0648110..03f855d901 100644 --- a/test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs +++ b/test/UnitTests/QueryParameters/SparseFieldsServiceTests.cs @@ -38,7 +38,7 @@ public void Parse_ValidSelection_CanParse() var attribute = new AttrAttribute(attrName) { InternalAttributeName = internalAttrName }; var idAttribute = new AttrAttribute("id") { InternalAttributeName = "Id" }; - var query = new KeyValuePair($"fields[{type}]", new StringValues(attrName)); + var query = new KeyValuePair($"fields", new StringValues(attrName)); var contextEntity = new ContextEntity { @@ -58,6 +58,57 @@ public void Parse_ValidSelection_CanParse() Assert.Equal(attribute, result[1]); } + [Fact] + public void Parse_TypeNameAsNavigation_Throws400ErrorWithRelationshipsOnlyMessage() + { + // arrange + const string type = "articles"; + const string attrName = "some-field"; + const string internalAttrName = "SomeField"; + var attribute = new AttrAttribute(attrName) { InternalAttributeName = internalAttrName }; + var idAttribute = new AttrAttribute("id") { InternalAttributeName = "Id" }; + + var query = new KeyValuePair($"fields[{type}]", new StringValues(attrName)); + + var contextEntity = new ContextEntity + { + EntityName = type, + Attributes = new List { attribute, idAttribute }, + Relationships = new List() + }; + var service = GetService(contextEntity); + + // act, assert + var ex = Assert.Throws(() => service.Parse(query)); + Assert.Contains("relationships only", ex.Message); + } + + [Fact] + public void Parse_DeeplyNestedSelection_Throws400ErrorWithDeeplyNestedMessage() + { + // arrange + const string type = "articles"; + const string relationship = "author.employer"; + const string attrName = "some-field"; + const string internalAttrName = "SomeField"; + var attribute = new AttrAttribute(attrName) { InternalAttributeName = internalAttrName }; + var idAttribute = new AttrAttribute("id") { InternalAttributeName = "Id" }; + + var query = new KeyValuePair($"fields[{relationship}]", new StringValues(attrName)); + + var contextEntity = new ContextEntity + { + EntityName = type, + Attributes = new List { attribute, idAttribute }, + Relationships = new List() + }; + var service = GetService(contextEntity); + + // act, assert + var ex = Assert.Throws(() => service.Parse(query)); + Assert.Contains("deeply nested", ex.Message); + } + [Fact] public void Parse_InvalidField_ThrowsJsonApiException() {