Move directory logic out of model. Use Gitlab:Shell class to interact with file system

app/models/namespace.rb

@@ -13,6 +13,8 @@
 #
 
 class Namespace < ActiveRecord::Base
+  include Gitlab::ShellAdapter
+
   attr_accessible :name, :description, :path
 
   has_many :projects, dependent: :destroy
@@ -31,7 +33,7 @@ class Namespace < ActiveRecord::Base
   delegate :name, to: :owner, allow_nil: true, prefix: true
 
   after_create :ensure_dir_exist
-  after_update :move_dir
+  after_update :move_dir, if: :path_changed?
   after_destroy :rm_dir
 
   scope :root, -> { where('type IS NULL') }
@@ -53,46 +55,32 @@ class Namespace < ActiveRecord::Base
   end
 
   def ensure_dir_exist
-    unless dir_exists?
+    gitlab_shell.add_namespace(path)
-      FileUtils.mkdir( namespace_full_path, mode: 0770 )
-    end
-  end
-
-  def dir_exists?
-    File.exists?(namespace_full_path)
-  end
-
-  def namespace_full_path
-    @namespace_full_path ||= File.join(Gitlab.config.gitlab_shell.repos_path, path)
-  end
-
-  def move_dir
-    if path_changed?
-      old_path = File.join(Gitlab.config.gitlab_shell.repos_path, path_was)
-      new_path = File.join(Gitlab.config.gitlab_shell.repos_path, path)
-      if File.exists?(new_path)
-        raise "Already exists"
-      end
-
-
-      begin
-        # Remove satellite when moving repo
-        if path_was.present?
-          satellites_path = File.join(Gitlab.config.satellites.path, path_was)
-          FileUtils.rm_r( satellites_path, force: true )
-        end
-
-        FileUtils.mv( old_path, new_path )
-        send_update_instructions
-      rescue Exception => e
-        raise "Namespace move error #{old_path} #{new_path}"
-      end
-    end
   end
 
   def rm_dir
-    dir_path = File.join(Gitlab.config.gitlab_shell.repos_path, path)
+    gitlab_shell.rm_namespace(path)
-    FileUtils.rm_r( dir_path, force: true )
+  end
+
+  def move_dir
+    if gitlab_shell.mv_namespace(path_was, path)
+      # If repositories moved successfully we need to remove old satellites
+      # and send update instructions to users.
+      # However we cannot allow rollback since we moved namespace dir
+      # So we basically we mute exceptions in next actions
+      begin
+        gitlab_shell.rm_satellites(path_was)
+        send_update_instructions
+      rescue
+        # Returning false does not rolback after_* transaction but gives
+        # us information about failing some of tasks
+        false
+      end
+    else
+      # if we cannot move namespace directory we should rollback
+      # db changes in order to prevent out of sync between db and fs
+      raise Exception.new('namespace directory cannot be moved')
+    end
   end
 
   def send_update_instructions

lib/gitlab/backend/shell.rb

@@ -65,13 +65,72 @@ module Gitlab
       system("#{gitlab_shell_user_home}/gitlab-shell/bin/gitlab-keys rm-key #{key_id} \"#{key_content}\"")
     end
 
+    # Add empty directory for storing repositories
+    #
+    # Ex.
+    #   add_namespace("gitlab")
+    #
+    def add_namespace(name)
+      FileUtils.mkdir(full_path(name), mode: 0770) unless exists?(name)
+    end
+
+    # Remove directory from repositories storage
+    # Every repository inside this directory will be removed too
+    #
+    # Ex.
+    #   rm_namespace("gitlab")
+    #
+    def rm_namespace(name)
+      FileUtils.rm_r(full_path(name), force: true)
+    end
+
+    # Move namespace directory inside repositories storage
+    #
+    # Ex.
+    #   mv_namespace("gitlab", "gitlabhq")
+    #
+    def mv_namespace(old_name, new_name)
+      return false if exists?(new_name) || !exists?(old_name)
+
+      FileUtils.mv(full_path(old_name), full_path(new_name))
+    end
+
+    # Remove GitLab Satellites for provided path (namespace or repo dir)
+    #
+    # Ex.
+    #   rm_satellites("gitlab")
+    #
+    #   rm_satellites("gitlab/gitlab-ci.git")
+    #
+    def rm_satellites(path)
+      raise ArgumentError.new("Path can't be blank") if path.blank?
+
+      satellites_path = File.join(Gitlab.config.satellites.path, path)
+      FileUtils.rm_r(satellites_path, force: true)
+    end
+
     def url_to_repo path
       Gitlab.config.gitlab_shell.ssh_path_prefix + "#{path}.git"
     end
 
+    protected
+
     def gitlab_shell_user_home
       File.expand_path("~#{Gitlab.config.gitlab_shell.ssh_user}")
     end
 
+    def repos_path
+      Gitlab.config.gitlab_shell.repos_path
+    end
+
+    def full_path(dir_name)
+      raise ArgumentError.new("Directory name can't be blank") if dir_name.blank?
+
+      File.join(repos_path, dir_name)
+    end
+
+    def exists?(dir_name)
+      File.exists?(full_path(dir_name))
+    end
   end
 end

spec/models/namespace_spec.rb

@@ -60,7 +60,7 @@ describe Namespace do
     end
 
     it "should raise error when dirtory exists" do
-      expect { @namespace.move_dir }.to raise_error("Already exists")
+      expect { @namespace.move_dir }.to raise_error("namespace directory cannot be moved")
     end
 
     it "should move dir if path changed" do