yifan-c commented on code in PR #41: URL: https://github.com/apache/cassandra-sidecar/pull/41#discussion_r1018551938
########## common/src/main/java/org/apache/cassandra/sidecar/common/JmxClient.java: ########## @@ -0,0 +1,237 @@ +/* + * 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.cassandra.sidecar.common; + +import java.io.IOException; +import java.net.MalformedURLException; +import java.rmi.server.RMIClientSocketFactory; +import java.rmi.server.RMISocketFactory; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Function; +import javax.management.JMX; +import javax.management.MBeanServerConnection; +import javax.management.MalformedObjectNameException; +import javax.management.Notification; +import javax.management.NotificationListener; +import javax.management.ObjectName; +import javax.management.remote.JMXConnectionNotification; +import javax.management.remote.JMXConnector; +import javax.management.remote.JMXConnectorFactory; +import javax.management.remote.JMXServiceURL; +import javax.rmi.ssl.SslRMIClientSocketFactory; + +import org.jetbrains.annotations.VisibleForTesting; + +/** + * A simple wrapper around a JMX connection that makes it easier to get proxy instances. + */ +public class JmxClient implements NotificationListener +{ + public static final String JMX_SERVICE_URL_FMT = "service:jmx:rmi://%s/jndi/rmi://%s:%d/jmxrmi"; + public static final String REGISTRY_CONTEXT_SOCKET_FACTORY = "com.sun.jndi.rmi.factory.socket"; + private final JMXServiceURL jmxServiceURL; + private MBeanServerConnection mBeanServerConnection; + private final Map<String, Object> jmxEnv; + private boolean connected = false; + + /** + * Creates a new client with the provided {@code host} and {@code port}. + * + * @param host the host of the JMX service + * @param port the port of the JMX service + */ + public JmxClient(String host, int port) + { + this(host, port, null, null, false); + } + + /** + * Creates a new client with the provided parameters + * + * @param host the host of the JMX service + * @param port the port of the JMX service + * @param role the JMX role used for authentication + * @param password the JMX role password used for authentication + * @param enableSSl true if SSL is enabled for JMX, false otherwise + */ + public JmxClient(String host, int port, String role, String password, boolean enableSSl) + { + this(buildJmxServiceURL(host, port), role, password, enableSSl); + } + + @VisibleForTesting + JmxClient(JMXServiceURL jmxServiceURL) + { + this(jmxServiceURL, null, null, false); + } + + @VisibleForTesting + JmxClient(JMXServiceURL jmxServiceURL, String role, String password) + { + this(jmxServiceURL, role, password, false); + } + + private JmxClient(JMXServiceURL jmxServiceURL, String role, String password, boolean enableSsl) + { + this.jmxServiceURL = jmxServiceURL; + + jmxEnv = new HashMap<>(); + if (role != null && password != null) + { + String[] credentials = new String[]{ role, password }; + jmxEnv.put(JMXConnector.CREDENTIALS, credentials); + } + jmxEnv.put(REGISTRY_CONTEXT_SOCKET_FACTORY, getRMIClientSocketFactory(enableSsl)); + } + + /** + * Applies a function through the JMX bean proxy + * + * @param clientClass the management interface that the MBean exports, which will + * also be implemented by the returned proxy + * @param remoteName the name of the MBean within {@code connection} to forward to + * @param func the remote function that will be executed through the proxy + * @param <C> the type of the proxy client + * @param <R> the type of the result returned by the proxy + * @return the result of the remote function after being executed via the proxy + */ + public <C, R> R apply(Class<C> clientClass, String remoteName, Function<C, R> func) + { + checkConnection(); + C client = getProxy(clientClass, remoteName); + return func.apply(client); + } + + /** + * Calls a remote function through the JMX bean proxy + * + * @param clientClass the management interface that the MBean exports, which will + * also be implemented by the returned proxy + * @param remoteName the name of the MBean within {@code connection} to forward to + * @param caller the consumer to be called through the proxy + * @param <C> the type of the proxy client + */ + public <C> void call(Class<C> clientClass, String remoteName, Consumer<C> caller) Review Comment: It is a bit counter-intuitive that `call` has no return value. I am thinking of `Callable`. Similarly, `client.apply` reads not well. Semantically, `apply` reminds me of function evaluation. But `JmxClient` is not a function, instead it is a client to invoke/call a remote procedure. How about remove both method and make `getProxy` public? This one the chained method call also looks fluent, and it no longer take a lambda, which can grow complex and look messy. For example, ```java jmxClient.getProxy(StorageJmxOperations.class, STORAGE_SERVICE_OBJ_NAME) .takeSnapshot(tag, options, keyspace + "." + table); ``` Remember to call `checkConnection()` in `getProxy`, so it is identical to `apply` and `call`. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

