Repository: incubator-geode Updated Branches: refs/heads/develop 71f6d677e -> 769d9b3ae
GEODE-136: Fix possible NullPointerException in Gfsh's 'list regions' command's GetRegionsFunction. * this closes #253 Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/769d9b3a Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/769d9b3a Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/769d9b3a Branch: refs/heads/develop Commit: 769d9b3ae6435beb967b7c2f648ee01aa909d271 Parents: 71f6d67 Author: Kevin Duling <[email protected]> Authored: Tue Oct 4 17:08:17 2016 -0700 Committer: Jinmei Liao <[email protected]> Committed: Fri Oct 7 07:43:49 2016 -0700 ---------------------------------------------------------------------- .../cli/functions/GetRegionsFunction.java | 69 +++++------- .../functions/GetRegionsFunctionJUnitTest.java | 111 +++++++++++++++++++ 2 files changed, 141 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/769d9b3a/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 bfb6164..658f012 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 @@ -20,55 +20,46 @@ import java.util.HashSet; import java.util.Set; import org.apache.geode.cache.Cache; -import org.apache.geode.cache.CacheClosedException; import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.Region; -import org.apache.geode.cache.execute.FunctionAdapter; +import org.apache.geode.cache.execute.Function; import org.apache.geode.cache.execute.FunctionContext; import org.apache.geode.internal.InternalEntity; import org.apache.geode.management.internal.cli.domain.RegionInformation; /** * Function that retrieves regions hosted on every member - * */ -public class GetRegionsFunction extends FunctionAdapter implements InternalEntity { +public class GetRegionsFunction implements Function, InternalEntity { + + private static final long serialVersionUID = 1L; + + @Override + public String getId() { + // TODO Auto-generated method stub + return GetRegionsFunction.class.toString(); + } + + @Override + public void execute(FunctionContext functionContext) { + try { + Cache cache = CacheFactory.getAnyInstance(); + Set<Region<?, ?>> regions = cache.rootRegions(); // should never return a null - /** - * - */ - private static final long serialVersionUID = 1L; + if (regions == null || regions.isEmpty()) { + functionContext.getResultSender().lastResult(null); + } else { + Set<RegionInformation> regionInformationSet = new HashSet<>(); - @Override - public String getId() { - // TODO Auto-generated method stub - return GetRegionsFunction.class.toString(); - } - - @Override - public void execute(FunctionContext functionContext) { - try { - - Cache cache = CacheFactory.getAnyInstance(); - Set <Region<?,?>> regions = cache.rootRegions(); - - if (regions.isEmpty() || regions == null) { - functionContext.getResultSender().lastResult(null); - } else { - //Set<RegionInformation> regionInformationSet = RegionInformation.getRegionInformation(regions, true); - Set<RegionInformation> regionInformationSet = new HashSet<RegionInformation>(); - - for (Region<?,?> region : regions) { - RegionInformation regInfo = new RegionInformation(region, true); - regionInformationSet.add(regInfo); - } - functionContext.getResultSender().lastResult(regionInformationSet.toArray()); - } - } catch (CacheClosedException e) { - functionContext.getResultSender().sendException(e); - } catch (Exception e) { - functionContext.getResultSender().sendException(e); - } - } + for (Region<?, ?> region : regions) { + RegionInformation regInfo = new RegionInformation(region, true); + regionInformationSet.add(regInfo); + } + functionContext.getResultSender().lastResult(regionInformationSet.toArray()); + } + } catch (Exception e) { + functionContext.getResultSender().sendException(e); + } + } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/769d9b3a/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 new file mode 100644 index 0000000..6c57160 --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/GetRegionsFunctionJUnitTest.java @@ -0,0 +1,111 @@ +package org.apache.geode.management.internal.cli.functions; + +import static org.mockito.Mockito.*; + +import java.util.HashSet; +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; +import org.apache.geode.cache.Scope; +import org.apache.geode.cache.execute.FunctionContext; +import org.apache.geode.cache.execute.ResultSender; +import org.apache.geode.distributed.internal.DistributionConfig; +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(); + Set<Region<?, ?>> regions = new HashSet<>(); + Set<Region<?, ?>> subregions = new HashSet<>(); + @Mock + private RegionAttributes regionAttributes; + @Mock + private AuthorizeRequest authzRequest; + @Mock + private LocalRegion region; + @Mock + private GemFireCacheImpl cache; + @Mock + private InternalResourceManager internalResourceManager; + @Mock + private FunctionContext functionContext; + @InjectMocks + private GetRegionsFunction getRegionsFunction; + + @Before + public void before() throws Exception { + System.setProperty(DistributionConfig.GEMFIRE_PREFIX + "statsDisabled", "true"); + getRegionsFunction = new GetRegionsFunction(); + MockitoAnnotations.initMocks(this); + + when(this.cache.getCancelCriterion()).thenReturn(mock(CancelCriterion.class)); + 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); + } + + @Test + public void testExecuteWithoutRegions() throws Exception { + getRegionsFunction.execute(functionContext); + } + + @Test + public void testExecuteWithRegions() throws Exception { + when(cache.rootRegions()).thenReturn(regions); + when(region.getFullPath()).thenReturn("/MyRegion"); + + when(region.getParentRegion()).thenReturn(null); + when(region.subregions(true)).thenReturn(subregions); + when(region.subregions(false)).thenReturn(subregions); + when(region.getAttributes()).thenReturn(regionAttributes); + when(regionAttributes.getDataPolicy()).thenReturn(mock(DataPolicy.class)); + when(regionAttributes.getScope()).thenReturn(mock(Scope.class)); + regions.add(region); + getRegionsFunction.execute(functionContext); + } + + private static class TestResultSender implements ResultSender { + + @Override + public void lastResult(final Object lastResult) { + } + + @Override + public void sendResult(final Object oneResult) { + } + + @Override + public void sendException(final Throwable t) { + throw new RuntimeException(t); + } + } + +}
