Skip to content
This repository
Browse code

Wrap AssetTagHelper's computed public path cache in a threadsafe store

  • Loading branch information...
commit 7359597004a63277851acbc87f0267d3d63e17eb 1 parent 0eef4e5
Joshua Peek josh authored
4 actionpack/lib/action_view/base.rb
@@ -200,10 +200,6 @@ module CompiledTemplates #:nodoc:
200 200 end
201 201 include CompiledTemplates
202 202
203   - # Cache public asset paths
204   - cattr_reader :computed_public_paths
205   - @@computed_public_paths = {}
206   -
207 203 def self.helper_modules #:nodoc:
208 204 helpers = []
209 205 Dir.entries(File.expand_path("#{File.dirname(__FILE__)}/helpers")).sort.each do |file|
58 actionpack/lib/action_view/helpers/asset_tag_helper.rb
@@ -5,12 +5,12 @@
5 5 module ActionView
6 6 module Helpers #:nodoc:
7 7 # This module provides methods for generating HTML that links views to assets such
8   - # as images, javascripts, stylesheets, and feeds. These methods do not verify
9   - # the assets exist before linking to them.
  8 + # as images, javascripts, stylesheets, and feeds. These methods do not verify
  9 + # the assets exist before linking to them.
10 10 #
11 11 # === Using asset hosts
12 12 # By default, Rails links to these assets on the current host in the public
13   - # folder, but you can direct Rails to link to assets from a dedicated assets server by
  13 + # folder, but you can direct Rails to link to assets from a dedicated assets server by
14 14 # setting ActionController::Base.asset_host in your <tt>config/environment.rb</tt>. For example,
15 15 # let's say your asset host is <tt>assets.example.com</tt>.
16 16 #
@@ -22,16 +22,16 @@ module Helpers #:nodoc:
22 22 #
23 23 # This is useful since browsers typically open at most two connections to a single host,
24 24 # which means your assets often wait in single file for their turn to load. You can
25   - # alleviate this by using a <tt>%d</tt> wildcard in <tt>asset_host</tt> (for example, "assets%d.example.com")
  25 + # alleviate this by using a <tt>%d</tt> wildcard in <tt>asset_host</tt> (for example, "assets%d.example.com")
26 26 # to automatically distribute asset requests among four hosts (e.g., "assets0.example.com" through "assets3.example.com")
27   - # so browsers will open eight connections rather than two.
  27 + # so browsers will open eight connections rather than two.
28 28 #
29 29 # image_tag("rails.png")
30 30 # => <img src="http://assets0.example.com/images/rails.png" alt="Rails" />
31 31 # stylesheet_link_tag("application")
32 32 # => <link href="http://assets3.example.com/stylesheets/application.css" media="screen" rel="stylesheet" type="text/css" />
33 33 #
34   - # To do this, you can either setup 4 actual hosts, or you can use wildcard DNS to CNAME
  34 + # To do this, you can either setup 4 actual hosts, or you can use wildcard DNS to CNAME
35 35 # the wildcard to a single asset host. You can read more about setting up your DNS CNAME records from
36 36 # your ISP.
37 37 #
@@ -86,7 +86,7 @@ module Helpers #:nodoc:
86 86 # asset far into the future, but still be able to instantly invalidate it by simply updating the file (and hence updating the timestamp,
87 87 # which then updates the URL as the timestamp is part of that, which in turn busts the cache).
88 88 #
89   - # 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
  89 + # 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
90 90 # advantage of this feature. Here's an example for Apache:
91 91 #
92 92 # # Asset Expiration
@@ -95,16 +95,17 @@ module Helpers #:nodoc:
95 95 # ExpiresDefault "access plus 1 year"
96 96 # </FilesMatch>
97 97 #
98   - # Also note that in order for this to work, all your application servers must return the same timestamps. This means that they must
  98 + # Also note that in order for this to work, all your application servers must return the same timestamps. This means that they must
99 99 # have their clocks synchronized. If one of them drift out of sync, you'll see different timestamps at random and the cache won't
100 100 # work. Which means that the browser will request the same assets over and over again even thought they didn't change. You can use
101   - # something like Live HTTP Headers for Firefox to verify that the cache is indeed working (and that the assets are not being
  101 + # something like Live HTTP Headers for Firefox to verify that the cache is indeed working (and that the assets are not being
102 102 # requested over and over).
103 103 module AssetTagHelper
104 104 ASSETS_DIR = defined?(Rails.public_path) ? Rails.public_path : "public"
105 105 JAVASCRIPTS_DIR = "#{ASSETS_DIR}/javascripts"
106 106 STYLESHEETS_DIR = "#{ASSETS_DIR}/stylesheets"
107   -
  107 + JAVASCRIPT_DEFAULT_SOURCES = ['prototype', 'effects', 'dragdrop', 'controls'].map(&:to_s).freeze unless const_defined?(:JAVASCRIPT_DEFAULT_SOURCES)
  108 +
108 109 # Returns a link tag that browsers and news readers can use to auto-detect
109 110 # an RSS or ATOM feed. The +type+ can either be <tt>:rss</tt> (default) or
110 111 # <tt>:atom</tt>. Control the link options in url_for format using the
@@ -154,10 +155,6 @@ def javascript_path(source)
154 155 end
155 156 alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route
156 157
157   - JAVASCRIPT_DEFAULT_SOURCES = ['prototype', 'effects', 'dragdrop', 'controls'] unless const_defined?(:JAVASCRIPT_DEFAULT_SOURCES)
158   - @@javascript_expansions = { :defaults => JAVASCRIPT_DEFAULT_SOURCES.dup }
159   - @@stylesheet_expansions = {}
160   -
161 158 # Returns an html script tag for each of the +sources+ provided. You
162 159 # can pass in the filename (.js extension is optional) of javascript files
163 160 # that exist in your public/javascripts directory for inclusion into the
@@ -193,7 +190,7 @@ def javascript_path(source)
193 190 #
194 191 # * = The application.js file is only referenced if it exists
195 192 #
196   - # Though it's not really recommended practice, if you need to extend the default JavaScript set for any reason
  193 + # Though it's not really recommended practice, if you need to extend the default JavaScript set for any reason
197 194 # (e.g., you're going to be using a certain .js file in every action), then take a look at the register_javascript_include_default method.
198 195 #
199 196 # You can also include all javascripts in the javascripts directory using <tt>:all</tt> as the source:
@@ -218,7 +215,7 @@ def javascript_path(source)
218 215 # You can also cache multiple javascripts into one file, which requires less HTTP connections to download and can better be
219 216 # compressed by gzip (leading to faster transfers). Caching will only happen if ActionController::Base.perform_caching
220 217 # is set to <tt>true</tt> (which is the case by default for the Rails production environment, but not for the development
221   - # environment).
  218 + # environment).
222 219 #
223 220 # ==== Examples
224 221 # javascript_include_tag :all, :cache => true # when ActionController::Base.perform_caching is false =>
@@ -259,6 +256,8 @@ def javascript_include_tag(*sources)
259 256 end
260 257 end
261 258
  259 + @@javascript_expansions = { :defaults => JAVASCRIPT_DEFAULT_SOURCES.dup }
  260 +
262 261 # Register one or more javascript files to be included when <tt>symbol</tt>
263 262 # is passed to <tt>javascript_include_tag</tt>. This method is typically intended
264 263 # to be called from plugin initialization to register javascript files
@@ -274,6 +273,8 @@ def self.register_javascript_expansion(expansions)
274 273 @@javascript_expansions.merge!(expansions)
275 274 end
276 275
  276 + @@stylesheet_expansions = {}
  277 +
277 278 # Register one or more stylesheet files to be included when <tt>symbol</tt>
278 279 # is passed to <tt>stylesheet_link_tag</tt>. This method is typically intended
279 280 # to be called from plugin initialization to register stylesheet files
@@ -439,9 +440,9 @@ def image_path(source)
439 440 # <img alt="Icon" height="32" src="/icons/icon.gif" width="32" />
440 441 # image_tag("/icons/icon.gif", :class => "menu_icon") # =>
441 442 # <img alt="Icon" class="menu_icon" src="/icons/icon.gif" />
442   - # image_tag("mouse.png", :mouseover => "/images/mouse_over.png") # =>
  443 + # image_tag("mouse.png", :mouseover => "/images/mouse_over.png") # =>
443 444 # <img src="/images/mouse.png" onmouseover="this.src='/images/mouse_over.png'" onmouseout="this.src='/images/mouse.png'" alt="Mouse" />
444   - # image_tag("mouse.png", :mouseover => image_path("mouse_over.png")) # =>
  445 + # image_tag("mouse.png", :mouseover => image_path("mouse_over.png")) # =>
445 446 # <img src="/images/mouse.png" onmouseover="this.src='/images/mouse_over.png'" onmouseout="this.src='/images/mouse.png'" alt="Mouse" />
446 447 def image_tag(source, options = {})
447 448 options.symbolize_keys!
@@ -454,23 +455,15 @@ def image_tag(source, options = {})
454 455 end
455 456
456 457 if mouseover = options.delete(:mouseover)
457   - options[:onmouseover] = "this.src='#{image_path(mouseover)}'"
458   - options[:onmouseout] = "this.src='#{image_path(options[:src])}'"
  458 + options[:onmouseover] = "this.src='#{image_path(mouseover)}'"
  459 + options[:onmouseout] = "this.src='#{image_path(options[:src])}'"
459 460 end
460 461
461 462 tag("img", options)
462 463 end
463 464
464 465 private
465   - def file_exist?(path)
466   - @@file_exist_cache ||= {}
467   - if !(@@file_exist_cache[path] ||= File.exist?(path))
468   - @@file_exist_cache[path] = true
469   - false
470   - else
471   - true
472   - end
473   - end
  466 + COMPUTED_PUBLIC_PATHS = ActiveSupport::Cache::MemoryStore.new.silence!.threadsafe!
474 467
475 468 # Add the the extension +ext+ if not present. Return full URLs otherwise untouched.
476 469 # Prefix with <tt>/dir/</tt> if lacking a leading +/+. Account for relative URL
@@ -490,7 +483,7 @@ def compute_public_path(source, dir, ext = nil, include_host = true)
490 483 dir, source, ext, include_host ].join
491 484 end
492 485
493   - ActionView::Base.computed_public_paths[cache_key] ||=
  486 + source = COMPUTED_PUBLIC_PATHS.fetch(cache_key) do
494 487 begin
495 488 source += ".#{ext}" if ext && File.extname(source).blank? || File.exist?(File.join(ASSETS_DIR, dir, "#{source}.#{ext}"))
496 489
@@ -507,8 +500,7 @@ def compute_public_path(source, dir, ext = nil, include_host = true)
507 500 rewrite_asset_path(source)
508 501 end
509 502 end
510   -
511   - source = ActionView::Base.computed_public_paths[cache_key]
  503 + end
512 504
513 505 if include_host && source !~ %r{^[-a-z]+://}
514 506 host = compute_asset_host(source)
@@ -594,7 +586,7 @@ def expand_javascript_sources(sources, recursive = false)
594 586 expanded_sources = sources.collect do |source|
595 587 determine_source(source, @@javascript_expansions)
596 588 end.flatten
597   - expanded_sources << "application" if sources.include?(:defaults) && file_exist?(File.join(JAVASCRIPTS_DIR, "application.js"))
  589 + expanded_sources << "application" if sources.include?(:defaults) && File.exist?(File.join(JAVASCRIPTS_DIR, "application.js"))
598 590 expanded_sources
599 591 end
600 592 end
5 actionpack/test/template/asset_tag_helper_test.rb
@@ -39,8 +39,7 @@ def host_with_port() 'localhost' end
39 39 @controller.request = @request
40 40
41 41 ActionView::Helpers::AssetTagHelper::reset_javascript_include_default
42   -
43   - ActionView::Base.computed_public_paths.clear
  42 + COMPUTED_PUBLIC_PATHS.clear
44 43 end
45 44
46 45 def teardown
@@ -161,7 +160,7 @@ def test_javascript_include_tag
161 160 ENV["RAILS_ASSET_ID"] = ""
162 161 JavascriptIncludeToTag.each { |method, tag| assert_dom_equal(tag, eval(method)) }
163 162
164   - ActionView::Base.computed_public_paths.clear
  163 + COMPUTED_PUBLIC_PATHS.clear
165 164
166 165 ENV["RAILS_ASSET_ID"] = "1"
167 166 assert_dom_equal(%(<script src="/javascripts/prototype.js?1" type="text/javascript"></script>\n<script src="/javascripts/effects.js?1" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js?1" type="text/javascript"></script>\n<script src="/javascripts/controls.js?1" type="text/javascript"></script>\n<script src="/javascripts/application.js?1" type="text/javascript"></script>), javascript_include_tag(:defaults))

0 comments on commit 7359597

Please sign in to comment.
Something went wrong with that request. Please try again.