Changed SortExpression.Elements type from IReadOnlyDictionary to IImmutableList

Bart Koelman · Bart Koelman · commit ae4a57bba481 · 2021-08-03T20:38:34.000+02:00
diff --git a/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs b/src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs
@@ -136,7 +136,7 @@ public override QueryExpression VisitSort(SortExpression expression, TArgument a
         {
             if (expression != null)
             {
-                IReadOnlyCollection<SortElementExpression> newElements = VisitSequence(expression.Elements, argument);
+                IImmutableList<SortElementExpression> newElements = VisitList(expression.Elements, argument);
 
                 if (newElements.Count != 0)
                 {
@@ -317,5 +317,21 @@ protected virtual IReadOnlyCollection<TExpression> VisitSequence<TExpression>(IE
 
             return newElements;
         }
+
+        protected virtual IImmutableList<TExpression> VisitList<TExpression>(IImmutableList<TExpression> elements, TArgument argument)
+            where TExpression : QueryExpression
+        {
+            ImmutableArray<TExpression>.Builder arrayBuilder = ImmutableArray.CreateBuilder<TExpression>(elements.Count);
+
+            foreach (TExpression element in elements)
+            {
+                if (Visit(element, argument) is TExpression newElement)
+                {
+                    arrayBuilder.Add(newElement);
+                }
+            }
+
+            return arrayBuilder.ToImmutable();
+        }
     }
 }
diff --git a/src/JsonApiDotNetCore/Queries/Expressions/SortExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/SortExpression.cs
@@ -1,5 +1,5 @@
 using System;
-using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using JetBrains.Annotations;
 
@@ -11,9 +11,9 @@ namespace JsonApiDotNetCore.Queries.Expressions
     [PublicAPI]
     public class SortExpression : QueryExpression
     {
-        public IReadOnlyCollection<SortElementExpression> Elements { get; }
+        public IImmutableList<SortElementExpression> Elements { get; }
 
-        public SortExpression(IReadOnlyCollection<SortElementExpression> elements)
+        public SortExpression(IImmutableList<SortElementExpression> elements)
         {
             ArgumentGuard.NotNullNorEmpty(elements, nameof(elements));
 
diff --git a/src/JsonApiDotNetCore/Queries/Internal/Parsing/SortParser.cs b/src/JsonApiDotNetCore/Queries/Internal/Parsing/SortParser.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using JetBrains.Annotations;
 using JsonApiDotNetCore.Configuration;
@@ -40,17 +41,18 @@ protected SortExpression ParseSort()
         {
             SortElementExpression firstElement = ParseSortElement();
 
-            List<SortElementExpression> elements = firstElement.AsList();
+            ImmutableArray<SortElementExpression>.Builder elementsBuilder = ImmutableArray.CreateBuilder<SortElementExpression>();
+            elementsBuilder.Add(firstElement);
 
             while (TokenStack.Any())
             {
                 EatSingleCharacterToken(TokenKind.Comma);
 
                 SortElementExpression nextElement = ParseSortElement();
-                elements.Add(nextElement);
+                elementsBuilder.Add(nextElement);
             }
 
-            return new SortExpression(elements);
+            return new SortExpression(elementsBuilder.ToImmutable());
         }
 
         protected SortElementExpression ParseSortElement()
diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs
@@ -461,7 +461,8 @@ protected virtual SortExpression GetSort(IReadOnlyCollection<QueryExpression> ex
             if (sort == null)
             {
                 AttrAttribute idAttribute = GetIdAttribute(resourceContext);
-                sort = new SortExpression(new SortElementExpression(new ResourceFieldChainExpression(idAttribute), true).AsArray());
+                var idAscendingSort = new SortElementExpression(new ResourceFieldChainExpression(idAttribute), true);
+                sort = new SortExpression(ImmutableArray.Create(idAscendingSort));
             }
 
             return sort;
diff --git a/src/JsonApiDotNetCore/Resources/JsonApiResourceDefinition.cs b/src/JsonApiDotNetCore/Resources/JsonApiResourceDefinition.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.ComponentModel;
 using System.Linq;
 using System.Linq.Expressions;
@@ -84,18 +85,18 @@ protected SortExpression CreateSortExpressionFromLambda(PropertySortOrder keySel
         {
             ArgumentGuard.NotNull(keySelectors, nameof(keySelectors));
 
-            var sortElements = new List<SortElementExpression>();
+            ImmutableArray<SortElementExpression>.Builder elementsBuilder = ImmutableArray.CreateBuilder<SortElementExpression>(keySelectors.Count);
 
             foreach ((Expression<Func<TResource, dynamic>> keySelector, ListSortDirection sortDirection) in keySelectors)
             {
                 bool isAscending = sortDirection == ListSortDirection.Ascending;
                 AttrAttribute attribute = ResourceGraph.GetAttributes(keySelector).Single();
 
                 var sortElement = new SortElementExpression(new ResourceFieldChainExpression(attribute), isAscending);
-                sortElements.Add(sortElement);
+                elementsBuilder.Add(sortElement);
             }
 
-            return new SortExpression(sortElements);
+            return new SortExpression(elementsBuilder.ToImmutable());
         }
 
         /// <inheritdoc />
diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarExpressionRewriter.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarExpressionRewriter.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using System.Reflection;
 using JsonApiDotNetCore;
@@ -27,7 +28,6 @@ public CarExpressionRewriter(IResourceContextProvider resourceContextProvider)
             ResourceContext carResourceContext = resourceContextProvider.GetResourceContext<Car>();
 
             _regionIdAttribute = carResourceContext.Attributes.Single(attribute => attribute.Property.Name == nameof(Car.RegionId));
-
             _licensePlateAttribute = carResourceContext.Attributes.Single(attribute => attribute.Property.Name == nameof(Car.LicensePlate));
         }
 
@@ -121,25 +121,25 @@ private FilterExpression CreateEqualityComparisonOnCompositeKey(ResourceFieldCha
 
         public override QueryExpression VisitSort(SortExpression expression, object argument)
         {
-            var newSortElements = new List<SortElementExpression>();
+            ImmutableArray<SortElementExpression>.Builder elementsBuilder = ImmutableArray.CreateBuilder<SortElementExpression>(expression.Elements.Count);
 
             foreach (SortElementExpression sortElement in expression.Elements)
             {
                 if (IsSortOnCarId(sortElement))
                 {
                     ResourceFieldChainExpression regionIdSort = ReplaceLastAttributeInChain(sortElement.TargetAttribute, _regionIdAttribute);
-                    newSortElements.Add(new SortElementExpression(regionIdSort, sortElement.IsAscending));
+                    elementsBuilder.Add(new SortElementExpression(regionIdSort, sortElement.IsAscending));
 
                     ResourceFieldChainExpression licensePlateSort = ReplaceLastAttributeInChain(sortElement.TargetAttribute, _licensePlateAttribute);
-                    newSortElements.Add(new SortElementExpression(licensePlateSort, sortElement.IsAscending));
+                    elementsBuilder.Add(new SortElementExpression(licensePlateSort, sortElement.IsAscending));
                 }
                 else
                 {
-                    newSortElements.Add(sortElement);
+                    elementsBuilder.Add(sortElement);
                 }
             }
 
-            return new SortExpression(newSortElements);
+            return new SortExpression(elementsBuilder.ToImmutable());
         }
 
         private static bool IsSortOnCarId(SortElementExpression sortElement)
diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarRepository.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CarRepository.cs
@@ -13,13 +13,13 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.CompositeKeys
     [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)]
     public sealed class CarRepository : EntityFrameworkCoreRepository<Car, string>
     {
-        private readonly IResourceGraph _resourceGraph;
+        private readonly CarExpressionRewriter _writer;
 
         public CarRepository(ITargetedFields targetedFields, IDbContextResolver contextResolver, IResourceGraph resourceGraph, IResourceFactory resourceFactory,
             IEnumerable<IQueryConstraintProvider> constraintProviders, ILoggerFactory loggerFactory)
             : base(targetedFields, contextResolver, resourceGraph, resourceFactory, constraintProviders, loggerFactory)
         {
-            _resourceGraph = resourceGraph;
+            _writer = new CarExpressionRewriter(resourceGraph);
         }
 
         protected override IQueryable<Car> ApplyQueryLayer(QueryLayer layer)
@@ -33,14 +33,12 @@ private void RecursiveRewriteFilterInLayer(QueryLayer queryLayer)
         {
             if (queryLayer.Filter != null)
             {
-                var writer = new CarExpressionRewriter(_resourceGraph);
-                queryLayer.Filter = (FilterExpression)writer.Visit(queryLayer.Filter, null);
+                queryLayer.Filter = (FilterExpression)_writer.Visit(queryLayer.Filter, null);
             }
 
             if (queryLayer.Sort != null)
             {
-                var writer = new CarExpressionRewriter(_resourceGraph);
-                queryLayer.Sort = (SortExpression)writer.Visit(queryLayer.Sort, null);
+                queryLayer.Sort = (SortExpression)_writer.Visit(queryLayer.Sort, null);
             }
 
             if (queryLayer.Projection != null)

Original file line number	Diff line number	Diff line change
`@@ -136,7 +136,7 @@ public override QueryExpression VisitSort(SortExpression expression, TArgument a`
`136`	`136`	`{`
`137`	`137`	`if (expression != null)`
`138`	`138`	`{`
`139`		`- IReadOnlyCollection<SortElementExpression> newElements = VisitSequence(expression.Elements, argument);`
	`139`	`+ IImmutableList<SortElementExpression> newElements = VisitList(expression.Elements, argument);`
`140`	`140`
`141`	`141`	`if (newElements.Count != 0)`
`142`	`142`	`{`
`@@ -317,5 +317,21 @@ protected virtual IReadOnlyCollection<TExpression> VisitSequence<TExpression>(IE`
`317`	`317`
`318`	`318`	`return newElements;`
`319`	`319`	`}`
	`320`	`+`
	`321`	`+ protected virtual IImmutableList<TExpression> VisitList<TExpression>(IImmutableList<TExpression> elements, TArgument argument)`
	`322`	`+ where TExpression : QueryExpression`
	`323`	`+ {`
	`324`	`+ ImmutableArray<TExpression>.Builder arrayBuilder = ImmutableArray.CreateBuilder<TExpression>(elements.Count);`
	`325`	`+`
	`326`	`+ foreach (TExpression element in elements)`
	`327`	`+ {`
	`328`	`+ if (Visit(element, argument) is TExpression newElement)`
	`329`	`+ {`
	`330`	`+ arrayBuilder.Add(newElement);`
	`331`	`+ }`
	`332`	`+ }`
	`333`	`+`
	`334`	`+ return arrayBuilder.ToImmutable();`
	`335`	`+ }`
`320`	`336`	`}`
`321`	`337`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`using System;`
`2`	`2`	`using System.Collections.Generic;`
	`3`	`+using System.Collections.Immutable;`
`3`	`4`	`using System.Linq;`
`4`	`5`	`using JetBrains.Annotations;`
`5`	`6`	`using JsonApiDotNetCore.Configuration;`
`@@ -40,17 +41,18 @@ protected SortExpression ParseSort()`
`40`	`41`	`{`
`41`	`42`	`SortElementExpression firstElement = ParseSortElement();`
`42`	`43`
`43`		`- List<SortElementExpression> elements = firstElement.AsList();`
	`44`	`+ ImmutableArray<SortElementExpression>.Builder elementsBuilder = ImmutableArray.CreateBuilder<SortElementExpression>();`
	`45`	`+ elementsBuilder.Add(firstElement);`
`44`	`46`
`45`	`47`	`while (TokenStack.Any())`
`46`	`48`	`{`
`47`	`49`	`EatSingleCharacterToken(TokenKind.Comma);`
`48`	`50`
`49`	`51`	`SortElementExpression nextElement = ParseSortElement();`
`50`		`- elements.Add(nextElement);`
	`52`	`+ elementsBuilder.Add(nextElement);`
`51`	`53`	`}`
`52`	`54`
`53`		`- return new SortExpression(elements);`
	`55`	`+ return new SortExpression(elementsBuilder.ToImmutable());`
`54`	`56`	`}`
`55`	`57`
`56`	`58`	`protected SortElementExpression ParseSortElement()`
Original file line number	Diff line number	Diff line change
`@@ -461,7 +461,8 @@ protected virtual SortExpression GetSort(IReadOnlyCollection<QueryExpression> ex`
`461`	`461`	`if (sort == null)`
`462`	`462`	`{`
`463`	`463`	`AttrAttribute idAttribute = GetIdAttribute(resourceContext);`
`464`		`- sort = new SortExpression(new SortElementExpression(new ResourceFieldChainExpression(idAttribute), true).AsArray());`
	`464`	`+ var idAscendingSort = new SortElementExpression(new ResourceFieldChainExpression(idAttribute), true);`
	`465`	`+ sort = new SortExpression(ImmutableArray.Create(idAscendingSort));`
`465`	`466`	`}`
`466`	`467`
`467`	`468`	`return sort;`
Original file line number	Diff line number	Diff line change
`@@ -13,13 +13,13 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.CompositeKeys`
`13`	`13`	`[UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)]`
`14`	`14`	`public sealed class CarRepository : EntityFrameworkCoreRepository<Car, string>`
`15`	`15`	`{`
`16`		`- private readonly IResourceGraph _resourceGraph;`
	`16`	`+ private readonly CarExpressionRewriter _writer;`
`17`	`17`
`18`	`18`	`public CarRepository(ITargetedFields targetedFields, IDbContextResolver contextResolver, IResourceGraph resourceGraph, IResourceFactory resourceFactory,`
`19`	`19`	`IEnumerable<IQueryConstraintProvider> constraintProviders, ILoggerFactory loggerFactory)`
`20`	`20`	`: base(targetedFields, contextResolver, resourceGraph, resourceFactory, constraintProviders, loggerFactory)`
`21`	`21`	`{`
`22`		`- _resourceGraph = resourceGraph;`
	`22`	`+ _writer = new CarExpressionRewriter(resourceGraph);`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`protected override IQueryable<Car> ApplyQueryLayer(QueryLayer layer)`
`@@ -33,14 +33,12 @@ private void RecursiveRewriteFilterInLayer(QueryLayer queryLayer)`
`33`	`33`	`{`
`34`	`34`	`if (queryLayer.Filter != null)`
`35`	`35`	`{`
`36`		`- var writer = new CarExpressionRewriter(_resourceGraph);`
`37`		`- queryLayer.Filter = (FilterExpression)writer.Visit(queryLayer.Filter, null);`
	`36`	`+ queryLayer.Filter = (FilterExpression)_writer.Visit(queryLayer.Filter, null);`
`38`	`37`	`}`
`39`	`38`
`40`	`39`	`if (queryLayer.Sort != null)`
`41`	`40`	`{`
`42`		`- var writer = new CarExpressionRewriter(_resourceGraph);`
`43`		`- queryLayer.Sort = (SortExpression)writer.Visit(queryLayer.Sort, null);`
	`41`	`+ queryLayer.Sort = (SortExpression)_writer.Visit(queryLayer.Sort, null);`
`44`	`42`	`}`
`45`	`43`
`46`	`44`	`if (queryLayer.Projection != null)`