From 7fdffd3f0bc224e815631cfe85b047cbec501cf7 Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Thu, 4 Jul 2024 18:48:32 +0300
Subject: [PATCH 1/7] test: add rendering test (with multiple copies of a
 complex object)

---
 spec/sideloading_spec.rb | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/spec/sideloading_spec.rb b/spec/sideloading_spec.rb
index 767b4695..dd284f4f 100644
--- a/spec/sideloading_spec.rb
+++ b/spec/sideloading_spec.rb
@@ -1070,6 +1070,40 @@ def department
       }.to_not raise_error
     end
 
+    context "works with multiple copies and resource-provided associations" do
+      before do
+        PORO::Position.class_eval do
+          attr_accessor :custom_department
+        end
+
+        # Just a simple resource-provided association
+        position_resource.belongs_to :custom_department, resource: department_resource, foreign_key: :department_id do
+          assign_each do |position, _|
+            position.department
+          end
+        end
+
+        params[:include] = "current_position.custom_department,positions"
+      end
+
+      it "construct full objects in the (squashed) included list" do
+        render
+
+        included = json["included"]
+        pos1_array = included.select { |i|
+          i["type"] == "positions" && i["id"] == position1.id.to_s
+        }
+
+        expect(pos1_array.length).to eq(1)
+
+        pos1 = pos1_array.first
+
+        expect(pos1["relationships"]).to be_present
+        expect(pos1["relationships"]["custom_department"]).to be_present
+        expect(pos1["relationships"]["custom_department"]["data"]).to be_present
+      end
+    end
+
     describe "across requests" do
       it "uses a different sideloaded resource" do
         ctx = double(current_user: :admin)

From 0d4fb9b3a09957a3bc64f784c1bd625c881dcf53 Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Fri, 5 Jul 2024 20:18:34 +0300
Subject: [PATCH 2/7] fix: add entity deduplication mechanics (primitive
 integration)

---
 lib/graphiti/configuration.rb      |  1 +
 lib/graphiti/query.rb              | 15 +++++++++++----
 lib/graphiti/resource/interface.rb |  2 +-
 lib/graphiti/resource_proxy.rb     |  1 +
 lib/graphiti/runner.rb             | 14 +++++++++++---
 lib/graphiti/scope.rb              | 30 ++++++++++++++++++++++++++++++
 lib/graphiti/sideload.rb           |  1 +
 spec/sideloading_spec.rb           | 26 +++++++++++++++++++++++++-
 8 files changed, 81 insertions(+), 9 deletions(-)

diff --git a/lib/graphiti/configuration.rb b/lib/graphiti/configuration.rb
index 121c4317..32e58bfc 100644
--- a/lib/graphiti/configuration.rb
+++ b/lib/graphiti/configuration.rb
@@ -16,6 +16,7 @@ class Configuration
     attr_accessor :typecast_reads
     attr_accessor :raise_on_missing_sidepost
     attr_accessor :before_sideload
+    attr_accessor :deduplicate_entities
 
     attr_reader :debug, :debug_models
 
diff --git a/lib/graphiti/query.rb b/lib/graphiti/query.rb
index 1b59ccbe..c099766c 100644
--- a/lib/graphiti/query.rb
+++ b/lib/graphiti/query.rb
@@ -2,9 +2,10 @@
 
 module Graphiti
   class Query
-    attr_reader :resource, :association_name, :params, :action
+    attr_reader :resource, :association_name, :params, :action, :populated_entities
 
-    def initialize(resource, params, association_name = nil, nested_include = nil, parents = [], action = nil)
+    def initialize(resource, params, association_name = nil, nested_include = nil, parents = [], action = nil,
+                   populated_entities: nil)
       @resource = resource
       @association_name = association_name
       @params = params
@@ -14,6 +15,7 @@ def initialize(resource, params, association_name = nil, nested_include = nil, p
       @include_param = nested_include || @params[:include]
       @parents = parents
       @action = parse_action(action)
+      @populated_entities = populated_entities
     end
 
     def association?
@@ -107,11 +109,16 @@ def sideloads
             # This way A) ensures sideloads are resolved
             # And B) ensures nested filters, sorts etc still work
             relationship_name = sideload ? sideload.name : key
-            hash[relationship_name] = Query.new sl_resource,
+
+            hash[relationship_name] = Query.new(
+              sl_resource,
               @params,
               key,
               sub_hash,
-              query_parents, :all
+              query_parents,
+              :all,
+              populated_entities: @populated_entities
+            )
           else
             handle_missing_sideload(key)
           end
diff --git a/lib/graphiti/resource/interface.rb b/lib/graphiti/resource/interface.rb
index 0f702cf3..ddbe9f9b 100644
--- a/lib/graphiti/resource/interface.rb
+++ b/lib/graphiti/resource/interface.rb
@@ -11,7 +11,7 @@ def cache_resource(expires_in: false)
 
         def all(params = {}, base_scope = nil)
           validate_request!(params)
-          _all(params, {}, base_scope)
+          _all(params, { deduplicate_entities: Graphiti.config.deduplicate_entities }, base_scope)
         end
 
         # @api private
diff --git a/lib/graphiti/resource_proxy.rb b/lib/graphiti/resource_proxy.rb
index a698f8d9..57f875e4 100644
--- a/lib/graphiti/resource_proxy.rb
+++ b/lib/graphiti/resource_proxy.rb
@@ -80,6 +80,7 @@ def data
         if records.empty? && raise_on_missing?
           raise Graphiti::Errors::RecordNotFound
         end
+
         records = records[0] if single?
         records
       end
diff --git a/lib/graphiti/runner.rb b/lib/graphiti/runner.rb
index d2b9d070..a3ac8c99 100644
--- a/lib/graphiti/runner.rb
+++ b/lib/graphiti/runner.rb
@@ -13,6 +13,11 @@ def initialize(resource_class, params, query = nil, action = nil)
       validator.validate!
 
       @deserialized_payload = validator.deserialized_payload
+      @deduplicate_entities = Graphiti.config.deduplicate_entities
+
+      if @deduplicate_entities
+        @populated_entities ||= {}
+      end
     end
 
     def jsonapi_resource
@@ -30,7 +35,7 @@ def jsonapi_context
     end
 
     def query
-      @query ||= Query.new(jsonapi_resource, params, nil, nil, [], @action)
+      @query ||= Query.new(jsonapi_resource, params, nil, nil, [], @action, populated_entities: @populated_entities)
     end
 
     def query_hash
@@ -64,9 +69,11 @@ def proxy(base = nil, opts = {})
         :sideload,
         :parent,
         :params,
-        :bypass_required_filters
+        :bypass_required_filters,
+        :deduplicate_entities
       scope = jsonapi_scope(base, scope_opts)
-      ResourceProxy.new jsonapi_resource,
+      ResourceProxy.new(
+        jsonapi_resource,
         scope,
         query,
         payload: deserialized_payload,
@@ -74,6 +81,7 @@ def proxy(base = nil, opts = {})
         raise_on_missing: opts[:raise_on_missing],
         cache: opts[:cache],
         cache_expires_in: opts[:cache_expires_in]
+      )
     end
   end
 end
diff --git a/lib/graphiti/scope.rb b/lib/graphiti/scope.rb
index 0ddfd313..a6f75805 100644
--- a/lib/graphiti/scope.rb
+++ b/lib/graphiti/scope.rb
@@ -11,6 +11,12 @@ def initialize(object, resource, query, opts = {})
       @object = @resource.around_scoping(@object, @query.hash) { |scope|
         apply_scoping(scope, opts)
       }
+
+      @deduplicate_entities = @opts[:deduplicate_entities]
+
+      if @deduplicate_entities
+        @populated_entities = @query.populated_entities || {}
+      end
     end
 
     def resolve
@@ -23,6 +29,9 @@ def resolve
           payload[:results]
         }
         resolved.compact!
+
+        deduplicate_entities(resolved) if @deduplicate_entities
+
         assign_serializer(resolved)
         yield resolved if block_given?
         @opts[:after_resolve]&.call(resolved)
@@ -167,5 +176,26 @@ def add_scoping(key, scoping_class, opts, default = {})
       @object = scoping_class.new(@resource, @query.hash, @object, opts).apply
       @unpaginated_object = @object unless key == :paginate
     end
+
+    def deduplicate_entities(resolved)
+      resolved.map! do |entity|
+        next entity unless entity.respond_to?(:id) # Leave no-id (although unusual) entities as is
+
+        if @populated_entities.key?(entity.class)
+          populated_entity = @populated_entities[entity.class]
+                               .find { |populated_entity| populated_entity&.id == entity.id }
+          if populated_entity.nil?
+            @populated_entities[entity.class].push(entity)
+
+            entity
+          else
+            populated_entity
+          end
+        else
+          @populated_entities[entity.class] = [entity]
+          entity
+        end
+      end
+    end
   end
 end
diff --git a/lib/graphiti/sideload.rb b/lib/graphiti/sideload.rb
index fdf02df5..7302091c 100644
--- a/lib/graphiti/sideload.rb
+++ b/lib/graphiti/sideload.rb
@@ -396,6 +396,7 @@ def load_options(parents, query)
         opts[:after_resolve] = ->(results) {
           fire_assign(parents, results)
         }
+        opts[:deduplicate_entities] = Graphiti.config.deduplicate_entities
       end
     end
 
diff --git a/spec/sideloading_spec.rb b/spec/sideloading_spec.rb
index dd284f4f..c33efed3 100644
--- a/spec/sideloading_spec.rb
+++ b/spec/sideloading_spec.rb
@@ -1084,9 +1084,29 @@ def department
         end
 
         params[:include] = "current_position.custom_department,positions"
+
+        Graphiti.config.deduplicate_entities = nil
       end
 
-      it "construct full objects in the (squashed) included list" do
+      it "constructs partial objects in the (squashed) included list with no deduplication" do
+        render
+
+        included = json["included"]
+        pos1_array = included.select { |i|
+          i["type"] == "positions" && i["id"] == position1.id.to_s
+        }
+
+        expect(pos1_array.length).to eq(1)
+
+        pos1 = pos1_array.first
+
+        expect(pos1["relationships"]).to be_present
+        expect(pos1["relationships"]["custom_department"]).to be_present
+        expect(pos1["relationships"]["custom_department"]["data"]).to be_nil
+      end
+
+      it "constructs full objects in the (squashed) included list with deduplication" do
+        Graphiti.config.deduplicate_entities = true
         render
 
         included = json["included"]
@@ -1102,6 +1122,10 @@ def department
         expect(pos1["relationships"]["custom_department"]).to be_present
         expect(pos1["relationships"]["custom_department"]["data"]).to be_present
       end
+
+      after do
+        Graphiti.config.deduplicate_entities = nil
+      end
     end
 
     describe "across requests" do

From 0162181c8f07445c4e3f87022b1c57f6aba4fd6b Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Mon, 8 Jul 2024 16:45:38 +0300
Subject: [PATCH 3/7] test: integrate sideload tests with deduplication
 mechanics

---
 spec/sideload_spec.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/spec/sideload_spec.rb b/spec/sideload_spec.rb
index 3103d5b3..0d4b3924 100644
--- a/spec/sideload_spec.rb
+++ b/spec/sideload_spec.rb
@@ -641,8 +641,10 @@ def self.name
         parent: "parent",
         sideload: instance,
         query: anything,
-        after_resolve: anything
+        after_resolve: anything,
+        deduplicate_entities: nil
       }
+
       expect(resource_class).to receive(:_all)
         .with(anything, expected, {type: :positions})
       instance.load(parents, query, "parent")

From 5a13aa8bfa3bc8dee631b2f29b576ac4d55ca666 Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Mon, 8 Jul 2024 17:44:50 +0300
Subject: [PATCH 4/7] chore: make linter happy

---
 lib/graphiti/query.rb              | 11 +++++++++--
 lib/graphiti/resource/interface.rb |  2 +-
 lib/graphiti/scope.rb              |  3 +--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/graphiti/query.rb b/lib/graphiti/query.rb
index c099766c..9bb6c991 100644
--- a/lib/graphiti/query.rb
+++ b/lib/graphiti/query.rb
@@ -4,8 +4,15 @@ module Graphiti
   class Query
     attr_reader :resource, :association_name, :params, :action, :populated_entities
 
-    def initialize(resource, params, association_name = nil, nested_include = nil, parents = [], action = nil,
-                   populated_entities: nil)
+    def initialize(
+      resource,
+      params,
+      association_name = nil,
+      nested_include = nil,
+      parents = [],
+      action = nil,
+      populated_entities: nil
+    )
       @resource = resource
       @association_name = association_name
       @params = params
diff --git a/lib/graphiti/resource/interface.rb b/lib/graphiti/resource/interface.rb
index ddbe9f9b..8fe7d388 100644
--- a/lib/graphiti/resource/interface.rb
+++ b/lib/graphiti/resource/interface.rb
@@ -11,7 +11,7 @@ def cache_resource(expires_in: false)
 
         def all(params = {}, base_scope = nil)
           validate_request!(params)
-          _all(params, { deduplicate_entities: Graphiti.config.deduplicate_entities }, base_scope)
+          _all(params, {deduplicate_entities: Graphiti.config.deduplicate_entities}, base_scope)
         end
 
         # @api private
diff --git a/lib/graphiti/scope.rb b/lib/graphiti/scope.rb
index a6f75805..683c0a70 100644
--- a/lib/graphiti/scope.rb
+++ b/lib/graphiti/scope.rb
@@ -182,8 +182,7 @@ def deduplicate_entities(resolved)
         next entity unless entity.respond_to?(:id) # Leave no-id (although unusual) entities as is
 
         if @populated_entities.key?(entity.class)
-          populated_entity = @populated_entities[entity.class]
-                               .find { |populated_entity| populated_entity&.id == entity.id }
+          populated_entity = @populated_entities[entity.class].find { |populated_entity| populated_entity&.id == entity.id }
           if populated_entity.nil?
             @populated_entities[entity.class].push(entity)
 

From 9d93c96ec1f3a0f0d5c1c1fe65b497c905904ccf Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Tue, 9 Jul 2024 15:26:19 +0300
Subject: [PATCH 5/7] feat: prevent deduplication + concurrency
 misconfiguration

---
 lib/graphiti/configuration.rb | 10 ++++++++++
 lib/graphiti/runner.rb        |  2 +-
 lib/graphiti/sideload.rb      |  2 +-
 spec/configuration_spec.rb    | 22 ++++++++++++++++++++++
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/lib/graphiti/configuration.rb b/lib/graphiti/configuration.rb
index 32e58bfc..a4eaeed8 100644
--- a/lib/graphiti/configuration.rb
+++ b/lib/graphiti/configuration.rb
@@ -65,6 +65,16 @@ def cache_rendering?
       end
     end
 
+    def deduplicated_rendering?
+      if @deduplicate_entities
+        raise "Deduplicated rendering is not compatible with concurrent fetching" if @concurrency
+
+        return true
+      end
+
+      false
+    end
+
     def schema_path
       @schema_path ||= raise("No schema_path defined! Set Graphiti.config.schema_path to save your schema.")
     end
diff --git a/lib/graphiti/runner.rb b/lib/graphiti/runner.rb
index a3ac8c99..b52ef35d 100644
--- a/lib/graphiti/runner.rb
+++ b/lib/graphiti/runner.rb
@@ -13,7 +13,7 @@ def initialize(resource_class, params, query = nil, action = nil)
       validator.validate!
 
       @deserialized_payload = validator.deserialized_payload
-      @deduplicate_entities = Graphiti.config.deduplicate_entities
+      @deduplicate_entities = Graphiti.config.deduplicated_rendering?
 
       if @deduplicate_entities
         @populated_entities ||= {}
diff --git a/lib/graphiti/sideload.rb b/lib/graphiti/sideload.rb
index 7302091c..46030880 100644
--- a/lib/graphiti/sideload.rb
+++ b/lib/graphiti/sideload.rb
@@ -396,7 +396,7 @@ def load_options(parents, query)
         opts[:after_resolve] = ->(results) {
           fire_assign(parents, results)
         }
-        opts[:deduplicate_entities] = Graphiti.config.deduplicate_entities
+        opts[:deduplicate_entities] = Graphiti.config.deduplicated_rendering?
       end
     end
 
diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb
index 9fe96d37..14e89012 100644
--- a/spec/configuration_spec.rb
+++ b/spec/configuration_spec.rb
@@ -187,4 +187,26 @@
       expect { Graphiti.config.cache_rendering? }.to raise_error(/You must configure a cache store in order to use cache_rendering/)
     end
   end
+
+  describe "#deduplicate_entities" do
+    it "defaults" do
+      expect(Graphiti.config.deduplicated_rendering?).to eq(false)
+    end
+
+    it "is settable" do
+      Graphiti.configure do |c|
+        c.deduplicate_entities = true
+      end
+      expect(Graphiti.config.deduplicated_rendering?).to eq(true)
+    end
+
+    it "warns about incompatibility between concurrency and deduplicated_rendering" do
+      Graphiti.configure do |c|
+        c.deduplicate_entities = true
+        c.concurrency = true
+      end
+
+      expect { Graphiti.config.deduplicated_rendering? }.to raise_error(/Deduplicated rendering is not compatible with concurrent fetching/)
+    end
+  end
 end

From 7856f054ad60f465141d8712291bf88c250eac6e Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Tue, 9 Jul 2024 15:47:56 +0300
Subject: [PATCH 6/7] feat: neatly integrate deduplication with queries

---
 lib/graphiti/query.rb              |  5 ++++-
 lib/graphiti/resource/interface.rb |  2 +-
 lib/graphiti/runner.rb             | 11 ++++++++++-
 lib/graphiti/scope.rb              |  2 +-
 lib/graphiti/sideload.rb           |  1 -
 spec/sideload_spec.rb              |  3 +--
 6 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/lib/graphiti/query.rb b/lib/graphiti/query.rb
index 9bb6c991..cc57baba 100644
--- a/lib/graphiti/query.rb
+++ b/lib/graphiti/query.rb
@@ -2,7 +2,7 @@
 
 module Graphiti
   class Query
-    attr_reader :resource, :association_name, :params, :action, :populated_entities
+    attr_reader :resource, :association_name, :params, :action, :deduplicate_entities, :populated_entities
 
     def initialize(
       resource,
@@ -11,6 +11,7 @@ def initialize(
       nested_include = nil,
       parents = [],
       action = nil,
+      deduplicate_entities: nil,
       populated_entities: nil
     )
       @resource = resource
@@ -22,6 +23,7 @@ def initialize(
       @include_param = nested_include || @params[:include]
       @parents = parents
       @action = parse_action(action)
+      @deduplicate_entities = deduplicate_entities
       @populated_entities = populated_entities
     end
 
@@ -124,6 +126,7 @@ def sideloads
               sub_hash,
               query_parents,
               :all,
+              deduplicate_entities: @deduplicate_entities,
               populated_entities: @populated_entities
             )
           else
diff --git a/lib/graphiti/resource/interface.rb b/lib/graphiti/resource/interface.rb
index 8fe7d388..0f702cf3 100644
--- a/lib/graphiti/resource/interface.rb
+++ b/lib/graphiti/resource/interface.rb
@@ -11,7 +11,7 @@ def cache_resource(expires_in: false)
 
         def all(params = {}, base_scope = nil)
           validate_request!(params)
-          _all(params, {deduplicate_entities: Graphiti.config.deduplicate_entities}, base_scope)
+          _all(params, {}, base_scope)
         end
 
         # @api private
diff --git a/lib/graphiti/runner.rb b/lib/graphiti/runner.rb
index b52ef35d..4ca2ec81 100644
--- a/lib/graphiti/runner.rb
+++ b/lib/graphiti/runner.rb
@@ -35,7 +35,16 @@ def jsonapi_context
     end
 
     def query
-      @query ||= Query.new(jsonapi_resource, params, nil, nil, [], @action, populated_entities: @populated_entities)
+      @query ||= Query.new(
+        jsonapi_resource,
+        params,
+        nil,
+        nil,
+        [],
+        @action,
+        deduplicate_entities: @deduplicate_entities,
+        populated_entities: @populated_entities
+      )
     end
 
     def query_hash
diff --git a/lib/graphiti/scope.rb b/lib/graphiti/scope.rb
index 683c0a70..021bb029 100644
--- a/lib/graphiti/scope.rb
+++ b/lib/graphiti/scope.rb
@@ -12,7 +12,7 @@ def initialize(object, resource, query, opts = {})
         apply_scoping(scope, opts)
       }
 
-      @deduplicate_entities = @opts[:deduplicate_entities]
+      @deduplicate_entities = @query.deduplicate_entities
 
       if @deduplicate_entities
         @populated_entities = @query.populated_entities || {}
diff --git a/lib/graphiti/sideload.rb b/lib/graphiti/sideload.rb
index 46030880..fdf02df5 100644
--- a/lib/graphiti/sideload.rb
+++ b/lib/graphiti/sideload.rb
@@ -396,7 +396,6 @@ def load_options(parents, query)
         opts[:after_resolve] = ->(results) {
           fire_assign(parents, results)
         }
-        opts[:deduplicate_entities] = Graphiti.config.deduplicated_rendering?
       end
     end
 
diff --git a/spec/sideload_spec.rb b/spec/sideload_spec.rb
index 0d4b3924..62429b6b 100644
--- a/spec/sideload_spec.rb
+++ b/spec/sideload_spec.rb
@@ -641,8 +641,7 @@ def self.name
         parent: "parent",
         sideload: instance,
         query: anything,
-        after_resolve: anything,
-        deduplicate_entities: nil
+        after_resolve: anything
       }
 
       expect(resource_class).to receive(:_all)

From 8a065ee20188d679410d6e8b45d7ac25cf053430 Mon Sep 17 00:00:00 2001
From: Andrei Mochalov <factyy@gmail.com>
Date: Fri, 23 Aug 2024 13:39:34 +0300
Subject: [PATCH 7/7] feat: use hashes instead of arrays for faster lookups

---
 lib/graphiti/scope.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/graphiti/scope.rb b/lib/graphiti/scope.rb
index 021bb029..50dfadd4 100644
--- a/lib/graphiti/scope.rb
+++ b/lib/graphiti/scope.rb
@@ -179,19 +179,21 @@ def add_scoping(key, scoping_class, opts, default = {})
 
     def deduplicate_entities(resolved)
       resolved.map! do |entity|
-        next entity unless entity.respond_to?(:id) # Leave no-id (although unusual) entities as is
+        next entity unless entity.respond_to?(:id) && !entity.id.nil? # Leave no-id (although unusual) entities as is
 
         if @populated_entities.key?(entity.class)
-          populated_entity = @populated_entities[entity.class].find { |populated_entity| populated_entity&.id == entity.id }
+          populated_entity = @populated_entities[entity.class][entity.id]
           if populated_entity.nil?
-            @populated_entities[entity.class].push(entity)
+            @populated_entities[entity.class][entity.id] = entity
 
             entity
           else
             populated_entity
           end
         else
-          @populated_entities[entity.class] = [entity]
+          @populated_entities[entity.class] = {}
+          @populated_entities[entity.class][entity.id] = entity
+
           entity
         end
       end