Address feedback

Signed-off-by: Federico Valeri <[email protected]>
strimzi · Jan 16, 2025 · 7868617 · 7868617
1 parent 415f9ee
commit 7868617
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 41 deletions.
diff --git a/...c-operator/src/main/java/io/strimzi/operator/topic/cruisecontrol/CruiseControlClient.java b/...c-operator/src/main/java/io/strimzi/operator/topic/cruisecontrol/CruiseControlClient.java
@@ -32,7 +32,7 @@ public interface CruiseControlClient {
      * @return Cruise Control client.
      */
     static CruiseControlClient create(Config config) {
-        return CruiseControlClientImpl.createInternal(config);
+        return new CruiseControlClientImpl(config);
     }
 
     /**
@@ -68,26 +68,103 @@ static CruiseControlClient create(Config config) {
 
     /**
      * Client configuration.
-     *
-     * @param serverHostname Server hostname.
-     * @param serverPort Server port.
-     * @param rackEnabled Whether rack awareness is enabled.
-     * @param sslEnabled Whether SSL is enabled.
-     * @param sslCertificate SSL certificate.
-     * @param authEnabled Whether authentication is enabled.
-     * @param authUsername Authentication username.
-     * @param authPassword Authentication password.
      */
-    record Config(
-        String serverHostname,
-        int serverPort,
-        boolean rackEnabled,
-        boolean sslEnabled,
-        byte[] sslCertificate,
-        boolean authEnabled,
-        String authUsername,
-        String authPassword
-    ) { }
+    class Config {
+        private final String serverHostname;
+        private final int serverPort;
+        private final boolean rackEnabled;
+        private final boolean sslEnabled;
+        private final byte[] sslCertificate;
+        private final boolean authEnabled;
+        private final String authUsername;
+        private final String authPassword;
+
+        /**
+         * Create new configuration.
+         *
+         * @param serverHostname Server hostname.
+         * @param serverPort Server port.
+         * @param rackEnabled Whether rack awareness is enabled.
+         * @param sslEnabled Whether SSL is enabled.
+         * @param sslCertificate SSL certificate.
+         * @param authEnabled Whether authentication is enabled.
+         * @param authUsername Authentication username.
+         * @param authPassword Authentication password.
+         */
+        public Config(String serverHostname,
+                      int serverPort,
+                      boolean rackEnabled,
+                      boolean sslEnabled,
+                      byte[] sslCertificate,
+                      boolean authEnabled,
+                      String authUsername,
+                      String authPassword) {
+            if (serverHostname == null || serverHostname.isBlank()) {
+                throw new IllegalArgumentException("Hostname is not set");
+            }
+            if (serverPort <= 0) {
+                throw new IllegalArgumentException("Port number is invalid");
+            }
+            if (sslEnabled && (sslCertificate == null || sslCertificate.length == 0)) {
+                throw new IllegalArgumentException("SSL certificate is not set");
+            }
+            if (authEnabled && (authUsername == null || authUsername.isBlank())) {
+                throw new IllegalArgumentException("Authentication username is not set");
+            }
+            if (authEnabled && (authPassword == null || authPassword.isBlank())) {
+                throw new IllegalArgumentException("Authentication password is not set");
+            }
+
+            this.serverHostname = serverHostname;
+            this.serverPort = serverPort;
+            this.rackEnabled = rackEnabled;
+            this.sslEnabled = sslEnabled;
+            this.sslCertificate = sslCertificate;
+            this.authEnabled = authEnabled;
+            this.authUsername = authUsername;
+            this.authPassword = authPassword;
+        }
+
+        /** @return Server hostname. */
+        public String serverHostname() {
+            return serverHostname;
+        }
+
+        /** @return Server port. */
+        public int serverPort() {
+            return serverPort;
+        }
+
+        /** @return Rack enabled. */
+        public boolean rackEnabled() {
+            return rackEnabled;
+        }
+
+        /** @return SSL enabled. */
+        public boolean sslEnabled() {
+            return sslEnabled;
+        }
+
+        /** @return SSL certificate. */
+        public byte[] sslCertificate() {
+            return sslCertificate;
+        }
+
+        /** @return Authentication enabled. */
+        public boolean authEnabled() {
+            return authEnabled;
+        }
+
+        /** @return Authentication username. */
+        public String authUsername() {
+            return authUsername;
+        }
+
+        /** @return Authentication password. */
+        public String authPassword() {
+            return authPassword;
+        }
+    }
 
     /**
      * Topic names grouped by replication factor value.

diff --git a/...erator/src/main/java/io/strimzi/operator/topic/cruisecontrol/CruiseControlClientImpl.java b/...erator/src/main/java/io/strimzi/operator/topic/cruisecontrol/CruiseControlClientImpl.java
@@ -54,32 +54,18 @@ public class CruiseControlClientImpl implements CruiseControlClient {
     private final ExecutorService httpClientExecutor;
     private HttpClient httpClient;
     private final ObjectMapper objectMapper;
-
-    private CruiseControlClientImpl(Config config) {
+
+    /**
+     * Create a new instance.
+     * 
+     * @param config Cruise Control configuration.
+     */
+    public CruiseControlClientImpl(Config config) {
         this.config = config;
         this.httpClientExecutor = Executors.newCachedThreadPool();
         this.httpClient = buildHttpClient();
         this.objectMapper = new ObjectMapper();
     }
-
-    static CruiseControlClient createInternal(Config config) {
-        if (config.serverHostname() == null || config.serverHostname().isBlank()) {
-            throw new IllegalArgumentException("Hostname is not set");
-        }
-        if (config.serverPort() <= 0) {
-            throw new IllegalArgumentException("Port number is invalid");
-        }
-        if (config.sslEnabled() && (config.sslCertificate() == null || config.sslCertificate().length == 0)) {
-            throw new IllegalArgumentException("SSL certificate is not set");
-        }
-        if (config.authEnabled() && (config.authUsername() == null || config.authUsername().isBlank())) {
-            throw new IllegalArgumentException("Authentication username is not set");
-        }
-        if (config.authEnabled() && (config.authPassword() == null || config.authPassword().isBlank())) {
-            throw new IllegalArgumentException("Authentication password is not set");
-        }
-        return new CruiseControlClientImpl(config);
-    }
 
     @Override
     public void close() {