From b0b8627d1e1ee0748f1468ce9489357dab482fbb Mon Sep 17 00:00:00 2001 From: Cameron Crockett Date: Mon, 14 May 2018 23:22:28 -0500 Subject: [PATCH] allow long strings to remain intact while parsing broadcast message Added fix for msg nil edge case. fixed comment wording code review issues, bumped version and changelog entry Fixed rebase issues Moved strip out of the function Fixes for code review comments Removed trailing whitespaces --- CHANGELOG | 3 + VERSION | 2 +- lib/gitlab_post_receive.rb | 30 ++++++-- spec/gitlab_post_receive_spec.rb | 119 +++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index af2bcb53e..f42f554d4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,6 @@ +v7.1.4 + - Don't truncate long strings in broadcast message (!202) + v7.1.3 - Use username instead of full name for identifying users (!204) diff --git a/VERSION b/VERSION index 1996c5044..b7f8ee41e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.1.3 +7.1.4 diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index 6009b19a7..cb9931d37 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -75,10 +75,14 @@ class GitlabPostReceive # Automatically wrap message at text_width (= 68) characters: # Splits the message up into the longest possible chunks matching # "". - # The last result is always an empty string (0 chars and the end-of-line), - # so drop that. - # message.scan returns a nested array of capture groups, so flatten. - lines = message.scan(/(.{,#{text_width}})(?:\s|$)/)[0...-1].flatten + + msg_start_idx = 0 + lines = [] + while msg_start_idx < message.length + parsed_line = parse_broadcast_msg(message[msg_start_idx..-1], text_width) + msg_start_idx += parsed_line.length + lines.push(parsed_line.strip) + end puts puts "=" * total_width @@ -95,4 +99,22 @@ class GitlabPostReceive puts puts "=" * total_width end + + private + + def parse_broadcast_msg(msg, text_length) + msg ||= "" + # just return msg if shorter than or equal to text length + return msg if msg.length <= text_length + + # search for word break shorter than text length + truncate_to_space = msg.match(/\A(.{,#{text_length}})(?=\s|$)(\s*)/).to_s + + if truncate_to_space.empty? + # search for word break longer than text length + truncate_to_space = msg.match(/\A\S+/).to_s + end + + truncate_to_space + end end diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index ec7e24879..46e615881 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -59,6 +59,43 @@ describe GitlabPostReceive do expect(gitlab_post_receive.exec).to eq(true) end + + context 'when contains long url string at end' do + let(:broadcast_message) { "test " * 10 + "message " * 10 + "https://localhost:5000/test/a/really/long/url/that/is/in/the/broadcast/message/do-not-truncate-when-url" } + + it 'doesnt truncate url' do + expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response) + assert_broadcast_message_printed_keep_long_url_end(gitlab_post_receive) + assert_new_mr_printed(gitlab_post_receive) + + expect(gitlab_post_receive.exec).to eq(true) + end + end + + context 'when contains long url string at start' do + let(:broadcast_message) { "https://localhost:5000/test/a/really/long/url/that/is/in/the/broadcast/message/do-not-truncate-when-url " + "test " * 10 + "message " * 11} + + it 'doesnt truncate url' do + expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response) + assert_broadcast_message_printed_keep_long_url_start(gitlab_post_receive) + assert_new_mr_printed(gitlab_post_receive) + + expect(gitlab_post_receive.exec).to eq(true) + end + end + + context 'when contains long url string in middle' do + let(:broadcast_message) { "test " * 11 + "https://localhost:5000/test/a/really/long/url/that/is/in/the/broadcast/message/do-not-truncate-when-url " + "message " * 11} + + it 'doesnt truncate url' do + expect_any_instance_of(GitlabNet).to receive(:post_receive).and_return(response) + assert_broadcast_message_printed_keep_long_url_middle(gitlab_post_receive) + assert_new_mr_printed(gitlab_post_receive) + + expect(gitlab_post_receive.exec).to eq(true) + end + end + end context 'when redirected message available' do @@ -147,4 +184,86 @@ describe GitlabPostReceive do def assert_project_created_message_printed(gitlab_post_receive) expect(gitlab_post_receive).to receive(:puts).with("This is a created project message") 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 + expect(gitlab_post_receive).to receive(:puts).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " test test test test test test test test test test message message" + ).ordered + expect(gitlab_post_receive).to receive(:puts).with( + " message message message message message message message message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + "https://localhost:5000/test/a/really/long/url/that/is/in/the/broadcast/message/do-not-truncate-when-url" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "========================================================================" + ).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 + expect(gitlab_post_receive).to receive(:puts).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + "https://localhost:5000/test/a/really/long/url/that/is/in/the/broadcast/message/do-not-truncate-when-url" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " test test test test test test test test test test message message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " message message message message message message message message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "========================================================================" + ).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 + expect(gitlab_post_receive).to receive(:puts).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " test test test test test test test test test test test" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + "https://localhost:5000/test/a/really/long/url/that/is/in/the/broadcast/message/do-not-truncate-when-url" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " message message message message message message message message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).with( + " message message message" + ).ordered + + expect(gitlab_post_receive).to receive(:puts).ordered + expect(gitlab_post_receive).to receive(:puts).with( + "========================================================================" + ).ordered + end end -- GitLab