Convert most revision modify views to retrying helper

Change-Id: I4e24beb1ef38831633a3e8945ba8cbff55c8226c
This commit is contained in:
Dave Borowitz
2017-05-02 12:40:02 -04:00
parent ae52afc058
commit 3c23f1dd79
10 changed files with 78 additions and 51 deletions

View File

@@ -380,7 +380,7 @@ public class CommentsIT extends AbstractDaemonTest {
ChangeResource changeRsrc =
changes.get().parse(TopLevelResource.INSTANCE, IdString.fromDecoded(changeId));
RevisionResource revRsrc = revisions.parse(changeRsrc, IdString.fromDecoded(revId));
postReview.get().apply(revRsrc, input, timestamp);
postReview.get().apply(batchUpdateFactory, revRsrc, input, timestamp);
Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
assertThat(result).isNotEmpty();
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));

View File

@@ -752,7 +752,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
assertThat(ts).isGreaterThan(c.getCreatedOn());
assertThat(ts).isLessThan(db.patchSets().get(psId).getCreatedOn());
RevisionResource revRsrc = parseCurrentRevisionResource(r.getChangeId());
postReview.get().apply(revRsrc, rin, ts);
postReview.get().apply(batchUpdateFactory, revRsrc, rin, ts);
checker.rebuildAndCheckChanges(id);
}
@@ -770,7 +770,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
Timestamp ts = new Timestamp(c.getCreatedOn().getTime() - 10000);
RevisionResource revRsrc = parseCurrentRevisionResource(r.getChangeId());
setApiUser(user);
postReview.get().apply(revRsrc, rin, ts);
postReview.get().apply(batchUpdateFactory, revRsrc, rin, ts);
checker.rebuildAndCheckChanges(id);
}

View File

@@ -224,6 +224,8 @@ class RevisionApiImpl implements RevisionApi {
@Override
public void submit(SubmitInput in) throws RestApiException {
try {
// TODO(dborowitz): Convert to RetryingRestModifyHandler. Requires converting MergeOp to a
// Factory that takes BatchUpdate.Factory. (Enough Factories yet?)
submit.apply(revision, in);
} catch (Exception e) {
throw asRestApiException("Cannot submit change", e);
@@ -238,6 +240,8 @@ class RevisionApiImpl implements RevisionApi {
@Override
public BinaryResult submitPreview(String format) throws RestApiException {
try {
// TODO(dborowitz): Convert to RetryingRestModifyHandler. Requires converting MergeOp to a
// Factory that takes BatchUpdate.Factory.
submitPreview.setFormat(format);
return submitPreview.apply(revision);
} catch (Exception e) {

View File

@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
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.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -32,6 +31,9 @@ import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -41,21 +43,27 @@ import java.io.IOException;
@Singleton
public class CherryPick
implements RestModifyView<RevisionResource, CherryPickInput>, UiAction<RevisionResource> {
extends RetryingRestModifyView<RevisionResource, CherryPickInput, ChangeInfo>
implements UiAction<RevisionResource> {
private final Provider<ReviewDb> dbProvider;
private final CherryPickChange cherryPickChange;
private final ChangeJson.Factory json;
@Inject
CherryPick(
Provider<ReviewDb> dbProvider, CherryPickChange cherryPickChange, ChangeJson.Factory json) {
RetryHelper retryHelper,
Provider<ReviewDb> dbProvider,
CherryPickChange cherryPickChange,
ChangeJson.Factory json) {
super(retryHelper);
this.dbProvider = dbProvider;
this.cherryPickChange = cherryPickChange;
this.json = json;
}
@Override
public ChangeInfo apply(RevisionResource revision, CherryPickInput input)
protected ChangeInfo applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource revision, CherryPickInput input)
throws OrmException, IOException, UpdateException, RestApiException {
final ChangeControl control = revision.getControl();
int parent = input.parent == null ? 1 : input.parent;
@@ -88,6 +96,7 @@ public class CherryPick
try {
Change.Id cherryPickedChangeId =
// TODO(dborowitz): Pass updateFactory here.
cherryPickChange.cherryPick(
revision.getChange(),
revision.getPatchSet(),

View File

@@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Comment;
@@ -37,6 +36,8 @@ import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -45,9 +46,9 @@ import com.google.inject.Singleton;
import java.util.Collections;
@Singleton
public class CreateDraftComment implements RestModifyView<RevisionResource, DraftInput> {
public class CreateDraftComment
extends RetryingRestModifyView<RevisionResource, DraftInput, Response<CommentInfo>> {
private final Provider<ReviewDb> db;
private final BatchUpdate.Factory updateFactory;
private final Provider<CommentJson> commentJson;
private final CommentsUtil commentsUtil;
private final PatchSetUtil psUtil;
@@ -56,13 +57,13 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
@Inject
CreateDraftComment(
Provider<ReviewDb> db,
BatchUpdate.Factory updateFactory,
RetryHelper retryHelper,
Provider<CommentJson> commentJson,
CommentsUtil commentsUtil,
PatchSetUtil psUtil,
PatchListCache patchListCache) {
super(retryHelper);
this.db = db;
this.updateFactory = updateFactory;
this.commentJson = commentJson;
this.commentsUtil = commentsUtil;
this.psUtil = psUtil;
@@ -70,7 +71,8 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
}
@Override
public Response<CommentInfo> apply(RevisionResource rsrc, DraftInput in)
protected Response<CommentInfo> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, DraftInput in)
throws RestApiException, UpdateException, OrmException {
if (Strings.isNullOrEmpty(in.path)) {
throw new BadRequestException("path must be non-empty");

View File

@@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -43,6 +42,8 @@ import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Order;
import com.google.gerrit.server.update.RepoContext;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -56,11 +57,11 @@ import org.eclipse.jgit.lib.ObjectId;
@Singleton
public class DeleteDraftPatchSet
implements RestModifyView<RevisionResource, Input>, UiAction<RevisionResource> {
extends RetryingRestModifyView<RevisionResource, Input, Response<?>>
implements UiAction<RevisionResource> {
public static class Input {}
private final Provider<ReviewDb> db;
private final BatchUpdate.Factory updateFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchSetUtil psUtil;
private final Provider<DeleteChangeOp> deleteChangeOpProvider;
@@ -70,14 +71,14 @@ public class DeleteDraftPatchSet
@Inject
public DeleteDraftPatchSet(
Provider<ReviewDb> db,
BatchUpdate.Factory updateFactory,
RetryHelper retryHelper,
PatchSetInfoFactory patchSetInfoFactory,
PatchSetUtil psUtil,
Provider<DeleteChangeOp> deleteChangeOpProvider,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
@GerritServerConfig Config cfg) {
super(retryHelper);
this.db = db;
this.updateFactory = updateFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.psUtil = psUtil;
this.deleteChangeOpProvider = deleteChangeOpProvider;
@@ -86,7 +87,8 @@ public class DeleteDraftPatchSet
}
@Override
public Response<?> apply(RevisionResource rsrc, Input input)
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, Input input)
throws RestApiException, UpdateException, OrmException, PermissionBackendException {
if (isDeletingOnlyPatchSet(rsrc)) {
// A change cannot have zero patch sets; the change is deleted instead.

View File

@@ -61,7 +61,6 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
@@ -101,10 +100,11 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdate.Factory;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.LabelVote;
import com.google.gson.Gson;
@@ -131,13 +131,13 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class PostReview implements RestModifyView<RevisionResource, ReviewInput> {
public class PostReview
extends RetryingRestModifyView<RevisionResource, ReviewInput, Response<ReviewResult>> {
private static final Logger log = LoggerFactory.getLogger(PostReview.class);
private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson();
private static final int DEFAULT_ROBOT_COMMENT_SIZE_LIMIT_IN_BYTES = 1024 * 1024;
private final Provider<ReviewDb> db;
private final BatchUpdate.Factory batchUpdateFactory;
private final ChangesCollection changes;
private final ChangeData.Factory changeDataFactory;
private final ApprovalsUtil approvalsUtil;
@@ -156,7 +156,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
@Inject
PostReview(
Provider<ReviewDb> db,
Factory batchUpdateFactory,
RetryHelper retryHelper,
ChangesCollection changes,
ChangeData.Factory changeDataFactory,
ApprovalsUtil approvalsUtil,
@@ -171,8 +171,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
NotesMigration migration,
NotifyUtil notifyUtil,
@GerritServerConfig Config gerritConfig) {
super(retryHelper);
this.db = db;
this.batchUpdateFactory = batchUpdateFactory;
this.changes = changes;
this.changeDataFactory = changeDataFactory;
this.commentsUtil = commentsUtil;
@@ -190,13 +190,15 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
}
@Override
public Response<ReviewResult> apply(RevisionResource revision, ReviewInput input)
protected Response<ReviewResult> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource revision, ReviewInput input)
throws RestApiException, UpdateException, OrmException, IOException,
PermissionBackendException {
return apply(revision, input, TimeUtil.nowTs());
return apply(updateFactory, revision, input, TimeUtil.nowTs());
}
public Response<ReviewResult> apply(RevisionResource revision, ReviewInput input, Timestamp ts)
public Response<ReviewResult> apply(
BatchUpdate.Factory updateFactory, RevisionResource revision, ReviewInput input, Timestamp ts)
throws RestApiException, UpdateException, OrmException, IOException,
PermissionBackendException {
// Respect timestamp, but truncate at change created-on time.
@@ -264,8 +266,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
output.labels = input.labels;
try (BatchUpdate bu =
batchUpdateFactory.create(
db.get(), revision.getChange().getProject(), revision.getUser(), ts)) {
updateFactory.create(db.get(), revision.getChange().getProject(), revision.getUser(), ts)) {
Account.Id id = revision.getUser().getAccountId();
boolean ccOrReviewer = false;
if (input.labels != null && !input.labels.isEmpty()) {

View File

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -67,14 +66,14 @@ import org.slf4j.LoggerFactory;
@Singleton
public class PublishDraftPatchSet
implements RestModifyView<RevisionResource, Input>, UiAction<RevisionResource> {
extends RetryingRestModifyView<RevisionResource, Input, Response<?>>
implements UiAction<RevisionResource> {
private static final Logger log = LoggerFactory.getLogger(PublishDraftPatchSet.class);
public static class Input {}
private final AccountResolver accountResolver;
private final ApprovalsUtil approvalsUtil;
private final BatchUpdate.Factory updateFactory;
private final CreateChangeSender.Factory createChangeSenderFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchSetUtil psUtil;
@@ -86,16 +85,16 @@ public class PublishDraftPatchSet
public PublishDraftPatchSet(
AccountResolver accountResolver,
ApprovalsUtil approvalsUtil,
BatchUpdate.Factory updateFactory,
RetryHelper retryHelper,
CreateChangeSender.Factory createChangeSenderFactory,
PatchSetInfoFactory patchSetInfoFactory,
PatchSetUtil psUtil,
Provider<ReviewDb> dbProvider,
ReplacePatchSetSender.Factory replacePatchSetFactory,
DraftPublished draftPublished) {
super(retryHelper);
this.accountResolver = accountResolver;
this.approvalsUtil = approvalsUtil;
this.updateFactory = updateFactory;
this.createChangeSenderFactory = createChangeSenderFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.psUtil = psUtil;
@@ -105,12 +104,19 @@ public class PublishDraftPatchSet
}
@Override
public Response<?> apply(RevisionResource rsrc, Input input)
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, Input input)
throws RestApiException, UpdateException {
return apply(rsrc.getUser(), rsrc.getChange(), rsrc.getPatchSet().getId(), rsrc.getPatchSet());
return apply(
updateFactory,
rsrc.getUser(),
rsrc.getChange(),
rsrc.getPatchSet().getId(),
rsrc.getPatchSet());
}
private Response<?> apply(CurrentUser u, Change c, PatchSet.Id psId, PatchSet ps)
private Response<?> apply(
BatchUpdate.Factory updateFactory, CurrentUser u, Change c, PatchSet.Id psId, PatchSet ps)
throws RestApiException, UpdateException {
try (BatchUpdate bu =
updateFactory.create(dbProvider.get(), c.getProject(), u, TimeUtil.nowTs())) {
@@ -148,6 +154,7 @@ public class PublishDraftPatchSet
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException {
return publish.apply(
updateFactory,
rsrc.getControl().getUser(),
rsrc.getChange(),
rsrc.getChange().currentPatchSetId(),

View File

@@ -19,7 +19,6 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -33,6 +32,8 @@ import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -42,10 +43,10 @@ import java.util.Collections;
@Singleton
public class PutDescription
implements RestModifyView<RevisionResource, PutDescription.Input>, UiAction<RevisionResource> {
extends RetryingRestModifyView<RevisionResource, PutDescription.Input, Response<String>>
implements UiAction<RevisionResource> {
private final Provider<ReviewDb> dbProvider;
private final ChangeMessagesUtil cmUtil;
private final BatchUpdate.Factory batchUpdateFactory;
private final PatchSetUtil psUtil;
public static class Input {
@@ -56,23 +57,24 @@ public class PutDescription
PutDescription(
Provider<ReviewDb> dbProvider,
ChangeMessagesUtil cmUtil,
BatchUpdate.Factory batchUpdateFactory,
RetryHelper retryHelper,
PatchSetUtil psUtil) {
super(retryHelper);
this.dbProvider = dbProvider;
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
this.psUtil = psUtil;
}
@Override
public Response<String> apply(RevisionResource rsrc, Input input)
protected Response<String> applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, Input input)
throws UpdateException, RestApiException, PermissionBackendException {
rsrc.permissions().check(ChangePermission.EDIT_DESCRIPTION);
ChangeControl ctl = rsrc.getControl();
Op op = new Op(input != null ? input : new Input(), rsrc.getPatchSet().getId());
try (BatchUpdate u =
batchUpdateFactory.create(
updateFactory.create(
dbProvider.get(), rsrc.getChange().getProject(), ctl.getUser(), TimeUtil.nowTs())) {
u.addOp(rsrc.getChange().getId(), op);
u.execute();

View File

@@ -59,13 +59,12 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class Rebase
public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput, ChangeInfo>
implements RestModifyView<RevisionResource, RebaseInput>, UiAction<RevisionResource> {
private static final Logger log = LoggerFactory.getLogger(Rebase.class);
private static final ImmutableSet<ListChangesOption> OPTIONS =
Sets.immutableEnumSet(ListChangesOption.CURRENT_REVISION, ListChangesOption.CURRENT_COMMIT);
private final BatchUpdate.Factory updateFactory;
private final GitRepositoryManager repoManager;
private final RebaseChangeOp.Factory rebaseFactory;
private final RebaseUtil rebaseUtil;
@@ -74,13 +73,13 @@ public class Rebase
@Inject
public Rebase(
BatchUpdate.Factory updateFactory,
RetryHelper retryHelper,
GitRepositoryManager repoManager,
RebaseChangeOp.Factory rebaseFactory,
RebaseUtil rebaseUtil,
ChangeJson.Factory json,
Provider<ReviewDb> dbProvider) {
this.updateFactory = updateFactory;
super(retryHelper);
this.repoManager = repoManager;
this.rebaseFactory = rebaseFactory;
this.rebaseUtil = rebaseUtil;
@@ -89,7 +88,8 @@ public class Rebase
}
@Override
public ChangeInfo apply(RevisionResource rsrc, RebaseInput input)
protected ChangeInfo applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, RebaseInput input)
throws EmailException, OrmException, UpdateException, RestApiException, IOException,
NoSuchChangeException, PermissionBackendException {
rsrc.permissions().database(dbProvider).check(ChangePermission.REBASE);
@@ -235,7 +235,7 @@ public class Rebase
} else if (!rsrc.getControl().isPatchVisible(ps, rebase.dbProvider.get())) {
throw new AuthException("current revision not accessible");
}
return rebase.apply(new RevisionResource(rsrc, ps), input);
return rebase.applyImpl(updateFactory, new RevisionResource(rsrc, ps), input);
}
}
}