Deprecate StripeObject#refresh_from

As discussed in #325, this deprecates the public visibility of
`#refresh_from` (by renaming it). It also adds some deprecation
infrastructure to produce warnings when it's used.

lib/stripe.rb

@@ -207,6 +207,8 @@ def self._uname_ver
 
   # DEPRECATED. Use `Util#encode_parameters` instead.
   def self.uri_encode(params)
+    Stripe::Util.warn_deprecated("Stripe.uri_encode",
+      extra: "Use Stripe::Util#encode_parameters instead.")
     Util.encode_parameters(params)
   end
 

lib/stripe/api_operations/delete.rb

@@ -4,7 +4,7 @@ module Delete
       def delete(params={}, opts={})
         opts = Util.normalize_opts(opts)
         response, opts = request(:delete, url, params, opts)
-        refresh_from(response, opts)
+        initialize_from(response, opts)
       end
     end
   end

lib/stripe/api_operations/update.rb

@@ -33,7 +33,7 @@ def save(params={})
           values.delete(:id)
 
           response, opts = request(:post, req_url, values)
-          refresh_from(response, opts)
+          initialize_from(response, opts)
         end
         self
       end

lib/stripe/api_resource.rb

@@ -22,7 +22,7 @@ def url
 
     def refresh
       response, opts = request(:get, url, @retrieve_params)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     def self.retrieve(id, opts={})

lib/stripe/application_fee.rb

@@ -8,7 +8,7 @@ def self.url
 
     def refund(params={}, opts={})
       response, opts = request(:post, refund_url, params, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     private

lib/stripe/charge.rb

@@ -6,39 +6,39 @@ class Charge < APIResource
 
     def refund(params={}, opts={})
       response, opts = request(:post, refund_url, params, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     def capture(params={}, opts={})
       response, opts = request(:post, capture_url, params, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     def update_dispute(params={}, opts={})
       response, opts = request(:post, dispute_url, params, opts)
-      refresh_from({ :dispute => response }, opts, true)
+      initialize_from({ :dispute => response }, opts, true)
       dispute
     end
 
     def close_dispute(params={}, opts={})
       response, opts = request(:post, close_dispute_url, params, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     def mark_as_fraudulent
       params = {
         :fraud_details => { :user_report => 'fraudulent' }
       }
       response, opts = request(:post, url, params)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     def mark_as_safe
       params = {
         :fraud_details => { :user_report => 'safe' }
       }
       response, opts = request(:post, url, params)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     private

lib/stripe/customer.rb

@@ -37,25 +37,25 @@ def create_upcoming_invoice(params={}, opts={})
 
     def cancel_subscription(params={}, opts={})
       response, opts = request(:delete, subscription_url, params, opts)
-      refresh_from({ :subscription => response }, opts, true)
+      initialize_from({ :subscription => response }, opts, true)
       subscription
     end
 
     def update_subscription(params={}, opts={})
       response, opts = request(:post, subscription_url, params, opts)
-      refresh_from({ :subscription => response }, opts, true)
+      initialize_from({ :subscription => response }, opts, true)
       subscription
     end
 
     def create_subscription(params={}, opts={})
       response, opts = request(:post, subscriptions_url, params, opts)
-      refresh_from({ :subscription => response }, opts, true)
+      initialize_from({ :subscription => response }, opts, true)
       subscription
     end
 
     def delete_discount
       _, opts = request(:delete, discount_url)
-      refresh_from({ :discount => nil }, opts, true)
+      initialize_from({ :discount => nil }, opts, true)
     end
 
     private

lib/stripe/dispute.rb

@@ -6,7 +6,7 @@ class Dispute < APIResource
 
     def close(params={}, opts={})
       response, opts = request(:post, close_url, params, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     def close_url

lib/stripe/invoice.rb

@@ -11,7 +11,7 @@ def self.upcoming(params, opts={})
 
     def pay(opts={})
       response, opts = request(:post, pay_url, {}, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     private

lib/stripe/order.rb

@@ -6,7 +6,7 @@ class Order < APIResource
 
     def pay(params, opts={})
       response, opts = request(:post, pay_url, params, opts)
-      refresh_from(response, opts)
+      initialize_from(response, opts)
     end
 
     private

lib/stripe/stripe_object.rb

@@ -22,7 +22,9 @@ def initialize(id=nil, opts={})
 
     def self.construct_from(values, opts={})
       values = Stripe::Util.symbolize_names(values)
-      self.new(values[:id]).refresh_from(values, opts)
+
+      # work around protected #initialize_from for now
+      self.new(values[:id]).send(:initialize_from, values, opts)
     end
 
     # Determines the equality of two Stripe objects. Stripe objects are
@@ -41,35 +43,17 @@ def inspect
       "#<#{self.class}:0x#{self.object_id.to_s(16)}#{id_string}> JSON: " + JSON.pretty_generate(@values)
     end
 
+    # Re-initializes the object based on a hash of values (usually one that's
+    # come back from an API call). Adds or removes value accessors as necessary
+    # and updates the state of internal data.
+    #
+    # Please don't use this method. If you're trying to do mass assignment, try
+    # #initialize_from instead.
     def refresh_from(values, opts, partial=false)
-      @opts = Util.normalize_opts(opts)
-      @original_values = Marshal.load(Marshal.dump(values)) # deep copy
-
-      removed = partial ? Set.new : Set.new(@values.keys - values.keys)
-      added = Set.new(values.keys - @values.keys)
-
-      # Wipe old state before setting new.  This is useful for e.g. updating a
-      # customer, where there is no persistent card parameter.  Mark those values
-      # which don't persist as transient
-
-      instance_eval do
-        remove_accessors(removed)
-        add_accessors(added, values)
-      end
-
-      removed.each do |k|
-        @values.delete(k)
-        @transient_values.add(k)
-        @unsaved_values.delete(k)
-      end
-
-      update_attributes_with_options(values, :opts => opts)
-      values.each do |k, _|
-        @transient_values.delete(k)
-        @unsaved_values.delete(k)
-      end
-
-      return self
+      Stripe::Util.warn_deprecated("#refresh_from",
+        extra: "If you're trying to perform mass-assignment, please consider " +
+          "using #update_attributes instead.")
+      initialize_from(values, opts, partial)
     end
 
     # Mass assigns attributes on the model.
@@ -294,6 +278,49 @@ def respond_to_missing?(symbol, include_private = false)
       @values && @values.has_key?(symbol) || super
     end
 
+    # Re-initializes the object based on a hash of values (usually one that's
+    # come back from an API call). Adds or removes value accessors as necessary
+    # and updates the state of internal data.
+    #
+    # Protected on purpose! Please do not expose.
+    #
+    # ==== Options
+    #
+    # * +:values:+ Hash used to update accessors and values.
+    # * +:opts:+ Options for StripeObject like an API key.
+    # * +:partial:+ Indicates that the re-initialization should not attempt to
+    #   remove accessors.
+    def initialize_from(values, opts, partial=false)
+      @opts = Util.normalize_opts(opts)
+      @original_values = Marshal.load(Marshal.dump(values)) # deep copy
+
+      removed = partial ? Set.new : Set.new(@values.keys - values.keys)
+      added = Set.new(values.keys - @values.keys)
+
+      # Wipe old state before setting new.  This is useful for e.g. updating a
+      # customer, where there is no persistent card parameter.  Mark those values
+      # which don't persist as transient
+
+      instance_eval do
+        remove_accessors(removed)
+        add_accessors(added, values)
+      end
+
+      removed.each do |k|
+        @values.delete(k)
+        @transient_values.add(k)
+        @unsaved_values.delete(k)
+      end
+
+      update_attributes_with_options(values, :opts => opts)
+      values.each do |k, _|
+        @transient_values.delete(k)
+        @unsaved_values.delete(k)
+      end
+
+      self
+    end
+
     # Mass assigns attributes on the model.
     #
     # This is a version of +update_attributes+ that takes some extra options

lib/stripe/subscription.rb

@@ -13,7 +13,7 @@ def self.retrieve(id, opts=nil)
 
     def delete_discount
       response, opts = request(:delete, discount_url)
-      refresh_from({ :discount => nil }, opts, true)
+      initialize_from({ :discount => nil }, opts, true)
     end
 
     private

lib/stripe/transfer.rb

@@ -6,7 +6,7 @@ class Transfer < APIResource
 
     def cancel
       response, api_key = Stripe.request(:post, cancel_url, @api_key)
-      refresh_from(response, api_key)
+      initialize_from(response, api_key)
     end
 
     def cancel_url

lib/stripe/util.rb

@@ -181,5 +181,16 @@ def self.check_api_key!(key)
       raise TypeError.new("api_key must be a string") unless key.is_a?(String)
       key
     end
+
+    def self.warn_deprecated(name, options = {})
+      # should not trigger on $VERBOSE = nil (false is "level 1", true is
+      # "level 2")
+      if $VERBOSE != nil
+        message = "Warning (stripe): #{name} is deprecated and will be " +
+          "removed in a future version."
+        message += " " + options[:extra] if options[:extra]
+        $stderr.puts(message)
+      end
+    end
   end
 end

test/stripe/metadata_test.rb

@@ -93,7 +93,7 @@ class MetadataTest < Test::Unit::TestCase
           obj.metadata['uuid'] = '6735'
         end
         params = {:metadata => {'type' => 'summer', 'uuid' => '6735'}}
-        curl_args = Stripe.uri_encode(params)
+        curl_args = Stripe::Util.encode_parameters(params)
         check_metadata({:metadata => {'type' => 'christmas'}},
                        curl_args,
                        update_actions)

test/stripe/util_test.rb

@@ -71,5 +71,32 @@ class UtilTest < Test::Unit::TestCase
       assert_raise { Stripe::Util.normalize_opts(nil) }
       assert_raise { Stripe::Util.normalize_opts(:api_key => nil) }
     end
+
+    should "#warn_deprecated produces a deprecation warning" do
+      old_stderr = $stderr
+      $stderr = StringIO.new
+      begin
+        Stripe::Util.warn_deprecated("#refresh_from", extra: "Don't use it.")
+        message = "Warning (stripe): #refresh_from is deprecated and will be " +
+          "removed in a future version. Don't use it.\n"
+        assert_equal message, $stderr.string
+      ensure
+        $stderr = old_stderr
+      end
+    end
+
+    should "#warn_deprecated is silent on $VERBOSE = nil" do
+      old_stderr = $stderr
+      old_verbose = $VERBOSE
+      $stderr = StringIO.new
+      $VERBOSE = nil
+      begin
+        Stripe::Util.warn_deprecated("#refresh_from", extra: "Don't use it.")
+        assert_equal "", $stderr.string
+      ensure
+        $stderr = old_stderr
+        $VERBOSE = old_verbose
+      end
+    end
   end
 end