Merge pull request #52 from block/myron/use-filter-node-interpreter/s…

…tep1

Reorder `filter_node_interpreter` to check `:empty` case first.

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

@@ -137,8 +137,6 @@ def process_not_expression(bool_node, expression, field_path)
         # this because we do not generate `any_satisfy` filters on `object` list fields (instead,
         # they get generated on their leaf fields).
         def process_list_any_filter_expression(bool_node, filter, field_path)
-          return if filter.nil? || filter == {}
-
           if filters_on_sub_fields?(filter)
             process_any_satisfy_filter_expression_on_nested_object_list(bool_node, filter, field_path)
           else
@@ -195,8 +193,6 @@ def process_any_satisfy_filter_expression_on_scalar_list(bool_node, filter, fiel
         # * `filter: {anyOf: [{field: null}]}` -> return all results
         # * `filter: {anyOf: [{field: null}, {field: ...}]}` -> return all results
         def process_any_of_expression(bool_node, expressions, field_path)
-          return if expressions.nil? || expressions == {}
-
           if expressions.empty?
             # When our `expressions` array is empty, we want to match no documents. However, that's
             # not the behavior the datastore will give us if we have an empty array in the query under
@@ -218,8 +214,6 @@ def process_any_of_expression(bool_node, expressions, field_path)
         end
 
         def process_all_of_expression(bool_node, expressions, field_path)
-          return if expressions.nil? || expressions == {}
-
           # `all_of` represents an AND. AND is the default way that `process_filter_hash` combines
           # filters so we just have to call it for each sub-expression.
           expressions.each do |sub_expression|
@@ -228,8 +222,6 @@ def process_all_of_expression(bool_node, expressions, field_path)
         end
 
         def process_operator_expression(bool_node, operator, expression, field_path)
-          return if expression.nil? || expression == {}
-
           # `operator` is a filtering operator, and `expression` is the value the filtering
           # operator should be applied to. The `op_applicator` lambda, when called, will
           # return a Clause instance (defined in this module).
@@ -317,8 +309,6 @@ def process_sub_field_expression(bool_node, expression, field_path)
         end
 
         def process_list_count_expression(bool_node, expression, field_path)
-          return if expression.nil? || expression == {}
-
           # Normally, we don't have to do anything special for list count expressions.
           # That's the case, for example, for an expression like:
           #

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

@@ -25,33 +25,30 @@ def initialize(runtime_metadata:)
         end
 
         def identify_node_type(field_or_op, sub_expression)
-          identify_by_field_or_op(field_or_op) || identify_by_sub_expr(sub_expression) || :unknown
-        end
-
-        def filter_operators
-          @filter_operators ||= build_filter_operators(runtime_metadata)
-        end
+          # `:not` must go before `:empty`, because `not: empty_filter` must be inverted from just  `empty_filter`.
+          return :not if field_or_op == schema_names.not
 
-        private
+          # The `:empty` check can go before all other checks; besides `not`, none of the other operators require special
+          # handling when the filter is empty, and we want to detect this as early as possible.
+          # Note: `any_of: [empty_filter]` does have special handling, but `any_of: empty_filter` does not.
+          return :empty if sub_expression.nil? || sub_expression == {}
 
-        def identify_by_field_or_op(field_or_op)
-          return :not if field_or_op == schema_names.not
           return :list_any_filter if field_or_op == schema_names.any_satisfy
           return :all_of if field_or_op == schema_names.all_of
           return :any_of if field_or_op == schema_names.any_of
           return :operator if filter_operators.key?(field_or_op)
           return :list_count if field_or_op == LIST_COUNTS_FIELD
+          return :sub_field if sub_expression.is_a?(::Hash)
 
-          nil
+          :unknown
         end
 
-        def identify_by_sub_expr(sub_expression)
-          return :empty if sub_expression.nil? || sub_expression == {}
-          return :sub_field if sub_expression.is_a?(::Hash)
-
-          nil
+        def filter_operators
+          @filter_operators ||= build_filter_operators(runtime_metadata)
         end
 
+        private
+
         def build_filter_operators(runtime_metadata)
           filter_by_time_of_day_script_id = runtime_metadata
             .static_script_ids_by_scoped_name

elasticgraph-graphql/sig/elastic_graph/graphql/filtering/filter_interpreter.rbs

@@ -32,14 +32,14 @@ module ElasticGraph
         def process_filter_hash: (stringOrSymbolHash, stringHash, FieldPath) -> void
         def filters_on_sub_fields?: (stringHash) -> bool
         def process_not_expression: (stringOrSymbolHash, stringHash?, FieldPath) -> void
-        def process_list_any_filter_expression: (stringOrSymbolHash, stringHash?, FieldPath) -> void
+        def process_list_any_filter_expression: (stringOrSymbolHash, stringHash, FieldPath) -> void
         def process_any_satisfy_filter_expression_on_nested_object_list: (stringOrSymbolHash, stringHash, FieldPath) -> void
         def process_any_satisfy_filter_expression_on_scalar_list: (stringOrSymbolHash, stringHash, FieldPath) -> void
-        def process_any_of_expression: (stringOrSymbolHash, ::Array[stringHash]?, FieldPath) -> void
-        def process_all_of_expression: (stringOrSymbolHash, ::Array[stringHash]?, FieldPath) -> void
-        def process_operator_expression: (stringOrSymbolHash, ::String, stringHash?, FieldPath) -> void
+        def process_any_of_expression: (stringOrSymbolHash, ::Array[stringHash], FieldPath) -> void
+        def process_all_of_expression: (stringOrSymbolHash, ::Array[stringHash], FieldPath) -> void
+        def process_operator_expression: (stringOrSymbolHash, ::String, stringHash, FieldPath) -> void
         def process_sub_field_expression: (stringOrSymbolHash, stringHash, FieldPath) -> void
-        def process_list_count_expression: (stringOrSymbolHash, stringHash?, FieldPath) -> void
+        def process_list_count_expression: (stringOrSymbolHash, stringHash, FieldPath) -> void
         def build_bool_hash: () { (stringOrSymbolHash) -> void } -> stringOrSymbolHash?
         def filters_to_range_including_zero?: (stringHash) -> bool
         def operator_excludes_zero?: (::String, untyped) -> bool

elasticgraph-graphql/sig/elastic_graph/graphql/filtering/filter_node_interpreter.rbs

@@ -29,8 +29,6 @@ module ElasticGraph
         ) -> void
 
         def identify_node_type: (::String, stringHash) -> nodeType
-        def identify_by_field_or_op: (::String) -> nodeType?
-        def identify_by_sub_expr: (stringHash) -> nodeType?
 
         attr_reader filter_operators: ::Hash[::String, ^(::String, untyped) -> queryClause?]