From e53c45b80c61e1a03b0c86efe116adbc4e91d07f Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@hawthorn.email>
Date: Mon, 30 Sep 2019 22:43:59 -0700
Subject: [PATCH] Introduce Resolver::PathParser

The template resolver is supposed to determine the handler, format, and
variant from the path in extract_handler_and_format_and_variant.

Previously this behaviour was close but didn't exactly match the
behaviour of finding templates, and in some cases (particularly with
handlers or formats missing) would return incorrect results.

This commit introduces Resolver::PathParser, a class which should be
able to accurately from any path inside a view directory be able to tell
us exactly the prefix, partial, variant, locale, format, variant, and
handler of that template.

This works by building a building a regexp from the known handlers and
file types. This requires that any resolvers have their cache cleared
when new handlers or types are registered (this was already somewhat the
requirement, since resolver lookups are cached, but this makes it
necessary in more situations).
---
 .../lib/action_view/template/resolver.rb      | 55 +++++++++++++++----
 .../test/actionpack/controller/layout_test.rb |  4 ++
 .../test/template/resolver_shared_tests.rb    |  4 --
 3 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb
index 0f79f6febe..99705c4784 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -35,6 +35,44 @@ module ActionView
       alias :to_s :to_str
     end
 
+    class PathParser
+      def initialize
+        @regex = build_path_regex
+      end
+
+      def build_path_regex
+        handlers = Template::Handlers.extensions.map { |x| Regexp.escape(x) }.join("|")
+        formats = Template::Types.symbols.map { |x| Regexp.escape(x) }.join("|")
+        locales = "[a-z]{2}(?:-[A-Z]{2})?"
+        variants = "[^.]*"
+
+        %r{
+          \A
+          (?:(?<prefix>.*)/)?
+          (?<partial>_)?
+          (?<action>.*?)
+          (?:\.(?<locale>#{locales}))??
+          (?:\.(?<format>#{formats}))??
+          (?:\+(?<variant>#{variants}))??
+          (?:\.(?<handler>#{handlers}))?
+          \z
+        }x
+      end
+
+      def parse(path)
+        match = @regex.match(path)
+        {
+          prefix: match[:prefix] || "",
+          action: match[:action],
+          partial: !!match[:partial],
+          locale: match[:locale]&.to_sym,
+          handler: match[:handler]&.to_sym,
+          format: match[:format]&.to_sym,
+          variant: match[:variant]
+        }
+      end
+    end
+
     # Threadsafe template cache
     class Cache #:nodoc:
       class SmallCache < Concurrent::Map
@@ -172,11 +210,13 @@ module ActionView
         @pattern = DEFAULT_PATTERN
       end
       @unbound_templates = Concurrent::Map.new
+      @path_parser = PathParser.new
       super()
     end
 
     def clear_cache
       @unbound_templates.clear
+      @path_parser = PathParser.new
       super()
     end
 
@@ -278,18 +318,11 @@ module ActionView
       # from the path, or the handler, we should return the array of formats given
       # to the resolver.
       def extract_handler_and_format_and_variant(path)
-        pieces = File.basename(path).split(".")
-        pieces.shift
+        details = @path_parser.parse(path)
 
-        extension = pieces.pop
-
-        handler = Template.handler_for_extension(extension)
-        format, variant = pieces.last.split(EXTENSIONS[:variants], 2) if pieces.last
-        format = if format
-          Template::Types[format]&.ref
-        elsif handler.respond_to?(:default_format) # default_format can return nil
-          handler.default_format
-        end
+        handler = Template.handler_for_extension(details[:handler])
+        format = details[:format] || handler.try(:default_format)
+        variant = details[:variant]
 
         # Template::Types[format] and handler.default_format can return nil
         [handler, format, variant]
diff --git a/actionview/test/actionpack/controller/layout_test.rb b/actionview/test/actionpack/controller/layout_test.rb
index f946e4360d..0b2627c290 100644
--- a/actionview/test/actionpack/controller/layout_test.rb
+++ b/actionview/test/actionpack/controller/layout_test.rb
@@ -18,9 +18,13 @@ end
 module TemplateHandlerHelper
   def with_template_handler(*extensions, handler)
     ActionView::Template.register_template_handler(*extensions, handler)
+    ActionController::Base.view_paths.paths.each(&:clear_cache)
+    ActionView::LookupContext::DetailsKey.clear
     yield
   ensure
     ActionView::Template.unregister_template_handler(*extensions)
+    ActionController::Base.view_paths.paths.each(&:clear_cache)
+    ActionView::LookupContext::DetailsKey.clear
   end
 end
 
diff --git a/actionview/test/template/resolver_shared_tests.rb b/actionview/test/template/resolver_shared_tests.rb
index 4f89420ed8..f513cca0bb 100644
--- a/actionview/test/template/resolver_shared_tests.rb
+++ b/actionview/test/template/resolver_shared_tests.rb
@@ -204,8 +204,6 @@ module ResolverSharedTests
   end
 
   def test_templates_no_format_with_variant
-    return skip
-
     with_file "test/hello_world+mobile.erb", "Hello HTML!"
 
     templates = resolver.find_all("hello_world", "test", false, locale: [], formats: [:html, :json], variants: :any, handlers: [:erb, :builder])
@@ -218,8 +216,6 @@ module ResolverSharedTests
   end
 
   def test_templates_no_format_or_handler_with_variant
-    return skip
-
     with_file "test/hello_world+mobile", "Hello HTML!"
 
     templates = resolver.find_all("hello_world", "test", false, locale: [], formats: [:html, :json], variants: :any, handlers: [:erb, :builder])