opendev/gerrit
Code Issues Proposed changes

MergeOp: add an internal dry run flag

Browse Source 
This adds a flag to our internal API (i.e. it is not exposed to the
user yet), that runs the MergeOp in a dry run. This includes
calculating all projects affected by the submission, checking for
possible submission problems (ACLs, merge conflicts, etc)

Change-Id: I90430a758b52694286e2b22cf9bf3eedeb82876c
Signed-off-by: Stefan Beller <sbeller@google.com>
This commit is contained in:
Stefan Beller 2016-09-07 09:43:56 -07:00
parent b98ae30e9e
commit 6ac85596f5
8 changed files with 87 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -222,7 +222,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
try (MergeOp op = mergeOpProvider.get()) {
ReviewDb db = dbProvider.get();
op.merge(db, change, caller, true, input);
op.merge(db, change, caller, true, input, false);
try {
change = changeNotesFactory
.createChecked(db, change.getProject(), change.getId()).getChange();

51
gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
View File

@ -379,7 +379,8 @@ public class BatchUpdate implements AutoCloseable {
}
static void execute(Collection<BatchUpdate> updates, Listener listener,
@Nullable RequestId requestId) throws UpdateException, RestApiException {
@Nullable RequestId requestId, boolean dryrun)
throws UpdateException, RestApiException {
if (updates.isEmpty()) {
return;
}
@ -401,17 +402,17 @@ public class BatchUpdate implements AutoCloseable {
}
listener.afterUpdateRepos();
for (BatchUpdate u : updates) {
u.executeRefUpdates();
u.executeRefUpdates(dryrun);
}
listener.afterRefUpdates();
for (BatchUpdate u : updates) {
u.executeChangeOps(updateChangesInParallel);
u.executeChangeOps(updateChangesInParallel, dryrun);
}
listener.afterUpdateChanges();
break;
case DB_BEFORE_REPO:
for (BatchUpdate u : updates) {
u.executeChangeOps(updateChangesInParallel);
u.executeChangeOps(updateChangesInParallel, dryrun);
}
listener.afterUpdateChanges();
for (BatchUpdate u : updates) {
@ -419,7 +420,7 @@ public class BatchUpdate implements AutoCloseable {
}
listener.afterUpdateRepos();
for (BatchUpdate u : updates) {
u.executeRefUpdates();
u.executeRefUpdates(dryrun);
}
listener.afterRefUpdates();
break;
@ -445,9 +446,10 @@ public class BatchUpdate implements AutoCloseable {
u.getUser().isIdentifiedUser() ? u.getUser().getAccountId() : null);
}
}
for (BatchUpdate u : updates) {
u.executePostOps();
if (!dryrun) {
for (BatchUpdate u : updates) {
u.executePostOps();
}
}
} catch (UpdateException | RestApiException e) {
// Propagate REST API exceptions thrown by operations; they commonly throw
@ -638,13 +640,17 @@ public class BatchUpdate implements AutoCloseable {
return this;
}
public Collection<ReceiveCommand> getRefUpdates() {
return commands.getCommands().values();
}
public void execute() throws UpdateException, RestApiException {
execute(Listener.NONE);
}
public void execute(Listener listener)
throws UpdateException, RestApiException {
execute(ImmutableList.of(this), listener, requestId);
execute(ImmutableList.of(this), listener, requestId, false);
}
private void executeUpdateRepo() throws UpdateException, RestApiException {
@ -674,7 +680,8 @@ public class BatchUpdate implements AutoCloseable {
}
}
private void executeRefUpdates() throws IOException, UpdateException {
private void executeRefUpdates(boolean dryrun)
throws IOException, UpdateException {
if (commands == null || commands.isEmpty()) {
logDebug("No ref updates to execute");
return;
@ -685,6 +692,10 @@ public class BatchUpdate implements AutoCloseable {
commands.addTo(batchRefUpdate);
logDebug("Executing batch of {} ref updates",
batchRefUpdate.getCommands().size());
if (dryrun) {
return;
}
batchRefUpdate.execute(revWalk, NullProgressMonitor.INSTANCE);
boolean ok = true;
for (ReceiveCommand cmd : batchRefUpdate.getCommands()) {
@ -698,8 +709,9 @@ public class BatchUpdate implements AutoCloseable {
}
}
private void executeChangeOps(boolean parallel)
throws UpdateException, RestApiException {
private void executeChangeOps(boolean parallel,
boolean dryrun) throws UpdateException,
RestApiException {
logDebug("Executing change ops (parallel? {})", parallel);
ListeningExecutorService executor = parallel
? changeUpdateExector
@ -722,7 +734,8 @@ public class BatchUpdate implements AutoCloseable {
List<ListenableFuture<?>> futures = new ArrayList<>(ops.keySet().size());
for (Map.Entry<Change.Id, Collection<Op>> e : ops.asMap().entrySet()) {
ChangeTask task =
new ChangeTask(e.getKey(), e.getValue(), Thread.currentThread());
new ChangeTask(e.getKey(), e.getValue(), Thread.currentThread(),
dryrun);
tasks.add(task);
if (!parallel) {
logDebug("Direct execution of task for ops: {}", ops);
@ -740,7 +753,9 @@ public class BatchUpdate implements AutoCloseable {
if (notesMigration.commitChangeWrites()) {
startNanos = System.nanoTime();
executeNoteDbUpdates(tasks);
if (!dryrun) {
executeNoteDbUpdates(tasks);
}
maybeLogSlowUpdate(startNanos, "NoteDb");
}
} catch (ExecutionException | InterruptedException e) {
@ -872,6 +887,7 @@ public class BatchUpdate implements AutoCloseable {
final Change.Id id;
private final Collection<Op> changeOps;
private final Thread mainThread;
private final boolean dryrun;
NoteDbUpdateManager.StagedResult noteDbResult;
boolean dirty;
@ -879,10 +895,11 @@ public class BatchUpdate implements AutoCloseable {
private String taskId;
private ChangeTask(Change.Id id, Collection<Op> changeOps,
Thread mainThread) {
Thread mainThread, boolean dryrun) {
this.id = id;
this.changeOps = changeOps;
this.mainThread = mainThread;
this.dryrun = dryrun;
}
@Override
@ -951,7 +968,9 @@ public class BatchUpdate implements AutoCloseable {
logDebug("Updating change");
db.changes().update(cs);
}
db.commit();
if (!dryrun) {
db.commit();
}
} finally {
db.rollback();
}

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

@ -230,6 +230,8 @@ public class MergeOp implements AutoCloseable {
private CommitStatus commits;
private ReviewDb db;
private SubmitInput submitInput;
private Set<Project.NameKey> allProjects;
private boolean dryrun;
@Inject
MergeOp(ChangeMessagesUtil cmUtil,
@ -400,10 +402,26 @@ public class MergeOp implements AutoCloseable {
}
}
/**
* Merges the given change.
*
* Depending on the server configuration, more changes may be affected, e.g.
* by submission of a topic or via superproject subscriptions. All affected
* changes are integrated using the projects integration strategy.
*
* @param db the review database.
* @param change the change to be merged.
* @param caller the identity of the caller
* @param checkSubmitRules whether the prolog submit rules should be evaluated
* @param submitInput parameters regarding the merge
* @throws OrmException an error occurred reading or writing the database.
* @throws RestApiException if an error occurred.
*/
public void merge(ReviewDb db, Change change, IdentifiedUser caller,
boolean checkSubmitRules, SubmitInput submitInput)
boolean checkSubmitRules, SubmitInput submitInput, boolean dryrun)
throws OrmException, RestApiException {
this.submitInput = submitInput;
this.dryrun = dryrun;
this.caller = caller;
this.ts = TimeUtil.nowTs();
submissionId = RequestId.forChange(change);
@ -470,16 +488,17 @@ public class MergeOp implements AutoCloseable {
commits.maybeFailVerbose();
SubmoduleOp submoduleOp = subOpFactory.create(branches, orm);
try {
List<SubmitStrategy> strategies = getSubmitStrategies(toSubmit, submoduleOp);
List<SubmitStrategy> strategies = getSubmitStrategies(toSubmit,
submoduleOp, dryrun);
Set<Project.NameKey> allProjects = submoduleOp.getProjectsInOrder();
// in case superproject subscription is disabled, allProjects would be null
if (allProjects == null) {
allProjects = projects;
}
BatchUpdate.execute(
orm.batchUpdates(allProjects),
this.allProjects = allProjects;
BatchUpdate.execute(orm.batchUpdates(allProjects),
new SubmitStrategyListener(submitInput, strategies, commits),
submissionId);
submissionId, dryrun);
} catch (UpdateException | SubmoduleException e) {
// BatchUpdate may have inadvertently wrapped an IntegrationException
// thrown by some legacy SubmitStrategyOp code that intended the error
@ -499,9 +518,17 @@ public class MergeOp implements AutoCloseable {
}
}
public Set<Project.NameKey> getAllProjects() {
return allProjects;
}
public MergeOpRepoManager getMergeOpRepoManager() {
return orm;
}
private List<SubmitStrategy> getSubmitStrategies(
Map<Branch.NameKey, BranchBatch> toSubmit, SubmoduleOp submoduleOp)
throws IntegrationException {
Map<Branch.NameKey, BranchBatch> toSubmit, SubmoduleOp submoduleOp,
boolean dryrun) throws IntegrationException {
List<SubmitStrategy> strategies = new ArrayList<>();
Set<Branch.NameKey> allBranches = submoduleOp.getBranchesInOrder();
// in case superproject subscription is disabled, allBranches would be null
@ -520,7 +547,7 @@ public class MergeOp implements AutoCloseable {
Set<CodeReviewCommit> commitsToSubmit = commits(submitting.changes());
ob.mergeTip = new MergeTip(ob.oldTip, commitsToSubmit);
SubmitStrategy strategy = createStrategy(or, ob.mergeTip, branch,
submitting.submitType(), ob.oldTip, submoduleOp);
submitting.submitType(), ob.oldTip, submoduleOp, dryrun);
strategies.add(strategy);
strategy.addOps(or.getUpdate(), commitsToSubmit);
} else {
@ -546,10 +573,12 @@ public class MergeOp implements AutoCloseable {
private SubmitStrategy createStrategy(OpenRepo or,
MergeTip mergeTip, Branch.NameKey destBranch, SubmitType submitType,
CodeReviewCommit branchTip, SubmoduleOp submoduleOp) throws IntegrationException {
CodeReviewCommit branchTip, SubmoduleOp submoduleOp, boolean dryrun)
throws IntegrationException {
return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins,
or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller,
mergeTip, commits, submissionId, submitInput.notify, submoduleOp);
mergeTip, commits, submissionId, submitInput.notify, submoduleOp,
dryrun);
}
private Set<RevCommit> getAlreadyAccepted(OpenRepo or,

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

@ -2123,7 +2123,7 @@ public class ReceiveCommits {
logDebug("Processing submit with tip change {} ({})",
tipChange.getId(), magicBranch.cmd.getNewId());
try (MergeOp op = mergeOpProvider.get()) {
op.merge(db, tipChange, user, false, new SubmitInput());
op.merge(db, tipChange, user, false, new SubmitInput(), false);
}
}

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

@ -334,7 +334,7 @@ public class SubmoduleOp {
}
}
BatchUpdate.execute(orm.batchUpdates(superProjects), Listener.NONE,
orm.getSubmissionId());
orm.getSubmissionId(), false);
} catch (RestApiException | UpdateException | IOException |
NoSuchProjectException e) {
throw new SubmoduleException("Cannot update gitlinks", e);

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

@ -96,7 +96,8 @@ public abstract class SubmitStrategy {
Set<RevCommit> alreadyAccepted,
RequestId submissionId,
NotifyHandling notifyHandling,
SubmoduleOp submoduleOp);
SubmoduleOp submoduleOp,
boolean dryrun);
}
final AccountCache accountCache;
@ -133,6 +134,7 @@ public abstract class SubmitStrategy {
final ProjectState project;
final MergeSorter mergeSorter;
final MergeUtil mergeUtil;
final boolean dryrun;
@AssistedInject
Arguments(
@ -165,7 +167,8 @@ public abstract class SubmitStrategy {
@Assisted RequestId submissionId,
@Assisted SubmitType submitType,
@Assisted NotifyHandling notifyHandling,
@Assisted SubmoduleOp submoduleOp) {
@Assisted SubmoduleOp submoduleOp,
@Assisted boolean dryrun) {
this.accountCache = accountCache;
this.approvalsUtil = approvalsUtil;
this.batchUpdateFactory = batchUpdateFactory;
@ -196,6 +199,7 @@ public abstract class SubmitStrategy {
this.submitType = submitType;
this.notifyHandling = notifyHandling;
this.submoduleOp = submoduleOp;
this.dryrun = dryrun;
this.project = checkNotNull(projectCache.get(destBranch.getParentKey()),
"project not found: %s", destBranch.getParentKey());

6
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java
View File

@ -54,12 +54,12 @@ public class SubmitStrategyFactory {
Repository repo, CodeReviewRevWalk rw, ObjectInserter inserter,
RevFlag canMergeFlag, Set<RevCommit> alreadyAccepted,
Branch.NameKey destBranch, IdentifiedUser caller, MergeTip mergeTip,
CommitStatus commits, RequestId submissionId, NotifyHandling notifyHandling,
SubmoduleOp submoduleOp)
CommitStatus commits, RequestId submissionId,
NotifyHandling notifyHandling, SubmoduleOp submoduleOp, boolean dryrun)
throws IntegrationException {
SubmitStrategy.Arguments args = argsFactory.create(submitType, destBranch,
commits, rw, caller, mergeTip, inserter, repo, canMergeFlag, db,
alreadyAccepted, submissionId, notifyHandling, submoduleOp);
alreadyAccepted, submissionId, notifyHandling, submoduleOp, dryrun);
switch (submitType) {
case CHERRY_PICK:
return new CherryPick(args);

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

@ -524,7 +524,7 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op {
} catch (Exception e) {
log.error("Cannot email merged notification for " + getId(), e);
}
if (mergeResultRev != null) {
if (mergeResultRev != null && !args.dryrun) {
args.changeMerged.fire(
updatedChange,
mergedPatchSet,