From f78328690962fa56dc52dea173dabf61f602cb7e Mon Sep 17 00:00:00 2001
From: Stefan Kalscheuer <stefan@stklcode.de>
Date: Tue, 8 Jun 2021 19:28:25 +0200
Subject: [PATCH] pass builder as constructor parameter directly

With increasing number of options the constructors become quite overloaded.
We now pass the builder as only argument instead.
---
 .../jvault/connector/HTTPVaultConnector.java  | 42 +++++-----
 .../builder/HTTPVaultConnectorBuilder.java    | 76 ++++++++++++++++++-
 .../HTTPVaultConnectorOfflineTest.java        |  6 +-
 .../HTTPVaultConnectorBuilderTest.java        | 67 ++++++++++++----
 4 files changed, 150 insertions(+), 41 deletions(-)

diff --git a/src/main/java/de/stklcode/jvault/connector/HTTPVaultConnector.java b/src/main/java/de/stklcode/jvault/connector/HTTPVaultConnector.java
index 3894cab..6a203a3 100644
--- a/src/main/java/de/stklcode/jvault/connector/HTTPVaultConnector.java
+++ b/src/main/java/de/stklcode/jvault/connector/HTTPVaultConnector.java
@@ -16,6 +16,7 @@
 
 package de.stklcode.jvault.connector;
 
+import de.stklcode.jvault.connector.builder.HTTPVaultConnectorBuilder;
 import de.stklcode.jvault.connector.exception.AuthorizationRequiredException;
 import de.stklcode.jvault.connector.exception.InvalidRequestException;
 import de.stklcode.jvault.connector.exception.VaultConnectorException;
@@ -107,23 +108,6 @@ public class HTTPVaultConnector implements VaultConnector {
                 + prefix);
     }
 
-    /**
-     * Create connector using hostname, schema, port, path and trusted certificate.
-     *
-     * @param hostname      The hostname
-     * @param useTLS        If TRUE, use HTTPS, otherwise HTTP
-     * @param port          The port
-     * @param prefix        HTTP API prefix (default: /v1/)
-     * @param trustedCaCert Trusted CA certificate
-     */
-    public HTTPVaultConnector(final String hostname,
-                              final boolean useTLS,
-                              final Integer port,
-                              final String prefix,
-                              final X509Certificate trustedCaCert) {
-        this(hostname, useTLS, DEFAULT_TLS_VERSION, port, prefix, trustedCaCert, 0, null);
-    }
-
     /**
      * Create connector using hostname, schema, port, path and trusted certificate.
      *
@@ -144,14 +128,34 @@ public class HTTPVaultConnector implements VaultConnector {
                               final X509Certificate trustedCaCert,
                               final int numberOfRetries,
                               final Integer timeout) {
-        this(((useTLS) ? "https" : "http")
+        this(
+                ((useTLS) ? "https" : "http")
                         + "://" + hostname
                         + ((port != null) ? ":" + port : "")
                         + prefix,
                 trustedCaCert,
                 numberOfRetries,
                 timeout,
-                tlsVersion);
+                tlsVersion
+        );
+    }
+
+    /**
+     * Create connector using a {@link HTTPVaultConnectorBuilder}.
+     *
+     * @param builder The builder.
+     */
+    public HTTPVaultConnector(final HTTPVaultConnectorBuilder builder) {
+        this.request = new RequestHelper(
+                ((builder.isWithTLS()) ? "https" : "http") + "://" +
+                        builder.getHost() +
+                        ((builder.getPort() != null) ? ":" + builder.getPort() : "") +
+                        builder.getPrefix(),
+                builder.getNumberOfRetries(),
+                builder.getTimeout(),
+                builder.getTlsVersion(),
+                builder.getTrustedCA()
+        );
     }
 
     /**
diff --git a/src/main/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilder.java b/src/main/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilder.java
index 4adf80c..0551980 100644
--- a/src/main/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilder.java
+++ b/src/main/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilder.java
@@ -84,6 +84,15 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Get hostname.
+     *
+     * @return Hostname or IP address
+     */
+    public String getHost() {
+        return this.host;
+    }
+
     /**
      * Set port (default: 8200).
      *
@@ -95,6 +104,15 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Set port..
+     *
+     * @return Vault TCP port
+     */
+    public Integer getPort() {
+        return this.port;
+    }
+
     /**
      * Set TLS usage (default: TRUE).
      *
@@ -106,6 +124,24 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Get TLS usage flag.
+     *
+     * @return use TLS or not
+     */
+    public boolean isWithTLS() {
+        return this.tls;
+    }
+
+    /**
+     * Get TLS version.
+     *
+     * @return TLS version.
+     */
+    public String getTlsVersion() {
+        return this.tlsVersion;
+    }
+
     /**
      * Set TLS usage (default: TRUE).
      *
@@ -152,7 +188,7 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
     /**
      * Set API prefix. Default is "/v1/" and changes should not be necessary for current state of development.
      *
-     * @param prefix Vault API prefix (default: "/v1/"
+     * @param prefix Vault API prefix (default: "/v1/")
      * @return self
      */
     public HTTPVaultConnectorBuilder withPrefix(final String prefix) {
@@ -160,6 +196,15 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Get API prefix.
+     *
+     * @return Vault API prefix.
+     */
+    public String getPrefix() {
+        return this.prefix;
+    }
+
     /**
      * Add a trusted CA certificate for HTTPS connections.
      *
@@ -189,6 +234,15 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Get the trusted CA certificate for HTTPS connections.
+     *
+     * @return path to certificate file, if specified.
+     */
+    public X509Certificate getTrustedCA() {
+        return this.trustedCA;
+    }
+
     /**
      * Set token for automatic authentication, using {@link #buildAndAuth()}.
      *
@@ -252,6 +306,15 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Get the number of retries to attempt on 5xx errors.
+     *
+     * @return The number of retries to attempt on 5xx errors (default: 0)
+     */
+    public int getNumberOfRetries() {
+        return this.numberOfRetries;
+    }
+
     /**
      * Define a custom timeout for the HTTP connection.
      *
@@ -264,9 +327,18 @@ public final class HTTPVaultConnectorBuilder implements VaultConnectorBuilder {
         return this;
     }
 
+    /**
+     * Get custom timeout for the HTTP connection.
+     *
+     * @return Timeout value in milliseconds.
+     */
+    public Integer getTimeout() {
+        return this.timeout;
+    }
+
     @Override
     public HTTPVaultConnector build() {
-        return new HTTPVaultConnector(host, tls, tlsVersion, port, prefix, trustedCA, numberOfRetries, timeout);
+        return new HTTPVaultConnector(this);
     }
 
     @Override
diff --git a/src/test/java/de/stklcode/jvault/connector/HTTPVaultConnectorOfflineTest.java b/src/test/java/de/stklcode/jvault/connector/HTTPVaultConnectorOfflineTest.java
index ee949be..bdd8227 100644
--- a/src/test/java/de/stklcode/jvault/connector/HTTPVaultConnectorOfflineTest.java
+++ b/src/test/java/de/stklcode/jvault/connector/HTTPVaultConnectorOfflineTest.java
@@ -19,6 +19,7 @@ package de.stklcode.jvault.connector;
 import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import de.stklcode.jvault.connector.builder.VaultConnectorBuilder;
 import de.stklcode.jvault.connector.exception.*;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
@@ -157,11 +158,6 @@ class HTTPVaultConnectorOfflineTest {
         assertThat("Unexpected base URL with custom prefix", getRequestHelperPrivate(connector, "baseURL"), is(expectedCustomPrefix));
         assertThat("Trusted CA cert set, but not specified", getRequestHelperPrivate(connector, "trustedCaCert"), is(nullValue()));
 
-        // Provide custom SSL context.
-        connector = new HTTPVaultConnector(hostname, true, port, prefix, trustedCaCert);
-        assertThat("Unexpected base URL with custom prefix", getRequestHelperPrivate(connector, "baseURL"), is(expectedCustomPrefix));
-        assertThat("Trusted CA cert not filled correctly", getRequestHelperPrivate(connector, "trustedCaCert"), is(trustedCaCert));
-
         // Specify number of retries.
         connector = new HTTPVaultConnector(url, trustedCaCert, retries);
         assertThat("Number of retries not set correctly", getRequestHelperPrivate(connector, "retries"), is(retries));
diff --git a/src/test/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilderTest.java b/src/test/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilderTest.java
index 9832496..aa43ad0 100644
--- a/src/test/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilderTest.java
+++ b/src/test/java/de/stklcode/jvault/connector/builder/HTTPVaultConnectorBuilderTest.java
@@ -18,6 +18,7 @@ package de.stklcode.jvault.connector.builder;
 
 import com.github.stefanbirkner.systemlambda.SystemLambda;
 import de.stklcode.jvault.connector.HTTPVaultConnector;
+import de.stklcode.jvault.connector.exception.ConnectionException;
 import de.stklcode.jvault.connector.exception.TlsException;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -25,13 +26,9 @@ import org.junit.jupiter.api.io.TempDir;
 import java.io.File;
 import java.lang.reflect.Field;
 import java.nio.file.NoSuchFileException;
-import java.util.concurrent.Callable;
 
 import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable;
-import static org.hamcrest.CoreMatchers.*;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
-import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.*;
 
 /**
  * JUnit test for HTTP Vault connector factory
@@ -47,6 +44,35 @@ class HTTPVaultConnectorBuilderTest {
     @TempDir
     File tempDir;
 
+    /**
+     * Test the builder.
+     */
+    @Test
+    void builderTest() throws Exception {
+        /* Minimal configuration */
+        HTTPVaultConnector connector = VaultConnectorBuilder.http().withHost("vault.example.com").build();
+
+        assertEquals("https://vault.example.com:8200/v1/", getRequestHelperPrivate(connector, "baseURL"), "URL not set correctly");
+        assertNull(getRequestHelperPrivate(connector, "trustedCaCert"), "Trusted CA cert set when no cert provided");
+        assertEquals(0, getRequestHelperPrivate(connector, "retries"), "Number of retries unexpectedly set");
+
+        /* Specify all options */
+        HTTPVaultConnectorBuilder builder = VaultConnectorBuilder.http()
+                .withHost("vault2.example.com")
+                .withoutTLS()
+                .withPort(1234)
+                .withPrefix("/foo/")
+                .withTimeout(5678)
+                .withNumberOfRetries(9);
+        connector = builder.build();
+
+        assertEquals("http://vault2.example.com:1234/foo/", getRequestHelperPrivate(connector, "baseURL"), "URL not set correctly");
+        assertNull(getRequestHelperPrivate(connector, "trustedCaCert"), "Trusted CA cert set when no cert provided");
+        assertEquals(9, getRequestHelperPrivate(connector, "retries"), "Unexpected number of retries");
+        assertEquals(5678, getRequestHelperPrivate(connector, "timeout"), "Number timeout value");
+        assertThrows(ConnectionException.class, builder::buildAndAuth, "Immediate authentication should throw exception without token");
+    }
+
     /**
      * Test building from environment variables
      */
@@ -60,9 +86,9 @@ class HTTPVaultConnectorBuilderTest {
             );
             HTTPVaultConnector connector = builder.build();
 
-            assertThat("URL nor set correctly", getRequestHelperPrivate(connector, "baseURL"), is(equalTo(VAULT_ADDR + "/v1/")));
-            assertThat("Trusted CA cert set when no cert provided", getRequestHelperPrivate(connector, "trustedCaCert"), is(nullValue()));
-            assertThat("Non-default number of retries, when none set", getRequestHelperPrivate(connector, "retries"), is(0));
+            assertEquals(VAULT_ADDR + "/v1/", getRequestHelperPrivate(connector, "baseURL"), "URL not set correctly");
+            assertNull(getRequestHelperPrivate(connector, "trustedCaCert"), "Trusted CA cert set when no cert provided");
+            assertEquals(0, getRequestHelperPrivate(connector, "retries"), "Non-default number of retries, when none set");
 
             return null;
         });
@@ -75,9 +101,9 @@ class HTTPVaultConnectorBuilderTest {
             );
             HTTPVaultConnector connector = builder.build();
 
-            assertThat("URL nor set correctly", getRequestHelperPrivate(connector, "baseURL"), is(equalTo(VAULT_ADDR + "/v1/")));
-            assertThat("Trusted CA cert set when no cert provided", getRequestHelperPrivate(connector, "trustedCaCert"), is(nullValue()));
-            assertThat("Number of retries not set correctly", getRequestHelperPrivate(connector, "retries"), is(VAULT_MAX_RETRIES));
+            assertEquals(VAULT_ADDR + "/v1/", getRequestHelperPrivate(connector, "baseURL"), "URL not set correctly");
+            assertNull(getRequestHelperPrivate(connector, "trustedCaCert"), "Trusted CA cert set when no cert provided");
+            assertEquals(VAULT_MAX_RETRIES, getRequestHelperPrivate(connector, "retries"), "Number of retries not set correctly");
 
             return null;
         });
@@ -90,8 +116,8 @@ class HTTPVaultConnectorBuilderTest {
                     () -> VaultConnectorBuilder.http().fromEnv(),
                     "Creation with unknown cert path failed."
             );
-            assertThat(e.getCause(), is(instanceOf(NoSuchFileException.class)));
-            assertThat(((NoSuchFileException) e.getCause()).getFile(), is(VAULT_CACERT));
+            assertTrue(e.getCause() instanceof NoSuchFileException);
+            assertEquals(VAULT_CACERT, ((NoSuchFileException) e.getCause()).getFile());
 
             return null;
         });
@@ -102,7 +128,18 @@ class HTTPVaultConnectorBuilderTest {
                     () -> VaultConnectorBuilder.http().fromEnv(),
                     "Factory creation from minimal environment failed"
             );
-            assertThat("Token nor set correctly", getPrivate(builder, "token"), is(equalTo(VAULT_TOKEN)));
+            assertEquals(VAULT_TOKEN, getPrivate(builder, "token"), "Token not set correctly");
+
+            return null;
+        });
+
+        /* Invalid URL */
+        withVaultEnv("This is not a valid URL!", null, VAULT_MAX_RETRIES.toString(), VAULT_TOKEN).execute(() -> {
+            assertThrows(
+                    ConnectionException.class,
+                    () -> VaultConnectorBuilder.http().fromEnv(),
+                    "Invalid URL from environment should raise an exception"
+            );
 
             return null;
         });
@@ -121,7 +158,7 @@ class HTTPVaultConnectorBuilderTest {
 
     private Object getPrivate(Object target, String fieldName) throws NoSuchFieldException, IllegalAccessException {
         Field field = target.getClass().getDeclaredField(fieldName);
-        if (field.isAccessible()) {
+        if (field.canAccess(target)) {
             return field.get(target);
         }
         field.setAccessible(true);