Merge commit 'mainstream/master'

6 files changed, 131 insertions, 28 deletions

diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb
index e1f221bd3e..803c6be53b 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -1,6 +1,7 @@
 require 'active_support'
 require 'active_support/core_ext/class/attribute_accessors'
 require 'active_support/core_ext/class/inheritable_attributes'
+require 'active_support/core_ext/hash/indifferent_access'
 require 'active_support/core_ext/kernel/reporting'
 require 'active_support/core_ext/module/attr_accessor_with_default'
 require 'active_support/core_ext/module/delegation'
@@ -162,7 +163,7 @@ module ActiveResource
   #
   # <tt>404</tt> is just one of the HTTP error response codes that Active Resource will handle with its own exception. The
   # following HTTP response codes will also result in these exceptions:
-  # 
+  #
   # * 200..399 - Valid response, no exception (other than 301, 302)
   # * 301, 302 - ActiveResource::Redirection
   # * 400 - ActiveResource::BadRequest
@@ -421,7 +422,7 @@ module ActiveResource
 
       attr_accessor_with_default(:collection_name) { ActiveSupport::Inflector.pluralize(element_name) } #:nodoc:
       attr_accessor_with_default(:primary_key, 'id') #:nodoc:
-      
+
       # Gets the \prefix for a resource's nested URL (e.g., <tt>prefix/collectionname/1.xml</tt>)
       # This method is regenerated at runtime based on what the \prefix is set to.
       def prefix(options={})
@@ -777,7 +778,7 @@ module ActiveResource
     #   my_other_course = Course.new(:name => "Philosophy: Reason and Being", :lecturer => "Ralph Cling")
     #   my_other_course.save
     def initialize(attributes = {})
-      @attributes     = {}
+      @attributes     = {}.with_indifferent_access
       @prefix_options = {}
       load(attributes)
     end
@@ -922,7 +923,7 @@ module ActiveResource
     def save
       new? ? create : update
     end
-    
+
     # Saves the resource.
     #
     # If the resource is new, it is created via +POST+, otherwise the
@@ -931,7 +932,7 @@ module ActiveResource
     # With <tt>save!</tt> validations always run. If any of them fail
     # ActiveResource::ResourceInvalid gets raised, and nothing is POSTed to
     # the remote system.
-    # See ActiveResource::Validations for more information. 
+    # See ActiveResource::Validations for more information.
     #
     # There's a series of callbacks associated with <tt>save!</tt>. If any
     # of the <tt>before_*</tt> callbacks return +false+ the action is
@@ -1106,6 +1107,36 @@ module ActiveResource
       self
     end
 
+    # Updates a single attribute and then saves the object.
+    #
+    # Note: Unlike ActiveRecord::Base.update_attribute, this method <b>is</b>
+    # subject to normal validation routines as an update sends the whole body
+    # of the resource in the request.  (See Validations).
+    #
+    # As such, this method is equivalent to calling update_attributes with a single attribute/value pair.
+    #
+    # If the saving fails because of a connection or remote service error, an
+    # exception will be raised.  If saving fails because the resource is
+    # invalid then <tt>false</tt> will be returned.
+    def update_attribute(name, value)
+      self.send("#{name}=".to_sym, value)
+      self.save
+    end
+
+    # Updates this resource with all the attributes from the passed-in Hash
+    # and requests that the record be saved.
+    #
+    # If the saving fails because of a connection or remote service error, an
+    # exception will be raised.  If saving fails because the resource is
+    # invalid then <tt>false</tt> will be returned.
+    #
+    # Note: Though this request can be made with a partial set of the
+    # resource's attributes, the full body of the request will still be sent
+    # in the save request to the remote service.
+    def update_attributes(attributes)
+      load(attributes) && save
+    end
+
     # For checking <tt>respond_to?</tt> without searching the attributes (which is faster).
     alias_method :respond_to_without_attributes?, :respond_to?
 
@@ -1126,7 +1157,6 @@ module ActiveResource
       super
     end
 
-
     protected
       def connection(refresh = false)
         self.class.connection(refresh)
diff --git a/activeresource/lib/active_resource/exceptions.rb b/activeresource/lib/active_resource/exceptions.rb
index 0631cdcf9f..0f4549fd73 100644
--- a/activeresource/lib/active_resource/exceptions.rb
+++ b/activeresource/lib/active_resource/exceptions.rb
@@ -8,7 +8,10 @@ module ActiveResource
     end
 
     def to_s
-      "Failed with #{response.code} #{response.message if response.respond_to?(:message)}"
+      message = "Failed."
+      message << "  Response code = #{response.code}." if response.respond_to?(:code)
+      message << "  Response message = #{response.message}." if response.respond_to?(:message)
+      message
     end
   end
 
diff --git a/activeresource/test/cases/base/load_test.rb b/activeresource/test/cases/base/load_test.rb
index 1952f5b5f0..189a4d81fe 100644
--- a/activeresource/test/cases/base/load_test.rb
+++ b/activeresource/test/cases/base/load_test.rb
@@ -15,26 +15,21 @@ module Highrise
 
   module Deeply
     module Nested
-
       class Note < ActiveResource::Base
         self.site = "http://37s.sunrise.i:3000"
       end
 
-       class Comment < ActiveResource::Base
-         self.site = "http://37s.sunrise.i:3000"
-       end
-
-       module TestDifferentLevels
-
-         class Note < ActiveResource::Base
-           self.site = "http://37s.sunrise.i:3000"
-         end
-
-       end
+      class Comment < ActiveResource::Base
+        self.site = "http://37s.sunrise.i:3000"
+      end
 
+      module TestDifferentLevels
+        class Note < ActiveResource::Base
+          self.site = "http://37s.sunrise.i:3000"
+        end
+      end
     end
   end
-
 end
 
 
@@ -68,6 +63,19 @@ class BaseLoadTest < Test::Unit::TestCase
     assert_equal @matz.stringify_keys, @person.load(@matz).attributes
   end
 
+  def test_after_load_attributes_are_accessible
+    assert_equal Hash.new, @person.attributes
+    assert_equal @matz.stringify_keys, @person.load(@matz).attributes
+    assert_equal @matz[:name], @person.attributes['name']
+  end
+
+  def test_after_load_attributes_are_accessible_via_indifferent_access
+    assert_equal Hash.new, @person.attributes
+    assert_equal @matz.stringify_keys, @person.load(@matz).attributes
+    assert_equal @matz[:name], @person.attributes['name']
+    assert_equal @matz[:name], @person.attributes[:name]
+  end
+
   def test_load_one_with_existing_resource
     address = @person.load(:street_address => @first_address).street_address
     assert_kind_of StreetAddress, address
@@ -143,7 +151,7 @@ class BaseLoadTest < Test::Unit::TestCase
     assert_kind_of String, places.first
     assert_equal @deep[:street][:state][:places].first, places.first
   end
-  
+
   def test_nested_collections_within_the_same_namespace
     n = Highrise::Note.new(:comments => [{ :name => "1" }])
     assert_kind_of Highrise::Comment, n.comments.first
@@ -158,6 +166,4 @@ class BaseLoadTest < Test::Unit::TestCase
     n = Highrise::Deeply::Nested::TestDifferentLevels::Note.new(:comments => [{ :name => "1" }])
     assert_kind_of Highrise::Deeply::Nested::Comment, n.comments.first
   end
-
-
 end
diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb
index 8c0217aad6..1593e25595 100644
--- a/activeresource/test/cases/base_test.rb
+++ b/activeresource/test/cases/base_test.rb
@@ -11,12 +11,12 @@ class BaseTest < Test::Unit::TestCase
     @matz  = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person')
     @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person')
     @greg  = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person')
-    @addy  = { :id => 1, :street => '12345 Street' }.to_xml(:root => 'address')
+    @addy  = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address')
     @default_request_headers = { 'Content-Type' => 'application/xml' }
     @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person")
     @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people')
     @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people')
-    @addresses = [{ :id => 1, :street => '12345 Street' }].to_xml(:root => 'addresses')
+    @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses')
 
     # - deep nested resource -
     # - Luis (Customer)
@@ -102,6 +102,9 @@ class BaseTest < Test::Unit::TestCase
     Person.password = nil
   end
 
+  ########################################################################
+  # Tests relating to setting up the API-connection configuration
+  ########################################################################
 
   def test_site_accessor_accepts_uri_or_string_argument
     site = URI.parse('http://localhost')
@@ -509,6 +512,11 @@ class BaseTest < Test::Unit::TestCase
     assert_not_equal(first_connection, second_connection, 'Connection should be re-created')
   end
 
+
+  ########################################################################
+  # Tests for setting up remote URLs for a given model (including adding
+  # parameters appropriately)
+  ########################################################################
   def test_collection_name
     assert_equal "people", Person.collection_name
   end
@@ -637,6 +645,10 @@ class BaseTest < Test::Unit::TestCase
     assert_equal [:person_id].to_set, StreetAddress.__send__(:prefix_parameters)
   end
 
+
+  ########################################################################
+  # Tests basic CRUD functions (find/save/create etc)
+  ########################################################################
   def test_respond_to
     matz = Person.find(1)
     assert matz.respond_to?(:name)
@@ -813,6 +825,55 @@ class BaseTest < Test::Unit::TestCase
     assert_raise(ActiveResource::ResourceConflict) { Person.find(2).save }
   end
 
+
+  ######
+  # update_attribute(s)(!)
+
+  def test_update_attribute_as_symbol
+    matz = Person.first
+    matz.expects(:save).returns(true)
+
+    assert_equal "Matz", matz.name
+    assert matz.update_attribute(:name, "David")
+    assert_equal "David", matz.name
+  end
+
+  def test_update_attribute_as_string
+    matz = Person.first
+    matz.expects(:save).returns(true)
+
+    assert_equal "Matz", matz.name
+    assert matz.update_attribute('name', "David")
+    assert_equal "David", matz.name
+  end
+
+
+  def test_update_attributes_as_symbols
+    addy = StreetAddress.first(:params => {:person_id => 1})
+    addy.expects(:save).returns(true)
+
+    assert_equal "12345 Street", addy.street
+    assert_equal "Australia", addy.country
+    assert addy.update_attributes(:street => '54321 Street', :country => 'USA')
+    assert_equal "54321 Street", addy.street
+    assert_equal "USA", addy.country
+  end
+
+  def test_update_attributes_as_strings
+    addy = StreetAddress.first(:params => {:person_id => 1})
+    addy.expects(:save).returns(true)
+
+    assert_equal "12345 Street", addy.street
+    assert_equal "Australia", addy.country
+    assert addy.update_attributes('street' => '54321 Street', 'country' => 'USA')
+    assert_equal "54321 Street", addy.street
+    assert_equal "USA", addy.country
+  end
+
+
+  #####
+  # Mayhem and destruction
+
   def test_destroy
     assert Person.find(1).destroy
     ActiveResource::HttpMock.respond_to do |mock|
@@ -852,7 +913,7 @@ class BaseTest < Test::Unit::TestCase
     end
     assert_raise(ActiveResource::ResourceNotFound) { StreetAddress.find(1, :params => { :person_id => 1 }) }
   end
-  
+
   def test_delete_with_410_gone
     assert Person.delete(1)
     ActiveResource::HttpMock.respond_to do |mock|
@@ -861,6 +922,9 @@ class BaseTest < Test::Unit::TestCase
     assert_raise(ActiveResource::ResourceGone) { Person.find(1) }
   end
 
+  ########################################################################
+  # Tests the more miscelaneous helper methods
+  ########################################################################
   def test_exists
     # Class method.
     assert !Person.exists?(nil)
diff --git a/activeresource/test/cases/observing_test.rb b/activeresource/test/cases/observing_test.rb
index 9599ff7b0f..925ec7a84a 100644
--- a/activeresource/test/cases/observing_test.rb
+++ b/activeresource/test/cases/observing_test.rb
@@ -10,7 +10,7 @@ class ObservingTest < Test::Unit::TestCase
 
     %w( after_create after_destroy after_save after_update
         before_create before_destroy before_save before_update).each do |method|
-          define_method(method) { log method }
+          define_method(method) { |*| log method }
     end
 
     private
diff --git a/activeresource/test/connection_test.rb b/activeresource/test/connection_test.rb
index d7466c65b4..2a3e04272a 100644
--- a/activeresource/test/connection_test.rb
+++ b/activeresource/test/connection_test.rb
@@ -83,7 +83,7 @@ class ConnectionTest < Test::Unit::TestCase
     begin
       handle_response ResponseHeaderStub.new(405, "HTTP Failed...", "GET, POST")
     rescue ActiveResource::MethodNotAllowed => e
-      assert_equal "Failed with 405 HTTP Failed...", e.message
+      assert_equal "Failed.  Response code = 405.  Response message = HTTP Failed....", e.message
       assert_equal [:get, :post], e.allowed_methods
     end
   end