Merge pull request #461 from json-api-dotnet/fix/#454

Omit null attributes in included array

Author: milosloub

src/JsonApiDotNetCore/Builders/DocumentBuilder.cs

@@ -138,13 +138,15 @@ public ResourceObject GetData(ContextEntity contextEntity, IIdentifiable entity,
 
             return data;
         }
-        private bool ShouldIncludeAttribute(AttrAttribute attr, object attributeValue)
+        private bool ShouldIncludeAttribute(AttrAttribute attr, object attributeValue, RelationshipAttribute relationship = null)
         {
             return OmitNullValuedAttribute(attr, attributeValue) == false
                     && attr.InternalAttributeName != nameof(Identifiable.Id)
                    && ((_jsonApiContext.QuerySet == null
                        || _jsonApiContext.QuerySet.Fields.Count == 0)
-                       || _jsonApiContext.QuerySet.Fields.Contains(attr.InternalAttributeName));
+                       || _jsonApiContext.QuerySet.Fields.Contains(relationship != null ?
+                            $"{relationship.InternalRelationshipName}.{attr.InternalAttributeName}" :
+                            attr.InternalAttributeName));
         }
 
         private bool OmitNullValuedAttribute(AttrAttribute attr, object attributeValue)
@@ -225,13 +227,13 @@ private List<ResourceObject> IncludeRelationshipChain(
             {
                 foreach (IIdentifiable includedEntity in hasManyNavigationEntity)
                 {
-                    included = AddIncludedEntity(included, includedEntity);
+                    included = AddIncludedEntity(included, includedEntity, relationship);
                     included = IncludeSingleResourceRelationships(included, includedEntity, relationship, relationshipChain, relationshipChainIndex);
                 }
             }
             else
             {
-                included = AddIncludedEntity(included, (IIdentifiable)navigationEntity);
+                included = AddIncludedEntity(included, (IIdentifiable)navigationEntity, relationship);
                 included = IncludeSingleResourceRelationships(included, (IIdentifiable)navigationEntity, relationship, relationshipChain, relationshipChainIndex);
             }
 
@@ -254,9 +256,9 @@ private List<ResourceObject> IncludeSingleResourceRelationships(
         }
 
 
-        private List<ResourceObject> AddIncludedEntity(List<ResourceObject> entities, IIdentifiable entity)
+        private List<ResourceObject> AddIncludedEntity(List<ResourceObject> entities, IIdentifiable entity, RelationshipAttribute relationship)
         {
-            var includedEntity = GetIncludedEntity(entity);
+            var includedEntity = GetIncludedEntity(entity, relationship);
 
             if (entities == null)
                 entities = new List<ResourceObject>();
@@ -270,7 +272,7 @@ private List<ResourceObject> AddIncludedEntity(List<ResourceObject> entities, II
             return entities;
         }
 
-        private ResourceObject GetIncludedEntity(IIdentifiable entity)
+        private ResourceObject GetIncludedEntity(IIdentifiable entity, RelationshipAttribute relationship)
         {
             if (entity == null) return null;
 
@@ -281,13 +283,14 @@ private ResourceObject GetIncludedEntity(IIdentifiable entity)
 
             data.Attributes = new Dictionary<string, object>();
 
-            foreach(var attr in contextEntity.Attributes)
+            contextEntity.Attributes.ForEach(attr =>
             {
-                if (attr.InternalAttributeName == nameof(Identifiable.Id))
-                    continue;
-
-                data.Attributes.Add(attr.PublicAttributeName, attr.GetValue(entity));
-            }
+                var attributeValue = attr.GetValue(entity);
+                if (ShouldIncludeAttribute(attr, attributeValue, relationship))
+                {
+                    data.Attributes.Add(attr.PublicAttributeName, attributeValue);
+                }
+            });
 
             return data;
         }

test/JsonApiDotNetCoreExampleTests/Acceptance/Extensibility/NullValuedAttributeHandlingTests.cs

@@ -21,12 +21,14 @@ public NullValuedAttributeHandlingTests(TestFixture<TestStartup> fixture)
         {
             _fixture = fixture;
             _dbContext = fixture.GetService<AppDbContext>();
+            var person = new Person { FirstName = "Bob", LastName = null };
             _todoItem = new TodoItem
             {
                 Description = null,
                 Ordinal = 1,
                 CreatedDate = DateTime.Now,
-                AchievedDate = DateTime.Now.AddDays(2)
+                AchievedDate = DateTime.Now.AddDays(2),
+                Owner = person
             };
             _todoItem = _dbContext.TodoItems.Add(_todoItem).Entity;
         }
@@ -86,9 +88,9 @@ public async Task CheckNullBehaviorCombination(bool? omitNullValuedAttributes, b
 
             var httpMethod = new HttpMethod("GET");
             var queryString = allowClientOverride.HasValue
-                ? $"?omitNullValuedAttributes={clientOverride}"
+                ? $"&omitNullValuedAttributes={clientOverride}"
                 : "";
-            var route = $"/api/v1/todo-items/{_todoItem.Id}{queryString}"; 
+            var route = $"/api/v1/todo-items/{_todoItem.Id}?include=owner{queryString}"; 
             var request = new HttpRequestMessage(httpMethod, route);
 
             // act
@@ -98,6 +100,7 @@ public async Task CheckNullBehaviorCombination(bool? omitNullValuedAttributes, b
 
             // assert. does response contain a null valued attribute
             Assert.Equal(omitsNulls, !deserializeBody.Data.Attributes.ContainsKey("description"));
+            Assert.Equal(omitsNulls, !deserializeBody.Included[0].Attributes.ContainsKey("last-name"));
 
         }
     }

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs

@@ -194,8 +194,7 @@ public async Task Fields_Query_Selects_Fieldset_With_HasOne()
             Assert.Equal(owner.StringId, included.Id);      
             Assert.Equal(owner.FirstName, included.Attributes["first-name"]);
             Assert.Equal((long)owner.Age, included.Attributes["age"]);
-            Assert.Null(included.Attributes["last-name"]);
-
+            Assert.DoesNotContain("last-name", included.Attributes.Keys);
         }
 
         [Fact]
@@ -233,8 +232,8 @@ public async Task Fields_Query_Selects_Fieldset_With_HasMany()
                 var todoItem = todoItems.FirstOrDefault(i => i.StringId == includedItem.Id);
                 Assert.NotNull(todoItem);
                 Assert.Equal(todoItem.Description, includedItem.Attributes["description"]);
-                Assert.Equal(default(long), includedItem.Attributes["ordinal"]);
-                Assert.Equal(default(DateTime), (DateTime)includedItem.Attributes["created-date"]);
+                Assert.DoesNotContain("ordinal", includedItem.Attributes.Keys);
+                Assert.DoesNotContain("created-date", includedItem.Attributes.Keys);
             }
         }
     }