Make default search query more useful

Preserve the existing short-circuit behavior for change IDs. OR
together a laundry list of potentially useful predicates in the
default case.

We use the new projects operator for prefix searching on projects,
rather than the hand-rolled substring search previously only found in
this method.

Change-Id: I21bba55e5ff79cd5daa40dd4076f829399164244
This commit is contained in:
Dave Borowitz
2014-03-25 12:38:37 -07:00
parent 4aed07b7ce
commit 4c27b10dc5
4 changed files with 101 additions and 57 deletions

View File

@@ -43,11 +43,7 @@ text and let Gerrit figure out the meaning:
Operators act as restrictions on the search. As more operators
are added to the same query string, they further restrict the
returned results. Search can also be performed by typing only a
text with no operator. It will try to match a project name by
substring.
E.g. Searching for just "gerrit is:starred" will try to match a
project name by "gerrit" as substring.
text with no operator, which will match against a variety of fields.
[[age]]
age:'AGE'::

View File

@@ -22,8 +22,6 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -56,7 +54,6 @@ import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.lib.AbbreviatedObjectId;
import org.eclipse.jgit.lib.Config;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
@@ -75,14 +72,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
private static final Pattern DEF_CHANGE =
Pattern.compile("^([1-9][0-9]*|[iI][0-9a-f]{4,}.*)$");
private static final Pattern PAT_COMMIT =
Pattern.compile("^([0-9a-fA-F]{4," + RevId.LEN + "})$");
private static final Pattern PAT_EMAIL =
Pattern.compile("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+");
private static final Pattern PAT_LABEL =
Pattern.compile("^[a-zA-Z][a-zA-Z0-9]*((=|>=|<=)[+-]?|[+-])\\d+$");
// NOTE: As new search operations are added, please keep the
// SearchSuggestOracle up to date.
@@ -688,52 +677,58 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
}
@Override
protected Predicate<ChangeData> defaultField(String query)
throws QueryParseException {
protected Predicate<ChangeData> defaultField(String query) {
if (query.startsWith("refs/")) {
return ref(query);
} else if (DEF_CHANGE.matcher(query).matches()) {
return change(query);
} else if (PAT_COMMIT.matcher(query).matches()) {
return commit(query);
} else if (PAT_EMAIL.matcher(query).find()) {
try {
return Predicate.or(owner(query), reviewer(query));
} catch (OrmException err) {
throw error("Cannot lookup user", err);
}
} else if (PAT_LABEL.matcher(query).find()) {
try {
return label(query);
} catch (OrmException err) {
throw error("Cannot lookup user", err);
}
} else {
// Try to match a project name by substring query.
final List<ProjectPredicate> predicate =
new ArrayList<ProjectPredicate>();
for (Project.NameKey name : args.projectCache.all()) {
if (name.get().toLowerCase().contains(query.toLowerCase())) {
predicate.add(new ProjectPredicate(name.get()));
}
}
// If two or more projects contains "query" as substring create an
// OrPredicate holding predicates for all these projects, otherwise if
// only one contains that, return only that one predicate by itself.
if (predicate.size() == 1) {
return predicate.get(0);
} else if (predicate.size() > 1) {
return Predicate.or(predicate);
}
throw error("Unsupported query:" + query);
}
List<Predicate<ChangeData>> predicates = Lists.newArrayListWithCapacity(9);
try {
predicates.add(commit(query));
} catch (IllegalArgumentException e) {
// Skip.
}
try {
predicates.add(owner(query));
} catch (OrmException | QueryParseException e) {
// Skip.
}
try {
predicates.add(reviewer(query));
} catch (OrmException | QueryParseException e) {
// Skip.
}
try {
predicates.add(file(query));
} catch (QueryParseException e) {
// Skip.
}
try {
predicates.add(label(query));
} catch (OrmException | QueryParseException e) {
// Skip.
}
try {
predicates.add(message(query));
} catch (QueryParseException e) {
// Skip.
}
try {
predicates.add(comment(query));
} catch (QueryParseException e) {
// Skip.
}
try {
predicates.add(projects(query));
} catch (QueryParseException e) {
// Skip.
}
predicates.add(ref(query));
predicates.add(branch(query));
predicates.add(topic(query));
return Predicate.or(predicates);
}
private Set<Account.Id> parseAccount(String who)

View File

@@ -111,6 +111,9 @@ public abstract class AbstractQueryChangesTest {
schemaCreator.create(db);
userId = accountManager.authenticate(AuthRequest.forUser("user"))
.getAccountId();
Account userAccount = db.accounts().get(userId);
userAccount.setPreferredEmail("user@example.com");
db.accounts().update(ImmutableList.of(userAccount));
user = userFactory.create(userId);
requestContext.setContext(new RequestContext() {
@@ -805,6 +808,51 @@ public abstract class AbstractQueryChangesTest {
assertResultEquals(change1, results.get(1));
}
@Test
public void byDefault() throws Exception {
TestRepository<InMemoryRepository> repo = createProject("repo");
Change change1 = newChange(repo, null, null, null, null).insert();
RevCommit commit2 = repo.parseBody(
repo.commit().message("foosubject").create());
Change change2 = newChange(repo, commit2, null, null, null).insert();
RevCommit commit3 = repo.parseBody(
repo.commit()
.add("Foo.java", "foo contents")
.create());
Change change3 = newChange(repo, commit3, null, null, null).insert();
ChangeInserter ins4 = newChange(repo, null, null, null, null);
Change change4 = ins4.insert();
ReviewInput ri4 = new ReviewInput();
ri4.message = "toplevel";
ri4.labels = ImmutableMap.<String, Short> of("Code-Review", (short) 1);
postReview.apply(new RevisionResource(
changes.parse(change4.getId()), ins4.getPatchSet()), ri4);
ChangeInserter ins5 = newChange(repo, null, null, null, null);
Change change5 = ins5.getChange();
change5.setTopic("feature5");
ins5.insert();
Change change6 = newChange(repo, null, null, null, "branch6").insert();
assertResultEquals(change1,
queryOne(Integer.toString(change1.getId().get())));
assertResultEquals(change2, queryOne("foosubject"));
assertResultEquals(change3, queryOne("Foo.java"));
assertResultEquals(change4, queryOne("Code-Review+1"));
assertResultEquals(change4, queryOne("toplevel"));
assertResultEquals(change5, queryOne("feature5"));
assertResultEquals(change6, queryOne("branch6"));
assertResultEquals(change6, queryOne("refs/heads/branch6"));
assertEquals(6, query("user@example.com").size());
assertEquals(6, query("repo").size());
}
protected ChangeInserter newChange(
TestRepository<InMemoryRepository> repo,
@Nullable RevCommit commit, @Nullable String key, @Nullable Integer owner,

View File

@@ -49,6 +49,11 @@ public class LuceneQueryChangesV7Test extends AbstractQueryChangesTest {
@Override
@Test
public void byProjectPrefix() {}
@Ignore
@Override
@Test
public void byDefault() {}
// End tests for features not supported in V7.
@Test