You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ud...@apache.org on 2017/10/06 18:16:00 UTC
[geode] branch develop updated: GEODE-3779: Cleaned up some error
handling code in new Protocol
This is an automated email from the ASF dual-hosted git repository.
udo 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 1f6cf28 GEODE-3779: Cleaned up some error handling code in new Protocol
1f6cf28 is described below
commit 1f6cf281b63e91fa13c082fdba63a1f828518154
Author: kohlmu-pivotal <uk...@pivotal.io>
AuthorDate: Fri Oct 6 11:15:45 2017 -0700
GEODE-3779: Cleaned up some error handling code in new Protocol
---
.../protocol/protobuf/ProtobufOpsProcessor.java | 8 ++---
.../operations/GetAllRequestOperationHandler.java | 37 +++++++++-------------
.../GetRegionRequestOperationHandler.java | 8 ++---
.../operations/GetRequestOperationHandler.java | 13 ++++----
.../operations/PutAllRequestOperationHandler.java | 14 ++++----
.../operations/PutRequestOperationHandler.java | 15 ++++-----
.../operations/RemoveRequestOperationHandler.java | 16 +++++-----
.../utilities/ProtobufResponseUtilities.java | 25 +++------------
8 files changed, 56 insertions(+), 80 deletions(-)
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/ProtobufOpsProcessor.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/ProtobufOpsProcessor.java
index 9b497c5..27bb909 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/ProtobufOpsProcessor.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/ProtobufOpsProcessor.java
@@ -25,6 +25,8 @@ import org.apache.geode.internal.protocol.protobuf.statistics.ProtobufClientStat
import org.apache.geode.internal.protocol.protobuf.utilities.ProtobufResponseUtilities;
import org.apache.geode.internal.serialization.SerializationService;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
/**
* This handles protobuf requests by determining the operation type of the request and dispatching
* it to the appropriate handler.
@@ -56,14 +58,12 @@ public class ProtobufOpsProcessor {
} else {
logger.warn("Received unauthorized request");
recordAuthorizationViolation(context);
- result = Failure.of(ProtobufResponseUtilities.makeErrorResponse(
- ProtocolErrorCode.AUTHORIZATION_FAILED.codeValue,
+ result = Failure.of(ProtobufResponseUtilities.makeErrorResponse(AUTHORIZATION_FAILED,
"User isn't authorized for this operation."));
}
} catch (InvalidExecutionContextException exception) {
logger.error("Invalid execution context found for operation {}", requestType);
- result = Failure.of(ProtobufResponseUtilities.makeErrorResponse(
- ProtocolErrorCode.UNSUPPORTED_OPERATION.codeValue,
+ result = Failure.of(ProtobufResponseUtilities.makeErrorResponse(UNSUPPORTED_OPERATION,
"Invalid execution context found for operation."));
}
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetAllRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetAllRequestOperationHandler.java
index 6947204..2001645 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetAllRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetAllRequestOperationHandler.java
@@ -41,6 +41,8 @@ import org.apache.geode.internal.serialization.SerializationService;
import org.apache.geode.internal.serialization.exception.UnsupportedEncodingTypeException;
import org.apache.geode.internal.serialization.registry.exception.CodecNotRegisteredForTypeException;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
@Experimental
public class GetAllRequestOperationHandler
implements OperationHandler<RegionAPI.GetAllRequest, RegionAPI.GetAllResponse> {
@@ -54,8 +56,8 @@ public class GetAllRequestOperationHandler
Region region = executionContext.getCache().getRegion(regionName);
if (region == null) {
logger.error("Received GetAll request for non-existing region {}", regionName);
- return Failure.of(ProtobufResponseUtilities
- .makeErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND.codeValue, "Region not found"));
+ return Failure
+ .of(ProtobufResponseUtilities.makeErrorResponse(REGION_NOT_FOUND, "Region not found"));
}
Map<Boolean, List<Object>> resultsCollection = request.getKeyList().stream()
@@ -82,32 +84,23 @@ public class GetAllRequestOperationHandler
return ProtobufUtilities.createEntry(serializationService, decodedKey, value);
} catch (CodecNotRegisteredForTypeException | UnsupportedEncodingTypeException ex) {
logger.error("Encoding not supported: {}", ex);
- return BasicTypes.KeyedError.newBuilder().setKey(key)
- .setError(BasicTypes.Error.newBuilder()
- .setErrorCode(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue)
- .setMessage("Encoding not supported."))
- .build();
+ return createKeyedError(key, "Encoding not supported.", VALUE_ENCODING_ERROR);
} catch (org.apache.geode.distributed.LeaseExpiredException | TimeoutException e) {
logger.error("Operation timed out: {}", e);
- return BasicTypes.KeyedError.newBuilder().setKey(key)
- .setError(BasicTypes.Error.newBuilder()
- .setErrorCode(ProtocolErrorCode.OPERATION_TIMEOUT.codeValue)
- .setMessage("Operation timed out: " + e.getMessage()))
- .build();
+ return createKeyedError(key, "Operation timed out: " + e.getMessage(), OPERATION_TIMEOUT);
} catch (CacheLoaderException | PartitionedRegionStorageException e) {
logger.error("Data unreachable: {}", e);
- return BasicTypes.KeyedError.newBuilder().setKey(key)
- .setError(BasicTypes.Error.newBuilder()
- .setErrorCode(ProtocolErrorCode.DATA_UNREACHABLE.codeValue)
- .setMessage("Data unreachable: " + e.getMessage()))
- .build();
+ return createKeyedError(key, "Data unreachable: " + e.getMessage(), DATA_UNREACHABLE);
} catch (NullPointerException | IllegalArgumentException e) {
logger.error("Invalid input: {}", e);
- return BasicTypes.KeyedError.newBuilder().setKey(key)
- .setError(BasicTypes.Error.newBuilder()
- .setErrorCode(ProtocolErrorCode.CONSTRAINT_VIOLATION.codeValue)
- .setMessage("Invalid input: " + e.getMessage()))
- .build();
+ return createKeyedError(key, "Invalid input: " + e.getMessage(), CONSTRAINT_VIOLATION);
}
}
+
+ private Object createKeyedError(BasicTypes.EncodedValue key, String errorMessage,
+ ProtocolErrorCode errorCode) {
+ return BasicTypes.KeyedError.newBuilder().setKey(key).setError(
+ BasicTypes.Error.newBuilder().setErrorCode(errorCode.codeValue).setMessage(errorMessage))
+ .build();
+ }
}
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRegionRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRegionRequestOperationHandler.java
index 48c52c8..78d3d5c 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRegionRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRegionRequestOperationHandler.java
@@ -24,7 +24,6 @@ import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.protocol.protobuf.BasicTypes;
import org.apache.geode.internal.protocol.operations.OperationHandler;
import org.apache.geode.internal.protocol.protobuf.Failure;
-import org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode;
import org.apache.geode.internal.protocol.protobuf.RegionAPI;
import org.apache.geode.internal.protocol.protobuf.Result;
import org.apache.geode.internal.protocol.protobuf.Success;
@@ -32,6 +31,8 @@ import org.apache.geode.internal.protocol.protobuf.utilities.ProtobufResponseUti
import org.apache.geode.internal.protocol.protobuf.utilities.ProtobufUtilities;
import org.apache.geode.internal.serialization.SerializationService;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
@Experimental
public class GetRegionRequestOperationHandler
implements OperationHandler<RegionAPI.GetRegionRequest, RegionAPI.GetRegionResponse> {
@@ -46,9 +47,8 @@ public class GetRegionRequestOperationHandler
Region region = executionContext.getCache().getRegion(regionName);
if (region == null) {
logger.error("Received GetRegion request for non-existing region {}", regionName);
- return Failure.of(
- ProtobufResponseUtilities.makeErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
- "No region exists for name: " + regionName));
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(REGION_NOT_FOUND,
+ "No region exists for name: " + regionName));
}
BasicTypes.Region protoRegion = ProtobufUtilities.createRegionMessageFromRegion(region);
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRequestOperationHandler.java
index 9ded016..e7702b8 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/GetRequestOperationHandler.java
@@ -34,6 +34,8 @@ import org.apache.geode.internal.serialization.SerializationService;
import org.apache.geode.internal.serialization.exception.UnsupportedEncodingTypeException;
import org.apache.geode.internal.serialization.registry.exception.CodecNotRegisteredForTypeException;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
@Experimental
public class GetRequestOperationHandler
implements OperationHandler<RegionAPI.GetRequest, RegionAPI.GetResponse> {
@@ -47,8 +49,8 @@ public class GetRequestOperationHandler
Region region = executionContext.getCache().getRegion(regionName);
if (region == null) {
logger.error("Received Get request for non-existing region {}", regionName);
- return Failure.of(ProtobufResponseUtilities
- .makeErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND.codeValue, "Region not found"));
+ return Failure
+ .of(ProtobufResponseUtilities.makeErrorResponse(REGION_NOT_FOUND, "Region not found"));
}
try {
@@ -64,12 +66,11 @@ public class GetRequestOperationHandler
return Success.of(RegionAPI.GetResponse.newBuilder().setResult(encodedValue).build());
} catch (UnsupportedEncodingTypeException ex) {
logger.error("Received Get request with unsupported encoding: {}", ex);
- return Failure.of(ProtobufResponseUtilities.makeErrorResponse(
- ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue, "Encoding not supported."));
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(VALUE_ENCODING_ERROR,
+ "Encoding not supported."));
} catch (CodecNotRegisteredForTypeException ex) {
logger.error("Got codec error when decoding Get request: {}", ex);
- return Failure.of(ProtobufResponseUtilities.makeErrorResponse(
- ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue,
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(VALUE_ENCODING_ERROR,
"Codec error in protobuf deserialization."));
}
}
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutAllRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutAllRequestOperationHandler.java
index ca454bb..300616b 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutAllRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutAllRequestOperationHandler.java
@@ -37,6 +37,8 @@ import org.apache.geode.internal.serialization.SerializationService;
import org.apache.geode.internal.serialization.exception.UnsupportedEncodingTypeException;
import org.apache.geode.internal.serialization.registry.exception.CodecNotRegisteredForTypeException;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
@Experimental
public class PutAllRequestOperationHandler
implements OperationHandler<RegionAPI.PutAllRequest, RegionAPI.PutAllResponse> {
@@ -51,9 +53,8 @@ public class PutAllRequestOperationHandler
if (region == null) {
logger.error("Received PutAll request for non-existing region {}", regionName);
- return Failure.of(
- ProtobufResponseUtilities.createAndLogErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND,
- "Region passed does not exist: " + regionName, logger, null));
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(REGION_NOT_FOUND,
+ "Region passed does not exist: " + regionName));
}
RegionAPI.PutAllResponse.Builder builder = RegionAPI.PutAllResponse.newBuilder()
@@ -71,13 +72,12 @@ public class PutAllRequestOperationHandler
region.put(decodedKey, decodedValue);
} catch (UnsupportedEncodingTypeException ex) {
- return buildAndLogKeyedError(entry, ProtocolErrorCode.VALUE_ENCODING_ERROR,
- "Encoding not supported", ex);
+ return buildAndLogKeyedError(entry, VALUE_ENCODING_ERROR, "Encoding not supported", ex);
} catch (CodecNotRegisteredForTypeException ex) {
- return buildAndLogKeyedError(entry, ProtocolErrorCode.VALUE_ENCODING_ERROR,
+ return buildAndLogKeyedError(entry, VALUE_ENCODING_ERROR,
"Codec error in protobuf deserialization", ex);
} catch (ClassCastException ex) {
- return buildAndLogKeyedError(entry, ProtocolErrorCode.CONSTRAINT_VIOLATION,
+ return buildAndLogKeyedError(entry, CONSTRAINT_VIOLATION,
"Invalid key or value type for region", ex);
}
return null;
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutRequestOperationHandler.java
index 72fbf3d..81c842d 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/PutRequestOperationHandler.java
@@ -24,7 +24,6 @@ import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.protocol.operations.OperationHandler;
import org.apache.geode.internal.protocol.protobuf.BasicTypes;
import org.apache.geode.internal.protocol.protobuf.Failure;
-import org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode;
import org.apache.geode.internal.protocol.protobuf.RegionAPI;
import org.apache.geode.internal.protocol.protobuf.Result;
import org.apache.geode.internal.protocol.protobuf.Success;
@@ -34,6 +33,8 @@ import org.apache.geode.internal.serialization.SerializationService;
import org.apache.geode.internal.serialization.exception.UnsupportedEncodingTypeException;
import org.apache.geode.internal.serialization.registry.exception.CodecNotRegisteredForTypeException;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
@Experimental
public class PutRequestOperationHandler
implements OperationHandler<RegionAPI.PutRequest, RegionAPI.PutResponse> {
@@ -47,9 +48,8 @@ public class PutRequestOperationHandler
Region region = executionContext.getCache().getRegion(regionName);
if (region == null) {
logger.warn("Received Put request for non-existing region: {}", regionName);
- return Failure.of(
- ProtobufResponseUtilities.makeErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
- "Region passed by client did not exist: " + regionName));
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(REGION_NOT_FOUND,
+ "Region passed by client did not exist: " + regionName));
}
try {
@@ -62,14 +62,13 @@ public class PutRequestOperationHandler
return Success.of(RegionAPI.PutResponse.newBuilder().build());
} catch (ClassCastException ex) {
logger.error("Received Put request with invalid key type: {}", ex);
- return Failure.of(ProtobufResponseUtilities.makeErrorResponse(
- ProtocolErrorCode.CONSTRAINT_VIOLATION.codeValue,
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(CONSTRAINT_VIOLATION,
"invalid key or value type for region " + regionName));
}
} catch (UnsupportedEncodingTypeException | CodecNotRegisteredForTypeException ex) {
logger.error("Got codec error when decoding Put request: {}", ex);
- return Failure.of(ProtobufResponseUtilities
- .makeErrorResponse(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue, ex.getMessage()));
+ return Failure
+ .of(ProtobufResponseUtilities.makeErrorResponse(VALUE_ENCODING_ERROR, ex.getMessage()));
}
}
}
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/RemoveRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/RemoveRequestOperationHandler.java
index d1e6f49..0e3b148 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/RemoveRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/operations/RemoveRequestOperationHandler.java
@@ -23,7 +23,6 @@ import org.apache.geode.internal.cache.tier.sockets.MessageExecutionContext;
import org.apache.geode.internal.exception.InvalidExecutionContextException;
import org.apache.geode.internal.protocol.operations.OperationHandler;
import org.apache.geode.internal.protocol.protobuf.Failure;
-import org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode;
import org.apache.geode.internal.protocol.protobuf.RegionAPI;
import org.apache.geode.internal.protocol.protobuf.Result;
import org.apache.geode.internal.protocol.protobuf.Success;
@@ -33,6 +32,8 @@ import org.apache.geode.internal.serialization.SerializationService;
import org.apache.geode.internal.serialization.exception.UnsupportedEncodingTypeException;
import org.apache.geode.internal.serialization.registry.exception.CodecNotRegisteredForTypeException;
+import static org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode.*;
+
@Experimental
public class RemoveRequestOperationHandler
implements OperationHandler<RegionAPI.RemoveRequest, RegionAPI.RemoveResponse> {
@@ -47,8 +48,8 @@ public class RemoveRequestOperationHandler
Region region = executionContext.getCache().getRegion(regionName);
if (region == null) {
logger.error("Received Remove request for non-existing region {}", regionName);
- return Failure.of(ProtobufResponseUtilities
- .makeErrorResponse(ProtocolErrorCode.REGION_NOT_FOUND.codeValue, "Region not found"));
+ return Failure
+ .of(ProtobufResponseUtilities.makeErrorResponse(REGION_NOT_FOUND, "Region not found"));
}
try {
@@ -59,13 +60,12 @@ public class RemoveRequestOperationHandler
} catch (UnsupportedEncodingTypeException ex) {
// can be thrown by encoding or decoding.
logger.error("Received Remove request with unsupported encoding: {}", ex);
- return Failure.of(ProtobufResponseUtilities.createAndLogErrorResponse(
- ProtocolErrorCode.VALUE_ENCODING_ERROR, "Encoding not supported.", logger, ex));
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(VALUE_ENCODING_ERROR,
+ "Encoding not supported: " + ex.getMessage()));
} catch (CodecNotRegisteredForTypeException ex) {
logger.error("Got codec error when decoding Remove request: {}", ex);
- return Failure.of(ProtobufResponseUtilities.createAndLogErrorResponse(
- ProtocolErrorCode.VALUE_ENCODING_ERROR, "Codec error in protobuf deserialization.",
- logger, ex));
+ return Failure.of(ProtobufResponseUtilities.makeErrorResponse(VALUE_ENCODING_ERROR,
+ "Codec error in protobuf deserialization: " + ex.getMessage()));
}
}
}
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/utilities/ProtobufResponseUtilities.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/utilities/ProtobufResponseUtilities.java
index 23724fb..6819d8b 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/utilities/ProtobufResponseUtilities.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/utilities/ProtobufResponseUtilities.java
@@ -36,25 +36,6 @@ import org.apache.geode.internal.protocol.protobuf.ProtocolErrorCode;
public abstract class ProtobufResponseUtilities {
/**
- * This creates response object containing a ClientProtocol.ErrorResponse, and also logs the
- * passed error message and exception (if present) to the provided logger.
- *
- * @param errorMessage - description of the error
- * @param logger - logger to write the error message to
- * @param ex - exception which should be logged
- * @return An error response containing the first three parameters.
- */
- public static ClientProtocol.ErrorResponse createAndLogErrorResponse(ProtocolErrorCode errorCode,
- String errorMessage, Logger logger, Exception ex) {
- if (ex != null) {
- logger.error(errorMessage, ex);
- } else {
- logger.error(errorMessage);
- }
- return makeErrorResponse(errorCode.codeValue, errorMessage);
- }
-
- /**
* This creates a response object containing a RegionAPI.GetRegionNamesResponse
*
* @param regionSet - A set of regions
@@ -70,9 +51,11 @@ public abstract class ProtobufResponseUtilities {
return builder.build();
}
- public static ClientProtocol.ErrorResponse makeErrorResponse(int errorCode, String message) {
+ public static ClientProtocol.ErrorResponse makeErrorResponse(ProtocolErrorCode errorCode,
+ String message) {
return ClientProtocol.ErrorResponse.newBuilder()
- .setError(BasicTypes.Error.newBuilder().setErrorCode(errorCode).setMessage(message))
+ .setError(
+ BasicTypes.Error.newBuilder().setErrorCode(errorCode.codeValue).setMessage(message))
.build();
}
}
--
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].