You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/07/22 22:14:32 UTC
[GitHub] [geode] Bill commented on a change in pull request #6684: GEODE-6588: Cleanup multi-user auth.
Bill commented on a change in pull request #6684:
URL: https://github.com/apache/geode/pull/6684#discussion_r675182145
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
##########
@@ -689,46 +679,38 @@ private StringBuffer getExceptionMessage(String exceptionName, int retryCount,
return message;
}
- private void authenticateIfRequired(Connection conn, Op op) {
- if (!conn.getServer().getRequiresCredentials()) {
+ private void authenticateIfRequired(final Connection connection, final Op op) {
+ if (!connection.getServer().getRequiresCredentials()) {
return;
}
- if (pool == null) {
Review comment:
comment: oh cool `pool` is `final` and set in the constructor, from a `@NotNull` parameter, so it can't be `null` yippee!
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProxyMembershipID.java
##########
@@ -176,149 +177,114 @@ public static ClientProxyMembershipID getClientId(DistributedMember member) {
return new ClientProxyMembershipID(member);
}
- public static byte[] initializeAndGetDSIdentity(DistributedSystem sys) {
- byte[] client_side_identity = null;
+ public static byte[] initializeAndGetDSIdentity(final DistributedSystem sys) {
+ final byte[] client_side_identity;
if (sys == null) {
- // DistributedSystem is required now before handshaking -Kirk
+ // DistributedSystem is required now before handshaking
throw new IllegalStateException(
"Attempting to handshake with CacheServer before creating DistributedSystem and Cache.");
}
- {
- systemMemberId = sys.getDistributedMember();
- try (HeapDataOutputStream hdos = new HeapDataOutputStream(256, KnownVersion.CURRENT)) {
- if (systemMemberId != null) {
- // update the durable id of the member identifier before serializing in case
- // a pool name has been established
- DurableClientAttributes attributes = systemMemberId.getDurableClientAttributes();
- if (attributes != null && attributes.getId().length() > 0) {
- ((InternalDistributedMember) systemMemberId).setDurableId(attributes.getId());
- }
+ systemMemberId = sys.getDistributedMember();
+ try (HeapDataOutputStream hdos = new HeapDataOutputStream(256, KnownVersion.CURRENT)) {
+ if (systemMemberId != null) {
+ // update the durable id of the member identifier before serializing in case
+ // a pool name has been established
+ DurableClientAttributes attributes = systemMemberId.getDurableClientAttributes();
+ if (attributes != null && attributes.getId().length() > 0) {
+ ((InternalDistributedMember) systemMemberId).setDurableId(attributes.getId());
}
- DataSerializer.writeObject(systemMemberId, hdos);
- client_side_identity = hdos.toByteArray();
- } catch (IOException ioe) {
- throw new InternalGemFireException(
- "Unable to serialize identity",
- ioe);
}
-
- system = sys;
+ DataSerializer.writeObject(systemMemberId, hdos);
+ client_side_identity = hdos.toByteArray();
+ } catch (IOException ioe) {
+ throw new InternalGemFireException("Unable to serialize identity", ioe);
}
+ system = sys;
return client_side_identity;
-
}
private ClientProxyMembershipID(int id, byte[] clientSideIdentity) {
- boolean specialCase = Boolean.getBoolean(GeodeGlossary.GEMFIRE_PREFIX + "SPECIAL_DURABLE");
- String durableID = this.system.getProperties().getProperty(DURABLE_CLIENT_ID);
+ this(clientSideIdentity, getUniqueId(id), systemMemberId);
+ }
+
+ private static int getUniqueId(final int id) {
+ final boolean specialCase =
+ Boolean.getBoolean(GeodeGlossary.GEMFIRE_PREFIX + "SPECIAL_DURABLE");
+ final String durableID = system.getProperties().getProperty(DURABLE_CLIENT_ID);
if (specialCase && durableID != null && (!durableID.equals(""))) {
- this.uniqueId = durable_synch_counter;
+ return durable_synch_counter;
} else {
- this.uniqueId = id;
+ return id;
}
- this.identity = clientSideIdentity;
- this.memberId = systemMemberId;
}
public ClientProxyMembershipID() {}
- public ClientProxyMembershipID(DistributedMember member) {
- this.uniqueId = 1;
- this.memberId = member;
+ public ClientProxyMembershipID(final DistributedMember member) {
+ this(null, 1, member);
updateID(member);
}
+ @VisibleForTesting
+ ClientProxyMembershipID(final byte[] identity, final int uniqueId,
+ final DistributedMember memberId) {
+ this.identity = identity;
+ this.uniqueId = uniqueId;
+ this.memberId = memberId;
+ }
private transient String _toString;
- // private transient int transientPort; // variable for debugging member ID issues
-
@Override
public String toString() {
- if (this.identity != null
+ if (identity != null
&& ((InternalDistributedMember) getDistributedMember()).getMembershipPort() == 0) {
- return this.toStringNoCache();
+ return toStringNoCache();
}
- if (this._toString == null) {
- this._toString = this.toStringNoCache();
+ if (_toString == null) {
+ _toString = toStringNoCache();
}
- return this._toString;
+ return _toString;
}
/**
* returns a string representation of this identifier, ignoring the toString cache
*/
public String toStringNoCache() {
- StringBuffer sb = new StringBuffer("identity(").append(getDSMembership()).append(",connection=")
- .append(uniqueId);
+ StringBuilder sb =
Review comment:
request: No need for a `StringBuilder` here. If you're bothering to change this code I recommend changing it to use the `+` operator.
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
##########
@@ -519,161 +509,161 @@ protected void handleException(Op op, Throwable e, Connection conn, int retryCou
handleException(e, conn, retryCount, finalAttempt, timeoutFatal);
}
- protected void handleException(Throwable e, Connection conn, int retryCount, boolean finalAttempt,
- boolean timeoutFatal) throws CacheRuntimeException {
- GemFireException exToThrow = null;
- String title;
- boolean invalidateServer = true;
- boolean warn = true;
- boolean forceThrow = false;
- Throwable cause = e;
+ protected void handleException(final Throwable throwable, final Connection connection,
+ final int retryCount, final boolean finalAttempt,
+ final boolean timeoutFatal) throws CacheRuntimeException {
- cancelCriterion.checkCancelInProgress(e);
+ cancelCriterion.checkCancelInProgress(throwable);
- if (logger.isDebugEnabled() && !(e instanceof java.io.EOFException)) {
- if (e instanceof java.net.SocketTimeoutException) {
+ if (logger.isDebugEnabled() && !(throwable instanceof java.io.EOFException)) {
+ if (throwable instanceof java.net.SocketTimeoutException) {
logger.debug("OpExecutor.handleException on Connection to {} read timed out",
- conn.getServer());
+ connection.getServer());
} else {
- logger.debug("OpExecutor.handleException on Connection to {}", conn.getServer(), e);
+ logger.debug("OpExecutor.handleException on Connection to {}", connection.getServer(),
+ throwable);
}
}
// first take care of all exceptions that should not invalidate the
// connection and do not need to be logged
- if (e instanceof MessageTooLargeException) {
+ GemFireException exceptionToThrow = null;
+ String title;
+ boolean invalidateServer = true;
+ boolean warn = true;
+ boolean forceThrow = false;
+ if (throwable instanceof MessageTooLargeException) {
title = null;
- exToThrow = new GemFireIOException("message is too large to transmit", e);
- } else if (e instanceof NotSerializableException) {
+ exceptionToThrow = new GemFireIOException("message is too large to transmit", throwable);
+ } else if (throwable instanceof NotSerializableException) {
title = null; // no message
- exToThrow = new SerializationException("Pool message failure", e);
- } else if (e instanceof BatchException || e instanceof BatchException70) {
+ exceptionToThrow = new SerializationException("Pool message failure", throwable);
+ } else if (throwable instanceof BatchException || throwable instanceof BatchException70) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof RegionDestroyedException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof RegionDestroyedException) {
invalidateServer = false;
title = null;
- exToThrow = (RegionDestroyedException) e;
- } else if (e instanceof GemFireSecurityException) {
+ exceptionToThrow = (RegionDestroyedException) throwable;
+ } else if (throwable instanceof GemFireSecurityException) {
title = null;
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof SerializationException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof SerializationException) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof CopyException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof CopyException) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof ClassNotFoundException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof ClassNotFoundException) {
title = null; // no message
- exToThrow = new ServerOperationException(e);
- } else if (e instanceof TransactionException) {
+ exceptionToThrow = new ServerOperationException(throwable);
+ } else if (throwable instanceof TransactionException) {
title = null; // no message
- exToThrow = (TransactionException) e;
+ exceptionToThrow = (TransactionException) throwable;
invalidateServer = false;
- } else if (e instanceof SynchronizationCommitConflictException) {
+ } else if (throwable instanceof SynchronizationCommitConflictException) {
title = null;
- exToThrow = (SynchronizationCommitConflictException) e;
+ exceptionToThrow = (SynchronizationCommitConflictException) throwable;
invalidateServer = false;
- } else if (e instanceof SocketException) {
- if ("Socket closed".equals(e.getMessage()) || "Connection reset".equals(e.getMessage())
- || "Connection refused: connect".equals(e.getMessage())
- || "Connection refused".equals(e.getMessage())) {
- title = e.getMessage();
+ } else if (throwable instanceof SocketException) {
+ if ("Socket closed".equals(throwable.getMessage())
+ || "Connection reset".equals(throwable.getMessage())
+ || "Connection refused: connect".equals(throwable.getMessage())
+ || "Connection refused".equals(throwable.getMessage())) {
+ title = throwable.getMessage();
} else {
title = "SocketException";
}
- } else if (e instanceof SocketTimeoutException) {
+ } else if (throwable instanceof SocketTimeoutException) {
invalidateServer = timeoutFatal;
title = "socket timed out on client";
- cause = null;
- } else if (e instanceof ConnectionDestroyedException) {
+ } else if (throwable instanceof ConnectionDestroyedException) {
invalidateServer = false;
title = "connection was asynchronously destroyed";
- cause = null;
- } else if (e instanceof java.io.EOFException) {
+ } else if (throwable instanceof java.io.EOFException) {
title = "closed socket on server";
- } else if (e instanceof IOException) {
+ } else if (throwable instanceof IOException) {
title = "IOException";
- } else if (e instanceof BufferUnderflowException) {
+ } else if (throwable instanceof BufferUnderflowException) {
title = "buffer underflow reading from server";
- } else if (e instanceof CancelException) {
+ } else if (throwable instanceof CancelException) {
title = "Cancelled";
warn = false;
- } else if (e instanceof InternalFunctionInvocationTargetException) {
+ } else if (throwable instanceof InternalFunctionInvocationTargetException) {
// In this case, function will be re executed
title = null;
- exToThrow = (InternalFunctionInvocationTargetException) e;
- } else if (e instanceof FunctionInvocationTargetException) {
+ exceptionToThrow = (InternalFunctionInvocationTargetException) throwable;
+ } else if (throwable instanceof FunctionInvocationTargetException) {
// in this case function will not be re executed
title = null;
- exToThrow = (GemFireException) e;
- } else if (e instanceof PutAllPartialResultException) {
+ exceptionToThrow = (GemFireException) throwable;
+ } else if (throwable instanceof PutAllPartialResultException) {
title = null;
- exToThrow = (PutAllPartialResultException) e;
+ exceptionToThrow = (PutAllPartialResultException) throwable;
invalidateServer = false;
} else {
- Throwable t = e.getCause();
+ Throwable t = throwable.getCause();
if ((t instanceof IOException)
|| (t instanceof SerializationException) || (t instanceof CopyException)
|| (t instanceof GemFireSecurityException) || (t instanceof ServerOperationException)
|| (t instanceof TransactionException) || (t instanceof CancelException)) {
- handleException(t, conn, retryCount, finalAttempt, timeoutFatal);
+ handleException(t, connection, retryCount, finalAttempt, timeoutFatal);
return;
- } else if (e instanceof ServerOperationException) {
+ } else if (throwable instanceof ServerOperationException) {
title = null; // no message
- exToThrow = (ServerOperationException) e;
+ exceptionToThrow = (ServerOperationException) throwable;
invalidateServer = false; // fix for bug #42225
- } else if (e instanceof FunctionException) {
+ } else if (throwable instanceof FunctionException) {
if (t instanceof InternalFunctionInvocationTargetException) {
// Client server to re-execute for node failure
- handleException(t, conn, retryCount, finalAttempt, timeoutFatal);
+ handleException(t, connection, retryCount, finalAttempt, timeoutFatal);
return;
} else {
title = null; // no message
- exToThrow = (FunctionException) e;
+ exceptionToThrow = (FunctionException) throwable;
}
- } else if (e instanceof ServerConnectivityException
- && e.getMessage().equals("Connection error while authenticating user")) {
+ } else if (throwable instanceof ServerConnectivityException
+ && throwable.getMessage().equals("Connection error while authenticating user")) {
title = null;
if (logger.isDebugEnabled()) {
- logger.debug(e.getMessage(), e);
+ logger.debug(throwable.getMessage(), throwable);
}
} else {
- title = e.toString();
+ title = throwable.toString();
forceThrow = true;
}
}
if (title != null) {
- conn.destroy();
+ connection.destroy();
if (invalidateServer) {
- endpointManager.serverCrashed(conn.getEndpoint());
+ endpointManager.serverCrashed(connection.getEndpoint());
}
boolean logEnabled = warn ? logger.isWarnEnabled() : logger.isDebugEnabled();
boolean msgNeeded = logEnabled || finalAttempt;
if (msgNeeded) {
- final StringBuffer sb = getExceptionMessage(title, retryCount, finalAttempt, conn);
+ final StringBuilder sb = getExceptionMessage(title, retryCount, finalAttempt, connection);
final String msg = sb.toString();
if (logEnabled) {
if (warn) {
logger.warn(msg);
} else {
- logger.debug(msg, e);
+ logger.debug(msg, throwable);
}
}
if (forceThrow || finalAttempt) {
- exToThrow = new ServerConnectivityException(msg, cause);
+ exceptionToThrow = new ServerConnectivityException(msg, throwable);
}
}
}
- if (exToThrow != null) {
- throw exToThrow;
+ if (exceptionToThrow != null) {
+ throw exceptionToThrow;
}
}
- private StringBuffer getExceptionMessage(String exceptionName, int retryCount,
- boolean finalAttempt, Connection connection) {
- StringBuffer message = new StringBuffer(200);
+ private StringBuilder getExceptionMessage(final String exceptionName, final int retryCount,
Review comment:
request: `StringBuilder` isn't needed here. Please replace with plain old string concatenation using the `+` operator and return a `String`. That'll eliminate one variable from the calling code too.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProxyMembershipID.java
##########
@@ -176,149 +177,114 @@ public static ClientProxyMembershipID getClientId(DistributedMember member) {
return new ClientProxyMembershipID(member);
}
- public static byte[] initializeAndGetDSIdentity(DistributedSystem sys) {
- byte[] client_side_identity = null;
+ public static byte[] initializeAndGetDSIdentity(final DistributedSystem sys) {
+ final byte[] client_side_identity;
if (sys == null) {
- // DistributedSystem is required now before handshaking -Kirk
+ // DistributedSystem is required now before handshaking
throw new IllegalStateException(
"Attempting to handshake with CacheServer before creating DistributedSystem and Cache.");
}
- {
- systemMemberId = sys.getDistributedMember();
- try (HeapDataOutputStream hdos = new HeapDataOutputStream(256, KnownVersion.CURRENT)) {
- if (systemMemberId != null) {
- // update the durable id of the member identifier before serializing in case
- // a pool name has been established
- DurableClientAttributes attributes = systemMemberId.getDurableClientAttributes();
- if (attributes != null && attributes.getId().length() > 0) {
- ((InternalDistributedMember) systemMemberId).setDurableId(attributes.getId());
- }
+ systemMemberId = sys.getDistributedMember();
+ try (HeapDataOutputStream hdos = new HeapDataOutputStream(256, KnownVersion.CURRENT)) {
+ if (systemMemberId != null) {
+ // update the durable id of the member identifier before serializing in case
+ // a pool name has been established
+ DurableClientAttributes attributes = systemMemberId.getDurableClientAttributes();
+ if (attributes != null && attributes.getId().length() > 0) {
+ ((InternalDistributedMember) systemMemberId).setDurableId(attributes.getId());
}
- DataSerializer.writeObject(systemMemberId, hdos);
- client_side_identity = hdos.toByteArray();
- } catch (IOException ioe) {
- throw new InternalGemFireException(
- "Unable to serialize identity",
- ioe);
}
-
- system = sys;
+ DataSerializer.writeObject(systemMemberId, hdos);
+ client_side_identity = hdos.toByteArray();
+ } catch (IOException ioe) {
+ throw new InternalGemFireException("Unable to serialize identity", ioe);
}
+ system = sys;
return client_side_identity;
-
}
private ClientProxyMembershipID(int id, byte[] clientSideIdentity) {
- boolean specialCase = Boolean.getBoolean(GeodeGlossary.GEMFIRE_PREFIX + "SPECIAL_DURABLE");
- String durableID = this.system.getProperties().getProperty(DURABLE_CLIENT_ID);
+ this(clientSideIdentity, getUniqueId(id), systemMemberId);
+ }
+
+ private static int getUniqueId(final int id) {
+ final boolean specialCase =
+ Boolean.getBoolean(GeodeGlossary.GEMFIRE_PREFIX + "SPECIAL_DURABLE");
+ final String durableID = system.getProperties().getProperty(DURABLE_CLIENT_ID);
if (specialCase && durableID != null && (!durableID.equals(""))) {
- this.uniqueId = durable_synch_counter;
+ return durable_synch_counter;
} else {
- this.uniqueId = id;
+ return id;
}
- this.identity = clientSideIdentity;
- this.memberId = systemMemberId;
}
public ClientProxyMembershipID() {}
- public ClientProxyMembershipID(DistributedMember member) {
- this.uniqueId = 1;
- this.memberId = member;
+ public ClientProxyMembershipID(final DistributedMember member) {
+ this(null, 1, member);
updateID(member);
}
+ @VisibleForTesting
+ ClientProxyMembershipID(final byte[] identity, final int uniqueId,
+ final DistributedMember memberId) {
+ this.identity = identity;
+ this.uniqueId = uniqueId;
+ this.memberId = memberId;
+ }
private transient String _toString;
- // private transient int transientPort; // variable for debugging member ID issues
-
@Override
public String toString() {
- if (this.identity != null
+ if (identity != null
&& ((InternalDistributedMember) getDistributedMember()).getMembershipPort() == 0) {
- return this.toStringNoCache();
+ return toStringNoCache();
}
- if (this._toString == null) {
- this._toString = this.toStringNoCache();
+ if (_toString == null) {
+ _toString = toStringNoCache();
}
- return this._toString;
+ return _toString;
}
/**
* returns a string representation of this identifier, ignoring the toString cache
*/
public String toStringNoCache() {
- StringBuffer sb = new StringBuffer("identity(").append(getDSMembership()).append(",connection=")
- .append(uniqueId);
+ StringBuilder sb =
+ new StringBuilder("identity(").append(getDSMembership()).append(",connection=")
+ .append(uniqueId);
if (identity != null) {
DurableClientAttributes dca = getDurableAttributes();
if (dca.getId().length() > 0) {
- sb.append(",durableAttributes=").append(dca).append(')').toString();
+ sb.append(",durableAttributes=").append(dca).append(')');
}
}
return sb.toString();
}
- /**
- * For Externalizable
- *
- * @see Externalizable
- */
@Override
public void writeExternal(ObjectOutput out) throws IOException {
- // if (this.transientPort == 0) {
- // InternalDistributedSystem.getLogger().warning(
- // String.format("%s",
- // "externalizing a client ID with zero port: " + this.toString(),
- // new Exception("Stack trace")));
- // }
- Assert.assertTrue(this.identity.length <= BYTES_32KB);
- out.writeShort(this.identity.length);
- out.write(this.identity);
- out.writeInt(this.uniqueId);
-
- }
+ if (identity.length > Short.MAX_VALUE) {
+ throw new IOException("HandShake identity length is too big");
+ }
- /** returns the externalized size of this object */
- public int getSerializedSize() {
- return 4 + identity.length + 4;
+ out.writeShort(identity.length);
+ out.write(identity);
+ out.writeInt(uniqueId);
}
- /**
- * For Externalizable
- *
- * @see Externalizable
- */
@Override
public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
int identityLength = in.readShort();
- if (identityLength > BYTES_32KB) {
- throw new IOException(
- "HandShake identity length is too big");
Review comment:
comment: dead code. nice catch!
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProxyMembershipID.java
##########
@@ -176,149 +177,114 @@ public static ClientProxyMembershipID getClientId(DistributedMember member) {
return new ClientProxyMembershipID(member);
}
- public static byte[] initializeAndGetDSIdentity(DistributedSystem sys) {
- byte[] client_side_identity = null;
+ public static byte[] initializeAndGetDSIdentity(final DistributedSystem sys) {
+ final byte[] client_side_identity;
if (sys == null) {
- // DistributedSystem is required now before handshaking -Kirk
+ // DistributedSystem is required now before handshaking
throw new IllegalStateException(
"Attempting to handshake with CacheServer before creating DistributedSystem and Cache.");
}
- {
- systemMemberId = sys.getDistributedMember();
- try (HeapDataOutputStream hdos = new HeapDataOutputStream(256, KnownVersion.CURRENT)) {
- if (systemMemberId != null) {
- // update the durable id of the member identifier before serializing in case
- // a pool name has been established
- DurableClientAttributes attributes = systemMemberId.getDurableClientAttributes();
- if (attributes != null && attributes.getId().length() > 0) {
- ((InternalDistributedMember) systemMemberId).setDurableId(attributes.getId());
- }
+ systemMemberId = sys.getDistributedMember();
+ try (HeapDataOutputStream hdos = new HeapDataOutputStream(256, KnownVersion.CURRENT)) {
+ if (systemMemberId != null) {
+ // update the durable id of the member identifier before serializing in case
+ // a pool name has been established
+ DurableClientAttributes attributes = systemMemberId.getDurableClientAttributes();
+ if (attributes != null && attributes.getId().length() > 0) {
+ ((InternalDistributedMember) systemMemberId).setDurableId(attributes.getId());
}
- DataSerializer.writeObject(systemMemberId, hdos);
- client_side_identity = hdos.toByteArray();
- } catch (IOException ioe) {
- throw new InternalGemFireException(
- "Unable to serialize identity",
- ioe);
}
-
- system = sys;
+ DataSerializer.writeObject(systemMemberId, hdos);
+ client_side_identity = hdos.toByteArray();
+ } catch (IOException ioe) {
+ throw new InternalGemFireException("Unable to serialize identity", ioe);
}
+ system = sys;
return client_side_identity;
-
}
private ClientProxyMembershipID(int id, byte[] clientSideIdentity) {
- boolean specialCase = Boolean.getBoolean(GeodeGlossary.GEMFIRE_PREFIX + "SPECIAL_DURABLE");
- String durableID = this.system.getProperties().getProperty(DURABLE_CLIENT_ID);
+ this(clientSideIdentity, getUniqueId(id), systemMemberId);
+ }
+
+ private static int getUniqueId(final int id) {
+ final boolean specialCase =
+ Boolean.getBoolean(GeodeGlossary.GEMFIRE_PREFIX + "SPECIAL_DURABLE");
+ final String durableID = system.getProperties().getProperty(DURABLE_CLIENT_ID);
if (specialCase && durableID != null && (!durableID.equals(""))) {
- this.uniqueId = durable_synch_counter;
+ return durable_synch_counter;
} else {
- this.uniqueId = id;
+ return id;
}
- this.identity = clientSideIdentity;
- this.memberId = systemMemberId;
}
public ClientProxyMembershipID() {}
- public ClientProxyMembershipID(DistributedMember member) {
- this.uniqueId = 1;
- this.memberId = member;
+ public ClientProxyMembershipID(final DistributedMember member) {
+ this(null, 1, member);
updateID(member);
Review comment:
comment: I love what you did to chain the constructors. I see you had to introduce a private pure function `getUniqueId()` to help.
I find it hokey that in this method we set `identity` to `null`, only to turn around and call the (mutating) `updateId()` right after that.
I was about to suggest extracting the guts of `updateID()` into a pure function you could call here to do likewise, until I went and looked at `updateID()` and saw it mutates no fewer than four (4) member variables. Ugh.
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/internal/OpExecutorImpl.java
##########
@@ -751,32 +733,28 @@ private void authenticateMultiuser(PoolImpl pool, Connection conn, UserAttribute
}
}
- private Object executeWithPossibleReAuthentication(Connection conn, Op op) throws Exception {
+ private Object executeWithPossibleReAuthentication(final Connection conn, final Op op)
+ throws Exception {
try {
return conn.execute(op);
-
- } catch (ServerConnectivityException sce) {
- Throwable cause = sce.getCause();
+ } catch (final ServerConnectivityException sce) {
+ final Throwable cause = sce.getCause();
if ((cause instanceof AuthenticationRequiredException
&& "User authorization attributes not found.".equals(cause.getMessage()))
|| sce.getMessage().contains("Connection error while authenticating user")) {
- // (ashetkar) Need a cleaner way of doing above check.
// 2nd exception-message above is from AbstractOp.sendMessage()
- PoolImpl pool =
- (PoolImpl) PoolManagerImpl.getPMI().find(endpointManager.getPoolName());
Review comment:
comment: I _think_ this is going to end up with the same pool. so ok awesome!
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org