Merge pull request #4984 from MikeMcQuaid/json-cache-store

cache_store: use JSON.

Library/Homebrew/cache_store.rb

@@ -1,6 +1,4 @@
-require "dbm"
 require "json"
-require "timeout"
 
 #
 # `CacheStoreDatabase` acts as an interface to a persistent storage mechanism
@@ -38,9 +36,10 @@ class CacheStoreDatabase
     db.delete(key)
   end
 
-  # Closes the underlying database (if it created and open).
+  # Closes the underlying database (if it is created and open).
   def close_if_open!
-    @db&.close
+    return unless @db
+    cache_path.atomic_write(JSON.dump(@db))
   end
 
   # Returns `true` if the cache file has been created for the given `@type`
@@ -52,55 +51,18 @@ class CacheStoreDatabase
 
   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
-
-  # Spend 5 seconds trying to read the DBM file. If it takes longer than this it
-  # has likely hung or segfaulted.
-  DBM_TEST_READ_TIMEOUT = 5
-
   # 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
   #
-  # @return [DBM] db
+  # @return [Hash] db
   def db
-    # DBM::WRCREAT: Creates the database if it does not already exist
     @db ||= begin
-      HOMEBREW_CACHE.mkpath
-      if created?
-        dbm_test_read_cmd = SystemCommand.new(
-          ENV["HOMEBREW_RUBY_PATH"],
-          args: [
-            "-rdbm",
-            "-e",
-            "DBM.open('#{dbm_file_path}', #{DATABASE_MODE}, DBM::READER).values.size",
-          ],
-          print_stderr: false,
-          must_succeed: true,
-        )
-        dbm_test_read_success = begin
-          Timeout.timeout(DBM_TEST_READ_TIMEOUT) do
-            dbm_test_read_cmd.run!
-            true
-          end
-        rescue ErrorDuringExecution, Timeout::Error
-          odebug "Failed to read #{dbm_file_path}!"
-          begin
-            Process.kill(:KILL, dbm_test_read_cmd.pid)
-          rescue Errno::ESRCH
-            # Process has already terminated.
-            nil
-          end
-          false
-        end
-        cache_path.delete unless dbm_test_read_success
-      end
-      DBM.open(dbm_file_path, DATABASE_MODE, DBM::WRCREAT)
+      JSON.parse(cache_path.read) if created?
+    rescue JSON::ParserError
+      nil
     end
+    @db ||= {}
   end
 
   # Creates a CacheStoreDatabase
@@ -111,20 +73,12 @@ class CacheStoreDatabase
     @type = type
   end
 
-  # `DBM` appends `.db` file extension to the path provided, which is why it's
-  # not included
-  #
-  # @return [String]
-  def dbm_file_path
-    "#{HOMEBREW_CACHE}/#{@type}"
-  end
-
   # The path where the database resides in the `HOMEBREW_CACHE` for the given
   # `@type`
   #
   # @return [String]
   def cache_path
-    Pathname("#{dbm_file_path}.db")
+    HOMEBREW_CACHE/"#{@type}.json"
   end
 end
 
@@ -166,21 +120,4 @@ class CacheStore
 
   # @return [CacheStoreDatabase]
   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
-  # into a JSON compatible string in `ruby_hash_to_json_string`, where it may
-  # later be parsed by `JSON.parse` in the `json_string_to_ruby_hash` method
-  #
-  # @param  [Hash] ruby `Hash` to be converted to `JSON` string
-  # @return [String]
-  def ruby_hash_to_json_string(hash)
-    hash.to_json
-  end
-
-  # @param  [String] `JSON` string to be converted to ruby `Hash`
-  # @return [Hash]
-  def json_string_to_ruby_hash(string)
-    JSON.parse(string)
-  end
 end

Library/Homebrew/linkage_cache_store.rb

@@ -35,7 +35,7 @@ class LinkageCacheStore < CacheStore
       EOS
     end
 
-    database.set @keg_path, ruby_hash_to_json_string(hash_values)
+    database.set @keg_path, hash_values
   end
 
   # @param  [Symbol] the type to fetch from the `LinkageCacheStore`
@@ -68,6 +68,6 @@ class LinkageCacheStore < CacheStore
     keg_cache = database.get(@keg_path)
     return {} unless keg_cache
 
-    json_string_to_ruby_hash(keg_cache)[type.to_s]
+    keg_cache[type.to_s]
   end
 end

Library/Homebrew/test/cache_store_spec.rb

@@ -97,10 +97,6 @@ describe CacheStoreDatabase do
 
   describe "#close_if_open!" do
     context "database open" do
-      before do
-        subject.instance_variable_set(:@db, instance_double(DBM, close: nil))
-      end
-
       it "does not raise an error when `close` is called on the database" do
         expect { subject.close_if_open! }.not_to raise_error(NoMethodError)
       end
@@ -118,7 +114,7 @@ describe CacheStoreDatabase do
   end
 
   describe "#created?" do
-    let(:cache_path) { Pathname("path/to/homebrew/cache/sample.db") }
+    let(:cache_path) { Pathname("path/to/homebrew/cache/sample.json") }
 
     before do
       allow(subject).to receive(:cache_path).and_return(cache_path)