Address review

5 files changed, 41 insertions, 25 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 73256df0591..c3fceddb9ca 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -753,7 +753,7 @@ class Repository
     author_email: nil, author_name: nil,
     start_branch_name: nil, start_project: project)
 
-    entry = tree_entry_at(start_branch_name || branch_name, path)
+    entry = start_project.repository.tree_entry_at(start_branch_name || branch_name, path)
     if entry
       if entry[:type] == :blob
         raise Gitlab::Git::Repository::InvalidBlobName.new(
@@ -865,7 +865,7 @@ class Repository
       end
 
       actions.each do |options|
-        index.__send__(options.delete(:action), options)
+        index.public_send(options.delete(:action), options)
       end
 
       options = {
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index 809fa32eca5..2c89f6e7f92 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -22,6 +22,12 @@ module Files
     def validate
       super
       params[:actions].each_with_index do |action, index|
+        if ACTIONS.include?(action[:action].to_s)
+          action[:action] = action[:action].to_sym
+        else
+          raise_error("Unknown action type `#{action[:action]}`.")
+        end
+        
         unless action[:file_path].present?
           raise_error("You must specify a file_path.")
         end
@@ -32,12 +38,6 @@ module Files
         regex_check(action[:file_path])
         regex_check(action[:previous_path]) if action[:previous_path]
 
-        if ACTIONS.include?(action[:action].to_s)
-          action[:action] = action[:action].to_sym
-        else
-          raise_error("Unknown action type `#{action[:action]}`.")
-        end
-
         if project.empty_repo? && action[:action] != :create
           raise_error("No files to #{action[:action]}.")
         end
diff --git a/lib/gitlab/git/index.rb b/lib/gitlab/git/index.rb
index 69a7a513281..546696e4bd1 100644
--- a/lib/gitlab/git/index.rb
+++ b/lib/gitlab/git/index.rb
@@ -10,20 +10,20 @@ module Gitlab
         @raw_index = repository.rugged.index
       end
 
-      delegate :read_tree, to: :raw_index
+      delegate :read_tree, :get, to: :raw_index
 
       def write_tree
         raw_index.write_tree(repository.rugged)
       end
 
-      def get(*args)
-        raw_index.get(*args)
+      def dir_exists?(path)
+        raw_index.find { |entry| entry[:path].start_with?("#{path}/") }
       end
 
       def create(options)
-        normalize_options!(options)
+        options = normalize_options(options)
 
-        file_entry = raw_index.get(options[:file_path])
+        file_entry = get(options[:file_path])
         if file_entry
           raise Gitlab::Git::Repository::InvalidBlobName.new("Filename already exists")
         end
@@ -32,13 +32,17 @@ module Gitlab
       end
 
       def create_dir(options)
-        normalize_options!(options)
+        options = normalize_options(options)
 
-        file_entry = raw_index.get(options[:file_path])
+        file_entry = get(options[:file_path])
         if file_entry
           raise Gitlab::Git::Repository::InvalidBlobName.new("Directory already exists as a file")
         end
 
+        if dir_exists?(options[:file_path])
+          raise Gitlab::Git::Repository::InvalidBlobName.new("Directory already exists")
+        end
+
         options = options.dup
         options[:file_path] += '/.gitkeep'
         options[:content] = ''
@@ -47,9 +51,9 @@ module Gitlab
       end
 
       def update(options)
-        normalize_options!(options)
+        options = normalize_options(options)
 
-        file_entry = raw_index.get(options[:file_path])
+        file_entry = get(options[:file_path])
         unless file_entry
           raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist")
         end
@@ -58,9 +62,9 @@ module Gitlab
       end
 
       def move(options)
-        normalize_options!(options)
+        options = normalize_options(options)
 
-        file_entry = raw_index.get(options[:previous_path])
+        file_entry = get(options[:previous_path])
         unless file_entry
           raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist")
         end
@@ -71,9 +75,9 @@ module Gitlab
       end
 
       def delete(options)
-        normalize_options!(options)
+        options = normalize_options(options)
 
-        file_entry = raw_index.get(options[:file_path])
+        file_entry = get(options[:file_path])
         unless file_entry
           raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist")
         end
@@ -83,13 +87,15 @@ module Gitlab
 
       private
 
-      def normalize_options!(options)
+      def normalize_options(options)
+        options = options.dup
         options[:file_path] = normalize_path(options[:file_path]) if options[:file_path]
         options[:previous_path] = normalize_path(options[:previous_path]) if options[:previous_path]
+        options
       end
 
       def normalize_path(path)
-        pathname = Gitlab::Git::PathHelper.normalize_path(path)
+        pathname = Gitlab::Git::PathHelper.normalize_path(path.dup)
 
         if pathname.each_filename.include?('..')
           raise Gitlab::Git::Repository::InvalidBlobName.new('Invalid path')
diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb
index 91ffbd138ce..d0c7ca60ddc 100644
--- a/spec/lib/gitlab/git/index_spec.rb
+++ b/spec/lib/gitlab/git/index_spec.rb
@@ -92,6 +92,16 @@ describe Gitlab::Git::Index, seed_helper: true do
         expect { index.create_dir(options) }.to raise_error('Directory already exists as a file')
       end
     end
+
+    context 'when a directory at that path exists' do
+      before do
+        options[:file_path] = 'files/executables'
+      end
+
+      it 'raises an error' do
+        expect { index.create_dir(options) }.to raise_error('Directory already exists')
+      end
+    end
   end
 
   describe '#update' do
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 58943fd7252..ae203fada12 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -291,7 +291,7 @@ describe Repository, models: true do
     end
   end
 
-  describe "#commit_dir" do
+  describe "#create_dir" do
     it "commits a change that creates a new directory" do
       expect do
         repository.create_dir(user, 'newdir',
@@ -424,7 +424,7 @@ describe Repository, models: true do
     end
   end
 
-  describe "#remove_file" do
+  describe "#delete_file" do
     it 'removes file successfully' do
       expect do
         repository.delete_file(user, 'README',