Revert "GEODE-7326: Add cache gets timers (#4201)"
This reverts commit 55980e37f0a57256fdd0af6f10e1d70ce1f90676.
diff --git a/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/CacheGetsTimerTest.java b/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/CacheGetsTimerTest.java
deleted file mode 100644
index 620c2b7..0000000
--- a/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/CacheGetsTimerTest.java
+++ /dev/null
@@ -1,527 +0,0 @@
-/*
- * 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.metrics;
-
-import static java.io.File.pathSeparatorChar;
-import static java.util.concurrent.TimeUnit.NANOSECONDS;
-import static java.util.stream.Collectors.toList;
-import static org.apache.geode.cache.execute.FunctionService.onServer;
-import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
-import static org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
-import static org.apache.geode.test.compiler.ClassBuilder.writeJarFromClasses;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.catchThrowable;
-
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.Serializable;
-import java.nio.file.Path;
-import java.util.Collection;
-import java.util.List;
-import java.util.Properties;
-
-import io.micrometer.core.instrument.Timer;
-import org.junit.After;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-
-import org.apache.geode.cache.Region;
-import org.apache.geode.cache.client.ClientCache;
-import org.apache.geode.cache.client.ClientCacheFactory;
-import org.apache.geode.cache.client.ClientRegionShortcut;
-import org.apache.geode.cache.execute.Function;
-import org.apache.geode.cache.execute.FunctionContext;
-import org.apache.geode.distributed.DistributedMember;
-import org.apache.geode.rules.ServiceJarRule;
-import org.apache.geode.security.AuthInitialize;
-import org.apache.geode.security.AuthenticationFailedException;
-import org.apache.geode.security.NotAuthorizedException;
-import org.apache.geode.security.ResourcePermission;
-import org.apache.geode.test.junit.rules.gfsh.GfshRule;
-
-public class CacheGetsTimerTest {
- private int locatorPort;
- private ClientCache clientCache;
- private Region<Object, Object> replicateRegion;
- private Region<Object, Object> partitionRegion;
-
- @Rule
- public GfshRule gfshRule = new GfshRule();
-
- @Rule
- public TemporaryFolder temporaryFolder = new TemporaryFolder();
-
- @Rule
- public ServiceJarRule serviceJarRule = new ServiceJarRule();
-
- @After
- public void tearDown() {
- if (clientCache != null) {
- clientCache.close();
- }
-
- if (locatorPort != 0) {
- String connectToLocatorCommand = "connect --locator=localhost[" + locatorPort + "]";
- String shutdownCommand = "shutdown --include-locators";
- gfshRule.execute(connectToLocatorCommand, shutdownCommand);
- }
- }
-
- @Test
- public void hitTimerRecordsCountAndTotalTime_ifGetsPerformedOnReplicateRegionWithExistingKey()
- throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- for (int i = 0; i < 5; i++) {
- replicateRegion.put(i, i);
- replicateRegion.get(i);
- }
-
- TimerValue hitTimerValue = hitTimerValueForRegion(replicateRegion);
-
- assertThat(hitTimerValue.region)
- .as("Cache gets hit timer region tag value")
- .isEqualTo(replicateRegion.getName());
-
- assertThat(hitTimerValue.result)
- .as("Cache gets hit timer result tag value")
- .isEqualTo("hit");
-
- assertThat(hitTimerValue.count)
- .as("Cache gets hit timer count for region " + replicateRegion.getName())
- .isEqualTo(5);
-
- assertThat(hitTimerValue.totalTime)
- .as("Cache gets hit timer total time for region " + replicateRegion.getName())
- .isGreaterThan(0);
- }
-
- @Test
- public void missTimerRecordsCountAndTotalTime_ifGetsPerformedOnReplicateRegionWithMissingKey()
- throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- for (int i = 0; i < 5; i++) {
- replicateRegion.get(i);
- }
-
- TimerValue missTimerValue = missTimerValueForRegion(replicateRegion);
-
- assertThat(missTimerValue.region)
- .as("Cache gets miss timer region tag value")
- .isEqualTo(replicateRegion.getName());
-
- assertThat(missTimerValue.result)
- .as("Cache gets miss timer result tag value")
- .isEqualTo("miss");
-
- assertThat(missTimerValue.count)
- .as("Cache gets miss timer count for region " + replicateRegion.getName())
- .isEqualTo(5);
-
- assertThat(missTimerValue.totalTime)
- .as("Cache gets miss timer total time for region " + replicateRegion.getName())
- .isGreaterThan(0);
- }
-
- @Test
- public void hitTimerRecordsCountAndTotalTime_ifGetsPerformedOnPartitionRegionWithExistingKey()
- throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- for (int i = 0; i < 5; i++) {
- partitionRegion.put(i, i);
- partitionRegion.get(i);
- }
-
- TimerValue hitTimerValue = hitTimerValueForRegion(partitionRegion);
-
- assertThat(hitTimerValue.region)
- .as("Cache gets hit timer region tag value")
- .isEqualTo(partitionRegion.getName());
-
- assertThat(hitTimerValue.result)
- .as("Cache gets hit timer result tag value")
- .isEqualTo("hit");
-
- assertThat(hitTimerValue.count)
- .as("Cache gets hit timer count for region " + partitionRegion.getName())
- .isEqualTo(5);
-
- assertThat(hitTimerValue.totalTime)
- .as("Cache gets hit timer total time for region " + partitionRegion.getName())
- .isGreaterThan(0);
- }
-
- @Test
- public void missTimerRecordsCountAndTotalTime_ifGetsPerformedOnPartitionRegionWithMissingKey()
- throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- for (int i = 0; i < 5; i++) {
- partitionRegion.get(i);
- }
-
- TimerValue missTimerValue = missTimerValueForRegion(partitionRegion);
-
- assertThat(missTimerValue.region)
- .as("Cache gets miss timer region tag value")
- .isEqualTo(partitionRegion.getName());
-
- assertThat(missTimerValue.result)
- .as("Cache gets miss timer result tag value")
- .isEqualTo("miss");
-
- assertThat(missTimerValue.count)
- .as("Cache gets miss timer count for region " + partitionRegion.getName())
- .isEqualTo(5);
-
- assertThat(missTimerValue.totalTime)
- .as("Cache gets miss timer total time for region " + partitionRegion.getName())
- .isGreaterThan(0);
- }
-
- @Test
- public void timersExistWithInitialValues_ifNoGetsPerformedOnReplicateRegion() throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- assertThat(allTimerValuesForRegion(replicateRegion))
- .as("All timer values for region " + replicateRegion.getName())
- .hasSize(2)
- .anyMatch(tv -> tv.result.equals("hit"))
- .anyMatch(tv -> tv.result.equals("miss"))
- .allMatch(tv -> tv.count == 0, "All timers have count of zero")
- .allMatch(tv -> tv.totalTime == 0, "All timers have total time of zero");
- }
-
- @Test
- public void allTimersRemoved_ifReplicateRegionDestroyed() throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- assertThat(allTimerValuesForRegion(replicateRegion))
- .as("All timer values before destroying region " + replicateRegion.getName())
- .hasSize(2);
-
- replicateRegion.destroyRegion();
-
- assertThat(allTimerValuesForRegion(replicateRegion))
- .as("All timer values after destroying region " + replicateRegion.getName())
- .isEmpty();
- }
-
- @Test
- public void timersExistWithInitialValues_ifNoGetsPerformedOnPartitionRegion() throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- assertThat(allTimerValuesForRegion(partitionRegion))
- .as("All timer values for region " + partitionRegion.getName())
- .hasSize(2)
- .anyMatch(tv -> tv.result.equals("hit"))
- .anyMatch(tv -> tv.result.equals("miss"))
- .allMatch(tv -> tv.count == 0, "All timers have count of zero")
- .allMatch(tv -> tv.totalTime == 0, "All timers have total time of zero");
- }
-
- @Test
- public void allTimersRemoved_ifPartitionRegionDestroyed() throws IOException {
- givenClusterWithTimeStatisticsEnabled();
-
- assertThat(allTimerValuesForRegion(partitionRegion))
- .as("All timer values before destroying region " + partitionRegion.getName())
- .hasSize(2);
-
- partitionRegion.destroyRegion();
-
- assertThat(allTimerValuesForRegion(partitionRegion))
- .as("All timer values after destroying region " + partitionRegion.getName())
- .isEmpty();
- }
-
- @Test
- public void timersRecordCountForReplicateRegion_ifTimeStatisticsDisabled() throws IOException {
- givenClusterWithTimeStatisticsDisabled();
-
- replicateRegion.put("existing-key", "existing-value");
- replicateRegion.get("existing-key");
- replicateRegion.get("missing-key");
-
- assertThat(allTimerValuesForRegion(replicateRegion))
- .as("All timer values for region " + replicateRegion.getName())
- .hasSize(2)
- .anyMatch(tv -> tv.result.equals("hit"))
- .anyMatch(tv -> tv.result.equals("miss"))
- .allMatch(tv -> tv.count == 1, "All timers have count of one")
- .allMatch(tv -> tv.totalTime == 0, "All timers have total time of zero");
- }
-
- @Test
- public void timersRecordCountForPartitionRegion_ifTimeStatisticsDisabled() throws IOException {
- givenClusterWithTimeStatisticsDisabled();
-
- partitionRegion.put("existing-key", "existing-value");
- partitionRegion.get("existing-key");
- partitionRegion.get("missing-key");
-
- assertThat(allTimerValuesForRegion(partitionRegion))
- .as("All timer values for region " + partitionRegion.getName())
- .hasSize(2)
- .anyMatch(tv -> tv.result.equals("hit"))
- .anyMatch(tv -> tv.result.equals("miss"))
- .allMatch(tv -> tv.count == 1, "All timers have count of one")
- .allMatch(tv -> tv.totalTime == 0, "All timers have total time of zero");
- }
-
- @Test
- public void timersDoNotRecord_ifGetThrows() throws IOException {
- givenClusterWithSecurityThatDeniesAllGetOperations();
-
- Throwable thrown = catchThrowable(() -> replicateRegion.get("key"));
-
- assertThat(thrown)
- .as("Exception from get operation")
- .hasCauseInstanceOf(NotAuthorizedException.class);
-
- assertThat(allTimerValuesForRegion(replicateRegion))
- .as("All timer values for region " + replicateRegion.getName())
- .hasSize(2)
- .allMatch(tv -> tv.count == 0, "All timers have count of zero");
- }
-
- private void givenClusterWithTimeStatisticsEnabled() throws IOException {
- startCluster(true, false);
- }
-
- private void givenClusterWithTimeStatisticsDisabled() throws IOException {
- startCluster(false, false);
- }
-
- private void givenClusterWithSecurityThatDeniesAllGetOperations() throws IOException {
- startCluster(true, true);
- }
-
- private void startCluster(boolean enableTimeStatistics, boolean enableSecurity)
- throws IOException {
- int[] availablePorts = getRandomAvailableTCPPorts(2);
-
- locatorPort = availablePorts[0];
- int serverPort = availablePorts[1];
-
- Path serviceJarPath = serviceJarRule.createJarFor("metrics-publishing-service.jar",
- MetricsPublishingService.class, SimpleMetricsPublishingService.class);
-
- Path helpersJarPath = temporaryFolder.getRoot().toPath()
- .resolve("helpers.jar").toAbsolutePath();
- writeJarFromClasses(helpersJarPath.toFile(), TimerValue.class,
- FetchCacheGetsTimerValues.class, DenyAllDataRead.class, ClientSecurityConfig.class);
-
- String startLocatorCommand = String.join(" ",
- "start locator",
- "--name=" + "locator",
- "--dir=" + temporaryFolder.newFolder("locator").getAbsolutePath(),
- "--port=" + locatorPort,
- "--classpath=" + helpersJarPath);
-
- String serverName = "server";
- String startServerCommand = String.join(" ",
- "start server",
- "--name=" + serverName,
- "--dir=" + temporaryFolder.newFolder(serverName).getAbsolutePath(),
- "--server-port=" + serverPort,
- "--locators=localhost[" + locatorPort + "]",
- "--classpath=" + serviceJarPath + pathSeparatorChar + helpersJarPath);
-
- if (enableTimeStatistics) {
- startServerCommand += " --enable-time-statistics";
- }
-
- if (enableSecurity) {
- File securityPropertiesFile = createSecurityPropertiesFile();
- String securityPropertiesFileOption =
- " --security-properties-file=" + securityPropertiesFile.getAbsolutePath();
- startLocatorCommand += securityPropertiesFileOption;
- startServerCommand += securityPropertiesFileOption;
- }
-
- String replicateRegionName = "ReplicateRegion";
- String createReplicateRegionCommand = String.join(" ",
- "create region",
- "--type=REPLICATE",
- "--name=" + replicateRegionName);
-
- String partitionRegionName = "PartitionRegion";
- String createPartitionRegionCommand = String.join(" ",
- "create region",
- "--type=PARTITION",
- "--name=" + partitionRegionName);
-
- gfshRule.execute(startLocatorCommand, startServerCommand, createReplicateRegionCommand,
- createPartitionRegionCommand);
-
- ClientCacheFactory clientCacheFactory = new ClientCacheFactory();
- if (enableSecurity) {
- clientCacheFactory.set("security-client-auth-init", ClientSecurityConfig.class.getName());
- }
-
- clientCache = clientCacheFactory
- .addPoolLocator("localhost", locatorPort)
- .create();
-
- replicateRegion = clientCache
- .createClientRegionFactory(ClientRegionShortcut.PROXY)
- .create(replicateRegionName);
-
- partitionRegion = clientCache
- .createClientRegionFactory(ClientRegionShortcut.PROXY)
- .create(partitionRegionName);
- }
-
- private File createSecurityPropertiesFile() throws IOException {
- Properties securityProperties = ClientSecurityConfig.securityProperties();
- File securityPropertiesFile = gfshRule.getTemporaryFolder().newFile("security.properties");
- securityProperties.store(new FileOutputStream(securityPropertiesFile), null);
- return securityPropertiesFile;
- }
-
- private TimerValue hitTimerValueForRegion(Region<?, ?> region) {
- return timerValueForRegionAndResult(region, "hit");
- }
-
- private TimerValue missTimerValueForRegion(Region<?, ?> region) {
- return timerValueForRegionAndResult(region, "miss");
- }
-
- private TimerValue timerValueForRegionAndResult(Region<?, ?> region, String resultTagValue) {
- List<TimerValue> cacheGetsTimerValues = allTimerValuesForRegion(region).stream()
- .filter(tv -> tv.result.equals(resultTagValue))
- .collect(toList());
-
- assertThat(cacheGetsTimerValues)
- .as("Timers for region " + region.getName() + " with result " + resultTagValue)
- .hasSize(1);
-
- return cacheGetsTimerValues.get(0);
- }
-
- private List<TimerValue> allTimerValuesForRegion(Region<?, ?> region) {
- @SuppressWarnings("unchecked")
- List<List<TimerValue>> timerValuesFromAllServers =
- (List<List<TimerValue>>) onServer(clientCache)
- .execute(new FetchCacheGetsTimerValues())
- .getResult();
-
- assertThat(timerValuesFromAllServers)
- .hasSize(1);
-
- return timerValuesFromAllServers.get(0).stream()
- .filter(tv -> tv.region.equals(region.getName()))
- .collect(toList());
- }
-
- static class TimerValue implements Serializable {
- final long count;
- final double totalTime;
- final String region;
- final String result;
-
- TimerValue(long count, double totalTime, String region, String result) {
- this.count = count;
- this.totalTime = totalTime;
- this.region = region;
- this.result = result;
- }
-
- @Override
- public String toString() {
- return "TimerValue{" +
- "count=" + count +
- ", totalTime=" + totalTime +
- ", region='" + region + '\'' +
- ", result='" + result + '\'' +
- '}';
- }
- }
-
- static class FetchCacheGetsTimerValues implements Function<Void> {
- private static final String ID = "FetchCacheGetsTimerValues";
-
- @Override
- public void execute(FunctionContext<Void> context) {
- Collection<Timer> timers = SimpleMetricsPublishingService.getRegistry()
- .find("geode.cache.gets")
- .timers();
-
- List<TimerValue> result = timers.stream()
- .map(FetchCacheGetsTimerValues::toTimerValues)
- .collect(toList());
-
- context.getResultSender().lastResult(result);
- }
-
- @Override
- public String getId() {
- return ID;
- }
-
- private static TimerValue toTimerValues(Timer t) {
- String region = t.getId().getTag("region");
- String result = t.getId().getTag("result");
-
- return new TimerValue(
- t.count(),
- t.totalTime(NANOSECONDS),
- region,
- result);
- }
- }
-
- public static class DenyAllDataRead implements org.apache.geode.security.SecurityManager {
- @Override
- public Object authenticate(Properties credentials) throws AuthenticationFailedException {
- String userName = credentials.getProperty(USER_NAME);
- if (userName == null) {
- throw new AuthenticationFailedException("No user name provided");
- }
- return userName;
- }
-
- @Override
- public boolean authorize(Object principal, ResourcePermission permission) {
- return !isDataRead(permission);
- }
-
- private static boolean isDataRead(ResourcePermission permission) {
- return ResourcePermission.Resource.DATA.equals(permission.getResource())
- && ResourcePermission.Operation.READ.equals(permission.getOperation());
- }
- }
-
- public static class ClientSecurityConfig implements AuthInitialize {
- @Override
- public Properties getCredentials(Properties securityProps, DistributedMember server,
- boolean isPeer) throws AuthenticationFailedException {
- return securityProperties();
- }
-
- static Properties securityProperties() {
- Properties securityProperties = new Properties();
- securityProperties.setProperty(SECURITY_MANAGER, DenyAllDataRead.class.getName());
- securityProperties.setProperty("security-username", "user");
- securityProperties.setProperty("security-password", "pass");
- return securityProperties;
- }
- }
-}
diff --git a/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/function/executions/FunctionExecutionsTimerNoResultTest.java b/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/function/executions/FunctionExecutionsTimerNoResultTest.java
index 4eb9bb6..7119b92 100644
--- a/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/function/executions/FunctionExecutionsTimerNoResultTest.java
+++ b/geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/function/executions/FunctionExecutionsTimerNoResultTest.java
@@ -56,7 +56,6 @@
private Region<Object, Object> replicateRegion;
private Region<Object, Object> partitionRegion;
private FunctionToTimeWithoutResult functionWithNoResult;
- private Duration functionDuration;
@Rule
public GfshRule gfshRule = new GfshRule();
@@ -66,6 +65,7 @@
@Rule
public ServiceJarRule serviceJarRule = new ServiceJarRule();
+ private Duration functionDuration;
@Before
public void setUp() throws IOException {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
index 7371ec0..b01eed8 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.internal.cache;
-
import org.apache.geode.StatisticDescriptor;
import org.apache.geode.Statistics;
import org.apache.geode.StatisticsFactory;
@@ -1082,8 +1081,6 @@
}
}
- public void endGetForClient(long start, boolean miss) {}
-
/**
* @param start the timestamp taken when the operation started
* @param isUpdate true if the put was an update (origin remote)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
index 1b8517b..38d207a 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalRegion.java
@@ -438,5 +438,4 @@
Set<String> getVisibleAsyncEventQueueIds();
- CachePerfStats getRegionPerfStats();
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 0c3e499..33a7ac8 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -3240,7 +3240,6 @@
return cachePerfStats;
}
- @Override
public CachePerfStats getRegionPerfStats() {
return cachePerfStats;
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java
index d3c9891..1de5477 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionPerfStats.java
@@ -14,26 +14,19 @@
*/
package org.apache.geode.internal.cache;
-import static java.util.concurrent.TimeUnit.NANOSECONDS;
-
import io.micrometer.core.instrument.Gauge;
import io.micrometer.core.instrument.MeterRegistry;
-import io.micrometer.core.instrument.Timer;
import org.apache.geode.StatisticsFactory;
import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.internal.statistics.StatisticsClock;
class RegionPerfStats extends CachePerfStats implements RegionStats {
- private static final String HIT_TAG_VALUE = "hit";
- private static final String MISS_TAG_VALUE = "miss";
private final CachePerfStats cachePerfStats;
private final StatisticsClock clock;
private final MeterRegistry meterRegistry;
private final Gauge entriesGauge;
- private final Timer cacheGetsHitTimer;
- private final Timer cacheGetsMissTimer;
RegionPerfStats(StatisticsFactory statisticsFactory, String textId, CachePerfStats cachePerfStats,
InternalRegion region, MeterRegistry meterRegistry, StatisticsClock clock) {
@@ -43,40 +36,24 @@
@VisibleForTesting
RegionPerfStats(StatisticsFactory statisticsFactory, String textId, StatisticsClock clock,
- CachePerfStats cachePerfStats, InternalRegion region, MeterRegistry meterRegistry) {
- this(statisticsFactory, textId, clock, cachePerfStats, region, meterRegistry,
- registerEntriesGauge(region, meterRegistry),
- registerCacheGetsTimer(region, meterRegistry, HIT_TAG_VALUE),
- registerCacheGetsTimer(region, meterRegistry, MISS_TAG_VALUE));
- }
-
- @VisibleForTesting
- RegionPerfStats(StatisticsFactory statisticsFactory, String textId, StatisticsClock clock,
- CachePerfStats cachePerfStats, InternalRegion region, MeterRegistry meterRegistry,
- Gauge entriesGauge, Timer cacheGetsHitTimer, Timer cacheGetsMissTimer) {
+ CachePerfStats cachePerfStats, InternalRegion region,
+ MeterRegistry meterRegistry) {
super(statisticsFactory, textId, clock);
-
this.clock = clock;
this.cachePerfStats = cachePerfStats;
this.meterRegistry = meterRegistry;
- this.entriesGauge = entriesGauge;
- this.cacheGetsHitTimer = cacheGetsHitTimer;
- this.cacheGetsMissTimer = cacheGetsMissTimer;
-
+ entriesGauge = Gauge.builder("geode.cache.entries", region::getLocalSize)
+ .description("Current number of entries in the region.")
+ .tag("region", region.getName())
+ .tag("data.policy", region.getDataPolicy().toString())
+ .baseUnit("entries")
+ .register(meterRegistry);
stats.setLongSupplier(entryCountId, region::getLocalSize);
}
@Override
protected void close() {
meterRegistry.remove(entriesGauge);
- entriesGauge.close();
-
- meterRegistry.remove(cacheGetsHitTimer);
- cacheGetsHitTimer.close();
-
- meterRegistry.remove(cacheGetsMissTimer);
- cacheGetsMissTimer.close();
-
super.close();
}
@@ -348,16 +325,6 @@
}
@Override
- public void endGetForClient(long start, boolean miss) {
- long totalNanos = clock.isEnabled() ? getTime() - start : 0;
- if (miss) {
- cacheGetsMissTimer.record(totalNanos, NANOSECONDS);
- } else {
- cacheGetsHitTimer.record(totalNanos, NANOSECONDS);
- }
- }
-
- @Override
public long endPut(long start, boolean isUpdate) {
long totalNanos = 0;
if (isUpdate) {
@@ -592,22 +559,4 @@
cachePerfStats.stats.incLong(compressionDecompressTimeId, time);
}
}
-
- private static Gauge registerEntriesGauge(InternalRegion region, MeterRegistry meterRegistry) {
- return Gauge.builder("geode.cache.entries", region::getLocalSize)
- .description("Current number of entries in the region.")
- .tag("region", region.getName())
- .tag("data.policy", region.getDataPolicy().toString())
- .baseUnit("entries")
- .register(meterRegistry);
- }
-
- private static Timer registerCacheGetsTimer(InternalRegion region, MeterRegistry meterRegistry,
- String resultTagValue) {
- return Timer.builder("geode.cache.gets")
- .description("Total time and count for GET requests from Java or native clients.")
- .tag("region", region.getName())
- .tag("result", resultTagValue)
- .register(meterRegistry);
- }
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
index 2fe6cc1..b0f9f06 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.internal.cache;
-
public interface RegionStats {
void incReliableQueuedOps(int inc);
@@ -89,8 +88,6 @@
void endGet(long start, boolean miss);
- void endGetForClient(long start, boolean miss);
-
long endPut(long start, boolean isUpdate);
void endPutAll(long start);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Get70.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Get70.java
index f8358de..67ed433 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Get70.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Get70.java
@@ -22,9 +22,7 @@
import org.apache.geode.cache.operations.GetOperationContext;
import org.apache.geode.cache.operations.internal.GetOperationContextImpl;
import org.apache.geode.distributed.internal.DistributionStats;
-import org.apache.geode.internal.cache.CachePerfStats;
import org.apache.geode.internal.cache.CachedDeserializable;
-import org.apache.geode.internal.cache.InternalRegion;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.cache.Token;
@@ -125,7 +123,8 @@
Region region = serverConnection.getCache().getRegion(regionName);
if (region == null) {
- String reason = String.format("%s was not found during get request", regionName);
+ String reason = String.format("%s was not found during get request",
+ regionName);
writeRegionDestroyedEx(clientMessage, regionName, reason, serverConnection);
serverConnection.setAsTrue(RESPONDED);
return;
@@ -222,8 +221,7 @@
}
stats.incWriteGetResponseTime(DistributionStats.getStatTime() - start);
- CachePerfStats regionPerfStats = ((InternalRegion) region).getRegionPerfStats();
- regionPerfStats.endGetForClient(startparam, entry.keyNotPresent);
+
}
/**
@@ -308,8 +306,12 @@
} else if (data instanceof byte[]) {
isObject = false;
}
- boolean keyNotPresent = !wasInvalid && (data == null || data == Token.TOMBSTONE);
- return new Entry(data, isObject, keyNotPresent, versionTag);
+ Entry result = new Entry();
+ result.value = data;
+ result.isObject = isObject;
+ result.keyNotPresent = !wasInvalid && (data == null || data == Token.TOMBSTONE);
+ result.versionTag = versionTag;
+ return result;
}
/**
@@ -358,23 +360,20 @@
data = cd.getValue();
}
}
- boolean keyNotPresent = !wasInvalid && (data == null || data == Token.TOMBSTONE);
- return new Entry(data, isObject, keyNotPresent, versionTag);
+ Entry result = new Entry();
+ result.value = data;
+ result.isObject = isObject;
+ result.keyNotPresent = !wasInvalid && (data == null || data == Token.TOMBSTONE);
+ result.versionTag = versionTag;
+ return result;
}
/** this is used to return results from getValueAndIsObject */
public static class Entry {
- public final Object value;
- public final boolean isObject;
- public final boolean keyNotPresent;
- public final VersionTag versionTag;
-
- public Entry(Object value, boolean isObject, boolean keyNotPresent, VersionTag versionTag) {
- this.value = value;
- this.isObject = isObject;
- this.keyNotPresent = keyNotPresent;
- this.versionTag = versionTag;
- }
+ public Object value;
+ public boolean isObject;
+ public boolean keyNotPresent;
+ public VersionTag versionTag;
@Override
public String toString() {
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetEntry70.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetEntry70.java
index 4af8da7..7dac4c9 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetEntry70.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/GetEntry70.java
@@ -64,6 +64,11 @@
data = snap;
tag = snap.getVersionTag();
}
- return new Entry(data, true, false, tag);
+ Get70.Entry result = new Get70.Entry();
+ result.value = data;
+ result.isObject = true;
+ result.keyNotPresent = false;
+ result.versionTag = tag;
+ return result;
}
}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java
index 97e6b10..6be79c8 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/RegionPerfStatsTest.java
@@ -14,9 +14,7 @@
*/
package org.apache.geode.internal.cache;
-import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static org.apache.geode.internal.statistics.StatisticsClockFactory.disabledClock;
-import static org.apache.geode.test.micrometer.MicrometerAssertions.assertThat;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
@@ -25,7 +23,6 @@
import static org.mockito.Mockito.when;
import static org.mockito.quality.Strictness.STRICT_STUBS;
-import java.util.Collection;
import java.util.function.LongSupplier;
import io.micrometer.core.instrument.Gauge;
@@ -44,40 +41,39 @@
import org.apache.geode.Statistics;
import org.apache.geode.StatisticsFactory;
import org.apache.geode.cache.DataPolicy;
-import org.apache.geode.internal.statistics.StatisticsClock;
public class RegionPerfStatsTest {
private static final String REGION_NAME = "region1";
private static final String TEXT_ID = "textId";
private static final DataPolicy DATA_POLICY = DataPolicy.PERSISTENT_REPLICATE;
- private static final long CLOCK_VALUE = 5L;
private MeterRegistry meterRegistry;
private CachePerfStats cachePerfStats;
private InternalRegion region;
+
private RegionPerfStats regionPerfStats;
+ private RegionPerfStats regionPerfStats2;
private Statistics statistics;
- private StatisticsClock statisticsClock;
@Rule
public MockitoRule mockitoRule = MockitoJUnit.rule().strictness(STRICT_STUBS);
- private StatisticsFactory statisticsFactory;
@Before
public void setUp() {
meterRegistry = new SimpleMeterRegistry();
cachePerfStats = mock(CachePerfStats.class);
+ StatisticsFactory statisticsFactory = mock(StatisticsFactory.class);
statistics = mock(Statistics.class);
region = mock(InternalRegion.class);
when(region.getName()).thenReturn(REGION_NAME);
when(region.getDataPolicy()).thenReturn(DATA_POLICY);
- statisticsFactory = mock(StatisticsFactory.class);
- when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(statistics);
- statisticsClock = mock(StatisticsClock.class);
- regionPerfStats = new RegionPerfStats(statisticsFactory, TEXT_ID, statisticsClock,
- cachePerfStats, region, meterRegistry);
+ when(statisticsFactory.createAtomicStatistics(any(), any())).thenReturn(statistics);
+
+ regionPerfStats =
+ new RegionPerfStats(statisticsFactory, TEXT_ID, disabledClock(), cachePerfStats, region,
+ meterRegistry);
}
@After
@@ -85,6 +81,9 @@
if (regionPerfStats != null) {
regionPerfStats.close();
}
+ if (regionPerfStats2 != null) {
+ regionPerfStats2.close();
+ }
}
@Test
@@ -100,30 +99,23 @@
@Test
public void constructor_createsEntriesGauge_taggedWithRegionName() {
- assertThat(entriesGauge())
- .as("geode.cache.entries gauge")
- .hasTag("region", REGION_NAME);
+ Gauge entriesGauge = meterRegistry
+ .find("geode.cache.entries")
+ .gauge();
+
+ assertThat(entriesGauge.getId().getTag("region"))
+ .as("region tag")
+ .isEqualTo(REGION_NAME);
}
@Test
public void constructor_createsEntriesGauge_taggedWithDataPolicy() {
- assertThat(entriesGauge())
- .as("geode.cache.entries gauge")
- .hasTag("data.policy", DATA_POLICY.toString());
- }
-
- @Test
- public void constructor_createsCacheGetsHitTimer_taggedWithRegionName() {
- assertThat(cacheGetsHitTimer())
- .as("geode.cache.gets timer with tag result=hit")
- .hasTag("region", REGION_NAME);
- }
-
- @Test
- public void constructor_createsCacheGetsMissTimer_taggedWithRegionName() {
- assertThat(cacheGetsMissTimer())
- .as("geode.cache.gets timer with tag result=miss")
- .hasTag("region", REGION_NAME);
+ Gauge entriesGauge = meterRegistry
+ .find("geode.cache.entries")
+ .gauge();
+ assertThat(entriesGauge.getId().getTag("data.policy"))
+ .as("data.policy tag")
+ .isEqualTo(DATA_POLICY.toString());
}
@Test
@@ -154,112 +146,20 @@
.find("geode.cache.entries")
.tag("region", REGION_NAME)
.gauge();
-
assertThat(entriesGauge.value()).isEqualTo(3);
}
@Test
- public void endGetForClient_recordsHitTimerCountAndTotalTime_ifCacheHitAndClockEnabled() {
- when(statisticsClock.isEnabled()).thenReturn(true);
- when(statisticsClock.getTime()).thenReturn(CLOCK_VALUE);
-
- regionPerfStats.endGetForClient(0, false);
-
- assertThat(cacheGetsHitTimer())
- .as("geode.cache.gets timer with tag result=hit")
- .hasCount(1)
- .hasTotalTime(NANOSECONDS, CLOCK_VALUE);
- }
-
- @Test
- public void endGetForClient_doesNotRecordMissTimerCountOrTotalTime_ifCacheHitAndClockEnabled() {
- when(statisticsClock.isEnabled()).thenReturn(true);
- when(statisticsClock.getTime()).thenReturn(CLOCK_VALUE);
-
- regionPerfStats.endGetForClient(0, false);
-
- assertThat(cacheGetsMissTimer())
- .as("geode.cache.gets timer with tag result=miss")
- .hasCount(0)
- .hasTotalTime(NANOSECONDS, 0);
- }
-
- @Test
- public void endGetForClient_recordsHitTimerCountOnly_ifCacheHitAndClockDisabled() {
- when(statisticsClock.isEnabled()).thenReturn(false);
-
- regionPerfStats.endGetForClient(0, false);
-
- assertThat(cacheGetsHitTimer())
- .as("geode.cache.gets timer with tag result=hit")
- .hasCount(1)
- .hasTotalTime(NANOSECONDS, 0);
- }
-
- @Test
- public void endGetForClient_recordsMissTimerCountAndTotalTime_ifCacheMissAndClockEnabled() {
- when(statisticsClock.isEnabled()).thenReturn(true);
- when(statisticsClock.getTime()).thenReturn(CLOCK_VALUE);
-
- regionPerfStats.endGetForClient(0, true);
-
- assertThat(cacheGetsMissTimer())
- .as("geode.cache.gets timer with tag result=miss")
- .hasCount(1)
- .hasTotalTime(NANOSECONDS, CLOCK_VALUE);
- }
-
- @Test
- public void endGetForClient_doesNotRecordHitTimerCountOrTotalTime_ifCacheMissAndClockEnabled() {
- when(statisticsClock.isEnabled()).thenReturn(true);
- when(statisticsClock.getTime()).thenReturn(CLOCK_VALUE);
-
- regionPerfStats.endGetForClient(0, true);
-
- assertThat(cacheGetsHitTimer())
- .as("geode.cache.gets timer with tag result=hit")
- .hasCount(0)
- .hasTotalTime(NANOSECONDS, 0);
- }
-
- @Test
- public void endGetForClient_recordsMissTimerCountOnly_ifCacheMissAndClockDisabled() {
- when(statisticsClock.isEnabled()).thenReturn(false);
-
- regionPerfStats.endGetForClient(0, true);
-
- assertThat(cacheGetsMissTimer())
- .as("geode.cache.gets timer with tag result=miss")
- .hasCount(1)
- .hasTotalTime(NANOSECONDS, 0);
- }
-
- @Test
- public void close_removesEntriesGaugeFromTheRegistry() {
- assertThat(metersNamed("geode.cache.entries"))
+ public void close_removesItsOwnMetersFromTheRegistry() {
+ assertThat(meterNamed("geode.cache.entries"))
.as("entries gauge before closing the stats")
- .hasSize(1);
+ .isNotNull();
regionPerfStats.close();
- assertThat(metersNamed("geode.cache.entries"))
+ assertThat(meterNamed("geode.cache.entries"))
.as("entries gauge after closing the stats")
- .hasSize(0);
-
- regionPerfStats = null;
- }
-
- @Test
- public void close_removesCacheGetsTimersFromTheRegistry() {
- assertThat(metersNamed("geode.cache.gets"))
- .as("cache gets timer before closing the stats")
- .hasSize(2);
-
- regionPerfStats.close();
-
- assertThat(metersNamed("geode.cache.gets"))
- .as("cache gets timer after closing the stats")
- .hasSize(0);
+ .isNull();
regionPerfStats = null;
}
@@ -273,55 +173,16 @@
regionPerfStats.close();
- assertThat(metersNamed(foreignMeterName))
+ assertThat(meterNamed(foreignMeterName))
.as("foreign meter after closing the stats")
- .hasSize(1);
+ .isNotNull();
regionPerfStats = null;
}
- @Test
- public void close_closesMeters() {
- Gauge entriesGauge = mock(Gauge.class);
- Timer cacheGetsHitTimer = mock(Timer.class);
- Timer cacheGetsMissTimer = mock(Timer.class);
- regionPerfStats = new RegionPerfStats(statisticsFactory, TEXT_ID, statisticsClock,
- cachePerfStats, region, mock(MeterRegistry.class), entriesGauge, cacheGetsHitTimer,
- cacheGetsMissTimer);
-
- regionPerfStats.close();
-
- verify(entriesGauge).close();
- verify(cacheGetsHitTimer).close();
- verify(cacheGetsMissTimer).close();
-
- regionPerfStats = null;
- }
-
- private Collection<Meter> metersNamed(String meterName) {
+ private Meter meterNamed(String meterName) {
return meterRegistry
.find(meterName)
- .meters();
- }
-
- private Gauge entriesGauge() {
- return meterRegistry
- .find("geode.cache.entries")
- .gauge();
- }
-
- private Timer cacheGetsHitTimer() {
- return cacheGetsTimer("hit");
- }
-
- private Timer cacheGetsMissTimer() {
- return cacheGetsTimer("miss");
- }
-
- private Timer cacheGetsTimer(String resultTagValue) {
- return meterRegistry
- .find("geode.cache.gets")
- .tag("result", resultTagValue)
- .timer();
+ .meter();
}
}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Get70Test.java b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Get70Test.java
index 234fc4a..b67b502 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Get70Test.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Get70Test.java
@@ -14,28 +14,23 @@
*/
package org.apache.geode.internal.cache.tier.sockets.command;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyBoolean;
-import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.ArgumentMatchers.isA;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-import java.io.IOException;
-
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.apache.geode.CancelCriterion;
import org.apache.geode.cache.operations.GetOperationContext;
-import org.apache.geode.internal.cache.CachePerfStats;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.LocalRegion;
import org.apache.geode.internal.cache.tier.CachedRegionHelper;
@@ -57,6 +52,7 @@
private static final String REGION_NAME = "region1";
private static final String KEY = "key1";
private static final Object CALLBACK_ARG = "arg";
+ private static final byte[] EVENT = new byte[8];
@Mock
private SecurityService securityService;
@@ -84,149 +80,95 @@
private Part valuePart;
@Mock
private GetOperationContext getOperationContext;
-
+ @InjectMocks
private Get70 get70;
@Before
public void setUp() throws Exception {
- get70 = new Get70();
+ this.get70 = new Get70();
MockitoAnnotations.initMocks(this);
- when(authzRequest.getAuthorize(any(), any(), any())).thenReturn(getOperationContext);
+ when(this.authzRequest.getAuthorize(any(), any(), any())).thenReturn(this.getOperationContext);
- when(cache.getRegion(isA(String.class))).thenReturn(region);
- when(cache.getCancelCriterion()).thenReturn(mock(CancelCriterion.class));
+ when(this.cache.getRegion(isA(String.class))).thenReturn(this.region);
+ when(this.cache.getCancelCriterion()).thenReturn(mock(CancelCriterion.class));
- when(getOperationContext.getCallbackArg()).thenReturn(CALLBACK_ARG);
+ when(this.getOperationContext.getCallbackArg()).thenReturn(CALLBACK_ARG);
- when(keyPart.getStringOrObject()).thenReturn(KEY);
+ when(this.keyPart.getStringOrObject()).thenReturn(KEY);
- when(message.getNumberOfParts()).thenReturn(3);
- when(message.getPart(eq(0))).thenReturn(regionNamePart);
- when(message.getPart(eq(1))).thenReturn(keyPart);
- when(message.getPart(eq(2))).thenReturn(valuePart);
+ when(this.message.getNumberOfParts()).thenReturn(3);
+ when(this.message.getPart(eq(0))).thenReturn(this.regionNamePart);
+ when(this.message.getPart(eq(1))).thenReturn(this.keyPart);
+ when(this.message.getPart(eq(2))).thenReturn(this.valuePart);
- when(regionNamePart.getCachedString()).thenReturn(REGION_NAME);
+ when(this.regionNamePart.getCachedString()).thenReturn(REGION_NAME);
- when(serverConnection.getCache()).thenReturn(cache);
- when(serverConnection.getCacheServerStats()).thenReturn(cacheServerStats);
- when(serverConnection.getAuthzRequest()).thenReturn(authzRequest);
- when(serverConnection.getResponseMessage()).thenReturn(responseMessage);
- when(serverConnection.getCachedRegionHelper()).thenReturn(mock(CachedRegionHelper.class));
- when(serverConnection.getErrorResponseMessage()).thenReturn(errorResponseMessage);
- when(serverConnection.getClientVersion()).thenReturn(Version.CURRENT);
+ when(this.serverConnection.getCache()).thenReturn(this.cache);
+ when(this.serverConnection.getCacheServerStats()).thenReturn(this.cacheServerStats);
+ when(this.serverConnection.getAuthzRequest()).thenReturn(this.authzRequest);
+ when(this.serverConnection.getResponseMessage()).thenReturn(this.responseMessage);
+ when(this.serverConnection.getCachedRegionHelper()).thenReturn(mock(CachedRegionHelper.class));
+ when(this.serverConnection.getErrorResponseMessage()).thenReturn(this.errorResponseMessage);
+ when(this.serverConnection.getClientVersion()).thenReturn(Version.CURRENT);
- when(valuePart.getObject()).thenReturn(CALLBACK_ARG);
-
- when(securityService.isClientSecurityRequired()).thenReturn(false);
-
- when(region.getRegionPerfStats()).thenReturn(mock(CachePerfStats.class));
+ when(this.valuePart.getObject()).thenReturn(CALLBACK_ARG);
}
@Test
public void noSecurityShouldSucceed() throws Exception {
- when(securityService.isClientSecurityRequired()).thenReturn(false);
+ when(this.securityService.isClientSecurityRequired()).thenReturn(false);
- get70.cmdExecute(message, serverConnection, securityService, 0);
- verify(responseMessage).send(serverConnection);
+ this.get70.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
+ verify(this.responseMessage).send(this.serverConnection);
}
@Test
public void integratedSecurityShouldSucceedIfAuthorized() throws Exception {
- givenIntegratedSecurity();
+ when(this.securityService.isClientSecurityRequired()).thenReturn(true);
+ when(this.securityService.isIntegratedSecurity()).thenReturn(true);
- get70.cmdExecute(message, serverConnection, securityService, 0);
+ this.get70.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
- verify(securityService).authorize(Resource.DATA, Operation.READ, REGION_NAME, KEY);
- verify(responseMessage).send(serverConnection);
+ verify(this.securityService).authorize(Resource.DATA, Operation.READ, REGION_NAME, KEY);
+ verify(this.responseMessage).send(this.serverConnection);
}
@Test
public void integratedSecurityShouldFailIfNotAuthorized() throws Exception {
- givenIntegratedSecurity();
- givenIntegratedSecurityNotAuthorized();
+ when(this.securityService.isClientSecurityRequired()).thenReturn(true);
+ when(this.securityService.isIntegratedSecurity()).thenReturn(true);
+ doThrow(new NotAuthorizedException("")).when(this.securityService).authorize(Resource.DATA,
+ Operation.READ, REGION_NAME, KEY);
- get70.cmdExecute(message, serverConnection, securityService, 0);
+ this.get70.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
- verify(securityService).authorize(Resource.DATA, Operation.READ, REGION_NAME, KEY);
- verify(errorResponseMessage).send(eq(serverConnection));
+ verify(this.securityService).authorize(Resource.DATA, Operation.READ, REGION_NAME, KEY);
+ verify(this.errorResponseMessage).send(eq(this.serverConnection));
}
@Test
public void oldSecurityShouldSucceedIfAuthorized() throws Exception {
- givenOldSecurity();
+ when(this.securityService.isClientSecurityRequired()).thenReturn(true);
+ when(this.securityService.isIntegratedSecurity()).thenReturn(false);
- get70.cmdExecute(message, serverConnection, securityService, 0);
+ this.get70.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
- verify(authzRequest).getAuthorize(eq(REGION_NAME), eq(KEY), eq(CALLBACK_ARG));
- verify(responseMessage).send(serverConnection);
+ verify(this.authzRequest).getAuthorize(eq(REGION_NAME), eq(KEY), eq(CALLBACK_ARG));
+ verify(this.responseMessage).send(this.serverConnection);
}
@Test
public void oldSecurityShouldFailIfNotAuthorized() throws Exception {
- givenOldSecurity();
- givenOldSecurityNotAuthorized();
-
- get70.cmdExecute(message, serverConnection, securityService, 0);
-
- verify(authzRequest).getAuthorize(eq(REGION_NAME), eq(KEY), eq(CALLBACK_ARG));
- verify(errorResponseMessage).send(eq(serverConnection));
- }
-
- @Test
- public void callsEndGetForClient_ifGetSucceedsWithHit() throws IOException {
- CachePerfStats regionPerfStats = mock(CachePerfStats.class);
- when(region.getRegionPerfStats()).thenReturn(regionPerfStats);
- when(region.getRetained(any(), any(), anyBoolean(), anyBoolean(), any(), any(), anyBoolean()))
- .thenReturn("data");
-
- get70.cmdExecute(message, serverConnection, securityService, 0);
-
- verify(regionPerfStats).endGetForClient(0, false);
- }
-
- @Test
- public void callsEndGetForClient_ifGetSucceedsWithMiss() throws IOException {
- CachePerfStats regionPerfStats = mock(CachePerfStats.class);
- when(region.getRegionPerfStats()).thenReturn(regionPerfStats);
- when(region.getRetained(any(), any(), anyBoolean(), anyBoolean(), any(), any(), anyBoolean()))
- .thenReturn(null);
-
- get70.cmdExecute(message, serverConnection, securityService, 0);
-
- verify(regionPerfStats).endGetForClient(0, true);
- }
-
- @Test
- public void doesNotCallEndGetForClient_ifGetFails() throws IOException {
- givenIntegratedSecurity();
- givenIntegratedSecurityNotAuthorized();
-
- CachePerfStats regionPerfStats = mock(CachePerfStats.class);
- when(region.getRegionPerfStats()).thenReturn(regionPerfStats);
-
- get70.cmdExecute(message, serverConnection, securityService, 0);
-
- verify(regionPerfStats, never()).endGetForClient(anyLong(), anyBoolean());
- }
-
- private void givenIntegratedSecurity() {
- when(securityService.isClientSecurityRequired()).thenReturn(true);
- when(securityService.isIntegratedSecurity()).thenReturn(true);
- }
-
- private void givenOldSecurity() {
- when(securityService.isClientSecurityRequired()).thenReturn(true);
- when(securityService.isIntegratedSecurity()).thenReturn(false);
- }
-
- private void givenIntegratedSecurityNotAuthorized() {
- doThrow(new NotAuthorizedException("")).when(securityService).authorize(Resource.DATA,
- Operation.READ, REGION_NAME, KEY);
- }
-
- private void givenOldSecurityNotAuthorized() {
- doThrow(new NotAuthorizedException("")).when(authzRequest).getAuthorize(eq(REGION_NAME),
+ when(this.securityService.isClientSecurityRequired()).thenReturn(true);
+ when(this.securityService.isIntegratedSecurity()).thenReturn(false);
+ doThrow(new NotAuthorizedException("")).when(this.authzRequest).getAuthorize(eq(REGION_NAME),
eq(KEY), eq(CALLBACK_ARG));
+
+ this.get70.cmdExecute(this.message, this.serverConnection, this.securityService, 0);
+
+ verify(this.authzRequest).getAuthorize(eq(REGION_NAME), eq(KEY), eq(CALLBACK_ARG));
+ verify(this.errorResponseMessage).send(eq(this.serverConnection));
}
+
}
diff --git a/geode-junit/src/main/java/org/apache/geode/test/assertj/Conditions.java b/geode-junit/src/main/java/org/apache/geode/test/assertj/Conditions.java
deleted file mode 100644
index 49beb2b..0000000
--- a/geode-junit/src/main/java/org/apache/geode/test/assertj/Conditions.java
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * 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.test.assertj;
-
-import org.assertj.core.api.Condition;
-
-public class Conditions {
- public static <T> Condition<T> equalTo(T value) {
- return new Condition<>(value::equals, value.toString());
- }
-}
diff --git a/geode-junit/src/main/java/org/apache/geode/test/micrometer/AbstractMeterAssert.java b/geode-junit/src/main/java/org/apache/geode/test/micrometer/AbstractMeterAssert.java
index c4c1871..7e831cd 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/micrometer/AbstractMeterAssert.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/micrometer/AbstractMeterAssert.java
@@ -38,7 +38,7 @@
*/
AbstractMeterAssert(M meter, Class<A> selfType) {
super(meter, selfType);
- meterId = meter == null ? null : meter.getId();
+ meterId = meter.getId();
}
/**
diff --git a/geode-junit/src/main/java/org/apache/geode/test/micrometer/TimerAssert.java b/geode-junit/src/main/java/org/apache/geode/test/micrometer/TimerAssert.java
index f7fe286..39ff146 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/micrometer/TimerAssert.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/micrometer/TimerAssert.java
@@ -14,8 +14,6 @@
*/
package org.apache.geode.test.micrometer;
-import static org.apache.geode.test.assertj.Conditions.equalTo;
-
import java.util.concurrent.TimeUnit;
import io.micrometer.core.instrument.Timer;
@@ -37,18 +35,6 @@
/**
* Verifies that the timer's count satisfies the given condition.
*
- * @param count the expected value of the timer's count
- * @return this assertion object
- * @throws AssertionError if the timer is {@code null}
- * @throws AssertionError if the timer's count is not equal to the given count
- */
- public TimerAssert hasCount(long count) {
- return hasCount(equalTo(count));
- }
-
- /**
- * Verifies that the timer's count satisfies the given condition.
- *
* @param condition the criteria against which to evaluate the timer's count
* @return this assertion object
* @throws AssertionError if the timer is {@code null}
@@ -67,19 +53,6 @@
* Verifies that the timer's total time satisfies the given condition.
*
* @param timeUnit the time unit to which to convert the total time before evaluating
- * @param totalTime the expected value of the timer's total time
- * @return this assertion object
- * @throws AssertionError if the timer is {@code null}
- * @throws AssertionError if the timer's converted total time is not equal to the given total time
- */
- public TimerAssert hasTotalTime(TimeUnit timeUnit, double totalTime) {
- return hasTotalTime(timeUnit, equalTo(totalTime));
- }
-
- /**
- * Verifies that the timer's total time satisfies the given condition.
- *
- * @param timeUnit the time unit to which to convert the total time before evaluating
* @param condition the criteria against which to evaluate the timer's total time
* @return this assertion object
* @throws AssertionError if the timer is {@code null}
