GemVersionPromoter refactor

UpdateOptions which was then renamed to DependencySearch is now called
GemVersionPromoter, cuz I can't name this damn class. It's in its own
file now, so there's that.

I took a shot at moving Resolver#search_for into it, but had naively
overlooked a few instance variables and such and it just didn't make as
much sense as I'd first envisioned. Probably some other smaller classes
in between perhaps.

GemVersionPromoter class now caching its results, too, and I moved out
the return from it back into Resolver as it made more sense there. As a
standalone class, it may make sense to have this actually implement
:major sorting, but maybe later.

4 files changed, 136 insertions, 134 deletions

diff --git a/lib/bundler/cli/update.rb b/lib/bundler/cli/update.rb
index fec5a543..cdf1dc10 100644
--- a/lib/bundler/cli/update.rb
+++ b/lib/bundler/cli/update.rb
@@ -40,7 +40,7 @@ module Bundler
       end
 
       patch_level = [:major, :minor, :patch].detect {|v| options.keys.include?(v.to_s) }
-      Bundler.definition.dependency_search.level = patch_level
+      Bundler.definition.gem_version_promoter.level = patch_level
 
       Bundler::Fetcher.disable_endpoint = options["full-index"]
 
diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index c1d4d18a..73e1c009 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -7,7 +7,7 @@ module Bundler
   class Definition
     include GemHelpers
 
-    attr_reader :dependencies, :platforms, :ruby_version, :locked_deps, :dependency_search
+    attr_reader :dependencies, :platforms, :ruby_version, :locked_deps, :gem_version_promoter
 
     # Given a gemfile and lockfile creates a Bundler definition
     #
@@ -94,7 +94,7 @@ module Bundler
       end
       @unlocking ||= @unlock[:ruby] ||= (!@locked_ruby_version ^ !@ruby_version)
 
-      @dependency_search = Resolver::DependencySearch.new(@locked_specs, @unlock[:gems])
+      @gem_version_promoter = GemVersionPromoter.new(@locked_specs, @unlock[:gems])
 
       current_platform = Bundler.rubygems.platforms.map {|p| generic(p) }.compact.last
       add_platform(current_platform)
@@ -223,7 +223,7 @@ module Bundler
         else
           # Run a resolve against the locally available gems
           Bundler.ui.debug("Found changes from the lockfile, re-resolving dependencies because #{change_reason}")
-          last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, ruby_version, dependency_search)
+          last_resolve.merge Resolver.resolve(expanded_dependencies, index, source_requirements, last_resolve, ruby_version, gem_version_promoter)
         end
       end
     end
diff --git a/lib/bundler/gem_version_promoter.rb b/lib/bundler/gem_version_promoter.rb
new file mode 100644
index 00000000..19de170a
--- /dev/null
+++ b/lib/bundler/gem_version_promoter.rb
@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+module Bundler
+  class GemVersionPromoter
+    attr_accessor :level, :strict, :minimal
+
+    def initialize(locked_specs = SpecSet.new([]), unlock_gems = [])
+      @level_default = :major
+      @level = @level_default
+      @strict = false
+      @minimal = false
+      @locked_specs = locked_specs
+      @unlock_gems = unlock_gems
+      @sort_versions = {}
+    end
+
+    def level=(value)
+      v = begin
+        case value
+        when String, Symbol
+          value.to_sym
+        end
+      end
+
+      @level = [:major, :minor, :patch].include?(v) ? v : @level_default
+    end
+
+    def sort_versions(dep, dep_specs)
+      before_result = "before sort_versions: #{debug_format_result(dep, dep_specs).inspect}"
+
+      result = @sort_versions[dep] ||= begin
+        gem_name = dep.name
+
+        # An Array per version returned, different entries for different platforms.
+        # We just need the version here so it's ok to hard code this to the first instance.
+        locked_spec = @locked_specs[gem_name].first
+
+        if @strict
+          filter_dep_specs(dep_specs, locked_spec)
+        else
+          sort_dep_specs(dep_specs, locked_spec)
+        end.tap do |specs|
+          if ENV["DEBUG_PATCH_RESOLVER"] # MODO: proper debug flag name, check and proper debug output
+            STDERR.puts before_result
+            STDERR.puts " after sort_versions: #{debug_format_result(dep, specs).inspect}"
+          end
+        end
+      end
+      result.dup # not ideal, but elsewhere in bundler the resulting array is occasionally emptied, corrupting the cache.
+    end
+
+  private
+
+    def filter_dep_specs(specs, locked_spec)
+      res = specs.select do |sg|
+        # SpecGroup is grouped by name/version, multiple entries for multiple platforms.
+        # We only need the name, which will be the same, so hard coding to first is ok.
+        gem_spec = sg.first
+
+        if locked_spec
+          gsv = gem_spec.version
+          lsv = locked_spec.version
+
+          must_match = @level == :minor ? [0] : [0, 1]
+
+          matches = must_match.map {|idx| gsv.segments[idx] == lsv.segments[idx] }
+          (matches.uniq == [true]) ? (gsv >= lsv) : false
+        else
+          true
+        end
+      end
+
+      sort_dep_specs(res, locked_spec)
+    end
+
+    # reminder: sort still filters anything older than locked version
+    # :major bundle update behavior can move a gem to an older version
+    # in order to satisfy the dependency tree.
+    def sort_dep_specs(specs, locked_spec)
+      return specs unless locked_spec
+      gem_name = locked_spec.name
+      locked_version = locked_spec.version
+
+      filtered = specs.select {|s| s.first.version >= locked_version }
+
+      filtered.sort do |a, b|
+        a_ver = a.first.version
+        b_ver = b.first.version
+        case
+        when a_ver.segments[0] != b_ver.segments[0]
+          b_ver <=> a_ver
+        when !(@level == :minor) && (a_ver.segments[1] != b_ver.segments[1])
+          b_ver <=> a_ver
+        when @minimal && !unlocking_gem?(gem_name)
+          b_ver <=> a_ver
+        when @minimal && unlocking_gem?(gem_name) &&
+          ![a_ver, b_ver].include?(locked_version) # MODO: revisit this case
+          b_ver <=> a_ver
+        else
+          a_ver <=> b_ver
+        end
+      end.tap do |result|
+        unless unlocking_gem?(gem_name)
+          move_version_to_end(specs, locked_version, result)
+        end
+      end
+    end
+
+    def unlocking_gem?(gem_name)
+      @unlock_gems.empty? || @unlock_gems.include?(gem_name)
+    end
+
+    def move_version_to_end(specs, version, result)
+      spec_group = specs.detect {|s| s.first.version.to_s == version.to_s }
+      return unless spec_group
+      result.reject! {|s| s.first.version.to_s == version.to_s }
+      result << spec_group
+    end
+
+    def debug_format_result(dep, res)
+      a = [dep.to_s,
+           res.map {|sg| [sg.version, sg.dependencies_for_activated_platforms.map {|dp| [dp.name, dp.requirement.to_s] }] }]
+      [a.first, a.last.map {|sg_data| [sg_data.first.version, sg_data.last.map {|aa| aa.join(" ") }] },
+       @level, @strict ? :strict : :not_strict, @minimal ? :minimal : :not_minimal]
+    end
+  end
+end
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index 51ce2c83..e687b862 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -172,14 +172,14 @@ module Bundler
     # ==== Returns
     # <GemBundle>,nil:: If the list of dependencies can be resolved, a
     #   collection of gemspecs is returned. Otherwise, nil is returned.
-    def self.resolve(requirements, index, source_requirements = {}, base = [], ruby_version = nil, dependency_search = DependencySearch.new)
+    def self.resolve(requirements, index, source_requirements = {}, base = [], ruby_version = nil, gem_version_promoter = GemVersionPromoter.new)
       base = SpecSet.new(base) unless base.is_a?(SpecSet)
-      resolver = new(index, source_requirements, base, ruby_version, dependency_search)
+      resolver = new(index, source_requirements, base, ruby_version, gem_version_promoter)
       result = resolver.start(requirements)
       SpecSet.new(result)
     end
 
-    def initialize(index, source_requirements, base, ruby_version, dependency_search = DependencySearch.new)
+    def initialize(index, source_requirements, base, ruby_version, gem_version_promoter = GemVersionPromoter.new)
       @index = index
       @source_requirements = source_requirements
       @base = base
@@ -188,7 +188,7 @@ module Bundler
       @base_dg = Molinillo::DependencyGraph.new
       @base.each {|ls| @base_dg.add_vertex(ls.name, Dependency.new(ls.name, ls.version), true) }
       @ruby_version = ruby_version
-      @dependency_search = dependency_search
+      @gem_version_promoter = gem_version_promoter
     end
 
     def start(requirements)
@@ -269,7 +269,8 @@ module Bundler
         end
       end
       platform_results = search.select {|sg| sg.for?(platform, @ruby_version) }.each {|sg| sg.activate_platform!(platform) }
-      @dependency_search.arrange_dep_specs(dependency, platform_results)
+      return platform_results if @gem_version_promoter.level == :major # default behavior
+      @gem_version_promoter.sort_versions(dependency, platform_results)
     end
 
     def index_for(dependency)
@@ -366,130 +367,5 @@ module Bundler
       end
       version_platform_strs.join(", ")
     end
-
-    class DependencySearch
-      attr_accessor :level, :strict, :minimal
-
-      def initialize(locked_specs = SpecSet.new([]), unlock_gems = [])
-        @level_default = :major
-        @level = @level_default
-        @strict = false
-        @minimal = false
-        @locked_specs = locked_specs
-        @unlock_gems = unlock_gems
-      end
-
-      def unlocking_gem?(gem_name)
-        @unlock_gems.empty? || @unlock_gems.include?(gem_name)
-      end
-
-      def level=(value)
-        # MODO: figure this out mo' bettah
-        v = begin
-          value.to_sym
-        rescue
-          nil
-        end
-
-        @level = [:major, :minor, :patch].include?(v) ? v : @level_default
-      end
-
-      def arrange_dep_specs(dep, dep_specs)
-        return dep_specs if @level == :major
-
-        # MODO: bring search_for in here (copy index_for one liner?)
-        super_result = "super search_for: #{debug_format_result(dep, dep_specs).inspect}"
-
-        # MODO: figure out caching here plus what search_for provides
-        # @conservative_search_for[dep] ||=
-        begin
-          gem_name = dep.name
-
-          # An Array per version returned, different entries for different platforms.
-          # We just need the version here so it's ok to hard code this to the first instance.
-          locked_spec = @locked_specs[gem_name].first
-
-          if @strict
-            filter_dep_specs(dep_specs, locked_spec)
-          else
-            sort_dep_specs(dep_specs, locked_spec)
-          end.tap do |specs|
-            if ENV["DEBUG_PATCH_RESOLVER"] # MODO: proper debug flag name, check and proper debug output
-              STDERR.puts super_result
-              STDERR.puts "after search_for: #{debug_format_result(dep, specs).inspect}"
-            end
-          end
-        end
-      end
-
-      def debug_format_result(dep, res)
-        a = [dep.to_s,
-             res.map {|sg| [sg.version, sg.dependencies_for_activated_platforms.map {|dp| [dp.name, dp.requirement.to_s] }] }]
-        [a.first, a.last.map {|sg_data| [sg_data.first.version, sg_data.last.map {|aa| aa.join(" ") }] },
-         @level, @strict ? :strict : :not_strict, @minimal ? :minimal : :not_minimal]
-      end
-
-      def filter_dep_specs(specs, locked_spec)
-        res = specs.select do |sg|
-          # SpecGroup is grouped by name/version, multiple entries for multiple platforms.
-          # We only need the name, which will be the same, so hard coding to first is ok.
-          gem_spec = sg.first
-
-          if locked_spec
-            gsv = gem_spec.version
-            lsv = locked_spec.version
-
-            must_match = @level == :minor ? [0] : [0, 1]
-
-            matches = must_match.map {|idx| gsv.segments[idx] == lsv.segments[idx] }
-            (matches.uniq == [true]) ? (gsv >= lsv) : false
-          else
-            true
-          end
-        end
-
-        sort_dep_specs(res, locked_spec)
-      end
-
-      # reminder: sort still filters anything older than locked version
-      # :major bundle update behavior can move a gem to an older version
-      # in order to satisfy the dependency tree.
-      def sort_dep_specs(specs, locked_spec)
-        return specs unless locked_spec
-        gem_name = locked_spec.name
-        locked_version = locked_spec.version
-
-        filtered = specs.select {|s| s.first.version >= locked_version }
-
-        filtered.sort do |a, b|
-          a_ver = a.first.version
-          b_ver = b.first.version
-          case
-          when a_ver.segments[0] != b_ver.segments[0]
-            b_ver <=> a_ver
-          when !(@level == :minor) && (a_ver.segments[1] != b_ver.segments[1])
-            b_ver <=> a_ver
-          when @minimal && !unlocking_gem?(gem_name)
-            b_ver <=> a_ver
-          when @minimal && unlocking_gem?(gem_name) &&
-            ![a_ver, b_ver].include?(locked_version) # MODO: revisit this case
-            b_ver <=> a_ver
-          else
-            a_ver <=> b_ver
-          end
-        end.tap do |result|
-          unless unlocking_gem?(gem_name)
-            move_version_to_end(specs, locked_version, result)
-          end
-        end
-      end
-
-      def move_version_to_end(specs, version, result)
-        spec_group = specs.detect {|s| s.first.version.to_s == version.to_s }
-        return unless spec_group
-        result.reject! {|s| s.first.version.to_s == version.to_s }
-        result << spec_group
-      end
-    end
   end
 end