From 6b530b0c65445ba187340bd95c40af7596eda964 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Tue, 19 Dec 2017 06:43:50 -0600 Subject: [PATCH 1/3] feat(AttrAttribute): add isSortable and isFilterable --- src/JsonApiDotNetCore/AssemblyInfo.cs | 4 ++++ .../Internal/Query/AttrFilterQuery.cs | 14 ++++++++++---- .../Internal/Query/RelatedAttrFilterQuery.cs | 10 ++++++++-- src/JsonApiDotNetCore/Models/AttrAttribute.cs | 15 +++++++++------ .../Models/RelationshipAttribute.cs | 8 ++++---- src/JsonApiDotNetCore/Services/QueryParser.cs | 3 +++ 6 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 src/JsonApiDotNetCore/AssemblyInfo.cs diff --git a/src/JsonApiDotNetCore/AssemblyInfo.cs b/src/JsonApiDotNetCore/AssemblyInfo.cs new file mode 100644 index 0000000000..f03399b449 --- /dev/null +++ b/src/JsonApiDotNetCore/AssemblyInfo.cs @@ -0,0 +1,4 @@ +using System.Runtime.CompilerServices; +[assembly:InternalsVisibleTo("UnitTests")] +[assembly:InternalsVisibleTo("JsonApiDotNetCoreExampleTests")] +[assembly:InternalsVisibleTo("NoEntityFrameworkTests")] diff --git a/src/JsonApiDotNetCore/Internal/Query/AttrFilterQuery.cs b/src/JsonApiDotNetCore/Internal/Query/AttrFilterQuery.cs index 74db2b342e..e94f872b54 100644 --- a/src/JsonApiDotNetCore/Internal/Query/AttrFilterQuery.cs +++ b/src/JsonApiDotNetCore/Internal/Query/AttrFilterQuery.cs @@ -17,14 +17,20 @@ public AttrFilterQuery( var attribute = GetAttribute(filterQuery.Attribute); - FilteredAttribute = attribute ?? throw new JsonApiException(400, $"'{filterQuery.Attribute}' is not a valid attribute."); + if(attribute == null) + throw new JsonApiException(400, $"'{filterQuery.Attribute}' is not a valid attribute."); + + if(attribute.IsFilterable == false) + throw new JsonApiException(400, $"Filter is not allowed for attribute '{attribute.PublicAttributeName}'."); + + FilteredAttribute = attribute; PropertyValue = filterQuery.Value; FilterOperation = GetFilterOperation(filterQuery.Operation); } - public AttrAttribute FilteredAttribute { get; set; } - public string PropertyValue { get; set; } - public FilterOperations FilterOperation { get; set; } + public AttrAttribute FilteredAttribute { get; } + public string PropertyValue { get; } + public FilterOperations FilterOperation { get; } private AttrAttribute GetAttribute(string attribute) => _jsonApiContext.RequestEntity.Attributes.FirstOrDefault( diff --git a/src/JsonApiDotNetCore/Internal/Query/RelatedAttrFilterQuery.cs b/src/JsonApiDotNetCore/Internal/Query/RelatedAttrFilterQuery.cs index 3b249c176c..52d7c66f41 100644 --- a/src/JsonApiDotNetCore/Internal/Query/RelatedAttrFilterQuery.cs +++ b/src/JsonApiDotNetCore/Internal/Query/RelatedAttrFilterQuery.cs @@ -19,12 +19,18 @@ public RelatedAttrFilterQuery( var relationship = GetRelationship(relationshipArray[0]); if (relationship == null) - throw new JsonApiException(400, $"{relationshipArray[0]} is not a valid relationship."); + throw new JsonApiException(400, $"{relationshipArray[1]} is not a valid relationship on {relationshipArray[0]}."); var attribute = GetAttribute(relationship, relationshipArray[1]); + + if(attribute == null) + throw new JsonApiException(400, $"'{filterQuery.Attribute}' is not a valid attribute."); + + if(attribute.IsFilterable == false) + throw new JsonApiException(400, $"Filter is not allowed for attribute '{attribute.PublicAttributeName}'."); FilteredRelationship = relationship; - FilteredAttribute = attribute ?? throw new JsonApiException(400, $"{relationshipArray[1]} is not a valid attribute on {relationshipArray[0]}."); + FilteredAttribute = attribute; PropertyValue = filterQuery.Value; FilterOperation = GetFilterOperation(filterQuery.Operation); } diff --git a/src/JsonApiDotNetCore/Models/AttrAttribute.cs b/src/JsonApiDotNetCore/Models/AttrAttribute.cs index f84fd229ef..9fa869aeb0 100644 --- a/src/JsonApiDotNetCore/Models/AttrAttribute.cs +++ b/src/JsonApiDotNetCore/Models/AttrAttribute.cs @@ -1,27 +1,30 @@ using System; -using System.Reflection; using JsonApiDotNetCore.Internal; namespace JsonApiDotNetCore.Models { public class AttrAttribute : Attribute { - public AttrAttribute(string publicName, bool isImmutable = false) + public AttrAttribute(string publicName, bool isImmutable = false, bool isFilterable = true, bool isSortable = true) { PublicAttributeName = publicName; IsImmutable = isImmutable; + IsFilterable = isFilterable; + IsSortable = isSortable; } - public AttrAttribute(string publicName, string internalName, bool isImmutable = false) + internal AttrAttribute(string publicName, string internalName, bool isImmutable = false) { PublicAttributeName = publicName; InternalAttributeName = internalName; IsImmutable = isImmutable; } - public string PublicAttributeName { get; set; } - public string InternalAttributeName { get; set; } - public bool IsImmutable { get; set; } + public string PublicAttributeName { get; } + public string InternalAttributeName { get; } + public bool IsImmutable { get; } + public bool IsFilterable { get; } + public bool IsSortable { get; } public object GetValue(object entity) { diff --git a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs index 93662032a5..4f6d42f47e 100644 --- a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs @@ -10,12 +10,12 @@ protected RelationshipAttribute(string publicName, Link documentLinks) DocumentLinks = documentLinks; } - public string PublicRelationshipName { get; set; } - public string InternalRelationshipName { get; set; } - public Type Type { get; set; } + public string PublicRelationshipName { get; } + public string InternalRelationshipName { get; } + public Type Type { get; } public bool IsHasMany => GetType() == typeof(HasManyAttribute); public bool IsHasOne => GetType() == typeof(HasOneAttribute); - public Link DocumentLinks { get; set; } = Link.All; + public Link DocumentLinks { get; } = Link.All; public abstract void SetValue(object entity, object newValue); diff --git a/src/JsonApiDotNetCore/Services/QueryParser.cs b/src/JsonApiDotNetCore/Services/QueryParser.cs index 2a97723347..d9dfab532d 100644 --- a/src/JsonApiDotNetCore/Services/QueryParser.cs +++ b/src/JsonApiDotNetCore/Services/QueryParser.cs @@ -159,6 +159,9 @@ protected virtual List ParseSortParameters(string value) var attribute = GetAttribute(propertyName); + if(attribute.IsSortable == false) + throw new JsonApiException(400, $"Sort is not allowed for attribute '{attribute.PublicAttributeName}'."); + sortParameters.Add(new SortQuery(direction, attribute)); }; From 80fb6db922b63d839ab72770220c220171a1fa9c Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Tue, 19 Dec 2017 19:57:34 -0600 Subject: [PATCH 2/3] fix(Attr/RelationshipAttibute): make setters internal --- benchmarks/Query/QueryParser_Benchmarks.cs | 4 +--- src/JsonApiDotNetCore/AssemblyInfo.cs | 1 + src/JsonApiDotNetCore/Models/AttrAttribute.cs | 2 +- src/JsonApiDotNetCore/Models/HasManyAttribute.cs | 6 +----- src/JsonApiDotNetCore/Models/HasOneAttribute.cs | 6 +----- src/JsonApiDotNetCore/Models/RelationshipAttribute.cs | 4 ++-- 6 files changed, 7 insertions(+), 16 deletions(-) diff --git a/benchmarks/Query/QueryParser_Benchmarks.cs b/benchmarks/Query/QueryParser_Benchmarks.cs index 61fe3b1bc2..de82baa60f 100644 --- a/benchmarks/Query/QueryParser_Benchmarks.cs +++ b/benchmarks/Query/QueryParser_Benchmarks.cs @@ -24,9 +24,7 @@ public QueryParser_Benchmarks() { var controllerContextMock = new Mock(); controllerContextMock.Setup(m => m.RequestEntity).Returns(new ContextEntity { Attributes = new List { - new AttrAttribute(ATTRIBUTE) { - InternalAttributeName = ATTRIBUTE - } + new AttrAttribute(ATTRIBUTE, ATTRIBUTE) } }); var options = new JsonApiOptions(); diff --git a/src/JsonApiDotNetCore/AssemblyInfo.cs b/src/JsonApiDotNetCore/AssemblyInfo.cs index f03399b449..f11bb5d594 100644 --- a/src/JsonApiDotNetCore/AssemblyInfo.cs +++ b/src/JsonApiDotNetCore/AssemblyInfo.cs @@ -2,3 +2,4 @@ [assembly:InternalsVisibleTo("UnitTests")] [assembly:InternalsVisibleTo("JsonApiDotNetCoreExampleTests")] [assembly:InternalsVisibleTo("NoEntityFrameworkTests")] +[assembly:InternalsVisibleTo("Benchmarks")] diff --git a/src/JsonApiDotNetCore/Models/AttrAttribute.cs b/src/JsonApiDotNetCore/Models/AttrAttribute.cs index 9fa869aeb0..5be036636d 100644 --- a/src/JsonApiDotNetCore/Models/AttrAttribute.cs +++ b/src/JsonApiDotNetCore/Models/AttrAttribute.cs @@ -21,7 +21,7 @@ internal AttrAttribute(string publicName, string internalName, bool isImmutable } public string PublicAttributeName { get; } - public string InternalAttributeName { get; } + public string InternalAttributeName { get; internal set; } public bool IsImmutable { get; } public bool IsFilterable { get; } public bool IsSortable { get; } diff --git a/src/JsonApiDotNetCore/Models/HasManyAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyAttribute.cs index 379458014b..b4fd1b42ec 100644 --- a/src/JsonApiDotNetCore/Models/HasManyAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyAttribute.cs @@ -1,14 +1,10 @@ -using System.Reflection; - namespace JsonApiDotNetCore.Models { public class HasManyAttribute : RelationshipAttribute { public HasManyAttribute(string publicName, Link documentLinks = Link.All) : base(publicName, documentLinks) - { - PublicRelationshipName = publicName; - } + { } public override void SetValue(object entity, object newValue) { diff --git a/src/JsonApiDotNetCore/Models/HasOneAttribute.cs b/src/JsonApiDotNetCore/Models/HasOneAttribute.cs index 296b71369e..0dd20e73e7 100644 --- a/src/JsonApiDotNetCore/Models/HasOneAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasOneAttribute.cs @@ -1,14 +1,10 @@ -using System.Reflection; - namespace JsonApiDotNetCore.Models { public class HasOneAttribute : RelationshipAttribute { public HasOneAttribute(string publicName, Link documentLinks = Link.All) : base(publicName, documentLinks) - { - PublicRelationshipName = publicName; - } + { } public override void SetValue(object entity, object newValue) { diff --git a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs index 4f6d42f47e..852b602bea 100644 --- a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs @@ -11,8 +11,8 @@ protected RelationshipAttribute(string publicName, Link documentLinks) } public string PublicRelationshipName { get; } - public string InternalRelationshipName { get; } - public Type Type { get; } + public string InternalRelationshipName { get; internal set; } + public Type Type { get; internal set; } public bool IsHasMany => GetType() == typeof(HasManyAttribute); public bool IsHasOne => GetType() == typeof(HasOneAttribute); public Link DocumentLinks { get; } = Link.All; From 84e4956aa1fd67211fc1fd80ec4c5d531d5a9421 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 3 Jan 2018 06:00:41 -0600 Subject: [PATCH 3/3] test(acceptance): verify filter/sort limitations --- .../Models/TodoItem.cs | 2 +- .../Acceptance/Spec/AttributeFilterTests.cs | 37 +++++++++++-------- .../Acceptance/Spec/AttributeSortTests.cs | 34 +++++++++++++++++ 3 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeSortTests.cs diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs index 7257835791..fecd16319d 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs @@ -22,7 +22,7 @@ public TodoItem() [Attr("created-date")] public DateTime CreatedDate { get; set; } - [Attr("achieved-date")] + [Attr("achieved-date", isFilterable: false, isSortable: false)] public DateTime? AchievedDate { get; set; } public int? OwnerId { get; set; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs index 740a61193c..dd7c673b4c 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeFilterTests.cs @@ -1,4 +1,5 @@ -using System.Linq; +using System; +using System.Linq; using System.Net; using System.Net.Http; using System.Threading.Tasks; @@ -8,8 +9,6 @@ using JsonApiDotNetCoreExample; using JsonApiDotNetCoreExample.Data; using JsonApiDotNetCoreExample.Models; -using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.TestHost; using Newtonsoft.Json; using Xunit; using Person = JsonApiDotNetCoreExample.Models.Person; @@ -44,17 +43,13 @@ public async Task Can_Filter_On_Guid_Properties() var todoItem = _todoItemFaker.Generate(); context.TodoItems.Add(todoItem); await context.SaveChangesAsync(); - - var builder = new WebHostBuilder() - .UseStartup(); + var httpMethod = new HttpMethod("GET"); var route = $"/api/v1/todo-items?filter[guid-property]={todoItem.GuidProperty}"; - var server = new TestServer(builder); - var client = server.CreateClient(); var request = new HttpRequestMessage(httpMethod, route); // act - var response = await client.SendAsync(request); + var response = await _fixture.Client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); var deserializedBody = _fixture .GetService() @@ -68,7 +63,6 @@ public async Task Can_Filter_On_Guid_Properties() Assert.Equal(todoItem.GuidProperty, todoItemResponse.GuidProperty); } - [Fact] public async Task Can_Filter_On_Related_Attrs() { @@ -79,17 +73,13 @@ public async Task Can_Filter_On_Related_Attrs() todoItem.Owner = person; context.TodoItems.Add(todoItem); await context.SaveChangesAsync(); - - var builder = new WebHostBuilder() - .UseStartup(); + var httpMethod = new HttpMethod("GET"); var route = $"/api/v1/todo-items?include=owner&filter[owner.first-name]={person.FirstName}"; - var server = new TestServer(builder); - var client = server.CreateClient(); var request = new HttpRequestMessage(httpMethod, route); // act - var response = await client.SendAsync(request); + var response = await _fixture.Client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); var documents = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); var included = documents.Included; @@ -101,5 +91,20 @@ public async Task Can_Filter_On_Related_Attrs() foreach(var item in included) Assert.Equal(person.FirstName, item.Attributes["first-name"]); } + + [Fact] + public async Task Cannot_Filter_If_Explicitly_Forbidden() + { + // arrange + var httpMethod = new HttpMethod("GET"); + var route = $"/api/v1/todo-items?include=owner&filter[achieved-date]={DateTime.UtcNow.Date}"; + var request = new HttpRequestMessage(httpMethod, route); + + // act + var response = await _fixture.Client.SendAsync(request); + + // assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeSortTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeSortTests.cs new file mode 100644 index 0000000000..8525f251d1 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/AttributeSortTests.cs @@ -0,0 +1,34 @@ +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using JsonApiDotNetCoreExample; +using Xunit; + +namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec +{ + [Collection("WebHostCollection")] + public class AttributeSortTests + { + private TestFixture _fixture; + + public AttributeSortTests(TestFixture fixture) + { + _fixture = fixture; + } + + [Fact] + public async Task Cannot_Sort_If_Explicitly_Forbidden() + { + // arrange + var httpMethod = new HttpMethod("GET"); + var route = $"/api/v1/todo-items?include=owner&sort=achieved-date"; + var request = new HttpRequestMessage(httpMethod, route); + + // act + var response = await _fixture.Client.SendAsync(request); + + // assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + } +}