Convert resource links from procs to serializable hash

recurly/recurly-app#1440 (comment)

Procs can't be properly marshalled, so hrefs are lost when
Recurly::Resource objects are cached. By storing links as a hash and
resolving the href without a proc, we can properly marshal them from
cache.

Acked-by: Stephen Celis <[email protected]>
Signed-off-by: Evan Owen <[email protected]>

Gemfile.lock

@@ -4,7 +4,7 @@ PATH
     recurly (2.1.8)
 
 GEM
-  remote: http://rubygems.org/
+  remote: https://rubygems.org/
   specs:
     addressable (2.2.6)
     bouncy-castle-java (1.5.0146.1)

lib/recurly/account.rb

@@ -51,8 +51,8 @@ def invoice!
     #   (e.g., the account is already opwn), and may raise an exception if the
     #   attempt fails.
     def reopen
-      return false unless self[:reopen]
-      reload self[:reopen].call
+      return false unless link? :reopen
+      reload follow_link :reopen
       true
     end
 

lib/recurly/api.rb

@@ -41,13 +41,13 @@ def accept_language=(language)
       # @return [Net::HTTPOK, Net::HTTPResponse]
       # @raise [ResponseError] With a non-2xx status code.
       def head uri, params = {}, options = {}
-        request :head, uri, { :params => params }.merge(options)
+        request :head, uri, { :params => params || {} }.merge(options)
       end
 
       # @return [Net::HTTPOK, Net::HTTPResponse]
       # @raise [ResponseError] With a non-2xx status code.
       def get uri, params = {}, options = {}
-        request :get, uri, { :params => params }.merge(options)
+        request :get, uri, { :params => params || {} }.merge(options)
       end
 
       # @return [Net::HTTPCreated, Net::HTTPResponse]
@@ -64,7 +64,7 @@ def put uri, body = nil, options = {}
 
       # @return [Net::HTTPNoContent, Net::HTTPResponse]
       # @raise [ResponseError] With a non-2xx status code.
-      def delete uri, options = {}
+      def delete uri, body = nil, options = {}
         request :delete, uri, options
       end
 

lib/recurly/coupon.rb

@@ -51,15 +51,15 @@ def save
     #   account = Account.find account_code
     #   coupon.redeem account
     def redeem account_or_code, currency = nil
-      return false unless self[:redeem]
+      return false unless link? :redeem
 
       account_code = if account_or_code.is_a? Account
         account_or_code.account_code
       else
         account_or_code
       end
 
-      Redemption.from_response self[:redeem].call(
+      Redemption.from_response follow_link(:redeem,
         :body => (redemption = redemptions.new(
           :account_code => account_code,
           :currency     => currency || Recurly.default_currency

lib/recurly/invoice.rb

@@ -40,8 +40,8 @@ class Invoice < Resource
     # @return [true, false] +true+ when successful, +false+ when unable to
     #   (e.g., the invoice is no longer open).
     def mark_successful
-      return false unless self[:mark_successful]
-      reload self[:mark_successful].call
+      return false unless link? :mark_successful
+      reload follow_link :mark_successful
       true
     end
 
@@ -50,8 +50,8 @@ def mark_successful
     # @return [true, false] +true+ when successful, +false+ when unable to
     #   (e.g., the invoice is no longer open).
     def mark_failed
-      return false unless self[:mark_failed]
-      reload self[:mark_failed].call
+      return false unless link? :mark_failed
+      reload follow_link :mark_failed
       true
     end
 

lib/recurly/resource.rb

@@ -393,31 +393,27 @@ def from_xml xml
 
         xml.each_element do |el|
           if el.name == 'a'
-            name, uri = el.attribute('name').value, el.attribute('href').value
-            record[name] = case el.attribute('method').to_s
-              when 'get', '' then proc { |*opts| API.get uri, {}, *opts }
-              when 'post'    then proc { |*opts| API.post uri, nil, *opts }
-              when 'put'     then proc { |*opts| API.put uri, nil, *opts }
-              when 'delete'  then proc { |*opts| API.delete uri, *opts }
-            end
+            record.links[el.attribute('name').value] = {
+              :method => el.attribute('method').to_s,
+              :href => el.attribute('href').value
+            }
             next
           end
 
           if el.children.empty? && href = el.attribute('href')
             resource_class = Recurly.const_get(
               Helper.classify(el.attribute('type') || el.name), false
             )
-            record[el.name] = case el.name
+            case el.name
             when *associations[:has_many]
-              Pager.new resource_class, :uri => href.value, :parent => record
+              record[el.name] = Pager.new(
+                resource_class, :uri => href.value, :parent => record
+              )
             when *(associations[:has_one] + associations[:belongs_to])
-              lambda {
-                begin
-                  relation = resource_class.from_response API.get(href.value)
-                  relation.attributes[member_name] = record
-                  relation
-                rescue Recurly::API::NotFound
-                end
+              record.links[el.name] = {
+                :resource_class => resource_class,
+                :method => :get,
+                :href => href.value
               }
             end
           else
@@ -633,9 +629,11 @@ def persisted?
     #   account[:last_name]                # => "Beneke"
     # @see #write_attribute
     def read_attribute key
-      value = attributes[key = key.to_s]
-      if value.respond_to?(:call) && self.class.reflect_on_association(key)
-        value = attributes[key] = value.call
+      key = key.to_s
+      if attributes.key? key
+        value = attributes[key]
+      elsif links.key?(key) && self.class.reflect_on_association(key)
+        value = attributes[key] = follow_link key
       end
       value
     end
@@ -677,6 +675,39 @@ def attributes= attributes = {}
       }
     end
 
+    # @return [Hash] The raw hash of record href links.
+    def links
+      @links ||= {}
+    end
+
+    # Whether a record has a link with the given name.
+    #
+    # @param key [Symbol, String] The name of the link to check for.
+    # @example
+    #   account.link? :billing_info # => true
+    def link? key
+      links.key?(key.to_s)
+    end
+
+    # Fetch the value of a link by following the associated href.
+    #
+    # @param key [Symbol, String] The name of the link to be followed.
+    # @param options [Hash] A hash of API options.
+    # @example
+    #   account.read_link :billing_info # => <Recurly::BillingInfo>
+    def follow_link key, options = {}
+      if link = links[key = key.to_s]
+        response = API.send link[:method], link[:href], nil, options
+        if resource_class = link[:resource_class]
+          response = resource_class.from_response response
+          response.attributes[self.class.member_name] = self
+        end
+        response
+      end
+    rescue Recurly::API::NotFound
+      raise unless resource_class
+    end
+
     # Serializes the record to XML.
     #
     # @return [String] An XML string.
@@ -865,8 +896,9 @@ def marshal_dump
         @href,
         changed_attributes,
         previous_changes,
+        response,
         etag,
-        response
+        links
       ]
     end
 
@@ -879,7 +911,8 @@ def marshal_load serialization
         @changed_attributes,
         @previous_changes,
         @response,
-        @etag = serialization
+        @etag,
+        @links = serialization
     end
 
     # @return [String]

lib/recurly/subscription.rb

@@ -97,8 +97,8 @@ def subscription_add_ons= subscription_add_ons
     #   subscription = account.subscriptions.first
     #   subscription.cancel # => true
     def cancel
-      return false unless self[:cancel]
-      reload self[:cancel].call
+      return false unless link? :cancel
+      reload follow_link :cancel
       true
     end
 
@@ -119,11 +119,11 @@ def cancel
     #   subscription = account.subscriptions.first
     #   subscription.terminate(:partial) # => true
     def terminate refund_type = :none
-      return false unless self[:terminate]
+      return false unless link? :terminate
       unless REFUND_TYPES.include? refund_type.to_s
         raise ArgumentError, "refund must be one of: #{REFUND_TYPES.join ', '}"
       end
-      reload self[:terminate].call(:params => { :refund => refund_type })
+      reload follow_link(:terminate, :params => { :refund => refund_type })
       true
     end
     alias destroy terminate
@@ -134,8 +134,8 @@ def terminate refund_type = :none
     #   (e.g., the subscription is already active), and may raise an exception
     #   if the reactivation fails.
     def reactivate
-      return false unless self[:reactivate]
-      reload self[:reactivate].call
+      return false unless link? :reactivate
+      reload follow_link :reactivate
       true
     end
 
@@ -145,8 +145,8 @@ def reactivate
     #   (e.g., the subscription is not active).
     # @param next_renewal_date [Time] when the subscription should renew.
     def postpone next_renewal_date
-      return false unless self[:postpone]
-      reload self[:postpone].call(
+      return false unless link? :postpone
+      reload follow_link(:postpone,
         :params => { :next_renewal_date => next_renewal_date }
       )
       true

lib/recurly/transaction.rb

@@ -69,9 +69,9 @@ def save
     # @param amount_in_cents [Integer, nil] The amount (in cents) to refund
     #   (refunds fully if nil).
     def refund amount_in_cents = nil
-      return false unless self[:refund]
+      return false unless link? :refund
       refund = self.class.from_response(
-        self[:refund].call :params => { :amount_in_cents => amount_in_cents }
+        follow_link :refund, :params => { :amount_in_cents => amount_in_cents }
       )
       refund.uuid == uuid ? copy_from(refund) && self : refund
     end

spec/recurly/account_spec.rb

@@ -80,12 +80,6 @@
 XML
     end
 
-    it 'handle empty values for embedded billing info' do
-      stub_api_request :post, 'accounts', 'accounts/create-201'
-      @account.billing_info = { :number => '', :verification_value => '' }
-      @account.save.must_equal true
-    end
-
     describe "persisted accounts" do
       before do
         @account.persist!

spec/recurly/resource_spec.rb

@@ -149,9 +149,9 @@ def to_param
           3.times { |n| record[:seasons][n].must_be_kind_of Integer }
           record[:never_gonna_happen]['season'].must_be_kind_of Integer
           stub_api_request(:put, 'resources/1/renew') { "HTTP/1.1 200\n" }
-          record[:renew].call
+          record.follow_link :renew
           stub_api_request(:delete, 'resources/1/cancel') { "HTTP/1.1 422\n" }
-          proc { record[:cancel].call }.must_raise API::UnprocessableEntity
+          proc { record.follow_link :cancel }.must_raise API::UnprocessableEntity
         end
       end
     end