Add more tests for anyOf

elasticgraph-graphql/lib/elastic_graph/graphql/filtering/filter_interpreter.rb

@@ -44,10 +44,7 @@ def build_query(filter_hashes, from_field_path: FieldPath.empty)
             end
           end
 
-          # ::Kernel.debugger
-          query = reduce_query(query)
-          # ::Kernel.debugger
-          query
+          reduce_query(query)
         end
 
         def to_s
@@ -65,18 +62,12 @@ def to_s
         def reduce_query(bool_node)
           return bool_node if bool_node.nil? || bool_node == {}
 
-          # ::Kernel.debugger
           inner_node = bool_node[:bool]
           nested_node = bool_node[:nested]
           return bool_node if inner_node.nil? && nested_node.nil?
-          # ::Kernel.debugger
 
-          # Using `key?` to check if `inner_node` has any keys, as using `[key]`
-          # results in the specified key getting added with an empty array as value,
-          # due to `build_bool_hash` method.
           # `reduce_must_not` must be called before `reduce_filter`, as the method can add an `expression` to `:filter`.
           if reduce_must(inner_node) && reduce_must_not(inner_node) && reduce_filter(inner_node) && reduce_should(inner_node) && reduce_nested(nested_node)
-            # ::Kernel.debugger
             bool_node = nil
           end
 
@@ -108,8 +99,6 @@ def reduce_filter(inner_node)
                 reduce_query(expression)
               end
             end
-            # ::Kernel.debugger
-            # ::Kernel.debugger
 
             return false unless expressions.empty?
           end
@@ -120,17 +109,16 @@ def reduce_filter(inner_node)
         def reduce_should(inner_node)
           if !inner_node.nil? && inner_node.key?(:should)
             expressions = inner_node[:should]
-            expressions.delete(BooleanQuery::MATCH_NONE_FILTER_HASH)
-            # ::Kernel.debugger
+            if expressions.count(BooleanQuery::MATCH_NONE_FILTER_HASH) != expressions.size
+              expressions.delete(BooleanQuery::MATCH_NONE_FILTER_HASH)
+            end
             if expressions.include?(BooleanQuery::MATCH_ALL_FILTER_HASH)
               expressions.clear
             else
               expressions = expressions.filter_map do |expression|
                 reduce_query(expression)
               end
-              # ::Kernel.debugger
             end
-            # ::Kernel.debugger
 
             return false unless expressions.empty?
           end
@@ -141,31 +129,25 @@ def reduce_should(inner_node)
         def reduce_must_not(inner_node)
           if !inner_node.nil? && inner_node.key?(:must_not)
             expressions = inner_node[:must_not]
-            # ::Kernel.debugger
             expressions = expressions.filter_map do |expression|
               reduce_query(expression)
             end
-            # ::Kernel.debugger
 
             if expressions.empty?
               inner_node[:filter] << BooleanQuery::MATCH_NONE
               inner_node.delete(:must_not)
               return true
             end
-            # ::Kernel.debugger
             return false
           end
 
           true
         end
 
         def reduce_nested(nested_node)
-          # ::Kernel.debugger
           if !nested_node.nil? && nested_node.key?(:query)
             expression = nested_node[:query]
-            # ::Kernel.debugger
             expression = reduce_query(expression)
-            # ::Kernel.debugger
 
             return false unless expression.nil?
           end
@@ -225,7 +207,6 @@ def process_empty_or_nil_expression(bool_node, field_or_op)
           else
             BooleanQuery::MATCH_ALL_FILTER.merge_into(bool_node)
           end
-          # ::Kernel.debugger
         end
 
         def process_not_expression(bool_node, expression, field_path)
@@ -437,15 +418,13 @@ def process_list_count_expression(bool_node, expression, field_path)
             ]}
           end
 
-          # ::Kernel.debugger
           process_sub_field_expression(bool_node, expression, field_path.counts_path)
-          # ::Kernel.debugger
         end
 
         def build_bool_hash(&block)
           bool_node = Hash.new { |h, k| h[k] = [] }.tap(&block)
 
-          # To ignore "empty" filter predicates we need to return `nil` here.
+          # To threat "empty" filter predicates as `true` we need to return `nil` here.
           return nil if bool_node.empty?
 
           # According to https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-bool-query.html#bool-min-should-match,

elasticgraph-graphql/spec/acceptance/search_spec.rb

@@ -310,6 +310,28 @@ module ElasticGraph
           string_hash_of(widget3, :id, :amount_cents)
         ])
 
+        # Test `not: {any_of: []}` results in all matching documents
+        widgets = list_widgets_with(:amount_cents,
+          filter: {not: {any_of: []}})
+
+        expect(widgets).to match([
+          string_hash_of(widget3, :id, :amount_cents),
+          string_hash_of(widget2, :id, :amount_cents),
+          string_hash_of(widget1, :id, :amount_cents)
+        ])
+
+        # Test `not: {any_of: emptyPredicate}` results in no matching documents
+        widgets = list_widgets_with(:amount_cents,
+          filter: {not: {any_of: [{name: {equal_to_any_of: nil}}]}})
+
+        expect(widgets).to eq []
+
+        # Test `not: {any_of: emptyPredicate && <non emptyPredicate>}` results in no matching documents
+        widgets = list_widgets_with(:amount_cents,
+          filter: {not: {any_of: [{name: {equal_to_any_of: nil}}, {amount_cents: {gt: 150}}]}})
+
+        expect(widgets).to eq []
+
         # Test `equal_to_any_of` with `[nil, other_value]`
         widgets = list_widgets_with(:amount_cents,
           filter: {name: {equal_to_any_of: [nil, "thing2", "", " ", "\n"]}}, # empty strings should be ignored.,
@@ -753,7 +775,7 @@ module ElasticGraph
 
           # Verify `all_of: [{not: null}]` works as expected.
           results = query_teams_with(filter: {seasons_nested: {all_of: [{not: nil}]}})
-          # All teams should be returned since the `nil` part of the filter expression is pruned.
+          # No teams should be returned since the `nil` part of the filter expression evaluates to `true`.
           expect(results).to eq []
 
           # Verify `all_of: [{not: null}]` works as expected.

elasticgraph-graphql/spec/integration/elastic_graph/graphql/datastore_query/filtering_spec.rb

@@ -889,7 +889,7 @@ def search_datastore(**options, &before_msearch)
         expect(results).to match_array(ids_of(widget2, widget4))
       end
 
-      specify "`equal_to_any_of: []` or `any_of: []` matches no documents, but `any_predicate: nil` or `field: {}` is ignored, matching all documents" do
+      specify "`equal_to_any_of: []` or `any_of: []` matches no documents, but `any_predicate: nil` or `field: {}` is treated as `true`, matching all documents" do
         index_into(
           graphql,
           widget1 = build(:widget),
@@ -898,6 +898,8 @@ def search_datastore(**options, &before_msearch)
 
         expect(search_with_filter("id", "equal_to_any_of", [])).to eq []
         expect(search_with_filter("id", "any_of", [])).to eq []
+        expect(search_with_freeform_filter({"not" => {"any_of" => []}})).to eq ids_of(widget1, widget2)
+        expect(search_with_freeform_filter({"any_of" => [{"any_of" => []}]})).to eq []
 
         expect(search_with_filter("id", "equal_to_any_of", nil)).to eq ids_of(widget1, widget2)
         expect(search_with_filter("amount_cents", "lt", nil)).to eq ids_of(widget1, widget2)
@@ -909,6 +911,10 @@ def search_datastore(**options, &before_msearch)
         expect(search_with_filter("name_text", "matches_query", nil)).to eq ids_of(widget1, widget2)
         expect(search_with_filter("name_text", "matches_phrase", nil)).to eq ids_of(widget1, widget2)
         expect(search_with_freeform_filter({"id" => {}})).to eq ids_of(widget1, widget2)
+        expect(search_with_freeform_filter({"any_of" => [{"id" => nil}]})).to eq ids_of(widget1, widget2)
+        expect(search_with_freeform_filter({"not" => {"any_of" => [{"id" => nil}]}})).to eq []
+        expect(search_with_freeform_filter({"any_of" => [{"id" => nil}, {"amount_cents" => {"lt" => 1000}}]})).to eq ids_of(widget1, widget2)
+        expect(search_with_freeform_filter({"not" => {"any_of" => [{"id" => nil}, {"amount_cents" => {"lt" => 1000}}]}})).to eq []
       end
 
       it "`equal_to_any_of:` with `nil` matches documents with null values or not null values" do

elasticgraph-graphql/spec/unit/elastic_graph/graphql/datastore_query/filtering_spec.rb

@@ -14,9 +14,8 @@ class GraphQL
     RSpec.describe DatastoreQuery, "filtering" do
       include_context "DatastoreQueryUnitSupport"
 
-      # TODO: RENAME
       let(:always_false_condition) do
-        {bool: {filter: Filtering::BooleanQuery::MATCH_NONE_FILTER.clauses}}
+        {bool: {filter: [Filtering::BooleanQuery::MATCH_NONE]}}
       end
 
       it "builds a `nil` datastore body when given no filters (passed as `nil`)" do
@@ -1405,14 +1404,40 @@ def expect_script_query_with_params(query, expected_params)
           ]})
         end
 
-        it "reduces an `any_of` composed entirely of empty predicates to a false condition" do
+        it "returns the standard always false filter for `any_of: []`" do
+          query = new_query(filter: {
+            "any_of" => []
+          })
+
+          expect(datastore_body_of(query)).to query_datastore_with(always_false_condition)
+        end
+
+        it "returns the standard always false filter for `any_of: [{any_of: []}]`" do
+          query = new_query(filter: {
+            "any_of" => [{"any_of" => []}]
+          })
+
+          expect(datastore_body_of(query)).to query_datastore_with({bool: {minimum_should_match: 1, should: [
+            always_false_condition
+          ]}})
+        end
+
+        it "applies no filtering to an `any_of` composed entirely of empty predicates" do
           query = new_query(filter: {
             "age" => {"any_of" => [{"gt" => nil}, {"lt" => nil}]}
           })
 
           expect(datastore_body_of(query)).to not_filter_datastore_at_all
         end
 
+        it "applies no filtering to an `any_of` composed of empty predicate and non empty predicate" do
+          query = new_query(filter: {
+            "age" => {"any_of" => [{"gt" => nil}, {"lt" => 36}]}
+          })
+
+          expect(datastore_body_of(query)).to not_filter_datastore_at_all
+        end
+
         it "does not filter at all when given only `any_of: nil` on a root field" do
           query = new_query(filter: {
             "age" => {"any_of" => nil}
@@ -1429,6 +1454,30 @@ def expect_script_query_with_params(query, expected_params)
           expect(datastore_body_of(query)).to not_filter_datastore_at_all
         end
 
+        it "filters to a false condition when given `not: {any_of: {age: nil}}` on a root field" do
+          query = new_query(filter: {
+            "not" => {"any_of" => [{"age" => nil}]}
+          })
+
+          expect(datastore_body_of(query)).to query_datastore_with(always_false_condition)
+        end
+
+        it "filters to a false condition when given `not: {any_of: nil}` on a sub field" do
+          query = new_query(filter: {
+            "age" => {"not" => {"any_of" => nil}}
+          })
+
+          expect(datastore_body_of(query)).to query_datastore_with(always_false_condition)
+        end
+
+        it "filters to a true condition when given `not: {any_of: []}` on a sub field" do
+          query = new_query(filter: {
+            "age" => {"not" => {"any_of" => []}}
+          })
+
+          expect(datastore_body_of(query)).to query_datastore_with({bool: {must_not: [always_false_condition]}})
+        end
+
         # Note: the GraphQL schema does not allow `any_of: {}` (`any_of` is a list field). However, we're testing
         # it here for completeness--as a defense-in-depth measure, it's good for the filter interpreter to handle
         # whatever is thrown at it. Including these tests allows us to exercise an edge case in the code that

elasticgraph-graphql/spec/unit/elastic_graph/graphql/datastore_query/search_index_expression_spec.rb

@@ -369,6 +369,24 @@ class GraphQL
 
               expect(parts).to target_all_widget_indices
             end
+
+            it "excludes no indices when we have an `any_of: [{anyof: []}]` filter because that will match all results" do
+              parts = search_index_expression_parts_for({"any_of" => [{"any_of" => []}]})
+
+              expect(parts).to target_all_widget_indices
+            end
+
+            it "excludes no indices when we have an `any_of: [{field: nil}]` filter because that will match all results" do
+              parts = search_index_expression_parts_for({"any_of" => [{"created_at" => nil}]})
+
+              expect(parts).to target_all_widget_indices
+            end
+
+            it "excludes no indices when we have an `any_of: [{field: nil}, {...}]` filter because that will match all results" do
+              parts = search_index_expression_parts_for({"any_of" => [{"created_at" => nil}, {"id" => {"equal_to_any_of" => "some-id"}}]})
+
+              expect(parts).to target_all_widget_indices
+            end
           end
 
           context "for a query that includes aggregations" do

elasticgraph-graphql/spec/unit/elastic_graph/graphql/datastore_query/shard_routing_spec.rb

@@ -259,6 +259,24 @@ def shard_routing_for(route_with_field_paths, filter_or_filters)
         })).to search_all_shards
       end
 
+      it "searches all shards when we have an `any_of: [{anyof: []}]` filter because that will match all results" do
+        expect(shard_routing_for(["name"], {
+          "any_of" => [{"any_of" => []}]
+        })).to search_all_shards
+      end
+
+      it "searches all shards when we have an `any_of: [{field: nil}]` filter because that will match all results" do
+        expect(shard_routing_for(["name"], {
+          "any_of" => [{"name" => nil}]
+        })).to search_all_shards
+      end
+
+      it "searches all shards when we have an `any_of: [{field: nil}, {...}]` filter because that will match all results" do
+        expect(shard_routing_for(["name"], {
+          "any_of" => [{"name" => nil}, {"id" => {"equal_to_any_of" => ["abc"]}}]
+        })).to search_all_shards
+      end
+
       describe "not" do
         it "searches all shards when there are values in an `equal_to_any_of` filter" do
           expect(shard_routing_for(["name"],