Refactor project and storage types

app/models/project.rb

@@ -32,7 +32,7 @@ class Project < ActiveRecord::Base
            :merge_requests_enabled?, :issues_enabled?, to: :project_feature,
                                                        allow_nil: true
 
-  delegate :base_dir, :disk_path, :ensure_storage_path_exist, :rename_repo, to: :storage
+  delegate :base_dir, :disk_path, :ensure_storage_path_exists, to: :storage
 
   default_value_for :archived, false
   default_value_for :visibility_level, gitlab_config_features.visibility_level
@@ -62,8 +62,8 @@ class Project < ActiveRecord::Base
 
   # Storage specific hooks
   after_initialize :use_hashed_storage
-  after_create :ensure_storage_path_exist
-  after_save :ensure_storage_path_exist, if: :namespace_id_changed?
+  after_create :ensure_storage_path_exists
+  after_save :ensure_storage_path_exists, if: :namespace_id_changed?
 
   acts_as_taggable
 
@@ -1262,6 +1262,50 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def rename_repo
+    new_full_path = build_full_path
+
+    Rails.logger.error "Attempting to rename #{full_path_was} -> #{new_full_path}"
+
+    if has_container_registry_tags?
+      Rails.logger.error "Project #{full_path_was} cannot be renamed because container registry tags are present!"
+
+      # we currently doesn't support renaming repository if it contains images in container registry
+      raise StandardError.new('Project cannot be renamed, because images are present in its container registry')
+    end
+
+    expire_caches_before_rename(full_path_was)
+
+    if storage.rename_repo
+      Gitlab::AppLogger.info "Project was renamed: #{full_path_was} -> #{new_full_path}"
+      rename_repo_notify!
+      after_rename_repo
+    else
+      Rails.logger.error "Repository could not be renamed: #{full_path_was} -> #{new_full_path}"
+
+      # if we cannot move namespace directory we should rollback
+      # db changes in order to prevent out of sync between db and fs
+      raise StandardError.new('repository cannot be renamed')
+    end
+  end
+
+  def rename_repo_notify!
+    send_move_instructions(full_path_was)
+    expires_full_path_cache
+
+    self.old_path_with_namespace = full_path_was
+    SystemHooksService.new.execute_hooks_for(self, :rename)
+
+    reload_repository!
+  end
+
+  def after_rename_repo
+    path_before_change = previous_changes['path'].first
+
+    Gitlab::UploadsTransfer.new.rename_project(path_before_change, self.path, namespace.full_path)
+    Gitlab::PagesTransfer.new.rename_project(path_before_change, self.path, namespace.full_path)
+  end
+
   def running_or_pending_build_count(force: false)
     Rails.cache.fetch(['projects', id, 'running_or_pending_build_count'], force: force) do
       builds.running_or_pending.count(:all)
@@ -1420,6 +1464,10 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def full_path_was
+    File.join(namespace.full_path, previous_changes['path'].first)
+  end
+
   alias_method :name_with_namespace, :full_name
   alias_method :human_name, :full_name
   # @deprecated cannot remove yet because it has an index with its name in elasticsearch

app/models/storage/hashed_project.rb

@@ -18,52 +18,15 @@ module Storage
     #
     # @return [String] combination of base_dir and the repository own name without `.git` or `.wiki.git` extensions
     def disk_path
-      "#{base_dir}/#{disk_hash}"
+      "#{base_dir}/#{disk_hash}" if disk_hash
     end
 
-    def ensure_storage_path_exist
+    def ensure_storage_path_exists
       gitlab_shell.add_namespace(repository_storage_path, base_dir)
     end
 
     def rename_repo
-      # TODO: We cannot wipe most of this method until we provide migration path for Container Registries
-      path_was = project.previous_changes['path'].first
-      old_path_with_namespace = File.join(namespace.full_path, path_was)
-      new_path_with_namespace = File.join(namespace.full_path, project.path)
-
-      Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}"
-
-      if project.has_container_registry_tags?
-        Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!"
-
-        # we currently doesn't support renaming repository if it contains images in container registry
-        raise StandardError.new('Project cannot be renamed, because images are present in its container registry')
-      end
-
-      begin
-        # TODO: we can avoid cache expiration if cache is based on UUID or just project_id
-        project.expire_caches_before_rename(old_path_with_namespace)
-        project.expires_full_path_cache
-
-        project.send_move_instructions(old_path_with_namespace)
-
-        project.old_path_with_namespace = old_path_with_namespace
-
-        SystemHooksService.new.execute_hooks_for(project, :rename)
-
-        project.reload_repository!
-      rescue => e
-        Rails.logger.error "Exception renaming #{old_path_with_namespace} -> #{new_path_with_namespace}: #{e}"
-        # Returning false does not rollback after_* transaction but gives
-        # us information about failing some of tasks
-        false
-      end
-
-      Gitlab::AppLogger.info "Project was renamed: #{old_path_with_namespace} -> #{new_path_with_namespace}"
-
-      # TODO: When we move Uploads and Pages to use UUID we can disable this transfers as well
-      Gitlab::UploadsTransfer.new.rename_project(path_was, project.path, namespace.full_path)
-      Gitlab::PagesTransfer.new.rename_project(path_was, project.path, namespace.full_path)
+      true
     end
 
     private

app/models/storage/legacy_project.rb

@@ -21,60 +21,31 @@ module Storage
       project.full_path
     end
 
-    def ensure_storage_path_exist
+    def ensure_storage_path_exists
       return unless namespace
 
       gitlab_shell.add_namespace(repository_storage_path, base_dir)
     end
 
     def rename_repo
-      path_was = project.previous_changes['path'].first
-      old_path_with_namespace = File.join(base_dir, path_was)
-      new_path_with_namespace = File.join(base_dir, project.path)
+      new_full_path = project.build_full_path
 
-      Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}"
-
-      if project.has_container_registry_tags?
-        Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!"
-
-        # we currently doesn't support renaming repository if it contains images in container registry
-        raise StandardError.new('Project cannot be renamed, because images are present in its container registry')
-      end
-
-      project.expire_caches_before_rename(old_path_with_namespace)
-
-      if gitlab_shell.mv_repository(repository_storage_path, old_path_with_namespace, new_path_with_namespace)
+      if gitlab_shell.mv_repository(repository_storage_path, project.full_path_was, new_full_path)
         # If repository moved successfully we need to send update instructions to users.
         # However we cannot allow rollback since we moved repository
         # So we basically we mute exceptions in next actions
         begin
-          gitlab_shell.mv_repository(repository_storage_path, "#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki")
-          project.send_move_instructions(old_path_with_namespace)
-          project.expires_full_path_cache
-
-          project.old_path_with_namespace = old_path_with_namespace
-
-          SystemHooksService.new.execute_hooks_for(project, :rename)
-
-          project.reload_repository!
+          gitlab_shell.mv_repository(repository_storage_path, "#{project.full_path_was}.wiki", "#{new_full_path}.wiki")
+          return true
         rescue => e
-          Rails.logger.error "Exception renaming #{old_path_with_namespace} -> #{new_path_with_namespace}: #{e}"
+          Rails.logger.error "Exception renaming #{project.full_path_was} -> #{new_full_path}: #{e}"
           # Returning false does not rollback after_* transaction but gives
           # us information about failing some of tasks
-          false
+          return false
         end
-      else
-        Rails.logger.error "Repository could not be renamed: #{old_path_with_namespace} -> #{new_path_with_namespace}"
-
-        # if we cannot move namespace directory we should rollback
-        # db changes in order to prevent out of sync between db and fs
-        raise StandardError.new('repository cannot be renamed')
       end
 
-      Gitlab::AppLogger.info "Project was renamed: #{old_path_with_namespace} -> #{new_path_with_namespace}"
-
-      Gitlab::UploadsTransfer.new.rename_project(path_was, project.path, base_dir)
-      Gitlab::PagesTransfer.new.rename_project(path_was, project.path, base_dir)
+      false
     end
   end
 end

lib/backup/repository.rb

@@ -75,7 +75,7 @@ module Backup
         path_to_project_repo = path_to_repo(project)
         path_to_project_bundle = path_to_bundle(project)
 
-        project.ensure_storage_path_exist
+        project.ensure_storage_path_exists
 
         cmd = if File.exist?(path_to_project_bundle)
                 %W(#{Gitlab.config.git.bin_path} clone --bare #{path_to_project_bundle} #{path_to_project_repo})

spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb

@@ -4,7 +4,7 @@ require Rails.root.join('db', 'post_migrate', '20170502101023_cleanup_namespacel
 describe CleanupNamespacelessPendingDeleteProjects do
   before do
     # Stub after_save callbacks that will fail when Project has no namespace
-    allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil)
+    allow_any_instance_of(Project).to receive(:ensure_storage_path_exists).and_return(nil)
     allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil)
   end
 

spec/models/project_spec.rb

@@ -2334,11 +2334,11 @@ describe Project do
       end
     end
 
-    describe '#ensure_storage_path_exist' do
+    describe '#ensure_storage_path_exists' do
       it 'delegates to gitlab_shell to ensure namespace is created' do
         expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, project.base_dir)
 
-        project.ensure_storage_path_exist
+        project.ensure_storage_path_exists
       end
     end
 
@@ -2425,11 +2425,11 @@ describe Project do
       end
     end
 
-    describe '#ensure_storage_path_exist' do
+    describe '#ensure_storage_path_exists' do
       it 'delegates to gitlab_shell to ensure namespace is created' do
         expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, '6b/86')
 
-        project.ensure_storage_path_exist
+        project.ensure_storage_path_exists
       end
     end
 

spec/workers/namespaceless_project_destroy_worker_spec.rb

@@ -5,7 +5,7 @@ describe NamespacelessProjectDestroyWorker do
 
   before do
     # Stub after_save callbacks that will fail when Project has no namespace
-    allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil)
+    allow_any_instance_of(Project).to receive(:ensure_storage_path_exists).and_return(nil)
     allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil)
   end