Handle response messages

3 files changed, 137 insertions, 112 deletions

diff --git a/changelogs/unreleased/mk-simplify-internal-post-receive-messages.yml b/changelogs/unreleased/mk-simplify-internal-post-receive-messages.yml
new file mode 100644
index 000000000..7e640ed07
--- /dev/null
+++ b/changelogs/unreleased/mk-simplify-internal-post-receive-messages.yml
@@ -0,0 +1,5 @@
+---
+title: Make it easier to add new kinds of internal post receive messages
+merge_request: 1410
+author:
+type: changed
diff --git a/ruby/gitlab-shell/lib/gitlab_post_receive.rb b/ruby/gitlab-shell/lib/gitlab_post_receive.rb
index 7c5bd19c9..e0107350d 100644
--- a/ruby/gitlab-shell/lib/gitlab_post_receive.rb
+++ b/ruby/gitlab-shell/lib/gitlab_post_receive.rb
@@ -6,6 +6,19 @@ require 'base64'
 require 'securerandom'
 
 class GitlabPostReceive
+  # A standard terminal window is (at least) 80 characters wide.
+  TERMINAL_WIDTH = 80
+  GIT_REMOTE_MESSAGE_PREFIX_LENGTH = "remote: ".length
+  TERMINAL_MESSAGE_PADDING = 2
+
+  # Git prefixes remote messages with "remote: ", so this width is subtracted
+  # from the width available to us.
+  MAX_MESSAGE_WIDTH = TERMINAL_WIDTH - GIT_REMOTE_MESSAGE_PREFIX_LENGTH
+
+  # Our centered text shouldn't start or end right at the edge of the window,
+  # so we add some horizontal padding: 2 chars on either side.
+  MAX_MESSAGE_TEXT_WIDTH = MAX_MESSAGE_WIDTH - 2 * TERMINAL_MESSAGE_PADDING
+
   attr_reader :config, :gl_repository, :repo_path, :changes, :jid
 
   def initialize(gl_repository, repo_path, actor, changes, push_options)
@@ -24,11 +37,8 @@ class GitlabPostReceive
     end
 
     return false unless response
-    print_formatted_alert_message(response['broadcast_message']) if response['broadcast_message']
-    print_merge_request_links(response['merge_request_urls']) if response['merge_request_urls']
-    puts response['redirected_message'] if response['redirected_message']
-    puts response['project_created_message'] if response['project_created_message']
-    print_warnings(response['warnings']) if response['warnings']
+
+    print_messages(response['messages'])
 
     response['reference_counter_decreased']
   rescue GitlabNet::ApiUnreachableError
@@ -41,73 +51,61 @@ class GitlabPostReceive
     @api ||= GitlabNet.new
   end
 
-  def print_merge_request_links(merge_request_urls)
-    return if merge_request_urls.empty?
-    puts
-    merge_request_urls.each { |mr| print_merge_request_link(mr) }
-  end
-
-  def print_merge_request_link(merge_request)
-    message =
-      if merge_request["new_merge_request"]
-        "To create a merge request for #{merge_request['branch_name']}, visit:"
-      else
-        "View merge request for #{merge_request['branch_name']}:"
-      end
+  def print_messages(response_messages)
+    return if response_messages.nil? || response_messages.none?
 
-    puts message
-    puts((" " * 2) + merge_request["url"])
     puts
-  end
 
-  def print_warnings(warnings)
-    message = "WARNINGS:\n#{warnings}"
-    print_formatted_alert_message(message)
+    response_messages.each do |message|
+      print_message(message)
+      puts
+    end
   end
 
-  def print_formatted_alert_message(message)
-    # A standard terminal window is (at least) 80 characters wide.
-    total_width = 80
-
-    # Git prefixes remote messages with "remote: ", so this width is subtracted
-    # from the width available to us.
-    total_width -= "remote: ".length # rubocop:disable Performance/FixedSize
+  def print_message(message)
+    case message['type']
+    when 'alert'
+      print_alert(message['message'])
+    when 'basic'
+      print_basic(message['message'])
+    end
+  end
 
-    # Our centered text shouldn't start or end right at the edge of the window,
-    # so we add some horizontal padding: 2 chars on either side.
-    text_width = total_width - 2 * 2
+  def print_basic(str)
+    puts str
+  end
 
-    # Automatically wrap message at text_width (= 68) characters:
+  def print_alert(message)
+    # Automatically wrap message at MAX_MESSAGE_TEXT_WIDTH (= 68) characters:
     # Splits the message up into the longest possible chunks matching
-    # "<between 0 and text_width characters><space or end-of-line>".
+    # "<between 0 and MAX_MESSAGE_TEXT_WIDTH characters><space or end-of-line>".
 
     msg_start_idx = 0
     lines = []
     while msg_start_idx < message.length
-      parsed_line = parse_broadcast_msg(message[msg_start_idx..-1], text_width)
+      parsed_line = parse_alert_message(message[msg_start_idx..-1], MAX_MESSAGE_TEXT_WIDTH)
       msg_start_idx += parsed_line.length
       lines.push(parsed_line.strip)
     end
 
-    puts
-    puts "=" * total_width
+    puts "=" * MAX_MESSAGE_WIDTH
     puts
 
     lines.each do |line|
       line.strip!
 
       # Center the line by calculating the left padding measured in characters.
-      line_padding = [(total_width - line.length) / 2, 0].max
+      line_padding = [(MAX_MESSAGE_WIDTH - line.length) / 2, 0].max
       puts((" " * line_padding) + line)
     end
 
     puts
-    puts "=" * total_width
+    puts "=" * MAX_MESSAGE_WIDTH
   end
 
   private
 
-  def parse_broadcast_msg(msg, text_length)
+  def parse_alert_message(msg, text_length)
     msg ||= ""
     # just return msg if shorter than or equal to text length
     return msg if msg.length <= text_length
diff --git a/ruby/gitlab-shell/spec/gitlab_post_receive_spec.rb b/ruby/gitlab-shell/spec/gitlab_post_receive_spec.rb
index d0618309c..b1fd3e3ab 100644
--- a/ruby/gitlab-shell/spec/gitlab_post_receive_spec.rb
+++ b/ruby/gitlab-shell/spec/gitlab_post_receive_spec.rb
@@ -15,19 +15,11 @@ describe GitlabPostReceive do
   let(:gitlab_post_receive) { GitlabPostReceive.new(gl_repository, repo_path, actor, wrongly_encoded_changes, push_options) }
   let(:broadcast_message) { "test " * 10 + "message " * 10 }
   let(:enqueued_at) { Time.new(2016, 6, 23, 6, 59) }
-  let(:new_merge_request_urls) do
-    [{
-      'branch_name' => 'new_branch',
-      'url' => 'http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch',
-      'new_merge_request' => true
-    }]
+  let(:new_merge_request_message) do
+    "To create a merge request for new_branch, visit:\n  http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch"
   end
-  let(:existing_merge_request_urls) do
-    [{
-      'branch_name' => 'feature_branch',
-      'url' => 'http://localhost/dzaporozhets/gitlab-ci/merge_requests/1',
-      'new_merge_request' => false
-    }]
+  let(:existing_merge_request_message) do
+    "View merge request for feature_branch:\n  http://localhost/dzaporozhets/gitlab-ci/merge_requests/1"
   end
 
   before do
@@ -48,15 +40,18 @@ describe GitlabPostReceive do
       let(:response) do
         {
           'reference_counter_decreased' => true,
-          'merge_request_urls' => new_merge_request_urls,
-          'broadcast_message' => broadcast_message
+          'messages' => [
+            { 'type' => 'alert', 'message' => broadcast_message },
+            { 'type' => 'basic', 'message' => new_merge_request_message },
+          ]
         }
       end
 
-      it 'prints the merge request urls and broadcast message' do
+      it 'prints the merge request message and broadcast message' do
         expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
+        assert_first_newline(gitlab_post_receive)
         assert_broadcast_message_printed(gitlab_post_receive)
-        assert_new_mr_printed(gitlab_post_receive)
+        assert_basic_message(gitlab_post_receive, new_merge_request_message)
 
         expect(gitlab_post_receive.exec).to eq(true)
       end
@@ -66,8 +61,9 @@ describe GitlabPostReceive do
 
         it 'doesnt truncate url' do
           expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
+          assert_first_newline(gitlab_post_receive)
           assert_broadcast_message_printed_keep_long_url_end(gitlab_post_receive)
-          assert_new_mr_printed(gitlab_post_receive)
+          assert_basic_message(gitlab_post_receive, new_merge_request_message)
 
           expect(gitlab_post_receive.exec).to eq(true)
         end
@@ -78,8 +74,9 @@ describe GitlabPostReceive do
 
         it 'doesnt truncate url' do
           expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
+          assert_first_newline(gitlab_post_receive)
           assert_broadcast_message_printed_keep_long_url_start(gitlab_post_receive)
-          assert_new_mr_printed(gitlab_post_receive)
+          assert_basic_message(gitlab_post_receive, new_merge_request_message)
 
           expect(gitlab_post_receive.exec).to eq(true)
         end
@@ -90,8 +87,9 @@ describe GitlabPostReceive do
 
         it 'doesnt truncate url' do
           expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
+          assert_first_newline(gitlab_post_receive)
           assert_broadcast_message_printed_keep_long_url_middle(gitlab_post_receive)
-          assert_new_mr_printed(gitlab_post_receive)
+          assert_basic_message(gitlab_post_receive, new_merge_request_message)
 
           expect(gitlab_post_receive.exec).to eq(true)
         end
@@ -102,78 +100,100 @@ describe GitlabPostReceive do
       let(:response) do
         {
           'reference_counter_decreased' => true,
-          'warnings' => 'My warning message'
+          'messages' => [
+            { 'type' => 'alert', 'message' => "WARNINGS:\nMy warning message" }
+          ]
         }
       end
 
       it 'treats the warning as a broadcast message' do
         expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
-        expect(gitlab_post_receive).to receive(:print_formatted_alert_message).with("WARNINGS:\nMy warning message")
+        expect(gitlab_post_receive).to receive(:print_alert).with("WARNINGS:\nMy warning message")
         expect(gitlab_post_receive.exec).to eq(true)
       end
     end
 
     context 'when redirected message available' do
-      let(:message) { "This is a redirected message" }
       let(:response) do
         {
           'reference_counter_decreased' => true,
-          'redirected_message' => message
+          'messages' => [
+            { 'type' => 'basic', 'message' => "This is a redirected message" }
+          ]
         }
       end
 
       it 'prints redirected message' do
         expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
-        assert_redirected_message_printed(gitlab_post_receive)
+        assert_first_newline(gitlab_post_receive)
+        assert_basic_message(gitlab_post_receive, "This is a redirected message")
         expect(gitlab_post_receive.exec).to eq(true)
       end
+    end
 
-      context 'when project created message is available' do
-        let(:message) { "This is a created project message" }
-        let(:response) do
-          {
-            'reference_counter_decreased' => true,
-            'project_created_message' => message
-          }
-        end
+    context 'when project created message is available' do
+      let(:response) do
+        {
+          'reference_counter_decreased' => true,
+          'messages' => [
+            { 'type' => 'basic', 'message' => "This is a created project message" }
+          ]
+        }
+      end
 
-        it 'prints project created message' do
-          expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
+      it 'prints project created message' do
+        expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
 
-          assert_project_created_message_printed(gitlab_post_receive)
+        assert_first_newline(gitlab_post_receive)
+        assert_basic_message(gitlab_post_receive, "This is a created project message")
 
-          expect(gitlab_post_receive.exec).to be true
-        end
+        expect(gitlab_post_receive.exec).to be true
       end
     end
-  end
 
-  private
+    context 'when there are zero messages' do
+      let(:response) do
+        {
+          'reference_counter_decreased' => true,
+          'messages' => []
+        }
+      end
 
-  def assert_new_mr_printed(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).ordered
-    expect(gitlab_post_receive).to receive(:puts).with(
-      "To create a merge request for new_branch, visit:"
-    ).ordered
-    expect(gitlab_post_receive).to receive(:puts).with(
-      "  http://localhost/dzaporozhets/gitlab-ci/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch"
-    ).ordered
-    expect(gitlab_post_receive).to receive(:puts).ordered
-  end
+      it 'does not print anything' do
+        expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
 
-  def assert_existing_mr_printed(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).ordered
-    expect(gitlab_post_receive).to receive(:puts).with(
-      "View merge request for feature_branch:"
-    ).ordered
-    expect(gitlab_post_receive).to receive(:puts).with(
-      "  http://localhost/dzaporozhets/gitlab-ci/merge_requests/1"
-    ).ordered
-    expect(gitlab_post_receive).to receive(:puts).ordered
+        expect(gitlab_post_receive).to_not receive(:puts)
+
+        expect(gitlab_post_receive.exec).to be true
+      end
+    end
+
+    context 'when there is no messages parameter' do
+      let(:response) do
+        {
+          'reference_counter_decreased' => true
+        }
+      end
+
+      it 'does not print anything' do
+        expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response)
+
+        expect(gitlab_post_receive).to_not receive(:puts)
+
+        expect(gitlab_post_receive.exec).to be true
+      end
+    end
   end
 
+  private
+
+  # Red herring: If you see a test failure like this, it is more likely that the
+  # content of one of the puts does not match. It probably does not mean the
+  # puts are out of order.
+  #
+  # Failure/Error: puts message
+  #   #<GitlabPostReceive:0x00007f97e0843008 ...<snip>... received :puts out of order
   def assert_broadcast_message_printed(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).ordered
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
@@ -190,18 +210,10 @@ describe GitlabPostReceive do
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
-  end
-
-  def assert_redirected_message_printed(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).with("This is a redirected message")
-  end
-
-  def assert_project_created_message_printed(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).with("This is a created project message")
+    expect(gitlab_post_receive).to receive(:puts).ordered
   end
 
   def assert_broadcast_message_printed_keep_long_url_end(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).ordered
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
@@ -222,10 +234,10 @@ describe GitlabPostReceive do
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
+    expect(gitlab_post_receive).to receive(:puts).ordered
   end
 
   def assert_broadcast_message_printed_keep_long_url_start(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).ordered
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
@@ -251,10 +263,10 @@ describe GitlabPostReceive do
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
+    expect(gitlab_post_receive).to receive(:puts).ordered
   end
 
   def assert_broadcast_message_printed_keep_long_url_middle(gitlab_post_receive)
-    expect(gitlab_post_receive).to receive(:puts).ordered
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
@@ -280,5 +292,15 @@ describe GitlabPostReceive do
     expect(gitlab_post_receive).to receive(:puts).with(
       "========================================================================"
     ).ordered
+    expect(gitlab_post_receive).to receive(:puts).ordered
+  end
+
+  def assert_basic_message(gitlab_post_receive, message)
+    expect(gitlab_post_receive).to receive(:puts).with(message).ordered
+    expect(gitlab_post_receive).to receive(:puts).ordered
+  end
+
+  def assert_first_newline(gitlab_post_receive)
+    expect(gitlab_post_receive).to receive(:puts).ordered
   end
 end