Skip to content

Implement Meilisearch::FilterBuilder class to convert Ruby hashes to filter strings #617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/meilisearch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
require 'meilisearch/task'
require 'meilisearch/client'
require 'meilisearch/index'
require 'meilisearch/filter_builder'

module Meilisearch
end
Expand Down
163 changes: 163 additions & 0 deletions lib/meilisearch/filter_builder.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# frozen_string_literal: true

module Meilisearch
class FilterBuilder
OPERATORS = {
eq: '=',
ne: '!=',
gt: '>',
gte: '>=',
lt: '<',
lte: '<=',
to: 'TO',
exists: 'EXISTS',
in: 'IN',
contains: 'CONTAINS',
starts_with: 'STARTS WITH',
is_empty: 'IS EMPTY',
is_null: 'IS NULL'
}.freeze

LOGICAL_OPERATORS = [:and, :or, :not].freeze

def self.from_hash(hash) = new.build(hash)

def build(filter)
case filter
when Hash then process_hash(filter)
when Array then filter.map { build(_1) }.join(' AND ')
when String, Numeric, TrueClass, FalseClass then format_value(filter)
when nil then 'null'
else raise ArgumentError, "Unsupported filter type: #{filter.class}"
end
end

private

def process_hash(hash)
if logical_operator?(hash)
build_logical_expression(hash)
elsif hash.size == 1
build_single_attribute_expression(hash)
Comment on lines +40 to +41
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
elsif hash.size == 1
build_single_attribute_expression(hash)

I don't think this actually needs to exist. build_single_attribute_expression can probably be inlined inside of build_multi_atttribute_expression.

else
build_multi_attribute_expression(hash)
end
end

def logical_operator?(hash) = hash.keys.any? { |k| LOGICAL_OPERATORS.include?(k.to_sym) }

def build_logical_expression(hash)
logical_op = hash.keys.find { |k| LOGICAL_OPERATORS.include?(k.to_sym) }
Comment on lines +47 to +50
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but you could find and any are not very different. Instead of having logical_operator? you could simply return if logical_op.nil? in build_logical_expression.

op_sym = logical_op.to_sym

case op_sym
when :and then build_and_expression(hash, logical_op)
when :or then build_or_expression(hash, logical_op)
when :not then build_not_expression(hash, logical_op)
else raise ArgumentError, "Unknown logical operator: #{logical_op}"
end
end

def build_and_expression(hash, logical_op)
conditions = get_conditions_array(hash, logical_op)
conditions.map { |c| wrap_complex_condition(build(c)) }.join(' AND ')
end

def build_or_expression(hash, logical_op)
conditions = get_conditions_array(hash, logical_op)
conditions.map { |c| wrap_complex_condition(build(c)) }.join(' OR ')
end

def build_not_expression(hash, logical_op) = "NOT (#{build(hash[logical_op] || hash[logical_op.to_s])})"

def get_conditions_array(hash, logical_op) = Array(hash[logical_op] || hash[logical_op.to_s])
Comment on lines +71 to +73
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see in the code, there cannot be a case when logical_op is not the same type as the key in the hash, since it comes from this expression:

      logical_op = hash.keys.find { |k| LOGICAL_OPERATORS.include?(k.to_sym) }

It will always be the correct type, and there is no need to convert it to a String.

Let me know if I misunderstood something.


def build_single_attribute_expression(hash)
attribute, conditions = hash.first

if conditions.is_a?(Hash)
process_attribute_conditions(attribute, conditions)
elsif conditions.nil?
"#{attribute} IS NULL"
elsif conditions.is_a?(Array)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a nitpick:

I would not consider this behavior to be consistent, since get_conditions_array works with array-like structs due to the call to Array().

It would not make sense to call Array() here but it might make sense to check if conditions responds to to_ary.

"#{attribute} IN #{format_value(conditions)}"
else
"#{attribute} = #{format_value(conditions)}"
end
end

def build_multi_attribute_expression(hash)
hash.map do |attribute, value|
build_single_attribute_expression({ attribute => value })
end.join(' AND ')
end

def process_attribute_conditions(attribute, conditions)
if conditions.is_a?(Hash)
expressions = build_operator_expressions(attribute, conditions)

if expressions.size > 1
expressions.map { |e| wrap_complex_condition(e) }.join(' AND ')
else
expressions.first
end
else
"#{attribute} = #{format_value(conditions)}"
end
end

def build_operator_expressions(attribute, conditions)
conditions.map do |operator, value|
operator = operator.to_sym

raise ArgumentError, "Unknown operator: #{operator}" unless OPERATORS.key?(operator)

build_operator_expression(attribute, operator, value)
end
end

def build_operator_expression(attribute, operator, value) # rubocop:disable Metrics/CyclomaticComplexity
case operator
when :exists then value ? "#{attribute} EXISTS" : "#{attribute} NOT EXISTS"
when :in then "#{attribute} IN [#{format_array_values(value)}]"
when :to then "#{attribute} #{format_value(value.first)} TO #{format_value(value.last)}"
when :is_empty then value ? "#{attribute} IS EMPTY" : "#{attribute} IS NOT EMPTY"
when :is_null then value ? "#{attribute} IS NULL" : "#{attribute} IS NOT NULL"
when :contains then "#{attribute} CONTAINS #{format_value(value)}"
when :starts_with then "#{attribute} STARTS WITH #{format_value(value)}"
else "#{attribute} #{OPERATORS[operator]} #{format_value(value)}"
end
end

def format_array_values(values) = Array(values).map { |v| format_value(v) }.join(', ')

def format_value(value)
case value
when String then format_string_value(value)
when Array then format_array_value(value)
when true then 'true'
when false then 'false'
when nil then 'null'
else value.to_s
end
end

def format_string_value(string) = needs_quoting?(string) ? "'#{string.gsub("'", "\\'")}'" : string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def format_string_value(string) = needs_quoting?(string) ? "'#{string.gsub("'", "\\'")}'" : string
def format_string_value(string) = needs_quoting?(string) ? "'#{string.gsub("'", "\\\\'")}'" : string

This does not have the output that you would expect. The replacement string is correctly \', but that is a special sequence in gsub, which substitutes in the value of the $' global variable, which is a regexp global variable that is string after the last match.

Try this in IRB:

irb(main):001> s = "puts 'hello world'"
=> "puts 'hello world'"
irb(main):002> s.gsub("'", "\\'")
=> "puts hello world'hello world"
irb(main):003> s.gsub("'", "\\\\'")
=> "puts \\'hello world\\'"

Documentation:

gsub documentation
regex global variables


def needs_quoting?(string)
string.include?(' ') ||
OPERATORS.value?(string.upcase) ||
LOGICAL_OPERATORS.map { |x| x.to_s.upcase }.include?(string.upcase)
end

def format_array_value(array) = "[#{array.map { |v| format_value(v) }.join(', ')}]"

def wrap_complex_condition(condition) = complex_condition?(condition) ? "(#{condition})" : condition

def complex_condition?(condition)
condition.include?(' AND ') ||
condition.include?(' OR ') ||
condition.match?(/\s(IN|CONTAINS|STARTS WITH|IS EMPTY|IS NULL|NOT EXISTS|IS NOT EMPTY|IS NOT NULL)\s/)
end
end
end
Loading