diff options
9 files changed, 215 insertions, 78 deletions
diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 6d35137428..8575d30335 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -55,14 +55,15 @@ module ActionController elsif args.empty? raise ArgumentError, "too few arguments to head" end - options = args.extract_options! - status = args.shift || options.delete(:status) || :ok + options = args.extract_options! + status = args.shift || options.delete(:status) || :ok + location = options.delete(:location) options.each do |key, value| headers[key.to_s.dasherize.split(/-/).map { |v| v.capitalize }.join("-")] = value.to_s end - render :nothing => true, :status => status + render :nothing => true, :status => status, :location => location end # Sets the etag and/or last_modified on the response and checks it against diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 9a6c8aa58b..837496e477 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -198,11 +198,11 @@ module ActionController #:nodoc: end # 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 + # requires that you set your class with a respond_to method with the # formats allowed: # # class PeopleController < ApplicationController - # respond_to :xml, :json + # respond_to :html, :xml, :json # # def index # @people = Person.find(:all) @@ -210,14 +210,43 @@ module ActionController #:nodoc: # 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. + # When a request comes, for example with format :xml, three steps happen: + # + # 1) respond_with searches for a template at people/index.xml; + # + # 2) if the template is not available, it will check if the given + # resource responds to :to_xml. + # + # 3) if a :location option was provided, redirect to the location with + # redirect status if a string was given, or render an action if a + # symbol was given. + # + # If all steps fail, a missing template error will be raised. + # + # === Supported options + # + # [status] + # Sets the response status. + # + # [head] + # Tell respond_with to set the content type, status and location header, + # but do not render the object, leaving the response body empty. This + # option only has effect if the resource is being rendered. If a + # template was found, it's going to be rendered anyway. + # + # [location] + # Sets the location header with the given value. It accepts a string, + # representing the location header value, or a symbol representing an + # action name. + # + # === Builtin HTTP verb semantics # - # If neither are available, it will raise an error. + # respond_with holds semantics for each HTTP verb. Depending on the verb + # and the resource status, respond_with will automatically set the options + # above. # - # respond_with holds semantics for each HTTP verb. The example above cover - # GET requests. Let's check a POST request example: + # Above we saw an example for GET requests, where actually no option is + # configured. A create action for POST requests, could be written as: # # def create # @person = Person.new(params[:person]) @@ -225,34 +254,40 @@ module ActionController #:nodoc: # 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). + # respond_with will inspect the @person object and check if we have any + # error. If errors are empty, it will add status and location to the options + # hash. Then the create action in case of success, is equivalent to this: # - # If no error was found, it will render the @people resource, with status - # created (201) and location header set to person_url(@people). + # respond_with(@person, :status => :created, :location => @person) # - # If you also want to provide html behavior in the method above, you can - # supply a block to customize it: + # From them on, the lookup happens as described above. Let's suppose a :xml + # request and we don't have a people/create.xml template. But since the + # @person object responds to :to_xml, it will render the newly created + # resource and set status and location. # - # class PeopleController < ApplicationController - # respond_to :html, :xml, :json # Add :html to respond_to definition - # - # def create - # @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 + # However, if the request is :html, a template is not available and @person + # does not respond to :to_html. But since a :location options was provided, + # it will redirect to it. + # + # In case of failures (when the @person could not be saved and errors are + # not empty), respond_with can be expanded as this: + # + # respond_with(@person.errors, :status => :unprocessable_entity, :location => :new) + # + # In other words, respond_with(@person) for POST requests is expanded + # internally into this: + # + # def create + # @person = Person.new(params[:person]) + # + # if @person.save + # respond_with(@person, :status => :created, :location => @person) + # else + # respond_with(@person.errors, :status => :unprocessable_entity, :location => :new) # end # end # - # It works similarly for PUT requests: + # For an update action for PUT requests, we would have: # # def update # @person = Person.find(params[:id]) @@ -260,10 +295,23 @@ module ActionController #:nodoc: # respond_with(@person) # end # - # In case of failures, it works as POST requests, but in success failures - # it just reply status ok (200) to the client. + # Which, in face of success and failure scenarios, can be expanded as: # - # A DELETE request also works in the same way: + # def update + # @person = Person.find(params[:id]) + # @person.update_attributes(params[:person]) + # + # if @person.save + # respond_with(@person, :status => :ok, :location => @person, :head => true) + # else + # respond_with(@person.errors, :status => :unprocessable_entity, :location => :edit) + # end + # end + # + # Notice that in case of success, we just need to reply :ok to the client. + # The option :head ensures that the object is not rendered. + # + # Finally, we have the destroy action with DELETE verb: # # def destroy # @person = Person.find(params[:id]) @@ -271,21 +319,30 @@ module ActionController #:nodoc: # respond_with(@person) # end # - # It just replies with status ok, indicating the record was successfuly - # destroyed. + # Which is expanded as: + # + # def destroy + # @person = Person.find(params[:id]) + # @person.destroy + # respond_with(@person, :status => :ok, :location => @person, :head => true) + # end + # + # In this case, since @person.destroyed? returns true, polymorphic urls will + # redirect to the collection url, instead of the resource url. # def respond_with(resource, options={}, &block) respond_to(&block) rescue ActionView::MissingTemplate => e format = self.formats.first resource = normalize_resource_options_by_verb(resource, options) + action = options.delete(:location) if options[:location].is_a?(Symbol) if resource.respond_to?(:"to_#{format}") - if options.delete(:no_content) - head options - else - render options.merge(format => resource) - end + options.delete(:head) ? head(options) : render(options.merge(format => resource)) + elsif action + render :action => action + elsif options[:location] + redirect_to options[:location] else raise e end @@ -296,16 +353,22 @@ module ActionController #:nodoc: # 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? + resource = resource_or_array.is_a?(Array) ? resource_or_array.last : resource_or_array - if has_errors && (request.post? || request.put?) - options.reverse_merge!(:status => :unprocessable_entity) + if resource.respond_to?(:errors) && !resource.errors.empty? + options[:status] ||= :unprocessable_entity + options[:location] ||= :new if request.post? + options[:location] ||= :edit if request.put? 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) + options[:location] ||= resource_or_array + + if request.post? + options[:status] ||= :created + else + options[:status] ||= :ok + options[:head] = true unless options.key?(:head) + end end return resource diff --git a/actionpack/lib/action_controller/routing/generation/polymorphic_routes.rb b/actionpack/lib/action_controller/routing/generation/polymorphic_routes.rb index 159d869a54..ee44160274 100644 --- a/actionpack/lib/action_controller/routing/generation/polymorphic_routes.rb +++ b/actionpack/lib/action_controller/routing/generation/polymorphic_routes.rb @@ -86,17 +86,16 @@ module ActionController else [ record_or_hash_or_array ] end - inflection = - case - when options[:action].to_s == "new" - args.pop - :singular - when record.respond_to?(:new_record?) && record.new_record? - args.pop - :plural - else - :singular - end + inflection = if options[:action].to_s == "new" + args.pop + :singular + elsif (record.respond_to?(:new_record?) && record.new_record?) || + (record.respond_to?(:destroyed?) && record.destroyed?) + args.pop + :plural + else + :singular + end args.delete_if {|arg| arg.is_a?(Symbol) || arg.is_a?(String)} diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 1d27e749ae..e9c8a3c10f 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -493,6 +493,10 @@ class RespondResource def errors [] end + + def destroyed? + false + end end class ParentResource @@ -508,7 +512,7 @@ end class RespondWithController < ActionController::Base respond_to :html, :json respond_to :xml, :except => :using_defaults - respond_to :js, :only => :using_defaults + respond_to :js, :only => [ :using_defaults, :using_resource ] def using_defaults respond_to do |format| @@ -547,12 +551,16 @@ protected self.response_body = js.respond_to?(:to_js) ? js.to_js : js end + def resources_url + request.host + "/resources" + end + def resource_url(resource) - request.host + "/resource/#{resource.to_param}" + request.host + "/resources/#{resource.to_param}" end def parent_resource_url(parent, resource) - request.host + "/parent/#{parent.to_param}/resource/#{resource.to_param}" + request.host + "/parents/#{parent.to_param}/resources/#{resource.to_param}" end end @@ -617,10 +625,10 @@ class RespondWithControllerTest < ActionController::TestCase end def test_using_resource - @request.accept = "text/html" + @request.accept = "text/javascript" get :using_resource - assert_equal "text/html", @response.content_type - assert_equal "Hello world!", @response.body + assert_equal "text/javascript", @response.content_type + assert_equal '$("body").visualEffect("highlight");', @response.body @request.accept = "application/xml" get :using_resource @@ -633,14 +641,30 @@ class RespondWithControllerTest < ActionController::TestCase end end - def test_using_resource_for_post + def test_using_resource_for_post_with_html + post :using_resource + assert_equal "text/html", @response.content_type + assert_equal 302, @response.status + assert_equal "www.example.com/resources/13", @response.location + assert @response.redirect? + + errors = { :name => :invalid } + RespondResource.any_instance.stubs(:errors).returns(errors) + post :using_resource + assert_equal "text/html", @response.content_type + assert_equal 200, @response.status + assert_equal "New world!\n", @response.body + assert_nil @response.location + end + + def test_using_resource_for_post_with_xml @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/resource/13", @response.location + assert_equal "www.example.com/resources/13", @response.location errors = { :name => :invalid } RespondResource.any_instance.stubs(:errors).returns(errors) @@ -651,14 +675,30 @@ class RespondWithControllerTest < ActionController::TestCase assert_nil @response.location end - def test_using_resource_for_put + def test_using_resource_for_put_with_html + put :using_resource + assert_equal "text/html", @response.content_type + assert_equal 302, @response.status + assert_equal "www.example.com/resources/13", @response.location + assert @response.redirect? + + errors = { :name => :invalid } + RespondResource.any_instance.stubs(:errors).returns(errors) + put :using_resource + assert_equal "text/html", @response.content_type + assert_equal 200, @response.status + assert_equal "Edit world!\n", @response.body + assert_nil @response.location + end + + def test_using_resource_for_put_with_xml @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 + assert_equal "www.example.com/resources/13", @response.location errors = { :name => :invalid } RespondResource.any_instance.stubs(:errors).returns(errors) @@ -666,16 +706,25 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "application/xml", @response.content_type assert_equal 422, @response.status assert_equal errors.to_xml, @response.body - assert_nil @response.location + assert_nil @response.location end - def test_using_resource_for_delete + def test_using_resource_for_delete_with_html + RespondResource.any_instance.stubs(:destroyed?).returns(true) + delete :using_resource + assert_equal "text/html", @response.content_type + assert_equal 302, @response.status + assert_equal "www.example.com/resources", @response.location + end + + def test_using_resource_for_delete_with_xml + RespondResource.any_instance.stubs(:destroyed?).returns(true) @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 + assert_equal "www.example.com/resources", @response.location end def test_using_resource_with_options @@ -692,14 +741,22 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal "JS", @response.body end - def test_using_resource_with_parent + def test_using_resource_with_parent_for_get + @request.accept = "application/xml" + get :using_resource_with_parent + assert_equal "application/xml", @response.content_type + assert_equal 200, @response.status + assert_equal "XML", @response.body + end + + def test_using_resource_with_parent_for_post @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 + assert_equal "www.example.com/parents/11/resources/13", @response.location errors = { :name => :invalid } RespondResource.any_instance.stubs(:errors).returns(errors) @@ -739,7 +796,7 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal 406, @response.status @request.accept = "text/javascript" - get :using_resource + get :default_overwritten assert_equal 406, @response.status end end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 947ffa9ea6..f2cd6dbb85 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -458,6 +458,10 @@ class TestController < ActionController::Base head :location => "/foo" end + def head_with_location_object + head :location => Customer.new("david") + end + def head_with_symbolic_status head :status => params[:status].intern end @@ -618,6 +622,10 @@ class TestController < ActionController::Base end private + def customer_url(customer) + request.host + "/customers/1" + end + def determine_layout case action_name when "hello_world", "layout_test", "rendering_without_layout", @@ -1084,6 +1092,13 @@ class RenderTest < ActionController::TestCase assert_response :ok end + def test_head_with_location_object + get :head_with_location_object + assert @response.body.blank? + assert_equal "www.nextangle.com/customers/1", @response.headers["Location"] + assert_response :ok + end + def test_head_with_custom_header get :head_with_custom_header assert @response.body.blank? diff --git a/actionpack/test/fixtures/respond_with/edit.html.erb b/actionpack/test/fixtures/respond_with/edit.html.erb new file mode 100644 index 0000000000..ae82dfa4fc --- /dev/null +++ b/actionpack/test/fixtures/respond_with/edit.html.erb @@ -0,0 +1 @@ +Edit world! diff --git a/actionpack/test/fixtures/respond_with/new.html.erb b/actionpack/test/fixtures/respond_with/new.html.erb new file mode 100644 index 0000000000..96c8f1b88b --- /dev/null +++ b/actionpack/test/fixtures/respond_with/new.html.erb @@ -0,0 +1 @@ +New world! diff --git a/actionpack/test/fixtures/respond_with/using_resource.html.erb b/actionpack/test/fixtures/respond_with/using_resource.html.erb deleted file mode 100644 index 6769dd60bd..0000000000 --- a/actionpack/test/fixtures/respond_with/using_resource.html.erb +++ /dev/null @@ -1 +0,0 @@ -Hello world!
\ No newline at end of file diff --git a/actionpack/test/fixtures/respond_with/using_resource.js.rjs b/actionpack/test/fixtures/respond_with/using_resource.js.rjs new file mode 100644 index 0000000000..737c175a4e --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_resource.js.rjs @@ -0,0 +1 @@ +page[:body].visual_effect :highlight |