Merge branch 'fix/admin-integrations' into 'master'
Fix new admin integrations not taking effect on existing project Closes #26376 See merge request !11069
This commit is contained in:
commit
143d88d5e6
|
@ -16,6 +16,8 @@ class Admin::ServicesController < Admin::ApplicationController
|
||||||
|
|
||||||
def update
|
def update
|
||||||
if service.update_attributes(service_params[:service])
|
if service.update_attributes(service_params[:service])
|
||||||
|
PropagateServiceTemplateWorker.perform_async(service.id) if service.active?
|
||||||
|
|
||||||
redirect_to admin_application_settings_services_path,
|
redirect_to admin_application_settings_services_path,
|
||||||
notice: 'Application settings saved successfully'
|
notice: 'Application settings saved successfully'
|
||||||
else
|
else
|
||||||
|
|
|
@ -0,0 +1,103 @@
|
||||||
|
module Projects
|
||||||
|
class PropagateServiceTemplate
|
||||||
|
BATCH_SIZE = 100
|
||||||
|
|
||||||
|
def self.propagate(*args)
|
||||||
|
new(*args).propagate
|
||||||
|
end
|
||||||
|
|
||||||
|
def initialize(template)
|
||||||
|
@template = template
|
||||||
|
end
|
||||||
|
|
||||||
|
def propagate
|
||||||
|
return unless @template.active?
|
||||||
|
|
||||||
|
Rails.logger.info("Propagating services for template #{@template.id}")
|
||||||
|
|
||||||
|
propagate_projects_with_template
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def propagate_projects_with_template
|
||||||
|
loop do
|
||||||
|
batch = project_ids_batch
|
||||||
|
|
||||||
|
bulk_create_from_template(batch) unless batch.empty?
|
||||||
|
|
||||||
|
break if batch.size < BATCH_SIZE
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def bulk_create_from_template(batch)
|
||||||
|
service_list = batch.map do |project_id|
|
||||||
|
service_hash.values << project_id
|
||||||
|
end
|
||||||
|
|
||||||
|
Project.transaction do
|
||||||
|
bulk_insert_services(service_hash.keys << 'project_id', service_list)
|
||||||
|
run_callbacks(batch)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def project_ids_batch
|
||||||
|
Project.connection.select_values(
|
||||||
|
<<-SQL
|
||||||
|
SELECT id
|
||||||
|
FROM projects
|
||||||
|
WHERE NOT EXISTS (
|
||||||
|
SELECT true
|
||||||
|
FROM services
|
||||||
|
WHERE services.project_id = projects.id
|
||||||
|
AND services.type = '#{@template.type}'
|
||||||
|
)
|
||||||
|
AND projects.pending_delete = false
|
||||||
|
AND projects.archived = false
|
||||||
|
LIMIT #{BATCH_SIZE}
|
||||||
|
SQL
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
def bulk_insert_services(columns, values_array)
|
||||||
|
ActiveRecord::Base.connection.execute(
|
||||||
|
<<-SQL.strip_heredoc
|
||||||
|
INSERT INTO services (#{columns.join(', ')})
|
||||||
|
VALUES #{values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')}
|
||||||
|
SQL
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
def service_hash
|
||||||
|
@service_hash ||=
|
||||||
|
begin
|
||||||
|
template_hash = @template.as_json(methods: :type).except('id', 'template', 'project_id')
|
||||||
|
|
||||||
|
template_hash.each_with_object({}) do |(key, value), service_hash|
|
||||||
|
value = value.is_a?(Hash) ? value.to_json : value
|
||||||
|
|
||||||
|
service_hash[ActiveRecord::Base.connection.quote_column_name(key)] =
|
||||||
|
ActiveRecord::Base.sanitize(value)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def run_callbacks(batch)
|
||||||
|
if active_external_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
|
||||||
|
|
||||||
|
def active_external_issue_tracker?
|
||||||
|
@template.issue_tracker? && !@template.default
|
||||||
|
end
|
||||||
|
|
||||||
|
def active_external_wiki?
|
||||||
|
@template.type == 'ExternalWikiService'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,21 @@
|
||||||
|
# Worker for updating any project specific caches.
|
||||||
|
class PropagateServiceTemplateWorker
|
||||||
|
include Sidekiq::Worker
|
||||||
|
include DedicatedSidekiqQueue
|
||||||
|
|
||||||
|
LEASE_TIMEOUT = 4.hours.to_i
|
||||||
|
|
||||||
|
def perform(template_id)
|
||||||
|
return unless try_obtain_lease_for(template_id)
|
||||||
|
|
||||||
|
Projects::PropagateServiceTemplate.propagate(Service.find_by(id: template_id))
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def try_obtain_lease_for(template_id)
|
||||||
|
Gitlab::ExclusiveLease.
|
||||||
|
new("propagate_service_template_worker:#{template_id}", timeout: LEASE_TIMEOUT).
|
||||||
|
try_obtain
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,4 @@
|
||||||
|
---
|
||||||
|
title: Fix new admin integrations not taking effect on existing projects
|
||||||
|
merge_request:
|
||||||
|
author:
|
|
@ -53,3 +53,4 @@
|
||||||
- [pages, 1]
|
- [pages, 1]
|
||||||
- [system_hook_push, 1]
|
- [system_hook_push, 1]
|
||||||
- [update_user_activity, 1]
|
- [update_user_activity, 1]
|
||||||
|
- [propagate_service_template, 1]
|
||||||
|
|
|
@ -23,4 +23,36 @@ describe Admin::ServicesController do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "#update" do
|
||||||
|
let(:project) { create(:empty_project) }
|
||||||
|
let!(:service) do
|
||||||
|
RedmineService.create(
|
||||||
|
project: project,
|
||||||
|
active: false,
|
||||||
|
template: true,
|
||||||
|
properties: {
|
||||||
|
project_url: 'http://abc',
|
||||||
|
issues_url: 'http://abc',
|
||||||
|
new_issue_url: 'http://abc'
|
||||||
|
}
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'calls the propagation worker when service is active' do
|
||||||
|
expect(PropagateServiceTemplateWorker).to receive(:perform_async).with(service.id)
|
||||||
|
|
||||||
|
put :update, id: service.id, service: { active: true }
|
||||||
|
|
||||||
|
expect(response).to have_http_status(302)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not call the propagation worker when service is not active' do
|
||||||
|
expect(PropagateServiceTemplateWorker).not_to receive(:perform_async)
|
||||||
|
|
||||||
|
put :update, id: service.id, service: { properties: {} }
|
||||||
|
|
||||||
|
expect(response).to have_http_status(302)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,103 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe Projects::PropagateServiceTemplate, services: true do
|
||||||
|
describe '.propagate' do
|
||||||
|
let!(:service_template) do
|
||||||
|
PushoverService.create(
|
||||||
|
template: true,
|
||||||
|
active: true,
|
||||||
|
properties: {
|
||||||
|
device: 'MyDevice',
|
||||||
|
sound: 'mic',
|
||||||
|
priority: 4,
|
||||||
|
user_key: 'asdf',
|
||||||
|
api_key: '123456789'
|
||||||
|
})
|
||||||
|
end
|
||||||
|
|
||||||
|
let!(:project) { create(:empty_project) }
|
||||||
|
|
||||||
|
it 'creates services for projects' do
|
||||||
|
expect(project.pushover_service).to be_nil
|
||||||
|
|
||||||
|
described_class.propagate(service_template)
|
||||||
|
|
||||||
|
expect(project.reload.pushover_service).to be_present
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates services for a project that has another service' do
|
||||||
|
BambooService.create(
|
||||||
|
template: true,
|
||||||
|
active: true,
|
||||||
|
project: project,
|
||||||
|
properties: {
|
||||||
|
bamboo_url: 'http://gitlab.com',
|
||||||
|
username: 'mic',
|
||||||
|
password: "password",
|
||||||
|
build_key: 'build'
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(project.pushover_service).to be_nil
|
||||||
|
|
||||||
|
described_class.propagate(service_template)
|
||||||
|
|
||||||
|
expect(project.reload.pushover_service).to be_present
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not create the service if it exists already' do
|
||||||
|
other_service = BambooService.create(
|
||||||
|
template: true,
|
||||||
|
active: true,
|
||||||
|
properties: {
|
||||||
|
bamboo_url: 'http://gitlab.com',
|
||||||
|
username: 'mic',
|
||||||
|
password: "password",
|
||||||
|
build_key: 'build'
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
Service.build_from_template(project.id, service_template).save!
|
||||||
|
Service.build_from_template(project.id, other_service).save!
|
||||||
|
|
||||||
|
expect { described_class.propagate(service_template) }.
|
||||||
|
not_to change { Service.count }
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates the service containing the template attributes' do
|
||||||
|
described_class.propagate(service_template)
|
||||||
|
|
||||||
|
expect(project.pushover_service.properties).to eq(service_template.properties)
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'bulk update' do
|
||||||
|
it 'creates services for all projects' do
|
||||||
|
project_total = 5
|
||||||
|
stub_const 'Projects::PropagateServiceTemplate::BATCH_SIZE', 3
|
||||||
|
|
||||||
|
project_total.times { create(:empty_project) }
|
||||||
|
|
||||||
|
expect { described_class.propagate(service_template) }.
|
||||||
|
to change { Service.count }.by(project_total + 1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'external tracker' do
|
||||||
|
it 'updates the project external tracker' do
|
||||||
|
service_template.update!(category: 'issue_tracker', default: false)
|
||||||
|
|
||||||
|
expect { described_class.propagate(service_template) }.
|
||||||
|
to change { project.reload.has_external_issue_tracker }.to(true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'external wiki' do
|
||||||
|
it 'updates the project external tracker' do
|
||||||
|
service_template.update!(type: 'ExternalWikiService')
|
||||||
|
|
||||||
|
expect { described_class.propagate(service_template) }.
|
||||||
|
to change { project.reload.has_external_wiki }.to(true)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,29 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe PropagateServiceTemplateWorker do
|
||||||
|
let!(:service_template) do
|
||||||
|
PushoverService.create(
|
||||||
|
template: true,
|
||||||
|
active: true,
|
||||||
|
properties: {
|
||||||
|
device: 'MyDevice',
|
||||||
|
sound: 'mic',
|
||||||
|
priority: 4,
|
||||||
|
user_key: 'asdf',
|
||||||
|
api_key: '123456789'
|
||||||
|
})
|
||||||
|
end
|
||||||
|
|
||||||
|
before do
|
||||||
|
allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:try_obtain).
|
||||||
|
and_return(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#perform' do
|
||||||
|
it 'calls the propagate service with the template' do
|
||||||
|
expect(Projects::PropagateServiceTemplate).to receive(:propagate).with(service_template)
|
||||||
|
|
||||||
|
subject.perform(service_template.id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue