Fix internal errors when 'destination:' refers to non-existing destination
When querying with the destination: predicate using a destination name
that does not exist, it is supposed to return an error:
"Unknown named destination"
However it was not, because of two problems:
- When the revision is null, VersionedAccountDestinations's call to
getPathInfos results in NPE due to dereferencing the revision.
- When no destinations exist, DestinationList.getDestinations returns
an empty list, but the ChangeQueryBuilder is only testing for null
Fix both of the bugs and add a test that querying "destination:" with
a non-existing destination results in the correct response.
Adding tests for existing destinations is not within the scope of this
change.
Change-Id: I7787a90a508f61ef19586f327c6be427a0870e69
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java
index 5116cfb..2bb4bb7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountDestinations.java
@@ -53,6 +53,9 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
+ if (revision == null) {
+ return;
+ }
String prefix = DestinationList.DIR_NAME + "/";
for (PathInfo p : getPathInfos(true)) {
if (p.fileMode == FileMode.REGULAR_FILE) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index e042b1d..17f52dc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -1047,7 +1047,7 @@
VersionedAccountDestinations d = VersionedAccountDestinations.forUser(self());
d.load(git);
Set<Branch.NameKey> destinations = d.getDestinationList().getDestinations(name);
- if (destinations != null) {
+ if (destinations != null && !destinations.isEmpty()) {
return new DestinationPredicate(destinations, name);
}
} catch (RepositoryNotFoundException e) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 933e695..e57fe1d 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -2105,6 +2105,16 @@
assertQuery("-assignee:" + user.getUserName(), change2);
}
+ @Test
+ public void userDestination() throws Exception {
+ TestRepository<Repo> repo = createProject("repo");
+ insert(repo, newChange(repo));
+
+ assertThatQueryException("destination:foo")
+ .hasMessageThat()
+ .isEqualTo("Unknown named destination: foo");
+ }
+
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
return newChange(repo, null, null, null, null);
}
