Merge pull request #149 from Research-Institute/hotfix/#148

fix(DocumentBuilder): do not include entities twice
json-api-dotnet · Aug 10, 2017 · e300bef · e300bef
2 parents 4a9968a + bf5f4f6
commit e300bef
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 12 deletions.
diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs
@@ -95,12 +95,10 @@ private Dictionary<string, object> GetMeta(IIdentifiable entity)
 
         private List<DocumentData> AppendIncludedObject(List<DocumentData> includedObject, ContextEntity contextEntity, IIdentifiable entity)
         {
-            var includedEntities = GetIncludedEntities(contextEntity, entity);
-            if (includedEntities.Count > 0)
+            var includedEntities = GetIncludedEntities(includedObject, contextEntity, entity);
+            if (includedEntities?.Count > 0)
             {
-                if (includedObject == null)
-                    includedObject = new List<DocumentData>();
-                includedObject.AddRange(includedEntities);
+                includedObject = includedEntities;
             }
 
             return includedObject;
@@ -174,10 +172,8 @@ private void AddRelationships(DocumentData data, ContextEntity contextEntity, II
             });
         }
 
-        private List<DocumentData> GetIncludedEntities(ContextEntity contextEntity, IIdentifiable entity)
+        private List<DocumentData> GetIncludedEntities(List<DocumentData> included, ContextEntity contextEntity, IIdentifiable entity)
         {
-            var included = new List<DocumentData>();
-
             contextEntity.Relationships.ForEach(r =>
             {
                 if (!RelationshipIsIncluded(r.PublicRelationshipName)) return;
@@ -186,20 +182,25 @@ private List<DocumentData> GetIncludedEntities(ContextEntity contextEntity, IIde
 
                 if (navigationEntity is IEnumerable hasManyNavigationEntity)
                     foreach (IIdentifiable includedEntity in hasManyNavigationEntity)
-                        AddIncludedEntity(included, includedEntity);
+                        included = AddIncludedEntity(included, includedEntity);
                 else
-                    AddIncludedEntity(included, (IIdentifiable)navigationEntity);
+                    included = AddIncludedEntity(included, (IIdentifiable)navigationEntity);
             });
 
             return included;
         }
 
-        private void AddIncludedEntity(List<DocumentData> entities, IIdentifiable entity)
+        private List<DocumentData> AddIncludedEntity(List<DocumentData> entities, IIdentifiable entity)
         {
             var includedEntity = GetIncludedEntity(entity);
 
+            if(entities == null)
+                entities = new List<DocumentData>();
+
             if(includedEntity != null && !entities.Any(doc => doc.Id == includedEntity.Id && doc.Type == includedEntity.Type))
                 entities.Add(includedEntity);
+
+            return entities;
         }
 
         private DocumentData GetIncludedEntity(IIdentifiable entity)

diff --git a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj
@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
-    <VersionPrefix>2.1.1</VersionPrefix>
+    <VersionPrefix>2.1.2</VersionPrefix>
     <TargetFrameworks>netstandard1.6</TargetFrameworks>
     <AssemblyName>JsonApiDotNetCore</AssemblyName>
     <PackageId>JsonApiDotNetCore</PackageId>

diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs
@@ -169,6 +169,41 @@ public async Task GET_Included_DoesNot_Duplicate_Records_ForMultipleRelationship
             Assert.Equal(1, documents.Included.Count);
         }
 
+        [Fact]
+        public async Task GET_Included_DoesNot_Duplicate_Records_If_HasOne_Exists_Twice()
+        {
+            // arrange
+            _context.People.RemoveRange(_context.People); // ensure all people have todo-items
+            _context.TodoItems.RemoveRange(_context.TodoItems);
+            var person = _personFaker.Generate();
+            var todoItem1 = _todoItemFaker.Generate();
+            var todoItem2 = _todoItemFaker.Generate();
+            todoItem1.Owner = person;
+            todoItem2.Owner = person;
+            _context.TodoItems.AddRange(new[] { todoItem1, todoItem2 });
+            _context.SaveChanges();
+
+            var builder = new WebHostBuilder()
+                .UseStartup<Startup>();
+
+            var httpMethod = new HttpMethod("GET");
+            var route = $"/api/v1/todo-items?include=owner";
+
+            var server = new TestServer(builder);
+            var client = server.CreateClient();
+            var request = new HttpRequestMessage(httpMethod, route);
+
+            // act
+            var response = await client.SendAsync(request);
+            var documents = JsonConvert.DeserializeObject<Documents>(await response.Content.ReadAsStringAsync());
+            var data = documents.Data;
+
+            // assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.NotEmpty(documents.Included);
+            Assert.Equal(1, documents.Included.Count);
+        }
+
         [Fact]
         public async Task GET_ById_Included_Contains_SideloadedData_ForOneToMany()
         {