Do not reload daemon if configuration file of pages does not change
This commit is contained in:
parent
4ae4a4799e
commit
ebda58e817
3 changed files with 48 additions and 22 deletions
|
@ -2,6 +2,8 @@
|
|||
|
||||
module Projects
|
||||
class UpdatePagesConfigurationService < BaseService
|
||||
include Gitlab::Utils::StrongMemoize
|
||||
|
||||
attr_reader :project
|
||||
|
||||
def initialize(project)
|
||||
|
@ -9,17 +11,25 @@ module Projects
|
|||
end
|
||||
|
||||
def execute
|
||||
if update_file(pages_config_file, pages_config.to_json)
|
||||
reload_daemon
|
||||
if file_equals?(pages_config_file, pages_config_json)
|
||||
return success(reload: false)
|
||||
end
|
||||
|
||||
success
|
||||
update_file(pages_config_file, pages_config_json)
|
||||
reload_daemon
|
||||
success(reload: true)
|
||||
rescue => e
|
||||
error(e.message)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def pages_config_json
|
||||
strong_memoize(:pages_config_json) do
|
||||
pages_config.to_json
|
||||
end
|
||||
end
|
||||
|
||||
def pages_config
|
||||
{
|
||||
domains: pages_domains_config,
|
||||
|
@ -69,28 +79,21 @@ module Projects
|
|||
end
|
||||
|
||||
def update_file(file, data)
|
||||
unless data
|
||||
FileUtils.remove(file, force: true)
|
||||
return true
|
||||
end
|
||||
|
||||
existing_data = read_file(file)
|
||||
if data == existing_data
|
||||
return false
|
||||
end
|
||||
|
||||
temp_file = "#{file}.#{SecureRandom.hex(16)}"
|
||||
File.open(temp_file, 'w') do |f|
|
||||
f.write(data)
|
||||
end
|
||||
FileUtils.move(temp_file, file, force: true)
|
||||
|
||||
true
|
||||
ensure
|
||||
# In case if the updating fails
|
||||
FileUtils.remove(temp_file, force: true)
|
||||
end
|
||||
|
||||
def file_equals?(file, data)
|
||||
existing_data = read_file(file)
|
||||
data == existing_data.to_s
|
||||
end
|
||||
|
||||
def read_file(file)
|
||||
File.open(file, 'r') do |f|
|
||||
f.read
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Do not reload daemon if configuration file of pages does not change
|
||||
merge_request:
|
||||
author:
|
||||
type: performance
|
|
@ -2,23 +2,41 @@ require 'spec_helper'
|
|||
|
||||
describe Projects::UpdatePagesConfigurationService do
|
||||
let(:project) { create(:project) }
|
||||
subject { described_class.new(project) }
|
||||
let(:service) { described_class.new(project) }
|
||||
|
||||
describe "#update" do
|
||||
let(:file) { Tempfile.new('pages-test') }
|
||||
|
||||
subject { service.execute }
|
||||
|
||||
after do
|
||||
file.close
|
||||
file.unlink
|
||||
end
|
||||
|
||||
it 'updates the .update file' do
|
||||
# Access this reference to ensure scoping works
|
||||
Projects::Settings # rubocop:disable Lint/Void
|
||||
expect(subject).to receive(:pages_config_file).and_return(file.path)
|
||||
expect(subject).to receive(:reload_daemon).and_call_original
|
||||
before do
|
||||
allow(service).to receive(:pages_config_file).and_return(file.path)
|
||||
end
|
||||
|
||||
expect(subject.execute).to eq({ status: :success })
|
||||
context 'when configuration changes' do
|
||||
it 'updates the .update file' do
|
||||
expect(service).to receive(:reload_daemon).and_call_original
|
||||
|
||||
expect(subject).to include(status: :success, reload: true)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when configuration does not change' do
|
||||
before do
|
||||
# we set the configuration
|
||||
service.execute
|
||||
end
|
||||
|
||||
it 'does not update the .update file' do
|
||||
expect(service).not_to receive(:reload_daemon)
|
||||
|
||||
expect(subject).to include(status: :success, reload: false)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue