Skip to content

Delegate properly to custom Resolver instances #1580

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

Merged
merged 3 commits into from
Mar 28, 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
5 changes: 5 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ else
gem 'rake', '~> 10.0' # rake 11 requires Ruby 1.9.3 or later
end

# Version 3 of mime-types 3 requires Ruby 2.0
if RUBY_VERSION < '2.0.0'
gem 'mime-types', '< 3'
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://travis-ci.org/rspec/rspec-rails/builds/118239747 failed to bundle because v2.6.4 of mail is out, which allows bundling with mime-types < 4 instead of, as earlier, mime-types < 3.


# Capybara versions that support RSpec 3 only support RUBY_VERSION >= 1.9.3
if RUBY_VERSION >= '1.9.3'
gem 'capybara', '~> 2.2.0', :require => false
Expand Down
1 change: 1 addition & 0 deletions example_app_generator/app/views/foo.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Static template named 'foo.html'
1 change: 1 addition & 0 deletions example_app_generator/app/views/some_templates/bar.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Static template named 'bar.html'
5 changes: 5 additions & 0 deletions example_app_generator/generate_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
| gem 'rake', '~> 10.0' # rake 11 requires Ruby 1.9.3 or later
|end
|
|# Version 3 of mime-types 3 requires Ruby 2.0
|if RUBY_VERSION < '2.0.0'
| gem 'mime-types', '< 3'
|end
|
|gem 'rspec-rails',
| :path => '#{rspec_rails_repo_path}',
| :groups => [:development, :test]
Expand Down
3 changes: 3 additions & 0 deletions example_app_generator/generate_stuff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ def setup_tasks

def final_tasks
copy_file 'spec/verify_active_record_spec.rb'
copy_file 'app/views/foo.html'
copy_file 'app/views/some_templates/bar.html'
copy_file 'spec/verify_custom_renderers_spec.rb'
copy_file 'spec/verify_fixture_warning_spec.rb'
run('bin/rake db:migrate')
if ::Rails::VERSION::STRING.to_f < 4.1
Expand Down
159 changes: 159 additions & 0 deletions example_app_generator/spec/verify_custom_renderers_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
require 'rails_helper'

RSpec.describe "template rendering", :type => :controller do
context "without render_views" do
context "with the standard renderers" do
controller do
def index
render :template => 'foo', :layout => false
end
end

it "renders the 'foo' template" do
get :index

expect(response).to render_template(:foo)
end

it "renders an empty string" do
get :index

expect(response.body).to eq("")
end
end

context "with a String path prepended to the view path" do
controller do
def index
prepend_view_path('app/views/some_templates')

render :template => 'bar', :layout => false
end
end

it "renders the 'bar' template" do
get :index

expect(response).to render_template(:bar)
end

it "renders an empty string" do
get :index

expect(response.body).to eq("")
end
end

context "with a custom renderer prepended to the view path" do
controller do
def index
prepend_view_path(MyResolver.new)

render :template => 'baz', :layout => false
end
end

it "renders the 'baz' template" do
get :index

expect(response).to render_template(:baz)
end

it "renders an empty string" do
get :index

expect(response.body).to eq("")
end
end
end

context "with render_views enabled" do
render_views

context "with the standard renderers" do
controller do
def index
render :template => 'foo', :layout => false
end
end

it "renders the 'foo' template" do
get :index

expect(response).to render_template(:foo)
end

it "renders the contents of the template" do
get :index

expect(response.body).to include("Static template named 'foo.html'")
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Great start, we just need to check that the views are empty (as render_views hasn't been used) and ideally have a second set of tests that checks that the views can be rendered when specified. Appreciate your work to get this towards the line!


context "with a String path prepended to the view path" do
controller do
def index
prepend_view_path('app/views/some_templates')

render :template => 'bar', :layout => false
end
end

it "renders the 'bar' template" do
get :index

expect(response).to render_template(:bar)
end

it "renders the contents of the template" do
get :index

expect(response.body).to include("Static template named 'bar.html'")
end
end

context "with a custom renderer prepended to the view path" do
controller do
def index
prepend_view_path(MyResolver.new)

render :template => 'baz', :layout => false
end
end

it "renders the 'baz' template" do
get :index

expect(response).to render_template(:baz)
end

it "renders the contents of the template" do
get :index

expect(response.body).to eq("Dynamic template with path '/baz'")
end
end
end

class MyResolver < ActionView::Resolver
private

def find_templates(name, prefix = nil, partial = false, details = {}, key = nil, locals = [])
name.prepend("_") if partial
path = [prefix, name].join("/")
template = find_template(name, path)

[template]
end

def find_template(name, path)
ActionView::Template.new(
"",
name,
lambda { |_template| %("Dynamic template with path '#{_template.virtual_path}'") },
:virtual_path => path,
:format => :html
)
end
end
end
59 changes: 50 additions & 9 deletions lib/rspec/rails/view_rendering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,18 @@ def render_views?
self.class.render_views? || !controller.class.respond_to?(:view_paths)
end

# Delegates find_all to the submitted path set and then returns templates
# with modified source
#
# @private
class EmptyTemplateResolver < ::ActionView::FileSystemResolver
private
class EmptyTemplateResolver
def self.build(path)
if path.is_a?(::ActionView::Resolver)
ResolverDecorator.new(path)
else
FileSystemResolver.new(path)
end
end

def find_templates(*args)
super.map do |template|
def self.nullify_template_rendering(templates)
templates.map do |template|
::ActionView::Template.new(
"",
template.identifier,
Expand All @@ -57,6 +60,44 @@ def find_templates(*args)
)
end
end

# Delegates all methods to the submitted resolver and for all methods
# that return a collection of `ActionView::Template` instances, return
# templates with modified source
#
# @private
class ResolverDecorator
def initialize(resolver)
@resolver = resolver
end

def method_missing(name, *args, &block)
result = @resolver.send(name, *args, &block)
nullify_templates(result)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize that inspecting return values’ type isn't particularly good design. The alternative is to override #find_all and #find_all_anywhere depending on Rails version – as far as I recall #find_all changed arity from 5 to 6 at some point.

end

private

def nullify_templates(collection)
return collection unless collection.is_a?(Enumerable)
return collection unless collection.all? { |element| element.is_a?(::ActionView::Template) }

EmptyTemplateResolver.nullify_template_rendering(collection)
end
end

# Delegates find_templates to the submitted path set and then returns
# templates with modified source
#
# @private
class FileSystemResolver < ::ActionView::FileSystemResolver
private

def find_templates(*args)
templates = super
EmptyTemplateResolver.nullify_template_rendering(templates)
end
end
end

# @private
Expand All @@ -81,13 +122,13 @@ def append_view_path(new_path)
private

def _path_decorator(*paths)
paths.map { |path| EmptyTemplateResolver.new(path) }
paths.map { |path| EmptyTemplateResolver.build(path) }
end
end

# @private
RESOLVER_CACHE = Hash.new do |hash, path|
hash[path] = EmptyTemplateResolver.new(path)
hash[path] = EmptyTemplateResolver.build(path)
end

included do
Expand Down
7 changes: 7 additions & 0 deletions spec/rspec/rails/view_rendering_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,13 @@ def example.controller
expect(controller.view_paths.map(&:to_s)).to match_paths 'app/views', 'app/legacy_views', 'app/others', 'app/more_views'
end

it 'supports manipulating view paths with resolvers' do
expect {
controller.prepend_view_path ActionView::Resolver.new
controller.append_view_path ActionView::Resolver.new
}.to_not raise_error
end

def match_paths(*paths)
eq paths.map { |path| File.expand_path path }
end
Expand Down