diff options
Diffstat (limited to 'actionview')
21 files changed, 262 insertions, 222 deletions
diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 256b90784a..bd7ce14e04 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,7 @@ +* Add support for nested hashes/arrays to `:params` option of `button_to` helper. + + *James Coleman* + ## Rails 5.0.0.beta2 (February 01, 2016) ## * Fix stripping the digest from the automatically generated img tag alt @@ -68,7 +72,7 @@ *Vasiliy Ermolovich* -* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising a error +* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising an error when the only input on the form is the `collection_radio_buttons`. *Mauro George* @@ -193,16 +197,18 @@ *Ulisses Almeida* -* Collection rendering automatically caches and fetches multiple partials. +* Collection rendering can cache and fetch multiple partials at once. Collections rendered as: ```ruby - <%= render @notifications %> - <%= render partial: 'notifications/notification', collection: @notifications, as: :notification %> + <%= render partial: 'notifications/notification', collection: @notifications, as: :notification, cached: true %> ``` - will now read several partials from cache at once, if the template starts with a cache call: + will read several partials from cache at once. The templates in the collection + that haven't been cached already will automatically be written to cache. Works + great alongside individual template fragment caching. For instance if the + template the collection renders is cached like: ```ruby # notifications/_notification.html.erb @@ -211,6 +217,9 @@ <% end %> ``` + Then any collection renders shares that cache when attempting to read multiple + ones at once. + *Kasper Timm Hansen* * Fixed a dependency tracker bug that caused template dependencies not diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 359b2f810c..f3c5d6c8df 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -4,13 +4,11 @@ require 'monitor' module ActionView class Digestor - cattr_reader(:cache) - @@cache = Concurrent::Map.new @@digest_monitor = Monitor.new class PerRequestDigestCacheExpiry < Struct.new(:app) # :nodoc: def call(env) - ActionView::Digestor.cache.clear + ActionView::LookupContext::DetailsKey.clear app.call(env) end end @@ -22,42 +20,42 @@ module ActionView # * <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 ].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) + finder.digest_cache[cache_key] || @@digest_monitor.synchronize do + finder.digest_cache.fetch(cache_key) do # re-check under lock + 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. - pre_stored = @@cache.put_if_absent(cache_key, false).nil? # put_if_absent returns nil on insertion + pre_stored = finder.digest_cache.put_if_absent(cache_key, false).nil? # put_if_absent returns nil on insertion PartialDigestor else Digestor end - @@cache[cache_key] = stored_digest = klass.new(options).digest + finder.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 + finder.digest_cache.delete_pair(cache_key, false) if pre_stored && !stored_digest end end attr_reader :name, :finder, :options - def initialize(name:, finder:, **options) + def initialize(name, finder, options = {}) @name, @finder = name, finder @options = options end @@ -80,7 +78,7 @@ module ActionView 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 diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 401f398721..4c7c4b91c6 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -126,44 +126,29 @@ module ActionView # # Now all you have to do is change that timestamp when the helper method changes. # - # === Automatic Collection Caching + # === Collection Caching # - # When rendering collections such as: + # When rendering a collection of objects that each use the same partial, a `cached` + # option can be passed. + # For collections rendered such: # - # <%= render @notifications %> - # <%= render partial: 'notifications/notification', collection: @notifications %> + # <%= render partial: 'notifications/notification', collection: @notifications, cached: true %> # - # If the notifications/_notification partial starts with a cache call as: + # The `cached: true` will make Action View's rendering read several templates + # from cache at once instead of one call per template. # - # <% cache notification do %> - # <%= notification.name %> - # <% end %> - # - # The collection can then automatically use any cached renders for that - # template by reading them at once instead of one by one. - # - # See ActionView::Template::Handlers::ERB.resource_cache_call_pattern for - # more information on what cache calls make a template eligible for this - # collection caching. - # - # The automatic cache multi read can be turned off like so: + # Templates in the collection not already cached are written to cache. # - # <%= render @notifications, cache: false %> + # Works great alongside individual template fragment caching. + # For instance if the template the collection renders is cached like: # - # === Explicit Collection Caching - # - # If the partial template doesn't start with a clean cache call as - # mentioned above, you can still benefit from collection caching by - # adding a special comment format anywhere in the template, like: - # - # <%# Template Collection: notification %> - # <% my_helper_that_calls_cache(some_arg, notification) do %> - # <%= notification.name %> + # # notifications/_notification.html.erb + # <% cache notification do %> + # <%# ... %> # <% end %> # - # The pattern used to match these is <tt>/# Template Collection: (\S+)/</tt>, - # so it's important that you type it out just so. - # You can only declare one collection in a partial template file. + # Any collection renders will find those cached templates when attempting + # to read multiple templates at once. def cache(name = {}, options = {}, &block) if controller.respond_to?(:perform_caching) && controller.perform_caching name_options = options.slice(:skip_digest, :virtual_path) diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb index 2562504896..42e7358a1d 100644 --- a/actionview/lib/action_view/helpers/tag_helper.rb +++ b/actionview/lib/action_view/helpers/tag_helper.rb @@ -154,6 +154,7 @@ module ActionView options.each_pair do |key, value| if TAG_PREFIXES.include?(key) && value.is_a?(Hash) value.each_pair do |k, v| + next if v.nil? output << sep output << prefix_tag_option(key, k, v, escape) end diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 3a4561a083..87218821ed 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -329,8 +329,8 @@ module ActionView inner_tags = method_tag.safe_concat(button).safe_concat(request_token_tag) if params - params.each do |param_name, value| - inner_tags.safe_concat tag(:input, type: "hidden", name: param_name, value: value.to_param) + to_form_params(params).each do |param| + inner_tags.safe_concat tag(:input, type: "hidden", name: param[:name], value: param[:value]) end end content_tag('form', inner_tags, form_options) @@ -595,6 +595,42 @@ module ActionView def method_tag(method) tag('input', type: 'hidden', name: '_method', value: method.to_s) end + + # Returns an array of hashes each containing :name and :value keys + # suitable for use as the names and values of form input fields: + # + # to_form_params(name: 'David', nationality: 'Danish') + # # => [{name: :name, value: 'David'}, {name: 'nationality', value: 'Danish'}] + # + # to_form_params(country: {name: 'Denmark'}) + # # => [{name: 'country[name]', value: 'Denmark'}] + # + # to_form_params(countries: ['Denmark', 'Sweden']}) + # # => [{name: 'countries[]', value: 'Denmark'}, {name: 'countries[]', value: 'Sweden'}] + # + # An optional namespace can be passed to enclose key names: + # + # to_form_params({ name: 'Denmark' }, 'country') + # # => [{name: 'country[name]', value: 'Denmark'}] + def to_form_params(attribute, namespace = nil) # :nodoc: + params = [] + case attribute + when Hash + attribute.each do |key, value| + prefix = namespace ? "#{namespace}[#{key}]" : key + params.push(*to_form_params(value, prefix)) + end + when Array + array_prefix = "#{namespace}[]" + attribute.each do |value| + params.push(*to_form_params(value, array_prefix)) + end + else + params << { name: namespace, value: attribute.to_param } + end + + params.sort_by { |pair| pair[:name] } + end end end end diff --git a/actionview/lib/action_view/log_subscriber.rb b/actionview/lib/action_view/log_subscriber.rb index 9047dbdd85..aa38db2a3a 100644 --- a/actionview/lib/action_view/log_subscriber.rb +++ b/actionview/lib/action_view/log_subscriber.rb @@ -20,7 +20,15 @@ module ActionView end end alias :render_partial :render_template - alias :render_collection :render_template + + def render_collection(event) + identifier = event.payload[:identifier] || 'templates' + + info do + " Rendered collection of #{from_rails_root(identifier)}" \ + " #{render_count(event.payload)} (#{event.duration.round(1)}ms)" + end + end def logger ActionView::Base.logger @@ -38,6 +46,14 @@ module ActionView def rails_root @root ||= "#{Rails.root}/" end + + def render_count(payload) + if payload[:cache_hits] + "[#{payload[:cache_hits]} / #{payload[:count]} cache hits]" + else + "[#{payload[:count]} times]" + end + end end end diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 126f289f55..86afedaa2d 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -55,9 +55,7 @@ module ActionView class DetailsKey #:nodoc: alias :eql? :equal? - alias :object_hash :hash - attr_reader :hash @details_keys = Concurrent::Map.new def self.get(details) @@ -72,8 +70,16 @@ module ActionView @details_keys.clear end + def self.empty?; @details_keys.empty?; end + + def self.digest_caches + @details_keys.values.map(&:digest_cache) + end + + attr_reader :digest_cache + def initialize - @hash = object_hash + @digest_cache = Concurrent::Map.new end end @@ -200,6 +206,10 @@ module ActionView self.view_paths = view_paths end + def digest_cache + details_key.digest_cache + end + def initialize_details(target, details) registered_details.each do |k| target[k] = details[k] || Accessors::DEFAULT_PROCS[k].call diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index aa77a77acf..23e672a95f 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -35,8 +35,12 @@ module ActionView end end - def instrument(name, options={}) - ActiveSupport::Notifications.instrument("render_#{name}.action_view", options){ yield } + def instrument(name, **options) + options[:identifier] ||= (@template && @template.identifier) || @path + + ActiveSupport::Notifications.instrument("render_#{name}.action_view", options) do |payload| + yield payload + end end def prepend_formats(formats) diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb index bdbf03191a..514804b08e 100644 --- a/actionview/lib/action_view/renderer/partial_renderer.rb +++ b/actionview/lib/action_view/renderer/partial_renderer.rb @@ -294,7 +294,7 @@ module ActionView def render(context, options, block) setup(context, options, block) - identifier = (@template = find_partial) ? @template.identifier : @path + @template = find_partial @lookup_context.rendered_format ||= begin if @template && @template.formats.present? @@ -305,11 +305,9 @@ module ActionView end if @collection - instrument(:collection, :identifier => identifier || "collection", :count => @collection.size) do - render_collection - end + render_collection else - instrument(:partial, :identifier => identifier) do + instrument(:partial) do render_partial end end @@ -318,15 +316,17 @@ module ActionView private def render_collection - return nil if @collection.blank? + instrument(:collection, count: @collection.size) do |payload| + return nil if @collection.blank? - if @options.key?(:spacer_template) - spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals) - end + if @options.key?(:spacer_template) + spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals) + end - cache_collection_render do - @template ? collection_with_template : collection_without_template - end.join(spacer).html_safe + cache_collection_render(payload) do + @template ? collection_with_template : collection_without_template + end.join(spacer).html_safe + end end def render_partial diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb index 5a294744e8..f7deba94ce 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -1,5 +1,3 @@ -require 'active_support/core_ext/object/try' - module ActionView module CollectionCaching # :nodoc: extend ActiveSupport::Concern @@ -11,38 +9,25 @@ module ActionView end private - def cache_collection_render - return yield unless cache_collection? + def cache_collection_render(instrumentation_payload) + return yield unless @options[:cached] keyed_collection = collection_by_cache_keys - partial_cache = collection_cache.read_multi(*keyed_collection.keys) + cached_partials = collection_cache.read_multi(*keyed_collection.keys) + instrumentation_payload[:cache_hits] = cached_partials.size - @collection = keyed_collection.reject { |key, _| partial_cache.key?(key) }.values - rendered_partials = @collection.any? ? yield.dup : [] + @collection = keyed_collection.reject { |key, _| cached_partials.key?(key) }.values + rendered_partials = @collection.empty? ? [] : yield - fetch_or_cache_partial(partial_cache, order_by: keyed_collection.each_key) do - rendered_partials.shift + index = 0 + fetch_or_cache_partial(cached_partials, order_by: keyed_collection.each_key) do + rendered_partials[index].tap { index += 1 } end end - def cache_collection? - @options.fetch(:cache, automatic_cache_eligible?) - end - - def automatic_cache_eligible? - @template && !callable_cache_key? && - @template.eligible_for_collection_caching?(as: @options[:as]) - end - - def callable_cache_key? - @options[:cache].respond_to?(:call) - end - def collection_by_cache_keys - seed = callable_cache_key? ? @options[:cache] : ->(i) { i } - @collection.each_with_object({}) do |item, hash| - hash[expanded_cache_key(seed.call(item))] = item + hash[expanded_cache_key(item)] = item end end @@ -52,10 +37,10 @@ module ActionView end def fetch_or_cache_partial(cached_partials, order_by:) - order_by.map do |key| - cached_partials.fetch(key) do + order_by.map do |cache_key| + cached_partials.fetch(cache_key) do yield.tap do |rendered_partial| - collection_cache.write(key, rendered_partial, @options[:cache_options]) + collection_cache.write(cache_key, rendered_partial) end end end diff --git a/actionview/lib/action_view/tasks/dependencies.rake b/actionview/lib/action_view/tasks/dependencies.rake index f394c319c1..9932ff8b6d 100644 --- a/actionview/lib/action_view/tasks/dependencies.rake +++ b/actionview/lib/action_view/tasks/dependencies.rake @@ -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 diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 15fc2b71a3..169ee55fdc 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -130,7 +130,6 @@ module ActionView @source = source @identifier = identifier @handler = handler - @cache_name = extract_resource_cache_name @compiled = false @original_encoding = nil @locals = details[:locals] || [] @@ -166,10 +165,6 @@ module ActionView @type ||= Types[@formats.first] if @formats.first end - def eligible_for_collection_caching?(as: nil) - @cache_name == (as || inferred_cache_name).to_s - end - # Receives a view object and return a template similar to self by using @virtual_path. # # This method is useful if you have a template object but it does not contain its source @@ -355,23 +350,5 @@ module ActionView ActiveSupport::Notifications.instrument("#{action}.action_view".freeze, payload, &block) end end - - EXPLICIT_COLLECTION = /# Template Collection: (?<resource_name>\w+)/ - - def extract_resource_cache_name - if match = @source.match(EXPLICIT_COLLECTION) || resource_cache_call_match - match[:resource_name] - end - end - - def resource_cache_call_match - if @handler.respond_to?(:resource_cache_call_pattern) - @source.match(@handler.resource_cache_call_pattern) - end - end - - def inferred_cache_name - @inferred_cache_name ||= @virtual_path.split('/'.freeze).last.sub('_'.freeze, ''.freeze) - end end end diff --git a/actionview/lib/action_view/template/error.rb b/actionview/lib/action_view/template/error.rb index ccee785d3e..3f38c3d2b9 100644 --- a/actionview/lib/action_view/template/error.rb +++ b/actionview/lib/action_view/template/error.rb @@ -135,13 +135,13 @@ module ActionView end def formatted_code_for(source_code, line_counter, indent, output) - start_value = (output == :html) ? {} : "" + start_value = (output == :html) ? {} : [] source_code.inject(start_value) do |result, line| line_counter += 1 if output == :html result.update(line_counter.to_s => "%#{indent}s %s\n" % ["", line]) else - result << "%#{indent}s: %s\n" % [line_counter, line] + result << "%#{indent}s: %s" % [line_counter, line] end end end diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb index 1f8459c24b..85a100ed4c 100644 --- a/actionview/lib/action_view/template/handlers/erb.rb +++ b/actionview/lib/action_view/template/handlers/erb.rb @@ -123,31 +123,6 @@ module ActionView ).src end - # Returns Regexp to extract a cached resource's name from a cache call at the - # first line of a template. - # The extracted cache name is captured as :resource_name. - # - # <% cache notification do %> # => notification - # - # The pattern should support templates with a beginning comment: - # - # <%# Still extractable even though there's a comment %> - # <% cache notification do %> # => notification - # - # But fail to extract a name if a resource association is cached. - # - # <% cache notification.event do %> # => nil - def resource_cache_call_pattern - /\A - (?:<%\#.*%>)* # optional initial comment - \s* # followed by optional spaces or newlines - <%\s*cache[\(\s] # followed by an ERB call to cache - \s* # followed by optional spaces or newlines - (?<resource_name>\w+) # capture the cache call argument as :resource_name - [\s\)] # followed by a space or close paren - /xm - end - private def valid_encoding(string, encoding) diff --git a/actionview/test/fixtures/digestor/messages/peek.html.erb b/actionview/test/fixtures/digestor/messages/peek.html.erb new file mode 100644 index 0000000000..84885ab0bc --- /dev/null +++ b/actionview/test/fixtures/digestor/messages/peek.html.erb @@ -0,0 +1,2 @@ +<%# Template Dependency: messages/message %> +<%= render "comments/comments" %> diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index ea86535b2a..9ff5f7126b 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -33,7 +33,6 @@ class TemplateDigestorTest < ActionView::TestCase def teardown Dir.chdir @cwd FileUtils.rm_r @tmp_dir - ActionView::Digestor.cache.clear end def test_top_level_change_reflected @@ -130,6 +129,16 @@ class TemplateDigestorTest < ActionView::TestCase end end + def test_getting_of_singly_nested_dependencies + singly_nested_dependencies = ["messages/header", "messages/form", "messages/message", "events/event", "comments/comment"] + assert_equal singly_nested_dependencies, nested_dependencies('messages/edit') + end + + def test_getting_of_doubly_nested_dependencies + doubly_nested = [{"comments/comments"=>["comments/comment"]}, "messages/message"] + assert_equal doubly_nested, nested_dependencies('messages/peek') + end + def test_nested_template_directory assert_digest_difference("messages/show") do change_template("messages/actions/_move") @@ -291,12 +300,12 @@ class TemplateDigestorTest < ActionView::TestCase def assert_digest_difference(template_name, options = {}) previous_digest = digest(template_name, options) - ActionView::Digestor.cache.clear + finder.digest_cache.clear yield assert_not_equal previous_digest, digest(template_name, options), "digest didn't change" - ActionView::Digestor.cache.clear + finder.digest_cache.clear end def digest(template_name, options = {}) @@ -308,11 +317,11 @@ class TemplateDigestorTest < ActionView::TestCase 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 diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 4776c18b0b..93a0701dcc 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -86,7 +86,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered test\/_customer.erb/, @logger.logged(:info).last) + assert_match(/Rendered collection of test\/_customer.erb \[2 times\]/, @logger.logged(:info).last) end end @@ -96,7 +96,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered customers\/_customer\.html\.erb/, @logger.logged(:info).last) + assert_match(/Rendered collection of customers\/_customer\.html\.erb \[2 times\]/, @logger.logged(:info).last) end end @@ -106,7 +106,21 @@ class AVLogSubscriberTest < ActiveSupport::TestCase wait assert_equal 1, @logger.logged(:info).size - assert_match(/Rendered collection/, @logger.logged(:info).last) + assert_match(/Rendered collection of templates/, @logger.logged(:info).last) + end + end + + def test_render_collection_with_cached_set + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + def @view.view_cache_dependencies; []; end + def @view.fragment_cache_key(*); 'ahoy `controller` dependency'; end + + @view.render(partial: 'customers/customer', collection: [ Customer.new('david'), Customer.new('mary') ], cached: true, + locals: { greeting: 'hi' }) + wait + + assert_equal 1, @logger.logged(:info).size + assert_match(/Rendered collection of customers\/_customer\.html\.erb \[0 \/ 2 cache hits\]/, @logger.logged(:info).last) end end end diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 333e0cca11..3561114d90 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -226,13 +226,13 @@ module RenderTestCases assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number - assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code.strip + assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end def test_render_error_indentation e = assert_raises(ActionView::Template::Error) { @view.render(:partial => "test/raise_indentation") } - error_lines = e.annoted_source_code.split("\n") + error_lines = e.annoted_source_code assert_match %r!error\shere!, e.message assert_equal "11", e.line_number assert_equal " 9: <p>Ninth paragraph</p>", error_lines.second @@ -252,7 +252,7 @@ module RenderTestCases assert_match %r!method.*doesnt_exist!, e.message assert_equal "", e.sub_template_message assert_equal "1", e.line_number - assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code.strip + assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end @@ -628,56 +628,59 @@ class LazyViewRenderTest < ActiveSupport::TestCase end end -class CachedCollectionViewRenderTest < CachedViewRenderTest +class CachedCollectionViewRenderTest < ActiveSupport::TestCase class CachedCustomer < Customer; end - teardown do - ActionView::PartialRenderer.collection_cache.clear - end + include RenderTestCases - test "with custom key" do - customer = Customer.new("david") - key = cache_key([customer, 'key'], "test/_customer") + # Ensure view path cache is primed + setup do + view_paths = ActionController::Base.view_paths + assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class - ActionView::PartialRenderer.collection_cache.write(key, 'Hello') + ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new - assert_equal "Hello", - @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'key'] }) + setup_view(view_paths) end - test "with caching with custom key and rendering with different key" do - customer = Customer.new("david") - key = cache_key([customer, 'key'], "test/_customer") + teardown do + GC.start + I18n.reload! + end - ActionView::PartialRenderer.collection_cache.write(key, 'Hello') + test "collection caching does not cache by default" do + customer = Customer.new("david", 1) + key = cache_key(customer, "test/_customer") - assert_equal "Hello: david", - @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'another_key'] }) + ActionView::PartialRenderer.collection_cache.write(key, 'Cached') + + assert_not_equal "Cached", + @view.render(partial: "test/customer", collection: [customer]) end - test "automatic caching with inferred cache name" do - customer = CachedCustomer.new("david") - key = cache_key(customer, "test/_cached_customer") + test "collection caching with partial that doesn't use fragment caching" do + customer = Customer.new("david", 1) + key = cache_key(customer, "test/_customer") ActionView::PartialRenderer.collection_cache.write(key, 'Cached') assert_equal "Cached", - @view.render(partial: "test/cached_customer", collection: [customer]) + @view.render(partial: "test/customer", collection: [customer], cached: true) end - test "automatic caching with as name" do - customer = CachedCustomer.new("david") - key = cache_key(customer, "test/_cached_customer_as") + test "collection caching with cached true" do + customer = CachedCustomer.new("david", 1) + key = cache_key(customer, "test/_cached_customer") ActionView::PartialRenderer.collection_cache.write(key, 'Cached') assert_equal "Cached", - @view.render(partial: "test/cached_customer_as", collection: [customer], as: :buyer) + @view.render(partial: "test/cached_customer", collection: [customer], cached: true) end private - def cache_key(names, virtual_path) + def cache_key(*names, virtual_path) digest = ActionView::Digestor.digest name: virtual_path, finder: @view.lookup_context, dependencies: [] - @view.fragment_cache_key([ *Array(names), digest ]) + @view.fragment_cache_key([ *names, digest ]) end end diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb index 6f7a78ccef..f3956a31f6 100644 --- a/actionview/test/template/tag_helper_test.rb +++ b/actionview/test/template/tag_helper_test.rb @@ -173,4 +173,10 @@ class TagHelperTest < ActionView::TestCase tag('a', { aria => { a_float: 3.14, a_big_decimal: BigDecimal.new("-123.456"), a_number: 1, string: 'hello', symbol: :foo, array: [1, 2, 3], hash: { key: 'value'}, string_with_quotes: 'double"quote"party"' } }) } end + + def test_link_to_data_nil_equal + div_type1 = content_tag(:div, 'test', { 'data-tooltip' => nil }) + div_type2 = content_tag(:div, 'test', { data: {tooltip: nil} }) + assert_dom_equal div_type1, div_type2 + end end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 921011b073..533c1c3219 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -192,38 +192,6 @@ class TestERBTemplate < ActiveSupport::TestCase assert_match(/\xFC/, e.message) end - def test_not_eligible_for_collection_caching_without_cache_call - [ - "<%= 'Hello' %>", - "<% cache_customer = 42 %>", - "<% cache customer.name do %><% end %>", - "<% my_cache customer do %><% end %>" - ].each do |body| - template = new_template(body, virtual_path: "test/foo/_customer") - assert_not template.eligible_for_collection_caching?, "Template #{body.inspect} should not be eligible for collection caching" - end - end - - def test_eligible_for_collection_caching_with_cache_call_or_explicit - [ - "<% cache customer do %><% end %>", - "<% cache(customer) do %><% end %>", - "<% cache( customer) do %><% end %>", - "<% cache( customer ) do %><% end %>", - "<%cache customer do %><% end %>", - "<% cache customer do %><% end %>", - " <% cache customer do %>\n<% end %>\n", - "<%# comment %><% cache customer do %><% end %>", - "<%# comment %>\n<% cache customer do %><% end %>", - "<%# comment\n line 2\n line 3 %>\n<% cache customer do %><% end %>", - "<%# comment 1 %>\n<%# comment 2 %>\n<% cache customer do %><% end %>", - "<%# comment 1 %>\n<%# Template Collection: customer %>\n<% my_cache customer do %><% end %>" - ].each do |body| - template = new_template(body, virtual_path: "test/foo/_customer") - assert template.eligible_for_collection_caching?, "Template #{body.inspect} should be eligible for collection caching" - end - end - def with_external_encoding(encoding) old = Encoding.default_external Encoding::Converter.new old, encoding if old != encoding diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb index 3010656166..d6a19a829f 100644 --- a/actionview/test/template/url_helper_test.rb +++ b/actionview/test/template/url_helper_test.rb @@ -71,6 +71,34 @@ class UrlHelperTest < ActiveSupport::TestCase assert_equal 'javascript:history.back()', url_for(:back) end + def test_to_form_params_with_hash + assert_equal( + [{ name: :name, value: 'David' }, { name: :nationality, value: 'Danish' }], + to_form_params(name: 'David', nationality: 'Danish') + ) + end + + def test_to_form_params_with_nested_hash + assert_equal( + [{ name: 'country[name]', value: 'Denmark' }], + to_form_params(country: { name: 'Denmark' }) + ) + end + + def test_to_form_params_with_array_nested_in_hash + assert_equal( + [{ name: 'countries[]', value: 'Denmark' }, { name: 'countries[]', value: 'Sweden' }], + to_form_params(countries: ['Denmark', 'Sweden']) + ) + end + + def test_to_form_params_with_namespace + assert_equal( + [{ name: 'country[name]', value: 'Denmark' }], + to_form_params({name: 'Denmark'}, 'country') + ) + end + def test_button_to_with_straight_url assert_dom_equal %{<form method="post" action="http://www.example.com" class="button_to"><input type="submit" value="Hello" /></form>}, button_to("Hello", "http://www.example.com") end @@ -189,8 +217,22 @@ class UrlHelperTest < ActiveSupport::TestCase def test_button_to_with_params assert_dom_equal( - %{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="foo" value="bar" /><input type="hidden" name="baz" value="quux" /></form>}, - button_to("Hello", "http://www.example.com", params: {foo: :bar, baz: "quux"}) + %{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="baz" value="quux" /><input type="hidden" name="foo" value="bar" /></form>}, + button_to("Hello", "http://www.example.com", params: { foo: :bar, baz: "quux" }) + ) + end + + def test_button_to_with_nested_hash_params + assert_dom_equal( + %{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="foo[bar]" value="baz" /></form>}, + button_to("Hello", "http://www.example.com", params: { foo: { bar: 'baz' } }) + ) + end + + def test_button_to_with_nested_array_params + assert_dom_equal( + %{<form action="http://www.example.com" class="button_to" method="post"><input type="submit" value="Hello" /><input type="hidden" name="foo[]" value="bar" /></form>}, + button_to("Hello", "http://www.example.com", params: { foo: ['bar'] }) ) end |