Merge pull request #38569 from jhawthorn/simplify_fixture_resolver

Simplify fixture resolver

actionview/lib/action_view/template/resolver.rb

@@ -204,9 +204,13 @@ module ActionView
         end
       end
 
+      def source_for_template(template)
+        Template::Sources::File.new(template)
+      end
+
       def build_unbound_template(template, virtual_path)
         handler, format, variant = extract_handler_and_format_and_variant(template)
-        source = Template::Sources::File.new(template)
+        source = source_for_template(template)
 
         UnboundTemplate.new(
           source,
@@ -316,14 +320,22 @@ module ActionView
     end
 
     private
-      def find_template_paths_from_details(path, details)
+      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.
         query = "#{escape_entry(File.join(@path, path))}*"
 
+        Dir[query].reject do |filename|
+          File.directory?(filename)
+        end
+      end
+
+      def find_template_paths_from_details(path, details)
+        candidates = find_candidate_template_paths(path)
+
         regex = build_regex(path, details)
 
-        Dir[query].uniq.reject do |filename|
+        candidates.uniq.reject do |filename|
           # This regex match does double duty of finding only files which match
           # details (instead of just matching the prefix) and also filtering for
           # case-insensitive file systems.

actionview/lib/action_view/testing/resolvers.rb

@@ -27,34 +27,17 @@ module ActionView #:nodoc:
     end
 
     private
-      def query(path, exts, _, locals, cache:)
-        regex = build_regex(path, exts)
-
-        @hash.select do |_path, _|
-          ("/" + _path).match?(regex)
-        end.map do |_path, source|
-          handler, format, variant = extract_handler_and_format_and_variant(_path)
-
-          Template.new(source, _path, handler,
-            virtual_path: path.virtual,
-            format: format,
-            variant: variant,
-            locals: locals
-          )
-        end.sort_by do |t|
-          match = ("/" + t.identifier).match(regex)
-          EXTENSIONS.keys.reverse.map do |ext|
-            if ext == :variants && exts[ext] == :any
-              match[ext].nil? ? 0 : 1
-            elsif match[ext].nil?
-              exts[ext].length
-            else
-              found = match[ext].to_sym
-              exts[ext].index(found)
-            end
-          end
+      def find_candidate_template_paths(path)
+        @hash.keys.select do |fixture|
+          fixture.start_with?(path.virtual)
+        end.map do |fixture|
+          "/#{fixture}"
         end
       end
+
+      def source_for_template(template)
+        @hash[template[1..template.size]]
+      end
   end
 
   class NullResolver < PathResolver

actionview/test/template/lookup_context_test.rb

@@ -190,33 +190,30 @@ class LookupContextTest < ActiveSupport::TestCase
     assert_equal 3, keys.uniq.size
   end
 
-  test "gives the key forward to the resolver, so it can be used as cache key" do
-    @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {})
+  test "uses details as part of cache key" do
+    fixtures = {
+      "test/_foo.erb" => "Foo",
+      "test/_foo.da.erb" => "Bar",
+    }
+    @lookup_context = build_lookup_context(ActionView::FixtureResolver.new(fixtures), {})
+
     template = @lookup_context.find("foo", %w(test), true)
+    original_template = template
     assert_equal "Foo", template.source
 
-    # Now we are going to change the template, but it won't change the returned template
-    # since we will hit the cache.
-    @lookup_context.view_paths.first.data["test/_foo.erb"] = "Bar"
+    # We should get the same template
     template = @lookup_context.find("foo", %w(test), true)
-    assert_equal "Foo", template.source
+    assert_same original_template, template
 
-    # This time we will change the locale. The updated template should be picked since
-    # lookup_context generated a new key after we changed the locale.
+    # Using a different locale we get a different view
     @lookup_context.locale = :da
     template = @lookup_context.find("foo", %w(test), true)
     assert_equal "Bar", template.source
 
-    # Now we will change back the locale and it will still pick the old template.
-    # This is expected because lookup_context will reuse the previous key for :en locale.
+    # Using en we get the original view
     @lookup_context.locale = :en
     template = @lookup_context.find("foo", %w(test), true)
-    assert_equal "Foo", template.source
-
-    # Finally, we can expire the cache. And the expected template will be used.
-    @lookup_context.view_paths.first.clear_cache
-    template = @lookup_context.find("foo", %w(test), true)
-    assert_equal "Bar", template.source
+    assert_same original_template, template
   end
 
   test "can disable the cache on demand" do