reset state between operations

jaredcnance · jaredcnance · commit 534822c14bb1 · 2018-07-09T16:25:17.000-05:00
diff --git a/src/JsonApiDotNetCore/Middleware/RequestMiddleware.cs b/src/JsonApiDotNetCore/Middleware/RequestMiddleware.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Threading.Tasks;
 using JsonApiDotNetCore.Internal;
+using JsonApiDotNetCore.Services;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Primitives;
 
@@ -9,16 +10,23 @@ namespace JsonApiDotNetCore.Middleware
     public class RequestMiddleware
     {
         private readonly RequestDelegate _next;
-        
+
         public RequestMiddleware(RequestDelegate next)
         {
             _next = next;
         }
 
-        public async Task Invoke(HttpContext context)
+        public async Task Invoke(HttpContext context, IJsonApiContext jsonApiContext)
         {
             if (IsValid(context))
+            {
+                // HACK: this currently results in allocation of
+                // objects that may or may not be used and even double allocation
+                // since the JsonApiContext is using field initializers
+                // Need to work on finding a better solution.
+                jsonApiContext.BeginOperation();
                 await _next(context);
+            }
         }
 
         private static bool IsValid(HttpContext context)
@@ -58,11 +66,11 @@ internal static bool ContainsMediaTypeParameters(string mediaType)
             var incomingMediaTypeSpan = mediaType.AsSpan();
 
             // if the content type is not application/vnd.api+json then continue on
-            if(incomingMediaTypeSpan.Length < Constants.ContentType.Length)
+            if (incomingMediaTypeSpan.Length < Constants.ContentType.Length)
                 return false;
 
             var incomingContentType = incomingMediaTypeSpan.Slice(0, Constants.ContentType.Length);
-            if(incomingContentType.SequenceEqual(Constants.ContentType.AsSpan()) == false)
+            if (incomingContentType.SequenceEqual(Constants.ContentType.AsSpan()) == false)
                 return false;
 
             // anything appended to "application/vnd.api+json;" will be considered a media type param
diff --git a/src/JsonApiDotNetCore/Services/IJsonApiContext.cs b/src/JsonApiDotNetCore/Services/IJsonApiContext.cs
@@ -152,5 +152,12 @@ public interface IJsonApiContext : IJsonApiRequest
 
         [Obsolete("Use the proxied method IControllerContext.GetControllerAttribute instead.")]
         TAttribute GetControllerAttribute<TAttribute>() where TAttribute : Attribute;
+
+        /// <summary>
+        /// **_Experimental_**: do not use. It is likely to change in the future.
+        /// 
+        /// Resets operational state information.
+        /// </summary>
+        void BeginOperation();
     }
 }
diff --git a/src/JsonApiDotNetCore/Services/JsonApiContext.cs b/src/JsonApiDotNetCore/Services/JsonApiContext.cs
@@ -47,13 +47,14 @@ public JsonApiContext(
         public PageManager PageManager { get; set; }
         public IMetaBuilder MetaBuilder { get; set; }
         public IGenericProcessorFactory GenericProcessorFactory { get; set; }
-        public Dictionary<AttrAttribute, object> AttributesToUpdate { get; set; } = new Dictionary<AttrAttribute, object>();
-        public Dictionary<RelationshipAttribute, object> RelationshipsToUpdate { get; set; } = new Dictionary<RelationshipAttribute, object>();
         public Type ControllerType { get; set; }
         public Dictionary<string, object> DocumentMeta { get; set; }
         public bool IsBulkOperationRequest { get; set; }
-        public HasManyRelationshipPointers HasManyRelationshipPointers { get; } = new HasManyRelationshipPointers();
-        public HasOneRelationshipPointers HasOneRelationshipPointers { get; } = new HasOneRelationshipPointers();
+
+        public Dictionary<AttrAttribute, object> AttributesToUpdate { get; set; } = new Dictionary<AttrAttribute, object>();
+        public Dictionary<RelationshipAttribute, object> RelationshipsToUpdate { get; set; } = new Dictionary<RelationshipAttribute, object>();
+        public HasManyRelationshipPointers HasManyRelationshipPointers { get; private set; } = new HasManyRelationshipPointers();
+        public HasOneRelationshipPointers HasOneRelationshipPointers { get; private set; } = new HasOneRelationshipPointers();
 
         public IJsonApiContext ApplyContext<T>(object controller)
         {
@@ -132,5 +133,14 @@ private PageManager GetPageManager()
         [Obsolete("Use the proxied method IControllerContext.GetControllerAttribute instead.")]
         public TAttribute GetControllerAttribute<TAttribute>() where TAttribute : Attribute
             => _controllerContext.GetControllerAttribute<TAttribute>();
+
+        public void BeginOperation()
+        {
+            IncludedRelationships = new List<string>();
+            AttributesToUpdate = new Dictionary<AttrAttribute, object>();
+            RelationshipsToUpdate = new Dictionary<RelationshipAttribute, object>();
+            HasManyRelationshipPointers = new HasManyRelationshipPointers();
+            HasOneRelationshipPointers = new HasOneRelationshipPointers();
+        }
     }
 }
diff --git a/src/JsonApiDotNetCore/Services/Operations/OperationsProcessor.cs b/src/JsonApiDotNetCore/Services/Operations/OperationsProcessor.cs
@@ -19,13 +19,16 @@ public class OperationsProcessor : IOperationsProcessor
     {
         private readonly IOperationProcessorResolver _processorResolver;
         private readonly DbContext _dbContext;
+        private readonly IJsonApiContext _jsonApiContext;
 
         public OperationsProcessor(
             IOperationProcessorResolver processorResolver,
-            IDbContextResolver dbContextResolver)
+            IDbContextResolver dbContextResolver,
+            IJsonApiContext jsonApiContext)
         {
             _processorResolver = processorResolver;
             _dbContext = dbContextResolver.GetContext();
+            _jsonApiContext = jsonApiContext;
         }
 
         public async Task<List<Operation>> ProcessAsync(List<Operation> inputOps)
@@ -40,6 +43,7 @@ public async Task<List<Operation>> ProcessAsync(List<Operation> inputOps)
                 {
                     foreach (var op in inputOps)
                     {
+                        _jsonApiContext.BeginOperation();
                         lastAttemptedOperation = op.Op;
                         await ProcessOperation(op, outputOps);
                         opIndex++;
@@ -75,7 +79,8 @@ private async Task ProcessOperation(Operation op, List<Operation> outputOps)
 
         private void ReplaceLocalIdsInResourceObject(ResourceObject resourceObject, List<Operation> outputOps)
         {
-            if (resourceObject == null) return;
+            if (resourceObject == null)
+                return;
 
             // it is strange to me that a top level resource object might use a lid.
             // by not replacing it, we avoid a case where the first operation is an 'add' with an 'lid'
diff --git a/test/UnitTests/Services/Operations/OperationsProcessorTests.cs b/test/UnitTests/Services/Operations/OperationsProcessorTests.cs
@@ -3,6 +3,7 @@
 using System.Threading.Tasks;
 using JsonApiDotNetCore.Data;
 using JsonApiDotNetCore.Models.Operations;
+using JsonApiDotNetCore.Services;
 using JsonApiDotNetCore.Services.Operations;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Infrastructure;
@@ -18,12 +19,14 @@ public class OperationsProcessorTests
         private readonly Mock<IOperationProcessorResolver> _resolverMock;
         public readonly Mock<DbContext> _dbContextMock;
         public readonly Mock<IDbContextResolver> _dbContextResolverMock;
+        public readonly Mock<IJsonApiContext> _jsonApiContextMock;
 
         public OperationsProcessorTests()
         {
             _resolverMock = new Mock<IOperationProcessorResolver>();
             _dbContextMock = new Mock<DbContext>();
             _dbContextResolverMock = new Mock<IDbContextResolver>();
+            _jsonApiContextMock = new Mock<IJsonApiContext>();
         }
 
         [Fact]
@@ -90,7 +93,7 @@ public async Task ProcessAsync_Performs_LocalId_ReplacementAsync_In_Relationship
                 .Returns(opProcessorMock.Object);
 
             _dbContextResolverMock.Setup(m => m.GetContext()).Returns(_dbContextMock.Object);
-            var operationsProcessor = new OperationsProcessor(_resolverMock.Object, _dbContextResolverMock.Object);
+            var operationsProcessor = new OperationsProcessor(_resolverMock.Object, _dbContextResolverMock.Object, _jsonApiContextMock.Object);
 
             // act
             var results = await operationsProcessor.ProcessAsync(operations);
@@ -173,7 +176,7 @@ public async Task ProcessAsync_Performs_LocalId_ReplacementAsync_In_References()
                 .Returns(updateOpProcessorMock.Object);
 
             _dbContextResolverMock.Setup(m => m.GetContext()).Returns(_dbContextMock.Object);
-            var operationsProcessor = new OperationsProcessor(_resolverMock.Object, _dbContextResolverMock.Object);
+            var operationsProcessor = new OperationsProcessor(_resolverMock.Object, _dbContextResolverMock.Object, _jsonApiContextMock.Object);
 
             // act
             var results = await operationsProcessor.ProcessAsync(operations);

Original file line number	Diff line number	Diff line change
`@@ -152,5 +152,12 @@ public interface IJsonApiContext : IJsonApiRequest`
`152`	`152`
`153`	`153`	`[Obsolete("Use the proxied method IControllerContext.GetControllerAttribute instead.")]`
`154`	`154`	`TAttribute GetControllerAttribute<TAttribute>() where TAttribute : Attribute;`
	`155`	`+`
	`156`	`+ /// <summary>`
	`157`	`+ /// _Experimental_: do not use. It is likely to change in the future.`
	`158`	`+ ///`
	`159`	`+ /// Resets operational state information.`
	`160`	`+ /// </summary>`
	`161`	`+ void BeginOperation();`
`155`	`162`	`}`
`156`	`163`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,13 +19,16 @@ public class OperationsProcessor : IOperationsProcessor`
`19`	`19`	`{`
`20`	`20`	`private readonly IOperationProcessorResolver _processorResolver;`
`21`	`21`	`private readonly DbContext _dbContext;`
	`22`	`+ private readonly IJsonApiContext _jsonApiContext;`
`22`	`23`
`23`	`24`	`public OperationsProcessor(`
`24`	`25`	`IOperationProcessorResolver processorResolver,`
`25`		`- IDbContextResolver dbContextResolver)`
	`26`	`+ IDbContextResolver dbContextResolver,`
	`27`	`+ IJsonApiContext jsonApiContext)`
`26`	`28`	`{`
`27`	`29`	`_processorResolver = processorResolver;`
`28`	`30`	`_dbContext = dbContextResolver.GetContext();`
	`31`	`+ _jsonApiContext = jsonApiContext;`
`29`	`32`	`}`
`30`	`33`
`31`	`34`	`public async Task<List<Operation>> ProcessAsync(List<Operation> inputOps)`
`@@ -40,6 +43,7 @@ public async Task<List<Operation>> ProcessAsync(List<Operation> inputOps)`
`40`	`43`	`{`
`41`	`44`	`foreach (var op in inputOps)`
`42`	`45`	`{`
	`46`	`+ _jsonApiContext.BeginOperation();`
`43`	`47`	`lastAttemptedOperation = op.Op;`
`44`	`48`	`await ProcessOperation(op, outputOps);`
`45`	`49`	`opIndex++;`
`@@ -75,7 +79,8 @@ private async Task ProcessOperation(Operation op, List<Operation> outputOps)`
`75`	`79`
`76`	`80`	`private void ReplaceLocalIdsInResourceObject(ResourceObject resourceObject, List<Operation> outputOps)`
`77`	`81`	`{`
`78`		`- if (resourceObject == null) return;`
	`82`	`+ if (resourceObject == null)`
	`83`	`+ return;`
`79`	`84`
`80`	`85`	`// it is strange to me that a top level resource object might use a lid.`
`81`	`86`	`// by not replacing it, we avoid a case where the first operation is an 'add' with an 'lid'`