Merge pull request #2514 from reitermarkus/refactor-cask-metadata

Refactor cask metadata directory methods.

7 files changed, 92 insertions, 84 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/cask.rb b/Library/Homebrew/cask/lib/hbc/cask.rb
index cf5f2b37a..a193a394e 100644
--- a/Library/Homebrew/cask/lib/hbc/cask.rb
+++ b/Library/Homebrew/cask/lib/hbc/cask.rb
@@ -1,10 +1,12 @@
 require "forwardable"
 
 require "hbc/dsl"
+require "hbc/metadata"
 
 module Hbc
   class Cask
     extend Forwardable
+    include Metadata
 
     attr_reader :token, :sourcefile_path
     def initialize(token, sourcefile_path: nil, &block)
@@ -20,55 +22,9 @@ module Hbc
       define_method(method_name) { @dsl.send(method_name) }
     end
 
-    METADATA_SUBDIR = ".metadata".freeze
-
-    def metadata_master_container_path
-      @metadata_master_container_path ||= caskroom_path.join(METADATA_SUBDIR)
-    end
-
-    def metadata_versioned_container_path
-      cask_version = version ? version : :unknown
-      metadata_master_container_path.join(cask_version.to_s)
-    end
-
-    def metadata_path(timestamp = :latest, create = false)
-      if create && timestamp == :latest
-        raise CaskError, "Cannot create metadata path when timestamp is :latest"
-      end
-      path = if timestamp == :latest
-        Pathname.glob(metadata_versioned_container_path.join("*")).sort.last
-      elsif timestamp == :now
-        Utils.nowstamp_metadata_path(metadata_versioned_container_path)
-      else
-        metadata_versioned_container_path.join(timestamp)
-      end
-      if create
-        odebug "Creating metadata directory #{path}"
-        FileUtils.mkdir_p path
-      end
-      path
-    end
-
-    def metadata_subdir(leaf, timestamp = :latest, create = false)
-      if create && timestamp == :latest
-        raise CaskError, "Cannot create metadata subdir when timestamp is :latest"
-      end
-      unless leaf.respond_to?(:length) && !leaf.empty?
-        raise CaskError, "Cannot create metadata subdir for empty leaf"
-      end
-      parent = metadata_path(timestamp, create)
-      return nil unless parent.respond_to?(:join)
-      subdir = parent.join(leaf)
-      if create
-        odebug "Creating metadata subdirectory #{subdir}"
-        FileUtils.mkdir_p subdir
-      end
-      subdir
-    end
-
     def timestamped_versions
-      Pathname.glob(metadata_master_container_path.join("*", "*"))
-              .map { |p| p.relative_path_from(metadata_master_container_path) }
+      Pathname.glob(metadata_timestamped_path(version: "*", timestamp: "*"))
+              .map { |p| p.relative_path_from(p.parent.parent) }
               .sort_by(&:basename) # sort by timestamp
               .map { |p| p.split.map(&:to_s) }
     end
diff --git a/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb b/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
index 6887aaf4f..1ee3230ad 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/uninstall.rb
@@ -12,15 +12,9 @@ module Hbc
 
           raise CaskNotInstalledError, cask unless cask.installed? || force
 
-          latest_installed_version = cask.timestamped_versions.last
-
-          unless latest_installed_version.nil?
-            latest_installed_cask_file = cask.metadata_master_container_path
-                                             .join(latest_installed_version.join(File::Separator),
-                                                   "Casks", "#{cask_token}.rb")
-
+          if cask.installed? && !cask.installed_caskfile.nil?
             # use the same cask file that was used for installation, if possible
-            cask = CaskLoader.load_from_file(latest_installed_cask_file) if latest_installed_cask_file.exist?
+            cask = CaskLoader.load_from_file(cask.installed_caskfile) if cask.installed_caskfile.exist?
           end
 
           Installer.new(cask, force: force).uninstall
diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb
index 824c1b1be..26022953b 100644
--- a/Library/Homebrew/cask/lib/hbc/installer.rb
+++ b/Library/Homebrew/cask/lib/hbc/installer.rb
@@ -295,15 +295,13 @@ module Hbc
     end
 
     def save_caskfile
-      unless (old_savedirs = Pathname.glob(@cask.metadata_path("*"))).empty?
-        old_savedirs.each(&:rmtree)
-      end
+      old_savedir = @cask.metadata_timestamped_path
 
       return unless @cask.sourcefile_path
 
-      savedir = @cask.metadata_subdir("Casks", :now, true)
-      savedir.mkpath
+      savedir = @cask.metadata_subdir("Casks", timestamp: :now, create: true)
       FileUtils.copy @cask.sourcefile_path, savedir
+      old_savedir.rmtree unless old_savedir.nil?
     end
 
     def uninstall
@@ -355,15 +353,15 @@ module Hbc
       gain_permissions_remove(@cask.staged_path) if !@cask.staged_path.nil? && @cask.staged_path.exist?
 
       # Homebrew-Cask metadata
-      if @cask.metadata_versioned_container_path.respond_to?(:children) &&
-         @cask.metadata_versioned_container_path.exist?
-        @cask.metadata_versioned_container_path.children.each do |subdir|
+      if @cask.metadata_versioned_path.respond_to?(:children) &&
+         @cask.metadata_versioned_path.exist?
+        @cask.metadata_versioned_path.children.each do |subdir|
           unless PERSISTENT_METADATA_SUBDIRS.include?(subdir.basename)
             gain_permissions_remove(subdir)
           end
         end
       end
-      @cask.metadata_versioned_container_path.rmdir_if_possible
+      @cask.metadata_versioned_path.rmdir_if_possible
       @cask.metadata_master_container_path.rmdir_if_possible
 
       # toplevel staged distribution
diff --git a/Library/Homebrew/cask/lib/hbc/metadata.rb b/Library/Homebrew/cask/lib/hbc/metadata.rb
new file mode 100644
index 000000000..344c38cee
--- /dev/null
+++ b/Library/Homebrew/cask/lib/hbc/metadata.rb
@@ -0,0 +1,73 @@
+module Hbc
+  module Metadata
+    METADATA_SUBDIR = ".metadata".freeze
+
+    def metadata_master_container_path
+      @metadata_master_container_path ||= caskroom_path.join(METADATA_SUBDIR)
+    end
+
+    def metadata_versioned_path(version: self.version)
+      cask_version = (version || :unknown).to_s
+
+      if cask_version.empty?
+        raise CaskError, "Cannot create metadata path with empty version."
+      end
+
+      metadata_master_container_path.join(cask_version)
+    end
+
+    def metadata_timestamped_path(version: self.version, timestamp: :latest, create: false)
+      if create && timestamp == :latest
+        raise CaskError, "Cannot create metadata path when timestamp is :latest."
+      end
+
+      path = if timestamp == :latest
+        Pathname.glob(metadata_versioned_path(version: version).join("*")).sort.last
+      else
+        timestamp = new_timestamp if timestamp == :now
+        metadata_versioned_path(version: version).join(timestamp)
+      end
+
+      if create && !path.directory?
+        odebug "Creating metadata directory #{path}."
+        path.mkpath
+      end
+
+      path
+    end
+
+    def metadata_subdir(leaf, version: self.version, timestamp: :latest, create: false)
+      if create && timestamp == :latest
+        raise CaskError, "Cannot create metadata subdir when timestamp is :latest."
+      end
+
+      unless leaf.respond_to?(:empty?) && !leaf.empty?
+        raise CaskError, "Cannot create metadata subdir for empty leaf."
+      end
+
+      parent = metadata_timestamped_path(version: version, timestamp: timestamp, create: create)
+
+      return nil if parent.nil?
+
+      subdir = parent.join(leaf)
+
+      if create && !subdir.directory?
+        odebug "Creating metadata subdirectory #{subdir}."
+        subdir.mkpath
+      end
+
+      subdir
+    end
+
+    private
+
+    def new_timestamp(time = Time.now)
+      time = time.utc
+
+      timestamp = time.strftime("%Y%m%d%H%M%S")
+      fraction = format("%.3f", time.to_f - time.to_i)[1..-1]
+
+      timestamp.concat(fraction)
+    end
+  end
+end
diff --git a/Library/Homebrew/cask/lib/hbc/utils.rb b/Library/Homebrew/cask/lib/hbc/utils.rb
index ecb565e8e..2a5acbc88 100644
--- a/Library/Homebrew/cask/lib/hbc/utils.rb
+++ b/Library/Homebrew/cask/lib/hbc/utils.rb
@@ -115,16 +115,5 @@ module Hbc
 
       opoo(poo.join(" ") + "\n" + error_message_with_suggestions)
     end
-
-    def self.nowstamp_metadata_path(container_path)
-      @timenow ||= Time.now.gmtime
-      return unless container_path.respond_to?(:join)
-
-      precision = 3
-      timestamp = @timenow.strftime("%Y%m%d%H%M%S")
-      fraction = format("%.#{precision}f", @timenow.to_f - @timenow.to_i)[1..-1]
-      timestamp.concat(fraction)
-      container_path.join(timestamp)
-    end
   end
 end
diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb
index 2ab966f82..3736f3c01 100644
--- a/Library/Homebrew/test/cask/cask_spec.rb
+++ b/Library/Homebrew/test/cask/cask_spec.rb
@@ -85,8 +85,8 @@ describe Hbc::Cask, :cask do
     it "proposes a versioned metadata directory name for each instance" do
       cask_token = "local-caffeine"
       c = Hbc::CaskLoader.load(cask_token)
-      metadata_path = Hbc.caskroom.join(cask_token, ".metadata", c.version)
-      expect(c.metadata_versioned_container_path.to_s).to eq(metadata_path.to_s)
+      metadata_timestamped_path = Hbc.caskroom.join(cask_token, ".metadata", c.version)
+      expect(c.metadata_versioned_path.to_s).to eq(metadata_timestamped_path.to_s)
     end
   end
 
diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb
index 7dd5b2bda..59d61bbdd 100644
--- a/Library/Homebrew/test/cask/installer_spec.rb
+++ b/Library/Homebrew/test/cask/installer_spec.rb
@@ -336,9 +336,8 @@ describe Hbc::Installer, :cask do
         Hbc::Installer.new(caffeine).install
       end
 
-      m_path = caffeine.metadata_path(:now, true)
-      expect(caffeine.metadata_path(:now, false)).to eq(m_path)
-      expect(caffeine.metadata_path(:latest)).to eq(m_path)
+      m_path = caffeine.metadata_timestamped_path(timestamp: :now, create: true)
+      expect(caffeine.metadata_timestamped_path(timestamp: :latest)).to eq(m_path)
     end
 
     it "generates and finds a metadata subdirectory for an installed Cask" do
@@ -349,9 +348,8 @@ describe Hbc::Installer, :cask do
       end
 
       subdir_name = "Casks"
-      m_subdir = caffeine.metadata_subdir(subdir_name, :now, true)
-      expect(caffeine.metadata_subdir(subdir_name, :now, false)).to eq(m_subdir)
-      expect(caffeine.metadata_subdir(subdir_name, :latest)).to eq(m_subdir)
+      m_subdir = caffeine.metadata_subdir(subdir_name, timestamp: :now, create: true)
+      expect(caffeine.metadata_subdir(subdir_name, timestamp: :latest)).to eq(m_subdir)
     end
   end