Includes query string parameters in top-level self link and paging li…

…nks (#698) * Includes query string parameters in top-level self link and paging links * Review feedback: rename QueryParameterDiscovery to QueryParameterParser * Review feedback: Make QueryParameterParser use IRequestQueryStringAccessor; make RequestQueryStringAccessor internal and register as singleton
nicolestandifer3 · Mar 25, 2020 · 24999c4 · 24999c4
1 parent e0f5a92
commit 24999c4
Show file tree

Hide file tree

Showing 10 changed files with 173 additions and 77 deletions.
diff --git a/benchmarks/Query/QueryParserBenchmarks.cs b/benchmarks/Query/QueryParserBenchmarks.cs
@@ -5,17 +5,19 @@
 using JsonApiDotNetCore.Internal.Contracts;
 using JsonApiDotNetCore.Managers;
 using JsonApiDotNetCore.Query;
+using JsonApiDotNetCore.QueryParameterServices.Common;
 using JsonApiDotNetCore.Services;
 using Microsoft.AspNetCore.Http;
-using Microsoft.Extensions.Primitives;
+using Microsoft.AspNetCore.WebUtilities;
 
 namespace Benchmarks.Query
 {
     [MarkdownExporter, SimpleJob(launchCount: 3, warmupCount: 10, targetCount: 20), MemoryDiagnoser]
     public class QueryParserBenchmarks
     {
-        private readonly QueryParameterDiscovery _queryParameterDiscoveryForSort;
-        private readonly QueryParameterDiscovery _queryParameterDiscoveryForAll;
+        private readonly FakeRequestQueryStringAccessor _queryStringAccessor = new FakeRequestQueryStringAccessor();
+        private readonly QueryParameterParser _queryParameterParserForSort;
+        private readonly QueryParameterParser _queryParameterParserForAll;
 
         public QueryParserBenchmarks()
         {
@@ -27,12 +29,13 @@ public QueryParserBenchmarks()
 
             IResourceDefinitionProvider resourceDefinitionProvider = DependencyFactory.CreateResourceDefinitionProvider(resourceGraph);
 
-            _queryParameterDiscoveryForSort = CreateQueryParameterDiscoveryForSort(resourceGraph, currentRequest, resourceDefinitionProvider, options);
-            _queryParameterDiscoveryForAll = CreateQueryParameterDiscoveryForAll(resourceGraph, currentRequest, resourceDefinitionProvider, options);
+            _queryParameterParserForSort = CreateQueryParameterDiscoveryForSort(resourceGraph, currentRequest, resourceDefinitionProvider, options, _queryStringAccessor);
+            _queryParameterParserForAll = CreateQueryParameterDiscoveryForAll(resourceGraph, currentRequest, resourceDefinitionProvider, options, _queryStringAccessor);
         }
 
-        private static QueryParameterDiscovery CreateQueryParameterDiscoveryForSort(IResourceGraph resourceGraph,
-            CurrentRequest currentRequest, IResourceDefinitionProvider resourceDefinitionProvider, IJsonApiOptions options)
+        private static QueryParameterParser CreateQueryParameterDiscoveryForSort(IResourceGraph resourceGraph,
+            CurrentRequest currentRequest, IResourceDefinitionProvider resourceDefinitionProvider,
+            IJsonApiOptions options, FakeRequestQueryStringAccessor queryStringAccessor)
         {
             ISortService sortService = new SortService(resourceDefinitionProvider, resourceGraph, currentRequest);
 
@@ -41,11 +44,12 @@ public QueryParserBenchmarks()
                 sortService
             };
 
-            return new QueryParameterDiscovery(options, queryServices);
+            return new QueryParameterParser(options, queryStringAccessor, queryServices);
         }
 
-        private static QueryParameterDiscovery CreateQueryParameterDiscoveryForAll(IResourceGraph resourceGraph,
-            CurrentRequest currentRequest, IResourceDefinitionProvider resourceDefinitionProvider, IJsonApiOptions options)
+        private static QueryParameterParser CreateQueryParameterDiscoveryForAll(IResourceGraph resourceGraph,
+            CurrentRequest currentRequest, IResourceDefinitionProvider resourceDefinitionProvider,
+            IJsonApiOptions options, FakeRequestQueryStringAccessor queryStringAccessor)
         {
             IIncludeService includeService = new IncludeService(resourceGraph, currentRequest);
             IFilterService filterService = new FilterService(resourceDefinitionProvider, resourceGraph, currentRequest);
@@ -61,40 +65,51 @@ public QueryParserBenchmarks()
                 omitNullService
             };
 
-            return new QueryParameterDiscovery(options, queryServices);
+            return new QueryParameterParser(options, queryStringAccessor, queryServices);
         }
 
         [Benchmark]
-        public void AscendingSort() => _queryParameterDiscoveryForSort.Parse(new QueryCollection(
-            new Dictionary<string, StringValues>
-            {
-                {"sort", BenchmarkResourcePublicNames.NameAttr}
-            }
-        ), null);
+        public void AscendingSort()
+        {
+            var queryString = $"?sort={BenchmarkResourcePublicNames.NameAttr}";
+
+            _queryStringAccessor.SetQueryString(queryString);
+            _queryParameterParserForSort.Parse(null);
+        }
 
         [Benchmark]
-        public void DescendingSort() => _queryParameterDiscoveryForSort.Parse(new QueryCollection(
-            new Dictionary<string, StringValues>
-            {
-                {"sort", $"-{BenchmarkResourcePublicNames.NameAttr}"}
-            }
-        ), null);
+        public void DescendingSort()
+        {
+            var queryString = $"?sort=-{BenchmarkResourcePublicNames.NameAttr}";
+
+            _queryStringAccessor.SetQueryString(queryString);
+            _queryParameterParserForSort.Parse(null);
+        }
 
         [Benchmark]
-        public void ComplexQuery() => Run(100, () => _queryParameterDiscoveryForAll.Parse(new QueryCollection(
-            new Dictionary<string, StringValues>
-            {
-                {$"filter[{BenchmarkResourcePublicNames.NameAttr}]", new StringValues(new[] {"abc", "eq:abc"})},
-                {"sort", $"-{BenchmarkResourcePublicNames.NameAttr}"},
-                {"include", "child"},
-                {"page[size]", "1"},
-                {"fields", BenchmarkResourcePublicNames.NameAttr}
-            }
-        ), null));
+        public void ComplexQuery() => Run(100, () =>
+        {
+            var queryString = $"?filter[{BenchmarkResourcePublicNames.NameAttr}]=abc,eq:abc&sort=-{BenchmarkResourcePublicNames.NameAttr}&include=child&page[size]=1&fields={BenchmarkResourcePublicNames.NameAttr}";
+
+            _queryStringAccessor.SetQueryString(queryString);
+            _queryParameterParserForAll.Parse(null);
+        });
 
         private void Run(int iterations, Action action) { 
             for (int i = 0; i < iterations; i++)
                 action();
         }
+
+        private sealed class FakeRequestQueryStringAccessor : IRequestQueryStringAccessor
+        {
+            public QueryString QueryString { get; private set; }
+            public IQueryCollection Query { get; private set; }
+
+            public void SetQueryString(string queryString)
+            {
+                QueryString = new QueryString(queryString);
+                Query = new QueryCollection(QueryHelpers.ParseQuery(queryString));
+            }
+        }
     }
 }
diff --git a/src/JsonApiDotNetCore/Builders/JsonApiApplicationBuilder.cs b/src/JsonApiDotNetCore/Builders/JsonApiApplicationBuilder.cs
@@ -20,6 +20,7 @@
 using JsonApiDotNetCore.Serialization.Server.Builders;
 using JsonApiDotNetCore.Serialization.Server;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using JsonApiDotNetCore.QueryParameterServices.Common;
 
 namespace JsonApiDotNetCore.Builders
 {
@@ -141,13 +142,15 @@ public void ConfigureServices()
             _services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();
             _services.AddSingleton(resourceGraph);
             _services.AddSingleton<IResourceContextProvider>(resourceGraph);
+            _services.AddSingleton<IRequestQueryStringAccessor, RequestQueryStringAccessor>();
+
             _services.AddScoped<ICurrentRequest, CurrentRequest>();
             _services.AddScoped<IScopedServiceProvider, RequestScopedServiceProvider>();
             _services.AddScoped<IJsonApiWriter, JsonApiWriter>();
             _services.AddScoped<IJsonApiReader, JsonApiReader>();
             _services.AddScoped<IGenericServiceFactory, GenericServiceFactory>();
             _services.AddScoped(typeof(RepositoryRelationshipUpdateHelper<>));
-            _services.AddScoped<IQueryParameterDiscovery, QueryParameterDiscovery>();
+            _services.AddScoped<IQueryParameterParser, QueryParameterParser>();
             _services.AddScoped<ITargetedFields, TargetedFields>();
             _services.AddScoped<IResourceDefinitionProvider, ResourceDefinitionProvider>();
             _services.AddScoped<IFieldsToSerialize, FieldsToSerialize>();

diff --git a/src/JsonApiDotNetCore/Middleware/QueryParameterFilter.cs b/src/JsonApiDotNetCore/Middleware/QueryParameterFilter.cs
@@ -8,15 +8,15 @@ namespace JsonApiDotNetCore.Middleware
 {
     public sealed class QueryParameterActionFilter : IAsyncActionFilter, IQueryParameterActionFilter
     {
-        private readonly IQueryParameterDiscovery _queryParser;
-        public QueryParameterActionFilter(IQueryParameterDiscovery queryParser) => _queryParser = queryParser;
+        private readonly IQueryParameterParser _queryParser;
+        public QueryParameterActionFilter(IQueryParameterParser queryParser) => _queryParser = queryParser;
 
         public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next)
         {
             // gets the DisableQueryAttribute if set on the controller that is targeted by the current request.
             DisableQueryAttribute disabledQuery = context.Controller.GetType().GetTypeInfo().GetCustomAttribute(typeof(DisableQueryAttribute)) as DisableQueryAttribute;
 
-            _queryParser.Parse(context.HttpContext.Request.Query, disabledQuery);
+            _queryParser.Parse(disabledQuery);
             await next();
         }
     }

diff --git a/src/JsonApiDotNetCore/QueryParameterServices/Common/IQueryParameterParser.cs b/src/JsonApiDotNetCore/QueryParameterServices/Common/IQueryParameterParser.cs
@@ -1,15 +1,14 @@
 using JsonApiDotNetCore.Controllers;
 using JsonApiDotNetCore.Query;
-using Microsoft.AspNetCore.Http;
 
 namespace JsonApiDotNetCore.Services
 {
     /// <summary>
     /// Responsible for populating the various service implementations of
     /// <see cref="IQueryParameterService"/>.
     /// </summary>
-    public interface IQueryParameterDiscovery
+    public interface IQueryParameterParser
     {
-        void Parse(IQueryCollection query, DisableQueryAttribute disabledQuery = null);
+        void Parse(DisableQueryAttribute disabledQuery = null);
     }
 }
diff --git a/src/JsonApiDotNetCore/QueryParameterServices/Common/IRequestQueryStringAccessor.cs b/src/JsonApiDotNetCore/QueryParameterServices/Common/IRequestQueryStringAccessor.cs
@@ -0,0 +1,10 @@
+using Microsoft.AspNetCore.Http;
+
+namespace JsonApiDotNetCore.QueryParameterServices.Common
+{
+    public interface IRequestQueryStringAccessor
+    {
+        QueryString QueryString { get; }
+        IQueryCollection Query { get; }
+    }
+}
diff --git a/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterParser.cs b/src/JsonApiDotNetCore/QueryParameterServices/Common/QueryParameterParser.cs
@@ -4,32 +4,34 @@
 using JsonApiDotNetCore.Controllers;
 using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Query;
-using Microsoft.AspNetCore.Http;
+using JsonApiDotNetCore.QueryParameterServices.Common;
 
 namespace JsonApiDotNetCore.Services
 {
     /// <inheritdoc/>
-    public class QueryParameterDiscovery : IQueryParameterDiscovery
+    public class QueryParameterParser : IQueryParameterParser
     {
         private readonly IJsonApiOptions _options;
+        private readonly IRequestQueryStringAccessor _queryStringAccessor;
         private readonly IEnumerable<IQueryParameterService> _queryServices;
 
-        public QueryParameterDiscovery(IJsonApiOptions options, IEnumerable<IQueryParameterService> queryServices)
+        public QueryParameterParser(IJsonApiOptions options, IRequestQueryStringAccessor queryStringAccessor, IEnumerable<IQueryParameterService> queryServices)
         {
             _options = options;
+            _queryStringAccessor = queryStringAccessor;
             _queryServices = queryServices;
         }
 
         /// <summary>
-        /// For a query parameter in <paramref name="query"/>, calls
+        /// For a parameter in the query string of the request URL, calls
         /// the <see cref="IQueryParameterService.Parse(KeyValuePair{string, Microsoft.Extensions.Primitives.StringValues})"/>
         /// method of the corresponding service.
         /// </summary>
-        public virtual void Parse(IQueryCollection query, DisableQueryAttribute disabled)
+        public virtual void Parse(DisableQueryAttribute disabled)
         {
             var disabledQuery = disabled?.QueryParams;
 
-            foreach (var pair in query)
+            foreach (var pair in _queryStringAccessor.Query)
             {
                 bool parsed = false;
                 foreach (var service in _queryServices)

diff --git a/src/JsonApiDotNetCore/QueryParameterServices/Common/RequestQueryStringAccessor.cs b/src/JsonApiDotNetCore/QueryParameterServices/Common/RequestQueryStringAccessor.cs
@@ -0,0 +1,17 @@
+using Microsoft.AspNetCore.Http;
+
+namespace JsonApiDotNetCore.QueryParameterServices.Common
+{
+    internal sealed class RequestQueryStringAccessor : IRequestQueryStringAccessor
+    {
+        private readonly IHttpContextAccessor _httpContextAccessor;
+
+        public QueryString QueryString => _httpContextAccessor.HttpContext.Request.QueryString;
+        public IQueryCollection Query => _httpContextAccessor.HttpContext.Request.Query;
+
+        public RequestQueryStringAccessor(IHttpContextAccessor httpContextAccessor)
+        {
+            _httpContextAccessor = httpContextAccessor;
+        }
+    }
+}
diff --git a/src/JsonApiDotNetCore/Serialization/Server/Builders/LinkBuilder.cs b/src/JsonApiDotNetCore/Serialization/Server/Builders/LinkBuilder.cs
@@ -1,3 +1,6 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using System.Text;
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Internal;
@@ -6,25 +9,30 @@
 using JsonApiDotNetCore.Models;
 using JsonApiDotNetCore.Models.Links;
 using JsonApiDotNetCore.Query;
+using JsonApiDotNetCore.QueryParameterServices.Common;
+using Microsoft.AspNetCore.Http;
 
 namespace JsonApiDotNetCore.Serialization.Server.Builders
 {
     public class LinkBuilder : ILinkBuilder
     {
         private readonly IResourceContextProvider _provider;
+        private readonly IRequestQueryStringAccessor _queryStringAccessor;
         private readonly ILinksConfiguration _options;
         private readonly ICurrentRequest _currentRequest;
         private readonly IPageService _pageService;
 
         public LinkBuilder(ILinksConfiguration options,
                            ICurrentRequest currentRequest,
                            IPageService pageService,
-                           IResourceContextProvider provider)
+                           IResourceContextProvider provider,
+                           IRequestQueryStringAccessor queryStringAccessor)
         {
             _options = options;
             _currentRequest = currentRequest;
             _pageService = pageService;
             _provider = provider;
+            _queryStringAccessor = queryStringAccessor;
         }
 
         /// <inheritdoc/>
@@ -101,6 +109,8 @@ private string GetSelfTopLevelLink(ResourceContext resourceContext)
                 builder.Append(_currentRequest.RequestRelationship.PublicRelationshipName);
             }
 
+            builder.Append(_queryStringAccessor.QueryString.Value);
+
             return builder.ToString();
         }
 
@@ -111,9 +121,24 @@ private string GetPageLink(ResourceContext resourceContext, int pageOffset, int
                 pageOffset = -pageOffset;
             }
 
-            return $"{GetBasePath()}/{resourceContext.ResourceName}?page[size]={pageSize}&page[number]={pageOffset}";
+            string queryString = BuildQueryString(parameters =>
+            {
+                parameters["page[size]"] = pageSize.ToString();
+                parameters["page[number]"] = pageOffset.ToString();
+            });
+
+            return $"{GetBasePath()}/{resourceContext.ResourceName}" + queryString;
         }
 
+        private string BuildQueryString(Action<Dictionary<string, string>> updateAction)
+        {
+            var parameters = _queryStringAccessor.Query.ToDictionary(pair => pair.Key, pair => pair.Value.ToString());
+            updateAction(parameters);
+            string queryString = QueryString.Create(parameters).Value;
+
+            queryString = queryString.Replace("%5B", "[").Replace("%5D", "]");
+            return queryString;
+        }
 
         /// <inheritdoc/>
         public ResourceLinks GetResourceLinks(string resourceName, string id)