Refactor cache store code.

Library/Homebrew/cache_store.rb

@@ -2,26 +2,59 @@ require "dbm"
 require "json"
 
 #
-# `DatabaseCache` acts as an interface to a persistent storage mechanism
+# `CacheStoreDatabase` acts as an interface to a persistent storage mechanism
 # residing in the `HOMEBREW_CACHE`
 #
-class DatabaseCache
+class CacheStoreDatabase
+  # Yields the cache store database.
+  # Closes the database after use if it has been loaded.
+  #
+  # @param  [Symbol] type
+  # @yield  [CacheStoreDatabase] self
+  def self.use(type)
+    database = CacheStoreDatabase.new(type)
+    return_value = yield(database)
+    database.close_if_open!
+    return_value
+  end
+
+  # Sets a value in the underlying database (and creates it if necessary).
+  def set(key, value)
+    db[key] = value
+  end
+
+  # Gets a value from the underlying database (if it already exists).
+  def get(key)
+    return unless created?
+    db[key]
+  end
+
+  # Gets a value from the underlying database (if it already exists).
+  def delete(key)
+    return unless created?
+    db.delete(key)
+  end
+
+  # Closes the underlying database (if it created and open).
+  def close_if_open!
+    @db&.close
+  end
+
+  # Returns `true` if the cache file has been created for the given `@type`
+  #
+  # @return [Boolean]
+  def created?
+    File.exist?(cache_path)
+  end
+
+  private
+
   # The mode of any created files will be 0664 (that is, readable and writable
   # by the owner and the group, and readable by everyone else). Files created
   # will also be modified by the process' umask value at the time of creation:
   #   https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html
   DATABASE_MODE = 0664
 
-  def self.use(type)
-    return_value = nil
-
-    DatabaseCache.new(type) do |database_cache|
-      return_value = yield(database_cache)
-    end
-
-    return_value
-  end
-
   # Lazily loaded database in read/write mode. If this method is called, a
   # database file with be created in the `HOMEBREW_CACHE` with name
   # corresponding to the `@type` instance variable
@@ -32,25 +65,12 @@ class DatabaseCache
     @db ||= DBM.open(dbm_file_path, DATABASE_MODE, DBM::WRCREAT)
   end
 
-  # Returns `true` if the cache is empty for the given `@type`
+  # Creates a CacheStoreDatabase
-  #
-  # @return [Boolean]
-  def empty?
-    !File.exist?(cache_path)
-  end
-
-  private
-
-  # Opens and yields the cache. Closes the database after use if it has been
-  # loaded
   #
   # @param  [Symbol] type
-  # @yield  [DatabaseCache] self
   # @return [nil]
   def initialize(type)
     @type = type
-    yield(self)
-    @db&.close
   end
 
   # `DBM` appends `.db` file extension to the path provided, which is why it's
@@ -75,10 +95,10 @@ end
 # storage mechanism
 #
 class CacheStore
-  # @param  [DBM] db
+  # @param  [CacheStoreDatabase] database
   # @return [nil]
-  def initialize(db)
+  def initialize(database)
-    @db = db
+    @database = database
   end
 
   # Inserts new values or updates existing cached values to persistent storage
@@ -106,8 +126,8 @@ class CacheStore
 
   protected
 
-  # @return [DBM]
+  # @return [CacheStoreDatabase]
-  attr_reader :db
+  attr_reader :database
 
   # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data,
   # to convert the ruby string back into a ruby `Hash`, the string is converted

Library/Homebrew/dev-cmd/linkage.rb

@@ -20,12 +20,12 @@ module Homebrew
   module_function
 
   def linkage
-    DatabaseCache.use(:linkage) do |database_cache|
+    CacheStoreDatabase.use(:linkage) do |db|
       ARGV.kegs.each do |keg|
         ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1
 
         use_cache = ARGV.include?("--cached") || ENV["HOMEBREW_LINKAGE_CACHE"]
-        result = LinkageChecker.new(keg, database_cache, use_cache)
+        result = LinkageChecker.new(keg, use_cache: use_cache, cache_db: db)
 
         if ARGV.include?("--test")
           result.display_test_output

Library/Homebrew/extend/os/mac/formula_cellar_checks.rb

@@ -66,11 +66,11 @@ module FormulaCellarChecks
     return unless formula.prefix.directory?
     keg = Keg.new(formula.prefix)
 
-    DatabaseCache.use(:linkage) do |database_cache|
+    CacheStoreDatabase.use(:linkage) do |db|
-      use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil?
+      checker = LinkageChecker.new keg, formula, cache_db: db,
-      checker = LinkageChecker.new(keg, database_cache, use_cache, formula)
+        use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil?
-
       next unless checker.broken_library_linkage?
+
       output = <<~EOS
         #{formula} has broken dynamic library links:
           #{checker.display_test_output}

Library/Homebrew/formula.rb

@@ -1527,9 +1527,9 @@ class Formula
     keg = opt_or_installed_prefix_keg
     return [] unless keg
 
-    undeclared_deps = DatabaseCache.use(:linkage) do |database_cache|
+    undeclared_deps = CacheStoreDatabase.use(:linkage) do |db|
-      use_cache = !ENV["HOMEBREW_LINKAGE_CACHE"].nil?
+      linkage_checker = LinkageChecker.new keg, self, cache_db: db,
-      linkage_checker = LinkageChecker.new(keg, database_cache, use_cache, self)
+        use_cache: !ENV["HOMEBREW_LINKAGE_CACHE"].nil?
       linkage_checker.undeclared_deps.map { |n| Dependency.new(n) }
     end
 

Library/Homebrew/formula_installer.rb

@@ -611,9 +611,9 @@ class FormulaInstaller
     puts summary
 
     # Updates the cache for a particular formula after doing an install
-    DatabaseCache.use(:linkage) do |database_cache|
+    CacheStoreDatabase.use(:linkage) do |db|
-      break if database_cache.empty?
+      break unless db.created?
-      LinkageChecker.new(keg, database_cache, false, formula)
+      LinkageChecker.new(keg, formula, cache_db: db)
     end
 
     # let's reset Utils.git_available? if we just installed git

Library/Homebrew/linkage_cache_store.rb

@@ -2,20 +2,23 @@ require "set"
 require "cache_store"
 
 #
-# `LinkageStore` provides methods to fetch and mutate linkage-specific data used
+# `LinkageCacheStore` provides methods to fetch and mutate linkage-specific data used
 # by the `brew linkage` command
 #
-class LinkageStore < CacheStore
+class LinkageCacheStore < CacheStore
-  ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs,
-                         :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze
-  HASH_LINKAGE_TYPES  = [:brewed_dylibs, :reverse_links, :broken_deps].freeze
-
   # @param  [String] keg_name
-  # @param  [DBM]    db
+  # @param  [CacheStoreDatabase] database
   # @return [nil]
-  def initialize(keg_name, db)
+  def initialize(keg_name, database)
     @keg_name = keg_name
-    super(db)
+    super(database)
+  end
+
+  # Returns `true` if the database has any value for the current `keg_name`
+  #
+  # @return [Boolean]
+  def has_keg_name?
+    !database.get(keg_name).nil?
   end
 
   # Inserts dylib-related information into the cache if it does not exist or
@@ -40,13 +43,13 @@ class LinkageStore < CacheStore
       end
     end
 
-    db[keg_name] = ruby_hash_to_json_string(
+    database.set keg_name, ruby_hash_to_json_string(
       array_values: format_array_values(array_values),
       hash_values: format_hash_values(hash_values),
     )
   end
 
-  # @param  [Symbol] the type to fetch from the `LinkageStore`
+  # @param  [Symbol] the type to fetch from the `LinkageCacheStore`
   # @raise  [TypeError] error if the type is not in `HASH_LINKAGE_TYPES` or
   #   `ARRAY_LINKAGE_TYPES`
   # @return [Hash | Array]
@@ -64,26 +67,32 @@ class LinkageStore < CacheStore
 
   # @return [nil]
   def flush_cache!
-    db.delete(keg_name)
+    database.delete(keg_name)
   end
 
   private
 
+  ARRAY_LINKAGE_TYPES = [:system_dylibs, :variable_dylibs, :broken_dylibs,
+    :indirect_deps, :undeclared_deps, :unnecessary_deps].freeze
+  HASH_LINKAGE_TYPES  = [:brewed_dylibs, :reverse_links, :broken_deps].freeze
+
   # @return [String] the key to lookup items in the `CacheStore`
   attr_reader :keg_name
 
-  # @param  [Symbol] the type to fetch from the `LinkageStore`
+  # @param  [Symbol] the type to fetch from the `LinkageCacheStore`
   # @return [Array]
   def fetch_array_values(type)
-    return [] unless db.key?(keg_name)
+    keg_cache = database.get(keg_name)
-    json_string_to_ruby_hash(db[keg_name])["array_values"][type.to_s]
+    return [] unless keg_cache
+    json_string_to_ruby_hash(keg_cache)["array_values"][type.to_s]
   end
 
   # @param  [Symbol] type
   # @return [Hash]
   def fetch_hash_values(type)
-    return {} unless db.key?(keg_name)
+    keg_cache = database.get(keg_name)
-    json_string_to_ruby_hash(db[keg_name])["hash_values"][type.to_s]
+    return {} unless keg_cache
+    json_string_to_ruby_hash(keg_cache)["hash_values"][type.to_s]
   end
 
   # Formats the linkage data for `array_values` into a kind which can be parsed

Library/Homebrew/linkage_checker.rb

@@ -3,13 +3,13 @@ require "formula"
 require "linkage_cache_store"
 
 class LinkageChecker
-  def initialize(keg, database_cache, use_cache = false, formula = nil)
+  def initialize(keg, formula = nil, use_cache: false, cache_db:)
     @keg = keg
     @formula = formula || resolve_formula(keg)
 
     if use_cache
-      @store = LinkageStore.new(keg.name, database_cache.db)
+      @store = LinkageCacheStore.new(keg.name, cache_db)
-      flush_cache_and_check_dylibs unless database_cache.db.key?(keg.name)
+      flush_cache_and_check_dylibs unless @store.has_keg_name?
     else
       flush_cache_and_check_dylibs
     end
@@ -50,7 +50,7 @@ class LinkageChecker
   end
 
   def undeclared_deps
-    @undeclared_deps ||= store.nil? ? [] : store.fetch_type(:undeclared_deps)
+    @undeclared_deps ||= store.fetch_type(:undeclared_deps)
   end
 
   private
@@ -60,37 +60,37 @@ class LinkageChecker
   # 'Hash-type' cache values
 
   def brewed_dylibs
-    @brewed_dylibs ||= store.nil? ? {} : store.fetch_type(:brewed_dylibs)
+    @brewed_dylibs ||= store.fetch_type(:brewed_dylibs)
   end
 
   def reverse_links
-    @reverse_links ||= store.nil? ? {} : store.fetch_type(:reverse_links)
+    @reverse_links ||= store.fetch_type(:reverse_links)
   end
 
   def broken_deps
-    @broken_deps ||= store.nil? ? {} : store.fetch_type(:broken_deps)
+    @broken_deps ||= store.fetch_type(:broken_deps)
   end
 
   # 'Path-type' cached values
 
   def system_dylibs
-    @system_dylibs ||= store.nil? ? [] : store.fetch_type(:system_dylibs)
+    @system_dylibs ||= store.fetch_type(:system_dylibs)
   end
 
   def broken_dylibs
-    @broken_dylibs ||= store.nil? ? [] : store.fetch_type(:broken_dylibs)
+    @broken_dylibs ||= store.fetch_type(:broken_dylibs)
   end
 
   def variable_dylibs
-    @variable_dylibs ||= store.nil? ? [] : store.fetch_type(:variable_dylibs)
+    @variable_dylibs ||= store.fetch_type(:variable_dylibs)
   end
 
   def indirect_deps
-    @indirect_deps ||= store.nil? ? [] : store.fetch_type(:indirect_deps)
+    @indirect_deps ||= store.fetch_type(:indirect_deps)
   end
 
   def unnecessary_deps
-    @unnecessary_deps ||= store.nil? ? [] : store.fetch_type(:unnecessary_deps)
+    @unnecessary_deps ||= store.fetch_type(:unnecessary_deps)
   end
 
   def dylib_to_dep(dylib)
@@ -108,7 +108,8 @@ class LinkageChecker
 
       # weakly loaded dylibs may not actually exist on disk, so skip them
       # when checking for broken linkage
-      file.dynamically_linked_libraries(except: :LC_LOAD_WEAK_DYLIB).each do |dylib|
+      file.dynamically_linked_libraries(except: :LC_LOAD_WEAK_DYLIB)
+          .each do |dylib|
         @reverse_links[dylib] << file
         next if checked_dylibs.include? dylib
         if dylib.start_with? "@"
@@ -139,7 +140,10 @@ class LinkageChecker
       end
     end
 
-    @indirect_deps, @undeclared_deps, @unnecessary_deps = check_undeclared_deps if formula
+    if formula
+      @indirect_deps, @undeclared_deps, @unnecessary_deps =
+        check_undeclared_deps
+    end
     store_dylibs!
   end