Changed AnyExpression.Constants type from IReadOnlyCollection to IImmutableSet

Author: Bart Koelman

src/JsonApiDotNetCore/Queries/Expressions/AnyExpression.cs

@@ -1,5 +1,5 @@
 using System;
-using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Linq;
 using System.Text;
 using JetBrains.Annotations;
@@ -14,9 +14,9 @@ namespace JsonApiDotNetCore.Queries.Expressions
     public class AnyExpression : FilterExpression
     {
         public ResourceFieldChainExpression TargetAttribute { get; }
-        public IReadOnlyCollection<LiteralConstantExpression> Constants { get; }
+        public IImmutableSet<LiteralConstantExpression> Constants { get; }
 
-        public AnyExpression(ResourceFieldChainExpression targetAttribute, IReadOnlyCollection<LiteralConstantExpression> constants)
+        public AnyExpression(ResourceFieldChainExpression targetAttribute, IImmutableSet<LiteralConstantExpression> constants)
         {
             ArgumentGuard.NotNull(targetAttribute, nameof(targetAttribute));
             ArgumentGuard.NotNull(constants, nameof(constants));
@@ -43,7 +43,7 @@ public override string ToString()
             builder.Append('(');
             builder.Append(TargetAttribute);
             builder.Append(',');
-            builder.Append(string.Join(",", Constants.Select(constant => constant.ToString())));
+            builder.Append(string.Join(",", Constants.Select(constant => constant.ToString()).OrderBy(value => value)));
             builder.Append(')');
 
             return builder.ToString();
@@ -63,7 +63,7 @@ public override bool Equals(object obj)
 
             var other = (AnyExpression)obj;
 
-            return TargetAttribute.Equals(other.TargetAttribute) && Constants.SequenceEqual(other.Constants);
+            return TargetAttribute.Equals(other.TargetAttribute) && Constants.SetEquals(other.Constants);
         }
 
         public override int GetHashCode()

src/JsonApiDotNetCore/Queries/Expressions/QueryExpressionRewriter.cs

@@ -186,7 +186,7 @@ public override QueryExpression VisitAny(AnyExpression expression, TArgument arg
             if (expression != null)
             {
                 var newTargetAttribute = Visit(expression.TargetAttribute, argument) as ResourceFieldChainExpression;
-                IReadOnlyCollection<LiteralConstantExpression> newConstants = VisitSequence(expression.Constants, argument);
+                IImmutableSet<LiteralConstantExpression> newConstants = VisitSet(expression.Constants, argument);
 
                 var newExpression = new AnyExpression(newTargetAttribute, newConstants);
                 return newExpression.Equals(expression) ? expression : newExpression;
@@ -333,5 +333,21 @@ protected virtual IImmutableList<TExpression> VisitList<TExpression>(IImmutableL
 
             return arrayBuilder.ToImmutable();
         }
+
+        protected virtual IImmutableSet<TExpression> VisitSet<TExpression>(IImmutableSet<TExpression> elements, TArgument argument)
+            where TExpression : QueryExpression
+        {
+            ImmutableHashSet<TExpression>.Builder setBuilder = ImmutableHashSet.CreateBuilder<TExpression>();
+
+            foreach (TExpression element in elements)
+            {
+                if (Visit(element, argument) is TExpression newElement)
+                {
+                    setBuilder.Add(newElement);
+                }
+            }
+
+            return setBuilder.ToImmutable();
+        }
     }
 }

src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs

@@ -196,39 +196,52 @@ protected AnyExpression ParseAny()
 
             EatSingleCharacterToken(TokenKind.Comma);
 
-            var constants = new List<LiteralConstantExpression>();
+            ImmutableHashSet<LiteralConstantExpression>.Builder constantsBuilder = ImmutableHashSet.CreateBuilder<LiteralConstantExpression>();
 
             LiteralConstantExpression constant = ParseConstant();
-            constants.Add(constant);
+            constantsBuilder.Add(constant);
 
             EatSingleCharacterToken(TokenKind.Comma);
 
             constant = ParseConstant();
-            constants.Add(constant);
+            constantsBuilder.Add(constant);
 
             while (TokenStack.TryPeek(out Token nextToken) && nextToken.Kind == TokenKind.Comma)
             {
                 EatSingleCharacterToken(TokenKind.Comma);
 
                 constant = ParseConstant();
-                constants.Add(constant);
+                constantsBuilder.Add(constant);
             }
 
             EatSingleCharacterToken(TokenKind.CloseParen);
 
+            IImmutableSet<LiteralConstantExpression> constantSet = constantsBuilder.ToImmutable();
+
             PropertyInfo targetAttributeProperty = targetAttribute.Fields[^1].Property;
 
             if (targetAttributeProperty.Name == nameof(Identifiable.Id))
             {
-                for (int index = 0; index < constants.Count; index++)
-                {
-                    string stringId = constants[index].Value;
-                    string id = DeObfuscateStringId(targetAttributeProperty.ReflectedType, stringId);
-                    constants[index] = new LiteralConstantExpression(id);
-                }
+                constantSet = DeObfuscateIdConstants(constantSet, targetAttributeProperty);
+            }
+
+            return new AnyExpression(targetAttribute, constantSet);
+        }
+
+        private IImmutableSet<LiteralConstantExpression> DeObfuscateIdConstants(IImmutableSet<LiteralConstantExpression> constantSet,
+            PropertyInfo targetAttributeProperty)
+        {
+            ImmutableHashSet<LiteralConstantExpression>.Builder idConstantsBuilder = ImmutableHashSet.CreateBuilder<LiteralConstantExpression>();
+
+            foreach (LiteralConstantExpression idConstant in constantSet)
+            {
+                string stringId = idConstant.Value;
+                string id = DeObfuscateStringId(targetAttributeProperty.ReflectedType, stringId);
+
+                idConstantsBuilder.Add(new LiteralConstantExpression(id));
             }
 
-            return new AnyExpression(targetAttribute, constants);
+            return idConstantsBuilder.ToImmutable();
         }
 
         protected HasExpression ParseHas()

src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs

@@ -297,7 +297,7 @@ private IncludeExpression RewriteIncludeForSecondaryEndpoint(IncludeExpression r
             return new IncludeExpression(parentElement.AsArray());
         }
 
-        private FilterExpression CreateFilterByIds<TId>(ICollection<TId> ids, AttrAttribute idAttribute, FilterExpression existingFilter)
+        private FilterExpression CreateFilterByIds<TId>(IReadOnlyCollection<TId> ids, AttrAttribute idAttribute, FilterExpression existingFilter)
         {
             var idChain = new ResourceFieldChainExpression(idAttribute);
 
@@ -310,7 +310,7 @@ private FilterExpression CreateFilterByIds<TId>(ICollection<TId> ids, AttrAttrib
             }
             else if (ids.Count > 1)
             {
-                List<LiteralConstantExpression> constants = ids.Select(id => new LiteralConstantExpression(id.ToString())).ToList();
+                ImmutableHashSet<LiteralConstantExpression> constants = ids.Select(id => new LiteralConstantExpression(id.ToString())).ToImmutableHashSet();
                 filter = new AnyExpression(idChain, constants);
             }
 

test/JsonApiDotNetCoreExampleTests/UnitTests/QueryStringParameters/FilterParseTests.cs

@@ -131,7 +131,7 @@ public void Reader_Read_Fails(string parameterName, string parameterValue, strin
         [InlineData("filter", "contains(title,'this')", null, "contains(title,'this')")]
         [InlineData("filter", "startsWith(title,'this')", null, "startsWith(title,'this')")]
         [InlineData("filter", "endsWith(title,'this')", null, "endsWith(title,'this')")]
-        [InlineData("filter", "any(title,'this','that','there')", null, "any(title,'this','that','there')")]
+        [InlineData("filter", "any(title,'this','that','there')", null, "any(title,'that','there','this')")]
         [InlineData("filter", "and(contains(title,'sales'),contains(title,'marketing'),contains(title,'advertising'))", null,
             "and(contains(title,'sales'),contains(title,'marketing'),contains(title,'advertising'))")]
         [InlineData("filter[posts]", "or(and(not(equals(author.userName,null)),not(equals(author.displayName,null))),not(has(comments,startsWith(text,'A'))))",