diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb index 93811b87e..10a963d7e 100644 --- a/app/logical/concerns/searchable.rb +++ b/app/logical/concerns/searchable.rb @@ -201,7 +201,7 @@ module Searchable end def attribute_matches(value, field, type = :integer) - operator, *args = PostQueryBuilder.parse_metatag_value(value, type) + operator, *args = RangeParser.parse(value, type) relation = where_operator(field, operator, *args) # XXX Hack to make negating the equality operator work correctly on nullable columns. @@ -215,7 +215,7 @@ module Searchable end relation - rescue PostQueryBuilder::ParseError + rescue RangeParser::ParseError none end @@ -223,15 +223,24 @@ module Searchable SearchContext.new(all, params, current_user).search_attributes(attributes) end + # Order according to the list of IDs in the given string. + # + # Post.order_custom("1,2,3") => [post #1, post #2, post #3] + def order_custom(string) + operator, ids = RangeParser.parse(string, :integer) + return none unless operator == :in + + in_order_of(:id, ids) + rescue RangeParser::ParseError + none + end + def apply_default_order(params) if params[:order] == "custom" - parse_ids = PostQueryBuilder.parse_range(params[:id], :integer) - if parse_ids[0] == :in - return in_order_of(:id, parse_ids[1]) - end + order_custom(params[:id]) + else + default_order end - - default_order end def default_order diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 61aafd0d2..7a58ac8a2 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -12,8 +12,6 @@ require "strscan" class PostQueryBuilder extend Memoist - class ParseError < StandardError; end - # How many tags a `blah*` search should match. MAX_WILDCARD_TAGS = 100 @@ -269,7 +267,13 @@ class PostQueryBuilder elsif post_query.has_metatag?(:date, :age) && post_query.find_metatag(:order).in?(["id_desc", nil]) relation = search_order(relation, "created_at_desc") elsif post_query.find_metatag(:order) == "custom" - relation = search_order_custom(relation, post_query.select_metatags(:id).map(&:value)) + ids = post_query.select_metatags(:id).map(&:value) + + if ids.size == 1 + relation = relation.order_custom(ids.first) + else + relation = relation.none + end elsif post_query.has_metatag?(:ordfav) # no-op else @@ -491,141 +495,4 @@ class PostQueryBuilder relation end - - def search_order_custom(relation, id_metatags) - return relation.none unless id_metatags.present? && id_metatags.size == 1 - - operator, ids = PostQueryBuilder.parse_range(id_metatags.first, :integer) - return relation.none unless operator == :in - - relation.in_order_of(:id, ids) - end - - concerning :ParseMethods do - class_methods do - # Parse a simple string value into a Ruby type. - # @param string [String] the value to parse - # @param type [Symbol] the value's type - # @return [Object] the parsed value - def parse_cast(string, type) - case type - when :enum - string.downcase - - when :integer - Integer(string) # raises ArgumentError if string is invalid - - when :float - Float(string) # raises ArgumentError if string is invalid - - when :md5 - raise ParseError, "#{string} is not a valid MD5" unless string.match?(/\A[0-9a-fA-F]{32}\z/) - string.downcase - - when :date, :datetime - date = Time.zone.parse(string) - raise ParseError, "#{string} is not a valid date" if date.nil? - date - - when :age - DurationParser.parse(string).ago - - when :interval - DurationParser.parse(string) - - when :ratio - string = string.tr(":", "/") # "2:3" => "2/3" - Rational(string).to_f.round(2) # raises ArgumentError or ZeroDivisionError if string is invalid - - when :filesize - raise ParseError, "#{string} is not a valid filesize" unless string =~ /\A(\d+(?:\.\d*)?|\d*\.\d+)([kKmM]?)[bB]?\Z/ - - size = Float($1) - unit = $2 - - conversion_factor = case unit - when /m/i - 1024 * 1024 - when /k/i - 1024 - else - 1 - end - - (size * conversion_factor).to_i - - else - raise NotImplementedError, "unrecognized type #{type} for #{string}" - end - - rescue ArgumentError, ZeroDivisionError => e - raise ParseError, e.message - end - - def parse_metatag_value(string, type) - if type == :enum - [:in, string.split(/[, ]+/).map { |x| parse_cast(x, type) }] - else - parse_range(string, type) - end - end - - # Parse a metatag range value of the given type. For example: 1..10. - # @param string [String] the metatag value - # @param type [Symbol] the value's type - def parse_range(string, type) - range = case string - when /\A(.+?)\.\.\.(.+)/ # A...B - lo, hi = [parse_cast($1, type), parse_cast($2, type)].sort - [:between, (lo...hi)] - when /\A(.+?)\.\.(.+)/ - lo, hi = [parse_cast($1, type), parse_cast($2, type)].sort - [:between, (lo..hi)] - when /\A<=(.+)/, /\A\.\.(.+)/ - [:lteq, parse_cast($1, type)] - when /\A<(.+)/ - [:lt, parse_cast($1, type)] - when /\A>=(.+)/, /\A(.+)\.\.\Z/ - [:gteq, parse_cast($1, type)] - when /\A>(.+)/ - [:gt, parse_cast($1, type)] - when /[, ]/ - [:in, string.split(/[, ]+/).map {|x| parse_cast(x, type)}] - when "any" - [:not_eq, nil] - when "none" - [:eq, nil] - else - # add a 5% tolerance for float and filesize values - if type == :float || (type == :filesize && string =~ /[km]b?\z/i) - value = parse_cast(string, type) - [:between, (value * 0.95..value * 1.05)] - elsif type.in?([:date, :age]) - value = parse_cast(string, type) - [:between, (value.beginning_of_day..value.end_of_day)] - else - [:eq, parse_cast(string, type)] - end - end - - range = reverse_range(range) if type == :age - range - end - - def reverse_range(range) - case range - in [:lteq, value] - [:gteq, value] - in [:lt, value] - [:gt, value] - in [:gteq, value] - [:lteq, value] - in [:gt, value] - [:lt, value] - else - range - end - end - end - end end diff --git a/app/logical/range_parser.rb b/app/logical/range_parser.rb new file mode 100644 index 000000000..617727d07 --- /dev/null +++ b/app/logical/range_parser.rb @@ -0,0 +1,153 @@ +# frozen_string_literal: true + +# Parse a simple inequality expression into an operator and value. Used for parsing +# metatag values (e.g. `score:>5`) and URL params (e.g. /comments?search[score]=>5). +# +# @example +# +# RangeParser.parse("5") => [:eq, 5] +# RangeParser.parse(">5") => [:gt, 5] +# RangeParser.parse(">=5") => [:gteq, 5] +# RangeParser.parse("<5") => [:lt, 5] +# RangeParser.parse("<=5") => [:lteq, 5] +# RangeParser.parse("5..") => [:lteq, 5] +# RangeParser.parse("..5") => [:gteq, 5] +# RangeParser.parse("5..10") => [:between, (5..10)] +# RangeParser.parse("5...10") => [:between, (5...10)] +# RangeParser.parse("5,6,7") => [:in, [5, 6, 7]] +# RangeParser.parse("any") => [:not_eq, nil] +# RangeParser.parse("none") => [:eq, nil] +# +class RangeParser + class ParseError < StandardError; end + + attr_reader :string, :type + + def self.parse(...) + new(...).parse + end + + # @param string [String] The expression to parse + # @param type [Symbol] The type of the expression (:enum, :integer, :float, :md5, :date, :datetime, :age, :interval, :ratio, or :filesize) + def initialize(string, type = :integer) + @string = string.to_s + @type = type + end + + # Parse a string expression into an operator and value. + # @return [(Symbol, Object)] The operator name and the value + def parse + range = case string + in _ if type == :enum + [:in, string.split(/[, ]+/).map { |x| parse_value(x) }] + in /\A(.+?)\.\.\.(.+)/ # A...B + lo, hi = [parse_value($1), parse_value($2)].sort + [:between, (lo...hi)] + in /\A(.+?)\.\.(.+)/ # A..B + lo, hi = [parse_value($1), parse_value($2)].sort + [:between, (lo..hi)] + in /\A<=(.+)/ | /\A\.\.(.+)/ # <=A, ..A + [:lteq, parse_value($1)] + in /\A<(.+)/ # =(.+)/ | /\A(.+)\.\.\z/ # >=A, A.. + [:gteq, parse_value($1)] + in /\A>(.+)/ # >A + [:gt, parse_value($1)] + in /[, ]/ # A,B,C + [:in, string.split(/[, ]+/).map { |x| parse_value(x) }] + in "any" + [:not_eq, nil] + in "none" + [:eq, nil] + in _ if type == :float + value = parse_value(string) + [:between, (value * 0.95..value * 1.05)] # add a 5% tolerance for float values + in /[km]b?\z/i if type == :filesize + value = parse_value(string) + [:between, (value * 0.95..value * 1.05)] # add a 5% tolerance for filesize values + in _ if type in :date | :age + value = parse_value(string) + [:between, (value.beginning_of_day..value.end_of_day)] + else + [:eq, parse_value(string)] + end + + range = reverse_range(range) if type == :age + range + end + + def reverse_range(range) + case range + in [:lteq, value] + [:gteq, value] + in [:lt, value] + [:gt, value] + in [:gteq, value] + [:lteq, value] + in [:gt, value] + [:lt, value] + else + range + end + end + + # Parse a simple string value into a Ruby type. + # + # @param string [String] the value to parse + # @return [Object] the parsed value + def parse_value(string) + case type + when :enum + string.downcase + + when :integer + Integer(string) # raises ArgumentError if string is invalid + + when :float + Float(string) # raises ArgumentError if string is invalid + + when :md5 + raise ParseError, "#{string} is not a valid MD5" unless string.match?(/\A[0-9a-fA-F]{32}\z/) + string.downcase + + when :date, :datetime + date = Time.zone.parse(string) + raise ParseError, "#{string} is not a valid date" if date.nil? + date + + when :age + DurationParser.parse(string).ago + + when :interval + DurationParser.parse(string) + + when :ratio + string = string.tr(":", "/") # "2:3" => "2/3" + Rational(string).to_f.round(2) # raises ArgumentError or ZeroDivisionError if string is invalid + + when :filesize + raise ParseError, "#{string} is not a valid filesize" unless string =~ /\A(\d+(?:\.\d*)?|\d*\.\d+)([kKmM]?)[bB]?\Z/ + + size = Float($1) + unit = $2 + + conversion_factor = case unit + when /m/i + 1024 * 1024 + when /k/i + 1024 + else + 1 + end + + (size * conversion_factor).to_i + + else + raise NotImplementedError, "unrecognized type #{type} for #{string}" + end + + rescue ArgumentError, ZeroDivisionError => e + raise ParseError, e.message + end +end diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index dfd57632a..6183f067f 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -310,6 +310,30 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest assert_equal(tags.first.id, response.parsed_body.first.fetch("id")) end + should "support ordering by search[order]=custom" do + tags = create_list(:tag, 2, post_count: 42) + get tags_path, params: { search: { id: "#{tags[0].id},#{tags[1].id}", order: "custom" } }, as: :json + + assert_response :success + assert_equal(tags.pluck(:id), response.parsed_body.pluck("id")) + end + + should "return nothing if the search[order]=custom param isn't accompanied by search[id]" do + tags = create_list(:tag, 2, post_count: 42) + get tags_path, params: { search: { order: "custom" } }, as: :json + + assert_response :success + assert_equal(0, response.parsed_body.size) + end + + should "return nothing if the search[order]=custom param isn't accompanied by a valid search[id]" do + tags = create_list(:tag, 2, post_count: 42) + get tags_path, params: { search: { id: ">1", order: "custom" } }, as: :json + + assert_response :success + assert_equal(0, response.parsed_body.size) + end + should "support the expiry parameter" do get posts_path, as: :json, params: { expiry: "1" }