Digests of files that can have dependencies on other files in the loa…

…d path need to reflect those dependencies (#188)

* Digests of files that can have dependencies on other files in the load path need to reflect those dependencies

benchmarks/css_asset_urls

@@ -25,5 +25,5 @@ compiler = Propshaft::Compiler::CssAssetUrls.new(assembly)
 
 Benchmark.ips do |x|
   x.config(time: 5, warmup: 2)
-  x.report("compile") { compiler.compile(asset.logical_path, asset.content) }
+  x.report("compile") { compiler.compile(asset, asset.content) }
 end

lib/propshaft/assembly.rb

@@ -15,7 +15,7 @@ def initialize(config)
   end
 
   def load_path
-    @load_path ||= Propshaft::LoadPath.new(config.paths, version: config.version)
+    @load_path ||= Propshaft::LoadPath.new(config.paths, compilers: compilers, version: config.version)
   end
 
   def resolver

lib/propshaft/asset.rb

@@ -2,10 +2,10 @@
 require "action_dispatch/http/mime_type"
 
 class Propshaft::Asset
-  attr_reader :path, :logical_path, :version
+  attr_reader :path, :logical_path, :load_path
 
-  def initialize(path, logical_path:, version: nil)
-    @path, @logical_path, @version = path, Pathname.new(logical_path), version
+  def initialize(path, logical_path:, load_path:)
+    @path, @logical_path, @load_path = path, Pathname.new(logical_path), load_path
   end
 
   def content
@@ -21,7 +21,7 @@ def length
   end
 
   def digest
-    @digest ||= Digest::SHA1.hexdigest("#{content}#{version}").first(8)
+    @digest ||= Digest::SHA1.hexdigest("#{content_with_compile_references}#{load_path.version}").first(8)
   end
 
   def digested_path
@@ -41,6 +41,10 @@ def ==(other_asset)
   end
 
   private
+    def content_with_compile_references
+      content + load_path.find_referenced_by(self).collect(&:content).join
+    end
+
     def already_digested?
       logical_path.to_s =~ /-([0-9a-zA-Z_-]{7,128})\.digested/
     end

lib/propshaft/compiler.rb

@@ -3,18 +3,23 @@
 # Base compiler from which other compilers can inherit
 class Propshaft::Compiler
   attr_reader :assembly
+  delegate :config, :load_path, to: :assembly
 
   def initialize(assembly)
     @assembly = assembly
   end
 
   # Override this in a specific compiler
-  def compile(logical_path, input)
+  def compile(asset, input)
     raise NotImplementedError
   end
 
+  def referenced_by(asset)
+    Set.new
+  end
+
   private
     def url_prefix
-      @url_prefix ||= File.join(assembly.config.relative_url_root.to_s, assembly.config.prefix.to_s).chomp("/")
+      @url_prefix ||= File.join(config.relative_url_root.to_s, config.prefix.to_s).chomp("/")
     end
 end

lib/propshaft/compiler/css_asset_urls.rb

@@ -5,8 +5,21 @@
 class Propshaft::Compiler::CssAssetUrls < Propshaft::Compiler
   ASSET_URL_PATTERN = /url\(\s*["']?(?!(?:\#|%23|data|http|\/\/))([^"'\s?#)]+)([#?][^"')]+)?\s*["']?\)/
 
-  def compile(logical_path, input)
-    input.gsub(ASSET_URL_PATTERN) { asset_url resolve_path(logical_path.dirname, $1), logical_path, $2, $1 }
+  def compile(asset, input)
+    input.gsub(ASSET_URL_PATTERN) { asset_url resolve_path(asset.logical_path.dirname, $1), asset.logical_path, $2, $1 }
+  end
+
+  def referenced_by(asset, references: Set.new)
+    asset.content.scan(ASSET_URL_PATTERN).each do |referenced_asset_url, _|
+      referenced_asset = load_path.find(resolve_path(asset.logical_path.dirname, referenced_asset_url))
+
+      if referenced_asset && references.exclude?(referenced_asset)
+        references << referenced_asset
+        references.merge referenced_by(referenced_asset, references: references)
+      end
+    end
+
+    references
   end
 
   private
@@ -21,7 +34,7 @@ def resolve_path(directory, filename)
     end
 
     def asset_url(resolved_path, logical_path, fingerprint, pattern)
-      if asset = assembly.load_path.find(resolved_path)
+      if asset = load_path.find(resolved_path)
         %[url("#{url_prefix}/#{asset.digested_path}#{fingerprint}")]
       else
         Propshaft.logger.warn "Unable to resolve '#{pattern}' for missing asset '#{resolved_path}' in #{logical_path}"

lib/propshaft/compiler/source_mapping_urls.rb

@@ -5,8 +5,8 @@
 class Propshaft::Compiler::SourceMappingUrls < Propshaft::Compiler
   SOURCE_MAPPING_PATTERN = %r{(//|/\*)# sourceMappingURL=(.+\.map)(\s*?\*\/)?\s*?\Z}
 
-  def compile(logical_path, input)
-    input.gsub(SOURCE_MAPPING_PATTERN) { source_mapping_url(logical_path, asset_path($2, logical_path), $1, $3) }
+  def compile(asset, input)
+    input.gsub(SOURCE_MAPPING_PATTERN) { source_mapping_url(asset.logical_path, asset_path($2, asset.logical_path), $1, $3) }
   end
 
   private
@@ -21,7 +21,7 @@ def asset_path(source_mapping_url, logical_path)
     end
 
     def source_mapping_url(logical_path, resolved_path, comment_start, comment_end)
-      if asset = assembly.load_path.find(resolved_path)
+      if asset = load_path.find(resolved_path)
         "#{comment_start}# sourceMappingURL=#{url_prefix}/#{asset.digested_path}#{comment_end}"
       else
         Propshaft.logger.warn "Removed sourceMappingURL comment for missing asset '#{resolved_path}' from #{logical_path}"

lib/propshaft/compilers.rb

@@ -23,11 +23,21 @@ def compile(asset)
     if relevant_registrations = registrations[asset.content_type.to_s]
       asset.content.dup.tap do |input|
         relevant_registrations.each do |compiler|
-          input.replace compiler.new(assembly).compile(asset.logical_path, input)
+          input.replace compiler.new(assembly).compile(asset, input)
         end
       end
     else
       asset.content
     end
   end
+
+  def referenced_by(asset)
+    Set.new.tap do |references|
+      if relevant_registrations = registrations[asset.content_type.to_s]
+        relevant_registrations.each do |compiler|
+          references.merge compiler.new(assembly).referenced_by(asset)
+        end
+      end
+    end
+  end
 end

lib/propshaft/load_path.rb

@@ -1,17 +1,20 @@
 require "propshaft/asset"
 
 class Propshaft::LoadPath
-  attr_reader :paths, :version
+  attr_reader :paths, :compilers, :version
 
-  def initialize(paths = [], version: nil)
-    @paths   = dedup(paths)
-    @version = version
+  def initialize(paths = [], compilers:, version: nil)
+    @paths, @compilers, @version = dedup(paths), compilers, version
   end
 
   def find(asset_name)
     assets_by_path[asset_name]
   end
 
+  def find_referenced_by(asset)
+    compilers.referenced_by(asset).delete(self)
+  end
+
   def assets(content_types: nil)
     if content_types
       assets_by_path.values.select { |asset| asset.content_type.in?(content_types) }
@@ -48,7 +51,7 @@ def assets_by_path
         paths.each do |path|
           without_dotfiles(all_files_from_tree(path)).each do |file|
             logical_path = file.relative_path_from(path)
-            mapped[logical_path.to_s] ||= Propshaft::Asset.new(file, logical_path: logical_path, version: version)
+            mapped[logical_path.to_s] ||= Propshaft::Asset.new(file, logical_path: logical_path, load_path: self)
           end if path.exist?
         end
       end

test/fixtures/assets/first_path/dependent/a.css

@@ -0,0 +1 @@
+@import url('b.css')

test/fixtures/assets/first_path/dependent/b.css

@@ -0,0 +1,2 @@
+@import url('c.css')
+@import url('missing.css')

test/fixtures/assets/first_path/dependent/c.css

@@ -0,0 +1,5 @@
+@import url('a.css')
+
+p {
+  color: red;
+}

test/propshaft/asset_test.rb

@@ -54,10 +54,51 @@ class Propshaft::AssetTest < ActiveSupport::TestCase
     assert_equal asset.digest.object_id, asset.digest.object_id
   end
 
+  test "digest depends on first level of compiler dependency" do
+    open_asset_with_reset("dependent/b.css") do |asset_file|
+      digest_before_dependency_change = find_asset("dependent/a.css").digest
+
+      asset_file.write "changes!"
+      asset_file.flush
+
+      digest_after_dependency_change = find_asset("dependent/a.css").digest
+
+      assert_not_equal digest_before_dependency_change, digest_after_dependency_change
+    end
+  end
+
+  test "digest depends on second level of compiler dependency" do
+    open_asset_with_reset("dependent/c.css") do |asset_file|
+      digest_before_dependency_change = find_asset("dependent/a.css").digest
+
+      asset_file.write "changes!"
+      asset_file.flush
+
+      digest_after_dependency_change = find_asset("dependent/a.css").digest
+
+      assert_not_equal digest_before_dependency_change, digest_after_dependency_change
+    end
+  end
+
   private
     def find_asset(logical_path)
       root_path = Pathname.new("#{__dir__}/../fixtures/assets/first_path")
       path = root_path.join(logical_path)
-      Propshaft::Asset.new(path, logical_path: logical_path)
+
+      assembly = Propshaft::Assembly.new(ActiveSupport::OrderedOptions.new.tap { |config|
+        config.paths = [ root_path ]
+        config.compilers = [[ "text/css", Propshaft::Compiler::CssAssetUrls ]]
+      })
+
+      Propshaft::Asset.new(path, logical_path: logical_path, load_path: assembly.load_path)
+    end
+
+    def open_asset_with_reset(logical_path)
+      dependency_path = Pathname.new("#{__dir__}/../fixtures/assets/first_path/#{logical_path}")
+      existing_dependency_content = File.read(dependency_path)
+
+      File.open(dependency_path, "a") { |f| yield f }
+    ensure
+      File.write(dependency_path, existing_dependency_content)
     end
 end

test/propshaft/compiler/css_asset_urls_test.rb

@@ -130,10 +130,11 @@ def compile_asset_with_content(content)
       root_path    = Pathname.new("#{__dir__}/../../fixtures/assets/vendor")
       logical_path = "foobar/source/test.css"
 
-      asset     = Propshaft::Asset.new(root_path.join(logical_path), logical_path: logical_path)
+      assembly = Propshaft::Assembly.new(@options)
+      assembly.compilers.register "text/css", Propshaft::Compiler::CssAssetUrls
+
+      asset     = Propshaft::Asset.new(root_path.join(logical_path), logical_path: logical_path, load_path: assembly.load_path)
       asset.stub :content, content do
-        assembly = Propshaft::Assembly.new(@options)
-        assembly.compilers.register "text/css", Propshaft::Compiler::CssAssetUrls
         assembly.compilers.compile(asset)
       end
     end

test/propshaft/compilers_test.rb

@@ -20,6 +20,7 @@ class Propshaft::CompilersTest < ActiveSupport::TestCase
   private
     def find_asset(logical_path)
       root_path = Pathname.new("#{__dir__}/../fixtures/assets/first_path")
-      Propshaft::Asset.new(root_path.join(logical_path), logical_path: logical_path)
+      load_path = Propshaft::LoadPath.new([ root_path ], compilers: Propshaft::Compilers.new(nil))
+      Propshaft::Asset.new(root_path.join(logical_path), logical_path: logical_path, load_path: load_path)
     end
 end

test/propshaft/load_path_test.rb

@@ -6,7 +6,7 @@ class Propshaft::LoadPathTest < ActiveSupport::TestCase
     @load_path = Propshaft::LoadPath.new [
       Pathname.new("#{__dir__}/../fixtures/assets/first_path"),
       Pathname.new("#{__dir__}/../fixtures/assets/second_path").to_s
-    ]
+    ], compilers: Propshaft::Compilers.new(nil)
   end
 
   test "find asset that only appears once in the paths" do
@@ -44,15 +44,15 @@ class Propshaft::LoadPathTest < ActiveSupport::TestCase
   end
 
   test "manifest with version" do
-    @load_path = Propshaft::LoadPath.new(@load_path.paths, version: "1")
+    @load_path = Propshaft::LoadPath.new(@load_path.paths, version: "1", compilers: Propshaft::Compilers.new(nil))
     @load_path.manifest.tap do |manifest|
       assert_equal "one-c9373b68.txt", manifest["one.txt"]
       assert_equal "nested/three-a41a5d38.txt", manifest["nested/three.txt"]
     end
   end
 
   test "missing load path directory" do
-    assert_nil Propshaft::LoadPath.new(Pathname.new("#{__dir__}/../fixtures/assets/nowhere")).find("missing")
+    assert_nil Propshaft::LoadPath.new(Pathname.new("#{__dir__}/../fixtures/assets/nowhere"), compilers: Propshaft::Compilers.new(nil)).find("missing")
   end
 
   test "deduplicate paths" do
@@ -62,7 +62,7 @@ class Propshaft::LoadPathTest < ActiveSupport::TestCase
       "app/assets/stylesheets",
       "app/assets/images",
       "app/assets"
-    ]
+    ], compilers: Propshaft::Compilers.new(nil)
 
     paths = load_path.paths
     assert_equal 2, paths.count
@@ -72,9 +72,8 @@ class Propshaft::LoadPathTest < ActiveSupport::TestCase
 
   private
     def find_asset(logical_path)
-      Propshaft::Asset.new(
-        Pathname.new("#{__dir__}/../fixtures/assets/first_path/#{logical_path}"),
-        logical_path: Pathname.new(logical_path)
-      )
+      root_path = Pathname.new("#{__dir__}/../fixtures/assets/first_path")
+      load_path = Propshaft::LoadPath.new([ root_path ], compilers: Propshaft::Compilers.new(nil))
+      Propshaft::Asset.new(root_path.join(logical_path), logical_path: logical_path, load_path: load_path)
     end
 end

test/propshaft/output_path_test.rb

@@ -64,7 +64,8 @@ class Propshaft::OutputPathTest < ActiveSupport::TestCase
 
   private
     def output_asset(filename, content, created_at: Time.now)
-      asset = Propshaft::Asset.new(nil, logical_path: filename)
+      load_path = Propshaft::LoadPath.new([], compilers: Propshaft::Compilers.new(nil))
+      asset = Propshaft::Asset.new(nil, logical_path: filename, load_path: load_path)
       asset.stub :content, content do
         output_path = @output_path.path.join(asset.digested_path)
         `touch -mt #{created_at.strftime('%y%m%d%H%M')} #{output_path}`

test/propshaft/resolver/dynamic_test.rb

@@ -3,7 +3,7 @@
 
 class Propshaft::Resolver::DynamicTest < ActiveSupport::TestCase
   setup do
-    @load_path = Propshaft::LoadPath.new Pathname.new("#{__dir__}/../../fixtures/assets/first_path")
+    @load_path = Propshaft::LoadPath.new Pathname.new("#{__dir__}/../../fixtures/assets/first_path"), compilers: Propshaft::Compilers.new(nil)
     @resolver  = Propshaft::Resolver::Dynamic.new(load_path: @load_path, prefix: "/assets")
   end
 

test/test_helper.rb

@@ -14,6 +14,8 @@ class ActiveSupport::TestCase
     def find_asset(logical_path, fixture_path:)
       root_path = Pathname.new("#{__dir__}/fixtures/assets/#{fixture_path}")
       path = root_path.join(logical_path)
-      Propshaft::Asset.new(path, logical_path: logical_path)
+      load_path = Propshaft::LoadPath.new([ root_path ], compilers: Propshaft::Compilers.new(nil))
+
+      Propshaft::Asset.new(path, logical_path: logical_path, load_path: load_path)
     end
 end