Merge pull request #41969 from jhawthorn/combine_file_system_resolvers

Combine FileSystemResolver classes

actionpack/test/controller/renderer_test.rb

@@ -109,7 +109,6 @@ class RendererTest < ActiveSupport::TestCase
     xml  = "<p>Hello world!</p>\n"
 
     assert_equal html, render["respond_to/using_defaults"]
-    assert_equal xml,  assert_deprecated { render["respond_to/using_defaults.xml.builder"] }
     assert_equal xml,  render["respond_to/using_defaults", formats: :xml]
   end
 

actionview/lib/action_view.rb

@@ -59,10 +59,7 @@ module ActionView
 
     autoload_at "action_view/template/resolver" do
       autoload :Resolver
-      autoload :PathResolver
       autoload :FileSystemResolver
-      autoload :OptimizedFileSystemResolver
-      autoload :FallbackFileSystemResolver
     end
 
     autoload_at "action_view/buffers" do

actionview/lib/action_view/path_set.rb

@@ -72,7 +72,7 @@ def typecast(paths)
         paths.map do |path|
           case path
           when Pathname, String
-            OptimizedFileSystemResolver.new path.to_s
+            FileSystemResolver.new path.to_s
           else
             path
           end

actionview/lib/action_view/template/resolver.rb

@@ -186,16 +186,18 @@ def cached(key, path_info, details, locals)
     end
   end
 
-  # An abstract class that implements a Resolver with path semantics.
-  class PathResolver < Resolver #:nodoc:
+  # A resolver that loads files from the filesystem.
+  class FileSystemResolver < Resolver
     EXTENSIONS = { locale: ".", formats: ".", variants: "+", handlers: "." }
-    DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}"
 
-    def initialize
-      @pattern = DEFAULT_PATTERN
+    attr_reader :path
+
+    def initialize(path)
+      raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver)
       @unbound_templates = Concurrent::Map.new
       @path_parser = PathParser.new
-      super
+      @path = File.expand_path(path)
+      super()
     end
 
     def clear_cache
@@ -204,6 +206,25 @@ def clear_cache
       super
     end
 
+    def to_s
+      @path.to_s
+    end
+    alias :to_path :to_s
+
+    def eql?(resolver)
+      self.class.equal?(resolver.class) && to_path == resolver.to_path
+    end
+    alias :== :eql?
+
+    def all_template_paths # :nodoc:
+      paths = Dir.glob("**/*", base: @path)
+      paths.reject do |filename|
+        File.directory?(File.join(@path, filename))
+      end.map do |filename|
+        filename.gsub(/\.[^\/]*\z/, "")
+      end.uniq
+    end
+
     private
       def _find_all(name, prefix, partial, details, key, locals)
         path = Path.build(name, prefix, partial)
@@ -250,50 +271,12 @@ def reject_files_external_to_app(files)
         files.reject { |filename| !inside_path?(@path, filename) }
       end
 
-      def find_template_paths_from_details(path, details)
-        if path.name.include?(".")
-          ActiveSupport::Deprecation.warn("Rendering actions with '.' in the name is deprecated: #{path}")
-        end
-
-        query = build_query(path, details)
-        find_template_paths(query)
-      end
-
-      def find_template_paths(query)
-        Dir[query].uniq.reject do |filename|
-          File.directory?(filename) ||
-            # deals with case-insensitive file systems.
-            !File.fnmatch(query, filename, File::FNM_EXTGLOB)
-        end
-      end
-
       def inside_path?(path, filename)
         filename = File.expand_path(filename)
         path = File.join(path, "")
         filename.start_with?(path)
       end
 
-      # Helper for building query glob string based on resolver's pattern.
-      def build_query(path, details)
-        query = @pattern.dup
-
-        prefix = path.prefix.empty? ? "" : "#{escape_entry(path.prefix)}\\1"
-        query.gsub!(/:prefix(\/)?/, prefix)
-
-        partial = escape_entry(path.partial? ? "_#{path.name}" : path.name)
-        query.gsub!(":action", partial)
-
-        details.each do |ext, candidates|
-          if ext == :variants && candidates == :any
-            query.gsub!(/:#{ext}/, "*")
-          else
-            query.gsub!(/:#{ext}/, "{#{candidates.compact.uniq.join(',')}}")
-          end
-        end
-
-        File.expand_path(query, @path)
-      end
-
       def escape_entry(entry)
         entry.gsub(/[*?{}\[\]]/, '\\\\\\&')
       end
@@ -311,45 +294,7 @@ def extract_handler_and_format_and_variant(path)
         # Template::Types[format] and handler.default_format can return nil
         [handler, format, variant]
       end
-  end
-
-  # A resolver that loads files from the filesystem.
-  class FileSystemResolver < PathResolver
-    attr_reader :path
-
-    def initialize(path)
-      raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver)
-      super()
-      @path = File.expand_path(path)
-    end
-
-    def to_s
-      @path.to_s
-    end
-    alias :to_path :to_s
 
-    def eql?(resolver)
-      self.class.equal?(resolver.class) && to_path == resolver.to_path
-    end
-    alias :== :eql?
-
-    def all_template_paths # :nodoc:
-      paths = Dir.glob("**/*", base: @path)
-      paths.reject do |filename|
-        File.directory?(File.join(@path, filename))
-      end.map do |filename|
-        filename.gsub(/\.[^\/]*\z/, "")
-      end.uniq
-    end
-  end
-
-  # An Optimized resolver for Rails' most common case.
-  class OptimizedFileSystemResolver < FileSystemResolver #:nodoc:
-    def initialize(path)
-      super(path)
-    end
-
-    private
       def find_candidate_template_paths(path)
         # Instead of checking for every possible path, as our other globs would
         # do, scan the directory for files with the right prefix.
@@ -362,8 +307,7 @@ def find_candidate_template_paths(path)
 
       def find_template_paths_from_details(path, details)
         if path.name.include?(".")
-          # Fall back to the unoptimized resolver, which will warn
-          return super
+          return []
         end
 
         candidates = find_candidate_template_paths(path)

actionview/lib/action_view/testing/resolvers.rb

@@ -7,7 +7,7 @@ module ActionView #:nodoc:
   # file system. This is used internally by Rails' own test suite, and is
   # useful for testing extensions that have no way of knowing what the file
   # system will look like at runtime.
-  class FixtureResolver < OptimizedFileSystemResolver
+  class FixtureResolver < FileSystemResolver
     def initialize(hash = {})
       super("")
       @hash = hash
@@ -36,10 +36,11 @@ def source_for_template(template)
       end
   end
 
-  class NullResolver < PathResolver
-    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)]
+  class NullResolver < Resolver
+    def find_templates(name, prefix, partial, details, locals = [])
+      path = Path.build(name, prefix, partial)
+      handler = ActionView::Template::Handlers::Raw
+      [ActionView::Template.new("Template generated by Null Resolver", path.virtual, handler, virtual_path: path.virtual, format: nil, variant: nil, locals: locals)]
     end
   end
 end

actionview/test/actionpack/controller/view_paths_test.rb

@@ -132,12 +132,10 @@ def test_view_paths_override_at_request_time
   end
 
   def test_decorate_view_paths_with_custom_resolver
-    decorator_class = Class.new(ActionView::PathResolver) do
+    decorator_class = Class.new(ActionView::Resolver) do
       def initialize(path_set)
         @path_set = path_set
-      end
-
-      def clear_cache
+        super()
       end
 
       def find_all(*args)

actionview/test/template/render_test.rb

@@ -331,13 +331,6 @@ def test_render_partial_collection
     assert_equal "Hello: davidHello: mary", @view.render(partial: "test/customer", collection: [ Customer.new("david"), Customer.new("mary") ])
   end
 
-  def test_render_partial_collection_with_partial_name_containing_dot
-    assert_deprecated do
-      assert_equal "Hello: davidHello: mary",
-        @view.render(partial: "test/customer.mobile", collection: [ Customer.new("david"), Customer.new("mary") ])
-    end
-  end
-
   def test_render_partial_collection_as_by_string
     assert_equal "david david davidmary mary mary",
       @view.render(partial: "test/customer_with_var", collection: [ Customer.new("david"), Customer.new("mary") ], as: "customer")
@@ -689,7 +682,7 @@ class CachedViewRenderTest < ActiveSupport::TestCase
   def setup
     ActionView::LookupContext::DetailsKey.clear
     view_paths = ActionController::Base.view_paths
-    assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class
+    assert_equal ActionView::FileSystemResolver, view_paths.first.class
     setup_view(view_paths)
   end
 
@@ -763,7 +756,7 @@ class CachedCustomer < Customer; end
     ActionView::LookupContext::DetailsKey.clear
 
     view_paths = ActionController::Base.view_paths
-    assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class
+    assert_equal ActionView::FileSystemResolver, view_paths.first.class
 
     ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new
 

actionview/test/template/resolver_shared_tests.rb

@@ -227,13 +227,9 @@ def test_templates_no_format_or_handler_with_variant
     assert_equal "mobile", templates[0].variant
   end
 
-  def test_virtual_path_is_preserved_with_dot
+  def test_returns_no_results_with_dot
     with_file "test/hello_world.html.erb", "Hello html!"
 
-    template = assert_deprecated { 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
+    assert_empty context.find_all("hello_world.html", "test", false, [], {})
   end
 end