Skip to content

Commit

Permalink
fix: Re-use resource class for remote sideloads to avoid memory leak (g…
Browse files Browse the repository at this point in the history
  • Loading branch information
PChambino authored Feb 27, 2024
1 parent 91c4210 commit d3e09c2
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lib/graphiti.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ def self.setup!
require "graphiti/request_validator"
require "graphiti/request_validators/validator"
require "graphiti/request_validators/update_validator"
require "graphiti/query"
require "graphiti/scope"
require "graphiti/deserializer"
require "graphiti/renderer"
Expand Down Expand Up @@ -176,6 +175,7 @@ def self.setup!
require "graphiti/extensions/boolean_attribute"
require "graphiti/extensions/temp_id"
require "graphiti/serializer"
require "graphiti/query"
require "graphiti/debugger"

if defined?(ActiveRecord)
Expand Down
9 changes: 6 additions & 3 deletions lib/graphiti/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,14 @@ def sideload_hash
end
end

class RemoteSideloadResource < ::Graphiti::Resource
self.remote = "_remote_sideload_".freeze
self.abstract_class = true # exclude from schema
end

def resource_for_sideload(sideload)
if @resource.remote?
Class.new(Graphiti::Resource) {
self.remote = "_remote_sideload_"
}.new
RemoteSideloadResource.new
else
sideload.resource
end
Expand Down
25 changes: 22 additions & 3 deletions spec/query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
before do
employee_resource.has_many :positions,
resource: position_resource
employee_resource.belongs_to :remote, remote: true
position_resource.belongs_to :department,
resource: department_resource
employee_resource.attribute :name, :string
Expand Down Expand Up @@ -986,11 +987,29 @@
end

describe "sideloads" do
before { params[:include] = "positions" }
subject(:sideloads) { instance.sideloads }

it "does not cascate the action" do
expect(sideloads.values.map(&:action)).to eq([:all])
context "when including an has_many resource" do
before { params[:include] = "positions" }

it "does not cascate the action" do
expect(sideloads.values.map(&:action)).to eq([:all])
end
end

context "when including a resource from a remote resource" do
before { params[:include] = "remote.resource" }

let(:sideloads_of_another_query) { described_class.new(resource, params).sideloads }

def resource_class_of_remote_sideload(sideloads)
sideloads.fetch(:remote).sideloads.fetch(:resource).resource.class
end

it "re-uses resource class across multiple queries (avoid memory leak)" do
expect(resource_class_of_remote_sideload(sideloads))
.to eq(resource_class_of_remote_sideload(sideloads_of_another_query))
end
end
end
end
Expand Down

0 comments on commit d3e09c2

Please sign in to comment.