Merge "GroupReference: Fix NPE in hashcode and compareTo if UUID wraps null"
diff --git a/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java b/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java
index e1c4b3b..774e0a3 100644
--- a/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java
+++ b/java/com/google/gerrit/extensions/restapi/AcceptsDelete.java
@@ -17,31 +17,17 @@
 /**
  * Optional interface for {@link RestCollection}.
  *
- * <p>This interface is used for 2 purposes:
- *
- * <ul>
- *   <li>to support {@code DELETE} directly on the collection itself
- *   <li>to support {@code DELETE} on a non-existing member of the collection (in order to create
- *       that member)
- * </ul>
+ * <p>This interface is used to support {@code DELETE} directly on the collection itself.
  *
  * <p>This interface is not supported for root collections.
  */
 public interface AcceptsDelete<P extends RestResource> {
   /**
-   * Handle
-   *
-   * <ul>
-   *   <li>{@code DELETE} directly on the collection itself (in this case id is {@code null})
-   *   <li>{@code DELETE} on a non-existing member of the collection (in this case id is not {@code
-   *       null})
-   * </ul>
+   * Handle {@code DELETE} directly on the collection itself.
    *
    * @param parent the collection
-   * @param id id of the non-existing collection member for which the {@code DELETE} request is
-   *     done, {@code null} if the {@code DELETE} request is done on the collection itself
    * @return a view to handle the {@code DELETE} request
    * @throws RestApiException the view cannot be constructed
    */
-  RestModifyView<P, ?> delete(P parent, IdString id) throws RestApiException;
+  RestModifyView<P, ?> delete(P parent) throws RestApiException;
 }
diff --git a/java/com/google/gerrit/extensions/restapi/RestApiModule.java b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
index d47b094..c1bb95e 100644
--- a/java/com/google/gerrit/extensions/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
@@ -29,6 +29,7 @@
   protected static final String DELETE = "DELETE";
   protected static final String POST = "POST";
   protected static final String CREATE = "CREATE";
+  protected static final String DELETE_MISSING = "DELETE_MISSING";
   protected static final String POST_ON_COLLECTION = "POST_ON_COLLECTION";
 
   protected <R extends RestResource> ReadViewBinder<R> get(TypeLiteral<RestView<R>> viewType) {
@@ -57,6 +58,11 @@
     return new CreateViewBinder<>(bind(viewType).annotatedWith(export(CREATE, "/")));
   }
 
+  protected <R extends RestResource> DeleteViewBinder<R> deleteMissing(
+      TypeLiteral<RestView<R>> viewType) {
+    return new DeleteViewBinder<>(bind(viewType).annotatedWith(export(DELETE_MISSING, "/")));
+  }
+
   protected <R extends RestResource> ReadViewBinder<R> get(
       TypeLiteral<RestView<R>> viewType, String name) {
     return new ReadViewBinder<>(view(viewType, GET, name));
@@ -203,6 +209,34 @@
     }
   }
 
+  public static class DeleteViewBinder<C extends RestResource> {
+    private final LinkedBindingBuilder<RestView<C>> binder;
+
+    private DeleteViewBinder(LinkedBindingBuilder<RestView<C>> binder) {
+      this.binder = binder;
+    }
+
+    public <P extends RestResource, T extends RestDeleteMissingView<P, C, ?>>
+        ScopedBindingBuilder to(Class<T> impl) {
+      return binder.to(impl);
+    }
+
+    public <P extends RestResource, T extends RestDeleteMissingView<P, C, ?>> void toInstance(
+        T impl) {
+      binder.toInstance(impl);
+    }
+
+    public <P extends RestResource, T extends RestDeleteMissingView<P, C, ?>>
+        ScopedBindingBuilder toProvider(Class<? extends Provider<? extends T>> providerType) {
+      return binder.toProvider(providerType);
+    }
+
+    public <P extends RestResource, T extends RestDeleteMissingView<P, C, ?>>
+        ScopedBindingBuilder toProvider(Provider<? extends T> provider) {
+      return binder.toProvider(provider);
+    }
+  }
+
   public static class ChildCollectionBinder<P extends RestResource> {
     private final LinkedBindingBuilder<RestView<P>> binder;
 
diff --git a/java/com/google/gerrit/extensions/restapi/RestDeleteMissingView.java b/java/com/google/gerrit/extensions/restapi/RestDeleteMissingView.java
new file mode 100644
index 0000000..1d58c80
--- /dev/null
+++ b/java/com/google/gerrit/extensions/restapi/RestDeleteMissingView.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2018 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.extensions.restapi;
+
+/**
+ * RestView that supports accepting input and deleting a resource that is missing.
+ *
+ * <p>The RestDeleteMissingView solely exists to support a special case for creating a change edit
+ * by deleting a path in the non-existing change edit. This interface should not be used for new
+ * REST API's.
+ *
+ * <p>The input must be supplied as JSON as the body of the HTTP request. Delete views can be
+ * invoked by the HTTP method {@code DELETE}.
+ *
+ * <p>The RestDeleteMissingView is only invoked when the parse method of the {@code RestCollection}
+ * throws {@link ResourceNotFoundException}, and hence the resource doesn't exist yet.
+ *
+ * @param <P> type of the parent resource
+ * @param <C> type of the child resource that id deleted
+ * @param <I> type of input the JSON parser will parse the input into.
+ */
+public interface RestDeleteMissingView<P extends RestResource, C extends RestResource, I>
+    extends RestView<C> {
+
+  /**
+   * Process the view operation by deleting the resource.
+   *
+   * @param parentResource parent resource of the resource that should be deleted
+   * @param input input after parsing from request.
+   * @return result to return to the client. Use {@link BinaryResult} to avoid automatic conversion
+   *     to JSON.
+   * @throws RestApiException if the resource creation is rejected
+   * @throws Exception the implementation of the view failed. The exception will be logged and HTTP
+   *     500 Internal Server Error will be returned to the client.
+   */
+  Object apply(P parentResource, IdString id, I input) throws Exception;
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 599e98f..70296a3 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -88,6 +88,7 @@
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestCollectionView;
 import com.google.gerrit.extensions.restapi.RestCreateView;
+import com.google.gerrit.extensions.restapi.RestDeleteMissingView;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.extensions.restapi.RestResource;
@@ -325,15 +326,28 @@
             checkPreconditions(req);
           }
         } catch (ResourceNotFoundException e) {
-          if (!path.isEmpty() || (!isPost(req) && !isPut(req))) {
+          if (!path.isEmpty()) {
             throw e;
           }
 
-          RestView<RestResource> createView = rc.views().get("gerrit", "CREATE./");
-          if (createView != null) {
-            viewData = new ViewData(null, createView);
-            status = SC_CREATED;
-            path.add(id);
+          if (isPost(req) || isPut(req)) {
+            RestView<RestResource> createView = rc.views().get("gerrit", "CREATE./");
+            if (createView != null) {
+              viewData = new ViewData(null, createView);
+              status = SC_CREATED;
+              path.add(id);
+            } else {
+              throw e;
+            }
+          } else if (isDelete(req)) {
+            RestView<RestResource> deleteView = rc.views().get("gerrit", "DELETE_MISSING./");
+            if (deleteView != null) {
+              viewData = new ViewData(null, deleteView);
+              status = SC_NO_CONTENT;
+              path.add(id);
+            } else {
+              throw e;
+            }
           } else {
             throw e;
           }
@@ -363,7 +377,7 @@
           } else if (c instanceof AcceptsDelete && isDelete(req)) {
             @SuppressWarnings("unchecked")
             AcceptsDelete<RestResource> ac = (AcceptsDelete<RestResource>) c;
-            viewData = new ViewData(null, ac.delete(rsrc, null));
+            viewData = new ViewData(null, ac.delete(rsrc));
           } else {
             throw new MethodNotAllowedException();
           }
@@ -388,11 +402,15 @@
             } else {
               throw e;
             }
-          } else if (c instanceof AcceptsDelete && isDelete(req)) {
-            @SuppressWarnings("unchecked")
-            AcceptsDelete<RestResource> ac = (AcceptsDelete<RestResource>) c;
-            viewData = new ViewData(viewData.pluginName, ac.delete(rsrc, id));
-            status = SC_NO_CONTENT;
+          } else if (isDelete(req)) {
+            RestView<RestResource> deleteView = c.views().get("gerrit", "DELETE_MISSING./");
+            if (deleteView != null) {
+              viewData = new ViewData(null, deleteView);
+              status = SC_NO_CONTENT;
+              path.add(id);
+            } else {
+              throw e;
+            }
           } else {
             throw e;
           }
@@ -440,6 +458,19 @@
             ServletUtils.consumeRequestBody(is);
           }
         }
+      } else if (viewData.view instanceof RestDeleteMissingView<?, ?, ?>) {
+        @SuppressWarnings("unchecked")
+        RestDeleteMissingView<RestResource, RestResource, Object> m =
+            (RestDeleteMissingView<RestResource, RestResource, Object>) viewData.view;
+
+        Type type = inputType(m);
+        inputRequestBody = parseRequest(req, type);
+        result = m.apply(rsrc, path.get(0), inputRequestBody);
+        if (inputRequestBody instanceof RawInput) {
+          try (InputStream is = req.getInputStream()) {
+            ServletUtils.consumeRequestBody(is);
+          }
+        }
       } else if (viewData.view instanceof RestCollectionView<?, ?, ?>) {
         @SuppressWarnings("unchecked")
         RestCollectionView<RestResource, RestResource, Object> m =
@@ -797,6 +828,24 @@
     return ((ParameterizedType) supertype).getActualTypeArguments()[2];
   }
 
+  private static Type inputType(RestDeleteMissingView<RestResource, RestResource, Object> m) {
+    // MyCreateView implements RestDeleteMissingView<SomeResource, SomeResource, MyInput>
+    TypeLiteral<?> typeLiteral = TypeLiteral.get(m.getClass());
+
+    // RestDeleteMissingView<SomeResource, SomeResource, MyInput>
+    // This is smart enough to resolve even when there are intervening subclasses, even if they have
+    // reordered type arguments.
+    TypeLiteral<?> supertypeLiteral = typeLiteral.getSupertype(RestDeleteMissingView.class);
+
+    Type supertype = supertypeLiteral.getType();
+    checkState(
+        supertype instanceof ParameterizedType,
+        "supertype of %s is not parameterized: %s",
+        typeLiteral,
+        supertypeLiteral);
+    return ((ParameterizedType) supertype).getActualTypeArguments()[2];
+  }
+
   private static Type inputType(RestCollectionView<RestResource, RestResource, Object> m) {
     // MyCollectionView implements RestCollectionView<SomeResource, SomeResource, MyInput>
     TypeLiteral<?> typeLiteral = TypeLiteral.get(m.getClass());
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index f1dd624..f4e659e 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -30,7 +30,6 @@
 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.IdentifiedUser;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -49,16 +48,11 @@
   static class Factory {
     private final ChangeData.Factory changeDataFactory;
     private final ChangeNotes.Factory notesFactory;
-    private final IdentifiedUser.GenericFactory identifiedUserFactory;
 
     @Inject
-    Factory(
-        ChangeData.Factory changeDataFactory,
-        ChangeNotes.Factory notesFactory,
-        IdentifiedUser.GenericFactory identifiedUserFactory) {
+    Factory(ChangeData.Factory changeDataFactory, ChangeNotes.Factory notesFactory) {
       this.changeDataFactory = changeDataFactory;
       this.notesFactory = notesFactory;
-      this.identifiedUserFactory = identifiedUserFactory;
     }
 
     ChangeControl create(
@@ -68,22 +62,17 @@
     }
 
     ChangeControl create(RefControl refControl, ChangeNotes notes) {
-      return new ChangeControl(changeDataFactory, identifiedUserFactory, refControl, notes);
+      return new ChangeControl(changeDataFactory, refControl, notes);
     }
   }
 
   private final ChangeData.Factory changeDataFactory;
-  private final IdentifiedUser.GenericFactory identifiedUserFactory;
   private final RefControl refControl;
   private final ChangeNotes notes;
 
   private ChangeControl(
-      ChangeData.Factory changeDataFactory,
-      IdentifiedUser.GenericFactory identifiedUserFactory,
-      RefControl refControl,
-      ChangeNotes notes) {
+      ChangeData.Factory changeDataFactory, RefControl refControl, ChangeNotes notes) {
     this.changeDataFactory = changeDataFactory;
-    this.identifiedUserFactory = identifiedUserFactory;
     this.refControl = refControl;
     this.notes = notes;
   }
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index efc2712..70eb3e5 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -19,7 +19,6 @@
 import com.google.gerrit.extensions.common.EditInfo;
 import com.google.gerrit.extensions.common.Input;
 import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.restapi.AcceptsDelete;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
@@ -30,9 +29,9 @@
 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.RestCollectionView;
 import com.google.gerrit.extensions.restapi.RestCreateView;
+import com.google.gerrit.extensions.restapi.RestDeleteMissingView;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.extensions.restapi.RestView;
@@ -59,7 +58,6 @@
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
@@ -70,10 +68,8 @@
 import org.kohsuke.args4j.Option;
 
 @Singleton
-public class ChangeEdits
-    implements ChildCollection<ChangeResource, ChangeEditResource>, AcceptsDelete<ChangeResource> {
+public class ChangeEdits implements ChildCollection<ChangeResource, ChangeEditResource> {
   private final DynamicMap<RestView<ChangeEditResource>> views;
-  private final DeleteFile.Factory deleteFileFactory;
   private final Provider<Detail> detail;
   private final ChangeEditUtil editUtil;
 
@@ -81,12 +77,10 @@
   ChangeEdits(
       DynamicMap<RestView<ChangeEditResource>> views,
       Provider<Detail> detail,
-      ChangeEditUtil editUtil,
-      DeleteFile.Factory deleteFileFactory) {
+      ChangeEditUtil editUtil) {
     this.views = views;
     this.detail = detail;
     this.editUtil = editUtil;
-    this.deleteFileFactory = deleteFileFactory;
   }
 
   @Override
@@ -110,20 +104,6 @@
   }
 
   /**
-   * This method is invoked if a DELETE request on a non-existing member is done. For change edits
-   * this is the case if a DELETE request for a file in a change edit is done and the change edit
-   * doesn't exist yet (and hence the parse method returned ResourceNotFoundException). In this case
-   * we want to create the change edit on the fly and delete the file with the given id in it.
-   */
-  @Override
-  public DeleteFile delete(ChangeResource parent, IdString id) throws RestApiException {
-    // It's safe to assume that id can never be null, because
-    // otherwise we would end up in dedicated endpoint for
-    // deleting of change edits and not a file in change edit
-    return deleteFileFactory.create(id.get());
-  }
-
-  /**
    * Create handler that is activated when collection element is accessed but doesn't exist, e. g.
    * PUT request with a path was called but change edit wasn't created yet. Change edit is created
    * and PUT handler is called.
@@ -146,26 +126,20 @@
     }
   }
 
-  public static class DeleteFile implements RestModifyView<ChangeResource, Input> {
-
-    public interface Factory {
-      DeleteFile create(String path);
-    }
-
+  public static class DeleteFile
+      implements RestDeleteMissingView<ChangeResource, ChangeEditResource, Input> {
     private final DeleteContent deleteContent;
-    private final String path;
 
     @Inject
-    DeleteFile(DeleteContent deleteContent, @Assisted String path) {
+    DeleteFile(DeleteContent deleteContent) {
       this.deleteContent = deleteContent;
-      this.path = path;
     }
 
     @Override
-    public Response<?> apply(ChangeResource rsrc, Input in)
+    public Response<?> apply(ChangeResource rsrc, IdString id, Input in)
         throws IOException, AuthException, ResourceConflictException, OrmException,
             PermissionBackendException {
-      return deleteContent.apply(rsrc, path);
+      return deleteContent.apply(rsrc, id.get());
     }
   }
 
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index dca64fd..5c13303 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -171,6 +171,7 @@
 
     child(CHANGE_KIND, "edit").to(ChangeEdits.class);
     create(CHANGE_EDIT_KIND).to(ChangeEdits.Create.class);
+    deleteMissing(CHANGE_EDIT_KIND).to(ChangeEdits.DeleteFile.class);
     postOnCollection(CHANGE_EDIT_KIND).to(ChangeEdits.Post.class);
     delete(CHANGE_KIND, "edit").to(DeleteChangeEdit.class);
     child(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class);
@@ -189,7 +190,6 @@
     get(CHANGE_MESSAGE_KIND).to(GetChangeMessage.class);
 
     factory(AccountLoader.Factory.class);
-    factory(ChangeEdits.DeleteFile.Factory.class);
     factory(ChangeInserter.Factory.class);
     factory(ChangeResource.Factory.class);
     factory(DeleteReviewerByEmailOp.Factory.class);
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteBranch.java b/java/com/google/gerrit/server/restapi/project/DeleteBranch.java
index aed372c..0134ce3 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteBranch.java
@@ -23,9 +23,7 @@
 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.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.BranchResource;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
@@ -38,17 +36,12 @@
 public class DeleteBranch implements RestModifyView<BranchResource, Input> {
 
   private final Provider<InternalChangeQuery> queryProvider;
-  private final DeleteRef.Factory deleteRefFactory;
-  private final PermissionBackend permissionBackend;
+  private final DeleteRef deleteRef;
 
   @Inject
-  DeleteBranch(
-      Provider<InternalChangeQuery> queryProvider,
-      DeleteRef.Factory deleteRefFactory,
-      PermissionBackend permissionBackend) {
+  DeleteBranch(Provider<InternalChangeQuery> queryProvider, DeleteRef deleteRef) {
     this.queryProvider = queryProvider;
-    this.deleteRefFactory = deleteRefFactory;
-    this.permissionBackend = permissionBackend;
+    this.deleteRef = deleteRef;
   }
 
   @Override
@@ -60,14 +53,11 @@
           "not allowed to delete branch " + rsrc.getBranchKey().get());
     }
 
-    permissionBackend.currentUser().ref(rsrc.getBranchKey()).check(RefPermission.DELETE);
-    rsrc.getProjectState().checkStatePermitsWrite();
-
     if (!queryProvider.get().setLimit(1).byBranchOpen(rsrc.getBranchKey()).isEmpty()) {
       throw new ResourceConflictException("branch " + rsrc.getBranchKey() + " has open changes");
     }
 
-    deleteRefFactory.create(rsrc).ref(rsrc.getRef()).prefix(R_HEADS).delete();
+    deleteRef.deleteSingleRef(rsrc.getProjectState(), rsrc.getRef(), R_HEADS);
     return Response.none();
   }
 }
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteBranches.java b/java/com/google/gerrit/server/restapi/project/DeleteBranches.java
index d8166e1..6e60193 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteBranches.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteBranches.java
@@ -16,6 +16,7 @@
 
 import static org.eclipse.jgit.lib.Constants.R_HEADS;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.Response;
@@ -30,11 +31,11 @@
 
 @Singleton
 public class DeleteBranches implements RestModifyView<ProjectResource, DeleteBranchesInput> {
-  private final DeleteRef.Factory deleteRefFactory;
+  private final DeleteRef deleteRef;
 
   @Inject
-  DeleteBranches(DeleteRef.Factory deleteRefFactory) {
-    this.deleteRefFactory = deleteRefFactory;
+  DeleteBranches(DeleteRef deleteRef) {
+    this.deleteRef = deleteRef;
   }
 
   @Override
@@ -43,7 +44,8 @@
     if (input == null || input.branches == null || input.branches.isEmpty()) {
       throw new BadRequestException("branches must be specified");
     }
-    deleteRefFactory.create(project).refs(input.branches).prefix(R_HEADS).delete();
+    deleteRef.deleteMultipleRefs(
+        project.getProjectState(), ImmutableSet.copyOf(input.branches), R_HEADS);
     return Response.none();
   }
 }
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteRef.java b/java/com/google/gerrit/server/restapi/project/DeleteRef.java
index 769eaf8..0016354 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteRef.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteRef.java
@@ -14,33 +14,35 @@
 
 package com.google.gerrit.server.restapi.project;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.gerrit.reviewdb.client.RefNames.isConfigRef;
 import static java.lang.String.format;
-import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.lib.Constants.R_REFS;
 import static org.eclipse.jgit.lib.Constants.R_TAGS;
 import static org.eclipse.jgit.transport.ReceiveCommand.Type.DELETE;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.RefValidationHelper;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
-import com.google.inject.assistedinject.Assisted;
+import com.google.inject.Singleton;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
 import org.eclipse.jgit.errors.LockFailedException;
 import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -52,6 +54,7 @@
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.transport.ReceiveCommand.Result;
 
+@Singleton
 public class DeleteRef {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
@@ -64,13 +67,6 @@
   private final GitReferenceUpdated referenceUpdated;
   private final RefValidationHelper refDeletionValidator;
   private final Provider<InternalChangeQuery> queryProvider;
-  private final ProjectResource resource;
-  private final List<String> refsToDelete;
-  private String prefix;
-
-  public interface Factory {
-    DeleteRef create(ProjectResource r);
-  }
 
   @Inject
   DeleteRef(
@@ -79,135 +75,164 @@
       GitRepositoryManager repoManager,
       GitReferenceUpdated referenceUpdated,
       RefValidationHelper.Factory refDeletionValidatorFactory,
-      Provider<InternalChangeQuery> queryProvider,
-      @Assisted ProjectResource resource) {
+      Provider<InternalChangeQuery> queryProvider) {
     this.identifiedUser = identifiedUser;
     this.permissionBackend = permissionBackend;
     this.repoManager = repoManager;
     this.referenceUpdated = referenceUpdated;
     this.refDeletionValidator = refDeletionValidatorFactory.create(DELETE);
     this.queryProvider = queryProvider;
-    this.resource = resource;
-    this.refsToDelete = new ArrayList<>();
   }
 
-  public DeleteRef ref(String ref) {
-    this.refsToDelete.add(ref);
-    return this;
+  /**
+   * Deletes a single ref from the repository.
+   *
+   * @param projectState the {@code ProjectState} of the project containing the target ref.
+   * @param ref the ref to be deleted.
+   * @throws IOException
+   * @throws ResourceConflictException
+   */
+  public void deleteSingleRef(ProjectState projectState, String ref)
+      throws IOException, ResourceConflictException, AuthException, PermissionBackendException {
+    deleteSingleRef(projectState, ref, null);
   }
 
-  public DeleteRef refs(List<String> refs) {
-    this.refsToDelete.addAll(refs);
-    return this;
-  }
-
-  public DeleteRef prefix(String prefix) {
-    this.prefix = prefix;
-    return this;
-  }
-
-  public void delete()
-      throws OrmException, IOException, ResourceConflictException, PermissionBackendException {
-    if (!refsToDelete.isEmpty()) {
-      try (Repository r = repoManager.openRepository(resource.getNameKey())) {
-        if (refsToDelete.size() == 1) {
-          deleteSingleRef(r);
-        } else {
-          deleteMultipleRefs(r);
-        }
-      }
-    }
-  }
-
-  private void deleteSingleRef(Repository r) throws IOException, ResourceConflictException {
-    String ref = refsToDelete.get(0);
+  /**
+   * Deletes a single ref from the repository.
+   *
+   * @param projectState the {@code ProjectState} of the project containing the target ref.
+   * @param ref the ref to be deleted.
+   * @param prefix the prefix of the ref.
+   * @throws IOException
+   * @throws ResourceConflictException
+   */
+  public void deleteSingleRef(ProjectState projectState, String ref, @Nullable String prefix)
+      throws IOException, ResourceConflictException, AuthException, PermissionBackendException {
     if (prefix != null && !ref.startsWith(R_REFS)) {
       ref = prefix + ref;
     }
-    RefUpdate.Result result;
-    RefUpdate u = r.updateRef(ref);
-    u.setExpectedOldObjectId(r.exactRef(ref).getObjectId());
-    u.setNewObjectId(ObjectId.zeroId());
-    u.setForceUpdate(true);
-    refDeletionValidator.validateRefOperation(resource.getName(), identifiedUser.get(), u);
-    int remainingLockFailureCalls = MAX_LOCK_FAILURE_CALLS;
-    for (; ; ) {
-      try {
-        result = u.delete();
-      } catch (LockFailedException e) {
-        result = RefUpdate.Result.LOCK_FAILURE;
-      } catch (IOException e) {
-        logger.atSevere().withCause(e).log("Cannot delete %s", ref);
-        throw e;
-      }
-      if (result == RefUpdate.Result.LOCK_FAILURE && --remainingLockFailureCalls > 0) {
+
+    projectState.checkStatePermitsWrite();
+    permissionBackend
+        .currentUser()
+        .project(projectState.getNameKey())
+        .ref(ref)
+        .check(RefPermission.DELETE);
+
+    try (Repository repository = repoManager.openRepository(projectState.getNameKey())) {
+      RefUpdate.Result result;
+      RefUpdate u = repository.updateRef(ref);
+      u.setExpectedOldObjectId(repository.exactRef(ref).getObjectId());
+      u.setNewObjectId(ObjectId.zeroId());
+      u.setForceUpdate(true);
+      refDeletionValidator.validateRefOperation(projectState.getName(), identifiedUser.get(), u);
+      int remainingLockFailureCalls = MAX_LOCK_FAILURE_CALLS;
+      for (; ; ) {
         try {
-          Thread.sleep(SLEEP_ON_LOCK_FAILURE_MS);
-        } catch (InterruptedException ie) {
-          // ignore
+          result = u.delete();
+        } catch (LockFailedException e) {
+          result = RefUpdate.Result.LOCK_FAILURE;
+        } catch (IOException e) {
+          logger.atSevere().withCause(e).log("Cannot delete %s", ref);
+          throw e;
         }
-      } else {
-        break;
+        if (result == RefUpdate.Result.LOCK_FAILURE && --remainingLockFailureCalls > 0) {
+          try {
+            Thread.sleep(SLEEP_ON_LOCK_FAILURE_MS);
+          } catch (InterruptedException ie) {
+            // ignore
+          }
+        } else {
+          break;
+        }
       }
-    }
 
-    switch (result) {
-      case NEW:
-      case NO_CHANGE:
-      case FAST_FORWARD:
-      case FORCED:
-        referenceUpdated.fire(
-            resource.getNameKey(), u, ReceiveCommand.Type.DELETE, identifiedUser.get().state());
-        break;
+      switch (result) {
+        case NEW:
+        case NO_CHANGE:
+        case FAST_FORWARD:
+        case FORCED:
+          referenceUpdated.fire(
+              projectState.getNameKey(),
+              u,
+              ReceiveCommand.Type.DELETE,
+              identifiedUser.get().state());
+          break;
 
-      case REJECTED_CURRENT_BRANCH:
-        logger.atSevere().log("Cannot delete %s: %s", ref, result.name());
-        throw new ResourceConflictException("cannot delete current branch");
+        case REJECTED_CURRENT_BRANCH:
+          logger.atSevere().log("Cannot delete %s: %s", ref, result.name());
+          throw new ResourceConflictException("cannot delete current branch");
 
-      case IO_FAILURE:
-      case LOCK_FAILURE:
-      case NOT_ATTEMPTED:
-      case REJECTED:
-      case RENAMED:
-      case REJECTED_MISSING_OBJECT:
-      case REJECTED_OTHER_REASON:
-      default:
-        logger.atSevere().log("Cannot delete %s: %s", ref, result.name());
-        throw new ResourceConflictException("cannot delete: " + result.name());
+        case IO_FAILURE:
+        case LOCK_FAILURE:
+        case NOT_ATTEMPTED:
+        case REJECTED:
+        case RENAMED:
+        case REJECTED_MISSING_OBJECT:
+        case REJECTED_OTHER_REASON:
+        default:
+          logger.atSevere().log("Cannot delete %s: %s", ref, result.name());
+          throw new ResourceConflictException("cannot delete: " + result.name());
+      }
     }
   }
 
-  private void deleteMultipleRefs(Repository r)
-      throws OrmException, IOException, ResourceConflictException, PermissionBackendException {
-    BatchRefUpdate batchUpdate = r.getRefDatabase().newBatchUpdate();
-    batchUpdate.setAtomic(false);
-    List<String> refs =
-        prefix == null
-            ? refsToDelete
-            : refsToDelete
-                .stream()
-                .map(ref -> ref.startsWith(R_REFS) ? ref : prefix + ref)
-                .collect(toList());
-    for (String ref : refs) {
-      batchUpdate.addCommand(createDeleteCommand(resource, r, ref));
+  /**
+   * Deletes a set of refs from the repository.
+   *
+   * @param projectState the {@code ProjectState} of the project whose refs are to be deleted.
+   * @param refsToDelete the refs to be deleted.
+   * @param prefix the prefix of the refs.
+   * @throws OrmException
+   * @throws IOException
+   * @throws ResourceConflictException
+   * @throws PermissionBackendException
+   */
+  public void deleteMultipleRefs(
+      ProjectState projectState, ImmutableSet<String> refsToDelete, String prefix)
+      throws OrmException, IOException, ResourceConflictException, PermissionBackendException,
+          AuthException {
+    if (refsToDelete.isEmpty()) {
+      return;
     }
-    try (RevWalk rw = new RevWalk(r)) {
-      batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
+
+    if (refsToDelete.size() == 1) {
+      deleteSingleRef(projectState, Iterables.getOnlyElement(refsToDelete), prefix);
+      return;
     }
-    StringBuilder errorMessages = new StringBuilder();
-    for (ReceiveCommand command : batchUpdate.getCommands()) {
-      if (command.getResult() == Result.OK) {
-        postDeletion(resource, command);
-      } else {
-        appendAndLogErrorMessage(errorMessages, command);
+
+    try (Repository repository = repoManager.openRepository(projectState.getNameKey())) {
+      BatchRefUpdate batchUpdate = repository.getRefDatabase().newBatchUpdate();
+      batchUpdate.setAtomic(false);
+      ImmutableSet<String> refs =
+          prefix == null
+              ? refsToDelete
+              : refsToDelete
+                  .stream()
+                  .map(ref -> ref.startsWith(R_REFS) ? ref : prefix + ref)
+                  .collect(toImmutableSet());
+      for (String ref : refs) {
+        batchUpdate.addCommand(createDeleteCommand(projectState, repository, ref));
       }
-    }
-    if (errorMessages.length() > 0) {
-      throw new ResourceConflictException(errorMessages.toString());
+      try (RevWalk rw = new RevWalk(repository)) {
+        batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
+      }
+      StringBuilder errorMessages = new StringBuilder();
+      for (ReceiveCommand command : batchUpdate.getCommands()) {
+        if (command.getResult() == Result.OK) {
+          postDeletion(projectState.getNameKey(), command);
+        } else {
+          appendAndLogErrorMessage(errorMessages, command);
+        }
+      }
+      if (errorMessages.length() > 0) {
+        throw new ResourceConflictException(errorMessages.toString());
+      }
     }
   }
 
-  private ReceiveCommand createDeleteCommand(ProjectResource project, Repository r, String refName)
+  private ReceiveCommand createDeleteCommand(
+      ProjectState projectState, Repository r, String refName)
       throws OrmException, IOException, ResourceConflictException, PermissionBackendException {
     Ref ref = r.getRefDatabase().getRef(refName);
     ReceiveCommand command;
@@ -227,7 +252,7 @@
       try {
         permissionBackend
             .currentUser()
-            .project(project.getNameKey())
+            .project(projectState.getNameKey())
             .ref(refName)
             .check(RefPermission.DELETE);
       } catch (AuthException denied) {
@@ -237,12 +262,12 @@
       }
     }
 
-    if (!project.getProjectState().statePermitsWrite()) {
+    if (!projectState.statePermitsWrite()) {
       command.setResult(Result.REJECTED_OTHER_REASON, "project state does not permit write");
     }
 
     if (!refName.startsWith(R_TAGS)) {
-      Branch.NameKey branchKey = new Branch.NameKey(project.getNameKey(), ref.getName());
+      Branch.NameKey branchKey = new Branch.NameKey(projectState.getNameKey(), ref.getName());
       if (!queryProvider.get().setLimit(1).byBranchOpen(branchKey).isEmpty()) {
         command.setResult(Result.REJECTED_OTHER_REASON, "it has open changes");
       }
@@ -252,7 +277,7 @@
     u.setForceUpdate(true);
     u.setExpectedOldObjectId(r.exactRef(refName).getObjectId());
     u.setNewObjectId(ObjectId.zeroId());
-    refDeletionValidator.validateRefOperation(project.getName(), identifiedUser.get(), u);
+    refDeletionValidator.validateRefOperation(projectState.getName(), identifiedUser.get(), u);
     return command;
   }
 
@@ -281,7 +306,7 @@
     errorMessages.append("\n");
   }
 
-  private void postDeletion(ProjectResource project, ReceiveCommand cmd) {
-    referenceUpdated.fire(project.getNameKey(), cmd, identifiedUser.get().state());
+  private void postDeletion(Project.NameKey project, ReceiveCommand cmd) {
+    referenceUpdated.fire(project, cmd, identifiedUser.get().state());
   }
 }
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteTag.java b/java/com/google/gerrit/server/restapi/project/DeleteTag.java
index bd5f444..f7cce11 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteTag.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteTag.java
@@ -21,9 +21,7 @@
 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.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.RefUtil;
 import com.google.gerrit.server.project.TagResource;
 import com.google.gwtorm.server.OrmException;
@@ -34,13 +32,11 @@
 @Singleton
 public class DeleteTag implements RestModifyView<TagResource, Input> {
 
-  private final PermissionBackend permissionBackend;
-  private final DeleteRef.Factory deleteRefFactory;
+  private final DeleteRef deleteRef;
 
   @Inject
-  DeleteTag(PermissionBackend permissionBackend, DeleteRef.Factory deleteRefFactory) {
-    this.permissionBackend = permissionBackend;
-    this.deleteRefFactory = deleteRefFactory;
+  DeleteTag(DeleteRef deleteRef) {
+    this.deleteRef = deleteRef;
   }
 
   @Override
@@ -53,13 +49,7 @@
       throw new MethodNotAllowedException("not allowed to delete " + tag);
     }
 
-    permissionBackend
-        .currentUser()
-        .project(resource.getNameKey())
-        .ref(tag)
-        .check(RefPermission.DELETE);
-    resource.getProjectState().checkStatePermitsWrite();
-    deleteRefFactory.create(resource).ref(tag).delete();
+    deleteRef.deleteSingleRef(resource.getProjectState(), tag);
     return Response.none();
   }
 }
diff --git a/java/com/google/gerrit/server/restapi/project/DeleteTags.java b/java/com/google/gerrit/server/restapi/project/DeleteTags.java
index 83fa1ea..bf2c524 100644
--- a/java/com/google/gerrit/server/restapi/project/DeleteTags.java
+++ b/java/com/google/gerrit/server/restapi/project/DeleteTags.java
@@ -16,6 +16,7 @@
 
 import static org.eclipse.jgit.lib.Constants.R_TAGS;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.extensions.api.projects.DeleteTagsInput;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.Response;
@@ -30,11 +31,11 @@
 
 @Singleton
 public class DeleteTags implements RestModifyView<ProjectResource, DeleteTagsInput> {
-  private final DeleteRef.Factory deleteRefFactory;
+  private final DeleteRef deleteRef;
 
   @Inject
-  DeleteTags(DeleteRef.Factory deleteRefFactory) {
-    this.deleteRefFactory = deleteRefFactory;
+  DeleteTags(DeleteRef deleteRef) {
+    this.deleteRef = deleteRef;
   }
 
   @Override
@@ -43,7 +44,8 @@
     if (input == null || input.tags == null || input.tags.isEmpty()) {
       throw new BadRequestException("tags must be specified");
     }
-    deleteRefFactory.create(project).refs(input.tags).prefix(R_TAGS).delete();
+    deleteRef.deleteMultipleRefs(
+        project.getProjectState(), ImmutableSet.copyOf(input.tags), R_TAGS);
     return Response.none();
   }
 }
diff --git a/java/com/google/gerrit/server/restapi/project/ListDashboards.java b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
index 06dbdb0..3808a2f 100644
--- a/java/com/google/gerrit/server/restapi/project/ListDashboards.java
+++ b/java/com/google/gerrit/server/restapi/project/ListDashboards.java
@@ -16,6 +16,7 @@
 
 import static com.google.gerrit.reviewdb.client.RefNames.REFS_DASHBOARDS;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.extensions.api.projects.DashboardInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -100,15 +101,15 @@
 
   private List<DashboardInfo> scan(ProjectState state, String project, boolean setDefault)
       throws ResourceNotFoundException, IOException, PermissionBackendException {
+    if (!state.statePermitsRead()) {
+      return ImmutableList.of();
+    }
+
     PermissionBackend.ForProject perm = permissionBackend.currentUser().project(state.getNameKey());
     try (Repository git = gitManager.openRepository(state.getNameKey());
         RevWalk rw = new RevWalk(git)) {
       List<DashboardInfo> all = new ArrayList<>();
       for (Ref ref : git.getRefDatabase().getRefsByPrefix(REFS_DASHBOARDS)) {
-        if (!state.statePermitsRead()) {
-          continue;
-        }
-
         try {
           perm.ref(ref.getName()).check(RefPermission.READ);
           all.addAll(scanDashboards(state.getProject(), git, rw, ref, project, setDefault));
diff --git a/java/com/google/gerrit/server/restapi/project/Module.java b/java/com/google/gerrit/server/restapi/project/Module.java
index f2047e0..a57438e 100644
--- a/java/com/google/gerrit/server/restapi/project/Module.java
+++ b/java/com/google/gerrit/server/restapi/project/Module.java
@@ -104,7 +104,6 @@
     put(PROJECT_KIND, "config").to(PutConfig.class);
     post(COMMIT_KIND, "cherrypick").to(CherryPickCommit.class);
 
-    factory(DeleteRef.Factory.class);
     factory(ProjectNode.Factory.class);
   }
 }
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index 0b7758e..0017e08 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -156,12 +156,12 @@
       Class<? extends RestApiException> errType,
       String errMsg)
       throws Exception {
-    exception.expect(errType);
+    BranchInput in = new BranchInput();
+    in.revision = revision;
     if (errMsg != null) {
       exception.expectMessage(errMsg);
     }
-    BranchInput in = new BranchInput();
-    in.revision = revision;
+    exception.expect(errType);
     branch(branch).create(in);
   }
 
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
index 008997b..330f2b8 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/DeleteBranchesIT.java
@@ -28,6 +28,7 @@
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
 import com.google.gerrit.extensions.api.projects.ProjectApi;
+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.reviewdb.client.RefNames;
@@ -64,7 +65,24 @@
   }
 
   @Test
-  public void deleteBranchesForbidden() throws Exception {
+  public void deleteOneBranchWithoutPermissionForbidden() throws Exception {
+    ImmutableList<String> branchToDelete = ImmutableList.of("refs/heads/test-1");
+
+    DeleteBranchesInput input = new DeleteBranchesInput();
+    input.branches = branchToDelete;
+    setApiUser(user);
+    try {
+      project().deleteBranches(input);
+      fail("Expected AuthException");
+    } catch (AuthException e) {
+      assertThat(e).hasMessageThat().isEqualTo("delete not permitted for refs/heads/test-1");
+    }
+    setApiUser(admin);
+    assertBranches(BRANCHES);
+  }
+
+  @Test
+  public void deleteMultiBranchesWithoutPermissionForbidden() throws Exception {
     DeleteBranchesInput input = new DeleteBranchesInput();
     input.branches = BRANCHES;
     setApiUser(user);
diff --git a/javatests/com/google/gerrit/index/query/NotPredicateTest.java b/javatests/com/google/gerrit/index/query/NotPredicateTest.java
index 88d8349..74eac61 100644
--- a/javatests/com/google/gerrit/index/query/NotPredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/NotPredicateTest.java
@@ -50,17 +50,26 @@
     final TestPredicate p = f("author", "bob");
     final Predicate<String> n = not(p);
 
-    exception.expect(UnsupportedOperationException.class);
-    n.getChildren().clear();
-    assertOnlyChild("clear", p, n);
+    try {
+      n.getChildren().clear();
+      fail("Expected UnsupportedOperationException");
+    } catch (UnsupportedOperationException e) {
+      assertOnlyChild("clear", p, n);
+    }
 
-    exception.expect(UnsupportedOperationException.class);
-    n.getChildren().remove(0);
-    assertOnlyChild("remove(0)", p, n);
+    try {
+      n.getChildren().remove(0);
+      fail("Expected UnsupportedOperationException");
+    } catch (UnsupportedOperationException e) {
+      assertOnlyChild("remove(0)", p, n);
+    }
 
-    exception.expect(UnsupportedOperationException.class);
-    n.getChildren().iterator().remove();
-    assertOnlyChild("remove(0)", p, n);
+    try {
+      n.getChildren().iterator().remove();
+      fail("Expected UnsupportedOperationException");
+    } catch (UnsupportedOperationException e) {
+      assertOnlyChild("remove()", p, n);
+    }
   }
 
   private static void assertOnlyChild(String o, Predicate<String> c, Predicate<String> p) {
diff --git a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
index 2e19d20..2031a14 100644
--- a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
@@ -163,8 +163,16 @@
     assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(3)))
         .containsExactly(ps1Comment1.key, true, ps1Comment2.key, true, ps2Comment1.key, true);
 
+    // Check that dryRun doesn't touch anything.
+    String refName = RefNames.changeMetaRef(c.getId());
+    ObjectId before = repo.getRefDatabase().getRef(refName).getObjectId();
+    ProjectMigrationResult dryRunResult = migrator.migrateProject(project, repo, true);
+    ObjectId after = repo.getRefDatabase().getRef(refName).getObjectId();
+    assertThat(before).isEqualTo(after);
+    assertThat(dryRunResult.refsUpdated).isEqualTo(ImmutableList.of(refName));
+
     ChangeNotes oldNotes = notes;
-    checkMigrate(project, ImmutableList.of(RefNames.changeMetaRef(c.getId())));
+    checkMigrate(project, ImmutableList.of(refName));
 
     // Comment content is the same.
     notes = newNotes(c);