You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by mc...@apache.org on 2018/11/27 00:13:41 UTC
[geode] branch develop updated: GEODE-6052: Use constructor DI to
avoid PowerMock in JMX tests (#2891)
This is an automated email from the ASF dual-hosted git repository.
mcmellawatt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 8aba8b9 GEODE-6052: Use constructor DI to avoid PowerMock in JMX tests (#2891)
8aba8b9 is described below
commit 8aba8b9f8f0301370cf55fcd8824b2a80e21f3d1
Author: Ryan McMahon <rm...@pivotal.io>
AuthorDate: Mon Nov 26 16:13:31 2018 -0800
GEODE-6052: Use constructor DI to avoid PowerMock in JMX tests (#2891)
---
.../internal/security/MBeanSecurityJUnitTest.java | 3 +-
.../geode/management/internal/MBeanJMXAdapter.java | 20 +++++----
.../internal/SystemManagementService.java | 19 ++++-----
.../management/internal/MBeanJMXAdapterTest.java | 47 ++++++----------------
4 files changed, 37 insertions(+), 52 deletions(-)
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
index e573a0e..c1130fb 100644
--- a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
@@ -35,6 +35,7 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.management.ManagementException;
import org.apache.geode.management.ManagementService;
import org.apache.geode.management.MemberMXBean;
@@ -106,7 +107,7 @@ public class MBeanSecurityJUnitTest {
() -> server.createMBean("FakeClassName", new ObjectName("GemFire", "name", "foo")))
.isInstanceOf(ReflectionException.class);
- MBeanJMXAdapter adapter = new MBeanJMXAdapter();
+ MBeanJMXAdapter adapter = new MBeanJMXAdapter(mock(DistributedMember.class));
assertThatThrownBy(() -> adapter.registerMBean(mock(DynamicMBean.class),
new ObjectName("MockDomain", "name", "mock"), false))
.isInstanceOf(ManagementException.class);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/MBeanJMXAdapter.java b/geode-core/src/main/java/org/apache/geode/management/internal/MBeanJMXAdapter.java
index 3a13c11..a76a85c 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/MBeanJMXAdapter.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/MBeanJMXAdapter.java
@@ -63,22 +63,21 @@ import org.apache.geode.management.RegionMXBean;
*
*/
public class MBeanJMXAdapter implements ManagementConstants {
-
/** The <code>MBeanServer</code> for this application */
public static MBeanServer mbeanServer = ManagementFactory.getPlatformMBeanServer();
- private Map<ObjectName, Object> localGemFireMBean;
+ private final Map<ObjectName, Object> localGemFireMBean;
- private DistributedMember distMember;
+ private final DistributedMember distMember;
- private Logger logger = LogService.getLogger();
+ private static final Logger logger = LogService.getLogger();
/**
* public constructor
*/
- public MBeanJMXAdapter() {
+ public MBeanJMXAdapter(DistributedMember distMember) {
this.localGemFireMBean = new ConcurrentHashMap<>();
- this.distMember = InternalDistributedSystem.getConnectedInstance().getDistributedMember();
+ this.distMember = distMember;
}
/**
@@ -165,7 +164,7 @@ public class MBeanJMXAdapter implements ManagementConstants {
} catch (InstanceAlreadyExistsException instanceAlreadyExistsException) {
// An InstanceAlreadyExistsException in this context means that the MBean
// has already been registered, so just log a warning message.
- logger.warn("MBean with ObjectName " + objectName + " has already been registered.");
+ logRegistrationWarning(objectName, true);
} catch (NullPointerException | NotCompliantMBeanException
| MBeanRegistrationException e) {
throw new ManagementException(e);
@@ -194,7 +193,7 @@ public class MBeanJMXAdapter implements ManagementConstants {
// has already been unregistered, so just log a debug message as it is
// essentially a no-op.
// has already been unregistered, so just log a warning message.
- logger.warn("MBean with ObjectName " + objectName + " has already been unregistered.");
+ logRegistrationWarning(objectName, false);
} catch (NullPointerException | MBeanRegistrationException e) {
throw new ManagementException(e);
}
@@ -561,4 +560,9 @@ public class MBeanJMXAdapter implements ManagementConstants {
return true;
}
+
+ void logRegistrationWarning(ObjectName objectName, boolean registering) {
+ logger.warn("MBean with ObjectName " + objectName + " has already been "
+ + (registering ? "registered." : "unregistered."));
+ }
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java b/geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java
index 2f069d4..0ffa38d 100755
--- a/geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/SystemManagementService.java
@@ -66,7 +66,7 @@ public class SystemManagementService extends BaseManagementService {
/**
* The concrete implementation of DistributedSystem that provides internal-only functionality.
*/
- private InternalDistributedSystem system;
+ private final InternalDistributedSystem system;
/**
* core component for distribution
@@ -77,7 +77,7 @@ public class SystemManagementService extends BaseManagementService {
* This is a notification hub to listen all the notifications emitted from all the MBeans in a
* peer cache./cache server
*/
- private NotificationHub notificationHub;
+ private final NotificationHub notificationHub;
/**
* whether the service is closed or not if cache is closed automatically this service will be
@@ -93,15 +93,15 @@ public class SystemManagementService extends BaseManagementService {
/**
* Adapter to interact with platform MBean server
*/
- private MBeanJMXAdapter jmxAdapter;
+ private final MBeanJMXAdapter jmxAdapter;
- private InternalCacheForClientAccess cache;
+ private final InternalCacheForClientAccess cache;
private FederatingManager federatingManager;
private final ManagementAgent agent;
- private ManagementResourceRepo repo;
+ private final ManagementResourceRepo repo;
/**
* This membership listener will listen on membership events after the node has transformed into a
@@ -113,9 +113,10 @@ public class SystemManagementService extends BaseManagementService {
* Proxy aggregator to create aggregate MBeans e.g. DistributedSystem and DistributedRegion
* GemFire comes with a default aggregator.
*/
- private List<ProxyListener> proxyListeners;
+ private final List<ProxyListener> proxyListeners;
- private UniversalListenerContainer universalListenerContainer = new UniversalListenerContainer();
+ private final UniversalListenerContainer universalListenerContainer =
+ new UniversalListenerContainer();
public static BaseManagementService newSystemManagementService(
InternalCacheForClientAccess cache) {
@@ -133,9 +134,9 @@ public class SystemManagementService extends BaseManagementService {
throw new DistributedSystemDisconnectedException(
"This connection to a distributed system has been disconnected.");
}
- this.jmxAdapter = new MBeanJMXAdapter();
- this.repo = new ManagementResourceRepo();
+ this.jmxAdapter = new MBeanJMXAdapter(this.system.getDistributedMember());
+ this.repo = new ManagementResourceRepo();
this.notificationHub = new NotificationHub(repo);
if (system.getConfig().getJmxManager()) {
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/MBeanJMXAdapterTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/MBeanJMXAdapterTest.java
index cc91c03..4f3a478 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/MBeanJMXAdapterTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/MBeanJMXAdapterTest.java
@@ -16,55 +16,34 @@
package org.apache.geode.management.internal;
import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
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.powermock.api.mockito.PowerMockito.mock;
-import static org.powermock.api.mockito.PowerMockito.mockStatic;
-import static org.powermock.api.mockito.PowerMockito.when;
+import static org.mockito.Mockito.when;
import javax.management.InstanceAlreadyExistsException;
import javax.management.InstanceNotFoundException;
import javax.management.MBeanServer;
import javax.management.ObjectName;
-import org.apache.logging.log4j.Logger;
import org.junit.Before;
import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.powermock.core.classloader.annotations.PowerMockIgnore;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.core.classloader.annotations.SuppressStaticInitializationFor;
-import org.powermock.modules.junit4.PowerMockRunner;
-
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.internal.logging.LogService;
-
-@RunWith(PowerMockRunner.class)
-@PrepareForTest({InternalDistributedSystem.class, LogService.class})
-@PowerMockIgnore({"javax.management.*", "javax.script.*"})
-@SuppressStaticInitializationFor({"org.apache.geode.internal.logging.LogService",
- "org.apache.geode.distributed.internal.InternalDistributedSystem"})
+
+import org.apache.geode.distributed.DistributedMember;
+
public class MBeanJMXAdapterTest {
private ObjectName objectName;
private MBeanServer mockMBeanServer;
- private Logger mockLogger;
+ private DistributedMember distMember;
@Before
public void setUp() throws Exception {
- mockStatic(InternalDistributedSystem.class);
- when(InternalDistributedSystem.getConnectedInstance())
- .thenReturn(mock(InternalDistributedSystem.class));
-
- mockStatic(LogService.class);
- mockLogger = mock(Logger.class);
- when(mockLogger.isDebugEnabled()).thenReturn(true);
- when(LogService.getLogger()).thenReturn(mockLogger);
-
mockMBeanServer = mock(MBeanServer.class);
objectName = new ObjectName("d:type=Foo,name=Bar");
+ distMember = mock(DistributedMember.class);
}
@Test
@@ -77,14 +56,14 @@ public class MBeanJMXAdapterTest {
// has already been unregistered
doThrow(new InstanceNotFoundException()).when(mockMBeanServer).unregisterMBean(objectName);
- MBeanJMXAdapter mBeanJMXAdapter = new MBeanJMXAdapter();
+ MBeanJMXAdapter mBeanJMXAdapter = spy(new MBeanJMXAdapter(distMember));
MBeanJMXAdapter.mbeanServer = mockMBeanServer;
mBeanJMXAdapter.unregisterMBean(objectName);
// InstanceNotFoundException should just log a debug message as it is essentially a no-op
// during unregistration
- verify(mockLogger, times(1)).warn(anyString());
+ verify(mBeanJMXAdapter, times(1)).logRegistrationWarning(any(ObjectName.class), eq(false));
}
@Test
@@ -98,13 +77,13 @@ public class MBeanJMXAdapterTest {
doThrow(new InstanceAlreadyExistsException()).when(mockMBeanServer)
.registerMBean(any(Object.class), eq(objectName));
- MBeanJMXAdapter mBeanJMXAdapter = new MBeanJMXAdapter();
+ MBeanJMXAdapter mBeanJMXAdapter = spy(new MBeanJMXAdapter(distMember));
MBeanJMXAdapter.mbeanServer = mockMBeanServer;
- mBeanJMXAdapter.registerMBeanProxy(mock(Object.class), objectName);
+ mBeanJMXAdapter.registerMBeanProxy(objectName, objectName);
// InstanceNotFoundException should just log a debug message as it is essentially a no-op
// during registration
- verify(mockLogger, times(1)).warn(anyString());
+ verify(mBeanJMXAdapter, times(1)).logRegistrationWarning(any(ObjectName.class), eq(true));
}
}