Split off BatchAbandon from Abandon REST handler
Batch abandoning changes is not available over REST, so it is better suited to another place. Change-Id: I09562c696e005aee9ea265bb6d2b01a3019482b7
This commit is contained in:
@@ -84,7 +84,7 @@ import com.google.gerrit.server.account.AccountCache;
|
|||||||
import com.google.gerrit.server.account.Accounts;
|
import com.google.gerrit.server.account.Accounts;
|
||||||
import com.google.gerrit.server.account.GroupBackend;
|
import com.google.gerrit.server.account.GroupBackend;
|
||||||
import com.google.gerrit.server.account.GroupCache;
|
import com.google.gerrit.server.account.GroupCache;
|
||||||
import com.google.gerrit.server.change.Abandon;
|
import com.google.gerrit.server.change.BatchAbandon;
|
||||||
import com.google.gerrit.server.change.ChangeResource;
|
import com.google.gerrit.server.change.ChangeResource;
|
||||||
import com.google.gerrit.server.change.FileContentUtil;
|
import com.google.gerrit.server.change.FileContentUtil;
|
||||||
import com.google.gerrit.server.change.RevisionResource;
|
import com.google.gerrit.server.change.RevisionResource;
|
||||||
@@ -237,7 +237,7 @@ public abstract class AbstractDaemonTest {
|
|||||||
@Inject protected SystemGroupBackend systemGroupBackend;
|
@Inject protected SystemGroupBackend systemGroupBackend;
|
||||||
@Inject protected MutableNotesMigration notesMigration;
|
@Inject protected MutableNotesMigration notesMigration;
|
||||||
@Inject protected ChangeNotes.Factory notesFactory;
|
@Inject protected ChangeNotes.Factory notesFactory;
|
||||||
@Inject protected Abandon changeAbandoner;
|
@Inject protected BatchAbandon batchAbandon;
|
||||||
|
|
||||||
protected EventRecorder eventRecorder;
|
protected EventRecorder eventRecorder;
|
||||||
protected GerritServer server;
|
protected GerritServer server;
|
||||||
|
|||||||
@@ -23,18 +23,15 @@ import com.google.gerrit.extensions.api.changes.AbandonInput;
|
|||||||
import com.google.gerrit.extensions.api.changes.NotifyHandling;
|
import com.google.gerrit.extensions.api.changes.NotifyHandling;
|
||||||
import com.google.gerrit.extensions.api.changes.RecipientType;
|
import com.google.gerrit.extensions.api.changes.RecipientType;
|
||||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||||
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
|
||||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
import com.google.gerrit.extensions.webui.UiAction;
|
import com.google.gerrit.extensions.webui.UiAction;
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
import com.google.gerrit.reviewdb.client.Change;
|
import com.google.gerrit.reviewdb.client.Change;
|
||||||
import com.google.gerrit.reviewdb.client.Project;
|
|
||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
import com.google.gerrit.server.CurrentUser;
|
import com.google.gerrit.server.CurrentUser;
|
||||||
import com.google.gerrit.server.notedb.ChangeNotes;
|
import com.google.gerrit.server.notedb.ChangeNotes;
|
||||||
import com.google.gerrit.server.permissions.ChangePermission;
|
import com.google.gerrit.server.permissions.ChangePermission;
|
||||||
import com.google.gerrit.server.permissions.PermissionBackendException;
|
import com.google.gerrit.server.permissions.PermissionBackendException;
|
||||||
import com.google.gerrit.server.query.change.ChangeData;
|
|
||||||
import com.google.gerrit.server.update.BatchUpdate;
|
import com.google.gerrit.server.update.BatchUpdate;
|
||||||
import com.google.gerrit.server.update.RetryHelper;
|
import com.google.gerrit.server.update.RetryHelper;
|
||||||
import com.google.gerrit.server.update.RetryingRestModifyView;
|
import com.google.gerrit.server.update.RetryingRestModifyView;
|
||||||
@@ -44,7 +41,6 @@ import com.google.inject.Inject;
|
|||||||
import com.google.inject.Provider;
|
import com.google.inject.Provider;
|
||||||
import com.google.inject.Singleton;
|
import com.google.inject.Singleton;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.util.Collection;
|
|
||||||
import org.eclipse.jgit.errors.ConfigInvalidException;
|
import org.eclipse.jgit.errors.ConfigInvalidException;
|
||||||
|
|
||||||
@Singleton
|
@Singleton
|
||||||
@@ -132,69 +128,6 @@ public class Abandon extends RetryingRestModifyView<ChangeResource, AbandonInput
|
|||||||
return op.getChange();
|
return op.getChange();
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* If an extension has more than one changes to abandon that belong to the same project, they
|
|
||||||
* should use the batch instead of abandoning one by one.
|
|
||||||
*
|
|
||||||
* <p>It's the caller's responsibility to ensure that all jobs inside the same batch have the
|
|
||||||
* matching project from its ChangeData. Violations will result in a ResourceConflictException.
|
|
||||||
*/
|
|
||||||
public void batchAbandon(
|
|
||||||
BatchUpdate.Factory updateFactory,
|
|
||||||
Project.NameKey project,
|
|
||||||
CurrentUser user,
|
|
||||||
Collection<ChangeData> changes,
|
|
||||||
String msgTxt,
|
|
||||||
NotifyHandling notifyHandling,
|
|
||||||
ListMultimap<RecipientType, Account.Id> accountsToNotify)
|
|
||||||
throws RestApiException, UpdateException {
|
|
||||||
if (changes.isEmpty()) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
Account account = user.isIdentifiedUser() ? user.asIdentifiedUser().getAccount() : null;
|
|
||||||
try (BatchUpdate u = updateFactory.create(dbProvider.get(), project, user, TimeUtil.nowTs())) {
|
|
||||||
for (ChangeData change : changes) {
|
|
||||||
if (!project.equals(change.project())) {
|
|
||||||
throw new ResourceConflictException(
|
|
||||||
String.format(
|
|
||||||
"Project name \"%s\" doesn't match \"%s\"",
|
|
||||||
change.project().get(), project.get()));
|
|
||||||
}
|
|
||||||
u.addOp(
|
|
||||||
change.getId(),
|
|
||||||
abandonOpFactory.create(account, msgTxt, notifyHandling, accountsToNotify));
|
|
||||||
}
|
|
||||||
u.execute();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public void batchAbandon(
|
|
||||||
BatchUpdate.Factory updateFactory,
|
|
||||||
Project.NameKey project,
|
|
||||||
CurrentUser user,
|
|
||||||
Collection<ChangeData> changes,
|
|
||||||
String msgTxt)
|
|
||||||
throws RestApiException, UpdateException {
|
|
||||||
batchAbandon(
|
|
||||||
updateFactory,
|
|
||||||
project,
|
|
||||||
user,
|
|
||||||
changes,
|
|
||||||
msgTxt,
|
|
||||||
NotifyHandling.ALL,
|
|
||||||
ImmutableListMultimap.of());
|
|
||||||
}
|
|
||||||
|
|
||||||
public void batchAbandon(
|
|
||||||
BatchUpdate.Factory updateFactory,
|
|
||||||
Project.NameKey project,
|
|
||||||
CurrentUser user,
|
|
||||||
Collection<ChangeData> changes)
|
|
||||||
throws RestApiException, UpdateException {
|
|
||||||
batchAbandon(
|
|
||||||
updateFactory, project, user, changes, "", NotifyHandling.ALL, ImmutableListMultimap.of());
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public UiAction.Description getDescription(ChangeResource rsrc) {
|
public UiAction.Description getDescription(ChangeResource rsrc) {
|
||||||
Change change = rsrc.getChange();
|
Change change = rsrc.getChange();
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ public class AbandonUtil {
|
|||||||
private final ChangeCleanupConfig cfg;
|
private final ChangeCleanupConfig cfg;
|
||||||
private final Provider<ChangeQueryProcessor> queryProvider;
|
private final Provider<ChangeQueryProcessor> queryProvider;
|
||||||
private final ChangeQueryBuilder queryBuilder;
|
private final ChangeQueryBuilder queryBuilder;
|
||||||
private final Abandon abandon;
|
private final BatchAbandon batchAbandon;
|
||||||
private final InternalUser internalUser;
|
private final InternalUser internalUser;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
@@ -51,11 +51,11 @@ public class AbandonUtil {
|
|||||||
InternalUser.Factory internalUserFactory,
|
InternalUser.Factory internalUserFactory,
|
||||||
Provider<ChangeQueryProcessor> queryProvider,
|
Provider<ChangeQueryProcessor> queryProvider,
|
||||||
ChangeQueryBuilder queryBuilder,
|
ChangeQueryBuilder queryBuilder,
|
||||||
Abandon abandon) {
|
BatchAbandon batchAbandon) {
|
||||||
this.cfg = cfg;
|
this.cfg = cfg;
|
||||||
this.queryProvider = queryProvider;
|
this.queryProvider = queryProvider;
|
||||||
this.queryBuilder = queryBuilder;
|
this.queryBuilder = queryBuilder;
|
||||||
this.abandon = abandon;
|
this.batchAbandon = batchAbandon;
|
||||||
internalUser = internalUserFactory.create();
|
internalUser = internalUserFactory.create();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -85,7 +85,7 @@ public class AbandonUtil {
|
|||||||
for (Project.NameKey project : abandons.keySet()) {
|
for (Project.NameKey project : abandons.keySet()) {
|
||||||
Collection<ChangeData> changes = getValidChanges(abandons.get(project), query);
|
Collection<ChangeData> changes = getValidChanges(abandons.get(project), query);
|
||||||
try {
|
try {
|
||||||
abandon.batchAbandon(updateFactory, project, internalUser, changes, message);
|
batchAbandon.batchAbandon(updateFactory, project, internalUser, changes, message);
|
||||||
count += changes.size();
|
count += changes.size();
|
||||||
} catch (Throwable e) {
|
} catch (Throwable e) {
|
||||||
StringBuilder msg = new StringBuilder("Failed to auto-abandon inactive change(s):");
|
StringBuilder msg = new StringBuilder("Failed to auto-abandon inactive change(s):");
|
||||||
|
|||||||
109
java/com/google/gerrit/server/change/BatchAbandon.java
Normal file
109
java/com/google/gerrit/server/change/BatchAbandon.java
Normal file
@@ -0,0 +1,109 @@
|
|||||||
|
// Copyright (C) 2012 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.change;
|
||||||
|
|
||||||
|
import com.google.common.collect.ImmutableListMultimap;
|
||||||
|
import com.google.common.collect.ListMultimap;
|
||||||
|
import com.google.gerrit.common.TimeUtil;
|
||||||
|
import com.google.gerrit.extensions.api.changes.NotifyHandling;
|
||||||
|
import com.google.gerrit.extensions.api.changes.RecipientType;
|
||||||
|
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
||||||
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
|
import com.google.gerrit.reviewdb.client.Project;
|
||||||
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
|
import com.google.gerrit.server.CurrentUser;
|
||||||
|
import com.google.gerrit.server.query.change.ChangeData;
|
||||||
|
import com.google.gerrit.server.update.BatchUpdate;
|
||||||
|
import com.google.gerrit.server.update.UpdateException;
|
||||||
|
import com.google.inject.Inject;
|
||||||
|
import com.google.inject.Provider;
|
||||||
|
import com.google.inject.Singleton;
|
||||||
|
import java.util.Collection;
|
||||||
|
|
||||||
|
@Singleton
|
||||||
|
public class BatchAbandon {
|
||||||
|
private final Provider<ReviewDb> dbProvider;
|
||||||
|
private final AbandonOp.Factory abandonOpFactory;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
BatchAbandon(Provider<ReviewDb> dbProvider, AbandonOp.Factory abandonOpFactory) {
|
||||||
|
this.dbProvider = dbProvider;
|
||||||
|
this.abandonOpFactory = abandonOpFactory;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* If an extension has more than one changes to abandon that belong to the same project, they
|
||||||
|
* should use the batch instead of abandoning one by one.
|
||||||
|
*
|
||||||
|
* <p>It's the caller's responsibility to ensure that all jobs inside the same batch have the
|
||||||
|
* matching project from its ChangeData. Violations will result in a ResourceConflictException.
|
||||||
|
*/
|
||||||
|
public void batchAbandon(
|
||||||
|
BatchUpdate.Factory updateFactory,
|
||||||
|
Project.NameKey project,
|
||||||
|
CurrentUser user,
|
||||||
|
Collection<ChangeData> changes,
|
||||||
|
String msgTxt,
|
||||||
|
NotifyHandling notifyHandling,
|
||||||
|
ListMultimap<RecipientType, Account.Id> accountsToNotify)
|
||||||
|
throws RestApiException, UpdateException {
|
||||||
|
if (changes.isEmpty()) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
Account account = user.isIdentifiedUser() ? user.asIdentifiedUser().getAccount() : null;
|
||||||
|
try (BatchUpdate u = updateFactory.create(dbProvider.get(), project, user, TimeUtil.nowTs())) {
|
||||||
|
for (ChangeData change : changes) {
|
||||||
|
if (!project.equals(change.project())) {
|
||||||
|
throw new ResourceConflictException(
|
||||||
|
String.format(
|
||||||
|
"Project name \"%s\" doesn't match \"%s\"",
|
||||||
|
change.project().get(), project.get()));
|
||||||
|
}
|
||||||
|
u.addOp(
|
||||||
|
change.getId(),
|
||||||
|
abandonOpFactory.create(account, msgTxt, notifyHandling, accountsToNotify));
|
||||||
|
}
|
||||||
|
u.execute();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public void batchAbandon(
|
||||||
|
BatchUpdate.Factory updateFactory,
|
||||||
|
Project.NameKey project,
|
||||||
|
CurrentUser user,
|
||||||
|
Collection<ChangeData> changes,
|
||||||
|
String msgTxt)
|
||||||
|
throws RestApiException, UpdateException {
|
||||||
|
batchAbandon(
|
||||||
|
updateFactory,
|
||||||
|
project,
|
||||||
|
user,
|
||||||
|
changes,
|
||||||
|
msgTxt,
|
||||||
|
NotifyHandling.ALL,
|
||||||
|
ImmutableListMultimap.of());
|
||||||
|
}
|
||||||
|
|
||||||
|
public void batchAbandon(
|
||||||
|
BatchUpdate.Factory updateFactory,
|
||||||
|
Project.NameKey project,
|
||||||
|
CurrentUser user,
|
||||||
|
Collection<ChangeData> changes)
|
||||||
|
throws RestApiException, UpdateException {
|
||||||
|
batchAbandon(
|
||||||
|
updateFactory, project, user, changes, "", NotifyHandling.ALL, ImmutableListMultimap.of());
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -66,8 +66,7 @@ public class AbandonIT extends AbstractDaemonTest {
|
|||||||
PushOneCommit.Result a = createChange();
|
PushOneCommit.Result a = createChange();
|
||||||
PushOneCommit.Result b = createChange();
|
PushOneCommit.Result b = createChange();
|
||||||
List<ChangeData> list = ImmutableList.of(a.getChange(), b.getChange());
|
List<ChangeData> list = ImmutableList.of(a.getChange(), b.getChange());
|
||||||
changeAbandoner.batchAbandon(
|
batchAbandon.batchAbandon(batchUpdateFactory, a.getChange().project(), user, list, "deadbeef");
|
||||||
batchUpdateFactory, a.getChange().project(), user, list, "deadbeef");
|
|
||||||
|
|
||||||
ChangeInfo info = get(a.getChangeId(), MESSAGES);
|
ChangeInfo info = get(a.getChangeId(), MESSAGES);
|
||||||
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
|
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
|
||||||
@@ -96,7 +95,7 @@ public class AbandonIT extends AbstractDaemonTest {
|
|||||||
exception.expect(ResourceConflictException.class);
|
exception.expect(ResourceConflictException.class);
|
||||||
exception.expectMessage(
|
exception.expectMessage(
|
||||||
String.format("Project name \"%s\" doesn't match \"%s\"", project2Name, project1Name));
|
String.format("Project name \"%s\" doesn't match \"%s\"", project2Name, project1Name));
|
||||||
changeAbandoner.batchAbandon(batchUpdateFactory, new Project.NameKey(project1Name), user, list);
|
batchAbandon.batchAbandon(batchUpdateFactory, new Project.NameKey(project1Name), user, list);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user