You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jm...@apache.org on 2020/04/08 20:52:24 UTC
[geode] branch develop updated: Revert "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.
jmelchior 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 b4fb6c8 Revert "GEODE-7938: added keys query param to GET, PUT, and DELETE /{region} endpoints (#4901)"
new 8e3c7a2 Merge pull request #4929 from jmelchio/slash-revert
b4fb6c8 is described below
commit b4fb6c8637e8135cf879a057f7aa3e9f807d60ff
Author: Joris Melchior <jo...@gmail.com>
AuthorDate: Wed Apr 8 14:26:24 2020 -0400
Revert "GEODE-7938: added keys query param to GET, PUT, and DELETE /{region} endpoints (#4901)"
This reverts commit 6263f1f9a4782433d013c6b759bda8c84216b595.
---
.../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, 160 insertions(+), 608 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 676a0fa..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
@@ -16,12 +16,9 @@ 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.Matchers.contains;
+import static org.hamcrest.CoreMatchers.containsString;
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;
@@ -32,10 +29,7 @@ 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;
@@ -106,8 +100,6 @@ 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<>();
@@ -118,30 +110,6 @@ 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")
@@ -201,14 +169,12 @@ 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(content().json(jsonResources.get(ORDER1_JSON)));
+ .andExpect(status().isConflict());
Order order = (Order) ((PdxInstance) orderRegion.get("1")).getObject();
assertThat(order).as("order should not be null").isNotNull();
@@ -216,24 +182,20 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- public void createEntryWithEncodedKey() throws Exception {
- String decodedKey = createKey(1);
- String encodedKey = encodeKey(decodedKey);
- mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
+ 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(content().string(""))
- .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
+ .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
- mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
+ mockMvc.perform(post("/v1/orders?key=" + key)
.content(jsonResources.get(ORDER1_JSON))
.with(POST_PROCESSOR))
- .andExpect(status().isConflict())
- .andExpect(content().json(jsonResources.get(ORDER1_JSON)))
- .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
+ .andExpect(status().isConflict());
- Order order = (Order) ((PdxInstance) orderRegion.get(decodedKey)).getObject();
+ Order order = (Order) ((PdxInstance) orderRegion.get(key)).getObject();
assertThat(order).as("order should not be null").isNotNull();
}
@@ -244,15 +206,12 @@ 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(content().json(jsonResources.get(ORDER1_ARRAY_JSON)));
-
+ .andExpect(status().isConflict());
@SuppressWarnings("unchecked")
List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get("1");
@@ -262,25 +221,21 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- public void createEntryWithJsonArrayOfOrdersWithEncodedKey() throws Exception {
- String decodedKey = createKey(1);
- String encodedKey = encodeKey(decodedKey);
- mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
+ 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(content().string(""))
- .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
+ .andExpect(header().string("Location", BASE_URL + "/orders/" + key));
- mockMvc.perform(put("/v1/orders?op=CREATE&keys=" + encodedKey)
+ mockMvc.perform(post("/v1/orders?key=" + key)
.content(jsonResources.get(ORDER1_ARRAY_JSON))
.with(POST_PROCESSOR))
- .andExpect(status().isConflict())
- .andExpect(content().json(jsonResources.get(ORDER1_ARRAY_JSON)))
- .andExpect(header().string("Location", BASE_URL + "/orders?keys=" + encodedKey));
+ .andExpect(status().isConflict());
@SuppressWarnings("unchecked")
- List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get(decodedKey);
+ List<PdxInstance> entries = (List<PdxInstance>) orderRegion.get(key);
Order order = (Order) entries.get(0).getObject();
assertThat(order).as("order should not be null").isNotNull();
}
@@ -358,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))
@@ -405,30 +377,7 @@ public class RestAccessControllerTest {
.with(POST_PROCESSOR))
.andExpect(status().isOk())
.andExpect(header().string("Location", BASE_URL + "/customers/" + keys));
- assertThat(customerRegion).hasSize(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 putAllWithQueryParam() 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?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);
+ assertThat(customerRegion.size()).isEqualTo(60);
for (int i = 1; i <= 60; i++) {
PdxInstance customer = customerRegion.get(String.valueOf(i));
assertThat(customer.getField("customerId").toString())
@@ -438,22 +387,22 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- public void putMultipleEncodedKeys() throws Exception {
+ public void putAllWithSlashes() throws Exception {
StringBuilder keysBuilder = new StringBuilder();
for (int i = 1; i < 60; i++) {
- keysBuilder.append(createEncodedKey(i)).append(',');
+ keysBuilder.append(i).append(SLASH).append(',');
}
- keysBuilder.append(createEncodedKey(60));
+ keysBuilder.append(60).append(SLASH);
String keys = keysBuilder.toString();
mockMvc.perform(
- put("/v1/customers?keys=" + keys)
+ put("/v1/customers/" + 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);
+ .andExpect(header().string("Location", BASE_URL + "/customers/" + keys));
+ assertThat(customerRegion.size()).isEqualTo(60);
for (int i = 1; i <= 60; i++) {
- PdxInstance customer = customerRegion.get(createKey(i));
+ PdxInstance customer = customerRegion.get(String.valueOf(i) + SLASH);
assertThat(customer.getField("customerId").toString())
.isEqualTo(Integer.valueOf(100 + i).toString());
}
@@ -461,27 +410,6 @@ public class RestAccessControllerTest {
@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))
@@ -570,30 +498,6 @@ 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))
@@ -716,13 +620,11 @@ public class RestAccessControllerTest {
@SuppressWarnings("unchecked")
@Test
@WithMockUser
- public void getAllCustomers() throws Exception {
+ public void getCustomers() 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))));
}
@@ -744,47 +646,11 @@ 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")
@@ -815,117 +681,25 @@ 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", contains(101, 102, 103, 104, 105)));
- }
-
- @Test
- @WithMockUser
- 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("$.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!")));
+ jsonPath("$.customers[*].customerId", containsInAnyOrder(101, 102, 103, 104, 105)));
}
@Test
@WithMockUser
- public void getSpecificKeysUsingQueryParamWithNonExistentKey() throws Exception {
- putAll();
- mockMvc.perform(get("/v1/customers?keys=1,doesNotExist&ignoreMissingKey=true")
- .with(POST_PROCESSOR))
+ 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(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!")));
+ jsonPath("$.customers[*].customerId", containsInAnyOrder(101, 102, 103, 104, 105)));
}
@Test
@@ -966,71 +740,12 @@ public class RestAccessControllerTest {
@Test
@WithMockUser
- 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))
+ 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());
- 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 1f8b6ff..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
@@ -18,7 +18,6 @@ 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;
@@ -29,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;
@@ -130,13 +131,6 @@ 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) {
@@ -147,13 +141,6 @@ 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");
@@ -162,22 +149,6 @@ 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);
@@ -604,7 +575,7 @@ public abstract class AbstractBaseController implements InitializingBean {
if (StringUtils.hasText(existingKey)) {
newKey = existingKey;
- if (domainObject != null && NumberUtils.isNumeric(newKey) && domainObjectId == null) {
+ if (NumberUtils.isNumeric(newKey) && domainObjectId == null) {
final Long newId = IdentifiableUtils.createId(NumberUtils.parseLong(newKey));
if (newKey.equals(newId.toString())) {
IdentifiableUtils.setId(domainObject, newId);
@@ -630,14 +601,6 @@ 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);
@@ -660,6 +623,16 @@ 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());
@@ -822,10 +795,7 @@ public abstract class AbstractBaseController implements InitializingBean {
}
}
- /**
- * @return if the opValue is CAS then the existingValue; otherwise null
- */
- String updateSingleKey(final String region, final String key, final String json,
+ ResponseEntity<String> updateSingleKey(final String region, final String key, final String json,
final String opValue) {
final JSONTypes jsonType = validateJsonAndFindType(json);
@@ -847,15 +817,22 @@ 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));
}
}
- return existingValue;
+
+ final HttpHeaders headers = new HttpHeaders();
+ headers.setLocation(toUri(region, key));
+ return new ResponseEntity<>(existingValue, headers,
+ (existingValue == null ? HttpStatus.OK : HttpStatus.CONFLICT));
}
- void updateMultipleKeys(final String region, final String[] keys, final String json) {
+ ResponseEntity<String> updateMultipleKeys(final String region, final String[] keys,
+ final String json) {
+
JsonNode jsonArr;
try {
jsonArr = objectMapper.readTree(json);
@@ -887,6 +864,10 @@ 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) {
@@ -993,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 ec13804..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;
@@ -30,7 +32,6 @@ 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;
@@ -111,64 +112,52 @@ public abstract class CommonCrudController extends AbstractBaseController {
}
/**
- * Delete data for one or more keys from a region
+ * Delete data for single key or specific keys in region
*
* @param region gemfire region
- * @param keys comma separated list of keys
* @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 one or more keys in a region. Deprecated in favor of /{region}?keys=.")
+ 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")})
public ResponseEntity<?> delete(@PathVariable("region") String region,
- @PathVariable("keys") String[] keys) {
- region = decode(region);
- return deleteRegionKeys(region, keys);
- }
-
- private ResponseEntity<?> deleteRegionKeys(String region, String[] keys) {
+ HttpServletRequest request) {
+ String[] keys = parseKeys(request, region);
securityService.authorize("WRITE", region, keys);
- logger.debug("Delete data for keys {} on region {}", ArrayUtils.toString((Object[]) keys),
+ logger.debug("Delete data for key {} on region {}", ArrayUtils.toString((Object[]) keys),
region);
+
+ region = decode(region);
+
deleteValues(region, keys);
return new ResponseEntity<>(HttpStatus.OK);
}
/**
- * Delete all data in region or just the given keys
+ * Delete all data in region
*
* @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 or the specified keys",
- notes = "Delete all data in the region or just the specified keys.")
+ @ApiOperation(value = "delete all data", notes = "Delete all data in the region")
@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")})
- public ResponseEntity<?> deleteAllOrGivenKeys(@PathVariable("region") String region,
- @RequestParam(value = "keys", required = false) final String[] encodedKeys) {
+ @PreAuthorize("@securityService.authorize('DATA', 'WRITE', #region)")
+ public ResponseEntity<?> delete(@PathVariable("region") String region) {
+ logger.debug("Deleting all data in Region ({})...", region);
+
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 a6e8c6f..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
@@ -14,7 +14,6 @@
*/
package org.apache.geode.rest.internal.web.controllers;
-import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -42,7 +41,6 @@ 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;
@@ -80,8 +78,7 @@ 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."
- + " The key is not decoded so if the key contains special characters use PUT /{region}?keys=EncodedKey&op=CREATE.")
+ @ApiOperation(value = "create entry", notes = "Create (put-if-absent) data in region")
@ApiResponses({@ApiResponse(code = 201, message = "Created."),
@ApiResponse(code = 400,
message = "Data specified (JSON doc) in the request body is invalid."),
@@ -94,14 +91,12 @@ 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);
- }
- private ResponseEntity<?> create(String region, String key, String json,
- boolean keyInQueryParam) {
- logger.debug("Create JSON document ({}) in Region ({}) with Key ({})...", json, region, key);
+ logger.debug(
+ "Posting (creating/putIfAbsent) JSON document ({}) to 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
@@ -113,11 +108,7 @@ public class PdxBasedCrudController extends CommonCrudController {
}
final HttpHeaders headers = new HttpHeaders();
- if (keyInQueryParam) {
- headers.setLocation(toUriWithKeys(new String[] {encode(key)}, region));
- } else {
- headers.setLocation(toUri(region, key));
- }
+ headers.setLocation(toUri(region, key));
if (existingPdxObj != null) {
final RegionEntryData<Object> data = new RegionEntryData<>(region);
@@ -130,44 +121,30 @@ public class PdxBasedCrudController extends CommonCrudController {
}
/**
- * 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).
+ * Read all or fixed number of data in a given Region
*
* @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 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.")
+ @ApiOperation(value = "read all data for region",
+ notes = "Read all data for region. Use limit param to get fixed or limited number of entries.")
@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,
- @RequestParam(value = "keys", required = false) final String[] encodedKeys,
- @RequestParam(value = "ignoreMissingKey", required = false) final String ignoreMissingKey) {
+ defaultValue = DEFAULT_GETALL_RESULT_LIMIT) final String limit) {
+ logger.debug("Reading all data in Region ({})...", region);
+
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);
@@ -187,7 +164,7 @@ public class PdxBasedCrudController extends CommonCrudController {
if ("ALL".equalsIgnoreCase(limit)) {
data.add(values);
- keyList = StringUtils.collectionToCommaDelimitedString(keys);
+ keyList = StringUtils.collectionToDelimitedString(keys, ",");
} else {
try {
int maxLimit = Integer.parseInt(limit);
@@ -203,7 +180,7 @@ public class PdxBasedCrudController extends CommonCrudController {
}
data.add(values.subList(0, maxLimit));
- keyList = StringUtils.collectionToCommaDelimitedString(keys.subList(0, maxLimit));
+ keyList = StringUtils.collectionToDelimitedString(keys.subList(0, maxLimit), ",");
} catch (NumberFormatException e) {
// limit param is not specified in proper format. set the HTTPHeader
@@ -213,7 +190,7 @@ public class PdxBasedCrudController extends CommonCrudController {
}
}
- headers.set(HttpHeaders.CONTENT_LOCATION, toUri(region, keyList).toASCIIString());
+ headers.set("Content-Location", toUri(region, keyList).toASCIIString());
return new ResponseEntity<RegionData<?>>(data, headers, HttpStatus.OK);
}
@@ -223,28 +200,26 @@ public class PdxBasedCrudController extends CommonCrudController {
* @param region gemfire region name
* @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 a region. Deprecated in favor of /{region}?keys=.")
+ 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.")})
- 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);
+ 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();
+ region = decode(region);
+
if (keys.length == 1) {
/* GET op on single key */
Object value = getValue(region, keys[0]);
@@ -255,15 +230,7 @@ public class PdxBasedCrudController extends CommonCrudController {
}
final RegionEntryData<Object> data = new RegionEntryData<>(region);
- 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());
+ headers.set("Content-Location", toUri(region, keys[0]).toASCIIString());
data.add(value);
return new ResponseEntity<RegionData<?>>(data, headers, HttpStatus.OK);
@@ -277,49 +244,28 @@ 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 = 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,
+ 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,
HttpStatus.BAD_REQUEST);
}
}
- // 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);
- }
+ 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));
- headers.set(HttpHeaders.CONTENT_LOCATION, uri.toASCIIString());
+ // 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());
final RegionData<Object> data = new RegionData<>(region);
- // 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));
- }
+ data.add(valueObjs.values());
return new ResponseEntity<RegionData<?>>(data, headers, HttpStatus.OK);
}
}
@@ -328,17 +274,16 @@ 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}/{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 keys in a region."
- + " Deprecated in favor of /{region}?keys=."
+ + " 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."),
@@ -350,111 +295,21 @@ 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
- 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);
+ return updateMultipleKeys(region, keys, json);
} else {
// put case
- 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));
+ return updateSingleKey(region, keys[0], json, opValue);
}
}
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 b4818e0..69ecbe3 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 possible update operation supported with REST APIs
+ * UpdateOp contains all posible update operation supported with REST APIs
*/
@SuppressWarnings("unused")