Commit 847e5a5c authored by Arturo Herrero's avatar Arturo Herrero

Pass ActiveRecord::Relation to use sub-queries

We don't need to pluck the set of ids into memory. We can pass the
ActiveRecord::Relation and use sub-queries.
parent 45b1b6a1
# frozen_string_literal: true # frozen_string_literal: true
class DataList class DataList
def initialize(batch_ids, data_fields_hash, klass) def initialize(batch, data_fields_hash, klass)
@batch_ids = batch_ids @batch = batch
@data_fields_hash = data_fields_hash @data_fields_hash = data_fields_hash
@klass = klass @klass = klass
end end
...@@ -13,15 +13,15 @@ class DataList ...@@ -13,15 +13,15 @@ class DataList
private private
attr_reader :batch_ids, :data_fields_hash, :klass attr_reader :batch, :data_fields_hash, :klass
def columns def columns
data_fields_hash.keys << 'service_id' data_fields_hash.keys << 'service_id'
end end
def values def values
batch_ids.map do |row| batch.map do |record|
data_fields_hash.values << row['id'] data_fields_hash.values << record['id']
end end
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
class ServiceList class ServiceList
def initialize(batch_ids, service_hash, association) def initialize(batch, service_hash, association)
@batch_ids = batch_ids @batch = batch
@service_hash = service_hash @service_hash = service_hash
@association = association @association = association
end end
...@@ -13,15 +13,15 @@ class ServiceList ...@@ -13,15 +13,15 @@ class ServiceList
private private
attr_reader :batch_ids, :service_hash, :association attr_reader :batch, :service_hash, :association
def columns def columns
(service_hash.keys << "#{association}_id") service_hash.keys << "#{association}_id"
end end
def values def values
batch_ids.map do |id| batch.select(:id).map do |record|
(service_hash.values << id) service_hash.values << record.id
end end
end end
end end
# frozen_string_literal: true # frozen_string_literal: true
class BulkCreateIntegrationService class BulkCreateIntegrationService
def initialize(integration, batch_ids, association) def initialize(integration, batch, association)
@integration = integration @integration = integration
@batch_ids = batch_ids @batch = batch
@association = association @association = association
end end
def execute def execute
service_list = ServiceList.new(batch_ids, service_hash, association).to_array service_list = ServiceList.new(batch, service_hash, association).to_array
Service.transaction do Service.transaction do
results = bulk_insert(*service_list) results = bulk_insert(*service_list)
...@@ -19,13 +19,13 @@ class BulkCreateIntegrationService ...@@ -19,13 +19,13 @@ class BulkCreateIntegrationService
bulk_insert(*data_list) bulk_insert(*data_list)
end end
run_callbacks(batch_ids) if association == 'project' run_callbacks(batch) if association == 'project'
end end
end end
private private
attr_reader :integration, :batch_ids, :association attr_reader :integration, :batch, :association
def bulk_insert(klass, columns, values_array) def bulk_insert(klass, columns, values_array)
items_to_insert = values_array.map { |array| Hash[columns.zip(array)] } items_to_insert = values_array.map { |array| Hash[columns.zip(array)] }
...@@ -34,13 +34,13 @@ class BulkCreateIntegrationService ...@@ -34,13 +34,13 @@ class BulkCreateIntegrationService
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def run_callbacks(batch_ids) def run_callbacks(batch)
if integration.issue_tracker? if integration.issue_tracker?
Project.where(id: batch_ids).update_all(has_external_issue_tracker: true) Project.where(id: batch.select(:id)).update_all(has_external_issue_tracker: true)
end end
if integration.type == 'ExternalWikiService' if integration.type == 'ExternalWikiService'
Project.where(id: batch_ids).update_all(has_external_wiki: true) Project.where(id: batch.select(:id)).update_all(has_external_wiki: true)
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -12,9 +12,9 @@ class PropagateIntegrationGroupWorker ...@@ -12,9 +12,9 @@ class PropagateIntegrationGroupWorker
integration = Service.find_by_id(integration_id) integration = Service.find_by_id(integration_id)
return unless integration return unless integration
batch_ids = Group.where(id: min_id..max_id).without_integration(integration).pluck(:id) batch = Group.where(id: min_id..max_id).without_integration(integration)
BulkCreateIntegrationService.new(integration, batch_ids, 'group').execute BulkCreateIntegrationService.new(integration, batch, 'group').execute
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
...@@ -12,9 +12,9 @@ class PropagateIntegrationProjectWorker ...@@ -12,9 +12,9 @@ class PropagateIntegrationProjectWorker
integration = Service.find_by_id(integration_id) integration = Service.find_by_id(integration_id)
return unless integration return unless integration
batch_ids = Project.where(id: min_id..max_id).without_integration(integration).pluck(:id) batch = Project.where(id: min_id..max_id).without_integration(integration)
BulkCreateIntegrationService.new(integration, batch_ids, 'project').execute BulkCreateIntegrationService.new(integration, batch, 'project').execute
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
...@@ -15,7 +15,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -15,7 +15,7 @@ RSpec.describe BulkCreateIntegrationService do
shared_examples 'creates integration from batch ids' do shared_examples 'creates integration from batch ids' do
it 'updates the inherited integrations' do it 'updates the inherited integrations' do
described_class.new(integration, batch_ids, association).execute described_class.new(integration, batch, association).execute
expect(created_integration.attributes.except(*excluded_attributes)) expect(created_integration.attributes.except(*excluded_attributes))
.to eq(integration.attributes.except(*excluded_attributes)) .to eq(integration.attributes.except(*excluded_attributes))
...@@ -25,7 +25,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -25,7 +25,7 @@ RSpec.describe BulkCreateIntegrationService do
let(:excluded_attributes) { %w[id service_id created_at updated_at] } let(:excluded_attributes) { %w[id service_id created_at updated_at] }
it 'updates the data fields from inherited integrations' do it 'updates the data fields from inherited integrations' do
described_class.new(integration, batch_ids, association).execute described_class.new(integration, batch, association).execute
expect(created_integration.reload.data_fields.attributes.except(*excluded_attributes)) expect(created_integration.reload.data_fields.attributes.except(*excluded_attributes))
.to eq(integration.data_fields.attributes.except(*excluded_attributes)) .to eq(integration.data_fields.attributes.except(*excluded_attributes))
...@@ -35,7 +35,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -35,7 +35,7 @@ RSpec.describe BulkCreateIntegrationService do
shared_examples 'updates inherit_from_id' do shared_examples 'updates inherit_from_id' do
it 'updates inherit_from_id attributes' do it 'updates inherit_from_id attributes' do
described_class.new(integration, batch_ids, association).execute described_class.new(integration, batch, association).execute
expect(created_integration.reload.inherit_from_id).to eq(integration.id) expect(created_integration.reload.inherit_from_id).to eq(integration.id)
end end
...@@ -43,7 +43,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -43,7 +43,7 @@ RSpec.describe BulkCreateIntegrationService do
shared_examples 'runs project callbacks' do shared_examples 'runs project callbacks' do
it 'updates projects#has_external_issue_tracker for issue tracker services' do it 'updates projects#has_external_issue_tracker for issue tracker services' do
described_class.new(integration, batch_ids, association).execute described_class.new(integration, batch, association).execute
expect(project.reload.has_external_issue_tracker).to eq(true) expect(project.reload.has_external_issue_tracker).to eq(true)
end end
...@@ -59,7 +59,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -59,7 +59,7 @@ RSpec.describe BulkCreateIntegrationService do
end end
it 'updates projects#has_external_wiki for external wiki services' do it 'updates projects#has_external_wiki for external wiki services' do
described_class.new(integration, batch_ids, association).execute described_class.new(integration, batch, association).execute
expect(project.reload.has_external_wiki).to eq(true) expect(project.reload.has_external_wiki).to eq(true)
end end
...@@ -72,7 +72,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -72,7 +72,7 @@ RSpec.describe BulkCreateIntegrationService do
context 'with a project association' do context 'with a project association' do
let!(:project) { create(:project) } let!(:project) { create(:project) }
let(:created_integration) { project.jira_service } let(:created_integration) { project.jira_service }
let(:batch_ids) { [project.id] } let(:batch) { Project.all }
let(:association) { 'project' } let(:association) { 'project' }
it_behaves_like 'creates integration from batch ids' it_behaves_like 'creates integration from batch ids'
...@@ -83,7 +83,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -83,7 +83,7 @@ RSpec.describe BulkCreateIntegrationService do
context 'with a group association' do context 'with a group association' do
let!(:group) { create(:group) } let!(:group) { create(:group) }
let(:created_integration) { Service.find_by(group: group) } let(:created_integration) { Service.find_by(group: group) }
let(:batch_ids) { [group.id] } let(:batch) { Group.all }
let(:association) { 'group' } let(:association) { 'group' }
it_behaves_like 'creates integration from batch ids' it_behaves_like 'creates integration from batch ids'
...@@ -97,7 +97,7 @@ RSpec.describe BulkCreateIntegrationService do ...@@ -97,7 +97,7 @@ RSpec.describe BulkCreateIntegrationService do
context 'with a project association' do context 'with a project association' do
let!(:project) { create(:project) } let!(:project) { create(:project) }
let(:created_integration) { project.jira_service } let(:created_integration) { project.jira_service }
let(:batch_ids) { [project.id] } let(:batch) { Project.all }
let(:association) { 'project' } let(:association) { 'project' }
it_behaves_like 'creates integration from batch ids' it_behaves_like 'creates integration from batch ids'
......
...@@ -10,7 +10,7 @@ RSpec.describe PropagateIntegrationGroupWorker do ...@@ -10,7 +10,7 @@ RSpec.describe PropagateIntegrationGroupWorker do
it 'calls to BulkCreateIntegrationService' do it 'calls to BulkCreateIntegrationService' do
expect(BulkCreateIntegrationService).to receive(:new) expect(BulkCreateIntegrationService).to receive(:new)
.with(integration, [group1.id, group2.id], 'group') .with(integration, match_array([group1, group2]), 'group')
.and_return(double(execute: nil)) .and_return(double(execute: nil))
subject.perform(integration.id, group1.id, group2.id) subject.perform(integration.id, group1.id, group2.id)
......
...@@ -10,7 +10,7 @@ RSpec.describe PropagateIntegrationProjectWorker do ...@@ -10,7 +10,7 @@ RSpec.describe PropagateIntegrationProjectWorker do
it 'calls to BulkCreateIntegrationService' do it 'calls to BulkCreateIntegrationService' do
expect(BulkCreateIntegrationService).to receive(:new) expect(BulkCreateIntegrationService).to receive(:new)
.with(integration, [project1.id, project2.id], 'project') .with(integration, match_array([project1, project2]), 'project')
.and_return(double(execute: nil)) .and_return(double(execute: nil))
subject.perform(integration.id, project1.id, project2.id) subject.perform(integration.id, project1.id, project2.id)
......
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