Store SubmitTypeRecord lazily in ChangeData

Add convenience method isOk() to SubmitTypeRecord to make it easier
for callers to check whether this result was successful.

Change-Id: I1551d5fdce208631f4d97d815fa882ff44502467
This commit is contained in:
Dave Borowitz 2015-12-17 12:49:37 -05:00
parent 44fd0aca3b
commit be4d7a7c9d
5 changed files with 31 additions and 34 deletions

View File

@ -56,6 +56,10 @@ public class SubmitTypeRecord {
this.errorMessage = errorMessage; this.errorMessage = errorMessage;
} }
public boolean isOk() {
return status == Status.OK;
}
@Override @Override
public String toString() { public String toString() {
StringBuilder sb = new StringBuilder(); StringBuilder sb = new StringBuilder();

View File

@ -697,7 +697,7 @@ public class MergeOp implements AutoCloseable {
continue; continue;
} }
SubmitType st = getSubmitType(cd, ps); SubmitType st = getSubmitType(cd);
if (st == null) { if (st == null) {
logProblem(changeId, "No submit type for change"); logProblem(changeId, "No submit type for change");
continue; continue;
@ -742,15 +742,10 @@ public class MergeOp implements AutoCloseable {
} }
} }
private SubmitType getSubmitType(ChangeData cd, PatchSet ps) { private SubmitType getSubmitType(ChangeData cd) {
try { try {
SubmitTypeRecord r = new SubmitRuleEvaluator(cd).setPatchSet(ps) SubmitTypeRecord str = cd.submitTypeRecord();
.getSubmitType(); return str.isOk() ? str.type : null;
if (r.status != SubmitTypeRecord.Status.OK) {
logError("Failed to get submit type for " + cd.getId());
return null;
}
return r.type;
} catch (OrmException e) { } catch (OrmException e) {
logError("Failed to get submit type for " + cd.getId(), e); logError("Failed to get submit type for " + cd.getId(), e);
return null; return null;

View File

@ -25,7 +25,6 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.Submit; import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -104,11 +103,12 @@ public class MergeSuperSet {
for (Change.Id cId : pc.get(project)) { for (Change.Id cId : pc.get(project)) {
ChangeData cd = changeDataFactory.create(db, cId); ChangeData cd = changeDataFactory.create(db, cId);
SubmitTypeRecord r = new SubmitRuleEvaluator(cd).getSubmitType(); SubmitTypeRecord str = cd.submitTypeRecord();
if (r.status != SubmitTypeRecord.Status.OK) { if (!str.isOk()) {
logErrorAndThrow("Failed to get submit type for " + cd.getId()); logErrorAndThrow("Failed to get submit type for " + cd.getId()
+ ": " + str.errorMessage);
} }
if (r.type == SubmitType.CHERRY_PICK) { if (str.type == SubmitType.CHERRY_PICK) {
ret.add(cd); ret.add(cd);
continue; continue;
} }

View File

@ -316,6 +316,7 @@ public class ChangeData {
private List<ChangeMessage> messages; private List<ChangeMessage> messages;
private List<SubmitRecord> submitRecords; private List<SubmitRecord> submitRecords;
private ChangedLines changedLines; private ChangedLines changedLines;
private SubmitTypeRecord submitTypeRecord;
private Boolean mergeable; private Boolean mergeable;
private Set<Account.Id> editsByUser; private Set<Account.Id> editsByUser;
private Set<Account.Id> reviewedBy; private Set<Account.Id> reviewedBy;
@ -753,6 +754,13 @@ public class ChangeData {
return submitRecords; return submitRecords;
} }
public SubmitTypeRecord submitTypeRecord() throws OrmException {
if (submitTypeRecord == null) {
submitTypeRecord = new SubmitRuleEvaluator(this).getSubmitType();
}
return submitTypeRecord;
}
public void setMergeable(Boolean mergeable) { public void setMergeable(Boolean mergeable) {
this.mergeable = mergeable; this.mergeable = mergeable;
} }
@ -772,18 +780,18 @@ public class ChangeData {
} }
try (Repository repo = repoManager.openRepository(c.getProject())) { try (Repository repo = repoManager.openRepository(c.getProject())) {
Ref ref = repo.getRefDatabase().exactRef(c.getDest().get()); Ref ref = repo.getRefDatabase().exactRef(c.getDest().get());
SubmitTypeRecord rec = new SubmitRuleEvaluator(this) SubmitTypeRecord str = submitTypeRecord();
.getSubmitType(); if (!str.isOk()) {
if (rec.status != SubmitTypeRecord.Status.OK) { // If submit type rules are broken, it's definitely not mergeable.
throw new OrmException( // No need to log, as SubmitRuleEvaluator already did it for us.
"Error in mergeability check: " + rec.errorMessage); return false;
} }
String mergeStrategy = mergeUtilFactory String mergeStrategy = mergeUtilFactory
.create(projectCache.get(c.getProject())) .create(projectCache.get(c.getProject()))
.mergeStrategyName(); .mergeStrategyName();
mergeable = mergeabilityCache.get( mergeable = mergeabilityCache.get(
ObjectId.fromString(ps.getRevision().get()), ObjectId.fromString(ps.getRevision().get()),
ref, rec.type, mergeStrategy, c.getDest(), repo); ref, str.type, mergeStrategy, c.getDest(), repo);
} catch (IOException e) { } catch (IOException e) {
throw new OrmException(e); throw new OrmException(e);
} }

View File

@ -16,7 +16,6 @@ package com.google.gerrit.server.query.change;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit;
@ -26,7 +25,6 @@ import com.google.gerrit.server.git.strategy.SubmitDryRun;
import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.OperatorPredicate; import com.google.gerrit.server.query.OperatorPredicate;
import com.google.gerrit.server.query.OrPredicate; import com.google.gerrit.server.query.OrPredicate;
import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.Predicate;
@ -98,14 +96,14 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
if (!otherChange.getDest().equals(c.getDest())) { if (!otherChange.getDest().equals(c.getDest())) {
return false; return false;
} }
SubmitType submitType = getSubmitType(object); SubmitTypeRecord str = object.submitTypeRecord();
if (submitType == null) { if (!str.isOk()) {
return false; return false;
} }
ObjectId other = ObjectId.fromString( ObjectId other = ObjectId.fromString(
object.currentPatchSet().getRevision().get()); object.currentPatchSet().getRevision().get());
ConflictKey conflictsKey = ConflictKey conflictsKey =
new ConflictKey(changeDataCache.getTestAgainst(), other, submitType, new ConflictKey(changeDataCache.getTestAgainst(), other, str.type,
changeDataCache.getProjectState().isUseContentMerge()); changeDataCache.getProjectState().isUseContentMerge());
Boolean conflicts = args.conflictsCache.getIfPresent(conflictsKey); Boolean conflicts = args.conflictsCache.getIfPresent(conflictsKey);
if (conflicts != null) { if (conflicts != null) {
@ -115,7 +113,7 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
args.repoManager.openRepository(otherChange.getProject()); args.repoManager.openRepository(otherChange.getProject());
CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) { CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
conflicts = !args.submitDryRun.run( conflicts = !args.submitDryRun.run(
submitType, repo, rw, otherChange.getDest(), str.type, repo, rw, otherChange.getDest(),
changeDataCache.getTestAgainst(), other, changeDataCache.getTestAgainst(), other,
getAlreadyAccepted(repo, rw)); getAlreadyAccepted(repo, rw));
args.conflictsCache.put(conflictsKey, conflicts); args.conflictsCache.put(conflictsKey, conflicts);
@ -131,14 +129,6 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
return 5; return 5;
} }
private SubmitType getSubmitType(ChangeData cd) throws OrmException {
SubmitTypeRecord r = new SubmitRuleEvaluator(cd).getSubmitType();
if (r.status != SubmitTypeRecord.Status.OK) {
return null;
}
return r.type;
}
private Set<RevCommit> getAlreadyAccepted(Repository repo, RevWalk rw) private Set<RevCommit> getAlreadyAccepted(Repository repo, RevWalk rw)
throws IntegrationException { throws IntegrationException {
try { try {