From 2446252cfd1f5a7d7329099e8d6371fcffa99971 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 29 Jun 2017 18:53:32 -0300 Subject: [PATCH 1/4] Expires full_path cache after project is renamed --- app/models/concerns/routable.rb | 11 +++++++++-- app/models/project.rb | 1 + spec/models/concerns/routable_spec.rb | 12 ++++++++++++ spec/models/project_spec.rb | 2 ++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index ec7796a9dbb..2305e01d3f1 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -103,8 +103,11 @@ module Routable def full_path return uncached_full_path unless RequestStore.active? - key = "routable/full_path/#{self.class.name}/#{self.id}" - RequestStore[key] ||= uncached_full_path + RequestStore[full_path_key] ||= uncached_full_path + end + + def expires_full_path_cache + RequestStore.delete(full_path_key) if RequestStore.active? end def build_full_path @@ -135,6 +138,10 @@ module Routable path_changed? || parent_changed? end + def full_path_key + @full_path_key ||= "routable/full_path/#{self.class.name}/#{self.id}" + end + def build_full_name if parent && name parent.human_name + ' / ' + name diff --git a/app/models/project.rb b/app/models/project.rb index a75c5209955..862ee027e54 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -963,6 +963,7 @@ class Project < ActiveRecord::Base begin gitlab_shell.mv_repository(repository_storage_path, "#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki") send_move_instructions(old_path_with_namespace) + expires_full_path_cache @old_path_with_namespace = old_path_with_namespace diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 65f05121b40..be82a601b36 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -132,6 +132,18 @@ describe Group, 'Routable' do end end + describe '#expires_full_path_cache' do + context 'with RequestStore active', :request_store do + it 'expires the full_path cache' do + expect(group).to receive(:uncached_full_path).twice.and_call_original + + 3.times { group.full_path } + group.expires_full_path_cache + 3.times { group.full_path } + end + end + end + describe '#full_name' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1390848ff4a..6ff4ec3d417 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1216,6 +1216,8 @@ describe Project, models: true do expect(project).to receive(:expire_caches_before_rename) + expect(project).to receive(:expires_full_path_cache) + project.rename_repo end From b3b034b849800e71ce8872a6f4db8fb6b9b91b4e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 29 Jun 2017 19:11:33 -0300 Subject: [PATCH 2/4] Expires full_path cache after repository is transferred --- app/services/projects/transfer_service.rb | 1 + spec/services/projects/transfer_service_spec.rb | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index fd701e33524..4bb98e5cb4e 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -78,6 +78,7 @@ module Projects Gitlab::PagesTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path) project.old_path_with_namespace = @old_path + project.expires_full_path_cache execute_system_hooks end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 76c52d55ae5..441a5276c56 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -30,6 +30,12 @@ describe Projects::TransferService, services: true do transfer_project(project, user, group) end + it 'expires full_path cache' do + expect(project).to receive(:expires_full_path_cache) + + transfer_project(project, user, group) + end + it 'executes system hooks' do expect_any_instance_of(Projects::TransferService).to receive(:execute_system_hooks) From 7fe97d940ee49afb85f5c8df0e9ab267ba40391d Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 29 Jun 2017 18:53:56 -0300 Subject: [PATCH 3/4] Add CHANGELOG --- changelogs/unreleased/fix-2801.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/fix-2801.yml diff --git a/changelogs/unreleased/fix-2801.yml b/changelogs/unreleased/fix-2801.yml new file mode 100644 index 00000000000..4f0aa06b6ba --- /dev/null +++ b/changelogs/unreleased/fix-2801.yml @@ -0,0 +1,4 @@ +--- +title: Expires full_path cache after a repository is renamed/transferred +merge_request: +author: From ea9dd29a76477e73104764d2e7f937bb2ccce8c0 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 3 Jul 2017 12:38:01 -0300 Subject: [PATCH 4/4] Reset @full_path to nil when cache expires --- app/models/concerns/routable.rb | 1 + spec/models/concerns/routable_spec.rb | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index 2305e01d3f1..ee108f010a6 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -108,6 +108,7 @@ module Routable def expires_full_path_cache RequestStore.delete(full_path_key) if RequestStore.active? + @full_path = nil end def build_full_path diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index be82a601b36..36aedd2f701 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -135,11 +135,12 @@ describe Group, 'Routable' do describe '#expires_full_path_cache' do context 'with RequestStore active', :request_store do it 'expires the full_path cache' do - expect(group).to receive(:uncached_full_path).twice.and_call_original + expect(group.full_path).to eq('foo') - 3.times { group.full_path } + group.route.update(path: 'bar', name: 'bar') group.expires_full_path_cache - 3.times { group.full_path } + + expect(group.full_path).to eq('bar') end end end