From 614e813161ad8f6e13ad19aedd577acdd976d9d6 Mon Sep 17 00:00:00 2001
From: Jonathan Hefner <jonathan@hefner.pro>
Date: Fri, 11 Sep 2020 11:43:27 -0500
Subject: [PATCH] Disentangle Action Text from ApplicationController

This commit allows Action Text to be used without having an
ApplicationController defined.  In doing so, it also fixes Action Text
attachments to render the correct URL host in mailers.

It also avoids allocating an ActionController::Renderer per request.

Fixes #37183.
Fixes #35578.
Fixes #36963.
Closes #38714.

Co-authored-by: Jeremy Daer <jeremydaer@gmail.com>
---
 actionpack/lib/action_controller/renderer.rb  |  1 +
 .../app/helpers/action_text/content_helper.rb |  1 +
 actiontext/lib/action_text.rb                 |  1 +
 .../attachments/trix_conversion.rb            |  2 +-
 actiontext/lib/action_text/content.rb         |  8 ++---
 actiontext/lib/action_text/engine.rb          | 21 ++++++++------
 actiontext/lib/action_text/rendering.rb       | 29 +++++++++++++++++++
 .../controllers/admin/messages_controller.rb  |  5 ++++
 .../app/controllers/application_controller.rb |  2 --
 .../app/controllers/messages_controller.rb    |  2 +-
 .../test/dummy/app/mailers/messages_mailer.rb |  6 ++++
 .../app/views/admin/messages/show.html.erb    | 16 ++++++++++
 .../app/views/layouts/application.html.erb    |  3 --
 .../dummy/app/views/messages/show.html.erb    |  9 ++----
 .../messages_mailer/notification.html.erb     |  1 +
 actiontext/test/dummy/config/routes.rb        |  5 +++-
 .../integration/controller_render_test.rb     | 24 +++++++++++++++
 .../test/integration/mailer_render_test.rb    | 23 +++++++++++++++
 actiontext/test/unit/content_test.rb          |  9 ++++++
 actionview/lib/action_view/base.rb            |  2 +-
 20 files changed, 139 insertions(+), 31 deletions(-)
 create mode 100644 actiontext/lib/action_text/rendering.rb
 create mode 100644 actiontext/test/dummy/app/controllers/admin/messages_controller.rb
 delete mode 100644 actiontext/test/dummy/app/controllers/application_controller.rb
 create mode 100644 actiontext/test/dummy/app/mailers/messages_mailer.rb
 create mode 100644 actiontext/test/dummy/app/views/admin/messages/show.html.erb
 create mode 100644 actiontext/test/dummy/app/views/messages_mailer/notification.html.erb
 create mode 100644 actiontext/test/integration/controller_render_test.rb
 create mode 100644 actiontext/test/integration/mailer_render_test.rb

diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb
index d91c40c054..5a43e3b3f9 100644
--- a/actionpack/lib/action_controller/renderer.rb
+++ b/actionpack/lib/action_controller/renderer.rb
@@ -99,6 +99,7 @@ module ActionController
       instance.set_response! controller.make_response!(request)
       instance.render_to_string(*args)
     end
+    alias_method :render_to_string, :render # :nodoc:
 
     private
       def normalize_keys(defaults, env)
diff --git a/actiontext/app/helpers/action_text/content_helper.rb b/actiontext/app/helpers/action_text/content_helper.rb
index 17b50fc556..0d0bb19de2 100644
--- a/actiontext/app/helpers/action_text/content_helper.rb
+++ b/actiontext/app/helpers/action_text/content_helper.rb
@@ -10,6 +10,7 @@ module ActionText
     mattr_accessor(:scrubber)
 
     def render_action_text_content(content)
+      self.prefix_partial_path_with_controller_namespace = false
       sanitize_action_text_content(render_action_text_attachments(content))
     end
 
diff --git a/actiontext/lib/action_text.rb b/actiontext/lib/action_text.rb
index 0dd6318f89..15fdf70b3b 100644
--- a/actiontext/lib/action_text.rb
+++ b/actiontext/lib/action_text.rb
@@ -16,6 +16,7 @@ module ActionText
   autoload :Fragment
   autoload :HtmlConversion
   autoload :PlainTextConversion
+  autoload :Rendering
   autoload :Serialization
   autoload :TrixAttachment
 
diff --git a/actiontext/lib/action_text/attachments/trix_conversion.rb b/actiontext/lib/action_text/attachments/trix_conversion.rb
index 15319f4e37..2829da1f59 100644
--- a/actiontext/lib/action_text/attachments/trix_conversion.rb
+++ b/actiontext/lib/action_text/attachments/trix_conversion.rb
@@ -28,7 +28,7 @@ module ActionText
       private
         def trix_attachment_content
           if partial_path = attachable.try(:to_trix_content_attachment_partial_path)
-            ActionText::Content.renderer.render(partial: partial_path, object: self, as: model_name.element)
+            ActionText::Content.render(partial: partial_path, object: self, as: model_name.element)
           end
         end
     end
diff --git a/actiontext/lib/action_text/content.rb b/actiontext/lib/action_text/content.rb
index 16bc6fe031..6126a4de49 100644
--- a/actiontext/lib/action_text/content.rb
+++ b/actiontext/lib/action_text/content.rb
@@ -1,12 +1,8 @@
 # frozen_string_literal: true
 
-require "active_support/core_ext/module/attribute_accessors_per_thread"
-
 module ActionText
   class Content
-    include Serialization
-
-    thread_cattr_accessor :renderer
+    include Rendering, Serialization
 
     attr_reader :fragment
 
@@ -88,7 +84,7 @@ module ActionText
     end
 
     def to_rendered_html_with_layout
-      renderer.render(partial: "action_text/content/layout", locals: { content: self })
+      render partial: "action_text/content/layout", locals: { content: self }
     end
 
     def to_s
diff --git a/actiontext/lib/action_text/engine.rb b/actiontext/lib/action_text/engine.rb
index 8749498158..9d5fd1dd25 100644
--- a/actiontext/lib/action_text/engine.rb
+++ b/actiontext/lib/action_text/engine.rb
@@ -37,21 +37,24 @@ module ActionText
     end
 
     initializer "action_text.helper" do
-      ActiveSupport.on_load(:action_controller_base) do
-        helper ActionText::Engine.helpers
+      %i[action_controller_base action_mailer].each do |abstract_controller|
+        ActiveSupport.on_load(abstract_controller) do
+          helper ActionText::Engine.helpers
+        end
       end
     end
 
-    initializer "action_text.renderer" do |app|
-      app.executor.to_run      { ActionText::Content.renderer = ApplicationController.renderer }
-      app.executor.to_complete { ActionText::Content.renderer = ApplicationController.renderer }
-
+    initializer "action_text.renderer" do
       ActiveSupport.on_load(:action_text_content) do
-        self.renderer = ApplicationController.renderer
+        self.renderer = Class.new(ActionController::Base).renderer
       end
 
-      ActiveSupport.on_load(:action_controller_base) do
-        before_action { ActionText::Content.renderer = ApplicationController.renderer.new(request.env) }
+      %i[action_controller_base action_mailer].each do |abstract_controller|
+        ActiveSupport.on_load(abstract_controller) do
+          around_action do |controller, action|
+            ActionText::Content.with_renderer(controller, &action)
+          end
+        end
       end
     end
 
diff --git a/actiontext/lib/action_text/rendering.rb b/actiontext/lib/action_text/rendering.rb
new file mode 100644
index 0000000000..caef71c6b4
--- /dev/null
+++ b/actiontext/lib/action_text/rendering.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require "active_support/concern"
+require "active_support/core_ext/module/attribute_accessors_per_thread"
+
+module ActionText
+  module Rendering #:nodoc:
+    extend ActiveSupport::Concern
+
+    included do
+      thread_cattr_accessor :renderer, instance_accessor: false
+      delegate :render, to: :class
+    end
+
+    class_methods do
+      def with_renderer(renderer)
+        previous_renderer = self.renderer
+        self.renderer = renderer
+        yield
+      ensure
+        self.renderer = previous_renderer
+      end
+
+      def render(*args, &block)
+        renderer.render_to_string(*args, &block)
+      end
+    end
+  end
+end
diff --git a/actiontext/test/dummy/app/controllers/admin/messages_controller.rb b/actiontext/test/dummy/app/controllers/admin/messages_controller.rb
new file mode 100644
index 0000000000..7c3316324b
--- /dev/null
+++ b/actiontext/test/dummy/app/controllers/admin/messages_controller.rb
@@ -0,0 +1,5 @@
+class Admin::MessagesController < ActionController::Base
+  def show
+    @message = Message.find(params[:id])
+  end
+end
diff --git a/actiontext/test/dummy/app/controllers/application_controller.rb b/actiontext/test/dummy/app/controllers/application_controller.rb
deleted file mode 100644
index 09705d12ab..0000000000
--- a/actiontext/test/dummy/app/controllers/application_controller.rb
+++ /dev/null
@@ -1,2 +0,0 @@
-class ApplicationController < ActionController::Base
-end
diff --git a/actiontext/test/dummy/app/controllers/messages_controller.rb b/actiontext/test/dummy/app/controllers/messages_controller.rb
index ec85acb10d..b323d50a27 100644
--- a/actiontext/test/dummy/app/controllers/messages_controller.rb
+++ b/actiontext/test/dummy/app/controllers/messages_controller.rb
@@ -1,4 +1,4 @@
-class MessagesController < ApplicationController
+class MessagesController < ActionController::Base
   before_action :set_message, only: [:show, :edit, :update, :destroy]
 
   # GET /messages
diff --git a/actiontext/test/dummy/app/mailers/messages_mailer.rb b/actiontext/test/dummy/app/mailers/messages_mailer.rb
new file mode 100644
index 0000000000..ce0af3b3ca
--- /dev/null
+++ b/actiontext/test/dummy/app/mailers/messages_mailer.rb
@@ -0,0 +1,6 @@
+class MessagesMailer < ApplicationMailer
+  def notification
+    @message = params[:message]
+    mail to: params[:recipient], subject: "NEW MESSAGE: #{@message.subject}"
+  end
+end
diff --git a/actiontext/test/dummy/app/views/admin/messages/show.html.erb b/actiontext/test/dummy/app/views/admin/messages/show.html.erb
new file mode 100644
index 0000000000..ba9067f201
--- /dev/null
+++ b/actiontext/test/dummy/app/views/admin/messages/show.html.erb
@@ -0,0 +1,16 @@
+<dl>
+  <dt>Subject</dt>
+  <dd>
+    <span id="subject"><%= @message.subject %></span>
+  </dd>
+
+  <dt>Content (Plain Text)</dt>
+  <dd>
+    <pre id="content-plain"><%= @message.content.to_plain_text %></pre>
+  </dd>
+
+  <dt>Content (HTML)</dt>
+  <dd>
+    <div id="content-html"><%= @message.content %></div>
+  </dd>
+</dl>
diff --git a/actiontext/test/dummy/app/views/layouts/application.html.erb b/actiontext/test/dummy/app/views/layouts/application.html.erb
index f221f512b7..281e4e3063 100644
--- a/actiontext/test/dummy/app/views/layouts/application.html.erb
+++ b/actiontext/test/dummy/app/views/layouts/application.html.erb
@@ -3,9 +3,6 @@
   <head>
     <title>Dummy</title>
     <%= csrf_meta_tags %>
-
-    <%= stylesheet_link_tag 'application', media: 'all' %>
-    <%= javascript_pack_tag 'application' %>
   </head>
 
   <body>
diff --git a/actiontext/test/dummy/app/views/messages/show.html.erb b/actiontext/test/dummy/app/views/messages/show.html.erb
index 25fad1efba..50fa953421 100644
--- a/actiontext/test/dummy/app/views/messages/show.html.erb
+++ b/actiontext/test/dummy/app/views/messages/show.html.erb
@@ -1,13 +1,8 @@
 <p id="notice"><%= notice %></p>
 
-<p>
-  <strong>Subject:</strong>
-  <%= @message.subject %>
-</p>
+<h1 id="subject"><%= @message.subject %></h1>
 
-<div class="trix-content">
-  <%= @message.content.html_safe %>
-</div>
+<div id="content"><%= @message.content %></div>
 
 <%= link_to 'Edit', edit_message_path(@message) %> |
 <%= link_to 'Back', messages_path %>
diff --git a/actiontext/test/dummy/app/views/messages_mailer/notification.html.erb b/actiontext/test/dummy/app/views/messages_mailer/notification.html.erb
new file mode 100644
index 0000000000..e65604cecf
--- /dev/null
+++ b/actiontext/test/dummy/app/views/messages_mailer/notification.html.erb
@@ -0,0 +1 @@
+<div id="message-content"><%= @message.content %></div>
diff --git a/actiontext/test/dummy/config/routes.rb b/actiontext/test/dummy/config/routes.rb
index 1fc667e242..e7b45701e1 100644
--- a/actiontext/test/dummy/config/routes.rb
+++ b/actiontext/test/dummy/config/routes.rb
@@ -1,4 +1,7 @@
 Rails.application.routes.draw do
   resources :messages
-  # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html
+
+  namespace :admin do
+    resources :messages, only: [:show]
+  end
 end
diff --git a/actiontext/test/integration/controller_render_test.rb b/actiontext/test/integration/controller_render_test.rb
new file mode 100644
index 0000000000..2e8812ed88
--- /dev/null
+++ b/actiontext/test/integration/controller_render_test.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class ActionText::ControllerRenderTest < ActionDispatch::IntegrationTest
+  test "uses current request environment" do
+    blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg")
+    message = Message.create!(content: ActionText::Content.new.append_attachables(blob))
+
+    host! "loocalhoost"
+    get message_path(message)
+    assert_select "#content img" do |imgs|
+      imgs.each { |img| assert_match %r"//loocalhoost/", img["src"] }
+    end
+  end
+
+  test "resolves partials when controller is namespaced" do
+    blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg")
+    message = Message.create!(content: ActionText::Content.new.append_attachables(blob))
+
+    get admin_message_path(message)
+    assert_select "#content-html .trix-content .attachment--jpg"
+  end
+end
diff --git a/actiontext/test/integration/mailer_render_test.rb b/actiontext/test/integration/mailer_render_test.rb
new file mode 100644
index 0000000000..e787379445
--- /dev/null
+++ b/actiontext/test/integration/mailer_render_test.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class ActionText::MailerRenderTest < ActionMailer::TestCase
+  test "uses default_url_options" do
+    original_default_url_options = ActionMailer::Base.default_url_options
+    ActionMailer::Base.default_url_options = { host: "hoost" }
+
+    blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpg")
+    message = Message.new(content: ActionText::Content.new.append_attachables(blob))
+
+    MessagesMailer.with(recipient: "test", message: message).notification.deliver_now
+
+    assert_select_email do
+      assert_select "#message-content img" do |imgs|
+        imgs.each { |img| assert_match %r"//hoost/", img["src"] }
+      end
+    end
+  ensure
+    ActionMailer::Base.default_url_options = original_default_url_options
+  end
+end
diff --git a/actiontext/test/unit/content_test.rb b/actiontext/test/unit/content_test.rb
index f77f48df14..c00ad181a5 100644
--- a/actiontext/test/unit/content_test.rb
+++ b/actiontext/test/unit/content_test.rb
@@ -107,6 +107,15 @@ class ActionText::ContentTest < ActiveSupport::TestCase
     assert_equal "<blockquote><div>#{attachment_html}</div></blockquote>", content_from_html(html).to_html
   end
 
+  test "renders with layout when ApplicationController is not defined" do
+    html = "<h1>Hello world</h1>"
+    rendered = content_from_html(html).to_rendered_html_with_layout
+
+    assert_includes rendered, html
+    assert_match %r/\A#{Regexp.escape '<div class="trix-content">'}/, rendered
+    assert_not defined?(::ApplicationController)
+  end
+
   private
     def content_from_html(html)
       ActionText::Content.new(html).tap do |content|
diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb
index 9ed1f5898c..d06c1287ef 100644
--- a/actionview/lib/action_view/base.rb
+++ b/actionview/lib/action_view/base.rb
@@ -151,7 +151,7 @@ module ActionView #:nodoc:
     # Specify whether rendering within namespaced controllers should prefix
     # the partial paths for ActiveModel objects with the namespace.
     # (e.g., an Admin::PostsController would render @post using /admin/posts/_post.erb)
-    cattr_accessor :prefix_partial_path_with_controller_namespace, default: true
+    class_attribute :prefix_partial_path_with_controller_namespace, default: true
 
     # Specify default_formats that can be rendered.
     cattr_accessor :default_formats