Allow empty filter expressions to be visited (#5214)

Co-authored-by: Pascal Senn <senn.pasc@gmail.com>
ChilliCream · Aug 9, 2022 · b369d8c · b369d8c
1 parent 4901fe6
commit b369d8c
Show file tree

Hide file tree

Showing 30 changed files with 313 additions and 114 deletions.
diff --git a/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableCombinator.cs b/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableCombinator.cs
@@ -1,4 +1,3 @@
-using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq.Expressions;
@@ -12,11 +11,12 @@ public override bool TryCombineOperations(
         QueryableFilterContext context,
         Queue<Expression> operations,
         FilterCombinator combinator,
-        [NotNullWhen(true)] out Expression combined)
+        [NotNullWhen(true)] out Expression? combined)
     {
         if (operations.Count == 0)
         {
-            throw ThrowHelper.Filtering_QueryableCombinator_QueueEmpty(this);
+            combined = default;
+            return false;
         }
 
         combined = operations.Dequeue();

diff --git a/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterOperationCombinator.cs b/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterOperationCombinator.cs
@@ -29,6 +29,6 @@ public abstract bool TryCombineOperations<TContext, T>(
         TContext context,
         Queue<T> operations,
         FilterCombinator combinator,
-        [NotNullWhen(true)] out T combined)
+        [NotNullWhen(true)] out T? combined)
         where TContext : FilterVisitorContext<T>;
 }
diff --git a/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterOperationCombinator~1.cs b/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterOperationCombinator~1.cs
@@ -23,7 +23,7 @@ public abstract bool TryCombineOperations(
         TContext context,
         Queue<T> operations,
         FilterCombinator combinator,
-        [NotNullWhen(true)] out T combined);
+        [NotNullWhen(true)] out T? combined);
 
     /// <inheritdoc />
     public override bool TryCombineOperations<TVisitorContext, TOperation>(

diff --git a/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterVisitor.cs b/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterVisitor.cs
@@ -54,6 +54,6 @@ protected override bool TryCombineOperations(
         TContext context,
         Queue<T> operations,
         FilterCombinator combinator,
-        [NotNullWhen(true)] out T combined) =>
+        [NotNullWhen(true)] out T? combined) =>
         _combinator.TryCombineOperations(context, operations, combinator, out combined);
 }
diff --git a/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterVisitorBase`2.cs b/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterVisitorBase`2.cs
@@ -27,7 +27,7 @@ protected abstract bool TryCombineOperations(
         TContext context,
         Queue<T> operations,
         FilterCombinator combinator,
-        [NotNullWhen(true)] out T combined);
+        [NotNullWhen(true)] out T? combined);
 
     protected override ISyntaxVisitorAction Leave(
         ObjectValueNode node,

diff --git a/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/QueryableFilterCombinatorTests.cs b/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/QueryableFilterCombinatorTests.cs
@@ -0,0 +1,58 @@
+using System.Threading.Tasks;
+using CookieCrumble;
+using HotChocolate.Execution;
+
+namespace HotChocolate.Data.Filters;
+
+public class QueryableFilterCombinatorTests
+{
+    private static readonly Foo[] _fooEntities =
+    {
+        new() { Bar = true },
+        new() { Bar = false }
+    };
+
+    private readonly SchemaCache _cache = new();
+
+    [Fact]
+    public async Task Create_Empty_Expression()
+    {
+        // arrange
+        var tester = _cache.CreateSchema<Foo, FooFilterInput>(_fooEntities);
+
+        // act
+        // assert
+        var res1 = await tester.ExecuteAsync(
+            QueryRequestBuilder.New()
+                .SetQuery("{ root(where: { }){ bar }}")
+                .Create());
+
+        await Snapshot.Create()
+            .Add(res1)
+            .MatchAsync();
+    }
+
+    public class Foo
+    {
+        public int Id { get; set; }
+
+        public bool Bar { get; set; }
+    }
+
+    public class FooNullable
+    {
+        public int Id { get; set; }
+
+        public bool? Bar { get; set; }
+    }
+
+    public class FooFilterInput
+        : FilterInputType<Foo>
+    {
+    }
+
+    public class FooNullableFilterInput
+        : FilterInputType<FooNullable>
+    {
+    }
+}
diff --git a/....InMemory.Tests/__snapshots__/QueryableFilterCombinatorTests.Create_Empty_Expression.snap b/....InMemory.Tests/__snapshots__/QueryableFilterCombinatorTests.Create_Empty_Expression.snap
@@ -0,0 +1,12 @@
+{
+  "data": {
+    "root": [
+      {
+        "bar": true
+      },
+      {
+        "bar": false
+      }
+    ]
+  }
+}
diff --git a/...napshots__/QueryableFirstOrDefaultTests.Create_DeepFilterObjectTwoProjections_NET6_0.snap b/...napshots__/QueryableFirstOrDefaultTests.Create_DeepFilterObjectTwoProjections_NET6_0.snap
@@ -1,19 +1,24 @@
 {
   "errors": [
     {
-      "message": "Unexpected Execution Error",
+      "message": "The provided value for filter \u0060all\u0060 of type FooNestedFilterInput is invalid. Null values are not supported.",
       "locations": [
         {
-          "line": 2,
-          "column": 25
+          "line": 1,
+          "column": 35
         }
       ],
       "path": [
         "root"
-      ]
+      ],
+      "extensions": {
+        "code": "HC0026",
+        "expectedType": "FooNestedFilterInput!",
+        "filterType": "FooNestedFilterInput"
+      }
     }
   ],
   "data": {
-    "root": null
+    "root": []
   }
-}
+}
diff --git a/...napshots__/QueryableFirstOrDefaultTests.Create_DeepFilterObjectTwoProjections_NET7_0.snap b/...napshots__/QueryableFirstOrDefaultTests.Create_DeepFilterObjectTwoProjections_NET7_0.snap
@@ -1,19 +1,24 @@
 {
   "errors": [
     {
-      "message": "Unexpected Execution Error",
+      "message": "The provided value for filter \u0060all\u0060 of type FooNestedFilterInput is invalid. Null values are not supported.",
       "locations": [
         {
-          "line": 2,
-          "column": 25
+          "line": 1,
+          "column": 35
         }
       ],
       "path": [
         "root"
-      ]
+      ],
+      "extensions": {
+        "code": "HC0026",
+        "expectedType": "FooNestedFilterInput!",
+        "filterType": "FooNestedFilterInput"
+      }
     }
   ],
   "data": {
-    "root": null
+    "root": []
   }
-}
+}
diff --git a/...hots__/QueryableFirstOrDefaultTests.Create_ListObjectDifferentLevelProjection_NET6_0.snap b/...hots__/QueryableFirstOrDefaultTests.Create_ListObjectDifferentLevelProjection_NET6_0.snap
@@ -1,19 +1,24 @@
 {
   "errors": [
     {
-      "message": "Unexpected Execution Error",
+      "message": "The provided value for filter \u0060all\u0060 of type FooNestedFilterInput is invalid. Null values are not supported.",
       "locations": [
         {
-          "line": 2,
-          "column": 25
+          "line": 1,
+          "column": 35
         }
       ],
       "path": [
         "root"
-      ]
+      ],
+      "extensions": {
+        "code": "HC0026",
+        "expectedType": "FooNestedFilterInput!",
+        "filterType": "FooNestedFilterInput"
+      }
     }
   ],
   "data": {
-    "root": null
+    "root": []
   }
-}
+}
diff --git a/...hots__/QueryableFirstOrDefaultTests.Create_ListObjectDifferentLevelProjection_NET7_0.snap b/...hots__/QueryableFirstOrDefaultTests.Create_ListObjectDifferentLevelProjection_NET7_0.snap
@@ -1,19 +1,24 @@
 {
   "errors": [
     {
-      "message": "Unexpected Execution Error",
+      "message": "The provided value for filter \u0060all\u0060 of type FooNestedFilterInput is invalid. Null values are not supported.",
       "locations": [
         {
-          "line": 2,
-          "column": 25
+          "line": 1,
+          "column": 35
         }
       ],
       "path": [
         "root"
-      ]
+      ],
+      "extensions": {
+        "code": "HC0026",
+        "expectedType": "FooNestedFilterInput!",
+        "filterType": "FooNestedFilterInput"
+      }
     }
   ],
   "data": {
-    "root": null
+    "root": []
   }
-}
+}
diff --git a/src/HotChocolate/MongoDb/src/Data/Driver/MongoDbFilterDefinition.cs b/src/HotChocolate/MongoDb/src/Data/Driver/MongoDbFilterDefinition.cs
@@ -7,6 +7,13 @@ namespace HotChocolate.Data.MongoDb;
 
 public abstract class MongoDbFilterDefinition : FilterDefinition<BsonDocument>
 {
+    private static readonly MongoDbFilterDefinition _empty =  new MongoDbEmptyFilterDefinition();
+
+    /// <summary>
+    /// Gets an empty filter. An empty filter matches everything.
+    /// </summary>
+    public static new MongoDbFilterDefinition Empty => MongoDbFilterDefinition._empty;
+
     public abstract BsonDocument Render(
         IBsonSerializer documentSerializer,
         IBsonSerializerRegistry serializerRegistry);
@@ -52,4 +59,14 @@ public override BsonDocument Render(
             return Render(documentSerializer, serializerRegistry);
         }
     }
+
+    internal sealed class MongoDbEmptyFilterDefinition : MongoDbFilterDefinition
+    {
+        public override BsonDocument Render(
+            IBsonSerializer documentSerializer,
+            IBsonSerializerRegistry serializerRegistry)
+        {
+            return new BsonDocument();
+        }
+    }
 }
diff --git a/src/HotChocolate/MongoDb/src/Data/Driver/MongoDbFilterOperation.cs b/src/HotChocolate/MongoDb/src/Data/Driver/MongoDbFilterOperation.cs
@@ -1,4 +1,3 @@
-using System;
 using System.Collections;
 using MongoDB.Bson;
 using MongoDB.Bson.IO;

diff --git a/src/HotChocolate/MongoDb/src/Data/Extensions/MongoDbFilterScopeExtensions.cs b/src/HotChocolate/MongoDb/src/Data/Extensions/MongoDbFilterScopeExtensions.cs
@@ -1,28 +1,17 @@
-using System.Diagnostics.CodeAnalysis;
-using System.Linq;
-using MongoDB.Bson;
-using MongoDB.Driver;
-
 namespace HotChocolate.Data.MongoDb.Filters;
 
 public static class MongoDbFilterScopeExtensions
 {
     public static string GetPath(this MongoDbFilterScope scope) =>
         string.Join(".", scope.Path.Reverse());
 
-    public static bool TryCreateQuery(
-        this MongoDbFilterScope scope,
-        [NotNullWhen(true)] out MongoDbFilterDefinition? query)
+    public static MongoDbFilterDefinition CreateQuery(this MongoDbFilterScope scope)
     {
-        query = null;
-
         if (scope.Level.Peek().Count == 0)
         {
-            return false;
+            return MongoDbFilterDefinition.Empty;
         }
 
-        query = new AndFilterDefinition(scope.Level.Peek().ToArray());
-
-        return true;
+        return new AndFilterDefinition(scope.Level.Peek().ToArray());
     }
 }
diff --git a/src/HotChocolate/MongoDb/src/Data/Extensions/MongoFilterVisitorContextExtensions.cs b/src/HotChocolate/MongoDb/src/Data/Extensions/MongoFilterVisitorContextExtensions.cs
@@ -10,20 +10,16 @@ public static class MongoFilterVisitorContextExtensions
     /// </summary>
     /// <param name="context">The context</param>
     /// <returns>The current scope</returns>
-    public static MongoDbFilterScope GetMongoFilterScope(
-        this MongoDbFilterVisitorContext context) =>
-        (MongoDbFilterScope)context.GetScope();
+    public static MongoDbFilterScope GetMongoFilterScope(this MongoDbFilterVisitorContext context)
+        => (MongoDbFilterScope)context.GetScope();
 
     /// <summary>
     /// Tries to build the query based on the items that are stored on the scope
     /// </summary>
     /// <param name="context">the context</param>
-    /// <param name="query">The query that was build</param>
     /// <returns>True in case the query has been build successfully, otherwise false</returns>
-    public static bool TryCreateQuery(
-        this MongoDbFilterVisitorContext context,
-        [NotNullWhen(true)] out MongoDbFilterDefinition? query)
+    public static MongoDbFilterDefinition CreateQuery(this MongoDbFilterVisitorContext context)
     {
-        return context.GetMongoFilterScope().TryCreateQuery(out query);
+        return context.GetMongoFilterScope().CreateQuery();
     }
 }
diff --git a/src/HotChocolate/MongoDb/src/Data/Filters/Handlers/List/MongoDbListOperationHandlerBase.cs b/src/HotChocolate/MongoDb/src/Data/Filters/Handlers/List/MongoDbListOperationHandlerBase.cs
@@ -1,8 +1,6 @@
-using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using HotChocolate.Configuration;
 using HotChocolate.Data.Filters;
-using HotChocolate.Internal;
 using HotChocolate.Language;
 using HotChocolate.Language.Visitors;
 
@@ -75,8 +73,7 @@ public override bool TryHandleLeave(
     {
         context.RuntimeTypes.Pop();
 
-        if (context.TryCreateQuery(out var query) &&
-            context.Scopes.Pop() is MongoDbFilterScope scope)
+        if (context.Scopes.Pop() is MongoDbFilterScope scope)
         {
             var path = context.GetMongoFilterScope().GetPath();
             var combinedOperations = HandleListOperation(
@@ -113,8 +110,7 @@ protected abstract MongoDbFilterDefinition HandleListOperation(
     /// </summary>
     /// <param name="scope">The scope where the definitions should be combined</param>
     /// <returns>A with and combined filter definition of all definitions of the scope</returns>
-    protected static MongoDbFilterDefinition CombineOperationsOfScope(
-        MongoDbFilterScope scope)
+    protected static MongoDbFilterDefinition CombineOperationsOfScope(MongoDbFilterScope scope)
     {
         var level = scope.Level.Peek();
         if (level.Count == 1)

diff --git a/src/HotChocolate/MongoDb/src/Data/Filters/MongoDbFilterCombinator.cs b/src/HotChocolate/MongoDb/src/Data/Filters/MongoDbFilterCombinator.cs
@@ -1,9 +1,5 @@
-using System;
-using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using HotChocolate.Data.Filters;
-using MongoDB.Bson;
-using MongoDB.Driver;
 
 namespace HotChocolate.Data.MongoDb.Filters;
 
@@ -16,11 +12,12 @@ public override bool TryCombineOperations(
         MongoDbFilterVisitorContext context,
         Queue<MongoDbFilterDefinition> operations,
         FilterCombinator combinator,
-        [NotNullWhen(true)] out MongoDbFilterDefinition combined)
+        [NotNullWhen(true)] out MongoDbFilterDefinition? combined)
     {
         if (operations.Count == 0)
         {
-            throw ThrowHelper.Filtering_MongoDbCombinator_QueueEmpty(this);
+            combined = default;
+            return false;
         }
 
         combined = combinator switch