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
This commit is contained in:
		@@ -61,10 +61,8 @@ import com.google.inject.util.Providers;
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
import org.eclipse.jgit.errors.ConfigInvalidException;
 | 
					import org.eclipse.jgit.errors.ConfigInvalidException;
 | 
				
			||||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
 | 
					import org.eclipse.jgit.errors.RepositoryNotFoundException;
 | 
				
			||||||
import org.eclipse.jgit.lib.AbbreviatedObjectId;
 | 
					 | 
				
			||||||
import org.eclipse.jgit.lib.Config;
 | 
					import org.eclipse.jgit.lib.Config;
 | 
				
			||||||
import org.eclipse.jgit.lib.Constants;
 | 
					import org.eclipse.jgit.lib.Constants;
 | 
				
			||||||
import org.eclipse.jgit.lib.ObjectId;
 | 
					 | 
				
			||||||
import org.eclipse.jgit.lib.Repository;
 | 
					import org.eclipse.jgit.lib.Repository;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
import java.io.IOException;
 | 
					import java.io.IOException;
 | 
				
			||||||
@@ -434,9 +432,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
 | 
				
			|||||||
  @Operator
 | 
					  @Operator
 | 
				
			||||||
  public Predicate<ChangeData> commit(String id) {
 | 
					  public Predicate<ChangeData> commit(String id) {
 | 
				
			||||||
    if (id.length() == Constants.OBJECT_ID_STRING_LENGTH) {
 | 
					    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
 | 
					  @Operator
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -19,27 +19,21 @@ import com.google.gerrit.server.index.ChangeField;
 | 
				
			|||||||
import com.google.gerrit.server.index.IndexPredicate;
 | 
					import com.google.gerrit.server.index.IndexPredicate;
 | 
				
			||||||
import com.google.gwtorm.server.OrmException;
 | 
					import com.google.gwtorm.server.OrmException;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
import org.eclipse.jgit.lib.AbbreviatedObjectId;
 | 
					 | 
				
			||||||
import org.eclipse.jgit.lib.ObjectId;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
class CommitPrefixPredicate extends IndexPredicate<ChangeData> {
 | 
					class CommitPrefixPredicate extends IndexPredicate<ChangeData> {
 | 
				
			||||||
  private final AbbreviatedObjectId abbrevId;
 | 
					  CommitPrefixPredicate(String prefix) {
 | 
				
			||||||
 | 
					    super(ChangeField.COMMIT, prefix);
 | 
				
			||||||
  CommitPrefixPredicate(AbbreviatedObjectId id) {
 | 
					 | 
				
			||||||
    super(ChangeField.COMMIT, id.name());
 | 
					 | 
				
			||||||
    this.abbrevId = id;
 | 
					 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  @Override
 | 
					  @Override
 | 
				
			||||||
  public boolean match(final ChangeData object) throws OrmException {
 | 
					  public boolean match(final ChangeData object) throws OrmException {
 | 
				
			||||||
 | 
					    String prefix = getValue().toLowerCase();
 | 
				
			||||||
    for (PatchSet p : object.patchSets()) {
 | 
					    for (PatchSet p : object.patchSets()) {
 | 
				
			||||||
      if (p.getRevision() != null && p.getRevision().get() != null) {
 | 
					      if (p.getRevision() != null
 | 
				
			||||||
        final ObjectId id = ObjectId.fromString(p.getRevision().get());
 | 
					          && p.getRevision().get() != null
 | 
				
			||||||
        if (abbrevId.prefixCompare(id) == 0) {
 | 
					          && p.getRevision().get().startsWith(prefix)) {
 | 
				
			||||||
        return true;
 | 
					        return true;
 | 
				
			||||||
      }
 | 
					      }
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
    }
 | 
					 | 
				
			||||||
    return false;
 | 
					    return false;
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -19,28 +19,22 @@ import com.google.gerrit.server.index.ChangeField;
 | 
				
			|||||||
import com.google.gerrit.server.index.IndexPredicate;
 | 
					import com.google.gerrit.server.index.IndexPredicate;
 | 
				
			||||||
import com.google.gwtorm.server.OrmException;
 | 
					import com.google.gwtorm.server.OrmException;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
import org.eclipse.jgit.lib.ObjectId;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
import java.util.Objects;
 | 
					import java.util.Objects;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class ExactCommitPredicate extends IndexPredicate<ChangeData> {
 | 
					class ExactCommitPredicate extends IndexPredicate<ChangeData> {
 | 
				
			||||||
  private final ObjectId id;
 | 
					  ExactCommitPredicate(String id) {
 | 
				
			||||||
 | 
					    super(ChangeField.COMMIT, id);
 | 
				
			||||||
  ExactCommitPredicate(ObjectId id) {
 | 
					 | 
				
			||||||
    super(ChangeField.COMMIT, id.name());
 | 
					 | 
				
			||||||
    this.id = id;
 | 
					 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  @Override
 | 
					  @Override
 | 
				
			||||||
  public boolean match(final ChangeData object) throws OrmException {
 | 
					  public boolean match(final ChangeData object) throws OrmException {
 | 
				
			||||||
    String idStr = id.name();
 | 
					    String id = getValue().toLowerCase();
 | 
				
			||||||
    for (PatchSet p : object.patchSets()) {
 | 
					    for (PatchSet p : object.patchSets()) {
 | 
				
			||||||
      if (p.getRevision() != null && p.getRevision().get() != null) {
 | 
					      if (p.getRevision() != null
 | 
				
			||||||
        if (Objects.equals(p.getRevision().get(), idStr)) {
 | 
					          && Objects.equals(p.getRevision().get(), id)) {
 | 
				
			||||||
        return true;
 | 
					        return true;
 | 
				
			||||||
      }
 | 
					      }
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
    }
 | 
					 | 
				
			||||||
    return false;
 | 
					    return false;
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -66,7 +66,7 @@ public class InternalChangeQuery {
 | 
				
			|||||||
    return new ChangeStatusPredicate(status);
 | 
					    return new ChangeStatusPredicate(status);
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  private static Predicate<ChangeData> commit(ObjectId id) {
 | 
					  private static Predicate<ChangeData> commit(String id) {
 | 
				
			||||||
    return new ExactCommitPredicate(id);
 | 
					    return new ExactCommitPredicate(id);
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -129,7 +129,7 @@ public class InternalChangeQuery {
 | 
				
			|||||||
      List<String> hashes, int batchSize) throws OrmException {
 | 
					      List<String> hashes, int batchSize) throws OrmException {
 | 
				
			||||||
    List<Predicate<ChangeData>> commits = new ArrayList<>(hashes.size());
 | 
					    List<Predicate<ChangeData>> commits = new ArrayList<>(hashes.size());
 | 
				
			||||||
    for (String s : hashes) {
 | 
					    for (String s : hashes) {
 | 
				
			||||||
      commits.add(commit(ObjectId.fromString(s)));
 | 
					      commits.add(commit(s));
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
    int numBatches = (hashes.size() / batchSize) + 1;
 | 
					    int numBatches = (hashes.size() / batchSize) + 1;
 | 
				
			||||||
    List<Predicate<ChangeData>> queries = new ArrayList<>(numBatches);
 | 
					    List<Predicate<ChangeData>> queries = new ArrayList<>(numBatches);
 | 
				
			||||||
@@ -164,7 +164,7 @@ public class InternalChangeQuery {
 | 
				
			|||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  public List<ChangeData> byCommit(ObjectId id) throws OrmException {
 | 
					  public List<ChangeData> byCommit(ObjectId id) throws OrmException {
 | 
				
			||||||
    return query(commit(id));
 | 
					    return query(commit(id.name()));
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  public List<ChangeData> byProjectGroups(Project.NameKey project,
 | 
					  public List<ChangeData> byProjectGroups(Project.NameKey project,
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user