Permalink
Browse files

Action Pack Variants

By default, variants in the templates will be picked up if a variant is set
and there's a match. The format will be:

  app/views/projects/show.html.erb
  app/views/projects/show.html+tablet.erb
  app/views/projects/show.html+phone.erb

If request.variant = :tablet is set, we'll automatically be rendering the
html+tablet template.

In the controller, we can also tailer to the variants with this syntax:

  class ProjectsController < ActionController::Base
    def show
      respond_to do |format|
        format.html do |html|
          @stars = @project.stars

          html.tablet { @notifications = @project.notifications }
          html.phone  { @chat_heads    = @project.chat_heads }
        end

        format.js
        format.atom
      end
    end
  end

The variant itself is nil by default, but can be set in before filters, like
so:

  class ApplicationController < ActionController::Base
    before_action do
      if request.user_agent =~ /iPad/
        request.variant = :tablet
      end
    end
  end

This is modeled loosely on custom mime types, but it's specifically not
intended to be used together. If you're going to make a custom mime type,
you don't need a variant. Variants are for variations on a single mime
types.
  • Loading branch information...
1 parent 4d64881 commit 2d3a6a0cb8df0360dd588a4d2fb260dd07cc9bcf @lukaszx0 lukaszx0 committed Dec 3, 2013
View
@@ -1,3 +1,32 @@
+* Introducing Variants
+
+ We often want to render different html/json/xml templates for phones,
+ tablets, and desktop browsers. Variants make it easy.
+
+ The request variant is a specialization of the request format, like :tablet,
+ :phone, or :desktop.
+
+ You can set the variant in a before_action:
+
+ request.variant = :tablet if request.user_agent =~ /iPad/
+
+ Respond to variants in the action just like you respond to formats:
+
+ respond_to do |format|
+ format.html do |html|
+ html.tablet # renders app/views/projects/show.html+tablet.erb
+ html.phone { extra_setup; render ... }
+ end
+ end
+
+ Provide separate templates for each format and variant:
+
+ app/views/projects/show.html.erb
+ app/views/projects/show.html+tablet.erb
+ app/views/projects/show.html+phone.erb
+
+ *Łukasz Strzałkowski*
+
* Fix header `Content-Type: #<Mime::NullType:...>` in localized template.
When localized template has no format in the template name,
@@ -23,7 +23,15 @@ def #{sym}(*args, &block) # def html(*args, &block)
protected
def method_missing(symbol, &block)
- mime_constant = Mime.const_get(symbol.upcase)
+ mime_const = symbol.upcase
@tenderlove

tenderlove Mar 11, 2014

Owner

Is this something we're really really really really sure isn't going to see user input? I mean really really really sure? Note how symbol count increases when we call upcase:

irb(main):003:0> Symbol.all_symbols.count
=> 3167
irb(main):004:0> :himom.upcase
=> :HIMOM
irb(main):005:0> Symbol.all_symbols.count
=> 3169
irb(main):006:0>
@lukaszx0

lukaszx0 Mar 11, 2014

Member

I think we can cast it to string here to avoid this problem.

@amolpujari

amolpujari Mar 11, 2014

great to see and learn. I recently saw one of my rails consoles as

Loading development environment (Rails 3.1.10)

Frame number: 0/4
[1] pry(main)> Symbol.all_symbols.count
=> 47415
[2] pry(main)> 

And I was simply thinking, can rails completely avoid symbols?

+
+ raise NoMethodError, "To respond to a custom format, register it as a MIME type first:" +
+ "http://guides.rubyonrails.org/action_controller_overview.html#restful-downloads." +
+ "If you meant to respond to a variant like :tablet or :phone, not a custom format," +
+ "be sure to nest your variant response within a format response: format.html" +
+ "{ |html| html.tablet { ..." unless Mime.const_defined?(mime_const)
+
+ mime_constant = Mime.const_get(mime_const)
if Mime::SET.include?(mime_constant)
AbstractController::Collector.generate_method_for_mime(mime_constant)
@@ -102,6 +102,8 @@ def _process_format(format)
# :api: private
def _normalize_render(*args, &block)
options = _normalize_args(*args, &block)
+ #TODO: remove defined? when we restore AP <=> AV dependency
+ options[:variant] = request.variant if defined?(request) && request.variant.present?
_normalize_options(options)
options
end
@@ -181,13 +181,40 @@ def clear_respond_to
# end
# end
#
+ # Formats can have different variants.
+ #
+ # The request variant is a specialization of the request format, like <tt>:tablet</tt>,
+ # <tt>:phone</tt>, or <tt>:desktop<tt>.
+ #
+ # We often want to render different html/json/xml templates for phones,
+ # tablets, and desktop browsers. Variants make it easy.
+ #
+ # You can set the variant in a +before_action+:
+ #
+ # request.variant = :tablet if request.user_agent =~ /iPad/
+ #
+ # Respond to variants in the action just like you respond to formats:
+ #
+ # respond_to do |format|
+ # format.html do |html|
+ # html.tablet # renders app/views/projects/show.html+tablet.erb
+ # html.phone { extra_setup; render ... }
+ # end
+ # end
+ #
+ # Provide separate templates for each format and variant:
+ #
+ # app/views/projects/show.html.erb
+ # app/views/projects/show.html+tablet.erb
+ # app/views/projects/show.html+phone.erb
+ #
# Be sure to check the documentation of +respond_with+ and
# <tt>ActionController::MimeResponds.respond_to</tt> for more examples.
def respond_to(*mimes, &block)
raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given?
if collector = retrieve_collector_from_mimes(mimes, &block)
- response = collector.response
+ response = collector.response(request.variant)
response ? response.call : render({})
end
end
@@ -327,7 +354,7 @@ def respond_with(*resources, &block)
if collector = retrieve_collector_from_mimes(&block)
options = resources.size == 1 ? {} : resources.extract_options!
options = options.clone
- options[:default_response] = collector.response
+ options[:default_response] = collector.response(request.variant)
(options.delete(:responder) || self.class.responder).call(self, resources, options)
end
end
@@ -417,13 +444,28 @@ def custom(mime_type, &block)
@responses[mime_type] ||= block
end
- def response
- @responses.fetch(format, @responses[Mime::ALL])
+ def response(variant)
+ response = @responses.fetch(format, @responses[Mime::ALL])
+ if response.nil? || response.arity == 0
+ response
+ else
+ lambda { response.call VariantFilter.new(variant) }
+ end
end
def negotiate_format(request)
@format = request.negotiate_mime(@responses.keys)
end
+
+ class VariantFilter
+ def initialize(variant)
+ @variant = variant
+ end
+
+ def method_missing(name)
+ yield if name == @variant
+ end
@tenderlove

tenderlove Mar 11, 2014

Owner

wtf?

Edit: Sorry, I'll be more specific than "wtf". Why doesn't this call super if name != @variant? Is there a way we can implement this without using method_missing?

@lukaszx0

lukaszx0 Mar 11, 2014

Member

It was changed and modified already a few times (because we were adding new features). Right now we don't have VariantFilter and it now we have VariantsCollector and whole thing works a bit differently: https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/metal/mime_responds.rb#L512-L544

@tenderlove

tenderlove Mar 11, 2014

Owner

@strzalek awesome. Thanks for the explanation! I'll poke at the new code for a bit. ❤️

+ end
end
end
end
@@ -10,6 +10,8 @@ module MimeNegotiation
self.ignore_accept_header = false
end
+ attr_reader :variant
+
# The MIME type of the HTTP request, such as Mime::XML.
#
# For backward compatibility, the post \format is extracted from the
@@ -64,6 +66,18 @@ def formats
end
end
+ # Sets the \variant for template
+ def variant=(variant)
+ if variant.is_a? Symbol
+ @variant = variant
+ else
+ raise ArgumentError, "request.variant must be set to a Symbol, not a #{variant.class}. For security reasons," +
+ "never directly set the variant to a user-provided value, like params[:variant].to_sym." +
+ "Check user-provided value against a whitelist first, then set the variant:"+
+ "request.variant = :tablet if params[:some_param] == 'tablet'"
+ end
@tenderlove

tenderlove Mar 11, 2014

Owner

What technical reason requires that we store the variant as a symbol? Can't we just call to_s on this an deal with strings internally?

@lukaszx0

lukaszx0 Mar 11, 2014

Member

I would love to keep it consistent. We keep format as symbol and it would be good to have variant as symbol as well.

@tenderlove

tenderlove Mar 11, 2014

Owner

I prefer we keep neither as symbols. Working with symbols inside the framework is a huge pain from a security perspective. Also, inside the framework it doesn't matter if we use strings.

@tenderlove

tenderlove Mar 11, 2014

Owner

@strzalek I read both comments. Neither of them provided technical reasons for the requirement that this be a symbol internally. Please describe exactly why it needs to be a symbol, otherwise I want to cast these to strings and remove the exception.

@lukaszx0

lukaszx0 Mar 11, 2014

Member

I have no problem casting it to string as long as we'll do it for both format and variant.

+ end
+
# Sets the \format by string extension, which can be used to force custom formats
# that are not controlled by the extension.
#
@@ -37,7 +37,7 @@ class TestCollector < ActiveSupport::TestCase
test "does not register unknown mime types" do
collector = MyCollector.new
- assert_raise NameError do
+ assert_raise NoMethodError do
collector.unknown
end
end
@@ -146,6 +146,26 @@ def iphone_with_html_response_type_without_layout
end
end
+ def variant_with_implicit_rendering
+ end
+
+ def variant_with_format_and_custom_render
+ request.variant = :mobile
+
+ respond_to do |type|
+ type.html { render text: "mobile" }
+ end
+ end
+
+ def multiple_variants_for_format
+ respond_to do |type|
+ type.html do |html|
+ html.tablet { render text: "tablet" }
+ html.phone { render text: "phone" }
+ end
+ end
+ end
+
protected
def set_layout
case action_name
@@ -490,4 +510,38 @@ def test_invalid_format
get :using_defaults, :format => "invalidformat"
end
end
+
+ def test_invalid_variant
+ @request.variant = :invalid
+ assert_raises(ActionView::MissingTemplate) do
+ get :variant_with_implicit_rendering
+ end
+ end
+
+ def test_variant_not_set_regular_template_missing
+ assert_raises(ActionView::MissingTemplate) do
+ get :variant_with_implicit_rendering
+ end
+ end
+
+ def test_variant_with_implicit_rendering
+ @request.variant = :mobile
+ get :variant_with_implicit_rendering
+ assert_equal "text/html", @response.content_type
+ assert_equal "mobile", @response.body
+ end
+
+ def test_variant_with_format_and_custom_render
+ @request.variant = :phone
+ get :variant_with_format_and_custom_render
+ assert_equal "text/html", @response.content_type
+ assert_equal "mobile", @response.body
+ end
+
+ def test_multiple_variants_for_format
+ @request.variant = :tablet
+ get :multiple_variants_for_format
+ assert_equal "text/html", @response.content_type
+ assert_equal "tablet", @response.body
+ end
end
@@ -844,6 +844,19 @@ def url_for(options = {})
end
end
+ test "setting variant" do
+ request = stub_request
+ request.variant = :mobile
+ assert_equal :mobile, request.variant
+ end
+
+ test "setting variant with non symbol value" do
+ request = stub_request
+ assert_raise ArgumentError do
+ request.variant = "mobile"
+ end
+ end
+
protected
def stub_request(env = {})
@@ -52,6 +52,7 @@ module Accessors #:nodoc:
locales
end
register_detail(:formats) { ActionView::Base.default_formats || [:html, :text, :js, :css, :xml, :json] }
+ register_detail(:variants) { [] }
register_detail(:handlers){ Template::Handlers.extensions }
class DetailsKey #:nodoc:
@@ -88,10 +88,14 @@ def rendered_format
private
- # Find and renders a template based on the options given.
+ # Find and render a template based on the options given.
# :api: private
def _render_template(options) #:nodoc:
+ variant = options[:variant]
+
lookup_context.rendered_format = nil if options[:formats]
+ lookup_context.variants = [variant] if variant
+
view_renderer.render(view_context, options)
end
@@ -162,8 +162,8 @@ def decorate(templates, path_info, details, locals) #:nodoc:
# An abstract class that implements a Resolver with path semantics.
class PathResolver < Resolver #:nodoc:
- EXTENSIONS = [:locale, :formats, :handlers]
- DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{.:handlers,}"
+ EXTENSIONS = { :locale => ".", :formats => ".", :variants => "+", :handlers => "." }
+ DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{+:variants,}{.:handlers,}"
def initialize(pattern=nil)
@pattern = pattern || DEFAULT_PATTERN
@@ -240,7 +240,9 @@ def extract_handler_and_format(path, default_formats)
end
handler = Template.handler_for_extension(extension)
- format = pieces.last && Template::Types[pieces.last]
+ format = pieces.last && pieces.last.split(EXTENSIONS[:variants], 2).first # remove variant from format
+ format &&= Template::Types[format]
+
[handler, format]
end
end
@@ -303,12 +305,13 @@ def eql?(resolver)
# An Optimized resolver for Rails' most common case.
class OptimizedFileSystemResolver < FileSystemResolver #:nodoc:
def build_query(path, details)
- exts = EXTENSIONS.map { |ext| details[ext] }
query = escape_entry(File.join(@path, path))
- query + exts.map { |ext|
- "{#{ext.compact.uniq.map { |e| ".#{e}," }.join}}"
- }.join
+ exts = EXTENSIONS.map do |ext, prefix|
+ "{#{details[ext].compact.uniq.map { |e| "#{prefix}#{e}," }.join}}"
+ end.join
+
+ query + exts
end
end
@@ -21,7 +21,7 @@ def to_s
def query(path, exts, formats)
query = ""
- EXTENSIONS.each do |ext|
+ EXTENSIONS.each_key do |ext|
query << '(' << exts[ext].map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)'
end
query = /^(#{Regexp.escape(path)})#{query}$/
@@ -36,6 +36,11 @@ def teardown
assert @lookup_context.formats.frozen?
end
+ test "provides getters and setters for variants" do
+ @lookup_context.variants = [:mobile]
+ assert_equal [:mobile], @lookup_context.variants
+ end
+
test "provides getters and setters for formats" do
@lookup_context.formats = [:html]
assert_equal [:html], @lookup_context.formats
@@ -254,7 +259,7 @@ def setup
test "if a single prefix is passed as a string and the lookup fails, MissingTemplate accepts it" do
e = assert_raise ActionView::MissingTemplate do
- details = {:handlers=>[], :formats=>[], :locale=>[]}
+ details = {:handlers=>[], :formats=>[], :variants=>[], :locale=>[]}
@lookup_context.view_paths.find("foo", "parent", true, details)
end
assert_match %r{Missing partial parent/_foo with .* Searched in:\n \* "/Path/to/views"\n}, e.message
@@ -3,13 +3,13 @@
class FixtureResolverTest < ActiveSupport::TestCase
def test_should_return_empty_list_for_unknown_path
resolver = ActionView::FixtureResolver.new()
- templates = resolver.find_all("path", "arbitrary", false, {:locale => [], :formats => [:html], :handlers => []})
+ templates = resolver.find_all("path", "arbitrary", false, {:locale => [], :formats => [:html], :variants => [], :handlers => []})
assert_equal [], templates, "expected an empty list of templates"
end
def test_should_return_template_for_declared_path
resolver = ActionView::FixtureResolver.new("arbitrary/path.erb" => "this text")
- templates = resolver.find_all("path", "arbitrary", false, {:locale => [], :formats => [:html], :handlers => [:erb]})
+ templates = resolver.find_all("path", "arbitrary", false, {:locale => [], :formats => [:html], :variants => [], :handlers => [:erb]})
assert_equal 1, templates.size, "expected one template"
assert_equal "this text", templates.first.source
assert_equal "arbitrary/path", templates.first.virtual_path

0 comments on commit 2d3a6a0

Please sign in to comment.