Merge branch '209831-improve-propagate-service' into 'master'

Extract PropagateService mixin

See merge request gitlab-org/gitlab!41701

app/models/service_list.rb

 # frozen_string_literal: true
 
 class ServiceList
-  def initialize(batch, service_hash, extra_hash = {})
-    @batch = batch
+  def initialize(batch_ids, service_hash)
+    @batch_ids = batch_ids
     @service_hash = service_hash
-    @extra_hash = extra_hash
   end
 
   def to_array
@@ -13,15 +12,15 @@ class ServiceList
 
   private
 
-  attr_reader :batch, :service_hash, :extra_hash
+  attr_reader :batch_ids, :service_hash
 
   def columns
-    (service_hash.keys << 'project_id') + extra_hash.keys
+    (service_hash.keys << 'project_id')
   end
 
   def values
-    batch.map do |project_id|
-      (service_hash.values << project_id) + extra_hash.values
+    batch_ids.map do |project_id|
+      (service_hash.values << project_id)
     end
   end
 end

app/services/admin/propagate_integration_service.rb

@@ -2,17 +2,7 @@
 
 module Admin
   class PropagateIntegrationService
-    BATCH_SIZE = 100
-
-    delegate :data_fields_present?, to: :integration
-
-    def self.propagate(integration)
-      new(integration).propagate
-    end
-
-    def initialize(integration)
-      @integration = integration
-    end
+    include PropagateService
 
     def propagate
       update_inherited_integrations
@@ -21,8 +11,6 @@ module Admin
 
     private
 
-    attr_reader :integration
-
     # rubocop: disable Cop/InBatches
     # rubocop: disable CodeReuse/ActiveRecord
     def update_inherited_integrations
@@ -50,61 +38,9 @@ module Admin
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    def create_integration_for_projects_without_integration
-      loop do
-        batch = Project.uncached { Project.ids_without_integration(integration, BATCH_SIZE) }
-
-        bulk_create_from_integration(batch) unless batch.empty?
-
-        break if batch.size < BATCH_SIZE
-      end
-    end
-
-    def bulk_create_from_integration(batch)
-      service_list = ServiceList.new(batch, service_hash, { 'inherit_from_id' => integration.id }).to_array
-
-      Project.transaction do
-        results = bulk_insert(*service_list)
-
-        if data_fields_present?
-          data_list = DataList.new(results, data_fields_hash, integration.data_fields.class).to_array
-
-          bulk_insert(*data_list)
-        end
-
-        run_callbacks(batch)
-      end
-    end
-
-    def bulk_insert(klass, columns, values_array)
-      items_to_insert = values_array.map { |array| Hash[columns.zip(array)] }
-
-      klass.insert_all(items_to_insert, returning: [:id])
-    end
-
-    # rubocop: disable CodeReuse/ActiveRecord
-    def run_callbacks(batch)
-      if integration.issue_tracker?
-        Project.where(id: batch).update_all(has_external_issue_tracker: true)
-      end
-
-      if active_external_wiki?
-        Project.where(id: batch).update_all(has_external_wiki: true)
-      end
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def active_external_wiki?
-      integration.type == 'ExternalWikiService'
-    end
-
     def service_hash
       @service_hash ||= integration.to_service_hash
         .tap { |json| json['inherit_from_id'] = integration.id }
     end
-
-    def data_fields_hash
-      @data_fields_hash ||= integration.to_data_fields_hash
-    end
   end
 end

app/services/admin/propagate_service_template.rb

+# frozen_string_literal: true
+
+module Admin
+  class PropagateServiceTemplate
+    include PropagateService
+
+    def propagate
+      return unless integration.active?
+
+      create_integration_for_projects_without_integration
+    end
+
+    private
+
+    def service_hash
+      @service_hash ||= integration.to_service_hash
+    end
+  end
+end

app/services/projects/propagate_service_template.rb → app/services/concerns/admin/propagate_service.rb

 # frozen_string_literal: true
 
-module Projects
-  class PropagateServiceTemplate
-    BATCH_SIZE = 100
+module Admin
+  module PropagateService
+    extend ActiveSupport::Concern
 
-    delegate :data_fields_present?, to: :template
+    BATCH_SIZE = 100
 
-    def self.propagate(template)
-      new(template).propagate
-    end
+    delegate :data_fields_present?, to: :integration
 
-    def initialize(template)
-      @template = template
+    class_methods do
+      def propagate(integration)
+        new(integration).propagate
+      end
     end
 
-    def propagate
-      return unless template.active?
-
-      propagate_projects_with_template
+    def initialize(integration)
+      @integration = integration
     end
 
     private
 
-    attr_reader :template
+    attr_reader :integration
 
-    def propagate_projects_with_template
+    def create_integration_for_projects_without_integration
       loop do
-        batch = Project.uncached { Project.ids_without_integration(template, BATCH_SIZE) }
+        batch_ids = Project.uncached { Project.ids_without_integration(integration, BATCH_SIZE) }
 
-        bulk_create_from_template(batch) unless batch.empty?
+        bulk_create_from_integration(batch_ids) unless batch_ids.empty?
 
-        break if batch.size < BATCH_SIZE
+        break if batch_ids.size < BATCH_SIZE
       end
     end
 
-    def bulk_create_from_template(batch)
-      service_list = ServiceList.new(batch, service_hash).to_array
+    def bulk_create_from_integration(batch_ids)
+      service_list = ServiceList.new(batch_ids, service_hash).to_array
 
-      Project.transaction do
+      Service.transaction do
         results = bulk_insert(*service_list)
 
         if data_fields_present?
-          data_list = DataList.new(results, data_fields_hash, template.data_fields.class).to_array
+          data_list = DataList.new(results, data_fields_hash, integration.data_fields.class).to_array
 
           bulk_insert(*data_list)
         end
 
-        run_callbacks(batch)
+        run_callbacks(batch_ids)
       end
     end
 
@@ -56,28 +54,20 @@ module Projects
       klass.insert_all(items_to_insert, returning: [:id])
     end
 
-    def service_hash
-      @service_hash ||= template.to_service_hash
-    end
-
-    def data_fields_hash
-      @data_fields_hash ||= template.to_data_fields_hash
-    end
-
     # rubocop: disable CodeReuse/ActiveRecord
-    def run_callbacks(batch)
-      if template.issue_tracker?
-        Project.where(id: batch).update_all(has_external_issue_tracker: true)
+    def run_callbacks(batch_ids)
+      if integration.issue_tracker?
+        Project.where(id: batch_ids).update_all(has_external_issue_tracker: true)
       end
 
-      if active_external_wiki?
-        Project.where(id: batch).update_all(has_external_wiki: true)
+      if integration.type == 'ExternalWikiService'
+        Project.where(id: batch_ids).update_all(has_external_wiki: true)
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
-    def active_external_wiki?
-      template.type == 'ExternalWikiService'
+    def data_fields_hash
+      @data_fields_hash ||= integration.to_data_fields_hash
     end
   end
 end

app/workers/propagate_service_template_worker.rb

@@ -12,7 +12,7 @@ class PropagateServiceTemplateWorker # rubocop:disable Scalability/IdempotentWor
   def perform(template_id)
     return unless try_obtain_lease_for(template_id)
 
-    Projects::PropagateServiceTemplate.propagate(Service.find_by(id: template_id))
+    Admin::PropagateServiceTemplate.propagate(Service.find_by(id: template_id))
   end
   # rubocop: enable CodeReuse/ActiveRecord
 

spec/services/projects/propagate_service_template_spec.rb → spec/services/admin/propagate_service_template_spec.rb

@@ -2,10 +2,10 @@
 
 require 'spec_helper'
 
-RSpec.describe Projects::PropagateServiceTemplate do
+RSpec.describe Admin::PropagateServiceTemplate do
   describe '.propagate' do
     let!(:service_template) do
-      PushoverService.create(
+      PushoverService.create!(
         template: true,
         active: true,
         push_events: false,
@@ -31,8 +31,7 @@ RSpec.describe Projects::PropagateServiceTemplate do
     end
 
     it 'creates services for a project that has another service' do
-      BambooService.create(
-        template: true,
+      BambooService.create!(
         active: true,
         project: project,
         properties: {
@@ -51,7 +50,7 @@ RSpec.describe Projects::PropagateServiceTemplate do
     end
 
     it 'does not create the service if it exists already' do
-      other_service = BambooService.create(
+      other_service = BambooService.create!(
         template: true,
         active: true,
         properties: {
@@ -110,7 +109,7 @@ RSpec.describe Projects::PropagateServiceTemplate do
       let(:project_total) { 5 }
 
       before do
-        stub_const 'Projects::PropagateServiceTemplate::BATCH_SIZE', 3
+        stub_const('Admin::PropagateServiceTemplate::BATCH_SIZE', 3)
 
         project_total.times { create(:project) }
 

spec/workers/propagate_service_template_worker_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe PropagateServiceTemplateWorker do
       stub_exclusive_lease("propagate_service_template_worker:#{template.id}",
         timeout: PropagateServiceTemplateWorker::LEASE_TIMEOUT)
 
-      expect(Projects::PropagateServiceTemplate)
+      expect(Admin::PropagateServiceTemplate)
         .to receive(:propagate)
         .with(template)