Skip to content

Commit

Permalink
Fix the lame config.action_controller.present? check scattered throug…
Browse files Browse the repository at this point in the history
…hout assets_path.
  • Loading branch information
josevalim committed Oct 5, 2011
1 parent db8db4a commit d9d1bb2
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 32 deletions.
16 changes: 3 additions & 13 deletions actionpack/lib/action_view/asset_paths.rb
Expand Up @@ -86,9 +86,7 @@ def compute_protocol(protocol)
end

def default_protocol
protocol = @config.action_controller.default_asset_host_protocol if @config.action_controller.present?
protocol ||= @config.default_asset_host_protocol
protocol || (has_request? ? :request : :relative)
@config.default_asset_host_protocol || (has_request? ? :request : :relative)
end

def invalid_asset_host!(help_message)
Expand Down Expand Up @@ -117,19 +115,11 @@ def compute_asset_host(source)
end

def relative_url_root
if config.action_controller.present?
config.action_controller.relative_url_root
else
config.relative_url_root
end
config.relative_url_root
end

def asset_host_config
if config.action_controller.present?
config.action_controller.asset_host
else
config.asset_host
end
config.asset_host
end

# Returns the current request if one exists.
Expand Down
7 changes: 4 additions & 3 deletions actionpack/lib/sprockets/assets.rake
Expand Up @@ -39,12 +39,13 @@ namespace :assets do
config = Rails.application.config
config.assets.compile = true
config.assets.digest = digest unless digest.nil?

config.assets.digests = {}

env = Rails.application.assets
env = Rails.application.assets
env.context_class.send :include, ::Sprockets::Helpers::PrecompileHelper

target = File.join(Rails.public_path, config.assets.prefix)
compiler = Sprockets::StaticCompiler.new(env,
compiler = Sprockets::StaticCompiler.new(env,
target,
config.assets.precompile,
:manifest_path => config.assets.manifest,
Expand Down
3 changes: 2 additions & 1 deletion actionpack/lib/sprockets/helpers.rb
@@ -1,5 +1,6 @@
module Sprockets
module Helpers
autoload :RailsHelper, "sprockets/helpers/rails_helper"
autoload :RailsHelper, "sprockets/helpers/rails_helper"
autoload :PrecompileHelper, "sprockets/helpers/precompile_helper"
end
end
13 changes: 13 additions & 0 deletions actionpack/lib/sprockets/helpers/precompile_helper.rb
@@ -0,0 +1,13 @@
module Sprockets
module Helpers
module PrecompileHelper
def controller
nil
end

def config
Rails.application.config.action_controller
end
end
end
end
3 changes: 0 additions & 3 deletions actionpack/lib/sprockets/helpers/rails_helper.rb
Expand Up @@ -8,9 +8,6 @@ module RailsHelper

def asset_paths
@asset_paths ||= begin
config = self.config if respond_to?(:config)
config ||= Rails.application.config
controller = self.controller if respond_to?(:controller)
paths = RailsHelper::AssetPaths.new(config, controller)
paths.asset_environment = asset_environment
paths.asset_digests = asset_digests
Expand Down
5 changes: 1 addition & 4 deletions actionpack/lib/sprockets/railtie.rb
Expand Up @@ -42,10 +42,7 @@ class Railtie < ::Rails::Railtie

ActiveSupport.on_load(:action_view) do
include ::Sprockets::Helpers::RailsHelper

app.assets.context_class.instance_eval do
include ::Sprockets::Helpers::RailsHelper
end
app.assets.context_class.send :include, ::Sprockets::Helpers::RailsHelper
end
end

Expand Down
19 changes: 11 additions & 8 deletions actionpack/test/template/sprockets_helper_test.rb
Expand Up @@ -31,7 +31,6 @@ def host_with_port() 'localhost' end
application = Struct.new(:config, :assets).new(config, @assets)
Rails.stubs(:application).returns(application)
@config = config
@config.action_controller ||= ActiveSupport::InheritableOptions.new
@config.perform_caching = true
@config.assets.digest = true
@config.assets.compile = true
Expand All @@ -41,6 +40,10 @@ def url_for(*args)
"http://www.example.com"
end

def config
@controller ? @controller.config : @config
end

test "asset_path" do
assert_match %r{/assets/logo-[0-9a-f]+.png},
asset_path("logo.png")
Expand Down Expand Up @@ -118,8 +121,8 @@ def url_for(*args)
end

test "stylesheets served without a controller in scope cannot access the request" do
remove_instance_variable("@controller")
@config.action_controller.asset_host = Proc.new do |asset, request|
@controller = nil
@config.asset_host = Proc.new do |asset, request|
fail "This should not have been called."
end
assert_raises ActionController::RoutingError do
Expand Down Expand Up @@ -156,10 +159,10 @@ def url_for(*args)
end

test "stylesheets served without a controller in do not use asset hosts when the default protocol is :request" do
remove_instance_variable("@controller")
@config.action_controller.asset_host = "assets-%d.example.com"
@config.action_controller.default_asset_host_protocol = :request
@config.action_controller.perform_caching = true
@controller = nil
@config.asset_host = "assets-%d.example.com"
@config.default_asset_host_protocol = :request
@config.perform_caching = true

assert_match %r{/assets/logo-[0-9a-f]+.png},
asset_path("logo.png")
Expand All @@ -173,7 +176,7 @@ def url_for(*args)

test "asset path with relative url root when controller isn't present but relative_url_root is" do
@controller = nil
@config.action_controller.relative_url_root = "/collaboration/hieraki"
@config.relative_url_root = "/collaboration/hieraki"
assert_equal "/collaboration/hieraki/images/logo.gif",
asset_path("/images/logo.gif")
end
Expand Down

5 comments on commit d9d1bb2

@spohlenz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change require an update to sass-rails? I'm seeing the following error:

undefined local variable or method `config' for #<#<Class:0x007fae39557870>:0x007fae3a4a82e8>

Backtrace (can post more if necessary):
/Users/sam/.rvm/gems/ruby-1.9.2-p290/bundler/gems/rails-d9d1bb2fb944/actionpack/lib/sprockets/helpers/rails_helper.rb:11:in `asset_paths'
/Users/sam/.rvm/gems/ruby-1.9.2-p290/bundler/gems/rails-d9d1bb2fb944/actionpack/lib/sprockets/helpers/rails_helper.rb:56:in `asset_path'
/Users/sam/.rvm/gems/ruby-1.9.2-p290/bundler/gems/rails-d9d1bb2fb944/actionpack/lib/sprockets/helpers/rails_helper.rb:61:in `image_path'
sass-rails (3.1.4) lib/sass/rails/template_handlers.rb:31:in `image_path'
sass-rails (3.1.4) lib/sass/rails/helpers.rb:25:in `image_url'
...

Seems the respond_to?(:config) check might be necessary in #asset_paths for the Sprockets context.

@josevalim
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this error happening on precompile or on a request?

@spohlenz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a request (with an empty cache). Precompile appears to be working fine.

@josevalim
Copy link
Contributor Author

@josevalim josevalim commented on d9d1bb2 Oct 5, 2011 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spohlenz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that fixes it. Thanks.

Please sign in to comment.