Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inheritance #1

Merged
merged 10 commits into from
Sep 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 17 additions & 12 deletions lib/wicked_pdf/pdf_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,28 @@ def self.prepended(base)

base.class_eval do
after_action :clean_temp_files
end
end

alias_method :render_without_wicked_pdf, :render
alias_method :render_to_string_without_wicked_pdf, :render_to_string

def render(options = nil, *args, &block)
render_with_wicked_pdf(options, *args, &block)
end
def render(options = nil, *args, &block)
render_with_wicked_pdf(options, *args, &block)
end

def render_to_string(options = nil, *args, &block)
render_to_string_with_wicked_pdf(options, *args, &block)
end
end
def render_to_string(options = nil, *args, &block)
render_to_string_with_wicked_pdf(options, *args, &block)
end

def render_with_wicked_pdf(options = nil, *args, &block)
if options.is_a?(Hash) && options.key?(:pdf)
log_pdf_creation
options[:basic_auth] = set_basic_auth(options)
make_and_send_pdf(options.delete(:pdf), (WickedPdf.config || {}).merge(options))
else
elsif respond_to?(:render_without_wicked_pdf)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. What determines whether this returns true or false?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If PdfHelper is included, this method gets created by alias_method_chain. If its prepended, this method does not exist

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah jeez, I hadn't seen their railtie.rb. Makes sense now.

# support alias_method_chain (module included)
render_without_wicked_pdf(options, *args, &block)
else
# support inheritance (module prepended)
method(:render).super_method.call(options, *args, &block)
end
end

Expand All @@ -49,8 +50,12 @@ def render_to_string_with_wicked_pdf(options = nil, *args, &block)
options[:basic_auth] = set_basic_auth(options)
options.delete :pdf
make_pdf((WickedPdf.config || {}).merge(options))
else
elsif respond_to?(:render_to_string_without_wicked_pdf)
# support alias_method_chain (module included)
render_to_string_without_wicked_pdf(options, *args, &block)
else
# support inheritance (module prepended)
method(:render_to_string).super_method.call(options, *args, &block)
end
end

Expand Down
63 changes: 63 additions & 0 deletions test/functional/pdf_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,29 @@ def render_to_string(opts = {})
end
end

module ActionControllerMock
class Base
def render(_)
[:base]
end

def render_to_string
end

def self.after_action(_)
end
end
end

class PdfHelperTest < ActionController::TestCase
module SomePatch
def render(_)
super.tap do |s|
s << :patched
end
end
end

def setup
@ac = ActionController::Base.new
end
Expand All @@ -22,4 +44,45 @@ def teardown
:header => { :html => { :template => 'hf.html.erb' } })
assert_match %r{^file:\/\/\/.*wicked_header_pdf.*\.html}, options[:header][:html][:url]
end

test 'should not interfere with already prepended patches' do
# Emulate railtie
if Rails::VERSION::MAJOR >= 5
# this spec tests the following:
# if another gem prepends a render method to ActionController::Base
# before wicked_pdf does, does calling render trigger an infinite loop?
# this spec fails with 6392bea1fe3a41682dfd7c20fd9c179b5a758f59 because PdfHelper
# aliases the render method prepended by the other gem to render_without_pdf, then
# base_evals its own definition of render, which calls render_with_pdf -> render_without_pdf.
# If the other gem uses the prepend inhertinance pattern (calling super instead of aliasing),
# when it calls super it calls the base_eval'd version of render instead of going up the
# inheritance chain, causing an infinite loop.

# This fiddling with consts is required to get around the fact that PdfHelper checks
# that it is being prepended to ActionController::Base
OriginalBase = ActionController::Base
ActionController.send(:remove_const, :Base)
ActionController.const_set(:Base, ActionControllerMock::Base)

# Emulate another gem being loaded before wicked
ActionController::Base.prepend(SomePatch)
ActionController::Base.prepend(::WickedPdf::PdfHelper)

begin
# test that wicked's render method is actually called
ac = ActionController::Base.new
ac.expects(:render_with_wicked_pdf)
ac.render(:cats)

# test that calling render does not trigger infinite loop
ac = ActionController::Base.new
assert_equal [:base, :patched], ac.render(:cats)
rescue SystemStackError
assert_equal true, false # force spec failure
ensure
ActionController.send(:remove_const, :Base)
ActionController.const_set(:Base, OriginalBase)
end
end
end
end