From bd38d9f2118b1005718f8db4292b21a73879409e Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 12 Oct 2012 14:57:38 -0500 Subject: [PATCH 01/15] Add asset_path and asset_url helpers --- .../helpers/asset_tag_helpers/asset_paths.rb | 3 +- .../action_view/helpers/asset_url_helper.rb | 68 ++++++++++++++----- .../test/template/asset_tag_helper_test.rb | 18 +++++ 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb index 35f91cec182b..90563d62015a 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb @@ -43,7 +43,8 @@ def rewrite_extension(source, dir, ext) # Break out the asset path rewrite in case plugins wish to put the asset id # someplace other than the query string. def rewrite_asset_path(source, dir, options = nil) - source = "/#{dir}/#{source}" unless source[0] == ?/ + dir = "/#{dir}" if dir && dir[0] != ?/ + source = File.join(dir.to_s, source) unless source[0] == ?/ path = config.asset_path if path && path.respond_to?(:call) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index 4554c0c473f9..35d312e61a6f 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -190,6 +190,46 @@ module Helpers #:nodoc: # RewriteRule ^/release-\d+/(images|javascripts|stylesheets)/(.*)$ /$1/$2 [L] # module AssetUrlHelper + ASSET_EXTENSIONS = { + javascript: 'js', + stylesheet: 'css' + } + + ASSET_PUBLIC_DIRECTORIES = { + audio: '/audios', + font: '/fonts', + image: '/images', + javascript: '/javascripts', + stylesheet: '/stylesheets', + video: '/videos' + } + + # Computes the path to asset in public directory. If :type + # options is set, a file extension will be appended and scoped + # to the corresponding public directory. + # + # All other asset *_path helpers delegate through this method. + # + # asset_path "application.js" # => /application.js + # asset_path "application", type: :javascript # => /javascripts/application.js + # asset_path "application", type: :stylesheet # => /stylesheets/application.css + # asset_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js + def asset_path(source, options = {}) + return "" unless source.present? + options[:ext] ||= ASSET_EXTENSIONS[options[:type]] if options[:type] + asset_paths.compute_public_path(source, ASSET_PUBLIC_DIRECTORIES[options[:type]], options) + end + alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with a asset_path named route + + # Computes the full URL to a asset in the public directory. This + # will use +asset_path+ internally, so most of their behaviors + # will be the same. + def asset_url(source, options = {}) + host = url_for(:only_path => false) + URI.join(host, path_to_asset(source, options)).to_s + end + alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route + # Computes the path to a javascript asset in the public javascripts directory. # If the +source+ filename has no extension, .js will be appended (except for explicit URIs) # Full paths from the document root will be passed through. @@ -201,14 +241,14 @@ module AssetUrlHelper # javascript_path "http://www.example.com/js/xmlhr" # => http://www.example.com/js/xmlhr # javascript_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js def javascript_path(source) - asset_paths.compute_public_path(source, 'javascripts', :ext => 'js') + path_to_asset(source, type: :javascript) end alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route # Computes the full URL to a javascript asset in the public javascripts directory. # This will use +javascript_path+ internally, so most of their behaviors will be the same. def javascript_url(source) - URI.join(current_host, path_to_javascript(source)).to_s + url_to_asset(source, type: :javascript) end alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route @@ -223,14 +263,14 @@ def javascript_url(source) # stylesheet_path "http://www.example.com/css/style" # => http://www.example.com/css/style # stylesheet_path "http://www.example.com/css/style.css" # => http://www.example.com/css/style.css def stylesheet_path(source) - asset_paths.compute_public_path(source, 'stylesheets', :ext => 'css', :protocol => :request) + path_to_asset(source, type: :stylesheet) end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route # Computes the full URL to a stylesheet asset in the public stylesheets directory. # This will use +stylesheet_path+ internally, so most of their behaviors will be the same. def stylesheet_url(source) - URI.join(current_host, path_to_stylesheet(source)).to_s + url_to_asset(source, type: :stylesheet) end alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route @@ -248,14 +288,14 @@ def stylesheet_url(source) # The alias +path_to_image+ is provided to avoid that. Rails uses the alias internally, and # plugin authors are encouraged to do so. def image_path(source) - source.present? ? asset_paths.compute_public_path(source, 'images') : "" + path_to_asset(source, type: :image) end alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route # Computes the full URL to an image asset. # This will use +image_path+ internally, so most of their behaviors will be the same. def image_url(source) - URI.join(current_host, path_to_image(source)).to_s + url_to_asset(source, type: :image) end alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route @@ -269,14 +309,14 @@ def image_url(source) # video_path("/trailers/hd.avi") # => /trailers/hd.avi # video_path("http://www.example.com/vid/hd.avi") # => http://www.example.com/vid/hd.avi def video_path(source) - asset_paths.compute_public_path(source, 'videos') + path_to_asset(source, type: :video) end alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route # Computes the full URL to a video asset in the public videos directory. # This will use +video_path+ internally, so most of their behaviors will be the same. def video_url(source) - URI.join(current_host, path_to_video(source)).to_s + url_to_asset(source, type: :video) end alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route @@ -290,14 +330,14 @@ def video_url(source) # audio_path("/sounds/horse.wav") # => /sounds/horse.wav # audio_path("http://www.example.com/sounds/horse.wav") # => http://www.example.com/sounds/horse.wav def audio_path(source) - asset_paths.compute_public_path(source, 'audios') + path_to_asset(source, type: :audio) end alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route # Computes the full URL to an audio asset in the public audios directory. # This will use +audio_path+ internally, so most of their behaviors will be the same. def audio_url(source) - URI.join(current_host, path_to_audio(source)).to_s + url_to_asset(source, type: :audio) end alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route @@ -310,14 +350,14 @@ def audio_url(source) # font_path("/dir/font.ttf") # => /dir/font.ttf # font_path("http://www.example.com/dir/font.ttf") # => http://www.example.com/dir/font.ttf def font_path(source) - asset_paths.compute_public_path(source, 'fonts') + path_to_asset(source, type: :font) end alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route # Computes the full URL to a font asset. # This will use +font_path+ internally, so most of their behaviors will be the same. def font_url(source) - URI.join(current_host, path_to_font(source)).to_s + url_to_asset(source, type: :font) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route @@ -325,10 +365,6 @@ def font_url(source) def asset_paths @asset_paths ||= AssetTagHelper::AssetPaths.new(config, controller) end - - def current_host - url_for(:only_path => false) - end end end end diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 754622c883f2..ac73e9515c39 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -55,6 +55,19 @@ def teardown ENV.delete('RAILS_ASSET_ID') end + AssetPathToTag = { + %(asset_path("foo")) => %(/foo), + %(asset_path("style.css")) => %(/style.css), + %(asset_path("xmlhr.js")) => %(/xmlhr.js), + %(asset_path("xml.png")) => %(/xml.png), + %(asset_path("dir/xml.png")) => %(/dir/xml.png), + %(asset_path("/dir/xml.png")) => %(/dir/xml.png), + + %(asset_path("style", type: :stylesheet)) => %(/stylesheets/style.css), + %(asset_path("xmlhr", type: :javascript)) => %(/javascripts/xmlhr.js), + %(asset_path("xml.png", type: :image)) => %(/images/xml.png) + } + AutoDiscoveryToTag = { %(auto_discovery_link_tag) => %(), %(auto_discovery_link_tag(:rss)) => %(), @@ -293,6 +306,11 @@ def test_autodiscovery_link_tag_deprecated_types assert_equal expected, result end + def test_asset_path_tag + ENV["RAILS_ASSET_ID"] = "" + AssetPathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } + end + def test_auto_discovery_link_tag AutoDiscoveryToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end From 1e2b0ce95e48463361111ceae6eed7d4ad5462f0 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 12 Oct 2012 16:56:00 -0500 Subject: [PATCH 02/15] Refactor AssetUrlHelper to make it friendly for plugins and extensions Add asset_path/url helper for a consolidated entry point Expose compute_asset_path as a public API Expose compute_asset_host as a public API Move RAILS_ASSET_ID to its own module, AssetIdHelper Removed AV::AssetPaths --- actionpack/lib/action_view.rb | 1 - actionpack/lib/action_view/asset_paths.rb | 143 ------------- actionpack/lib/action_view/helpers.rb | 1 + .../action_view/helpers/asset_id_helper.rb | 154 ++++++++++++++ .../action_view/helpers/asset_tag_helper.rb | 1 + .../helpers/asset_tag_helpers/asset_paths.rb | 94 --------- .../action_view/helpers/asset_url_helper.rb | 189 ++++++++---------- actionpack/lib/action_view/railtie.rb | 2 +- .../test/template/asset_tag_helper_test.rb | 36 +++- 9 files changed, 275 insertions(+), 346 deletions(-) delete mode 100644 actionpack/lib/action_view/asset_paths.rb create mode 100644 actionpack/lib/action_view/helpers/asset_id_helper.rb delete mode 100644 actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 091b0d8cd2ce..8bbf52382a54 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -29,7 +29,6 @@ module ActionView extend ActiveSupport::Autoload eager_autoload do - autoload :AssetPaths autoload :Base autoload :Context autoload :CompiledTemplates, "action_view/context" diff --git a/actionpack/lib/action_view/asset_paths.rb b/actionpack/lib/action_view/asset_paths.rb deleted file mode 100644 index 4bbb31b3ee04..000000000000 --- a/actionpack/lib/action_view/asset_paths.rb +++ /dev/null @@ -1,143 +0,0 @@ -require 'zlib' -require 'active_support/core_ext/file' - -module ActionView - class AssetPaths #:nodoc: - URI_REGEXP = %r{^[-a-z]+://|^(?:cid|data):|^//} - - attr_reader :config, :controller - - def initialize(config, controller = nil) - @config = config - @controller = controller - end - - # Add the extension +ext+ if not present. Return full or scheme-relative URLs otherwise untouched. - # Prefix with /dir/ if lacking a leading +/+. Account for relative URL - # roots. Rewrite the asset path for cache-busting asset ids. Include - # asset host, if configured, with the correct request protocol. - # - # When :relative (default), the protocol will be determined by the client using current protocol - # When :request, the protocol will be the request protocol - # Otherwise, the protocol is used (E.g. :http, :https, etc) - def compute_public_path(source, dir, options = {}) - source = source.to_s - return source if is_uri?(source) - - source = rewrite_extension(source, dir, options[:ext]) if options[:ext] - source = rewrite_asset_path(source, dir, options) - source = rewrite_relative_url_root(source, relative_url_root) - source = rewrite_host_and_protocol(source, options[:protocol]) - source - end - - # Return the filesystem path for the source - def compute_source_path(source, dir, ext) - source = rewrite_extension(source, dir, ext) if ext - - sources = [] - sources << config.assets_dir - sources << dir unless source[0] == ?/ - sources << source - - File.join(sources) - end - - def is_uri?(path) - path =~ URI_REGEXP - end - - private - - def rewrite_extension(source, dir, ext) - raise NotImplementedError - end - - def rewrite_asset_path(source, path = nil) - raise NotImplementedError - end - - def rewrite_relative_url_root(source, relative_url_root) - relative_url_root && !source.starts_with?("#{relative_url_root}/") ? "#{relative_url_root}#{source}" : source - end - - def has_request? - controller.respond_to?(:request) - end - - def rewrite_host_and_protocol(source, protocol = nil) - host = compute_asset_host(source) - if host && !is_uri?(host) - if (protocol || default_protocol) == :request && !has_request? - host = nil - else - host = "#{compute_protocol(protocol)}#{host}" - end - end - host ? "#{host}#{source}" : source - end - - def compute_protocol(protocol) - protocol ||= default_protocol - case protocol - when :relative - "//" - when :request - unless @controller - invalid_asset_host!("The protocol requested was :request. Consider using :relative instead.") - end - @controller.request.protocol - else - "#{protocol}://" - end - end - - def default_protocol - @config.default_asset_host_protocol || (has_request? ? :request : :relative) - end - - def invalid_asset_host!(help_message) - raise ActionView::MissingRequestError, "This asset host cannot be computed without a request in scope. #{help_message}" - end - - # Pick an asset host for this source. Returns +nil+ if no host is set, - # the host if no wildcard is set, the host interpolated with the - # numbers 0-3 if it contains %d (the number is the source hash mod 4), - # or the value returned from invoking call on an object responding to call - # (proc or otherwise). - def compute_asset_host(source) - if host = asset_host_config - if host.respond_to?(:call) - args = [source] - arity = arity_of(host) - if (arity > 1 || arity < -2) && !has_request? - invalid_asset_host!("Remove the second argument to your asset_host Proc if you do not need the request, or make it optional.") - end - args << current_request if (arity > 1 || arity < 0) && has_request? - host.call(*args) - else - (host =~ /%d/) ? host % (Zlib.crc32(source) % 4) : host - end - end - end - - def relative_url_root - config.relative_url_root || current_request.try(:script_name) - end - - def asset_host_config - config.asset_host - end - - # Returns the current request if one exists. - def current_request - controller.request if has_request? - end - - # Returns the arity of a callable - def arity_of(callable) - callable.respond_to?(:arity) ? callable.arity : callable.method(:call).arity - end - - end -end diff --git a/actionpack/lib/action_view/helpers.rb b/actionpack/lib/action_view/helpers.rb index dad50a58a15d..cc140871892e 100644 --- a/actionpack/lib/action_view/helpers.rb +++ b/actionpack/lib/action_view/helpers.rb @@ -3,6 +3,7 @@ module Helpers #:nodoc: extend ActiveSupport::Autoload autoload :ActiveModelHelper + autoload :AssetIdHelper autoload :AssetTagHelper autoload :AssetUrlHelper autoload :AtomFeedHelper diff --git a/actionpack/lib/action_view/helpers/asset_id_helper.rb b/actionpack/lib/action_view/helpers/asset_id_helper.rb new file mode 100644 index 000000000000..ba8e5e90500f --- /dev/null +++ b/actionpack/lib/action_view/helpers/asset_id_helper.rb @@ -0,0 +1,154 @@ +require 'thread' +require 'active_support/core_ext/file' +require 'active_support/core_ext/module/attribute_accessors' + +module ActionView + # = Action View Asset Cache ID Helpers + # + # Rails appends asset's timestamps to public asset paths. This allows + # you to set a cache-expiration date for the asset far into the future, but + # still be able to instantly invalidate it by simply updating the file (and + # hence updating the timestamp, which then updates the URL as the timestamp + # is part of that, which in turn busts the cache). + # + # It's the responsibility of the web server you use to set the far-future + # expiration date on cache assets that you need to take advantage of this + # feature. Here's an example for Apache: + # + # # Asset Expiration + # ExpiresActive On + # + # ExpiresDefault "access plus 1 year" + # + # + # Also note that in order for this to work, all your application servers must + # return the same timestamps. This means that they must have their clocks + # synchronized. If one of them drifts out of sync, you'll see different + # timestamps at random and the cache won't work. In that case the browser + # will request the same assets over and over again even thought they didn't + # change. You can use something like Live HTTP Headers for Firefox to verify + # that the cache is indeed working. + # + # This strategy works well enough for most server setups and requires the + # least configuration, but if you deploy several application servers at + # different times - say to handle a temporary spike in load - then the + # asset time stamps will be out of sync. In a setup like this you may want + # to set the way that asset paths are generated yourself. + # + # Altering the asset paths that Rails generates can be done in two ways. + # The easiest is to define the RAILS_ASSET_ID environment variable. The + # contents of this variable will always be used in preference to + # calculated timestamps. A more complex but flexible way is to set + # ActionController::Base.config.asset_path to a proc + # that takes the unmodified asset path and returns the path needed for + # your asset caching to work. Typically you'd do something like this in + # config/environments/production.rb: + # + # # Normally you'd calculate RELEASE_NUMBER at startup. + # RELEASE_NUMBER = 12345 + # config.action_controller.asset_path = proc { |asset_path| + # "/release-#{RELEASE_NUMBER}#{asset_path}" + # } + # + # This example would cause the following behavior on all servers no + # matter when they were deployed: + # + # image_tag("rails.png") + # # => Rails + # stylesheet_link_tag("application") + # # => + # + # Changing the asset_path does require that your web servers have + # knowledge of the asset template paths that you rewrite to so it's not + # suitable for out-of-the-box use. To use the example given above you + # could use something like this in your Apache VirtualHost configuration: + # + # + # # Some browsers still send conditional-GET requests if there's a + # # Last-Modified header or an ETag header even if they haven't + # # reached the expiry date sent in the Expires header. + # Header unset Last-Modified + # Header unset ETag + # FileETag None + # + # # Assets requested using a cache-busting filename should be served + # # only once and then cached for a really long time. The HTTP/1.1 + # # spec frowns on hugely-long expiration times though and suggests + # # that assets which never expire be served with an expiration date + # # 1 year from access. + # ExpiresActive On + # ExpiresDefault "access plus 1 year" + # + # + # # We use cached-busting location names with the far-future expires + # # headers to ensure that if a file does change it can force a new + # # request. The actual asset filenames are still the same though so we + # # need to rewrite the location from the cache-busting location to the + # # real asset location so that we can serve it. + # RewriteEngine On + # RewriteRule ^/release-\d+/(images|javascripts|stylesheets)/(.*)$ /$1/$2 [L] + # + module Helpers #:nodoc: + module AssetIdHelper + # You can enable or disable the asset tag ids cache. + # With the cache enabled, the asset tag helper methods will make fewer + # expensive file system calls (the default implementation checks the file + # system timestamp). However this prevents you from modifying any asset + # files while the server is running. + # + # ActionView::Helpers::AssetIdHelper.cache_asset_ids = false + mattr_accessor :cache_asset_ids + + # Add or change an asset id in the asset id cache. This can be used + # for SASS on Heroku. + # :api: public + def add_to_asset_ids_cache(source, asset_id) + self.asset_ids_cache_guard.synchronize do + self.asset_ids_cache[source] = asset_id + end + end + + mattr_accessor :asset_ids_cache + self.asset_ids_cache = {} + + mattr_accessor :asset_ids_cache_guard + self.asset_ids_cache_guard = Mutex.new + + # Use the RAILS_ASSET_ID environment variable or the source's + # modification time as its cache-busting asset id. + def rails_asset_id(source) + if asset_id = ENV["RAILS_ASSET_ID"] + asset_id + else + if self.cache_asset_ids && (asset_id = self.asset_ids_cache[source]) + asset_id + else + path = File.join(config.assets_dir, source) + asset_id = File.exist?(path) ? File.mtime(path).to_i.to_s : '' + + if self.cache_asset_ids + add_to_asset_ids_cache(source, asset_id) + end + + asset_id + end + end + end + + # Override +compute_asset_path+ to add asset id query strings to + # generated urls. See +compute_asset_path+ in AssetUrlHelper. + def compute_asset_path(source, options = {}) + source = super(source, options) + path = config.asset_path + + if path && path.respond_to?(:call) + path.call(source) + elsif path && path.is_a?(String) + path % [source] + elsif asset_id = rails_asset_id(source) + asset_id.empty? ? source : "#{source}?#{asset_id}" + end + end + end + end +end diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 4eac6514df6d..3beea2eb7c6c 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -19,6 +19,7 @@ module AssetTagHelper extend ActiveSupport::Concern include AssetUrlHelper + include AssetIdHelper include TagHelper # Returns an HTML script tag for each of the +sources+ provided. diff --git a/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb b/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb deleted file mode 100644 index 90563d62015a..000000000000 --- a/actionpack/lib/action_view/helpers/asset_tag_helpers/asset_paths.rb +++ /dev/null @@ -1,94 +0,0 @@ -require 'thread' -require 'active_support/core_ext/file' -require 'active_support/core_ext/module/attribute_accessors' - -module ActionView - module Helpers - module AssetTagHelper - - class AssetPaths < ::ActionView::AssetPaths #:nodoc: - # You can enable or disable the asset tag ids cache. - # With the cache enabled, the asset tag helper methods will make fewer - # expensive file system calls (the default implementation checks the file - # system timestamp). However this prevents you from modifying any asset - # files while the server is running. - # - # ActionView::Helpers::AssetTagHelper::AssetPaths.cache_asset_ids = false - mattr_accessor :cache_asset_ids - - # Add or change an asset id in the asset id cache. This can be used - # for SASS on Heroku. - # :api: public - def add_to_asset_ids_cache(source, asset_id) - self.asset_ids_cache_guard.synchronize do - self.asset_ids_cache[source] = asset_id - end - end - - private - - def rewrite_extension(source, dir, ext) - source_ext = File.extname(source) - - source_with_ext = if source_ext.empty? - "#{source}.#{ext}" - elsif ext != source_ext[1..-1] - with_ext = "#{source}.#{ext}" - with_ext if File.exist?(File.join(config.assets_dir, dir, with_ext)) - end - - source_with_ext || source - end - - # Break out the asset path rewrite in case plugins wish to put the asset id - # someplace other than the query string. - def rewrite_asset_path(source, dir, options = nil) - dir = "/#{dir}" if dir && dir[0] != ?/ - source = File.join(dir.to_s, source) unless source[0] == ?/ - path = config.asset_path - - if path && path.respond_to?(:call) - return path.call(source) - elsif path && path.is_a?(String) - return path % [source] - end - - asset_id = rails_asset_id(source) - if asset_id.empty? - source - else - "#{source}?#{asset_id}" - end - end - - mattr_accessor :asset_ids_cache - self.asset_ids_cache = {} - - mattr_accessor :asset_ids_cache_guard - self.asset_ids_cache_guard = Mutex.new - - # Use the RAILS_ASSET_ID environment variable or the source's - # modification time as its cache-busting asset id. - def rails_asset_id(source) - if asset_id = ENV["RAILS_ASSET_ID"] - asset_id - else - if self.cache_asset_ids && (asset_id = self.asset_ids_cache[source]) - asset_id - else - path = File.join(config.assets_dir, source) - asset_id = File.exist?(path) ? File.mtime(path).to_i.to_s : '' - - if self.cache_asset_ids - add_to_asset_ids_cache(source, asset_id) - end - - asset_id - end - end - end - end - - end - end -end diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index 35d312e61a6f..8cbcdfe99ea4 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -1,4 +1,4 @@ -require 'action_view/helpers/asset_tag_helpers/asset_paths' +require 'zlib' module ActionView # = Action View Asset URL Helpers @@ -104,104 +104,12 @@ module Helpers #:nodoc: # "http://asset%d.example.com", "https://asset1.example.com" # ) # - # === Customizing the asset path - # - # By default, Rails appends asset's timestamps to all asset paths. This allows - # you to set a cache-expiration date for the asset far into the future, but - # still be able to instantly invalidate it by simply updating the file (and - # hence updating the timestamp, which then updates the URL as the timestamp - # is part of that, which in turn busts the cache). - # - # It's the responsibility of the web server you use to set the far-future - # expiration date on cache assets that you need to take advantage of this - # feature. Here's an example for Apache: - # - # # Asset Expiration - # ExpiresActive On - # - # ExpiresDefault "access plus 1 year" - # - # - # Also note that in order for this to work, all your application servers must - # return the same timestamps. This means that they must have their clocks - # synchronized. If one of them drifts out of sync, you'll see different - # timestamps at random and the cache won't work. In that case the browser - # will request the same assets over and over again even thought they didn't - # change. You can use something like Live HTTP Headers for Firefox to verify - # that the cache is indeed working. - # - # This strategy works well enough for most server setups and requires the - # least configuration, but if you deploy several application servers at - # different times - say to handle a temporary spike in load - then the - # asset time stamps will be out of sync. In a setup like this you may want - # to set the way that asset paths are generated yourself. - # - # Altering the asset paths that Rails generates can be done in two ways. - # The easiest is to define the RAILS_ASSET_ID environment variable. The - # contents of this variable will always be used in preference to - # calculated timestamps. A more complex but flexible way is to set - # ActionController::Base.config.asset_path to a proc - # that takes the unmodified asset path and returns the path needed for - # your asset caching to work. Typically you'd do something like this in - # config/environments/production.rb: - # - # # Normally you'd calculate RELEASE_NUMBER at startup. - # RELEASE_NUMBER = 12345 - # config.action_controller.asset_path = proc { |asset_path| - # "/release-#{RELEASE_NUMBER}#{asset_path}" - # } - # - # This example would cause the following behavior on all servers no - # matter when they were deployed: - # - # image_tag("rails.png") - # # => Rails - # stylesheet_link_tag("application") - # # => - # - # Changing the asset_path does require that your web servers have - # knowledge of the asset template paths that you rewrite to so it's not - # suitable for out-of-the-box use. To use the example given above you - # could use something like this in your Apache VirtualHost configuration: - # - # - # # Some browsers still send conditional-GET requests if there's a - # # Last-Modified header or an ETag header even if they haven't - # # reached the expiry date sent in the Expires header. - # Header unset Last-Modified - # Header unset ETag - # FileETag None - # - # # Assets requested using a cache-busting filename should be served - # # only once and then cached for a really long time. The HTTP/1.1 - # # spec frowns on hugely-long expiration times though and suggests - # # that assets which never expire be served with an expiration date - # # 1 year from access. - # ExpiresActive On - # ExpiresDefault "access plus 1 year" - # - # - # # We use cached-busting location names with the far-future expires - # # headers to ensure that if a file does change it can force a new - # # request. The actual asset filenames are still the same though so we - # # need to rewrite the location from the cache-busting location to the - # # real asset location so that we can serve it. - # RewriteEngine On - # RewriteRule ^/release-\d+/(images|javascripts|stylesheets)/(.*)$ /$1/$2 [L] - # module AssetUrlHelper - ASSET_EXTENSIONS = { - javascript: 'js', - stylesheet: 'css' - } + URI_REGEXP = %r{^[-a-z]+://|^(?:cid|data):|^//} - ASSET_PUBLIC_DIRECTORIES = { - audio: '/audios', - font: '/fonts', - image: '/images', - javascript: '/javascripts', - stylesheet: '/stylesheets', - video: '/videos' + ASSET_EXTENSIONS = { + javascript: '.js', + stylesheet: '.css' } # Computes the path to asset in public directory. If :type @@ -215,9 +123,28 @@ module AssetUrlHelper # asset_path "application", type: :stylesheet # => /stylesheets/application.css # asset_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js def asset_path(source, options = {}) + source = source.to_s return "" unless source.present? - options[:ext] ||= ASSET_EXTENSIONS[options[:type]] if options[:type] - asset_paths.compute_public_path(source, ASSET_PUBLIC_DIRECTORIES[options[:type]], options) + return source if source =~ URI_REGEXP + + if File.extname(source).empty? && (ext = ASSET_EXTENSIONS[options[:type]]) + source = "#{source}#{ext}" + end + + if source[0] != ?/ + source = compute_asset_path(source, options) + end + + current_request = controller.request if controller.respond_to?(:request) + if relative_url_root = config.relative_url_root || current_request.try(:script_name) + source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/") + end + + if host = compute_asset_host(source, options) + source = "#{host}#{source}" + end + + source end alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with a asset_path named route @@ -225,11 +152,66 @@ def asset_path(source, options = {}) # will use +asset_path+ internally, so most of their behaviors # will be the same. def asset_url(source, options = {}) - host = url_for(:only_path => false) - URI.join(host, path_to_asset(source, options)).to_s + path_to_asset(source, options.merge(:protocol => :request)) end alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route + # Maps asset types to public directory. + ASSET_PUBLIC_DIRECTORIES = { + audio: '/audios', + font: '/fonts', + image: '/images', + javascript: '/javascripts', + stylesheet: '/stylesheets', + video: '/videos' + } + + # Computes asset path to public directory. Plugins and + # extensions can override this method to point to custom assets + # or generate digested paths or query strings. + def compute_asset_path(source, options = {}) + dir = ASSET_PUBLIC_DIRECTORIES[options[:type]] || "" + File.join(dir, source) + end + + # Pick an asset host for this source. Returns +nil+ if no host is set, + # the host if no wildcard is set, the host interpolated with the + # numbers 0-3 if it contains %d (the number is the source hash mod 4), + # or the value returned from invoking call on an object responding to call + # (proc or otherwise). + def compute_asset_host(source = "", options = {}) + if controller.respond_to?(:request) + request = controller.request + end + + host = config.asset_host + host ||= request.base_url if request && options[:protocol] == :request + return unless host + + if host.respond_to?(:call) + arity = host.respond_to?(:arity) ? host.arity : host.method(:call).arity + args = [source] + args << request if request && (arity > 1 || arity < 0) + host = host.call(*args) + elsif host =~ /%d/ + host = host % (Zlib.crc32(source) % 4) + end + + if host =~ URI_REGEXP + host + else + protocol = options[:protocol] || config.default_asset_host_protocol || (request ? :request : :relative) + case protocol + when :relative + "//#{host}" + when :request + "#{request.protocol}#{host}" + else + "#{protocol}://#{host}" + end + end + end + # Computes the path to a javascript asset in the public javascripts directory. # If the +source+ filename has no extension, .js will be appended (except for explicit URIs) # Full paths from the document root will be passed through. @@ -360,11 +342,6 @@ def font_url(source) url_to_asset(source, type: :font) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route - - private - def asset_paths - @asset_paths ||= AssetTagHelper::AssetPaths.new(config, controller) - end end end end diff --git a/actionpack/lib/action_view/railtie.rb b/actionpack/lib/action_view/railtie.rb index 55f6ea552273..2d5f664402a0 100644 --- a/actionpack/lib/action_view/railtie.rb +++ b/actionpack/lib/action_view/railtie.rb @@ -23,7 +23,7 @@ class Railtie < Rails::Railtie initializer "action_view.cache_asset_ids" do |app| unless app.config.cache_classes ActiveSupport.on_load(:action_view) do - ActionView::Helpers::AssetTagHelper::AssetPaths.cache_asset_ids = false + ActionView::Helpers::AssetIdHelper.cache_asset_ids = false end end end diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index ac73e9515c39..2059501fccfb 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -42,6 +42,7 @@ def setup def protocol() 'http://' end def ssl?() false end def host_with_port() 'localhost' end + def base_url() 'http://www.example.com' end end.new @controller.request = @request @@ -602,7 +603,7 @@ def setup @controller = BasicController.new @controller.config.relative_url_root = "/collaboration/hieraki" - @request = Struct.new(:protocol).new("gopher://") + @request = Struct.new(:protocol, :base_url).new("gopher://", "gopher://www.example.com") @controller.request = @request end @@ -617,10 +618,33 @@ def test_should_compute_proper_path assert_dom_equal(%(/collaboration/hieraki/images/xml.png), image_path("xml.png")) end + def test_should_return_nothing_if_asset_host_isnt_configured + assert_equal nil, compute_asset_host("foo") + end + + def test_should_current_request_host_is_always_returned_for_request + assert_equal "gopher://www.example.com", compute_asset_host("foo", :protocol => :request) + end + def test_should_ignore_relative_root_path_on_complete_url assert_dom_equal(%(http://www.example.com/images/xml.png), image_path("http://www.example.com/images/xml.png")) end + def test_should_return_simple_string_asset_host + @controller.config.asset_host = "assets.example.com" + assert_equal "gopher://assets.example.com", compute_asset_host("foo") + end + + def test_should_return_relative_asset_host + @controller.config.asset_host = "assets.example.com" + assert_equal "//assets.example.com", compute_asset_host("foo", :protocol => :relative) + end + + def test_should_return_custom_protocol_asset_host + @controller.config.asset_host = "assets.example.com" + assert_equal "ftp://assets.example.com", compute_asset_host("foo", :protocol => "ftp") + end + def test_should_compute_proper_path_with_asset_host @controller.config.asset_host = "assets.example.com" assert_dom_equal(%(), auto_discovery_link_tag) @@ -653,6 +677,11 @@ def test_should_compute_proper_url_with_asset_host_and_default_protocol assert_dom_equal(%(gopher://assets.example.com/collaboration/hieraki/images/xml.png), image_url("xml.png")) end + def test_should_return_asset_host_with_protocol + @controller.config.asset_host = "http://assets.example.com" + assert_equal "http://assets.example.com", compute_asset_host("foo") + end + def test_should_ignore_asset_host_on_complete_url @controller.config.asset_host = "http://assets.example.com" assert_dom_equal(%(), stylesheet_link_tag("http://bar.example.com/stylesheets/style.css")) @@ -663,6 +692,11 @@ def test_should_ignore_asset_host_on_scheme_relative_url assert_dom_equal(%(), stylesheet_link_tag("//bar.example.com/stylesheets/style.css")) end + def test_should_wildcard_asset_host + @controller.config.asset_host = 'http://a%d.example.com' + assert_match(%r(http://a[0123].example.com), compute_asset_host("foo")) + end + def test_should_wildcard_asset_host_between_zero_and_four @controller.config.asset_host = 'http://a%d.example.com' assert_match(%r(http://a[0123].example.com/collaboration/hieraki/images/xml.png), image_path('xml.png')) From c3cff4d4219c556a5bfe4cbd72b96723b1633122 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 12 Oct 2012 17:07:17 -0500 Subject: [PATCH 03/15] Ensure AssetUrlHelper can be mixed into AC::Base --- .../action_view/helpers/asset_url_helper.rb | 19 +++++++---- .../test/template/asset_tag_helper_test.rb | 33 +++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index 8cbcdfe99ea4..f2213efbb945 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -135,8 +135,7 @@ def asset_path(source, options = {}) source = compute_asset_path(source, options) end - current_request = controller.request if controller.respond_to?(:request) - if relative_url_root = config.relative_url_root || current_request.try(:script_name) + if relative_url_root = config.relative_url_root || asset_request.try(:script_name) source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/") end @@ -180,10 +179,7 @@ def compute_asset_path(source, options = {}) # or the value returned from invoking call on an object responding to call # (proc or otherwise). def compute_asset_host(source = "", options = {}) - if controller.respond_to?(:request) - request = controller.request - end - + request = asset_request host = config.asset_host host ||= request.base_url if request && options[:protocol] == :request return unless host @@ -342,6 +338,17 @@ def font_url(source) url_to_asset(source, type: :font) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route + + private + # Get current request if self is a controller. If self is a + # view, check the parent controller's request. + def asset_request + if respond_to?(:request) + request + elsif respond_to?(:controller) && controller.respond_to?(:request) + controller.request + end + end end end end diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 2059501fccfb..08c927a4ab0a 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -13,6 +13,8 @@ def config class AssetTagHelperTest < ActionView::TestCase tests ActionView::Helpers::AssetTagHelper + attr_reader :request + def setup super silence_warnings do @@ -598,6 +600,8 @@ def test_caching_image_path_with_caching_and_proc_asset_host_using_request class AssetTagHelperNonVhostTest < ActionView::TestCase tests ActionView::Helpers::AssetTagHelper + attr_reader :request + def setup super @controller = BasicController.new @@ -720,3 +724,32 @@ def test_assert_css_and_js_of_the_same_name_return_correct_extension assert_dom_equal(%(/collaboration/hieraki/stylesheets/foo.css), stylesheet_path("foo")) end end + +class AssetUrlHelperControllerTest < ActionView::TestCase + tests ActionView::Helpers::AssetUrlHelper + + def setup + super + + @controller = BasicController.new + @controller.extend ActionView::Helpers::AssetUrlHelper + + @request = Class.new do + attr_accessor :script_name + def protocol() 'http://' end + def ssl?() false end + def host_with_port() 'www.example.com' end + def base_url() 'http://www.example.com' end + end.new + + @controller.request = @request + end + + def test_asset_path + assert_equal "/foo", @controller.asset_path("foo") + end + + def test_asset_url + assert_equal "http://www.example.com/foo", @controller.asset_url("foo") + end +end From dee3a192744ee11bf3fe5ac69aa099a328288e81 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 12 Oct 2012 17:09:32 -0500 Subject: [PATCH 04/15] JAVASCRIPTS_DIR, STYLESHEETS_DIR, ASSETS_DIR don't even exist anymore --- .../test/template/asset_tag_helper_test.rb | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 08c927a4ab0a..43737b81e60d 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -17,25 +17,6 @@ class AssetTagHelperTest < ActionView::TestCase def setup super - silence_warnings do - ActionView::Helpers::AssetTagHelper.send( - :const_set, - :JAVASCRIPTS_DIR, - File.dirname(__FILE__) + "/../fixtures/public/javascripts" - ) - - ActionView::Helpers::AssetTagHelper.send( - :const_set, - :STYLESHEETS_DIR, - File.dirname(__FILE__) + "/../fixtures/public/stylesheets" - ) - - ActionView::Helpers::AssetTagHelper.send( - :const_set, - :ASSETS_DIR, - File.dirname(__FILE__) + "/../fixtures/public" - ) - end @controller = BasicController.new @@ -353,15 +334,6 @@ def test_javascript_include_tag_is_html_safe assert javascript_include_tag("prototype").html_safe? end - def test_all_javascript_expansion_not_include_application_js_if_not_exists - FileUtils.mv(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.js'), - File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.bak')) - assert_no_match(/application\.js/, javascript_include_tag(:all)) - ensure - FileUtils.mv(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.bak'), - File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'application.js')) - end - def test_stylesheet_path ENV["RAILS_ASSET_ID"] = "" StylePathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } From aa493f04d35c2aa3207db0ee6f5568b09ea764a9 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Fri, 12 Oct 2012 17:28:53 -0500 Subject: [PATCH 05/15] All asset url helpers should pass options --- .../action_view/helpers/asset_url_helper.rb | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index f2213efbb945..9255c1e2f665 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -218,15 +218,15 @@ def compute_asset_host(source = "", options = {}) # javascript_path "/dir/xmlhr" # => /dir/xmlhr.js # javascript_path "http://www.example.com/js/xmlhr" # => http://www.example.com/js/xmlhr # javascript_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js - def javascript_path(source) - path_to_asset(source, type: :javascript) + def javascript_path(source, options = {}) + path_to_asset(source, {type: :javascript}.merge(options)) end alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route # Computes the full URL to a javascript asset in the public javascripts directory. # This will use +javascript_path+ internally, so most of their behaviors will be the same. - def javascript_url(source) - url_to_asset(source, type: :javascript) + def javascript_url(source, options = {}) + url_to_asset(source, {type: :javascript}.merge(options)) end alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route @@ -240,15 +240,15 @@ def javascript_url(source) # stylesheet_path "/dir/style.css" # => /dir/style.css # stylesheet_path "http://www.example.com/css/style" # => http://www.example.com/css/style # stylesheet_path "http://www.example.com/css/style.css" # => http://www.example.com/css/style.css - def stylesheet_path(source) - path_to_asset(source, type: :stylesheet) + def stylesheet_path(source, options = {}) + path_to_asset(source, {type: :stylesheet}.merge(options)) end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route # Computes the full URL to a stylesheet asset in the public stylesheets directory. # This will use +stylesheet_path+ internally, so most of their behaviors will be the same. - def stylesheet_url(source) - url_to_asset(source, type: :stylesheet) + def stylesheet_url(source, options = {}) + url_to_asset(source, {type: :stylesheet}.merge(options)) end alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route @@ -265,15 +265,15 @@ def stylesheet_url(source) # If you have images as application resources this method may conflict with their named routes. # The alias +path_to_image+ is provided to avoid that. Rails uses the alias internally, and # plugin authors are encouraged to do so. - def image_path(source) - path_to_asset(source, type: :image) + def image_path(source, options = {}) + path_to_asset(source, {type: :image}.merge(options)) end alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route # Computes the full URL to an image asset. # This will use +image_path+ internally, so most of their behaviors will be the same. - def image_url(source) - url_to_asset(source, type: :image) + def image_url(source, options = {}) + url_to_asset(source, {type: :image}.merge(options)) end alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route @@ -286,15 +286,15 @@ def image_url(source) # video_path("trailers/hd.avi") # => /videos/trailers/hd.avi # video_path("/trailers/hd.avi") # => /trailers/hd.avi # video_path("http://www.example.com/vid/hd.avi") # => http://www.example.com/vid/hd.avi - def video_path(source) - path_to_asset(source, type: :video) + def video_path(source, options = {}) + path_to_asset(source, {type: :video}.merge(options)) end alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route # Computes the full URL to a video asset in the public videos directory. # This will use +video_path+ internally, so most of their behaviors will be the same. - def video_url(source) - url_to_asset(source, type: :video) + def video_url(source, options = {}) + url_to_asset(source, {type: :video}.merge(options)) end alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route @@ -307,15 +307,15 @@ def video_url(source) # audio_path("sounds/horse.wav") # => /audios/sounds/horse.wav # audio_path("/sounds/horse.wav") # => /sounds/horse.wav # audio_path("http://www.example.com/sounds/horse.wav") # => http://www.example.com/sounds/horse.wav - def audio_path(source) - path_to_asset(source, type: :audio) + def audio_path(source, options = {}) + path_to_asset(source, {type: :audio}.merge(options)) end alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route # Computes the full URL to an audio asset in the public audios directory. # This will use +audio_path+ internally, so most of their behaviors will be the same. - def audio_url(source) - url_to_asset(source, type: :audio) + def audio_url(source, options = {}) + url_to_asset(source, {type: :audio}.merge(options)) end alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route @@ -327,15 +327,15 @@ def audio_url(source) # font_path("dir/font.ttf") # => /assets/dir/font.ttf # font_path("/dir/font.ttf") # => /dir/font.ttf # font_path("http://www.example.com/dir/font.ttf") # => http://www.example.com/dir/font.ttf - def font_path(source) - path_to_asset(source, type: :font) + def font_path(source, options = {}) + path_to_asset(source, {type: :font}.merge(options)) end alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route # Computes the full URL to a font asset. # This will use +font_path+ internally, so most of their behaviors will be the same. - def font_url(source) - url_to_asset(source, type: :font) + def font_url(source, options = {}) + url_to_asset(source, {type: :font}.merge(options)) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route From 5dfeb1b852c6c4a7b9c4aa3fa8dc5f25b516f9f1 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 13 Oct 2012 09:59:57 -0500 Subject: [PATCH 06/15] Add a few more compute_asset_path tests --- actionpack/test/template/asset_tag_helper_test.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 43737b81e60d..5dc854d56151 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -295,6 +295,14 @@ def test_asset_path_tag AssetPathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end + def test_compute_asset_public_path + assert_equal "/robots.txt", compute_asset_path("robots.txt") + assert_equal "/robots.txt", compute_asset_path("/robots.txt") + assert_equal "/javascripts/foo.js", compute_asset_path("foo.js", :type => :javascript) + assert_equal "/javascripts/foo.js", compute_asset_path("/foo.js", :type => :javascript) + assert_equal "/stylesheets/foo.css", compute_asset_path("foo.css", :type => :stylesheet) + end + def test_auto_discovery_link_tag AutoDiscoveryToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end From 60a4fffd83e94ad4471570b16f9d954f04bc0300 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sat, 13 Oct 2012 10:13:47 -0500 Subject: [PATCH 07/15] Allow asset url config to be undefined --- .../action_view/helpers/asset_url_helper.rb | 6 ++- .../test/template/asset_tag_helper_test.rb | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index 9255c1e2f665..a0fcac59a3ea 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -135,7 +135,9 @@ def asset_path(source, options = {}) source = compute_asset_path(source, options) end - if relative_url_root = config.relative_url_root || asset_request.try(:script_name) + relative_url_root = (defined?(config.relative_url_root) && config.relative_url_root) || + (asset_request && asset_request.script_name) + if relative_url_root source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/") end @@ -180,7 +182,7 @@ def compute_asset_path(source, options = {}) # (proc or otherwise). def compute_asset_host(source = "", options = {}) request = asset_request - host = config.asset_host + host = config.asset_host if defined? config.asset_host host ||= request.base_url if request && options[:protocol] == :request return unless host diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 5dc854d56151..8435db3166ae 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -733,3 +733,44 @@ def test_asset_url assert_equal "http://www.example.com/foo", @controller.asset_url("foo") end end + +class AssetUrlHelperEmptyModuleTest < ActionView::TestCase + tests ActionView::Helpers::AssetUrlHelper + + def setup + super + + @module = Module.new + @module.extend ActionView::Helpers::AssetUrlHelper + end + + def test_asset_path + assert_equal "/foo", @module.asset_path("foo") + end + + def test_asset_url + assert_equal "/foo", @module.asset_url("foo") + end + + def test_asset_url_with_request + @module.instance_eval do + def request + Struct.new(:base_url, :script_name).new("http://www.example.com", nil) + end + end + + assert @module.request + assert_equal "http://www.example.com/foo", @module.asset_url("foo") + end + + def test_asset_url_with_config_asset_host + @module.instance_eval do + def config + Struct.new(:asset_host).new("http://www.example.com") + end + end + + assert @module.config.asset_host + assert_equal "http://www.example.com/foo", @module.asset_url("foo") + end +end From e6451a5599e92fa07e7c39562bacfd1824d199f8 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 09:40:08 -0500 Subject: [PATCH 08/15] Just check request instead of controller.request --- .../lib/action_view/helpers/asset_url_helper.rb | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index a0fcac59a3ea..a7dccb8fc776 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -136,7 +136,7 @@ def asset_path(source, options = {}) end relative_url_root = (defined?(config.relative_url_root) && config.relative_url_root) || - (asset_request && asset_request.script_name) + (respond_to?(:request) && request.try(:script_name)) if relative_url_root source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/") end @@ -181,7 +181,7 @@ def compute_asset_path(source, options = {}) # or the value returned from invoking call on an object responding to call # (proc or otherwise). def compute_asset_host(source = "", options = {}) - request = asset_request + request = self.request if respond_to?(:request) host = config.asset_host if defined? config.asset_host host ||= request.base_url if request && options[:protocol] == :request return unless host @@ -340,17 +340,6 @@ def font_url(source, options = {}) url_to_asset(source, {type: :font}.merge(options)) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route - - private - # Get current request if self is a controller. If self is a - # view, check the parent controller's request. - def asset_request - if respond_to?(:request) - request - elsif respond_to?(:controller) && controller.respond_to?(:request) - controller.request - end - end end end end From 511382bfddb1400a461db84224369551ec93caa6 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 09:41:34 -0500 Subject: [PATCH 09/15] merge! default asset tag options --- .../action_view/helpers/asset_url_helper.rb | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index a7dccb8fc776..72a9dff82cec 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -221,14 +221,14 @@ def compute_asset_host(source = "", options = {}) # javascript_path "http://www.example.com/js/xmlhr" # => http://www.example.com/js/xmlhr # javascript_path "http://www.example.com/js/xmlhr.js" # => http://www.example.com/js/xmlhr.js def javascript_path(source, options = {}) - path_to_asset(source, {type: :javascript}.merge(options)) + path_to_asset(source, {type: :javascript}.merge!(options)) end alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route # Computes the full URL to a javascript asset in the public javascripts directory. # This will use +javascript_path+ internally, so most of their behaviors will be the same. def javascript_url(source, options = {}) - url_to_asset(source, {type: :javascript}.merge(options)) + url_to_asset(source, {type: :javascript}.merge!(options)) end alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route @@ -243,14 +243,14 @@ def javascript_url(source, options = {}) # stylesheet_path "http://www.example.com/css/style" # => http://www.example.com/css/style # stylesheet_path "http://www.example.com/css/style.css" # => http://www.example.com/css/style.css def stylesheet_path(source, options = {}) - path_to_asset(source, {type: :stylesheet}.merge(options)) + path_to_asset(source, {type: :stylesheet}.merge!(options)) end alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route # Computes the full URL to a stylesheet asset in the public stylesheets directory. # This will use +stylesheet_path+ internally, so most of their behaviors will be the same. def stylesheet_url(source, options = {}) - url_to_asset(source, {type: :stylesheet}.merge(options)) + url_to_asset(source, {type: :stylesheet}.merge!(options)) end alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route @@ -268,14 +268,14 @@ def stylesheet_url(source, options = {}) # The alias +path_to_image+ is provided to avoid that. Rails uses the alias internally, and # plugin authors are encouraged to do so. def image_path(source, options = {}) - path_to_asset(source, {type: :image}.merge(options)) + path_to_asset(source, {type: :image}.merge!(options)) end alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route # Computes the full URL to an image asset. # This will use +image_path+ internally, so most of their behaviors will be the same. def image_url(source, options = {}) - url_to_asset(source, {type: :image}.merge(options)) + url_to_asset(source, {type: :image}.merge!(options)) end alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route @@ -289,14 +289,14 @@ def image_url(source, options = {}) # video_path("/trailers/hd.avi") # => /trailers/hd.avi # video_path("http://www.example.com/vid/hd.avi") # => http://www.example.com/vid/hd.avi def video_path(source, options = {}) - path_to_asset(source, {type: :video}.merge(options)) + path_to_asset(source, {type: :video}.merge!(options)) end alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route # Computes the full URL to a video asset in the public videos directory. # This will use +video_path+ internally, so most of their behaviors will be the same. def video_url(source, options = {}) - url_to_asset(source, {type: :video}.merge(options)) + url_to_asset(source, {type: :video}.merge!(options)) end alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route @@ -310,14 +310,14 @@ def video_url(source, options = {}) # audio_path("/sounds/horse.wav") # => /sounds/horse.wav # audio_path("http://www.example.com/sounds/horse.wav") # => http://www.example.com/sounds/horse.wav def audio_path(source, options = {}) - path_to_asset(source, {type: :audio}.merge(options)) + path_to_asset(source, {type: :audio}.merge!(options)) end alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route # Computes the full URL to an audio asset in the public audios directory. # This will use +audio_path+ internally, so most of their behaviors will be the same. def audio_url(source, options = {}) - url_to_asset(source, {type: :audio}.merge(options)) + url_to_asset(source, {type: :audio}.merge!(options)) end alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route @@ -330,14 +330,14 @@ def audio_url(source, options = {}) # font_path("/dir/font.ttf") # => /dir/font.ttf # font_path("http://www.example.com/dir/font.ttf") # => http://www.example.com/dir/font.ttf def font_path(source, options = {}) - path_to_asset(source, {type: :font}.merge(options)) + path_to_asset(source, {type: :font}.merge!(options)) end alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route # Computes the full URL to a font asset. # This will use +font_path+ internally, so most of their behaviors will be the same. def font_url(source, options = {}) - url_to_asset(source, {type: :font}.merge(options)) + url_to_asset(source, {type: :font}.merge!(options)) end alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route end From 3db69909b9d4549128984c4c76f8a51eca3d79e6 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 09:47:16 -0500 Subject: [PATCH 10/15] :fire: Rails asset id support --- .../lib/abstract_controller/asset_paths.rb | 2 +- actionpack/lib/action_controller/railtie.rb | 1 - actionpack/lib/action_view/helpers.rb | 1 - .../action_view/helpers/asset_id_helper.rb | 154 ------------------ .../action_view/helpers/asset_tag_helper.rb | 1 - actionpack/lib/action_view/railtie.rb | 8 - .../test/template/asset_tag_helper_test.rb | 81 +-------- 7 files changed, 2 insertions(+), 246 deletions(-) delete mode 100644 actionpack/lib/action_view/helpers/asset_id_helper.rb diff --git a/actionpack/lib/abstract_controller/asset_paths.rb b/actionpack/lib/abstract_controller/asset_paths.rb index 822254b1a403..e6170228d9ce 100644 --- a/actionpack/lib/abstract_controller/asset_paths.rb +++ b/actionpack/lib/abstract_controller/asset_paths.rb @@ -3,7 +3,7 @@ module AssetPaths #:nodoc: extend ActiveSupport::Concern included do - config_accessor :asset_host, :asset_path, :assets_dir, :javascripts_dir, + config_accessor :asset_host, :assets_dir, :javascripts_dir, :stylesheets_dir, :default_asset_host_protocol, :relative_url_root end end diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index ee0f053bad1a..3e44155f7363 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -34,7 +34,6 @@ class Railtie < Rails::Railtie #:nodoc: options.stylesheets_dir ||= paths["public/stylesheets"].first # Ensure readers methods get compiled - options.asset_path ||= app.config.asset_path options.asset_host ||= app.config.asset_host options.relative_url_root ||= app.config.relative_url_root diff --git a/actionpack/lib/action_view/helpers.rb b/actionpack/lib/action_view/helpers.rb index cc140871892e..dad50a58a15d 100644 --- a/actionpack/lib/action_view/helpers.rb +++ b/actionpack/lib/action_view/helpers.rb @@ -3,7 +3,6 @@ module Helpers #:nodoc: extend ActiveSupport::Autoload autoload :ActiveModelHelper - autoload :AssetIdHelper autoload :AssetTagHelper autoload :AssetUrlHelper autoload :AtomFeedHelper diff --git a/actionpack/lib/action_view/helpers/asset_id_helper.rb b/actionpack/lib/action_view/helpers/asset_id_helper.rb deleted file mode 100644 index ba8e5e90500f..000000000000 --- a/actionpack/lib/action_view/helpers/asset_id_helper.rb +++ /dev/null @@ -1,154 +0,0 @@ -require 'thread' -require 'active_support/core_ext/file' -require 'active_support/core_ext/module/attribute_accessors' - -module ActionView - # = Action View Asset Cache ID Helpers - # - # Rails appends asset's timestamps to public asset paths. This allows - # you to set a cache-expiration date for the asset far into the future, but - # still be able to instantly invalidate it by simply updating the file (and - # hence updating the timestamp, which then updates the URL as the timestamp - # is part of that, which in turn busts the cache). - # - # It's the responsibility of the web server you use to set the far-future - # expiration date on cache assets that you need to take advantage of this - # feature. Here's an example for Apache: - # - # # Asset Expiration - # ExpiresActive On - # - # ExpiresDefault "access plus 1 year" - # - # - # Also note that in order for this to work, all your application servers must - # return the same timestamps. This means that they must have their clocks - # synchronized. If one of them drifts out of sync, you'll see different - # timestamps at random and the cache won't work. In that case the browser - # will request the same assets over and over again even thought they didn't - # change. You can use something like Live HTTP Headers for Firefox to verify - # that the cache is indeed working. - # - # This strategy works well enough for most server setups and requires the - # least configuration, but if you deploy several application servers at - # different times - say to handle a temporary spike in load - then the - # asset time stamps will be out of sync. In a setup like this you may want - # to set the way that asset paths are generated yourself. - # - # Altering the asset paths that Rails generates can be done in two ways. - # The easiest is to define the RAILS_ASSET_ID environment variable. The - # contents of this variable will always be used in preference to - # calculated timestamps. A more complex but flexible way is to set - # ActionController::Base.config.asset_path to a proc - # that takes the unmodified asset path and returns the path needed for - # your asset caching to work. Typically you'd do something like this in - # config/environments/production.rb: - # - # # Normally you'd calculate RELEASE_NUMBER at startup. - # RELEASE_NUMBER = 12345 - # config.action_controller.asset_path = proc { |asset_path| - # "/release-#{RELEASE_NUMBER}#{asset_path}" - # } - # - # This example would cause the following behavior on all servers no - # matter when they were deployed: - # - # image_tag("rails.png") - # # => Rails - # stylesheet_link_tag("application") - # # => - # - # Changing the asset_path does require that your web servers have - # knowledge of the asset template paths that you rewrite to so it's not - # suitable for out-of-the-box use. To use the example given above you - # could use something like this in your Apache VirtualHost configuration: - # - # - # # Some browsers still send conditional-GET requests if there's a - # # Last-Modified header or an ETag header even if they haven't - # # reached the expiry date sent in the Expires header. - # Header unset Last-Modified - # Header unset ETag - # FileETag None - # - # # Assets requested using a cache-busting filename should be served - # # only once and then cached for a really long time. The HTTP/1.1 - # # spec frowns on hugely-long expiration times though and suggests - # # that assets which never expire be served with an expiration date - # # 1 year from access. - # ExpiresActive On - # ExpiresDefault "access plus 1 year" - # - # - # # We use cached-busting location names with the far-future expires - # # headers to ensure that if a file does change it can force a new - # # request. The actual asset filenames are still the same though so we - # # need to rewrite the location from the cache-busting location to the - # # real asset location so that we can serve it. - # RewriteEngine On - # RewriteRule ^/release-\d+/(images|javascripts|stylesheets)/(.*)$ /$1/$2 [L] - # - module Helpers #:nodoc: - module AssetIdHelper - # You can enable or disable the asset tag ids cache. - # With the cache enabled, the asset tag helper methods will make fewer - # expensive file system calls (the default implementation checks the file - # system timestamp). However this prevents you from modifying any asset - # files while the server is running. - # - # ActionView::Helpers::AssetIdHelper.cache_asset_ids = false - mattr_accessor :cache_asset_ids - - # Add or change an asset id in the asset id cache. This can be used - # for SASS on Heroku. - # :api: public - def add_to_asset_ids_cache(source, asset_id) - self.asset_ids_cache_guard.synchronize do - self.asset_ids_cache[source] = asset_id - end - end - - mattr_accessor :asset_ids_cache - self.asset_ids_cache = {} - - mattr_accessor :asset_ids_cache_guard - self.asset_ids_cache_guard = Mutex.new - - # Use the RAILS_ASSET_ID environment variable or the source's - # modification time as its cache-busting asset id. - def rails_asset_id(source) - if asset_id = ENV["RAILS_ASSET_ID"] - asset_id - else - if self.cache_asset_ids && (asset_id = self.asset_ids_cache[source]) - asset_id - else - path = File.join(config.assets_dir, source) - asset_id = File.exist?(path) ? File.mtime(path).to_i.to_s : '' - - if self.cache_asset_ids - add_to_asset_ids_cache(source, asset_id) - end - - asset_id - end - end - end - - # Override +compute_asset_path+ to add asset id query strings to - # generated urls. See +compute_asset_path+ in AssetUrlHelper. - def compute_asset_path(source, options = {}) - source = super(source, options) - path = config.asset_path - - if path && path.respond_to?(:call) - path.call(source) - elsif path && path.is_a?(String) - path % [source] - elsif asset_id = rails_asset_id(source) - asset_id.empty? ? source : "#{source}?#{asset_id}" - end - end - end - end -end diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 3beea2eb7c6c..4eac6514df6d 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -19,7 +19,6 @@ module AssetTagHelper extend ActiveSupport::Concern include AssetUrlHelper - include AssetIdHelper include TagHelper # Returns an HTML script tag for each of the +sources+ provided. diff --git a/actionpack/lib/action_view/railtie.rb b/actionpack/lib/action_view/railtie.rb index 2d5f664402a0..3875d88a9f9c 100644 --- a/actionpack/lib/action_view/railtie.rb +++ b/actionpack/lib/action_view/railtie.rb @@ -20,14 +20,6 @@ class Railtie < Rails::Railtie ActiveSupport.on_load(:action_view) { self.logger ||= Rails.logger } end - initializer "action_view.cache_asset_ids" do |app| - unless app.config.cache_classes - ActiveSupport.on_load(:action_view) do - ActionView::Helpers::AssetIdHelper.cache_asset_ids = false - end - end - end - initializer "action_view.set_configs" do |app| ActiveSupport.on_load(:action_view) do app.config.action_view.each do |k,v| diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 8435db3166ae..30bd5c159c1e 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -35,10 +35,6 @@ def url_for(*args) "http://www.example.com" end - def teardown - ENV.delete('RAILS_ASSET_ID') - end - AssetPathToTag = { %(asset_path("foo")) => %(/foo), %(asset_path("style.css")) => %(/style.css), @@ -291,7 +287,6 @@ def test_autodiscovery_link_tag_deprecated_types end def test_asset_path_tag - ENV["RAILS_ASSET_ID"] = "" AssetPathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -323,8 +318,7 @@ def test_url_to_javascript_alias_for_javascript_url UrlToJavascriptToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end - def test_javascript_include_tag_with_blank_asset_id - ENV["RAILS_ASSET_ID"] = "" + def test_javascript_include_tag JavascriptIncludeToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -343,7 +337,6 @@ def test_javascript_include_tag_is_html_safe end def test_stylesheet_path - ENV["RAILS_ASSET_ID"] = "" StylePathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -352,7 +345,6 @@ def test_path_to_stylesheet_alias_for_stylesheet_path end def test_stylesheet_url - ENV["RAILS_ASSET_ID"] = "" StyleUrlToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -361,7 +353,6 @@ def test_url_to_stylesheet_alias_for_stylesheet_url end def test_stylesheet_link_tag - ENV["RAILS_ASSET_ID"] = "" StyleLinkToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -376,7 +367,6 @@ def test_stylesheet_link_tag_with_missing_source end def test_stylesheet_link_tag_is_html_safe - ENV["RAILS_ASSET_ID"] = "" assert stylesheet_link_tag('dir/file').html_safe? assert stylesheet_link_tag('dir/other/file', 'dir/file2').html_safe? end @@ -386,7 +376,6 @@ def test_stylesheet_link_tag_escapes_options end def test_stylesheet_link_tag_should_not_output_the_same_asset_twice - ENV["RAILS_ASSET_ID"] = "" assert_dom_equal %(\n), stylesheet_link_tag('wellington', 'wellington', 'amsterdam') end @@ -428,21 +417,6 @@ def test_favicon_link_tag FaviconLinkToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end - def test_image_tag_windows_behaviour - old_asset_id, ENV["RAILS_ASSET_ID"] = ENV["RAILS_ASSET_ID"], "1" - # This simulates the behavior of File#exist? on windows when testing a file ending in "." - # If the file "rails.png" exists, windows will return true when asked if "rails.png." exists (notice trailing ".") - # OS X, linux etc will return false in this case. - File.stubs(:exist?).with('template/../fixtures/public/images/rails.png.').returns(true) - assert_equal 'Rails', image_tag('rails.png') - ensure - if old_asset_id - ENV["RAILS_ASSET_ID"] = old_asset_id - else - ENV.delete("RAILS_ASSET_ID") - end - end - def test_video_path VideoPathToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) } end @@ -491,27 +465,6 @@ def test_video_audio_tag_does_not_modify_options assert_equal({:autoplay => true}, options) end - def test_timebased_asset_id - expected_time = File.mtime(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).to_i.to_s - assert_equal %(Rails), image_tag("rails.png") - end - - def test_string_asset_id - @controller.config.asset_path = "/assets.v12345%s" - - expected_path = "/assets.v12345/images/rails.png" - assert_equal %(Rails), image_tag("rails.png") - end - - def test_proc_asset_id - @controller.config.asset_path = Proc.new do |asset_path| - "/assets.v12345#{asset_path}" - end - - expected_path = "/assets.v12345/images/rails.png" - assert_equal %(Rails), image_tag("rails.png") - end - def test_image_tag_interpreting_email_cid_correctly # An inline image has no need for an alt tag to be automatically generated from the cid: assert_equal '', image_tag("cid:thi%25%25sis@acontentid") @@ -521,37 +474,6 @@ def test_image_tag_interpreting_email_adding_optional_alt_tag assert_equal 'Image', image_tag("cid:thi%25%25sis@acontentid", :alt => "Image") end - def test_timebased_asset_id_with_relative_url_root - @controller.config.relative_url_root = "/collaboration/hieraki" - expected_time = File.mtime(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).to_i.to_s - assert_equal %(Rails), image_tag("rails.png") - end - - # Same as above, but with script_name - def test_timebased_asset_id_with_script_name - @request.script_name = "/collaboration/hieraki" - expected_time = File.mtime(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).to_i.to_s - assert_equal %(Rails), image_tag("rails.png") - end - - def test_should_skip_asset_id_on_complete_url - assert_equal %(Rails), image_tag("http://www.example.com/rails.png") - end - - def test_should_skip_asset_id_on_scheme_relative_url - assert_equal %(Rails), image_tag("//www.example.com/rails.png") - end - - def test_should_use_preset_asset_id - ENV["RAILS_ASSET_ID"] = "4500" - assert_equal %(Rails), image_tag("rails.png") - end - - def test_preset_empty_asset_id - ENV["RAILS_ASSET_ID"] = "" - assert_equal %(Rails), image_tag("rails.png") - end - def test_should_not_modify_source_string source = '/images/rails.png' copy = source.dup @@ -560,7 +482,6 @@ def test_should_not_modify_source_string end def test_caching_image_path_with_caching_and_proc_asset_host_using_request - ENV['RAILS_ASSET_ID'] = '' @controller.config.asset_host = Proc.new do |source, request| if request.ssl? "#{request.protocol}#{request.host_with_port}" From 7dba1599d9092a8362956a3fab23b2c60eedea63 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 09:51:20 -0500 Subject: [PATCH 11/15] Remove old asset_path from rails config --- actionmailer/lib/action_mailer/railtie.rb | 1 - .../lib/rails/application/configuration.rb | 6 +----- .../test/application/configuration_test.rb | 20 ------------------- 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/actionmailer/lib/action_mailer/railtie.rb b/actionmailer/lib/action_mailer/railtie.rb index abf6ad80cf16..59dc26841fc9 100644 --- a/actionmailer/lib/action_mailer/railtie.rb +++ b/actionmailer/lib/action_mailer/railtie.rb @@ -22,7 +22,6 @@ class Railtie < Rails::Railtie # :nodoc: options.queue ||= app.queue # make sure readers methods get compiled - options.asset_path ||= app.config.asset_path options.asset_host ||= app.config.asset_host options.relative_url_root ||= app.config.relative_url_root diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index a7a35c268527..6b9cbd8cd1c2 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -6,7 +6,7 @@ module Rails class Application class Configuration < ::Rails::Engine::Configuration - attr_accessor :asset_host, :asset_path, :assets, :autoflush_log, + attr_accessor :asset_host, :assets, :autoflush_log, :cache_classes, :cache_store, :consider_all_requests_local, :console, :eager_load, :exceptions_app, :file_watcher, :filter_parameters, :force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags, @@ -64,10 +64,6 @@ def initialize(*) @assets.logger = nil end - def compiled_asset_path - "/" - end - def encoding=(value) @encoding = value silence_warnings do diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 07d47dc67bf7..3ca0889cf57f 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -162,11 +162,6 @@ def teardown assert AppTemplate::Application, AppTemplate::Application.config.eager_load_namespaces end - test "asset_path defaults to nil for application" do - require "#{app_path}/config/environment" - assert_equal nil, AppTemplate::Application.config.asset_path - end - test "the application can be eager loaded even when there are no frameworks" do FileUtils.rm_rf("#{app_path}/config/environments") add_to_config <<-RUBY @@ -441,21 +436,6 @@ def index end end - test "config.asset_path is not passed through env" do - make_basic_app do |app| - app.config.asset_path = "/omg%s" - end - - class ::OmgController < ActionController::Base - def index - render :inline => "<%= image_path('foo.jpg') %>" - end - end - - get "/" - assert_equal "/omg/images/foo.jpg", last_response.body - end - test "config.action_view.cache_template_loading with cache_classes default" do add_to_config "config.cache_classes = true" require "#{app_path}/config/environment" From 8d7fc73e98e3e70d8ded7f5936f7b69ae3de56b2 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 10:22:22 -0500 Subject: [PATCH 12/15] Use sprockets-rails branch for now --- railties/lib/rails/generators/app_base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index e761e26b04e1..0b922d29a498 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -195,7 +195,7 @@ def assets_gemfile_entry # Gems used only for assets and not required # in production environments by default. group :assets do - gem 'sprockets-rails', github: 'rails/sprockets-rails' + gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'new-asset-url-api' gem 'sass-rails', github: 'rails/sass-rails' gem 'coffee-rails', github: 'rails/coffee-rails' @@ -209,7 +209,7 @@ def assets_gemfile_entry # Gems used only for assets and not required # in production environments by default. group :assets do - gem 'sprockets-rails', github: 'rails/sprockets-rails' + gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'new-asset-url-api' gem 'sass-rails', '~> 4.0.0.beta' gem 'coffee-rails', '~> 4.0.0.beta' From 9d412388218b67a10830bdb30611198c5acdad50 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 10:23:07 -0500 Subject: [PATCH 13/15] Sprockets-rails branch --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 0fff8a8967ed..94906a4b12ca 100644 --- a/Gemfile +++ b/Gemfile @@ -20,7 +20,7 @@ gem 'activerecord-deprecated_finders', github: 'rails/activerecord-deprecated_fi # it being automatically loaded by sprockets gem 'uglifier', require: false -gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'master' +gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'new-asset-url-api' group :doc do # The current sdoc cannot generate GitHub links due From 6601917ad96aa7e0fdaf96058cafe01fbf00bc12 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 10:57:32 -0500 Subject: [PATCH 14/15] Extract compute_asset_extname and allow extname to be disabled --- .../action_view/helpers/asset_url_helper.rb | 22 +++++++++++++------ .../test/template/asset_tag_helper_test.rb | 21 +++++++++++++----- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_url_helper.rb b/actionpack/lib/action_view/helpers/asset_url_helper.rb index 72a9dff82cec..d75e4c0edc80 100644 --- a/actionpack/lib/action_view/helpers/asset_url_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_url_helper.rb @@ -107,11 +107,6 @@ module Helpers #:nodoc: module AssetUrlHelper URI_REGEXP = %r{^[-a-z]+://|^(?:cid|data):|^//} - ASSET_EXTENSIONS = { - javascript: '.js', - stylesheet: '.css' - } - # Computes the path to asset in public directory. If :type # options is set, a file extension will be appended and scoped # to the corresponding public directory. @@ -127,8 +122,8 @@ def asset_path(source, options = {}) return "" unless source.present? return source if source =~ URI_REGEXP - if File.extname(source).empty? && (ext = ASSET_EXTENSIONS[options[:type]]) - source = "#{source}#{ext}" + if extname = compute_asset_extname(source, options) + source = "#{source}#{extname}" end if source[0] != ?/ @@ -157,6 +152,19 @@ def asset_url(source, options = {}) end alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route + ASSET_EXTENSIONS = { + javascript: '.js', + stylesheet: '.css' + } + + # Compute extname to append to asset path. Returns nil if + # nothing should be added. + def compute_asset_extname(source, options = {}) + return if options[:extname] == false + extname = options[:extname] || ASSET_EXTENSIONS[options[:type]] + extname if extname && File.extname(source) != extname + end + # Maps asset types to public directory. ASSET_PUBLIC_DIRECTORIES = { audio: '/audios', diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 30bd5c159c1e..26a229e2ba5c 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -43,6 +43,11 @@ def url_for(*args) %(asset_path("dir/xml.png")) => %(/dir/xml.png), %(asset_path("/dir/xml.png")) => %(/dir/xml.png), + %(asset_path("script.min")) => %(/script.min), + %(asset_path("script.min.js")) => %(/script.min.js), + %(asset_path("style.min")) => %(/style.min), + %(asset_path("style.min.css")) => %(/style.min.css), + %(asset_path("style", type: :stylesheet)) => %(/stylesheets/style.css), %(asset_path("xmlhr", type: :javascript)) => %(/javascripts/xmlhr.js), %(asset_path("xml.png", type: :image)) => %(/images/xml.png) @@ -66,7 +71,9 @@ def url_for(*args) JavascriptPathToTag = { %(javascript_path("xmlhr")) => %(/javascripts/xmlhr.js), %(javascript_path("super/xmlhr")) => %(/javascripts/super/xmlhr.js), - %(javascript_path("/super/xmlhr.js")) => %(/super/xmlhr.js) + %(javascript_path("/super/xmlhr.js")) => %(/super/xmlhr.js), + %(javascript_path("xmlhr.min")) => %(/javascripts/xmlhr.min.js), + %(javascript_path("xmlhr.min.js")) => %(/javascripts/xmlhr.min.js) } PathToJavascriptToTag = { @@ -91,7 +98,6 @@ def url_for(*args) %(javascript_include_tag("bank")) => %(), %(javascript_include_tag("bank.js")) => %(), %(javascript_include_tag("bank", :lang => "vbscript")) => %(), - %(javascript_include_tag("common.javascript", "/elsewhere/cools")) => %(\n), %(javascript_include_tag("http://example.com/all")) => %(), %(javascript_include_tag("http://example.com/all.js")) => %(), @@ -102,14 +108,17 @@ def url_for(*args) %(stylesheet_path("bank")) => %(/stylesheets/bank.css), %(stylesheet_path("bank.css")) => %(/stylesheets/bank.css), %(stylesheet_path('subdir/subdir')) => %(/stylesheets/subdir/subdir.css), - %(stylesheet_path('/subdir/subdir.css')) => %(/subdir/subdir.css) + %(stylesheet_path('/subdir/subdir.css')) => %(/subdir/subdir.css), + %(stylesheet_path("style.min")) => %(/stylesheets/style.min.css), + %(stylesheet_path("style.min.css")) => %(/stylesheets/style.min.css) } PathToStyleToTag = { %(path_to_stylesheet("style")) => %(/stylesheets/style.css), %(path_to_stylesheet("style.css")) => %(/stylesheets/style.css), %(path_to_stylesheet('dir/file')) => %(/stylesheets/dir/file.css), - %(path_to_stylesheet('/dir/file.rcss')) => %(/dir/file.rcss) + %(path_to_stylesheet('/dir/file.rcss', :extname => false)) => %(/dir/file.rcss), + %(path_to_stylesheet('/dir/file', :extname => '.rcss')) => %(/dir/file.rcss) } StyleUrlToTag = { @@ -123,7 +132,8 @@ def url_for(*args) %(url_to_stylesheet("style")) => %(http://www.example.com/stylesheets/style.css), %(url_to_stylesheet("style.css")) => %(http://www.example.com/stylesheets/style.css), %(url_to_stylesheet('dir/file')) => %(http://www.example.com/stylesheets/dir/file.css), - %(url_to_stylesheet('/dir/file.rcss')) => %(http://www.example.com/dir/file.rcss) + %(url_to_stylesheet('/dir/file.rcss', :extname => false)) => %(http://www.example.com/dir/file.rcss), + %(url_to_stylesheet('/dir/file', :extname => '.rcss')) => %(http://www.example.com/dir/file.rcss) } StyleLinkToTag = { @@ -132,7 +142,6 @@ def url_for(*args) %(stylesheet_link_tag("/elsewhere/file")) => %(), %(stylesheet_link_tag("subdir/subdir")) => %(), %(stylesheet_link_tag("bank", :media => "all")) => %(), - %(stylesheet_link_tag("random.styles", "/elsewhere/file")) => %(\n), %(stylesheet_link_tag("http://www.example.com/styles/style")) => %(), %(stylesheet_link_tag("http://www.example.com/styles/style.css")) => %(), From eafc2b0580ab7cbea12224746c9047f0cca11e2c Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Mon, 15 Oct 2012 11:46:05 -0500 Subject: [PATCH 15/15] Use sprockets-rails master --- Gemfile | 2 +- railties/lib/rails/generators/app_base.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 94906a4b12ca..0fff8a8967ed 100644 --- a/Gemfile +++ b/Gemfile @@ -20,7 +20,7 @@ gem 'activerecord-deprecated_finders', github: 'rails/activerecord-deprecated_fi # it being automatically loaded by sprockets gem 'uglifier', require: false -gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'new-asset-url-api' +gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'master' group :doc do # The current sdoc cannot generate GitHub links due diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 0b922d29a498..e761e26b04e1 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -195,7 +195,7 @@ def assets_gemfile_entry # Gems used only for assets and not required # in production environments by default. group :assets do - gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'new-asset-url-api' + gem 'sprockets-rails', github: 'rails/sprockets-rails' gem 'sass-rails', github: 'rails/sass-rails' gem 'coffee-rails', github: 'rails/coffee-rails' @@ -209,7 +209,7 @@ def assets_gemfile_entry # Gems used only for assets and not required # in production environments by default. group :assets do - gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: 'new-asset-url-api' + gem 'sprockets-rails', github: 'rails/sprockets-rails' gem 'sass-rails', '~> 4.0.0.beta' gem 'coffee-rails', '~> 4.0.0.beta'