From ec944567db3b85110523a057869e8c51f9d598ec Mon Sep 17 00:00:00 2001 From: Matt Bessey Date: Sat, 28 Oct 2017 17:06:51 -0700 Subject: [PATCH 1/6] support append-able default batch value --- lib/batch_loader.rb | 7 ++++--- lib/batch_loader/executor_proxy.rb | 22 ++++++++++++++++++---- spec/batch_loader_spec.rb | 16 ++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/lib/batch_loader.rb b/lib/batch_loader.rb index eb8b8dc..ea2fee7 100644 --- a/lib/batch_loader.rb +++ b/lib/batch_loader.rb @@ -22,7 +22,8 @@ def initialize(item:) @item = item end - def batch(cache: true, &batch_block) + def batch(default_value: nil, cache: true, &batch_block) + @default_value = default_value.respond_to?(:call) ? default_value.call : default_value @cache = cache @batch_block = batch_block __executor_proxy.add(item: @item) @@ -80,7 +81,7 @@ def __ensure_batched @batch_block.call(items, loader) items.each do |item| next if __executor_proxy.value_loaded?(item: item) - loader.call(item, nil) # use "nil" for not loaded item after succesfull batching + loader.call(item, @default_value) end __executor_proxy.delete(items: items) end @@ -109,7 +110,7 @@ def __purge_cache def __executor_proxy @__executor_proxy ||= begin raise NoBatchError.new("Please provide a batch block first") unless @batch_block - BatchLoader::ExecutorProxy.new(&@batch_block) + BatchLoader::ExecutorProxy.new(@default_value, &@batch_block) end end diff --git a/lib/batch_loader/executor_proxy.rb b/lib/batch_loader/executor_proxy.rb index 91e2405..2c1ac68 100644 --- a/lib/batch_loader/executor_proxy.rb +++ b/lib/batch_loader/executor_proxy.rb @@ -4,9 +4,11 @@ class BatchLoader class ExecutorProxy - attr_reader :block, :global_executor + attr_reader :default_value, :block, :global_executor - def initialize(&block) + def initialize(default_value, &block) + @default_value = default_value + @value_appendable = @default_value.respond_to?(:push) @block = block @block_hash_key = block.source_location @global_executor = BatchLoader::Executor.ensure_current @@ -25,11 +27,19 @@ def delete(items:) end def load(item:, value:) - loaded[item] = value + loaded[item] = if value_appendable? + loaded_value(item: item).push(value) + else + value + end end def loaded_value(item:) - loaded[item] + if value_loaded?(item: item) + loaded[item] + else + @default_value.dup + end end def value_loaded?(item:) @@ -40,6 +50,10 @@ def unload_value(item:) loaded.delete(item) end + def value_appendable? + @value_appendable + end + private def items_to_load diff --git a/spec/batch_loader_spec.rb b/spec/batch_loader_spec.rb index a7009a9..f79cadc 100644 --- a/spec/batch_loader_spec.rb +++ b/spec/batch_loader_spec.rb @@ -98,6 +98,22 @@ expect(lazy).to eq(2) end + + it 'supports alternative default values' do + lazy = BatchLoader.for(1).batch(default_value: 123) do |nums, loader| + # No-op, so default is returned + end + + expect(lazy).to eq(123) + end + + it 'appends to the default value when possible' do + lazy = BatchLoader.for(1).batch(default_value: [1]) do |nums, loader| + nums.each { |num| loader.call(num, 123) } + end + + expect(lazy).to eq([1, 123]) + end end describe '#inspect' do From 321d8bcec012f29d4bd690705d429616d74fe195 Mon Sep 17 00:00:00 2001 From: Matt Bessey Date: Sat, 28 Oct 2017 17:37:48 -0700 Subject: [PATCH 2/6] WIP callable syntax --- lib/batch_loader.rb | 3 ++- lib/batch_loader/executor_callable.rb | 24 ++++++++++++++++++ spec/batch_loader_spec.rb | 35 +++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 lib/batch_loader/executor_callable.rb diff --git a/lib/batch_loader.rb b/lib/batch_loader.rb index ea2fee7..3481c8c 100644 --- a/lib/batch_loader.rb +++ b/lib/batch_loader.rb @@ -4,6 +4,7 @@ require_relative "./batch_loader/version" require_relative "./batch_loader/executor_proxy" +require_relative "./batch_loader/executor_callable" require_relative "./batch_loader/middleware" require_relative "./batch_loader/graphql" @@ -76,7 +77,7 @@ def __ensure_batched return if __executor_proxy.value_loaded?(item: @item) items = __executor_proxy.list_items - loader = ->(item, value) { __executor_proxy.load(item: item, value: value) } + loader = ExecutorCallable.new(__executor_proxy) @batch_block.call(items, loader) items.each do |item| diff --git a/lib/batch_loader/executor_callable.rb b/lib/batch_loader/executor_callable.rb new file mode 100644 index 0000000..e8e5a31 --- /dev/null +++ b/lib/batch_loader/executor_callable.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class BatchLoader + class ExecutorCallable + attr_reader :executor_proxy + + NULL_VALUE = :batch_loader_null + + def initialize(executor_proxy) + @executor_proxy = executor_proxy + end + + def call(item, value = NULL_VALUE) + if block_given? + raise ArgumentError, "Please pass a value or a block, not both" if value != NULL_VALUE + next_value = yield executor_proxy.loaded_value(item: item) + else + raise ArgumentError, "Please pass a value or a block" if value == NULL_VALUE + next_value = value + end + executor_proxy.load(item: item, value: next_value) + end + end +end diff --git a/spec/batch_loader_spec.rb b/spec/batch_loader_spec.rb index f79cadc..f56540c 100644 --- a/spec/batch_loader_spec.rb +++ b/spec/batch_loader_spec.rb @@ -114,6 +114,41 @@ expect(lazy).to eq([1, 123]) end + + it 'supports setting the value from return of loader.call block' do + lazy = BatchLoader.for(1).batch(default_value: {}) do |nums, loader| + nums.each do |num| + loader.call(num) { |vector| vector[num] = "ok"; vector } + end + end + + expect(lazy).to eq(1 => "ok") + end + + + context "called with block and value syntax" do + it 'raises ArgumentError' do + lazy = BatchLoader.for(1).batch(default_value: {}) do |nums, loader| + nums.each do |num| + loader.call(num, "one value") { "too many values" } + end + end + + expect { lazy.sync }.to raise_error(ArgumentError) + end + end + + context "called with neither block nor value syntax" do + it 'raises ArgumentError' do + lazy = BatchLoader.for(1).batch do |nums, loader| + nums.each do |num| + loader.call(num) + end + end + + expect { lazy.sync }.to raise_error(ArgumentError) + end + end end describe '#inspect' do From 2ad2229e50a3989b3c81347b97be186d71cd35ac Mon Sep 17 00:00:00 2001 From: Matt Bessey Date: Sun, 29 Oct 2017 20:13:55 -0700 Subject: [PATCH 3/6] [default value] do not attempt to call default value --- lib/batch_loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/batch_loader.rb b/lib/batch_loader.rb index 3481c8c..07007b2 100644 --- a/lib/batch_loader.rb +++ b/lib/batch_loader.rb @@ -24,7 +24,7 @@ def initialize(item:) end def batch(default_value: nil, cache: true, &batch_block) - @default_value = default_value.respond_to?(:call) ? default_value.call : default_value + @default_value = default_value @cache = cache @batch_block = batch_block __executor_proxy.add(item: @item) From f7d1e940382d46d11ca24b65610d3a968d86ae4d Mon Sep 17 00:00:00 2001 From: Matt Bessey Date: Sun, 29 Oct 2017 20:14:43 -0700 Subject: [PATCH 4/6] inline executor callable turns out you can pass blocks to lambdas! who knew? not i. --- lib/batch_loader.rb | 12 ++++++++++-- lib/batch_loader/executor_callable.rb | 24 ------------------------ 2 files changed, 10 insertions(+), 26 deletions(-) delete mode 100644 lib/batch_loader/executor_callable.rb diff --git a/lib/batch_loader.rb b/lib/batch_loader.rb index 07007b2..5a5a5e6 100644 --- a/lib/batch_loader.rb +++ b/lib/batch_loader.rb @@ -4,7 +4,6 @@ require_relative "./batch_loader/version" require_relative "./batch_loader/executor_proxy" -require_relative "./batch_loader/executor_callable" require_relative "./batch_loader/middleware" require_relative "./batch_loader/graphql" @@ -12,6 +11,7 @@ class BatchLoader IMPLEMENTED_INSTANCE_METHODS = %i[object_id __id__ __send__ singleton_method_added __sync respond_to? batch inspect].freeze REPLACABLE_INSTANCE_METHODS = %i[batch inspect].freeze LEFT_INSTANCE_METHODS = (IMPLEMENTED_INSTANCE_METHODS - REPLACABLE_INSTANCE_METHODS).freeze + NULL_VALUE = :batch_loader_null NoBatchError = Class.new(StandardError) @@ -77,7 +77,15 @@ def __ensure_batched return if __executor_proxy.value_loaded?(item: @item) items = __executor_proxy.list_items - loader = ExecutorCallable.new(__executor_proxy) + loader = -> (item, value = NULL_VALUE, &block) { + if block + raise ArgumentError, "Please pass a value or a block, not both" if value != NULL_VALUE + next_value = block.call(__executor_proxy.loaded_value(item: item)) + else + next_value = value + end + __executor_proxy.load(item: item, value: next_value) + } @batch_block.call(items, loader) items.each do |item| diff --git a/lib/batch_loader/executor_callable.rb b/lib/batch_loader/executor_callable.rb deleted file mode 100644 index e8e5a31..0000000 --- a/lib/batch_loader/executor_callable.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -class BatchLoader - class ExecutorCallable - attr_reader :executor_proxy - - NULL_VALUE = :batch_loader_null - - def initialize(executor_proxy) - @executor_proxy = executor_proxy - end - - def call(item, value = NULL_VALUE) - if block_given? - raise ArgumentError, "Please pass a value or a block, not both" if value != NULL_VALUE - next_value = yield executor_proxy.loaded_value(item: item) - else - raise ArgumentError, "Please pass a value or a block" if value == NULL_VALUE - next_value = value - end - executor_proxy.load(item: item, value: next_value) - end - end -end From 7b8cfa68da75a37495482086211f57d8da7b165b Mon Sep 17 00:00:00 2001 From: Matt Bessey Date: Sun, 29 Oct 2017 20:15:31 -0700 Subject: [PATCH 5/6] less magic: dont auto push to default value --- lib/batch_loader/executor_proxy.rb | 11 +---------- spec/batch_loader_spec.rb | 31 ++++++------------------------ 2 files changed, 7 insertions(+), 35 deletions(-) diff --git a/lib/batch_loader/executor_proxy.rb b/lib/batch_loader/executor_proxy.rb index 2c1ac68..deb6019 100644 --- a/lib/batch_loader/executor_proxy.rb +++ b/lib/batch_loader/executor_proxy.rb @@ -8,7 +8,6 @@ class ExecutorProxy def initialize(default_value, &block) @default_value = default_value - @value_appendable = @default_value.respond_to?(:push) @block = block @block_hash_key = block.source_location @global_executor = BatchLoader::Executor.ensure_current @@ -27,11 +26,7 @@ def delete(items:) end def load(item:, value:) - loaded[item] = if value_appendable? - loaded_value(item: item).push(value) - else - value - end + loaded[item] = value end def loaded_value(item:) @@ -50,10 +45,6 @@ def unload_value(item:) loaded.delete(item) end - def value_appendable? - @value_appendable - end - private def items_to_load diff --git a/spec/batch_loader_spec.rb b/spec/batch_loader_spec.rb index f56540c..7ec20a2 100644 --- a/spec/batch_loader_spec.rb +++ b/spec/batch_loader_spec.rb @@ -107,25 +107,18 @@ expect(lazy).to eq(123) end - it 'appends to the default value when possible' do - lazy = BatchLoader.for(1).batch(default_value: [1]) do |nums, loader| - nums.each { |num| loader.call(num, 123) } - end - - expect(lazy).to eq([1, 123]) - end - - it 'supports setting the value from return of loader.call block' do - lazy = BatchLoader.for(1).batch(default_value: {}) do |nums, loader| + it 'supports memoizing repeated calls to the same item, via a block' do + lazy = BatchLoader.for(1).batch(default_value: []) do |nums, loader| nums.each do |num| - loader.call(num) { |vector| vector[num] = "ok"; vector } + loader.call(num) { |memo| memo.push(num) } + loader.call(num) { |memo| memo.push(num + 1) } + loader.call(num) { |memo| memo.push(num + 2) } end end - expect(lazy).to eq(1 => "ok") + expect(lazy).to eq([1,2,3]) end - context "called with block and value syntax" do it 'raises ArgumentError' do lazy = BatchLoader.for(1).batch(default_value: {}) do |nums, loader| @@ -137,18 +130,6 @@ expect { lazy.sync }.to raise_error(ArgumentError) end end - - context "called with neither block nor value syntax" do - it 'raises ArgumentError' do - lazy = BatchLoader.for(1).batch do |nums, loader| - nums.each do |num| - loader.call(num) - end - end - - expect { lazy.sync }.to raise_error(ArgumentError) - end - end end describe '#inspect' do From 6b6ccb860c88ba954daae5bc81673b55278b33f1 Mon Sep 17 00:00:00 2001 From: Matt Bessey Date: Sun, 29 Oct 2017 20:39:13 -0700 Subject: [PATCH 6/6] add examples documenting new features --- CHANGELOG.md | 3 ++- README.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5e46bb..4a637b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,8 @@ that you can set version constraints properly. #### [Unreleased](https://github.com/exAspArk/batch-loader/compare/v1.0.4...HEAD) -* WIP +* `Added`: `default_value` override option. +* `Added`: `loader.call {}` block syntax, for memoizing repeat calls to the same item. #### [v1.0.4](https://github.com/exAspArk/batch-loader/compare/v1.0.3...v1.0.4) diff --git a/README.md b/README.md index e0897ba..1a3301c 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ This gem provides a generic lazy batching mechanism to avoid N+1 DB queries, HTT * [Highlights](#highlights) * [Usage](#usage) * [Why?](#why) - * [Basic example](#basic-example) + * [Basic examples](#basic-examples) * [How it works](#how-it-works) * [RESTful API example](#restful-api-example) * [GraphQL example](#graphql-example) @@ -97,7 +97,7 @@ puts users # Users But the problem here is that `load_posts` now depends on the child association and knows that it has to preload data for future use. And it'll do it every time, even if it's not necessary. Can we do better? Sure! -### Basic example +### Basic examples With `BatchLoader` we can rewrite the code above: @@ -125,6 +125,51 @@ puts users # Users SELECT * FROM users WHERE id IN As we can see, batching is isolated and described right in a place where it's needed. +For batches where there is no item in response to a call, we normally return nil. However, you can use `default_value:` to return something else instead. This is particularly useful for 1:Many relationships, where you + +```ruby +def load_posts(ids) + Post.where(id: ids) +end + +def load_user(post) + BatchLoader.for(post.user_id).batch(default_value: NullUser.new) do |user_ids, loader| + User.where(id: user_ids).each { |user| loader.call(user.id, user) } + end +end + +posts = load_posts([1, 2, 3]) + + +users = posts.map do |post| + load_user(post) +end + +puts users +``` + +For batches where the value is some kind of collection, such as an Array or Hash, `loader` also supports being called with a block, which yields the _current_ value, and returns the _next_ value. This is extremely useful for 1:Many relationships: + +```ruby +def load_users(ids) + User.where(id: ids) +end + +def load_comments(user) + BatchLoader.for(user.id).batch(default_value: []) do |comment_ids, loader| + Comment.where(user_id: user_ids).each do |comment| + loader.call(user.id) { |memo| memo.push(comment) } + end + end +end + +users = load_users([1, 2, 3]) + +comments = users.map do |user| + load_comments(user) +end +``` + ### How it works In general, `BatchLoader` returns a lazy object. Each lazy object knows which data it needs to load and how to batch the query. As soon as you need to use the lazy objects, they will be automatically loaded once without N+1 queries.