]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-17271 update issue changelog with information about webhook
authorAurelien Poscia <aurelien.poscia@sonarsource.com>
Thu, 15 Sep 2022 10:05:00 +0000 (12:05 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 19 Sep 2022 20:03:08 +0000 (20:03 +0000)
19 files changed:
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java
server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeDto.java
server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDtoTest.java
server/sonar-webserver-api/src/test/java/org/sonar/server/qualitygate/changeevent/QGChangeEventListenersImplTest.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/IssueChangeWSSupport.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/ChangelogAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/IssueUpdater.java
server/sonar-webserver-webapi/src/main/resources/org/sonar/server/issue/ws/changelog-example.json
server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/IssueChangeWSSupportTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/ChangelogActionTest.java
sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java
sonar-core/src/main/java/org/sonar/core/issue/IssueChangeContext.java
sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java
sonar-core/src/test/java/org/sonar/core/issue/FieldDiffsTest.java
sonar-core/src/test/java/org/sonar/core/issue/IssueChangeContextTest.java
sonar-ws/src/main/protobuf/ws-commons.proto

index af4bf0da1a6aea1582cf139b905c1a98322efdf5..475d7a477f63f611af7d6e3a6087fef6c5fc6c5d 100644 (file)
@@ -152,13 +152,15 @@ public class IssueLifecycle {
   /**
    * Copy a diff from another issue
    */
-  private static Optional<FieldDiffs> copyFieldDiffOfIssueFromOtherBranch(String issueKey, FieldDiffs c) {
+  private static Optional<FieldDiffs> copyFieldDiffOfIssueFromOtherBranch(String issueKey, FieldDiffs source) {
     FieldDiffs result = new FieldDiffs();
     result.setIssueKey(issueKey);
-    result.setUserUuid(c.userUuid());
-    result.setCreationDate(c.creationDate());
+    source.userUuid().ifPresent(result::setUserUuid);
+    source.webhookSource().ifPresent(result::setWebhookSource);
+    source.externalUser().ifPresent(result::setExternalUser);
+    result.setCreationDate(source.creationDate());
     // Don't copy "file" changelogs as they refer to file uuids that might later be purged
-    c.diffs().entrySet().stream()
+    source.diffs().entrySet().stream()
       .filter(e -> !e.getKey().equals(IssueFieldsSetter.FILE))
       .forEach(e -> result.setDiff(e.getKey(), e.getValue().oldValue(), e.getValue().newValue()));
     if (result.diffs().isEmpty()) {
index d7cc18c9076978efc5396dfe5ecc73b69a16595d..d6bd0fddb89a22814df5fe918d552ad265879b42 100644 (file)
@@ -245,14 +245,8 @@ public class ProtobufIssueDiskCache implements DiskCache<DefaultIssue> {
     IssueCache.FieldDiffs.Builder builder = IssueCache.FieldDiffs.newBuilder()
       .setCreationDate(fieldDiffs.creationDate().getTime());
 
-    if (fieldDiffs.issueKey() != null) {
-      builder.setIssueKey(fieldDiffs.issueKey());
-    }
-
-    String userUuid = fieldDiffs.userUuid();
-    if (userUuid != null) {
-      builder.setUserUuid(userUuid);
-    }
+    fieldDiffs.issueKey().ifPresent(builder::setIssueKey);
+    fieldDiffs.userUuid().ifPresent(builder::setUserUuid);
 
     for (Map.Entry<String, FieldDiffs.Diff> e : fieldDiffs.diffs().entrySet()) {
       IssueCache.Diff.Builder diffBuilder = IssueCache.Diff.newBuilder();
index 28d34eef594f547bcc4e457365028c7e1ff26a1b..cc9f085590209a782855a31236e34f97c450b99c 100644 (file)
@@ -156,10 +156,10 @@ public class IssueLifecycleTest {
       .containsOnly(tuple("raw", commentDate, "user_uuid", "A comment"));
     assertThat(raw.changes()).hasSize(2);
     assertThat(raw.changes().get(0).creationDate()).isEqualTo(diffDate);
-    assertThat(raw.changes().get(0).userUuid()).isEqualTo("user_uuid");
-    assertThat(raw.changes().get(0).issueKey()).isEqualTo("raw");
+    assertThat(raw.changes().get(0).userUuid()).contains("user_uuid");
+    assertThat(raw.changes().get(0).issueKey()).contains("raw");
     assertThat(raw.changes().get(0).diffs()).containsOnlyKeys("severity");
-    assertThat(raw.changes().get(1).userUuid()).isEqualTo("default_user_uuid");
+    assertThat(raw.changes().get(1).userUuid()).contains("default_user_uuid");
     assertThat(raw.changes().get(1).diffs()).containsOnlyKeys(IssueFieldsSetter.FROM_BRANCH);
     assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_BRANCH).oldValue()).isEqualTo("#2");
     assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_BRANCH).newValue()).isEqualTo("master");
@@ -213,15 +213,47 @@ public class IssueLifecycleTest {
       .containsOnly(tuple("raw", commentDate, "user_uuid", "A comment"));
     assertThat(raw.changes()).hasSize(2);
     assertThat(raw.changes().get(0).creationDate()).isEqualTo(diffDate);
-    assertThat(raw.changes().get(0).userUuid()).isEqualTo("user_uuid");
-    assertThat(raw.changes().get(0).issueKey()).isEqualTo("raw");
+    assertThat(raw.changes().get(0).userUuid()).contains("user_uuid");
+    assertThat(raw.changes().get(0).issueKey()).contains("raw");
     assertThat(raw.changes().get(0).diffs()).containsOnlyKeys("severity");
-    assertThat(raw.changes().get(1).userUuid()).isEqualTo("default_user_uuid");
+    assertThat(raw.changes().get(1).userUuid()).contains("default_user_uuid");
     assertThat(raw.changes().get(1).diffs()).containsOnlyKeys(IssueFieldsSetter.FROM_BRANCH);
     assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_BRANCH).oldValue()).isEqualTo("sourceBranch-1");
     assertThat(raw.changes().get(1).get(IssueFieldsSetter.FROM_BRANCH).newValue()).isEqualTo("#1");
   }
 
+  @Test
+  public void copyExistingIssuesFromSourceBranchOfPullRequest_copyFieldDiffsCorrectly() {
+    String pullRequestKey = "1";
+    Branch branch = mock(Branch.class);
+    when(branch.getType()).thenReturn(BranchType.PULL_REQUEST);
+    when(branch.getName()).thenReturn("sourceBranch-1");
+    when(branch.getPullRequestKey()).thenReturn(pullRequestKey);
+    analysisMetadataHolder.setBranch(branch);
+    analysisMetadataHolder.setPullRequestKey(pullRequestKey);
+    DefaultIssue destIssue = new DefaultIssue()
+      .setKey("raw");
+    DefaultIssue sourceIssue = new DefaultIssue()
+      .setKey("issue");
+    sourceIssue.setResolution("resolution");
+    sourceIssue.setStatus("status");
+
+    FieldDiffs sourceFieldDiffs = new FieldDiffs();
+    sourceIssue.addChange(sourceFieldDiffs
+      .setCreationDate(new Date())
+      .setIssueKey("short")
+      .setUserUuid("user_uuid")
+      .setExternalUser("toto")
+      .setWebhookSource("github")
+      .setDiff("severity", "MINOR", "MAJOR"));
+
+    underTest.copyExistingIssueFromSourceBranchToPullRequest(destIssue, sourceIssue);
+
+    FieldDiffs actualFieldDiffs = destIssue.changes().iterator().next();
+    assertThat(actualFieldDiffs.issueKey()).contains(destIssue.key());
+    assertThat(actualFieldDiffs).usingRecursiveComparison().ignoringFields("issueKey").isEqualTo(sourceFieldDiffs);
+  }
+
   @Test
   public void copyExistingIssuesFromSourceBranchOfPullRequest_only_works_for_pull_requests() {
     DefaultIssue raw = new DefaultIssue()
index 4de49abe586b3c12e9ccba78374aea17320f4108..4d7332ddd6c249211591e36610b8790dd7d59b41 100644 (file)
@@ -81,7 +81,7 @@ public final class IssueChangeDto implements Serializable {
     IssueChangeDto dto = newDto(issueKey);
     dto.setChangeType(IssueChangeDto.TYPE_FIELD_CHANGE);
     dto.setChangeData(diffs.toEncodedString());
-    dto.setUserUuid(diffs.userUuid());
+    dto.setUserUuid(diffs.userUuid().orElse(null));
     Date createdAt = requireNonNull(diffs.creationDate(), "Diffs created at must not be null");
     dto.setIssueChangeCreationDate(createdAt.getTime());
     dto.setProjectUuid(projectUuid);
index 6dbeb0a4a52d99d76bd3f7ff60c2c525ed9390d2..a96813de475e3bd4f6d69aa0a3828bac7873405c 100644 (file)
@@ -115,8 +115,8 @@ public class IssueChangeDtoTest {
       .setIssueChangeCreationDate(System2.INSTANCE.now());
 
     FieldDiffs diffs = changeDto.toFieldDiffs();
-    assertThat(diffs.userUuid()).isEqualTo("user_uuid");
-    assertThat(diffs.issueKey()).isEqualTo("ABCDE");
+    assertThat(diffs.userUuid()).contains("user_uuid");
+    assertThat(diffs.issueKey()).contains("ABCDE");
     assertThat(diffs.creationDate()).isNotNull();
   }
 
@@ -130,8 +130,8 @@ public class IssueChangeDtoTest {
       .setCreatedAt(System2.INSTANCE.now());
 
     FieldDiffs diffs = changeDto.toFieldDiffs();
-    assertThat(diffs.userUuid()).isEqualTo("user_uuid");
-    assertThat(diffs.issueKey()).isEqualTo("ABCDE");
+    assertThat(diffs.userUuid()).contains("user_uuid");
+    assertThat(diffs.issueKey()).contains("ABCDE");
     assertThat(diffs.creationDate()).isNotNull();
   }
 
index 44288f025486bb7e0270b275de76f8f2a4ac63c5..4a25dc9339567262c2f9a35e27586955b1ea777a 100644 (file)
@@ -82,7 +82,6 @@ public class QGChangeEventListenersImplTest {
   @Test
   public void broadcastOnIssueChange_has_no_effect_when_issues_are_empty() {
     underTest.broadcastOnIssueChange(emptyList(), singletonList(component1QGChangeEvent), false);
-
     verifyNoInteractions(listener1, listener2, listener3);
   }
 
index 373841895e0e14317a821e9c7970ad0b64068b8b..e251da155b5e8d94795ae6e817930545938cd361 100644 (file)
@@ -147,12 +147,15 @@ public class IssueChangeWSSupport {
   private Map<String, UserDto> loadUsers(DbSession dbSession, Map<String, List<FieldDiffs>> changesByRuleKey,
     Map<String, List<IssueChangeDto>> commentsByIssueKey, Set<UserDto> preloadedUsers) {
     Set<String> userUuids = Stream.concat(
-      changesByRuleKey.values().stream()
-        .flatMap(Collection::stream)
-        .map(FieldDiffs::userUuid),
-      commentsByIssueKey.values().stream()
-        .flatMap(Collection::stream)
-        .map(IssueChangeDto::getUserUuid))
+        changesByRuleKey.values().stream()
+          .flatMap(Collection::stream)
+          .map(FieldDiffs::userUuid)
+          .filter(Optional::isPresent)
+          .map(Optional::get),
+        commentsByIssueKey.values().stream()
+          .flatMap(Collection::stream)
+          .map(IssueChangeDto::getUserUuid)
+      )
       .filter(Objects::nonNull)
       .collect(toSet());
     if (userUuids.isEmpty()) {
@@ -168,8 +171,8 @@ public class IssueChangeWSSupport {
     }
 
     return Stream.concat(
-      preloadedUsers.stream(),
-      dbClient.userDao().selectByUuids(dbSession, missingUsersUuids).stream())
+        preloadedUsers.stream(),
+        dbClient.userDao().selectByUuids(dbSession, missingUsersUuids).stream())
       .filter(t -> userUuids.contains(t.getUuid()))
       .collect(uniqueIndex(UserDto::getUuid, userUuids.size()));
   }
@@ -200,8 +203,8 @@ public class IssueChangeWSSupport {
     }
 
     return Stream.concat(
-      preloadedComponents.stream(),
-      dbClient.componentDao().selectByUuids(dbSession, missingFileUuids).stream())
+        preloadedComponents.stream(),
+        dbClient.componentDao().selectByUuids(dbSession, missingFileUuids).stream())
       .filter(t -> fileUuids.contains(t.uuid()))
       .collect(uniqueIndex(ComponentDto::uuid, fileUuids.size()));
   }
@@ -227,16 +230,17 @@ public class IssueChangeWSSupport {
 
   private Function<FieldDiffs, Common.Changelog> toWsChangelog(FormattingContext formattingContext) {
     return change -> {
-      String userUUuid = change.userUuid();
       Common.Changelog.Builder changelogBuilder = Common.Changelog.newBuilder();
       changelogBuilder.setCreationDate(formatDateTime(change.creationDate()));
-      formattingContext.getUserByUuid(userUUuid)
+      change.userUuid().flatMap(formattingContext::getUserByUuid)
         .ifPresent(user -> {
           changelogBuilder.setUser(user.getLogin());
           changelogBuilder.setIsUserActive(user.isActive());
           ofNullable(user.getName()).ifPresent(changelogBuilder::setUserName);
           ofNullable(emptyToNull(user.getEmail())).ifPresent(email -> changelogBuilder.setAvatar(avatarFactory.create(user)));
         });
+      change.externalUser().ifPresent(changelogBuilder::setExternalUser);
+      change.webhookSource().ifPresent(changelogBuilder::setWebhookSource);
       change.diffs().entrySet().stream()
         .map(toWsDiff(formattingContext))
         .forEach(changelogBuilder::addDiffs);
index 848b32b3f65175adb33d38a901d7f88818b33fe6..271400e7b74d886652e44aa8a175c2bf71ba16fb 100644 (file)
@@ -56,6 +56,7 @@ public class ChangelogAction implements IssuesWsAction {
         "Requires the 'Browse' permission on the project of the specified issue.")
       .setSince("4.1")
       .setChangelog(
+        new Change("9.7", "'externalUser' and 'webhookSource' information added to the answer"),
         new Change("6.3", "changes on effort is expressed with the raw value in minutes (instead of the duration previously)"))
       .setHandler(this)
       .setResponseExample(Resources.getResource(IssuesWs.class, "changelog-example.json"));
index 3e45b2163b3811d75d02c275ed7ee3ccb844c8dc..ceb3870e7a34912d884e7674992886ed197ec2fa 100644 (file)
@@ -49,6 +49,7 @@ import org.sonar.server.notification.NotificationManager;
 import static com.google.common.base.Preconditions.checkState;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
+import static org.apache.commons.lang.StringUtils.isNotEmpty;
 import static org.sonar.db.component.BranchType.PULL_REQUEST;
 
 public class IssueUpdater {
@@ -86,7 +87,8 @@ public class IssueUpdater {
 
     if (context.refreshMeasures()) {
       List<DefaultIssue> changedIssues = result.getIssues().stream().map(IssueDto::toDefaultIssue).collect(MoreCollectors.toList(result.getIssues().size()));
-      issueChangePostProcessor.process(dbSession, changedIssues, singleton(component), context.fromAlm());
+      boolean isChangeFromWebhook = isNotEmpty(context.getWebhookSource());
+      issueChangePostProcessor.process(dbSession, changedIssues, singleton(component), isChangeFromWebhook);
     }
 
     return result;
index 45a0a287e07931a5332f1946847e518352a9f7f8..04454c24a7c42a54c9c6e20c94230c577f09e95d 100644 (file)
@@ -6,6 +6,8 @@
       "isUserActive": true,
       "avatar": "b0d8c6e5ea589e6fc3d3e08afb1873bb",
       "creationDate": "2014-03-04T23:03:44+0100",
+      "externalUser": "toto",
+      "webhookSource": "github",
       "diffs": [
         {
           "key": "severity",
index 6d5c97af7baa8fb991436579d5a96b7bfe7cffb3..0022fb45ac67c69f83dc936b160474a93befc27d 100644 (file)
@@ -20,6 +20,7 @@
 package org.sonar.server.issue;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.MoreCollectors;
 import com.tngtech.java.junit.dataprovider.DataProvider;
 import com.tngtech.java.junit.dataprovider.DataProviderRunner;
 import com.tngtech.java.junit.dataprovider.UseDataProvider;
@@ -525,6 +526,26 @@ public class IssueChangeWSSupportTest {
     assertThat(wsChangelog.getDiffsList().get(3).getNewValue()).isEqualTo("e");
   }
 
+  @Test
+  public void formatChangelog_handlesCorrectlyExternalUserAndWebhookSource() {
+    IssueDto issue = dbTester.issues().insertIssue();
+
+    IssueChangeDto issueChangeDto = newFieldChange(issue)
+      .setChangeData(new FieldDiffs()
+        .setDiff("f_change_" + 1, null, null)
+        .setExternalUser("toto")
+        .setWebhookSource("github")
+        .toEncodedString());
+
+    dbTester.issues().insertChange(issueChangeDto);
+
+    FormattingContext formattingContext = underTest.newFormattingContext(dbTester.getSession(), singleton(issue), Load.CHANGE_LOG);
+
+    Changelog changeLog = underTest.formatChangelog(issue, formattingContext).collect(MoreCollectors.onlyElement());
+    assertThat(changeLog.getExternalUser()).isEqualTo("toto");
+    assertThat(changeLog.getWebhookSource()).isEqualTo("github");
+  }
+
   @Test
   @UseDataProvider("loadAllOrChangelog")
   public void formatChangelog_returns_user_details_if_exists(Load load) {
index fedb3a435e1062e94d0e88ab99a0e52bcabae0f2..1408612864f67b106c4f83116d28d6e887be6dc4 100644 (file)
@@ -42,6 +42,7 @@ import org.sonar.server.issue.IssueFinder;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
+import org.sonarqube.ws.Common;
 import org.sonarqube.ws.Common.Changelog.Diff;
 import org.sonarqube.ws.Issues.ChangelogWsResponse;
 
@@ -57,7 +58,6 @@ import static org.sonar.test.JsonAssert.assertJson;
 
 public class ChangelogActionTest {
 
-
   @Rule
   public DbTester db = DbTester.create(System2.INSTANCE);
 
@@ -83,17 +83,30 @@ public class ChangelogActionTest {
     IssueDto issueDto = insertNewIssue();
     userSession.logIn("john")
       .addProjectPermission(USER, project, file);
-    db.issues().insertFieldDiffs(issueDto, new FieldDiffs().setUserUuid(user.getUuid()).setDiff("severity", "MAJOR", "BLOCKER").setCreationDate(new Date()));
+    FieldDiffs fieldDiffs = createFieldDiffs(user);
+    db.issues().insertFieldDiffs(issueDto, fieldDiffs);
 
     ChangelogWsResponse result = call(issueDto.getKey());
 
     assertThat(result.getChangelogList()).hasSize(1);
-    assertThat(result.getChangelogList().get(0).getUser()).isNotNull().isEqualTo(user.getLogin());
-    assertThat(result.getChangelogList().get(0).getUserName()).isNotNull().isEqualTo(user.getName());
-    assertThat(result.getChangelogList().get(0).getIsUserActive()).isTrue();
-    assertThat(result.getChangelogList().get(0).getAvatar()).isNotNull().isEqualTo("93942e96f5acd83e2e047ad8fe03114d");
-    assertThat(result.getChangelogList().get(0).getCreationDate()).isNotEmpty();
-    assertThat(result.getChangelogList().get(0).getDiffsList()).extracting(Diff::getKey, Diff::getOldValue, Diff::getNewValue).containsOnly(tuple("severity", "MAJOR", "BLOCKER"));
+    Common.Changelog changelog = result.getChangelogList().get(0);
+    assertThat(changelog.getUser()).isNotNull().isEqualTo(user.getLogin());
+    assertThat(changelog.getUserName()).isNotNull().isEqualTo(user.getName());
+    assertThat(changelog.getIsUserActive()).isTrue();
+    assertThat(changelog.getAvatar()).isNotNull().isEqualTo("93942e96f5acd83e2e047ad8fe03114d");
+    assertThat(changelog.getCreationDate()).isNotEmpty();
+    assertThat(changelog.getExternalUser()).isEqualTo(fieldDiffs.externalUser().orElse(null));
+    assertThat(changelog.getWebhookSource()).isEqualTo(fieldDiffs.webhookSource().orElse(null));
+    assertThat(changelog.getDiffsList()).extracting(Diff::getKey, Diff::getOldValue, Diff::getNewValue).containsOnly(tuple("severity", "MAJOR", "BLOCKER"));
+  }
+
+  private static FieldDiffs createFieldDiffs(UserDto user) {
+    return new FieldDiffs()
+      .setUserUuid(user.getUuid())
+      .setDiff("severity", "MAJOR", "BLOCKER")
+      .setCreationDate(new Date())
+      .setExternalUser("toto")
+      .setWebhookSource("github");
   }
 
   @Test
@@ -274,7 +287,10 @@ public class ChangelogActionTest {
       .addProjectPermission(USER, project, file);
     db.issues().insertFieldDiffs(issueDto, new FieldDiffs()
       .setUserUuid(user.getUuid())
-      .setDiff("severity", "MAJOR", "BLOCKER").setCreationDate(new Date())
+      .setDiff("severity", "MAJOR", "BLOCKER")
+      .setWebhookSource("github")
+      .setExternalUser("toto")
+      .setCreationDate(new Date())
       .setCreationDate(DateUtils.parseDateTime("2014-03-04T23:03:44+0100")));
 
     String result = tester.newRequest().setParam("issue", issueDto.getKey()).execute().getInput();
index b48470bcc01586b9ae86892fbd0d1b70fd45457b..ceabe70333c3804f81b18c17887399692e0dae88 100644 (file)
@@ -33,8 +33,8 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.Optional;
+import java.util.Set;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.lang.StringUtils;
@@ -502,6 +502,8 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure.
         currentChange = new FieldDiffs();
         currentChange.setUserUuid(context.userUuid());
         currentChange.setCreationDate(context.date());
+        currentChange.setWebhookSource(context.getWebhookSource());
+        currentChange.setExternalUser(context.getExternalUser());
       }
       currentChange.setDiff(field, oldValue, newValue);
     }
index 1c9e19321163b1ff9dfcf08e69350ef8b91dd048..d3ae735b67eb85a1f3ed5d8bf5cab5b45da5fb07 100644 (file)
@@ -21,17 +21,22 @@ package org.sonar.core.issue;
 
 import java.io.Serializable;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.Date;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.lang.StringUtils;
 
 import static com.google.common.base.Strings.emptyToNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
+import static org.apache.commons.lang.StringUtils.isNotBlank;
+import static org.apache.commons.lang.StringUtils.trimToNull;
 
 /**
  * PLUGINS MUST NOT USE THIS CLASS, EXCEPT FOR UNIT TESTING.
@@ -40,15 +45,19 @@ import static com.google.common.base.Strings.isNullOrEmpty;
  */
 public class FieldDiffs implements Serializable {
   private static final String CHAR_TO_ESCAPE = "|,{}=:";
-  public static final String ASSIGNEE = "assignee";
-  public static final String ENCODING_PREFIX = "{base64:";
-  public static final String ENCODING_SUFFIX = "}";
+  private static final String ASSIGNEE = "assignee";
+  private static final String ENCODING_PREFIX = "{base64:";
+  private static final String ENCODING_SUFFIX = "}";
+  private static final String WEBHOOK_SOURCE = "webhookSource";
+  private static final String EXTERNAL_USER_KEY = "externalUser";
 
   private final Map<String, Diff> diffs = new LinkedHashMap<>();
 
-  private String issueKey;
-  private String userUuid;
-  private Date creationDate;
+  private String issueKey = null;
+  private String userUuid = null;
+  private Date creationDate = null;
+  private String externalUser = null;
+  private String webhookSource = null;
 
   public Map<String, Diff> diffs() {
     if (diffs.containsKey(ASSIGNEE)) {
@@ -66,9 +75,8 @@ public class FieldDiffs implements Serializable {
     return diffs.get(field);
   }
 
-  @CheckForNull
-  public String userUuid() {
-    return userUuid;
+  public Optional<String> userUuid() {
+    return Optional.ofNullable(userUuid);
   }
 
   public FieldDiffs setUserUuid(@Nullable String s) {
@@ -85,9 +93,8 @@ public class FieldDiffs implements Serializable {
     return this;
   }
 
-  @CheckForNull
-  public String issueKey() {
-    return issueKey;
+  public Optional<String> issueKey() {
+    return Optional.ofNullable(issueKey);
   }
 
   public FieldDiffs setIssueKey(@Nullable String issueKey) {
@@ -95,6 +102,24 @@ public class FieldDiffs implements Serializable {
     return this;
   }
 
+  public Optional<String> externalUser() {
+    return Optional.ofNullable(externalUser);
+  }
+
+  public FieldDiffs setExternalUser(@Nullable String externalUser) {
+    this.externalUser = externalUser;
+    return this;
+  }
+
+  public Optional<String> webhookSource() {
+    return Optional.ofNullable(webhookSource);
+  }
+
+  public FieldDiffs setWebhookSource(@Nullable String webhookSource) {
+    this.webhookSource = webhookSource;
+    return this;
+  }
+
   @SuppressWarnings("unchecked")
   public FieldDiffs setDiff(String field, @Nullable Serializable oldValue, @Nullable Serializable newValue) {
     Diff diff = diffs.get(field);
@@ -121,23 +146,22 @@ public class FieldDiffs implements Serializable {
   }
 
   private String serialize(boolean shouldEncode) {
-    StringBuilder sb = new StringBuilder();
-    boolean notFirst = false;
-    for (Map.Entry<String, Diff> entry : diffs.entrySet()) {
-      if (notFirst) {
-        sb.append(',');
-      } else {
-        notFirst = true;
-      }
-      sb.append(entry.getKey());
-      sb.append('=');
-      if (shouldEncode) {
-        sb.append(entry.getValue().toEncodedString());
-      } else {
-        sb.append(entry.getValue().toString());
-      }
+    List<String> serializedStrings = new ArrayList<>();
+    if (isNotBlank(webhookSource)) {
+      serializedStrings.add(WEBHOOK_SOURCE + "=" + webhookSource);
     }
-    return sb.toString();
+    if (isNotBlank(externalUser)) {
+      serializedStrings.add(EXTERNAL_USER_KEY + "=" + externalUser);
+    }
+    diffs.entrySet().stream()
+      .map(entry -> serializeKeyValuePair(shouldEncode, entry.getKey(), entry.getValue()))
+      .forEach(serializedStrings::add);
+    return StringUtils.join(serializedStrings, ",");
+  }
+
+  private static String serializeKeyValuePair(boolean shouldEncode, String key, Diff values) {
+    String serializedValues = shouldEncode ? values.toEncodedString() : values.toString();
+    return key + "=" + serializedValues;
   }
 
   public static FieldDiffs parse(@Nullable String s) {
@@ -152,21 +176,32 @@ public class FieldDiffs implements Serializable {
       }
 
       String[] keyValues = field.split("=", 2);
+      String key = keyValues[0];
       if (keyValues.length == 2) {
         String values = keyValues[1];
-        int split = values.indexOf('|');
-        if (split > -1) {
-          diffs.setDiff(keyValues[0], emptyToNull(values.substring(0, split)), emptyToNull(values.substring(split + 1)));
+        if (EXTERNAL_USER_KEY.equals(key)) {
+          diffs.setExternalUser(trimToNull(values));
+        } else if (WEBHOOK_SOURCE.equals(key)) {
+          diffs.setWebhookSource(trimToNull(values));
         } else {
-          diffs.setDiff(keyValues[0], null, emptyToNull(values));
+          addDiff(diffs, key, values);
         }
       } else {
-        diffs.setDiff(keyValues[0], null, null);
+        diffs.setDiff(key, null, null);
       }
     }
     return diffs;
   }
 
+  private static void addDiff(FieldDiffs diffs, String key, String values) {
+    int split = values.indexOf('|');
+    if (split > -1) {
+      diffs.setDiff(key, emptyToNull(values.substring(0, split)), emptyToNull(values.substring(split + 1)));
+    } else {
+      diffs.setDiff(key, null, emptyToNull(values));
+    }
+  }
+
   @SuppressWarnings("unchecked")
   Diff decode(Diff encoded) {
     return new Diff(
index a81a0ac22248ddd40cc8c6a7b8e5efea494ba7a5..629da34db1a2708ab7315890b1513a544a9e15b8 100644 (file)
@@ -33,14 +33,17 @@ public class IssueChangeContext implements Serializable {
   private final Date date;
   private final boolean scan;
   private final boolean refreshMeasures;
-  private final boolean fromAlm;
+  private final String externalUser;
+  private final String webhookSource;
 
-  private IssueChangeContext(@Nullable String userUuid, Date date, boolean scan, boolean refreshMeasures, boolean fromAlm) {
+  private IssueChangeContext(Date date, boolean scan, boolean refreshMeasures, @Nullable String userUuid, @Nullable String externalUser,
+    @Nullable String webhookSource) {
     this.userUuid = userUuid;
     this.date = requireNonNull(date);
     this.scan = scan;
     this.refreshMeasures = refreshMeasures;
-    this.fromAlm = fromAlm;
+    this.externalUser = externalUser;
+    this.webhookSource = webhookSource;
   }
 
   @CheckForNull
@@ -60,8 +63,14 @@ public class IssueChangeContext implements Serializable {
     return refreshMeasures;
   }
 
-  public boolean fromAlm() {
-    return fromAlm;
+  @Nullable
+  public String getExternalUser() {
+    return externalUser;
+  }
+
+  @Nullable
+  public String getWebhookSource() {
+    return webhookSource;
   }
 
   @Override
@@ -73,26 +82,13 @@ public class IssueChangeContext implements Serializable {
       return false;
     }
     IssueChangeContext that = (IssueChangeContext) o;
-    return scan == that.scan &&
-      Objects.equals(userUuid, that.userUuid) &&
-      Objects.equals(date, that.date) &&
-      refreshMeasures == that.refreshMeasures;
+    return scan == that.scan && refreshMeasures == that.refreshMeasures &&  Objects.equals(userUuid, that.userUuid) && date.equals(that.date)
+      && Objects.equals(externalUser, that.getExternalUser()) && Objects.equals(webhookSource, that.getWebhookSource());
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(userUuid, date, scan, refreshMeasures, fromAlm);
-  }
-
-  @Override
-  public String toString() {
-    return "IssueChangeContext{" +
-      "userUuid='" + userUuid + '\'' +
-      ", date=" + date +
-      ", scan=" + scan +
-      ", refreshMeasures=" + refreshMeasures +
-      ", fromAlm=" + fromAlm +
-      '}';
+    return Objects.hash(userUuid, date, scan, refreshMeasures, externalUser, webhookSource);
   }
 
   public static IssueChangeContextBuilder newBuilder() {
@@ -112,7 +108,8 @@ public class IssueChangeContext implements Serializable {
     private Date date;
     private boolean scan = false;
     private boolean refreshMeasures = false;
-    private boolean fromAlm = false;
+    private String externalUser;
+    private String webhookSource;
 
     private IssueChangeContextBuilder() {
     }
@@ -137,13 +134,18 @@ public class IssueChangeContext implements Serializable {
       return this;
     }
 
-    public IssueChangeContextBuilder withFromAlm() {
-      this.fromAlm = true;
+    public IssueChangeContextBuilder setExternalUser(@Nullable String externalUser) {
+      this.externalUser = externalUser;
+      return this;
+    }
+
+    public IssueChangeContextBuilder setWebhookSource(@Nullable String webhookSource) {
+      this.webhookSource = webhookSource;
       return this;
     }
 
     public IssueChangeContext build() {
-      return new IssueChangeContext(userUuid, date, scan, refreshMeasures, fromAlm);
+      return new IssueChangeContext(date, scan, refreshMeasures, userUuid, externalUser, webhookSource);
     }
   }
 }
index 2a5091278b89a7fa4765e6ddcd543a7fd9d3775d..9f16990058b8689c07e753b9636fa47621941cb9 100644 (file)
@@ -30,6 +30,7 @@ import org.sonar.api.utils.Duration;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class DefaultIssueTest {
 
@@ -198,9 +199,17 @@ public class DefaultIssueTest {
   @Test
   public void all_changes_contain_current_change() {
     IssueChangeContext issueChangeContext = mock(IssueChangeContext.class);
-    DefaultIssue issue = new DefaultIssue().setKey("AAA").setFieldChange(issueChangeContext, "actionPlan", "1.0", "1.1");
+    when(issueChangeContext.getExternalUser()).thenReturn("toto");
+    when(issueChangeContext.getWebhookSource()).thenReturn("github");
+
+    DefaultIssue issue = new DefaultIssue()
+      .setKey("AAA")
+      .setFieldChange(issueChangeContext, "actionPlan", "1.0", "1.1");
 
     assertThat(issue.changes()).hasSize(1);
+    FieldDiffs actualDiffs = issue.changes().iterator().next();
+    assertThat(actualDiffs.externalUser()).contains(issueChangeContext.getExternalUser());
+    assertThat(actualDiffs.webhookSource()).contains(issueChangeContext.getWebhookSource());
   }
 
   @Test
index dd2cb1d362ee01cb215d29da961ead64e1d74a13..330fe78c0c62cc417dd2349cdf5169d97095f376 100644 (file)
@@ -123,10 +123,12 @@ public class FieldDiffsTest {
 
   @Test
   public void test_toString() {
+    diffs.setWebhookSource("github");
+    diffs.setExternalUser("toto");
     diffs.setDiff("severity", "BLOCKER", "INFO");
     diffs.setDiff("resolution", "OPEN", "FIXED");
 
-    assertThat(diffs.toString()).isEqualTo("severity=BLOCKER|INFO,resolution=OPEN|FIXED");
+    assertThat(diffs).hasToString("webhookSource=github,externalUser=toto,severity=BLOCKER|INFO,resolution=OPEN|FIXED");
   }
 
   @Test
@@ -142,14 +144,17 @@ public class FieldDiffsTest {
     diffs.setDiff("severity", null, "INFO");
     diffs.setDiff("assignee", "user1", null);
 
-    assertThat(diffs.toString()).isEqualTo("severity=INFO,assignee=user1|");
+    assertThat(diffs).hasToString("severity=INFO,assignee=user1|");
   }
 
   @Test
   public void test_parse() {
-    diffs = FieldDiffs.parse("severity=BLOCKER|INFO,resolution=OPEN|FIXED,donut=|new,gambas=miam,acme=old|");
+    diffs = FieldDiffs.parse("severity=BLOCKER|INFO,webhookSource=github,resolution=OPEN|FIXED,donut=|new,gambas=miam,acme=old|,externalUser=charlie");
     assertThat(diffs.diffs()).hasSize(5);
 
+    assertThat(diffs.webhookSource()).contains("github");
+    assertThat(diffs.externalUser()).contains("charlie");
+
     FieldDiffs.Diff diff = diffs.diffs().get("severity");
     assertThat(diff.oldValue()).isEqualTo("BLOCKER");
     assertThat(diff.newValue()).isEqualTo("INFO");
@@ -187,7 +192,10 @@ public class FieldDiffsTest {
 
   @Test
   public void test_parse_empty_values() {
-    diffs = FieldDiffs.parse("severity=INFO,resolution=");
+    diffs = FieldDiffs.parse("severity=INFO,resolution=,webhookSource=,externalUser=");
+
+    assertThat(diffs.externalUser()).isEmpty();
+    assertThat(diffs.webhookSource()).isEmpty();
     assertThat(diffs.diffs()).hasSize(2);
 
     FieldDiffs.Diff diff = diffs.diffs().get("severity");
index d5467d90002f0202ea40309a806e755c539dd118..dedc1b551dbd58f4a173e8d7ca7f9435882851a5 100644 (file)
@@ -32,6 +32,8 @@ public class IssueChangeContextTest {
 
   private static final Date NOW = new Date();
   private static final String USER_UUID = "user_uuid";
+  private static final String EXTERNAL_USER = "toto@tata.com";
+  private static final String WEBHOOK_SOURCE = "github";
 
   private IssueChangeContext context;
 
@@ -39,14 +41,14 @@ public class IssueChangeContextTest {
   public void test_issueChangeContextByScanBuilder() {
     context = issueChangeContextByScanBuilder(NOW).build();
 
-    verifyContext(null, true, false, false);
+    verifyContext(true, false, null, null, null);
   }
 
   @Test
   public void test_issueChangeContextByUserBuilder() {
     context = issueChangeContextByUserBuilder(NOW, USER_UUID).build();
 
-    verifyContext(USER_UUID, false, false, false);
+    verifyContext(false, false, USER_UUID, null, null);
   }
 
   @Test
@@ -56,16 +58,27 @@ public class IssueChangeContextTest {
       .withRefreshMeasures()
       .setUserUuid(USER_UUID)
       .setDate(NOW)
-      .withFromAlm()
+      .setExternalUser(EXTERNAL_USER)
+      .setWebhookSource(WEBHOOK_SOURCE)
       .build();
 
-    verifyContext(USER_UUID, true, true, true);
+    verifyContext(true, true, USER_UUID, EXTERNAL_USER, WEBHOOK_SOURCE);
   }
 
   @Test
   public void test_equal() {
-    context = IssueChangeContext.newBuilder().setUserUuid(USER_UUID).setDate(NOW).build();
-    IssueChangeContext equalContext = IssueChangeContext.newBuilder().setUserUuid(USER_UUID).setDate(NOW).build();
+    context = IssueChangeContext.newBuilder()
+      .setUserUuid(USER_UUID)
+      .setDate(NOW)
+      .setExternalUser(EXTERNAL_USER)
+      .setWebhookSource(WEBHOOK_SOURCE)
+      .build();
+    IssueChangeContext equalContext = IssueChangeContext.newBuilder()
+      .setUserUuid(USER_UUID)
+      .setDate(NOW)
+      .setExternalUser(EXTERNAL_USER)
+      .setWebhookSource(WEBHOOK_SOURCE)
+      .build();
     IssueChangeContext notEqualContext = IssueChangeContext.newBuilder().setUserUuid("other_user_uuid").setDate(NOW).build();
 
     assertThat(context).isEqualTo(context)
@@ -79,24 +92,17 @@ public class IssueChangeContextTest {
   public void test_hashCode() {
     context = IssueChangeContext.newBuilder().setUserUuid(USER_UUID).setDate(NOW).build();
 
-    assertThat(context.hashCode()).isEqualTo(Objects.hash(USER_UUID, NOW, false, false, false));
+    assertThat(context.hashCode()).isEqualTo(Objects.hash(USER_UUID, NOW, false, false, null, null));
   }
 
-  @Test
-  public void test_toString() {
-    context = IssueChangeContext.newBuilder().setUserUuid(USER_UUID).setDate(NOW).build();
-    String expected = "IssueChangeContext{userUuid='user_uuid', date=" + NOW + ", scan=false, refreshMeasures=false, fromAlm=false}";
-
-    assertThat(context).hasToString(expected);
-  }
-
-  private void verifyContext(@Nullable String userUuid, boolean scan, boolean refreshMeasures, boolean fromAlm) {
+  private void verifyContext(boolean scan, boolean refreshMeasures, @Nullable String userUuid, @Nullable String externalUser,
+    @Nullable String webhookSource) {
     assertThat(context.userUuid()).isEqualTo(userUuid);
     assertThat(context.date()).isEqualTo(NOW);
     assertThat(context.scan()).isEqualTo(scan);
     assertThat(context.refreshMeasures()).isEqualTo(refreshMeasures);
-    assertThat(context.fromAlm()).isEqualTo(fromAlm);
+    assertThat(context.getExternalUser()).isEqualTo(externalUser);
+    assertThat(context.getWebhookSource()).isEqualTo(webhookSource);
   }
 
-
 }
index c24c1d5d5e63dda028f6f5ee9d54107b560bf209..08e17ad049f48b5a2499fd87fd0cebd8be03f746 100644 (file)
@@ -129,6 +129,8 @@ message Changelog {
   repeated Diff diffs = 5;
   optional string avatar = 6;
   optional bool isUserActive = 7;
+  optional string externalUser = 8;
+  optional string webhookSource = 9;
 
   message Diff {
     optional string key = 1;