Index files affected by merge commits
This commit has the effect of changing ChangeData.currentFilePaths()
to return the diff of a merge commit to be between the merge commit's
new tree, and its 1st parent. That allows ChangeField.PATH to index
the files affected by a merge commit, which supports the conflicts
operator to identify open merges that may conflict with a change.
The only caller of PatchListCache.getDiffSummary() is
ChangeData.currentFilePaths(). The only caller of
ChangeData.currentFilePaths() is ChangeField.PATH (for indexing)
and query operators that test against a change.
A side-effect of this commit is the changed lines (added, deleted,
delta) for merge commits will now report the delta introduced by the
merge, rather than the delta from the automerge.
The conflicts operator now detects a few less cases of conflicts. The
most obvious change is a merge uploaded to the wrong branch may not
show a conflict with other changes on its current branch when its 1st
parent (from the other branch) introduces a conflicting change.
The conflicts operator now assumes merges are made "correctly", such
that `git log --first-parent` will traverse the main-branch history,
and not the side-branch. This requires users to do something like:
git checkout -b my-merge origin/master
git merge side-branch
git push origin my-merge:refs/for/master
When creating a merge commit to merge "side-branch" into "master".
Since this is a typical workflow with git, most merges should already
fit this expected pattern.
A new change index schema version 46 is required to ensure changes are
reindexed with this new computation for the PATH field.
Bug: issue 3052
Change-Id: Iaa854447ed9abb77b3f8fe6c818723e804e0a8d4
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 634cea0..58634a5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -50,6 +50,7 @@
import com.google.gerrit.extensions.api.changes.ReviewResult;
import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.client.Comment.Range;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -92,7 +93,11 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListKey;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -129,6 +134,7 @@
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
@Singleton
public class PostReview
@@ -206,14 +212,14 @@
protected Response<ReviewResult> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource revision, ReviewInput input)
throws RestApiException, UpdateException, OrmException, IOException,
- PermissionBackendException, ConfigInvalidException {
+ PermissionBackendException, ConfigInvalidException, PatchListNotAvailableException {
return apply(updateFactory, revision, input, TimeUtil.nowTs());
}
public Response<ReviewResult> apply(
BatchUpdate.Factory updateFactory, RevisionResource revision, ReviewInput input, Timestamp ts)
throws RestApiException, UpdateException, OrmException, IOException,
- PermissionBackendException, ConfigInvalidException {
+ PermissionBackendException, ConfigInvalidException, PatchListNotAvailableException {
// Respect timestamp, but truncate at change created-on time.
ts = Ordering.natural().max(ts, revision.getChange().getCreatedOn());
if (revision.getEdit().isPresent()) {
@@ -553,7 +559,7 @@
private <T extends CommentInput> void checkComments(
RevisionResource revision, Map<String, List<T>> commentsPerPath)
- throws OrmException, BadRequestException {
+ throws BadRequestException, PatchListNotAvailableException {
Set<String> revisionFilePaths = getAffectedFilePaths(revision);
for (Map.Entry<String, List<T>> entry : commentsPerPath.entrySet()) {
String path = entry.getKey();
@@ -569,9 +575,14 @@
}
}
- private Set<String> getAffectedFilePaths(RevisionResource revision) throws OrmException {
- ChangeData changeData = changeDataFactory.create(db.get(), revision.getNotes());
- return new HashSet<>(changeData.filePaths(revision.getPatchSet()));
+ private Set<String> getAffectedFilePaths(RevisionResource revision)
+ throws PatchListNotAvailableException {
+ ObjectId newId = ObjectId.fromString(revision.getPatchSet().getRevision().get());
+ DiffSummaryKey key =
+ DiffSummaryKey.fromPatchListKey(
+ PatchListKey.againstDefaultBase(newId, Whitespace.IGNORE_NONE));
+ DiffSummary ds = patchListCache.getDiffSummary(key, revision.getProject());
+ return new HashSet<>(ds.getPaths());
}
private static void ensurePathRefersToAvailableOrMagicFile(
@@ -600,7 +611,7 @@
private void checkRobotComments(
RevisionResource revision, Map<String, List<RobotCommentInput>> in)
- throws BadRequestException, OrmException {
+ throws BadRequestException, PatchListNotAvailableException {
cleanUpComments(in);
for (Map.Entry<String, List<RobotCommentInput>> e : in.entrySet()) {
String commentPath = e.getKey();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
index 51e47a0..8ccf081 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
@@ -267,7 +267,7 @@
if (!cd.currentFilePaths().contains(AccountConfig.ACCOUNT_CONFIG)) {
return;
}
- } catch (OrmException e) {
+ } catch (IOException | OrmException e) {
log.error("Cannot validate account update", e);
throw new MergeValidationException("account validation unavailable");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index a1c7f14..24bde80 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -145,10 +145,13 @@
.buildRepeatable(cd -> firstNonNull(cd.currentFilePaths(), ImmutableList.of()));
public static Set<String> getFileParts(ChangeData cd) throws OrmException {
- List<String> paths = cd.currentFilePaths();
- if (paths == null) {
- return ImmutableSet.of();
+ List<String> paths;
+ try {
+ paths = cd.currentFilePaths();
+ } catch (IOException e) {
+ throw new OrmException(e);
}
+
Splitter s = Splitter.on('/').omitEmptyStrings();
Set<String> r = new HashSet<>();
for (String path : paths) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 129c8ac..41e8d10 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -86,7 +86,9 @@
ChangeField.PENDING_REVIEWER,
ChangeField.PENDING_REVIEWER_BY_EMAIL);
- static final Schema<ChangeData> V45 = schema(V44, ChangeField.REVERT_OF);
+ @Deprecated static final Schema<ChangeData> V45 = schema(V44, ChangeField.REVERT_OF);
+
+ static final Schema<ChangeData> V46 = schema(V45);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
index c32a3f6..728d227 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
@@ -30,9 +30,6 @@
IntraLineDiff getIntraLineDiff(IntraLineDiffKey key, IntraLineDiffArgs args);
- DiffSummary getDiffSummary(Change change, PatchSet patchSet)
- throws PatchListNotAvailableException;
-
DiffSummary getDiffSummary(DiffSummaryKey key, Project.NameKey project)
throws PatchListNotAvailableException;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index b985723..7777400 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -164,16 +164,6 @@
}
@Override
- public DiffSummary getDiffSummary(Change change, PatchSet patchSet)
- throws PatchListNotAvailableException {
- Project.NameKey project = change.getProject();
- ObjectId b = ObjectId.fromString(patchSet.getRevision().get());
- Whitespace ws = Whitespace.IGNORE_NONE;
- return getDiffSummary(
- DiffSummaryKey.fromPatchListKey(PatchListKey.againstDefaultBase(b, ws)), project);
- }
-
- @Override
public DiffSummary getDiffSummary(DiffSummaryKey key, Project.NameKey project)
throws PatchListNotAvailableException {
try {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index f8443ff..30bb760 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -35,6 +35,7 @@
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Account;
@@ -67,7 +68,9 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
@@ -367,8 +370,8 @@
private Collection<PatchSet> patchSets;
private ListMultimap<PatchSet.Id, PatchSetApproval> allApprovals;
private List<PatchSetApproval> currentApprovals;
- private Map<Integer, List<String>> files;
- private Map<Integer, Optional<DiffSummary>> diffSummaries;
+ private List<String> currentFiles;
+ private Optional<DiffSummary> diffSummary;
private Collection<Comment> publishedComments;
private Collection<RobotComment> robotComments;
private CurrentUser visibleTo;
@@ -391,6 +394,7 @@
private List<ReviewerStatusUpdate> reviewerUpdates;
private PersonIdent author;
private PersonIdent committer;
+ private int parentCount;
private Integer unresolvedCommentCount;
private LabelTypes labelTypes;
@@ -466,86 +470,61 @@
return allUsersName;
}
- private Map<Integer, List<String>> initFiles() {
- if (files == null) {
- files = new HashMap<>();
- }
- return files;
- }
-
public void setCurrentFilePaths(List<String> filePaths) throws OrmException {
PatchSet ps = currentPatchSet();
if (ps != null) {
- initFiles().put(ps.getPatchSetId(), ImmutableList.copyOf(filePaths));
+ currentFiles = ImmutableList.copyOf(filePaths);
}
}
- public List<String> currentFilePaths() throws OrmException {
- PatchSet ps = currentPatchSet();
- return ps != null ? filePaths(ps) : null;
- }
-
- public List<String> filePaths(PatchSet ps) throws OrmException {
- Integer psId = ps.getPatchSetId();
- List<String> r = initFiles().get(psId);
- if (r == null) {
- Change c = change();
- if (c == null) {
- return null;
+ public List<String> currentFilePaths() throws IOException, OrmException {
+ if (currentFiles == null) {
+ if (!lazyLoad) {
+ return Collections.emptyList();
}
-
- Optional<DiffSummary> p = getDiffSummary(c, ps);
- if (!p.isPresent()) {
- List<String> emptyFileList = Collections.emptyList();
- if (lazyLoad) {
- files.put(ps.getPatchSetId(), emptyFileList);
- }
- return emptyFileList;
- }
-
- r = p.get().getPaths();
- files.put(psId, r);
+ Optional<DiffSummary> p = getDiffSummary();
+ currentFiles = p.map(DiffSummary::getPaths).orElse(Collections.emptyList());
}
- return r;
+ return currentFiles;
}
- private Optional<DiffSummary> getDiffSummary(Change c, PatchSet ps) {
- Integer psId = ps.getId().get();
- if (diffSummaries == null) {
- diffSummaries = new HashMap<>();
- }
- Optional<DiffSummary> r = diffSummaries.get(psId);
- if (r == null) {
+ private Optional<DiffSummary> getDiffSummary() throws OrmException, IOException {
+ if (diffSummary == null) {
if (!lazyLoad) {
return Optional.empty();
}
- try {
- r = Optional.of(patchListCache.getDiffSummary(c, ps));
- } catch (PatchListNotAvailableException e) {
- r = Optional.empty();
+
+ Change c = change();
+ PatchSet ps = currentPatchSet();
+ if (c == null || ps == null || !loadCommitData()) {
+ return Optional.empty();
}
- diffSummaries.put(psId, r);
+
+ ObjectId id = ObjectId.fromString(ps.getRevision().get());
+ Whitespace ws = Whitespace.IGNORE_NONE;
+ PatchListKey pk =
+ parentCount > 1
+ ? PatchListKey.againstParentNum(1, id, ws)
+ : PatchListKey.againstDefaultBase(id, ws);
+ DiffSummaryKey key = DiffSummaryKey.fromPatchListKey(pk);
+ try {
+ diffSummary = Optional.of(patchListCache.getDiffSummary(key, c.getProject()));
+ } catch (PatchListNotAvailableException e) {
+ diffSummary = Optional.empty();
+ }
}
- return r;
+ return diffSummary;
}
- private Optional<ChangedLines> computeChangedLines() throws OrmException {
- Change c = change();
- if (c == null) {
- return Optional.empty();
- }
- PatchSet ps = currentPatchSet();
- if (ps == null) {
- return Optional.empty();
- }
- Optional<DiffSummary> ds = getDiffSummary(c, ps);
+ private Optional<ChangedLines> computeChangedLines() throws OrmException, IOException {
+ Optional<DiffSummary> ds = getDiffSummary();
if (ds.isPresent()) {
return Optional.of(ds.get().getChangedLines());
}
return Optional.empty();
}
- public Optional<ChangedLines> changedLines() throws OrmException {
+ public Optional<ChangedLines> changedLines() throws OrmException, IOException {
if (changedLines == null) {
if (!lazyLoad) {
return Optional.empty();
@@ -744,6 +723,7 @@
commitFooters = c.getFooterLines();
author = c.getAuthorIdent();
committer = c.getCommitterIdent();
+ parentCount = c.getParentCount();
}
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index 7890edd..064e08e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -17,8 +17,8 @@
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.IntegrationException;
@@ -28,20 +28,15 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
import com.google.gwtorm.server.OrmException;
-import com.google.inject.Provider;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.revwalk.filter.RevFilter;
-import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.treewalk.filter.TreeFilter;
public class ConflictsPredicate {
// UI code may depend on this string, so use caution when changing.
@@ -51,9 +46,15 @@
public static Predicate<ChangeData> create(Arguments args, String value, Change c)
throws QueryParseException, OrmException {
- ChangeDataCache changeDataCache =
- new ChangeDataCache(c, args.db, args.changeDataFactory, args.projectCache);
- List<String> files = listFiles(c, args, changeDataCache);
+ ChangeData cd;
+ List<String> files;
+ try {
+ cd = args.changeDataFactory.create(args.db.get(), c);
+ files = cd.currentFilePaths();
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+
if (3 + files.size() > args.indexConfig.maxTerms()) {
// Short-circuit with a nice error message if we exceed the index
// backend's term limit. This assumes that "conflicts:foo" is the entire
@@ -73,61 +74,29 @@
and.add(new RefPredicate(c.getDest().get()));
and.add(Predicate.not(new LegacyChangeIdPredicate(c.getId())));
and.add(Predicate.or(filePredicates));
+
+ ChangeDataCache changeDataCache = new ChangeDataCache(cd, args.projectCache);
and.add(new CheckConflict(ChangeQueryBuilder.FIELD_CONFLICTS, value, args, c, changeDataCache));
return Predicate.and(and);
}
- private static List<String> listFiles(Change c, Arguments args, ChangeDataCache changeDataCache)
- throws OrmException {
- try (Repository repo = args.repoManager.openRepository(c.getProject());
- RevWalk rw = new RevWalk(repo)) {
- RevCommit ps = rw.parseCommit(changeDataCache.getTestAgainst());
- if (ps.getParentCount() > 1) {
- String dest = c.getDest().get();
- Ref destBranch = repo.getRefDatabase().exactRef(dest);
- rw.setRevFilter(RevFilter.MERGE_BASE);
- rw.markStart(rw.parseCommit(destBranch.getObjectId()));
- rw.markStart(ps);
- RevCommit base = rw.next();
- // TODO(zivkov): handle the case with multiple merge bases
-
- List<String> files = new ArrayList<>();
- try (TreeWalk tw = new TreeWalk(repo)) {
- if (base != null) {
- tw.setFilter(TreeFilter.ANY_DIFF);
- tw.addTree(base.getTree());
- }
- tw.addTree(ps.getTree());
- tw.setRecursive(true);
- while (tw.next()) {
- files.add(tw.getPathString());
- }
- }
- return files;
- }
- return args.changeDataFactory.create(args.db.get(), c).currentFilePaths();
- } catch (IOException e) {
- throw new OrmException(e);
- }
- }
-
private static final class CheckConflict extends ChangeOperatorPredicate {
private final Arguments args;
- private final Change c;
+ private final Branch.NameKey dest;
private final ChangeDataCache changeDataCache;
CheckConflict(
String field, String value, Arguments args, Change c, ChangeDataCache changeDataCache) {
super(field, value);
this.args = args;
- this.c = c;
+ this.dest = c.getDest();
this.changeDataCache = changeDataCache;
}
@Override
public boolean match(ChangeData object) throws OrmException {
Change otherChange = object.change();
- if (otherChange == null || !otherChange.getDest().equals(c.getDest())) {
+ if (otherChange == null || !otherChange.getDest().equals(dest)) {
return false;
}
@@ -136,13 +105,17 @@
return false;
}
+ ProjectState projectState;
+ try {
+ projectState = changeDataCache.getProjectState();
+ } catch (NoSuchProjectException | OrmException e) {
+ return false;
+ }
+
ObjectId other = ObjectId.fromString(object.currentPatchSet().getRevision().get());
ConflictKey conflictsKey =
new ConflictKey(
- changeDataCache.getTestAgainst(),
- other,
- str.type,
- changeDataCache.getProjectState().isUseContentMerge());
+ changeDataCache.getTestAgainst(), other, str.type, projectState.isUseContentMerge());
Boolean conflicts = args.conflictsCache.getIfPresent(conflictsKey);
if (conflicts != null) {
return conflicts;
@@ -187,41 +160,31 @@
}
}
- public static class ChangeDataCache {
- protected final Change change;
- protected final Provider<ReviewDb> db;
- protected final ChangeData.Factory changeDataFactory;
- protected final ProjectCache projectCache;
+ private static class ChangeDataCache {
+ private final ChangeData cd;
+ private final ProjectCache projectCache;
- protected ObjectId testAgainst;
- protected ProjectState projectState;
- protected Set<ObjectId> alreadyAccepted;
+ private ObjectId testAgainst;
+ private ProjectState projectState;
+ private Set<ObjectId> alreadyAccepted;
- public ChangeDataCache(
- Change change,
- Provider<ReviewDb> db,
- ChangeData.Factory changeDataFactory,
- ProjectCache projectCache) {
- this.change = change;
- this.db = db;
- this.changeDataFactory = changeDataFactory;
+ ChangeDataCache(ChangeData cd, ProjectCache projectCache) {
+ this.cd = cd;
this.projectCache = projectCache;
}
- protected ObjectId getTestAgainst() throws OrmException {
+ ObjectId getTestAgainst() throws OrmException {
if (testAgainst == null) {
- testAgainst =
- ObjectId.fromString(
- changeDataFactory.create(db.get(), change).currentPatchSet().getRevision().get());
+ testAgainst = ObjectId.fromString(cd.currentPatchSet().getRevision().get());
}
return testAgainst;
}
- protected ProjectState getProjectState() {
+ ProjectState getProjectState() throws OrmException, NoSuchProjectException {
if (projectState == null) {
- projectState = projectCache.get(change.getProject());
+ projectState = projectCache.get(cd.project());
if (projectState == null) {
- throw new IllegalStateException(new NoSuchProjectException(change.getProject()));
+ throw new NoSuchProjectException(cd.project());
}
}
return projectState;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java
index 56ed797..fc00283 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsPathPredicate.java
@@ -16,6 +16,7 @@
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gwtorm.server.OrmException;
+import java.io.IOException;
import java.util.Collections;
import java.util.List;
@@ -26,8 +27,13 @@
@Override
public boolean match(ChangeData object) throws OrmException {
- List<String> files = object.currentFilePaths();
- return files != null && Collections.binarySearch(files, value) >= 0;
+ List<String> files;
+ try {
+ files = object.currentFilePaths();
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+ return Collections.binarySearch(files, value) >= 0;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
index ca21247..46b4cd5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/RegexPathPredicate.java
@@ -17,6 +17,7 @@
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.util.RegexListSearcher;
import com.google.gwtorm.server.OrmException;
+import java.io.IOException;
import java.util.List;
public class RegexPathPredicate extends ChangeRegexPredicate {
@@ -26,15 +27,13 @@
@Override
public boolean match(ChangeData object) throws OrmException {
- List<String> files = object.currentFilePaths();
- if (files != null) {
- return RegexListSearcher.ofStrings(getValue()).hasMatch(files);
+ List<String> files;
+ try {
+ files = object.currentFilePaths();
+ } catch (IOException e) {
+ throw new OrmException(e);
}
- // 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 RegexListSearcher.ofStrings(getValue()).hasMatch(files);
}
@Override
