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/08 15:13:26 UTC
[geode] branch develop updated: GEODE-7938: added keys query param
to GET, PUT, and DELETE /{region} endpoints (#4901)
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 6263f1f GEODE-7938: added keys query param to GET, PUT, and DELETE /{region} endpoints (#4901)
6263f1f is described below
commit 6263f1f9a4782433d013c6b759bda8c84216b595
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Wed Apr 8 08:13:01 2020 -0700
GEODE-7938: added keys query param to GET, PUT, and DELETE /{region} endpoints (#4901)
* added keys query param to GET, PUT, and DELETE /{region} endpoints
* when getting multiple keys, results are now ordered as the request was ordered
* the code that tests for missing keys now uses the getAll result instead of sending multiple messages and having a race in which key is deleted after the old check but before getAll
* Added endpoint for PUT {region}?keys=encodedKey&op=CREATE. The CREATE is a new op.
This allows users to create entries that have an encodedKey. Before they could only create using POST and its key was never decoded.
---
.../web/controllers/RestAccessControllerTest.java | 395 ++++++++++++++++++---
.../web/controllers/AbstractBaseController.java | 85 +++--
.../web/controllers/CommonCrudController.java | 45 ++-
.../web/controllers/PdxBasedCrudController.java | 241 ++++++++++---
.../internal/web/controllers/support/UpdateOp.java | 2 +-
5 files changed, 608 insertions(+), 160 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 73f183d..676a0fa 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
@@ -16,9 +16,12 @@ package org.apache.geode.rest.internal.web.controllers;
import static org.apache.geode.test.matchers.JsonEquivalence.jsonEquals;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.startsWith;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.head;
@@ -29,7 +32,10 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+import java.io.UnsupportedEncodingException;
import java.net.URL;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.HashMap;
@@ -100,6 +106,8 @@ public class RestAccessControllerTest {
private static final String ORDER_CAS_WRONG_OLD_JSON = "order-cas-wrong-old.json";
private static final String SLASH = "/";
+ private static final String KEY_PREFIX = "/?+ @&./";
+ private static final String KEY_SUFFIX = "/?+ @&./";
private static Map<String, String> jsonResources = new HashMap<>();
@@ -110,6 +118,30 @@ public class RestAccessControllerTest {
private static Region<?, ?> orderRegion;
private static Region<String, PdxInstance> customerRegion;
+ private static String createKey(int keyNumber) {
+ return KEY_PREFIX + "KEY" + Integer.toString(keyNumber) + KEY_SUFFIX;
+ }
+
+ private static String createEncodedKey(int keyNumber) {
+ return encodeKey(createKey(keyNumber));
+ }
+
+ private static String encodeKey(String key) {
+ try {
+ return URLEncoder.encode(key, "UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ private static String decodeKey(String encodedKey) {
+ try {
+ return URLDecoder.decode(encodedKey, "UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
@ClassRule
public static ServerStarterRule rule = new ServerStarterRule()
.withProperty("log-level", "warn")
@@ -169,12 +201,14 @@ public class RestAccessControllerTest {
.content(jsonResources.get(ORDER1_JSON))
.with(POST_PROCESSOR))
.andExpect(status().isCreated())
+ .andExpect(content().string(""))
.andExpect(header().string("Location", BASE_URL + "/orders/1"));
mockMvc.perform(post("/v1/orders?key=1")
.content(jsonResources.get(ORDER1_JSON))
.with(POST_PROCESSOR))
- .andExpect(status().isConflict());
+ .andExpect(status().isConflict())
+ .andExpect(content().json(jsonResources.get(ORDER1_JSON)));
Order order = (Order) ((PdxInstance) orderRegion.get("1")).getObject();
assertThat(order).as("order should not be null").isNotNull();
@@ -182,20 +216,24 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- public void postEntryWithSlashKey() throws Exception {
- String key = "1" + SLASH + "2";
- mockMvc.perform(post("/v1/orders?key=" + key)
+ public void createEntryWithEncodedKey() throws Exception {
+ String decodedKey = createKey(1);
+ String encodedKey = encodeKey(decodedKey);
+ mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
.content(jsonResources.get(ORDER1_JSON))
.with(POST_PROCESSOR))
.andExpect(status().isCreated())
- .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
+ .andExpect(content().string(""))
+ .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
- mockMvc.perform(post("/v1/orders?key=" + key)
+ mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
.content(jsonResources.get(ORDER1_JSON))
.with(POST_PROCESSOR))
- .andExpect(status().isConflict());
+ .andExpect(status().isConflict())
+ .andExpect(content().json(jsonResources.get(ORDER1_JSON)))
+ .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
- Order order = (Order) ((PdxInstance) orderRegion.get(key)).getObject();
+ Order order = (Order) ((PdxInstance) orderRegion.get(decodedKey)).getObject();
assertThat(order).as("order should not be null").isNotNull();
}
@@ -206,12 +244,15 @@ public class RestAccessControllerTest {
.content(jsonResources.get(ORDER1_ARRAY_JSON))
.with(POST_PROCESSOR))
.andExpect(status().isCreated())
+ .andExpect(content().string(""))
.andExpect(header().string("Location", BASE_URL + "/orders/1"));
mockMvc.perform(post("/v1/orders?key=1")
.content(jsonResources.get(ORDER1_ARRAY_JSON))
.with(POST_PROCESSOR))
- .andExpect(status().isConflict());
+ .andExpect(status().isConflict())
+ .andExpect(content().json(jsonResources.get(ORDER1_ARRAY_JSON)));
+
@SuppressWarnings("unchecked")
List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get("1");
@@ -221,21 +262,25 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- public void postEntryWithSlashKeysAndJsonArrayOfOrders() throws Exception {
- String key = "1" + SLASH + "2";
- mockMvc.perform(post("/v1/orders?key=" + key)
+ public void createEntryWithJsonArrayOfOrdersWithEncodedKey() throws Exception {
+ String decodedKey = createKey(1);
+ String encodedKey = encodeKey(decodedKey);
+ mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
.content(jsonResources.get(ORDER1_ARRAY_JSON))
.with(POST_PROCESSOR))
.andExpect(status().isCreated())
- .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
+ .andExpect(content().string(""))
+ .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
- mockMvc.perform(post("/v1/orders?key=" + key)
+ mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
.content(jsonResources.get(ORDER1_ARRAY_JSON))
.with(POST_PROCESSOR))
- .andExpect(status().isConflict());
+ .andExpect(status().isConflict())
+ .andExpect(content().json(jsonResources.get(ORDER1_ARRAY_JSON)))
+ .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
@SuppressWarnings("unchecked")
- List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get(key);
+ List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get(decodedKey);
Order order = (Order) entries.get(0).getObject();
assertThat(order).as("order should not be null").isNotNull();
}
@@ -313,23 +358,6 @@ 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))
@@ -377,7 +405,7 @@ public class RestAccessControllerTest {
.with(POST_PROCESSOR))
.andExpect(status().isOk())
.andExpect(header().string("Location", BASE_URL + "/customers/" + keys));
- assertThat(customerRegion.size()).isEqualTo(60);
+ assertThat(customerRegion).hasSize(60);
for (int i = 1; i <= 60; i++) {
PdxInstance customer = customerRegion.get(String.valueOf(i));
assertThat(customer.getField("customerId").toString())
@@ -387,22 +415,22 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- public void putAllWithSlashes() throws Exception {
+ public void putAllWithQueryParam() throws Exception {
StringBuilder keysBuilder = new StringBuilder();
for (int i = 1; i < 60; i++) {
- keysBuilder.append(i).append(SLASH).append(',');
+ keysBuilder.append(i).append(',');
}
- keysBuilder.append(60).append(SLASH);
+ keysBuilder.append(60);
String keys = keysBuilder.toString();
mockMvc.perform(
- put("/v1/customers/" + keys)
+ put("/v1/customers?keys=" + 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);
+ .andExpect(header().string("Location", BASE_URL + "/customers?keys=" + keys));
+ assertThat(customerRegion).hasSize(60);
for (int i = 1; i <= 60; i++) {
- PdxInstance customer = customerRegion.get(String.valueOf(i) + SLASH);
+ PdxInstance customer = customerRegion.get(String.valueOf(i));
assertThat(customer.getField("customerId").toString())
.isEqualTo(Integer.valueOf(100 + i).toString());
}
@@ -410,6 +438,50 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void putMultipleEncodedKeys() throws Exception {
+ StringBuilder keysBuilder = new StringBuilder();
+ for (int i = 1; i < 60; i++) {
+ keysBuilder.append(createEncodedKey(i)).append(',');
+ }
+ keysBuilder.append(createEncodedKey(60));
+ String keys = keysBuilder.toString();
+ mockMvc.perform(
+ put("/v1/customers?keys=" + keys)
+ .content(jsonResources.get(CUSTOMER_LIST_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Location", BASE_URL + "/customers?keys=" + keys));
+ assertThat(customerRegion).hasSize(60);
+ for (int i = 1; i <= 60; i++) {
+ PdxInstance customer = customerRegion.get(createKey(i));
+ assertThat(customer.getField("customerId").toString())
+ .isEqualTo(Integer.valueOf(100 + i).toString());
+ }
+ }
+
+ @Test
+ @WithMockUser
+ public void putSingleEncodedKey() throws Exception {
+ String decodedKey = createKey(32);
+ String encodedKey = encodeKey(decodedKey);
+
+ mockMvc.perform(
+ put("/v1/orders?keys=" + encodedKey)
+ .content(jsonResources.get(ORDER2_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
+
+ assertThat(orderRegion).hasSize(1);
+ PdxInstance customer = customerRegion.get(createKey(32));
+ assertThat(orderRegion.containsKey(decodedKey)).isTrue();
+ Order order = (Order) ((PdxInstance) orderRegion.get(decodedKey)).getObject();
+ assertThat(order.getPurchaseOrderNo()).isEqualTo(112);
+ assertThat(order.getCustomerId()).isEqualTo(102);
+ }
+
+ @Test
+ @WithMockUser
public void failPutAllWithInvalidJson() throws Exception {
mockMvc.perform(put("/v1/customers/1,2,3,4")
.content(jsonResources.get(MALFORMED_JSON))
@@ -498,6 +570,30 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void failPutWithInvalidOp() throws Exception {
+ mockMvc.perform(put("/v1/orders/1?op=BOGUS")
+ .content(jsonResources.get(ORDER1_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isBadRequest())
+ .andExpect(
+ jsonPath("$.cause", is(
+ "The op parameter (BOGUS) is not valid. Valid values are PUT, REPLACE, or CAS.")));
+ }
+
+ @Test
+ @WithMockUser
+ public void failPutWithKeyParamWithInvalidOp() throws Exception {
+ mockMvc.perform(put("/v1/orders?op=BOGUS&keys=1")
+ .content(jsonResources.get(ORDER1_JSON))
+ .with(POST_PROCESSOR))
+ .andExpect(status().isBadRequest())
+ .andExpect(
+ jsonPath("$.cause", is(
+ "The op parameter (BOGUS) is not valid. Valid values are PUT, CREATE, REPLACE, or CAS.")));
+ }
+
+ @Test
+ @WithMockUser
public void failPutWithReplaceFailsOnEmptyRegion() throws Exception {
mockMvc.perform(put("/v1/empty/10?op=REPLACE")
.content(jsonResources.get(ORDER1_JSON))
@@ -620,11 +716,13 @@ public class RestAccessControllerTest {
@SuppressWarnings("unchecked")
@Test
@WithMockUser
- public void getCustomers() throws Exception {
+ public void getAllCustomers() throws Exception {
putAll();
mockMvc.perform(get("/v1/customers?limit=ALL")
.with(POST_PROCESSOR))
.andExpect(status().isOk())
+ .andExpect(
+ header().string("Content-Location", startsWith(BASE_URL + "/customers/")))
.andExpect(jsonPath("$.customers", jsonEquals(jsonResources.get(CUSTOMER_LIST_JSON))));
}
@@ -646,11 +744,47 @@ public class RestAccessControllerTest {
mockMvc.perform(get("/v1/customers?limit=10")
.with(POST_PROCESSOR))
.andExpect(status().isOk())
+ .andExpect(
+ header().string("Content-Location", startsWith(BASE_URL + "/customers/")))
.andExpect(jsonPath("$.customers.length()", is(10)));
}
@Test
@WithMockUser
+ public void getCustomersWithLimitOverRegionSize() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?limit=70")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(
+ header().string("Content-Location", startsWith(BASE_URL + "/customers/")))
+ .andExpect(jsonPath("$.customers.length()", is(60)));
+ }
+
+ @Test
+ @WithMockUser
+ public void getCustomersWithBogusLimitFails() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?limit=bogus")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isBadRequest())
+ .andExpect(
+ jsonPath("$.cause", is("limit param (bogus) is not valid!")));
+ }
+
+ @Test
+ @WithMockUser
+ public void getCustomersWithNegativeLimitFails() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?limit=-2")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isBadRequest())
+ .andExpect(
+ jsonPath("$.cause", is("Negative limit param (-2) is not valid!")));
+ }
+
+ @Test
+ @WithMockUser
public void getAllKeys() throws Exception {
putAll();
mockMvc.perform(get("/v1/customers/keys")
@@ -681,25 +815,117 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
+ public void getSpecificKey() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers/1")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers/1"))
+ .andExpect(
+ jsonPath("$.customerId", equalTo(101)));
+ }
+
+ @Test
+ @WithMockUser
public void getSpecificKeys() throws Exception {
putAll();
mockMvc.perform(get("/v1/customers/1,2,3,4,5?ignoreMissingKey=false")
.with(POST_PROCESSOR))
.andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers/1,2,3,4,5"))
.andExpect(
- jsonPath("$.customers[*].customerId", containsInAnyOrder(101, 102, 103, 104, 105)));
+ jsonPath("$.customers[*].customerId", contains(101, 102, 103, 104, 105)));
}
@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))
+ public void getSpecificKeyUsingQueryParam() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?keys=1")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers?keys=1"))
+ .andExpect(
+ jsonPath("$.customerId", equalTo(101)));
+ }
+
+ @Test
+ @WithMockUser
+ public void getSpecificKeysUsingQueryParam() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?keys=5,4,3,2,1&ignoreMissingKey=false")
+ .with(POST_PROCESSOR))
.andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers?keys=5,4,3,2,1"))
+ .andExpect(
+ jsonPath("$.customers[*].customerId", contains(105, 104, 103, 102, 101)));
+ }
+
+ @Test
+ @WithMockUser
+ public void getSpecificNonExistentKeyFails() throws Exception {
+ mockMvc.perform(get("/v1/customers?keys=nonExistentKey")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isNotFound())
.andExpect(
- jsonPath("$.customers[*].customerId", containsInAnyOrder(101, 102, 103, 104, 105)));
+ jsonPath("$.cause",
+ is("Key (nonExistentKey) does not exist for region (customers) in cache!")));
+ }
+
+ @Test
+ @WithMockUser
+ public void getSpecificNonExistentKeyWithIgnoreMissingKeyStillFails() throws Exception {
+ // ignoreMissingKey=true ignored when single key is given
+ mockMvc.perform(get("/v1/customers?keys=nonExistentKey&ignoreMissingKey=true")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isNotFound())
+ .andExpect(
+ jsonPath("$.cause",
+ is("Key (nonExistentKey) does not exist for region (customers) in cache!")));
+ }
+
+ @Test
+ @WithMockUser
+ public void getSpecificKeysUsingQueryParamWithNonExistentKey() throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?keys=1,doesNotExist&ignoreMissingKey=true")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers?keys=1,doesNotExist"))
+ .andExpect(content().string("{\n"
+ + " \"customers\" : [ {\n"
+ + " \"@type\" : \"org.apache.geode.rest.internal.web.controllers.Customer\",\n"
+ + " \"customerId\" : 101,\n"
+ + " \"firstName\" : \"Vishal\",\n"
+ + " \"lastName\" : \"Roa\"\n"
+ + " }, null ]\n"
+ + "}"));
+ }
+
+ @Test
+ @WithMockUser
+ public void getSpecificKeysUsingQueryParamWithNonExistentKeyFailsWhenNotIgnoring()
+ throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?keys=1,doesNotExist,doesNotExist2&ignoreMissingKey=false")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isBadRequest())
+ .andExpect(
+ jsonPath("$.cause",
+ is("Requested keys (doesNotExist,doesNotExist2) not exist in region (customers)")));
+ }
+
+ @Test
+ @WithMockUser
+ public void getSpecificKeysUsingQueryParamWithNonExistentKeyFailsWithBogusIgnoringValue()
+ throws Exception {
+ putAll();
+ mockMvc.perform(get("/v1/customers?keys=1,doesNotExist,doesNotExist2&ignoreMissingKey=bogus")
+ .with(POST_PROCESSOR))
+ .andExpect(status().isBadRequest())
+ .andExpect(
+ jsonPath("$.cause", is(
+ "ignoreMissingKey param (bogus) is not valid. valid usage is ignoreMissingKey=true!")));
}
@Test
@@ -740,12 +966,71 @@ 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))
+ public void deleteMultipleKeysWithQueryParam() throws Exception {
+ putAll();
+ mockMvc.perform(delete("/v1/customers?keys=2,3,4,5")
+ .with(POST_PROCESSOR))
.andExpect(status().isOk());
+ assertThat(customerRegion).hasSize(60 - 4);
+ assertThat(customerRegion.containsKey("2")).isFalse();
+ assertThat(customerRegion.containsKey("3")).isFalse();
+ assertThat(customerRegion.containsKey("4")).isFalse();
+ assertThat(customerRegion.containsKey("5")).isFalse();
+ }
+
+ @Test
+ @WithMockUser
+ public void getMultipleEncodedKeys() throws Exception {
+ putMultipleEncodedKeys();
+ StringBuilder keyBuilder = new StringBuilder();
+ for (int i = 2; i <= 5; i++) {
+ keyBuilder.append(createEncodedKey(i));
+ if (i != 5) {
+ keyBuilder.append(',');
+ }
+ }
+ String keys = keyBuilder.toString();
+ mockMvc.perform(get("/v1/customers?keys=" + keys)
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers?keys=" + keys))
+ .andExpect(
+ jsonPath("$.customers[*].customerId", contains(102, 103, 104, 105)));
+
+ }
+
+ @Test
+ @WithMockUser
+ public void getSingleEncodedKey() throws Exception {
+ putMultipleEncodedKeys();
+ String keys = createEncodedKey(7);
+ mockMvc.perform(get("/v1/customers?keys=" + keys)
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk())
+ .andExpect(header().string("Content-Location", BASE_URL + "/customers?keys=" + keys))
+ .andExpect(
+ jsonPath("$.customerId", equalTo(107)));
+ }
+
+ @Test
+ @WithMockUser
+ public void deleteMultipleEncodedKeys() throws Exception {
+ putMultipleEncodedKeys();
+ StringBuilder keyBuilder = new StringBuilder();
+ for (int i = 2; i <= 5; i++) {
+ keyBuilder.append(createEncodedKey(i));
+ if (i != 5) {
+ keyBuilder.append(',');
+ }
+ }
+ String keys = keyBuilder.toString();
+ mockMvc.perform(delete("/v1/customers?keys=" + keys)
+ .with(POST_PROCESSOR))
+ .andExpect(status().isOk());
+ assertThat(customerRegion).hasSize(60 - 4);
+ for (int i = 2; i <= 5; i++) {
+ assertThat(customerRegion.containsKey(createKey(i))).isFalse();
+ }
}
@Test
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 3377063..1f8b6ff 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
@@ -18,6 +18,7 @@ import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URLDecoder;
+import java.net.URLEncoder;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -28,8 +29,6 @@ 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;
@@ -131,6 +130,13 @@ public abstract class AbstractBaseController implements InitializingBean {
.pathSegment(pathSegments).build().toUri();
}
+ URI toUriWithKeys(String[] keys, final String... pathSegments) {
+ return ServletUriComponentsBuilder.fromCurrentContextPath().path(getRestApiVersion())
+ .pathSegment(pathSegments)
+ .queryParam("keys", StringUtils.arrayToCommaDelimitedString(keys))
+ .build(true).toUri();
+ }
+
protected abstract String getRestApiVersion();
String validateQuery(String queryInUrl, String queryInBody) {
@@ -141,6 +147,13 @@ public abstract class AbstractBaseController implements InitializingBean {
return (StringUtils.hasText(queryInUrl) ? decode(queryInUrl) : queryInBody);
}
+ String encode(String value) {
+ if (value == null) {
+ throw new GemfireRestException("could not process null value specified in query String");
+ }
+ return encode(value, DEFAULT_ENCODING);
+ }
+
String decode(final String value) {
if (value == null) {
throw new GemfireRestException("could not process null value specified in query String");
@@ -149,6 +162,22 @@ public abstract class AbstractBaseController implements InitializingBean {
return decode(value, DEFAULT_ENCODING);
}
+ String[] decode(String[] values) {
+ String[] result = new String[values.length];
+ for (int i = 0; i < values.length; i++) {
+ result[i] = decode(values[i]);
+ }
+ return result;
+ }
+
+ String[] encode(String[] values) {
+ String[] result = new String[values.length];
+ for (int i = 0; i < values.length; i++) {
+ result[i] = encode(values[i]);
+ }
+ return result;
+ }
+
protected PdxInstance convert(final String json) {
try {
return (StringUtils.hasText(json) ? JSONFormatter.fromJSON(json) : null);
@@ -575,7 +604,7 @@ public abstract class AbstractBaseController implements InitializingBean {
if (StringUtils.hasText(existingKey)) {
newKey = existingKey;
- if (NumberUtils.isNumeric(newKey) && domainObjectId == null) {
+ if (domainObject != null && NumberUtils.isNumeric(newKey) && domainObjectId == null) {
final Long newId = IdentifiableUtils.createId(NumberUtils.parseLong(newKey));
if (newKey.equals(newId.toString())) {
IdentifiableUtils.setId(domainObject, newId);
@@ -601,6 +630,14 @@ public abstract class AbstractBaseController implements InitializingBean {
return newKey;
}
+ private String encode(final String value, final String encoding) {
+ try {
+ return URLEncoder.encode(value, encoding);
+ } catch (UnsupportedEncodingException e) {
+ throw new GemfireRestException("Server has encountered unsupported encoding!");
+ }
+ }
+
private String decode(final String value, final String encoding) {
try {
return URLDecoder.decode(value, encoding);
@@ -623,16 +660,6 @@ public abstract class AbstractBaseController implements InitializingBean {
}
}
- List<String> checkForMultipleKeysExist(String region, String... keys) {
- List<String> unknownKeys = new ArrayList<>();
- for (String key : keys) {
- if (!getRegion(region).containsKey(key)) {
- unknownKeys.add(key);
- }
- }
- return unknownKeys;
- }
-
protected Object[] getKeys(final String regionNamePath, Object[] keys) {
return (!(keys == null || keys.length == 0) ? keys
: getRegion(regionNamePath).keySet().toArray());
@@ -795,7 +822,10 @@ public abstract class AbstractBaseController implements InitializingBean {
}
}
- ResponseEntity<String> updateSingleKey(final String region, final String key, final String json,
+ /**
+ * @return if the opValue is CAS then the existingValue; otherwise null
+ */
+ String updateSingleKey(final String region, final String key, final String json,
final String opValue) {
final JSONTypes jsonType = validateJsonAndFindType(json);
@@ -817,22 +847,15 @@ public abstract class AbstractBaseController implements InitializingBean {
default:
if (JSONTypes.JSON_ARRAY.equals(jsonType)) {
putValue(region, key, convertJsonArrayIntoPdxCollection(json));
- // putValue(region, key, convertJsonIntoPdxCollection(json));
} else {
putValue(region, key, convert(json));
}
}
-
- final HttpHeaders headers = new HttpHeaders();
- headers.setLocation(toUri(region, key));
- return new ResponseEntity<>(existingValue, headers,
- (existingValue == null ? HttpStatus.OK : HttpStatus.CONFLICT));
+ return existingValue;
}
- ResponseEntity<String> updateMultipleKeys(final String region, final String[] keys,
- final String json) {
-
+ void updateMultipleKeys(final String region, final String[] keys, final String json) {
JsonNode jsonArr;
try {
jsonArr = objectMapper.readTree(json);
@@ -864,10 +887,6 @@ public abstract class AbstractBaseController implements InitializingBean {
if (!CollectionUtils.isEmpty(map)) {
putPdxValues(region, map);
}
-
- HttpHeaders headers = new HttpHeaders();
- headers.setLocation(toUri(region, StringUtils.arrayToCommaDelimitedString(keys)));
- return new ResponseEntity<>(headers, HttpStatus.OK);
}
JSONTypes validateJsonAndFindType(String json) {
@@ -974,16 +993,4 @@ 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 4d85865..ec13804 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,8 +19,6 @@ 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;
@@ -32,6 +30,7 @@ import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
+import org.springframework.web.bind.annotation.RequestParam;
import org.apache.geode.cache.LowMemoryException;
import org.apache.geode.cache.Region;
@@ -112,52 +111,64 @@ public abstract class CommonCrudController extends AbstractBaseController {
}
/**
- * Delete data for single key or specific keys in region
+ * Delete data for one or more keys from a region
*
* @param region gemfire region
+ * @param keys comma separated list of keys
* @return JSON document containing result
*/
- @RequestMapping(method = RequestMethod.DELETE, value = "/{region}/**",
+ @RequestMapping(method = RequestMethod.DELETE, value = "/{region}/{keys}",
produces = {APPLICATION_JSON_UTF8_VALUE})
@ApiOperation(value = "delete data for key(s)",
- notes = "Delete data for one or more keys in a region. The keys, ** in the endpoint, are a comma separated list.")
+ notes = "Delete data for one or more keys in a region. Deprecated in favor of /{region}?keys=.")
@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")})
public ResponseEntity<?> delete(@PathVariable("region") String region,
- 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);
-
+ @PathVariable("keys") String[] keys) {
region = decode(region);
+ return deleteRegionKeys(region, keys);
+ }
+ private ResponseEntity<?> deleteRegionKeys(String region, String[] keys) {
+ securityService.authorize("WRITE", region, keys);
+ logger.debug("Delete data for keys {} on region {}", ArrayUtils.toString((Object[]) keys),
+ region);
deleteValues(region, keys);
return new ResponseEntity<>(HttpStatus.OK);
}
/**
- * Delete all data in region
+ * Delete all data in region or just the given keys
*
* @param region gemfire region
+ * @param encodedKeys optional comma separated list of keys
* @return JSON document containing result
*/
@RequestMapping(method = RequestMethod.DELETE, value = "/{region}")
- @ApiOperation(value = "delete all data", notes = "Delete all data in the region")
+ @ApiOperation(value = "delete all data or the specified keys",
+ notes = "Delete all data in the region or just the specified keys.")
@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 does not exist"),
@ApiResponse(code = 500, message = "if GemFire throws an error or exception")})
- @PreAuthorize("@securityService.authorize('DATA', 'WRITE', #region)")
- public ResponseEntity<?> delete(@PathVariable("region") String region) {
- logger.debug("Deleting all data in Region ({})...", region);
-
+ public ResponseEntity<?> deleteAllOrGivenKeys(@PathVariable("region") String region,
+ @RequestParam(value = "keys", required = false) final String[] encodedKeys) {
region = decode(region);
+ if (encodedKeys == null || encodedKeys.length == 0) {
+ return deleteAllRegionData(region);
+ } else {
+ String[] decodedKeys = decode(encodedKeys);
+ return deleteRegionKeys(region, decodedKeys);
+ }
+ }
+ private ResponseEntity<?> deleteAllRegionData(String region) {
+ securityService.authorize("DATA", "WRITE", region);
+ logger.debug("Deleting all data in Region ({})...", region);
deleteValues(region);
return new ResponseEntity<>(HttpStatus.OK);
}
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 ce2ddd4..a6e8c6f 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
@@ -14,6 +14,7 @@
*/
package org.apache.geode.rest.internal.web.controllers;
+import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -41,6 +42,7 @@ import org.apache.geode.logging.internal.log4j.api.LogService;
import org.apache.geode.rest.internal.web.controllers.support.JSONTypes;
import org.apache.geode.rest.internal.web.controllers.support.RegionData;
import org.apache.geode.rest.internal.web.controllers.support.RegionEntryData;
+import org.apache.geode.rest.internal.web.controllers.support.UpdateOp;
import org.apache.geode.rest.internal.web.exception.ResourceNotFoundException;
import org.apache.geode.rest.internal.web.util.ArrayUtils;
@@ -78,7 +80,8 @@ public class PdxBasedCrudController extends CommonCrudController {
*/
@RequestMapping(method = RequestMethod.POST, value = "/{region}",
consumes = APPLICATION_JSON_UTF8_VALUE, produces = {APPLICATION_JSON_UTF8_VALUE})
- @ApiOperation(value = "create entry", notes = "Create (put-if-absent) data in region")
+ @ApiOperation(value = "create entry", notes = "Create (put-if-absent) data in region."
+ + " The key is not decoded so if the key contains special characters use PUT /{region}?keys=EncodedKey&op=CREATE.")
@ApiResponses({@ApiResponse(code = 201, message = "Created."),
@ApiResponse(code = 400,
message = "Data specified (JSON doc) in the request body is invalid."),
@@ -91,12 +94,14 @@ public class PdxBasedCrudController extends CommonCrudController {
public ResponseEntity<?> create(@PathVariable("region") String region,
@RequestParam(value = "key", required = false) String key, @RequestBody final String json) {
key = generateKey(key);
+ region = decode(region);
+ return create(region, key, json, false);
+ }
- logger.debug(
- "Posting (creating/putIfAbsent) JSON document ({}) to Region ({}) with Key ({})...", json,
- region, key);
+ private ResponseEntity<?> create(String region, String key, String json,
+ boolean keyInQueryParam) {
+ logger.debug("Create JSON document ({}) in Region ({}) with Key ({})...", json, region, key);
- region = decode(region);
Object existingPdxObj;
// Check whether the user has supplied single JSON doc or Array of JSON docs
@@ -108,7 +113,11 @@ public class PdxBasedCrudController extends CommonCrudController {
}
final HttpHeaders headers = new HttpHeaders();
- headers.setLocation(toUri(region, key));
+ if (keyInQueryParam) {
+ headers.setLocation(toUriWithKeys(new String[] {encode(key)}, region));
+ } else {
+ headers.setLocation(toUri(region, key));
+ }
if (existingPdxObj != null) {
final RegionEntryData<Object> data = new RegionEntryData<>(region);
@@ -121,30 +130,44 @@ public class PdxBasedCrudController extends CommonCrudController {
}
/**
- * Read all or fixed number of data in a given Region
+ * For the given region either gets all the region's data (with an optional limit),
+ * or gets the region's data for the given keys (optionally ignoring missing keys).
*
* @param region gemfire region name
* @param limit total number of entries requested
+ * @param encodedKeys an optional comma separated list of encoded keys to read
+ * @param ignoreMissingKey if true and reading more than one key then if a key is missing ignore
+ * it
* @return JSON document
*/
@RequestMapping(method = RequestMethod.GET, value = "/{region}",
produces = APPLICATION_JSON_UTF8_VALUE)
- @ApiOperation(value = "read all data for region",
- notes = "Read all data for region. Use limit param to get fixed or limited number of entries.")
+ @ApiOperation(value = "read all data for region or the specified keys",
+ notes = "If reading all data for region then the limit parameter can be used to give the maximum number of values to return."
+ + " If reading specific keys then the ignoreMissingKey parameter can be used to not fail if a key is missing.")
@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('DATA', 'READ', #region)")
public ResponseEntity<?> read(@PathVariable("region") String region,
@RequestParam(value = "limit",
- defaultValue = DEFAULT_GETALL_RESULT_LIMIT) final String limit) {
- logger.debug("Reading all data in Region ({})...", region);
-
+ defaultValue = DEFAULT_GETALL_RESULT_LIMIT) final String limit,
+ @RequestParam(value = "keys", required = false) final String[] encodedKeys,
+ @RequestParam(value = "ignoreMissingKey", required = false) final String ignoreMissingKey) {
region = decode(region);
+ if (encodedKeys == null || encodedKeys.length == 0) {
+ return getAllRegionData(region, limit);
+ } else {
+ String[] decodedKeys = decode(encodedKeys);
+ return getRegionKeys(region, ignoreMissingKey, decodedKeys, true);
+ }
+ }
+ private ResponseEntity<?> getAllRegionData(String region, String limit) {
+ securityService.authorize("DATA", "READ", region);
+ logger.debug("Reading all data in Region ({})...", region);
Map<Object, Object> valueObjs = null;
final RegionData<Object> data = new RegionData<>(region);
@@ -164,7 +187,7 @@ public class PdxBasedCrudController extends CommonCrudController {
if ("ALL".equalsIgnoreCase(limit)) {
data.add(values);
- keyList = StringUtils.collectionToDelimitedString(keys, ",");
+ keyList = StringUtils.collectionToCommaDelimitedString(keys);
} else {
try {
int maxLimit = Integer.parseInt(limit);
@@ -180,7 +203,7 @@ public class PdxBasedCrudController extends CommonCrudController {
}
data.add(values.subList(0, maxLimit));
- keyList = StringUtils.collectionToDelimitedString(keys.subList(0, maxLimit), ",");
+ keyList = StringUtils.collectionToCommaDelimitedString(keys.subList(0, maxLimit));
} catch (NumberFormatException e) {
// limit param is not specified in proper format. set the HTTPHeader
@@ -190,7 +213,7 @@ public class PdxBasedCrudController extends CommonCrudController {
}
}
- headers.set("Content-Location", toUri(region, keyList).toASCIIString());
+ headers.set(HttpHeaders.CONTENT_LOCATION, toUri(region, keyList).toASCIIString());
return new ResponseEntity<RegionData<?>>(data, headers, HttpStatus.OK);
}
@@ -200,26 +223,28 @@ public class PdxBasedCrudController extends CommonCrudController {
* @param region gemfire region name
* @return JSON document
*/
- @RequestMapping(method = RequestMethod.GET, value = "/{region}/**",
+ @RequestMapping(method = RequestMethod.GET, value = "/{region}/{keys}",
produces = APPLICATION_JSON_UTF8_VALUE)
@ApiOperation(value = "read data for specific keys",
- notes = "Read data for specific set of keys in a region. The keys, ** in the endpoint, are a comma separated list.")
+ notes = "Read data for specific set of keys in a region. Deprecated in favor of /{region}?keys=.")
@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.")})
- public ResponseEntity<?> read(@PathVariable("region") String region,
- @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();
+ public ResponseEntity<?> readKeys(@PathVariable("region") String region,
+ @PathVariable("keys") final String[] keys,
+ @RequestParam(value = "ignoreMissingKey", required = false) final String ignoreMissingKey) {
region = decode(region);
+ return getRegionKeys(region, ignoreMissingKey, keys, false);
+ }
+ private ResponseEntity<?> getRegionKeys(String region, String ignoreMissingKey, String[] keys,
+ boolean keysInQueryParam) {
+ logger.debug("Reading data for keys ({}) in Region ({})", ArrayUtils.toString(keys), region);
+ securityService.authorize("READ", region, keys);
+ final HttpHeaders headers = new HttpHeaders();
if (keys.length == 1) {
/* GET op on single key */
Object value = getValue(region, keys[0]);
@@ -230,7 +255,15 @@ public class PdxBasedCrudController extends CommonCrudController {
}
final RegionEntryData<Object> data = new RegionEntryData<>(region);
- headers.set("Content-Location", toUri(region, keys[0]).toASCIIString());
+ URI uri;
+ if (keysInQueryParam) {
+ String[] encodedKeys = encode(keys);
+ String encodedRegion = encode(region);
+ uri = this.toUriWithKeys(encodedKeys, encodedRegion);
+ } else {
+ uri = toUri(region, keys[0]);
+ }
+ headers.set(HttpHeaders.CONTENT_LOCATION, uri.toASCIIString());
data.add(value);
return new ResponseEntity<RegionData<?>>(data, headers, HttpStatus.OK);
@@ -244,28 +277,49 @@ public class PdxBasedCrudController extends CommonCrudController {
return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST);
}
+ final Map<Object, Object> valueObjs = getValues(region, keys);
+ // valueObjs will have as its keys all of "keys".
+ // valueObjs will have a null value if the key did not exist.
+ // So if ignoreMissingKey is false we can use "null" values to detect the missing keys.
if (!("true".equalsIgnoreCase(ignoreMissingKey))) {
- List<String> unknownKeys = checkForMultipleKeysExist(region, keys);
- if (unknownKeys.size() > 0) {
- String unknownKeysAsStr = StringUtils.collectionToDelimitedString(unknownKeys, ",");
- String erroString = String.format("Requested keys (%1$s) not exist in region (%2$s)",
- StringUtils.collectionToDelimitedString(unknownKeys, ","), region);
- return new ResponseEntity<>(convertErrorAsJson(erroString), headers,
+ List<String> unknownKeys = new ArrayList<>();
+ // use "keys" to iterate so that we get the original key ordering from user.
+ for (String key : keys) {
+ if (valueObjs.get(key) == null) {
+ unknownKeys.add(key);
+ }
+ }
+ if (!unknownKeys.isEmpty()) {
+ String unknownKeysAsStr = StringUtils.collectionToCommaDelimitedString(unknownKeys);
+ String errorString = String.format("Requested keys (%1$s) not exist in region (%2$s)",
+ unknownKeysAsStr, region);
+ return new ResponseEntity<>(convertErrorAsJson(errorString), headers,
HttpStatus.BAD_REQUEST);
}
}
- final Map<Object, Object> valueObjs = getValues(region, keys);
-
- // Do we need to remove null values from Map..?
- // To Remove null value entries from map.
- // valueObjs.values().removeAll(Collections.singleton(null));
+ // The dev rest api was already released with null values being returned
+ // for non-existent keys.
+ // Order the keys in the result after the array of keys given to this method.
+ // Previous code returned them in random order which made the result harder to test and use.
+
+ URI uri;
+ if (keysInQueryParam) {
+ String[] encodedKeys = encode(keys);
+ String encodedRegion = encode(region);
+ uri = this.toUriWithKeys(encodedKeys, encodedRegion);
+ } else {
+ String keyList = StringUtils.arrayToCommaDelimitedString(keys);
+ uri = toUri(region, keyList);
+ }
- // currently we are not removing keys having value null from the result.
- String keyList = StringUtils.collectionToDelimitedString(valueObjs.keySet(), ",");
- headers.set("Content-Location", toUri(region, keyList).toASCIIString());
+ headers.set(HttpHeaders.CONTENT_LOCATION, uri.toASCIIString());
final RegionData<Object> data = new RegionData<>(region);
- data.add(valueObjs.values());
+ // add the values in the same order as the original keys
+ // the code used to use valueObj.values() which used "hash" ordering.
+ for (String key : keys) {
+ data.add(valueObjs.get(key));
+ }
return new ResponseEntity<RegionData<?>>(data, headers, HttpStatus.OK);
}
}
@@ -274,16 +328,17 @@ public class PdxBasedCrudController extends CommonCrudController {
* Update data for a key or set of keys
*
* @param region gemfire data region
+ * @param keys comma separated list of keys
* @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}/**",
+ @RequestMapping(method = RequestMethod.PUT, value = "/{region}/{keys}",
consumes = {APPLICATION_JSON_UTF8_VALUE}, produces = {
APPLICATION_JSON_UTF8_VALUE})
@ApiOperation(value = "update data for key",
notes = "Update or insert (put) data for keys in a region."
- + " The keys, ** in the endpoint, are a comma separated list."
+ + " Deprecated in favor of /{region}?keys=."
+ " 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."),
@@ -295,21 +350,111 @@ 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") String[] keys,
@RequestParam(value = "op", defaultValue = "PUT") final String opValue,
@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);
+ if (!validOp(opValue)) {
+ String errorMessage = String.format(
+ "The op parameter (%1$s) is not valid. Valid values are PUT, REPLACE, or CAS.",
+ opValue);
+ return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST);
+ }
if (keys.length > 1) {
// putAll case
- return updateMultipleKeys(region, keys, json);
+ updateMultipleKeys(region, keys, json);
+ HttpHeaders headers = new HttpHeaders();
+ headers.setLocation(toUri(region, StringUtils.arrayToCommaDelimitedString(keys)));
+ return new ResponseEntity<>(headers, HttpStatus.OK);
+ } else {
+ // put case
+ Object existingValue = updateSingleKey(region, keys[0], json, opValue);
+ final HttpHeaders headers = new HttpHeaders();
+ headers.setLocation(toUri(region, keys[0]));
+ return new ResponseEntity<>(existingValue, headers,
+ (existingValue == null ? HttpStatus.OK : HttpStatus.CONFLICT));
+ }
+ }
+
+ private boolean validOp(String opValue) {
+ try {
+ UpdateOp.valueOf(opValue.toUpperCase());
+ return true;
+ } catch (IllegalArgumentException ex) {
+ return false;
+ }
+ }
+
+ /**
+ * Update data for a key or set of keys
+ *
+ * @param encodedRegion gemfire data region
+ * @param encodedKeys comma separated list of keys
+ * @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}",
+ consumes = {APPLICATION_JSON_UTF8_VALUE}, produces = {
+ APPLICATION_JSON_UTF8_VALUE})
+ @ApiOperation(value = "update data for key(s)",
+ notes = "Update or insert (put) data for keys in a region."
+ + " The keys are a comma separated list."
+ + " If multiple keys are given then put (create or update) the data for each key."
+ + " The op parameter is ignored if more than one key is given."
+ + " If op=PUT, the default, create or update data for the given key."
+ + " If op=CREATE, create data for the given key if and only if the key does not exit in the region."
+ + " If op=REPLACE, update (replace) data for the given 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 = 201, message = "For op=CREATE on success."),
+ @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 or if key is not mapped to some value for REPLACE or CAS."),
+ @ApiResponse(code = 409,
+ message = "For op=CREATE, key already exist in region. For op=CAS, @old value does not match to the current value in region."),
+ @ApiResponse(code = 500, message = "GemFire throws an error or exception.")})
+ public ResponseEntity<?> updateKeys(@PathVariable("region") final String encodedRegion,
+ @RequestParam(value = "keys") final String[] encodedKeys,
+ @RequestParam(value = "op", defaultValue = "PUT") final String opValue,
+ @RequestBody final String json) {
+
+ String decodedRegion = decode(encodedRegion);
+ String[] decodedKeys = decode(encodedKeys);
+ if (!validOp(opValue) && !opValue.equalsIgnoreCase("CREATE")) {
+ String errorMessage = String.format(
+ "The op parameter (%1$s) is not valid. Valid values are PUT, CREATE, REPLACE, or CAS.",
+ opValue);
+ return new ResponseEntity<>(convertErrorAsJson(errorMessage), HttpStatus.BAD_REQUEST);
+ }
+
+ if (decodedKeys.length > 1) {
+ // putAll case
+ logger.debug("updating keys ({}) for region ({}) op={}", decodedKeys, decodedRegion, opValue);
+ securityService.authorize("WRITE", decodedRegion, decodedKeys);
+ updateMultipleKeys(decodedRegion, decodedKeys, json);
+ HttpHeaders headers = new HttpHeaders();
+ headers.setLocation(toUriWithKeys(encodedKeys, encodedRegion));
+ return new ResponseEntity<>(headers, HttpStatus.OK);
+ } else if (opValue.equalsIgnoreCase("CREATE")) {
+ securityService.authorize("DATA", "WRITE", decodedRegion);
+ return create(decodedRegion, decodedKeys[0], json, true);
} else {
// put case
- return updateSingleKey(region, keys[0], json, opValue);
+ logger.debug("updating keys ({}) for region ({}) op={}", decodedKeys, decodedRegion, opValue);
+ securityService.authorize("WRITE", decodedRegion, decodedKeys);
+ Object existingValue = updateSingleKey(decodedRegion, decodedKeys[0], json, opValue);
+ final HttpHeaders headers = new HttpHeaders();
+ headers.setLocation(toUriWithKeys(encodedKeys, encodedRegion));
+ return new ResponseEntity<>(existingValue, headers,
+ (existingValue == null ? HttpStatus.OK : HttpStatus.CONFLICT));
}
}
diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/UpdateOp.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/UpdateOp.java
index 69ecbe3..b4818e0 100644
--- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/UpdateOp.java
+++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/support/UpdateOp.java
@@ -16,7 +16,7 @@
package org.apache.geode.rest.internal.web.controllers.support;
/**
- * UpdateOp contains all posible update operation supported with REST APIs
+ * UpdateOp contains all possible update operation supported with REST APIs
*/
@SuppressWarnings("unused")