Encapsulate git operations for conflict resolution into lib

3 files changed, 52 insertions, 60 deletions

diff --git a/app/services/merge_requests/conflicts/resolve_service.rb b/app/services/merge_requests/conflicts/resolve_service.rb
index 6b6e231f4f9..a7900860c7e 100644
--- a/app/services/merge_requests/conflicts/resolve_service.rb
+++ b/app/services/merge_requests/conflicts/resolve_service.rb
@@ -1,54 +1,10 @@
 module MergeRequests
   module Conflicts
     class ResolveService < MergeRequests::Conflicts::BaseService
-      MissingFiles = Class.new(Gitlab::Conflict::ResolutionError)
-
       def execute(current_user, params)
-        rugged = merge_request.source_project.repository.rugged
-
-        Gitlab::Conflict::FileCollection.for_resolution(merge_request) do |conflicts_for_resolution|
-          merge_index = conflicts_for_resolution.merge_index
-
-          params[:files].each do |file_params|
-            conflict_file = conflicts_for_resolution.file_for_path(file_params[:old_path], file_params[:new_path])
-
-            write_resolved_file_to_index(merge_index, rugged, conflict_file, file_params)
-          end
-
-          unless merge_index.conflicts.empty?
-            missing_files = merge_index.conflicts.map { |file| file[:ours][:path] }
-
-            raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}"
-          end
-
-          commit_params = {
-            message: params[:commit_message] || conflicts_for_resolution.default_commit_message,
-            parents: [conflicts_for_resolution.our_commit, conflicts_for_resolution.their_commit].map(&:oid),
-            tree: merge_index.write_tree(rugged)
-          }
-
-          conflicts_for_resolution
-            .project
-            .repository
-            .resolve_conflicts(current_user, merge_request.source_branch, commit_params)
-        end
-      end
-
-      private
-
-      def write_resolved_file_to_index(merge_index, rugged, file, params)
-        if params[:sections]
-          new_file = file.resolve_lines(params[:sections]).map(&:text).join("\n")
-
-          new_file << "\n" if file.our_blob.data.ends_with?("\n")
-        elsif params[:content]
-          new_file = file.resolve_content(params[:content])
-        end
-
-        our_path = file.our_path
+        conflicts = Gitlab::Conflict::FileCollection.for_resolution(merge_request)
 
-        merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode)
-        merge_index.conflict_remove(our_path)
+        conflicts.resolve(current_user, params[:commit_message], params[:files])
       end
     end
   end
diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb
index 90f83e0f810..4fedfe8691a 100644
--- a/lib/gitlab/conflict/file_collection.rb
+++ b/lib/gitlab/conflict/file_collection.rb
@@ -2,8 +2,9 @@ module Gitlab
   module Conflict
     class FileCollection
       ConflictSideMissing = Class.new(StandardError)
+      MissingFiles = Class.new(ResolutionError)
 
-      attr_reader :merge_request, :our_commit, :their_commit, :project
+      attr_reader :merge_request, :our_commit, :their_commit, :project, :read_only
 
       delegate :repository, to: :project
 
@@ -13,22 +14,41 @@ module Gitlab
         # the time because this fetches a ref into the source project, which
         # isn't needed for reading.
         def for_resolution(merge_request)
-          project = merge_request.source_project
-
-          new(merge_request, project).tap do |file_collection|
-            project
-              .repository
-              .with_repo_branch_commit(merge_request.target_project.repository.raw_repository, merge_request.target_branch) do
-
-              yield file_collection
-            end
-          end
+          new(merge_request, merge_request.source_project, false)
         end
 
         # We don't need to do `with_repo_branch_commit` here, because the target
         # project always fetches source refs when creating merge request diffs.
         def read_only(merge_request)
-          new(merge_request, merge_request.target_project)
+          new(merge_request, merge_request.target_project, true)
+        end
+      end
+
+      def resolve(user, commit_message, files)
+        raise "can't resolve a read-only Conflict File Collection" if read_only
+
+        repository.with_repo_branch_commit(merge_request.target_project.repository.raw, merge_request.target_branch) do
+          rugged = repository.rugged
+
+          files.each do |file_params|
+            conflict_file = file_for_path(file_params[:old_path], file_params[:new_path])
+
+            write_resolved_file_to_index(merge_index, rugged, conflict_file, file_params)
+          end
+
+          unless merge_index.conflicts.empty?
+            missing_files = merge_index.conflicts.map { |file| file[:ours][:path] }
+
+            raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}"
+          end
+
+          commit_params = {
+            message: commit_message || default_commit_message,
+            parents: [our_commit, their_commit].map(&:oid),
+            tree: merge_index.write_tree(rugged)
+          }
+
+          repository.resolve_conflicts(user, merge_request.source_branch, commit_params)
         end
       end
 
@@ -75,11 +95,27 @@ EOM
 
       private
 
-      def initialize(merge_request, project)
+      def write_resolved_file_to_index(merge_index, rugged, file, params)
+        if params[:sections]
+          new_file = file.resolve_lines(params[:sections]).map(&:text).join("\n")
+
+          new_file << "\n" if file.our_blob.data.ends_with?("\n")
+        elsif params[:content]
+          new_file = file.resolve_content(params[:content])
+        end
+
+        our_path = file.our_path
+
+        merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode)
+        merge_index.conflict_remove(our_path)
+      end
+
+      def initialize(merge_request, project, read_only)
         @merge_request = merge_request
         @our_commit = merge_request.source_branch_head.raw.rugged_commit
         @their_commit = merge_request.target_branch_head.raw.rugged_commit
         @project = project
+        @read_only = read_only
       end
     end
   end
diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb
index a1f7dc44d31..9dc6e3c3494 100644
--- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb
+++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb
@@ -248,7 +248,7 @@ describe MergeRequests::Conflicts::ResolveService do
 
       it 'raises a MissingFiles error' do
         expect { service.execute(user, invalid_params) }
-          .to raise_error(described_class::MissingFiles)
+          .to raise_error(Gitlab::Conflict::FileCollection::MissingFiles)
       end
     end
   end