Fixes #23644 - Fix > >= < <= version/release searches

app/lib/katello/util/package.rb

@@ -3,12 +3,22 @@ module Util
     module Package
       SUFFIX_RE = /\.(rpm)$/
       ARCH_RE = /\.([^.\-]*)$/
-      EPOCH_RE = /([0-9]+):/
-      NVRE_RE = /^(?:([0-9]+):)?(.*)-([^-]*)-([^-]*)$/
+      NVRE_RE = /^(.*)-(?:([0-9]+):)?([^-]*)-([^-]*)$/
+      EVR_RE = /^(?:([0-9]+):)?(.*?)(?:-([^-]*))?$/
       SUPPORTED_ARCHS = %w(noarch i386 i686 ppc64 s390x x86_64 ia64).freeze
 
+      # is able to take both nvre and nvrea and parse it correctly
+      def self.parse_nvrea_nvre(name)
+        package = self.parse_nvrea(name)
+        if package && SUPPORTED_ARCHS.include?(package[:arch])
+          return package
+        else
+          return self.parse_nvre(name)
+        end
+      end
+
       #parses package nvrea and stores it in a hash
-      #epoch:name-ve.rs.ion-rel.e.ase.arch.rpm
+      #name-epoch:ve.rs.ion-rel.e.ase.arch.rpm
       def self.parse_nvrea(name)
         name, suffix = extract_suffix(name)
         name, arch = extract_arch(name)
@@ -20,26 +30,24 @@ def self.parse_nvrea(name)
       end
 
       #parses package nvre and stores it in a hash
-      #epoch:name-ve.rs.ion-rel.e.ase.rpm
+      #name-epoch:ve.rs.ion-rel.e.ase.rpm
       def self.parse_nvre(name)
         name, suffix = extract_suffix(name)
 
         if (match = NVRE_RE.match(name))
           {:suffix => suffix,
-           :epoch => match[1],
-           :name => match[2],
+           :name => match[1],
+           :epoch => match[2],
            :version => match[3],
            :release => match[4]}.delete_if { |_k, v| v.nil? }
         end
       end
 
-      # is able to take both nvre and nvrea and parse it correctly
-      def self.parse_nvrea_nvre(name)
-        package = self.parse_nvrea(name)
-        if package && SUPPORTED_ARCHS.include?(package[:arch])
-          return package
-        else
-          return self.parse_nvre(name)
+      def self.parse_evr(evr)
+        if (match = EVR_RE.match(evr))
+          {:epoch => match[1],
+           :version => match[2],
+           :release => match[3]}.delete_if { |_k, v| v.nil? }
         end
       end
 

app/lib/katello/util/package_filter.rb

@@ -15,53 +15,55 @@ def initialize(relation, evr, operator = nil)
       end
 
       def extract_epoch_version_release(evr)
-        match = case evr
-                when /\A(\d+):(.*)-(.*)\z/
-                  evr.match(/\A(?<epoch>\d+):(?<version>.*)-(?<release>.*)\z/)
-                when /\A(\d+):(.*)\z/
-                  evr.match(/\A(?<epoch>\d+):(?<version>.*)\z/)
-                when /\A(.*)-(.*)\z/
-                  evr.match(/\A(?<version>.*)-(?<release>.*)\z/)
-                else
-                  evr.match(/\A(?<version>.*)\z/)
-                end
-        self.version = Package.sortable_version(match[:version])
-        self.epoch = match[:epoch] rescue nil
-        self.release = (match[:release] rescue nil) ? Package.sortable_version(match[:release]) : nil
+        parsed = Package.parse_evr(evr)
+        self.epoch = parsed[:epoch].to_i # nil or blank becomes 0
+        v = parsed[:version]
+        self.version = (v.nil? || v.blank?) ? '' : Package.sortable_version(v)
+        r = parsed[:release]
+        self.release = (r.nil? || r.blank?) ? '' : Package.sortable_version(r)
       end
 
       def results
-        version_clause = "#{convert(:version_sortable)} #{operator} #{convert(':version')}"
-        version_clause = add_release_clause(version_clause) unless release.blank?
-        version_clause = add_epoch_clause(version_clause) unless epoch.blank?
+        if operator == EQUAL
+          conditions = epoch_clause(operator)
+          conditions += ' AND ' + version_clause(operator) unless version.blank?
+          conditions += ' AND ' + release_clause(operator) unless release.blank?
+        else
+          conditions = ''
+          unless version.blank?
+            unless release.blank?
+              conditions = " OR (#{version_clause('=')} AND #{release_clause(operator)})"
+            end
+            conditions = " OR (#{epoch_clause('=')} AND (#{version_clause(operator)}#{conditions}))"
+          end
+          conditions = "#{epoch_clause(operator)}#{conditions}"
+        end
 
-        self.relation.where(version_clause, :version => version, :release => release, :epoch => epoch)
+        self.relation.where(conditions, :version => version, :release => release, :epoch => epoch)
       end
 
-      def add_release_clause(version_clause)
-        clause = "(#{convert(:version_sortable)} = #{convert(':version')} AND #{convert(:release_sortable)} #{operator} #{convert(':release')})"
+      def epoch_clause(op)
+        "CAST(epoch AS INT) #{op} :epoch"
+      end
 
-        # if we're using EQUAL, match: version = X AND release = Y
-        # else if we're using something like greater than, we need:
-        #   (version > X) OR (version = X AND release > Y)
-        if operator == EQUAL
-          clause
+      def version_clause(op)
+        if op == '='
+          "version_sortable = :version"
         else
-          "#{version_clause} OR #{clause}"
+          "#{convert(:version_sortable)} #{op} #{convert(':version')}"
         end
       end
 
-      def add_epoch_clause(version_clause)
-        if operator == EQUAL
-          clause = "(epoch = :epoch AND (%s))"
+      def release_clause(op)
+        if op == '='
+          "release_sortable = :release"
         else
-          clause = "epoch #{operator} :epoch OR (epoch = :epoch AND (%s))"
+          "#{convert(:release_sortable)} #{op} #{convert(':release')}"
         end
-        clause % version_clause
       end
 
       def convert(name = '?')
-        "convert_to(#{name}, 'SQL_ASCII')"
+        "#{name} COLLATE \"C\""
       end
     end
   end

app/models/katello/rpm.rb

@@ -12,10 +12,11 @@ class Rpm < Katello::Model
     has_many :content_facets, :through => :content_facet_applicable_rpms, :class_name => "Katello::Host::ContentFacet"
 
     scoped_search :on => :name, :complete_value => true
-    scoped_search :on => :version, :complete_value => true
-    scoped_search :on => :release, :complete_value => true
+    scoped_search :on => :version, :complete_value => true, :ext_method => :scoped_search_version
+    scoped_search :on => :release, :complete_value => true, :ext_method => :scoped_search_release
     scoped_search :on => :arch, :complete_value => true
     scoped_search :on => :epoch, :complete_value => true
+    scoped_search :on => :evr, :ext_method => :scoped_search_evr
     scoped_search :on => :filename, :complete_value => true
     scoped_search :on => :sourcerpm, :complete_value => true
     scoped_search :on => :checksum
@@ -30,6 +31,147 @@ def self.repository_association_class
       RepositoryRpm
     end
 
+    def self.scoped_search_version(_key, operator, value)
+      self.scoped_search_sortable('version', operator, value)
+    end
+
+    def self.scoped_search_release(_key, operator, value)
+      self.scoped_search_sortable('release', operator, value)
+    end
+
+    def self.scoped_search_sortable(column, operator, value)
+      if ['>', '>=', '<', '<='].include?(operator)
+        conditions = "#{self.table_name}.#{column}_sortable COLLATE \"C\" #{operator} ? COLLATE \"C\""
+        parameter = Util::Package.sortable_version(value)
+        return { :conditions => conditions, :parameter => [parameter] }
+      end
+
+      # Use the default behavior for all other operators.
+      # Unfortunately there is no way to call the default behavior from here, so
+      # we replicate the default behavior from sql_test() in
+      # https://github.com/wvanbergen/scoped_search/blob/master/lib/scoped_search/query_builder.rb
+      if ['LIKE', 'NOT LIKE'].include?(operator)
+        conditions = "#{self.table_name}.#{column} #{operator} ?"
+        parameter = (value !~ /^\%|\*/ && value !~ /\%|\*$/) ? "%#{value}%" : value.tr_s('%*', '%')
+        return { :conditions => conditions, :parameter => [parameter] }
+      elsif ['IN', 'NOT IN'].include?(operator)
+        conditions = "#{self.table_name}.#{column} #{operator} (#{value.split(',').collect { '?' }.join(',')})"
+        parameters = value.split(',').collect { |v| v.strip }
+        return { :conditions => conditions, :parameter => parameters }
+      else
+        conditions = "#{self.table_name}.#{column} #{operator} ?"
+        parameter = value
+        return { :conditions => conditions, :parameter => [parameter] }
+      end
+    end
+
+    def self.scoped_search_evr(_key, operator, value)
+      if ['=', '<>'].include?(operator)
+        return self.scoped_search_evr_equal(operator, value)
+      elsif ['IN', 'NOT IN'].include?(operator)
+        return self.scoped_search_evr_in(operator, value)
+      elsif ['LIKE', 'NOT LIKE'].include?(operator)
+        return self.scoped_search_evr_like(operator, value)
+      elsif ['>', '>=', '<', '<='].include?(operator)
+        return self.scoped_search_evr_compare(operator, value)
+      else
+        return {}
+      end
+    end
+
+    def self.scoped_search_evr_equal(operator, value)
+      joiner = (operator == '=' ? 'AND' : 'OR')
+      evr = Util::Package.parse_evr(value)
+      (e, v, r) = [evr[:epoch], evr[:version], evr[:release]]
+      e = e.to_i # nil or blank becomes 0
+      conditions = "CAST(#{self.table_name}.epoch AS INT) #{operator} ?"
+      parameters = [e]
+      unless v.nil? || v.blank?
+        conditions += " #{joiner} #{self.table_name}.version #{operator} ?"
+        parameters += [v]
+      end
+      unless r.nil? || r.blank?
+        conditions += " #{joiner} #{self.table_name}.release #{operator} ?"
+        parameters += [r]
+      end
+      return { :conditions => conditions, :parameter => parameters }
+    end
+
+    def self.scoped_search_evr_in(operator, value)
+      op = (operator == 'IN' ? '=' : '<>')
+      joiner1 = (operator == 'IN' ? 'AND' : 'OR')
+      joiner2 = (operator == 'IN' ? 'OR' : 'AND')
+      conditions = []
+      parameters = []
+      value.split(',').collect { |v| v.strip }.each do |val|
+        evr = Util::Package.parse_evr(val)
+        (e, v, r) = [evr[:epoch], evr[:version], evr[:release]]
+        e = e.to_i # nil or blank becomes 0
+        condition = "CAST(#{self.table_name}.epoch AS INT) #{op} ?"
+        parameters += [e]
+        unless v.nil? || v.blank?
+          condition += " #{joiner1} #{self.table_name}.version #{op} ?"
+          parameters += [v]
+        end
+        unless r.nil? || r.blank?
+          condition += " #{joiner1} #{self.table_name}.release #{op} ?"
+          parameters += [r]
+        end
+        conditions += ["(#{condition})"]
+      end
+      return { :conditions => conditions.join(" #{joiner2} "), :parameter => parameters }
+    end
+
+    def self.scoped_search_evr_like(operator, value)
+      val = (value !~ /^\%|\*/ && value !~ /\%|\*$/) ? "%#{value}%" : value.tr_s('%*', '%')
+      evr = Util::Package.parse_evr(val)
+      (e, v, r) = [evr[:epoch], evr[:version], evr[:release]]
+      conditions = []
+      parameters = []
+      unless e.nil? || e.blank?
+        conditions += ["CAST(#{self.table_name}.epoch AS VARCHAR(10)) #{operator} ?"]
+        parameters += [e]
+      end
+      unless v.nil? || v.blank?
+        conditions += ["#{self.table_name}.version #{operator} ?"]
+        parameters += [v]
+      end
+      unless r.nil? || r.blank?
+        conditions += ["#{self.table_name}.release #{operator} ?"]
+        parameters += [r]
+      end
+      return {} if conditions.empty?
+      joiner = (operator == 'LIKE' ? 'AND' : 'OR')
+      return { :conditions => conditions.join(" #{joiner} "), :parameter => parameters }
+    end
+
+    def self.scoped_search_evr_compare(operator, value)
+      evr = Util::Package.parse_evr(value)
+      (e, v, r) = [evr[:epoch], evr[:version], evr[:release]]
+      e = e.to_i # nil or blank becomes 0
+      conditions = ''
+      if v.nil? || v.blank?
+        conditions = "CAST(#{self.table_name}.epoch AS INT) #{operator} ?"
+        parameters = [e]
+      else
+        sv = Util::Package.sortable_version(v)
+        if r.nil? || r.blank?
+          conditions = "#{self.table_name}.version_sortable COLLATE \"C\" #{operator} ? COLLATE \"C\""
+          parameters = [sv]
+        else
+          conditions =
+            "#{self.table_name}.version_sortable COLLATE \"C\" #{operator[0]} ? COLLATE \"C\" OR " \
+            "(#{self.table_name}.version_sortable = ? AND " \
+             "#{self.table_name}.release_sortable COLLATE \"C\" #{operator} ? COLLATE \"C\")"
+          sv = Util::Package.sortable_version(v)
+          parameters = [sv, sv, Util::Package.sortable_version(r)]
+        end
+        conditions = "CAST(#{self.table_name}.epoch AS INT) #{operator[0]} ? OR (CAST(#{self.table_name}.epoch AS INT) = ? AND (#{conditions}))"
+        parameters = [e, e] + parameters
+      end
+      return { :conditions => conditions, :parameter => parameters }
+    end
+
     def self.search_version_range(min = nil, max = nil)
       query = self.all
       query = Katello::Util::PackageFilter.new(query, min, Katello::Util::PackageFilter::GREATER_THAN).results if min.present?

test/fixtures/models/katello_repository_rpms.yml

@@ -6,6 +6,10 @@ fedora_two:
   rpm_id: <%= ActiveRecord::FixtureSet.identify(:two) %>
   repository_id: <%= ActiveRecord::FixtureSet.identify(:fedora_17_x86_64) %>
 
+fedora_three:
+  rpm_id: <%= ActiveRecord::FixtureSet.identify(:three) %>
+  repository_id: <%= ActiveRecord::FixtureSet.identify(:fedora_17_x86_64) %>
+
 fedora_one_two:
   rpm_id: <%= ActiveRecord::FixtureSet.identify(:one_two) %>
   repository_id: <%= ActiveRecord::FixtureSet.identify(:fedora_17_x86_64) %>
@@ -16,4 +20,4 @@ fedora_dev_one:
 
 fedora_dev_two:
   rpm_id: <%= ActiveRecord::FixtureSet.identify(:two) %>
-  repository_id: <%= ActiveRecord::FixtureSet.identify(:fedora_17_x86_64_dev) %>
+  repository_id: <%= ActiveRecord::FixtureSet.identify(:fedora_17_x86_64_dev) %>

test/fixtures/models/katello_rpms.yml

@@ -1,35 +1,47 @@
 one:
   name:     one
   uuid:     one-uuid
+  epoch:    0
   version:  1.0
   release:  1.el7
+  version_sortable: 01-1.01-0
+  release_sortable: 01-1.$el.01-7
   arch:     x86_64
   filename: one-1.1.rpm
   nvra:     one-1.0-1.el7.x86_64
 
 two:
   name:     two
   uuid:     two-uuid
+  epoch:    0
   version:  1.0
   release:  1.el7
+  version_sortable: 01-1.01-0
+  release_sortable: 01-1.$el.01-7
   arch:     x86_64
   filename: two-2.1.rpm
   nvra:     two-1.0-1.el7.x86_64
 
 three:
   name:     three
   uuid:     three-uuid
+  epoch:    0
   version:  99
   release:  108
+  version_sortable: 02-99
+  release_sortable: 03-108
   arch:     x86_64
   filename: three-99-108.rpm
   nvra:     three-99-108.x86_64
 
 one_two:
   name:     one
   uuid:     one-uuid-two
+  epoch:    0
   version:  1.0
   release:  2.el7
+  version_sortable: 01-1.01-0
+  release_sortable: 01-2.$el.01-7
   arch:     x86_64
   filename: one-1.2.rpm
-  nvra:     one-1.0-2.el7.x86_64
+  nvra:     one-1.0-2.el7.x86_64