Replace REST collections for rebase/publish of change edits with REST views

Currently rebase and publish of change edits are implemented as REST
collections that only support POST on the collection, but no
listing/parsing of members. This adds unneeded complexity. It's cleaner
to just bind normal REST views to handle these POST requests.

ChangesRestApiBindingsIT.changeEditEndpoints() verifies that the
/changes/<change-id>/edit:rebase and /changes/<change-id>/edit:publish
REST endpoints are still reachable. In addition ChangeEditIT has
specific tests for the change edit REST API, in particular
rebaseEditRest and publishEditRest, that ensure that these REST
endpoints are still working.

Change-Id: I5918f72b2478b0e0f1353e57c900c613a20b32d9
Signed-off-by: Edwin Kempin <ekempin@google.com>

java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java

@@ -47,8 +47,8 @@ public class ChangeEditApiImpl implements ChangeEditApi {
   private final ChangeEdits.Detail editDetail;
   private final ChangeEdits.Post changeEditsPost;
   private final DeleteChangeEdit deleteChangeEdit;
-  private final RebaseChangeEdit.Rebase rebaseChangeEdit;
-  private final PublishChangeEdit.Publish publishChangeEdit;
+  private final RebaseChangeEdit rebaseChangeEdit;
+  private final PublishChangeEdit publishChangeEdit;
   private final ChangeEdits.Get changeEditsGet;
   private final ChangeEdits.Put changeEditsPut;
   private final ChangeEdits.DeleteContent changeEditDeleteContent;
@@ -62,8 +62,8 @@ public class ChangeEditApiImpl implements ChangeEditApi {
       ChangeEdits.Detail editDetail,
       ChangeEdits.Post changeEditsPost,
       DeleteChangeEdit deleteChangeEdit,
-      RebaseChangeEdit.Rebase rebaseChangeEdit,
-      PublishChangeEdit.Publish publishChangeEdit,
+      RebaseChangeEdit rebaseChangeEdit,
+      PublishChangeEdit publishChangeEdit,
       ChangeEdits.Get changeEditsGet,
       ChangeEdits.Put changeEditsPut,
       ChangeEdits.DeleteContent changeEditDeleteContent,

java/com/google/gerrit/server/change/ChangeEditResource.java

@@ -32,10 +32,6 @@ import com.google.inject.TypeLiteral;
 public class ChangeEditResource implements RestResource {
   public static final TypeLiteral<RestView<ChangeEditResource>> CHANGE_EDIT_KIND =
       new TypeLiteral<RestView<ChangeEditResource>>() {};
-  public static final TypeLiteral<RestView<ChangeEditResource.Publish>> CHANGE_EDIT_PUBLISH_KIND =
-      new TypeLiteral<RestView<ChangeEditResource.Publish>>() {};
-  public static final TypeLiteral<RestView<ChangeEditResource.Rebase>> CHANGE_EDIT_REBASE_KIND =
-      new TypeLiteral<RestView<ChangeEditResource.Rebase>>() {};
 
   private final ChangeResource change;
   private final ChangeEdit edit;
@@ -64,16 +60,4 @@ public class ChangeEditResource implements RestResource {
   public String getPath() {
     return path;
   }
-
-  public static class Publish extends ChangeEditResource {
-    public Publish(ChangeResource change, ChangeEdit edit, String path) {
-      super(change, edit, path);
-    }
-  }
-
-  public static class Rebase extends ChangeEditResource {
-    public Rebase(ChangeResource change, ChangeEdit edit, String path) {
-      super(change, edit, path);
-    }
-  }
 }

java/com/google/gerrit/server/restapi/change/Module.java

@@ -15,8 +15,6 @@
 package com.google.gerrit.server.restapi.change;
 
 import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_KIND;
-import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_PUBLISH_KIND;
-import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_REBASE_KIND;
 import static com.google.gerrit.server.change.ChangeMessageResource.CHANGE_MESSAGE_KIND;
 import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
 import static com.google.gerrit.server.change.CommentResource.COMMENT_KIND;
@@ -67,8 +65,6 @@ public class Module extends RestApiModule {
     DynamicMap.mapOf(binder(), REVIEWER_KIND);
     DynamicMap.mapOf(binder(), REVISION_KIND);
     DynamicMap.mapOf(binder(), CHANGE_EDIT_KIND);
-    DynamicMap.mapOf(binder(), CHANGE_EDIT_PUBLISH_KIND);
-    DynamicMap.mapOf(binder(), CHANGE_EDIT_REBASE_KIND);
     DynamicMap.mapOf(binder(), VOTE_KIND);
     DynamicMap.mapOf(binder(), CHANGE_MESSAGE_KIND);
 
@@ -174,10 +170,8 @@ public class Module extends RestApiModule {
     deleteMissing(CHANGE_EDIT_KIND).to(ChangeEdits.DeleteFile.class);
     postOnCollection(CHANGE_EDIT_KIND).to(ChangeEdits.Post.class);
     deleteOnCollection(CHANGE_EDIT_KIND).to(DeleteChangeEdit.class);
-    child(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class);
-    postOnCollection(CHANGE_EDIT_PUBLISH_KIND).to(PublishChangeEdit.Publish.class);
-    child(CHANGE_KIND, "edit:rebase").to(RebaseChangeEdit.class);
-    postOnCollection(CHANGE_EDIT_REBASE_KIND).to(RebaseChangeEdit.Rebase.class);
+    post(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class);
+    post(CHANGE_KIND, "edit:rebase").to(RebaseChangeEdit.class);
     put(CHANGE_KIND, "edit:message").to(ChangeEdits.EditMessage.class);
     get(CHANGE_KIND, "edit:message").to(ChangeEdits.GetMessage.class);
     put(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Put.class);

java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java

@@ -15,15 +15,9 @@
 package com.google.gerrit.server.restapi.change;
 
 import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
-import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.ChildCollection;
-import com.google.gerrit.extensions.restapi.IdString;
 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.RestView;
-import com.google.gerrit.server.change.ChangeEditResource;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.change.NotifyUtil;
 import com.google.gerrit.server.edit.ChangeEdit;
@@ -32,7 +26,7 @@ import com.google.gerrit.server.project.ContributorAgreementsChecker;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryingRestCollectionModifyView;
+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;
@@ -43,74 +37,44 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 
 @Singleton
 public class PublishChangeEdit
-    implements ChildCollection<ChangeResource, ChangeEditResource.Publish> {
-
-  private final DynamicMap<RestView<ChangeEditResource.Publish>> views;
+    extends RetryingRestModifyView<ChangeResource, PublishChangeEditInput, Response<?>> {
+  private final ChangeEditUtil editUtil;
+  private final NotifyUtil notifyUtil;
+  private final ContributorAgreementsChecker contributorAgreementsChecker;
 
   @Inject
-  PublishChangeEdit(DynamicMap<RestView<ChangeEditResource.Publish>> views) {
-    this.views = views;
+  PublishChangeEdit(
+      RetryHelper retryHelper,
+      ChangeEditUtil editUtil,
+      NotifyUtil notifyUtil,
+      ContributorAgreementsChecker contributorAgreementsChecker) {
+    super(retryHelper);
+    this.editUtil = editUtil;
+    this.notifyUtil = notifyUtil;
+    this.contributorAgreementsChecker = contributorAgreementsChecker;
   }
 
   @Override
-  public DynamicMap<RestView<ChangeEditResource.Publish>> views() {
-    return views;
-  }
-
-  @Override
-  public RestView<ChangeResource> list() throws ResourceNotFoundException {
-    throw new ResourceNotFoundException();
-  }
-
-  @Override
-  public ChangeEditResource.Publish parse(ChangeResource parent, IdString id)
-      throws ResourceNotFoundException {
-    throw new ResourceNotFoundException();
-  }
-
-  @Singleton
-  public static class Publish
-      extends RetryingRestCollectionModifyView<
-          ChangeResource, ChangeEditResource.Publish, PublishChangeEditInput, Response<?>> {
-
-    private final ChangeEditUtil editUtil;
-    private final NotifyUtil notifyUtil;
-    private final ContributorAgreementsChecker contributorAgreementsChecker;
-
-    @Inject
-    Publish(
-        RetryHelper retryHelper,
-        ChangeEditUtil editUtil,
-        NotifyUtil notifyUtil,
-        ContributorAgreementsChecker contributorAgreementsChecker) {
-      super(retryHelper);
-      this.editUtil = editUtil;
-      this.notifyUtil = notifyUtil;
-      this.contributorAgreementsChecker = contributorAgreementsChecker;
+  protected Response<?> applyImpl(
+      BatchUpdate.Factory updateFactory, ChangeResource rsrc, PublishChangeEditInput in)
+      throws IOException, OrmException, RestApiException, UpdateException, ConfigInvalidException,
+          NoSuchProjectException {
+    contributorAgreementsChecker.check(rsrc.getProject(), rsrc.getUser());
+    Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getNotes(), rsrc.getUser());
+    if (!edit.isPresent()) {
+      throw new ResourceConflictException(
+          String.format("no edit exists for change %s", rsrc.getChange().getChangeId()));
     }
-
-    @Override
-    protected Response<?> applyImpl(
-        BatchUpdate.Factory updateFactory, ChangeResource rsrc, PublishChangeEditInput in)
-        throws IOException, OrmException, RestApiException, UpdateException, ConfigInvalidException,
-            NoSuchProjectException {
-      contributorAgreementsChecker.check(rsrc.getProject(), rsrc.getUser());
-      Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getNotes(), rsrc.getUser());
-      if (!edit.isPresent()) {
-        throw new ResourceConflictException(
-            String.format("no edit exists for change %s", rsrc.getChange().getChangeId()));
-      }
-      if (in == null) {
-        in = new PublishChangeEditInput();
-      }
-      editUtil.publish(
-          updateFactory,
-          rsrc.getNotes(),
-          rsrc.getUser(),
-          edit.get(),
-          in.notify,
-          notifyUtil.resolveAccounts(in.notifyDetails));
-      return Response.none();
+    if (in == null) {
+      in = new PublishChangeEditInput();
     }
+    editUtil.publish(
+        updateFactory,
+        rsrc.getNotes(),
+        rsrc.getUser(),
+        edit.get(),
+        in.notify,
+        notifyUtil.resolveAccounts(in.notifyDetails));
+    return Response.none();
   }
 }

java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java

@@ -15,16 +15,10 @@
 package com.google.gerrit.server.restapi.change;
 
 import com.google.gerrit.extensions.common.Input;
-import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.extensions.restapi.ChildCollection;
-import com.google.gerrit.extensions.restapi.IdString;
 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.RestView;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.change.ChangeEditResource;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.edit.ChangeEditModifier;
 import com.google.gerrit.server.git.GitRepositoryManager;
@@ -32,7 +26,7 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryingRestCollectionModifyView;
+import com.google.gerrit.server.update.RetryingRestModifyView;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -40,61 +34,30 @@ import java.io.IOException;
 import org.eclipse.jgit.lib.Repository;
 
 @Singleton
-public class RebaseChangeEdit
-    implements ChildCollection<ChangeResource, ChangeEditResource.Rebase> {
-
-  private final DynamicMap<RestView<ChangeEditResource.Rebase>> views;
+public class RebaseChangeEdit extends RetryingRestModifyView<ChangeResource, Input, Response<?>> {
+  private final GitRepositoryManager repositoryManager;
+  private final ChangeEditModifier editModifier;
 
   @Inject
-  RebaseChangeEdit(DynamicMap<RestView<ChangeEditResource.Rebase>> views) {
-    this.views = views;
+  RebaseChangeEdit(
+      RetryHelper retryHelper,
+      GitRepositoryManager repositoryManager,
+      ChangeEditModifier editModifier) {
+    super(retryHelper);
+    this.repositoryManager = repositoryManager;
+    this.editModifier = editModifier;
   }
 
   @Override
-  public DynamicMap<RestView<ChangeEditResource.Rebase>> views() {
-    return views;
-  }
-
-  @Override
-  public RestView<ChangeResource> list() throws ResourceNotFoundException {
-    throw new ResourceNotFoundException();
-  }
-
-  @Override
-  public ChangeEditResource.Rebase parse(ChangeResource parent, IdString id)
-      throws ResourceNotFoundException {
-    throw new ResourceNotFoundException();
-  }
-
-  @Singleton
-  public static class Rebase
-      extends RetryingRestCollectionModifyView<
-          ChangeResource, ChangeEditResource.Rebase, Input, Response<?>> {
-    private final GitRepositoryManager repositoryManager;
-    private final ChangeEditModifier editModifier;
-
-    @Inject
-    Rebase(
-        RetryHelper retryHelper,
-        GitRepositoryManager repositoryManager,
-        ChangeEditModifier editModifier) {
-      super(retryHelper);
-      this.repositoryManager = repositoryManager;
-      this.editModifier = editModifier;
-    }
-
-    @Override
-    protected Response<?> applyImpl(
-        BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input in)
-        throws AuthException, ResourceConflictException, IOException, OrmException,
-            PermissionBackendException {
-      Project.NameKey project = rsrc.getProject();
-      try (Repository repository = repositoryManager.openRepository(project)) {
-        editModifier.rebaseEdit(repository, rsrc.getNotes());
-      } catch (InvalidChangeOperationException e) {
-        throw new ResourceConflictException(e.getMessage());
-      }
-      return Response.none();
+  protected Response<?> applyImpl(BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input in)
+      throws AuthException, ResourceConflictException, IOException, OrmException,
+          PermissionBackendException {
+    Project.NameKey project = rsrc.getProject();
+    try (Repository repository = repositoryManager.openRepository(project)) {
+      editModifier.rebaseEdit(repository, rsrc.getNotes());
+    } catch (InvalidChangeOperationException e) {
+      throw new ResourceConflictException(e.getMessage());
     }
+    return Response.none();
   }
 }