From 4d63e01fa052749f742e90bcb3889cbae6152c92 Mon Sep 17 00:00:00 2001
From: Rick Olson <technoweenie@gmail.com>
Date: Wed, 20 Sep 2006 19:31:17 +0000
Subject: [PATCH] fixed failing tests now that non-GET requests are sent with
 .xml file ext.  Extracted #id_from_response as an entry point for customizing
 how a created resource gets its own ID.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5153 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
---
 activeresource/CHANGELOG                   |  3 +++
 activeresource/lib/active_resource/base.rb |  7 ++++-
 activeresource/test/base_errors_test.rb    |  2 +-
 activeresource/test/base_test.rb           | 31 ++++++++++++++--------
 4 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/activeresource/CHANGELOG b/activeresource/CHANGELOG
index c81f7daacb..6906c17ac7 100644
--- a/activeresource/CHANGELOG
+++ b/activeresource/CHANGELOG
@@ -1,5 +1,8 @@
 *SVN*
 
+* Extracted #id_from_response as an entry point for customizing how a created resource gets its own ID.
+  By default, it extracts from the Location response header.
+
 * Optimistic locking: raise ActiveResource::ResourceConflict on 409 Conflict response. [Jeremy Kemper]
 
     # Example controller action
diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb
index 89724b1814..647fd90bb5 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -164,10 +164,15 @@ module ActiveResource
 
       def create
         returning connection.post(self.class.collection_path(prefix_options), to_xml) do |resp|
-          self.id = resp['Location'][/\/([^\/]*?)(\.\w+)?$/, 1]
+          self.id = id_from_response(resp)
         end
       end
 
+      # takes a response from a typical create post and pulls the ID out
+      def id_from_response(response)
+        response['Location'][/\/([^\/]*?)(\.\w+)?$/, 1]
+      end
+
     private
       def find_or_create_resource_for_collection(name)
         find_or_create_resource_for(name.to_s.singularize)
diff --git a/activeresource/test/base_errors_test.rb b/activeresource/test/base_errors_test.rb
index 25a368535f..5125adf218 100644
--- a/activeresource/test/base_errors_test.rb
+++ b/activeresource/test/base_errors_test.rb
@@ -4,7 +4,7 @@ require "fixtures/person"
 class BaseErrorsTest < Test::Unit::TestCase
   def setup
     ActiveResource::HttpMock.respond_to do |mock|
-      mock.post "/people", "<?xml version=\"1.0\" encoding=\"UTF-8\"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error></errors>", 400
+      mock.post "/people.xml", "<?xml version=\"1.0\" encoding=\"UTF-8\"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error></errors>", 400
     end
     @exception = nil
     @person    = Person.new(:name => '', :age => '')
diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb
index d8c654350b..2a3091ecba 100644
--- a/activeresource/test/base_test.rb
+++ b/activeresource/test/base_test.rb
@@ -10,22 +10,22 @@ class BaseTest < Test::Unit::TestCase
     ActiveResource::HttpMock.respond_to do |mock|
       mock.get    "/people/1.xml",             @matz
       mock.get    "/people/2.xml",             @david
-      mock.put    "/people/1",                 nil, 204
-      mock.delete "/people/1",                 nil, 200
-      mock.delete "/people/2",                 nil, 400
-      mock.post   "/people",                   nil, 201, 'Location' => '/people/5.xml'
+      mock.put    "/people/1.xml",             nil, 204
+      mock.delete "/people/1.xml",             nil, 200
+      mock.delete "/people/2.xml",             nil, 400
+      mock.post   "/people.xml",               nil, 201, 'Location' => '/people/5.xml'
       mock.get    "/people/99.xml",            nil, 404
       mock.get    "/people.xml",               "<people>#{@matz}#{@david}</people>"
       mock.get    "/people/1/addresses.xml",   "<addresses>#{@addy}</addresses>"
       mock.get    "/people/1/addresses/1.xml", @addy
-      mock.put    "/people/1/addresses/1",     nil, 204
-      mock.delete "/people/1/addresses/1",     nil, 200
-      mock.post   "/people/1/addresses",       nil, 201, 'Location' => '/people/1/addresses/5'
+      mock.put    "/people/1/addresses/1.xml", nil, 204
+      mock.delete "/people/1/addresses/1.xml", nil, 200
+      mock.post   "/people/1/addresses.xml",   nil, 201, 'Location' => '/people/1/addresses/5'
       mock.get    "/people//addresses.xml",    nil, 404
       mock.get    "/people//addresses/1.xml",  nil, 404
-      mock.put    "/people//addresses/1",      nil, 404
-      mock.delete "/people//addresses/1",      nil, 404
-      mock.post   "/people//addresses",        nil, 404
+      mock.put    "/people//addresses/1.xml",  nil, 404
+      mock.delete "/people//addresses/1.xml",  nil, 404
+      mock.post   "/people//addresses.xml",    nil, 404
     end
   end
 
@@ -111,6 +111,15 @@ class BaseTest < Test::Unit::TestCase
     assert_equal '5', rick.id
   end
 
+  def test_id_from_response
+    p = Person.new
+    resp = {'Location' => '/foo/bar/1'}
+    assert_equal '1', p.send(:id_from_response, resp)
+    
+    resp['Location'] << '.xml'
+    assert_equal '1', p.send(:id_from_response, resp)
+  end
+
   def test_create_with_custom_prefix
     matzs_house = StreetAddress.new({}, {:person_id => 1})
     matzs_house.save
@@ -136,7 +145,7 @@ class BaseTest < Test::Unit::TestCase
   def test_update_conflict
     ActiveResource::HttpMock.respond_to do |mock|
       mock.get "/people/2.xml", @david
-      mock.put "/people/2", nil, 409
+      mock.put "/people/2.xml", nil, 409
     end
     assert_raises(ActiveResource::ResourceConflict) { Person.find(2).save }
   end