From 4ae4a4799e52f9382560388a3c08296f5e596db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Wed, 16 Jan 2019 12:48:50 +0100 Subject: Update pages config only when changed --- .../projects/update_pages_configuration_service.rb | 23 +++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb index abf40b3ad7a..43eb110c994 100644 --- a/app/services/projects/update_pages_configuration_service.rb +++ b/app/services/projects/update_pages_configuration_service.rb @@ -9,8 +9,10 @@ module Projects end def execute - update_file(pages_config_file, pages_config.to_json) - reload_daemon + if update_file(pages_config_file, pages_config.to_json) + reload_daemon + end + success rescue => e error(e.message) @@ -69,7 +71,12 @@ module Projects def update_file(file, data) unless data FileUtils.remove(file, force: true) - return + return true + end + + existing_data = read_file(file) + if data == existing_data + return false end temp_file = "#{file}.#{SecureRandom.hex(16)}" @@ -77,9 +84,19 @@ module Projects 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 read_file(file) + File.open(file, 'r') do |f| + f.read + end + rescue + nil + end end end -- cgit v1.2.1 From ebda58e817b843116a9e39ffcac05c9d5aa39535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Wed, 16 Jan 2019 13:00:43 +0100 Subject: Do not reload daemon if configuration file of pages does not change --- .../projects/update_pages_configuration_service.rb | 33 ++++++++++++---------- .../update-pages-config-only-when-changed.yml | 5 ++++ .../update_pages_configuration_service_spec.rb | 32 ++++++++++++++++----- 3 files changed, 48 insertions(+), 22 deletions(-) create mode 100644 changelogs/unreleased/update-pages-config-only-when-changed.yml diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb index 43eb110c994..674071ad92a 100644 --- a/app/services/projects/update_pages_configuration_service.rb +++ b/app/services/projects/update_pages_configuration_service.rb @@ -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 diff --git a/changelogs/unreleased/update-pages-config-only-when-changed.yml b/changelogs/unreleased/update-pages-config-only-when-changed.yml new file mode 100644 index 00000000000..8d9e02df678 --- /dev/null +++ b/changelogs/unreleased/update-pages-config-only-when-changed.yml @@ -0,0 +1,5 @@ +--- +title: Do not reload daemon if configuration file of pages does not change +merge_request: +author: +type: performance diff --git a/spec/services/projects/update_pages_configuration_service_spec.rb b/spec/services/projects/update_pages_configuration_service_spec.rb index e4d4e6ff3dd..7f5ef3129d7 100644 --- a/spec/services/projects/update_pages_configuration_service_spec.rb +++ b/spec/services/projects/update_pages_configuration_service_spec.rb @@ -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 + + 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.execute).to eq({ status: :success }) + expect(subject).to include(status: :success, reload: false) + end end end end -- cgit v1.2.1