Fix NPE when the replicated refs is not found locally
When the replication done events are received, the local ref
may or may not be replicated locally.
Do not fail with NPE but just trace a warning on the error log
saying what happened.
Also, do not act on ref-replicated event but rather on the
ref-replication-done event, which gives more guarantee that
the replication has successfully reached all nodes.
Change-Id: I50acc663b586cf2191a74b577ead0ffdbfb6f363
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java
index bcdf63a..848f2fa 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdate.java
@@ -32,7 +32,7 @@
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.multisite.SharedRefDatabaseWrapper;
import com.googlesource.gerrit.plugins.multisite.forwarder.Context;
-import com.googlesource.gerrit.plugins.replication.RefReplicatedEvent;
+import com.googlesource.gerrit.plugins.replication.RefReplicationDoneEvent;
import java.io.IOException;
import java.util.Optional;
import java.util.Set;
@@ -46,7 +46,6 @@
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.transport.RemoteRefUpdate;
@Singleton
public class ProjectVersionRefUpdate implements EventListener {
@@ -76,27 +75,21 @@
updateProducerProjectVersionUpdate((RefUpdatedEvent) event);
}
- // Consumers of the Event use RefReplicatedEvent to trigger the version update
- if (Context.isForwardedEvent() && event instanceof RefReplicatedEvent) {
- updateConsumerProjectVersion((RefReplicatedEvent) event);
+ // Consumers of the Event use RefReplicationDoneEvent to trigger the version update
+ if (Context.isForwardedEvent() && event instanceof RefReplicationDoneEvent) {
+ updateConsumerProjectVersion((RefReplicationDoneEvent) event);
}
}
- private void updateConsumerProjectVersion(RefReplicatedEvent refReplicatedEvent) {
- Project.NameKey projectNameKey = refReplicatedEvent.getProjectNameKey();
- if (!refReplicatedEvent.refStatus.equals(RemoteRefUpdate.Status.OK)) {
- logger.atFine().log(
- String.format(
- "Skipping version update for %s. RefReplicatedEvent failed with %s",
- projectNameKey.get(), refReplicatedEvent.refStatus));
- return;
- }
- if (refReplicatedEvent.getRefName().startsWith(SEQUENCE_REF_PREFIX)) {
+ private void updateConsumerProjectVersion(RefReplicationDoneEvent refReplicationDoneEvent) {
+ Project.NameKey projectNameKey = refReplicationDoneEvent.getProjectNameKey();
+
+ if (refReplicationDoneEvent.getRefName().startsWith(SEQUENCE_REF_PREFIX)) {
logger.atFine().log("Found Sequence ref, skipping update for " + projectNameKey.get());
return;
}
try {
- updateLocalProjectVersion(projectNameKey, refReplicatedEvent.getRefName());
+ updateLocalProjectVersion(projectNameKey, refReplicationDoneEvent.getRefName());
} catch (LocalProjectVersionUpdateException e) {
logger.atSevere().withCause(e).log(
"Issue encountered when updating version for project " + projectNameKey);
@@ -112,10 +105,17 @@
try {
Project.NameKey projectNameKey = refUpdatedEvent.getProjectNameKey();
Ref currentProjectVersionRef = getLocalProjectVersionRef(refUpdatedEvent.getProjectNameKey());
- ObjectId newProjectVersionObjectId =
+ Optional<ObjectId> newProjectVersionObjectId =
updateLocalProjectVersion(projectNameKey, refUpdatedEvent.getRefName());
- updateSharedProjectVersion(
- projectNameKey, currentProjectVersionRef, newProjectVersionObjectId);
+
+ if (newProjectVersionObjectId.isPresent()) {
+ updateSharedProjectVersion(
+ projectNameKey, currentProjectVersionRef, newProjectVersionObjectId.get());
+ } else {
+ logger.atWarning().log(
+ "Ref %s not found on projet %s: skipping project version update",
+ refUpdatedEvent.getRefName(), projectNameKey);
+ }
} catch (LocalProjectVersionUpdateException | SharedProjectVersionUpdateException e) {
logger.atSevere().withCause(e).log(
"Issue encountered when updating version for project "
@@ -151,16 +151,20 @@
}
}
- private Long getLastRefUpdatedTimestamp(Project.NameKey projectNameKey, String refName)
+ private Optional<Long> getLastRefUpdatedTimestamp(Project.NameKey projectNameKey, String refName)
throws LocalProjectVersionUpdateException {
logger.atFine().log(
String.format(
"Getting last ref updated time for project %s, ref %s", projectNameKey.get(), refName));
try (Repository repository = gitRepositoryManager.openRepository(projectNameKey)) {
Ref ref = repository.findRef(refName);
+ if (ref == null) {
+ logger.atWarning().log("Unable to find ref " + refName + " in project " + projectNameKey);
+ return Optional.empty();
+ }
try (RevWalk walk = new RevWalk(repository)) {
RevCommit commit = walk.parseCommit(ref.getObjectId());
- return Integer.toUnsignedLong(commit.getCommitTime());
+ return Optional.of(Integer.toUnsignedLong(commit.getCommitTime()));
}
} catch (IOException ioe) {
String message =
@@ -252,22 +256,26 @@
}
}
- private ObjectId updateLocalProjectVersion(Project.NameKey projectNameKey, String refName)
- throws LocalProjectVersionUpdateException {
- Long lastRefUpdatedTimestamp = getLastRefUpdatedTimestamp(projectNameKey, refName);
+ private Optional<ObjectId> updateLocalProjectVersion(
+ Project.NameKey projectNameKey, String refName) throws LocalProjectVersionUpdateException {
+ Optional<Long> lastRefUpdatedTimestamp = getLastRefUpdatedTimestamp(projectNameKey, refName);
+ if (!lastRefUpdatedTimestamp.isPresent()) {
+ return Optional.empty();
+ }
+
logger.atFine().log("Updating local version for project " + projectNameKey.get());
try (Repository repository = gitRepositoryManager.openRepository(projectNameKey)) {
- RefUpdate refUpdate = getProjectVersionRefUpdate(repository, lastRefUpdatedTimestamp);
+ RefUpdate refUpdate = getProjectVersionRefUpdate(repository, lastRefUpdatedTimestamp.get());
RefUpdate.Result result = refUpdate.update();
if (!isSuccessful(result)) {
String message =
String.format(
"RefUpdate failed with result %s for: project=%s, version=%d",
- result.name(), projectNameKey.get(), lastRefUpdatedTimestamp);
+ result.name(), projectNameKey.get(), lastRefUpdatedTimestamp.get());
logger.atSevere().log(message);
throw new LocalProjectVersionUpdateException(message);
}
- return refUpdate.getNewObjectId();
+ return Optional.of(refUpdate.getNewObjectId());
} catch (IOException e) {
String message = "Cannot create versioning command for " + projectNameKey.get();
logger.atSevere().withCause(e).log(message);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
index 7d38ac1..49b4bbf 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/ProjectVersionRefUpdateTest.java
@@ -29,6 +29,7 @@
import com.googlesource.gerrit.plugins.multisite.forwarder.Context;
import com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb.RefFixture;
import com.googlesource.gerrit.plugins.replication.RefReplicatedEvent;
+import com.googlesource.gerrit.plugins.replication.RefReplicationDoneEvent;
import com.googlesource.gerrit.plugins.replication.ReplicationState;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
@@ -127,15 +128,10 @@
}
@Test
- public void consumerShouldUpdateProjectVersionUponRefReplicatedEvent() throws IOException {
+ public void consumerShouldUpdateProjectVersionUponRefReplicationDoneEvent() throws IOException {
Context.setForwardedEvent(true);
- RefReplicatedEvent refReplicatedEvent =
- new RefReplicatedEvent(
- A_TEST_PROJECT_NAME,
- A_TEST_REF_NAME,
- "targetNode",
- ReplicationState.RefPushResult.SUCCEEDED,
- RemoteRefUpdate.Status.OK);
+ RefReplicationDoneEvent refReplicatedEvent =
+ new RefReplicationDoneEvent(A_TEST_PROJECT_NAME, A_TEST_REF_NAME, 1);
new ProjectVersionRefUpdate(repoManager, sharedRefDb).onEvent(refReplicatedEvent);
@@ -206,6 +202,7 @@
public void getLocalProjectVersionShouldReturnCorrectValue() throws IOException {
updateLocalVersion();
Ref ref = repo.getRepository().findRef(MULTI_SITE_VERSIONING_REF);
+ assertThat(ref).isNotNull();
Optional<Long> version =
new ProjectVersionRefUpdate(repoManager, sharedRefDb)
@@ -217,13 +214,8 @@
private void updateLocalVersion() {
Context.setForwardedEvent(true);
- RefReplicatedEvent refReplicatedEvent =
- new RefReplicatedEvent(
- A_TEST_PROJECT_NAME,
- A_TEST_REF_NAME,
- "targetNode",
- ReplicationState.RefPushResult.SUCCEEDED,
- RemoteRefUpdate.Status.OK);
+ RefReplicationDoneEvent refReplicatedEvent =
+ new RefReplicationDoneEvent(A_TEST_PROJECT_NAME, A_TEST_REF_NAME, 1);
new ProjectVersionRefUpdate(repoManager, sharedRefDb).onEvent(refReplicatedEvent);
}
}
