Improve coverage of Propshaft::Manifest

rafaelfranca · rafaelfranca · commit 11bab84c1314 · 2025-07-04T23:49:11.000Z
Also do some edits in the other files to make the code look better.
diff --git a/lib/propshaft/assembly.rb b/lib/propshaft/assembly.rb
@@ -17,7 +17,6 @@ def initialize(config)
   end
 
   def load_path
-
     @load_path ||= Propshaft::LoadPath.new(
       config.paths,
       compilers: compilers,
diff --git a/lib/propshaft/asset.rb b/lib/propshaft/asset.rb
@@ -34,10 +34,10 @@ def digest
     @digest ||= Digest::SHA1.hexdigest("#{content_with_compile_references}#{load_path.version}").first(8)
   end
 
-  # Following the Subresource Integrity spec draft
-  # https://w3c.github.io/webappsec-subresource-integrity/
-  # allowing only sha256, sha384, and sha512
   def integrity(hash_algorithm:)
+    # Following the Subresource Integrity spec draft
+    # https://w3c.github.io/webappsec-subresource-integrity/
+    # allowing only sha256, sha384, and sha512
     bitlen = case hash_algorithm
       when "sha256"
         256
diff --git a/lib/propshaft/load_path.rb b/lib/propshaft/load_path.rb
@@ -42,7 +42,7 @@ def asset_paths_by_glob(glob)
   end
 
   def manifest
-    Propshaft::Manifest.new(integrity_hash_algorithm:).tap do |manifest|
+    Propshaft::Manifest.new(integrity_hash_algorithm: integrity_hash_algorithm).tap do |manifest|
       assets.each { |asset| manifest.push_asset(asset) }
     end
   end
diff --git a/lib/propshaft/manifest.rb b/lib/propshaft/manifest.rb
@@ -2,14 +2,41 @@ module Propshaft
   class Manifest
     class ManifestEntry
       attr_reader :logical_path, :digested_path, :integrity
+
       def initialize(logical_path:, digested_path:, integrity:)
         @logical_path = logical_path
         @digested_path = digested_path
         @integrity = integrity
       end
 
       def to_h
-        { digested_path:, integrity: }
+        { digested_path: digested_path, integrity: integrity}
+      end
+    end
+
+    class << self
+      def from_path(manifest_path)
+        manifest = Manifest.new
+
+        serialized_manifest = JSON.parse(manifest_path.read, symbolize_names: false)
+
+        serialized_manifest.each_pair do |key, value|
+          # Compatibility mode to be able to
+          # read the old "simple manifest" format
+          digested_path, integrity = if value.is_a?(String)
+            [value, nil]
+          else
+            [value["digested_path"], value["integrity"]]
+          end
+
+          entry = ManifestEntry.new(
+            logical_path: key, digested_path: digested_path, integrity: integrity
+          )
+
+          manifest.push(entry)
+        end
+
+        manifest
       end
     end
 
@@ -22,7 +49,7 @@ def push_asset(asset)
       entry = ManifestEntry.new(
         logical_path: asset.logical_path.to_s,
         digested_path: asset.digested_path.to_s,
-        integrity: @integrity_hash_algorithm && asset.integrity(hash_algorithm: @integrity_hash_algorithm)
+        integrity: integrity_hash_algorithm && asset.integrity(hash_algorithm: integrity_hash_algorithm)
       )
 
       push(entry)
@@ -31,45 +58,19 @@ def push_asset(asset)
     def push(entry)
       @entries[entry.logical_path] = entry
     end
-
-    def <<(asset)
-      push(asset)
-    end
+    alias_method :<<, :push
 
     def [](logical_path)
       @entries[logical_path]
     end
 
     def to_json
-      Hash.new.tap do |serialized_manifest|
-        @entries.values.each do |manifest_entry|
-          serialized_manifest[manifest_entry.logical_path] = manifest_entry.to_h
-        end
+      @entries.transform_values do |manifest_entry|
+        manifest_entry.to_h
       end.to_json
     end
 
-    class << self
-      def from_path(manifest_path)
-        Manifest.new.tap do |manifest|
-          JSON.parse(manifest_path.read, symbolize_names: false).tap do |serialized_manifest|
-            serialized_manifest.each_pair do |key, value|
-              # Compatibility mode to be able to
-              # read the old "simple manifest" format
-              digested_path, integrity = if value.is_a?(String)
-                [value, nil]
-              else
-                [value["digested_path"], value["integrity"]]
-              end
-
-              entry = ManifestEntry.new(
-                logical_path: key, digested_path:, integrity:
-              )
-
-              manifest.push(entry)
-            end
-          end
-        end
-      end
-    end
+    private
+      attr_reader :integrity_hash_algorithm
   end
 end
diff --git a/lib/propshaft/resolver/dynamic.rb b/lib/propshaft/resolver/dynamic.rb
@@ -7,14 +7,15 @@ def initialize(load_path:, prefix:)
     end
 
     def resolve(logical_path)
-      if asset = load_path.find(logical_path)
+      if asset = find_asset(logical_path)
         File.join prefix, asset.digested_path
       end
     end
 
     def integrity(logical_path)
       hash_algorithm = load_path.integrity_hash_algorithm
-      if (asset = load_path.find(logical_path)) && hash_algorithm
+
+      if hash_algorithm && (asset = find_asset(logical_path))
         asset.integrity(hash_algorithm:)
       end
     end
@@ -24,5 +25,10 @@ def read(logical_path, options = {})
         asset.content(**options)
       end
     end
+
+    private
+      def find_asset(logical_path)
+        load_path.find(logical_path)
+      end
   end
 end
diff --git a/test/fixtures/new_manifest_format/.manifest.json b/test/fixtures/new_manifest_format/.manifest.json
diff --git a/test/fixtures/new_manifest_format/one-f2e1ec14.txt b/test/fixtures/new_manifest_format/one-f2e1ec14.txt
@@ -0,0 +1 @@
+One from first path
diff --git a/test/fixtures/new_manifest_format/one-f2e1ec15.txt.map b/test/fixtures/new_manifest_format/one-f2e1ec15.txt.map
@@ -0,0 +1 @@
+One from first path map
diff --git a/test/propshaft/assembly_test.rb b/test/propshaft/assembly_test.rb
@@ -4,7 +4,7 @@
 
 class Propshaft::AssemblyTest < ActiveSupport::TestCase
   test "uses static resolver when manifest is present" do
-    assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config| 
+    assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config|
       config.output_path = Pathname.new("#{__dir__}/../fixtures/output")
       config.manifest_path = config.output_path.join(".manifest.json")
       config.prefix = "/assets"
@@ -14,7 +14,7 @@ class Propshaft::AssemblyTest < ActiveSupport::TestCase
   end
 
   test "uses dynamic resolver when manifest is missing" do
-    assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config| 
+    assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config|
       config.output_path = Pathname.new("#{__dir__}/../fixtures/assets")
       config.manifest_path = config.output_path.join(".manifest.json")
       config.prefix = "/assets"
@@ -43,30 +43,4 @@ class Propshaft::AssemblyTest < ActiveSupport::TestCase
 
     assert assembly.processor.is_a?(Propshaft::Processor)
   end
-
-  class Propshaft::AssemblyTest::WithExtensibleManifest < ActiveSupport::TestCase
-    test "uses static resolver when manifest is present" do
-      assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config|
-        config.output_path = Pathname.new("#{__dir__}/../fixtures/output")
-        config.manifest_path = config.output_path.join(".extensible_manifest.json")
-        config.prefix = "/assets"
-
-        config.integrity_hash_algorithm = "sha384"
-      })
-
-      assert assembly.resolver.is_a?(Propshaft::Resolver::Static)
-    end
-
-    test "uses dynamic resolver when manifest is missing" do
-      assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config|
-        config.output_path = Pathname.new("#{__dir__}/../fixtures/assets")
-        config.manifest_path = config.output_path.join(".extensible_manifest.json")
-        config.prefix = "/assets"
-
-        config.integrity_hash_algorithm = "sha384"
-      })
-
-      assert assembly.resolver.is_a?(Propshaft::Resolver::Dynamic)
-    end
-  end
 end
diff --git a/test/propshaft/asset_test.rb b/test/propshaft/asset_test.rb
@@ -64,7 +64,6 @@ class Propshaft::AssetTest < ActiveSupport::TestCase
     exception = assert_raises StandardError do
       find_asset("one.txt").integrity(hash_algorithm: "md5")
     end
-
     assert_equal "Subresource Integrity hash algorithm must be one of SHA2 family (sha256, sha384, sha512)", exception.message
   end
 
diff --git a/test/propshaft/manifest_test.rb b/test/propshaft/manifest_test.rb
@@ -28,6 +28,94 @@ class Propshaft::ManifestTest < ActiveSupport::TestCase
     assert_nil manifest_entry["integrity"]
   end
 
+  test "loads from new extensible manifest format" do
+    manifest_path = Pathname.new("#{__dir__}/../fixtures/new_manifest_format/.manifest.json")
+    manifest = Propshaft::Manifest.from_path(manifest_path)
+
+    entry = manifest["one.txt"]
+    assert_not_nil entry
+    assert_equal "one.txt", entry.logical_path
+    assert_equal "one-f2e1ec14.txt", entry.digested_path
+    assert_equal "sha384-LdS8l2QTAF8bD8WPb8QSQv0skTWHhmcnS2XU5LBkVQneGzqIqnDRskQtJvi7ADMe", entry.integrity
+  end
+
+  test "loads from old simple manifest format" do
+    manifest_path = Pathname.new("#{__dir__}/../fixtures/output/.manifest.json")
+    manifest = Propshaft::Manifest.from_path(manifest_path)
+
+    entry = manifest["one.txt"]
+    assert_not_nil entry
+    assert_equal "one.txt", entry.logical_path
+    assert_equal "one-f2e1ec14.txt", entry.digested_path
+    assert_nil entry.integrity
+  end
+
+  test "push method adds entry to manifest" do
+    manifest = Propshaft::Manifest.new
+    entry = Propshaft::Manifest::ManifestEntry.new(
+      logical_path: "test.js",
+      digested_path: "test-abc123.js",
+      integrity: "sha384-test"
+    )
+
+    manifest.push(entry)
+    retrieved_entry = manifest["test.js"]
+
+    assert_equal entry, retrieved_entry
+    assert_equal "test.js", retrieved_entry.logical_path
+    assert_equal "test-abc123.js", retrieved_entry.digested_path
+    assert_equal "sha384-test", retrieved_entry.integrity
+  end
+
+  test "<< alias works for push method" do
+    manifest = Propshaft::Manifest.new
+    entry = Propshaft::Manifest::ManifestEntry.new(
+      logical_path: "test.css",
+      digested_path: "test-def456.css",
+      integrity: nil
+    )
+
+    manifest << entry
+    retrieved_entry = manifest["test.css"]
+
+    assert_equal entry, retrieved_entry
+    assert_equal "test.css", retrieved_entry.logical_path
+    assert_equal "test-def456.css", retrieved_entry.digested_path
+    assert_nil retrieved_entry.integrity
+  end
+
+  test "[] accessor returns nil for missing entries" do
+    manifest = Propshaft::Manifest.new
+    assert_nil manifest["nonexistent.js"]
+  end
+
+  test "push_asset method creates entry from asset" do
+    manifest = Propshaft::Manifest.new(integrity_hash_algorithm: "sha384")
+    asset = find_asset("one.txt")
+
+    manifest.push_asset(asset)
+    entry = manifest["one.txt"]
+
+    assert_not_nil entry
+    assert_equal "one.txt", entry.logical_path
+    assert_equal "one-f2e1ec14.txt", entry.digested_path
+    assert_not_nil entry.integrity
+    assert entry.integrity.start_with?("sha384-")
+  end
+
+  test "push_asset without integrity algorithm" do
+    manifest = Propshaft::Manifest.new
+    asset = find_asset("one.txt")
+
+    manifest.push_asset(asset)
+    entry = manifest["one.txt"]
+
+    assert_not_nil entry
+    assert_equal "one.txt", entry.logical_path
+    assert_equal "one-f2e1ec14.txt", entry.digested_path
+    assert_nil entry.integrity
+  end
+
   private
     def create_manifest(integrity_hash_algorithm = nil)
       Propshaft::Manifest.new(integrity_hash_algorithm:).tap do |manifest|
diff --git a/test/propshaft/output_path_test.rb b/test/propshaft/output_path_test.rb
@@ -8,7 +8,6 @@ class Propshaft::OutputPathTest < ActiveSupport::TestCase
   setup do
     @manifest    = {
       ".manifest.json": ".manifest.json",
-      ".extensible_manifest.json": ".extensible_manifest.json",
       "one.txt": "one-f2e1ec14.txt",
       "one.txt.map": "one-f2e1ec15.txt.map"
     }.stringify_keys
diff --git a/test/propshaft/processor_test.rb b/test/propshaft/processor_test.rb
diff --git a/test/propshaft/resolver/dynamic_test.rb b/test/propshaft/resolver/dynamic_test.rb
diff --git a/test/propshaft/resolver/static_test.rb b/test/propshaft/resolver/static_test.rb
