From 7985115bd1301db867935b52a689ccfc32f13794 Mon Sep 17 00:00:00 2001 From: Florian Zschocke Date: Sat, 10 Dec 2016 16:02:21 +0100 Subject: [PATCH] Fix user mention regular expression and group replacement. The regular expression used for user mentions used to work only inside sentences. Also, since it tested for whitespace, the whitespace would get replaced, too, which would join lines together. Instead the new regex uses boundary matchers to match against word boundaires. As these are not capturing only the actual user mention can be captured and is then replaced. Also, this way the regex can ignore punctuation like in "@jim, look at this." Since Gibtlit now requires Java 7 we can use named capture groups. This makes the use of a centrally defined regular expression much safer. The (admittedly only) group to capture the user name is named "user" and can be referenced by this name. By using the name instead of a group number, the regex could be changed without the code using it breaking because the group number changed. A simple test is added for user mentions, which unfortunately has to deal with the full markdown replacement, too. Fixes #985 --- src/main/java/com/gitblit/Constants.java | 2 +- .../java/com/gitblit/models/TicketModel.java | 2 +- .../com/gitblit/tickets/TicketNotifier.java | 2 +- .../java/com/gitblit/utils/MarkdownUtils.java | 2 +- .../com/gitblit/tests/MarkdownUtilsTest.java | 74 ++++++++++++++++++- 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/gitblit/Constants.java b/src/main/java/com/gitblit/Constants.java index acfc2042..c7112835 100644 --- a/src/main/java/com/gitblit/Constants.java +++ b/src/main/java/com/gitblit/Constants.java @@ -67,7 +67,7 @@ public class Constants { * This regular expression is used when searching for "mentions" in tickets * (when someone writes @thisOtherUser) */ - public static final String REGEX_TICKET_MENTION = "\\s@([^\\s]+)"; + public static final String REGEX_TICKET_MENTION = "\\B@(?[^\\s]+)\\b"; public static final String ZIP_PATH = "/zip/"; diff --git a/src/main/java/com/gitblit/models/TicketModel.java b/src/main/java/com/gitblit/models/TicketModel.java index 924400f5..65e29dc0 100644 --- a/src/main/java/com/gitblit/models/TicketModel.java +++ b/src/main/java/com/gitblit/models/TicketModel.java @@ -778,7 +778,7 @@ public class TicketModel implements Serializable, Comparable { Pattern mentions = Pattern.compile(Constants.REGEX_TICKET_MENTION); Matcher m = mentions.matcher(text); while (m.find()) { - String username = m.group(1); + String username = m.group("user"); plusList(Field.mentions, username); } } catch (Exception e) { diff --git a/src/main/java/com/gitblit/tickets/TicketNotifier.java b/src/main/java/com/gitblit/tickets/TicketNotifier.java index 1d7e4f24..b913db25 100644 --- a/src/main/java/com/gitblit/tickets/TicketNotifier.java +++ b/src/main/java/com/gitblit/tickets/TicketNotifier.java @@ -576,7 +576,7 @@ public class TicketNotifier { Pattern p = Pattern.compile(Constants.REGEX_TICKET_MENTION); Matcher m = p.matcher(lastChange.comment.text); while (m.find()) { - String username = m.group(); + String username = m.group("user"); ccs.add(username); } } diff --git a/src/main/java/com/gitblit/utils/MarkdownUtils.java b/src/main/java/com/gitblit/utils/MarkdownUtils.java index 794d54ab..8371b3c6 100644 --- a/src/main/java/com/gitblit/utils/MarkdownUtils.java +++ b/src/main/java/com/gitblit/utils/MarkdownUtils.java @@ -138,7 +138,7 @@ public class MarkdownUtils { String canonicalUrl = settings.getString(Keys.web.canonicalUrl, "https://localhost:8443"); // emphasize and link mentions - String mentionReplacement = String.format(" **[@$1](%1s/user/$1)**", canonicalUrl); + String mentionReplacement = String.format("**[@${user}](%1s/user/${user})**", canonicalUrl); text = text.replaceAll(Constants.REGEX_TICKET_MENTION, mentionReplacement); // link ticket refs diff --git a/src/test/java/com/gitblit/tests/MarkdownUtilsTest.java b/src/test/java/com/gitblit/tests/MarkdownUtilsTest.java index e40f1057..bc7aad49 100644 --- a/src/test/java/com/gitblit/tests/MarkdownUtilsTest.java +++ b/src/test/java/com/gitblit/tests/MarkdownUtilsTest.java @@ -15,8 +15,14 @@ */ package com.gitblit.tests; +import java.util.HashMap; +import java.util.Map; + import org.junit.Test; +import com.gitblit.IStoredSettings; +import com.gitblit.Keys; +import com.gitblit.tests.mock.MemorySettings; import com.gitblit.utils.MarkdownUtils; public class MarkdownUtilsTest extends GitblitUnitTest { @@ -39,4 +45,70 @@ public class MarkdownUtilsTest extends GitblitUnitTest { assertEquals("
<test>
", MarkdownUtils.transformMarkdown("
<test>
")); } -} \ No newline at end of file + + + @Test + public void testUserMentions() { + IStoredSettings settings = getSettings(); + String repositoryName = "test3"; + String mentionHtml = "@%1$s"; + + String input = "@j.doe"; + String output = "

" + String.format(mentionHtml, "j.doe") + "

"; + assertEquals(output, MarkdownUtils.transformGFM(settings, input, repositoryName)); + + input = " @j.doe"; + output = "

" + String.format(mentionHtml, "j.doe") + "

"; + assertEquals(output, MarkdownUtils.transformGFM(settings, input, repositoryName)); + + input = "@j.doe."; + output = "

" + String.format(mentionHtml, "j.doe") + ".

"; + assertEquals(output, MarkdownUtils.transformGFM(settings, input, repositoryName)); + + input = "To @j.doe: ask @jim.beam!"; + output = "

To " + String.format(mentionHtml, "j.doe") + + ": ask " + String.format(mentionHtml, "jim.beam") + "!

"; + assertEquals(output, MarkdownUtils.transformGFM(settings, input, repositoryName)); + + input = "@sta.rt\n" + + "\n" + + "User mentions in tickets are broken.\n" + + "So:\n" + + "@mc_guyver can fix this.\n" + + "@j.doe, can you test after the fix by @m+guyver?\n" + + "Please review this, @jim.beam!\n" + + "Was reported by @jill and @j!doe from jane@doe yesterday.\n" + + "\n" + + "@jack.daniels can vote for john@wayne.name hopefully.\n" + + "@en.de"; + output = "

" + String.format(mentionHtml, "sta.rt") + "

" + + "

" + "User mentions in tickets are broken.
" + + "So:
" + + String.format(mentionHtml, "mc_guyver") + " can fix this.
" + + String.format(mentionHtml, "j.doe") + ", can you test after the fix by " + String.format(mentionHtml, "m+guyver") + "?
" + + "Please review this, " + String.format(mentionHtml, "jim.beam") + "!
" + + "Was reported by " + String.format(mentionHtml, "jill") + + " and " + String.format(mentionHtml, "j!doe") + + " from jane@doe yesterday." + + "

" + + "

" + String.format(mentionHtml, "jack.daniels") + " can vote for " + + "john@wayne.name hopefully.
" + + String.format(mentionHtml, "en.de") + + "

"; + assertEquals(output, MarkdownUtils.transformGFM(settings, input, repositoryName)); + + } + + + + + private MemorySettings getSettings() { + Map backingMap = new HashMap(); + + backingMap.put(Keys.web.canonicalUrl, "http://localhost"); + backingMap.put(Keys.web.shortCommitIdLength, "7"); + + MemorySettings ms = new MemorySettings(backingMap); + return ms; + } +} -- 2.39.5