[rubygems/rubygems] Extract a `Gem::SpecificationRecord` class

This class handles all logic to handle the list of specifications, given
a set of GEM_PATH directories. Makes `Gem::Specification` has less
responsibilities and will help with fixing some bugs next.

https://github.com/rubygems/rubygems/commit/df280dbbed

3 files changed, 202 insertions, 87 deletions

diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb
index e369516bdd..e27901923e 100644
--- a/lib/rubygems/specification.rb
+++ b/lib/rubygems/specification.rb
@@ -11,6 +11,7 @@ require_relative "deprecate"
 require_relative "basic_specification"
 require_relative "stub_specification"
 require_relative "platform"
+require_relative "specification_record"
 require_relative "util/list"
 
 require "rbconfig"
@@ -179,19 +180,9 @@ class Gem::Specification < Gem::BasicSpecification
     @@default_value[k].nil?
   end
 
-  def self.clear_specs # :nodoc:
-    @@all = nil
-    @@stubs = nil
-    @@stubs_by_name = {}
-    @@spec_with_requirable_file = {}
-    @@active_stub_with_requirable_file = {}
-  end
-  private_class_method :clear_specs
-
-  clear_specs
-
   # Sentinel object to represent "not found" stubs
   NOT_FOUND = Struct.new(:to_spec, :this).new # :nodoc:
+  deprecate_constant :NOT_FOUND
 
   # Tracking removed method calls to warn users during build time.
   REMOVED_METHODS = [:rubyforge_project=, :mark_version].freeze # :nodoc:
@@ -770,7 +761,7 @@ class Gem::Specification < Gem::BasicSpecification
   attr_accessor :specification_version
 
   def self._all # :nodoc:
-    @@all ||= Gem.loaded_specs.values | stubs.map(&:to_spec)
+    specification_record.all
   end
 
   def self.clear_load_cache # :nodoc:
@@ -788,26 +779,9 @@ class Gem::Specification < Gem::BasicSpecification
     end
   end
 
-  def self.gemspec_stubs_in(dir, pattern)
+  def self.gemspec_stubs_in(dir, pattern) # :nodoc:
     Gem::Util.glob_files_in_dir(pattern, dir).map {|path| yield path }.select(&:valid?)
   end
-  private_class_method :gemspec_stubs_in
-
-  def self.installed_stubs(dirs, pattern)
-    map_stubs(dirs, pattern) do |path, base_dir, gems_dir|
-      Gem::StubSpecification.gemspec_stub(path, base_dir, gems_dir)
-    end
-  end
-  private_class_method :installed_stubs
-
-  def self.map_stubs(dirs, pattern) # :nodoc:
-    dirs.flat_map do |dir|
-      base_dir = File.dirname dir
-      gems_dir = File.join base_dir, "gems"
-      gemspec_stubs_in(dir, pattern) {|path| yield path, base_dir, gems_dir }
-    end
-  end
-  private_class_method :map_stubs
 
   def self.each_spec(dirs) # :nodoc:
     each_gemspec(dirs) do |path|
@@ -820,13 +794,7 @@ class Gem::Specification < Gem::BasicSpecification
   # Returns a Gem::StubSpecification for every installed gem
 
   def self.stubs
-    @@stubs ||= begin
-      pattern = "*.gemspec"
-      stubs = stubs_for_pattern(pattern, false)
-
-      @@stubs_by_name = stubs.select {|s| Gem::Platform.match_spec? s }.group_by(&:name)
-      stubs
-    end
+    specification_record.stubs
   end
 
   ##
@@ -845,13 +813,7 @@ class Gem::Specification < Gem::BasicSpecification
   # only returns stubs that match Gem.platforms
 
   def self.stubs_for(name)
-    if @@stubs
-      @@stubs_by_name[name] || []
-    else
-      @@stubs_by_name[name] ||= stubs_for_pattern("#{name}-*.gemspec").select do |s|
-        s.name == name
-      end
-    end
+    specification_record.stubs_for(name)
   end
 
   ##
@@ -859,12 +821,7 @@ class Gem::Specification < Gem::BasicSpecification
   # optionally filtering out specs not matching the current platform
   #
   def self.stubs_for_pattern(pattern, match_platform = true) # :nodoc:
-    installed_stubs = installed_stubs(dirs, pattern)
-    installed_stubs.select! {|s| Gem::Platform.match_spec? s } if match_platform
-    stubs = installed_stubs + default_stubs(pattern)
-    stubs = stubs.uniq(&:full_name)
-    _resort!(stubs)
-    stubs
+    specification_record.stubs_for_pattern(pattern, match_platform)
   end
 
   def self._resort!(specs) # :nodoc:
@@ -893,23 +850,14 @@ class Gem::Specification < Gem::BasicSpecification
   # properly sorted.
 
   def self.add_spec(spec)
-    return if _all.include? spec
-
-    _all << spec
-    stubs << spec
-    (@@stubs_by_name[spec.name] ||= []) << spec
-
-    _resort!(@@stubs_by_name[spec.name])
-    _resort!(stubs)
+    specification_record.add_spec(spec)
   end
 
   ##
   # Removes +spec+ from the known specs.
 
   def self.remove_spec(spec)
-    _all.delete spec.to_spec
-    stubs.delete spec
-    (@@stubs_by_name[spec.name] || []).delete spec
+    specification_record.remove_spec(spec)
   end
 
   ##
@@ -926,15 +874,14 @@ class Gem::Specification < Gem::BasicSpecification
   # Sets the known specs to +specs+.
 
   def self.all=(specs)
-    @@stubs_by_name = specs.group_by(&:name)
-    @@all = @@stubs = specs
+    specification_record.all = specs
   end
 
   ##
   # Return full names of all specs in sorted order.
 
   def self.all_names
-    _all.map(&:full_name)
+    specification_record.all_names
   end
 
   ##
@@ -1024,12 +971,7 @@ class Gem::Specification < Gem::BasicSpecification
   # Return the best specification that contains the file matching +path+.
 
   def self.find_by_path(path)
-    path = path.dup.freeze
-    spec = @@spec_with_requirable_file[path] ||= stubs.find do |s|
-      s.contains_requirable_file? path
-    end || NOT_FOUND
-
-    spec.to_spec
+    specification_record.find_by_path(path)
   end
 
   ##
@@ -1037,11 +979,7 @@ class Gem::Specification < Gem::BasicSpecification
   # amongst the specs that are not activated.
 
   def self.find_inactive_by_path(path)
-    stub = stubs.find do |s|
-      next if s.activated?
-      s.contains_requirable_file? path
-    end
-    stub&.to_spec
+    specification_record.find_inactive_by_path(path)
   end
 
   ##
@@ -1049,11 +987,7 @@ class Gem::Specification < Gem::BasicSpecification
   # those already activated.
 
   def self.find_active_stub_by_path(path)
-    stub = @@active_stub_with_requirable_file[path] ||= stubs.find do |s|
-      s.activated? && s.contains_requirable_file?(path)
-    end || NOT_FOUND
-
-    stub.this
+    specification_record.find_active_stub_by_path(path)
   end
 
   ##
@@ -1120,14 +1054,14 @@ class Gem::Specification < Gem::BasicSpecification
   # +prerelease+ is true.
 
   def self.latest_specs(prerelease = false)
-    _latest_specs stubs, prerelease
+    specification_record.latest_specs(prerelease)
   end
 
   ##
   # Return the latest installed spec for gem +name+.
 
   def self.latest_spec_for(name)
-    latest_specs(true).find {|installed_spec| installed_spec.name == name }
+    specification_record.latest_spec_for(name)
   end
 
   def self._latest_specs(specs, prerelease = false) # :nodoc:
@@ -1265,7 +1199,7 @@ class Gem::Specification < Gem::BasicSpecification
   def self.reset
     @@dirs = nil
     Gem.pre_reset_hooks.each(&:call)
-    clear_specs
+    @specification_record = nil
     clear_load_cache
     unresolved = unresolved_deps
     unless unresolved.empty?
@@ -1286,6 +1220,13 @@ class Gem::Specification < Gem::BasicSpecification
     Gem.post_reset_hooks.each(&:call)
   end
 
+  ##
+  # Keeps track of all currently known specifications
+
+  def self.specification_record
+    @specification_record ||= Gem::SpecificationRecord.new(dirs)
+  end
+
   # DOC: This method needs documented or nodoc'd
   def self.unresolved_deps
     @unresolved_deps ||= Hash.new {|h, n| h[n] = Gem::Dependency.new n }
diff --git a/lib/rubygems/specification_record.rb b/lib/rubygems/specification_record.rb
new file mode 100644
index 0000000000..bdce0b3a09
--- /dev/null
+++ b/lib/rubygems/specification_record.rb
@@ -0,0 +1,171 @@
+# frozen_string_literal: true
+
+module Gem
+  class SpecificationRecord
+    def initialize(dirs)
+      @all = nil
+      @stubs = nil
+      @stubs_by_name = {}
+      @spec_with_requirable_file = {}
+      @active_stub_with_requirable_file = {}
+
+      @dirs = dirs
+    end
+
+    # Sentinel object to represent "not found" stubs
+    NOT_FOUND = Struct.new(:to_spec, :this).new
+    private_constant :NOT_FOUND
+
+    ##
+    # Returns the list of all specifications in the record
+
+    def all
+      @all ||= Gem.loaded_specs.values | stubs.map(&:to_spec)
+    end
+
+    ##
+    # Returns a Gem::StubSpecification for every specification in the record
+
+    def stubs
+      @stubs ||= begin
+        pattern = "*.gemspec"
+        stubs = stubs_for_pattern(pattern, false)
+
+        @stubs_by_name = stubs.select {|s| Gem::Platform.match_spec? s }.group_by(&:name)
+        stubs
+      end
+    end
+
+    ##
+    # Returns a Gem::StubSpecification for every specification in the record
+    # named +name+ only returns stubs that match Gem.platforms
+
+    def stubs_for(name)
+      if @stubs
+        @stubs_by_name[name] || []
+      else
+        @stubs_by_name[name] ||= stubs_for_pattern("#{name}-*.gemspec").select do |s|
+          s.name == name
+        end
+      end
+    end
+
+    ##
+    # Finds stub specifications matching a pattern in the record, optionally
+    # filtering out specs not matching the current platform
+
+    def stubs_for_pattern(pattern, match_platform = true)
+      installed_stubs = installed_stubs(pattern)
+      installed_stubs.select! {|s| Gem::Platform.match_spec? s } if match_platform
+      stubs = installed_stubs + Gem::Specification.default_stubs(pattern)
+      stubs = stubs.uniq(&:full_name)
+      Gem::Specification._resort!(stubs)
+      stubs
+    end
+
+    ##
+    # Adds +spec+ to the the record, keeping the collection properly sorted.
+
+    def add_spec(spec)
+      return if all.include? spec
+
+      all << spec
+      stubs << spec
+      (@stubs_by_name[spec.name] ||= []) << spec
+
+      Gem::Specification._resort!(@stubs_by_name[spec.name])
+      Gem::Specification._resort!(stubs)
+    end
+
+    ##
+    # Removes +spec+ from the record.
+
+    def remove_spec(spec)
+      all.delete spec.to_spec
+      stubs.delete spec
+      (@stubs_by_name[spec.name] || []).delete spec
+    end
+
+    ##
+    # Sets the specs known by the record to +specs+.
+
+    def all=(specs)
+      @stubs_by_name = specs.group_by(&:name)
+      @all = @stubs = specs
+    end
+
+    ##
+    # Return full names of all specs in the record in sorted order.
+
+    def all_names
+      all.map(&:full_name)
+    end
+
+    ##
+    # Return the best specification in the record that contains the file matching +path+.
+
+    def find_by_path(path)
+      path = path.dup.freeze
+      spec = @spec_with_requirable_file[path] ||= stubs.find do |s|
+        s.contains_requirable_file? path
+      end || NOT_FOUND
+
+      spec.to_spec
+    end
+
+    ##
+    # Return the best specification in the record that contains the file
+    # matching +path+ amongst the specs that are not activated.
+
+    def find_inactive_by_path(path)
+      stub = stubs.find do |s|
+        next if s.activated?
+        s.contains_requirable_file? path
+      end
+      stub&.to_spec
+    end
+
+    ##
+    # Return the best specification in the record that contains the file
+    # matching +path+, among those already activated.
+
+    def find_active_stub_by_path(path)
+      stub = @active_stub_with_requirable_file[path] ||= stubs.find do |s|
+        s.activated? && s.contains_requirable_file?(path)
+      end || NOT_FOUND
+
+      stub.this
+    end
+
+    ##
+    # Return the latest specs in the record, optionally including prerelease
+    # specs if +prerelease+ is true.
+
+    def latest_specs(prerelease)
+      Gem::Specification._latest_specs stubs, prerelease
+    end
+
+    ##
+    # Return the latest installed spec in the record for gem +name+.
+
+    def latest_spec_for(name)
+      latest_specs(true).find {|installed_spec| installed_spec.name == name }
+    end
+
+    private
+
+    def installed_stubs(pattern)
+      map_stubs(pattern) do |path, base_dir, gems_dir|
+        Gem::StubSpecification.gemspec_stub(path, base_dir, gems_dir)
+      end
+    end
+
+    def map_stubs(pattern)
+      @dirs.flat_map do |dir|
+        base_dir = File.dirname dir
+        gems_dir = File.join base_dir, "gems"
+        Gem::Specification.gemspec_stubs_in(dir, pattern) {|path| yield path, base_dir, gems_dir }
+      end
+    end
+  end
+end
diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb
index f9f063c797..9395e34f75 100644
--- a/test/rubygems/test_gem_specification.rb
+++ b/test/rubygems/test_gem_specification.rb
@@ -967,7 +967,10 @@ dependencies: []
 
   def test_self_stubs_for_lazy_loading
     Gem.loaded_specs.clear
-    Gem::Specification.class_variable_set(:@@stubs, nil)
+
+    specification_record = Gem::Specification.specification_record
+
+    specification_record.instance_variable_set(:@stubs, nil)
 
     dir_standard_specs = File.join Gem.dir, "specifications"
 
@@ -975,9 +978,9 @@ dependencies: []
     save_gemspec("b-1", "1", dir_standard_specs) {|s| s.name = "b" }
 
     assert_equal ["a-1"], Gem::Specification.stubs_for("a").map(&:full_name)
-    assert_equal 1, Gem::Specification.class_variable_get(:@@stubs_by_name).length
+    assert_equal 1, specification_record.instance_variable_get(:@stubs_by_name).length
     assert_equal ["b-1"], Gem::Specification.stubs_for("b").map(&:full_name)
-    assert_equal 2, Gem::Specification.class_variable_get(:@@stubs_by_name).length
+    assert_equal 2, specification_record.instance_variable_get(:@stubs_by_name).length
 
     assert_equal(
       Gem::Specification.stubs_for("a").map(&:object_id),
@@ -986,7 +989,7 @@ dependencies: []
 
     Gem.loaded_specs.delete "a"
     Gem.loaded_specs.delete "b"
-    Gem::Specification.class_variable_set(:@@stubs, nil)
+    specification_record.instance_variable_set(:@stubs, nil)
   end
 
   def test_self_stubs_for_no_lazy_loading_after_all_specs_setup