Do not try to delete inexistent refs
Delete ref throws NPE when ref marked to be deleted doesn't exist
in the local repository. This can happen if repo is out of sync
or ref was deleted via different channel (for example via apply object).
Deleting a non-existent ref will just be a NOOP.
The apply-object API will still return a 200.
Bug: Issue 15873
Change-Id: Ic945a44268d7998f312f067ed009ae8af221a63f
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java
index ea5b35a..62c7e25 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java
@@ -37,6 +37,7 @@
import java.io.IOException;
import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
@@ -72,10 +73,16 @@
throw new ResourceNotFoundException(String.format("Project %s was not found", name));
}
+ Optional<Ref> ref = getRef(name, refName);
+ if (!ref.isPresent()) {
+ logger.atFine().log("Ref %s was not found in project %s", refName, name);
+ return;
+ }
+
try {
Context.setLocalEvent(true);
- deleteRef(name, refName);
+ deleteRef(name, ref.get());
eventDispatcher
.get()
@@ -119,17 +126,24 @@
}
}
- private RefUpdateState deleteRef(Project.NameKey name, String refName) throws IOException {
+ private Optional<Ref> getRef(Project.NameKey repo, String refName) throws IOException {
+ try (Repository repository = gitManager.openRepository(repo)) {
+ Ref ref = repository.exactRef(refName);
+ return Optional.ofNullable(ref);
+ }
+ }
+ private RefUpdateState deleteRef(Project.NameKey name, Ref ref) throws IOException {
try (Repository repository = gitManager.openRepository(name)) {
+
RefUpdate.Result result;
- RefUpdate u = repository.updateRef(refName);
- u.setExpectedOldObjectId(repository.exactRef(refName).getObjectId());
+ RefUpdate u = repository.updateRef(ref.getName());
+ u.setExpectedOldObjectId(ref.getObjectId());
u.setNewObjectId(ObjectId.zeroId());
u.setForceUpdate(true);
result = u.delete();
- return new RefUpdateState(refName, result);
+ return new RefUpdateState(ref.getName(), result);
}
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java
index 30d212c..15cfa4b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -50,6 +51,8 @@
public class DeleteRefCommandTest {
private static final String TEST_SOURCE_LABEL = "test-source-label";
private static final String TEST_REF_NAME = "refs/changes/01/1/1";
+
+ private static final String NON_EXISTING_REF_NAME = "refs/changes/01/11101/1";
private static final NameKey TEST_PROJECT_NAME = Project.nameKey("test-project");
@Mock private PullReplicationStateLogger fetchStateLog;
@@ -97,4 +100,13 @@
assertThat(fetchEvent.getProjectNameKey()).isEqualTo(TEST_PROJECT_NAME);
assertThat(fetchEvent.getRefName()).isEqualTo(TEST_REF_NAME);
}
+
+ @Test
+ public void shouldHandleNonExistingRef() throws Exception {
+ when(refDb.exactRef(anyString())).thenReturn(null);
+
+ objectUnderTest.deleteRef(TEST_PROJECT_NAME, NON_EXISTING_REF_NAME, TEST_SOURCE_LABEL);
+
+ verify(eventDispatcher, never()).postEvent(any());
+ }
}
