Re: [PR] ZOOKEEPER-4835: Make use of Netty optional when no SSL is used [zookeeper]

["PDavid (via GitHub)" <gi...@apache.org>]

PDavid commented on code in PR #2374:
URL: https://github.com/apache/zookeeper/pull/2374#discussion_r3341447135


##########
zookeeper-server/src/test/java/org/apache/zookeeper/cli/HexDumpOutputFormatterTest.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.cli;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.nio.charset.StandardCharsets;
+import org.junit.jupiter.api.Test;
+
+public class HexDumpOutputFormatterTest {

Review Comment:
   Many thanks for adding this test. 👍 



##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxnFactory.java:
##########
@@ -228,6 +236,49 @@ public static ServerCnxnFactory createFactory() throws IOException {
         }
     }
 
+    /**
+     * Creates a ServerCnxnFactory, defaulting to NettyServerCnxnFactory when
+     * {@code secure} is {@code true} and no explicit factory is configured via
+     * the {@value #ZOOKEEPER_SERVER_CNXN_FACTORY} system property. SSL/TLS
+     * requires Netty; if Netty is not present on the classpath a helpful
+     * {@link IOException} is thrown.
+     *
+     * @param secure {@code true} when the factory will be used for secure (SSL/TLS) connections
+     * @return a new ServerCnxnFactory instance
+     * @throws IOException if the factory cannot be instantiated
+     */
+    public static ServerCnxnFactory createFactory(boolean secure) throws IOException {
+        String serverCnxnFactoryName = System.getProperty(ZOOKEEPER_SERVER_CNXN_FACTORY);
+        if (serverCnxnFactoryName == null) {
+            if (secure) {
+                serverCnxnFactoryName = "org.apache.zookeeper.server.NettyServerCnxnFactory";
+            } else {
+                serverCnxnFactoryName = NIOServerCnxnFactory.class.getName();
+            }
+        }
+        try {
+            ServerCnxnFactory serverCnxnFactory = (ServerCnxnFactory) Class.forName(serverCnxnFactoryName)
+                                                                           .getDeclaredConstructor()
+                                                                           .newInstance();
+            LOG.info("Using {} as server connection factory", serverCnxnFactoryName);
+            return serverCnxnFactory;
+        } catch (Exception e) {
+            String msg = "Couldn't instantiate " + serverCnxnFactoryName;
+            if (secure) {
+                msg += ". SSL/TLS support requires Netty; please add netty-handler"
+                    + " (and optionally netty-tcnative-boringssl-static) to your project's dependencies.";
+            }
+            throw new IOException(msg, e);
+        } catch (NoClassDefFoundError e) {
+            String msg = "Couldn't instantiate " + serverCnxnFactoryName;
+            if (secure) {
+                msg += ". SSL/TLS support requires Netty; please add netty-handler"
+                    + " (and optionally netty-tcnative-boringssl-static) to your project's dependencies.";
+            }
+            throw new IOException(msg, e);
+        }
+    }

Review Comment:
   Sorry, I missed this last time. Here we could also use multi-catch instead of duplicating the catch block code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org