Refactor ExternalUrlKeyReader

courtneyeh · May 6, 2024 · 7837c7c · 7837c7c
1 parent 4770e62
commit 7837c7c
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 38 deletions.
diff --git a/.../client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java b/.../client/src/main/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReader.java
@@ -33,55 +33,48 @@ public class ExternalUrlKeyReader {
   private static final Duration FIXED_DELAY = Duration.ofSeconds(5);
   private static final int MAX_RETRIES = 59;
 
-  private final String url;
+  private final URL url;
   private final ObjectMapper mapper;
   private final AsyncRunner asyncRunner;
 
   public ExternalUrlKeyReader(
-      final String url, final ObjectMapper mapper, final AsyncRunner asyncRunner) {
-    this.url = url;
+      final String urlString, final ObjectMapper mapper, final AsyncRunner asyncRunner) {
+    this.url = getUrl(urlString);
     this.mapper = mapper;
     this.asyncRunner = asyncRunner;
   }
 
   public Stream<BLSPublicKey> readKeys() {
-    final String[] keys =
-        readUrl()
-            .exceptionallyCompose(
-                ex -> {
-                  if (ex instanceof MalformedURLException) {
-                    return SafeFuture.failedFuture(
-                        new InvalidConfigurationException(
-                            "Failed to load public keys from invalid URL: " + url, ex));
-                  }
-                  return retry();
-                })
-            .join();
+    final String[] keys = getKeysWithRetry().join();
     return Arrays.stream(keys).map(key -> BLSPublicKey.fromSSZBytes(Bytes.fromHexString(key)));
   }
 
-  private SafeFuture<String[]> readUrl() {
-    try {
-      return SafeFuture.completedFuture(mapper.readValue(new URL(url), String[].class));
-    } catch (IOException e) {
-      return SafeFuture.failedFuture(e);
-    }
-  }
-
   @VisibleForTesting
-  SafeFuture<String[]> retry() {
+  SafeFuture<String[]> getKeysWithRetry() {
     return asyncRunner
-        .runWithRetry(
-            () -> {
-              STATUS_LOG.failedToLoadPublicKeysFromUrl(url);
-              return readUrl();
-            },
-            FIXED_DELAY,
-            MAX_RETRIES)
+        .runWithRetry(this::readUrl, FIXED_DELAY, MAX_RETRIES)
         .exceptionallyCompose(
             ex ->
                 SafeFuture.failedFuture(
                     new InvalidConfigurationException(
                         "Failed to load public keys from URL: " + url, ex)));
   }
+
+  private SafeFuture<String[]> readUrl() {
+    try {
+      return SafeFuture.completedFuture(mapper.readValue(url, String[].class));
+    } catch (IOException e) {
+      STATUS_LOG.failedToLoadPublicKeysFromUrl(url.toString());
+      return SafeFuture.failedFuture(e);
+    }
+  }
+
+  private URL getUrl(final String url) {
+    try {
+      return new URL(url);
+    } catch (MalformedURLException e) {
+      throw new InvalidConfigurationException(
+          "Failed to load public keys from invalid URL: " + url, e);
+    }
+  }
 }
diff --git a/...ent/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java b/...ent/src/test/java/tech/pegasys/teku/validator/client/loader/ExternalUrlKeyReaderTest.java
@@ -91,11 +91,8 @@ void readKeys_validUrlReturnsInvalidKeys() throws IOException {
   @Test
   void readKeys_malformedUrlString() {
     final String invalidUrl = "invalid:url";
-    final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(invalidUrl, mapper, asyncRunner);
-
-    assertThatThrownBy(reader::readKeys)
-        .isInstanceOf(CompletionException.class)
-        .hasCauseInstanceOf(InvalidConfigurationException.class)
+    assertThatThrownBy(() -> new ExternalUrlKeyReader(invalidUrl, mapper, asyncRunner))
+        .isInstanceOf(InvalidConfigurationException.class)
         .hasMessageContaining("Failed to load public keys from invalid URL: " + invalidUrl)
         .hasRootCauseInstanceOf(MalformedURLException.class);
     verifyNoInteractions(mapper);
@@ -109,7 +106,7 @@ void readKeysWithRetry_unreachableUrlRetryUntilReachable() throws IOException {
         .thenReturn(expectedKeys);
     final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
 
-    final SafeFuture<String[]> keys = reader.retry();
+    final SafeFuture<String[]> keys = reader.getKeysWithRetry();
     for (int i = 0; i < 3; i++) {
       assertThat(keys).isNotCompleted();
       timeProvider.advanceTimeBy(DELAY);
@@ -125,7 +122,7 @@ void readKeysWithRetry_unreachableUrlRetryUntilMaxRetries() throws IOException {
     when(mapper.readValue(any(URL.class), eq(String[].class))).thenThrow(exception);
     final ExternalUrlKeyReader reader = new ExternalUrlKeyReader(VALID_URL, mapper, asyncRunner);
 
-    final SafeFuture<String[]> keys = reader.retry();
+    final SafeFuture<String[]> keys = reader.getKeysWithRetry();
     for (int i = 0; i < MAX_RETRIES; i++) {
       assertThat(keys).isNotCompleted();
       timeProvider.advanceTimeBy(DELAY);