Correctly throw error instead of NPE if topic is empty.
Change-Id: I0d7291bdd1dec690077026ee1498272f154a8b90
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
index 60a4818..6293fb0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/MergeValidator.java
@@ -109,6 +109,11 @@
for (String downstreamBranch : downstreamBranches) {
boolean dsExists = false;
QueryBuilder queryBuilder = new QueryBuilder();
+ if (upstreamChange.topic == null || upstreamChange.topic == "") {
+ // If topic is null or empty, we immediately know that downstream is missing.
+ missingDownstreamBranches.add(downstreamBranch);
+ continue;
+ }
queryBuilder.addParameter("topic", upstreamChange.topic);
queryBuilder.addParameter("branch", downstreamBranch);
queryBuilder.addParameter("status", "open");
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/QueryBuilder.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/QueryBuilder.java
index 2aa0ba9..9defd08 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/QueryBuilder.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/QueryBuilder.java
@@ -28,6 +28,9 @@
}
public void addParameter(String key, String value) throws InvalidQueryParameterException {
+ if (key == null || value == null) {
+ throw new InvalidQueryParameterException("Cannot use null value for key or value of query.");
+ }
if (value.contains("\"") && (value.contains("{") || value.contains("}"))) {
// Gerrit does not support search string escaping as of 5/16/2017
// see https://bugs.chromium.org/p/gerrit/issues/detail?id=5617
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
index cf42a56..b060b69 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
@@ -344,6 +345,54 @@
merge(result);
}
+ @Test
+ public void testTopicEditedListener_nullTopic() throws Exception {
+ Project.NameKey manifestNameKey = defaultSetup();
+ // Create initial change
+ PushOneCommit.Result result = createChange("subject", "filename", "content", "testtopic");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = result.getChange().project().get();
+ createBranch(new Branch.NameKey(projectName, "ds_one"));
+ createBranch(new Branch.NameKey(projectName, "ds_two"));
+ pushConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two");
+ // After we upload our config, we upload a new patchset to create the downstreams
+ amendChange(result.getChangeId());
+ result.assertOkStatus();
+ gApi.changes().id(result.getChangeId()).topic(null);
+ int changeNumber = result.getChange().getId().id;
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ "Failed to submit 1 change due to the following problems:\nChange "
+ + changeNumber
+ + ": Missing downstream branches ds_one, ds_two. Please recreate the automerges.");
+ // +2 and submit
+ merge(result);
+ }
+
+ @Test
+ public void testTopicEditedListener_emptyTopic() throws Exception {
+ Project.NameKey manifestNameKey = defaultSetup();
+ // Create initial change
+ PushOneCommit.Result result = createChange("subject", "filename", "content", "testtopic");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = result.getChange().project().get();
+ createBranch(new Branch.NameKey(projectName, "ds_one"));
+ createBranch(new Branch.NameKey(projectName, "ds_two"));
+ pushConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two");
+ // After we upload our config, we upload a new patchset to create the downstreams
+ amendChange(result.getChangeId());
+ result.assertOkStatus();
+ gApi.changes().id(result.getChangeId()).topic("");
+ int changeNumber = result.getChange().getId().id;
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ "Failed to submit 1 change due to the following problems:\nChange "
+ + changeNumber
+ + ": Missing downstream branches ds_one, ds_two. Please recreate the automerges.");
+ // +2 and submit
+ merge(result);
+ }
+
private Project.NameKey defaultSetup() throws Exception {
Project.NameKey manifestNameKey = createProject("platform/manifest");
setupTestRepo("default.xml", manifestNameKey, "master", "default.xml");
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java
index 54ff231..47b5492 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/MergeValidatorIT.java
@@ -69,6 +69,48 @@
}
@Test
+ public void testNoMissingDownstreamMerges_nullTopic() throws Exception {
+ // Create initial change
+ PushOneCommit.Result result = createChange("subject", "filename", "content");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = result.getChange().change().getProject().get();
+ createBranch(new Branch.NameKey(projectName, "ds_one"));
+ pushConfig("automerger.config", projectName, "ds_one");
+ // After we upload our config, we upload a new patchset to create the downstreams
+ amendChange(result.getChangeId());
+ result.assertOkStatus();
+ gApi.changes().id(result.getChangeId()).topic(null);
+ int changeNumber = result.getChange().getId().id;
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ "Failed to submit 1 change due to the following problems:\nChange "
+ + changeNumber
+ + ": Missing downstream branches ds_one. Please recreate the automerges.");
+ merge(result);
+ }
+
+ @Test
+ public void testNoMissingDownstreamMerges_emptyTopic() throws Exception {
+ // Create initial change
+ PushOneCommit.Result result = createChange("subject", "filename", "content");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = result.getChange().change().getProject().get();
+ createBranch(new Branch.NameKey(projectName, "ds_one"));
+ pushConfig("automerger.config", projectName, "ds_one");
+ // After we upload our config, we upload a new patchset to create the downstreams
+ amendChange(result.getChangeId());
+ result.assertOkStatus();
+ gApi.changes().id(result.getChangeId()).topic("");
+ int changeNumber = result.getChange().getId().id;
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ "Failed to submit 1 change due to the following problems:\nChange "
+ + changeNumber
+ + ": Missing downstream branches ds_one. Please recreate the automerges.");
+ merge(result);
+ }
+
+ @Test
public void testNoMissingDownstreamMerges_branchWithQuotes() throws Exception {
// Create initial change
PushOneCommit.Result result = createChange("subject", "filename", "content", "testtopic");
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/QueryBuilderTest.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/QueryBuilderTest.java
index 2179602..dd99802 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/QueryBuilderTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/QueryBuilderTest.java
@@ -37,6 +37,21 @@
}
@Test
+ public void nullTest() throws Exception {
+ queryBuilder.addParameter("status", "open");
+ exception.expect(InvalidQueryParameterException.class);
+ exception.expectMessage("Cannot use null value for key or value of query.");
+ queryBuilder.addParameter("topic", null);
+ }
+
+ @Test
+ public void emptyStringTest() throws Exception {
+ queryBuilder.addParameter("status", "open");
+ queryBuilder.addParameter("topic", "");
+ assertThat(queryBuilder.get()).isEqualTo("topic:\"\" status:\"open\"");
+ }
+
+ @Test
public void removeParameterTest() throws Exception {
queryBuilder.addParameter("status", "open");
queryBuilder.addParameter("branch", "master");
