You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2018/11/15 23:26:42 UTC

[geode] branch develop updated: GEODE-5074 REST dev client should not access or modify internal regions

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 <bs...@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);
   }
 }