Commit 7e241158 authored by Alex Kalderimis's avatar Alex Kalderimis

Rename set_all to bulk_update

This changes the name of Gitlab::Database::SetAll to
Gitlab::Database::BulkUpdate, which is a more accurate name.
parent 71a5dc94
...@@ -111,7 +111,7 @@ module RelativePositioning ...@@ -111,7 +111,7 @@ module RelativePositioning
{ relative_position: desired_pos.clamp(lower_bound, upper_bound) } { relative_position: desired_pos.clamp(lower_bound, upper_bound) }
end end
::Gitlab::Database::SetAll.set_all([:relative_position], mapping, &:model_class) ::Gitlab::Database::BulkUpdate.execute([:relative_position], mapping, &:model_class)
end end
end end
......
...@@ -33,7 +33,7 @@ down to ARel - the `UpdateManager` just does not support `update from`, so this ...@@ -33,7 +33,7 @@ down to ARel - the `UpdateManager` just does not support `update from`, so this
is not expressible. is not expressible.
The good news: We supply an abstraction to help you generate these kinds of The good news: We supply an abstraction to help you generate these kinds of
updates, called `Gitlab::Database::SetAll`. This constructs queries such as the updates, called `Gitlab::Database::BulkUpdate`. This constructs queries such as the
above, and uses binding parameters to avoid SQL injection. above, and uses binding parameters to avoid SQL injection.
## Usage ## Usage
...@@ -51,7 +51,7 @@ issue_a = Issue.find(..) ...@@ -51,7 +51,7 @@ issue_a = Issue.find(..)
issue_b = Issue.find(..) issue_b = Issue.find(..)
# Issues a single query: # Issues a single query:
::Gitlab::Database::SetAll.set_all(%i[title weight], { ::Gitlab::Database::BulkUpdate.execute(%i[title weight], {
issue_a => { title: 'Very difficult issue', weight: 8 }, issue_a => { title: 'Very difficult issue', weight: 8 },
issue_b => { title: 'Very easy issue', weight: 1 } issue_b => { title: 'Very easy issue', weight: 1 }
}) })
...@@ -69,7 +69,7 @@ issue_b = Issue.find(..) ...@@ -69,7 +69,7 @@ issue_b = Issue.find(..)
merge_request = MergeRequest.find(..) merge_request = MergeRequest.find(..)
# Issues two queries # Issues two queries
::Gitlab::Database::SetAll.set_all(%i[title], { ::Gitlab::Database::BulkUpdate.execute(%i[title], {
issue_a => { title: 'A' }, issue_a => { title: 'A' },
issue_b => { title: 'B' }, issue_b => { title: 'B' },
merge_request => { title: 'B' } merge_request => { title: 'B' }
...@@ -90,7 +90,7 @@ objects = Foo.from_union([ ...@@ -90,7 +90,7 @@ objects = Foo.from_union([
mapping = objects.to_h { |obj| [obj, bazzes[obj.id] } mapping = objects.to_h { |obj| [obj, bazzes[obj.id] }
# Issues at most 2 queries # Issues at most 2 queries
::Gitlab::Database::SetAll.set_all(%i[baz], mapping) do |obj| ::Gitlab::Database::BulkUpdate.execute(%i[baz], mapping) do |obj|
obj.object_type.constantize obj.object_type.constantize
end end
``` ```
......
...@@ -23,13 +23,13 @@ module Gitlab ...@@ -23,13 +23,13 @@ module Gitlab
# issue_b => { title: 'That title', relative_position: 173 } # issue_b => { title: 'That title', relative_position: 173 }
# } # }
# #
# ::Gitlab::Database::SetAll.set_all(%i[title relative_position], mapping) # ::Gitlab::Database::BulkUpdate.execute(%i[title relative_position], mapping)
# #
# Note that this is a very low level tool, and operates on the raw column # Note that this is a very low level tool, and operates on the raw column
# values. Enums/state fields must be translated into their underlying # values. Enums/state fields must be translated into their underlying
# representations, for example, and no hooks will be called. # representations, for example, and no hooks will be called.
# #
module SetAll module BulkUpdate
LIST_SEPARATOR = ', ' LIST_SEPARATOR = ', '
class Setter class Setter
...@@ -58,13 +58,15 @@ module Gitlab ...@@ -58,13 +58,15 @@ module Gitlab
raise ArgumentError, 'invalid columns' if columns.blank? || columns.any? { |c| !c.is_a?(Symbol) } raise ArgumentError, 'invalid columns' if columns.blank? || columns.any? { |c| !c.is_a?(Symbol) }
raise ArgumentError, 'cannot set ID' if columns.include?(:id) raise ArgumentError, 'cannot set ID' if columns.include?(:id)
([:id] | columns).map do |name| ([:id] | columns).map { |name| column_definition(model, name) }
end
def self.column_definition(model, name)
definition = model.column_for_attribute(name) definition = model.column_for_attribute(name)
raise ArgumentError, "Unknown column: #{name}" unless definition.type raise ArgumentError, "Unknown column: #{name}" unless definition.type
definition definition
end end
end
def self.value_mapping(mapping) def self.value_mapping(mapping)
raise ArgumentError, 'invalid mapping' if mapping.blank? raise ArgumentError, 'invalid mapping' if mapping.blank?
...@@ -79,7 +81,7 @@ module Gitlab ...@@ -79,7 +81,7 @@ module Gitlab
def log_name def log_name
strong_memoize(:log_name) do strong_memoize(:log_name) do
"SetAll #{table_name} #{columns.drop(1).map(&:name)}:#{mapping.size}" "BulkUpdate #{table_name} #{columns.drop(1).map(&:name)}:#{mapping.size}"
end end
end end
...@@ -152,7 +154,7 @@ module Gitlab ...@@ -152,7 +154,7 @@ module Gitlab
end end
end end
def self.set_all(columns, mapping, &to_class) def self.execute(columns, mapping, &to_class)
raise ArgumentError if mapping.blank? raise ArgumentError if mapping.blank?
entries_by_class = mapping.group_by { |k, v| block_given? ? to_class.call(k) : k.class } entries_by_class = mapping.group_by { |k, v| block_given? ? to_class.call(k) : k.class }
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Database::SetAll do RSpec.describe Gitlab::Database::BulkUpdate do
describe 'error states' do describe 'error states' do
let(:columns) { %i[title] } let(:columns) { %i[title] }
...@@ -19,33 +19,33 @@ RSpec.describe Gitlab::Database::SetAll do ...@@ -19,33 +19,33 @@ RSpec.describe Gitlab::Database::SetAll do
end end
it 'does not raise errors on valid inputs' do it 'does not raise errors on valid inputs' do
expect { described_class.set_all(columns, mapping) }.not_to raise_error expect { described_class.execute(columns, mapping) }.not_to raise_error
end end
it 'expects a non-empty list of column names' do it 'expects a non-empty list of column names' do
expect { described_class.set_all([], mapping) }.to raise_error(ArgumentError) expect { described_class.execute([], mapping) }.to raise_error(ArgumentError)
end end
it 'expects all columns to be symbols' do it 'expects all columns to be symbols' do
expect { described_class.set_all([1], mapping) }.to raise_error(ArgumentError) expect { described_class.execute([1], mapping) }.to raise_error(ArgumentError)
end end
it 'expects all columns to be valid columns on the tables' do it 'expects all columns to be valid columns on the tables' do
expect { described_class.set_all([:foo], mapping) }.to raise_error(ArgumentError) expect { described_class.execute([:foo], mapping) }.to raise_error(ArgumentError)
end end
it 'refuses to set ID' do it 'refuses to set ID' do
expect { described_class.set_all([:id], mapping) }.to raise_error(ArgumentError) expect { described_class.execute([:id], mapping) }.to raise_error(ArgumentError)
end end
it 'expects a non-empty mapping' do it 'expects a non-empty mapping' do
expect { described_class.set_all(columns, []) }.to raise_error(ArgumentError) expect { described_class.execute(columns, []) }.to raise_error(ArgumentError)
end end
it 'expects all map values to be Hash instances' do it 'expects all map values to be Hash instances' do
bad_map = mapping.merge(build(:issue) => 2) bad_map = mapping.merge(build(:issue) => 2)
expect { described_class.set_all(columns, bad_map) }.to raise_error(ArgumentError) expect { described_class.execute(columns, bad_map) }.to raise_error(ArgumentError)
end end
end end
...@@ -56,7 +56,7 @@ RSpec.describe Gitlab::Database::SetAll do ...@@ -56,7 +56,7 @@ RSpec.describe Gitlab::Database::SetAll do
end end
expect do expect do
described_class.set_all(%i[username admin], mapping) described_class.execute(%i[username admin], mapping)
end.not_to exceed_query_limit(1) end.not_to exceed_query_limit(1)
# We have optimistically updated the values # We have optimistically updated the values
...@@ -84,7 +84,7 @@ RSpec.describe Gitlab::Database::SetAll do ...@@ -84,7 +84,7 @@ RSpec.describe Gitlab::Database::SetAll do
} }
expect do expect do
described_class.set_all(%i[title], mapping) described_class.execute(%i[title], mapping)
end.not_to exceed_query_limit(2) end.not_to exceed_query_limit(2)
expect([mr_a, i_a, i_b].map { |x| x.reset.title }) expect([mr_a, i_a, i_b].map { |x| x.reset.title })
...@@ -103,7 +103,7 @@ RSpec.describe Gitlab::Database::SetAll do ...@@ -103,7 +103,7 @@ RSpec.describe Gitlab::Database::SetAll do
i_b => { title: 'Issue b' } i_b => { title: 'Issue b' }
} }
described_class.set_all(%i[title], mapping) described_class.execute(%i[title], mapping)
expect([i_a, i_b].map { |x| x.reset.title }) expect([i_a, i_b].map { |x| x.reset.title })
.to eq(['Issue a', 'Issue b']) .to eq(['Issue a', 'Issue b'])
......
...@@ -41,7 +41,7 @@ RSpec.describe RelativePositioning::Mover do ...@@ -41,7 +41,7 @@ RSpec.describe RelativePositioning::Mover do
[issue, { relative_position: pos }] [issue, { relative_position: pos }]
end end
::Gitlab::Database::SetAll.set_all([:relative_position], mapping) ::Gitlab::Database::BulkUpdate.execute([:relative_position], mapping)
end end
def ids_in_position_order def ids_in_position_order
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment