Merge pull request #278 from jeremy/explicit-asset-lookup-locations

Restore manifest/env asset lookup order and make it configurable

Author: jeremy

README.md

@@ -74,10 +74,6 @@ config.assets.version = 'v2'
 
 Defaults to `/assets`. Changes the directory to compile assets to.
 
-**`config.assets.manifest`**
-
-Defines the full path to be used for the asset precompiler's manifest file. Defaults to a randomly-generated filename in the `config.assets.prefix` directory within the public folder.
-
 **`config.assets.digest`**
 
 When enabled, fingerprints will be added to asset filenames.
@@ -108,6 +104,24 @@ config.assets.configure do |env|
 end
 ```
 
+**`config.assets.resolve_with`**
+
+A list of `:environment` and `:manifest` symbols that defines the order that
+we try to find assets: manifest first, environment second? Manifest only?
+
+By default, we check the manifest first if asset digests are enabled and debug
+is not enabled, then we check the environment if compiling is enabled:
+```
+# Dev where debug is true, or digests are disabled
+%i[ environment ]
+
+# Dev default, or production with compile enabled.
+%i[ manifest environment ]
+
+# Production default.
+%i[ manifest ]
+```
+
 
 ## Complementary plugins
 

lib/sprockets/rails/helper.rb

@@ -20,9 +20,12 @@ def initialize(source)
       include ActionView::Helpers::AssetTagHelper
       include Sprockets::Rails::Utils
 
-      VIEW_ACCESSORS = [:assets_environment, :assets_manifest,
-                        :assets_precompile, :precompiled_asset_checker,
-                        :assets_prefix, :digest_assets, :debug_assets]
+      VIEW_ACCESSORS = [
+        :assets_environment, :assets_manifest,
+        :assets_precompile, :precompiled_asset_checker,
+        :assets_prefix, :digest_assets, :debug_assets,
+        :resolve_assets_with
+      ]
 
       def self.included(klass)
         klass.class_attribute(*VIEW_ACCESSORS)
@@ -73,13 +76,8 @@ def compute_asset_path(path, options = {})
       #
       # Returns String path or nil if no asset was found.
       def asset_digest_path(path, options = {})
-        if assets_environment
-          if asset = assets_environment[path]
-            raise_unless_precompiled_asset asset.logical_path unless options[:debug]
-            asset.digest_path
-          end
-        else
-          assets_manifest.assets[path]
+        resolve_asset do |resolver|
+          resolver.digest_path path, options[:debug]
         end
       end
 
@@ -92,14 +90,8 @@ def asset_digest_path(path, options = {})
       def asset_integrity(path, options = {})
         path = path_with_extname(path, options)
 
-        if assets_environment
-          if asset = assets_environment[path]
-            asset.integrity
-          end
-        elsif digest_path = assets_manifest.assets[path]
-          if metadata = assets_manifest.files[digest_path]
-            metadata["integrity"]
-          end
+        resolve_asset do |resolver|
+          resolver.integrity path, options[:debug]
         end
       end
 
@@ -197,15 +189,10 @@ def request_debug_assets?
         # Internal method to support multifile debugging. Will
         # eventually be removed w/ Sprockets 3.x.
         def lookup_debug_asset(path, options = {})
-          if assets_environment && asset = assets_environment[path_with_extname(path, options), pipeline: :debug]
-            raise_unless_precompiled_asset asset.logical_path.sub('.debug', '')
-            asset
-          end
-        end
+          path = path_with_extname(path, options)
 
-        def raise_unless_precompiled_asset(logical_path)
-          if !precompiled_asset_checker.call(logical_path)
-            raise AssetNotPrecompiled.new(logical_path)
+          resolve_asset do |resolver|
+            resolver.find_debug_asset path, options[:debug]
           end
         end
 
@@ -214,6 +201,103 @@ def path_with_extname(path, options)
           path = path.to_s
           "#{path}#{compute_asset_extname(path, options)}"
         end
+
+        # Try each asset resolver and return the first non-nil result.
+        def resolve_asset
+          asset_resolver_strategies.detect do |resolver|
+            if result = yield(resolver)
+              break result
+            end
+          end
+        end
+
+        # List of resolvers in `config.assets.resolve_with` order.
+        def asset_resolver_strategies
+          @asset_resolver_strategies ||=
+            Array(resolve_assets_with).map do |name|
+              HelperAssetResolvers[name].new(self)
+            end
+        end
+    end
+
+    # Use a separate module since Helper is mixed in and we needn't pollute
+    # the class namespace with our internals.
+    module HelperAssetResolvers #:nodoc:
+      def self.[](name)
+        case name
+        when :manifest
+          Manifest
+        when :environment
+          Environment
+        else
+          raise ArgumentError, "Unrecognized asset resolver: #{name.inspect}. Expected :manifest or :environment"
+        end
+      end
+
+      class Manifest #:nodoc:
+        def initialize(view)
+          @manifest = view.assets_manifest
+          raise ArgumentError, 'config.assets.resolve_with includes :manifest, but app.assets_manifest is nil' unless @manifest
+        end
+
+        def digest_path(path, debug = false)
+          @manifest.assets[path]
+        end
+
+        def integrity(path, debug = false)
+          if meta = metadata(path, debug)
+            meta["integrity"]
+          end
+        end
+
+        def find_debug_asset(path, debug = false)
+          nil
+        end
+
+        private
+          def metadata(path, debug = false)
+            if digest_path = digest_path(path, debug)
+              @manifest.files[digest_path]
+            end
+          end
+      end
+
+      class Environment #:nodoc:
+        def initialize(view)
+          raise ArgumentError, 'config.assets.resolve_with includes :environment, but app.assets is nil' unless view.assets_environment
+          @env = view.assets_environment
+          @precompiled_asset_checker = view.precompiled_asset_checker
+        end
+
+        def digest_path(path, debug = false)
+          if asset = find_asset(path)
+            raise_unless_precompiled_asset asset.logical_path unless debug
+            asset.digest_path
+          end
+        end
+
+        def integrity(path, debug = false)
+          find_asset(path).try :integrity
+        end
+
+        def find_debug_asset(path, options = {})
+          if asset = find_asset(path, pipeline: :debug)
+            raise_unless_precompiled_asset asset.logical_path.sub('.debug', '')
+            asset
+          end
+        end
+
+        private
+          def find_asset(path, options = {})
+            @env[path, options]
+          end
+
+          def raise_unless_precompiled_asset(logical_path)
+            if !@precompiled_asset_checker.call(logical_path)
+              raise Helper::AssetNotPrecompiled.new(logical_path)
+            end
+          end
+      end
     end
   end
 end

lib/sprockets/railtie.rb

@@ -166,8 +166,15 @@ def self.build_manifest(app)
           mount app.assets => config.assets.prefix
         end
       end
+
       app.assets_manifest = build_manifest(app)
 
+      if config.assets.resolve_with.nil?
+        config.assets.resolve_with = []
+        config.assets.resolve_with << :manifest if config.assets.digest && !config.assets.debug
+        config.assets.resolve_with << :environment if config.assets.compile
+      end
+
       ActionDispatch::Routing::RouteWrapper.class_eval do
         class_attribute :assets_prefix
 
@@ -192,6 +199,8 @@ def self.build_manifest(app)
         self.assets_environment = app.assets
         self.assets_manifest = app.assets_manifest
 
+        self.resolve_assets_with = config.assets.resolve_with
+
         # Expose the app precompiled asset check to the view
         self.precompiled_asset_checker = -> logical_path { app.asset_precompiled? logical_path }
       end

test/test_helper.rb

@@ -21,10 +21,11 @@ def setup
 
     @view = ActionView::Base.new
     @view.extend ::Sprockets::Rails::Helper
-    @view.assets_environment = @assets
-    @view.assets_manifest    = @manifest
-    @view.assets_prefix      = "/assets"
-    @view.assets_precompile  = %w( manifest.js )
+    @view.assets_environment  = @assets
+    @view.assets_manifest     = @manifest
+    @view.resolve_assets_with = [ :manifest, :environment ]
+    @view.assets_prefix       = "/assets"
+    @view.assets_precompile   = %w( manifest.js )
     precompiled_assets = @manifest.find(@view.assets_precompile).map(&:logical_path)
     @view.precompiled_asset_checker = -> logical_path { precompiled_assets.include? logical_path }
     @view.request = ActionDispatch::Request.new({
@@ -36,6 +37,7 @@ def setup
 
     @foo_js_integrity  = @assets['foo.js'].integrity
     @foo_css_integrity = @assets['foo.css'].integrity
+    @bar_js_integrity  = @assets['bar.js'].integrity
 
     @foo_js_digest  = @assets['foo.js'].etag
     @foo_css_digest = @assets['foo.css'].etag
@@ -401,7 +403,7 @@ def test_javascript_include_tag_integrity
     assert_dom_equal %(<script src="/assets/foo-#{@foo_js_digest}.js" integrity="#{@foo_js_integrity}"></script>),
       @view.javascript_include_tag(:foo, integrity: true)
 
-    assert_dom_equal %(<script src="/assets/foo-#{@foo_js_digest}.js" integrity="#{@foo_js_integrity}"></script>\n<script src="/assets/bar-#{@bar_js_digest}.js" integrity="sha256-g0JYFeYSYGXe376R0JrRzS6CpYpC1HiqtwBsVt/XAWU="></script>),
+    assert_dom_equal %(<script src="/assets/foo-#{@foo_js_digest}.js" integrity="#{@foo_js_integrity}"></script>\n<script src="/assets/bar-#{@bar_js_digest}.js" integrity="#{@bar_js_integrity}"></script>),
       @view.javascript_include_tag(:foo, :bar, integrity: true)
   end
 
@@ -645,6 +647,7 @@ def setup
     @view.digest_assets = true
     @view.assets_environment = nil
     @view.assets_manifest = @manifest
+    @view.resolve_assets_with = [ :manifest ]
   end
 
   def test_javascript_include_tag
@@ -727,32 +730,48 @@ def test_stylesheet_link_tag_integrity
   end
 end
 
-class StaleManifestVsEnvironmentHelperTest < HelperTest
+class AssetResolverOrderingTest < HelperTest
   def setup
     super
 
     @view.digest_assets = true
 
     @view.assets_manifest = Sprockets::Manifest.new(@assets, FIXTURES_PATH).tap do |stale|
       stale.assets["foo.js"] = "foo-stale.js"
-      @manifest.files["foo-stale.js"] = { "integrity" => "stale-manifest" }
-      @manifest.files["foo-#{@foo_js_digest}.js"] = { "integrity" => "current-manifest" }
+      stale.files["foo-stale.js"] = { "integrity" => "stale-manifest" }
 
       stale.assets["foo.css"] = "foo-stale.css"
-      @manifest.files["foo-stale.css"] = { "integrity" => "stale-manifest" }
-      @manifest.files["foo-#{@foo_css_digest}.css"] = { "integrity" => "current-manifest" }
+      stale.files["foo-stale.css"] = { "integrity" => "stale-manifest" }
     end
   end
 
   def test_digest_prefers_asset_environment_over_manifest
+    @view.resolve_assets_with = [ :environment, :manifest ]
+
     assert_equal "foo-#{@foo_js_digest}.js", @view.asset_digest_path("foo.js")
     assert_equal "foo-#{@foo_css_digest}.css", @view.asset_digest_path("foo.css")
-  end
 
-  def test_digest_prefers_asset_environment_over_manifest
     assert_equal @foo_js_integrity, @view.asset_integrity("foo.js")
     assert_equal @foo_css_integrity, @view.asset_integrity("foo.css")
   end
+
+  def test_try_resolvers_until_first_result
+    @view.resolve_assets_with = [ :manifest, :environment ]
+
+    assert_equal 'foo-stale.js', @view.asset_digest_path('foo.js')
+    assert_equal "bar-#{@bar_js_digest}.js", @view.asset_digest_path('bar.js')
+    assert_nil @view.asset_digest_path('nonexistent')
+
+    assert_equal 'stale-manifest', @view.asset_integrity('foo.js')
+    assert_equal @bar_js_integrity, @view.asset_integrity('bar.js')
+    assert_nil @view.asset_integrity('nonexistent')
+  end
+
+  def test_obeys_asset_resolver_order
+    @view.resolve_assets_with = []
+    assert_nil @view.asset_digest_path('foo.js')
+    assert_nil @view.asset_integrity('foo.js')
+  end
 end
 
 class AssetUrlHelperLinksTarget < HelperTest