Skip to content

Commit

Permalink
push kwargs up to the user facing API
Browse files Browse the repository at this point in the history
this lets us leverage Ruby's kwarg handling (exceptions for missing
params, etc) ASAP which allows us to skip active support method calls
and make sure the exception stack is closer to where the user called the
methods.
  • Loading branch information
tenderlove committed Feb 12, 2016
1 parent 6e8d70e commit ac9d32b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 13 deletions.
18 changes: 9 additions & 9 deletions actionview/lib/action_view/digestor.rb
Expand Up @@ -22,23 +22,23 @@ class << self
# * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt>
# * <tt>dependencies</tt> - An array of dependent views
# * <tt>partial</tt> - Specifies whether the template is a partial
def digest(options)
options.assert_valid_keys(:name, :finder, :dependencies, :partial)
def digest(name:, finder:, **options)
options.assert_valid_keys(:dependencies, :partial)

cache_key = ([ options[:name], options[:finder].details_key.hash ].compact + Array.wrap(options[:dependencies])).join('.')
cache_key = ([ name, finder.details_key.hash ].compact + Array.wrap(options[:dependencies])).join('.')

# this is a correctly done double-checked locking idiom
# (Concurrent::Map's lookups have volatile semantics)
@@cache[cache_key] || @@digest_monitor.synchronize do
@@cache.fetch(cache_key) do # re-check under lock
compute_and_store_digest(cache_key, options)
compute_and_store_digest(cache_key, name, finder, options)
end
end
end

private
def compute_and_store_digest(cache_key, options) # called under @@digest_monitor lock
klass = if options[:partial] || options[:name].include?("/_")
def compute_and_store_digest(cache_key, name, finder, options) # called under @@digest_monitor lock
klass = if options[:partial] || name.include?("/_")
# Prevent re-entry or else recursive templates will blow the stack.
# There is no need to worry about other threads seeing the +false+ value,
# as they will then have to wait for this thread to let go of the @@digest_monitor lock.
Expand All @@ -48,7 +48,7 @@ def compute_and_store_digest(cache_key, options) # called under @@digest_monitor
Digestor
end

@@cache[cache_key] = stored_digest = klass.new(options).digest
@@cache[cache_key] = stored_digest = klass.new(name, finder, options).digest
ensure
# something went wrong or ActionView::Resolver.caching? is false, make sure not to corrupt the @@cache
@@cache.delete_pair(cache_key, false) if pre_stored && !stored_digest
Expand All @@ -57,7 +57,7 @@ def compute_and_store_digest(cache_key, options) # called under @@digest_monitor

attr_reader :name, :finder, :options

def initialize(name:, finder:, **options)
def initialize(name, finder, options = {})
@name, @finder = name, finder
@options = options
end
Expand All @@ -80,7 +80,7 @@ def dependencies

def nested_dependencies
dependencies.collect do |dependency|
dependencies = PartialDigestor.new(name: dependency, finder: finder).nested_dependencies
dependencies = PartialDigestor.new(dependency, finder).nested_dependencies
dependencies.any? ? { dependency => dependencies } : dependency
end
end
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/tasks/dependencies.rake
Expand Up @@ -2,13 +2,13 @@ namespace :cache_digests do
desc 'Lookup nested dependencies for TEMPLATE (like messages/show or comments/_comment.html)'
task :nested_dependencies => :environment do
abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present?
puts JSON.pretty_generate ActionView::Digestor.new(name: CacheDigests.template_name, finder: CacheDigests.finder).nested_dependencies
puts JSON.pretty_generate ActionView::Digestor.new(CacheDigests.template_name, CacheDigests.finder).nested_dependencies
end

desc 'Lookup first-level dependencies for TEMPLATE (like messages/show or comments/_comment.html)'
task :dependencies => :environment do
abort 'You must provide TEMPLATE for the task to run' unless ENV['TEMPLATE'].present?
puts JSON.pretty_generate ActionView::Digestor.new(name: CacheDigests.template_name, finder: CacheDigests.finder).dependencies
puts JSON.pretty_generate ActionView::Digestor.new(CacheDigests.template_name, CacheDigests.finder).dependencies
end

class CacheDigests
Expand Down
4 changes: 2 additions & 2 deletions actionview/test/template/digestor_test.rb
Expand Up @@ -308,11 +308,11 @@ def digest(template_name, options = {})
end

def dependencies(template_name)
ActionView::Digestor.new({ name: template_name, finder: finder }).dependencies
ActionView::Digestor.new(template_name, finder).dependencies
end

def nested_dependencies(template_name)
ActionView::Digestor.new({ name: template_name, finder: finder }).nested_dependencies
ActionView::Digestor.new(template_name, finder).nested_dependencies
end

def finder
Expand Down

1 comment on commit ac9d32b

@dhh
Copy link
Member

@dhh dhh commented on ac9d32b Feb 16, 2016

Choose a reason for hiding this comment

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

❤️ – would be nice to do this throughout Rails.

Please sign in to comment.