diff --git a/lib/graphiti/configuration.rb b/lib/graphiti/configuration.rb index 121c4317..a4eaeed8 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 @@ -64,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/query.rb b/lib/graphiti/query.rb index 1b59ccbe..cc57baba 100644 --- a/lib/graphiti/query.rb +++ b/lib/graphiti/query.rb @@ -2,9 +2,18 @@ module Graphiti class Query - attr_reader :resource, :association_name, :params, :action - - def initialize(resource, params, association_name = nil, nested_include = nil, parents = [], action = nil) + attr_reader :resource, :association_name, :params, :action, :deduplicate_entities, :populated_entities + + def initialize( + resource, + params, + association_name = nil, + nested_include = nil, + parents = [], + action = nil, + deduplicate_entities: nil, + populated_entities: nil + ) @resource = resource @association_name = association_name @params = params @@ -14,6 +23,8 @@ def initialize(resource, params, association_name = nil, nested_include = nil, p @include_param = nested_include || @params[:include] @parents = parents @action = parse_action(action) + @deduplicate_entities = deduplicate_entities + @populated_entities = populated_entities end def association? @@ -107,11 +118,17 @@ 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, + deduplicate_entities: @deduplicate_entities, + populated_entities: @populated_entities + ) else handle_missing_sideload(key) end 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..4ca2ec81 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.deduplicated_rendering? + + if @deduplicate_entities + @populated_entities ||= {} + end end def jsonapi_resource @@ -30,7 +35,16 @@ def jsonapi_context end def query - @query ||= Query.new(jsonapi_resource, params, nil, nil, [], @action) + @query ||= Query.new( + jsonapi_resource, + params, + nil, + nil, + [], + @action, + deduplicate_entities: @deduplicate_entities, + populated_entities: @populated_entities + ) end def query_hash @@ -64,9 +78,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 +90,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..50dfadd4 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 = @query.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,27 @@ 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) && !entity.id.nil? # Leave no-id (although unusual) entities as is + + if @populated_entities.key?(entity.class) + populated_entity = @populated_entities[entity.class][entity.id] + if populated_entity.nil? + @populated_entities[entity.class][entity.id] = entity + + entity + else + populated_entity + end + else + @populated_entities[entity.class] = {} + @populated_entities[entity.class][entity.id] = entity + + entity + end + end + end 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 diff --git a/spec/sideload_spec.rb b/spec/sideload_spec.rb index 3103d5b3..62429b6b 100644 --- a/spec/sideload_spec.rb +++ b/spec/sideload_spec.rb @@ -643,6 +643,7 @@ def self.name query: anything, after_resolve: anything } + expect(resource_class).to receive(:_all) .with(anything, expected, {type: :positions}) instance.load(parents, query, "parent") diff --git a/spec/sideloading_spec.rb b/spec/sideloading_spec.rb index 767b4695..c33efed3 100644 --- a/spec/sideloading_spec.rb +++ b/spec/sideloading_spec.rb @@ -1070,6 +1070,64 @@ 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" + + Graphiti.config.deduplicate_entities = nil + end + + 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"] + 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 + + after do + Graphiti.config.deduplicate_entities = nil + end + end + describe "across requests" do it "uses a different sideloaded resource" do ctx = double(current_user: :admin)