Merge pull request #458 from stripe/brandur-generalize-save-nested-resource

Generalize saving nested resources

lib/stripe/account.rb

@@ -1,10 +1,17 @@
 module Stripe
   class Account < APIResource
+    extend Gem::Deprecate
     extend Stripe::APIOperations::Create
     extend Stripe::APIOperations::List
     include Stripe::APIOperations::Delete
     include Stripe::APIOperations::Save
 
+    save_nested_resource :external_account
+
+    # This method is deprecated. Please use `#external_account=` instead.
+    save_nested_resource :bank_account
+    deprecate :bank_account=, "#external_account=", 2017, 8
+
     def resource_url
       if self['id']
         super
@@ -28,23 +35,6 @@ module Stripe
       super(id, opts)
     end
 
-    # Set or replace an account's external account.
-    def external_account=(value)
-      super
-
-      # The parent setter will perform certain useful operations like
-      # converting to an APIResource if appropriate.
-      value = self.external_account
-
-      # Note that external_account may be a card, but could also be a tokenized
-      # card's ID (which is a string), and so we check its type here.
-      if value.is_a?(APIResource)
-        value.save_with_parent = true
-      end
-
-      value
-    end
-
     def reject(params={}, opts={})
       opts = Util.normalize_opts(opts)
       response, opts = request(:post, resource_url + '/reject', params, opts)
@@ -110,22 +100,6 @@ module Stripe
 
     ARGUMENT_NOT_PROVIDED = Object.new
 
-    # Set or replace an account's bank account.
-    #
-    # This method is deprecated. Please use #external_account instead.
-    def bank_account=(value)
-      super
-      value = self.bank_account
-
-      if value.is_a?(APIResource)
-        value.save_with_parent = true
-      end
-
-      value
-    end
-    extend Gem::Deprecate
-    deprecate :bank_account=, "#external_account=", 2017, 8
-
     private
 
     def serialize_additional_owners(legal_entity, additional_owners)

lib/stripe/api_resource.rb

@@ -20,6 +20,32 @@ module Stripe
       "/v1/#{CGI.escape(class_name.downcase)}s"
     end
 
+    # A metaprogramming call that specifies that a field of a resource can be
+    # its own type of API resource (say a nested card under an account for
+    # example), and if that resource is set, it should be transmitted to the
+    # API on a create or update. Doing so is not the default behavior because
+    # API resources should normally be persisted on their own RESTful
+    # endpoints.
+    def self.save_nested_resource(name)
+      define_method(:"#{name}=") do |value|
+        super(value)
+
+        # The parent setter will perform certain useful operations like
+        # converting to an APIResource if appropriate. Refresh our argument
+        # value to whatever it mutated to.
+        value = send(name)
+
+        # Note that the value may be subresource, but could also be a scalar
+        # (like a tokenized card's ID for example), so we check the type before
+        # setting #save_with_parent here.
+        if value.is_a?(APIResource)
+          value.save_with_parent = true
+        end
+
+        value
+      end
+    end
+
     def resource_url
       unless id = self['id']
         raise InvalidRequestError.new("Could not determine which URL to request: #{self.class} instance has invalid ID: #{id.inspect}", 'id')

lib/stripe/customer.rb

@@ -5,22 +5,7 @@ module Stripe
     include Stripe::APIOperations::Save
     extend Stripe::APIOperations::List
 
-    # Set or replace a customer's default source.
-    def source=(value)
-      super
-
-      # The parent setter will perform certain useful operations like
-      # converting to an APIResource if appropriate.
-      value = self.source
-
-      # Note that source may be a card, but could also be a tokenized card's ID
-      # (which is a string), and so we check its type here.
-      if value.is_a?(APIResource)
-        value.save_with_parent = true
-      end
-
-      value
-    end
+    save_nested_resource :source
 
     def add_invoice_item(params, opts={})
       opts = @opts.merge(Util.normalize_opts(opts))

lib/stripe/subscription.rb

@@ -5,6 +5,8 @@ module Stripe
     include Stripe::APIOperations::Save
     include Stripe::APIOperations::Delete
 
+    save_nested_resource :source
+
     def delete_discount
       _, opts = request(:delete, discount_url)
       initialize_from({ :discount => nil }, opts, true)

test/stripe/account_test.rb

@@ -290,49 +290,7 @@ module Stripe
       assert_equal(expected, obj.serialize_params)
     end
 
-    context "#external_account=" do
-      should "can have a token source set" do
-        a = Stripe::Account.new("test_account")
-        a.external_account = "tok_123"
-        assert_equal "tok_123", a.external_account
-      end
-
-      should "set a flag if given an object source" do
-        a = Stripe::Account.new("test_account")
-        a.external_account = {
-          :object => 'card'
-        }
-        assert_equal true, a.external_account.save_with_parent
-      end
-    end
-
     context "#bank_account=" do
-      should "can have a token source set" do
-        old_stderr = $stderr
-        $stderr = StringIO.new
-        begin
-          a = Stripe::Account.new("test_account")
-          a.bank_account = "tok_123"
-          assert_equal "tok_123", a.bank_account
-        ensure
-          $stderr = old_stderr
-        end
-      end
-
-      should "set a flag if given an object source" do
-        old_stderr = $stderr
-        $stderr = StringIO.new
-        begin
-          a = Stripe::Account.new("test_account")
-          a.bank_account = {
-            :object => 'bank_account'
-          }
-          assert_equal true, a.bank_account.save_with_parent
-        ensure
-          $stderr = old_stderr
-        end
-      end
-
       should "warn that #bank_account= is deprecated" do
         old_stderr = $stderr
         $stderr = StringIO.new

test/stripe/api_resource_test.rb

@@ -3,6 +3,26 @@ require File.expand_path('../../test_helper', __FILE__)
 
 module Stripe
   class ApiResourceTest < Test::Unit::TestCase
+    class NestedTestAPIResource < Stripe::APIResource
+      save_nested_resource :external_account
+    end
+
+    context ".save_nested_resource" do
+      should "can have a scalar set" do
+        r = NestedTestAPIResource.new("test_resource")
+        r.external_account = "tok_123"
+        assert_equal "tok_123", r.external_account
+      end
+
+      should "set a flag if given an object source" do
+        r = NestedTestAPIResource.new("test_resource")
+        r.external_account = {
+          :object => 'card'
+        }
+        assert_equal true, r.external_account.save_with_parent
+      end
+    end
+
     should "creating a new APIResource should not fetch over the network" do
       @mock.expects(:get).never
       Stripe::Customer.new("someid")