Convert DeleteDraftChange to BatchUpdate
Change-Id: Id4395f92b0c70e86970d479276e9c9e0cc8c7d8f
This commit is contained in:
		| @@ -36,7 +36,6 @@ import com.google.gerrit.server.git.BatchUpdate; | |||||||
| import com.google.gerrit.server.git.GitRepositoryManager; | import com.google.gerrit.server.git.GitRepositoryManager; | ||||||
| import com.google.gerrit.server.git.UpdateException; | import com.google.gerrit.server.git.UpdateException; | ||||||
| import com.google.gerrit.server.git.validators.CommitValidators; | import com.google.gerrit.server.git.validators.CommitValidators; | ||||||
| import com.google.gerrit.server.index.ChangeIndexer; |  | ||||||
| import com.google.gerrit.server.mail.RevertedSender; | import com.google.gerrit.server.mail.RevertedSender; | ||||||
| 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; | ||||||
| @@ -186,7 +185,6 @@ public class ChangeUtil { | |||||||
|   private final ChangeInserter.Factory changeInserterFactory; |   private final ChangeInserter.Factory changeInserterFactory; | ||||||
|   private final GitRepositoryManager gitManager; |   private final GitRepositoryManager gitManager; | ||||||
|   private final GitReferenceUpdated gitRefUpdated; |   private final GitReferenceUpdated gitRefUpdated; | ||||||
|   private final ChangeIndexer indexer; |  | ||||||
|   private final BatchUpdate.Factory updateFactory; |   private final BatchUpdate.Factory updateFactory; | ||||||
|   private final ChangeMessagesUtil changeMessagesUtil; |   private final ChangeMessagesUtil changeMessagesUtil; | ||||||
|   private final ChangeUpdate.Factory changeUpdateFactory; |   private final ChangeUpdate.Factory changeUpdateFactory; | ||||||
| @@ -201,7 +199,6 @@ public class ChangeUtil { | |||||||
|       ChangeInserter.Factory changeInserterFactory, |       ChangeInserter.Factory changeInserterFactory, | ||||||
|       GitRepositoryManager gitManager, |       GitRepositoryManager gitManager, | ||||||
|       GitReferenceUpdated gitRefUpdated, |       GitReferenceUpdated gitRefUpdated, | ||||||
|       ChangeIndexer indexer, |  | ||||||
|       BatchUpdate.Factory updateFactory, |       BatchUpdate.Factory updateFactory, | ||||||
|       ChangeMessagesUtil changeMessagesUtil, |       ChangeMessagesUtil changeMessagesUtil, | ||||||
|       ChangeUpdate.Factory changeUpdateFactory, |       ChangeUpdate.Factory changeUpdateFactory, | ||||||
| @@ -214,7 +211,6 @@ public class ChangeUtil { | |||||||
|     this.changeInserterFactory = changeInserterFactory; |     this.changeInserterFactory = changeInserterFactory; | ||||||
|     this.gitManager = gitManager; |     this.gitManager = gitManager; | ||||||
|     this.gitRefUpdated = gitRefUpdated; |     this.gitRefUpdated = gitRefUpdated; | ||||||
|     this.indexer = indexer; |  | ||||||
|     this.updateFactory = updateFactory; |     this.updateFactory = updateFactory; | ||||||
|     this.changeMessagesUtil = changeMessagesUtil; |     this.changeMessagesUtil = changeMessagesUtil; | ||||||
|     this.changeUpdateFactory = changeUpdateFactory; |     this.changeUpdateFactory = changeUpdateFactory; | ||||||
| @@ -345,19 +341,6 @@ public class ChangeUtil { | |||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   // TODO(dborowitz): Delete after migrating DeleteDraftChange to BatchUpdate. |  | ||||||
|   public void deleteDraftChangeInNewTransaction(Change change) |  | ||||||
|       throws NoSuchChangeException, OrmException, IOException { |  | ||||||
|     db.get().changes().beginTransaction(change.getId()); |  | ||||||
|     try { |  | ||||||
|       deleteDraftChange(change); |  | ||||||
|       db.get().commit(); |  | ||||||
|       indexer.delete(change.getId()); |  | ||||||
|     } finally { |  | ||||||
|       db.get().rollback(); |  | ||||||
|     } |  | ||||||
|   } |  | ||||||
|  |  | ||||||
|   public void deleteDraftChange(Change change) |   public void deleteDraftChange(Change change) | ||||||
|       throws NoSuchChangeException, OrmException, IOException { |       throws NoSuchChangeException, OrmException, IOException { | ||||||
|     ReviewDb db = this.db.get(); |     ReviewDb db = this.db.get(); | ||||||
|   | |||||||
| @@ -22,7 +22,9 @@ import com.google.gerrit.extensions.restapi.RestResource.HasETag; | |||||||
| import com.google.gerrit.extensions.restapi.RestView; | import com.google.gerrit.extensions.restapi.RestView; | ||||||
| import com.google.gerrit.reviewdb.client.AccountGroup; | import com.google.gerrit.reviewdb.client.AccountGroup; | ||||||
| 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.server.CurrentUser; | import com.google.gerrit.server.CurrentUser; | ||||||
|  | import com.google.gerrit.server.IdentifiedUser; | ||||||
| import com.google.gerrit.server.notedb.ChangeNotes; | import com.google.gerrit.server.notedb.ChangeNotes; | ||||||
| import com.google.gerrit.server.project.ChangeControl; | import com.google.gerrit.server.project.ChangeControl; | ||||||
| import com.google.gerrit.server.project.ProjectState; | import com.google.gerrit.server.project.ProjectState; | ||||||
| @@ -49,6 +51,10 @@ public class ChangeResource implements RestResource, HasETag { | |||||||
|     return control; |     return control; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public IdentifiedUser getUser() { | ||||||
|  |     return getControl().getUser().asIdentifiedUser(); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   public Change.Id getId() { |   public Change.Id getId() { | ||||||
|     return getControl().getId(); |     return getControl().getId(); | ||||||
|   } |   } | ||||||
| @@ -57,6 +63,10 @@ public class ChangeResource implements RestResource, HasETag { | |||||||
|     return getControl().getChange(); |     return getControl().getChange(); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   public Project.NameKey getProject() { | ||||||
|  |     return getChange().getProject(); | ||||||
|  |   } | ||||||
|  |  | ||||||
|   public ChangeNotes getNotes() { |   public ChangeNotes getNotes() { | ||||||
|     return getControl().getNotes(); |     return getControl().getNotes(); | ||||||
|   } |   } | ||||||
|   | |||||||
| @@ -14,18 +14,24 @@ | |||||||
|  |  | ||||||
| package com.google.gerrit.server.change; | package com.google.gerrit.server.change; | ||||||
|  |  | ||||||
|  | import com.google.gerrit.common.TimeUtil; | ||||||
| import com.google.gerrit.extensions.restapi.AuthException; | import com.google.gerrit.extensions.restapi.AuthException; | ||||||
| import com.google.gerrit.extensions.restapi.MethodNotAllowedException; | import com.google.gerrit.extensions.restapi.MethodNotAllowedException; | ||||||
| import com.google.gerrit.extensions.restapi.ResourceConflictException; | import com.google.gerrit.extensions.restapi.ResourceConflictException; | ||||||
| import com.google.gerrit.extensions.restapi.ResourceNotFoundException; | import com.google.gerrit.extensions.restapi.ResourceNotFoundException; | ||||||
| import com.google.gerrit.extensions.restapi.Response; | 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.RestModifyView; | ||||||
| import com.google.gerrit.extensions.webui.UiAction; | import com.google.gerrit.extensions.webui.UiAction; | ||||||
|  | import com.google.gerrit.reviewdb.client.Change; | ||||||
| import com.google.gerrit.reviewdb.client.Change.Status; | import com.google.gerrit.reviewdb.client.Change.Status; | ||||||
| import com.google.gerrit.reviewdb.server.ReviewDb; | import com.google.gerrit.reviewdb.server.ReviewDb; | ||||||
| import com.google.gerrit.server.ChangeUtil; | import com.google.gerrit.server.ChangeUtil; | ||||||
| import com.google.gerrit.server.change.DeleteDraftChange.Input; | import com.google.gerrit.server.change.DeleteDraftChange.Input; | ||||||
| import com.google.gerrit.server.config.GerritServerConfig; | import com.google.gerrit.server.config.GerritServerConfig; | ||||||
|  | import com.google.gerrit.server.git.BatchUpdate; | ||||||
|  | import com.google.gerrit.server.git.BatchUpdate.ChangeContext; | ||||||
|  | import com.google.gerrit.server.git.UpdateException; | ||||||
| import com.google.gerrit.server.project.NoSuchChangeException; | import com.google.gerrit.server.project.NoSuchChangeException; | ||||||
| import com.google.gwtorm.server.OrmException; | import com.google.gwtorm.server.OrmException; | ||||||
| import com.google.inject.Inject; | import com.google.inject.Inject; | ||||||
| @@ -42,45 +48,59 @@ public class DeleteDraftChange implements | |||||||
|   public static class Input { |   public static class Input { | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   protected final Provider<ReviewDb> dbProvider; |   private final Provider<ReviewDb> db; | ||||||
|  |   private final BatchUpdate.Factory updateFactory; | ||||||
|   private final ChangeUtil changeUtil; |   private final ChangeUtil changeUtil; | ||||||
|   private final boolean allowDrafts; |   private final boolean allowDrafts; | ||||||
|  |  | ||||||
|   @Inject |   @Inject | ||||||
|   public DeleteDraftChange(Provider<ReviewDb> dbProvider, |   public DeleteDraftChange(Provider<ReviewDb> db, | ||||||
|  |       BatchUpdate.Factory updateFactory, | ||||||
|       ChangeUtil changeUtil, |       ChangeUtil changeUtil, | ||||||
|       @GerritServerConfig Config cfg) { |       @GerritServerConfig Config cfg) { | ||||||
|     this.dbProvider = dbProvider; |     this.db = db; | ||||||
|  |     this.updateFactory = updateFactory; | ||||||
|     this.changeUtil = changeUtil; |     this.changeUtil = changeUtil; | ||||||
|     this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true); |     this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public Response<?> apply(ChangeResource rsrc, Input input) |   public Response<?> apply(ChangeResource rsrc, Input input) | ||||||
|       throws ResourceConflictException, AuthException, |       throws RestApiException, UpdateException { | ||||||
|       ResourceNotFoundException, MethodNotAllowedException, |     try (BatchUpdate bu = updateFactory.create( | ||||||
|       OrmException, IOException { |         db.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) { | ||||||
|     if (rsrc.getChange().getStatus() != Status.DRAFT) { |       Change.Id id = rsrc.getChange().getId(); | ||||||
|       throw new ResourceConflictException("Change is not a draft"); |       bu.addOp(id, new Op()); | ||||||
|  |       bu.execute(); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     if (!rsrc.getControl().canDeleteDraft(dbProvider.get())) { |  | ||||||
|       throw new AuthException("Not permitted to delete this draft change"); |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     if (!allowDrafts) { |  | ||||||
|       throw new MethodNotAllowedException("draft workflow is disabled"); |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     try { |  | ||||||
|       changeUtil.deleteDraftChangeInNewTransaction(rsrc.getChange()); |  | ||||||
|     } catch (NoSuchChangeException e) { |  | ||||||
|       throw new ResourceNotFoundException(e.getMessage()); |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     return Response.none(); |     return Response.none(); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   private class Op extends BatchUpdate.Op { | ||||||
|  |     @Override | ||||||
|  |     public void updateChange(ChangeContext ctx) | ||||||
|  |         throws RestApiException, OrmException, IOException { | ||||||
|  |       if (ctx.getChange().getStatus() != Status.DRAFT) { | ||||||
|  |         throw new ResourceConflictException("Change is not a draft"); | ||||||
|  |       } | ||||||
|  |  | ||||||
|  |       if (!ctx.getChangeControl().canDeleteDraft(ctx.getDb())) { | ||||||
|  |         throw new AuthException("Not permitted to delete this draft change"); | ||||||
|  |       } | ||||||
|  |  | ||||||
|  |       if (!allowDrafts) { | ||||||
|  |         throw new MethodNotAllowedException("draft workflow is disabled"); | ||||||
|  |       } | ||||||
|  |  | ||||||
|  |       try { | ||||||
|  |         changeUtil.deleteDraftChange(ctx.getChange()); | ||||||
|  |         ctx.markDeleted(); | ||||||
|  |       } catch (NoSuchChangeException e) { | ||||||
|  |         throw new ResourceNotFoundException(e.getMessage()); | ||||||
|  |       } | ||||||
|  |     } | ||||||
|  |   } | ||||||
|  |  | ||||||
|   @Override |   @Override | ||||||
|   public UiAction.Description getDescription(ChangeResource rsrc) { |   public UiAction.Description getDescription(ChangeResource rsrc) { | ||||||
|     try { |     try { | ||||||
| @@ -88,7 +108,7 @@ public class DeleteDraftChange implements | |||||||
|         .setTitle("Delete draft change " + rsrc.getId()) |         .setTitle("Delete draft change " + rsrc.getId()) | ||||||
|         .setVisible(allowDrafts |         .setVisible(allowDrafts | ||||||
|             && rsrc.getChange().getStatus() == Status.DRAFT |             && rsrc.getChange().getStatus() == Status.DRAFT | ||||||
|             && rsrc.getControl().canDeleteDraft(dbProvider.get())); |             && rsrc.getControl().canDeleteDraft(db.get())); | ||||||
|     } catch (OrmException e) { |     } catch (OrmException e) { | ||||||
|       throw new IllegalStateException(e); |       throw new IllegalStateException(e); | ||||||
|     } |     } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Dave Borowitz
					Dave Borowitz