From b10e2e0ce87669f52f05ff4cc48c6df1458f1875 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 1 Sep 2016 11:39:37 +0100
Subject: [PATCH 1/9] Add instrumentation to conflict classes

---
 config/initializers/metrics.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb
index 52522e099e7d..937987ed5f38 100644
--- a/config/initializers/metrics.rb
+++ b/config/initializers/metrics.rb
@@ -67,6 +67,7 @@
       ['app', 'finders']                    => ['app', 'finders'],
       ['app', 'mailers', 'emails']          => ['app', 'mailers'],
       ['app', 'services', '**']             => ['app', 'services'],
+      ['lib', 'gitlab', 'conflicts']        => ['lib'],
       ['lib', 'gitlab', 'diff']             => ['lib'],
       ['lib', 'gitlab', 'email', 'message'] => ['lib']
     }
-- 
GitLab


From 37d81c7fc38af2ac94bf0ae598311640ddc53593 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 1 Sep 2016 13:59:10 +0100
Subject: [PATCH 2/9] Allow setting content for resolutions

When reading conflicts:

1. Add a `type` field. `text` works as before, and has `sections`;
   `text-editor` is a file with ambiguous conflict markers that can only
   be resolved in an editor.
2. Add a `content_path` field pointing to a JSON representation of the
   file's content for a single file.
3. Hitting `content_path` returns a similar datastructure to the `file`,
   but without the `content_path` and `sections` fields, and with a
   `content` field containing the full contents of the file (with
   conflict markers).

When writing conflicts:

1. Instead of `sections` being at the top level, they are now in a
   `files` array. This matches the read format better.
2. The `files` array contains file hashes, each of which must contain:
   a. `new_path`
   b. `old_path`
   c. EITHER `sections` (which works as before) or `content` (with the
      full content of the resolved file).
---
 app/controllers/application_controller.rb     |   7 +-
 .../projects/merge_requests_controller.rb     |  20 ++-
 app/models/merge_request.rb                   |   2 +-
 .../merge_requests/resolve_service.rb         |  24 ++-
 config/routes.rb                              |   1 +
 lib/gitlab/conflict/file.rb                   |  51 +++++-
 lib/gitlab/conflict/file_collection.rb        |   4 +
 lib/gitlab/conflict/parser.rb                 |  15 +-
 lib/gitlab/conflict/resolution_error.rb       |   6 +
 .../merge_requests_controller_spec.rb         | 169 +++++++++++++++++-
 .../merge_requests/resolve_service_spec.rb    | 139 +++++++++++++-
 11 files changed, 396 insertions(+), 42 deletions(-)
 create mode 100644 lib/gitlab/conflict/resolution_error.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index ebc2a4651ba5..44f6ba2a8801 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -118,7 +118,12 @@ def render_403
   end
 
   def render_404
-    render file: Rails.root.join("public", "404"), layout: false, status: "404"
+    respond_to do |format|
+      format.json { head :not_found }
+      format.any do
+        render file: Rails.root.join("public", "404"), layout: false, status: "404"
+      end
+    end
   end
 
   def no_cache_headers
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 4f5f3b6aa09d..a6ce66458a3b 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -9,15 +9,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   before_action :module_enabled
   before_action :merge_request, only: [
-    :edit, :update, :show, :diffs, :commits, :conflicts, :builds, :pipelines, :merge, :merge_check,
+    :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines, :merge, :merge_check,
     :ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts
   ]
   before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines]
-  before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :builds, :pipelines]
+  before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines]
   before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check]
   before_action :define_commit_vars, only: [:diffs]
   before_action :define_diff_comment_vars, only: [:diffs]
-  before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :pipelines]
+  before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines]
 
   # Allow read any merge_request
   before_action :authorize_read_merge_request!
@@ -28,7 +28,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   # Allow modify merge_request
   before_action :authorize_update_merge_request!, only: [:close, :edit, :update, :remove_wip, :sort]
 
-  before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts]
+  before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :conflict_for_path, :resolve_conflicts]
 
   def index
     terms = params['issue_search']
@@ -164,6 +164,16 @@ def conflicts
     end
   end
 
+  def conflict_for_path
+    return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui?
+
+    file = @merge_request.conflicts.file_for_path(params[:old_path], params[:new_path])
+
+    return render_404 unless file
+
+    render json: file, full_content: true
+  end
+
   def resolve_conflicts
     return render_404 unless @merge_request.conflicts_can_be_resolved_in_ui?
 
@@ -178,7 +188,7 @@ def resolve_conflicts
       flash[:notice] = 'All merge conflicts were resolved. The merge request can now be merged.'
 
       render json: { redirect_to: namespace_project_merge_request_url(@project.namespace, @project, @merge_request, resolved_conflicts: true) }
-    rescue Gitlab::Conflict::File::MissingResolution => e
+    rescue Gitlab::Conflict::ResolutionError => e
       render status: :bad_request, json: { message: e.message }
     end
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 62163e740007..a187d33ae08b 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -808,7 +808,7 @@ def conflicts_can_be_resolved_in_ui?
       # files.
       conflicts.files.each(&:lines)
       @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0
-    rescue Rugged::OdbError, Gitlab::Conflict::Parser::ParserError, Gitlab::Conflict::FileCollection::ConflictSideMissing
+    rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing
       @conflicts_can_be_resolved_in_ui = false
     end
   end
diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb
index 19caa038c441..d22a1d3e0ad3 100644
--- a/app/services/merge_requests/resolve_service.rb
+++ b/app/services/merge_requests/resolve_service.rb
@@ -1,5 +1,8 @@
 module MergeRequests
   class ResolveService < MergeRequests::BaseService
+    class MissingFiles < Gitlab::Conflict::ResolutionError
+    end
+
     attr_accessor :conflicts, :rugged, :merge_index, :merge_request
 
     def execute(merge_request)
@@ -10,8 +13,16 @@ def execute(merge_request)
 
       fetch_their_commit!
 
-      conflicts.files.each do |file|
-        write_resolved_file_to_index(file, params[:sections])
+      params[:files].each do |file_params|
+        conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path])
+
+        write_resolved_file_to_index(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 = {
@@ -23,8 +34,13 @@ def execute(merge_request)
       project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params)
     end
 
-    def write_resolved_file_to_index(file, resolutions)
-      new_file = file.resolve_lines(resolutions).map(&:text).join("\n")
+    def write_resolved_file_to_index(file, params)
+      new_file = if params[:sections]
+                   file.resolve_lines(params[:sections]).map(&:text).join("\n")
+                 elsif params[:content]
+                   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)
diff --git a/config/routes.rb b/config/routes.rb
index 262a174437a0..453ef8bbaa58 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -729,6 +729,7 @@
             get :commits
             get :diffs
             get :conflicts
+            get :conflict_for_path
             get :builds
             get :pipelines
             get :merge_check
diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb
index dff9e29c6a5f..26a9f1702987 100644
--- a/lib/gitlab/conflict/file.rb
+++ b/lib/gitlab/conflict/file.rb
@@ -4,12 +4,12 @@ class File
       include Gitlab::Routing.url_helpers
       include IconsHelper
 
-      class MissingResolution < StandardError
+      class MissingResolution < ResolutionError
       end
 
       CONTEXT_LINES = 3
 
-      attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository
+      attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository, :type
 
       def initialize(merge_file_result, conflict, merge_request:)
         @merge_file_result = merge_file_result
@@ -21,12 +21,24 @@ def initialize(merge_file_result, conflict, merge_request:)
         @match_line_headers = {}
       end
 
+      def content
+        merge_file_result[:data]
+      end
+
       # Array of Gitlab::Diff::Line objects
       def lines
-        @lines ||= Gitlab::Conflict::Parser.new.parse(merge_file_result[:data],
+        return @lines if defined?(@lines)
+
+        begin
+          @type = 'text'
+          @lines = Gitlab::Conflict::Parser.new.parse(content,
                                                       our_path: our_path,
                                                       their_path: their_path,
                                                       parent_file: self)
+        rescue Gitlab::Conflict::Parser::ParserError
+          @type = 'text-editor'
+          @lines = nil
+        end
       end
 
       def resolve_lines(resolution)
@@ -53,6 +65,14 @@ def resolve_lines(resolution)
         end.compact
       end
 
+      def resolve_content(resolution)
+        if resolution == content
+          raise MissingResolution, "Resolved content has no changes for file #{our_path}"
+        end
+
+        resolution
+      end
+
       def highlight_lines!
         their_file = lines.reject { |line| line.type == 'new' }.map(&:text).join("\n")
         our_file = lines.reject { |line| line.type == 'old' }.map(&:text).join("\n")
@@ -170,21 +190,36 @@ def update_match_line_text(match_line, line)
         match_line.text = "@@ -#{match_line.old_pos},#{line.old_pos} +#{match_line.new_pos},#{line.new_pos} @@#{header}"
       end
 
-      def as_json(opts = nil)
-        {
+      def as_json(opts = {})
+        json_hash = {
           old_path: their_path,
           new_path: our_path,
           blob_icon: file_type_icon_class('file', our_mode, our_path),
           blob_path: namespace_project_blob_path(merge_request.project.namespace,
                                                  merge_request.project,
-                                                 ::File.join(merge_request.diff_refs.head_sha, our_path)),
-          sections: sections
+                                                 ::File.join(merge_request.diff_refs.head_sha, our_path))
         }
+
+        if opts[:full_content]
+          json_hash.merge(content: content)
+        else
+          json_hash.merge!(sections: sections) if type == 'text'
+
+          json_hash.merge(type: type, content_path: content_path)
+        end
+      end
+
+      def content_path
+        conflict_for_path_namespace_project_merge_request_path(merge_request.project.namespace,
+                                                               merge_request.project,
+                                                               merge_request,
+                                                               old_path: their_path,
+                                                               new_path: our_path)
       end
 
       # Don't try to print merge_request or repository.
       def inspect
-        instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode].map do |instance_variable|
+        instance_variables = [:merge_file_result, :their_path, :our_path, :our_mode, :type].map do |instance_variable|
           value = instance_variable_get("@#{instance_variable}")
 
           "#{instance_variable}=\"#{value}\""
diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb
index bbd0427a2c82..fa5bd4649d47 100644
--- a/lib/gitlab/conflict/file_collection.rb
+++ b/lib/gitlab/conflict/file_collection.rb
@@ -30,6 +30,10 @@ def files
         end
       end
 
+      def file_for_path(old_path, new_path)
+        files.find { |file| file.their_path == old_path && file.our_path == new_path }
+      end
+
       def as_json(opts = nil)
         {
           target_branch: merge_request.target_branch,
diff --git a/lib/gitlab/conflict/parser.rb b/lib/gitlab/conflict/parser.rb
index 2d4d55daeeb9..530113e79762 100644
--- a/lib/gitlab/conflict/parser.rb
+++ b/lib/gitlab/conflict/parser.rb
@@ -1,19 +1,24 @@
 module Gitlab
   module Conflict
     class Parser
-      class ParserError < StandardError
+      class UnresolvableError < StandardError
       end
 
-      class UnexpectedDelimiter < ParserError
+      class UnmergeableFile < UnresolvableError
       end
 
-      class MissingEndDelimiter < ParserError
+      class UnsupportedEncoding < UnresolvableError
+      end
+
+      # Recoverable errors - the conflict can be resolved in an editor, but not with
+      # sections.
+      class ParserError < StandardError
       end
 
-      class UnmergeableFile < ParserError
+      class UnexpectedDelimiter < ParserError
       end
 
-      class UnsupportedEncoding < ParserError
+      class MissingEndDelimiter < ParserError
       end
 
       def parse(text, our_path:, their_path:, parent_file: nil)
diff --git a/lib/gitlab/conflict/resolution_error.rb b/lib/gitlab/conflict/resolution_error.rb
new file mode 100644
index 000000000000..a0f2006bc245
--- /dev/null
+++ b/lib/gitlab/conflict/resolution_error.rb
@@ -0,0 +1,6 @@
+module Gitlab
+  module Conflict
+    class ResolutionError < StandardError
+    end
+  end
+end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index a219400d75fc..4ccf20b21ceb 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -564,7 +564,7 @@ def go(format: 'html')
     context 'when the conflicts cannot be resolved in the UI' do
       before do
         allow_any_instance_of(Gitlab::Conflict::Parser).
-          to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnexpectedDelimiter)
+          to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile)
 
         get :conflicts,
             namespace_id: merge_request_with_conflicts.project.namespace.to_param,
@@ -638,26 +638,97 @@ def go(format: 'html')
     end
   end
 
+  describe 'GET conflict_for_path' do
+    let(:json_response) { JSON.parse(response.body) }
+
+    def conflict_for_path(path)
+      get :conflict_for_path,
+          namespace_id: merge_request_with_conflicts.project.namespace.to_param,
+          project_id: merge_request_with_conflicts.project.to_param,
+          id: merge_request_with_conflicts.iid,
+          old_path: path,
+          new_path: path,
+          format: 'json'
+    end
+
+    context 'when the conflicts cannot be resolved in the UI' do
+      before do
+        allow_any_instance_of(Gitlab::Conflict::Parser).
+          to receive(:parse).and_raise(Gitlab::Conflict::Parser::UnmergeableFile)
+
+        conflict_for_path('files/ruby/regex.rb')
+      end
+
+
+      it 'returns a 404 status code' do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'when the file does not exist cannot be resolved in the UI' do
+      before { conflict_for_path('files/ruby/regexp.rb') }
+
+      it 'returns a 404 status code' do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context 'with an existing file' do
+      let(:path) { 'files/ruby/regex.rb' }
+
+      before { conflict_for_path(path) }
+
+      it 'returns a 200 status code' do
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'returns the file in JSON format' do
+        content = merge_request_with_conflicts.conflicts.file_for_path(path, path).content
+
+        expect(json_response).to include('old_path' => path,
+                                         'new_path' => path,
+                                         'blob_icon' => 'file-text-o',
+                                         'blob_path' => a_string_ending_with(path),
+                                         'content' => content)
+      end
+    end
+  end
+
   context 'POST resolve_conflicts' do
     let(:json_response) { JSON.parse(response.body) }
     let!(:original_head_sha) { merge_request_with_conflicts.diff_head_sha }
 
-    def resolve_conflicts(sections)
+    def resolve_conflicts(files)
       post :resolve_conflicts,
            namespace_id: merge_request_with_conflicts.project.namespace.to_param,
            project_id: merge_request_with_conflicts.project.to_param,
            id: merge_request_with_conflicts.iid,
            format: 'json',
-           sections: sections,
+           files: files,
            commit_message: 'Commit message'
     end
 
     context 'with valid params' do
       before do
-        resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head',
-                          '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
-                          '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
-                          '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin')
+        resolved_files = [
+          {
+            'new_path' => 'files/ruby/popen.rb',
+            'old_path' => 'files/ruby/popen.rb',
+            'sections' => {
+              '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head'
+            }
+          }, {
+            'new_path' => 'files/ruby/regex.rb',
+            'old_path' => 'files/ruby/regex.rb',
+            'sections' => {
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+            }
+          }
+        ]
+
+        resolve_conflicts(resolved_files)
       end
 
       it 'creates a new commit on the branch' do
@@ -672,7 +743,23 @@ def resolve_conflicts(sections)
 
     context 'when sections are missing' do
       before do
-        resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head')
+        resolved_files = [
+          {
+            'new_path' => 'files/ruby/popen.rb',
+            'old_path' => 'files/ruby/popen.rb',
+            'sections' => {
+              '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head'
+            }
+          }, {
+            'new_path' => 'files/ruby/regex.rb',
+            'old_path' => 'files/ruby/regex.rb',
+            'sections' => {
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head'
+            }
+          }
+        ]
+
+        resolve_conflicts(resolved_files)
       end
 
       it 'returns a 400 error' do
@@ -680,7 +767,71 @@ def resolve_conflicts(sections)
       end
 
       it 'has a message with the name of the first missing section' do
-        expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9')
+        expect(json_response['message']).to include('6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21')
+      end
+
+      it 'does not create a new commit' do
+        expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
+      end
+    end
+
+    context 'when files are missing' do
+      before do
+        resolved_files = [
+          {
+            'new_path' => 'files/ruby/regex.rb',
+            'old_path' => 'files/ruby/regex.rb',
+            'sections' => {
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+            }
+          }
+        ]
+
+        resolve_conflicts(resolved_files)
+      end
+
+      it 'returns a 400 error' do
+        expect(response).to have_http_status(:bad_request)
+      end
+
+      it 'has a message with the name of the missing file' do
+        expect(json_response['message']).to include('files/ruby/popen.rb')
+      end
+
+      it 'does not create a new commit' do
+        expect(original_head_sha).to eq(merge_request_with_conflicts.source_branch_head.sha)
+      end
+    end
+
+    context 'when a file has identical content to the conflict' do
+      before do
+        resolved_files = [
+          {
+            'new_path' => 'files/ruby/popen.rb',
+            'old_path' => 'files/ruby/popen.rb',
+            'content' => merge_request_with_conflicts.conflicts.file_for_path('files/ruby/popen.rb', 'files/ruby/popen.rb').content
+          }, {
+            'new_path' => 'files/ruby/regex.rb',
+            'old_path' => 'files/ruby/regex.rb',
+            'sections' => {
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+              '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+            }
+          }
+        ]
+
+        resolve_conflicts(resolved_files)
+      end
+
+      it 'returns a 400 error' do
+        expect(response).to have_http_status(:bad_request)
+      end
+
+      it 'has a message with the path of the problem file' do
+        expect(json_response['message']).to include('files/ruby/popen.rb')
       end
 
       it 'does not create a new commit' do
diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb
index d71932458fa0..e667e93bea40 100644
--- a/spec/services/merge_requests/resolve_service_spec.rb
+++ b/spec/services/merge_requests/resolve_service_spec.rb
@@ -24,15 +24,26 @@
   end
 
   describe '#execute' do
-    context 'with valid params' do
+    context 'with section params' do
       let(:params) do
         {
-          sections: {
-            '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head',
-            '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
-            '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
-            '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
-          },
+          files: [
+            {
+              old_path: 'files/ruby/popen.rb',
+              new_path: 'files/ruby/popen.rb',
+              sections: {
+                '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head'
+              }
+            }, {
+              old_path: 'files/ruby/regex.rb',
+              new_path: 'files/ruby/regex.rb',
+              sections: {
+                '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+                '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+                '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+              }
+            }
+          ],
           commit_message: 'This is a commit message!'
         }
       end
@@ -74,8 +85,96 @@
       end
     end
 
-    context 'when a resolution is missing' do
-      let(:invalid_params) { { sections: { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head' } } }
+    context 'with content and sections params' do
+      let(:popen_content) { "class Popen\nend" }
+
+      let(:params) do
+        {
+          files: [
+            {
+              old_path: 'files/ruby/popen.rb',
+              new_path: 'files/ruby/popen.rb',
+              content: popen_content
+            }, {
+              old_path: 'files/ruby/regex.rb',
+              new_path: 'files/ruby/regex.rb',
+              sections: {
+                '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+                '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+                '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+              }
+            }
+          ],
+          commit_message: 'This is a commit message!'
+        }
+      end
+
+      before do
+        MergeRequests::ResolveService.new(project, user, params).execute(merge_request)
+      end
+
+      it 'creates a commit with the message' do
+        expect(merge_request.source_branch_head.message).to eq(params[:commit_message])
+      end
+
+      it 'creates a commit with the correct parents' do
+        expect(merge_request.source_branch_head.parents.map(&:id)).
+          to eq(['1450cd639e0bc6721eb02800169e464f212cde06',
+                 '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b'])
+      end
+
+      it 'sets the content to the content given' do
+        blob = merge_request.source_project.repository.blob_at(merge_request.source_branch_head.sha,
+                                                               'files/ruby/popen.rb')
+
+        expect(blob.data).to eq(popen_content)
+      end
+    end
+
+    context 'when a resolution section is missing' do
+      let(:invalid_params) do
+        {
+          files: [
+            {
+              old_path: 'files/ruby/popen.rb',
+              new_path: 'files/ruby/popen.rb',
+              content: ''
+            }, {
+              old_path: 'files/ruby/regex.rb',
+              new_path: 'files/ruby/regex.rb',
+              sections: { '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' }
+            }
+          ],
+          commit_message: 'This is a commit message!'
+        }
+      end
+
+      let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) }
+
+      it 'raises a MissingResolution error' do
+        expect { service.execute(merge_request) }.
+          to raise_error(Gitlab::Conflict::File::MissingResolution)
+      end
+    end
+
+    context 'when the content of a file is unchanged' do
+      let(:invalid_params) do
+        {
+          files: [
+            {
+              old_path: 'files/ruby/popen.rb',
+              new_path: 'files/ruby/popen.rb',
+              content: ''
+            }, {
+              old_path: 'files/ruby/regex.rb',
+              new_path: 'files/ruby/regex.rb',
+              content: merge_request.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content
+            }
+          ],
+          commit_message: 'This is a commit message!'
+        }
+      end
+
       let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) }
 
       it 'raises a MissingResolution error' do
@@ -83,5 +182,27 @@
           to raise_error(Gitlab::Conflict::File::MissingResolution)
       end
     end
+
+    context 'when a file is missing' do
+      let(:invalid_params) do
+        {
+          files: [
+            {
+              old_path: 'files/ruby/popen.rb',
+              new_path: 'files/ruby/popen.rb',
+              content: ''
+            }
+          ],
+          commit_message: 'This is a commit message!'
+        }
+      end
+
+      let(:service) { MergeRequests::ResolveService.new(project, user, invalid_params) }
+
+      it 'raises a MissingFiles error' do
+        expect { service.execute(merge_request) }.
+          to raise_error(MergeRequests::ResolveService::MissingFiles)
+      end
+    end
   end
 end
-- 
GitLab


From e0505b51c0e6ccbadd8506b988abf26fc2b75f9c Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 1 Sep 2016 16:19:16 +0100
Subject: [PATCH 3/9] Make RuboCop happy

---
 spec/controllers/projects/merge_requests_controller_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 4ccf20b21ceb..2ce195e3541c 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -659,7 +659,6 @@ def conflict_for_path(path)
         conflict_for_path('files/ruby/regex.rb')
       end
 
-
       it 'returns a 404 status code' do
         expect(response).to have_http_status(:not_found)
       end
-- 
GitLab


From 8a26b0f1667955c0e586c07a71ffe6cb843bd78f Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 1 Sep 2016 16:20:29 +0100
Subject: [PATCH 4/9] Fix specs

---
 app/controllers/application_controller.rb | 4 ++--
 spec/support/test_env.rb                  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 44f6ba2a8801..cf19758cca2a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -119,10 +119,10 @@ def render_403
 
   def render_404
     respond_to do |format|
-      format.json { head :not_found }
-      format.any do
+      format.html do
         render file: Rails.root.join("public", "404"), layout: false, status: "404"
       end
+      format.any { head :not_found }
     end
   end
 
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 0097dbf8fadc..3f598b4fc366 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -24,10 +24,10 @@ module TestEnv
     'expand-collapse-lines'              => '238e82d',
     'video'                              => '8879059',
     'crlf-diff'                          => '5938907',
-    'conflict-start'                     => '75284c7',
+    'conflict-start'                     => '824be60',
     'conflict-resolvable'                => '1450cd6',
     'conflict-binary-file'               => '259a6fb',
-    'conflict-contains-conflict-markers' => '5e0964c',
+    'conflict-contains-conflict-markers' => '78a3086',
     'conflict-missing-side'              => 'eb227b3',
     'conflict-non-utf8'                  => 'd0a293c',
     'conflict-too-large'                 => '39fa04f',
-- 
GitLab


From 0958eee1d5cc8fb4637c19671e2c2c4e2072e43a Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 1 Sep 2016 16:50:53 +0100
Subject: [PATCH 5/9] Fix MR model spec

---
 spec/models/merge_request_spec.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 5bf3b8e609e6..a91ba3e0aff8 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -944,12 +944,6 @@ def create_merge_request(source_branch)
       expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_falsey
     end
 
-    it 'returns a falsey value when the conflicts contain a file with ambiguous conflict markers' do
-      merge_request = create_merge_request('conflict-contains-conflict-markers')
-
-      expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_falsey
-    end
-
     it 'returns a falsey value when the conflicts contain a file edited in one branch and deleted in another' do
       merge_request = create_merge_request('conflict-missing-side')
 
@@ -961,6 +955,12 @@ def create_merge_request(source_branch)
 
       expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_truthy
     end
+
+    it 'returns a truthy value when the conflicts have to be resolved in an editor' do
+      merge_request = create_merge_request('conflict-contains-conflict-markers')
+
+      expect(merge_request.conflicts_can_be_resolved_in_ui?).to be_truthy
+    end
   end
 
   describe "#forked_source_project_missing?" do
-- 
GitLab


From c032fb0a7abe22d033e005035ed1143bf835ccbf Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 1 Sep 2016 17:44:35 +0100
Subject: [PATCH 6/9] Fix resolve service specs

---
 spec/services/merge_requests/resolve_service_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/services/merge_requests/resolve_service_spec.rb b/spec/services/merge_requests/resolve_service_spec.rb
index e667e93bea40..388abb6a0dfc 100644
--- a/spec/services/merge_requests/resolve_service_spec.rb
+++ b/spec/services/merge_requests/resolve_service_spec.rb
@@ -60,7 +60,7 @@
         it 'creates a commit with the correct parents' do
           expect(merge_request.source_branch_head.parents.map(&:id)).
             to eq(['1450cd639e0bc6721eb02800169e464f212cde06',
-                   '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b'])
+                   '824be604a34828eb682305f0d963056cfac87b2d'])
         end
       end
 
@@ -120,7 +120,7 @@
       it 'creates a commit with the correct parents' do
         expect(merge_request.source_branch_head.parents.map(&:id)).
           to eq(['1450cd639e0bc6721eb02800169e464f212cde06',
-                 '75284c70dd26c87f2a3fb65fd5a1f0b0138d3a6b'])
+                 '824be604a34828eb682305f0d963056cfac87b2d'])
       end
 
       it 'sets the content to the content given' do
-- 
GitLab


From 9f7eaa052abf85f80c8f93de9a43ffed20b50def Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Fri, 2 Sep 2016 16:16:54 +0100
Subject: [PATCH 7/9] Add JSON Schema

---
 .../merge_requests_controller_spec.rb         |   4 +
 spec/fixtures/api/schemas/conflicts.json      | 137 ++++++++++++++++++
 2 files changed, 141 insertions(+)
 create mode 100644 spec/fixtures/api/schemas/conflicts.json

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 2ce195e3541c..8bd47ca5c182 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -591,6 +591,10 @@ def go(format: 'html')
             format: 'json'
       end
 
+      it 'matches the schema' do
+        expect(response).to match_response_schema('conflicts')
+      end
+
       it 'includes meta info about the MR' do
         expect(json_response['commit_message']).to include('Merge branch')
         expect(json_response['commit_sha']).to match(/\h{40}/)
diff --git a/spec/fixtures/api/schemas/conflicts.json b/spec/fixtures/api/schemas/conflicts.json
new file mode 100644
index 000000000000..a947783d505c
--- /dev/null
+++ b/spec/fixtures/api/schemas/conflicts.json
@@ -0,0 +1,137 @@
+{
+  "type": "object",
+  "required": [
+    "commit_message",
+    "commit_sha",
+    "source_branch",
+    "target_branch",
+    "files"
+  ],
+  "properties": {
+    "commit_message": {"type": "string"},
+    "commit_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+    "source_branch": {"type": "string"},
+    "target_branch": {"type": "string"},
+    "files": {
+      "type": "array",
+      "items": {
+        "oneOf": [
+          { "$ref": "#/definitions/conflict-text-with-sections" },
+          { "$ref": "#/definitions/conflict-text-for-editor" }
+        ]
+      }
+    }
+  },
+  "definitions": {
+    "conflict-base": {
+      "type": "object",
+      "required": [
+        "old_path",
+        "new_path",
+        "blob_icon",
+        "blob_path"
+      ],
+      "properties": {
+        "old_path": {"type": "string"},
+        "new_path": {"type": "string"},
+        "blob_icon": {"type": "string"},
+        "blob_path": {"type": "string"}
+      }
+    },
+    "conflict-text-for-editor": {
+      "allOf": [
+        {"$ref": "#/definitions/conflict-base"},
+        {
+          "type": "object",
+          "required": [
+            "type",
+            "content_path"
+          ],
+          "properties": {
+            "type": {"type": {"enum": ["text-editor"]}},
+            "content_path": {"type": "string"}
+          }
+        }
+      ]
+    },
+    "conflict-text-with-sections": {
+      "allOf": [
+        {"$ref": "#/definitions/conflict-base"},
+        {
+          "type": "object",
+          "required": [
+            "type",
+            "content_path",
+            "sections"
+          ],
+          "properties": {
+            "type": {"type": {"enum": ["text"]}},
+            "content_path": {"type": "string"},
+            "sections": {
+              "type": "array",
+              "items": {
+                "oneOf": [
+                  { "$ref": "#/definitions/section-context" },
+                  { "$ref": "#/definitions/section-conflict" }
+                ]
+              }
+            }
+          }
+        }
+      ]
+    },
+    "section-base": {
+      "type": "object",
+      "required": [
+        "conflict",
+        "lines"
+      ],
+      "properties": {
+        "conflict": {"type": "boolean"},
+        "lines": {
+          "type": "array",
+          "items": {
+            "type": "object",
+            "required": [
+              "old_line",
+              "new_line",
+              "text",
+              "rich_text"
+            ],
+            "properties": {
+              "type": {"type": "string"},
+              "old_line": {"type": "string"},
+              "new_line": {"type": "string"},
+              "text": {"type": "string"},
+              "rich_text": {"type": "string"}
+            }
+          }
+        }
+      }
+    },
+    "section-context": {
+      "allOf": [
+        {"$ref": "#/definitions/section-base"},
+        {
+          "type": "object",
+          "properties": {
+            "conflict": {"enum": [false]}
+          }
+        }
+      ]
+    },
+    "section-conflict": {
+      "allOf": [
+        {"$ref": "#/definitions/section-base"},
+        {
+          "type": "object",
+          "required": ["id"],
+          "properties": {
+            "conflict": {"enum": [true]},
+            "id": {"type": "string"}
+          }
+        }
+      ]
+    }
+  }
+}
-- 
GitLab


From a60aa155fb9898bae9b8ce769ff7fe922914145e Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Wed, 7 Sep 2016 12:28:13 +0100
Subject: [PATCH 8/9] Simplify conflict file JSON creation

---
 lib/gitlab/conflict/file.rb | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/lib/gitlab/conflict/file.rb b/lib/gitlab/conflict/file.rb
index 26a9f1702987..661e43d3fa9f 100644
--- a/lib/gitlab/conflict/file.rb
+++ b/lib/gitlab/conflict/file.rb
@@ -9,7 +9,7 @@ class MissingResolution < ResolutionError
 
       CONTEXT_LINES = 3
 
-      attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository, :type
+      attr_reader :merge_file_result, :their_path, :our_path, :our_mode, :merge_request, :repository
 
       def initialize(merge_file_result, conflict, merge_request:)
         @merge_file_result = merge_file_result
@@ -25,6 +25,12 @@ def content
         merge_file_result[:data]
       end
 
+      def type
+        lines unless @type
+
+        @type.inquiry
+      end
+
       # Array of Gitlab::Diff::Line objects
       def lines
         return @lines if defined?(@lines)
@@ -200,12 +206,14 @@ def as_json(opts = {})
                                                  ::File.join(merge_request.diff_refs.head_sha, our_path))
         }
 
-        if opts[:full_content]
-          json_hash.merge(content: content)
-        else
-          json_hash.merge!(sections: sections) if type == 'text'
-
-          json_hash.merge(type: type, content_path: content_path)
+        json_hash.tap do |json_hash|
+          if opts[:full_content]
+            json_hash[:content] = content
+          else
+            json_hash[:sections] = sections if type.text?
+            json_hash[:type] = type
+            json_hash[:content_path] = content_path
+          end
         end
       end
 
-- 
GitLab


From 923cf9d199c72db5c8e8c5042b343c861e3fb471 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Tue, 20 Sep 2016 09:16:35 +0300
Subject: [PATCH 9/9] We now support resolving conflicts with ambiguous markers

---
 spec/features/merge_requests/conflicts_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb
index 759edf8ec80c..1e8aeacea057 100644
--- a/spec/features/merge_requests/conflicts_spec.rb
+++ b/spec/features/merge_requests/conflicts_spec.rb
@@ -42,7 +42,6 @@ def create_merge_request(source_branch)
   UNRESOLVABLE_CONFLICTS = {
     'conflict-too-large' => 'when the conflicts contain a large file',
     'conflict-binary-file' => 'when the conflicts contain a binary file',
-    'conflict-contains-conflict-markers' => 'when the conflicts contain a file with ambiguous conflict markers',
     'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another',
     'conflict-non-utf8' => 'when the conflicts contain a non-UTF-8 file',
   }
-- 
GitLab