diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 2ce7446d0f..263d06c85a 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -163,7 +163,6 @@ import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.submit.MergeOp; import com.google.gerrit.server.submit.MergeOpRepoManager; -import com.google.gerrit.server.submit.SubmoduleException; import com.google.gerrit.server.submit.SubmoduleOp; import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdateOp; @@ -763,8 +762,8 @@ class ReceiveCommits { orm.setContext(TimeUtil.nowTs(), user, NotifyResolver.Result.none()); SubmoduleOp op = subOpFactory.create(branches, orm); op.updateSuperProjects(); - } catch (SubmoduleException e) { - logger.atSevere().withCause(e).log("Can't update the superprojects"); + } 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 800fe0e143..dc24a9d877 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -622,7 +622,7 @@ public class MergeOp implements AutoCloseable { throw new ResourceNotFoundException(e.getMessage()); } catch (IOException e) { throw new StorageException(e); - } catch (SubmoduleException e) { + } catch (SubmoduleConflictException e) { throw new IntegrationConflictException(e.getMessage(), e); } catch (UpdateException e) { if (e.getCause() instanceof LockFailureException) { diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index 16255280a8..a4141be119 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -552,10 +552,13 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { return args.submoduleOp.composeGitlinksCommit(args.destBranch, commit); } catch (IOException e) { throw new StorageException( - "cannot update gitlink for the commit at branch: " + args.destBranch, e); - } catch (SubmoduleException e) { + String.format("cannot update gitlink for the commit at branch %s", args.destBranch), e); + } catch (SubmoduleConflictException e) { throw new IntegrationConflictException( - "cannot update gitlink for the commit at branch: " + args.destBranch, e); + String.format( + "cannot update gitlink for the commit at branch %s: %s", + args.destBranch, e.getMessage()), + e); } } } diff --git a/java/com/google/gerrit/server/submit/SubmoduleException.java b/java/com/google/gerrit/server/submit/SubmoduleConflictException.java similarity index 56% rename from java/com/google/gerrit/server/submit/SubmoduleException.java rename to java/com/google/gerrit/server/submit/SubmoduleConflictException.java index 2367d0a520..23f583e9cd 100644 --- a/java/com/google/gerrit/server/submit/SubmoduleException.java +++ b/java/com/google/gerrit/server/submit/SubmoduleConflictException.java @@ -1,4 +1,4 @@ -// Copyright (C) 2011 The Android Open Source Project +// 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. @@ -14,19 +14,18 @@ package com.google.gerrit.server.submit; +import com.google.gerrit.extensions.restapi.ResourceConflictException; + /** - * Indicates the gitlink's update cannot be processed at this time. + * Exception to be thrown if any submodule operation is not possible due to conflicts. * - *

Message should be considered user-visible. + *

Throwing this exception results in a {@code 409 Conflict} response to the calling user. The + * exception message is returned as error message to the user. */ -public class SubmoduleException extends Exception { +public class SubmoduleConflictException extends ResourceConflictException { private static final long serialVersionUID = 1L; - SubmoduleException(String msg) { - super(msg, null); - } - - SubmoduleException(String msg, Throwable why) { - super(msg, why); + public SubmoduleConflictException(String msg) { + super(msg); } } diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java index 9c0bc77507..b48076194c 100644 --- a/java/com/google/gerrit/server/submit/SubmoduleOp.java +++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java @@ -28,6 +28,7 @@ import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; import com.google.gerrit.entities.SubmoduleSubscription; +import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.config.GerritServerConfig; @@ -116,7 +117,7 @@ public class SubmoduleOp { } public SubmoduleOp create(Set updatedBranches, MergeOpRepoManager orm) - throws SubmoduleException { + throws SubmoduleConflictException { return new SubmoduleOp( gitmodulesFactory, serverIdent.get(), cfg, projectCache, updatedBranches, orm); } @@ -166,7 +167,7 @@ public class SubmoduleOp { ProjectCache projectCache, Set updatedBranches, MergeOpRepoManager orm) - throws SubmoduleException { + throws SubmoduleConflictException { this.gitmodulesFactory = gitmodulesFactory; this.myIdent = myIdent; this.projectCache = projectCache; @@ -199,7 +200,6 @@ public class SubmoduleOp { * * * @return the ordered set to be stored in {@link #sortedBranches}. - * @throws SubmoduleException if an error occurred walking projects. */ // TODO(dborowitz): This setup process is hard to follow, in large part due to the accumulation of // mutable maps, which makes this whole class difficult to understand. @@ -215,7 +215,8 @@ public class SubmoduleOp { // // In addition to improving readability, this approach has the advantage of making (1) and (2) // testable using small tests. - private ImmutableSet calculateSubscriptionMaps() throws SubmoduleException { + private ImmutableSet calculateSubscriptionMaps() + throws SubmoduleConflictException { if (!enableSuperProjectSubscriptions) { logger.atFine().log("Updating superprojects disabled"); return null; @@ -244,11 +245,11 @@ public class SubmoduleOp { BranchNameKey current, LinkedHashSet currentVisited, LinkedHashSet allVisited) - throws SubmoduleException { + throws SubmoduleConflictException { logger.atFine().log("Now processing %s", current); if (currentVisited.contains(current)) { - throw new SubmoduleException( + throw new SubmoduleConflictException( "Branch level circular subscriptions detected: " + printCircularPath(currentVisited, current)); } @@ -270,7 +271,7 @@ public class SubmoduleOp { affectedBranches.add(sub.getSubmodule()); } } catch (IOException e) { - throw new SubmoduleException("Cannot find superprojects for " + current, e); + throw new StorageException("Cannot find superprojects for " + current, e); } currentVisited.remove(current); allVisited.add(current); @@ -396,7 +397,7 @@ public class SubmoduleOp { return ret; } - public void updateSuperProjects() throws SubmoduleException { + public void updateSuperProjects() throws RestApiException { ImmutableSet projects = getProjectsInOrder(); if (projects == null) { return; @@ -416,19 +417,19 @@ public class SubmoduleOp { } } BatchUpdate.execute(orm.batchUpdates(superProjects), BatchUpdateListener.NONE, false); - } catch (RestApiException | UpdateException | IOException | NoSuchProjectException e) { - throw new SubmoduleException("Cannot update gitlinks", e); + } catch (UpdateException | IOException | NoSuchProjectException e) { + throw new StorageException("Cannot update gitlinks", e); } } /** Create a separate gitlink commit */ private CodeReviewCommit composeGitlinksCommit(BranchNameKey subscriber) - throws IOException, SubmoduleException { + throws IOException, SubmoduleConflictException { OpenRepo or; try { or = orm.getRepo(subscriber.project()); } catch (NoSuchProjectException | IOException e) { - throw new SubmoduleException("Cannot access superproject", e); + throw new StorageException("Cannot access superproject", e); } CodeReviewCommit currentCommit; @@ -437,7 +438,7 @@ public class SubmoduleOp { } else { Ref r = or.repo.exactRef(subscriber.branch()); if (r == null) { - throw new SubmoduleException( + throw new SubmoduleConflictException( "The branch was probably deleted from the subscriber repository"); } currentCommit = or.rw.parseCommit(r.getObjectId()); @@ -493,12 +494,12 @@ public class SubmoduleOp { /** Amend an existing commit with gitlink updates */ CodeReviewCommit composeGitlinksCommit(BranchNameKey subscriber, CodeReviewCommit currentCommit) - throws IOException, SubmoduleException { + throws IOException, SubmoduleConflictException { OpenRepo or; try { or = orm.getRepo(subscriber.project()); } catch (NoSuchProjectException | IOException e) { - throw new SubmoduleException("Cannot access superproject", e); + throw new StorageException("Cannot access superproject", e); } StringBuilder msgbuf = new StringBuilder(); @@ -534,13 +535,13 @@ public class SubmoduleOp { private RevCommit updateSubmodule( DirCache dc, DirCacheEditor ed, StringBuilder msgbuf, SubmoduleSubscription s) - throws SubmoduleException, IOException { + throws SubmoduleConflictException, IOException { logger.atFine().log("Updating gitlink for %s", s); OpenRepo subOr; try { subOr = orm.getRepo(s.getSubmodule().project()); } catch (NoSuchProjectException | IOException e) { - throw new SubmoduleException("Cannot access submodule", e); + throw new StorageException("Cannot access submodule", e); } DirCacheEntry dce = dc.getEntry(s.getPath()); @@ -554,7 +555,7 @@ public class SubmoduleOp { + s.getSubmodule().project().get() + " but entry " + "doesn't have gitlink file mode."; - throw new SubmoduleException(errMsg); + throw new SubmoduleConflictException(errMsg); } // Parse the current gitlink entry commit in the subproject repo. This is used to add a // shortlog for this submodule to the commit message in the superproject. @@ -617,8 +618,7 @@ public class SubmoduleOp { SubmoduleSubscription s, OpenRepo subOr, RevCommit newCommit, - RevCommit oldCommit) - throws SubmoduleException { + RevCommit oldCommit) { msgbuf.append("* Update "); msgbuf.append(s.getPath()); msgbuf.append(" from branch '"); @@ -659,7 +659,7 @@ public class SubmoduleOp { msgbuf.append(message); } } catch (IOException e) { - throw new SubmoduleException( + throw new StorageException( "Could not perform a revwalk to create superproject commit message", e); } } @@ -676,7 +676,7 @@ public class SubmoduleOp { return dc; } - ImmutableSet getProjectsInOrder() throws SubmoduleException { + ImmutableSet getProjectsInOrder() throws SubmoduleConflictException { LinkedHashSet projects = new LinkedHashSet<>(); for (Project.NameKey project : branchesByProject.keySet()) { addAllSubmoduleProjects(project, new LinkedHashSet<>(), projects); @@ -692,9 +692,9 @@ public class SubmoduleOp { Project.NameKey project, LinkedHashSet current, LinkedHashSet projects) - throws SubmoduleException { + throws SubmoduleConflictException { if (current.contains(project)) { - throw new SubmoduleException( + throw new SubmoduleConflictException( "Project level circular subscriptions detected: " + printCircularPath(current, project)); } diff --git a/plugins/delete-project b/plugins/delete-project index da0811ef34..e8a249ae1b 160000 --- a/plugins/delete-project +++ b/plugins/delete-project @@ -1 +1 @@ -Subproject commit da0811ef34ecdb4cb7183beec60085aff9acc3db +Subproject commit e8a249ae1b68f43d5941f0c1a64ccdbd0a395d4a