Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/main/java/fr/techad/sonar/GerritConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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());
Expand All @@ -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 + "]";
}
}
4 changes: 2 additions & 2 deletions src/main/java/fr/techad/sonar/GerritPostJob.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/fr/techad/sonar/PropertyKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/org/sonar/l10n/gerrit.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/org/sonar/l10n/gerrit_fr.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions src/test/java/fr/techad/sonar/GerritConfigurationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "changeid";
private final String REVISION_ID = "674ac754f91e64a0efb8087e59a176484bd534d1";
private GerritConfiguration gerritConfiguration;

Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -393,8 +393,8 @@ public void shouldInvalidateConfigurationWithBlankBranchName() {
}

@Test
public void shouldInvalidateConfigurationWithBlankChangeId() {
gerritConfiguration.setChangeId("");
public void shouldInvalidateConfigurationWithBlankChangeNumber() {
gerritConfiguration.setChangeNumber("");
gerritConfiguration.assertGerritConfiguration();
Assertions.assertFalse(gerritConfiguration.isValid());
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/fr/techad/sonar/GerritPostJobTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, "changeid")
.setProperty(PropertyKey.GERRIT_REVISION_ID, "revisionid")
.setProperty(PropertyKey.GERRIT_LABEL, "Code-Review");
}
Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -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()
Expand All @@ -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~changeid/revisions/revisionid/review")
.withMethod("POST"))
.respond(
response()
Expand All @@ -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, "changeid")
;

String response = getRestConnector().setReview("review");
Expand All @@ -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()
Expand All @@ -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()
Expand Down