Bajena · Bajena · Jan 16, 2020 · Jan 14, 2020 · Jan 16, 2020 · Jan 16, 2020
diff --git a/README.md b/README.md
@@ -64,7 +64,9 @@ class UserSerializer < BaseSerializer
   # The previous one is a shorthand for the following lines:
   lazy_relationship :blog_posts, loader: AmsLazyRelationships::Loaders::Association.new("User", :blog_posts)
   has_many :blog_posts, serializer: BlogPostSerializer do |serializer|
-    serializer.lazy_blog_posts
+    # non-proc custom finder will work as well, but it can produce redundant sql
+    # queries, please see [Example 2: Modifying the relationship before rendering](#example-2-modifying-the-relationship-before-rendering)
+    -> { serializer.lazy_blog_posts }
-    -> { serializer.lazy_blog_posts }
+    serializer.lazy_blog_posts
-    -> { serializer.lazy_blog_posts }
+    serializer.lazy_blog_posts
   end
 
   lazy_has_one :poro_model, loader: AmsLazyRelationships::Loaders::Direct.new(:poro_model) { |object| PoroModel.new(object) }
@@ -116,6 +118,16 @@ end
 #### Example 2: Modifying the relationship before rendering
 Sometimes it may happen that you need to process the relationship before rendering, e.g. decorate the records. In this case the gem provides a special method (in our case `lazy_comments`) for each defined relationship. Check out the example - we'll decorate every comment before serializing:
 
+```ruby
+class BlogPostSerializer < BaseSerializer
+  lazy_has_many :comments do |serializer|
+    -> { serializer.lazy_comments.map(&:decorate) }
+   end
+end
+```
+
+Despite the fact that non-block custom finder such as
+
 ```ruby
 class BlogPostSerializer < BaseSerializer
   lazy_has_many :comments do |serializer|
@@ -124,6 +136,29 @@ class BlogPostSerializer < BaseSerializer
 end
 ```
 
+will work still, it's better to implement it in a form of lambda, in order to avoid redundant SQL queries when `include_data` AMS setting appears to be `false`:
+
+```ruby
+class BlogPostSerializer < BaseSerializer
+  lazy_has_many :comments do |serializer|
+    include_data :if_sideloaded
+    -> { serializer.lazy_comments.map(&:decorate) }
+   end
+end
+```
+
+Feel free to skip custom lazy finder for association if your goal is just to define `include_data` setting and/or to specify some links and metas:
+
+```ruby
+class BlogPostSerializer < BaseSerializer
+  lazy_has_many :comments do
+    include_data :if_sideloaded
-    include_data :if_sideloaded
+    include_data false
-    include_data :if_sideloaded
+    include_data false
+    link :self, 'a link'
+    meta name: 'Dan Brown'
+   end
+end
+```
+
 #### Example 3: Introducing loader classes
 Under the hood ams_lazy_relationships uses special loader classes to batch load the relationships. By default the gem uses serializer class names and relationship names to instantiate correct loaders, but it may happen that e.g. your serializer's class name doesn't match the model name (e.g. your model's name is `BlogPost` but the serializer's name is `PostSerializer`).
 

diff --git a/lib/ams_lazy_relationships.rb b/lib/ams_lazy_relationships.rb
@@ -4,6 +4,7 @@
 require "active_model_serializers"
 
 require "ams_lazy_relationships/version"
+require "ams_lazy_relationships/extensions"
 require "ams_lazy_relationships/loaders"
 require "ams_lazy_relationships/core"
 

diff --git a/lib/ams_lazy_relationships/core/relationship_wrapper_methods.rb b/lib/ams_lazy_relationships/core/relationship_wrapper_methods.rb
@@ -19,7 +19,7 @@ def define_relationship_wrapper_methods
         define_singleton_method(
           "lazy_#{relationship_type}"
         ) do |relationship_name, options = {}, &block|
-          send(:define_lazy_association, relationship_type, relationship_name, options, block)
+          define_lazy_association(relationship_type, relationship_name, options, block)
         end
       end
     end
@@ -29,11 +29,20 @@ def define_lazy_association(type, name, options, block)
 
       real_relationship_options = options.except(*lazy_relationship_option_keys)
 
-      block ||= lambda do |serializer|
-        serializer.public_send("lazy_#{name}")
-      end
+      public_send(type, name.to_sym, real_relationship_options) do |serializer|
+        block_value = instance_exec(serializer, &block) if block
 
-      public_send(type, name.to_sym, real_relationship_options, &block)
+        if block && block_value != :nil
+          # respect the custom finder for lazy association
+          # @see https://github.com/rails-api/active_model_serializers/blob/v0.10.10/lib/active_model/serializer/reflection.rb#L165-L168
+          block_value
+        else
+          # provide default lazy association finder in a form of lambda,
+          # in order to play nice with possible `include_data` setting.
+          # @see lib/ams_lazy_relationships/extensions/reflection.rb
+          serializer.method("lazy_#{name}")
+        end
+      end
 
       lazy_relationship(name, options.slice(*lazy_relationship_option_keys))
     end

diff --git a/lib/ams_lazy_relationships/extensions.rb b/lib/ams_lazy_relationships/extensions.rb
@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+require "ams_lazy_relationships/extensions/reflection"
+
+module AmsLazyRelationships
+  module Extensions
+  end
+end
diff --git a/lib/ams_lazy_relationships/extensions/reflection.rb b/lib/ams_lazy_relationships/extensions/reflection.rb
@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+# There is a general problem inside AMS related to custom association finder
+# combined with `include_data` setting:
+#
+#   class BlogPostSerializer < BaseSerializer
+#     belongs_to :category do
+#       include_data :if_sideloaded
+#       object.categories.last
+#     end
+#   end
+#
+# The problem is that `belongs_to` block will be fully evaluated each time for
+# each object, and only after that AMS is able to take into account
+# `include_data` mode -
+# https://github.com/rails-api/active_model_serializers/blob/v0.10.10/lib/active_model/serializer/reflection.rb#L162-L163
+#
+#   def value(serializer, include_slice)
+#     # ...
+#     block_value = instance_exec(serializer, &block) if block
+#     return unless include_data?(include_slice)
+#     # ...
+#   end
+#
+# That causing redundant (and so huge potentially!) SQL queries and AR objects
+# allocation when `include_data` appears to be `false` but `belongs_to` block
+# defines instant (not a kind of AR::Relation) custom association finder.
+#
+# Described problem is a very specific use case for pure AMS applications.
+# The bad news is that `ams_lazy_relationships` always utilizes the
+# association block -
+# https://github.com/Bajena/ams_lazy_relationships/blob/v0.2.0/lib/ams_lazy_relationships/core/relationship_wrapper_methods.rb#L32-L36
+#
+# def define_lazy_association(type, name, options, block)
+#   #...
+#   block ||= lambda do |serializer|
+#     serializer.public_send("lazy_#{name}")
+#   end
+#
+#   public_send(type, name.to_sym, real_relationship_options, &block)
+#   #...
+# end
+#
+# This way we break `include_data` optimizations for the host application.
+#
+# In order to overcome that we are forced to monkey-patch
+# `AmsLazyRelationships::Extensions::Reflection#value` method and make it to be
+# ready for Proc returned by association block. This way we will use a kind of
+#
+#   block ||= lambda do |serializer|
+#     -> { serializer.public_send("lazy_#{name}") }
+#   end
+#
+# as association block, then AMS will evaluate it, get the value of `include_data`
+# setting, make a decision do we need to continue with that association, if so -
+# will finally evaluate the proc with lazy relationship inside it.
+
+module AmsLazyRelationships
+  module Extensions
+    module Reflection
+      def value(*)
+        case (block_value = super)
+        when Proc, Method then block_value.call
+        else block_value
+        end
+      end
+    end
+  end
+end
+
+::ActiveModel::Serializer::Reflection.prepend AmsLazyRelationships::Extensions::Reflection
diff --git a/spec/core_spec.rb b/spec/core_spec.rb
@@ -866,4 +866,147 @@ class UserSerializer < BaseTestSerializer
       end
     end
   end
+
+  describe 'include_data AMS setting' do
+    shared_examples 'lazy loader when custom finder is specified' do
+      let(:adapter_class) { json_api_adapter_class }
+      let(:includes) { ['blog_posts'] }
+      let(:blog_post_data) { json.dig(:data, :relationships, :blog_posts, :data) }
+
+      it 'loads the association' do
+        expect { json }
+          .to make_database_queries(count: 1, matching: 'blog_posts')
+
+        expect(blog_post_data).to be_present
+      end
+    end
+
+    context 'proc-like custom finder' do
+      let(:level0_serializer_class) do
+        module Serializer14
+          class User1Serializer < BaseTestSerializer
+            lazy_has_many :blog_posts do |serializer|
+              -> { serializer.lazy_blog_posts }
+            end
+          end
+        end
+
+        Serializer14::User1Serializer
+      end
+
+      include_examples 'lazy loader when custom finder is specified'
+    end
+
+    context 'non-proc custom finder' do
+      let(:level0_serializer_class) do
+        module Serializer14
+          class User2Serializer < BaseTestSerializer
+            lazy_has_many :blog_posts do |serializer|
+              serializer.lazy_blog_posts
+            end
+          end
+        end
+
+        Serializer14::User2Serializer
+      end
+
+      include_examples 'lazy loader when custom finder is specified'
+    end
+
+    next unless AMS_VERSION >= Gem::Version.new("0.10.3")
+
+    shared_examples 'lazy loader when include_data option is set' do
+      let(:adapter_class) { json_api_adapter_class }
+      let(:includes) { ['blog_posts'] }
+      let(:category_data) { json.dig(:included, 0, :relationships, :category, :data) }
+
+      it 'does not fire unnecessary SQL query' do
+        expect { json }
+          .to make_database_queries(count: 1, matching: 'blog_posts')
+          .and make_database_queries(count: 0, matching: 'categories')
+
+        expect(category_data).to be_nil
+      end
+
+      context 'when sideloaded' do
+        let(:includes) { ['blog_posts.category'] }
+
+        it 'fires single SQL query' do
+          expect { json }
+            .to make_database_queries(count: 1, matching: 'blog_posts')
+            .and make_database_queries(count: 1, matching: 'categories')
+
+          expect(category_data).to be_present
+        end
+      end
+    end
+
+    context 'include_data disabled globally' do
+      let(:level0_serializer_class) do
+        module Serializer15
+          class BlogPost1Serializer < BaseTestSerializer
+            lazy_belongs_to :category
+          end
+
+          class User1Serializer < BaseTestSerializer
+            lazy_has_many :blog_posts, serializer: BlogPost1Serializer
+          end
+        end
+
+        Serializer15::User1Serializer
+      end
+
+      around do |example|
+        backup = ActiveModel::Serializer.config.include_data_default
+        ActiveModel::Serializer.config.include_data_default = :if_sideloaded
+
+        example.run
+
+        ActiveModel::Serializer.config.include_data_default = backup
+      end
+
+      include_examples 'lazy loader when include_data option is set'
+    end
+
+    context 'include_data disabled locally with custom finder' do
+      let(:level0_serializer_class) do
+        module Serializer15
+          class BlogPost2Serializer < BaseTestSerializer
+            lazy_belongs_to :category do |serializer|
+              include_data :if_sideloaded
+              -> { serializer.lazy_category }
+            end
+          end
+
+          class User2Serializer < BaseTestSerializer
+            lazy_has_many :blog_posts, serializer: BlogPost2Serializer
+          end
+        end
+
+        Serializer15::User2Serializer
+      end
+
+      include_examples 'lazy loader when include_data option is set'
+    end
+
+    context 'include_data disabled locally without custom finder' do
+      let(:level0_serializer_class) do
+        module Serializer15
+          class BlogPost3Serializer < BaseTestSerializer
+            lazy_belongs_to :category do
+              include_data :if_sideloaded
+            end
+          end
+
+          class User3Serializer < BaseTestSerializer
+            lazy_has_many :blog_posts, serializer: BlogPost3Serializer
+          end
+        end
+
+        Serializer15::User3Serializer
+      end
+
+      include_examples 'lazy loader when include_data option is set'
+    end
+  end
 end