From 7a4a679dbada2e1fcfc28db8c47dd32e03afc1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 30 Jul 2009 22:43:37 +0200 Subject: Remove any resource logic from respond_to. --- .../lib/action_controller/base/mime_responds.rb | 31 ++++++++++------------ 1 file changed, 14 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index f4a4007a43..485668d543 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -177,19 +177,21 @@ module ActionController #:nodoc: # 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 - 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, - options, &responder.response_for(format)) + self.formats = [format.to_sym] + + if response = responder.response_for(format) + response.call + else + default_render + end else head :not_acceptable end @@ -257,26 +259,21 @@ module ActionController #:nodoc: # 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) - self.formats = [format.to_sym] - return yield if block_given? - begin - default_render + respond_to(&block) rescue ActionView::MissingTemplate => e - if resource && resource.respond_to?(:"to_#{format.to_sym}") - render options.merge(format.to_sym => resource) + format = self.formats.first + + if resource.respond_to?(:"to_#{format}") + render options.merge(format => resource) else raise e end end end + protected + # Collect mimes declared in the class method respond_to valid for the # current action. # -- cgit v1.2.3 From 5b7e81efec649b424037c68a93bddad1bc4e0c23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 31 Jul 2009 11:59:05 +0200 Subject: Allow respond_with to deal with http verb accordingly. --- .../lib/action_controller/base/mime_responds.rb | 111 ++++++++++++++------- actionpack/test/controller/mime_responds_test.rb | 65 +++++++++++- 2 files changed, 136 insertions(+), 40 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base/mime_responds.rb b/actionpack/lib/action_controller/base/mime_responds.rb index 485668d543..9a6c8aa58b 100644 --- a/actionpack/lib/action_controller/base/mime_responds.rb +++ b/actionpack/lib/action_controller/base/mime_responds.rb @@ -202,11 +202,11 @@ module ActionController #:nodoc: # formats allowed: # # class PeopleController < ApplicationController - # respond_to :html, :xml, :json + # respond_to :xml, :json # # def index # @people = Person.find(:all) - # respond_with(@person) + # respond_with(@people) # end # end # @@ -216,64 +216,101 @@ module ActionController #:nodoc: # # 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: + # respond_with holds semantics for each HTTP verb. The example above cover + # GET requests. Let's check a POST request example: + # + # def create + # @person = Person.new(params[:person]) + # @person.save + # respond_with(@person) + # end + # + # Since the request is a POST, respond_with will check wether @people + # resource have errors or not. If it has errors, it will render the error + # object with unprocessable entity status (422). + # + # If no error was found, it will render the @people resource, with status + # created (201) and location header set to person_url(@people). + # + # If you also want to provide html behavior in the method above, you can + # supply a block to customize it: # # class PeopleController < ApplicationController - # respond_to :xml, :json + # respond_to :html, :xml, :json # Add :html to respond_to definition # # 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) + # @person = Person.new(params[:pe]) + # + # respond_with(@person) do |format| + # if @person.save + # flash[:notice] = 'Person was successfully created.' + # format.html { redirect_to @person } + # else + # format.html { render :action => "new" } + # end # 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: + # It works similarly for PUT requests: # - # class PeopleController < ApplicationController - # respond_to :html, :xml, :json + # def update + # @person = Person.find(params[:id]) + # @person.update_attributes(params[:person]) + # respond_with(@person) + # end # - # def create - # @person = Person.new(params[:person]) + # In case of failures, it works as POST requests, but in success failures + # it just reply status ok (200) to the client. # - # if @person.save - # options = { :status => :ok, :location => person_url(@person) } + # A DELETE request also works in the same way: # - # 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 + # def destroy + # @person = Person.find(params[:id]) + # @person.destroy + # respond_with(@person) # end # + # It just replies with status ok, indicating the record was successfuly + # destroyed. + # def respond_with(resource, options={}, &block) - begin - respond_to(&block) - rescue ActionView::MissingTemplate => e - format = self.formats.first + respond_to(&block) + rescue ActionView::MissingTemplate => e + format = self.formats.first + resource = normalize_resource_options_by_verb(resource, options) - if resource.respond_to?(:"to_#{format}") - render options.merge(format => resource) + if resource.respond_to?(:"to_#{format}") + if options.delete(:no_content) + head options else - raise e + render options.merge(format => resource) end + else + raise e end end protected + # Change respond with behavior based on the HTTP verb. + # + def normalize_resource_options_by_verb(resource_or_array, options) + resource = resource_or_array.is_a?(Array) ? resource_or_array.last : resource_or_array + has_errors = resource.respond_to?(:errors) && !resource.errors.empty? + + if has_errors && (request.post? || request.put?) + options.reverse_merge!(:status => :unprocessable_entity) + return resource.errors + elsif request.post? + options.reverse_merge!(:status => :created, :location => resource_or_array) + elsif !request.get? + options.reverse_merge!(:status => :ok, :no_content => true) + end + + return resource + end + # Collect mimes declared in the class method respond_to valid for the # current action. # diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 117f4ea4f0..1db951fdfe 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -474,6 +474,14 @@ end class RespondResource undef_method :to_json + def self.model_name + @_model_name ||= ActiveModel::Name.new(name) + end + + def to_param + 13 + end + def to_xml "XML" end @@ -481,6 +489,10 @@ class RespondResource def to_js "JS" end + + def errors + [] + end end class RespondWithController < ActionController::Base @@ -520,6 +532,10 @@ protected self.content_type ||= Mime::JS self.response_body = js.respond_to?(:to_js) ? js.to_js : js end + + def respond_resource_url(id) + request.host + "/respond/resource/#{id.to_param}" + end end class InheritedRespondWithController < RespondWithController @@ -593,6 +609,51 @@ class RespondWithControllerTest < ActionController::TestCase end end + def test_using_resource_for_post + @request.accept = "application/xml" + + post :using_resource + assert_equal "application/xml", @response.content_type + assert_equal 201, @response.status + assert_equal "XML", @response.body + assert_equal "www.example.com/respond/resource/13", @response.location + + errors = { :name => :invalid } + RespondResource.any_instance.stubs(:errors).returns(errors) + post :using_resource + assert_equal "application/xml", @response.content_type + assert_equal 422, @response.status + assert_equal errors.to_xml, @response.body + assert_nil @response.location + end + + def test_using_resource_for_put + @request.accept = "application/xml" + + put :using_resource + assert_equal "application/xml", @response.content_type + assert_equal 200, @response.status + assert_equal " ", @response.body + assert_nil @response.location + + errors = { :name => :invalid } + RespondResource.any_instance.stubs(:errors).returns(errors) + put :using_resource + assert_equal "application/xml", @response.content_type + assert_equal 422, @response.status + assert_equal errors.to_xml, @response.body + assert_nil @response.location + end + + def test_using_resource_for_delete + @request.accept = "application/xml" + delete :using_resource + assert_equal "application/xml", @response.content_type + assert_equal 200, @response.status + assert_equal " ", @response.body + assert_nil @response.location + end + def test_using_resource_with_options @request.accept = "application/xml" get :using_resource_with_options @@ -648,8 +709,6 @@ class RespondWithControllerTest < ActionController::TestCase end class AbstractPostController < ActionController::Base - respond_to :html, :iphone - self.view_paths = File.dirname(__FILE__) + "/../fixtures/post_test/" end @@ -658,7 +717,7 @@ class PostController < AbstractPostController around_filter :with_iphone def index - respond_to # It will use formats declared above + respond_to(:html, :iphone) end protected -- cgit v1.2.3 From b2d24baf790fee4f932fa32a8ae94f0212d14ad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 31 Jul 2009 20:56:53 +0200 Subject: Added tests for nested resources. --- actionpack/test/controller/mime_responds_test.rb | 60 +++++++++++++++++++----- 1 file changed, 48 insertions(+), 12 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 1db951fdfe..1d27e749ae 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -475,7 +475,7 @@ class RespondResource undef_method :to_json def self.model_name - @_model_name ||= ActiveModel::Name.new(name) + @_model_name ||= ActiveModel::Name.new("resource") end def to_param @@ -495,6 +495,16 @@ class RespondResource end end +class ParentResource + def self.model_name + @_model_name ||= ActiveModel::Name.new("parent") + end + + def to_param + 11 + end +end + class RespondWithController < ActionController::Base respond_to :html, :json respond_to :xml, :except => :using_defaults @@ -510,6 +520,12 @@ class RespondWithController < ActionController::Base respond_to(:js, :xml) end + def default_overwritten + respond_to do |format| + format.html { render :text => "HTML" } + end + end + def using_resource respond_with(RespondResource.new) end @@ -520,10 +536,8 @@ class RespondWithController < ActionController::Base end end - def default_overwritten - respond_to do |format| - format.html { render :text => "HTML" } - end + def using_resource_with_parent + respond_with([ParentResource.new, RespondResource.new]) end protected @@ -533,8 +547,12 @@ protected self.response_body = js.respond_to?(:to_js) ? js.to_js : js end - def respond_resource_url(id) - request.host + "/respond/resource/#{id.to_param}" + def resource_url(resource) + request.host + "/resource/#{resource.to_param}" + end + + def parent_resource_url(parent, resource) + request.host + "/parent/#{parent.to_param}/resource/#{resource.to_param}" end end @@ -592,6 +610,12 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "

Hello world!

\n", @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_using_resource @request.accept = "text/html" get :using_resource @@ -616,7 +640,7 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "application/xml", @response.content_type assert_equal 201, @response.status assert_equal "XML", @response.body - assert_equal "www.example.com/respond/resource/13", @response.location + assert_equal "www.example.com/resource/13", @response.location errors = { :name => :invalid } RespondResource.any_instance.stubs(:errors).returns(errors) @@ -668,10 +692,22 @@ 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 + def test_using_resource_with_parent + @request.accept = "application/xml" + + post :using_resource_with_parent + assert_equal "application/xml", @response.content_type + assert_equal 201, @response.status + assert_equal "XML", @response.body + assert_equal "www.example.com/parent/11/resource/13", @response.location + + errors = { :name => :invalid } + RespondResource.any_instance.stubs(:errors).returns(errors) + post :using_resource + assert_equal "application/xml", @response.content_type + assert_equal 422, @response.status + assert_equal errors.to_xml, @response.body + assert_nil @response.location end def test_clear_respond_to -- cgit v1.2.3