opendev/gerrit
Code Issues Proposed changes

Merge changes from topics 'mergeop-refactor', 'change-data-submit-type'

Browse Source 
* changes:
  Tie CommitMergeStatus explicitly to submit strategies
  CommitMergeStatus: Remove REVISION_GONE and NO_PATCH_SET
  MergeValidationException: Allow arbitrary string messages
  MergeOp: Simplify updateChangeStatus
  MergeOp: Check commit status separately after merge strategies
  Remove unnecessary calls to SubmitRuleEvaluator#setPatchSet
  ChangeScreen: Use submit type from ChangeInfo
  Expose submit type in ChangeInfo
  Store SubmitTypeRecord lazily in ChangeData
  SubmitTypeRecord: Make fields final and document them
This commit is contained in:
Dave Borowitz
2015-12-25 18:10:52 +00:00
committed by Gerrit Code Review
parent 6a55317d4c 29f6366973
commit 684ecf9ed7
30 changed files with 290 additions and 402 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -58,6 +58,7 @@ import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.api.changes.FixInput;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -403,10 +404,13 @@ public class ChangeJson {
out.topic = in.getTopic();
out.hashtags = ctl.getNotes().load().getHashtags();
out.changeId = in.getKey().get();
// TODO(dborowitz): This gets the submit type, so we could include that in
// the response and avoid making a request to /submit_type from the UI.
out.mergeable = in.getStatus() == Change.Status.MERGED
? null : cd.isMergeable();
if (in.getStatus() != Change.Status.MERGED) {
SubmitTypeRecord str = cd.submitTypeRecord();
if (str.isOk()) {
out.submitType = str.type;
}
out.mergeable = cd.isMergeable();
}
out.submittable = Submit.submittable(cd);
ChangedLines changedLines = cd.changedLines();
if (changedLines != null) {

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
View File

@@ -110,7 +110,6 @@ public class ReviewerJson {
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
for (SubmitRecord rec : new SubmitRuleEvaluator(cd)
.setPatchSet(ps)
.setFastEvalLabels(true)
.setAllowDraft(true)
.evaluate()) {

29
gerrit-server/src/main/java/com/google/gerrit/server/git/CodeReviewCommit.java
View File

@@ -20,13 +20,13 @@ import com.google.common.base.Function;
import com.google.common.collect.Ordering;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -63,33 +63,6 @@ public class CodeReviewCommit extends RevCommit {
return new CodeReviewRevWalk(reader);
}
static CodeReviewCommit revisionGone(ChangeControl ctl) {
return error(ctl, CommitMergeStatus.REVISION_GONE);
}
static CodeReviewCommit noPatchSet(ChangeControl ctl) {
return error(ctl, CommitMergeStatus.NO_PATCH_SET);
}
/**
* Create an error commit.
* <p>
* Should only be used for error statuses such that there is no possible
* non-zero commit on which we could call {@link
* #setStatusCode(CommitMergeStatus)}, enumerated in the methods above.
*
* @param ctl control for change that caused this error
* @param s status
* @return new commit instance
*/
private static CodeReviewCommit error(ChangeControl ctl,
CommitMergeStatus s) {
CodeReviewCommit r = new CodeReviewCommit(ObjectId.zeroId());
r.setControl(ctl);
r.statusCode = s;
return r;
}
public static class CodeReviewRevWalk extends RevWalk {
private CodeReviewRevWalk(Repository repo) {
super(repo);

301
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -18,9 +18,12 @@ import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.auto.value.AutoValue;
import com.google.common.base.CharMatcher;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.ImmutableList;
@@ -57,12 +60,12 @@ import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
import com.google.gerrit.server.git.strategy.SubmitStrategy;
import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
import com.google.gerrit.server.git.validators.MergeValidationException;
import com.google.gerrit.server.git.validators.MergeValidators;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
@@ -333,9 +336,7 @@ public class MergeOp implements AutoCloseable {
}
List<SubmitRecord> results = cd.getSubmitRecords();
if (results == null) {
results = new SubmitRuleEvaluator(cd)
.setPatchSet(patchSet)
.evaluate();
results = new SubmitRuleEvaluator(cd).evaluate();
cd.setSubmitRecords(results);
}
if (findOkRecord(results).isPresent()) {
@@ -497,17 +498,17 @@ public class MergeOp implements AutoCloseable {
}
failFast(cs); // Done checks that don't involve running submit strategies.
for (Branch.NameKey branch : cbb.keySet()) {
OpenRepo or = openRepo(branch.getParentKey());
OpenBranch ob = or.getBranch(branch);
BranchBatch submitting = toSubmit.get(branch);
SubmitStrategy strategy = createStrategy(or, branch,
submitting.submitType(), ob.oldTip, caller);
ob.mergeTip = preMerge(strategy, submitting.changes(), ob.oldTip);
}
checkMergeStrategyResults(cs, toSubmit.values());
for (Project.NameKey project : br.keySet()) {
OpenRepo or = openRepo(project);
for (Branch.NameKey branch : br.get(project)) {
OpenBranch ob = or.getBranch(branch);
BranchBatch submitting = toSubmit.get(branch);
SubmitStrategy strategy = createStrategy(or, branch,
submitting.submitType(), ob.oldTip, caller);
ob.mergeTip = preMerge(strategy, submitting.changes(), ob.oldTip);
updateChangeStatus(ob, submitting.changes(), true, caller);
or.ins.flush();
}
openRepo(project).ins.flush();
}
Set<Branch.NameKey> done =
@@ -521,7 +522,7 @@ public class MergeOp implements AutoCloseable {
boolean updated = updateBranch(or, branch, caller);
BranchBatch submitting = toSubmit.get(branch);
updateChangeStatus(ob, submitting.changes(), false, caller);
updateChangeStatus(ob, submitting.changes(), caller);
updateSubmoduleSubscriptions(ob, subOp);
if (updated) {
fireRefUpdated(ob);
@@ -692,12 +693,11 @@ public class MergeOp implements AutoCloseable {
mergeValidators.validatePreMerge(
or.repo, commit, or.project, destBranch, ps.getId());
} catch (MergeValidationException mve) {
commit.setStatusCode(mve.getStatus());
problems.put(changeId, mve.getStatus().toString());
problems.put(changeId, mve.getMessage());
continue;
}
SubmitType st = getSubmitType(cd, ps);
SubmitType st = getSubmitType(cd);
if (st == null) {
logProblem(changeId, "No submit type for change");
continue;
@@ -742,15 +742,10 @@ public class MergeOp implements AutoCloseable {
}
}
private SubmitType getSubmitType(ChangeData cd, PatchSet ps) {
private SubmitType getSubmitType(ChangeData cd) {
try {
SubmitTypeRecord r = new SubmitRuleEvaluator(cd).setPatchSet(ps)
.getSubmitType();
if (r.status != SubmitTypeRecord.Status.OK) {
logError("Failed to get submit type for " + cd.getId());
return null;
}
return r.type;
SubmitTypeRecord str = cd.submitTypeRecord();
return str.isOk() ? str.type : null;
} catch (OrmException e) {
logError("Failed to get submit type for " + cd.getId(), e);
return null;
@@ -853,112 +848,111 @@ public class MergeOp implements AutoCloseable {
return "";
}
private void updateChangeStatus(OpenBranch ob, List<ChangeData> submitted,
boolean dryRun, IdentifiedUser caller) throws NoSuchChangeException,
IntegrationException, ResourceConflictException, OrmException {
if (!dryRun) {
logDebug("Updating change status for {} changes", submitted.size());
} else {
logDebug("Checking change state for {} changes in a dry run",
submitted.size());
}
private Iterable<ChangeData> flattenBatches(Collection<BranchBatch> batches) {
return FluentIterable.from(batches)
.transformAndConcat(new Function<BranchBatch, List<ChangeData>>() {
@Override
public List<ChangeData> apply(BranchBatch batch) {
return batch.changes();
}
});
}
for (ChangeData cd : submitted) {
Change c = cd.change();
CodeReviewCommit commit = commits.get(c.getId());
private void checkMergeStrategyResults(ChangeSet cs,
Collection<BranchBatch> batches) throws ResourceConflictException {
for (ChangeData cd : flattenBatches(batches)) {
Change.Id id = cd.getId();
CodeReviewCommit commit = commits.get(id);
CommitMergeStatus s = commit != null ? commit.getStatusCode() : null;
if (s == null) {
// Shouldn't ever happen, but leave the change alone. We'll pick
// it up on the next pass.
//
logDebug("Submitted change {} did not appear in set of new commits"
+ " produced by merge strategy", c.getId());
problems.put(id,
"internal error: change not processed by merge strategy");
continue;
}
switch (s) {
case CLEAN_MERGE:
case CLEAN_REBASE:
case CLEAN_PICK:
case ALREADY_MERGED:
break; // Merge strategy accepted this change.
if (!dryRun) {
try {
setApproval(cd, caller);
} catch (IOException e) {
throw new OrmException(e);
}
}
case PATH_CONFLICT:
case REBASE_MERGE_CONFLICT:
case MANUAL_RECURSIVE_MERGE:
case CANNOT_CHERRY_PICK_ROOT:
case NOT_FAST_FORWARD:
// TODO(dborowitz): Reformat these messages to be more appropriate for
// short problem descriptions.
problems.put(id,
CharMatcher.is('\n').collapseFrom(s.getMessage(), ' '));
break;
String txt = s.getMessage();
logDebug("Status of change {} ({}) on {}: {}", c.getId(), commit.name(),
c.getDest(), s);
// If mergeTip is null merge failed and mergeResultRev will not be read.
ObjectId mergeResultRev = ob.mergeTip != null
? ob.mergeTip.getMergeResults().get(commit) : null;
// The change notes must be forcefully reloaded so that the SUBMIT
// approval that we added earlier is visible
commit.notes().reload();
try {
ChangeMessage msg;
switch (s) {
case CLEAN_MERGE:
if (!dryRun) {
setMerged(c, message(c, txt + getByAccountName(commit)),
mergeResultRev);
}
break;
case MISSING_DEPENDENCY:
problems.put(id, "depends on change that was not submitted");
break;
case CLEAN_REBASE:
case CLEAN_PICK:
if (!dryRun) {
setMerged(c, message(c, txt + " as " + commit.name()
+ getByAccountName(commit)), mergeResultRev);
}
break;
case ALREADY_MERGED:
if (!dryRun) {
setMerged(c, null, mergeResultRev);
}
break;
case PATH_CONFLICT:
case REBASE_MERGE_CONFLICT:
case MANUAL_RECURSIVE_MERGE:
case CANNOT_CHERRY_PICK_ROOT:
case NOT_FAST_FORWARD:
case INVALID_PROJECT_CONFIGURATION:
case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED:
case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE:
case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND:
case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT:
case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN:
setNew(commit.notes(), message(c, txt));
throw new ResourceConflictException("Cannot merge " + commit.name()
+ "\n" + s.getMessage());
case MISSING_DEPENDENCY:
logDebug("Change {} is missing dependency", c.getId());
throw new IntegrationException(
"Cannot merge " + commit.name() + "\n" + s.getMessage());
case REVISION_GONE:
logDebug("Commit not found for change {}", c.getId());
msg = new ChangeMessage(
new ChangeMessage.Key(
c.getId(),
ChangeUtil.messageUUID(db)),
null,
TimeUtil.nowTs(),
c.currentPatchSetId());
msg.setMessage("Failed to read commit for this patch set");
setNew(commit.notes(), msg);
throw new IntegrationException(msg.getMessage());
default:
msg = message(c, "Unspecified merge failure: " + s.name());
setNew(commit.notes(), msg);
throw new IntegrationException(msg.getMessage());
}
} catch (OrmException | IOException err) {
logWarn("Error updating change status for " + c.getId(), err);
default:
problems.put(id, "unspecified merge failure: " + s);
break;
}
}
failFast(cs);
}
private void updateChangeStatus(OpenBranch ob, List<ChangeData> submitted,
IdentifiedUser caller) throws ResourceConflictException {
List<Change.Id> problemChanges = new ArrayList<>(submitted.size());
logDebug("Updating change status for {} changes", submitted.size());
for (ChangeData cd : submitted) {
Change.Id id = cd.getId();
try {
Change c = cd.change();
CodeReviewCommit commit = commits.get(id);
CommitMergeStatus s = commit != null ? commit.getStatusCode() : null;
logDebug("Status of change {} ({}) on {}: {}", id, commit.name(),
c.getDest(), s);
checkState(s != null,
"status not set for change %s; expected to previously fail fast",
id);
setApproval(cd, caller);
ObjectId mergeResultRev = ob.mergeTip != null
? ob.mergeTip.getMergeResults().get(commit) : null;
String txt = s.getMessage();
// The change notes must be forcefully reloaded so that the SUBMIT
// approval that we added earlier is visible
commit.notes().reload();
if (s == CommitMergeStatus.CLEAN_MERGE) {
setMerged(c, message(c, txt + getByAccountName(commit)),
mergeResultRev);
} else if (s == CommitMergeStatus.CLEAN_REBASE
|| s == CommitMergeStatus.CLEAN_PICK) {
setMerged(c, message(c, txt + " as " + commit.name()
+ getByAccountName(commit)), mergeResultRev);
} else if (s == CommitMergeStatus.ALREADY_MERGED) {
setMerged(c, null, mergeResultRev);
} else {
throw new IllegalStateException("unexpected status " + s +
" for change " + c.getId() + "; expected to previously fail fast");
}
} catch (OrmException | IOException err) {
logWarn("Error updating change status for " + id, err);
problemChanges.add(id);
}
}
if (problemChanges.isEmpty()) {
return;
}
StringBuilder msg = new StringBuilder("Error updating status of change");
if (problemChanges.size() == 1) {
msg.append(' ').append(problemChanges.iterator().next());
} else {
msg.append('s').append(Joiner.on(", ").join(problemChanges));
}
throw new ResourceConflictException(msg.toString());
}
private void updateSubmoduleSubscriptions(OpenBranch ob, SubmoduleOp subOp) {
@@ -1202,69 +1196,6 @@ public class MergeOp implements AutoCloseable {
}
}
private ChangeControl changeControl(Change c) throws NoSuchChangeException {
return changeControlFactory.controlFor(
c, identifiedUserFactory.create(c.getOwner()));
}
private void setNew(ChangeNotes notes, final ChangeMessage msg)
throws NoSuchChangeException, IOException {
Change c = notes.getChange();
Change change = null;
ChangeUpdate update = null;
try {
db.changes().beginTransaction(c.getId());
try {
change = db.changes().atomicUpdate(
c.getId(),
new AtomicUpdate<Change>() {
@Override
public Change update(Change c) {
if (c.getStatus().isOpen()) {
c.setStatus(Change.Status.NEW);
ChangeUtil.updated(c);
}
return c;
}
});
ChangeControl control = changeControl(change);
//TODO(yyonas): atomic change is not propagated.
update = updateFactory.create(control, c.getLastUpdatedOn());
if (msg != null) {
cmUtil.addChangeMessage(db, update, msg);
}
db.commit();
} finally {
db.rollback();
}
} catch (OrmException err) {
logWarn("Cannot record merge failure message", err);
}
if (update != null) {
update.commit();
}
indexer.index(db, change);
PatchSetApproval submitter = null;
try {
submitter = approvalsUtil.getSubmitter(
db, notes, notes.getChange().currentPatchSetId());
} catch (Exception e) {
logError("Cannot get submitter for change " + notes.getChangeId(), e);
}
if (submitter != null) {
try {
hooks.doMergeFailedHook(c,
accountCache.get(submitter.getAccountId()).getAccount(),
db.patchSets().get(c.currentPatchSetId()), msg.getMessage(), db);
} catch (OrmException ex) {
logError("Cannot run hook for merge failed " + c.getId(), ex);
}
}
}
private void abandonAllOpenChanges(Project.NameKey destProject)
throws NoSuchChangeException {
try {

1
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSorter.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.git;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevCommitList;

10
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java
View File

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

1
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@@ -36,6 +36,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;

1
gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.server.git;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.strategy.CommitMergeStatus;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;

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

@@ -179,6 +179,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -270,6 +271,9 @@ public class ReceiveCommits {
public RestApiException apply(Exception input) {
if (input instanceof RestApiException) {
return (RestApiException) input;
} else if ((input instanceof ExecutionException)
&& (input.getCause() instanceof RestApiException)) {
return (RestApiException) input.getCause();
}
return new RestApiException("Error inserting change/patchset", input);
}
@@ -844,6 +848,9 @@ public class ReceiveCommits {
f.checkedGet();
}
magicBranch.cmd.setResult(OK);
} catch (ResourceConflictException e) {
addMessage(e.getMessage());
reject(magicBranch.cmd, "conflict");
} catch (RestApiException err) {
log.error("Can't insert change/patchset for " + project.getName()
+ ". " + err.getMessage(), err);

1
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@@ -28,7 +28,6 @@ import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.git.MergeIdenticalTreeException;

58
gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java → gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CommitMergeStatus.java
View File

@@ -12,97 +12,55 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.git;
package com.google.gerrit.server.git.strategy;
/**
* Status codes set on {@link com.google.gerrit.server.git.CodeReviewCommit}s by
* {@link SubmitStrategy} implementations.
*/
public enum CommitMergeStatus {
/** */
CLEAN_MERGE("Change has been successfully merged"),
/** */
CLEAN_PICK("Change has been successfully cherry-picked"),
/** */
CLEAN_REBASE("Change has been successfully rebased"),
/** */
ALREADY_MERGED(""),
/** */
PATH_CONFLICT("Change could not be merged due to a path conflict.\n"
+ "\n"
+ "Please rebase the change locally and upload the rebased commit for review."),
/** */
REBASE_MERGE_CONFLICT(
"Change could not be merged due to a conflict.\n"
+ "\n"
+ "Please rebase the change locally and upload the rebased commit for review."),
/** */
MISSING_DEPENDENCY(""),
/** */
NO_PATCH_SET(""),
/** */
REVISION_GONE(""),
/** */
MANUAL_RECURSIVE_MERGE("The change requires a local merge to resolve.\n"
+ "\n"
+ "Please merge (or rebase) the change locally and upload the resolution for review."),
/** */
CANNOT_CHERRY_PICK_ROOT("Cannot cherry-pick an initial commit onto an existing branch.\n"
+ "\n"
+ "Please merge the change locally and upload the merge commit for review."),
/** */
CANNOT_REBASE_ROOT("Cannot rebase an initial commit onto an existing branch.\n"
+ "\n"
+ "Please merge the change locally and upload the merge commit for review."),
/** */
NOT_FAST_FORWARD("Project policy requires all submissions to be a fast-forward.\n"
+ "\n"
+ "Please rebase the change locally and upload again for review."),
/** */
INVALID_PROJECT_CONFIGURATION("Change contains an invalid project configuration."),
/** */
INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED(
"Change contains an invalid project configuration:\n"
+ "One of the plugin configuration parameters has a value that is not permitted."),
/** */
INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE(
"Change contains an invalid project configuration:\n"
+ "One of the plugin configuration parameters is not editable."),
/** */
INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND(
"Change contains an invalid project configuration:\n"
+ "Parent project does not exist."),
/** */
INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT(
"Change contains an invalid project configuration:\n"
+ "The root project cannot have a parent."),
/** */
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN(
"Change contains a project configuration that changes the parent project.\n"
+ "The change must be submitted by a Gerrit administrator.");
+ "Please rebase the change locally and upload again for review.");
private String message;
CommitMergeStatus(String message){
private CommitMergeStatus(String message) {
this.message = message;
}
public String getMessage(){
public String getMessage() {
return message;
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOnly.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.git.strategy;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.git.MergeTip;

1
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
View File

@@ -27,7 +27,6 @@ import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
import com.google.gerrit.server.git.BatchUpdate.Context;
import com.google.gerrit.server.git.BatchUpdate.RepoContext;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.IntegrationException;
import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.RebaseSorter;

17
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationException.java
View File

@@ -14,19 +14,18 @@
package com.google.gerrit.server.git.validators;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.validators.ValidationException;
/**
* Exception that occurs during a validation step before merging changes.
* <p>
* Used by {@link MergeValidationListener}s provided by plugins. Messages should
* be considered human-readable.
*/
public class MergeValidationException extends ValidationException {
private static final long serialVersionUID = 1L;
private final CommitMergeStatus status;
public MergeValidationException(CommitMergeStatus status) {
super(status.toString());
this.status = status;
}
public CommitMergeStatus getStatus() {
return status;
public MergeValidationException(String msg) {
super(msg);
}
}

42
gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java
View File

@@ -30,7 +30,6 @@ import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.ProjectConfigEntry;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
@@ -75,6 +74,26 @@ public class MergeValidators {
public static class ProjectConfigValidator implements
MergeValidationListener {
private static final String INVALID_CONFIG =
"Change contains an invalid project configuration.";
private static final String PARENT_NOT_FOUND =
"Change contains an invalid project configuration:\n"
+ "Parent project does not exist.";
private static final String PLUGIN_VALUE_NOT_EDITABLE =
"Change contains an invalid project configuration:\n"
+ "One of the plugin configuration parameters is not editable.";
private static final String PLUGIN_VALUE_NOT_PERMITTED =
"Change contains an invalid project configuration:\n"
+ "One of the plugin configuration parameters has a value that is not"
+ " permitted.";
private static final String ROOT_NO_PARENT =
"Change contains an invalid project configuration:\n"
+ "The root project cannot have a parent.";
private static final String SET_BY_ADMIN =
"Change contains a project configuration that changes the parent"
+ " project.\n"
+ "The change must be submitted by a Gerrit administrator.";
private final AllProjectsName allProjectsName;
private final ReviewDb db;
private final ProjectCache projectCache;
@@ -119,27 +138,23 @@ public class MergeValidators {
if (oldParent == null) {
// update of the 'All-Projects' project
if (newParent != null) {
throw new MergeValidationException(CommitMergeStatus.
INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT);
throw new MergeValidationException(ROOT_NO_PARENT);
}
} else {
if (!oldParent.equals(newParent)) {
PatchSetApproval psa =
approvalsUtil.getSubmitter(db, commit.notes(), patchSetId);
if (psa == null) {
throw new MergeValidationException(CommitMergeStatus.
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
throw new MergeValidationException(SET_BY_ADMIN);
}
final IdentifiedUser submitter =
identifiedUserFactory.create(psa.getAccountId());
if (!submitter.getCapabilities().canAdministrateServer()) {
throw new MergeValidationException(CommitMergeStatus.
SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
throw new MergeValidationException(SET_BY_ADMIN);
}
if (projectCache.get(newParent) == null) {
throw new MergeValidationException(CommitMergeStatus.
INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND);
throw new MergeValidationException(PARENT_NOT_FOUND);
}
}
}
@@ -155,19 +170,16 @@ public class MergeValidators {
if ((value == null ? oldValue != null : !value.equals(oldValue)) &&
!configEntry.isEditable(destProject)) {
throw new MergeValidationException(CommitMergeStatus.
INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE);
throw new MergeValidationException(PLUGIN_VALUE_NOT_EDITABLE);
}
if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType())
&& value != null && !configEntry.getPermittedValues().contains(value)) {
throw new MergeValidationException(CommitMergeStatus.
INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED);
throw new MergeValidationException(PLUGIN_VALUE_NOT_PERMITTED);
}
}
} catch (ConfigInvalidException | IOException e) {
throw new MergeValidationException(CommitMergeStatus.
INVALID_PROJECT_CONFIGURATION);
throw new MergeValidationException(INVALID_CONFIG);
}
}
}

23
gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
View File

@@ -73,14 +73,7 @@ public class SubmitRuleEvaluator {
}
public static SubmitTypeRecord defaultTypeError() {
return createTypeError(DEFAULT_MSG);
}
public static SubmitTypeRecord createTypeError(String err) {
SubmitTypeRecord rec = new SubmitTypeRecord();
rec.status = SubmitTypeRecord.Status.RULE_ERROR;
rec.errorMessage = err;
return rec;
return SubmitTypeRecord.error(DEFAULT_MSG);
}
/**
@@ -242,7 +235,9 @@ public class SubmitRuleEvaluator {
try {
if (!control.isDraftVisible(cd.db(), cd)) {
return createRuleError("Patch set " + patchSet.getId() + " not found");
} else if (patchSet.isDraft()) {
}
initPatchSet();
if (patchSet.isDraft()) {
return createRuleError("Cannot submit draft patch sets");
} else {
return createRuleError("Cannot submit draft changes");
@@ -386,15 +381,17 @@ public class SubmitRuleEvaluator {
try {
if (control.getChange().getStatus() == Change.Status.DRAFT
&& !control.isDraftVisible(cd.db(), cd)) {
return createTypeError("Patch set " + patchSet.getId() + " not found");
return SubmitTypeRecord.error(
"Patch set " + patchSet.getId() + " not found");
}
if (patchSet.isDraft() && !control.isDraftVisible(cd.db(), cd)) {
return createTypeError("Patch set " + patchSet.getId() + " not found");
return SubmitTypeRecord.error(
"Patch set " + patchSet.getId() + " not found");
}
} catch (OrmException err) {
String msg = "Cannot read patch set " + patchSet.getId();
log.error(msg, err);
return createTypeError(msg);
return SubmitTypeRecord.error(msg);
}
List<Term> results;
@@ -446,7 +443,7 @@ public class SubmitRuleEvaluator {
}
return defaultTypeError();
} else {
return createTypeError(err);
return SubmitTypeRecord.error(err);
}
}

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

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

18
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.query.change;
import com.google.common.collect.Lists;
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.server.ReviewDb;
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.ProjectCache;
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.OrPredicate;
import com.google.gerrit.server.query.Predicate;
@@ -98,14 +96,14 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
if (!otherChange.getDest().equals(c.getDest())) {
return false;
}
SubmitType submitType = getSubmitType(object);
if (submitType == null) {
SubmitTypeRecord str = object.submitTypeRecord();
if (!str.isOk()) {
return false;
}
ObjectId other = ObjectId.fromString(
object.currentPatchSet().getRevision().get());
ConflictKey conflictsKey =
new ConflictKey(changeDataCache.getTestAgainst(), other, submitType,
new ConflictKey(changeDataCache.getTestAgainst(), other, str.type,
changeDataCache.getProjectState().isUseContentMerge());
Boolean conflicts = args.conflictsCache.getIfPresent(conflictsKey);
if (conflicts != null) {
@@ -115,7 +113,7 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
args.repoManager.openRepository(otherChange.getProject());
CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
conflicts = !args.submitDryRun.run(
submitType, repo, rw, otherChange.getDest(),
str.type, repo, rw, otherChange.getDest(),
changeDataCache.getTestAgainst(), other,
getAlreadyAccepted(repo, rw));
args.conflictsCache.put(conflictsKey, conflicts);
@@ -131,14 +129,6 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
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)
throws IntegrationException {
try {