[
https://issues.apache.org/jira/browse/GEODE-10096?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17500316#comment-17500316
]
ASF GitHub Bot commented on GEODE-10096:
----------------------------------------
pdxcodemonkey commented on a change in pull request #938:
URL: https://github.com/apache/geode-native/pull/938#discussion_r817962442
##########
File path: cppcache/src/TcrConnection.hpp
##########
@@ -33,22 +33,28 @@
#include "util/synchronized_set.hpp"
#define DEFAULT_TIMEOUT_RETRIES 12
-#define PRIMARY_SERVER_TO_CLIENT 101
-#define SECONDARY_SERVER_TO_CLIENT 102
-#define SUCCESSFUL_SERVER_TO_CLIENT 105
-#define UNSUCCESSFUL_SERVER_TO_CLIENT 106
-#define CLIENT_TO_SERVER 100
-#define REPLY_OK 59
-#define REPLY_REFUSED 60
-#define REPLY_INVALID 61
-#define REPLY_SSL_ENABLED 21
-#define REPLY_AUTHENTICATION_REQUIRED 62
-#define REPLY_AUTHENTICATION_FAILED 63
-#define REPLY_DUPLICATE_DURABLE_CLIENT 64
-
-#define SECURITY_CREDENTIALS_NONE 0
-#define SECURITY_CREDENTIALS_NORMAL 1
-#define SECURITY_MULTIUSER_NOTIFICATIONCHANNEL 3
+
+enum class acceptor : ::std::int8_t {
+ CLIENT_TO_SERVER = 100,
+ PRIMARY_SERVER_TO_CLIENT = 101,
+ SECONDARY_SERVER_TO_CLIENT = 102
+};
+enum class acceptance_codes : ::std::int8_t {
+ REPLY_SSL_ENABLED = 21,
+ REPLY_OK = 59,
+ REPLY_REFUSED = 60,
+ REPLY_INVALID = 61,
+ REPLY_AUTHENTICATION_REQUIRED = 62,
+ REPLY_AUTHENTICATION_FAILED = 63,
+ REPLY_DUPLICATE_DURABLE_CLIENT = 64,
+ SUCCESSFUL_SERVER_TO_CLIENT = 105,
+ UNSUCCESSFUL_SERVER_TO_CLIENT = 106
+};
+enum class security : ::std::uint8_t {
+ SECURITY_CREDENTIALS_NONE = 0,
+ SECURITY_CREDENTIALS_NORMAL = 1,
+ SECURITY_MULTIUSER_NOTIFICATIONCHANNEL = 3
+};
Review comment:
Really love the separation of concerns here, it's much easier now to
tell in the implementation that the switch statements are correct, for
instance. Quick question - do any of these values need to be in the header?
They don't seem to be referenced anywhere outside of TcrConnection code...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Handshake "acceptance codes" should be an enum class
> ----------------------------------------------------
>
> Key: GEODE-10096
> URL: https://issues.apache.org/jira/browse/GEODE-10096
> Project: Geode
> Issue Type: Improvement
> Components: native client
> Reporter: Blake Bender
> Priority: Major
>
> In the method TcrConnection::initTcrConnection, the following block of code
> appears:
> {code:java}
> switch (acceptanceCode[0]) {
> case REPLY_OK:
> case SUCCESSFUL_SERVER_TO_CLIENT:
> LOGFINER("Handshake reply: %u,%u,%u", acceptanceCode[0],
> serverQueueStatus[0], recvMsgLen2);
> if (isClientNotification)
> readHandshakeInstantiatorMsg(connectTimeout);
> break;
> case REPLY_AUTHENTICATION_FAILED: {
> AuthenticationFailedException ex(
> reinterpret_cast<char*>(recvMessage.data()));
> m_conn.reset();
> throwException(ex);
> }
> case REPLY_AUTHENTICATION_REQUIRED: {
> AuthenticationRequiredException ex(
> reinterpret_cast<char*>(recvMessage.data()));
> m_conn.reset();
> throwException(ex);
> }
> case REPLY_DUPLICATE_DURABLE_CLIENT: {
> DuplicateDurableClientException ex(
> reinterpret_cast<char*>(recvMessage.data()));
> m_conn.reset();
> throwException(ex);
> }
> case REPLY_REFUSED:
> case REPLY_INVALID:
> case UNSUCCESSFUL_SERVER_TO_CLIENT: {
> LOGERROR("Handshake rejected by server[%s]: %s",
> m_endpointObj->name().c_str(),
> reinterpret_cast<char*>(recvMessage.data()));
> auto message = std::string("TcrConnection::TcrConnection: ") +
> "Handshake rejected by server: " +
> reinterpret_cast<char*>(recvMessage.data());
> CacheServerException ex(message);
> m_conn.reset();
> throw ex;
> }
> {code}
> These response codes are unique to the server handshake, and not used
> anywhere else in the code. We need to remove the #defines for them and put
> them in a proper enum class.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
