diff options
author | George Claghorn <george.claghorn@gmail.com> | 2015-02-13 23:41:19 -0500 |
---|---|---|
committer | George Claghorn <george@basecamp.com> | 2015-03-24 12:49:27 -0500 |
commit | 9d9cc4777be3787ed3645d704f02e5ba1228be13 (patch) | |
tree | 1175ed3d033c08cdadedc15f50b144f085e60854 | |
parent | d024bad4d1f8307a66fd6684dc658fddee37147e (diff) | |
download | rails-9d9cc4777be3787ed3645d704f02e5ba1228be13.tar.gz rails-9d9cc4777be3787ed3645d704f02e5ba1228be13.tar.bz2 rails-9d9cc4777be3787ed3645d704f02e5ba1228be13.zip |
Provide friendlier access to request variants
Closes #18933.
-rw-r--r-- | actionpack/CHANGELOG.md | 24 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/mime_responds.rb | 13 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/mime_negotiation.rb | 41 | ||||
-rw-r--r-- | actionpack/test/dispatch/request_test.rb | 49 |
4 files changed, 90 insertions, 37 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3f6cb5a5b1..e3e5787183 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,13 +1,27 @@ +* Provide friendlier access to request variants. + + request.variant = :phone + request.variant.phone? # true + request.variant.tablet? # false + + request.variant = [:phone, :tablet] + request.variant.phone? # true + request.variant.desktop? # false + request.variant.any?(:phone, :desktop) # true + request.variant.any?(:desktop, :watch) # false + + *George Claghorn* + * Fix handling of empty X_FORWARDED_HOST header in raw_host_with_port - Previously, an empty X_FORWARDED_HOST header would cause - Actiondispatch::Http:URL.raw_host_with_port to return nil, causing - Actiondispatch::Http:URL.host to raise a NoMethodError. + Previously, an empty X_FORWARDED_HOST header would cause + Actiondispatch::Http:URL.raw_host_with_port to return nil, causing + Actiondispatch::Http:URL.host to raise a NoMethodError. - *Adam Forsyth* + *Adam Forsyth* * Drop request class from RouteSet constructor. - + If you would like to use a custom request class, please subclass and implement the `request_class` method. diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 7dae171215..fab1be3459 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -288,16 +288,17 @@ module ActionController #:nodoc: end def variant - if @variant.nil? + if @variant.empty? @variants[:none] || @variants[:any] - elsif (@variants.keys & @variant).any? - @variant.each do |v| - return @variants[v] if @variants.key?(v) - end else - @variants[:any] + @variants[variant_key] end end + + private + def variant_key + @variant.find { |variant| @variants.key?(variant) } || :any + end end end end diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index 53a98c5d0a..6544aff7d2 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -10,8 +10,6 @@ module ActionDispatch 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 @@ -75,18 +73,22 @@ module ActionDispatch # Sets the \variant for template. def variant=(variant) - if variant.is_a?(Symbol) - @variant = [variant] - elsif variant.nil? || variant.is_a?(Array) && variant.any? && variant.all?{ |v| v.is_a?(Symbol) } - @variant = variant + variant = Array(variant) + + if variant.all? { |v| v.is_a?(Symbol) } + @variant = VariantInquirer.new(variant) else - raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols, not a #{variant.class}. " \ + raise ArgumentError, "request.variant must be set to a Symbol or an Array of Symbols. " \ "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[:variant] == 'tablet'" end end + def variant + @variant ||= VariantInquirer.new + end + # Sets the \format by string extension, which can be used to force custom formats # that are not controlled by the extension. # @@ -139,6 +141,31 @@ module ActionDispatch order.include?(Mime::ALL) ? format : nil end + class VariantInquirer # :nodoc: + delegate :each, :empty?, to: :@variants + + def initialize(variants = []) + @variants = variants + end + + def any?(*candidates) + (@variants & candidates).any? + end + + def to_ary + @variants + end + + private + def method_missing(name, *args) + if name[-1] == '?' + any? name[0..-2].to_sym + else + super + end + end + end + protected BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/ diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 61cc4dcd7e..ab492665c9 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -1128,35 +1128,46 @@ class RequestEtag < BaseRequestTest end class RequestVariant < BaseRequestTest - test "setting variant" do - request = stub_request + setup do + @request = stub_request + end - request.variant = :mobile - assert_equal [:mobile], request.variant + test 'setting variant to a symbol' do + @request.variant = :phone - request.variant = [:phone, :tablet] - assert_equal [:phone, :tablet], request.variant + assert @request.variant.phone? + assert_not @request.variant.tablet? + assert @request.variant.any?(:phone, :tablet) + assert_not @request.variant.any?(:tablet, :desktop) + end - assert_raise ArgumentError do - request.variant = [:phone, "tablet"] - end + test 'setting variant to an array of symbols' do + @request.variant = [:phone, :tablet] - assert_raise ArgumentError do - request.variant = "yolo" - end + assert @request.variant.phone? + assert @request.variant.tablet? + assert_not @request.variant.desktop? + assert @request.variant.any?(:tablet, :desktop) + assert_not @request.variant.any?(:desktop, :watch) end - test "reset variant" do - request = stub_request + test 'clearing variant' do + @request.variant = nil - request.variant = nil - assert_equal nil, request.variant + assert @request.variant.empty? + assert_not @request.variant.phone? + assert_not @request.variant.any?(:phone, :tablet) end - test "setting variant with non symbol value" do - request = stub_request + test 'setting variant to a non-symbol value' do + assert_raise ArgumentError do + @request.variant = 'phone' + end + end + + test 'setting variant to an array containing a non-symbol value' do assert_raise ArgumentError do - request.variant = "mobile" + @request.variant = [:phone, 'tablet'] end end end |