Merge branch 'stop-dynamic-routable-creation' into 'master'

Stop building Route rows on the fly

See merge request gitlab-org/gitlab-ce!20313

4 files changed, 2 insertions, 48 deletions

diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 0176a12a131..cb91f8fbac8 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -90,34 +90,17 @@ module Routable
   end
 
   def full_name
-    if route && route.name.present?
-      @full_name ||= route.name # rubocop:disable Gitlab/ModuleWithInstanceVariables
-    else
-      update_route if persisted?
-
-      build_full_name
-    end
+    route&.name || build_full_name
   end
 
-  # Every time `project.namespace.becomes(Namespace)` is called for polymorphic_path,
-  # a new instance is instantiated, and we end up duplicating the same query to retrieve
-  # the route. Caching this per request ensures that even if we have multiple instances,
-  # we will not have to duplicate work, avoiding N+1 queries in some cases.
   def full_path
-    return uncached_full_path unless RequestStore.active? && persisted?
-
-    RequestStore[full_path_key] ||= uncached_full_path
+    route&.path || build_full_path
   end
 
   def full_path_components
     full_path.split('/')
   end
 
-  def expires_full_path_cache
-    RequestStore.delete(full_path_key) if RequestStore.active?
-    @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  end
-
   def build_full_path
     if parent && path
       parent.full_path + '/' + path
@@ -138,16 +121,6 @@ module Routable
     self.errors[:path].concat(route_path_errors) if route_path_errors
   end
 
-  def uncached_full_path
-    if route && route.path.present?
-      @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables
-    else
-      update_route if persisted?
-
-      build_full_path
-    end
-  end
-
   def full_name_changed?
     name_changed? || parent_changed?
   end
@@ -156,10 +129,6 @@ 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
@@ -168,18 +137,9 @@ module Routable
     end
   end
 
-  def update_route
-    return if Gitlab::Database.read_only?
-
-    prepare_route
-    route.save
-  end
-
   def prepare_route
     route || build_route(source: self)
     route.path = build_full_path
     route.name = build_full_name
-    @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
-    @full_name = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
   end
 end
diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb
index ae6595320cb..f5225cd81ed 100644
--- a/app/models/concerns/storage/legacy_namespace.rb
+++ b/app/models/concerns/storage/legacy_namespace.rb
@@ -11,8 +11,6 @@ module Storage
                      Namespace.find(parent_id_was) # raise NotFound early if needed
                    end
 
-      expires_full_path_cache
-
       move_repositories
 
       if parent_changed?
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 7034c633268..c1dc2f55346 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -304,7 +304,6 @@ class Namespace < ActiveRecord::Base
 
   def write_projects_repository_config
     all_projects.find_each do |project|
-      project.expires_full_path_cache # we need to clear cache to validate renames correctly
       project.write_repository_config
     end
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 32315dfaa01..b876270116e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1238,8 +1238,6 @@ class Project < ActiveRecord::Base
     return true if skip_disk_validation
     return false unless repository_storage
 
-    expires_full_path_cache # we need to clear cache to validate renames correctly
-
     # Check if repository with same path already exists on disk we can
     # skip this for the hashed storage because the path does not change
     if legacy_storage? && repository_with_same_path_already_exists?
@@ -1618,7 +1616,6 @@ class Project < ActiveRecord::Base
     # When we import a project overwriting the original project, there
     # is a move operation. In that case we don't want to send the instructions.
     send_move_instructions(full_path_was) unless import_started?
-    expires_full_path_cache
 
     self.old_path_with_namespace = full_path_was
     SystemHooksService.new.execute_hooks_for(self, :rename)