opendev/gerrit
Code Issues Proposed changes

Support exact match on file parts in file: operator

Browse Source 
Keep the old "file" field around (using that name in the index for
backwards compatibility), but call it PATH in the code to reflect the
fact that it matches the full path. Add a new "filepart" repeated
field that splits the filename on '/' and adds each unique component
as a field value.

Use "path:" to search only the path, and "file:" to search either the
file or the full path. There is only support for single path
components or the full path, not arbitrary sequences of subcomponents.
The latter is possible but would result in combinatorial blowup of
terms. If users really want to search on that, they can use multiple
"file:" terms (discarding ordering) or use regex search.

Regex search is still over the entire path, but this is implemented at
the ChangeQueryBuilder level and may be changed in the future.

Change-Id: I4f6e06bc3c07989f96e4e74e64113f521c05b9e3
This commit is contained in:
Dave Borowitz
2013-12-27 11:38:01 -08:00
committed by Shawn Pearce
parent a76877cfb0
commit 64e0b0762f
12 changed files with 194 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
Documentation/user-search.txt
View File

@@ -182,8 +182,8 @@ comment:'TEXT'::
+
Changes that match 'TEXT' string in any comment left by a reviewer.
[[file]]
file:'PATH', f:'PATH'::
[[path]]
path:'PATH'::
+
Matches any change touching file at 'PATH'. By default exact path
matching is used, but regular expressions can be enabled by starting
@@ -202,6 +202,18 @@ ones using a bracket expression). For example, to match all XML
files named like 'name1.xml', 'name2.xml', and 'name3.xml' use
`file:"^name[1-3].xml"`.
[[file]]
file:'NAME', f:'NAME'::
+
Matches any change touching a file containing the path component
'NAME'. For example a `file:src` will match changes that modify
files named `gerrit-server/src/main/java/Foo.java`. Name matching
is exact match, `file:Foo.java` finds any change touching a file
named exactly `Foo.java` and does not match `AbstractFoo.java`.
+
Regular expression matching can be enabled by starting the string
with `^`. In this mode `file:` is an alias of `path:` (see above).
[[has]]
has:draft::
+

30
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.index;
import com.google.common.base.Splitter;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.Account;
@@ -160,10 +161,11 @@ public class ChangeField {
}
};
/** List of filenames modified in the current patch set. */
public static final FieldDef<ChangeData, Iterable<String>> FILE =
/** List of full file paths modified in the current patch set. */
public static final FieldDef<ChangeData, Iterable<String>> PATH =
new FieldDef.Repeatable<ChangeData, String>(
ChangeQueryBuilder.FIELD_FILE, FieldType.EXACT, false) {
// Named for backwards compatibility.
"file", FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
@@ -171,6 +173,28 @@ public class ChangeField {
}
};
public static Set<String> getFileParts(ChangeData cd) throws OrmException {
Splitter s = Splitter.on('/').omitEmptyStrings();
Set<String> r = Sets.newHashSet();
for (String path : cd.currentFilePaths()) {
for (String part : s.split(path)) {
r.add(part);
}
}
return r;
}
/** Components of each file path modified in the current patch set. */
public static final FieldDef<ChangeData, Iterable<String>> FILE_PART =
new FieldDef.Repeatable<ChangeData, String>(
"filepart", FieldType.EXACT, false) {
@Override
public Iterable<String> get(ChangeData input, FillArgs args)
throws OrmException {
return getFileParts(input);
}
};
/** Owner/creator of the change. */
public static final FieldDef<ChangeData, Integer> OWNER =
new FieldDef.Single<ChangeData, Integer>(

32
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
View File

@@ -40,7 +40,7 @@ public class ChangeSchemas {
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.LEGACY_SORTKEY,
ChangeField.FILE,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -60,7 +60,7 @@ public class ChangeSchemas {
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.LEGACY_SORTKEY,
ChangeField.FILE,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -81,7 +81,7 @@ public class ChangeSchemas {
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.SORTKEY,
ChangeField.FILE,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -105,7 +105,7 @@ public class ChangeSchemas {
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.SORTKEY,
ChangeField.FILE,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
@@ -121,6 +121,30 @@ public class ChangeSchemas {
// For upgrade to Lucene 4.6.0 index format only.
static final Schema<ChangeData> V6 = release(V5.getFields().values());
static final Schema<ChangeData> V7 = release(
ChangeField.LEGACY_ID,
ChangeField.ID,
ChangeField.STATUS,
ChangeField.PROJECT,
ChangeField.REF,
ChangeField.TOPIC,
ChangeField.UPDATED,
ChangeField.SORTKEY,
ChangeField.FILE_PART,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
ChangeField.TR,
ChangeField.LABEL,
ChangeField.REVIEWED,
ChangeField.COMMIT_MESSAGE,
ChangeField.COMMENT,
ChangeField.CHANGE,
ChangeField.APPROVAL,
ChangeField.MERGEABLE);
private static Schema<ChangeData> release(Collection<FieldDef<ChangeData, ?>> fields) {
return new Schema<ChangeData>(true, fields);
}

4
gerrit-server/src/main/java/com/google/gerrit/server/index/RegexPredicate.java
View File

@@ -18,4 +18,8 @@ public abstract class RegexPredicate<I> extends IndexPredicate<I> {
protected RegexPredicate(FieldDef<I, ?> def, String value) {
super(def, value);
}
protected RegexPredicate(FieldDef<I, ?> def, String name, String value) {
super(def, name, value);
}
}

14
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
View File

@@ -99,6 +99,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
public static final String FIELD_OWNER = "owner";
public static final String FIELD_OWNERIN = "ownerin";
public static final String FIELD_PARENTPROJECT = "parentproject";
public static final String FIELD_PATH = "path";
public static final String FIELD_PROJECT = "project";
public static final String FIELD_REF = "ref";
public static final String FIELD_REVIEWER = "reviewer";
@@ -384,9 +385,18 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
@Operator
public Predicate<ChangeData> file(String file) throws QueryParseException {
if (file.startsWith("^")) {
return new RegexFilePredicate(file);
return new RegexPathPredicate(FIELD_FILE, file);
} else {
return new EqualsFilePredicate(file);
return EqualsFilePredicate.create(args, file);
}
}
@Operator
public Predicate<ChangeData> path(String path) throws QueryParseException {
if (path.startsWith("^")) {
return new RegexPathPredicate(FIELD_PATH, path);
} else {
return new EqualsPathPredicate(FIELD_PATH, path);
}
}

3
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
View File

@@ -70,7 +70,8 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
List<Predicate<ChangeData>> filePredicates =
Lists.newArrayListWithCapacity(files.size());
for (String file : files) {
filePredicates.add(new EqualsFilePredicate(file));
filePredicates.add(
new EqualsPathPredicate(ChangeQueryBuilder.FIELD_PATH, file));
}
List<Predicate<ChangeData>> predicatesForOneChange =

30
gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsFilePredicate.java
View File

@@ -16,31 +16,31 @@ package com.google.gerrit.server.query.change;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
import com.google.gwtorm.server.OrmException;
import java.util.Collections;
import java.util.List;
class EqualsFilePredicate extends IndexPredicate<ChangeData> {
static Predicate<ChangeData> create(Arguments args, String value) {
Predicate<ChangeData> eqPath =
new EqualsPathPredicate(ChangeQueryBuilder.FIELD_FILE, value);
if (!args.indexes.getSearchIndex().getSchema().getFields().containsKey(
ChangeField.FILE_PART.getName())) {
return eqPath;
}
return Predicate.or(eqPath, new EqualsFilePredicate(value));
}
private final String value;
EqualsFilePredicate(String value) {
super(ChangeField.FILE, value);
private EqualsFilePredicate(String value) {
super(ChangeField.FILE_PART, ChangeQueryBuilder.FIELD_FILE, value);
this.value = value;
}
@Override
public boolean match(ChangeData object) throws OrmException {
List<String> files = object.currentFilePaths();
if (files != null) {
return Collections.binarySearch(files, value) >= 0;
} else {
// The ChangeData can't do expensive lookups right now. Bypass
// them and include the result anyway. We might be able to do
// a narrow later on to a smaller set.
//
return true;
}
return ChangeField.getFileParts(object).contains(value);
}
@Override

42
gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java Normal file
View File

@@ -0,0 +1,42 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.query.change;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gwtorm.server.OrmException;
import java.util.Collections;
import java.util.List;
class EqualsPathPredicate extends IndexPredicate<ChangeData> {
private final String value;
EqualsPathPredicate(String fieldName, String value) {
super(ChangeField.PATH, fieldName, value);
this.value = value;
}
@Override
public boolean match(ChangeData object) throws OrmException {
List<String> files = object.currentFilePaths();
return files != null && Collections.binarySearch(files, value) >= 0;
}
@Override
public int getCost() {
return 1;
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexFilePredicate.java → gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
View File

@@ -25,7 +25,7 @@ import dk.brics.automaton.RunAutomaton;
import java.util.Collections;
import java.util.List;
class RegexFilePredicate extends RegexPredicate<ChangeData> {
class RegexPathPredicate extends RegexPredicate<ChangeData> {
private final RunAutomaton pattern;
private final String prefixBegin;
@@ -33,8 +33,8 @@ class RegexFilePredicate extends RegexPredicate<ChangeData> {
private final int prefixLen;
private final boolean prefixOnly;
RegexFilePredicate(String re) {
super(ChangeField.FILE, re);
RegexPathPredicate(String fieldName, String re) {
super(ChangeField.PATH, re);
if (re.startsWith("^")) {
re = re.substring(1);

2
gerrit-server/src/test/java/com/google/gerrit/server/index/FakeIndex.java
View File

@@ -30,7 +30,7 @@ class FakeIndex implements ChangeIndex {
static Schema<ChangeData> V2 = new Schema<ChangeData>(2, false,
ImmutableList.of(
ChangeField.STATUS,
ChangeField.FILE,
ChangeField.PATH,
ChangeField.SORTKEY));
private static class Source implements ChangeDataSource {

42
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@@ -574,13 +574,16 @@ public abstract class AbstractQueryChangesTest {
TestRepository<InMemoryRepository> repo = createProject("repo");
RevCommit commit = repo.parseBody(
repo.commit().message("one")
.add("file1", "contents1").add("file2", "contents2")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
assertTrue(query("file:file").isEmpty());
assertResultEquals(change, queryOne("file:dir"));
assertResultEquals(change, queryOne("file:file1"));
assertResultEquals(change, queryOne("file:file2"));
assertResultEquals(change, queryOne("file:dir/file1"));
assertResultEquals(change, queryOne("file:dir/file2"));
}
@Test
@@ -588,12 +591,43 @@ public abstract class AbstractQueryChangesTest {
TestRepository<InMemoryRepository> repo = createProject("repo");
RevCommit commit = repo.parseBody(
repo.commit().message("one")
.add("file1", "contents1").add("file2", "contents2")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
assertTrue(query("file:file.*").isEmpty());
assertResultEquals(change, queryOne("file:^file.*"));
assertTrue(query("file:.*file.*").isEmpty());
assertTrue(query("file:^file.*").isEmpty()); // Whole path only.
assertResultEquals(change, queryOne("file:^dir.file.*"));
}
@Test
public void byPathExact() throws Exception {
TestRepository<InMemoryRepository> repo = createProject("repo");
RevCommit commit = repo.parseBody(
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
assertTrue(query("path:file").isEmpty());
assertTrue(query("path:dir").isEmpty());
assertTrue(query("path:file1").isEmpty());
assertTrue(query("path:file2").isEmpty());
assertResultEquals(change, queryOne("path:dir/file1"));
assertResultEquals(change, queryOne("path:dir/file2"));
}
@Test
public void byPathRegex() throws Exception {
TestRepository<InMemoryRepository> repo = createProject("repo");
RevCommit commit = repo.parseBody(
repo.commit().message("one")
.add("dir/file1", "contents1").add("dir/file2", "contents2")
.create());
Change change = newChange(repo, commit, null, null, null).insert();
assertTrue(query("path:.*file.*").isEmpty());
assertResultEquals(change, queryOne("path:^dir.file.*"));
}
@Test

16
gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexFilePredicateTest.java → gerrit-server/src/test/java/com/google/gerrit/server/query/change/RegexPathPredicateTest.java
View File

@@ -23,10 +23,10 @@ import java.util.Arrays;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
public class RegexFilePredicateTest {
public class RegexPathPredicateTest {
@Test
public void testPrefixOnlyOptimization() throws OrmException {
RegexFilePredicate p = predicate("^a/b/.*");
RegexPathPredicate p = predicate("^a/b/.*");
assertTrue(p.match(change("a/b/source.c")));
assertFalse(p.match(change("source.c")));
@@ -36,7 +36,7 @@ public class RegexFilePredicateTest {
@Test
public void testPrefixReducesSearchSpace() throws OrmException {
RegexFilePredicate p = predicate("^a/b/.*\\.[ch]");
RegexPathPredicate p = predicate("^a/b/.*\\.[ch]");
assertTrue(p.match(change("a/b/source.c")));
assertFalse(p.match(change("a/b/source.res")));
assertFalse(p.match(change("source.res")));
@@ -46,7 +46,7 @@ public class RegexFilePredicateTest {
@Test
public void testFileExtension_Constant() throws OrmException {
RegexFilePredicate p = predicate("^.*\\.res");
RegexPathPredicate p = predicate("^.*\\.res");
assertTrue(p.match(change("test.res")));
assertTrue(p.match(change("foo/bar/test.res")));
assertFalse(p.match(change("test.res.bar")));
@@ -54,7 +54,7 @@ public class RegexFilePredicateTest {
@Test
public void testFileExtension_CharacterGroup() throws OrmException {
RegexFilePredicate p = predicate("^.*\\.[ch]");
RegexPathPredicate p = predicate("^.*\\.[ch]");
assertTrue(p.match(change("test.c")));
assertTrue(p.match(change("test.h")));
assertFalse(p.match(change("test.C")));
@@ -71,14 +71,14 @@ public class RegexFilePredicateTest {
@Test
public void testExactMatch() throws OrmException {
RegexFilePredicate p = predicate("^foo.c");
RegexPathPredicate p = predicate("^foo.c");
assertTrue(p.match(change("foo.c")));
assertFalse(p.match(change("foo.cc")));
assertFalse(p.match(change("bar.c")));
}
private static RegexFilePredicate predicate(String pattern) {
return new RegexFilePredicate(pattern);
private static RegexPathPredicate predicate(String pattern) {
return new RegexPathPredicate(ChangeQueryBuilder.FIELD_PATH, pattern);
}
private static ChangeData change(String... files) {