Allow you to disable eager loading on relationships mentioned in includes

nagirrab · nagirrab · commit 75eadc12b643 · 2016-07-20T09:15:48.000-04:00
diff --git a/README.md b/README.md
@@ -541,6 +541,7 @@ The relationship methods (`relationship`, `has_one`, and `has_many`) support the
  * `polymorphic` - set to true to identify relationships that are polymorphic.
  * `relation_name` - the name of the relation to use on the model. A lambda may be provided which allows conditional selection of the relation based on the context.
  * `always_include_linkage_data` - if set to true, the relationship includes linkage data. Defaults to false if not set.
+ * `eager_load_on_include` - if set to false, will not include this relationship in join SQL when requested via an include. You usually want to leave this on, but it will break 'relationships' which are not active record, for example if you want to expose a tree using the `ancestry` gem or similar, or the SQL query becomes too large to handle. Defaults to true if not set.
 
 `to_one` relationships support the additional option:
  * `foreign_key_on` - defaults to `:self`. To indicate that the foreign key is on the related resource specify `:related`.
diff --git a/lib/jsonapi/include_directives.rb b/lib/jsonapi/include_directives.rb
@@ -19,7 +19,9 @@ class IncludeDirectives
     #   }
     # }
 
-    def initialize(includes_array)
+    def initialize(resource_klass, includes_array, force_eager_load: false)
+      @resource_klass = resource_klass
+      @force_eager_load = force_eager_load
       @include_directives_hash = { include_related: {} }
       includes_array.each do |include|
         parse_include(include)
@@ -38,16 +40,27 @@ def model_includes
 
     def get_related(current_path)
       current = @include_directives_hash
+      current_resource_klass = @resource_klass
       current_path.split('.').each do |fragment|
         fragment = fragment.to_sym
-        current[:include_related][fragment] ||= { include: false, include_related: {} }
+
+        if current_resource_klass
+          current_relationship = current_resource_klass._relationships[fragment]
+          current_resource_klass = current_relationship.try(:resource_klass)
+        else
+          warn "[RELATIONSHIP NOT FOUND] Relationship could not be found for #{current_path}."
+        end
+
+        include_in_join = @force_eager_load || !current_relationship || current_relationship.eager_load_on_include
+
+        current[:include_related][fragment] ||= { include: false, include_related: {}, include_in_join: include_in_join }
         current = current[:include_related][fragment]
       end
       current
     end
 
     def get_includes(directive)
-      directive[:include_related].map do |name, directive|
+      directive[:include_related].select { |k,v| v[:include_in_join] }.map do |name, directive|
         sub = get_includes(directive)
         sub.any? ? { name => sub } : name
       end
diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb
@@ -2,7 +2,7 @@ module JSONAPI
   class Relationship
     attr_reader :acts_as_set, :foreign_key, :options, :name,
                 :class_name, :polymorphic, :always_include_linkage_data,
-                :parent_resource
+                :parent_resource, :eager_load_on_include
 
     def initialize(name, options = {})
       @name = name.to_s
@@ -13,6 +13,7 @@ def initialize(name, options = {})
       @relation_name = options.fetch(:relation_name, @name)
       @polymorphic = options.fetch(:polymorphic, false) == true
       @always_include_linkage_data = options.fetch(:always_include_linkage_data, false) == true
+      @eager_load_on_include = options.fetch(:eager_load_on_include, true) == true
     end
 
     alias_method :polymorphic?, :polymorphic
diff --git a/lib/jsonapi/request_parser.rb b/lib/jsonapi/request_parser.rb
@@ -225,7 +225,7 @@ def parse_include_directives(include)
         include.push(unformat_key(included_resource).to_s)
       end
 
-      @include_directives = JSONAPI::IncludeDirectives.new(include)
+      @include_directives = JSONAPI::IncludeDirectives.new(@resource_klass, include)
     end
 
     def parse_filters(filters)
diff --git a/lib/jsonapi/resource.rb b/lib/jsonapi/resource.rb
@@ -702,7 +702,7 @@ def apply_filters(records, filters, options = {})
         end
 
         if required_includes.any?
-          records = apply_includes(records, options.merge(include_directives: IncludeDirectives.new(required_includes)))
+          records = apply_includes(records, options.merge(include_directives: IncludeDirectives.new(self, required_includes, force_eager_load: true)))
         end
 
         records
diff --git a/lib/jsonapi/resource_serializer.rb b/lib/jsonapi/resource_serializer.rb
@@ -16,13 +16,14 @@ class ResourceSerializer
     # serializer_options: additional options that will be passed to resource meta and links lambdas
 
     def initialize(primary_resource_klass, options = {})
-      @primary_class_name = primary_resource_klass._type
-      @fields             = options.fetch(:fields, {})
-      @include            = options.fetch(:include, [])
-      @include_directives = options[:include_directives]
-      @key_formatter      = options.fetch(:key_formatter, JSONAPI.configuration.key_formatter)
-      @id_formatter       = ValueFormatter.value_formatter_for(:id)
-      @link_builder       = generate_link_builder(primary_resource_klass, options)
+      @primary_resource_klass = primary_resource_klass
+      @primary_class_name     = primary_resource_klass._type
+      @fields                 = options.fetch(:fields, {})
+      @include                = options.fetch(:include, [])
+      @include_directives     = options[:include_directives]
+      @key_formatter          = options.fetch(:key_formatter, JSONAPI.configuration.key_formatter)
+      @id_formatter           = ValueFormatter.value_formatter_for(:id)
+      @link_builder           = generate_link_builder(primary_resource_klass, options)
       @always_include_to_one_linkage_data = options.fetch(:always_include_to_one_linkage_data,
                                                           JSONAPI.configuration.always_include_to_one_linkage_data)
       @always_include_to_many_linkage_data = options.fetch(:always_include_to_many_linkage_data,
@@ -41,7 +42,7 @@ def serialize_to_hash(source)
       is_resource_collection = source.respond_to?(:to_ary)
 
       @included_objects = {}
-      @include_directives ||= JSONAPI::IncludeDirectives.new(@include)
+      @include_directives ||= JSONAPI::IncludeDirectives.new(@primary_resource_klass, @include)
 
       process_primary(source, @include_directives.include_directives)
 
diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb
@@ -890,7 +890,7 @@ class PostResource < JSONAPI::Resource
 
   has_one :author, class_name: 'Person'
   has_one :section
-  has_many :tags, acts_as_set: true, inverse_relationship: :posts
+  has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false
   has_many :comments, acts_as_set: false, inverse_relationship: :post
 
   # Not needed - just for testing
diff --git a/test/unit/serializer/include_directives_test.rb b/test/unit/serializer/include_directives_test.rb
@@ -4,45 +4,49 @@
 class IncludeDirectivesTest < ActiveSupport::TestCase
 
   def test_one_level_one_include
-    directives = JSONAPI::IncludeDirectives.new(['posts']).include_directives
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts']).include_directives
 
     assert_hash_equals(
       {
         include_related: {
           posts: {
             include: true,
-            include_related:{}
+            include_related:{},
+            include_in_join: true
           }
         }
       },
       directives)
   end
 
   def test_one_level_multiple_includes
-    directives = JSONAPI::IncludeDirectives.new(['posts', 'comments', 'tags']).include_directives
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts', 'comments', 'tags']).include_directives
 
     assert_hash_equals(
       {
         include_related: {
           posts: {
             include: true,
-            include_related:{}
+            include_related:{},
+            include_in_join: true
           },
           comments: {
             include: true,
-            include_related:{}
+            include_related:{},
+            include_in_join: true
           },
           tags: {
             include: true,
-            include_related:{}
+            include_related:{},
+            include_in_join: true
           }
         }
       },
       directives)
   end
 
   def test_two_levels_include_full_path
-    directives = JSONAPI::IncludeDirectives.new(['posts.comments']).include_directives
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts.comments']).include_directives
 
     assert_hash_equals(
       {
@@ -52,17 +56,41 @@ def test_two_levels_include_full_path
             include_related:{
               comments: {
                 include: true,
-                include_related:{}
+                include_related:{},
+                include_in_join: true
               }
-            }
+            },
+            include_in_join: true
+          }
+        }
+      },
+      directives)
+  end
+
+  def test_no_eager_join
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts.tags']).include_directives
+
+    assert_hash_equals(
+      {
+        include_related: {
+          posts: {
+            include: true,
+            include_related:{
+              tags: {
+                include: true,
+                include_related:{},
+                include_in_join: false
+              }
+            },
+            include_in_join: true
           }
         }
       },
       directives)
   end
 
   def test_two_levels_include_full_path_redundant
-    directives = JSONAPI::IncludeDirectives.new(['posts','posts.comments']).include_directives
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts','posts.comments']).include_directives
 
     assert_hash_equals(
       {
@@ -72,17 +100,19 @@ def test_two_levels_include_full_path_redundant
             include_related:{
               comments: {
                 include: true,
-                include_related:{}
+                include_related:{},
+                include_in_join: true
               }
-            }
+            },
+            include_in_join: true
           }
         }
       },
       directives)
   end
 
   def test_three_levels_include_full
-    directives = JSONAPI::IncludeDirectives.new(['posts.comments.tags']).include_directives
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts.comments.tags']).include_directives
 
     assert_hash_equals(
       {
@@ -95,19 +125,22 @@ def test_three_levels_include_full
                 include_related:{
                   tags: {
                     include: true,
-                    include_related:{}
+                    include_related:{},
+                    include_in_join: true
                   }
-                }
+                },
+                include_in_join: true
               }
-            }
+            },
+            include_in_join: true
           }
         }
       },
       directives)
   end
 
   def test_three_levels_include_full_model_includes
-    directives = JSONAPI::IncludeDirectives.new(['posts.comments.tags'])
+    directives = JSONAPI::IncludeDirectives.new(PersonResource, ['posts.comments.tags'])
     assert_array_equals([{:posts=>[{:comments=>[:tags]}]}], directives.model_includes)
   end
 end
