[
https://issues.apache.org/jira/browse/GEODE-4090?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16293482#comment-16293482
]
ASF GitHub Bot commented on GEODE-4090:
---------------------------------------
upthewaterspout closed pull request #1172: GEODE-4090: Add ErrorCode mirroring
ProtocolErrorCode enum into protobuf
URL: https://github.com/apache/geode/pull/1172
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
b/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
index d6be91b232..9ab7d2f203 100644
--- a/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
+++ b/geode-protobuf-messages/src/main/proto/v1/basicTypes.proto
@@ -68,7 +68,7 @@ message Server {
}
message Error {
- int32 errorCode = 1;
+ ErrorCode errorCode = 1;
string message = 2;
}
@@ -76,3 +76,27 @@ message KeyedError {
EncodedValue key = 1;
Error error = 2;
}
+
+enum ErrorCode {
+ INVALID_ERROR_CODE = 0;
+ GENERIC_FAILURE = 1000;
+ VALUE_ENCODING_ERROR = 1100;
+ UNSUPPORTED_VERSION = 1101;
+ UNSUPPORTED_OPERATION = 1102;
+ UNSUPPORTED_AUTHENTICATION_MODE = 1103;
+ HANDSHAKE_REQUIRED = 1104;
+ AUTHENTICATION_FAILED = 1200;
+ AUTHORIZATION_FAILED = 1201;
+ ALREADY_AUTHENTICATED = 1202;
+ AUTHENTICATION_NOT_SUPPORTED = 1203;
+ LOW_MEMORY = 1300;
+ DATA_UNREACHABLE = 1301;
+ OPERATION_TIMEOUT = 1302;
+ CONSTRAINT_VIOLATION = 2000;
+ BAD_QUERY = 2001;
+ REGION_NOT_FOUND = 2100;
+ QUERY_PARAMETER_MISMATCH = 2200;
+ QUERY_BIND_FAILURE = 2201;
+ QUERY_NOT_PERMITTED = 2202;
+ QUERY_TIMEOUT = 2203;
+}
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 88eef66a5f..2b717fc546 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
@@ -104,8 +104,8 @@ private Object processOneMessage(SerializationService
serializationService, Regi
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))
+ return
BasicTypes.KeyedError.newBuilder().setKey(key).setError(BasicTypes.Error.newBuilder()
+
.setErrorCode(ProtobufUtilities.getProtobufErrorCode(errorCode)).setMessage(errorMessage))
.build();
}
}
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
index c556c71d82..0af9ab8b8a 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutAllRequestOperationHandler.java
@@ -91,8 +91,8 @@
logger.error(message, ex);
return BasicTypes.KeyedError.newBuilder().setKey(entry.getKey())
- .setError(
-
BasicTypes.Error.newBuilder().setErrorCode(errorCode.codeValue).setMessage(message))
+ .setError(BasicTypes.Error.newBuilder()
+
.setErrorCode(ProtobufUtilities.getProtobufErrorCode(errorCode)).setMessage(message))
.build();
}
}
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufResponseUtilities.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufResponseUtilities.java
index 23ab351360..7b24d6032b 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufResponseUtilities.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufResponseUtilities.java
@@ -53,8 +53,8 @@
public static ClientProtocol.ErrorResponse
makeErrorResponse(ProtocolErrorCode errorCode,
String message) {
return ClientProtocol.ErrorResponse.newBuilder()
- .setError(
-
BasicTypes.Error.newBuilder().setErrorCode(errorCode.codeValue).setMessage(message))
+ .setError(BasicTypes.Error.newBuilder()
+
.setErrorCode(ProtobufUtilities.getProtobufErrorCode(errorCode)).setMessage(message))
.build();
}
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
index 183e039708..3b900eaed1 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/utilities/ProtobufUtilities.java
@@ -19,6 +19,7 @@
import org.apache.geode.annotations.Experimental;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionAttributes;
+import org.apache.geode.internal.protocol.ProtocolErrorCode;
import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
import org.apache.geode.internal.protocol.protobuf.v1.EncodingTypeTranslator;
@@ -170,6 +171,16 @@ public static Object decodeValue(SerializationService
serializationService,
}
}
+ /**
+ * This will convert a ProtocolErrorCode to a BasicTypes.ErrorCode for
protobuf
+ *
+ * @param errorCode - incoming ProtocolErrorCode
+ * @return matching protobuf error code
+ */
+ public static BasicTypes.ErrorCode getProtobufErrorCode(ProtocolErrorCode
errorCode) {
+ return BasicTypes.ErrorCode.forNumber(errorCode.codeValue);
+ }
+
/**
* @return a Protobuf BasicTypes.Region message that represents the {@link
Region}
*/
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
index f3f68f401f..c1c4672a10 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthenticationIntegrationTest.java
@@ -189,7 +189,7 @@ public void skippingAuthenticationFails() throws Exception {
ClientProtocol.Message errorResponse =
protobufProtocolSerializer.deserialize(inputStream);
assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
errorResponse.getResponse().getResponseAPICase());
- assertEquals(AUTHENTICATION_FAILED.codeValue,
+ assertEquals(BasicTypes.ErrorCode.AUTHENTICATION_FAILED,
errorResponse.getResponse().getErrorResponse().getError().getErrorCode());
verifyConnectionClosed();
}
@@ -289,7 +289,7 @@ public void legacyClientAuthenticatorSet() throws Exception
{
ClientProtocol.Message errorResponse =
protobufProtocolSerializer.deserialize(inputStream);
assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
errorResponse.getResponse().getResponseAPICase());
- assertEquals(UNSUPPORTED_AUTHENTICATION_MODE.codeValue,
+ assertEquals(BasicTypes.ErrorCode.UNSUPPORTED_AUTHENTICATION_MODE,
errorResponse.getResponse().getErrorResponse().getError().getErrorCode());
verifyConnectionClosed();
}
@@ -310,7 +310,7 @@ public void legacyPeerAuthenticatorSet() throws Exception {
ClientProtocol.Message errorResponse =
protobufProtocolSerializer.deserialize(inputStream);
assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
errorResponse.getResponse().getResponseAPICase());
- assertEquals(UNSUPPORTED_AUTHENTICATION_MODE.codeValue,
+ assertEquals(BasicTypes.ErrorCode.UNSUPPORTED_AUTHENTICATION_MODE,
errorResponse.getResponse().getErrorResponse().getError().getErrorCode());
verifyConnectionClosed();
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
index cc69333b6a..bc3a5158b3 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
@@ -206,7 +206,7 @@ private void
validateOperationAuthorized(ClientProtocol.Message message, InputSt
ClientProtocol.Message response =
protobufProtocolSerializer.deserialize(inputStream);
assertEquals(expectedResponseType,
response.getResponse().getResponseAPICase());
if (expectedResponseType ==
ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE) {
- Assert.assertEquals(ProtocolErrorCode.AUTHORIZATION_FAILED.codeValue,
+ Assert.assertEquals(BasicTypes.ErrorCode.AUTHORIZATION_FAILED,
response.getResponse().getErrorResponse().getError().getErrorCode());
}
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ErrorCodesJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ErrorCodesJUnitTest.java
new file mode 100644
index 0000000000..b341faa426
--- /dev/null
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/ErrorCodesJUnitTest.java
@@ -0,0 +1,45 @@
+/*
+ * 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;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.internal.protocol.ProtocolErrorCode;
+import org.apache.geode.test.junit.categories.UnitTest;
+
+@Category(UnitTest.class)
+public class ErrorCodesJUnitTest {
+ @Test
+ public void testProtobufErrorCodesMatchProtocolErrorCodes() {
+ BasicTypes.ErrorCode[] protobufErrorCodes = BasicTypes.ErrorCode.values();
+ ProtocolErrorCode[] protocolErrorCodes = ProtocolErrorCode.values();
+
+ // These arrays should identical except the protobuf is required to have a
0 value, and defines
+ // an UNRECOGNIZED entry
+ assertEquals(protobufErrorCodes.length - 2, protocolErrorCodes.length);
+ for (int i = 0; i < protobufErrorCodes.length; ++i) {
+ if (i == 0) {
+ assertEquals(BasicTypes.ErrorCode.INVALID_ERROR_CODE,
protobufErrorCodes[i]);
+ } else if (i == protobufErrorCodes.length - 1) {
+ assertEquals(BasicTypes.ErrorCode.UNRECOGNIZED, protobufErrorCodes[i]);
+ } else {
+ assertEquals(protobufErrorCodes[i].getNumber(), protocolErrorCodes[i -
1].codeValue);
+ }
+ }
+ }
+}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
index fc52d33068..bbfd7cd2a1 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
@@ -38,6 +38,7 @@
import org.apache.geode.internal.protocol.ProtocolErrorCode;
import
org.apache.geode.internal.protocol.exception.InvalidProtocolMessageException;
import
org.apache.geode.internal.protocol.protobuf.statistics.ProtobufClientStatisticsImpl;
+import org.apache.geode.internal.protocol.protobuf.v1.BasicTypes;
import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
import org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI;
import org.apache.geode.internal.protocol.protobuf.v1.LocatorAPI;
@@ -149,7 +150,7 @@ public void testInvalidOperationReturnsFailure()
ClientProtocol.Response messageResponse =
getAvailableServersResponseMessage.getResponse();
assertEquals(ClientProtocol.Response.ResponseAPICase.ERRORRESPONSE,
messageResponse.getResponseAPICase());
- assertEquals(ProtocolErrorCode.UNSUPPORTED_OPERATION.codeValue,
+ assertEquals(BasicTypes.ErrorCode.UNSUPPORTED_OPERATION,
messageResponse.getErrorResponse().getError().getErrorCode());
validateStats(messagesReceived + 1, messagesSent + 1,
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionRequestOperationHandlerJUnitTest.java
index ec141fdad2..bdcec29ee9 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRegionRequestOperationHandlerJUnitTest.java
@@ -98,7 +98,7 @@ public void processReturnsNoCacheRegions() throws Exception {
Assert.assertTrue(result instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) result.getErrorMessage();
- Assert.assertEquals(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
+ Assert.assertEquals(BasicTypes.ErrorCode.REGION_NOT_FOUND,
errorMessage.getError().getErrorCode());
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
index aafdbdbe33..72f275948f 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/GetRequestOperationHandlerJUnitTest.java
@@ -87,7 +87,7 @@ public void
processReturnsUnsucessfulResponseForInvalidRegion() throws Exception
Assert.assertTrue(response instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) response.getErrorMessage();
- Assert.assertEquals(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
+ Assert.assertEquals(BasicTypes.ErrorCode.REGION_NOT_FOUND,
errorMessage.getError().getErrorCode());
}
@@ -128,7 +128,7 @@ public void processReturnsErrorWhenUnableToDecodeRequest()
throws Exception {
Assert.assertTrue(response instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) response.getErrorMessage();
- Assert.assertEquals(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue,
+ Assert.assertEquals(BasicTypes.ErrorCode.VALUE_ENCODING_ERROR,
errorMessage.getError().getErrorCode());
}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandlerJUnitTest.java
index 8e5a6903dc..ef07fcabdf 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/PutRequestOperationHandlerJUnitTest.java
@@ -98,8 +98,7 @@ public void test_invalidEncodingType() throws Exception {
assertTrue(result instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) result.getErrorMessage();
- assertEquals(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue,
- errorMessage.getError().getErrorCode());
+ assertEquals(BasicTypes.ErrorCode.VALUE_ENCODING_ERROR,
errorMessage.getError().getErrorCode());
}
@Test
@@ -112,8 +111,7 @@ public void test_RegionNotFound() throws Exception {
assertTrue(result instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) result.getErrorMessage();
- assertEquals(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
- errorMessage.getError().getErrorCode());
+ assertEquals(BasicTypes.ErrorCode.REGION_NOT_FOUND,
errorMessage.getError().getErrorCode());
}
@Test
@@ -127,8 +125,7 @@ public void test_RegionThrowsClasscastException() throws
Exception {
assertTrue(result instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) result.getErrorMessage();
- assertEquals(ProtocolErrorCode.CONSTRAINT_VIOLATION.codeValue,
- errorMessage.getError().getErrorCode());
+ assertEquals(BasicTypes.ErrorCode.CONSTRAINT_VIOLATION,
errorMessage.getError().getErrorCode());
}
private RegionAPI.PutRequest generateTestRequest()
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandlerJUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandlerJUnitTest.java
index 64bb3520bc..e8bd857d39 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandlerJUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/RemoveRequestOperationHandlerJUnitTest.java
@@ -83,8 +83,7 @@ public void
processReturnsUnsucessfulResponseForInvalidRegion() throws Exception
assertTrue(result instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) result.getErrorMessage();
- assertEquals(ProtocolErrorCode.REGION_NOT_FOUND.codeValue,
- errorMessage.getError().getErrorCode());
+ assertEquals(BasicTypes.ErrorCode.REGION_NOT_FOUND,
errorMessage.getError().getErrorCode());
}
@Test
@@ -116,8 +115,7 @@ public void processReturnsErrorWhenUnableToDecodeRequest()
throws Exception {
assertTrue(result instanceof Failure);
ClientProtocol.ErrorResponse errorMessage =
(ClientProtocol.ErrorResponse) result.getErrorMessage();
- assertEquals(ProtocolErrorCode.VALUE_ENCODING_ERROR.codeValue,
- errorMessage.getError().getErrorCode());
+ assertEquals(BasicTypes.ErrorCode.VALUE_ENCODING_ERROR,
errorMessage.getError().getErrorCode());
}
private ClientProtocol.Request generateTestRequest(boolean missingRegion,
boolean missingKey)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Use an enum for Error.errorCode, rather than an int32
> -----------------------------------------------------
>
> Key: GEODE-4090
> URL: https://issues.apache.org/jira/browse/GEODE-4090
> Project: Geode
> Issue Type: New Feature
> Components: client/server
> Reporter: Brian Baynes
>
> Currently, the error codes in the new protocol are returned as integers and
> are documented on the wiki in
> https://cwiki.apache.org/confluence/display/GEODE/Error+Codes and in the code
> in ProtocolErrorCodes.
> Protobuf has support for enums, we should use an enum instead of an int32 for
> the error code so that the list of possible errors is part of the protobuf
> specification. This will make it clearer to the user what each error code
> means and will prevent them from having to put the constants in their own
> code.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
