You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2016/12/12 22:38:58 UTC
[19/19] geode git commit: GEODE-1027: add unit test for previously
committed fix
GEODE-1027: add unit test for previously committed fix
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/1f49cbcc
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/1f49cbcc
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/1f49cbcc
Branch: refs/heads/feature/GEODE-1027
Commit: 1f49cbcc8cbaad5c0c2ccb8063a67bd6bb88a9b9
Parents: 8eb09bd
Author: Kirk Lund <kl...@apache.org>
Authored: Wed Dec 7 15:33:51 2016 -0800
Committer: Kirk Lund <kl...@apache.org>
Committed: Mon Dec 12 14:38:27 2016 -0800
----------------------------------------------------------------------
.../internal/statistics/SampleCollector.java | 4 +-
.../internal/statistics/StatisticsMonitor.java | 28 ++---
.../geode/internal/statistics/ValueMonitor.java | 20 ++--
.../internal/beans/stats/MBeanStatsMonitor.java | 54 +++++----
.../internal/statistics/FakeValueMonitor.java | 37 ++++++
.../beans/stats/MBeanStatsMonitorTest.java | 117 +++++++++++++++++++
6 files changed, 212 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/1f49cbcc/geode-core/src/main/java/org/apache/geode/internal/statistics/SampleCollector.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/SampleCollector.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/SampleCollector.java
index 9f1b343..2abbecd 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/SampleCollector.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/SampleCollector.java
@@ -115,7 +115,7 @@ public class SampleCollector {
*
* @throws IllegalStateException if no SampleCollector has been created and initialized yet
*/
- public static StatMonitorHandler getStatMonitorHandler() {
+ static StatMonitorHandler getStatMonitorHandler() {
// sync SampleCollector.class and then instance.sampleHandlers
synchronized (SampleCollector.class) {
if (instance == null) {
@@ -321,7 +321,7 @@ public class SampleCollector {
}
/** For testing only */
- public StatArchiveHandler getStatArchiveHandler() {
+ StatArchiveHandler getStatArchiveHandler() {
synchronized (this.sampleHandlers) {
return this.statArchiveHandler;
}
http://git-wip-us.apache.org/repos/asf/geode/blob/1f49cbcc/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
index 16f71e2..a00a7a3 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java
@@ -29,14 +29,13 @@ public abstract class StatisticsMonitor {
private final Object mutex = new Object();
- private final ConcurrentHashSet<StatisticsListener> listeners =
- new ConcurrentHashSet<StatisticsListener>();
+ private final ConcurrentHashSet<StatisticsListener> listeners = new ConcurrentHashSet<>();
- private final ConcurrentHashSet<StatisticId> statisticIds = new ConcurrentHashSet<StatisticId>();
+ private final ConcurrentHashSet<StatisticId> statisticIds = new ConcurrentHashSet<>();
public StatisticsMonitor() {}
- public StatisticsMonitor addStatistic(StatisticId statId) {
+ public StatisticsMonitor addStatistic(final StatisticId statId) {
if (statId == null) {
throw new NullPointerException("StatisticId is null");
}
@@ -46,7 +45,7 @@ public abstract class StatisticsMonitor {
return this;
}
- public StatisticsMonitor removeStatistic(StatisticId statId) {
+ public StatisticsMonitor removeStatistic(final StatisticId statId) {
if (statId == null) {
throw new NullPointerException("StatisticId is null");
}
@@ -56,7 +55,7 @@ public abstract class StatisticsMonitor {
return this;
}
- public final void addListener(StatisticsListener listener) {
+ public void addListener(final StatisticsListener listener) {
if (listener == null) {
throw new NullPointerException("StatisticsListener is null");
}
@@ -68,7 +67,7 @@ public abstract class StatisticsMonitor {
}
}
- public final void removeListener(StatisticsListener listener) {
+ public void removeListener(final StatisticsListener listener) {
if (listener == null) {
throw new NullPointerException("StatisticsListener is null");
}
@@ -93,24 +92,25 @@ public abstract class StatisticsMonitor {
* @param millisTimeStamp the real time in millis of the sample
* @param resourceInstances resources with one or more updated values
*/
- protected void monitor(long millisTimeStamp, List<ResourceInstance> resourceInstances) {
+ protected void monitor(final long millisTimeStamp,
+ final List<ResourceInstance> resourceInstances) {
monitorStatisticIds(millisTimeStamp, resourceInstances);
}
- private final void monitorStatisticIds(long millisTimeStamp,
- List<ResourceInstance> resourceInstances) {
+ private void monitorStatisticIds(final long millisTimeStamp,
+ final List<ResourceInstance> resourceInstances) {
if (!this.statisticIds.isEmpty()) {
// TODO:
}
}
- protected final void notifyListeners(StatisticsNotification notification) {
+ protected void notifyListeners(final StatisticsNotification notification) {
for (StatisticsListener listener : this.listeners) {
listener.handleNotification(notification);
}
}
- protected final Object mutex() {
+ protected Object mutex() {
return this.mutex;
}
@@ -137,7 +137,9 @@ public abstract class StatisticsMonitor {
return sb.toString();
}
- /** Override to append to toString() */
+ /**
+ * Override to append to toString()
+ */
protected StringBuilder appendToString() {
return null;
}
http://git-wip-us.apache.org/repos/asf/geode/blob/1f49cbcc/geode-core/src/main/java/org/apache/geode/internal/statistics/ValueMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/ValueMonitor.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/ValueMonitor.java
index 1dc6fd0..e46723c 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/statistics/ValueMonitor.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/ValueMonitor.java
@@ -43,31 +43,31 @@ import org.apache.geode.internal.CopyOnWriteHashSet;
* @since GemFire 7.0
* @see org.apache.geode.Statistics
*/
-public final class ValueMonitor extends StatisticsMonitor {
+public class ValueMonitor extends StatisticsMonitor {
public enum Type {
CHANGE, MATCH, DIFFER
}
- private final CopyOnWriteHashSet<Statistics> statistics = new CopyOnWriteHashSet<Statistics>();
+ private final CopyOnWriteHashSet<Statistics> statistics = new CopyOnWriteHashSet<>();
public ValueMonitor() {
super();
}
@Override
- public ValueMonitor addStatistic(StatisticId statId) {
+ public ValueMonitor addStatistic(final StatisticId statId) {
super.addStatistic(statId);
return this;
}
@Override
- public ValueMonitor removeStatistic(StatisticId statId) {
+ public ValueMonitor removeStatistic(final StatisticId statId) {
super.removeStatistic(statId);
return this;
}
- public ValueMonitor addStatistics(Statistics statistics) {
+ public ValueMonitor addStatistics(final Statistics statistics) {
if (statistics == null) {
throw new NullPointerException("Statistics is null");
}
@@ -75,7 +75,7 @@ public final class ValueMonitor extends StatisticsMonitor {
return this;
}
- public ValueMonitor removeStatistics(Statistics statistics) {
+ public ValueMonitor removeStatistics(final Statistics statistics) {
if (statistics == null) {
throw new NullPointerException("Statistics is null");
}
@@ -83,14 +83,16 @@ public final class ValueMonitor extends StatisticsMonitor {
return this;
}
- protected void monitor(long millisTimeStamp, List<ResourceInstance> resourceInstances) {
+ protected void monitor(final long millisTimeStamp,
+ final List<ResourceInstance> resourceInstances) {
super.monitor(millisTimeStamp, resourceInstances);
monitorStatistics(millisTimeStamp, resourceInstances);
}
- protected void monitorStatistics(long millisTimeStamp, List<ResourceInstance> resourceInstances) {
+ protected void monitorStatistics(final long millisTimeStamp,
+ final List<ResourceInstance> resourceInstances) {
if (!this.statistics.isEmpty()) {
- Map<StatisticId, Number> stats = new HashMap<StatisticId, Number>();
+ Map<StatisticId, Number> stats = new HashMap<>();
for (ResourceInstance resource : resourceInstances) {
if (this.statistics.contains(resource.getStatistics())) {
ResourceType resourceType = resource.getResourceType();
http://git-wip-us.apache.org/repos/asf/geode/blob/1f49cbcc/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitor.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitor.java
index 390fb89..c1744f9 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitor.java
@@ -17,11 +17,12 @@ package org.apache.geode.management.internal.beans.stats;
import java.util.HashMap;
import java.util.Map;
+import org.apache.logging.log4j.Logger;
+
import org.apache.geode.StatisticDescriptor;
import org.apache.geode.Statistics;
import org.apache.geode.StatisticsType;
-import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.i18n.LogWriterI18n;
+import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.statistics.StatisticId;
import org.apache.geode.internal.statistics.StatisticNotFoundException;
import org.apache.geode.internal.statistics.StatisticsListener;
@@ -30,11 +31,11 @@ import org.apache.geode.internal.statistics.ValueMonitor;
/**
* Class to get mappings of stats name to their values
- *
- *
*/
public class MBeanStatsMonitor implements StatisticsListener {
+ private static final Logger logger = LogService.getLogger();
+
protected ValueMonitor monitor;
/**
@@ -44,17 +45,17 @@ public class MBeanStatsMonitor implements StatisticsListener {
protected String monitorName;
- private LogWriterI18n logger;
+ public MBeanStatsMonitor(final String name) {
+ this(name, new ValueMonitor());
+ }
- public MBeanStatsMonitor(String name) {
+ MBeanStatsMonitor(final String name, final ValueMonitor monitor) {
this.monitorName = name;
- this.monitor = new ValueMonitor();
+ this.monitor = monitor;
this.statsMap = new DefaultHashMap();
- this.logger = InternalDistributedSystem.getLoggerI18n();
-
}
- public void addStatisticsToMonitor(Statistics stats) {
+ public void addStatisticsToMonitor(final Statistics stats) {
monitor.addListener(this);// if already listener is added this will be a no-op
// Initialize the stats with the current values.
StatisticsType type = stats.getType();
@@ -65,7 +66,7 @@ public class MBeanStatsMonitor implements StatisticsListener {
monitor.addStatistics(stats);
}
- public void removeStatisticsFromMonitor(Statistics stats) {
+ public void removeStatisticsFromMonitor(final Statistics stats) {
statsMap.clear();
}
@@ -73,13 +74,13 @@ public class MBeanStatsMonitor implements StatisticsListener {
monitor.removeListener(this);
}
- public Number getStatistic(String statName) {
- return statsMap.get(statName) != null ? statsMap.get(statName) : 0;
+ public Number getStatistic(final String statName) {
+ Number value = statsMap.get(statName);
+ return value != null ? value : 0;
}
@Override
- public void handleNotification(StatisticsNotification notification) {
-
+ public void handleNotification(final StatisticsNotification notification) {
for (StatisticId statId : notification) {
StatisticDescriptor descriptor = statId.getStatisticDescriptor();
String name = descriptor.getName();
@@ -91,33 +92,38 @@ public class MBeanStatsMonitor implements StatisticsListener {
}
log(name, value);
statsMap.put(name, value);
-
}
}
- protected void log(String name, Number value) {
-
- if (logger != null && logger.finestEnabled()) {
- logger.finest("Monitor = " + monitorName + " descriptor = " + name + " And Value = " + value);
+ protected void log(final String name, final Number value) {
+ if (logger.isTraceEnabled()) {
+ logger.trace("Monitor = {} descriptor = {} And value = {}", monitorName, name, value);
}
}
- public static class DefaultHashMap {
- private Map<String, Number> internalMap = new HashMap<String, Number>();
+ public static class DefaultHashMap { // TODO: delete this class
+ private Map<String, Number> internalMap = new HashMap<>();
public DefaultHashMap() {}
- public Number get(String key) {
+ public Number get(final String key) {
return internalMap.get(key) != null ? internalMap.get(key) : 0;
}
- public void put(String key, Number value) {
+ public void put(final String key, final Number value) {
internalMap.put(key, value);
}
public void clear() {
internalMap.clear();
}
+
+ /**
+ * For testing only
+ */
+ Map<String, Number> getInternalMap() {
+ return this.internalMap;
+ }
}
}
http://git-wip-us.apache.org/repos/asf/geode/blob/1f49cbcc/geode-core/src/test/java/org/apache/geode/internal/statistics/FakeValueMonitor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/statistics/FakeValueMonitor.java b/geode-core/src/test/java/org/apache/geode/internal/statistics/FakeValueMonitor.java
new file mode 100644
index 0000000..0b3a27d
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/statistics/FakeValueMonitor.java
@@ -0,0 +1,37 @@
+/*
+ * 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.internal.statistics;
+
+import static org.mockito.Mockito.mock;
+
+/**
+ * Fake ValueMonitor which uses mocked StatMonitorHandler as a collaborator.
+ */
+public class FakeValueMonitor extends ValueMonitor {
+
+ private StatMonitorHandler statMonitorHandler;
+
+ public FakeValueMonitor() {
+ this(mock(StatMonitorHandler.class));
+ }
+
+ public FakeValueMonitor(StatMonitorHandler statMonitorHandler) {
+ this.statMonitorHandler = statMonitorHandler;
+ }
+
+ StatMonitorHandler getStatMonitorHandler() {
+ return this.statMonitorHandler;
+ }
+}
http://git-wip-us.apache.org/repos/asf/geode/blob/1f49cbcc/geode-core/src/test/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitorTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitorTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitorTest.java
new file mode 100644
index 0000000..18d7f81
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/beans/stats/MBeanStatsMonitorTest.java
@@ -0,0 +1,117 @@
+/*
+ * 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.beans.stats;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
+
+import org.apache.geode.StatisticDescriptor;
+import org.apache.geode.Statistics;
+import org.apache.geode.StatisticsType;
+import org.apache.geode.i18n.LogWriterI18n;
+import org.apache.geode.internal.statistics.FakeValueMonitor;
+import org.apache.geode.internal.statistics.ValueMonitor;
+import org.apache.geode.management.internal.beans.stats.MBeanStatsMonitor.DefaultHashMap;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class MBeanStatsMonitorTest {
+
+ private ValueMonitor statsMonitor;
+
+ private StatisticDescriptor[] descriptors;
+
+ private Map<String, Number> expectedStatsMap;
+
+ @Spy
+ private DefaultHashMap statsMap;
+ @Mock
+ private LogWriterI18n logWriter;
+ @Mock
+ private Statistics stats;
+ @Mock
+ private StatisticsType statsType;
+
+ @InjectMocks
+ private MBeanStatsMonitor mbeanStatsMonitor;
+
+ @Rule
+ public TestName testName = new TestName();
+
+ @Before
+ public void setUp() throws Exception {
+ this.statsMonitor = spy(new FakeValueMonitor());
+ this.mbeanStatsMonitor =
+ new MBeanStatsMonitor(this.testName.getMethodName(), this.statsMonitor);
+ MockitoAnnotations.initMocks(this);
+
+ this.expectedStatsMap = new HashMap<>();
+ this.descriptors = new StatisticDescriptor[3];
+ for (int i = 0; i < this.descriptors.length; i++) {
+ String key = "stat-" + String.valueOf(i + 1);
+ Number value = i + 1;
+
+ this.expectedStatsMap.put(key, value);
+
+ this.descriptors[i] = mock(StatisticDescriptor.class);
+ when(this.descriptors[i].getName()).thenReturn(key);
+ when(this.stats.get(this.descriptors[i])).thenReturn(value);
+ }
+
+ when(this.statsType.getStatistics()).thenReturn(this.descriptors);
+ when(this.stats.getType()).thenReturn(this.statsType);
+ }
+
+ @Test
+ public void addStatisticsToMonitorShouldAddToInternalMap() throws Exception {
+ this.mbeanStatsMonitor.addStatisticsToMonitor(this.stats);
+
+ assertThat(statsMap.getInternalMap()).containsAllEntriesOf(this.expectedStatsMap);
+ }
+
+ @Test
+ public void addStatisticsToMonitorShouldAddListener() throws Exception {
+ this.mbeanStatsMonitor.addStatisticsToMonitor(this.stats);
+
+ verify(this.statsMonitor, times(1)).addListener(this.mbeanStatsMonitor);
+ }
+
+ @Test
+ public void addStatisticsToMonitorShouldAddStatistics() throws Exception {
+ this.mbeanStatsMonitor.addStatisticsToMonitor(this.stats);
+
+ verify(this.statsMonitor, times(1)).addStatistics(this.stats);
+ }
+
+ @Test
+ public void addNullStatisticsToMonitorShouldThrowNPE() throws Exception {
+ assertThatThrownBy(() -> this.mbeanStatsMonitor.addStatisticsToMonitor(null))
+ .isExactlyInstanceOf(NullPointerException.class);
+ }
+
+}