opendev/gerrit
Code Issues Proposed changes

Use exactRef() when possible

Browse Source 
This avoids ambiguous lookups when refs/heads/refs/meta/config exists.
Instead only refs/meta/config or nothing is returned.

exactRef() can also be optimized by JGit implementations to lookup a
ref, potentially saving time by avoiding unnecessary reads of the
storage system.

Change-Id: I1fd6750e7dce935597cf46331fe7cb6dc21473bb
This commit is contained in:
Shawn Pearce 2015-06-20 18:36:45 -07:00
parent 56e12f2277
commit 9f1d70c7c7
26 changed files with 86 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java
View File

@ -47,7 +47,6 @@ public class ListBranchesIT extends AbstractDaemonTest {
@TestProjectInput(createEmptyCommit = false)
public void listBranchesOfEmptyProject() throws Exception {
assertBranches(ImmutableList.of(
branch("HEAD", null, false),
branch("refs/meta/config", null, false)),
list().get());
}

2
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/RefNames.java
View File

@ -58,7 +58,7 @@ public class RefNames {
public static final String EDIT_PREFIX = "edit-";
public static String fullName(String ref) {
return (ref.startsWith(REFS) ? "" : REFS_HEADS) + ref;
return ref.startsWith(REFS) ? ref : REFS_HEADS + ref;
}
public static final String shortName(String ref) {

2
gerrit-server/src/main/java/com/google/gerrit/server/ProjectUtil.java
View File

@ -41,7 +41,7 @@ public class ProjectUtil {
IOException {
final Repository repo = repoManager.openRepository(branch.getParentKey());
try {
boolean exists = repo.getRef(branch.get()) != null;
boolean exists = repo.getRefDatabase().exactRef(branch.get()) != null;
if (!exists) {
exists = repo.getFullBranch().equals(branch.get());
}

9
gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
View File

@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.project.ChangeControl;
@ -70,11 +71,7 @@ public class CherryPick implements RestModifyView<RevisionResource, CherryPickIn
throw new AuthException("Cherry pick not permitted");
}
String refName = input.destination;
if (!refName.startsWith("refs/")) {
refName = "refs/heads/" + input.destination;
}
String refName = RefNames.fullName(input.destination);
RefControl refControl = control.getProjectControl().controlForRef(refName);
if (!refControl.canUpload()) {
throw new AuthException("Not allowed to cherry pick "
@ -85,7 +82,7 @@ public class CherryPick implements RestModifyView<RevisionResource, CherryPickIn
try {
Change.Id cherryPickedChangeId =
cherryPickChange.cherryPick(revision.getChange(),
revision.getPatchSet(), input.message, input.destination,
revision.getPatchSet(), input.message, refName,
refControl);
return json.format(cherryPickedChangeId);
} catch (InvalidChangeOperationException e) {

12
gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
View File

@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
@ -109,25 +110,26 @@ public class CherryPickChange {
}
public Change.Id cherryPick(Change change, PatchSet patch,
final String message, final String destinationBranch,
final String message, final String ref,
final RefControl refControl) throws NoSuchChangeException,
OrmException, MissingObjectException,
IncorrectObjectTypeException, IOException,
InvalidChangeOperationException, MergeException {
if (destinationBranch == null || destinationBranch.length() == 0) {
if (Strings.isNullOrEmpty(ref)) {
throw new InvalidChangeOperationException(
"Cherry Pick: Destination branch cannot be null or empty");
}
Project.NameKey project = change.getProject();
String destinationBranch = RefNames.shortName(ref);
IdentifiedUser identifiedUser = (IdentifiedUser) currentUser.get();
try (Repository git = gitManager.openRepository(project);
RevWalk revWalk = new RevWalk(git)) {
Ref destRef = git.getRef(destinationBranch);
Ref destRef = git.getRefDatabase().exactRef(ref);
if (destRef == null) {
throw new InvalidChangeOperationException("Branch "
+ destinationBranch + " does not exist.");
throw new InvalidChangeOperationException(String.format(
"Branch %s does not exist.", destinationBranch));
}
final RevCommit mergeTip = revWalk.parseCommit(destRef.getObjectId());

43
gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
View File

@ -18,6 +18,7 @@ import static com.google.gerrit.server.ChangeUtil.PS_ID_ORDER;
import static com.google.gerrit.server.ChangeUtil.TO_PS_ID;
import com.google.auto.value.AutoValue;
import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.collect.Collections2;
import com.google.common.collect.FluentIterable;
@ -243,6 +244,21 @@ public class ConsistencyChecker {
patchSetsBySha = MultimapBuilder.hashKeys(all.size())
.treeSetValues(PS_ID_ORDER)
.build();
Map<String, Ref> refs;
try {
refs = repo.getRefDatabase().exactRef(
Lists.transform(all, new Function<PatchSet, String>() {
@Override
public String apply(PatchSet ps) {
return ps.getId().toRefName();
}
}).toArray(new String[all.size()]));
} catch (IOException e) {
error("error reading refs", e);
refs = Collections.emptyMap();
}
for (PatchSet ps : all) {
// Check revision format.
int psNum = ps.getId().get();
@ -256,21 +272,16 @@ public class ConsistencyChecker {
// Check ref existence.
ProblemInfo refProblem = null;
try {
Ref ref = repo.getRef(refName);
if (ref == null) {
refProblem = problem("Ref missing: " + refName);
} else if (!objId.equals(ref.getObjectId())) {
String actual = ref.getObjectId() != null
? ref.getObjectId().name()
: "null";
refProblem = problem(String.format(
"Expected %s to point to %s, found %s",
ref.getName(), objId.name(), actual));
}
} catch (IOException e) {
error("Error reading ref: " + refName, e);
refProblem = lastProblem();
Ref ref = refs.get(refName);
if (ref == null) {
refProblem = problem("Ref missing: " + refName);
} else if (!objId.equals(ref.getObjectId())) {
String actual = ref.getObjectId() != null
? ref.getObjectId().name()
: "null";
refProblem = problem(String.format(
"Expected %s to point to %s, found %s",
ref.getName(), objId.name(), actual));
}
// Check object existence.
@ -306,7 +317,7 @@ public class ConsistencyChecker {
String refName = change.getDest().get();
Ref dest;
try {
dest = repo.getRef(refName);
dest = repo.getRefDatabase().exactRef(refName);
} catch (IOException e) {
problem("Failed to look up destination ref: " + refName);
return;

2
gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
View File

@ -175,7 +175,7 @@ public class CreateChange implements
parentCommit = ObjectId.fromString(ps.getRevision().get());
groups = ps.getGroups();
} else {
Ref destRef = git.getRef(refName);
Ref destRef = git.getRefDatabase().exactRef(refName);
if (destRef == null) {
throw new UnprocessableEntityException(String.format(
"Branch %s does not exist.", refName));

2
gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelatedByAncestors.java
View File

@ -76,7 +76,7 @@ class GetRelatedByAncestors {
throws RepositoryNotFoundException, IOException, OrmException {
try (Repository git = gitMgr.openRepository(rsrc.getChange().getProject());
RevWalk rw = new RevWalk(git)) {
Ref ref = git.getRef(rsrc.getChange().getDest().get());
Ref ref = git.getRefDatabase().exactRef(rsrc.getChange().getDest().get());
RelatedInfo info = new RelatedInfo();
info.changes = walk(rsrc, rw, ref);
return info;

9
gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
View File

@ -47,6 +47,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Map;
import java.util.Objects;
public class Mergeable implements RestReadView<RevisionResource> {
@ -116,7 +117,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
return result;
}
Ref ref = git.getRef(change.getDest().get());
Ref ref = git.getRefDatabase().exactRef(change.getDest().get());
ProjectState projectState = projectCache.get(change.getProject());
String strategy = mergeUtilFactory.create(projectState)
.mergeStrategyName();
@ -135,8 +136,10 @@ public class Mergeable implements RestReadView<RevisionResource> {
BranchOrderSection branchOrder = projectState.getBranchOrderSection();
if (branchOrder != null) {
int prefixLen = Constants.R_HEADS.length();
for (String n : branchOrder.getMoreStable(ref.getName())) {
Ref other = git.getRef(n);
String[] names = branchOrder.getMoreStable(ref.getName());
Map<String, Ref> refs = git.getRefDatabase().exactRef(names);
for (String n : names) {
Ref other = refs.get(n);
if (other == null) {
continue;
}

2
gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChange.java
View File

@ -221,7 +221,7 @@ public class RebaseChange {
if (baseRev == null) {
// We are dependent on a merged PatchSet or have no PatchSet
// dependencies at all.
Ref destRef = git.getRef(destBranch.get());
Ref destRef = git.getRefDatabase().exactRef(destBranch.get());
if (destRef == null) {
throw new InvalidChangeOperationException(
"The destination branch does not exist: " + destBranch.get());

2
gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
View File

@ -59,7 +59,7 @@ public class BanCommit {
public static NoteMap loadRejectCommitsMap(Repository repo, RevWalk walk)
throws IOException {
try {
Ref ref = repo.getRef(RefNames.REFS_REJECT_COMMITS);
Ref ref = repo.getRefDatabase().exactRef(RefNames.REFS_REJECT_COMMITS);
if (ref == null) {
return NoteMap.newEmptyMap();
}

8
gerrit-server/src/main/java/com/google/gerrit/server/git/BranchOrderSection.java
View File

@ -40,12 +40,12 @@ public class BranchOrderSection {
}
}
public List<String> getMoreStable(String branch) {
public String[] getMoreStable(String branch) {
int i = order.indexOf(RefNames.fullName(branch));
if (0 <= i) {
return order.subList(i + 1, order.size());
} else {
return ImmutableList.of();
List<String> r = order.subList(i + 1, order.size());
return r.toArray(new String[r.size()]);
}
return new String[] {};
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/git/NotesBranchUtil.java
View File

@ -189,7 +189,7 @@ public class NotesBranchUtil {
}
private void loadBase(String notesBranch) throws IOException {
Ref branch = db.getRef(notesBranch);
Ref branch = db.getRefDatabase().exactRef(notesBranch);
if (branch != null) {
baseCommit = revWalk.parseCommit(branch.getObjectId());
base = NoteMap.read(revWalk.getObjectReader(), baseCommit);
@ -240,7 +240,7 @@ public class NotesBranchUtil {
}
} else {
throw new ConcurrentRefUpdateException("Failed to lock the ref: "
+ notesBranch, db.getRef(notesBranch), result);
+ notesBranch, refUpdate.getRef(), result);
}
} else if (result == Result.REJECTED) {

14
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@ -1203,10 +1203,8 @@ public class ReceiveCommits {
String parse(CmdLineParser clp, Repository repo, Set<String> refs)
throws CmdLineException {
String ref = MagicBranch.getDestBranchName(cmd.getRefName());
if (!ref.startsWith(Constants.R_REFS)) {
ref = Constants.R_HEADS + ref;
}
String ref = RefNames.fullName(
MagicBranch.getDestBranchName(cmd.getRefName()));
int optionStart = ref.indexOf('%');
if (0 < optionStart) {
@ -1364,7 +1362,13 @@ public class ReceiveCommits {
} else if (newChangeForAllNotInTarget) {
String destBranch = magicBranch.dest.get();
try {
ObjectId baseHead = repo.getRef(destBranch).getObjectId();
Ref r = repo.getRefDatabase().exactRef(destBranch);
if (r == null) {
reject(cmd, destBranch + " not found");
return;
}
ObjectId baseHead = r.getObjectId();
magicBranch.baseCommit =
Collections.singletonList(walk.parseCommit(baseHead));
} catch (IOException ex) {

2
gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
View File

@ -96,7 +96,7 @@ public abstract class VersionedMetaData {
* @throws ConfigInvalidException
*/
public void load(Repository db) throws IOException, ConfigInvalidException {
Ref ref = db.getRef(getRefName());
Ref ref = db.getRefDatabase().exactRef(getRefName());
load(db, ref != null ? ref.getObjectId() : null);
}

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java
View File

@ -81,7 +81,7 @@ public abstract class AbstractChangeNotes<T> extends VersionedMetaData {
try {
repo = repoManager.openMetadataRepository(getProjectName());
try {
Ref ref = repo.getRef(getRefName());
Ref ref = repo.getRefDatabase().exactRef(getRefName());
return ref != null ? ref.getObjectId() : null;
} finally {
repo.close();

2
gerrit-server/src/main/java/com/google/gerrit/server/notedb/CommentsInNotesUtil.java
View File

@ -93,7 +93,7 @@ public class CommentsInNotesUtil {
Multimap<RevId, PatchLineComment> comments,
Status status)
throws IOException, ConfigInvalidException {
Ref ref = repo.getRef(refName);
Ref ref = repo.getRefDatabase().exactRef(refName);
if (ref == null) {
return null;
}

2
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
View File

@ -318,7 +318,7 @@ public class PatchListLoader extends CacheLoader<PatchListKey, PatchList> {
+ hash.substring(0, 2)
+ "/"
+ hash.substring(2);
Ref ref = repo.getRef(refName);
Ref ref = repo.getRefDatabase().exactRef(refName);
if (ref != null && ref.getObjectId() != null) {
return rw.parseTree(ref.getObjectId());
}

6
gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
View File

@ -154,13 +154,13 @@ public class CreateBranch implements RestModifyView<ProjectResource, Input> {
hooks.doRefUpdatedHook(name, u, identifiedUser.get().getAccount());
break;
case LOCK_FAILURE:
if (repo.getRef(ref) != null) {
if (repo.getRefDatabase().exactRef(ref) != null) {
throw new ResourceConflictException("branch \"" + ref
+ "\" already exists");
}
String refPrefix = getRefPrefix(ref);
while (!Constants.R_HEADS.equals(refPrefix)) {
if (repo.getRef(refPrefix) != null) {
if (repo.getRefDatabase().exactRef(refPrefix) != null) {
throw new ResourceConflictException("Cannot create branch \""
+ ref + "\" since it conflicts with branch \"" + refPrefix
+ "\".");
@ -229,7 +229,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, Input> {
Iterable<Ref> refs = Iterables.concat(
refDb.getRefs(Constants.R_HEADS).values(),
refDb.getRefs(Constants.R_TAGS).values());
Ref rc = refDb.getRef(RefNames.REFS_CONFIG);
Ref rc = refDb.exactRef(RefNames.REFS_CONFIG);
if (rc != null) {
refs = Iterables.concat(refs, Collections.singleton(rc));
}

2
gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
View File

@ -50,7 +50,7 @@ public class GetHead implements RestReadView<ProjectResource> {
public String apply(ProjectResource rsrc) throws AuthException,
ResourceNotFoundException, IOException {
try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) {
Ref head = repo.getRef(Constants.HEAD);
Ref head = repo.getRefDatabase().exactRef(Constants.HEAD);
if (head == null) {
throw new ResourceNotFoundException(Constants.HEAD);
} else if (head.isSymbolic()) {

15
gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java
View File

@ -115,9 +115,10 @@ public class ListBranches implements RestReadView<ProjectResource> {
db.getRefDatabase().getRefs(Constants.R_HEADS).values();
refs = new ArrayList<>(heads.size() + 3);
refs.addAll(heads);
addRef(db, refs, Constants.HEAD);
addRef(db, refs, RefNames.REFS_CONFIG);
addRef(db, refs, RefNames.REFS_USERS_DEFAULT);
refs.addAll(db.getRefDatabase().exactRef(
Constants.HEAD,
RefNames.REFS_CONFIG,
RefNames.REFS_USERS_DEFAULT).values());
} catch (RepositoryNotFoundException noGitRepository) {
throw new ResourceNotFoundException();
}
@ -183,14 +184,6 @@ public class ListBranches implements RestReadView<ProjectResource> {
}
}
private static void addRef(Repository db, List<Ref> refs, String name)
throws IOException {
Ref ref = db.getRef(name);
if (ref != null) {
refs.add(ref);
}
}
private FluentIterable<BranchInfo> filterBranches(
FluentIterable<BranchInfo> branches) throws BadRequestException {
if (!Strings.isNullOrEmpty(matchSubstring)) {

2
gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
View File

@ -91,7 +91,7 @@ public class ListProjects implements RestReadView<TopLevelResource> {
PERMISSIONS {
@Override
boolean matches(Repository git) throws IOException {
Ref head = git.getRef(Constants.HEAD);
Ref head = git.getRefDatabase().exactRef(Constants.HEAD);
return head != null
&& head.isSymbolic()
&& RefNames.REFS_CONFIG.equals(head.getLeaf().getName());

2
gerrit-server/src/main/java/com/google/gerrit/server/project/ListTags.java
View File

@ -104,7 +104,7 @@ public class ListTags implements RestReadView<ProjectResource> {
if (!tagName.startsWith(Constants.R_TAGS)) {
tagName = Constants.R_TAGS + tagName;
}
Ref ref = repo.getRefDatabase().getRef(tagName);
Ref ref = repo.getRefDatabase().exactRef(tagName);
if (ref != null && !visibleTags(resource.getControl(), repo,
ImmutableMap.of(ref.getName(), ref)).isEmpty()) {
return createTagInfo(ref, rw);

2
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
View File

@ -166,7 +166,7 @@ public class ProjectState {
try {
Repository git = gitMgr.openRepository(getProject().getNameKey());
try {
Ref ref = git.getRef(RefNames.REFS_CONFIG);
Ref ref = git.getRefDatabase().exactRef(RefNames.REFS_CONFIG);
if (ref == null || ref.getObjectId() == null) {
return true;
}

8
gerrit-server/src/main/java/com/google/gerrit/server/project/SetHead.java
View File

@ -33,12 +33,14 @@ import com.google.inject.Singleton;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Map;
@Singleton
public class SetHead implements RestModifyView<ProjectResource, Input> {
@ -77,12 +79,14 @@ public class SetHead implements RestModifyView<ProjectResource, Input> {
Repository repo = null;
try {
repo = repoManager.openRepository(rsrc.getNameKey());
if (repo.getRef(ref) == null) {
Map<String, Ref> cur =
repo.getRefDatabase().exactRef(Constants.HEAD, ref);
if (!cur.containsKey(ref)) {
throw new UnprocessableEntityException(String.format(
"Ref Not Found: %s", ref));
}
final String oldHead = repo.getRef(Constants.HEAD).getTarget().getName();
final String oldHead = cur.get(Constants.HEAD).getTarget().getName();
final String newHead = ref;
if (!oldHead.equals(newHead)) {
final RefUpdate u = repo.updateRef(Constants.HEAD, true);

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@ -730,7 +730,7 @@ public class ChangeData {
return null;
}
try (Repository repo = repoManager.openRepository(c.getProject())) {
Ref ref = repo.getRef(c.getDest().get());
Ref ref = repo.getRefDatabase().exactRef(c.getDest().get());
SubmitTypeRecord rec = new SubmitRuleEvaluator(this)
.getSubmitType();
if (rec.status != SubmitTypeRecord.Status.OK) {