[geode] branch feature/GEODE-4377 updated: GEODE-4377: Don't catch encoding exceptions in OperationHandlers

2018-02-13 Thread bschuchardt
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-4377
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-4377 by this 
push:
 new 6f251af  GEODE-4377: Don't catch encoding exceptions in 
OperationHandlers
6f251af is described below

commit 6f251af8830a728eaedfda6f72e949e344710504
Author: Bruce Schuchardt 
AuthorDate: Tue Feb 13 10:40:49 2018 -0800

GEODE-4377: Don't catch encoding exceptions in OperationHandlers

Addressing review comments.

I've added unit tests to ensure that a Success response is returned to
the client if there are any encoding/decoding errors encountered while
processing entries.

In ProtobufResponseUtilities I've modified the error message to contain
the toString() of the exception, which will include the exception class
name and the exception's message.
---
 .../operations/GetAllRequestOperationHandler.java  | 41 ++---
 .../operations/PutAllRequestOperationHandler.java  | 34 +-
 .../v1/utilities/ProtobufResponseUtilities.java|  6 ++--
 .../GetAllRequestOperationHandlerJUnitTest.java| 33 +
 .../GetRequestOperationHandlerJUnitTest.java   |  9 -
 .../PutAllRequestOperationHandlerJUnitTest.java| 42 ++
 6 files changed, 110 insertions(+), 55 deletions(-)

diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
index d643ec6..c0ffbbb 100644
--- 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetAllRequestOperationHandler.java
@@ -61,16 +61,8 @@ public class GetAllRequestOperationHandler
 RegionAPI.GetAllResponse.Builder responseBuilder = 
RegionAPI.GetAllResponse.newBuilder();
 try {
   
messageExecutionContext.getCache().setReadSerializedForCurrentThread(true);
-
-  for (BasicTypes.EncodedValue key : request.getKeyList()) {
-Object entry = processOneMessage(serializationService, region, key);
-if (entry instanceof BasicTypes.Entry) {
-  responseBuilder.addEntries((BasicTypes.Entry) entry);
-} else {
-  responseBuilder.addFailures((BasicTypes.KeyedError) entry);
-}
-  }
-
+  request.getKeyList().stream()
+  .forEach((key) -> processSingleKey(responseBuilder, 
serializationService, region, key));
 } finally {
   
messageExecutionContext.getCache().setReadSerializedForCurrentThread(false);
   messageExecutionContext.getStatistics().endOperation(startTime);
@@ -79,27 +71,36 @@ public class GetAllRequestOperationHandler
 return Success.of(responseBuilder.build());
   }
 
-  private Object processOneMessage(ProtobufSerializationService 
serializationService, Region region,
-  BasicTypes.EncodedValue key) throws DecodingException {
+  private void processSingleKey(RegionAPI.GetAllResponse.Builder 
responseBuilder,
+  ProtobufSerializationService serializationService, Region region,
+  BasicTypes.EncodedValue key) {
 try {
+
   Object decodedKey = serializationService.decode(key);
   Object value = region.get(decodedKey);
-  return ProtobufUtilities.createEntry(serializationService, decodedKey, 
value);
-} catch (EncodingException ex) {
-  logger.error("Encoding not supported: {}", ex);
-  return createKeyedError(key, "Encoding not supported.", INVALID_REQUEST);
+  BasicTypes.Entry entry =
+  ProtobufUtilities.createEntry(serializationService, decodedKey, 
value);
+  responseBuilder.addEntries(entry);
+
 } catch (DecodingException ex) {
-  throw ex;
+  logger.info("Key encoding not supported: {}", ex);
+  responseBuilder
+  .addFailures(buildKeyedError(key, "Key encoding not supported.", 
INVALID_REQUEST));
+} catch (EncodingException ex) {
+  logger.info("Value encoding not supported: {}", ex);
+  responseBuilder
+  .addFailures(buildKeyedError(key, "Value encoding not supported.", 
INVALID_REQUEST));
 } catch (Exception ex) {
-  logger.info("Failure in protobuf getAll operation for key: " + key, ex);
-  return createKeyedError(key, ex.toString(), SERVER_ERROR);
+  logger.warn("Failure in protobuf getAll operation for key: " + key, ex);
+  responseBuilder.addFailures(buildKeyedError(key, ex.toString(), 
SERVER_ERROR));
 }
   }
 
-  private Object createKeyedError(BasicTypes.EncodedValue key, String 
errorMessage,
+  private BasicTypes.KeyedError buildKeyedError(BasicTypes.EncodedValue key, 
String errorMessage,
   ProtobufErrorCo

[geode] branch feature/GEODE-4377 updated: GEODE-4377: Don't catch encoding exceptions in OperationHandlers

2018-02-12 Thread bschuchardt
This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch feature/GEODE-4377
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-4377 by this 
push:
 new 8989ab5  GEODE-4377: Don't catch encoding exceptions in 
OperationHandlers
8989ab5 is described below

commit 8989ab58ee21c8095449764eac4589adb6624b5a
Author: Bruce Schuchardt 
AuthorDate: Mon Feb 12 16:09:03 2018 -0800

GEODE-4377: Don't catch encoding exceptions in OperationHandlers

Adding missing file: DecodingException.java
---
 .../serialization/exception/DecodingException.java | 31 ++
 1 file changed, 31 insertions(+)

diff --git 
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/serialization/exception/DecodingException.java
 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/serialization/exception/DecodingException.java
new file mode 100644
index 000..93cedff
--- /dev/null
+++ 
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/serialization/exception/DecodingException.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more 
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional 
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache 
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the 
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software 
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY 
KIND, either express
+ * or implied. See the License for the specific language governing permissions 
and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.protocol.protobuf.v1.serialization.exception;
+
+import org.apache.geode.annotations.Experimental;
+
+/**
+ * This indicates an encoding type that we don't know how to handle.
+ */
+@Experimental
+public class DecodingException extends Exception {
+  public DecodingException(String message) {
+super(message);
+  }
+
+  public DecodingException(String message, Throwable cause) {
+super(message, cause);
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
bschucha...@apache.org.