Merge pull request #9582 from ruby/cooldown-local-stub-bypass

Apply cooldown to locally installed gem versions

Author: hsbt

bundler/lib/bundler/remote_specification.rb

@@ -12,7 +12,7 @@ class RemoteSpecification
 
     attr_reader :name, :version, :platform
     attr_writer :dependencies
-    attr_accessor :source, :remote, :locked_platform
+    attr_accessor :source, :remote, :locked_platform, :created_at
 
     def initialize(name, version, platform, spec_fetcher)
       @name         = name

bundler/lib/bundler/resolver.rb

@@ -422,7 +422,7 @@ def filter_matching_specs(specs, requirements)
     end
 
     def filter_specs(specs, package)
-      filter_cooldown(filter_remote_specs(filter_prereleases(specs, package), package))
+      filter_remote_specs(filter_cooldown(filter_prereleases(specs, package)), package)
     end
 
     def filter_prereleases(specs, package)
@@ -433,19 +433,24 @@ def filter_prereleases(specs, package)
 
     def filter_cooldown(specs)
       return specs if specs.empty?
-      excluded = cooldown_excluded_specs(specs)
-      return specs if excluded.empty?
-      specs - excluded
+      excluded_versions = cooldown_excluded_versions(specs)
+      return specs if excluded_versions.empty?
+      specs.reject {|s| excluded_versions.include?([s.name, s.version]) }
     end
 
-    def cooldown_excluded_specs(specs)
-      specs.select {|spec| cooldown_excluded?(spec) }
+    def cooldown_excluded_versions(specs)
+      excluded = {}
+      specs.each do |spec|
+        next unless cooldown_excluded?(spec)
+        excluded[[spec.name, spec.version]] = true
+      end
+      excluded
     end
 
     def cooldown_hint(specs)
-      excluded = cooldown_excluded_specs(specs)
-      return nil if excluded.empty?
-      "#{excluded.size} version#{"s" if excluded.size > 1} excluded by the cooldown setting; pass `--cooldown 0` to bypass"
+      excluded_versions = cooldown_excluded_versions(specs)
+      return nil if excluded_versions.empty?
+      "#{excluded_versions.size} version#{"s" if excluded_versions.size > 1} excluded by the cooldown setting; pass `--cooldown 0` to bypass"
     end
 
     def cooldown_excluded?(spec)

bundler/lib/bundler/source/rubygems.rb

@@ -150,6 +150,13 @@ def specs
           # sources, and large_idx.merge! small_idx is way faster than
           # small_idx.merge! large_idx.
           index = @allow_remote ? remote_specs.dup : Index.new
+
+          # Snapshot per-version `created_at` from the remote info before installed
+          # / cached specs overwrite the EndpointSpecification objects that carry
+          # it. The cooldown filter consults `created_at` on every candidate, so
+          # local stubs need the published date back-filled to participate.
+          remote_created_at = collect_remote_created_at(index)
+
           index.merge!(cached_specs) if @allow_cached
           index.merge!(installed_specs) if @allow_local
 
@@ -163,6 +170,8 @@ def specs
             end
           end
 
+          backfill_created_at(index, remote_created_at) unless remote_created_at.empty?
+
           index
         end
       end
@@ -470,6 +479,31 @@ def cache_path
 
       private
 
+      def collect_remote_created_at(index)
+        return {} unless @allow_remote
+
+        snapshot = {}
+        index.each do |spec|
+          next unless spec.respond_to?(:created_at) && spec.created_at
+          # Remember the remote that supplied the date too: when a source has
+          # several remotes with different per-URI cooldown settings we must
+          # restore the same one during backfill so `effective_cooldown` agrees.
+          snapshot[[spec.name, spec.version]] = [spec.created_at, spec.remote]
+        end
+        snapshot
+      end
+
+      def backfill_created_at(index, snapshot)
+        index.each do |spec|
+          next unless spec.respond_to?(:created_at=)
+          next if spec.created_at
+          remote_created_at, remote = snapshot[[spec.name, spec.version]]
+          next unless remote_created_at
+          spec.created_at = remote_created_at
+          spec.remote ||= remote if remote && spec.respond_to?(:remote=)
+        end
+      end
+
       def lockfile_remotes
         @lockfile_remotes || credless_remotes
       end

spec/bundler/resolver/cooldown_spec.rb

@@ -7,8 +7,8 @@ def remote(cooldown:)
     instance_double(Bundler::Source::Rubygems::Remote, effective_cooldown: cooldown)
   end
 
-  def spec(created_at:, remote:)
-    Struct.new(:created_at, :remote).new(created_at, remote)
+  def spec(created_at:, remote:, name: "myrack", version: "1.0.0")
+    Struct.new(:name, :version, :created_at, :remote).new(name, Gem::Version.new(version), created_at, remote)
   end
 
   describe "#filter_cooldown" do
@@ -18,8 +18,8 @@ def spec(created_at:, remote:)
       let(:r) { remote(cooldown: 7) }
 
       it "rejects versions published within the window" do
-        recent = spec(created_at: now - (2 * 86_400), remote: r)
-        old = spec(created_at: now - (30 * 86_400), remote: r)
+        recent = spec(version: "1.1.0", created_at: now - (2 * 86_400), remote: r)
+        old = spec(version: "1.0.0", created_at: now - (30 * 86_400), remote: r)
 
         expect(resolver.send(:filter_cooldown, [recent, old])).to eq([old])
       end
@@ -32,14 +32,37 @@ def spec(created_at:, remote:)
 
       it "leaves rolling-delay history intact" do
         # 7-day cooldown with frequent releases must still expose an older candidate.
-        in_cooldown = spec(created_at: now - 86_400, remote: r)
-        also_in_cooldown = spec(created_at: now - (3 * 86_400), remote: r)
-        eligible = spec(created_at: now - (10 * 86_400), remote: r)
+        in_cooldown = spec(version: "1.2.0", created_at: now - 86_400, remote: r)
+        also_in_cooldown = spec(version: "1.1.0", created_at: now - (3 * 86_400), remote: r)
+        eligible = spec(version: "1.0.0", created_at: now - (10 * 86_400), remote: r)
 
         result = resolver.send(:filter_cooldown, [in_cooldown, also_in_cooldown, eligible])
 
         expect(result).to eq([eligible])
       end
+
+      it "drops every spec sharing an excluded [name, version] tuple" do
+        # The cooldown check is by version, not per-spec: a StubSpecification for an
+        # in-cooldown release would otherwise slip through on local install paths.
+        endpoint = spec(version: "2.0.0", created_at: now - 86_400, remote: r)
+        local_stub = Struct.new(:name, :version).new("myrack", Gem::Version.new("2.0.0"))
+        eligible = spec(version: "1.0.0", created_at: now - (30 * 86_400), remote: r)
+
+        result = resolver.send(:filter_cooldown, [endpoint, local_stub, eligible])
+
+        expect(result).to eq([eligible])
+      end
+
+      it "keeps stub-only versions that no endpoint marks as in cooldown" do
+        # If no remote spec carries created_at for a version, cooldown cannot judge it;
+        # the stub stays in.
+        local_only = Struct.new(:name, :version).new("myrack", Gem::Version.new("2.0.0"))
+        eligible = spec(version: "1.0.0", created_at: now - (30 * 86_400), remote: r)
+
+        result = resolver.send(:filter_cooldown, [local_only, eligible])
+
+        expect(result).to eq([local_only, eligible])
+      end
     end
 
     context "when created_at is missing (blank metadata)" do
@@ -111,9 +134,15 @@ def spec(created_at:, remote:)
     end
 
     it "uses plural wording when multiple versions are excluded" do
-      excluded = Array.new(3) { spec(created_at: now - 86_400, remote: r) }
+      excluded = %w[1.0.0 1.1.0 1.2.0].map {|v| spec(version: v, created_at: now - 86_400, remote: r) }
 
       expect(resolver.send(:cooldown_hint, excluded)).to match(/3 versions excluded/)
     end
+
+    it "counts each unique version once even when multiple spec instances share it" do
+      duplicates = Array.new(3) { spec(created_at: now - 86_400, remote: r) }
+
+      expect(resolver.send(:cooldown_hint, duplicates)).to match(/1 version excluded/)
+    end
   end
 end

spec/install/cooldown_spec.rb

@@ -215,6 +215,32 @@
       expect(out).to match(/ripe_gem.*in cooldown for \d+ more day/)
     end
 
+    it "excludes a locally-installed version that is still within the cooldown window" do
+      system_gems "ripe_gem-2.0.0", gem_repo: gem_repo3
+
+      gemfile <<-G
+        source "https://gem.repo3"
+        gem "ripe_gem"
+      G
+
+      bundle "install --cooldown 7", artifice: "compact_index_cooldown"
+
+      expect(the_bundle).to include_gems("ripe_gem 1.0.0")
+    end
+
+    it "selects a locally-installed in-cooldown version when --cooldown 0 bypasses the filter" do
+      system_gems "ripe_gem-2.0.0", gem_repo: gem_repo3
+
+      gemfile <<-G
+        source "https://gem.repo3"
+        gem "ripe_gem"
+      G
+
+      bundle "install --cooldown 0", artifice: "compact_index_cooldown"
+
+      expect(the_bundle).to include_gems("ripe_gem 2.0.0")
+    end
+
     it "surfaces a cooldown hint when bundle update filters every candidate" do
       gemfile <<-G
         source "https://gem.repo3"