From 7de041d7194f219b48601636367b5fd512aff68c Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 22 Jul 2015 11:02:18 -0700 Subject: [PATCH] Don't wrap SHA-1s when searching for commits (Abbreviated)ObjectId.fromString would throw IllegalArgumentException if the value is not commit-shaped, which results in an ugly error shown to the user. Plus, it's extra unnecessary allocations. Instead, just pass the strings in verbatim. This might return zero results in case of a typo, but that's already the behavior of most other operators. Change-Id: I905a32a6d1f2524f9349f7a19815bca03b991125 --- .../query/change/ChangeQueryBuilder.java | 6 ++---- .../query/change/CommitPrefixPredicate.java | 20 +++++++------------ .../query/change/ExactCommitPredicate.java | 18 ++++++----------- .../query/change/InternalChangeQuery.java | 6 +++--- 4 files changed, 18 insertions(+), 32 deletions(-) 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 ce3dd8fc06..5210577735 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 @@ -61,10 +61,8 @@ import com.google.inject.util.Providers; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; -import org.eclipse.jgit.lib.AbbreviatedObjectId; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import java.io.IOException; @@ -434,9 +432,9 @@ public class ChangeQueryBuilder extends QueryBuilder { @Operator public Predicate commit(String id) { if (id.length() == Constants.OBJECT_ID_STRING_LENGTH) { - return new ExactCommitPredicate(ObjectId.fromString(id)); + return new ExactCommitPredicate(id); } - return new CommitPrefixPredicate(AbbreviatedObjectId.fromString(id)); + return new CommitPrefixPredicate(id); } @Operator diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitPrefixPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitPrefixPredicate.java index c10421f80f..596665174d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitPrefixPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/CommitPrefixPredicate.java @@ -19,25 +19,19 @@ import com.google.gerrit.server.index.ChangeField; import com.google.gerrit.server.index.IndexPredicate; import com.google.gwtorm.server.OrmException; -import org.eclipse.jgit.lib.AbbreviatedObjectId; -import org.eclipse.jgit.lib.ObjectId; - class CommitPrefixPredicate extends IndexPredicate { - private final AbbreviatedObjectId abbrevId; - - CommitPrefixPredicate(AbbreviatedObjectId id) { - super(ChangeField.COMMIT, id.name()); - this.abbrevId = id; + CommitPrefixPredicate(String prefix) { + super(ChangeField.COMMIT, prefix); } @Override public boolean match(final ChangeData object) throws OrmException { + String prefix = getValue().toLowerCase(); for (PatchSet p : object.patchSets()) { - if (p.getRevision() != null && p.getRevision().get() != null) { - final ObjectId id = ObjectId.fromString(p.getRevision().get()); - if (abbrevId.prefixCompare(id) == 0) { - return true; - } + if (p.getRevision() != null + && p.getRevision().get() != null + && p.getRevision().get().startsWith(prefix)) { + return true; } } return false; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitPredicate.java index b109febd47..0528a13963 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ExactCommitPredicate.java @@ -19,26 +19,20 @@ import com.google.gerrit.server.index.ChangeField; import com.google.gerrit.server.index.IndexPredicate; import com.google.gwtorm.server.OrmException; -import org.eclipse.jgit.lib.ObjectId; - import java.util.Objects; class ExactCommitPredicate extends IndexPredicate { - private final ObjectId id; - - ExactCommitPredicate(ObjectId id) { - super(ChangeField.COMMIT, id.name()); - this.id = id; + ExactCommitPredicate(String id) { + super(ChangeField.COMMIT, id); } @Override public boolean match(final ChangeData object) throws OrmException { - String idStr = id.name(); + String id = getValue().toLowerCase(); for (PatchSet p : object.patchSets()) { - if (p.getRevision() != null && p.getRevision().get() != null) { - if (Objects.equals(p.getRevision().get(), idStr)) { - return true; - } + if (p.getRevision() != null + && Objects.equals(p.getRevision().get(), id)) { + return true; } } return false; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java index 64a21bcbfb..4a612dc7c0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java @@ -66,7 +66,7 @@ public class InternalChangeQuery { return new ChangeStatusPredicate(status); } - private static Predicate commit(ObjectId id) { + private static Predicate commit(String id) { return new ExactCommitPredicate(id); } @@ -129,7 +129,7 @@ public class InternalChangeQuery { List hashes, int batchSize) throws OrmException { List> commits = new ArrayList<>(hashes.size()); for (String s : hashes) { - commits.add(commit(ObjectId.fromString(s))); + commits.add(commit(s)); } int numBatches = (hashes.size() / batchSize) + 1; List> queries = new ArrayList<>(numBatches); @@ -164,7 +164,7 @@ public class InternalChangeQuery { } public List byCommit(ObjectId id) throws OrmException { - return query(commit(id)); + return query(commit(id.name())); } public List byProjectGroups(Project.NameKey project,