This is an automated email from the ASF dual-hosted git repository. bschuchardt 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 320b191 GEODE-5074 REST dev client should not access or modify internal regions 320b191 is described below commit 320b191c8286bb52be9cf93ccdbacd3e0a9981df Author: Bruce Schuchardt <bschucha...@pivotal.io> AuthorDate: Thu Nov 15 15:24:05 2018 -0800 GEODE-5074 REST dev client should not access or modify internal regions The Developer REST API was accessing a raw cache for all operations. I've switched it to use a filtered InternalCacheForClientAccess cache instead. I added a new method to the filtered cach to allow the REST code to get its Query Store region. --- .../geode/rest/internal/web/RestServersIntegrationTest.java | 7 +++++++ .../geode/internal/cache/InternalCacheForClientAccess.java | 11 +++++++++++ .../rest/internal/web/controllers/AbstractBaseController.java | 7 ++++--- .../rest/internal/web/controllers/CommonCrudController.java | 9 ++++++++- .../rest/internal/web/controllers/QueryAccessController.java | 2 +- .../rest/internal/web/controllers/support/CacheProvider.java | 4 ++-- .../internal/web/controllers/support/CacheProviderImpl.java | 9 +++++++-- 7 files changed, 40 insertions(+), 9 deletions(-) diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestServersIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestServersIntegrationTest.java index 25d5cb8..1804ad7 100644 --- a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestServersIntegrationTest.java +++ b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestServersIntegrationTest.java @@ -62,6 +62,13 @@ public class RestServersIntegrationTest { } @Test + public void testGetOnInternalRegion() { + assertResponse( + restClient.doGet("/__PR", null, null)) + .hasStatusCode(HttpStatus.SC_NOT_FOUND); + } + + @Test public void testServerStartedOnDefaultPort() throws Exception { JsonNode jsonArray = assertResponse(restClient.doGet("/servers", null, null)) .hasStatusCode(HttpStatus.SC_OK).getJsonObject(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java index f9ca77e..0b5bc81 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/InternalCacheForClientAccess.java @@ -137,6 +137,17 @@ public class InternalCacheForClientAccess implements InternalCache { return result; } + /** + * Server-side code using an InternalCacheForClientAccess may need to + * get an Internal Region not normally accesible and may use this method to + * do so. The REST API, for instance, needs to get at a Query store + * region that is not otherwise accessible through the getRegion methods. + */ + public <K, V> Region<K, V> getInternalRegion(String path) { + Region<K, V> result = delegate.getRegion(path); + return result; + } + @Override public Region getRegion(String path, boolean returnDestroyedRegion) { Region result = delegate.getRegion(path, returnDestroyedRegion); diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java index 568a7b2..9cac156 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java @@ -62,6 +62,7 @@ import org.apache.geode.distributed.LeaseExpiredException; import org.apache.geode.distributed.internal.ClusterDistributionManager; import org.apache.geode.distributed.internal.membership.InternalDistributedMember; import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.internal.cache.InternalCacheForClientAccess; import org.apache.geode.internal.logging.LogService; import org.apache.geode.pdx.JSONFormatter; import org.apache.geode.pdx.JSONFormatterException; @@ -109,8 +110,8 @@ public abstract class AbstractBaseController implements InitializingBean { JSONUtils.setObjectMapper(objectMapper); } - protected InternalCache getCache() { - InternalCache cache = cacheProvider.getInternalCache(); + protected InternalCacheForClientAccess getCache() { + InternalCacheForClientAccess cache = cacheProvider.getCache(); Assert.state(cache != null, "The Gemfire Cache reference was not properly initialized"); return cache; } @@ -402,7 +403,7 @@ public abstract class AbstractBaseController implements InitializingBean { } Region<String, String> getQueryStore(final String namePath) { - return ValidationUtils.returnValueThrowOnNull(getCache().<String, String>getRegion(namePath), + return ValidationUtils.returnValueThrowOnNull(getCache().getInternalRegion(namePath), new GemfireRestException(String.format("Query store does not exist!", namePath))); } diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java index c5de53b..52737b3 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/CommonCrudController.java @@ -15,6 +15,7 @@ package org.apache.geode.rest.internal.web.controllers; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -38,6 +39,7 @@ import org.apache.geode.cache.execute.Execution; import org.apache.geode.cache.execute.FunctionException; import org.apache.geode.cache.execute.FunctionService; import org.apache.geode.cache.execute.ResultCollector; +import org.apache.geode.internal.cache.InternalRegion; import org.apache.geode.internal.cache.execute.util.FindRestEnabledServersFunction; import org.apache.geode.internal.logging.LogService; import org.apache.geode.rest.internal.web.controllers.support.RestServersResultCollector; @@ -73,7 +75,12 @@ public abstract class CommonCrudController extends AbstractBaseController { logger.debug("Listing all resources (Regions) in Geode..."); final HttpHeaders headers = new HttpHeaders(); headers.setLocation(toUri()); - final Set<Region<?, ?>> regions = getCache().rootRegions(); + final Set<Region<?, ?>> regions = new HashSet<>(); + for (InternalRegion region : getCache().getApplicationRegions()) { + if (region instanceof Region) { + regions.add(region); + } + } ; String listRegionsAsJson = JSONUtils.formulateJsonForListRegions(regions, "regions"); return new ResponseEntity<>(listRegionsAsJson, headers, HttpStatus.OK); } diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java index 18d0f81..a8c7038 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java @@ -244,7 +244,7 @@ public class QueryAccessController extends AbstractBaseController { Query compiledQuery = compiledQueries.get(queryId); if (compiledQuery == null) { // This is first time the query is seen by this server. - final String oql = getValue(PARAMETERIZED_QUERIES_REGION, queryId, false); + final String oql = getQueryStore(PARAMETERIZED_QUERIES_REGION).get(queryId); ValidationUtils.returnValueThrowOnNull(oql, new ResourceNotFoundException( String.format("No Query with ID (%1$s) was found!", queryId))); diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProvider.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProvider.java index 19d2755..730f44b 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProvider.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProvider.java @@ -14,9 +14,9 @@ */ package org.apache.geode.rest.internal.web.controllers.support; -import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.internal.cache.InternalCacheForClientAccess; public interface CacheProvider { - InternalCache getInternalCache(); + InternalCacheForClientAccess getCache(); } diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProviderImpl.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProviderImpl.java index 8c00923..e3536a9 100644 --- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProviderImpl.java +++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/CacheProviderImpl.java @@ -18,12 +18,17 @@ import org.springframework.stereotype.Component; import org.apache.geode.internal.cache.GemFireCacheImpl; import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.internal.cache.InternalCacheForClientAccess; @Component("cacheProvider") public class CacheProviderImpl implements CacheProvider { @Override - public InternalCache getInternalCache() { - return GemFireCacheImpl.getExisting(); + public InternalCacheForClientAccess getCache() { + InternalCache result = GemFireCacheImpl.getExisting(); + if (result == null) { + return null; + } + return new InternalCacheForClientAccess(result); } }