From c457f6e5dd60a059ec003ca07127e8e935410b06 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Tue, 28 Aug 2018 14:11:06 -0400 Subject: [PATCH 1/6] failing test + document how to get started --- README.md | 38 +++++++++++++++++++ .../Models/TodoItem.cs | 6 +++ .../Acceptance/TodoItemsControllerTests.cs | 23 +++++++++++ 3 files changed, 67 insertions(+) diff --git a/README.md b/README.md index 81ebba79bf..f614dc4dbc 100644 --- a/README.md +++ b/README.md @@ -75,3 +75,41 @@ public class Startup } } ``` + +### Development + +Restore all nuget packages with: + +```bash +dotnet restore +``` + +#### Testing + +Running tests locally requires access to a postgresql database. +If you have docker installed, this can be propped up via: + +```bash +docker run --rm --name jsonapi-dotnet-core-testing \ + -e POSTGRES_DB=JsonApiDotNetCoreExample \ + -e POSTGRES_USER=postgres \ + -e POSTGRES_PASSWORD=postgres \ + -p 5432:5432 \ + postgres +``` + +And then to run the tests: + +```bash +dotnet test +``` + +#### Cleaning + +Sometimes the compiled files can be dirty / corrupt from other branches / failed builds. +If your bash prompt supports the globstar, you can recursively delete the `bin` and `obj` directories: + +```bash +shopt -s globstar +rm -rf **/bin && rm -rf **/obj +``` \ No newline at end of file diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs index fecd16319d..7ae957f4a5 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/TodoItem.cs @@ -24,6 +24,12 @@ public TodoItem() [Attr("achieved-date", isFilterable: false, isSortable: false)] public DateTime? AchievedDate { get; set; } + + + [Attr("updated-date")] + public DateTime? UpdatedDate { get; set; } + + public int? OwnerId { get; set; } public int? AssigneeId { get; set; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index 0600fb402b..3350fd1340 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -90,6 +90,29 @@ public async Task Can_Filter_TodoItems() Assert.Equal(todoItem.Ordinal, todoItemResult.Ordinal); } + [Fact] + public async Task Can_Filter_TodoItems_Using_NotEqual_Operator() + { + // Arrange + var todoItem = _todoItemFaker.Generate(); + todoItem.UpdatedDate = null; + _context.TodoItems.Add(todoItem); + _context.SaveChanges(); + + var httpMethod = new HttpMethod("GET"); + var route = $"/api/v1/todo-items?filter[updated-date]=ne:null"; + var request = new HttpRequestMessage(httpMethod, route); + + // Act + var response = await _fixture.Client.SendAsync(request); + var body = await response.Content.ReadAsStringAsync(); + var deserializedBody = _fixture.GetService().DeserializeList(body); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Empty(deserializedBody); + } + [Fact] public async Task Can_Filter_TodoItems_Using_Like_Operator() { From a3d9e6c0c6fb5707df5703e0ce7faf38068263d6 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Tue, 28 Aug 2018 15:17:28 -0400 Subject: [PATCH 2/6] fixed it --- .../Extensions/IQueryableExtensions.cs | 12 ++++++++++-- .../Acceptance/TodoItemsControllerTests.cs | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs index 03f74b9e59..20e6902ee9 100644 --- a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs @@ -127,9 +127,14 @@ public static IQueryable Filter(this IQueryable sourc return source.Where(lambdaIn); } else - { // convert the incoming value to the target value type + { + var isNullabe = IsNullable(property.PropertyType); + var propertyValue = filterQuery.PropertyValue; + var value = isNullabe && propertyValue == "" ? null : propertyValue; + + // convert the incoming value to the target value type // "1" -> 1 - var convertedValue = TypeHelper.ConvertType(filterQuery.PropertyValue, property.PropertyType); + var convertedValue = TypeHelper.ConvertType(value, property.PropertyType); // {model} var parameter = Expression.Parameter(concreteType, "model"); // {model.Id} @@ -204,6 +209,9 @@ public static IQueryable Filter(this IQueryable sourc } } + private static bool IsNullable(Type type) => type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>); + + private static Expression GetFilterExpressionLambda(Expression left, Expression right, FilterOperations operation) { Expression body; diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index 3350fd1340..6044463764 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -100,7 +100,7 @@ public async Task Can_Filter_TodoItems_Using_NotEqual_Operator() _context.SaveChanges(); var httpMethod = new HttpMethod("GET"); - var route = $"/api/v1/todo-items?filter[updated-date]=ne:null"; + var route = $"/api/v1/todo-items?filter[updated-date]=ne:"; var request = new HttpRequestMessage(httpMethod, route); // Act From b34bd3f6e875961c4515fcf716714f39291ccb1f Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Tue, 28 Aug 2018 17:34:07 -0400 Subject: [PATCH 3/6] DateTime doesn't have an IS method.... so I don't know how we'll use it.... --- .../Extensions/IQueryableExtensions.cs | 36 ++++++++++++++++--- .../Internal/Query/FilterOperations.cs | 4 ++- .../Acceptance/TodoItemsControllerTests.cs | 30 ++++++++++++++-- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs index 20e6902ee9..c94ca4a2c7 100644 --- a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs @@ -113,24 +113,44 @@ public static IQueryable Filter(this IQueryable sourc var concreteType = typeof(TSource); var property = concreteType.GetProperty(filterQuery.FilteredAttribute.InternalAttributeName); + var op = filterQuery.FilterOperation; if (property == null) throw new ArgumentException($"'{filterQuery.FilteredAttribute.InternalAttributeName}' is not a valid property of '{concreteType}'"); try { - if (filterQuery.FilterOperation == FilterOperations.@in || filterQuery.FilterOperation == FilterOperations.nin) + if (op == FilterOperations.@in || op == FilterOperations.nin) { string[] propertyValues = filterQuery.PropertyValue.Split(','); - var lambdaIn = ArrayContainsPredicate(propertyValues, property.Name, filterQuery.FilterOperation); + var lambdaIn = ArrayContainsPredicate(propertyValues, property.Name, op); return source.Where(lambdaIn); } + else if (op == FilterOperations.@is || op == FilterOperations.isnot) { + var parameter = Expression.Parameter(concreteType, "model"); + // {model.Id} + var left = Expression.PropertyOrField(parameter, property.Name); + var right = Expression.Constant(filterQuery.PropertyValue, typeof(string)); + + var body = GetFilterExpressionLambda(left, right, op); + var lambda = Expression.Lambda>(body, parameter); + + return source.Where(lambda); + } else { var isNullabe = IsNullable(property.PropertyType); var propertyValue = filterQuery.PropertyValue; - var value = isNullabe && propertyValue == "" ? null : propertyValue; + var value = propertyValue; + + if (op == FilterOperations.@isnot || op == FilterOperations.isnot) + { + if (isNullabe && propertyValue == "null") + { + value = null; + } + } // convert the incoming value to the target value type // "1" -> 1 @@ -142,7 +162,7 @@ public static IQueryable Filter(this IQueryable sourc // {1} var right = Expression.Constant(convertedValue, property.PropertyType); - var body = GetFilterExpressionLambda(left, right, filterQuery.FilterOperation); + var body = GetFilterExpressionLambda(left, right, op); var lambda = Expression.Lambda>(body, parameter); @@ -244,6 +264,14 @@ private static Expression GetFilterExpressionLambda(Expression left, Expression case FilterOperations.ne: body = Expression.NotEqual(left, right); break; + case FilterOperations.isnot: + // {model.Id != null} + body = Expression.NotEqual(left, right); + break; + case FilterOperations.@is: + // {model.Id == null} + body = Expression.Equal(left, right); + break; default: throw new JsonApiException(500, $"Unknown filter operation {operation}"); } diff --git a/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs b/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs index 554e6f98a4..ec76afa87e 100644 --- a/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs +++ b/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs @@ -11,6 +11,8 @@ public enum FilterOperations like = 5, ne = 6, @in = 7, // prefix with @ to use keyword - nin = 8 + nin = 8, + @is = 9, + isnot = 10 } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index 6044463764..991a4f26d3 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using System.Net; @@ -91,7 +92,30 @@ public async Task Can_Filter_TodoItems() } [Fact] - public async Task Can_Filter_TodoItems_Using_NotEqual_Operator() + public async Task Can_Filter_TodoItems_Using_IsNot_Operator() + { + // Arrange + var todoItem = _todoItemFaker.Generate(); + todoItem.UpdatedDate = new DateTime(); + _context.TodoItems.Add(todoItem); + _context.SaveChanges(); + + var httpMethod = new HttpMethod("GET"); + var route = $"/api/v1/todo-items?filter[updated-date]=isnot:null"; + var request = new HttpRequestMessage(httpMethod, route); + + // Act + var response = await _fixture.Client.SendAsync(request); + var body = await response.Content.ReadAsStringAsync(); + var deserializedBody = _fixture.GetService().DeserializeList(body); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotEmpty(deserializedBody); + } + + [Fact] + public async Task Can_Filter_TodoItems_Using_Is_Operator() { // Arrange var todoItem = _todoItemFaker.Generate(); @@ -100,7 +124,7 @@ public async Task Can_Filter_TodoItems_Using_NotEqual_Operator() _context.SaveChanges(); var httpMethod = new HttpMethod("GET"); - var route = $"/api/v1/todo-items?filter[updated-date]=ne:"; + var route = $"/api/v1/todo-items?filter[updated-date]=is:null"; var request = new HttpRequestMessage(httpMethod, route); // Act @@ -110,7 +134,7 @@ public async Task Can_Filter_TodoItems_Using_NotEqual_Operator() // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Empty(deserializedBody); + Assert.NotEmpty(deserializedBody); } [Fact] From 84a78a0f28661b3b19786356975ac6f5e8ec70e9 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Wed, 29 Aug 2018 10:45:52 -0400 Subject: [PATCH 4/6] apply pr feedback --- README.md | 4 +-- .../Extensions/IQueryableExtensions.cs | 26 +++++-------------- .../Internal/Query/FilterOperations.cs | 4 +-- .../Acceptance/TodoItemsControllerTests.cs | 16 +++++++----- 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index f614dc4dbc..6bbb7b0710 100644 --- a/README.md +++ b/README.md @@ -107,9 +107,7 @@ dotnet test #### Cleaning Sometimes the compiled files can be dirty / corrupt from other branches / failed builds. -If your bash prompt supports the globstar, you can recursively delete the `bin` and `obj` directories: ```bash -shopt -s globstar -rm -rf **/bin && rm -rf **/obj +dotnet clean ``` \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs index c94ca4a2c7..8afab4d45a 100644 --- a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs @@ -127,11 +127,12 @@ public static IQueryable Filter(this IQueryable sourc return source.Where(lambdaIn); } - else if (op == FilterOperations.@is || op == FilterOperations.isnot) { + else if (op == FilterOperations.isnotnull || op == FilterOperations.isnull) { + // {model} var parameter = Expression.Parameter(concreteType, "model"); // {model.Id} var left = Expression.PropertyOrField(parameter, property.Name); - var right = Expression.Constant(filterQuery.PropertyValue, typeof(string)); + var right = Expression.Constant(null); var body = GetFilterExpressionLambda(left, right, op); var lambda = Expression.Lambda>(body, parameter); @@ -139,22 +140,9 @@ public static IQueryable Filter(this IQueryable sourc return source.Where(lambda); } else - { - var isNullabe = IsNullable(property.PropertyType); - var propertyValue = filterQuery.PropertyValue; - var value = propertyValue; - - if (op == FilterOperations.@isnot || op == FilterOperations.isnot) - { - if (isNullabe && propertyValue == "null") - { - value = null; - } - } - - // convert the incoming value to the target value type + { // convert the incoming value to the target value type // "1" -> 1 - var convertedValue = TypeHelper.ConvertType(value, property.PropertyType); + var convertedValue = TypeHelper.ConvertType(filterQuery.PropertyValue, property.PropertyType); // {model} var parameter = Expression.Parameter(concreteType, "model"); // {model.Id} @@ -264,11 +252,11 @@ private static Expression GetFilterExpressionLambda(Expression left, Expression case FilterOperations.ne: body = Expression.NotEqual(left, right); break; - case FilterOperations.isnot: + case FilterOperations.isnotnull: // {model.Id != null} body = Expression.NotEqual(left, right); break; - case FilterOperations.@is: + case FilterOperations.isnull: // {model.Id == null} body = Expression.Equal(left, right); break; diff --git a/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs b/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs index ec76afa87e..60ae0af012 100644 --- a/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs +++ b/src/JsonApiDotNetCore/Internal/Query/FilterOperations.cs @@ -12,7 +12,7 @@ public enum FilterOperations ne = 6, @in = 7, // prefix with @ to use keyword nin = 8, - @is = 9, - isnot = 10 + isnull = 9, + isnotnull = 10 } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index 991a4f26d3..bd29195193 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -92,7 +92,7 @@ public async Task Can_Filter_TodoItems() } [Fact] - public async Task Can_Filter_TodoItems_Using_IsNot_Operator() + public async Task Can_Filter_TodoItems_Using_IsNotNull_Operator() { // Arrange var todoItem = _todoItemFaker.Generate(); @@ -101,21 +101,23 @@ public async Task Can_Filter_TodoItems_Using_IsNot_Operator() _context.SaveChanges(); var httpMethod = new HttpMethod("GET"); - var route = $"/api/v1/todo-items?filter[updated-date]=isnot:null"; + var route = $"/api/v1/todo-items?filter[updated-date]=isnotnull:"; var request = new HttpRequestMessage(httpMethod, route); // Act var response = await _fixture.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); var deserializedBody = _fixture.GetService().DeserializeList(body); // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotEmpty(deserializedBody); } [Fact] - public async Task Can_Filter_TodoItems_Using_Is_Operator() + public async Task Can_Filter_TodoItems_Using_IsNull_Operator() { // Arrange var todoItem = _todoItemFaker.Generate(); @@ -124,16 +126,18 @@ public async Task Can_Filter_TodoItems_Using_Is_Operator() _context.SaveChanges(); var httpMethod = new HttpMethod("GET"); - var route = $"/api/v1/todo-items?filter[updated-date]=is:null"; + var route = $"/api/v1/todo-items?filter[updated-date]=isnull:"; var request = new HttpRequestMessage(httpMethod, route); // Act var response = await _fixture.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); var deserializedBody = _fixture.GetService().DeserializeList(body); // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotEmpty(deserializedBody); } From e0b3cefd0bccdbae39cdf0b339a6394160de73f2 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 30 Aug 2018 11:34:43 -0400 Subject: [PATCH 5/6] add additional assertions to test specific todoitem return --- .../Acceptance/TodoItemsControllerTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index bd29195193..22a887656c 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -114,6 +114,9 @@ public async Task Can_Filter_TodoItems_Using_IsNotNull_Operator() // Assert Assert.NotEmpty(deserializedBody); + + foreach (var todoItemResult in deserializedBody) + Assert.Equal(todoItem.Ordinal, todoItemResult.Ordinal); } [Fact] @@ -139,6 +142,9 @@ public async Task Can_Filter_TodoItems_Using_IsNull_Operator() // Assert Assert.NotEmpty(deserializedBody); + + foreach (var todoItemResult in deserializedBody) + Assert.Equal(todoItem.Ordinal, todoItemResult.Ordinal); } [Fact] From edd0f225cf2a9a6273313dae3d74e9aa9edf425c Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Mon, 3 Sep 2018 20:28:07 -0700 Subject: [PATCH 6/6] fix test -- make assertions directly related to the feature under test --- .../Acceptance/TodoItemsControllerTests.cs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index 22a887656c..ccc9a1e870 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -97,7 +97,11 @@ public async Task Can_Filter_TodoItems_Using_IsNotNull_Operator() // Arrange var todoItem = _todoItemFaker.Generate(); todoItem.UpdatedDate = new DateTime(); - _context.TodoItems.Add(todoItem); + + var otherTodoItem = _todoItemFaker.Generate(); + otherTodoItem.UpdatedDate = null; + + _context.TodoItems.AddRange(new[] { todoItem, otherTodoItem }); _context.SaveChanges(); var httpMethod = new HttpMethod("GET"); @@ -110,13 +114,11 @@ public async Task Can_Filter_TodoItems_Using_IsNotNull_Operator() Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadAsStringAsync(); - var deserializedBody = _fixture.GetService().DeserializeList(body); + var todoItems = _fixture.GetService().DeserializeList(body); // Assert - Assert.NotEmpty(deserializedBody); - - foreach (var todoItemResult in deserializedBody) - Assert.Equal(todoItem.Ordinal, todoItemResult.Ordinal); + Assert.NotEmpty(todoItems); + Assert.All(todoItems, t => Assert.NotNull(t.UpdatedDate)); } [Fact] @@ -125,7 +127,11 @@ public async Task Can_Filter_TodoItems_Using_IsNull_Operator() // Arrange var todoItem = _todoItemFaker.Generate(); todoItem.UpdatedDate = null; - _context.TodoItems.Add(todoItem); + + var otherTodoItem = _todoItemFaker.Generate(); + otherTodoItem.UpdatedDate = new DateTime(); + + _context.TodoItems.AddRange(new[] { todoItem, otherTodoItem }); _context.SaveChanges(); var httpMethod = new HttpMethod("GET"); @@ -138,13 +144,11 @@ public async Task Can_Filter_TodoItems_Using_IsNull_Operator() Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadAsStringAsync(); - var deserializedBody = _fixture.GetService().DeserializeList(body); + var todoItems = _fixture.GetService().DeserializeList(body); // Assert - Assert.NotEmpty(deserializedBody); - - foreach (var todoItemResult in deserializedBody) - Assert.Equal(todoItem.Ordinal, todoItemResult.Ordinal); + Assert.NotEmpty(todoItems); + Assert.All(todoItems, t => Assert.Null(t.UpdatedDate)); } [Fact]