From c4d1075bd366e89a070afd5d6bf859af276c9507 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Tue, 28 Jul 2009 19:04:34 -0700 Subject: Add support for error_messages_for(@obj) --- .../lib/action_view/helpers/active_model_helper.rb | 30 +++++++++++++++++----- .../template/active_record_helper_i18n_test.rb | 7 +++-- .../test/template/active_record_helper_test.rb | 10 ++++++++ activemodel/lib/active_model/naming.rb | 3 ++- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb index 0f122d9232..4fd7f7d83c 100644 --- a/actionpack/lib/action_view/helpers/active_model_helper.rb +++ b/actionpack/lib/action_view/helpers/active_model_helper.rb @@ -160,11 +160,24 @@ module ActionView # # error_messages_for 'user' # + # You can also supply an object: + # + # error_messages_for @user + # + # This will use the last part of the model name in the presentation. For instance, if + # this is a MyKlass::User object, this will use "user" as the name in the String. This + # is taken from MyKlass::User.model_name.human, which can be overridden. + # # To specify more than one object, you simply list them; optionally, you can add an extra :object_name parameter, which # will be the name used in the header message: # # error_messages_for 'user_common', 'user', :object_name => 'user' # + # You can also use a number of objects, which will have the same naming semantics + # as a single object. + # + # error_messages_for @user, @post + # # If the objects cannot be located as instance variables, you can add an extra :object parameter which gives the actual # object (or array of objects to use): # @@ -176,15 +189,20 @@ module ActionView def error_messages_for(*params) options = params.extract_options!.symbolize_keys - if object = options.delete(:object) - objects = [object].flatten - else - objects = params.collect {|object_name| instance_variable_get("@#{object_name}") }.compact + objects = Array.wrap(options.delete(:object) || params).map do |object| + unless object.respond_to?(:to_model) + object = instance_variable_get("@#{object}") + object = convert_to_model(object) + else + object = object.to_model + options[:object_name] ||= object.class.model_name.human + end + object end - objects.map! {|o| convert_to_model(o) } + objects.compact! - count = objects.inject(0) {|sum, object| sum + object.errors.count } + count = objects.inject(0) {|sum, object| sum + object.errors.count } unless count.zero? html = {} [:id, :class].each do |key| diff --git a/actionpack/test/template/active_record_helper_i18n_test.rb b/actionpack/test/template/active_record_helper_i18n_test.rb index 63032e4e5c..047f81be29 100644 --- a/actionpack/test/template/active_record_helper_i18n_test.rb +++ b/actionpack/test/template/active_record_helper_i18n_test.rb @@ -3,11 +3,14 @@ require 'abstract_unit' class ActiveRecordHelperI18nTest < Test::Unit::TestCase include ActionView::Context include ActionView::Helpers::ActiveModelHelper - + attr_reader :request def setup @object = stub :errors => stub(:count => 1, :full_messages => ['full_messages']) + @object.stubs :to_model => @object + @object.stubs :class => stub(:model_name => stub(:human => "")) + @object_name = 'book_seller' @object_name_without_underscore = 'book seller' @@ -39,7 +42,7 @@ class ActiveRecordHelperI18nTest < Test::Unit::TestCase I18n.expects(:t).with('', :default => '', :count => 1, :scope => [:activerecord, :models]).once.returns '' error_messages_for(:object => @object, :locale => 'en') end - + def test_error_messages_for_given_object_name_it_translates_object_name I18n.expects(:t).with(:header, :locale => 'en', :scope => [:activerecord, :errors, :template], :count => 1, :model => @object_name_without_underscore).returns "1 error prohibited this #{@object_name_without_underscore} from being saved" I18n.expects(:t).with(@object_name, :default => @object_name_without_underscore, :count => 1, :scope => [:activerecord, :models]).once.returns @object_name_without_underscore diff --git a/actionpack/test/template/active_record_helper_test.rb b/actionpack/test/template/active_record_helper_test.rb index b07ce6cf5d..ec3384f15d 100644 --- a/actionpack/test/template/active_record_helper_test.rb +++ b/actionpack/test/template/active_record_helper_test.rb @@ -295,6 +295,16 @@ class ActiveRecordHelperTest < ActionView::TestCase assert_equal '', error_messages_for('user', :object => nil) end + def test_error_messages_for_model_objects + error = error_messages_for(@post) + assert_dom_equal %(

1 error prohibited this post from being saved

There were problems with the following fields:

), + error + + error = error_messages_for(@user, @post) + assert_dom_equal %(

2 errors prohibited this user from being saved

There were problems with the following fields:

), + error + end + def test_form_with_string_multipart assert_dom_equal( %(


\n


), diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index ffb44e3824..b8c2a367b4 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -2,7 +2,7 @@ require 'active_support/inflector' module ActiveModel class Name < String - attr_reader :singular, :plural, :element, :collection, :partial_path + attr_reader :singular, :plural, :element, :collection, :partial_path, :human alias_method :cache_key, :collection def initialize(name) @@ -10,6 +10,7 @@ module ActiveModel @singular = ActiveSupport::Inflector.underscore(self).tr('/', '_').freeze @plural = ActiveSupport::Inflector.pluralize(@singular).freeze @element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(self)).freeze + @human = @element.gsub(/_/, " ") @collection = ActiveSupport::Inflector.tableize(self).freeze @partial_path = "#{@collection}/#{@element}".freeze end -- cgit v1.2.3 From 3f445b316d34a49a8b6f27bde72979828baefaa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 28 Jul 2009 15:49:59 +0200 Subject: Refactor Responder to only calculate available mime types. Those are sent to the controller that knows what to do with it (render a block or call default render). Signed-off-by: Yehuda Katz --- .../lib/action_controller/base/mime_responds.rb | 129 ++++++++++++--------- actionpack/test/controller/mime_responds_test.rb | 6 +- 2 files changed, 74 insertions(+), 61 deletions(-) diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index ed0d58dba1..7320b0f858 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -1,5 +1,6 @@ module ActionController #:nodoc: module MimeResponds #:nodoc: + # Without web-service support, an action which collects the data for displaying a list of people # might look something like this: # @@ -92,40 +93,39 @@ module ActionController #:nodoc: # environment.rb as follows. # # Mime::Type.register "image/jpg", :jpg - def respond_to(*types, &block) - raise ArgumentError, "respond_to takes either types or a block, never both" unless types.any? ^ block - block ||= lambda { |responder| types.each { |type| responder.send(type) } } - responder = Responder.new(self) - block.call(responder) - responder.respond - end - - class Responder #:nodoc: - - def initialize(controller) - @controller = controller - @request = controller.request - @response = controller.response + def respond_to(*mimes, &block) + raise ArgumentError, "respond_to takes either types or a block, never both" unless mimes.any? ^ block - @mime_type_priority = @request.formats + responder = Responder.new(request.formats) - @order = [] - @responses = {} + if block_given? + block.call(responder) + else + mimes.each { |mime| responder.send(mime) } end - def custom(mime_type, &block) - mime_type = mime_type.is_a?(Mime::Type) ? mime_type : Mime::Type.lookup(mime_type.to_s) - - @order << mime_type + mime = responder.respond - @responses[mime_type] ||= Proc.new do - # TODO: Remove this when new base is merged in - @controller.formats = [mime_type.to_sym] - @controller.content_type = mime_type - @controller.template.formats = [mime_type.to_sym] + if mime + self.formats = [mime.to_sym] + self.content_type = mime + self.template.formats = [mime.to_sym] - block_given? ? block.call : @controller.send(:render, :action => @controller.action_name) + if response = responder.response_for(mime) + response.call + else + default_render end + else + head :not_acceptable + end + end + + class Responder #:nodoc: + + def initialize(priorities) + @mime_type_priority = priorities + @order, @responses = [], {} end def any(*args, &block) @@ -135,51 +135,64 @@ module ActionController #:nodoc: custom(@mime_type_priority.first, &block) end end - - def self.generate_method_for_mime(mime) - sym = mime.is_a?(Symbol) ? mime : mime.to_sym - const = sym.to_s.upcase - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{sym}(&block) # def html(&block) - custom(Mime::#{const}, &block) # custom(Mime::HTML, &block) - end # end - RUBY + + def custom(mime_type, &block) + mime_type = mime_type.is_a?(Mime::Type) ? mime_type : Mime::Type.lookup(mime_type.to_s) + + @order << mime_type + @responses[mime_type] ||= block end - Mime::SET.each do |mime| - generate_method_for_mime(mime) + def respond + available_mimes.first end + def response_for(mime) + @responses[mime] + end + + # Compares mimes sent by the client (@mime_type_priorities) with the ones + # that the user configured in the controller respond_to. Returns them + # all in an array. + # + def available_mimes + mimes = [] + + @mime_type_priority.each do |priority| + if priority == Mime::ALL + mimes << @order.first unless mimes.include?(@order.first) + elsif @order.include?(priority) + mimes << priority + end + end + + mimes << Mime::ALL if @order.include?(Mime::ALL) + mimes + end + + protected + def method_missing(symbol, &block) mime_constant = Mime.const_get(symbol.to_s.upcase) - + if Mime::SET.include?(mime_constant) - self.class.generate_method_for_mime(mime_constant) + generate_method_for_mime(mime_constant) send(symbol, &block) else super end end - def respond - for priority in @mime_type_priority - if priority == Mime::ALL - @responses[@order.first].call - return - else - if @responses[priority] - @responses[priority].call - return # mime type match found, be happy and return - end - end - end - - if @order.include?(Mime::ALL) - @responses[Mime::ALL].call - else - @controller.send :head, :not_acceptable - end + def generate_method_for_mime(mime) + sym = mime.is_a?(Symbol) ? mime : mime.to_sym + const = sym.to_s.upcase + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{sym}(&block) # def html(&block) + custom(Mime::#{const}, &block) # custom(Mime::HTML, &block) + end # end + RUBY end + end end end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 93ca34c41c..bee327998b 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -436,9 +436,9 @@ class MimeControllerTest < ActionController::TestCase def test_render_action_for_html @controller.instance_eval do def render(*args) - unless args.empty? - @action = args.first[:action] || action_name - end + @action = args.first[:action] unless args.empty? + @action ||= action_name + response.body = "#{@action} - #{@template.formats}" end end -- cgit v1.2.3 From 3e8ba616efdd453dcfdd8fba78ed35fc3c1885de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 28 Jul 2009 21:29:29 +0200 Subject: Refactor even more Responder. Move mime negotiation to request and added respond_to class method. Signed-off-by: Yehuda Katz --- .../lib/action_controller/base/mime_responds.rb | 150 ++++++++++++++------- actionpack/lib/action_dispatch/http/request.rb | 42 ++++-- actionpack/test/controller/mime_responds_test.rb | 22 +-- 3 files changed, 145 insertions(+), 69 deletions(-) diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index 7320b0f858..d717388045 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -1,5 +1,57 @@ module ActionController #:nodoc: module MimeResponds #:nodoc: + extend ActiveSupport::Concern + + included do + class_inheritable_reader :mimes_for_respond_to + respond_to # Set mimes_for_respond_to hash + end + + module ClassMethods + # Defines mimes that are rendered by default when invoking respond_with. + # + # Examples: + # + # respond_to :html, :xml, :json + # + # All actions on your controller will respond to :html, :xml and :json. + # + # But if you want to specify it based on your actions, you can use only and + # except: + # + # respond_to :html + # respond_to :xml, :json, :except => [ :edit ] + # + # The definition above explicits that all actions respond to :html. And all + # actions except :edit respond to :xml and :json. + # + # You can specify also only parameters: + # + # respond_to :rjs, :only => :create + # + def respond_to(*mimes) + options = mimes.extract_options! + mimes_hash = {} + + only_actions = Array(options.delete(:only)) + except_actions = Array(options.delete(:except)) + + mimes.each do |mime| + mime = mime.to_sym + mimes_hash[mime] = {} + mimes_hash[mime][:only] = only_actions unless only_actions.empty? + mimes_hash[mime][:except] = except_actions unless except_actions.empty? + end + + write_inheritable_hash(:mimes_for_respond_to, mimes_hash) + end + + # Clear all mimes in respond_to. + # + def clear_respond_to! + mimes_for_respond_to.each { |k,v| mimes[k] = { :only => [] } } + end + end # Without web-service support, an action which collects the data for displaying a list of people # might look something like this: @@ -94,24 +146,20 @@ module ActionController #:nodoc: # # Mime::Type.register "image/jpg", :jpg def respond_to(*mimes, &block) - raise ArgumentError, "respond_to takes either types or a block, never both" unless mimes.any? ^ block + responder = Responder.new - responder = Responder.new(request.formats) + block.call(responder) if block_given? - if block_given? - block.call(responder) - else - mimes.each { |mime| responder.send(mime) } - end + mimes = collect_mimes_from_class_level if mimes.empty? + mimes.each { |mime| responder.send(mime) } - mime = responder.respond + if format = request.negotiate_mime(responder.order) + # TODO It should be just: self.formats = [ :foo ] + self.formats = [format.to_sym] + self.content_type = format + self.template.formats = [format.to_sym] - if mime - self.formats = [mime.to_sym] - self.content_type = mime - self.template.formats = [mime.to_sym] - - if response = responder.response_for(mime) + if response = responder.response_for(format) response.call else default_render @@ -121,10 +169,31 @@ module ActionController #:nodoc: end end + protected + + # Collect mimes declared in the class method respond_to valid for the + # current action. + # + def collect_mimes_from_class_level #:nodoc: + action = action_name.to_sym + + mimes_for_respond_to.keys.select do |mime| + config = mimes_for_respond_to[mime] + + if config[:except] + !config[:except].include?(action) + elsif config[:only] + config[:only].include?(action) + else + true + end + end + end + class Responder #:nodoc: + attr_accessor :order - def initialize(priorities) - @mime_type_priority = priorities + def initialize @order, @responses = [], {} end @@ -132,7 +201,7 @@ module ActionController #:nodoc: if args.any? args.each { |type| send(type, &block) } else - custom(@mime_type_priority.first, &block) + custom(Mime::ALL, &block) end end @@ -143,56 +212,35 @@ module ActionController #:nodoc: @responses[mime_type] ||= block end - def respond - available_mimes.first - end - def response_for(mime) - @responses[mime] + @responses[mime] || @responses[Mime::ALL] end - # Compares mimes sent by the client (@mime_type_priorities) with the ones - # that the user configured in the controller respond_to. Returns them - # all in an array. - # - def available_mimes - mimes = [] - - @mime_type_priority.each do |priority| - if priority == Mime::ALL - mimes << @order.first unless mimes.include?(@order.first) - elsif @order.include?(priority) - mimes << priority - end - end - - mimes << Mime::ALL if @order.include?(Mime::ALL) - mimes + def self.generate_method_for_mime(mime) + sym = mime.is_a?(Symbol) ? mime : mime.to_sym + const = sym.to_s.upcase + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{sym}(&block) # def html(&block) + custom(Mime::#{const}, &block) # custom(Mime::HTML, &block) + end # end + RUBY end - protected + Mime::SET.each do |mime| + generate_method_for_mime(mime) + end def method_missing(symbol, &block) mime_constant = Mime.const_get(symbol.to_s.upcase) if Mime::SET.include?(mime_constant) - generate_method_for_mime(mime_constant) + self.class.generate_method_for_mime(mime_constant) send(symbol, &block) else super end end - def generate_method_for_mime(mime) - sym = mime.is_a?(Symbol) ? mime : mime.to_sym - const = sym.to_s.upcase - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{sym}(&block) # def html(&block) - custom(Mime::#{const}, &block) # custom(Mime::HTML, &block) - end # end - RUBY - end - end end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 3f23a5af7a..5f9463eb91 100755 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -161,7 +161,7 @@ module ActionDispatch # GET /posts/5.xml | request.format => Mime::XML # GET /posts/5.xhtml | request.format => Mime::HTML # GET /posts/5 | request.format => Mime::HTML or MIME::JS, or request.accepts.first depending on the value of ActionController::Base.use_accept_header - + # def format(view_path = []) @env["action_dispatch.request.format"] ||= if parameters[:format] @@ -173,13 +173,11 @@ module ActionDispatch end end + # Expand raw_formats by converting Mime::ALL to the Mime::SET. + # def formats if ActionController::Base.use_accept_header - if param = parameters[:format] - Array.wrap(Mime[param]) - else - accepts.dup - end.tap do |ret| + raw_formats.tap do |ret| if ret == ONLY_ALL ret.replace Mime::SET elsif all = ret.index(Mime::ALL) @@ -187,7 +185,7 @@ module ActionDispatch end end else - [format] + Mime::SET + raw_formats + Mime::SET end end @@ -232,7 +230,7 @@ module ActionDispatch def xml_http_request? !(@env['HTTP_X_REQUESTED_WITH'] !~ /XMLHttpRequest/i) end - alias xhr? :xml_http_request? + alias :xhr? :xml_http_request? # Which IP addresses are "trusted proxies" that can be stripped from # the right-hand-side of X-Forwarded-For @@ -485,7 +483,35 @@ EOM session['flash'] || {} end + # Receives an array of mimes and return the first user sent mime that + # matches the order array. + # + def negotiate_mime(order) + raw_formats.each do |priority| + if priority == Mime::ALL + return order.first + elsif order.include?(priority) + return priority + end + end + + order.include?(Mime::ALL) ? formats.first : nil + end + private + + def raw_formats + if ActionController::Base.use_accept_header + if param = parameters[:format] + Array.wrap(Mime[param]) + else + accepts.dup + end + else + [format] + end + end + def named_host?(host) !(host.nil? || /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.match(host)) end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index bee327998b..f2c20417f8 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -86,6 +86,7 @@ class RespondToController < ActionController::Base type.mobile { render :text => "Mobile" } end ensure + Mime::SET.delete(:mobile) Mime.module_eval { remove_const :MOBILE if const_defined?(:MOBILE) } end @@ -98,6 +99,7 @@ class RespondToController < ActionController::Base end ensure + Mime::SET.delete(:mobile) Mime.module_eval { remove_const :MOBILE if const_defined?(:MOBILE) } end @@ -132,6 +134,7 @@ class RespondToController < ActionController::Base end ensure + Mime::SET.delete(:iphone) Mime.module_eval { remove_const :IPHONE if const_defined?(:IPHONE) } end @@ -145,6 +148,7 @@ class RespondToController < ActionController::Base end ensure + Mime::SET.delete(:iphone) Mime.module_eval { remove_const :IPHONE if const_defined?(:IPHONE) } end @@ -467,7 +471,13 @@ class MimeControllerTest < ActionController::TestCase end end +class ClassRespondToController < ActionController::Base + +end + class AbstractPostController < ActionController::Base + respond_to :html, :iphone + self.view_paths = File.dirname(__FILE__) + "/../fixtures/post_test/" end @@ -476,10 +486,7 @@ class PostController < AbstractPostController around_filter :with_iphone def index - respond_to do |type| - type.html - type.iphone - end + respond_to # It will use formats declared above end protected @@ -489,17 +496,12 @@ protected request.format = "iphone" if request.env["HTTP_ACCEPT"] == "text/iphone" yield ensure + Mime::SET.delete(:iphone) Mime.module_eval { remove_const :IPHONE if const_defined?(:IPHONE) } end end class SuperPostController < PostController - def index - respond_to do |type| - type.html - type.iphone - end - end end class MimeControllerLayoutsTest < ActionController::TestCase -- cgit v1.2.3 From 7e280c3bff8ac25f1c1938aeaeb1d0b4c0fbb726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 09:36:29 +0200 Subject: Remove Mime::ALL from Mime::SET. Signed-off-by: Yehuda Katz --- actionpack/lib/action_controller/base/mime_responds.rb | 1 + actionpack/lib/action_dispatch/http/mime_types.rb | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index d717388045..dff4d7b943 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -204,6 +204,7 @@ module ActionController #:nodoc: custom(Mime::ALL, &block) end end + alias :all :any def custom(mime_type, &block) mime_type = mime_type.is_a?(Mime::Type) ? mime_type : Mime::Type.lookup(mime_type.to_s) diff --git a/actionpack/lib/action_dispatch/http/mime_types.rb b/actionpack/lib/action_dispatch/http/mime_types.rb index 7c28cac419..68f37d2f65 100644 --- a/actionpack/lib/action_dispatch/http/mime_types.rb +++ b/actionpack/lib/action_dispatch/http/mime_types.rb @@ -2,7 +2,6 @@ # http://www.iana.org/assignments/media-types/ Mime::Type.register "text/html", :html, %w( application/xhtml+xml ), %w( xhtml ) -Mime::Type.register "*/*", :all Mime::Type.register "text/plain", :text, [], %w(txt) Mime::Type.register "text/javascript", :js, %w( application/javascript application/x-javascript ) Mime::Type.register "text/css", :css @@ -18,4 +17,7 @@ Mime::Type.register "application/x-www-form-urlencoded", :url_encoded_form # http://www.ietf.org/rfc/rfc4627.txt # http://www.json.org/JSONRequest.html -Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) \ No newline at end of file +Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest ) + +# Create Mime::ALL but do not add it to the SET. +Mime::ALL = Mime::Type.new("*/*", :all, []) -- cgit v1.2.3 From b51632d34d99ee9cae3287c60887b25fdf7a3618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 09:53:02 +0200 Subject: Improve request test coverage by adding formats and negotiate_mime tests. Signed-off-by: Yehuda Katz --- actionpack/test/dispatch/request_test.rb | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 3a85db8aa5..948eeeb001 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -396,10 +396,49 @@ class RequestTest < ActiveSupport::TestCase assert_equal({"bar" => 2}, request.query_parameters) end + test "formats" do + request = stub_request 'HTTP_ACCEPT' => 'text/html' + request.expects(:parameters).at_least_once.returns({}) + assert_equal [ Mime::HTML ], request.formats + + request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8' + request.expects(:parameters).at_least_once.returns({}) + assert_equal with_set(Mime::XML, Mime::HTML), request.formats + + begin + ActionController::Base.use_accept_header, old = + false, ActionController::Base.use_accept_header + + request = stub_request + request.expects(:parameters).at_least_once.returns({ :format => :txt }) + assert_equal with_set(Mime::TEXT), request.formats + ensure + ActionController::Base.use_accept_header = old + end + end + + test "negotiate_mime" do + request = stub_request 'HTTP_ACCEPT' => 'text/html' + request.expects(:parameters).at_least_once.returns({}) + + assert_equal nil, request.negotiate_mime([Mime::XML, Mime::JSON]) + assert_equal Mime::HTML, request.negotiate_mime([Mime::XML, Mime::HTML]) + assert_equal Mime::HTML, request.negotiate_mime([Mime::XML, Mime::ALL]) + + request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8' + request.expects(:parameters).at_least_once.returns({}) + assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV]) + assert_equal Mime::CSV, request.negotiate_mime([Mime::CSV, Mime::YAML]) + end + protected def stub_request(env={}) ActionDispatch::Request.new(env) end + def with_set(*args) + args + Mime::SET + end + end -- cgit v1.2.3 From 67b2d08c0a64ddec3a0c4e1c0b5d96bd418cceef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 10:09:21 +0200 Subject: Ensure that the proper accept header value is set during tests. Signed-off-by: Yehuda Katz --- .../lib/action_controller/base/mime_responds.rb | 4 +- actionpack/test/dispatch/request_test.rb | 56 +++++++++++----------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index dff4d7b943..ece4920a23 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -146,12 +146,12 @@ module ActionController #:nodoc: # # Mime::Type.register "image/jpg", :jpg def respond_to(*mimes, &block) + raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given? responder = Responder.new - block.call(responder) if block_given? mimes = collect_mimes_from_class_level if mimes.empty? - mimes.each { |mime| responder.send(mime) } + mimes.each { |mime| responder.custom(mime) } if format = request.negotiate_mime(responder.order) # TODO It should be just: self.formats = [ :foo ] diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 948eeeb001..8ebf9aa186 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -338,16 +338,11 @@ class RequestTest < ActiveSupport::TestCase end test "XMLHttpRequest" do - begin - ActionController::Base.use_accept_header, old = - false, ActionController::Base.use_accept_header - + with_accept_header false do request = stub_request 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest' request.expects(:parameters).at_least_once.returns({}) assert request.xhr? assert_equal Mime::JS, request.format - ensure - ActionController::Base.use_accept_header = old end end @@ -396,39 +391,38 @@ class RequestTest < ActiveSupport::TestCase assert_equal({"bar" => 2}, request.query_parameters) end - test "formats" do - request = stub_request 'HTTP_ACCEPT' => 'text/html' - request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::HTML ], request.formats - - request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8' - request.expects(:parameters).at_least_once.returns({}) - assert_equal with_set(Mime::XML, Mime::HTML), request.formats + test "formats with accept header" do + with_accept_header true do + request = stub_request 'HTTP_ACCEPT' => 'text/html' + request.expects(:parameters).at_least_once.returns({}) + assert_equal [ Mime::HTML ], request.formats - begin - ActionController::Base.use_accept_header, old = - false, ActionController::Base.use_accept_header + request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8' + request.expects(:parameters).at_least_once.returns({}) + assert_equal with_set(Mime::XML, Mime::HTML), request.formats + end + with_accept_header false do request = stub_request request.expects(:parameters).at_least_once.returns({ :format => :txt }) assert_equal with_set(Mime::TEXT), request.formats - ensure - ActionController::Base.use_accept_header = old end end test "negotiate_mime" do - request = stub_request 'HTTP_ACCEPT' => 'text/html' - request.expects(:parameters).at_least_once.returns({}) + with_accept_header true do + request = stub_request 'HTTP_ACCEPT' => 'text/html' + request.expects(:parameters).at_least_once.returns({}) - assert_equal nil, request.negotiate_mime([Mime::XML, Mime::JSON]) - assert_equal Mime::HTML, request.negotiate_mime([Mime::XML, Mime::HTML]) - assert_equal Mime::HTML, request.negotiate_mime([Mime::XML, Mime::ALL]) + assert_equal nil, request.negotiate_mime([Mime::XML, Mime::JSON]) + assert_equal Mime::HTML, request.negotiate_mime([Mime::XML, Mime::HTML]) + assert_equal Mime::HTML, request.negotiate_mime([Mime::XML, Mime::ALL]) - request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8' - request.expects(:parameters).at_least_once.returns({}) - assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV]) - assert_equal Mime::CSV, request.negotiate_mime([Mime::CSV, Mime::YAML]) + request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8' + request.expects(:parameters).at_least_once.returns({}) + assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV]) + assert_equal Mime::CSV, request.negotiate_mime([Mime::CSV, Mime::YAML]) + end end protected @@ -441,4 +435,10 @@ protected args + Mime::SET end + def with_accept_header(value) + ActionController::Base.use_accept_header, old = value, ActionController::Base.use_accept_header + yield + ensure + ActionController::Base.use_accept_header = old + end end -- cgit v1.2.3 From bbe86077c2148559f7548520303b2e683ff86119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 11:13:08 +0200 Subject: Added tests for respond_to class method. Signed-off-by: Yehuda Katz --- .../lib/action_controller/base/mime_responds.rb | 2 +- actionpack/test/controller/mime_responds_test.rb | 70 +++++++++++++++++++++- .../fixtures/respond_with/using_defaults.html.erb | 1 + .../fixtures/respond_with/using_defaults.js.rjs | 1 + .../using_defaults_with_type_list.js.rjs | 1 + .../using_defaults_with_type_list.xml.builder | 1 + 6 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 actionpack/test/fixtures/respond_with/using_defaults.html.erb create mode 100644 actionpack/test/fixtures/respond_with/using_defaults.js.rjs create mode 100644 actionpack/test/fixtures/respond_with/using_defaults_with_type_list.js.rjs create mode 100644 actionpack/test/fixtures/respond_with/using_defaults_with_type_list.xml.builder diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index ece4920a23..0ce6660c98 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -151,7 +151,7 @@ module ActionController #:nodoc: block.call(responder) if block_given? mimes = collect_mimes_from_class_level if mimes.empty? - mimes.each { |mime| responder.custom(mime) } + mimes.each { |mime| responder.send(mime) } if format = request.negotiate_mime(responder.order) # TODO It should be just: self.formats = [ :foo ] diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index f2c20417f8..48b343272e 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -166,7 +166,7 @@ class RespondToController < ActionController::Base end end -class MimeControllerTest < ActionController::TestCase +class RespondToControllerTest < ActionController::TestCase tests RespondToController def setup @@ -471,8 +471,74 @@ class MimeControllerTest < ActionController::TestCase end end -class ClassRespondToController < ActionController::Base +class RespondWithController < ActionController::Base + respond_to :html + respond_to :xml, :except => :using_defaults + respond_to :js, :only => :using_defaults + def using_defaults + respond_to do |format| + format.csv { render :text => "CSV" } + end + end + + def using_defaults_with_type_list + respond_to(:js, :xml) + end +end + +class RespondWithControllerTest < ActionController::TestCase + tests RespondWithController + + def setup + super + ActionController::Base.use_accept_header = true + @request.host = "www.example.com" + end + + def teardown + super + ActionController::Base.use_accept_header = false + end + + def test_using_defaults + @request.accept = "*/*" + get :using_defaults + assert_equal "text/html", @response.content_type + assert_equal 'Hello world!', @response.body + + @request.accept = "text/csv" + get :using_defaults + assert_equal "text/csv", @response.content_type + assert_equal "CSV", @response.body + + @request.accept = "text/javascript" + get :using_defaults + assert_equal "text/javascript", @response.content_type + assert_equal '$("body").visualEffect("highlight");', @response.body + end + + def test_using_defaults_with_type_list + @request.accept = "*/*" + get :using_defaults_with_type_list + assert_equal "text/javascript", @response.content_type + assert_equal '$("body").visualEffect("highlight");', @response.body + + @request.accept = "application/xml" + get :using_defaults_with_type_list + assert_equal "application/xml", @response.content_type + assert_equal "

Hello world!

\n", @response.body + end + + def test_not_acceptable + @request.accept = "application/xml" + get :using_defaults + assert_equal 406, @response.status + + @request.accept = "text/html" + get :using_defaults_with_type_list + assert_equal 406, @response.status + end end class AbstractPostController < ActionController::Base diff --git a/actionpack/test/fixtures/respond_with/using_defaults.html.erb b/actionpack/test/fixtures/respond_with/using_defaults.html.erb new file mode 100644 index 0000000000..6769dd60bd --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_defaults.html.erb @@ -0,0 +1 @@ +Hello world! \ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_defaults.js.rjs b/actionpack/test/fixtures/respond_with/using_defaults.js.rjs new file mode 100644 index 0000000000..469fcd8e15 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_defaults.js.rjs @@ -0,0 +1 @@ +page[:body].visual_effect :highlight \ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_defaults_with_type_list.js.rjs b/actionpack/test/fixtures/respond_with/using_defaults_with_type_list.js.rjs new file mode 100644 index 0000000000..469fcd8e15 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_defaults_with_type_list.js.rjs @@ -0,0 +1 @@ +page[:body].visual_effect :highlight \ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_defaults_with_type_list.xml.builder b/actionpack/test/fixtures/respond_with/using_defaults_with_type_list.xml.builder new file mode 100644 index 0000000000..598d62e2fc --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_defaults_with_type_list.xml.builder @@ -0,0 +1 @@ +xml.p "Hello world!" \ No newline at end of file -- cgit v1.2.3 From 09de34ca56598ae5d0302a14715b2a11b6cc9845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 12:18:03 +0200 Subject: Added respond_with. Signed-off-by: Yehuda Katz --- .../lib/action_controller/base/mime_responds.rb | 165 +++++++++++++++++---- actionpack/test/controller/mime_responds_test.rb | 70 ++++++++- .../fixtures/respond_with/using_resource.html.erb | 1 + 3 files changed, 209 insertions(+), 27 deletions(-) create mode 100644 actionpack/test/fixtures/respond_with/using_resource.html.erb diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index 0ce6660c98..f7c1b071e7 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -145,8 +145,45 @@ module ActionController #:nodoc: # environment.rb as follows. # # Mime::Type.register "image/jpg", :jpg + # + # Respond to also allows you to specify a common block for different formats by using any: + # + # def index + # @people = Person.find(:all) + # + # respond_to do |format| + # format.html + # format.any(:xml, :json) { render request.format.to_sym => @people } + # end + # end + # + # In the example above, if the format is xml, it will render: + # + # render :xml => @people + # + # Or if the format is json: + # + # render :json => @people + # + # Since this is a common pattern, you can use the class method respond_to + # with the respond_with method to have the same results: + # + # class PeopleController < ApplicationController + # respond_to :html, :xml, :json + # + # def index + # @people = Person.find(:all) + # respond_with(@person) + # end + # end + # + # Be sure to check respond_with and respond_to documentation for more examples. + # def respond_to(*mimes, &block) + options = mimes.extract_options! raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given? + + resource = options.delete(:with) responder = Responder.new block.call(responder) if block_given? @@ -154,41 +191,117 @@ module ActionController #:nodoc: mimes.each { |mime| responder.send(mime) } if format = request.negotiate_mime(responder.order) - # TODO It should be just: self.formats = [ :foo ] - self.formats = [format.to_sym] - self.content_type = format - self.template.formats = [format.to_sym] + respond_to_block_or_template_or_resource(format, resource, + options, &responder.response_for(format)) + else + head :not_acceptable + end + end - if response = responder.response_for(format) - response.call + # respond_with allows you to respond an action with a given resource. It + # requires that you set your class with a :respond_to method with the + # formats allowed: + # + # class PeopleController < ApplicationController + # respond_to :html, :xml, :json + # + # def index + # @people = Person.find(:all) + # respond_with(@person) + # end + # end + # + # When a request comes with format :xml, the respond_with will first search + # for a template as person/index.xml, if the template is not available, it + # will see if the given resource responds to :to_xml. + # + # If neither are available, it will raise an error. + # + # Extra parameters given to respond_with are used when :to_format is invoked. + # This allows you to set status and location for several formats at the same + # time. Consider this restful controller response on create for both xml + # and json formats: + # + # class PeopleController < ApplicationController + # respond_to :xml, :json + # + # def create + # @person = Person.new(params[:person]) + # + # if @person.save + # respond_with(@person, :status => :ok, :location => person_url(@person)) + # else + # respond_with(@person.errors, :status => :unprocessable_entity) + # end + # end + # end + # + # Finally, respond_with also accepts blocks, as in respond_to. Let's take + # the same controller and create action above and add common html behavior: + # + # class PeopleController < ApplicationController + # respond_to :html, :xml, :json + # + # def create + # @person = Person.new(params[:person]) + # + # if @person.save + # options = { :status => :ok, :location => person_url(@person) } + # + # respond_with(@person, options) do |format| + # format.html { redirect_to options[:location] } + # end + # else + # respond_with(@person.errors, :status => :unprocessable_entity) do + # format.html { render :action => :new } + # end + # end + # end + # end + # + def respond_with(resource, options={}, &block) + respond_to(options.merge!(:with => resource), &block) + end + + protected + + def respond_to_block_or_template_or_resource(format, resource, options) + # TODO It should be just: self.formats = [ :foo ] + self.formats = [format.to_sym] + self.content_type = format + self.template.formats = [format.to_sym] + + return yield if block_given? + + begin + default_render + rescue ActionView::MissingTemplate => e + if resource && resource.respond_to?("to_#{format.to_sym}") + render options.merge(format.to_sym => resource) else - default_render + raise e end - else - head :not_acceptable end end - protected + # Collect mimes declared in the class method respond_to valid for the + # current action. + # + def collect_mimes_from_class_level #:nodoc: + action = action_name.to_sym - # Collect mimes declared in the class method respond_to valid for the - # current action. - # - def collect_mimes_from_class_level #:nodoc: - action = action_name.to_sym - - mimes_for_respond_to.keys.select do |mime| - config = mimes_for_respond_to[mime] - - if config[:except] - !config[:except].include?(action) - elsif config[:only] - config[:only].include?(action) - else - true - end + mimes_for_respond_to.keys.select do |mime| + config = mimes_for_respond_to[mime] + + if config[:except] + !config[:except].include?(action) + elsif config[:only] + config[:only].include?(action) + else + true end end + end class Responder #:nodoc: attr_accessor :order diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 48b343272e..369d683d23 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -471,8 +471,20 @@ class RespondToControllerTest < ActionController::TestCase end end +class RespondResource + undef_method :to_json + + def to_xml + "XML" + end + + def to_js + "JS" + end +end + class RespondWithController < ActionController::Base - respond_to :html + respond_to :html, :json respond_to :xml, :except => :using_defaults respond_to :js, :only => :using_defaults @@ -485,6 +497,23 @@ class RespondWithController < ActionController::Base def using_defaults_with_type_list respond_to(:js, :xml) end + + def using_resource + respond_with(RespondResource.new) + end + + def using_resource_with_options + respond_with(RespondResource.new, :status => :unprocessable_entity) do |format| + format.js + end + end + +protected + + def _render_js(js, options) + self.content_type ||= Mime::JS + self.response_body = js.respond_to?(:to_js) ? js.to_js : js + end end class RespondWithControllerTest < ActionController::TestCase @@ -530,6 +559,37 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "

Hello world!

\n", @response.body end + def test_using_resource + @request.accept = "text/html" + get :using_resource + assert_equal "text/html", @response.content_type + assert_equal "Hello world!", @response.body + + @request.accept = "application/xml" + get :using_resource + assert_equal "application/xml", @response.content_type + assert_equal "XML", @response.body + + @request.accept = "application/json" + assert_raise ActionView::MissingTemplate do + get :using_resource + end + end + + def test_using_resource_with_options + @request.accept = "application/xml" + get :using_resource_with_options + assert_equal "application/xml", @response.content_type + assert_equal 422, @response.status + assert_equal "XML", @response.body + + @request.accept = "text/javascript" + get :using_resource_with_options + assert_equal "text/javascript", @response.content_type + assert_equal 422, @response.status + assert_equal "JS", @response.body + end + def test_not_acceptable @request.accept = "application/xml" get :using_defaults @@ -538,6 +598,14 @@ class RespondWithControllerTest < ActionController::TestCase @request.accept = "text/html" get :using_defaults_with_type_list assert_equal 406, @response.status + + @request.accept = "application/json" + get :using_defaults_with_type_list + assert_equal 406, @response.status + + @request.accept = "text/javascript" + get :using_resource + assert_equal 406, @response.status end end diff --git a/actionpack/test/fixtures/respond_with/using_resource.html.erb b/actionpack/test/fixtures/respond_with/using_resource.html.erb new file mode 100644 index 0000000000..6769dd60bd --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_resource.html.erb @@ -0,0 +1 @@ +Hello world! \ No newline at end of file -- cgit v1.2.3 From fa0cf663fe6a6393a3ba117505703e587da4ddc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 14:32:56 +0200 Subject: Add a couple more tests to respond_with. Signed-off-by: Yehuda Katz --- .../lib/action_controller/base/mime_responds.rb | 19 +++++------ actionpack/test/controller/mime_responds_test.rb | 38 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index f7c1b071e7..5a70931941 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -4,7 +4,7 @@ module ActionController #:nodoc: included do class_inheritable_reader :mimes_for_respond_to - respond_to # Set mimes_for_respond_to hash + clear_respond_to end module ClassMethods @@ -31,25 +31,22 @@ module ActionController #:nodoc: # def respond_to(*mimes) options = mimes.extract_options! - mimes_hash = {} only_actions = Array(options.delete(:only)) except_actions = Array(options.delete(:except)) mimes.each do |mime| mime = mime.to_sym - mimes_hash[mime] = {} - mimes_hash[mime][:only] = only_actions unless only_actions.empty? - mimes_hash[mime][:except] = except_actions unless except_actions.empty? + mimes_for_respond_to[mime] = {} + mimes_for_respond_to[mime][:only] = only_actions unless only_actions.empty? + mimes_for_respond_to[mime][:except] = except_actions unless except_actions.empty? end - - write_inheritable_hash(:mimes_for_respond_to, mimes_hash) end # Clear all mimes in respond_to. # - def clear_respond_to! - mimes_for_respond_to.each { |k,v| mimes[k] = { :only => [] } } + def clear_respond_to + write_inheritable_attribute(:mimes_for_respond_to, ActiveSupport::OrderedHash.new) end end @@ -185,10 +182,10 @@ module ActionController #:nodoc: resource = options.delete(:with) responder = Responder.new - block.call(responder) if block_given? mimes = collect_mimes_from_class_level if mimes.empty? mimes.each { |mime| responder.send(mime) } + block.call(responder) if block_given? if format = request.negotiate_mime(responder.order) respond_to_block_or_template_or_resource(format, resource, @@ -276,7 +273,7 @@ module ActionController #:nodoc: begin default_render rescue ActionView::MissingTemplate => e - if resource && resource.respond_to?("to_#{format.to_sym}") + if resource && resource.respond_to?(:"to_#{format.to_sym}") render options.merge(format.to_sym => resource) else raise e diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 369d683d23..558fc47695 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -508,6 +508,12 @@ class RespondWithController < ActionController::Base end end + def default_overwritten + respond_to do |format| + format.html { render :text => "HTML" } + end + end + protected def _render_js(js, options) @@ -516,6 +522,17 @@ protected end end +class InheritedRespondWithController < RespondWithController + clear_respond_to + respond_to :xml, :json + + def index + respond_with(RespondResource.new) do |format| + format.json { render :text => "JSON" } + end + end +end + class RespondWithControllerTest < ActionController::TestCase tests RespondWithController @@ -590,6 +607,27 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "JS", @response.body end + def test_default_overwritten + get :default_overwritten + assert_equal "text/html", @response.content_type + assert_equal "HTML", @response.body + end + + def test_clear_respond_to + @controller = InheritedRespondWithController.new + @request.accept = "text/html" + get :index + assert_equal 406, @response.status + end + + def test_first_in_respond_to_has_higher_priority + @controller = InheritedRespondWithController.new + @request.accept = "*/*" + get :index + assert_equal "application/xml", @response.content_type + assert_equal "XML", @response.body + end + def test_not_acceptable @request.accept = "application/xml" get :using_defaults -- cgit v1.2.3 From d209aea7d88849b7c7a080d729045ce7c051ef1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Jul 2009 19:31:03 +0200 Subject: Remove last TODO. Signed-off-by: Yehuda Katz --- actionpack/lib/action_controller/base/mime_responds.rb | 4 ---- actionpack/lib/action_controller/base/renderer.rb | 3 +-- actionpack/lib/action_dispatch/http/mime_type.rb | 2 +- actionpack/test/controller/mime_responds_test.rb | 2 +- 4 files changed, 3 insertions(+), 8 deletions(-) diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index 5a70931941..f4a4007a43 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -263,11 +263,7 @@ module ActionController #:nodoc: protected def respond_to_block_or_template_or_resource(format, resource, options) - # TODO It should be just: self.formats = [ :foo ] self.formats = [format.to_sym] - self.content_type = format - self.template.formats = [format.to_sym] - return yield if block_given? begin diff --git a/actionpack/lib/action_controller/base/renderer.rb b/actionpack/lib/action_controller/base/renderer.rb index 2fab501302..572da451ff 100644 --- a/actionpack/lib/action_controller/base/renderer.rb +++ b/actionpack/lib/action_controller/base/renderer.rb @@ -11,11 +11,10 @@ module ActionController def render(options) super - options[:_template] ||= _action_view._partial self.content_type ||= begin mime = options[:_template].mime_type formats.include?(mime && mime.to_sym) || formats.include?(:all) ? mime : Mime::Type.lookup_by_extension(formats.first) - end + end.to_s response_body end diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 27f27e27fe..cc989d6625 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -84,7 +84,7 @@ module Mime end def lookup_by_extension(extension) - EXTENSION_LOOKUP[extension] + EXTENSION_LOOKUP[extension.to_s] end # Registers an alias that's not used on mime type lookup, but can be referenced directly. Especially useful for diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 558fc47695..117f4ea4f0 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -443,7 +443,7 @@ class RespondToControllerTest < ActionController::TestCase @action = args.first[:action] unless args.empty? @action ||= action_name - response.body = "#{@action} - #{@template.formats}" + response.body = "#{@action} - #{formats}" end end -- cgit v1.2.3