diff options
Diffstat (limited to 'actionview')
45 files changed, 623 insertions, 243 deletions
diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 43688fc8a7..be67aff543 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,16 @@ +* Only clear ActionView cache in development on file changes + + To speed up development mode, view caches are only cleared when files in + the view paths have changed. Applications which have implemented custom + `ActionView::Resolver` subclasses may need to add their own cache clearing. + + *John Hawthorn* + +* Fix `ActionView::FixtureResolver` so that it handles template variants correctly. + + *Edward Rudd* + + ## Rails 6.0.0.beta3 (March 11, 2019) ## * Only accept formats from registered mime types @@ -14,18 +27,19 @@ ## Rails 6.0.0.beta2 (February 25, 2019) ## -* ActionView::Template.finalize_compiled_template_methods is deprecated with +* `ActionView::Template.finalize_compiled_template_methods` is deprecated with no replacement. *tenderlove* -* config.action_view.finalize_compiled_template_methods is deprecated with +* `config.action_view.finalize_compiled_template_methods` is deprecated with no replacement. *tenderlove* * Ensure unique DOM IDs for collection inputs with float values. - Fixes #34974 + + Fixes #34974. *Mark Edmondson* diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index 8cb4648a67..7f85bf2a5e 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -44,7 +44,7 @@ module ActionView autoload :Rendering autoload :RoutingUrlFor autoload :Template - autoload :FileTemplate + autoload :UnboundTemplate autoload :ViewPaths autoload_under "renderer" do @@ -81,6 +81,7 @@ module ActionView end end + autoload :CacheExpiry autoload :TestCase def self.eager_load! diff --git a/actionview/lib/action_view/cache_expiry.rb b/actionview/lib/action_view/cache_expiry.rb new file mode 100644 index 0000000000..820afc093d --- /dev/null +++ b/actionview/lib/action_view/cache_expiry.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module ActionView + class CacheExpiry + class Executor + def initialize(watcher:) + @cache_expiry = CacheExpiry.new(watcher: watcher) + end + + def before(target) + @cache_expiry.clear_cache_if_necessary + end + end + + def initialize(watcher:) + @watched_dirs = nil + @watcher_class = watcher + @watcher = nil + end + + def clear_cache_if_necessary + watched_dirs = dirs_to_watch + if watched_dirs != @watched_dirs + @watched_dirs = watched_dirs + @watcher = @watcher_class.new([], watched_dirs) do + clear_cache + end + @watcher.execute + else + @watcher.execute_if_updated + end + end + + def clear_cache + ActionView::LookupContext::DetailsKey.clear + end + + private + + def dirs_to_watch + fs_paths = all_view_paths.grep(FileSystemResolver) + fs_paths.map(&:path).sort.uniq + end + + def all_view_paths + ActionView::ViewPaths.all_view_paths.flat_map(&:paths) + end + end +end diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 9fa8d7eab1..97a6da3634 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -6,12 +6,6 @@ module ActionView class Digestor @@digest_mutex = Mutex.new - module PerExecutionDigestCacheExpiry - def self.before(target) - ActionView::LookupContext::DetailsKey.clear - end - end - class << self # Supported options: # diff --git a/actionview/lib/action_view/file_template.rb b/actionview/lib/action_view/file_template.rb deleted file mode 100644 index dea02176eb..0000000000 --- a/actionview/lib/action_view/file_template.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -require "action_view/template" - -module ActionView - class FileTemplate < Template - def initialize(filename, handler, details) - @filename = filename - - super(nil, filename, handler, details) - end - - def source - File.binread @filename - end - - def refresh(_) - self - end - - # Exceptions are marshalled when using the parallel test runner with DRb, so we need - # to ensure that references to the template object can be marshalled as well. This means forgoing - # the marshalling of the compiler mutex and instantiating that again on unmarshalling. - def marshal_dump # :nodoc: - [ @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ] - end - - def marshal_load(array) # :nodoc: - @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array - @compile_mutex = Mutex.new - end - end -end diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb index c0996049f0..5d4ff36425 100644 --- a/actionview/lib/action_view/helpers/form_tag_helper.rb +++ b/actionview/lib/action_view/helpers/form_tag_helper.rb @@ -24,7 +24,7 @@ module ActionView mattr_accessor :default_enforce_utf8, default: true - # Starts a form tag that points the action to a url configured with <tt>url_for_options</tt> just like + # Starts a form tag that points the action to a URL configured with <tt>url_for_options</tt> just like # ActionController::Base#url_for. The method for the form defaults to POST. # # ==== Options diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index d63ada3890..df83dff681 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -553,7 +553,7 @@ module ActionView url_string = URI.parser.unescape(url_for(options)).force_encoding(Encoding::BINARY) # We ignore any extra parameters in the request_uri if the - # submitted url doesn't have any either. This lets the function + # submitted URL doesn't have any either. This lets the function # work with things like ?order=asc # the behaviour can be disabled with check_parameters: true request_uri = url_string.index("?") || check_parameters ? request.fullpath : request.path diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index fd3d025cbf..b9a8cc129c 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -130,9 +130,8 @@ module ActionView end alias :find_template :find - def find_file(name, prefixes = [], partial = false, keys = [], options = {}) - @view_paths.find_file(*args_for_lookup(name, prefixes, partial, keys, options)) - end + alias :find_file :find + deprecate :find_file def find_all(name, prefixes = [], partial = false, keys = [], options = {}) @view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys, options)) diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb index 691b53e2da..d54749d8e3 100644 --- a/actionview/lib/action_view/path_set.rb +++ b/actionview/lib/action_view/path_set.rb @@ -48,12 +48,11 @@ module ActionView #:nodoc: find_all(*args).first || raise(MissingTemplate.new(self, *args)) end - def find_file(path, prefixes = [], *args) - _find_all(path, prefixes, args, true).first || raise(MissingTemplate.new(self, path, prefixes, *args)) - end + alias :find_file :find + deprecate :find_file def find_all(path, prefixes = [], *args) - _find_all path, prefixes, args, false + _find_all path, prefixes, args end def exists?(path, prefixes, *args) @@ -71,15 +70,11 @@ module ActionView #:nodoc: private - def _find_all(path, prefixes, args, outside_app) + def _find_all(path, prefixes, args) prefixes = [prefixes] if String === prefixes prefixes.each do |prefix| paths.each do |resolver| - if outside_app - templates = resolver.find_all_anywhere(path, prefix, *args) - else - templates = resolver.find_all(path, prefix, *args) - end + templates = resolver.find_all(path, prefix, *args) return templates unless templates.empty? end end diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index a25e1d3d02..8bd526cdf9 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -81,7 +81,7 @@ module ActionView initializer "action_view.per_request_digest_cache" do |app| ActiveSupport.on_load(:action_view) do unless ActionView::Resolver.caching? - app.executor.to_run ActionView::Digestor::PerExecutionDigestCacheExpiry + app.executor.to_run ActionView::CacheExpiry::Executor.new(watcher: app.config.file_watcher) end end end 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 ed59033e27..ca692699a6 100644 --- a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb +++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb @@ -17,13 +17,13 @@ module ActionView # Result is a hash with the key represents the # key used for cache lookup and the value is the item # on which the partial is being rendered - keyed_collection = collection_by_cache_keys(view, template) + keyed_collection, ordered_keys = collection_by_cache_keys(view, template) # Pull all partials from cache # Result is a hash, key matches the entry in # `keyed_collection` where the cache was retrieved and the # value is the value that was present in the cache - cached_partials = collection_cache.read_multi(*keyed_collection.keys) + cached_partials = collection_cache.read_multi(*keyed_collection.keys) instrumentation_payload[:cache_hits] = cached_partials.size # Extract the items for the keys that are not found @@ -40,11 +40,15 @@ module ActionView rendered_partials = @collection.empty? ? [] : yield index = 0 - fetch_or_cache_partial(cached_partials, template, order_by: keyed_collection.each_key) do + keyed_partials = fetch_or_cache_partial(cached_partials, template, order_by: keyed_collection.each_key) do # This block is called once # for every cache miss while preserving order. rendered_partials[index].tap { index += 1 } end + + ordered_keys.map do |key| + keyed_partials[key] + end end def callable_cache_key? @@ -56,8 +60,10 @@ module ActionView digest_path = view.digest_path_from_template(template) - @collection.each_with_object({}) do |item, hash| - hash[expanded_cache_key(seed.call(item), view, template, digest_path)] = item + @collection.each_with_object([{}, []]) do |item, (hash, ordered_keys)| + key = expanded_cache_key(seed.call(item), view, template, digest_path) + ordered_keys << key + hash[key] = item end end @@ -82,15 +88,16 @@ module ActionView # If the partial is not already cached it will also be # written back to the underlying cache store. def fetch_or_cache_partial(cached_partials, template, order_by:) - order_by.map do |cache_key| - if content = cached_partials[cache_key] - build_rendered_template(content, template) - else - yield.tap do |rendered_partial| - collection_cache.write(cache_key, rendered_partial.body) - end + order_by.each_with_object({}) do |cache_key, hash| + hash[cache_key] = + if content = cached_partials[cache_key] + build_rendered_template(content, template) + else + yield.tap do |rendered_partial| + collection_cache.write(cache_key, rendered_partial.body) + end + end end - end end end end diff --git a/actionview/lib/action_view/renderer/streaming_template_renderer.rb b/actionview/lib/action_view/renderer/streaming_template_renderer.rb index 279ef3c680..19fa399e2c 100644 --- a/actionview/lib/action_view/renderer/streaming_template_renderer.rb +++ b/actionview/lib/action_view/renderer/streaming_template_renderer.rb @@ -34,7 +34,7 @@ module ActionView return unless logger message = +"\n#{exception.class} (#{exception.message}):\n" - message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) + message << exception.annotated_source_code.to_s if exception.respond_to?(:annotated_source_code) message << " " << exception.backtrace.join("\n ") logger.fatal("#{message}\n\n") end diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index 83b990b081..4aab3f913f 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -26,7 +26,12 @@ module ActionView elsif options.key?(:html) Template::HTML.new(options[:html], formats.first) elsif options.key?(:file) - @lookup_context.with_fallbacks.find_file(options[:file], nil, false, keys, @details) + if File.exist?(options[:file]) + Template::RawFile.new(options[:file]) + else + ActiveSupport::Deprecation.warn "render file: should be given the absolute path to a file" + @lookup_context.with_fallbacks.find_template(options[:file], nil, false, keys, @details) + end elsif options.key?(:inline) handler = Template.handler_for_extension(options[:type] || "erb") format = if handler.respond_to?(:default_format) @@ -49,14 +54,14 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. def render_template(view, template, layout_name, locals) - render_with_layout(view, layout_name, template, locals) do |layout| + render_with_layout(view, template, layout_name, locals) do |layout| instrument(:template, identifier: template.identifier, layout: layout.try(:virtual_path)) do template.render(view, locals) { |*name| view._layout_for(*name) } end end end - def render_with_layout(view, path, template, locals) + def render_with_layout(view, template, path, locals) layout = path && find_layout(path, locals.keys, [formats.first]) content = yield(layout) @@ -84,6 +89,7 @@ module ActionView when String begin if layout.start_with?("/") + ActiveSupport::Deprecation.warn "Rendering layouts from an absolute path is deprecated." @lookup_context.with_fallbacks.find_template(layout, nil, false, [], details) else @lookup_context.find_template(layout, nil, false, [], details) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 6e3af1536a..b80bf56c1b 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -113,16 +113,18 @@ module ActionView eager_autoload do autoload :Error + autoload :RawFile autoload :Handlers autoload :HTML autoload :Inline + autoload :Sources autoload :Text autoload :Types end extend Template::Handlers - attr_reader :source, :identifier, :handler, :original_encoding, :updated_at + attr_reader :identifier, :handler, :original_encoding, :updated_at attr_reader :variable, :format, :variant, :locals, :virtual_path def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, virtual_path: nil, updated_at: nil) @@ -139,7 +141,7 @@ module ActionView @virtual_path = virtual_path @variable = if @virtual_path - base = @virtual_path[-1] == "/" ? "" : File.basename(@virtual_path) + base = @virtual_path[-1] == "/" ? "" : ::File.basename(@virtual_path) base =~ /\A_?(.*?)(?:\.\w+)*\z/ $1.to_sym end @@ -163,6 +165,7 @@ module ActionView deprecate def formats; Array(format); end deprecate def variants=(_); end deprecate def variants; [variant]; end + deprecate def refresh(_); self; end # Returns whether the underlying handler supports streaming. If so, # a streaming buffer *may* be passed when it starts rendering. @@ -189,25 +192,6 @@ module ActionView @type ||= Types[format] 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 - # anymore since it was already compiled. In such cases, all you need to do is to call - # refresh passing in the view object. - # - # Notice this method raises an error if the template to be refreshed does not have a - # virtual path set (true just for inline templates). - def refresh(view) - raise "A template needs to have a virtual path in order to be refreshed" unless @virtual_path - lookup = view.lookup_context - pieces = @virtual_path.split("/") - name = pieces.pop - partial = !!name.sub!(/^_/, "") - lookup.disable_cache do - lookup.find_template(name, [ pieces.join("/") ], partial, @locals) - end - end - def short_identifier @short_identifier ||= defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier end @@ -216,6 +200,10 @@ module ActionView "#<#{self.class.name} #{short_identifier} locals=#{@locals.inspect}>" end + def source + @source.to_s + end + # This method is responsible for properly setting the encoding of the # source. Until this point, we assume that the source is BINARY data. # If no additional information is supplied, we assume the encoding is @@ -297,9 +285,6 @@ module ActionView compile(mod) end - # Just discard the source if we have a virtual path. This - # means we can get the template back. - @source = nil if @virtual_path @compiled = true end end @@ -331,6 +316,7 @@ module ActionView # Make sure that the resulting String to be eval'd is in the # encoding of the code + original_source = source source = +<<-end_src def #{method_name}(local_assigns, output_buffer) @virtual_path = #{@virtual_path.inspect};#{locals_code};#{code} @@ -351,7 +337,14 @@ module ActionView raise WrongEncodingError.new(source, Encoding.default_internal) end - mod.module_eval(source, identifier, 0) + begin + mod.module_eval(source, identifier, 0) + rescue SyntaxError + # Account for when code in the template is not syntactically valid; e.g. if we're using + # ERB and the user writes <%= foo( %>, attempting to call a helper `foo` and interpolate + # the result into the template, but missing an end parenthesis. + raise SyntaxErrorInTemplate.new(self, original_source) + end end def handle_render_error(view, e) @@ -359,12 +352,7 @@ module ActionView e.sub_template_of(self) raise e else - template = self - unless template.source - template = refresh(view) - template.encode! - end - raise Template::Error.new(template) + raise Template::Error.new(self) end end diff --git a/actionview/lib/action_view/template/error.rb b/actionview/lib/action_view/template/error.rb index 4e3c02e05e..d0ea03e228 100644 --- a/actionview/lib/action_view/template/error.rb +++ b/actionview/lib/action_view/template/error.rb @@ -109,7 +109,7 @@ module ActionView end end - def annoted_source_code + def annotated_source_code source_extract(4) end @@ -138,4 +138,24 @@ module ActionView end TemplateError = Template::Error + + class SyntaxErrorInTemplate < TemplateError #:nodoc + def initialize(template, offending_code_string) + @offending_code_string = offending_code_string + super(template) + end + + def message + <<~MESSAGE + Encountered a syntax error while rendering template: check #{@offending_code_string} + MESSAGE + end + + def annotated_source_code + @offending_code_string.split("\n").map.with_index(1) { |line, index| + indentation = " " * 4 + "#{index}:#{indentation}#{line}" + } + end + end end diff --git a/actionview/lib/action_view/template/handlers.rb b/actionview/lib/action_view/template/handlers.rb index ddaac7a100..6450513003 100644 --- a/actionview/lib/action_view/template/handlers.rb +++ b/actionview/lib/action_view/template/handlers.rb @@ -45,12 +45,12 @@ module ActionView #:nodoc: unless params.find_all { |type, _| type == :req || type == :opt }.length >= 2 ActiveSupport::Deprecation.warn <<~eowarn - Single arity template handlers are deprecated. Template handlers must + Single arity template handlers are deprecated. Template handlers must now accept two parameters, the view object and the source for the view object. Change: - >> #{handler.class}#call(#{params.map(&:last).join(", ")}) + >> #{handler}.call(#{params.map(&:last).join(", ")}) To: - >> #{handler.class}#call(#{params.map(&:last).join(", ")}, source) + >> #{handler}.call(#{params.map(&:last).join(", ")}, source) eowarn handler = LegacyHandlerWrapper.new(handler) end diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index 307b852440..e602aa117a 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -25,7 +25,7 @@ module ActionView src = @src view = Class.new(ActionView::Base) { include action_view_erb_handler_context._routes.url_helpers - class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0) + class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", defined?(@filename) ? @filename : "(erubi)", 0) }.empty view._run(:_template, nil, {}, ActionView::OutputBuffer.new) end diff --git a/actionview/lib/action_view/template/raw_file.rb b/actionview/lib/action_view/template/raw_file.rb new file mode 100644 index 0000000000..623351305f --- /dev/null +++ b/actionview/lib/action_view/template/raw_file.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module ActionView #:nodoc: + # = Action View RawFile Template + class Template #:nodoc: + class RawFile #:nodoc: + attr_accessor :type, :format + + def initialize(filename) + @filename = filename.to_s + extname = ::File.extname(filename).delete(".") + @type = Template::Types[extname] || Template::Types[:text] + @format = @type.symbol + end + + def identifier + @filename + end + + def render(*args) + ::File.read(@filename) + end + + def formats; Array(format); end + deprecate :formats + end + end +end diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 1c577348e5..ce53eb046d 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -118,17 +118,12 @@ module ActionView locals = locals.map(&:to_s).sort!.freeze cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, false, locals) + _find_all(name, prefix, partial, details, key, locals) end end - def find_all_anywhere(name, prefix, partial = false, details = {}, key = nil, locals = []) - locals = locals.map(&:to_s).sort!.freeze - - cached(key, [name, prefix, partial], details, locals) do - find_templates(name, prefix, partial, details, true, locals) - end - end + alias :find_all_anywhere :find_all + deprecate :find_all_anywhere def find_all_with_query(query) # :nodoc: @cache.cache_query(query) { find_template_paths(File.join(@path, query)) } @@ -136,13 +131,17 @@ module ActionView private + def _find_all(name, prefix, partial, details, key, locals) + find_templates(name, prefix, partial, details, locals) + end + delegate :caching?, to: :class # This is what child classes implement. No defaults are needed # because Resolver guarantees that the arguments are present and # normalized. - def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) - raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, outside_app_allowed = false, locals = []) method" + def find_templates(name, prefix, partial, details, locals = []) + raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, locals = []) method" end # Handles templates caching. If a key is given and caching is on @@ -168,34 +167,57 @@ module ActionView DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}" def initialize(pattern = nil) - @pattern = pattern || DEFAULT_PATTERN + if pattern + ActiveSupport::Deprecation.warn "Specifying a custom path for #{self.class} is deprecated. Implement a custom Resolver subclass instead." + @pattern = pattern + else + @pattern = DEFAULT_PATTERN + end + @unbound_templates = Concurrent::Map.new + super() + end + + def clear_cache + @unbound_templates.clear super() end private - def find_templates(name, prefix, partial, details, outside_app_allowed = false, locals) + def _find_all(name, prefix, partial, details, key, locals) path = Path.build(name, prefix, partial) - query(path, details, details[:formats], outside_app_allowed, locals) + query(path, details, details[:formats], locals, cache: !!key) end - def query(path, details, formats, outside_app_allowed, locals) + def query(path, details, formats, locals, cache:) template_paths = find_template_paths_from_details(path, details) - template_paths = reject_files_external_to_app(template_paths) unless outside_app_allowed + template_paths = reject_files_external_to_app(template_paths) template_paths.map do |template| - build_template(template, path.virtual, locals) + unbound_template = + if cache + @unbound_templates.compute_if_absent([template, path.virtual]) do + build_unbound_template(template, path.virtual) + end + else + build_unbound_template(template, path.virtual) + end + + unbound_template.bind_locals(locals) end end - def build_template(template, virtual_path, locals) + def build_unbound_template(template, virtual_path) handler, format, variant = extract_handler_and_format_and_variant(template) + source = Template::Sources::File.new(template) - FileTemplate.new(File.expand_path(template), handler, + UnboundTemplate.new( + source, + template, + handler, virtual_path: virtual_path, format: format, variant: variant, - locals: locals ) end @@ -273,45 +295,10 @@ module ActionView end end - # A resolver that loads files from the filesystem. It allows setting your own - # resolving pattern. Such pattern can be a glob string supported by some variables. - # - # ==== Examples - # - # Default pattern, loads views the same way as previous versions of rails, eg. when you're - # looking for <tt>users/new</tt> it will produce query glob: <tt>users/new{.{en},}{.{html,js},}{.{erb,haml},}</tt> - # - # FileSystemResolver.new("/path/to/views", ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}") - # - # This one allows you to keep files with different formats in separate subdirectories, - # eg. <tt>users/new.html</tt> will be loaded from <tt>users/html/new.erb</tt> or <tt>users/new.html.erb</tt>, - # <tt>users/new.js</tt> from <tt>users/js/new.erb</tt> or <tt>users/new.js.erb</tt>, etc. - # - # FileSystemResolver.new("/path/to/views", ":prefix/{:formats/,}:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}") - # - # If you don't specify a pattern then the default will be used. - # - # In order to use any of the customized resolvers above in a Rails application, you just need - # to configure ActionController::Base.view_paths in an initializer, for example: - # - # ActionController::Base.view_paths = FileSystemResolver.new( - # Rails.root.join("app/views"), - # ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}", - # ) - # - # ==== Pattern format and variables - # - # Pattern has to be a valid glob string, and it allows you to use the - # following variables: - # - # * <tt>:prefix</tt> - usually the controller path - # * <tt>:action</tt> - name of the action - # * <tt>:locale</tt> - possible locale versions - # * <tt>:formats</tt> - possible request formats (for example html, json, xml...) - # * <tt>:variants</tt> - possible request variants (for example phone, tablet...) - # * <tt>:handlers</tt> - possible handlers (for example erb, haml, builder...) - # + # A resolver that loads files from the filesystem. class FileSystemResolver < PathResolver + attr_reader :path + def initialize(path, pattern = nil) raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver) super(pattern) @@ -331,6 +318,10 @@ module ActionView # An Optimized resolver for Rails' most common case. class OptimizedFileSystemResolver < FileSystemResolver #:nodoc: + def initialize(path) + super(path) + end + private def find_template_paths_from_details(path, details) @@ -386,12 +377,18 @@ module ActionView # The same as FileSystemResolver but does not allow templates to store # a virtual path since it is invalid for such resolvers. class FallbackFileSystemResolver < FileSystemResolver #:nodoc: + private_class_method :new + def self.instances [new(""), new("/")] end - def build_template(template, virtual_path, locals) - super(template, nil, locals) + def build_unbound_template(template, _) + super(template, nil) + end + + def reject_files_external_to_app(files) + files end end end diff --git a/actionview/lib/action_view/template/sources.rb b/actionview/lib/action_view/template/sources.rb new file mode 100644 index 0000000000..8b89535741 --- /dev/null +++ b/actionview/lib/action_view/template/sources.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module ActionView + class Template + module Sources + extend ActiveSupport::Autoload + + eager_autoload do + autoload :File + end + end + end +end diff --git a/actionview/lib/action_view/template/sources/file.rb b/actionview/lib/action_view/template/sources/file.rb new file mode 100644 index 0000000000..2d3a7dec7f --- /dev/null +++ b/actionview/lib/action_view/template/sources/file.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module ActionView + class Template + module Sources + class File + def initialize(filename) + @filename = filename + end + + def to_s + ::File.binread @filename + end + end + end + end +end diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index a16dc0096e..1bedf44934 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -23,10 +23,10 @@ module ActionView #:nodoc: private - def query(path, exts, _, _, locals) + def query(path, exts, _, locals, cache:) query = +"" - EXTENSIONS.each_key do |ext| - query << "(" << exts[ext].map { |e| e && Regexp.escape(".#{e}") }.join("|") << "|)" + EXTENSIONS.each do |ext, prefix| + query << "(" << exts[ext].map { |e| e && Regexp.escape("#{prefix}#{e}") }.join("|") << "|)" end query = /^(#{Regexp.escape(path)})#{query}$/ @@ -47,7 +47,7 @@ module ActionView #:nodoc: end class NullResolver < PathResolver - def query(path, exts, _, _, locals) + def query(path, exts, _, locals, cache:) handler, format, variant = extract_handler_and_format_and_variant(path) [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals)] end diff --git a/actionview/lib/action_view/unbound_template.rb b/actionview/lib/action_view/unbound_template.rb new file mode 100644 index 0000000000..db69b6d016 --- /dev/null +++ b/actionview/lib/action_view/unbound_template.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "concurrent/map" + +module ActionView + class UnboundTemplate + def initialize(source, identifer, handler, options) + @source = source + @identifer = identifer + @handler = handler + @options = options + + @templates = Concurrent::Map.new(initial_capacity: 2) + end + + def bind_locals(locals) + @templates[locals] ||= build_template(locals) + end + + private + + def build_template(locals) + options = @options.merge(locals: locals) + Template.new( + @source, + @identifer, + @handler, + options + ) + end + end +end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index fe317bf39e..2c1b277b41 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -205,3 +205,5 @@ class ActiveSupport::TestCase skip message if defined?(JRUBY_VERSION) end end + +require_relative "../../tools/test_common" diff --git a/actionview/test/actionpack/abstract/abstract_controller_test.rb b/actionview/test/actionpack/abstract/abstract_controller_test.rb index 4d4e2b8ef2..eecc19d413 100644 --- a/actionview/test/actionpack/abstract/abstract_controller_test.rb +++ b/actionview/test/actionpack/abstract/abstract_controller_test.rb @@ -229,11 +229,11 @@ module AbstractController end class ActionMissingRespondToActionController < AbstractController::Base - # No actions - private - def action_missing(action_name) - self.response_body = "success" - end + # No actions + private + def action_missing(action_name) + self.response_body = "success" + end end class RespondToActionController < AbstractController::Base diff --git a/actionview/test/actionpack/abstract/render_test.rb b/actionview/test/actionpack/abstract/render_test.rb index d863548a5c..e4e8ac93b2 100644 --- a/actionview/test/actionpack/abstract/render_test.rb +++ b/actionview/test/actionpack/abstract/render_test.rb @@ -26,7 +26,7 @@ module AbstractController end def file - render file: "some/file" + ActiveSupport::Deprecation.silence { render file: "some/file" } end def inline diff --git a/actionview/test/actionpack/controller/layout_test.rb b/actionview/test/actionpack/controller/layout_test.rb index 838b564c5d..f946e4360d 100644 --- a/actionview/test/actionpack/controller/layout_test.rb +++ b/actionview/test/actionpack/controller/layout_test.rb @@ -233,7 +233,9 @@ class LayoutSetInResponseTest < ActionController::TestCase def test_absolute_pathed_layout @controller = AbsolutePathLayoutController.new - get :hello + assert_deprecated do + get :hello + end assert_equal "layout_test.erb hello.erb", @response.body.strip end end diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 52c3c54d96..c8ce7366d1 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -872,48 +872,64 @@ class RenderTest < ActionController::TestCase # :ported: def test_render_file_with_instance_variables - get :render_file_with_instance_variables + assert_deprecated do + get :render_file_with_instance_variables + end assert_equal "The secret is in the sauce\n", @response.body end def test_render_file - get :hello_world_file + assert_deprecated do + get :hello_world_file + end assert_equal "Hello world!", @response.body end # :ported: def test_render_file_not_using_full_path - get :render_file_not_using_full_path + assert_deprecated do + get :render_file_not_using_full_path + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_not_using_full_path_with_dot_in_path - get :render_file_not_using_full_path_with_dot_in_path + assert_deprecated do + get :render_file_not_using_full_path_with_dot_in_path + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_using_pathname - get :render_file_using_pathname + assert_deprecated do + get :render_file_using_pathname + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_with_locals - get :render_file_with_locals + assert_deprecated do + get :render_file_with_locals + end assert_equal "The secret is in the sauce\n", @response.body end # :ported: def test_render_file_as_string_with_locals - get :render_file_as_string_with_locals + assert_deprecated do + get :render_file_as_string_with_locals + end assert_equal "The secret is in the sauce\n", @response.body end # :assessed: def test_render_file_from_template - get :render_file_from_template + assert_deprecated do + get :render_file_from_template + end assert_equal "The secret is in the sauce\n", @response.body end @@ -1133,11 +1149,19 @@ class RenderTest < ActionController::TestCase end def test_bad_render_to_string_still_throws_exception - assert_raise(ActionView::MissingTemplate) { get :render_to_string_with_exception } + assert_deprecated do + assert_raise(ActionView::MissingTemplate) do + get :render_to_string_with_exception + end + end end def test_render_to_string_that_throws_caught_exception_doesnt_break_assigns - assert_nothing_raised { get :render_to_string_with_caught_exception } + assert_deprecated do + assert_nothing_raised do + get :render_to_string_with_caught_exception + end + end assert_equal "i'm before the render", @controller.instance_variable_get(:@before) assert_equal "i'm after the render", @controller.instance_variable_get(:@after) end diff --git a/actionview/test/fixtures/test/_cached_set.erb b/actionview/test/fixtures/test/_cached_set.erb new file mode 100644 index 0000000000..cd492fc519 --- /dev/null +++ b/actionview/test/fixtures/test/_cached_set.erb @@ -0,0 +1 @@ +<%= cached_set.first %> | <%= cached_set.second %> | <%= cached_set.third %> | <%= cached_set.fourth %> | <%= cached_set.fifth %> diff --git a/actionview/test/fixtures/test/syntax_error.html.erb b/actionview/test/fixtures/test/syntax_error.html.erb new file mode 100644 index 0000000000..4004a2b187 --- /dev/null +++ b/actionview/test/fixtures/test/syntax_error.html.erb @@ -0,0 +1,4 @@ +<%= foo( + 1, + 2, +%> diff --git a/actionview/test/template/fallback_file_system_resolver_test.rb b/actionview/test/template/fallback_file_system_resolver_test.rb index 304cdb8a03..fa770f3a15 100644 --- a/actionview/test/template/fallback_file_system_resolver_test.rb +++ b/actionview/test/template/fallback_file_system_resolver_test.rb @@ -4,7 +4,7 @@ require "abstract_unit" class FallbackFileSystemResolverTest < ActiveSupport::TestCase def setup - @root_resolver = ActionView::FallbackFileSystemResolver.new("/") + @root_resolver = ActionView::FallbackFileSystemResolver.send(:new, "/") end def test_should_have_no_virtual_path diff --git a/actionview/test/template/file_system_resolver_test.rb b/actionview/test/template/file_system_resolver_test.rb new file mode 100644 index 0000000000..aa03fdcb13 --- /dev/null +++ b/actionview/test/template/file_system_resolver_test.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "template/resolver_shared_tests" + +class FileSystemResolverTest < ActiveSupport::TestCase + include ResolverSharedTests + + def resolver + ActionView::FileSystemResolver.new(tmpdir) + end +end diff --git a/actionview/test/template/javascript_helper_test.rb b/actionview/test/template/javascript_helper_test.rb index 4c28aeaee1..f974e5ae0c 100644 --- a/actionview/test/template/javascript_helper_test.rb +++ b/actionview/test/template/javascript_helper_test.rb @@ -54,7 +54,7 @@ class JavaScriptHelperTest < ActionView::TestCase assert_equal "foo", output_buffer, "javascript_tag without a block should not concat to output_buffer" end - # Setting the :extname option will control what extension (if any) is appended to the url for assets + # Setting the :extname option will control what extension (if any) is appended to the URL for assets def test_javascript_include_tag assert_dom_equal "<script src='/foo.js'></script>", javascript_include_tag("/foo") assert_dom_equal "<script src='/foo'></script>", javascript_include_tag("/foo", extname: false) diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 85735139c1..8b160a7336 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -51,9 +51,20 @@ class AVLogSubscriberTest < ActiveSupport::TestCase def @view.combined_fragment_cache_key(*); "ahoy `controller` dependency"; end end + def test_render_template_template + Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do + @view.render(template: "test/hello_world") + wait + + assert_equal 2, @logger.logged(:info).size + assert_match(/Rendering test\/hello_world\.erb/, @logger.logged(:info).first) + assert_match(/Rendered test\/hello_world\.erb/, @logger.logged(:info).last) + end + end + def test_render_file_template Rails.stub(:root, File.expand_path(FIXTURE_LOAD_PATH)) do - @view.render(file: "test/hello_world") + @view.render(file: "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") wait assert_equal 2, @logger.logged(:info).size diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 3e357fe1a7..72e1f50fdf 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -143,16 +143,16 @@ class LookupContextTest < ActiveSupport::TestCase assert_deprecated do @lookup_context.with_fallbacks do assert_equal 3, @lookup_context.view_paths.size - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[0] + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[1] end end @lookup_context = @lookup_context.with_fallbacks assert_equal 3, @lookup_context.view_paths.size - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("") - assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.new("/") + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[0] + assert_includes @lookup_context.view_paths, ActionView::FallbackFileSystemResolver.instances[1] end test "add fallbacks just once in nested fallbacks calls" do diff --git a/actionview/test/template/optimized_file_system_resolver_test.rb b/actionview/test/template/optimized_file_system_resolver_test.rb new file mode 100644 index 0000000000..c0c64357ce --- /dev/null +++ b/actionview/test/template/optimized_file_system_resolver_test.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "template/resolver_shared_tests" + +class OptimizedFileSystemResolverTest < ActiveSupport::TestCase + include ResolverSharedTests + + def resolver + ActionView::OptimizedFileSystemResolver.new(tmpdir) + end +end diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index f0fed601f8..f0b5d7d95e 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -53,15 +53,20 @@ module RenderTestCases assert_match(/You invoked render but did not give any of (.+) option\./, e.message) end + def test_render_template + assert_equal "Hello world!", @view.render(template: "test/hello_world") + end + + def test_render_file - assert_equal "Hello world!", @view.render(file: "test/hello_world") + assert_equal "Hello world!", assert_deprecated { @view.render(file: "test/hello_world") } end # Test if :formats, :locale etc. options are passed correctly to the resolvers. def test_render_file_with_format - assert_match "<h1>No Comment</h1>", @view.render(file: "comments/empty", formats: [:html]) - assert_match "<error>No Comment</error>", @view.render(file: "comments/empty", formats: [:xml]) - assert_match "<error>No Comment</error>", @view.render(file: "comments/empty", formats: :xml) + assert_match "<h1>No Comment</h1>", assert_deprecated { @view.render(file: "comments/empty", formats: [:html]) } + assert_match "<error>No Comment</error>", assert_deprecated { @view.render(file: "comments/empty", formats: [:xml]) } + assert_match "<error>No Comment</error>", assert_deprecated { @view.render(file: "comments/empty", formats: :xml) } end def test_render_template_with_format @@ -94,8 +99,8 @@ module RenderTestCases end def test_render_file_with_locale - assert_equal "<h1>Kein Kommentar</h1>", @view.render(file: "comments/empty", locale: [:de]) - assert_equal "<h1>Kein Kommentar</h1>", @view.render(file: "comments/empty", locale: :de) + assert_equal "<h1>Kein Kommentar</h1>", assert_deprecated { @view.render(file: "comments/empty", locale: [:de]) } + assert_equal "<h1>Kein Kommentar</h1>", assert_deprecated { @view.render(file: "comments/empty", locale: :de) } end def test_render_template_with_locale @@ -107,8 +112,8 @@ module RenderTestCases end def test_render_file_with_handlers - assert_equal "<h1>No Comment</h1>\n", @view.render(file: "comments/empty", handlers: [:builder]) - assert_equal "<h1>No Comment</h1>\n", @view.render(file: "comments/empty", handlers: :builder) + assert_equal "<h1>No Comment</h1>\n", assert_deprecated { @view.render(file: "comments/empty", handlers: [:builder]) } + assert_equal "<h1>No Comment</h1>\n", assert_deprecated { @view.render(file: "comments/empty", handlers: :builder) } end def test_render_template_with_handlers @@ -156,22 +161,27 @@ module RenderTestCases assert_equal "Elastica", @view.render(template: "/shared") end - def test_render_file_with_full_path + def test_render_file_with_full_path_no_extension template_path = File.expand_path("../fixtures/test/hello_world", __dir__) + assert_equal "Hello world!", assert_deprecated { @view.render(file: template_path) } + end + + def test_render_file_with_full_path + template_path = File.expand_path("../fixtures/test/hello_world.erb", __dir__) assert_equal "Hello world!", @view.render(file: template_path) end def test_render_file_with_instance_variables - assert_equal "The secret is in the sauce\n", @view.render(file: "test/render_file_with_ivar") + assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/render_file_with_ivar") } end def test_render_file_with_locals locals = { secret: "in the sauce" } - assert_equal "The secret is in the sauce\n", @view.render(file: "test/render_file_with_locals", locals: locals) + assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/render_file_with_locals", locals: locals) } end def test_render_file_not_using_full_path_with_dot_in_path - assert_equal "The secret is in the sauce\n", @view.render(file: "test/dot.directory/render_file_with_ivar") + assert_equal "The secret is in the sauce\n", assert_deprecated { @view.render(file: "test/dot.directory/render_file_with_ivar") } end def test_render_partial_from_default @@ -259,18 +269,24 @@ module RenderTestCases "and is followed by any combination of letters, numbers and underscores.", e.message end + def test_render_template_with_syntax_error + e = assert_raises(ActionView::Template::Error) { @view.render(template: "test/syntax_error") } + assert_match %r!syntax!, e.message + assert_equal "1: <%= foo(", e.annotated_source_code[0].strip + end + def test_render_partial_with_errors e = assert_raises(ActionView::Template::Error) { @view.render(partial: "test/raise") } 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[0].strip + assert_equal "1: <%= doesnt_exist %>", e.annotated_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 + error_lines = e.annotated_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 @@ -286,11 +302,11 @@ module RenderTestCases end def test_render_file_with_errors - e = assert_raises(ActionView::Template::Error) { @view.render(file: File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) } + e = assert_raises(ActionView::Template::Error) { assert_deprecated { @view.render(file: File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) } } 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[0].strip + assert_equal "1: <%= doesnt_exist %>", e.annotated_source_code[0].strip assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end @@ -795,6 +811,28 @@ class CachedCollectionViewRenderTest < ActiveSupport::TestCase end end + test "collection caching with repeated collection" do + sets = [ + [1, 2, 3, 4, 5], + [1, 2, 3, 4, 4], + [1, 2, 3, 4, 5], + [1, 2, 3, 4, 4], + [1, 2, 3, 4, 6] + ] + + result = @view.render(partial: "test/cached_set", collection: sets, cached: true) + + splited_result = result.split("\n") + assert_equal 5, splited_result.count + assert_equal [ + "1 | 2 | 3 | 4 | 5", + "1 | 2 | 3 | 4 | 4", + "1 | 2 | 3 | 4 | 5", + "1 | 2 | 3 | 4 | 4", + "1 | 2 | 3 | 4 | 6" + ], splited_result + end + private def cache_key(*names, virtual_path) digest = ActionView::Digestor.digest name: virtual_path, format: :html, finder: @view.lookup_context, dependencies: [] diff --git a/actionview/test/template/resolver_patterns_test.rb b/actionview/test/template/resolver_patterns_test.rb index 8122de779f..22815c8dbe 100644 --- a/actionview/test/template/resolver_patterns_test.rb +++ b/actionview/test/template/resolver_patterns_test.rb @@ -6,7 +6,10 @@ class ResolverPatternsTest < ActiveSupport::TestCase def setup path = File.expand_path("../fixtures", __dir__) pattern = ":prefix/{:formats/,}:action{.:formats,}{+:variants,}{.:handlers,}" - @resolver = ActionView::FileSystemResolver.new(path, pattern) + + assert_deprecated do + @resolver = ActionView::FileSystemResolver.new(path, pattern) + end end def test_should_return_empty_list_for_unknown_path diff --git a/actionview/test/template/resolver_shared_tests.rb b/actionview/test/template/resolver_shared_tests.rb new file mode 100644 index 0000000000..8b47c5bc89 --- /dev/null +++ b/actionview/test/template/resolver_shared_tests.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true + +module ResolverSharedTests + attr_reader :tmpdir + + def run(*args) + capture_exceptions do + Dir.mktmpdir(nil, __dir__) { |dir| @tmpdir = dir; super } + end + end + + def with_file(filename, source = "File at #{filename}") + path = File.join(tmpdir, filename) + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, source) + end + + def context + @context ||= ActionView::LookupContext.new(resolver) + end + + def test_can_find_with_no_extensions + with_file "test/hello_world", "Hello default!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: [:phone], handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello default!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_nil templates[0].format + assert_nil templates[0].variant + assert_kind_of ActionView::Template::Handlers::Raw, templates[0].handler + end + + def test_can_find_with_just_handler + with_file "test/hello_world.erb", "Hello erb!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: [:phone], handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello erb!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_nil templates[0].format + assert_nil templates[0].variant + assert_kind_of ActionView::Template::Handlers::ERB, templates[0].handler + end + + def test_can_find_with_format_and_handler + with_file "test/hello_world.text.builder", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html, :text], variants: [:phone], handlers: [:erb, :builder]) + assert_equal 1, templates.size + assert_equal "Hello plain text!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_equal :text, templates[0].format + assert_nil templates[0].variant + assert_kind_of ActionView::Template::Handlers::Builder, templates[0].handler + end + + def test_can_find_with_variant_format_and_handler + with_file "test/hello_world.html+phone.erb", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: [:phone], handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello plain text!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_equal :html, templates[0].format + assert_equal "phone", templates[0].variant + assert_kind_of ActionView::Template::Handlers::ERB, templates[0].handler + end + + def test_can_find_with_any_variant_format_and_handler + with_file "test/hello_world.html+phone.erb", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [:en], formats: [:html], variants: :any, handlers: [:erb]) + assert_equal 1, templates.size + assert_equal "Hello plain text!", templates[0].source + assert_equal "test/hello_world", templates[0].virtual_path + assert_equal :html, templates[0].format + assert_equal "phone", templates[0].variant + assert_kind_of ActionView::Template::Handlers::ERB, templates[0].handler + end + + def test_doesnt_find_template_with_wrong_details + with_file "test/hello_world.html.erb", "Hello plain text!" + + templates = resolver.find_all("hello_world", "test", false, locale: [], formats: [:xml], variants: :any, handlers: [:builder]) + assert_equal 0, templates.size + + templates = resolver.find_all("hello_world", "test", false, locale: [], formats: [:xml], variants: :any, handlers: [:erb]) + assert_equal 0, templates.size + end + + def test_found_template_is_cached + with_file "test/hello_world.html.erb", "Hello HTML!" + + a = context.find("hello_world", "test", false, [], {}) + b = context.find("hello_world", "test", false, [], {}) + assert_same a, b + end + + def test_different_templates_when_cache_disabled + with_file "test/hello_world.html.erb", "Hello HTML!" + + a = context.find("hello_world", "test", false, [], {}) + b = context.disable_cache { context.find("hello_world", "test", false, [], {}) } + c = context.find("hello_world", "test", false, [], {}) + + # disable_cache should give us a new object + assert_not_same a, b + + # but it should not clear the cache + assert_same a, c + end + + def test_same_template_from_different_details_is_same_object + with_file "test/hello_world.html.erb", "Hello HTML!" + + a = context.find("hello_world", "test", false, [], locale: [:en]) + b = context.find("hello_world", "test", false, [], locale: [:fr]) + assert_same a, b + end + + def test_templates_with_optional_locale_shares_common_object + with_file "test/hello_world.text.erb", "Generic plain text!" + with_file "test/hello_world.fr.text.erb", "Texte en Francais!" + + en = context.find_all("hello_world", "test", false, [], locale: [:en]) + fr = context.find_all("hello_world", "test", false, [], locale: [:fr]) + + assert_equal 1, en.size + assert_equal 2, fr.size + + assert_equal "Generic plain text!", en[0].source + assert_equal "Texte en Francais!", fr[0].source + assert_equal "Generic plain text!", fr[1].source + + assert_same en[0], fr[1] + end + + def test_virtual_path_is_preserved_with_dot + with_file "test/hello_world.html.erb", "Hello html!" + + template = context.find("hello_world.html", "test", false, [], {}) + assert_equal "test/hello_world.html", template.virtual_path + + template = context.find("hello_world", "test", false, [], {}) + assert_equal "test/hello_world", template.virtual_path + end +end diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb index a5b59a700e..a5e673e71e 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -47,12 +47,12 @@ class FiberedTest < SetupFiberedBase end def test_render_file - assert_equal "Hello world!", buffered_render(file: "test/hello_world") + assert_equal "Hello world!", assert_deprecated { buffered_render(file: "test/hello_world") } end def test_render_file_with_locals locals = { secret: "in the sauce" } - assert_equal "The secret is in the sauce\n", buffered_render(file: "test/render_file_with_locals", locals: locals) + assert_equal "The secret is in the sauce\n", assert_deprecated { buffered_render(file: "test/render_file_with_locals", locals: locals) } end def test_render_partial diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 71fb99115b..049e0bc129 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -91,10 +91,10 @@ class TestERBTemplate < ActiveSupport::TestCase assert_equal "<%= hello %>", render end - def test_template_loses_its_source_after_rendering + def test_template_does_not_lose_its_source_after_rendering @template = new_template render - assert_nil @template.source + assert_equal "<%= hello %>", @template.source end def test_template_does_not_lose_its_source_after_rendering_if_it_does_not_have_a_virtual_path @@ -121,24 +121,10 @@ class TestERBTemplate < ActiveSupport::TestCase assert_equal "hellopartialhello", render end - def test_refresh_with_templates + def test_refresh_is_deprecated @template = new_template("Hello", virtual_path: "test/foo/bar", locals: [:key]) - assert_called_with(@context.lookup_context, :find_template, ["bar", %w(test/foo), false, [:key]], returns: "template") do - assert_equal "template", @template.refresh(@context) - end - end - - def test_refresh_with_partials - @template = new_template("Hello", virtual_path: "test/_foo", locals: [:key]) - assert_called_with(@context.lookup_context, :find_template, ["foo", %w(test), true, [:key]], returns: "partial") do - assert_equal "partial", @template.refresh(@context) - end - end - - def test_refresh_raises_an_error_without_virtual_path - @template = new_template("Hello", virtual_path: nil) - assert_raise RuntimeError do - @template.refresh(@context) + assert_deprecated do + assert_same @template, @template.refresh(@context) end end diff --git a/actionview/test/template/testing/fixture_resolver_test.rb b/actionview/test/template/testing/fixture_resolver_test.rb index afb6686dac..6d0317e0c9 100644 --- a/actionview/test/template/testing/fixture_resolver_test.rb +++ b/actionview/test/template/testing/fixture_resolver_test.rb @@ -17,4 +17,14 @@ class FixtureResolverTest < ActiveSupport::TestCase assert_equal "arbitrary/path", templates.first.virtual_path assert_nil templates.first.format end + + def test_should_match_templates_with_variants + resolver = ActionView::FixtureResolver.new("arbitrary/path.html+variant.erb" => "this text") + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: [:variant], handlers: [:erb]) + assert_equal 1, templates.size, "expected one template" + assert_equal "this text", templates.first.source + assert_equal "arbitrary/path", templates.first.virtual_path + assert_equal :html, templates.first.format + assert_equal "variant", templates.first.variant + end end diff --git a/actionview/test/ujs/public/test/call-remote.js b/actionview/test/ujs/public/test/call-remote.js index 0f92007007..fb033491f9 100644 --- a/actionview/test/ujs/public/test/call-remote.js +++ b/actionview/test/ujs/public/test/call-remote.js @@ -53,7 +53,7 @@ asyncTest('form default method is GET', 1, function() { }) }) -asyncTest('form url is picked up from "action"', 1, function() { +asyncTest('form URL is picked up from "action"', 1, function() { buildForm({ method: 'post' }) submit(function(e, data, status, xhr) { @@ -61,7 +61,7 @@ asyncTest('form url is picked up from "action"', 1, function() { }) }) -asyncTest('form url is read from "action" not "href"', 1, function() { +asyncTest('form URL is read from "action" not "href"', 1, function() { buildForm({ method: 'post', href: '/echo2' }) submit(function(e, data, status, xhr) { @@ -69,7 +69,7 @@ asyncTest('form url is read from "action" not "href"', 1, function() { }) }) -asyncTest('form url is read from submit button "formaction" if submit is triggered by that button', 1, function() { +asyncTest('form URL is read from submit button "formaction" if submit is triggered by that button', 1, function() { var submitButton = $('<input type="submit" formaction="/echo">') buildForm({ method: 'post', href: '/echo2' }) diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index 55d39b0a52..9e41067549 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -121,7 +121,7 @@ asyncTest('clicking on a link with both query string in href and data-params', 4 App.assertGetRequest(data) equal(data.params.data1, 'value1', 'ajax arguments should have key data1 with right value') equal(data.params.data2, 'value2', 'ajax arguments should have key data2 with right value') - equal(data.params.data3, 'value3', 'query string in url should be passed to server with right value') + equal(data.params.data3, 'value3', 'query string in URL should be passed to server with right value') }) .bindNative('ajax:complete', function() { start() }) .triggerNative('click') @@ -135,7 +135,7 @@ asyncTest('clicking on a link with both query string in href and data-params wit App.assertPostRequest(data) equal(data.params.data1, 'value1', 'ajax arguments should have key data1 with right value') equal(data.params.data2, 'value2', 'ajax arguments should have key data2 with right value') - equal(data.params.data3, 'value3', 'query string in url should be passed to server with right value') + equal(data.params.data3, 'value3', 'query string in URL should be passed to server with right value') }) .bindNative('ajax:complete', function() { start() }) .triggerNative('click') diff --git a/actionview/test/ujs/public/test/settings.js b/actionview/test/ujs/public/test/settings.js index 682d044403..ff2b057012 100644 --- a/actionview/test/ujs/public/test/settings.js +++ b/actionview/test/ujs/public/test/settings.js @@ -18,7 +18,7 @@ App.assertPostRequest = function(requestEnv) { } App.assertRequestPath = function(requestEnv, path) { - equal(requestEnv['PATH_INFO'], path, 'request should be sent to right url') + equal(requestEnv['PATH_INFO'], path, 'request should be sent to right URL') } App.getVal = function(el) { |