Skip to content

Commit

Permalink
Interpret field_or_op before sub_expression
Browse files Browse the repository at this point in the history
  • Loading branch information
acerbusace committed Nov 4, 2024
1 parent 651fc20 commit 9af11b3
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ def filters_on_sub_fields?(expression)
end

def process_not_expression(bool_node, expression, field_path)
return if expression.nil? || expression == {}

sub_filter = build_bool_hash do |inner_node|
process_filter_hash(inner_node, expression, field_path)
end
Expand Down Expand Up @@ -133,6 +135,8 @@ 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
Expand Down Expand Up @@ -184,6 +188,8 @@ def process_any_satisfy_filter_expression_on_scalar_list(bool_node, filter, fiel
end

def process_any_of_expression(bool_node, expressions, field_path)
return if expressions.nil? || expressions == {}

shoulds = expressions.filter_map do |expression|
build_bool_hash do |inner_bool_node|
process_filter_hash(inner_bool_node, expression, field_path)
Expand All @@ -199,6 +205,8 @@ 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|
Expand All @@ -207,6 +215,8 @@ 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).
Expand Down Expand Up @@ -294,6 +304,8 @@ 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:
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,29 @@ def initialize(runtime_metadata:)
end

def identify_node_type(field_or_op, sub_expression)
return :empty if sub_expression.nil? || 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

private

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)
:unknown
end

def filter_operators
@filter_operators ||= build_filter_operators(runtime_metadata)
def identify_by_sub_expr(sub_expression)
return :empty if sub_expression.nil? || sub_expression == {}
return :sub_field if sub_expression.is_a?(::Hash)
end

private

def build_filter_operators(runtime_metadata)
filter_by_time_of_day_script_id = runtime_metadata
.static_script_ids_by_scoped_name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ 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?]

Expand All @@ -42,4 +44,4 @@ module ElasticGraph
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,13 @@ class GraphQL
})
end

it "ignores `count` filter predicates that have a `nil` value" do
it "ignores `count` filter predicates that have a `nil` or `{}` value" do
query = new_query(filter: {"past_names" => {LIST_COUNTS_FIELD => nil}})
expect(datastore_body_of(query)).to not_filter_datastore_at_all

query = new_query(filter: {"past_names" => {LIST_COUNTS_FIELD => {}}})
expect(datastore_body_of(query)).to not_filter_datastore_at_all

query = new_query(filter: {"past_names" => {LIST_COUNTS_FIELD => {"gt" => nil}}})
expect(datastore_body_of(query)).to not_filter_datastore_at_all

Expand Down

0 comments on commit 9af11b3

Please sign in to comment.