diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java index c1f99beb6b..43c10bf07c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java @@ -186,8 +186,6 @@ public class Submit implements RestModifyView, ReviewDb db = dbProvider.get(); op.merge(db, change, caller, true); change = db.changes().get(change.getId()); - } catch (NoSuchChangeException e) { - throw new OrmException("Submission failed", e); } if (change == null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 785cc88292..3545c9b8f5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -326,22 +326,27 @@ public class MergeOp implements AutoCloseable { commits = new CommitStatus(); } - private OpenRepo openRepo(Project.NameKey project) + private OpenRepo getRepo(Project.NameKey project) { + OpenRepo or = openRepos.get(project); + checkState(or != null, "repo not yet opened: %s", project); + return or; + } + + private void openRepo(Project.NameKey project) throws NoSuchProjectException, IOException { - OpenRepo repo = openRepos.get(project); - if (repo == null) { - ProjectState projectState = projectCache.get(project); - if (projectState == null) { - throw new NoSuchProjectException(project); - } - try { - repo = new OpenRepo(repoManager.openRepository(project), projectState); - } catch (RepositoryNotFoundException e) { - throw new NoSuchProjectException(project); - } - openRepos.put(project, repo); + checkState(!openRepos.containsKey(project), + "repo already opened: %s", project); + ProjectState projectState = projectCache.get(project); + if (projectState == null) { + throw new NoSuchProjectException(project); + } + try { + OpenRepo or = + new OpenRepo(repoManager.openRepository(project), projectState); + openRepos.put(project, or); + } catch (RepositoryNotFoundException e) { + throw new NoSuchProjectException(project); } - return repo; } @Override @@ -474,8 +479,7 @@ public class MergeOp implements AutoCloseable { } public void merge(ReviewDb db, Change change, IdentifiedUser caller, - boolean checkSubmitRules) throws NoSuchChangeException, - OrmException, ResourceConflictException { + boolean checkSubmitRules) throws OrmException, ResourceConflictException { this.caller = caller; updateSubmissionId(change); this.db = db; @@ -523,22 +527,22 @@ public class MergeOp implements AutoCloseable { } private void integrateIntoHistory(ChangeSet cs) - throws IntegrationException, NoSuchChangeException, - ResourceConflictException { + throws IntegrationException, ResourceConflictException { logDebug("Beginning merge attempt on {}", cs); Map toSubmit = new HashMap<>(); logDebug("Perform the merges"); try { Multimap br = cs.branchesByProject(); Multimap cbb = cs.changesByBranch(); + openRepos(br.keySet()); for (Branch.NameKey branch : cbb.keySet()) { - OpenRepo or = openRepo(branch.getParentKey()); + OpenRepo or = getRepo(branch.getParentKey()); toSubmit.put(branch, validateChangeList(or, cbb.get(branch))); } failFast(cs); // Done checks that don't involve running submit strategies. for (Branch.NameKey branch : cbb.keySet()) { - OpenRepo or = openRepo(branch.getParentKey()); + OpenRepo or = getRepo(branch.getParentKey()); OpenBranch ob = or.getBranch(branch); BranchBatch submitting = toSubmit.get(branch); SubmitStrategy strategy = createStrategy(or, branch, @@ -547,7 +551,7 @@ public class MergeOp implements AutoCloseable { } checkMergeStrategyResults(cs, toSubmit.values()); for (Project.NameKey project : br.keySet()) { - openRepo(project).ins.flush(); + getRepo(project).ins.flush(); } Set done = @@ -555,7 +559,7 @@ public class MergeOp implements AutoCloseable { logDebug("Write out the new branch tips"); SubmoduleOp subOp = subOpProvider.get(); for (Project.NameKey project : br.keySet()) { - OpenRepo or = openRepo(project); + OpenRepo or = getRepo(project); for (Branch.NameKey branch : br.get(project)) { OpenBranch ob = or.getBranch(branch); boolean updated = updateBranch(or, branch); @@ -574,10 +578,6 @@ public class MergeOp implements AutoCloseable { checkState(done.equals(cbb.keySet()), "programmer error: did not process" + " all branches in input set.\nExpected: %s\nActual: %s", done, cbb.keySet()); - } catch (NoSuchProjectException noProject) { - logWarn("Project " + noProject.project() + " no longer exists, " - + "abandoning open changes"); - abandonAllOpenChanges(noProject.project()); } catch (OrmException e) { throw new IntegrationException("Cannot query the database", e); } catch (IOException e) { @@ -602,8 +602,7 @@ public class MergeOp implements AutoCloseable { private SubmitStrategy createStrategy(OpenRepo or, Branch.NameKey destBranch, SubmitType submitType, - CodeReviewCommit branchTip) - throws IntegrationException, NoSuchProjectException { + CodeReviewCommit branchTip) throws IntegrationException { return submitStrategyFactory.create(submitType, db, or.repo, or.rw, or.ins, or.canMergeFlag, getAlreadyAccepted(or, branchTip), destBranch, caller, commits); @@ -1209,19 +1208,33 @@ public class MergeOp implements AutoCloseable { }); } - private void abandonAllOpenChanges(Project.NameKey destProject) - throws NoSuchChangeException { + private void openRepos(Collection projects) + throws IntegrationException { + for (Project.NameKey project : projects) { + try { + openRepo(project); + } catch (NoSuchProjectException noProject) { + logWarn("Project " + noProject.project() + " no longer exists, " + + "abandoning open changes"); + abandonAllOpenChanges(noProject.project()); + } catch (IOException e) { + throw new IntegrationException("Error opening project " + project, e); + } + } + } + + private void abandonAllOpenChanges(Project.NameKey destProject) { try { for (ChangeData cd : internalChangeQuery.byProjectOpen(destProject)) { abandonOneChange(cd.change()); } - } catch (IOException | OrmException e) { - logWarn("Cannot abandon changes for deleted project ", e); + } catch (NoSuchChangeException | IOException | OrmException e) { + logWarn("Cannot abandon changes for deleted project " + destProject, e); } } private void abandonOneChange(Change change) throws OrmException, - NoSuchChangeException, IOException { + NoSuchChangeException, IOException { db.changes().beginTransaction(change.getId()); //TODO(dborowitz): support InternalUser in ChangeUpdate diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 8defdb01dc..ce952d5139 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -1830,8 +1830,6 @@ public class ReceiveCommits { try (MergeOp op = mergeOpProvider.get()) { op.merge(db, rsrc.getChange(), changeCtl.getUser().asIdentifiedUser(), false); - } catch (NoSuchChangeException e) { - throw new OrmException(e); } addMessage(""); Change c = db.changes().get(rsrc.getChange().getId()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java index a8c912789e..3ec78064c4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java @@ -139,7 +139,8 @@ public abstract class SubmitStrategy { this.db = db; this.alreadyAccepted = alreadyAccepted; - this.project = projectCache.get(destBranch.getParentKey()); + this.project = checkNotNull(projectCache.get(destBranch.getParentKey()), + "project not found: %s", destBranch.getParentKey()); this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag); this.mergeUtil = mergeUtilFactory.create(project); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java index 65fee208d2..91396d84e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java @@ -21,7 +21,6 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.IntegrationException; import com.google.gerrit.server.git.MergeOp.CommitStatus; -import com.google.gerrit.server.project.NoSuchProjectException; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -51,12 +50,9 @@ public class SubmitStrategyFactory { Repository repo, CodeReviewRevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag, Set alreadyAccepted, Branch.NameKey destBranch, IdentifiedUser caller, CommitStatus commits) - throws IntegrationException, NoSuchProjectException { + throws IntegrationException { SubmitStrategy.Arguments args = argsFactory.create(destBranch, commits, rw, caller, inserter, repo, canMergeFlag, db, alreadyAccepted); - if (args.project == null) { - throw new NoSuchProjectException(destBranch.getParentKey()); - } switch (submitType) { case CHERRY_PICK: return new CherryPick(args);