Do not delete branches concurrently.

Deleting multiple branches from the UI was resulting in a 500 error when
branches are in the packed-refs. This was happening because for each
branch to be deleted, a separate DELETE request was fired to the server.
The concurrent deletion was failing because JGit was not able to lock
the pack-ref file to remove the branch from it since the first deletion
that was getting executed already locked the file.

Add REST API call to delete multiple branches and modify
ProjectApi.deleteBranch to use that new call. When multiple branches
are deleted from the UI, one request is sent to the server to delete
all the selected branches.

Bug: issue 2706
Change-Id: I786533eb57048b1c9c67a386f0be37bf1a39853f
This commit is contained in:
Hugo Arès
2015-01-12 15:09:06 -05:00
parent b67addf513
commit ef8e320808
5 changed files with 251 additions and 13 deletions

View File

@@ -1121,6 +1121,38 @@ Deletes a branch.
HTTP/1.1 204 No Content
----
[[delete-branches]]
=== Delete Branches
--
'POST /projects/link:#project-name[\{project-name\}]/branches:delete'
--
Delete one or more branches.
The branches to be deleted must be provided in the request body as a
link:#delete-branches-input[DeleteBranchesInput] entity.
.Request
----
POST /projects/MyProject/branches:delete HTTP/1.0
Content-Type: application/json;charset=UTF-8
{
"branches": [
"stable-1.0",
"stable-2.0"
]
}
----
.Response
----
HTTP/1.1 204 No Content
----
If some branches could not be deleted, the response is "`409 Conflict`" and the
error message is contained in the response body.
[[get-content]]
=== Get Content
--
@@ -2060,6 +2092,18 @@ in a dashboard.
Tokens such as `${project}` are not resolved.
|===========================
[[delete-branches-input]]
=== DeleteBranchesInput
The `DeleteBranchesInput` entity contains information about branches that should
be deleted.
[options="header",width="50%",cols="1,6"]
|==========================
|Field Name |Description
|`branches` |A list of branch names that identify the branches that should be
deleted.
|==========================
[[gc-input]]
=== GCInput
The `GCInput` entity contains information to run the Git garbage

View File

@@ -15,7 +15,6 @@ package com.google.gerrit.client.projects;
import com.google.gerrit.client.VoidResult;
import com.google.gerrit.client.projects.ConfigInfo.ConfigParameterValue;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.NativeMap;
import com.google.gerrit.client.rpc.NativeString;
import com.google.gerrit.client.rpc.RestApi;
@@ -60,21 +59,20 @@ public class ProjectApi {
}
/**
* Delete branches. For each branch to be deleted a separate DELETE request is
* fired to the server. The {@code onSuccess} method of the provided callback
* is invoked once after all requests succeeded. If any request fails the
* callbacks' {@code onFailure} method is invoked. In a failure case it can be
* that still some of the branches were successfully deleted.
* Delete branches. One call is fired to the server to delete all the
* branches.
*/
public static void deleteBranches(Project.NameKey name,
Set<String> refs, AsyncCallback<VoidResult> cb) {
CallbackGroup group = new CallbackGroup();
for (String ref : refs) {
project(name).view("branches").id(ref)
.delete(group.add(cb));
cb = CallbackGroup.emptyCallback();
if (refs.size() == 1) {
project(name).view("branches").id(refs.iterator().next()).delete(cb);
} else {
DeleteBranchesInput d = DeleteBranchesInput.create();
for (String ref : refs) {
d.add_branch(ref);
}
project(name).view("branches:delete").post(d, cb);
}
group.done();
}
public static void getConfig(Project.NameKey name,
@@ -292,4 +290,18 @@ public class ProjectApi {
final native void setRef(String r) /*-{ if(r)this.ref=r; }-*/;
}
private static class DeleteBranchesInput extends JavaScriptObject {
static DeleteBranchesInput create() {
DeleteBranchesInput d = createObject().cast();
d.init();
return d;
}
protected DeleteBranchesInput() {
}
final native void init() /*-{ this.branches = []; }-*/;
final native void add_branch(String b) /*-{ this.branches.push(b); }-*/;
}
}

View File

@@ -104,7 +104,7 @@ public class DeleteBranch implements RestModifyView<BranchResource, Input>{
break;
case REJECTED_CURRENT_BRANCH:
log.warn("Cannot delete " + rsrc.getBranchKey() + ": " + result.name());
log.error("Cannot delete " + rsrc.getBranchKey() + ": " + result.name());
throw new ResourceConflictException("cannot delete current branch");
default:

View File

@@ -0,0 +1,181 @@
// Copyright (C) 2015 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.project;
import static java.lang.String.format;
import com.google.common.collect.Lists;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.SubmoduleSubscription;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.DeleteBranches.Input;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.List;
@Singleton
class DeleteBranches implements RestModifyView<ProjectResource, Input> {
private static final Logger log = LoggerFactory.getLogger(DeleteBranches.class);
static class Input {
List<String> branches;
static Input init(Input in) {
if (in == null) {
in = new Input();
}
if (in.branches == null) {
in.branches = Lists.newArrayListWithCapacity(1);
}
return in;
}
}
private final Provider<IdentifiedUser> identifiedUser;
private final GitRepositoryManager repoManager;
private final Provider<ReviewDb> dbProvider;
private final Provider<InternalChangeQuery> queryProvider;
private final GitReferenceUpdated referenceUpdated;
private final ChangeHooks hooks;
@Inject
DeleteBranches(Provider<IdentifiedUser> identifiedUser,
GitRepositoryManager repoManager,
Provider<ReviewDb> dbProvider,
Provider<InternalChangeQuery> queryProvider,
GitReferenceUpdated referenceUpdated,
ChangeHooks hooks) {
this.identifiedUser = identifiedUser;
this.repoManager = repoManager;
this.dbProvider = dbProvider;
this.queryProvider = queryProvider;
this.referenceUpdated = referenceUpdated;
this.hooks = hooks;
}
@Override
public Response<?> apply(ProjectResource project, Input input)
throws OrmException, IOException, ResourceConflictException {
input = Input.init(input);
Repository r = repoManager.openRepository(project.getNameKey());
try {
BatchRefUpdate batchUpdate = r.getRefDatabase().newBatchUpdate();
for (String branch : input.branches) {
batchUpdate.addCommand(createDeleteCommand(project, r, branch));
}
RevWalk rw = new RevWalk(r);
try {
batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
} finally {
rw.release();
}
StringBuilder errorMessages = new StringBuilder();
for (ReceiveCommand command : batchUpdate.getCommands()) {
if (command.getResult() == Result.OK) {
postDeletion(project, command);
} else {
appendAndLogErrorMessage(errorMessages, command);
}
}
if (errorMessages.length() > 0) {
throw new ResourceConflictException(errorMessages.toString());
}
} finally {
r.close();
}
return Response.none();
}
private ReceiveCommand createDeleteCommand(ProjectResource project,
Repository r, String branch) throws OrmException, IOException {
Ref ref = r.getRefDatabase().getRef(branch);
ReceiveCommand command;
if (ref == null) {
command = new ReceiveCommand(ObjectId.zeroId(), ObjectId.zeroId(), branch);
command.setResult(Result.REJECTED_OTHER_REASON,
"it doesn't exist or you do not have permission to delete it");
return command;
}
command =
new ReceiveCommand(ref.getObjectId(), ObjectId.zeroId(), ref.getName());
Branch.NameKey branchKey =
new Branch.NameKey(project.getNameKey(), ref.getName());
if (!project.getControl().controlForRef(branchKey).canDelete()) {
command.setResult(Result.REJECTED_OTHER_REASON,
"it doesn't exist or you do not have permission to delete it");
}
if (!queryProvider.get().setLimit(1).byBranchOpen(branchKey).isEmpty()) {
command.setResult(Result.REJECTED_OTHER_REASON, "it has open changes");
}
return command;
}
private void appendAndLogErrorMessage(StringBuilder errorMessages,
ReceiveCommand cmd) {
String msg = null;
switch (cmd.getResult()) {
case REJECTED_CURRENT_BRANCH:
msg = format("Cannot delete %s: it is the current branch",
cmd.getRefName());
break;
case REJECTED_OTHER_REASON:
msg = format("Cannot delete %s: %s", cmd.getRefName(), cmd.getMessage());
break;
default:
msg = format("Cannot delete %s: %s", cmd.getRefName(), cmd.getResult());
break;
}
log.error(msg);
errorMessages.append(msg);
errorMessages.append("\n");
}
private void postDeletion(ProjectResource project, ReceiveCommand cmd)
throws OrmException {
referenceUpdated.fire(project.getNameKey(), cmd.getRefName(),
cmd.getOldId(), cmd.getNewId());
Branch.NameKey branchKey =
new Branch.NameKey(project.getNameKey(), cmd.getRefName());
hooks.doRefUpdatedHook(branchKey, cmd.getOldId(), cmd.getNewId(),
identifiedUser.get().getAccount());
ResultSet<SubmoduleSubscription> submoduleSubscriptions =
dbProvider.get().submoduleSubscriptions().bySuperProject(branchKey);
dbProvider.get().submoduleSubscriptions().delete(submoduleSubscriptions);
}
}

View File

@@ -64,6 +64,7 @@ public class Module extends RestApiModule {
put(BRANCH_KIND).to(PutBranch.class);
get(BRANCH_KIND).to(GetBranch.class);
delete(BRANCH_KIND).to(DeleteBranch.class);
post(PROJECT_KIND, "branches:delete").to(DeleteBranches.class);
install(new FactoryModuleBuilder().build(CreateBranch.Factory.class));
get(BRANCH_KIND, "reflog").to(GetReflog.class);
child(BRANCH_KIND, "files").to(FilesCollection.class);