From 66492a225281cbaee8d52b54ea9db3e5e5ccdebc Mon Sep 17 00:00:00 2001
From: jaredcnance <jaredcnance@gmail.com>
Date: Thu, 26 Jul 2018 21:22:48 -0700
Subject: [PATCH 1/2] fix(GetOpProcessor): #340.1 not properly handling HasMany
relationships
---
.../Builders/DocumentBuilder.cs | 1 -
.../Operations/Processors/GetOpProcessor.cs | 40 ++++++++++++++++---
.../Get/GetRelationshipTests.cs | 37 ++++++++++++++++-
3 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs
index c6f5f999b4..ce060f57b7 100644
--- a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs
+++ b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs
@@ -138,7 +138,6 @@ public DocumentData GetData(ContextEntity contextEntity, IIdentifiable entity, I
return data;
}
-
private bool ShouldIncludeAttribute(AttrAttribute attr, object attributeValue)
{
return OmitNullValuedAttribute(attr, attributeValue) == false
diff --git a/src/JsonApiDotNetCore/Services/Operations/Processors/GetOpProcessor.cs b/src/JsonApiDotNetCore/Services/Operations/Processors/GetOpProcessor.cs
index a3cb6e7da8..954bfadd55 100644
--- a/src/JsonApiDotNetCore/Services/Operations/Processors/GetOpProcessor.cs
+++ b/src/JsonApiDotNetCore/Services/Operations/Processors/GetOpProcessor.cs
@@ -1,3 +1,4 @@
+using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
@@ -83,10 +84,10 @@ public async Task<Operation> ProcessAsync(Operation operation)
};
operationResult.Data = string.IsNullOrWhiteSpace(operation.Ref.Id)
- ? await GetAllAsync(operation)
- : string.IsNullOrWhiteSpace(operation.Ref.Relationship)
- ? await GetByIdAsync(operation)
- : await GetRelationshipAsync(operation);
+ ? await GetAllAsync(operation)
+ : string.IsNullOrWhiteSpace(operation.Ref.Relationship)
+ ? await GetByIdAsync(operation)
+ : await GetRelationshipAsync(operation);
return operationResult;
}
@@ -135,11 +136,38 @@ private async Task<object> GetRelationshipAsync(Operation operation)
// when no generic parameter is available
var relationshipType = _contextGraph.GetContextEntity(operation.GetResourceTypeName())
.Relationships.Single(r => r.Is(operation.Ref.Relationship)).Type;
+
var relatedContextEntity = _jsonApiContext.ContextGraph.GetContextEntity(relationshipType);
- var doc = _documentBuilder.GetData(relatedContextEntity, result as IIdentifiable); // TODO: if this is safe, then it should be cast in the GetRelationshipAsync call
+ if (result == null)
+ return null;
+
+ if (result is IIdentifiable singleResource)
+ return GetData(relatedContextEntity, singleResource);
- return doc;
+ if (result is IEnumerable multipleResults)
+ return GetData(relatedContextEntity, multipleResults);
+
+ throw new JsonApiException(500,
+ $"An unexpected type was returned from '{_getRelationship.GetType()}.{nameof(IGetRelationshipService<T, TId>.GetRelationshipAsync)}'.",
+ detail: $"Type '{result.GetType()} does not implement {nameof(IIdentifiable)} nor {nameof(IEnumerable<IIdentifiable>)}'");
+ }
+
+ private DocumentData GetData(ContextEntity contextEntity, IIdentifiable singleResource)
+ {
+ return _documentBuilder.GetData(contextEntity, singleResource);
+ }
+
+ private List<DocumentData> GetData(ContextEntity contextEntity, IEnumerable multipleResults)
+ {
+ var resources = new List<DocumentData>();
+ foreach (var singleResult in multipleResults)
+ {
+ if (singleResult is IIdentifiable resource)
+ resources.Add(_documentBuilder.GetData(contextEntity, resource));
+ }
+
+ return resources;
}
private TId GetReferenceId(Operation operation) => TypeHelper.ConvertType<TId>(operation.Ref.Id);
diff --git a/test/OperationsExampleTests/Get/GetRelationshipTests.cs b/test/OperationsExampleTests/Get/GetRelationshipTests.cs
index 87edaba282..0aeef6f3ec 100644
--- a/test/OperationsExampleTests/Get/GetRelationshipTests.cs
+++ b/test/OperationsExampleTests/Get/GetRelationshipTests.cs
@@ -16,7 +16,7 @@ public class GetRelationshipTests : Fixture, IDisposable
private readonly Faker _faker = new Faker();
[Fact]
- public async Task Can_Get_Article_Author()
+ public async Task Can_Get_HasOne_Relationship()
{
// arrange
var context = GetService<AppDbContext>();
@@ -48,5 +48,40 @@ public async Task Can_Get_Article_Author()
Assert.Equal(author.Id.ToString(), resourceObject.Id);
Assert.Equal("authors", resourceObject.Type);
}
+
+ [Fact]
+ public async Task Can_Get_HasMany_Relationship()
+ {
+ // arrange
+ var context = GetService<AppDbContext>();
+ var author = AuthorFactory.Get();
+ var article = ArticleFactory.Get();
+ article.Author = author;
+ context.Articles.Add(article);
+ context.SaveChanges();
+
+ var content = new
+ {
+ operations = new[] {
+ new Dictionary<string, object> {
+ { "op", "get"},
+ { "ref", new { type = "authors", id = author.StringId, relationship = "articles" } }
+ }
+ }
+ };
+
+ // act
+ var (response, data) = await PatchAsync<OperationsDocument>("api/bulk", content);
+
+ // assert
+ Assert.NotNull(response);
+ Assert.NotNull(data);
+ Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+ Assert.Single(data.Operations);
+
+ var resourceObject = data.Operations.Single().DataList.Single();
+ Assert.Equal(article.Id.ToString(), resourceObject.Id);
+ Assert.Equal("articles", resourceObject.Type);
+ }
}
}
From 8e065ce18c30cd38406ebecd29cf440032bfd319 Mon Sep 17 00:00:00 2001
From: jaredcnance <jaredcnance@gmail.com>
Date: Thu, 26 Jul 2018 21:45:30 -0700
Subject: [PATCH 2/2] fix(OperationProcessorResolver): #340.2 wrong type throws
null ref
---
.../Operations/OperationProcessorResolver.cs | 22 +++-
test/OperationsExampleTests/Get/GetTests.cs | 118 +++++++++++-------
.../OperationsProcessorResolverTests.cs | 102 +++++++++++++++
3 files changed, 188 insertions(+), 54 deletions(-)
create mode 100644 test/UnitTests/Services/Operations/OperationsProcessorResolverTests.cs
diff --git a/src/JsonApiDotNetCore/Services/Operations/OperationProcessorResolver.cs b/src/JsonApiDotNetCore/Services/Operations/OperationProcessorResolver.cs
index 1004ed30dc..a702638c5d 100644
--- a/src/JsonApiDotNetCore/Services/Operations/OperationProcessorResolver.cs
+++ b/src/JsonApiDotNetCore/Services/Operations/OperationProcessorResolver.cs
@@ -51,7 +51,8 @@ public IOpProcessor LocateCreateService(Operation operation)
{
var resource = operation.GetResourceTypeName();
- var contextEntity = _context.ContextGraph.GetContextEntity(resource);
+ var contextEntity = GetResourceMetadata(resource);
+
var processor = _processorFactory.GetProcessor<IOpProcessor>(
typeof(ICreateOpProcessor<,>), contextEntity.EntityType, contextEntity.IdentityType
);
@@ -64,7 +65,8 @@ public IOpProcessor LocateGetService(Operation operation)
{
var resource = operation.GetResourceTypeName();
- var contextEntity = _context.ContextGraph.GetContextEntity(resource);
+ var contextEntity = GetResourceMetadata(resource);
+
var processor = _processorFactory.GetProcessor<IOpProcessor>(
typeof(IGetOpProcessor<,>), contextEntity.EntityType, contextEntity.IdentityType
);
@@ -77,7 +79,8 @@ public IOpProcessor LocateRemoveService(Operation operation)
{
var resource = operation.GetResourceTypeName();
- var contextEntity = _context.ContextGraph.GetContextEntity(resource);
+ var contextEntity = GetResourceMetadata(resource);
+
var processor = _processorFactory.GetProcessor<IOpProcessor>(
typeof(IRemoveOpProcessor<,>), contextEntity.EntityType, contextEntity.IdentityType
);
@@ -90,9 +93,7 @@ public IOpProcessor LocateUpdateService(Operation operation)
{
var resource = operation.GetResourceTypeName();
- var contextEntity = _context.ContextGraph.GetContextEntity(resource);
- if (contextEntity == null)
- throw new JsonApiException(400, $"This API does not expose a resource of type '{resource}'.");
+ var contextEntity = GetResourceMetadata(resource);
var processor = _processorFactory.GetProcessor<IOpProcessor>(
typeof(IUpdateOpProcessor<,>), contextEntity.EntityType, contextEntity.IdentityType
@@ -100,5 +101,14 @@ public IOpProcessor LocateUpdateService(Operation operation)
return processor;
}
+
+ private ContextEntity GetResourceMetadata(string resourceName)
+ {
+ var contextEntity = _context.ContextGraph.GetContextEntity(resourceName);
+ if(contextEntity == null)
+ throw new JsonApiException(400, $"This API does not expose a resource of type '{resourceName}'.");
+
+ return contextEntity;
+ }
}
}
diff --git a/test/OperationsExampleTests/Get/GetTests.cs b/test/OperationsExampleTests/Get/GetTests.cs
index 78e7eeb976..f0d3fdffd8 100644
--- a/test/OperationsExampleTests/Get/GetTests.cs
+++ b/test/OperationsExampleTests/Get/GetTests.cs
@@ -1,51 +1,73 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Net;
-using System.Threading.Tasks;
-using Bogus;
-using JsonApiDotNetCore.Models.Operations;
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Net;
+using System.Threading.Tasks;
+using Bogus;
+using JsonApiDotNetCore.Models.Operations;
using JsonApiDotNetCoreExample.Data;
-using OperationsExampleTests.Factories;
-using Xunit;
-
-namespace OperationsExampleTests
-{
- public class GetByIdTests : Fixture, IDisposable
- {
+using OperationsExampleTests.Factories;
+using Xunit;
+
+namespace OperationsExampleTests
+{
+ public class GetByIdTests : Fixture, IDisposable
+ {
private readonly Faker _faker = new Faker();
-
- [Fact]
- public async Task Can_Get_Authors()
- {
- // arrange
- var expectedCount = _faker.Random.Int(1, 10);
- var context = GetService<AppDbContext>();
- context.Articles.RemoveRange(context.Articles);
- context.Authors.RemoveRange(context.Authors);
- var authors = AuthorFactory.Get(expectedCount);
- context.AddRange(authors);
- context.SaveChanges();
-
- var content = new
- {
- operations = new[] {
- new Dictionary<string, object> {
- { "op", "get"},
- { "ref", new { type = "authors" } }
- }
- }
- };
-
- // act
- var result = await PatchAsync<OperationsDocument>("api/bulk", content);
-
- // assert
- Assert.NotNull(result.response);
- Assert.NotNull(result.data);
- Assert.Equal(HttpStatusCode.OK, result.response.StatusCode);
- Assert.Single(result.data.Operations);
- Assert.Equal(expectedCount, result.data.Operations.Single().DataList.Count);
+
+ [Fact]
+ public async Task Can_Get_Authors()
+ {
+ // arrange
+ var expectedCount = _faker.Random.Int(1, 10);
+ var context = GetService<AppDbContext>();
+ context.Articles.RemoveRange(context.Articles);
+ context.Authors.RemoveRange(context.Authors);
+ var authors = AuthorFactory.Get(expectedCount);
+ context.AddRange(authors);
+ context.SaveChanges();
+
+ var content = new
+ {
+ operations = new[] {
+ new Dictionary<string, object> {
+ { "op", "get"},
+ { "ref", new { type = "authors" } }
+ }
+ }
+ };
+
+ // act
+ var result = await PatchAsync<OperationsDocument>("api/bulk", content);
+
+ // assert
+ Assert.NotNull(result.response);
+ Assert.NotNull(result.data);
+ Assert.Equal(HttpStatusCode.OK, result.response.StatusCode);
+ Assert.Single(result.data.Operations);
+ Assert.Equal(expectedCount, result.data.Operations.Single().DataList.Count);
}
- }
-}
+
+ [Fact]
+ public async Task Get_Non_Existent_Type_Returns_400()
+ {
+ // arrange
+ var content = new
+ {
+ operations = new[] {
+ new Dictionary<string, object> {
+ { "op", "get"},
+ { "ref", new { type = "non-existent-type" } }
+ }
+ }
+ };
+
+ // act
+ var result = await PatchAsync<OperationsDocument>("api/bulk", content);
+
+ // assert
+ Assert.Equal(HttpStatusCode.BadRequest, result.response.StatusCode);
+ }
+ }
+}
diff --git a/test/UnitTests/Services/Operations/OperationsProcessorResolverTests.cs b/test/UnitTests/Services/Operations/OperationsProcessorResolverTests.cs
new file mode 100644
index 0000000000..1e7026f2ee
--- /dev/null
+++ b/test/UnitTests/Services/Operations/OperationsProcessorResolverTests.cs
@@ -0,0 +1,102 @@
+using JsonApiDotNetCore.Builders;
+using JsonApiDotNetCore.Internal;
+using JsonApiDotNetCore.Internal.Generics;
+using JsonApiDotNetCore.Models.Operations;
+using JsonApiDotNetCore.Services;
+using JsonApiDotNetCore.Services.Operations;
+using Moq;
+using Xunit;
+
+namespace UnitTests.Services
+{
+ public class OperationProcessorResolverTests
+ {
+ private readonly Mock<IGenericProcessorFactory> _processorFactoryMock;
+ public readonly Mock<IJsonApiContext> _jsonApiContextMock;
+
+ public OperationProcessorResolverTests()
+ {
+ _processorFactoryMock = new Mock<IGenericProcessorFactory>();
+ _jsonApiContextMock = new Mock<IJsonApiContext>();
+ }
+
+ [Fact]
+ public void LocateCreateService_Throws_400_For_Entity_Not_Registered()
+ {
+ // arrange
+ _jsonApiContextMock.Setup(m => m.ContextGraph).Returns(new ContextGraphBuilder().Build());
+ var service = GetService();
+ var op = new Operation
+ {
+ Ref = new ResourceReference
+ {
+ Type = "non-existent-type"
+ }
+ };
+
+ // act, assert
+ var e = Assert.Throws<JsonApiException>(() => service.LocateCreateService(op));
+ Assert.Equal(400, e.GetStatusCode());
+ }
+
+ [Fact]
+ public void LocateGetService_Throws_400_For_Entity_Not_Registered()
+ {
+ // arrange
+ _jsonApiContextMock.Setup(m => m.ContextGraph).Returns(new ContextGraphBuilder().Build());
+ var service = GetService();
+ var op = new Operation
+ {
+ Ref = new ResourceReference
+ {
+ Type = "non-existent-type"
+ }
+ };
+
+ // act, assert
+ var e = Assert.Throws<JsonApiException>(() => service.LocateGetService(op));
+ Assert.Equal(400, e.GetStatusCode());
+ }
+
+ [Fact]
+ public void LocateRemoveService_Throws_400_For_Entity_Not_Registered()
+ {
+ // arrange
+ _jsonApiContextMock.Setup(m => m.ContextGraph).Returns(new ContextGraphBuilder().Build());
+ var service = GetService();
+ var op = new Operation
+ {
+ Ref = new ResourceReference
+ {
+ Type = "non-existent-type"
+ }
+ };
+
+ // act, assert
+ var e = Assert.Throws<JsonApiException>(() => service.LocateRemoveService(op));
+ Assert.Equal(400, e.GetStatusCode());
+ }
+
+ [Fact]
+ public void LocateUpdateService_Throws_400_For_Entity_Not_Registered()
+ {
+ // arrange
+ _jsonApiContextMock.Setup(m => m.ContextGraph).Returns(new ContextGraphBuilder().Build());
+ var service = GetService();
+ var op = new Operation
+ {
+ Ref = new ResourceReference
+ {
+ Type = "non-existent-type"
+ }
+ };
+
+ // act, assert
+ var e = Assert.Throws<JsonApiException>(() => service.LocateUpdateService(op));
+ Assert.Equal(400, e.GetStatusCode());
+ }
+
+ private OperationProcessorResolver GetService()
+ => new OperationProcessorResolver(_processorFactoryMock.Object, _jsonApiContextMock.Object);
+ }
+}