Merge branch 'fix/#445' of github.com:roblankey/JsonApiDotNetCore into fix/#445

roblankey · roblankey · commit 135853b0df0f · 2018-11-18T03:04:49.000-05:00
diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs
@@ -218,6 +218,8 @@ private List<ResourceObject> IncludeRelationshipChain(
                 throw new JsonApiException(400, $"{parentEntity.EntityName} does not contain relationship {requestedRelationship}");
 
             var navigationEntity = _jsonApiContext.ResourceGraph.GetRelationshipValue(parentResource, relationship);
+            if(navigationEntity == null)
+                return included;
             if (navigationEntity is IEnumerable hasManyNavigationEntity)
             {
                 foreach (IIdentifiable includedEntity in hasManyNavigationEntity)
diff --git a/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs b/src/JsonApiDotNetCore/Extensions/IQueryableExtensions.cs
@@ -1,10 +1,12 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
 using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Internal.Query;
+using JsonApiDotNetCore.Models;
 using JsonApiDotNetCore.Services;
 
 namespace JsonApiDotNetCore.Extensions
@@ -297,29 +299,111 @@ private static IQueryable<TSource> CallGenericWhereMethod<TSource>(IQueryable<TS
         }
 
         public static IQueryable<TSource> Select<TSource>(this IQueryable<TSource> source, List<string> columns)
-        {
-            if (columns == null || columns.Count == 0)
-                return source;
+            => CallGenericSelectMethod(source, columns);
 
-            var sourceType = source.ElementType;
+        private static IQueryable<TSource> CallGenericSelectMethod<TSource>(IQueryable<TSource> source, List<string> columns)
+        {
+            var sourceBindings = new List<MemberAssignment>();
+            var sourceType = typeof(TSource);
+            var parameter = Expression.Parameter(source.ElementType, "x");
+            var sourceProperties = new List<string>() { };
+
+            // Store all property names to it's own related property (name as key)
+            var nestedTypesAndProperties = new Dictionary<string, List<string>>();
+            foreach (var column in columns)
+            {
+                var props = column.Split('.');
+                if (props.Length > 1) // Nested property
+                {
+                    if (nestedTypesAndProperties.TryGetValue(props[0], out var properties) == false)
+                        nestedTypesAndProperties.Add(props[0], new List<string>() { nameof(Identifiable.Id), props[1] });
+                    else
+                        properties.Add(props[1]);
+                }
+                else
+                    sourceProperties.Add(props[0]);
+            }
 
-            var resultType = typeof(TSource);
+            // Bind attributes on TSource
+            sourceBindings = sourceProperties.Select(prop => Expression.Bind(sourceType.GetProperty(prop), Expression.PropertyOrField(parameter, prop))).ToList();
 
-            // {model}
-            var parameter = Expression.Parameter(sourceType, "model");
-
-            var bindings = columns.Select(column => Expression.Bind(
-                resultType.GetProperty(column), Expression.PropertyOrField(parameter, column)));
+            // Bind attributes on nested types
+            var nestedBindings = new List<MemberAssignment>();
+            Expression bindExpression;
+            foreach (var item in nestedTypesAndProperties)
+            {
+                var nestedProperty = sourceType.GetProperty(item.Key);
+                var nestedPropertyType = nestedProperty.PropertyType;
+                // [HasMany] attribute
+                if (nestedPropertyType != typeof(string) && typeof(IEnumerable).IsAssignableFrom(nestedPropertyType))
+                {
+                    // Concrete type of Collection
+                    var singleType = nestedPropertyType.GetGenericArguments().Single();
+                    // {y}
+                    var nestedParameter = Expression.Parameter(singleType, "y");
+                    nestedBindings = item.Value.Select(prop => Expression.Bind(
+                        singleType.GetProperty(prop), Expression.PropertyOrField(nestedParameter, prop))).ToList();
+
+                    // { new Item() }
+                    var newNestedExp = Expression.New(singleType);
+                    var initNestedExp = Expression.MemberInit(newNestedExp, nestedBindings);
+                    // { y => new Item() {Id = y.Id, Name = y.Name}}
+                    var body = Expression.Lambda(initNestedExp, nestedParameter);
+                    // { x.Items }
+                    Expression propertyExpression = Expression.Property(parameter, nestedProperty.Name);
+                    // { x.Items.Select(y => new Item() {Id = y.Id, Name = y.Name}) }
+                    Expression selectMethod = Expression.Call(
+                        typeof(Enumerable),
+                        "Select",
+                        new Type[] { singleType, singleType },
+                        propertyExpression, body);
+
+                    // { x.Items.Select(y => new Item() {Id = y.Id, Name = y.Name}).ToList() }
+                    bindExpression = Expression.Call(
+                         typeof(Enumerable),
+                         "ToList",
+                         new Type[] { singleType },
+                         selectMethod);
+                }
+                // [HasOne] attribute
+                else
+                {
+                    // {x.Owner}
+                    var srcBody = Expression.PropertyOrField(parameter, item.Key);
+                    foreach (var nested in item.Value)
+                    {
+                        // {x.Owner.Name}
+                        var nestedBody = Expression.PropertyOrField(srcBody, nested);
+                        var propInfo = nestedPropertyType.GetProperty(nested);
+                        nestedBindings.Add(Expression.Bind(propInfo, nestedBody));
+                    }
+                    // { new Owner() }
+                    var newExp = Expression.New(nestedPropertyType);
+                    // { new Owner() { Id = x.Owner.Id, Name = x.Owner.Name }}
+                    var newInit = Expression.MemberInit(newExp, nestedBindings);
+
+                    // Handle nullable relationships
+                    // { Owner = x.Owner == null ? null : new Owner() {...} }
+                    bindExpression = Expression.Condition(
+                           Expression.Equal(srcBody, Expression.Constant(null)),
+                           Expression.Convert(Expression.Constant(null), nestedPropertyType),
+                           newInit
+                         );
+                }
 
-            // { new Model () { Property = model.Property } }
-            var body = Expression.MemberInit(Expression.New(resultType), bindings);
+                sourceBindings.Add(Expression.Bind(nestedProperty, bindExpression));
+                nestedBindings.Clear();
+            }
 
-            // { model => new TodoItem() { Property = model.Property } }
-            var selector = Expression.Lambda(body, parameter);
+            var sourceInit = Expression.MemberInit(Expression.New(sourceType), sourceBindings);
+            var finalBody = Expression.Lambda(sourceInit, parameter);
 
-            return source.Provider.CreateQuery<TSource>(
-                Expression.Call(typeof(Queryable), "Select", new[] { sourceType, resultType },
-                source.Expression, Expression.Quote(selector)));
+            return source.Provider.CreateQuery<TSource>(Expression.Call(
+                typeof(Queryable),
+                "Select",
+                new[] { source.ElementType, typeof(TSource) },
+                source.Expression,
+                Expression.Quote(finalBody)));
         }
 
         public static IQueryable<T> PageForward<T>(this IQueryable<T> source, int pageSize, int pageNumber)
diff --git a/src/JsonApiDotNetCore/Internal/Query/BaseAttrQuery.cs b/src/JsonApiDotNetCore/Internal/Query/BaseAttrQuery.cs
@@ -1,5 +1,6 @@
 using JsonApiDotNetCore.Models;
 using JsonApiDotNetCore.Services;
+using System;
 using System.Linq;
 
 namespace JsonApiDotNetCore.Internal.Query
@@ -15,7 +16,21 @@ public abstract class BaseAttrQuery
 
         public BaseAttrQuery(IJsonApiContext jsonApiContext, BaseQuery baseQuery)
         {
-            _jsonApiContext = jsonApiContext;
+            _jsonApiContext = jsonApiContext ?? throw new ArgumentNullException(nameof(jsonApiContext));
+
+            if(_jsonApiContext.RequestEntity == null) 
+                throw new ArgumentException($"{nameof(IJsonApiContext)}.{nameof(_jsonApiContext.RequestEntity)} cannot be null. "
+                    + "This property contains the ResourceGraph node for the requested entity. "
+                    + "If this is a unit test, you need to mock this member. "
+                    + "See this issue to check the current status of improved test guidelines: "
+                    + "https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/251", nameof(jsonApiContext));
+            
+            if(_jsonApiContext.ResourceGraph == null) 
+                throw new ArgumentException($"{nameof(IJsonApiContext)}.{nameof(_jsonApiContext.ResourceGraph)} cannot be null. "
+                    + "If this is a unit test, you need to construct a graph containing the resources being tested. "
+                    + "See this issue to check the current status of improved test guidelines: "
+                    + "https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/251", nameof(jsonApiContext));
+            
             if (baseQuery.IsAttributeOfRelationship)
             {
                 Relationship = GetRelationship(baseQuery.Relationship);
diff --git a/src/JsonApiDotNetCore/Internal/Query/SortQuery.cs b/src/JsonApiDotNetCore/Internal/Query/SortQuery.cs
@@ -8,7 +8,7 @@ namespace JsonApiDotNetCore.Internal.Query
     /// </summary>
     public class SortQuery : BaseQuery
     {
-        [Obsolete("Use constructor overload (SortDirection, string) instead.", error: true)]
+        [Obsolete("Use constructor overload (SortDirection, string) instead. The string should be the publicly exposed attribute name.", error: true)]
         public SortQuery(SortDirection direction, AttrAttribute sortedAttribute)
             : base(sortedAttribute.PublicAttributeName) { }
 
diff --git a/src/JsonApiDotNetCore/Services/EntityResourceService.cs b/src/JsonApiDotNetCore/Services/EntityResourceService.cs
@@ -247,7 +247,12 @@ private async Task<TResource> GetWithRelationshipsAsync(TId id)
                 query = _entities.Include(query, r);
             });
 
-            var value = await _entities.FirstOrDefaultAsync(query);
+            TEntity value;
+            // https://github.com/aspnet/EntityFrameworkCore/issues/6573
+            if (_jsonApiContext.QuerySet?.Fields?.Count > 0)
+                value = query.FirstOrDefault();
+            else
+                value = await _entities.FirstOrDefaultAsync(query);
 
             return MapOut(value);
         }
diff --git a/src/JsonApiDotNetCore/Services/QueryParser.cs b/src/JsonApiDotNetCore/Services/QueryParser.cs
@@ -182,25 +182,34 @@ protected virtual List<string> ParseFieldsQuery(string key, string value)
         {
             // expected: fields[TYPE]=prop1,prop2
             var typeName = key.Split(QueryConstants.OPEN_BRACKET, QueryConstants.CLOSE_BRACKET)[1];
+            var includedFields = new List<string> { nameof(Identifiable.Id) };
 
-            const string ID = "Id";
-            var includedFields = new List<string> { ID };
-
-            // this will not support nested inclusions, it requires that the typeName is the current request type
-            if (string.Equals(typeName, _controllerContext.RequestEntity.EntityName, StringComparison.OrdinalIgnoreCase) == false)
+            var relationship = _controllerContext.RequestEntity.Relationships.SingleOrDefault(a => a.Is(typeName));
+            if (relationship == default && string.Equals(typeName, _controllerContext.RequestEntity.EntityName, StringComparison.OrdinalIgnoreCase) == false)
                 return includedFields;
 
             var fields = value.Split(QueryConstants.COMMA);
             foreach (var field in fields)
             {
-                var attr = _controllerContext.RequestEntity
-                    .Attributes
-                    .SingleOrDefault(a => a.Is(field));
+                if (relationship != default)
+                {
+                    var relationProperty = _options.ResourceGraph.GetContextEntity(relationship.Type);
+                    var attr = relationProperty.Attributes.SingleOrDefault(a => a.Is(field));
+                    if(attr == null)
+                        throw new JsonApiException(400, $"'{relationship.Type.Name}' does not contain '{field}'.");
 
-                if (attr == null) throw new JsonApiException(400, $"'{_controllerContext.RequestEntity.EntityName}' does not contain '{field}'.");
+                    // e.g. "Owner.Name"
+                    includedFields.Add(relationship.InternalRelationshipName + "." + attr.InternalAttributeName);
+                }
+                else
+                {
+                    var attr = _controllerContext.RequestEntity.Attributes.SingleOrDefault(a => a.Is(field));
+                    if (attr == null)
+                        throw new JsonApiException(400, $"'{_controllerContext.RequestEntity.EntityName}' does not contain '{field}'.");
 
-                var internalAttrName = attr.InternalAttributeName;
-                includedFields.Add(internalAttrName);
+                    // e.g. "Name"
+                    includedFields.Add(attr.InternalAttributeName);
+                }
             }
 
             return includedFields;
diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Linq;
 using System.Net;
 using System.Net.Http;
@@ -358,5 +359,56 @@ public async Task Request_ToIncludeRelationshipMarkedCanIncludeFalse_Returns_400
             // assert
             Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
         }
+
+        [Fact]
+        public async Task Can_Ignore_Null_Parent_In_Nested_Include()
+        {
+            // arrange
+            var todoItem = _todoItemFaker.Generate();
+            todoItem.Owner = _personFaker.Generate();
+            todoItem.CreatedDate = DateTime.Now;
+            _context.TodoItems.Add(todoItem);
+            _context.SaveChanges();
+
+            var todoItemWithNullOwner = _todoItemFaker.Generate();
+            todoItemWithNullOwner.Owner = null;
+            todoItemWithNullOwner.CreatedDate = DateTime.Now;
+            _context.TodoItems.Add(todoItemWithNullOwner);
+            _context.SaveChanges();
+
+            var builder = new WebHostBuilder()
+                .UseStartup<Startup>();
+
+            var httpMethod = new HttpMethod("GET");
+          
+            var route = $"/api/v1/todo-items?sort=-created-date&page[size]=2&include=owner.role"; // last two todo-items
+
+            var server = new TestServer(builder);
+            var client = server.CreateClient();
+            var request = new HttpRequestMessage(httpMethod, route);
+
+            // act
+            var response = await client.SendAsync(request);
+            var responseString = await response.Content.ReadAsStringAsync();
+            var documents = JsonConvert.DeserializeObject<Documents>(responseString);
+
+            // assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.Single(documents.Included);
+
+            var ownerValueNull = documents.Data
+                .First(i => i.Id == todoItemWithNullOwner.StringId)
+                .Relationships.First(i => i.Key == "owner")
+                .Value.SingleData;
+
+            Assert.Null(ownerValueNull);
+
+            var ownerValue = documents.Data
+                .First(i => i.Id == todoItem.StringId)
+                .Relationships.First(i => i.Key == "owner")
+                .Value.SingleData;
+
+            Assert.NotNull(ownerValue);
+        }
     }
 }
diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/SparseFieldSetTests.cs
diff --git a/test/UnitTests/Services/QueryParser_Tests.cs b/test/UnitTests/Services/QueryParser_Tests.cs

Original file line number	Diff line number	Diff line change
`@@ -218,6 +218,8 @@ private List<ResourceObject> IncludeRelationshipChain(`
`218`	`218`	`throw new JsonApiException(400, $"{parentEntity.EntityName} does not contain relationship {requestedRelationship}");`
`219`	`219`
`220`	`220`	`var navigationEntity = _jsonApiContext.ResourceGraph.GetRelationshipValue(parentResource, relationship);`
	`221`	`+ if(navigationEntity == null)`
	`222`	`+ return included;`
`221`	`223`	`if (navigationEntity is IEnumerable hasManyNavigationEntity)`
`222`	`224`	`{`
`223`	`225`	`foreach (IIdentifiable includedEntity in hasManyNavigationEntity)`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ namespace JsonApiDotNetCore.Internal.Query`
`8`	`8`	`/// </summary>`
`9`	`9`	`public class SortQuery : BaseQuery`
`10`	`10`	`{`
`11`		`- [Obsolete("Use constructor overload (SortDirection, string) instead.", error: true)]`
	`11`	`+ [Obsolete("Use constructor overload (SortDirection, string) instead. The string should be the publicly exposed attribute name.", error: true)]`
`12`	`12`	`public SortQuery(SortDirection direction, AttrAttribute sortedAttribute)`
`13`	`13`	`: base(sortedAttribute.PublicAttributeName) { }`
`14`	`14`