commit c3f9993d54c5c48dc6271366027d6fec130d2c90 Author: Ivan Frade Date: Thu Sep 24 11:29:39 2020 -0700 Use submission to update superprojects Change-Id: I9a3a9fd4516892cd51f1b0c27bdb8c5074321e20 diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 00571b8..cd378b2 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -172,6 +172,8 @@ import com.google.gerrit.server.update.Context; import com.google.gerrit.server.update.RepoContext; import com.google.gerrit.server.update.RepoOnlyOp; import com.google.gerrit.server.update.RetryHelper; +import com.google.gerrit.server.update.SubmissionExecutor; +import com.google.gerrit.server.update.SuperprojectUpdateSubmissionListener; import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.util.LabelVote; import com.google.gerrit.server.util.MagicBranch; @@ -725,7 +727,8 @@ class ReceiveCommits { project.getNameKey(), user.materializedCopy(), TimeUtil.nowTs()); ObjectInserter ins = repo.newObjectInserter(); ObjectReader reader = ins.newReader(); - RevWalk rw = new RevWalk(reader)) { + RevWalk rw = new RevWalk(reader); + MergeOpRepoManager orm = ormProvider.get()) { bu.setRepository(repo, rw, ins); bu.setRefLogMessage("push"); @@ -737,21 +740,28 @@ class ReceiveCommits { } } logger.atFine().log("Added %d additional ref updates", added); - bu.execute(); - branches = bu.getSuccessfullyUpdatedBranches(/* dryrun=*/ false); + + SubmissionExecutor submissionExecutor = + new SubmissionExecutor(false, new SuperprojectUpdateSubmissionListener(subOpFactory)); + + submissionExecutor.execute(ImmutableList.of(bu)); + + orm.setContext(TimeUtil.nowTs(), user, NotifyResolver.Result.none()); + submissionExecutor.afterExecutions(orm); + + branches = bu.getSuccessfullyUpdatedBranches(false); } catch (UpdateException | RestApiException e) { throw new StorageException(e); } + // This could be moved into a SubmissionListener branches.values().stream() .filter(c -> isHead(c) || isConfig(c)) .forEach( c -> { // Most post-update steps should happen in UpdateOneRefOp#postUpdate. The only steps - // that - // should happen in this loops are things that can't happen within one BatchUpdate - // because - // they involve kicking off an additional BatchUpdate. + // that should happen in this loops are things that can't happen within one + // BatchUpdate because they involve kicking off an additional BatchUpdate. switch (c.getType()) { case CREATE: case UPDATE: @@ -765,17 +775,6 @@ class ReceiveCommits { break; } }); - - // Update superproject gitlinks if required. - if (!branches.isEmpty()) { - try (MergeOpRepoManager orm = ormProvider.get()) { - orm.setContext(TimeUtil.nowTs(), user, NotifyResolver.Result.none()); - SubmoduleOp op = subOpFactory.create(branches, orm); - op.updateSuperProjects(false); - } catch (RestApiException e) { - logger.atWarning().withCause(e).log("Can't update the superprojects"); - } - } } } diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java index c53402a..a65d297 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -77,6 +77,9 @@ import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.RetryHelper; +import com.google.gerrit.server.update.SubmissionExecutor; +import com.google.gerrit.server.update.SubmissionListener; +import com.google.gerrit.server.update.SuperprojectUpdateSubmissionListener; import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.util.time.TimeUtil; import com.google.inject.Inject; @@ -98,7 +101,6 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.transport.ReceiveCommand; /** * Merges changes in submission order into a single branch. @@ -238,8 +240,6 @@ public class MergeOp implements AutoCloseable { // Changes that were updated by this MergeOp. private final Map updatedChanges; - // Refs that were updated by this MergeOp. - private final Map updatedRefs; private Timestamp ts; private SubmissionId submissionId; @@ -286,7 +286,6 @@ public class MergeOp implements AutoCloseable { this.topicMetrics = topicMetrics; this.changeDataFactory = changeDataFactory; this.updatedChanges = new HashMap<>(); - this.updatedRefs = new HashMap<>(); } @Override @@ -498,6 +497,10 @@ public class MergeOp implements AutoCloseable { topicMetrics.topicSubmissions.increment(); } + SubmissionListener updateSuperprojectsAfterSubmission = + new SuperprojectUpdateSubmissionListener(submoduleOpFactory); + SubmissionExecutor submissionExecutor = + new SubmissionExecutor(dryrun, updateSuperprojectsAfterSubmission); RetryTracker retryTracker = new RetryTracker(); retryHelper .changeUpdate( @@ -518,7 +521,7 @@ public class MergeOp implements AutoCloseable { logger.atFine().log("Bypassing submit rules"); bypassSubmitRules(cs, isRetry); } - integrateIntoHistory(cs); + integrateIntoHistory(cs, submissionExecutor); return null; }) .listener(retryTracker) @@ -527,9 +530,7 @@ public class MergeOp implements AutoCloseable { // submit. .defaultTimeoutMultiplier(cs.projects().size()) .call(); - - SubmoduleOp submoduleOp = submoduleOpFactory.create(updatedRefs, orm); - submoduleOp.updateSuperProjects(dryrun); + submissionExecutor.afterExecutions(orm); if (projects > 1) { topicMetrics.topicSubmissionsCompleted.increment(); @@ -595,7 +596,8 @@ public class MergeOp implements AutoCloseable { } } - private void integrateIntoHistory(ChangeSet cs) throws RestApiException, UpdateException { + private void integrateIntoHistory(ChangeSet cs, SubmissionExecutor submissionExecutor) + throws RestApiException, UpdateException { checkArgument(!cs.furtherHiddenChanges(), "cannot integrate hidden changes into history"); logger.atFine().log("Beginning merge attempt on %s", cs); Map toSubmit = new HashMap<>(); @@ -628,16 +630,14 @@ public class MergeOp implements AutoCloseable { this.allProjects = updateOrderCalculator.getProjectsInOrder(); List batchUpdates = orm.batchUpdates(allProjects); try { - BatchUpdate.execute( - batchUpdates, - ImmutableList.of(new SubmitStrategyListener(submitInput, strategies, commitStatus)), - dryrun); + submissionExecutor.setAdditionalBatchUpdateListeners( + ImmutableList.of(new SubmitStrategyListener(submitInput, strategies, commitStatus))); + submissionExecutor.execute(batchUpdates); } finally { // If the BatchUpdate fails it can be that merging some of the changes was actually - // successful. This is why we must to collect the updated changes and refs also when an + // successful. This is why we must to collect the updated changes also when an // exception was thrown. strategies.forEach(s -> updatedChanges.putAll(s.getUpdatedChanges())); - batchUpdates.forEach(bu -> updatedRefs.putAll(bu.getSuccessfullyUpdatedBranches(dryrun))); // Do not leave executed BatchUpdates in the OpenRepos if (!dryrun) { diff --git a/java/com/google/gerrit/server/update/SuperprojectUpdateSubmissionListener.java b/java/com/google/gerrit/server/update/SuperprojectUpdateSubmissionListener.java new file mode 100644 index 0000000..d6dca38 --- /dev/null +++ b/java/com/google/gerrit/server/update/SuperprojectUpdateSubmissionListener.java @@ -0,0 +1,83 @@ +// Copyright (C) 2020 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.update; + +import com.google.common.collect.ImmutableList; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.entities.BranchNameKey; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.server.submit.MergeOpRepoManager; +import com.google.gerrit.server.submit.SubmoduleOp; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.eclipse.jgit.transport.ReceiveCommand; + +/** Update superprojects after submission is done */ +public class SuperprojectUpdateSubmissionListener implements SubmissionListener { + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + private final SubmoduleOp.Factory subOpFactory; + private final Map updatedBranches = new HashMap<>(); + private ImmutableList batchUpdates = ImmutableList.of(); + private boolean dryrun; + + public SuperprojectUpdateSubmissionListener(SubmoduleOp.Factory subOpFactory) { + this.subOpFactory = subOpFactory; + } + + @Override + public void setDryrun() { + this.dryrun = true; + } + + @Override + public void beforeBatchUpdates(Collection updates) { + if (!batchUpdates.isEmpty()) { + // This is a retry. Save previous updates, as they are not in the new BatchUpdate. + collectSuccessfullUpdates(); + } + this.batchUpdates = ImmutableList.copyOf(updates); + } + + @Override + public void afterSubmission(MergeOpRepoManager orm) { + collectSuccessfullUpdates(); + // Update superproject gitlinks if required. + if (!updatedBranches.isEmpty()) { + try { + SubmoduleOp op = subOpFactory.create(updatedBranches, orm); + op.updateSuperProjects(dryrun); + } catch (RestApiException e) { + logger.atWarning().withCause(e).log("Can't update the superprojects"); + } + } + } + + @Override + public Optional listensToBatchUpdates() { + return Optional.empty(); + } + + private void collectSuccessfullUpdates() { + if (!this.batchUpdates.isEmpty()) { + for (BatchUpdate bu : batchUpdates) { + updatedBranches.putAll(bu.getSuccessfullyUpdatedBranches(dryrun)); + } + } + } +}