Be more consistent about object ids used in ref operation validation
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
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 769c7d2..580de95 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 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 091cba3..732d47b 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.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 @@
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 f4fa446..7d53fec 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 @@
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);