You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2020/04/01 16:38:42 UTC
[geode] branch develop updated: GEODE-7938: change dev rest api to
support slashes in key parameters (#4885)
This is an automated email from the ASF dual-hosted git repository.
dschneider 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 0cb54ca GEODE-7938: change dev rest api to support slashes in key parameters (#4885)
0cb54ca is described below
commit 0cb54ca91cd874a42b9b8ec5a9bafdda35e0e6a9
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Wed Apr 1 09:38:18 2020 -0700
GEODE-7938: change dev rest api to support slashes in key parameters (#4885)
* Now uses ** instead of {keys}. This allows keys
to have a slash in them without the client needing
to encode the keys. Since the server no longer decodes
then this should not break any existing use cases.
* The swagger UI does not know what to do with ** but swagger docs were added describing that ** is a comma separated list of keys.
---
.../web/controllers/RestAccessControllerTest.java | 125 ++++++++++++++++++++-
.../web/controllers/AbstractBaseController.java | 14 +++
.../web/controllers/CommonCrudController.java | 12 +-
.../web/controllers/PdxBasedCrudController.java | 30 ++---
4 files changed, 157 insertions(+), 24 deletions(-)
diff --git a/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/RestAccessControllerTest.java b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/RestAccessControllerTest.java
index 383d9cc..73f183d 100644
--- a/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/RestAccessControllerTest.java
+++ b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/RestAccessControllerTest.java
@@ -99,6 +99,8 @@ public class RestAccessControllerTest {
private static final String ORDER_CAS_NEW_JSON = "order-cas-new.json";
private static final String ORDER_CAS_WRONG_OLD_JSON = "order-cas-wrong-old.json";
+ private static final String SLASH = "/";
+
private static Map<String, String> jsonResources = new HashMap<>();
private static RequestPostProcessor POST_PROCESSOR = new StandardRequestPostProcessor();
@@ -106,7 +108,7 @@ public class RestAccessControllerTest {
private MockMvc mockMvc;
private static Region<?, ?> orderRegion;
- private static Region<?, ?> customerRegion;
+ private static Region<String, PdxInstance> customerRegion;
@ClassRule
public static ServerStarterRule rule = new ServerStarterRule()
@@ -180,6 +182,25 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void postEntryWithSlashKey() throws Exception {
+ String key = "1" + SLASH + "2";
+ mockMvc.perform(post("/v1/orders?key=" + key)
+ .content(jsonResources.get(ORDER1_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isCreated())
+ .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
+
+ mockMvc.perform(post("/v1/orders?key=" + key)
+ .content(jsonResources.get(ORDER1_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isConflict());
+
+ Order order = (Order) ((PdxInstance) orderRegion.get(key)).getObject();
+ assertThat(order).as("order should not be null").isNotNull();
+ }
+
+ @Test
+ @WithMockUser
public void postEntryWithJsonArrayOfOrders() throws Exception {
mockMvc.perform(post("/v1/orders?key=1")
.content(jsonResources.get(ORDER1_ARRAY_JSON))
@@ -200,6 +221,27 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void postEntryWithSlashKeysAndJsonArrayOfOrders() throws Exception {
+ String key = "1" + SLASH + "2";
+ mockMvc.perform(post("/v1/orders?key=" + key)
+ .content(jsonResources.get(ORDER1_ARRAY_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isCreated())
+ .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
+
+ mockMvc.perform(post("/v1/orders?key=" + key)
+ .content(jsonResources.get(ORDER1_ARRAY_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isConflict());
+
+ @SuppressWarnings("unchecked")
+ List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get(key);
+ Order order = (Order) entries.get(0).getObject();
+ assertThat(order).as("order should not be null").isNotNull();
+ }
+
+ @Test
+ @WithMockUser
public void failPostEntryWithInvalidJson() throws Exception {
mockMvc.perform(post("/v1/orders?key=1")
.content(jsonResources.get(MALFORMED_JSON))
@@ -271,6 +313,23 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void putEntryWithSlashKey() throws Exception {
+ String key = "1" + SLASH + "2";
+ mockMvc.perform(put("/v1/orders/" + key)
+ .content(jsonResources.get(ORDER2_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
+
+ mockMvc.perform(put("/v1/orders/" + key)
+ .content(jsonResources.get(ORDER2_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
+ }
+
+ @Test
+ @WithMockUser
public void failPutEntryWithInvalidJson() throws Exception {
mockMvc.perform(put("/v1/orders/1")
.content(jsonResources.get(MALFORMED_JSON))
@@ -306,13 +365,47 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
public void putAll() throws Exception {
+ StringBuilder keysBuilder = new StringBuilder();
+ for (int i = 1; i < 60; i++) {
+ keysBuilder.append(i).append(',');
+ }
+ keysBuilder.append(60);
+ String keys = keysBuilder.toString();
mockMvc.perform(
- put("/v1/customers/1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60")
+ put("/v1/customers/" + keys)
.content(jsonResources.get(CUSTOMER_LIST_JSON))
.with(POST_PROCESSOR))
.andExpect(status().isOk())
- .andExpect(header().string("Location", BASE_URL
- + "/customers/1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60"));
+ .andExpect(header().string("Location", BASE_URL + "/customers/" + keys));
+ assertThat(customerRegion.size()).isEqualTo(60);
+ for (int i = 1; i <= 60; i++) {
+ PdxInstance customer = customerRegion.get(String.valueOf(i));
+ assertThat(customer.getField("customerId").toString())
+ .isEqualTo(Integer.valueOf(100 + i).toString());
+ }
+ }
+
+ @Test
+ @WithMockUser
+ public void putAllWithSlashes() throws Exception {
+ StringBuilder keysBuilder = new StringBuilder();
+ for (int i = 1; i < 60; i++) {
+ keysBuilder.append(i).append(SLASH).append(',');
+ }
+ keysBuilder.append(60).append(SLASH);
+ String keys = keysBuilder.toString();
+ mockMvc.perform(
+ put("/v1/customers/" + keys)
+ .content(jsonResources.get(CUSTOMER_LIST_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Location", BASE_URL + "/customers/" + keys));
+ assertThat(customerRegion.size()).isEqualTo(60);
+ for (int i = 1; i <= 60; i++) {
+ PdxInstance customer = customerRegion.get(String.valueOf(i) + SLASH);
+ assertThat(customer.getField("customerId").toString())
+ .isEqualTo(Integer.valueOf(100 + i).toString());
+ }
}
@Test
@@ -590,7 +683,7 @@ public class RestAccessControllerTest {
@WithMockUser
public void getSpecificKeys() throws Exception {
putAll();
- mockMvc.perform(get("/v1/customers/1,2,3,4,5")
+ mockMvc.perform(get("/v1/customers/1,2,3,4,5?ignoreMissingKey=false")
.with(POST_PROCESSOR))
.andExpect(status().isOk())
.andExpect(
@@ -599,6 +692,18 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void getSpecificKeysWithSlashes() throws Exception {
+ putAllWithSlashes();
+ mockMvc.perform(get("/v1/customers/1" + SLASH + ",2" + SLASH + ",3" + SLASH
+ + ",4" + SLASH + ",5" + SLASH)
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(
+ jsonPath("$.customers[*].customerId", containsInAnyOrder(101, 102, 103, 104, 105)));
+ }
+
+ @Test
+ @WithMockUser
public void getSpecificKeysFromUnknownRegion() throws Exception {
mockMvc.perform(get("/v1/unknown/1,2,3,4,5")
.with(POST_PROCESSOR))
@@ -635,6 +740,16 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void deleteMultipleKeysWithSlashes() throws Exception {
+ putAllWithSlashes();
+ mockMvc.perform(delete("/v1/customers/1" + SLASH + ",2" + SLASH + ",3"
+ + SLASH + ",4" + SLASH + ",5" + SLASH)
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk());
+ }
+
+ @Test
+ @WithMockUser
public void deleteAllKeys() throws Exception {
putAll();
mockMvc.perform(delete("/v1/customers")
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 7bc1bb0..3377063 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
@@ -28,6 +28,8 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
+import javax.servlet.http.HttpServletRequest;
+
import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.JsonParser;
@@ -972,4 +974,16 @@ public abstract class AbstractBaseController implements InitializingBean {
targetedMembers.add(cache.getDistributedSystem().getDistributedMember());
return targetedMembers;
}
+
+ protected String[] parseKeys(HttpServletRequest request, String region) {
+ String uri = request.getRequestURI();
+ int regionIndex = uri.indexOf("/" + region + "/");
+ if (regionIndex == -1) {
+ throw new IllegalStateException(
+ String.format("Could not find the region (%1$s) in the URI (%2$s)", region, uri));
+ }
+ int keysIndex = regionIndex + region.length() + 2;
+ String keysString = uri.substring(keysIndex);
+ return keysString.split(",");
+ }
}
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 19fa789..4d85865 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
@@ -19,6 +19,8 @@ import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import javax.servlet.http.HttpServletRequest;
+
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiResponse;
import io.swagger.annotations.ApiResponses;
@@ -113,21 +115,21 @@ public abstract class CommonCrudController extends AbstractBaseController {
* Delete data for single key or specific keys in region
*
* @param region gemfire region
- * @param keys for which data is requested
* @return JSON document containing result
*/
- @RequestMapping(method = RequestMethod.DELETE, value = "/{region}/{keys}",
+ @RequestMapping(method = RequestMethod.DELETE, value = "/{region}/**",
produces = {APPLICATION_JSON_UTF8_VALUE})
@ApiOperation(value = "delete data for key(s)",
- notes = "Delete data for single key or specific keys in region")
+ notes = "Delete data for one or more keys in a region. The keys, ** in the endpoint, are a comma separated list.")
@ApiResponses({@ApiResponse(code = 200, message = "OK"),
@ApiResponse(code = 401, message = "Invalid Username or Password."),
@ApiResponse(code = 403, message = "Insufficient privileges for operation."),
@ApiResponse(code = 404, message = "Region or key(s) does not exist"),
@ApiResponse(code = 500, message = "GemFire throws an error or exception")})
- @PreAuthorize("@securityService.authorize('WRITE', #region, #keys)")
public ResponseEntity<?> delete(@PathVariable("region") String region,
- @PathVariable("keys") final String[] keys) {
+ HttpServletRequest request) {
+ String[] keys = parseKeys(request, region);
+ securityService.authorize("WRITE", region, keys);
logger.debug("Delete data for key {} on region {}", ArrayUtils.toString((Object[]) keys),
region);
diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
index 3385e36..ce2ddd4 100644
--- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
+++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/PdxBasedCrudController.java
@@ -18,6 +18,8 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import javax.servlet.http.HttpServletRequest;
+
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiResponse;
@@ -196,23 +198,23 @@ public class PdxBasedCrudController extends CommonCrudController {
* Reading data for set of keys
*
* @param region gemfire region name
- * @param keys string containing comma separated keys
* @return JSON document
*/
- @RequestMapping(method = RequestMethod.GET, value = "/{region}/{keys}",
+ @RequestMapping(method = RequestMethod.GET, value = "/{region}/**",
produces = APPLICATION_JSON_UTF8_VALUE)
@ApiOperation(value = "read data for specific keys",
- notes = "Read data for specific set of keys in region.")
+ notes = "Read data for specific set of keys in a region. The keys, ** in the endpoint, are a comma separated list.")
@ApiResponses({@ApiResponse(code = 200, message = "OK."),
@ApiResponse(code = 400, message = "Bad Request."),
@ApiResponse(code = 401, message = "Invalid Username or Password."),
@ApiResponse(code = 403, message = "Insufficient privileges for operation."),
@ApiResponse(code = 404, message = "Region does not exist."),
@ApiResponse(code = 500, message = "GemFire throws an error or exception.")})
- @PreAuthorize("@securityService.authorize('READ', #region, #keys)")
public ResponseEntity<?> read(@PathVariable("region") String region,
- @PathVariable("keys") final String[] keys,
- @RequestParam(value = "ignoreMissingKey", required = false) final String ignoreMissingKey) {
+ @RequestParam(value = "ignoreMissingKey", required = false) final String ignoreMissingKey,
+ HttpServletRequest request) {
+ String[] keys = parseKeys(request, region);
+ securityService.authorize("READ", region, keys);
logger.debug("Reading data for keys ({}) in Region ({})", ArrayUtils.toString(keys), region);
final HttpHeaders headers = new HttpHeaders();
@@ -272,18 +274,18 @@ public class PdxBasedCrudController extends CommonCrudController {
* Update data for a key or set of keys
*
* @param region gemfire data region
- * @param keys keys for which update operation is requested
* @param opValue type of update (put, replace, cas etc)
* @param json new data for the key(s)
* @return JSON document
*/
- @RequestMapping(method = RequestMethod.PUT, value = "/{region}/{keys}",
+ @RequestMapping(method = RequestMethod.PUT, value = "/{region}/**",
consumes = {APPLICATION_JSON_UTF8_VALUE}, produces = {
APPLICATION_JSON_UTF8_VALUE})
@ApiOperation(value = "update data for key",
- notes = "Update or insert (put) data for key in region."
- + "op=REPLACE, update (replace) data with key if and only if the key exists in region"
- + "op=CAS update (compare-and-set) value having key with a new value if and only if the \"@old\" value sent matches the current value for the key in region")
+ notes = "Update or insert (put) data for keys in a region."
+ + " The keys, ** in the endpoint, are a comma separated list."
+ + " If op=REPLACE, update (replace) data with key if and only if the key exists in the region."
+ + " If op=CAS update (compare-and-set) value having key with a new value if and only if the \"@old\" value sent matches the current value for the key in the region.")
@ApiResponses({@ApiResponse(code = 200, message = "OK."),
@ApiResponse(code = 400, message = "Bad Request."),
@ApiResponse(code = 401, message = "Invalid Username or Password."),
@@ -293,11 +295,11 @@ public class PdxBasedCrudController extends CommonCrudController {
@ApiResponse(code = 409,
message = "For CAS, @old value does not match to the current value in region"),
@ApiResponse(code = 500, message = "GemFire throws an error or exception.")})
- @PreAuthorize("@securityService.authorize('WRITE', #region, #keys)")
public ResponseEntity<?> update(@PathVariable("region") String region,
- @PathVariable("keys") final String[] keys,
@RequestParam(value = "op", defaultValue = "PUT") final String opValue,
- @RequestBody final String json) {
+ @RequestBody final String json, HttpServletRequest request) {
+ String[] keys = parseKeys(request, region);
+ securityService.authorize("WRITE", region, keys);
logger.debug("updating key(s) for region ({}) ", region);
region = decode(region);