Favor `public_folder: true` over `public_*`

Adding all those `public_*` methods is a bit heavy handed, we can change the API to instead  use `public_folder: true`. Change was pretty easy since it was already implemented that way.

3 files changed, 47 insertions, 169 deletions

diff --git a/actionview/lib/action_view/helpers/asset_tag_helper.rb b/actionview/lib/action_view/helpers/asset_tag_helper.rb
index 2e31bfbc9a..4ff2b8c872 100644
--- a/actionview/lib/action_view/helpers/asset_tag_helper.rb
+++ b/actionview/lib/action_view/helpers/asset_tag_helper.rb
@@ -193,13 +193,6 @@ module ActionView
         }.merge!(options.symbolize_keys))
       end
 
-      # Returns a link tag for a favicon asset in the public
-      # folder. This uses +favicon_link_tag+ and skips any asset
-      # lookups by assuming any assets are in the `public` folder.
-      def public_favicon_link_tag(source="favicon.ico", options={})
-        favicon_link_tag(source, { public_folder: true }.merge!(options))
-      end
-
       # Returns an HTML image tag for the +source+. The +source+ can be a full
       # path or a file.
       #
@@ -244,13 +237,6 @@ module ActionView
         tag("img", options)
       end
 
-      # Returns an HTML image tag for the source asset in the public
-      # folder. This uses +image_tag+ and skips any asset
-      # lookups by assuming any assets are in the `public` folder.
-      def public_image_tag(source, options={})
-        image_tag(source,  { public_folder: true }.merge!(options))
-      end
-
       # Returns a string suitable for an HTML image tag alt attribute.
       # The +src+ argument is meant to be an image file path.
       # The method removes the basename of the file path and the digest,
@@ -314,20 +300,12 @@ module ActionView
         options = sources.extract_options!.symbolize_keys
         public_poster_folder = options.delete(:public_poster_folder)
         sources << options
-        multiple_sources_tag("video", sources) do |options|
+        multiple_sources_tag_builder("video", sources) do |options|
           options[:poster] = path_to_image(options[:poster], public_folder: public_poster_folder) if options[:poster]
           options[:width], options[:height] = extract_dimensions(options.delete(:size)) if options[:size]
         end
       end
 
-      # Returns an HTML video tag for the source asset in the public
-      # folder. This uses +video_tag+ and skips any asset
-      # lookups by assuming any assets are in the `public` folder.
-      def public_video_tag(*sources)
-        options = sources.extract_options!
-        video_tag(*sources, { public_folder: true , public_poster_folder: true }.merge!(options))
-      end
-
       # Returns an HTML audio tag for the +source+.
       # The +source+ can be full path or file that exists in
       # your public audios directory.
@@ -341,18 +319,11 @@ module ActionView
       #   audio_tag("sound.wav", "sound.mid")
       #   # => <audio><source src="/audios/sound.wav" /><source src="/audios/sound.mid" /></audio>
       def audio_tag(*sources)
-        multiple_sources_tag("audio", sources)
-      end
-
-      # Returns an HTML audio tag for the source asset in the public
-      # folder. This uses +audio_tag+ and skips any asset
-      def public_audio_tag(*sources)
-        options = sources.extract_options!
-        audio_tag(*sources, { public_folder: true }.merge!(options))
+        multiple_sources_tag_builder("audio", sources)
       end
 
       private
-        def multiple_sources_tag(type, sources)
+        def multiple_sources_tag_builder(type, sources)
           options       = sources.extract_options!.symbolize_keys
           public_folder = options.delete(:public_folder)
           sources.flatten!
diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb
index a21536dd9d..50b13c0271 100644
--- a/actionview/lib/action_view/helpers/asset_url_helper.rb
+++ b/actionview/lib/action_view/helpers/asset_url_helper.rb
@@ -167,14 +167,6 @@ module ActionView
       end
       alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route
 
-      # Computes the path to an asset in the public folder.
-      # This uses +asset_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_asset_path(source, options = {})
-        path_to_asset(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_asset, :public_asset_path # aliased to avoid conflicts with an public_asset_path named route
-
       # Computes the full URL to an asset in the public directory. This
       # will use +asset_path+ internally, so most of their behaviors
       # will be the same. If :host options is set, it overwrites global
@@ -190,14 +182,6 @@ module ActionView
       end
       alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route
 
-      # Computes the full URL to an asset in the public folder.
-      # This uses +asset_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_asset_url(source, options = {})
-        url_to_asset(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_asset, :public_asset_path # aliased to avoid conflicts with a public_asset_path named route
-
       ASSET_EXTENSIONS = {
         javascript: ".js",
         stylesheet: ".css"
@@ -284,14 +268,6 @@ module ActionView
       end
       alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route
 
-      # Computes the path to a javascript asset in the public folder.
-      # This uses +javascript_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_javascript_path(source, options = {})
-        path_to_javascript(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_javascript, :public_javascript_path # aliased to avoid conflicts with a public_javascript_path named route
-
       # Computes the full URL to a JavaScript asset in the public javascripts directory.
       # This will use +javascript_path+ internally, so most of their behaviors will be the same.
       # Since +javascript_url+ is based on +asset_url+ method you can set :host options. If :host
@@ -304,14 +280,6 @@ module ActionView
       end
       alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route
 
-      # Computes the full URL to a javascript asset in the public folder.
-      # This uses +javascript_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_javascript_url(source, options = {})
-        url_to_javascript(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_javascript, :public_javascript_path # aliased to avoid conflicts with a public_javascript_path named route
-
       # Computes the path to a stylesheet asset in the public stylesheets directory.
       # If the +source+ filename has no extension, .css will be appended (except for explicit URIs).
       # Full paths from the document root will be passed through.
@@ -327,14 +295,6 @@ module ActionView
       end
       alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route
 
-      # Computes the path to a stylesheet asset in the public folder.
-      # This uses +stylesheet_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_stylesheet_path(source, options = {})
-        path_to_stylesheet(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_stylesheet, :public_stylesheet_path # aliased to avoid conflicts with a public_stylesheet_path named route
-
       # Computes the full URL to a stylesheet asset in the public stylesheets directory.
       # This will use +stylesheet_path+ internally, so most of their behaviors will be the same.
       # Since +stylesheet_url+ is based on +asset_url+ method you can set :host options. If :host
@@ -347,14 +307,6 @@ module ActionView
       end
       alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route
 
-      # Computes the full URL to a stylesheet asset in the public folder.
-      # This uses +stylesheet_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_stylesheet_url(source, options = {})
-        url_to_stylesheet(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_stylesheet, :public_stylesheet_path # aliased to avoid conflicts with a public_stylesheet_path named route
-
       # Computes the path to an image asset.
       # Full paths from the document root will be passed through.
       # Used internally by +image_tag+ to build the image path:
@@ -373,14 +325,6 @@ module ActionView
       end
       alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route
 
-      # Computes the path to a image asset in the public folder.
-      # This uses +image_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_image_path(source, options = {})
-        path_to_image(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_image, :public_image_path # aliased to avoid conflicts with a public_image_path named route
-
       # Computes the full URL to an image asset.
       # This will use +image_path+ internally, so most of their behaviors will be the same.
       # Since +image_url+ is based on +asset_url+ method you can set :host options. If :host
@@ -393,14 +337,6 @@ module ActionView
       end
       alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route
 
-      # Computes the full URL to a image asset in the public folder.
-      # This uses +image_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_image_url(source, options = {})
-        url_to_image(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_image, :public_image_path # aliased to avoid conflicts with a public_image_path named route
-
       # Computes the path to a video asset in the public videos directory.
       # Full paths from the document root will be passed through.
       # Used internally by +video_tag+ to build the video path.
@@ -415,14 +351,6 @@ module ActionView
       end
       alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route
 
-      # Computes the path to a video asset in the public folder.
-      # This uses +video_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_video_path(source, options = {})
-        path_to_video(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_video, :public_video_path # aliased to avoid conflicts with a public_video_path named route
-
       # Computes the full URL to a video asset in the public videos directory.
       # This will use +video_path+ internally, so most of their behaviors will be the same.
       # Since +video_url+ is based on +asset_url+ method you can set :host options. If :host
@@ -435,14 +363,6 @@ module ActionView
       end
       alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route
 
-      # Computes the full URL to a video asset in the public folder.
-      # This uses +video_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_video_url(source, options = {})
-        url_to_video(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_video, :public_video_path # aliased to avoid conflicts with a public_video_path named route
-
       # Computes the path to an audio asset in the public audios directory.
       # Full paths from the document root will be passed through.
       # Used internally by +audio_tag+ to build the audio path.
@@ -457,14 +377,6 @@ module ActionView
       end
       alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route
 
-      # Computes the path to a audio asset in the public folder.
-      # This uses +audio_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_audio_path(source, options = {})
-        path_to_audio(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_audio, :public_audio_path # aliased to avoid conflicts with a public_audio_path named route
-
       # Computes the full URL to an audio asset in the public audios directory.
       # This will use +audio_path+ internally, so most of their behaviors will be the same.
       # Since +audio_url+ is based on +asset_url+ method you can set :host options. If :host
@@ -477,14 +389,6 @@ module ActionView
       end
       alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route
 
-      # Computes the full URL to a audio asset in the public folder.
-      # This uses +audio_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_audio_url(source, options = {})
-        url_to_audio(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_audio, :public_audio_path # aliased to avoid conflicts with a public_audio_path named route
-
       # Computes the path to a font asset.
       # Full paths from the document root will be passed through.
       #
@@ -498,14 +402,6 @@ module ActionView
       end
       alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route
 
-      # Computes the path to a font asset in the public folder.
-      # This uses +font_path+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_font_path(source, options = {})
-        path_to_font(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_font, :public_font_path # aliased to avoid conflicts with a public_font_path named route
-
       # Computes the full URL to a font asset.
       # This will use +font_path+ internally, so most of their behaviors will be the same.
       # Since +font_url+ is based on +asset_url+ method you can set :host options. If :host
@@ -517,14 +413,6 @@ module ActionView
         url_to_asset(source, { type: :font }.merge!(options))
       end
       alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route
-
-      # Computes the full URL to a font asset in the public folder.
-      # This uses +font_url+ and skips any asset lookups by assuming the asset is in the
-      # `public` folder.
-      def public_font_url(source, options = {})
-        url_to_font(source, { public_folder: true }.merge!(options))
-      end
-      alias_method :path_to_public_font, :public_font_path # aliased to avoid conflicts with a public_font_url named route
     end
   end
 end
diff --git a/railties/test/application/asset_debugging_test.rb b/railties/test/application/asset_debugging_test.rb
index 1d27cecd3c..e6a660cf25 100644
--- a/railties/test/application/asset_debugging_test.rb
+++ b/railties/test/application/asset_debugging_test.rb
@@ -72,23 +72,23 @@ module ApplicationTests
     test "public path and tag methods are not over-written by the asset pipeline" do
       contents = "doesnotexist"
       cases = {
-        public_asset_path:             %r{/#{ contents }},
-        public_image_path:             %r{/images/#{ contents }},
-        public_video_path:             %r{/videos/#{ contents }},
-        public_audio_path:             %r{/audios/#{ contents }},
-        public_font_path:              %r{/fonts/#{ contents }},
-        public_javascript_path:        %r{/javascripts/#{ contents }},
-        public_stylesheet_path:        %r{/stylesheets/#{ contents }},
-        public_image_tag:              %r{<img src="/images/#{ contents }"},
-        public_favicon_link_tag:       %r{<link rel="shortcut icon" type="image/x-icon" href="/images/#{ contents }" />},
-        public_stylesheet_link_tag:    %r{<link rel="stylesheet" media="screen" href="/stylesheets/#{ contents }.css" />},
-        public_javascript_include_tag: %r{<script src="/javascripts/#{ contents }.js">},
-        public_audio_tag:              %r{<audio src="/audios/#{ contents }"></audio>},
-        public_video_tag:              %r{<video src="/videos/#{ contents }"></video>}
+        asset_path:             %r{/#{ contents }},
+        image_path:             %r{/images/#{ contents }},
+        video_path:             %r{/videos/#{ contents }},
+        audio_path:             %r{/audios/#{ contents }},
+        font_path:              %r{/fonts/#{ contents }},
+        javascript_path:        %r{/javascripts/#{ contents }},
+        stylesheet_path:        %r{/stylesheets/#{ contents }},
+        image_tag:              %r{<img src="/images/#{ contents }"},
+        favicon_link_tag:       %r{<link rel="shortcut icon" type="image/x-icon" href="/images/#{ contents }" />},
+        stylesheet_link_tag:    %r{<link rel="stylesheet" media="screen" href="/stylesheets/#{ contents }.css" />},
+        javascript_include_tag: %r{<script src="/javascripts/#{ contents }.js">},
+        audio_tag:              %r{<audio src="/audios/#{ contents }"></audio>},
+        video_tag:              %r{<video src="/videos/#{ contents }"></video>}
       }
 
       cases.each do |(view_method, tag_match)|
-        app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}' %>"
+        app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}', public_folder: true %>"
 
         app "development"
 
@@ -104,17 +104,17 @@ module ApplicationTests
     test "public url methods are not over-written by the asset pipeline" do
       contents = "doesnotexist"
       cases = {
-        public_asset_url:       %r{http://example.org/#{ contents }},
-        public_image_url:       %r{http://example.org/images/#{ contents }},
-        public_video_url:       %r{http://example.org/videos/#{ contents }},
-        public_audio_url:       %r{http://example.org/audios/#{ contents }},
-        public_font_url:        %r{http://example.org/fonts/#{ contents }},
-        public_javascript_url:  %r{http://example.org/javascripts/#{ contents }},
-        public_stylesheet_url:  %r{http://example.org/stylesheets/#{ contents }},
+        asset_url:       %r{http://example.org/#{ contents }},
+        image_url:       %r{http://example.org/images/#{ contents }},
+        video_url:       %r{http://example.org/videos/#{ contents }},
+        audio_url:       %r{http://example.org/audios/#{ contents }},
+        font_url:        %r{http://example.org/fonts/#{ contents }},
+        javascript_url:  %r{http://example.org/javascripts/#{ contents }},
+        stylesheet_url:  %r{http://example.org/stylesheets/#{ contents }},
       }
 
       cases.each do |(view_method, tag_match)|
-        app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}' %>"
+        app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}', public_folder: true %>"
 
         app "development"
 
@@ -127,10 +127,29 @@ module ApplicationTests
       end
     end
 
-    test "public path methods do not use the asset pipeline" do
+    test "{ public_folder: true } does not use the asset pipeline" do
       cases = {
-        asset_path:        /\/assets\/application-.*.\.js/,
-        public_asset_path: /application.js/,
+        /\/assets\/application-.*.\.js/ => {},
+        /application.js/                => { public_folder: true},
+      }
+      cases.each do |(tag_match, options_hash)|
+        app_file "app/views/posts/index.html.erb", "<%= asset_path('application.js', #{ options_hash }) %>"
+
+        app "development"
+
+        class ::PostsController < ActionController::Base ; end
+
+        get "/posts?debug_assets=true"
+
+        body = last_response.body.strip
+        assert_match(tag_match, body, "Expected `asset_path` with `#{ options_hash}` to produce a match to #{ tag_match }, but did not: #{ body }")
+      end
+    end
+
+    test "public_compute_asset_path does not use the asset pipeline" do
+      cases = {
+        compute_asset_path:        /\/assets\/application-.*.\.js/,
+        public_compute_asset_path: /application.js/,
       }
 
       cases.each do |(view_method, tag_match)|