GEODE-6682: Create unit tests for MBeanProxyInvocationHandler (#4213)

Create MBeanProxyInvocationHandlerTest

Cleanup related management classes:
* MBeanProxyInvocationHandler
* MXBeanProxyInvocationHandler
* OpenTypeUtil
* OpenTypeConverter
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java b/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java
index e54ce8a..0a558a4 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyFactory.java
@@ -14,7 +14,6 @@
  */
 package org.apache.geode.management.internal;
 
-import java.beans.IntrospectionException;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -24,6 +23,7 @@
 
 import org.apache.logging.log4j.Logger;
 
+import org.apache.geode.annotations.VisibleForTesting;
 import org.apache.geode.cache.EntryNotFoundException;
 import org.apache.geode.cache.Region;
 import org.apache.geode.distributed.DistributedMember;
@@ -34,9 +34,11 @@
 /**
  * Instance of this class is responsible for proxy creation/deletion etc.
  *
+ * <p>
  * If a member is added/removed proxy factory is responsible for creating removing the corresponding
  * proxies for that member.
  *
+ * <p>
  * It also maintains a proxy repository {@link MBeanProxyInfoRepository} for quick access to the
  * proxy instances
  */
@@ -46,79 +48,59 @@
   /**
    * Proxy repository contains several indexes to search proxies in an efficient manner.
    */
-  private MBeanProxyInfoRepository proxyRepo;
+  private final MBeanProxyInfoRepository proxyRepo;
+  private final MBeanJMXAdapter jmxAdapter;
+  private final SystemManagementService service;
 
-
-  /**
-   * Interface between GemFire federation layer and Java JMX layer
-   */
-  private MBeanJMXAdapter jmxAdapter;
-
-  private SystemManagementService service;
-
-  /**
-   * @param jmxAdapter adapter to interface between JMX and GemFire
-   * @param service management service
-   */
   public MBeanProxyFactory(MBeanJMXAdapter jmxAdapter, SystemManagementService service) {
-
     this.jmxAdapter = jmxAdapter;
-    this.proxyRepo = new MBeanProxyInfoRepository();
     this.service = service;
+    proxyRepo = new MBeanProxyInfoRepository();
   }
 
   /**
    * Creates a single proxy and adds a {@link ProxyInfo} to proxy repository
    * {@link MBeanProxyInfoRepository}
-   *
-   * @param member {@link org.apache.geode.distributed.DistributedMember}
-   * @param objectName {@link javax.management.ObjectName} of the Bean
-   * @param monitoringRegion monitoring region containing the proxies
    */
-  public void createProxy(DistributedMember member, ObjectName objectName,
-      Region<String, Object> monitoringRegion, Object newVal) {
-
+  void createProxy(DistributedMember member, ObjectName objectName,
+      Region<String, Object> monitoringRegion, Object newValue) {
     try {
-      FederationComponent federationComponent = (FederationComponent) newVal;
-      String interfaceClassName = federationComponent.getMBeanInterfaceClass();
+      FederationComponent federation = (FederationComponent) newValue;
+      String interfaceClassName = federation.getMBeanInterfaceClass();
 
       Class interfaceClass = ClassLoadUtil.classFromName(interfaceClassName);
 
-      Object object = MBeanProxyInvocationHandler.newProxyInstance(member, monitoringRegion,
-          objectName, federationComponent, interfaceClass);
+      Object proxy = MBeanProxyInvocationHandler.newProxyInstance(member, monitoringRegion,
+          objectName, federation, interfaceClass);
 
-      jmxAdapter.registerMBeanProxy(object, objectName);
+      jmxAdapter.registerMBeanProxy(proxy, objectName);
 
       if (logger.isDebugEnabled()) {
         logger.debug("Registered ObjectName : {}", objectName);
       }
 
-      ProxyInfo proxyInfo = new ProxyInfo(interfaceClass, object, objectName);
+      ProxyInfo proxyInfo = new ProxyInfo(interfaceClass, proxy, objectName);
       proxyRepo.addProxyToRepository(member, proxyInfo);
 
-      service.afterCreateProxy(objectName, interfaceClass, object, (FederationComponent) newVal);
+      service.afterCreateProxy(objectName, interfaceClass, proxy, (FederationComponent) newValue);
 
       if (logger.isDebugEnabled()) {
         logger.debug("Proxy Created for : {}", objectName);
       }
 
-    } catch (ClassNotFoundException | IntrospectionException e) {
+    } catch (ClassNotFoundException e) {
       throw new ManagementException(e);
     }
-
   }
 
   /**
    * This method will create all the proxies for a given DistributedMember. It does not throw any
    * exception to its caller. It handles the error and logs error messages
    *
+   * <p>
    * It will be called from GII or when a member joins the system
-   *
-   * @param member {@link org.apache.geode.distributed.DistributedMember}
-   * @param monitoringRegion monitoring region containing the proxies
    */
-  public void createAllProxies(DistributedMember member, Region<String, Object> monitoringRegion) {
-
+  void createAllProxies(DistributedMember member, Region<String, Object> monitoringRegion) {
     if (logger.isDebugEnabled()) {
       logger.debug("Creating proxy for: {}", member.getId());
     }
@@ -126,30 +108,25 @@
     Set<Map.Entry<String, Object>> mbeans = monitoringRegion.entrySet();
 
     for (Map.Entry<String, Object> mbean : mbeans) {
-
-      ObjectName objectName = null;
       try {
-        objectName = ObjectName.getInstance(mbean.getKey());
+        ObjectName objectName = ObjectName.getInstance(mbean.getKey());
+
         if (logger.isDebugEnabled()) {
-          logger.debug("Creating proxy for ObjectName: " + objectName.toString());
+          logger.debug("Creating proxy for ObjectName {}", objectName);
         }
 
         createProxy(member, objectName, monitoringRegion, mbean.getValue());
       } catch (Exception e) {
-        logger.warn("Create Proxy failed for {} with exception {}", objectName, e.getMessage(), e);
+        logger.warn("Create Proxy failed for {} with exception {}", mbean.getKey(), e.getMessage(),
+            e);
       }
-
     }
   }
 
   /**
    * Removes all proxies for a given member
-   *
-   * @param member {@link org.apache.geode.distributed.DistributedMember}
-   * @param monitoringRegion monitoring region containing the proxies
    */
-  public void removeAllProxies(DistributedMember member, Region<String, Object> monitoringRegion) {
-
+  void removeAllProxies(DistributedMember member, Region<String, Object> monitoringRegion) {
     Set<Entry<String, Object>> entries = monitoringRegion.entrySet();
 
     if (logger.isDebugEnabled()) {
@@ -158,12 +135,13 @@
 
     for (Entry<String, Object> entry : entries) {
       String key = null;
-      Object val;
       try {
-        key = entry.getKey();// MBean Name in String format.
-        val = entry.getValue(); // Federation Component
+        // MBean Name in String format.
+        key = entry.getKey();
+        // Federation Component
+        Object federation = entry.getValue();
         ObjectName mbeanName = ObjectName.getInstance(key);
-        removeProxy(member, mbeanName, val);
+        removeProxy(member, mbeanName, federation);
       } catch (EntryNotFoundException entryNotFoundException) {
         // Entry has already been removed by another thread, so no need to remove it
         logProxyAlreadyRemoved(member, entry);
@@ -177,15 +155,11 @@
 
   /**
    * Removes the proxy
-   *
-   * @param member {@link org.apache.geode.distributed.DistributedMember}
-   * @param objectName {@link javax.management.ObjectName} of the Bean
    */
-  public void removeProxy(DistributedMember member, ObjectName objectName, Object oldVal) {
-
+  void removeProxy(DistributedMember member, ObjectName objectName, Object oldValue) {
     try {
       if (logger.isDebugEnabled()) {
-        logger.debug("Removing proxy for ObjectName: {}", objectName);
+        logger.debug("Removing proxy for ObjectName {}", objectName);
 
       }
 
@@ -193,12 +167,12 @@
       proxyRepo.removeProxy(member, objectName);
       if (proxyInfo != null) {
         service.afterRemoveProxy(objectName, proxyInfo.getProxyInterface(),
-            proxyInfo.getProxyInstance(), (FederationComponent) oldVal);
+            proxyInfo.getProxyInstance(), (FederationComponent) oldValue);
       }
       jmxAdapter.unregisterMBean(objectName);
 
       if (logger.isDebugEnabled()) {
-        logger.debug("Removed proxy for ObjectName: {}", objectName);
+        logger.debug("Removed proxy for ObjectName {}", objectName);
       }
 
     } catch (Exception e) {
@@ -208,8 +182,7 @@
     }
   }
 
-  public void updateProxy(ObjectName objectName, ProxyInfo proxyInfo, Object newObject,
-      Object oldObject) {
+  void updateProxy(ObjectName objectName, ProxyInfo proxyInfo, Object newObject, Object oldObject) {
     try {
       if (proxyInfo != null) {
         Class interfaceClass = proxyInfo.getProxyInterface();
@@ -222,55 +195,48 @@
   }
 
   /**
-   * Find a particular proxy instance for a {@link javax.management.ObjectName} ,
-   * {@link org.apache.geode.distributed.DistributedMember} and interface class If the proxy
-   * interface does not implement the given interface class a {@link java.lang.ClassCastException}.
-   * will be thrown
+   * Find a particular proxy instance for a {@link ObjectName}, {@link DistributedMember} and
+   * interface class If the proxy interface does not implement the given interface class a
+   * {@link ClassCastException} will be thrown.
    *
-   * @param objectName {@link javax.management.ObjectName} of the MBean
+   * @param objectName {@link ObjectName} of the MBean
    * @param interfaceClass interface class implemented by proxy
+   *
    * @return an instance of proxy exposing the given interface
    */
-  public <T> T findProxy(ObjectName objectName, Class<T> interfaceClass) {
-
+  <T> T findProxy(ObjectName objectName, Class<T> interfaceClass) {
     return proxyRepo.findProxyByName(objectName, interfaceClass);
-
-
   }
 
-  public ProxyInfo findProxyInfo(ObjectName objectName) {
+  ProxyInfo findProxyInfo(ObjectName objectName) {
     return proxyRepo.findProxyInfo(objectName);
   }
 
   /**
-   * Find a set of proxies given a {@link org.apache.geode.distributed.DistributedMember}
+   * Find a set of proxies given a {@link DistributedMember}.
    *
-   * @param member {@link org.apache.geode.distributed.DistributedMember}
-   * @return a set of {@link javax.management.ObjectName}
+   * @param member {@link DistributedMember}
+   *
+   * @return a set of {@link ObjectName}
    */
   public Set<ObjectName> findAllProxies(DistributedMember member) {
-
     return proxyRepo.findProxySet(member);
-
   }
 
   /**
    * This will return the last updated time of the proxyMBean
    *
-   * @param objectName {@link javax.management.ObjectName} of the MBean
+   * @param objectName {@link ObjectName} of the MBean
    * @return last updated time of the proxy
    */
-  public long getLastUpdateTime(ObjectName objectName) {
-
-    ProxyInterface proxyObj = findProxy(objectName, ProxyInterface.class);
-
-    return proxyObj.getLastRefreshedTime();
-
+  long getLastUpdateTime(ObjectName objectName) {
+    ProxyInterface proxyInterface = findProxy(objectName, ProxyInterface.class);
+    return proxyInterface.getLastRefreshedTime();
   }
 
+  @VisibleForTesting
   void logProxyAlreadyRemoved(DistributedMember member, Entry<String, Object> entry) {
     logger.warn("Proxy for entry {} and member {} has already been removed", entry,
         member.getId());
   }
-
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyInvocationHandler.java b/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyInvocationHandler.java
index 9c92fca..928a2f4 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyInvocationHandler.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/MBeanProxyInvocationHandler.java
@@ -14,7 +14,7 @@
  */
 package org.apache.geode.management.internal;
 
-import java.beans.IntrospectionException;
+import java.io.InvalidObjectException;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
@@ -22,6 +22,7 @@
 import java.util.List;
 
 import javax.management.JMX;
+import javax.management.ListenerNotFoundException;
 import javax.management.MBeanException;
 import javax.management.MBeanInfo;
 import javax.management.MBeanNotificationInfo;
@@ -32,79 +33,44 @@
 import javax.management.NotificationFilter;
 import javax.management.NotificationListener;
 import javax.management.ObjectName;
+import javax.management.openmbean.OpenDataException;
 
 import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.SystemFailure;
+import org.apache.geode.annotations.VisibleForTesting;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.execute.FunctionService;
 import org.apache.geode.cache.execute.ResultCollector;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.internal.logging.LogService;
 
-
-
 /**
  * This class is the proxy handler for all the proxies created for federated MBeans. Its designed
  * with Java proxy mechanism. All data calls are delegated to the federation components. All method
  * calls are routed to specified members via Function service
- *
- *
  */
-
 public class MBeanProxyInvocationHandler implements InvocationHandler {
-
   private static final Logger logger = LogService.getLogger();
 
-  /**
-   * Name of the MBean
-   */
-  private ObjectName objectName;
-
-  /**
-   * The monitoring region where this Object resides.
-   */
-  private Region<String, Object> monitoringRegion;
-
-  /**
-   * The member to which this proxy belongs
-   */
-
-  private DistributedMember member;
-
-
-  /**
-   * emitter is a helper class for sending notifications on behalf of the proxy
-   */
+  private final ObjectName objectName;
+  private final Region<String, Object> monitoringRegion;
+  private final DistributedMember member;
   private final NotificationBroadcasterSupport emitter;
-
   private final ProxyInterface proxyImpl;
+  private final boolean isMXBean;
 
-  private boolean isMXBean;
+  private MXBeanProxyInvocationHandler mxBeanProxyInvocationHandler;
 
-  private MXBeanProxyInvocationHandler mxbeanInvocationRef;
-
-
-
-  /**
-   *
-   * @param member member to which this MBean belongs
-   * @param monitoringRegion corresponding MonitoringRegion
-   * @param objectName ObjectName of the MBean
-   * @param interfaceClass on which interface the proxy to be exposed
-   */
-  public static Object newProxyInstance(DistributedMember member,
-      Region<String, Object> monitoringRegion, ObjectName objectName,
-      FederationComponent federationComponent, Class interfaceClass)
-      throws ClassNotFoundException, IntrospectionException {
+  static Object newProxyInstance(DistributedMember member, Region<String, Object> monitoringRegion,
+      ObjectName objectName, FederationComponent federationComponent, Class interfaceClass) {
     boolean isMXBean = JMX.isMXBeanInterface(interfaceClass);
     boolean notificationBroadcaster = federationComponent.isNotificationEmitter();
 
-    InvocationHandler handler =
+    InvocationHandler invocationHandler =
         new MBeanProxyInvocationHandler(member, objectName, monitoringRegion, isMXBean);
 
     Class[] interfaces;
-
     if (notificationBroadcaster) {
       interfaces =
           new Class[] {interfaceClass, ProxyInterface.class, NotificationBroadCasterProxy.class};
@@ -113,201 +79,170 @@
     }
 
     Object proxy = Proxy.newProxyInstance(MBeanProxyInvocationHandler.class.getClassLoader(),
-        interfaces, handler);
+        interfaces, invocationHandler);
 
     return interfaceClass.cast(proxy);
   }
 
-  /**
-   *
-   * @param member member to which this MBean belongs
-   * @param objectName ObjectName of the MBean
-   * @param monitoringRegion corresponding MonitoringRegion
-   */
   private MBeanProxyInvocationHandler(DistributedMember member, ObjectName objectName,
-      Region<String, Object> monitoringRegion, boolean isMXBean)
-      throws IntrospectionException, ClassNotFoundException {
+      Region<String, Object> monitoringRegion, boolean isMXBean) {
+    this(member, objectName, monitoringRegion, isMXBean, new NotificationBroadcasterSupport(),
+        new ProxyInterfaceImpl());
+  }
+
+  @VisibleForTesting
+  MBeanProxyInvocationHandler(DistributedMember member, ObjectName objectName,
+      Region<String, Object> monitoringRegion, boolean isMXBean,
+      NotificationBroadcasterSupport emitter, ProxyInterface proxyImpl) {
     this.member = member;
     this.objectName = objectName;
     this.monitoringRegion = monitoringRegion;
-    this.emitter = new NotificationBroadcasterSupport();
-    this.proxyImpl = new ProxyInterfaceImpl();
     this.isMXBean = isMXBean;
-
+    this.emitter = emitter;
+    this.proxyImpl = proxyImpl;
   }
 
   /**
    * Inherited method from Invocation handler All object state requests are delegated to the
    * federated component.
    *
+   * <p>
    * All setters and operations() are delegated to the function service.
    *
-   * Notification emmitter methods are also delegated to the function service
+   * <p>
+   * Notification emitter methods are also delegated to the function service
    */
   @Override
-  public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-
+  public Object invoke(Object proxy, Method method, Object[] arguments)
+      throws MBeanException, ListenerNotFoundException, InvalidObjectException, OpenDataException {
     if (logger.isTraceEnabled()) {
       logger.trace("Invoking Method {}", method.getName());
     }
-    final Class methodClass = method.getDeclaringClass();
 
-
-
+    Class methodClass = method.getDeclaringClass();
     if (methodClass.equals(NotificationBroadcaster.class)
-        || methodClass.equals(NotificationEmitter.class))
-      return invokeBroadcasterMethod(proxy, method, args);
+        || methodClass.equals(NotificationEmitter.class)) {
+      return invokeBroadcasterMethod(method, arguments);
+    }
 
-    final String methodName = method.getName();
-    final Class[] paramTypes = method.getParameterTypes();
-    final Class returnType = method.getReturnType();
+    String methodName = method.getName();
+    Class[] parameterTypes = method.getParameterTypes();
+    Class returnType = method.getReturnType();
 
-
-
-    final int nargs = (args == null) ? 0 : args.length;
+    int argumentCount = arguments == null ? 0 : arguments.length;
 
     if (methodName.equals("setLastRefreshedTime")) {
-      proxyImpl.setLastRefreshedTime((Long) args[0]);
+      proxyImpl.setLastRefreshedTime((Long) arguments[0]);
       return null;
     }
     if (methodName.equals("getLastRefreshedTime")) {
       return proxyImpl.getLastRefreshedTime();
     }
-
     if (methodName.equals("sendNotification")) {
-      sendNotification(args[0]);
+      sendNotification(arguments[0]);
       return null;
     }
 
     // local or not: equals, toString, hashCode
-    if (shouldDoLocally(proxy, method)) {
-      return doLocally(proxy, method, args);
+    if (shouldDoLocally(method)) {
+      return doLocally(method, arguments);
     }
 
     // Support For MXBean open types
     if (isMXBean) {
       MXBeanProxyInvocationHandler p = findMXBeanProxy(objectName, methodClass, this);
-      return p.invoke(proxy, method, args);
+      return p.invoke(proxy, method, arguments);
     }
 
-    if (methodName.startsWith("get") && methodName.length() > 3 && nargs == 0
+    if (methodName.startsWith("get") && methodName.length() > 3 && argumentCount == 0
         && !returnType.equals(Void.TYPE)) {
       return delegateToObjectState(methodName.substring(3));
     }
 
-    if (methodName.startsWith("is") && methodName.length() > 2 && nargs == 0
+    if (methodName.startsWith("is") && methodName.length() > 2 && argumentCount == 0
         && (returnType.equals(Boolean.TYPE) || returnType.equals(Boolean.class))) {
       return delegateToObjectState(methodName.substring(2));
     }
 
-    final String[] signature = new String[paramTypes.length];
-    for (int i = 0; i < paramTypes.length; i++)
-      signature[i] = paramTypes[i].getName();
-
-    if (methodName.startsWith("set") && methodName.length() > 3 && nargs == 1
-        && returnType.equals(Void.TYPE)) {
-      return delegateToFucntionService(objectName, methodName, args, signature);
-
+    String[] signature = new String[parameterTypes.length];
+    for (int i = 0; i < parameterTypes.length; i++) {
+      signature[i] = parameterTypes[i].getName();
     }
 
-    return delegateToFucntionService(objectName, methodName, args, signature);
-
-  }
-
-
-
-  /**
-   * As this proxy may behave as an notification emitter it delegates to the member
-   * NotificationBroadcasterSupport object
-   *
-   */
-  private void sendNotification(Object notification) {
-    emitter.sendNotification((Notification) notification);
+    return delegateToFunctionService(objectName, methodName, arguments, signature);
   }
 
   /**
    * This will get the data from Object state which is replicated across the hidden region
-   * FederataionComponent being the carrier.
-   *
+   * FederationComponent being the carrier.
    */
-  protected Object delegateToObjectState(String attributeName) throws Throwable {
-
-
-    Object returnObj;
+  Object delegateToObjectState(String attributeName) throws MBeanException {
     try {
-      FederationComponent fedComp =
+      FederationComponent federation =
           (FederationComponent) monitoringRegion.get(objectName.toString());
-      returnObj = fedComp.getValue(attributeName);
-    } catch (IllegalArgumentException e) {
-      throw new MBeanException(e);
+      return federation.getValue(attributeName);
     } catch (Exception e) {
       throw new MBeanException(e);
     } catch (VirtualMachineError e) {
       SystemFailure.initiateFailure(e);
       throw e;
-    } catch (Throwable th) {
+    } catch (Throwable t) {
       SystemFailure.checkFailure();
-      throw new MBeanException(new Exception(th.getLocalizedMessage()));
+      throw new MBeanException(new Exception(t.getLocalizedMessage()));
     }
-    return returnObj;
   }
 
   /**
    * It will call the Generic function to execute the method on the remote VM
-   *
-   * @param objectName ObjectName of the MBean
-   * @param methodName method name
-   * @param args arguments to the methods
-   * @param signature signature of the method
-   * @return result Object
    */
-  protected Object delegateToFucntionService(ObjectName objectName, String methodName,
-      Object[] args, String[] signature) throws Throwable {
+  Object delegateToFunctionService(ObjectName objectName, String methodName, Object[] arguments,
+      String[] signature) throws MBeanException {
+    Object[] functionArguments = new Object[5];
+    functionArguments[0] = objectName;
+    functionArguments[1] = methodName;
+    functionArguments[2] = signature;
+    functionArguments[3] = arguments;
+    functionArguments[4] = member.getName();
 
-    Object[] functionArgs = new Object[5];
-    functionArgs[0] = objectName;
-    functionArgs[1] = methodName;
-    functionArgs[2] = signature;
-    functionArgs[3] = args;
-    functionArgs[4] = member.getName();
-    List<Object> result = null;
+    List<Object> result;
     try {
-
-      ResultCollector rc = FunctionService.onMember(member).setArguments(functionArgs)
+      ResultCollector resultCollector = FunctionService.onMember(member)
+          .setArguments(functionArguments)
           .execute(ManagementConstants.MGMT_FUNCTION_ID);
-      result = (List<Object>) rc.getResult();
-      // Exceptions of ManagementFunctions
-
-
+      result = (List<Object>) resultCollector.getResult();
     } catch (Exception e) {
       if (logger.isDebugEnabled()) {
-        logger.debug(" Exception while Executing Funtion {}", e.getMessage(), e);
+        logger.debug(" Exception while Executing Function {}", e.getMessage(), e);
       }
       // Only in case of Exception caused for Function framework.
       return null;
     } catch (VirtualMachineError e) {
       SystemFailure.initiateFailure(e);
       throw e;
-    } catch (Throwable th) {
+    } catch (Throwable t) {
       SystemFailure.checkFailure();
       if (logger.isDebugEnabled()) {
-        logger.debug(" Exception while Executing Funtion {}", th.getMessage(), th);
+        logger.debug(" Error while Executing Function {}", t.getMessage(), t);
       }
       return null;
     }
 
     return checkErrors(result.get(ManagementConstants.RESULT_INDEX));
-
   }
 
-  private Object checkErrors(Object lastResult) throws Throwable {
+  /**
+   * As this proxy may behave as an notification emitter it delegates to the member
+   * NotificationBroadcasterSupport object
+   */
+  private void sendNotification(Object notification) {
+    emitter.sendNotification((Notification) notification);
+  }
 
-
+  private Object checkErrors(Object lastResult) throws MBeanException {
     if (lastResult instanceof MBeanException) {
       // Convert all MBean public API exceptions to MBeanException
-      throw (Exception) lastResult;
+      throw (MBeanException) lastResult;
     }
-
     if (lastResult instanceof Exception) {
       return null;
     }
@@ -321,186 +256,153 @@
    * The call will delegate to Managed Node for NotificationHub to register a local listener to
    * listen for notification from the MBean
    *
+   * <p>
    * Moreover it will also add the client to local listener list by adding to the contained emitter.
-   *
-   * @param proxy the proxy object
-   * @param method method to be invoked
-   * @param args method arguments
-   * @return result value if any
    */
-  private Object invokeBroadcasterMethod(Object proxy, Method method, Object[] args)
-      throws Throwable {
-    final String methodName = method.getName();
-    final int nargs = (args == null) ? 0 : args.length;
+  private Object invokeBroadcasterMethod(Method method, Object[] arguments)
+      throws MBeanException, ListenerNotFoundException {
+    String methodName = method.getName();
+    int argumentCount = arguments == null ? 0 : arguments.length;
 
-    final Class[] paramTypes = method.getParameterTypes();
-    final String[] signature = new String[paramTypes.length];
+    Class[] parameterTypes = method.getParameterTypes();
+    String[] signature = new String[parameterTypes.length];
 
-    if (methodName.equals("addNotificationListener")) {
+    switch (methodName) {
+      case "addNotificationListener": {
+        // The various throws of IllegalArgumentException here should not happen, since we know what
+        // the methods in NotificationBroadcaster and NotificationEmitter are.
 
-      /*
-       * The various throws of IllegalArgumentException here should not happen, since we know what
-       * the methods in NotificationBroadcaster and NotificationEmitter are.
-       */
+        if (argumentCount != 3) {
+          throw new IllegalArgumentException(
+              "Bad arg count to addNotificationListener: " + argumentCount);
+        }
 
-      if (nargs != 3) {
-        final String msg = "Bad arg count to addNotificationListener: " + nargs;
-        throw new IllegalArgumentException(msg);
-      }
-      /*
-       * Other inconsistencies will produce ClassCastException below.
-       */
+        // Other inconsistencies will produce ClassCastException below.
 
-      NotificationListener listener = (NotificationListener) args[0];
-      NotificationFilter filter = (NotificationFilter) args[1];
-      Object handback = args[2];
-      emitter.addNotificationListener(listener, filter, handback);
-      delegateToFucntionService(objectName, methodName, null, signature);
-      return null;
-
-    } else if (methodName.equals("removeNotificationListener")) {
-      /*
-       * NullPointerException if method with no args, but that shouldn't happen because removeNL
-       * does have args.
-       */
-      NotificationListener listener = (NotificationListener) args[0];
-
-      switch (nargs) {
-        case 1:
-          emitter.removeNotificationListener(listener);
-          /**
-           * No need to send listener and filter details to other members. We only need to send a
-           * message saying remove the listner registered for this object on your side. Fixes Bug[
-           * #47075 ]
-           */
-          delegateToFucntionService(objectName, methodName, null, signature);
-          return null;
-
-        case 3:
-          NotificationFilter filter = (NotificationFilter) args[1];
-          Object handback = args[2];
-          emitter.removeNotificationListener(listener, filter, handback);
-
-          delegateToFucntionService(objectName, methodName, null, signature);
-          return null;
-
-        default:
-          final String msg = "Bad arg count to removeNotificationListener: " + nargs;
-          throw new IllegalArgumentException(msg);
+        NotificationListener listener = (NotificationListener) arguments[0];
+        NotificationFilter filter = (NotificationFilter) arguments[1];
+        Object handback = arguments[2];
+        emitter.addNotificationListener(listener, filter, handback);
+        delegateToFunctionService(objectName, methodName, null, signature);
+        return null;
       }
 
-    } else if (methodName.equals("getNotificationInfo")) {
+      case "removeNotificationListener": {
+        // NullPointerException if method with no args, but that shouldn't happen because
+        // removeNotificationListener does have args.
+        NotificationListener listener = (NotificationListener) arguments[0];
 
-      if (args != null) {
-        throw new IllegalArgumentException("getNotificationInfo has " + "args");
+        switch (argumentCount) {
+          case 1:
+            emitter.removeNotificationListener(listener);
+
+            // No need to send listener and filter details to other members. We only need to send a
+            // message saying remove the listener registered for this object on your side.
+
+            delegateToFunctionService(objectName, methodName, null, signature);
+            return null;
+
+          case 3:
+            NotificationFilter filter = (NotificationFilter) arguments[1];
+            Object handback = arguments[2];
+            emitter.removeNotificationListener(listener, filter, handback);
+
+            delegateToFunctionService(objectName, methodName, null, signature);
+            return null;
+
+          default:
+            throw new IllegalArgumentException(
+                "Bad arg count to removeNotificationListener: " + argumentCount);
+        }
       }
 
-      if (!MBeanJMXAdapter.mbeanServer.isRegistered(objectName)) {
-        return new MBeanNotificationInfo[0];
-      }
+      case "getNotificationInfo":
+        if (arguments != null) {
+          throw new IllegalArgumentException("getNotificationInfo has " + "args");
+        }
 
-      /**
-       * MBean info is delegated to function service as intention is to get the info of the actual
-       * mbean rather than the proxy
-       */
+        if (!MBeanJMXAdapter.mbeanServer.isRegistered(objectName)) {
+          return new MBeanNotificationInfo[0];
+        }
 
+        // MBean info is delegated to function service as intention is to get the info of the actual
+        // mbean rather than the proxy
 
-      Object obj = delegateToFucntionService(objectName, methodName, args, signature);
-      if (obj instanceof String) {
-        return new MBeanNotificationInfo[0];
-      }
-      MBeanInfo info = (MBeanInfo) obj;
-      return info.getNotifications();
+        Object obj = delegateToFunctionService(objectName, methodName, arguments, signature);
+        if (obj instanceof String) {
+          return new MBeanNotificationInfo[0];
+        }
+        MBeanInfo info = (MBeanInfo) obj;
+        return info.getNotifications();
 
-    } else {
-      throw new IllegalArgumentException("Bad method name: " + methodName);
+      default:
+        throw new IllegalArgumentException("Bad method name: " + methodName);
     }
   }
 
+  private boolean shouldDoLocally(Method method) {
+    String methodName = method.getName();
+    if ((methodName.equals("hashCode") || methodName.equals("toString"))
+        && method.getParameterTypes().length == 0) {
+      return true;
+    }
+    return methodName.equals("equals")
+        && Arrays.equals(method.getParameterTypes(), new Class[] {Object.class});
+  }
 
+  private Object doLocally(Method method, Object[] arguments) {
+    String methodName = method.getName();
+    FederationComponent federation =
+        (FederationComponent) monitoringRegion.get(objectName.toString());
+
+    switch (methodName) {
+      case "equals":
+        return federation.equals(arguments[0]);
+      case "toString":
+        return federation.toString();
+      case "hashCode":
+        return federation.hashCode();
+    }
+
+    throw new IllegalArgumentException("Unexpected method name: " + methodName);
+  }
+
+  private MXBeanProxyInvocationHandler findMXBeanProxy(ObjectName objectName,
+      Class<?> mbeanInterface, MBeanProxyInvocationHandler handler) {
+    if (mxBeanProxyInvocationHandler == null) {
+      synchronized (this) {
+        try {
+          mxBeanProxyInvocationHandler =
+              new MXBeanProxyInvocationHandler(objectName, mbeanInterface, handler);
+        } catch (IllegalArgumentException e) {
+          String message =
+              "Cannot make MXBean proxy for " + mbeanInterface.getName() + ": " + e.getMessage();
+          throw new IllegalArgumentException(message, e.getCause());
+        }
+      }
+    }
+    return mxBeanProxyInvocationHandler;
+  }
 
   /**
    * Internal implementation of all the generic proxy methods
-   *
-   *
    */
-  private class ProxyInterfaceImpl implements ProxyInterface {
-    /**
-     * last refreshed time of the proxy
-     */
-    private long lastRefreshedTime;
+  private static class ProxyInterfaceImpl implements ProxyInterface {
 
-    /**
-     * Constructore
-     */
-    public ProxyInterfaceImpl() {
-      this.lastRefreshedTime = System.currentTimeMillis();
+    private volatile long lastRefreshedTime;
+
+    private ProxyInterfaceImpl() {
+      lastRefreshedTime = System.currentTimeMillis();
     }
 
-    /**
-     * Last refreshed time
-     */
     @Override
     public long getLastRefreshedTime() {
       return lastRefreshedTime;
     }
 
-    /**
-     * sets the proxy refresh time
-     */
     @Override
     public void setLastRefreshedTime(long lastRefreshedTime) {
       this.lastRefreshedTime = lastRefreshedTime;
     }
-
   }
-
-  private boolean shouldDoLocally(Object proxy, Method method) {
-    final String methodName = method.getName();
-    if ((methodName.equals("hashCode") || methodName.equals("toString"))
-        && method.getParameterTypes().length == 0)
-      return true;
-    if (methodName.equals("equals")
-        && Arrays.equals(method.getParameterTypes(), new Class[] {Object.class}))
-      return true;
-    return false;
-  }
-
-  private Object doLocally(Object proxy, Method method, Object[] args) {
-    final String methodName = method.getName();
-    FederationComponent fedComp = (FederationComponent) monitoringRegion.get(objectName.toString());
-    if (methodName.equals("equals")) {
-
-      return fedComp.equals(args[0]);
-
-    } else if (methodName.equals("toString")) {
-      return fedComp.toString();
-    } else if (methodName.equals("hashCode")) {
-      return fedComp.hashCode();
-    }
-
-    throw new RuntimeException("Unexpected method name: " + methodName);
-  }
-
-  private MXBeanProxyInvocationHandler findMXBeanProxy(ObjectName objectName,
-      Class<?> mxbeanInterface, MBeanProxyInvocationHandler handler) throws Throwable {
-    MXBeanProxyInvocationHandler proxyRef = mxbeanInvocationRef;
-
-    if (mxbeanInvocationRef == null) {
-      synchronized (this) {
-        try {
-          mxbeanInvocationRef =
-              new MXBeanProxyInvocationHandler(objectName, mxbeanInterface, handler);
-        } catch (IllegalArgumentException e) {
-          String msg =
-              "Cannot make MXBean proxy for " + mxbeanInterface.getName() + ": " + e.getMessage();
-          throw new IllegalArgumentException(msg, e.getCause());
-        }
-
-      }
-
-    }
-    return mxbeanInvocationRef;
-  }
-
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/MXBeanProxyInvocationHandler.java b/geode-core/src/main/java/org/apache/geode/management/internal/MXBeanProxyInvocationHandler.java
index 6a726cb..b78287b 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/MXBeanProxyInvocationHandler.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/MXBeanProxyInvocationHandler.java
@@ -14,6 +14,9 @@
  */
 package org.apache.geode.management.internal;
 
+import static java.util.Arrays.asList;
+
+import java.io.InvalidObjectException;
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Comparator;
@@ -21,69 +24,79 @@
 import java.util.Map;
 import java.util.Set;
 
+import javax.management.MBeanException;
 import javax.management.ObjectName;
+import javax.management.openmbean.OpenDataException;
 
-import org.apache.geode.LogWriter;
 import org.apache.geode.annotations.Immutable;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
 
 /**
  * This proxy handler handles all the method call invoked on an MXBean It follows same route as
  * MBeanProxyInvocationHandler Only difference is after obtaining the result it transforms the open
  * type to the actual java type
- *
- *
  */
 public class MXBeanProxyInvocationHandler {
 
-  private ObjectName objectName;
+  @Immutable
+  private static final Comparator<Method> METHOD_ORDER_COMPARATOR = new MethodOrder();
 
-  private MBeanProxyInvocationHandler proxyHandler;
+  private final ObjectName objectName;
+  private final MBeanProxyInvocationHandler proxyHandler;
+  private final Map<Method, MethodHandler> methodHandlerMap;
 
-  private final Map<Method, MethodHandler> methodHandlerMap = OpenTypeUtil.newMap();
-
-  private LogWriter logger;
-
-  public MXBeanProxyInvocationHandler(ObjectName objectName, Class<?> mxbeanInterface,
-      MBeanProxyInvocationHandler proxyHandler) throws Exception {
-
-    if (mxbeanInterface == null)
-      throw new IllegalArgumentException("Null parameter");
-
-    this.objectName = objectName;
-
-    this.proxyHandler = proxyHandler;
-
-    this.logger = InternalDistributedSystem.getLogger();
-    this.initHandlers(mxbeanInterface);
+  MXBeanProxyInvocationHandler(ObjectName objectName, Class<?> mxbeanInterface,
+      MBeanProxyInvocationHandler proxyHandler) throws IllegalArgumentException {
+    this(objectName, mxbeanInterface, proxyHandler, OpenTypeUtil.newMap());
   }
 
-  // Introspect the mbeanInterface and initialize this object's maps.
-  //
-  private void initHandlers(Class<?> mbeanInterface) throws Exception {
-    final Method[] methodArray = mbeanInterface.getMethods();
+  private MXBeanProxyInvocationHandler(ObjectName objectName, Class<?> mxbeanInterface,
+      MBeanProxyInvocationHandler proxyHandler, Map<Method, MethodHandler> methodHandlerMap)
+      throws IllegalArgumentException {
+    if (mxbeanInterface == null) {
+      throw new IllegalArgumentException("mxbeanInterface must not be null");
+    }
 
-    final List<Method> methods = eliminateCovariantMethods(methodArray);
+    this.objectName = objectName;
+    this.proxyHandler = proxyHandler;
+    this.methodHandlerMap = methodHandlerMap;
 
-    for (Method m : methods) {
-      String name = m.getName();
+    initHandlers(mxbeanInterface);
+  }
 
-      String attrName = "";
+  public Object invoke(Object proxy, Method method, Object[] arguments)
+      throws InvalidObjectException, OpenDataException, MBeanException {
+    MethodHandler handler = methodHandlerMap.get(method);
+    OpenMethod convertingMethod = handler.getConvertingMethod();
+
+    Object[] openArgs = convertingMethod.toOpenParameters(arguments);
+    Object result = handler.invoke(proxy, method, openArgs);
+    return convertingMethod.fromOpenReturnValue(result);
+  }
+
+  private void initHandlers(Class<?> mxbeanInterface) {
+    Method[] methodArray = mxbeanInterface.getMethods();
+    List<Method> methods = eliminateCovariantMethods(methodArray);
+
+    for (Method method : methods) {
+      String name = method.getName();
+
+      String attributeName = "";
       if (name.startsWith("get")) {
-        attrName = name.substring(3);
-      } else if (name.startsWith("is") && m.getReturnType() == boolean.class) {
-        attrName = name.substring(2);
+        attributeName = name.substring(3);
+      } else if (name.startsWith("is") && method.getReturnType() == boolean.class) {
+        attributeName = name.substring(2);
       }
 
-      if (attrName.length() != 0 && m.getParameterTypes().length == 0
-          && m.getReturnType() != void.class) { // For Getters
-
-        methodHandlerMap.put(m, new GetterHandler(attrName, OpenMethod.from(m)));
-      } else if (name.startsWith("set") && name.length() > 3 && m.getParameterTypes().length == 1
-          && m.getReturnType() == void.class) { // For Setteres
-        methodHandlerMap.put(m, new SetterHandler(attrName, OpenMethod.from(m)));
+      if (!attributeName.isEmpty() && method.getParameterTypes().length == 0
+          && method.getReturnType() != void.class) {
+        // For Getters
+        methodHandlerMap.put(method, new GetterHandler(attributeName, OpenMethod.from(method)));
+      } else if (name.startsWith("set") && name.length() > 3
+          && method.getParameterTypes().length == 1 && method.getReturnType() == void.class) {
+        // For Setters
+        methodHandlerMap.put(method, new SetterHandler(attributeName, OpenMethod.from(method)));
       } else {
-        methodHandlerMap.put(m, new OpHandler(attrName, OpenMethod.from(m)));
+        methodHandlerMap.put(method, new OpHandler(attributeName, OpenMethod.from(method)));
       }
     }
   }
@@ -92,32 +105,116 @@
    * Eliminate methods that are overridden with a covariant return type. Reflection will return both
    * the original and the overriding method but we need only the overriding one is of interest
    *
-   * @return the method after eliminating covariant menthods
+   * @return the method after eliminating covariant methods
    */
-  static List<Method> eliminateCovariantMethods(Method[] methodArray) {
+  private static List<Method> eliminateCovariantMethods(Method[] methodArray) {
+    int methodCount = methodArray.length;
+    Method[] sorted = methodArray.clone();
+    Arrays.sort(sorted, METHOD_ORDER_COMPARATOR);
+    Set<Method> overridden = OpenTypeUtil.newSet();
 
-    final int len = methodArray.length;
-    final Method[] sorted = methodArray.clone();
-    Arrays.sort(sorted, MethodOrder.instance);
-    final Set<Method> overridden = OpenTypeUtil.newSet();
-    for (int i = 1; i < len; i++) {
-      final Method m0 = sorted[i - 1];
-      final Method m1 = sorted[i];
+    for (int i = 1; i < methodCount; i++) {
+      Method m0 = sorted[i - 1];
+      Method m1 = sorted[i];
 
-      if (!m0.getName().equals(m1.getName()))
+      if (!m0.getName().equals(m1.getName())) {
         continue;
+      }
 
       if (Arrays.equals(m0.getParameterTypes(), m1.getParameterTypes())) {
         overridden.add(m0);
       }
     }
 
-    final List<Method> methods = OpenTypeUtil.newList(Arrays.asList(methodArray));
+    List<Method> methods = OpenTypeUtil.newList(asList(methodArray));
     methods.removeAll(overridden);
     return methods;
   }
 
   /**
+   * Handler for MXBean Proxy
+   */
+  private abstract static class MethodHandler {
+
+    private final String name;
+    private final OpenMethod convertingMethod;
+
+    MethodHandler(String name, OpenMethod convertingMethod) {
+      this.name = name;
+      this.convertingMethod = convertingMethod;
+    }
+
+    String getName() {
+      return name;
+    }
+
+    abstract Object invoke(Object proxy, Method method, Object[] arguments) throws MBeanException;
+
+    private OpenMethod getConvertingMethod() {
+      return convertingMethod;
+    }
+  }
+
+  private class GetterHandler extends MethodHandler {
+
+    private GetterHandler(String attributeName, OpenMethod convertingMethod) {
+      super(attributeName, convertingMethod);
+    }
+
+    @Override
+    Object invoke(Object proxy, Method method, Object[] arguments) throws MBeanException {
+      String methodName = method.getName();
+      String attributeName = "";
+      if (methodName.startsWith("get")) {
+        attributeName = methodName.substring(3);
+      } else if (methodName.startsWith("is") && method.getReturnType() == boolean.class) {
+        attributeName = methodName.substring(2);
+      }
+      return proxyHandler.delegateToObjectState(attributeName);
+    }
+  }
+
+  private class SetterHandler extends MethodHandler {
+
+    private SetterHandler(String attributeName, OpenMethod convertingMethod) {
+      super(attributeName, convertingMethod);
+    }
+
+    @Override
+    Object invoke(Object proxy, Method method, Object[] arguments) throws MBeanException {
+      String methodName = method.getName();
+      Class[] parameterTypes = method.getParameterTypes();
+      String[] signature = new String[parameterTypes.length];
+
+      for (int i = 0; i < parameterTypes.length; i++) {
+        signature[i] = parameterTypes[i].getName();
+      }
+
+      return proxyHandler.delegateToFunctionService(objectName, methodName, arguments, signature);
+    }
+  }
+
+  private class OpHandler extends MethodHandler {
+
+    private OpHandler(String operationName, OpenMethod convertingMethod) {
+      super(operationName, convertingMethod);
+    }
+
+    @Override
+    Object invoke(Object proxy, Method method, Object[] arguments) throws MBeanException {
+      String methodName = method.getName();
+      Class[] parameterTypes = method.getParameterTypes();
+      String[] signature = new String[parameterTypes.length];
+
+      for (int i = 0; i < parameterTypes.length; i++) {
+        signature[i] = parameterTypes[i].getName();
+      }
+
+      return proxyHandler.delegateToFunctionService(objectName, methodName, arguments, signature);
+    }
+  }
+
+  /**
    * A comparator that defines a total order so that methods have the same name and identical
    * signatures appear next to each others. The methods are sorted in such a way that methods which
    * override each other will sit next to each other, with the overridden method first - e.g. Object
@@ -126,121 +223,31 @@
    * (see eliminateCovariantMethods).
    **/
   private static class MethodOrder implements Comparator<Method> {
+
     @Override
     public int compare(Method a, Method b) {
-      final int cmp = a.getName().compareTo(b.getName());
-      if (cmp != 0)
+      int cmp = a.getName().compareTo(b.getName());
+      if (cmp != 0) {
         return cmp;
-      final Class<?>[] aparams = a.getParameterTypes();
-      final Class<?>[] bparams = b.getParameterTypes();
-      if (aparams.length != bparams.length)
+      }
+      Class<?>[] aparams = a.getParameterTypes();
+      Class<?>[] bparams = b.getParameterTypes();
+      if (aparams.length != bparams.length) {
         return aparams.length - bparams.length;
+      }
       if (!Arrays.equals(aparams, bparams)) {
         return Arrays.toString(aparams).compareTo(Arrays.toString(bparams));
       }
-      final Class<?> aret = a.getReturnType();
-      final Class<?> bret = b.getReturnType();
-      if (aret == bret)
+      Class<?> aret = a.getReturnType();
+      Class<?> bret = b.getReturnType();
+      if (aret == bret) {
         return 0;
+      }
 
-      if (aret.isAssignableFrom(bret))
+      if (aret.isAssignableFrom(bret)) {
         return -1;
+      }
       return +1;
     }
-
-    @Immutable
-    public static final MethodOrder instance = new MethodOrder();
   }
-
-  /**
-   * Hanlder for MXBean Proxy
-   *
-   *
-   */
-  private abstract class MethodHandler {
-    MethodHandler(String name, OpenMethod cm) {
-      this.name = name;
-      this.convertingMethod = cm;
-    }
-
-    String getName() {
-      return name;
-    }
-
-    OpenMethod getConvertingMethod() {
-      return convertingMethod;
-    }
-
-    abstract Object invoke(Object proxy, Method method, Object[] args) throws Throwable;
-
-    private final String name;
-    private final OpenMethod convertingMethod;
-  }
-
-  private class GetterHandler extends MethodHandler {
-    GetterHandler(String attributeName, OpenMethod cm) {
-      super(attributeName, cm);
-    }
-
-    @Override
-    Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-      assert (args == null || args.length == 0);
-      final String methodName = method.getName();
-      String attrName = "";
-      if (methodName.startsWith("get")) {
-        attrName = methodName.substring(3);
-      } else if (methodName.startsWith("is") && method.getReturnType() == boolean.class) {
-        attrName = methodName.substring(2);
-
-      }
-      return proxyHandler.delegateToObjectState(attrName);
-    }
-  }
-
-  private class SetterHandler extends MethodHandler {
-    SetterHandler(String attributeName, OpenMethod cm) {
-      super(attributeName, cm);
-    }
-
-    @Override
-    Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-      final String methodName = method.getName();
-      final Class[] paramTypes = method.getParameterTypes();
-      final String[] signature = new String[paramTypes.length];
-      for (int i = 0; i < paramTypes.length; i++)
-        signature[i] = paramTypes[i].getName();
-      return proxyHandler.delegateToFucntionService(objectName, methodName, args, signature);
-
-    }
-  }
-
-  private class OpHandler extends MethodHandler {
-
-    OpHandler(String operationName, OpenMethod cm) {
-      super(operationName, cm);
-
-    }
-
-    @Override
-    Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-      final String methodName = method.getName();
-      final Class[] paramTypes = method.getParameterTypes();
-      final String[] signature = new String[paramTypes.length];
-      for (int i = 0; i < paramTypes.length; i++)
-        signature[i] = paramTypes[i].getName();
-      return proxyHandler.delegateToFucntionService(objectName, methodName, args, signature);
-    }
-
-  }
-
-  public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
-
-    MethodHandler handler = methodHandlerMap.get(method);
-    OpenMethod cm = handler.getConvertingMethod();
-
-    Object[] openArgs = cm.toOpenParameters(args);
-    Object result = handler.invoke(proxy, method, openArgs);
-    return cm.fromOpenReturnValue(result);
-  }
-
 }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeConverter.java b/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeConverter.java
index 12826cc..feebdab 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeConverter.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeConverter.java
@@ -402,7 +402,7 @@
       if (propertyName == null)
         continue;
 
-      Method old = getterMap.put(OpenTypeUtil.decapitalize(propertyName), method);
+      Method old = getterMap.put(OpenTypeUtil.toCamelCase(propertyName), method);
       if (old != null) {
         final String msg = "Class " + c.getName() + " has method name clash: " + old.getName()
             + ", " + method.getName();
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeUtil.java b/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeUtil.java
index ba6ce3e..792edae 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeUtil.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/OpenTypeUtil.java
@@ -16,12 +16,9 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -29,59 +26,38 @@
 import java.util.TreeMap;
 
 /**
- * Various uitlity methods for open type conversion
- *
- *
+ * Various utility methods for open type conversion
  */
-
 public class OpenTypeUtil {
 
   static <K, V> Map<K, V> newMap() {
-    return new HashMap<K, V>();
-  }
-
-  static <K, V> Map<K, V> newSynchronizedMap() {
-    return Collections.synchronizedMap(OpenTypeUtil.<K, V>newMap());
+    return new HashMap<>();
   }
 
   static <K, V> IdentityHashMap<K, V> newIdentityHashMap() {
-    return new IdentityHashMap<K, V>();
-  }
-
-  static <K, V> Map<K, V> newSynchronizedIdentityHashMap() {
-    Map<K, V> map = newIdentityHashMap();
-    return Collections.synchronizedMap(map);
+    return new IdentityHashMap<>();
   }
 
   static <K, V> SortedMap<K, V> newSortedMap() {
-    return new TreeMap<K, V>();
-  }
-
-  static <K, V> SortedMap<K, V> newSortedMap(Comparator<? super K> comp) {
-    return new TreeMap<K, V>(comp);
-  }
-
-  static <K, V> Map<K, V> newInsertionOrderMap() {
-    return new LinkedHashMap<K, V>();
+    return new TreeMap<>();
   }
 
   static <E> Set<E> newSet() {
-    return new HashSet<E>();
+    return new HashSet<>();
   }
 
   static <E> Set<E> newSet(Collection<E> c) {
-    return new HashSet<E>(c);
+    return new HashSet<>(c);
   }
 
   static <E> List<E> newList() {
-    return new ArrayList<E>();
+    return new ArrayList<>();
   }
 
   static <E> List<E> newList(Collection<E> c) {
-    return new ArrayList<E>(c);
+    return new ArrayList<>(c);
   }
 
-  @SuppressWarnings("unchecked")
   public static <T> T cast(Object x) {
     return (T) x;
   }
@@ -90,22 +66,17 @@
    * Utility method to take a string and convert it to normal Java variable name capitalization.
    *
    * @param name The string to be made in camel case.
+   *
    * @return The camel case version of the string.
    */
-  public static String decapitalize(String name) {
-    if (name == null || name.length() == 0) {
+  static String toCamelCase(String name) {
+    if (name == null || name.isEmpty()) {
       return name;
     }
     int offset1 = Character.offsetByCodePoints(name, 0, 1);
-    if (offset1 < name.length() && Character.isUpperCase(name.codePointAt(offset1)))
+    if (offset1 < name.length() && Character.isUpperCase(name.codePointAt(offset1))) {
       return name;
+    }
     return name.substring(0, offset1).toLowerCase() + name.substring(offset1);
   }
-
-  protected static String capitalize(String name) {
-    if (name == null || name.length() == 0)
-      return name;
-    int offset1 = name.offsetByCodePoints(0, 1);
-    return name.substring(0, offset1).toUpperCase() + name.substring(offset1);
-  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyFactoryTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyFactoryTest.java
index 73fc15d..6821b6b 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyFactoryTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyFactoryTest.java
@@ -12,21 +12,20 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 package org.apache.geode.management.internal;
 
+import static java.util.Collections.singleton;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import org.junit.Before;
 import org.junit.Test;
 
 import org.apache.geode.cache.EntryNotFoundException;
@@ -34,24 +33,35 @@
 import org.apache.geode.distributed.DistributedMember;
 
 public class MBeanProxyFactoryTest {
+
+  private MBeanJMXAdapter jmxAdapter;
+  private Region<String, Object> region;
+  private Map.Entry<String, Object> regionEntry;
+  private SystemManagementService managementService;
+
+  @Before
+  public void setUp() {
+    jmxAdapter = mock(MBeanJMXAdapter.class);
+    managementService = mock(SystemManagementService.class);
+    region = mock(Region.class);
+    regionEntry = mock(Map.Entry.class);
+  }
+
   @Test
   public void removeAllProxiesEntryNotFoundLogged() {
-    MBeanProxyFactory mBeanProxyFactory =
-        spy(new MBeanProxyFactory(mock(MBeanJMXAdapter.class),
-            mock(SystemManagementService.class)));
-    Region mockRegion = mock(Region.class);
-    Set entrySet = new HashSet<Map.Entry<String, Object>>();
+    Set<Map.Entry<String, Object>> entrySet = new HashSet<>(singleton(regionEntry));
+    when(region.entrySet())
+        .thenReturn(entrySet);
+    when(regionEntry.getKey())
+        .thenThrow(new EntryNotFoundException("test"));
 
-    Map.Entry mockEntry = mock(Map.Entry.class);
-    doThrow(new EntryNotFoundException("Test EntryNotFoundException")).when(mockEntry).getKey();
+    MBeanProxyFactory mBeanProxyFactory = spy(new MBeanProxyFactory(jmxAdapter, managementService));
 
-    entrySet.add(mockEntry);
-
-    doReturn(entrySet).when(mockRegion).entrySet();
-    mBeanProxyFactory.removeAllProxies(mock(DistributedMember.class), mockRegion);
+    mBeanProxyFactory.removeAllProxies(mock(DistributedMember.class), region);
 
     // EntryNotFoundException should just result in a warning as it implies
     // the proxy has already been removed and the entry has already been destroyed
-    verify(mBeanProxyFactory, times(1)).logProxyAlreadyRemoved(any(), any());
+    verify(mBeanProxyFactory)
+        .logProxyAlreadyRemoved(any(), any());
   }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyInvocationHandlerTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyInvocationHandlerTest.java
new file mode 100644
index 0000000..ec90aba
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/MBeanProxyInvocationHandlerTest.java
@@ -0,0 +1,104 @@
+/*
+ * 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.geode.management.internal;
+
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.lang.reflect.Method;
+
+import javax.management.NotificationBroadcasterSupport;
+import javax.management.ObjectName;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.Region;
+import org.apache.geode.distributed.DistributedMember;
+
+public class MBeanProxyInvocationHandlerTest {
+
+  private DistributedMember member;
+  private ObjectName objectName;
+  private Region<String, Object> monitoringRegion;
+  private boolean isMXBean;
+  private NotificationBroadcasterSupport emitter;
+  private ProxyInterface proxyImpl;
+
+  @Before
+  public void setUp() {
+    member = mock(DistributedMember.class);
+    objectName = mock(ObjectName.class);
+    monitoringRegion = mock(Region.class);
+    isMXBean = false;
+    emitter = mock(NotificationBroadcasterSupport.class);
+    proxyImpl = mock(ProxyInterface.class);
+  }
+
+  @Test
+  public void invoke_methodName_getLastRefreshedTime_delegatesToProxyImpl() throws Exception {
+    MBeanProxyInvocationHandler mBeanProxyInvocationHandler =
+        new MBeanProxyInvocationHandler(member, objectName, monitoringRegion, isMXBean, emitter,
+            proxyImpl);
+    Method getLastRefreshedTimeMethod = proxyImpl.getClass()
+        .getMethod("getLastRefreshedTime");
+
+    mBeanProxyInvocationHandler.invoke(proxyImpl, getLastRefreshedTimeMethod, new Object[] {});
+
+    verify(proxyImpl)
+        .getLastRefreshedTime();
+  }
+
+  @Test
+  public void invoke_methodName_setLastRefreshedTime_delegatesToProxyImpl() throws Exception {
+    MBeanProxyInvocationHandler mBeanProxyInvocationHandler =
+        new MBeanProxyInvocationHandler(member, objectName, monitoringRegion, isMXBean, emitter,
+            proxyImpl);
+    Method setLastRefreshedTimeMethod = proxyImpl.getClass()
+        .getMethod("setLastRefreshedTime", long.class);
+    long lastRefreshedTimeParameter = 24;
+
+    mBeanProxyInvocationHandler.invoke(proxyImpl, setLastRefreshedTimeMethod,
+        new Object[] {lastRefreshedTimeParameter});
+
+    verify(proxyImpl)
+        .setLastRefreshedTime(eq(lastRefreshedTimeParameter));
+  }
+
+  @Test
+  public void invoke_methodName_sendNotification_delegatesToProxyImpl() throws Exception {
+    ProxyWithNotificationBroadcasterSupport proxyWithNotificationBroadcasterSupport =
+        mock(ProxyWithNotificationBroadcasterSupport.class);
+    MBeanProxyInvocationHandler mBeanProxyInvocationHandler =
+        new MBeanProxyInvocationHandler(member, objectName, monitoringRegion, isMXBean, emitter,
+            proxyWithNotificationBroadcasterSupport);
+    Method setLastRefreshedTimeMethod = proxyWithNotificationBroadcasterSupport.getClass()
+        .getMethod("setLastRefreshedTime", long.class);
+    long lastRefreshedTimeParameter = 24;
+
+    mBeanProxyInvocationHandler.invoke(proxyWithNotificationBroadcasterSupport,
+        setLastRefreshedTimeMethod, new Object[] {lastRefreshedTimeParameter});
+
+    verify(proxyWithNotificationBroadcasterSupport)
+        .setLastRefreshedTime(eq(lastRefreshedTimeParameter));
+  }
+
+  private abstract class ProxyWithNotificationBroadcasterSupport
+      extends NotificationBroadcasterSupport
+      implements ProxyInterface {
+    // nothing
+  }
+}