Port Abandon to use the BatchUpdate interface
Change-Id: Ib8e31239530ddc40c3ecddcf99a9950ca7c36544
This commit is contained in:
committed by
Dave Borowitz
parent
4d31d96633
commit
c3d31222e6
@@ -85,9 +85,14 @@ public class ChangeIT extends AbstractDaemonTest {
|
|||||||
@Test
|
@Test
|
||||||
public void abandon() throws Exception {
|
public void abandon() throws Exception {
|
||||||
PushOneCommit.Result r = createChange();
|
PushOneCommit.Result r = createChange();
|
||||||
|
assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.NEW);
|
||||||
gApi.changes()
|
gApi.changes()
|
||||||
.id(r.getChangeId())
|
.id(r.getChangeId())
|
||||||
.abandon();
|
.abandon();
|
||||||
|
ChangeInfo info = get(r.getChangeId());
|
||||||
|
assertThat(info.status).isEqualTo(ChangeStatus.ABANDONED);
|
||||||
|
assertThat(Iterables.getLast(info.messages).message.toLowerCase())
|
||||||
|
.contains("abandoned");
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ import com.google.gerrit.server.change.Revert;
|
|||||||
import com.google.gerrit.server.change.Revisions;
|
import com.google.gerrit.server.change.Revisions;
|
||||||
import com.google.gerrit.server.change.SubmittedTogether;
|
import com.google.gerrit.server.change.SubmittedTogether;
|
||||||
import com.google.gerrit.server.change.SuggestReviewers;
|
import com.google.gerrit.server.change.SuggestReviewers;
|
||||||
|
import com.google.gerrit.server.git.UpdateException;
|
||||||
import com.google.gerrit.server.project.InvalidChangeOperationException;
|
import com.google.gerrit.server.project.InvalidChangeOperationException;
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
@@ -166,7 +167,7 @@ class ChangeApiImpl implements ChangeApi {
|
|||||||
public void abandon(AbandonInput in) throws RestApiException {
|
public void abandon(AbandonInput in) throws RestApiException {
|
||||||
try {
|
try {
|
||||||
abandon.apply(change, in);
|
abandon.apply(change, in);
|
||||||
} catch (OrmException | IOException e) {
|
} catch (OrmException | UpdateException e) {
|
||||||
throw new RestApiException("Cannot abandon change", e);
|
throw new RestApiException("Cannot abandon change", e);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,25 +16,29 @@ package com.google.gerrit.server.change;
|
|||||||
|
|
||||||
import com.google.common.base.Strings;
|
import com.google.common.base.Strings;
|
||||||
import com.google.gerrit.common.ChangeHooks;
|
import com.google.gerrit.common.ChangeHooks;
|
||||||
|
import com.google.gerrit.common.TimeUtil;
|
||||||
import com.google.gerrit.extensions.api.changes.AbandonInput;
|
import com.google.gerrit.extensions.api.changes.AbandonInput;
|
||||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||||
import com.google.gerrit.extensions.restapi.AuthException;
|
import com.google.gerrit.extensions.restapi.AuthException;
|
||||||
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
import com.google.gerrit.extensions.restapi.ResourceConflictException;
|
||||||
|
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||||
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.ChangeMessage;
|
import com.google.gerrit.reviewdb.client.ChangeMessage;
|
||||||
|
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
import com.google.gerrit.server.ChangeMessagesUtil;
|
import com.google.gerrit.server.ChangeMessagesUtil;
|
||||||
import com.google.gerrit.server.ChangeUtil;
|
import com.google.gerrit.server.ChangeUtil;
|
||||||
import com.google.gerrit.server.IdentifiedUser;
|
import com.google.gerrit.server.IdentifiedUser;
|
||||||
import com.google.gerrit.server.index.ChangeIndexer;
|
import com.google.gerrit.server.git.BatchUpdate;
|
||||||
|
import com.google.gerrit.server.git.BatchUpdate.ChangeOp;
|
||||||
|
import com.google.gerrit.server.git.UpdateException;
|
||||||
import com.google.gerrit.server.mail.AbandonedSender;
|
import com.google.gerrit.server.mail.AbandonedSender;
|
||||||
import com.google.gerrit.server.mail.ReplyToChangeSender;
|
import com.google.gerrit.server.mail.ReplyToChangeSender;
|
||||||
import com.google.gerrit.server.notedb.ChangeUpdate;
|
import com.google.gerrit.server.notedb.ChangeUpdate;
|
||||||
import com.google.gerrit.server.project.ChangeControl;
|
import com.google.gerrit.server.project.ChangeControl;
|
||||||
import com.google.gwtorm.server.AtomicUpdate;
|
|
||||||
import com.google.gwtorm.server.OrmException;
|
import com.google.gwtorm.server.OrmException;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
import com.google.inject.Provider;
|
import com.google.inject.Provider;
|
||||||
@@ -43,7 +47,9 @@ import com.google.inject.Singleton;
|
|||||||
import org.slf4j.Logger;
|
import org.slf4j.Logger;
|
||||||
import org.slf4j.LoggerFactory;
|
import org.slf4j.LoggerFactory;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.util.Collections;
|
||||||
|
import java.util.concurrent.Callable;
|
||||||
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
|
|
||||||
@Singleton
|
@Singleton
|
||||||
public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
|
public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
|
||||||
@@ -54,100 +60,96 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
|
|||||||
private final AbandonedSender.Factory abandonedSenderFactory;
|
private final AbandonedSender.Factory abandonedSenderFactory;
|
||||||
private final Provider<ReviewDb> dbProvider;
|
private final Provider<ReviewDb> dbProvider;
|
||||||
private final ChangeJson.Factory json;
|
private final ChangeJson.Factory json;
|
||||||
private final ChangeIndexer indexer;
|
|
||||||
private final ChangeUpdate.Factory updateFactory;
|
|
||||||
private final ChangeMessagesUtil cmUtil;
|
private final ChangeMessagesUtil cmUtil;
|
||||||
|
private final BatchUpdate.Factory batchUpdateFactory;
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
Abandon(ChangeHooks hooks,
|
Abandon(ChangeHooks hooks,
|
||||||
AbandonedSender.Factory abandonedSenderFactory,
|
AbandonedSender.Factory abandonedSenderFactory,
|
||||||
Provider<ReviewDb> dbProvider,
|
Provider<ReviewDb> dbProvider,
|
||||||
ChangeJson.Factory json,
|
ChangeJson.Factory json,
|
||||||
ChangeIndexer indexer,
|
ChangeMessagesUtil cmUtil,
|
||||||
ChangeUpdate.Factory updateFactory,
|
BatchUpdate.Factory batchUpdateFactory) {
|
||||||
ChangeMessagesUtil cmUtil) {
|
|
||||||
this.hooks = hooks;
|
this.hooks = hooks;
|
||||||
this.abandonedSenderFactory = abandonedSenderFactory;
|
this.abandonedSenderFactory = abandonedSenderFactory;
|
||||||
this.dbProvider = dbProvider;
|
this.dbProvider = dbProvider;
|
||||||
this.json = json;
|
this.json = json;
|
||||||
this.indexer = indexer;
|
|
||||||
this.updateFactory = updateFactory;
|
|
||||||
this.cmUtil = cmUtil;
|
this.cmUtil = cmUtil;
|
||||||
|
this.batchUpdateFactory = batchUpdateFactory;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public ChangeInfo apply(ChangeResource req, AbandonInput input)
|
public ChangeInfo apply(ChangeResource req,
|
||||||
throws AuthException, ResourceConflictException, OrmException,
|
final AbandonInput input)
|
||||||
IOException {
|
throws RestApiException, UpdateException, OrmException {
|
||||||
ChangeControl control = req.getControl();
|
ChangeControl control = req.getControl();
|
||||||
IdentifiedUser caller = (IdentifiedUser) control.getCurrentUser();
|
IdentifiedUser caller = (IdentifiedUser) control.getCurrentUser();
|
||||||
Change change = req.getChange();
|
|
||||||
if (!control.canAbandon()) {
|
if (!control.canAbandon()) {
|
||||||
throw new AuthException("abandon not permitted");
|
throw new AuthException("abandon not permitted");
|
||||||
} else if (!change.getStatus().isOpen()) {
|
|
||||||
throw new ResourceConflictException("change is " + status(change));
|
|
||||||
} else if (change.getStatus() == Change.Status.DRAFT) {
|
|
||||||
throw new ResourceConflictException("draft changes cannot be abandoned");
|
|
||||||
}
|
}
|
||||||
change = abandon(control, input.message, caller.getAccount());
|
Change change = abandon(control, input.message, caller.getAccount());
|
||||||
return json.create(ChangeJson.NO_OPTIONS).format(change);
|
return json.create(ChangeJson.NO_OPTIONS).format(change);
|
||||||
}
|
}
|
||||||
|
|
||||||
public Change abandon(ChangeControl control,
|
public Change abandon(ChangeControl control,
|
||||||
String msgTxt, Account acc) throws ResourceConflictException,
|
final String msgTxt, final Account acc) throws RestApiException,
|
||||||
OrmException, IOException {
|
UpdateException {
|
||||||
Change change;
|
final Change.Id id = control.getChange().getId();
|
||||||
ChangeMessage message;
|
final AtomicReference<Change> change = new AtomicReference<>();
|
||||||
ChangeUpdate update;
|
final AtomicReference<PatchSet> patchSet = new AtomicReference<>();
|
||||||
Change.Id changeId = control.getChange().getId();
|
final AtomicReference<ChangeMessage> message = new AtomicReference<>();
|
||||||
ReviewDb db = dbProvider.get();
|
|
||||||
db.changes().beginTransaction(changeId);
|
try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
|
||||||
try {
|
control.getChange().getProject(), TimeUtil.nowTs())) {
|
||||||
change = db.changes().atomicUpdate(
|
u.addChangeOp(new ChangeOp(control) {
|
||||||
changeId,
|
@Override
|
||||||
new AtomicUpdate<Change>() {
|
public void call(ReviewDb db, ChangeUpdate update) throws OrmException,
|
||||||
@Override
|
ResourceConflictException {
|
||||||
public Change update(Change change) {
|
Change c = db.changes().get(id);
|
||||||
if (change.getStatus().isOpen()) {
|
if (c == null || !c.getStatus().isOpen()) {
|
||||||
change.setStatus(Change.Status.ABANDONED);
|
throw new ResourceConflictException("change is " + status(c));
|
||||||
ChangeUtil.updated(change);
|
} else if (c.getStatus() == Change.Status.DRAFT) {
|
||||||
return change;
|
throw new ResourceConflictException(
|
||||||
}
|
"draft changes cannot be abandoned");
|
||||||
return null;
|
|
||||||
}
|
}
|
||||||
});
|
c.setStatus(Change.Status.ABANDONED);
|
||||||
if (change == null) {
|
ChangeUtil.updated(c);
|
||||||
throw new ResourceConflictException("change is "
|
db.changes().update(Collections.singleton(c));
|
||||||
+ status(db.changes().get(changeId)));
|
|
||||||
}
|
|
||||||
|
|
||||||
//TODO(yyonas): atomic update was not propagated
|
ChangeMessage m = newMessage(
|
||||||
update = updateFactory.create(control, change.getLastUpdatedOn());
|
msgTxt, acc != null ? acc.getId() : null, c);
|
||||||
message = newMessage(msgTxt, acc != null ? acc.getId() : null, change);
|
cmUtil.addChangeMessage(db, update, m);
|
||||||
cmUtil.addChangeMessage(db, update, message);
|
|
||||||
db.commit();
|
|
||||||
} finally {
|
|
||||||
db.rollback();
|
|
||||||
}
|
|
||||||
update.commit();
|
|
||||||
|
|
||||||
indexer.index(db, change);
|
change.set(c);
|
||||||
try {
|
message.set(m);
|
||||||
ReplyToChangeSender cm = abandonedSenderFactory.create(change.getId());
|
patchSet.set(db.patchSets().get(c.currentPatchSetId()));
|
||||||
if (acc != null) {
|
}
|
||||||
cm.setFrom(acc.getId());
|
});
|
||||||
}
|
u.addPostOp(new Callable<Void>() {
|
||||||
cm.setChangeMessage(message);
|
@Override
|
||||||
cm.send();
|
public Void call() throws OrmException {
|
||||||
} catch (Exception e) {
|
Change c = change.get();
|
||||||
log.error("Cannot email update for change " + change.getChangeId(), e);
|
try {
|
||||||
|
ReplyToChangeSender cm = abandonedSenderFactory.create(id);
|
||||||
|
if (acc != null) {
|
||||||
|
cm.setFrom(acc.getId());
|
||||||
|
}
|
||||||
|
cm.setChangeMessage(message.get());
|
||||||
|
cm.send();
|
||||||
|
} catch (Exception e) {
|
||||||
|
log.error("Cannot email update for change " + id, e);
|
||||||
|
}
|
||||||
|
hooks.doChangeAbandonedHook(c,
|
||||||
|
acc,
|
||||||
|
patchSet.get(),
|
||||||
|
Strings.emptyToNull(msgTxt),
|
||||||
|
dbProvider.get());
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
u.execute();
|
||||||
}
|
}
|
||||||
hooks.doChangeAbandonedHook(change,
|
return change.get();
|
||||||
acc,
|
|
||||||
db.patchSets().get(change.currentPatchSetId()),
|
|
||||||
Strings.emptyToNull(msgTxt),
|
|
||||||
db);
|
|
||||||
return change;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
Reference in New Issue
Block a user