Support change~branch~id in query syntax

It is odd that we describe this in the REST API as a canonical way of
referring to a change, but don't support it in the query syntax. Since
the query parser is also used by the /r handler, this fixes that there
as well.

Unlike in the ChangesCollection parser, allow prefix searches for the
id portion, to be consistent with other search operators.

Change-Id: I55e1cc33caf907cb0ff17dae7a81a46156b6f562
This commit is contained in:
Dave Borowitz 2014-12-10 08:19:32 -08:00
parent 665e7bda73
commit b364901445
3 changed files with 48 additions and 7 deletions

View File

@ -188,6 +188,6 @@ fragment NON_WORD
| '?'
| '[' | ']'
| '{' | '}'
| '~'
// | '~' permit
)
;

View File

@ -15,6 +15,7 @@
package com.google.gerrit.server.query.change;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GroupReference;
@ -30,6 +31,7 @@ import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupBackends;
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters;
@ -71,8 +73,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
private static final Pattern PAT_LEGACY_ID = Pattern.compile("^[1-9][0-9]*$");
private static final Pattern PAT_CHANGE_ID =
Pattern.compile("^[iI][0-9a-f]{4,}.*$");
private static final Pattern DEF_CHANGE =
Pattern.compile("^([1-9][0-9]*|[iI][0-9a-f]{4,}.*)$");
private static final Pattern DEF_CHANGE = Pattern.compile(
"^(?:[1-9][0-9]*|(?:[^~]+~[^~]+~)?[iI][0-9a-f]{4,}.*)$");
// NOTE: As new search operations are added, please keep the
// SearchSuggestOracle up to date.
@ -260,15 +262,21 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
}
@Operator
public Predicate<ChangeData> change(String query) {
public Predicate<ChangeData> change(String query) throws QueryParseException {
if (PAT_LEGACY_ID.matcher(query).matches()) {
return new LegacyChangeIdPredicate(args, Change.Id.parse(query));
} else if (PAT_CHANGE_ID.matcher(query).matches()) {
return new ChangeIdPredicate(args, parseChangeId(query));
}
Optional<ChangeTriplet> triplet = ChangeTriplet.parse(query);
if (triplet.isPresent()) {
return Predicate.and(
project(triplet.get().project().get()),
branch(triplet.get().branch().get()),
new ChangeIdPredicate(args, parseChangeId(triplet.get().id().get())));
}
throw new IllegalArgumentException();
throw new QueryParseException("Invalid change format");
}
@Operator
@ -713,7 +721,11 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
if (query.startsWith("refs/")) {
return ref(query);
} else if (DEF_CHANGE.matcher(query).matches()) {
return change(query);
try {
return change(query);
} catch (QueryParseException e) {
// Skip.
}
}
List<Predicate<ChangeData>> predicates = Lists.newArrayListWithCapacity(9);

View File

@ -50,6 +50,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeTriplet;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
@ -206,6 +207,33 @@ public abstract class AbstractQueryChangesTest {
}
}
@Test
public void byTriplet() throws Exception {
TestRepository<InMemoryRepository> repo = createProject("repo");
Change change = newChange(repo, null, null, null, "branch").insert();
String k = change.getKey().get();
assertResultEquals(change, queryOne("repo~branch~" + k));
assertResultEquals(change, queryOne("change:repo~branch~" + k));
assertResultEquals(change, queryOne("repo~refs/heads/branch~" + k));
assertResultEquals(change, queryOne("change:repo~refs/heads/branch~" + k));
assertResultEquals(change, queryOne("repo~branch~" + k.substring(0, 10)));
assertResultEquals(change,
queryOne("change:repo~branch~" + k.substring(0, 10)));
assertThat(query("foo~bar")).isEmpty();
assertBadQuery("change:foo~bar");
assertThat(query("otherrepo~branch~" + k)).isEmpty();
assertThat(query("change:otherrepo~branch~" + k)).isEmpty();
assertThat(query("repo~otherbranch~" + k)).isEmpty();
assertThat(query("change:repo~otherbranch~" + k)).isEmpty();
assertThat(query("repo~branch~I0000000000000000000000000000000000000000"))
.isEmpty();
assertThat(query(
"change:repo~branch~I0000000000000000000000000000000000000000"))
.isEmpty();
}
@Test
public void byStatus() throws Exception {
TestRepository<InMemoryRepository> repo = createProject("repo");
@ -990,6 +1018,7 @@ public abstract class AbstractQueryChangesTest {
assertResultEquals(change1,
queryOne(Integer.toString(change1.getId().get())));
assertResultEquals(change1, queryOne(ChangeTriplet.format(change1)));
assertResultEquals(change2, queryOne("foosubject"));
assertResultEquals(change3, queryOne("Foo.java"));
assertResultEquals(change4, queryOne("Code-Review+1"));