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.

7 files changed, 141 insertions, 142 deletions

diff --git a/lib/bundler.rb b/lib/bundler.rb
index 0ba58e518c..b8eed25d44 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -30,6 +30,7 @@ module Bundler
   autoload :Fetcher,                "bundler/fetcher"
   autoload :GemHelper,              "bundler/gem_helper"
   autoload :GemHelpers,             "bundler/gem_helpers"
+  autoload :GemVersionPromoter,     "bundler/gem_version_promoter"
   autoload :Graph,                  "bundler/graph"
   autoload :Index,                  "bundler/index"
   autoload :Installer,              "bundler/installer"
diff --git a/lib/bundler/cli/update.rb b/lib/bundler/cli/update.rb
index fec5a5435b..cdf1dc1054 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 c1d4d18aa6..73e1c009d0 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 0000000000..19de170a45
--- /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 51ce2c8394..e687b86216 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
diff --git a/spec/resolver/basic_spec.rb b/spec/resolver/basic_spec.rb
index 99ffb429cc..41b9f8c84b 100644
--- a/spec/resolver/basic_spec.rb
+++ b/spec/resolver/basic_spec.rb
@@ -178,12 +178,8 @@ describe "Resolving" do
       should_consv_resolve_and_include [:minor, :minimal], [], %w(foo-1.4.4 bar-2.0.4)
     end
 
-    it 'will not revert to a previous version'
-
-    it 'has taken care of all MODOs'
-
-    it 'has moved DependencySearch into its own file'
-
-    it 'has moved search_for impl out of Resolver into DependencySearch' # method called by Molinillo, but bulk can go off to dep_search
+    it "will not revert to a previous version"
+    it "has taken care of all MODOs"
+    it "bring over all sort/filter specs from bundler-patch"
   end
 end
diff --git a/spec/support/indexes.rb b/spec/support/indexes.rb
index f704b5cf17..7de62eefeb 100644
--- a/spec/support/indexes.rb
+++ b/spec/support/indexes.rb
@@ -58,7 +58,7 @@ module Spec
     def should_consv_resolve_and_include(opts, unlock, specs)
       # empty unlock means unlock all
       opts = Array(opts)
-      search = Bundler::Resolver::DependencySearch.new(@locked, unlock).tap do |s|
+      search = Bundler::GemVersionPromoter.new(@locked, unlock).tap do |s|
         s.level = opts.first
         s.strict = opts.include?(:strict)
         s.minimal = opts.include?(:minimal)