Better group assets for clean (#2863)

For cleaning, group assets by paths with removing the hash part and
sort them with mtime within each group to determine their versions.

Author: zunda

lib/webpacker/commands.rb

@@ -17,20 +17,21 @@ def initialize(webpacker)
   #
   def clean(count = 2, age = 3600)
     if config.public_output_path.exist? && config.public_manifest_path.exist?
-      versions
-        .sort
-        .reverse
-        .each_with_index
-        .drop_while do |(mtime, _), index|
-          max_age = [0, Time.now - Time.at(mtime)].max
-          max_age < age || index < count
+      packs
+        .map do |paths|
+          paths.map { |path| [Time.now - File.mtime(path), path] }
+          .sort
+          .reject.with_index do |(file_age, _), index|
+            file_age < age || index < count
+          end
+          .map { |_, path| path }
         end
-        .each do |(_, files), index|
-          files.each do |file|
-            if File.file?(file)
-              File.delete(file)
-              logger.info "Removed #{file}"
-            end
+        .flatten
+        .compact
+        .each do |file|
+          if File.file?(file)
+            File.delete(file)
+            logger.info "Removed #{file}"
           end
         end
     end
@@ -54,12 +55,15 @@ def compile
   end
 
   private
-    def versions
+    def packs
       all_files       = Dir.glob("#{config.public_output_path}/**/*")
       manifest_config = Dir.glob("#{config.public_manifest_path}*")
 
       packs = all_files - manifest_config - current_version
-      packs.reject { |file| File.directory?(file) }.group_by { |file| File.mtime(file).utc.to_i }
+      packs.reject { |file| File.directory?(file) }.group_by do |path|
+        base, _, ext = File.basename(path).scan(/(.*)(-[\da-f]+)(\.\w+)/).flatten
+        "#{File.dirname(path)}/#{base}#{ext}"
+      end.values
     end
 
     def current_version

test/command_test.rb

@@ -31,3 +31,79 @@ def test_clean_command_works_with_nested_hashes_and_without_any_compiled_files
     end
   end
 end
+
+class ClearCommandVersioningTest < Minitest::Test
+  def setup
+    @now = Time.parse("2021-01-01 12:34:56 UTC")
+    # Test assets to be kept and deleted, path and mtime
+    @prev_files = {
+      # recent versions to be kept with Webpacker.commands.clean(count = 2)
+      "js/application-deadbeef.js" => @now - 4000,
+      "js/common-deadbeee.js" => @now - 4002,
+      "css/common-deadbeed.css" => @now - 4004,
+      "media/images/logo-deadbeeb.css" => @now - 4006,
+      "js/application-1eadbeef.js" => @now - 8000,
+      "js/common-1eadbeee.js" => @now - 8002,
+      "css/common-1eadbeed.css" => @now - 8004,
+      "media/images/logo-1eadbeeb.css" => @now - 8006,
+      # new files to be kept with Webpacker.commands.clean(age = 3600)
+      "js/brandnew-0001.js" => @now,
+      "js/brandnew-0002.js" => @now - 10,
+      "js/brandnew-0003.js" => @now - 20,
+      "js/brandnew-0004.js" => @now - 40,
+    }.transform_keys { |path| "#{Webpacker.config.public_output_path}/#{path}" }
+    @expired_files = {
+      # old files that are outside count = 2 or age = 3600 and to be deleted
+      "js/application-0eadbeef.js" => @now - 9000,
+      "js/common-0eadbeee.js" => @now - 9002,
+      "css/common-0eadbeed.css" => @now - 9004,
+      "js/brandnew-0005.js" => @now - 3640,
+    }.transform_keys { |path| "#{Webpacker.config.public_output_path}/#{path}" }
+    @all_files = @prev_files.merge(@expired_files)
+    @dir_glob_stub = Proc.new { |arg|
+      case arg
+      when "#{Webpacker.config.public_output_path}/**/*"
+        @all_files.keys
+      else
+        []
+      end
+    }
+    @file_mtime_stub = Proc.new { |longpath|
+      @all_files[longpath]
+    }
+    @file_delete_mock = Minitest::Mock.new
+    @expired_files.keys.each do |longpath|
+      @file_delete_mock.expect(:delete, 1, [longpath])
+    end
+    @file_delete_stub = Proc.new { |longpath|
+      if @prev_files.has_key?(longpath)
+        flunk "#{longpath} should not be deleted"
+      else
+        @file_delete_mock.delete(longpath)
+      end
+    }
+  end
+
+  def time_and_files_stub(&proc)
+    Time.stub :now, @now do
+      Dir.stub :glob, @dir_glob_stub do
+        File.stub :directory?, false do
+          File.stub :file?, true do
+            File.stub :mtime, @file_mtime_stub do
+              File.stub :delete, @file_delete_stub do
+                yield proc
+              end
+            end
+          end
+        end
+      end
+    end
+    @file_delete_mock.verify
+  end
+
+  def test_clean_command_with_versioned_files
+    time_and_files_stub do
+      assert Webpacker.commands.clean
+    end
+  end
+end