GEODE-393: GetRegionFunction uses the cache in the FunctionContext
Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/e5021747 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/e5021747 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/e5021747 Branch: refs/heads/feature/GEM-1483 Commit: e5021747ee11219215551e74d1146d95de7411fe Parents: acdf2e8 Author: Jinmei Liao <[email protected]> Authored: Fri Jul 21 14:27:11 2017 -0700 Committer: Jinmei Liao <[email protected]> Committed: Mon Jul 24 08:58:44 2017 -0700 ---------------------------------------------------------------------- .../cli/functions/GetRegionsFunction.java | 3 +- .../functions/GetRegionsFunctionJUnitTest.java | 20 +---- .../cli/functions/GetRegionsFunctionTest.java | 88 ++++++++++++++++++++ 3 files changed, 93 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/e5021747/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java index d524924..6571dca 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunction.java @@ -18,7 +18,6 @@ import java.util.HashSet; import java.util.Set; import org.apache.geode.cache.Cache; -import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.Region; import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionContext; @@ -41,7 +40,7 @@ public class GetRegionsFunction implements Function, InternalEntity { @Override public void execute(FunctionContext functionContext) { try { - Cache cache = CacheFactory.getAnyInstance(); + Cache cache = functionContext.getCache(); Set<Region<?, ?>> regions = cache.rootRegions(); // should never return a null if (regions == null || regions.isEmpty()) { http://git-wip-us.apache.org/repos/asf/geode/blob/e5021747/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionJUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionJUnitTest.java index bfd6b91..ad43d81 100644 --- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionJUnitTest.java @@ -14,7 +14,8 @@ */ package org.apache.geode.management.internal.cli.functions; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.util.HashSet; import java.util.Set; @@ -22,17 +23,11 @@ import java.util.Set; import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; import org.apache.geode.CancelCriterion; -import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.DataPolicy; import org.apache.geode.cache.Region; import org.apache.geode.cache.RegionAttributes; @@ -44,13 +39,9 @@ import org.apache.geode.distributed.internal.InternalDistributedSystem; import org.apache.geode.internal.cache.GemFireCacheImpl; import org.apache.geode.internal.cache.LocalRegion; import org.apache.geode.internal.cache.control.InternalResourceManager; -import org.apache.geode.internal.security.AuthorizeRequest; import org.apache.geode.test.junit.categories.UnitTest; @Category(UnitTest.class) -@RunWith(PowerMockRunner.class) -@PowerMockIgnore("*.UnitTest") -@PrepareForTest({CacheFactory.class}) public class GetRegionsFunctionJUnitTest { TestResultSender testResultSender = new TestResultSender(); @@ -58,8 +49,7 @@ public class GetRegionsFunctionJUnitTest { Set<Region<?, ?>> subregions = new HashSet<>(); @Mock private RegionAttributes regionAttributes; - @Mock - private AuthorizeRequest authzRequest; + @Mock private LocalRegion region; @Mock @@ -81,9 +71,7 @@ public class GetRegionsFunctionJUnitTest { when(this.cache.getDistributedSystem()).thenReturn(mock(InternalDistributedSystem.class)); when(this.cache.getResourceManager()).thenReturn(this.internalResourceManager); when(functionContext.getResultSender()).thenReturn(testResultSender); - - PowerMockito.mockStatic(CacheFactory.class); - when(CacheFactory.getAnyInstance()).thenReturn(cache); + when(functionContext.getCache()).thenReturn(cache); } @Test http://git-wip-us.apache.org/repos/asf/geode/blob/e5021747/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java new file mode 100644 index 0000000..77db7cd --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionTest.java @@ -0,0 +1,88 @@ +/* + * 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.cli.functions; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.awaitility.core.ConditionTimeoutException; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.CacheFactory; +import org.apache.geode.cache.execute.FunctionContext; +import org.apache.geode.cache.execute.ResultSender; +import org.apache.geode.test.junit.categories.UnitTest; + +@Category(UnitTest.class) +public class GetRegionsFunctionTest { + + private enum STATE { + INITIAL, BLOCKING, FINISHED + }; + + private static GetRegionsFunctionTest.STATE lockingThreadState = + GetRegionsFunctionTest.STATE.INITIAL; + private static AtomicBoolean lockAquired = new AtomicBoolean(false); + private static AtomicBoolean functionExecuted = new AtomicBoolean(false); + + private GetRegionsFunction getRegionsFunction; + private FunctionContext functionContext; + + @Before + public void before() { + getRegionsFunction = new GetRegionsFunction(); + functionContext = mock(FunctionContext.class); + } + + @Test + public void doNotUseCacheFacotryToGetCache() throws Exception { + // start a thread that would hold on to the CacheFactory's class lock + new Thread(() -> { + synchronized (CacheFactory.class) { + lockAquired.set(true); + lockingThreadState = GetRegionsFunctionTest.STATE.BLOCKING; + try { + await().atMost(10, TimeUnit.SECONDS).untilTrue(functionExecuted); + } catch (ConditionTimeoutException e) { + e.printStackTrace(); + lockingThreadState = GetRegionsFunctionTest.STATE.FINISHED; + } + } + }).start(); + + // wait till the blocking thread aquired the lock on CacheFactory + await().atMost(1, TimeUnit.SECONDS).untilTrue(lockAquired); + when(functionContext.getCache()).thenReturn(mock(Cache.class)); + when(functionContext.getResultSender()).thenReturn(mock(ResultSender.class)); + + // execute a function that would get the cache, make sure that's not waiting on the lock + // of CacheFactory + getRegionsFunction.execute(functionContext); + assertThat(lockingThreadState).isEqualTo(lockingThreadState.BLOCKING); + + + // this will make the awaitility call in the thread return + functionExecuted.set(true); + } +}
