-
Notifications
You must be signed in to change notification settings - Fork 52
Open
Description
With the deprecation of BatchLoader.for
in favor of BatchLoader::GraphQL.for
in #62, we noticed there was a change in the API. We have a test that looks something like:
require 'spec_helper'
require 'batch-loader'
RSpec.describe Gitlab::Utils::BatchLoader do
let(:stubbed_loader) do
double( # rubocop:disable RSpec/VerifiedDoubles
'Loader',
load_lazy_method: [],
)
end
let(:test_module) do
Module.new do
def self.lazy_method(id)
BatchLoader.for(id).batch(key: :my_batch_name) do |ids, loader|
stubbed_loader.load_lazy_method(ids)
ids.each { |id| loader.call(id, id) }
end
end
end
before do
BatchLoader::Executor.clear_current
allow(test_module).to receive(:stubbed_loader).and_return(stubbed_loader)
end
describe '.clear_key' do
it 'clears batched items which match the specified batch key' do
test_module.lazy_method(1)
described_class.clear_key(:my_batch_name)
test_module.lazy_method(4).to_i
end
end
When we changed BatchLoader::for
to use BatchLoader::GraphQL.for
, this failed:
1) Gitlab::Utils::BatchLoader.clear_key clears batched items which match the specified batch key
Failure/Error: test_module.lazy_method(4).to_i
NoMethodError:
undefined method `to_i' for #<BatchLoader::GraphQL:0x00007f2dd33f6d50 @batch_loader=#<BatchLoader:0x545480>>
Did you mean? to_s
# ./spec/lib/gitlab/utils/batch_loader_spec.rb:x:in `block (3 levels) in <main>'
This happens because BatchLoader
has a method_missing
that auto-syncs:
batch-loader/lib/batch_loader.rb
Line 73 in e8148d8
__sync!.public_send(method_name, *args, &block) |
Is the expectation now that we must call sync
to evaluate a lazy method? Or should BatchLoader::GraphQL
also implement its own method_missing
like the following?
def method_missing(method_name, *args, &block)
sync.public_send(method_name, *args, &block)
end
Metadata
Metadata
Assignees
Labels
No labels