Merge pull request #307 from s4u/key-not-found-fix

propagate KeyNotFound on first server to client
s4u · Jul 16, 2021 · ab9eb8d · ab9eb8d
2 parents 13c0bda + 03f65b3
commit ab9eb8d
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 13 deletions.
diff --git a/src/main/java/org/simplify4u/plugins/keyserver/PGPKeysCache.java b/src/main/java/org/simplify4u/plugins/keyserver/PGPKeysCache.java
@@ -42,22 +42,21 @@
 import static org.simplify4u.plugins.utils.ExceptionUtils.getMessage;
 
 import io.vavr.control.Try;
+import lombok.extern.slf4j.Slf4j;
 import org.bouncycastle.openpgp.PGPException;
 import org.bouncycastle.openpgp.PGPPublicKeyRing;
 import org.simplify4u.plugins.pgp.KeyId;
 import org.simplify4u.plugins.pgp.PublicKeyUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Manage PGP keys local cache.
  *
  * @author Slawomir Jaranowski.
  */
+@Slf4j
 @Named
 public class PGPKeysCache {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(PGPKeysCache.class);
     private static final String NL = System.lineSeparator();
     private static final Object LOCK = new Object();
 
@@ -71,10 +70,12 @@ public class PGPKeysCache {
 
     /**
      * Init Keys cache.
-     * @param cachePath a path where cache will be stored
-     * @param keyServers a list of key servers addresses
-     * @param loadBalance if use key servers list in balance mode
+     *
+     * @param cachePath      a path where cache will be stored
+     * @param keyServers     a list of key servers addresses
+     * @param loadBalance    if use key servers list in balance mode
      * @param clientSettings a kay server client settings
+     *
      * @throws IOException in case of problems
      */
     public void init(File cachePath, String keyServers, boolean loadBalance, KeyServerClientSettings clientSettings)
@@ -153,7 +154,9 @@ public String getUrlForShowKey(KeyId keyID) {
      * Return Public Key Ring from local cache or from key server.
      *
      * @param keyID a keyId for lookup
+     *
      * @return Public Key Ring for given key
+     *
      * @throws IOException in case of problems
      */
     public PGPPublicKeyRing getKeyRing(KeyId keyID) throws IOException {
@@ -291,7 +294,10 @@ protected Optional<PGPPublicKeyRing> executeWithClient(KeyServerExecutor executo
                 lastClient = client;
                 return ret;
             } catch (IOException e) {
-                lastException = e;
+                if (!(lastException instanceof PGPKeyNotFound)) {
+                    // if key was not found on one server - don't override
+                    lastException = e;
+                }
                 LOGGER.warn("{} throw exception: {} - {} try next client", client, getMessage(e), getName());
             }
             return Optional.empty();

diff --git a/src/test/java/org/simplify4u/plugins/keyserver/PGPKeysCacheTest.java b/src/test/java/org/simplify4u/plugins/keyserver/PGPKeysCacheTest.java
@@ -333,11 +333,11 @@ public void loadBalanceIterateByAllServer() throws IOException {
     }
 
     @DataProvider(name = "keyServerListWithFallBack")
-    public Object[] keyServerListWithFallBack() {
+    public Object[][] keyServerListWithFallBack() {
 
-        return new Object[]{
-                new KeyServerListFallback(),
-                new KeyServerListLoadBalance()
+        return new Object[][]{
+                {new KeyServerListFallback()},
+                {new KeyServerListLoadBalance()}
         };
     }
 
@@ -406,12 +406,12 @@ public void throwsExceptionForAllFailedExecute(KeyServerList keyServerList) thro
     }
 
     @Test(dataProvider = "keyServerListWithFallBack")
-    public void throwsPGPKeyNotFoundWhenKeyNotFoundOnAnyServer(KeyServerList keyServerList) throws IOException {
+    public void throwsPGPKeyNotFoundWhenKeyNotFoundOnLastServer(KeyServerList keyServerList) throws IOException {
 
         PGPKeysServerClient client1 = mock(PGPKeysServerClient.class);
         PGPKeysServerClient client2 = mock(PGPKeysServerClient.class);
 
-        doThrow(new PGPKeyNotFound()).when(client1).copyKeyToOutputStream(KEY_ID_1, null, null);
+        doThrow(new IOException()).when(client1).copyKeyToOutputStream(KEY_ID_1, null, null);
         doThrow(new PGPKeyNotFound()).when(client2).copyKeyToOutputStream(KEY_ID_1, null, null);
 
         keyServerList.withClients(Arrays.asList(client1, client2));
@@ -434,4 +434,34 @@ public void throwsPGPKeyNotFoundWhenKeyNotFoundOnAnyServer(KeyServerList keyServ
         verify(client2).copyKeyToOutputStream(KEY_ID_1, null, null);
         verifyNoMoreInteractions(client2);
     }
+
+    @Test(dataProvider = "keyServerListWithFallBack")
+    public void throwsPGPKeyNotFoundWhenKeyNotFoundOnFirstServer(KeyServerList keyServerList) throws IOException {
+
+        PGPKeysServerClient client1 = mock(PGPKeysServerClient.class);
+        PGPKeysServerClient client2 = mock(PGPKeysServerClient.class);
+
+        doThrow(new PGPKeyNotFound()).when(client1).copyKeyToOutputStream(KEY_ID_1, null, null);
+        doThrow(new IOException()).when(client2).copyKeyToOutputStream(KEY_ID_1, null, null);
+
+        keyServerList.withClients(Arrays.asList(client1, client2));
+
+        assertThatCode(() ->
+                keyServerList.execute(client -> {
+                    client.copyKeyToOutputStream(KEY_ID_1, null, null);
+                    return null;
+                })
+        ).isExactlyInstanceOf(PGPKeyNotFound.class);
+
+        verify(keysCacheLogger).warn(eq("{} throw exception: {} - {} try next client"), eq(client1), isNull(), anyString());
+        verify(keysCacheLogger).warn(eq("{} throw exception: {} - {} try next client"), eq(client2), isNull(), anyString());
+        verify(keysCacheLogger).error("All servers from list was failed");
+        verifyNoMoreInteractions(keysCacheLogger);
+
+        verify(client1).copyKeyToOutputStream(KEY_ID_1, null, null);
+        verifyNoMoreInteractions(client1);
+
+        verify(client2).copyKeyToOutputStream(KEY_ID_1, null, null);
+        verifyNoMoreInteractions(client2);
+    }
 }