Merge pull request #53 from richmolj/master

Improve recursive sideloading logic

Author: richmolj

lib/jsonapi_compliable/sideload.rb

@@ -43,6 +43,21 @@ def initialize(name, type: nil, resource: nil, polymorphic: false, primary_key:
       extend @resource_class.config[:adapter].sideloading_module
     end
 
+    # @api private
+    def self.max_depth
+      @max_depth || 2
+    end
+
+    # Set maximum levels of sideload recursion
+    # /authors?comments.authors would be one level
+    # /authors?comments.authors.comments.authors would be two levels
+    # etc
+    #
+    # Default max depth is 2
+    def self.max_depth=(val)
+      @max_depth = val
+    end
+
     # @see #resource_class
     # @return [Resource] an instance of +#resource_class+
     def resource
@@ -331,6 +346,19 @@ def sideload(name)
       @sideloads[name]
     end
 
+    # @api private
+    def all_sideloads
+      {}.tap do |all|
+        if polymorphic?
+          polymorphic_groups.each_pair do |type, sl|
+            all.merge!(sl.resource.sideloading.all_sideloads)
+          end
+        else
+          all.merge!(@sideloads.merge(resource.sideloading.sideloads))
+        end
+      end
+    end
+
     # Looks at all nested sideload, and all nested sideloads for the
     # corresponding Resources, and returns an Include Directive hash
     #
@@ -354,27 +382,20 @@ def sideload(name)
     #
     # @return [Hash] The nested include hash
     # @api private
-    def to_hash(processed = [])
-      # Cut off at 5 recursions
-      if processed.select { |p| p == self }.length == 5
-        return { name => {} }
-      end
-      processed << self
+    def to_hash(depth_chain = [], parent = nil)
+      depth = depth_chain.select { |arr| arr == [parent, self] }.length
+      return {} if depth >= self.class.max_depth
 
-      result = { name => {} }.tap do |hash|
-        @sideloads.each_pair do |key, sideload|
-          hash[name][key] = sideload.to_hash(processed)[key] || {}
+      unless (parent && parent.name == :base) || name == :base
+        depth_chain += [[parent, self]]
+      end
 
-          if sideload.polymorphic?
-            sideload.polymorphic_groups.each_pair do |type, sl|
-              hash[name][key].merge!(nested_sideload_hash(sl, processed))
-            end
-          else
-            hash[name][key].merge!(nested_sideload_hash(sideload, processed))
-          end
+      { name => {} }.tap do |hash|
+        all_sideloads.each_pair do |key, sl|
+          sideload_hash = sl.to_hash(depth_chain, self)
+          hash[name].merge!(sideload_hash)
         end
       end
-      result
     end
 
     # @api private

lib/jsonapi_compliable/util/relationship_payload.rb

@@ -47,6 +47,7 @@ def payload_for(sideload, relationship_payload)
           type     = relationship_payload[:meta][:jsonapi_type]
           sideload = sideload.polymorphic_child_for_type(type)
         end
+        relationship_payload[:meta][:method] ||= :update
 
         {
           sideload: sideload,

spec/sideload_spec.rb

@@ -172,34 +172,118 @@ def foo
   end
 
   describe '#to_hash' do
-    # Set this up to catch any recursive trap
-    let(:resource) do
-      resource = JsonapiCompliable::Resource
-      resource.allow_sideload :bing, resource: JsonapiCompliable::Resource
-      resource
+    before do
+      stub_const('ResourceA', Class.new(JsonapiCompliable::Resource))
+      stub_const('ResourceB', Class.new(JsonapiCompliable::Resource))
+      stub_const('ResourceC', Class.new(JsonapiCompliable::Resource))
+      stub_const('ResourceD', Class.new(JsonapiCompliable::Resource))
+      stub_const('ResourceE', Class.new(JsonapiCompliable::Resource))
     end
 
-    before do
-      resource = JsonapiCompliable::Resource
-      instance.allow_sideload :bar, resource: resource do
-        allow_sideload :baz, resource: resource do
-          allow_sideload :bazoo, resource: resource
+    around do |e|
+      original = JsonapiCompliable::Sideload.max_depth
+      JsonapiCompliable::Sideload.max_depth = 5
+      e.run
+      JsonapiCompliable::Sideload.max_depth = original
+    end
+
+    subject { ResourceA.new.sideloading.to_hash[:base] }
+
+    context 'when simple' do
+      before do
+        ResourceA.allow_sideload :b, resource: ResourceB
+        ResourceB.allow_sideload :c, resource: ResourceC
+        ResourceC.allow_sideload :d, resource: ResourceD
+      end
+
+      it 'returns all sideloads in a nested hash' do
+        expect(subject).to eq(b: { c: { d: {} } })
+      end
+    end
+
+    context 'when recursive' do
+      before do
+        ResourceA.allow_sideload :b, resource: ResourceB
+        ResourceB.allow_sideload :a, resource: ResourceA
+      end
+
+      it 'allows 5 levels of recursion' do
+        expect(subject).to eq({
+          b: {
+            a: { # one
+              b: {
+                a: { # two
+                  b: {
+                    a: { # three
+                      b: {
+                        a: { # four
+                          b: {
+                            a: { # five
+                              b: {}
+                            }
+                          }
+                        }
+                      }
+                    }
+                  }
+                }
+              }
+            }
+          }
+        })
+      end
+    end
+
+    context 'when polymorphic' do
+      before do
+        ResourceA.allow_sideload :polly, polymorphic: true do
+          allow_sideload 'WhenTypeB', resource: ResourceB
+          allow_sideload 'WhenTypeC', resource: ResourceC
         end
+
+        ResourceB.allow_sideload :d, resource: ResourceD
+        ResourceC.allow_sideload :e, resource: ResourceE
+      end
+
+      it 'returns the correct nested hash' do
+        expect(subject[:polly]).to eq({
+          d: {},
+          e: {}
+        })
       end
-      instance.allow_sideload :blah, resource: resource
     end
 
-    it 'recursively builds a hash of sideloads' do
-      expect(instance.to_hash).to eq({
-        foo: {
-          bar: {
-            baz: {
-              bazoo: {}
+    context 'when polymorphic AND recursive' do
+      before do
+        ResourceA.allow_sideload :polly, polymorphic: true do
+          allow_sideload 'WhenTypeB', resource: ResourceB
+        end
+        ResourceB.allow_sideload :a, resource: ResourceA
+      end
+
+      it 'allows 5 levels of recursion' do
+        expect(subject[:polly]).to eq({
+          a: { # one
+            polly: {
+              a: { # two
+                polly: {
+                  a: { # three
+                    polly: {
+                      a: { # four
+                        polly: {
+                          a: { # five
+                            polly: {}
+                          }
+                        }
+                      }
+                    }
+                  }
+                }
+              }
             }
-          },
-          blah: {}
-        }
-      })
+          }
+        })
+      end
     end
   end
 end