use <project>~<numericId> as change id in requests

mildis · mildis · commit 43d8ec8a2a3d · 2019-06-21T13:43:35.000+02:00
Change-Id: I64b2314f4e33e6de1dd3a7a98347482af460f120
diff --git a/src/main/java/fr/techad/sonar/GerritConfiguration.java b/src/main/java/fr/techad/sonar/GerritConfiguration.java
@@ -39,7 +39,7 @@ public class GerritConfiguration {
 
     private String projectName;
     private String branchName;
-    private String changeId;
+    private String changeNumber;
     private String revisionId;
 
     public GerritConfiguration(Settings settings) {
@@ -70,7 +70,7 @@ public GerritConfiguration(Settings settings) {
 
         this.setProjectName(settings.getString(PropertyKey.GERRIT_PROJECT));
         this.setBranchName(settings.getString(PropertyKey.GERRIT_BRANCH));
-        this.setChangeId(settings.getString(PropertyKey.GERRIT_CHANGE_ID));
+        this.setChangeNumber(settings.getString(PropertyKey.GERRIT_CHANGE_NUMBER));
         this.setRevisionId(settings.getString(PropertyKey.GERRIT_REVISION_ID));
 
         this.assertGerritConfiguration();
@@ -88,9 +88,9 @@ public GerritConfiguration enable(boolean serverEnabled) {
 
     public boolean isEnabled() {
         boolean ret = enabled;
-        if (StringUtils.isEmpty(changeId) || StringUtils.isEmpty(revisionId)) {
+        if (StringUtils.isEmpty(changeNumber) || StringUtils.isEmpty(revisionId)) {
             LOG.info(
-                "[GERRIT PLUGIN] changeId or revisionId is empty. Not called from Gerrit ? Soft-disabling myself.");
+                "[GERRIT PLUGIN] changeNumber or revisionId is empty. Not called from Gerrit ? Soft-disabling myself.");
             ret = false;
         }
         return ret;
@@ -292,12 +292,12 @@ public GerritConfiguration setBranchName(String branchName) {
         return this;
     }
 
-    public String getChangeId() {
-        return changeId;
+    public String getChangeNumber() {
+        return changeNumber;
     }
 
-    public GerritConfiguration setChangeId(String changeId) {
-        this.changeId = changeId;
+    public GerritConfiguration setChangeNumber(String changeNumber) {
+        this.changeNumber = changeNumber;
         return this;
     }
 
@@ -337,7 +337,7 @@ void assertGerritConfiguration() {
         }
 
         if (StringUtils.isBlank(label) || StringUtils.isBlank(projectName) || StringUtils.isBlank(branchName)
-            || StringUtils.isBlank(changeId) || StringUtils.isBlank(revisionId)) {
+            || StringUtils.isBlank(changeNumber) || StringUtils.isBlank(revisionId)) {
             valid = false;
             if (isEnabled() || LOG.isDebugEnabled()) {
                 LOG.error("[GERRIT PLUGIN] ReviewConfiguration is not valid : {}", this.toString());
@@ -356,6 +356,6 @@ public String toString() {
             + ", issueComment=" + issueComment + ", threshold=" + threshold + ", voteNoIssue=" + voteNoIssue
             + ",voteBelowThreshold=" + voteBelowThreshold + ",voteAboveThreshold=" + voteAboveThreshold
             + ",commentNewIssuesOnly=" + commentNewIssuesOnly + ", projectName=" + projectName + ", branchName="
-            + branchName + ", changeId=" + changeId + ", revisionId=" + revisionId + "]";
+            + branchName + ", changeNumber=" + changeNumber + ", revisionId=" + revisionId + "]";
     }
 }
diff --git a/src/main/java/fr/techad/sonar/GerritPostJob.java b/src/main/java/fr/techad/sonar/GerritPostJob.java
@@ -43,7 +43,7 @@ public GerritPostJob(Settings settings, GerritConfiguration gerritConfiguration,
     @Override
     public void describe(PostJobDescriptor descriptor) {
         descriptor.name("GERRIT PLUGIN");
-        descriptor.requireProperty(PropertyKey.GERRIT_CHANGE_ID);
+        descriptor.requireProperty(PropertyKey.GERRIT_CHANGE_NUMBER);
     }
 
     @Override
@@ -98,7 +98,7 @@ public void execute(PostJobContext postJobContext) {
                     gerritConfiguration.getLabel());
             }
 
-            LOG.debug("[GERRIT PLUGIN] Send review for ChangeId={}, RevisionId={}", gerritConfiguration.getChangeId(),
+            LOG.debug("[GERRIT PLUGIN] Send review for ChangeNumber={}, RevisionId={}", gerritConfiguration.getChangeNumber(),
                 gerritConfiguration.getRevisionId());
 
             gerritFacade.setReview(reviewInput);
diff --git a/src/main/java/fr/techad/sonar/PropertyKey.java b/src/main/java/fr/techad/sonar/PropertyKey.java
@@ -7,7 +7,7 @@ public final class PropertyKey {
     public static final String GERRIT_PORT = "GERRIT_PORT";
     public static final String GERRIT_PROJECT = "GERRIT_PROJECT";
     public static final String GERRIT_BRANCH = "GERRIT_BRANCH";
-    public static final String GERRIT_CHANGE_ID = "GERRIT_CHANGE_ID";
+    public static final String GERRIT_CHANGE_NUMBER = "GERRIT_CHANGE_NUMBER";
     public static final String GERRIT_REVISION_ID = "GERRIT_PATCHSET_REVISION";
     public static final String GERRIT_USERNAME = "GERRIT_USERNAME";
     public static final String GERRIT_PASSWORD = "GERRIT_PASSWORD"; // NOSONAR
diff --git a/src/main/java/fr/techad/sonar/gerrit/network/rest/GerritRestConnector.java b/src/main/java/fr/techad/sonar/gerrit/network/rest/GerritRestConnector.java
@@ -34,7 +34,7 @@
 public class GerritRestConnector implements GerritConnector {
     private static final Logger LOG = Loggers.get(GerritRestConnector.class);
     private static final String URI_AUTH_PREFIX = "/a";
-    private static final String URI_CHANGES = "/changes/%s~%s~%s";
+    private static final String URI_CHANGES = "/changes/%s~%s";
     private static final String URI_REVISIONS = "/revisions/%s";
     private static final String URI_LIST_FILES_SUFFIX = "/files/";
     private static final String URI_SET_REVIEW = "/review";
@@ -161,7 +161,7 @@ public String rootUriBuilder() {
             uri = uri.concat(URI_AUTH_PREFIX);
         }
         uri = uri.concat(String.format(URI_CHANGES, encode(gerritConfiguration.getProjectName()),
-            encode(gerritConfiguration.getBranchName()), encode(gerritConfiguration.getChangeId())));
+            encode(gerritConfiguration.getChangeNumber())));
         uri = uri.concat(String.format(URI_REVISIONS, encode(gerritConfiguration.getRevisionId())));
 
         LOG.debug("[GERRIT PLUGIN] Built URI : {}", uri);
diff --git a/src/main/java/fr/techad/sonar/gerrit/network/ssh/GerritSshConnector.java b/src/main/java/fr/techad/sonar/gerrit/network/ssh/GerritSshConnector.java
@@ -20,7 +20,7 @@
 
 public class GerritSshConnector implements GerritConnector {
     private static final Logger LOG = Loggers.get(GerritSshConnector.class);
-    private static final String CMD_LIST_FILES = "gerrit query --format=JSON --files --current-patch-set status:open change:%s limit:1";
+    private static final String CMD_LIST_FILES = "gerrit query --format=JSON --files --current-patch-set -- status:open change:%s limit:1";
     private static final String CMD_SET_REVIEW = "gerrit review %s -j";
     private static final String SSH_KNWON_HOSTS = ".ssh/known_hosts";
     private static final String SSH_STRICT_NO = "StrictHostKeyChecking=no";
@@ -41,9 +41,9 @@ public String listFiles() throws IOException {
         SshClient sshClient = getSshClient();
 
         LOG.debug("[GERRIT PLUGIN] Execute command SSH {}",
-            String.format(CMD_LIST_FILES, gerritConfiguration.getChangeId()));
+            String.format(CMD_LIST_FILES, gerritConfiguration.getChangeNumber()));
 
-        Result cmdResult = sshClient.executeCommand(String.format(CMD_LIST_FILES, gerritConfiguration.getChangeId()),
+        Result cmdResult = sshClient.executeCommand(String.format(CMD_LIST_FILES, gerritConfiguration.getChangeNumber()),
             userAtHost);
 
         return cmdResult.stdoutAsText("UTF-8");
diff --git a/src/main/resources/org/sonar/l10n/gerrit.properties b/src/main/resources/org/sonar/l10n/gerrit.properties
@@ -28,8 +28,8 @@ property.GERRIT_MESSAGE.name=Review message
 property.GERRIT_MESSAGE.description=Define the message which will be added in comment
 property.GERRIT_ISSUE_COMMENT.name=Issue comment
 property.GERRIT_ISSUE_COMMENT.description=Define the message which will be used to comment issues
-property.GERRIT_CHANGE_ID.name=Change ID
-property.GERRIT_CHANGE_ID.description=Leave blank to delegate the definition by gerrit
+property.GERRIT_CHANGE_NUMBER.name=Change number
+property.GERRIT_CHANGE_NUMBER.description=Leave blank to delegate the definition by gerrit
 property.GERRIT_REVISION_ID.name=Patchset Revision ID
 property.GERRIT_REVISION_ID.description=Leave blank to delegate the definition by gerrit
 property.GERRIT_THRESHOLD.name=Alert threshold
diff --git a/src/main/resources/org/sonar/l10n/gerrit_fr.properties b/src/main/resources/org/sonar/l10n/gerrit_fr.properties
@@ -28,8 +28,8 @@ property.GERRIT_MESSAGE.name=Message de r\u00e9vision
 property.GERRIT_MESSAGE.description=D\u00e9fini le message qui sera ajout\u00e9 en commentaire de la r\u00e9vision
 property.GERRIT_ISSUE_COMMENT.name=Commentaire des d\u00e9fauts
 property.GERRIT_ISSUE_COMMENT.description=D\u00e9fini le message qui sera ajout\u00e9 en commentaire des d\u00e9fauts
-property.GERRIT_CHANGE_ID.name=ID du changement
-property.GERRIT_CHANGE_ID.description=Ne rien renseigner pour laisser Gerrit g\u00e9rer
+property.GERRIT_CHANGE_NUMBER.name=Num\u00e9ro du changement
+property.GERRIT_CHANGE_NUMBER.description=Ne rien renseigner pour laisser Gerrit g\u00e9rer
 property.GERRIT_REVISION_ID.name=ID de la r\u00e9vision
 property.GERRIT_REVISION_ID.description=Ne rien renseigner pour laisser Gerrit g\u00e9rer
 property.GERRIT_THRESHOLD.name=Seuil des alertes
diff --git a/src/test/java/fr/techad/sonar/GerritConfigurationTest.java b/src/test/java/fr/techad/sonar/GerritConfigurationTest.java
@@ -16,7 +16,7 @@ public class GerritConfigurationTest {
     private final String LABEL = "Code-Review";
     private final String PROJECT = "example";
     private final String BRANCH = "example";
-    private final String CHANGE_ID = "I8473b95934b5732ac55d26311a706c9c2bde9940";
+    private final String CHANGE_NUMBER = "12345";
     private final String REVISION_ID = "674ac754f91e64a0efb8087e59a176484bd534d1";
     private GerritConfiguration gerritConfiguration;
 
@@ -27,7 +27,7 @@ public void setUp() {
             .setProperty(PropertyKey.GERRIT_PORT, PORT.toString()).setProperty(PropertyKey.GERRIT_USERNAME, USERNAME)
             .setProperty(PropertyKey.GERRIT_PASSWORD, PASSWORD).setProperty(PropertyKey.GERRIT_BASE_PATH, "")
             .setProperty(PropertyKey.GERRIT_PROJECT, PROJECT).setProperty(PropertyKey.GERRIT_BRANCH, BRANCH)
-            .setProperty(PropertyKey.GERRIT_CHANGE_ID, CHANGE_ID)
+            .setProperty(PropertyKey.GERRIT_CHANGE_NUMBER, CHANGE_NUMBER)
             .setProperty(PropertyKey.GERRIT_REVISION_ID, REVISION_ID)
             .setProperty(PropertyKey.GERRIT_LABEL, LABEL);
         gerritConfiguration = new GerritConfiguration(settings);
@@ -93,9 +93,9 @@ public void shouldNotValidateIfBranchNameIsBlank() throws GerritPluginException
     }
 
     @Test
-    public void shouldNotValidateIfChangeIdIsBlank() throws GerritPluginException {
+    public void shouldNotValidateIfChangeNumberIsBlank() throws GerritPluginException {
         // given
-        gerritConfiguration.setChangeId("");
+        gerritConfiguration.setChangeNumber("");
         // when
         gerritConfiguration.assertGerritConfiguration();
         // then
@@ -346,8 +346,8 @@ public void shouldGetBranchName() {
     }
 
     @Test
-    public void shouldGetChangeId() {
-        Assertions.assertEquals(CHANGE_ID, this.gerritConfiguration.getChangeId());
+    public void shouldGetChangeNumber() {
+        Assertions.assertEquals(CHANGE_NUMBER, this.gerritConfiguration.getChangeNumber());
     }
 
     @Test
@@ -393,8 +393,8 @@ public void shouldInvalidateConfigurationWithBlankBranchName() {
     }
 
     @Test
-    public void shouldInvalidateConfigurationWithBlankChangeId() {
-        gerritConfiguration.setChangeId("");
+    public void shouldInvalidateConfigurationWithBlankChangeNumber() {
+        gerritConfiguration.setChangeNumber("");
         gerritConfiguration.assertGerritConfiguration();
         Assertions.assertFalse(gerritConfiguration.isValid());
     }
diff --git a/src/test/java/fr/techad/sonar/GerritPostJobTest.java b/src/test/java/fr/techad/sonar/GerritPostJobTest.java
@@ -63,7 +63,7 @@ public void setUp() {
             .setProperty(PropertyKey.GERRIT_HOST, "localhost")
             .appendProperty(PropertyKey.GERRIT_PORT, "10800")
             .setProperty(PropertyKey.GERRIT_PROJECT, "project")
-            .setProperty(PropertyKey.GERRIT_CHANGE_ID, "changeid")
+            .setProperty(PropertyKey.GERRIT_CHANGE_NUMBER, "changenumber")
             .setProperty(PropertyKey.GERRIT_REVISION_ID, "revisionid")
             .setProperty(PropertyKey.GERRIT_VOTE_NO_ISSUE, "1")
             .setProperty(PropertyKey.GERRIT_VOTE_ISSUE_ABOVE_THRESHOLD, "-2")
@@ -87,7 +87,7 @@ void shouldDescribeTheJob() {
         verify(descriptor).requireProperty(describeStringCaptor.capture());
         String property = describeStringCaptor.getValue();
         Assertions.assertEquals("GERRIT PLUGIN", name);
-        Assertions.assertEquals(PropertyKey.GERRIT_CHANGE_ID, property);
+        Assertions.assertEquals(PropertyKey.GERRIT_CHANGE_NUMBER, property);
     }
 
     @Test
diff --git a/src/test/java/fr/techad/sonar/gerrit/network/rest/GerritRestConnectorTest.java b/src/test/java/fr/techad/sonar/gerrit/network/rest/GerritRestConnectorTest.java
@@ -65,7 +65,7 @@ public void setUp() {
             .setProperty(PropertyKey.GERRIT_HOST, "localhost")
             .appendProperty(PropertyKey.GERRIT_PORT, "10800")
             .setProperty(PropertyKey.GERRIT_PROJECT, "project")
-            .setProperty(PropertyKey.GERRIT_CHANGE_ID, "changeid")
+            .setProperty(PropertyKey.GERRIT_CHANGE_NUMBER, "changenumber")
             .setProperty(PropertyKey.GERRIT_REVISION_ID, "revisionid")
             .setProperty(PropertyKey.GERRIT_LABEL, "Code-Review");
     }
@@ -87,7 +87,7 @@ public void shouldAggregateBasicParamsWhenAuthenticated() throws GerritPluginExc
         GerritRestConnector gerritRestConnector = getRestConnector();
 
         // then
-        assertEquals("/a/changes/project~branch~changeid/revisions/revisionid",
+        assertEquals("/a/changes/project~changeid/revisions/revisionid",
             gerritRestConnector.rootUriBuilder());
     }
 
@@ -103,7 +103,7 @@ public void shouldEncodeBranchWithSlash() throws GerritPluginException {
         GerritRestConnector gerritRestConnector = getRestConnector();
 
         // then
-        assertEquals("/a/changes/project~branch%2Fsubbranch~changeid/revisions/revisionid",
+        assertEquals("/a/changes/project~changeid/revisions/revisionid",
             gerritRestConnector.rootUriBuilder());
     }
 
@@ -119,7 +119,7 @@ public void shouldPrependCustomBasePath() throws GerritPluginException {
         GerritRestConnector gerritRestConnector = getRestConnector();
 
         // then
-        assertEquals("/r/a/changes/project~branch%2Fsubbranch~changeid/revisions/revisionid",
+        assertEquals("/r/a/changes/project~changeid/revisions/revisionid",
             gerritRestConnector.rootUriBuilder());
     }
 
@@ -133,7 +133,7 @@ public void shouldAggregateBasicParamsWhenAnonymous() throws GerritPluginExcepti
         GerritRestConnector gerritRestConnector = getRestConnector();
 
         // then
-        assertEquals("/changes/project~branch%2Fsubbranch~changeid/revisions/revisionid",
+        assertEquals("/changes/project~changeid/revisions/revisionid",
             gerritRestConnector.rootUriBuilder());
     }
 
@@ -147,15 +147,15 @@ public void shouldPrependCustomBasePathWhenAnonymous() throws GerritPluginExcept
         GerritRestConnector gerritRestConnector = getRestConnector();
 
         // then
-        assertEquals("/r/changes/project~branch%2Fsubbranch~changeid/revisions/revisionid",
+        assertEquals("/r/changes/project~changeid/revisions/revisionid",
             gerritRestConnector.rootUriBuilder());
     }
 
     @Test
     public void shouldSetReview() throws IOException {
         mockServer.when(
             request()
-                .withPath("/a/changes/project~branch~changeid/revisions/revisionid/review")
+                .withPath("/a/changes/project~changeid/revisions/revisionid/review")
                 .withMethod("POST"))
             .respond(
                 response()
@@ -179,7 +179,7 @@ public void shouldSetReview() throws IOException {
     public void shouldSetReviewWithNullResponseBody() throws IOException {
         mockServer.when(
             request()
-                .withPath("/a/changes/project~branch~changeid2/revisions/revisionid/review")
+                .withPath("/a/changes/project~12345/revisions/revisionid/review")
                 .withMethod("POST"))
             .respond(
                 response()
@@ -193,7 +193,7 @@ public void shouldSetReviewWithNullResponseBody() throws IOException {
             .setProperty(PropertyKey.GERRIT_PASSWORD, "sonar")
             .appendProperty(PropertyKey.GERRIT_BASE_PATH, "")
             .setProperty(PropertyKey.GERRIT_BRANCH, "branch")
-            .setProperty(PropertyKey.GERRIT_CHANGE_ID, "changeid2")
+            .setProperty(PropertyKey.GERRIT_CHANGE_NUMBER, "12345")
         ;
 
         String response = getRestConnector().setReview("review");
@@ -204,7 +204,7 @@ public void shouldSetReviewWithNullResponseBody() throws IOException {
     public void shouldSetReviewAsAnonymous() throws IOException {
         mockServer.when(
             request()
-                .withPath("/changes/project~branch~changeid/revisions/revisionid/review")
+                .withPath("/changes/project~changeid/revisions/revisionid/review")
                 .withMethod("POST"))
             .respond(
                 response()
@@ -225,7 +225,7 @@ public void shouldSetReviewAsAnonymous() throws IOException {
     public void shouldListFiles() throws IOException {
         mockServer.when(
             request()
-                .withPath("/a/changes/project~branch~changeid/revisions/revisionid/files/")
+                .withPath("/a/changes/project~changeid/revisions/revisionid/files/")
                 .withMethod("GET"))
             .respond(
                 response()

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ public GerritPostJob(Settings settings, GerritConfiguration gerritConfiguration,`
`43`	`43`	`@Override`
`44`	`44`	`public void describe(PostJobDescriptor descriptor) {`
`45`	`45`	`descriptor.name("GERRIT PLUGIN");`
`46`		`- descriptor.requireProperty(PropertyKey.GERRIT_CHANGE_ID);`
	`46`	`+ descriptor.requireProperty(PropertyKey.GERRIT_CHANGE_NUMBER);`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`@Override`
`@@ -98,7 +98,7 @@ public void execute(PostJobContext postJobContext) {`
`98`	`98`	`gerritConfiguration.getLabel());`
`99`	`99`	`}`
`100`	`100`
`101`		`- LOG.debug("[GERRIT PLUGIN] Send review for ChangeId={}, RevisionId={}", gerritConfiguration.getChangeId(),`
	`101`	`+ LOG.debug("[GERRIT PLUGIN] Send review for ChangeNumber={}, RevisionId={}", gerritConfiguration.getChangeNumber(),`
`102`	`102`	`gerritConfiguration.getRevisionId());`
`103`	`103`
`104`	`104`	`gerritFacade.setReview(reviewInput);`