Use an Optional<ReplicateRefUpdate> in list()
Using an Optional is safer than using nulls, and made it easy to also
fix the list() API to simply skip things that aren't present instead of
making the API users do that.
Add methods to ReplicateRefUpdate to create the
Optional<ReplicateRefUpdate> instead of creating them inside the list()
method; this helps separate concerns better.
Change-Id: Ib04a59be49d85b289d7dd94b9f2427819324153c
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
index 5a4d56c..94b3f31 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
@@ -203,10 +203,6 @@
try {
replaying = true;
for (ReplicationTasksStorage.ReplicateRefUpdate t : replicationTasksStorage.listWaiting()) {
- if (t == null) {
- repLog.atWarning().log("Encountered null replication event in ReplicationTasksStorage");
- continue;
- }
try {
fire(new URIish(t.uri), Project.nameKey(t.project), t.ref);
} catch (URISyntaxException e) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
index 4ef0d13..6884185 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
@@ -32,6 +32,7 @@
import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.List;
+import java.util.Optional;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.transport.URIish;
@@ -66,6 +67,24 @@
private boolean disableDeleteForTesting;
public static class ReplicateRefUpdate {
+ public static Optional<ReplicateRefUpdate> createOptionally(Path file) {
+ try {
+ return Optional.of(create(file));
+ } catch (NoSuchFileException e) {
+ logger.atFine().log("File %s not found while reading task", file);
+ } catch (IOException e) {
+ if (!e.getMessage().equals("Is a directory")) {
+ logger.atSevere().withCause(e).log("Error while reading task %", file);
+ }
+ }
+ return Optional.empty();
+ }
+
+ public static ReplicateRefUpdate create(Path file) throws IOException {
+ String json = new String(Files.readAllBytes(file), UTF_8);
+ return GSON.fromJson(json, ReplicateRefUpdate.class);
+ }
+
public final String project;
public final String ref;
public final String uri;
@@ -153,17 +172,9 @@
List<ReplicateRefUpdate> results = new ArrayList<>();
try (DirectoryStream<Path> events = Files.newDirectoryStream(tasks)) {
for (Path path : events) {
- if (Files.isRegularFile(path)) {
- try {
- String json = new String(Files.readAllBytes(path), UTF_8);
- results.add(GSON.fromJson(json, ReplicateRefUpdate.class));
- } catch (NoSuchFileException ex) {
- logger.atFine().log(
- "File %s not found while listing waiting tasks (likely in-flight or completed by another node)",
- path);
- } catch (IOException e) {
- logger.atSevere().withCause(e).log("Error when firing pending event %s", path);
- }
+ Optional<ReplicateRefUpdate> update = ReplicateRefUpdate.createOptionally(path);
+ if (update.isPresent()) {
+ results.add(update.get());
} else if (Files.isDirectory(path)) {
try {
results.addAll(list(path));
