Be more consistent about object ids used in ref operation validationv2.13.7

In CreateBranch, a RefUpdate is created with expectedOldObjectId set
but not oldObjectId. In DeleteBranch[es], none of expectedOldObjectId,
oldObjectId, and newObjectId is set.

As a result, when RefOperationValidators uses oldObjectId and
newObjectId to create a ReceiveCommand, they can be null, resulting in
NullPointerException when a listener dereferences them.

So use expectedOldObjectId instead of oldObjectId in
RefOperationValidators. Before a ref update has been performed, only
the former is meant to be set.

In DeleteBranch[es], set the expectedOldObjectId and the newObjectId
(which is zero). This way, in RefOperationValidators we know both values
will always be set.

Bug: Issue 5817
Helped-By: Jonathan Nieder <jrn@google.com>
Change-Id: If1f3a6179fa789077731a16e4b731227a73be7f2

3 files changed, 8 insertions, 2 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java
index 769c7d27ac..580de95e62 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java
@@ -41,7 +41,7 @@ public class RefOperationValidators {
   }
 
   public static ReceiveCommand getCommand(RefUpdate update, ReceiveCommand.Type type) {
-    return new ReceiveCommand(update.getOldObjectId(), update.getNewObjectId(),
+    return new ReceiveCommand(update.getExpectedOldObjectId(), update.getNewObjectId(),
         update.getName(), type);
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
index 091cba364f..732d47bb14 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
@@ -29,6 +29,7 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import org.eclipse.jgit.errors.LockFailedException;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.ReceiveCommand;
@@ -80,7 +81,10 @@ public class DeleteBranch implements RestModifyView<BranchResource, Input> {
 
     try (Repository r = repoManager.openRepository(rsrc.getNameKey())) {
       RefUpdate.Result result;
-      RefUpdate u = r.updateRef(rsrc.getRef());
+      String ref = rsrc.getRef();
+      RefUpdate u = r.updateRef(ref);
+      u.setExpectedOldObjectId(r.exactRef(ref).getObjectId());
+      u.setNewObjectId(ObjectId.zeroId());
       u.setForceUpdate(true);
       refDeletionValidator.validateRefOperation(
           rsrc.getName(), identifiedUser.get(), u);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
index f4fa4468ee..7d53fecc8b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranches.java
@@ -127,6 +127,8 @@ public class DeleteBranches
       command.setResult(Result.REJECTED_OTHER_REASON, "it has open changes");
     }
     RefUpdate u = r.updateRef(branch);
+    u.setExpectedOldObjectId(r.exactRef(branch).getObjectId());
+    u.setNewObjectId(ObjectId.zeroId());
     u.setForceUpdate(true);
     refDeletionValidator.validateRefOperation(
         project.getName(), identifiedUser.get(), u);