Merge pull request #832 from cerebris/complex_include_tests

Adds tests and fix for complex include missing relationships

Author: dgeb

lib/jsonapi/resource_serializer.rb

@@ -173,7 +173,7 @@ def process_primary(source, include_directives)
         source.each { |resource| process_primary(resource, include_directives) }
       else
         return {} if source.nil?
-        add_included_object(object_hash(source, include_directives), true)
+        add_resource(source, include_directives, true)
       end
     end
 
@@ -285,9 +285,9 @@ def relationships_hash(source, fetchable_fields, include_directives = {})
             id = resource.id
             relationships_only = already_serialized?(relationship.type, id)
             if include_linkage && !relationships_only
-              add_included_object(object_hash(resource, ia))
+              add_resource(resource, ia)
             elsif include_linked_children || relationships_only
-              relationships_hash(resource, ia)
+              relationships_hash(resource, fetchable_fields, ia)
             end
           end
         end
@@ -311,12 +311,7 @@ def cached_relationships_hash(source, include_directives)
           end
 
           source.preloaded_fragments[key].each do |id, f|
-            unless already_serialized?(relationship.type, id)
-              obj_hash = object_hash(f, ia)
-              unless self_referential_and_already_in_source(f)
-                add_included_object(obj_hash)
-              end
-            end
+            add_resource(f, ia)
 
             if h.has_key?(key)
               # The hash already has everything we need except the :data field
@@ -467,14 +462,29 @@ def set_primary(type, id)
       @included_objects[type][id][:primary] = true
     end
 
-    # Collects the hashes for all objects processed by the serializer
-    def add_included_object(obj_hash, primary = false)
-      is_cached = obj_hash.is_a?(JSONAPI::CachedResourceFragment)
-      type = is_cached ? obj_hash.type : obj_hash['type']
-      id = is_cached ? obj_hash.id : obj_hash['id']
+    def add_resource(source, include_directives, primary = false)
+      type = source.is_a?(JSONAPI::CachedResourceFragment) ? source.type : source.class._type
+      id = source.id
 
       @included_objects[type] ||= {}
-      @included_objects[type][id] = { primary: primary, object_hash: obj_hash }
+      existing = @included_objects[type][id]
+
+      if existing.nil?
+        obj_hash = object_hash(source, include_directives)
+        @included_objects[type][id] = {
+            primary: primary,
+            object_hash: obj_hash,
+            includes: Set.new(include_directives[:include_related].keys)
+        }
+      else
+        include_related = Set.new(include_directives[:include_related].keys)
+        unless existing[:includes].superset?(include_related)
+          obj_hash = object_hash(source, include_directives)
+          @included_objects[type][id][:object_hash].deep_merge!(obj_hash)
+          @included_objects[type][id][:includes].add(include_related)
+          @included_objects[type][id][:primary] = existing[:primary] | primary
+        end
+      end
     end
 
     def generate_link_builder(primary_resource_klass, options)

test/controllers/controller_test.rb

@@ -3638,3 +3638,69 @@ def test_caching_with_join_to_resource_with_sql_fragment
     assert_response :success
   end
 end
+
+class Api::BoxesControllerTest < ActionController::TestCase
+  def test_complex_includes_base
+    assert_cacheable_get :index
+    assert_response :success
+  end
+
+  def test_complex_includes_two_level
+    assert_cacheable_get :index, params: {include: 'things,things.user'}
+
+    assert_response :success
+
+    # The test is hardcoded with the include order. This should be changed at some point since either thing could come first and still be valid
+    assert_equal '1', json_response['included'][0]['id']
+    assert_equal 'things', json_response['included'][0]['type']
+    assert_equal '1',  json_response['included'][0]['relationships']['user']['data']['id']
+    assert_nil json_response['included'][0]['relationships']['things']['data']
+
+    assert_equal '2', json_response['included'][1]['id']
+    assert_equal 'things', json_response['included'][1]['type']
+    assert_equal '1', json_response['included'][1]['relationships']['user']['data']['id']
+    assert_nil json_response['included'][1]['relationships']['things']['data']
+
+    assert_equal '1', json_response['included'][2]['id']
+    assert_equal 'users', json_response['included'][2]['type']
+    assert_nil json_response['included'][2]['relationships']['things']['data']
+  end
+
+  def test_complex_includes_things_nested_things
+    assert_cacheable_get :index, params: {include: 'things,things.things'}
+
+    assert_response :success
+
+    # The test is hardcoded with the include order. This should be changed at some point since either thing could come first and still be valid
+    assert_equal '2', json_response['included'][0]['id']
+    assert_equal 'things', json_response['included'][0]['type']
+    assert_nil json_response['included'][0]['relationships']['user']['data']
+    assert_equal '1', json_response['included'][0]['relationships']['things']['data'][0]['id']
+
+    assert_equal '1', json_response['included'][1]['id']
+    assert_equal 'things', json_response['included'][1]['type']
+    assert_nil json_response['included'][1]['relationships']['user']['data']
+    assert_equal '2', json_response['included'][1]['relationships']['things']['data'][0]['id']
+  end
+
+  def test_complex_includes_nested_things_secondary_users
+    assert_cacheable_get :index, params: {include: 'things,things.user,things.things'}
+
+    assert_response :success
+
+    # The test is hardcoded with the include order. This should be changed at some point since either thing could come first and still be valid
+    assert_equal '1', json_response['included'][2]['id']
+    assert_equal 'users', json_response['included'][2]['type']
+    assert_nil json_response['included'][2]['relationships']['things']['data']
+
+    assert_equal '2', json_response['included'][0]['id']
+    assert_equal 'things', json_response['included'][0]['type']
+    assert_equal '1',  json_response['included'][0]['relationships']['user']['data']['id']
+    assert_equal '1',  json_response['included'][0]['relationships']['things']['data'][0]['id']
+
+    assert_equal '1', json_response['included'][1]['id']
+    assert_equal 'things', json_response['included'][1]['type']
+    assert_equal '1',  json_response['included'][1]['relationships']['user']['data']['id']
+    assert_equal '2',  json_response['included'][1]['relationships']['things']['data'][0]['id']
+  end
+end

test/fixtures/active_record.rb

@@ -264,6 +264,33 @@
   create_table :questionables, force: true do |t|
     t.timestamps null: false
   end
+
+  create_table :boxes, force: true  do |t|
+    t.string :name
+    t.timestamps null: false
+  end
+
+  create_table :things, force: true  do |t|
+    t.string :name
+    t.references :user
+    t.references :box
+
+    t.timestamps null: false
+  end
+
+  create_table :users, force: true  do |t|
+    t.string :name
+    t.timestamps null: false
+  end
+
+  create_table :related_things, force: true  do |t|
+    t.string :name
+    t.references :from, references: :thing
+    t.references :to, references: :thing
+
+    t.timestamps null: false
+  end
+
   # special cases
 end
 
@@ -555,6 +582,27 @@ class Make < ActiveRecord::Base
 class WebPage < ActiveRecord::Base
 end
 
+class Box < ActiveRecord::Base
+  has_many :things
+end
+
+class User < ActiveRecord::Base
+  has_many :things
+end
+
+class Thing < ActiveRecord::Base
+  belongs_to :box
+  belongs_to :user
+
+  has_many :related_things, foreign_key: :from_id
+  has_many :things, through: :related_things, source: :to
+end
+
+class RelatedThing < ActiveRecord::Base
+  belongs_to :from, class_name: Thing, foreign_key: :from_id
+  belongs_to :to, class_name: Thing, foreign_key: :to_id
+end
+
 module Api
   module V7
     class Client < Customer
@@ -823,6 +871,11 @@ class NumerosTelefoneController < JSONAPI::ResourceController
   end
 end
 
+module Api
+  class BoxesController < JSONAPI::ResourceController
+  end
+end
+
 ### RESOURCES
 class BaseResource < JSONAPI::Resource
   abstract
@@ -1712,6 +1765,23 @@ def show
   end
 end
 
+module Api
+  class BoxResource < JSONAPI::Resource
+    has_many :things
+  end
+
+  class ThingResource < JSONAPI::Resource
+    has_one :box
+    has_one :user
+
+    has_many :things
+  end
+
+  class UserResource < JSONAPI::Resource
+    has_many :things
+  end
+end
+
 ### PORO Data - don't do this in a production app
 $breed_data = BreedData.new
 $breed_data.add(Breed.new(0, 'persian'))

test/fixtures/boxes.yml

@@ -0,0 +1,2 @@
+box_1:
+  id: 1

test/fixtures/related_things.yml

@@ -0,0 +1,9 @@
+related_thing_1:
+  id: 1
+  from_id: 1
+  to_id: 2
+
+related_thing_2:
+  id: 2
+  from_id: 2
+  to_id: 1

test/fixtures/things.yml

@@ -0,0 +1,9 @@
+thing_1:
+  id: 1
+  user_id: 1
+  box_id: 1
+
+thing_2:
+  id: 2
+  user_id: 1
+  box_id: 1

test/fixtures/users.yml

@@ -0,0 +1,2 @@
+user_1:
+  id: 1

test/test_helper.rb

@@ -256,6 +256,8 @@ class CatResource < JSONAPI::Resource
   jsonapi_resources :authors
 
   namespace :api do
+    jsonapi_resources :boxes
+
     namespace :v1 do
       jsonapi_resources :people
       jsonapi_resources :comments