You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bs...@apache.org on 2018/01/23 16:40:50 UTC
[geode] branch develop updated: GEODE-4317 add error handling to
the experimental ProtobufDriver's Region implementation
This is an automated email from the ASF dual-hosted git repository.
bschuchardt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 63a96d9 GEODE-4317 add error handling to the experimental ProtobufDriver's Region implementation
63a96d9 is described below
commit 63a96d9e6512fe4ba11f2e4afe382c6977322f9f
Author: Bruce Schuchardt <bs...@pivotal.io>
AuthorDate: Mon Jan 22 08:56:20 2018 -0800
GEODE-4317 add error handling to the experimental ProtobufDriver's Region implementation
I've added checks to ProtobufRegion for "error" responses from the server.
Along the way I found we were converting some keys to their string
representations and fixed that as well.
The exceptions thrown show the error text returned by the server. The
handling of putAll responses was already looking for errors but wasn't
including the server's message text so I changed it to do so.
This closes #1322
---
.../geode/experimental/driver/ProtobufRegion.java | 69 ++++++++++++----------
.../experimental/driver/DriverConnectionTest.java | 7 ---
.../experimental/driver/RegionIntegrationTest.java | 44 +++++++++++++-
3 files changed, 82 insertions(+), 38 deletions(-)
diff --git a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java
index 7204be8..c8a9ce2 100644
--- a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java
+++ b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufRegion.java
@@ -61,19 +61,18 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
this.socket = socket;
}
- @Override
- public V get(K key) throws IOException {
- final OutputStream outputStream = socket.getOutputStream();
- ClientProtocol.Message.newBuilder()
- .setRequest(ClientProtocol.Request.newBuilder().setGetRequest(RegionAPI.GetRequest
- .newBuilder().setRegionName(name).setKey(ValueEncoder.encodeValue(key))))
- .build().writeDelimitedTo(outputStream);
-
+ private ClientProtocol.Response readResponse() throws IOException {
final InputStream inputStream = socket.getInputStream();
- return (V) ValueEncoder.decodeValue(ClientProtocol.Message.parseDelimitedFrom(inputStream)
- .getResponse().getGetResponse().getResult());
+ ClientProtocol.Response response =
+ ClientProtocol.Message.parseDelimitedFrom(inputStream).getResponse();
+ final ClientProtocol.ErrorResponse errorResponse = response.getErrorResponse();
+ if (errorResponse != null && errorResponse.hasError()) {
+ throw new IOException(errorResponse.getError().getMessage());
+ }
+ return response;
}
+
@Override
public RegionAttributes getRegionAttributes() throws IOException {
final OutputStream outputStream = socket.getOutputStream();
@@ -82,13 +81,23 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
.setGetRegionRequest(RegionAPI.GetRegionRequest.newBuilder().setRegionName(name)))
.build().writeDelimitedTo(outputStream);
- final InputStream inputStream = socket.getInputStream();
- return new RegionAttributes(ClientProtocol.Message.parseDelimitedFrom(inputStream).getResponse()
- .getGetRegionResponse().getRegion());
+ return new RegionAttributes(readResponse().getGetRegionResponse().getRegion());
}
@Override
+ public V get(K key) throws IOException {
+ final OutputStream outputStream = socket.getOutputStream();
+ ClientProtocol.Message.newBuilder()
+ .setRequest(ClientProtocol.Request.newBuilder().setGetRequest(RegionAPI.GetRequest
+ .newBuilder().setRegionName(name).setKey(ValueEncoder.encodeValue(key))))
+ .build().writeDelimitedTo(outputStream);
+
+ final ClientProtocol.Response response = readResponse();
+ return (V) ValueEncoder.decodeValue(response.getGetResponse().getResult());
+ }
+
+ @Override
public Map<K, V> getAll(Collection<K> keys) throws IOException {
Map<K, V> values = new HashMap<>();
@@ -96,15 +105,21 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
RegionAPI.GetAllRequest.Builder getAllRequest = RegionAPI.GetAllRequest.newBuilder();
getAllRequest.setRegionName(name);
for (K key : keys) {
- getAllRequest.addKey(ValueEncoder.encodeValue(key.toString()));
+ getAllRequest.addKey(ValueEncoder.encodeValue(key));
}
ClientProtocol.Message.newBuilder()
.setRequest(ClientProtocol.Request.newBuilder().setGetAllRequest(getAllRequest)).build()
.writeDelimitedTo(outputStream);
- final InputStream inputStream = socket.getInputStream();
- final RegionAPI.GetAllResponse getAllResponse =
- ClientProtocol.Message.parseDelimitedFrom(inputStream).getResponse().getGetAllResponse();
+ final RegionAPI.GetAllResponse getAllResponse = readResponse().getGetAllResponse();
+ Map<Object, String> failures = new HashMap<>();
+ if (getAllResponse.getFailuresCount() > 0) {
+ for (BasicTypes.KeyedError keyedError : getAllResponse.getFailuresList()) {
+ failures.put(ValueEncoder.decodeValue(keyedError.getKey()),
+ keyedError.getError().getMessage());
+ }
+ throw new IOException("Unable to process the following keys: " + failures);
+ }
for (BasicTypes.Entry entry : getAllResponse.getEntriesList()) {
values.put((K) ValueEncoder.decodeValue(entry.getKey()),
(V) ValueEncoder.decodeValue(entry.getValue()));
@@ -122,8 +137,7 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
.setEntry(ValueEncoder.encodeEntry(key, value))))
.build().writeDelimitedTo(outputStream);
- final InputStream inputStream = socket.getInputStream();
- ClientProtocol.Message.parseDelimitedFrom(inputStream).getResponse().getPutResponse();
+ readResponse();
}
@Override
@@ -138,18 +152,14 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
.setRequest(ClientProtocol.Request.newBuilder().setPutAllRequest(putAllRequest)).build()
.writeDelimitedTo(outputStream);
- final InputStream inputStream = socket.getInputStream();
- final RegionAPI.PutAllResponse putAllResponse =
- ClientProtocol.Message.parseDelimitedFrom(inputStream).getResponse().getPutAllResponse();
+ final RegionAPI.PutAllResponse putAllResponse = readResponse().getPutAllResponse();
if (0 < putAllResponse.getFailedKeysCount()) {
- StringBuilder builder = new StringBuilder();
+ Map<Object, String> failures = new HashMap<>();
for (BasicTypes.KeyedError keyedError : putAllResponse.getFailedKeysList()) {
- if (0 < builder.length()) {
- builder.append(", ");
- }
- builder.append(ValueEncoder.decodeValue(keyedError.getKey()).toString());
+ failures.put(ValueEncoder.decodeValue(keyedError.getKey()),
+ keyedError.getError().getMessage());
}
- throw new IOException("Unable to put the following keys: " + builder.toString());
+ throw new IOException("Unable to put the following keys: " + failures);
}
}
@@ -162,7 +172,6 @@ public class ProtobufRegion<K, V> implements Region<K, V> {
.newBuilder().setRegionName(name).setKey(ValueEncoder.encodeValue(key))))
.build().writeDelimitedTo(outputStream);
- final InputStream inputStream = socket.getInputStream();
- ClientProtocol.Message.parseDelimitedFrom(inputStream).getResponse().getRemoveResponse();
+ readResponse();
}
}
diff --git a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/DriverConnectionTest.java b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/DriverConnectionTest.java
index f28a9c5..c840d9e 100644
--- a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/DriverConnectionTest.java
+++ b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/DriverConnectionTest.java
@@ -43,13 +43,6 @@ public class DriverConnectionTest {
@Rule
public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
- /** a JSON document */
- private static final String jsonDocument =
- "{" + System.lineSeparator() + " \"name\" : \"Charlemagne\"," + System.lineSeparator()
- + " \"age\" : 1276," + System.lineSeparator() + " \"nationality\" : \"french\","
- + System.lineSeparator() + " \"emailAddress\" : \"none\"" + System.lineSeparator() + "}";
-
-
private Locator locator;
private Cache cache;
private Driver driver;
diff --git a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java
index 465fff8..72a5da3 100644
--- a/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java
+++ b/geode-experimental-driver/src/test/java/org/apache/geode/experimental/driver/RegionIntegrationTest.java
@@ -14,13 +14,17 @@
*/
package org.apache.geode.experimental.driver;
-import static org.apache.geode.internal.Assert.assertTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
import java.util.Properties;
+import java.util.Set;
import org.junit.After;
import org.junit.Before;
@@ -145,4 +149,42 @@ public class RegionIntegrationTest {
assertNull(region.get(document));
}
+ @Test(expected = IOException.class)
+ public void putWithBadJSONKeyAndValue() throws IOException {
+ Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
+ JSONWrapper document = JSONWrapper.wrapJSON("{\"name\":\"Johan\"]");
+ region.put(document, document);
+ }
+
+ @Test(expected = IOException.class)
+ public void putAllWithBadJSONKeyAndValue() throws IOException {
+ Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
+ Map<JSONWrapper, JSONWrapper> putAllMap = new HashMap<>();
+ JSONWrapper document = JSONWrapper.wrapJSON("{\"name\":\"Johan\"]");
+ putAllMap.put(document, document);
+ region.putAll(putAllMap);
+ }
+
+ @Test(expected = IOException.class)
+ public void getWithBadJSONKey() throws IOException {
+ Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
+ region.get(JSONWrapper.wrapJSON("{\"name\":\"Johan\"]"));
+ }
+
+ @Test(expected = IOException.class)
+ public void getAllWithBadJSONKeyAndValue() throws IOException {
+ Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
+ Set<JSONWrapper> getAllKeys = new HashSet<>();
+ JSONWrapper document = JSONWrapper.wrapJSON("{\"name\":\"Johan\"]");
+ getAllKeys.add(document);
+ Map<JSONWrapper, JSONWrapper> result = region.getAll(getAllKeys);
+ assertTrue("missing key " + document + " in " + result, result.containsKey(document));
+ assertNull(result.get(document));
+ }
+
+ @Test(expected = IOException.class)
+ public void removeWithBadJSONKey() throws IOException {
+ Region<JSONWrapper, JSONWrapper> region = driver.getRegion("region");
+ region.remove(JSONWrapper.wrapJSON("{\"name\":\"Johan\"]"));
+ }
}
--
To stop receiving notification emails like this one, please contact
bschuchardt@apache.org.