You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by jb...@apache.org on 2021/01/28 01:05:40 UTC
[geode] 09/16: GEODE-6588: Static analyzer cleanup.
This is an automated email from the ASF dual-hosted git repository.
jbarrett pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 088e08b10ee9af6e16d37f18ab54f5552c6e56b7
Author: Jacob Barrett <jb...@pivotal.io>
AuthorDate: Sat Jan 16 09:58:12 2021 -0800
GEODE-6588: Static analyzer cleanup.
---
.../cache/tier/sockets/BaseCommandQuery.java | 105 ++++++++-------------
.../cache/tier/sockets/ChunkedMessage.java | 82 +++++++---------
.../cache/tier/sockets/ServerConnection.java | 12 +--
3 files changed, 75 insertions(+), 124 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
index b2933fd..bbf81ef 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/BaseCommandQuery.java
@@ -16,7 +16,6 @@ package org.apache.geode.internal.cache.tier.sockets;
import java.io.IOException;
import java.util.Collection;
-import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -57,7 +56,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
protected boolean processQuery(final Message msg,
final Query query,
final String queryString,
- final Set regionNames,
+ final Set<String> regionNames,
final long start,
final ServerCQ cqQuery,
final QueryOperationContext queryContext,
@@ -77,7 +76,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
protected boolean processQueryUsingParams(final Message msg,
final Query query,
final String queryString,
- final Set regionNames,
+ final Set<String> regionNames,
long start,
final ServerCQ cqQuery,
QueryOperationContext queryContext,
@@ -114,8 +113,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
// For now we assume the results are a SelectResults
// which is the only possibility now, but this may change
// in the future if we support arbitrary queries
- Object result = null;
-
+ Object result;
if (params != null) {
result = query.execute(params);
} else {
@@ -126,9 +124,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
// of the regions involved in the query have been destroyed
// or not. If yes, throw an Exception.
// This is a workaround/fix for Bug 36969
- Iterator itr = regionNames.iterator();
- while (itr.hasNext()) {
- String regionName = (String) itr.next();
+ for (final String regionName : regionNames) {
if (crHelper.getRegion(regionName) == null) {
throw new RegionDestroyedException(
"Region destroyed during the execution of the query",
@@ -148,17 +144,13 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
if (result instanceof SelectResults) {
- SelectResults selectResults = (SelectResults) result;
+ SelectResults<?> selectResults = (SelectResults<?>) result;
if (logger.isDebugEnabled()) {
logger.debug("Query Result size for : {} is {}", query.getQueryString(),
selectResults.size());
}
- CollectionType collectionType = null;
- boolean sendCqResultsWithKey = true;
- boolean isStructs = false;
-
// check if resultset has serialized objects, so that they could be sent
// as ObjectPartList
boolean hasSerializedObjects = ((DefaultQuery) query).isKeepSerialized();
@@ -177,20 +169,15 @@ public abstract class BaseCommandQuery extends BaseCommand {
// Get the collection type (which includes the element type)
// (used to generate the appropriate instance on the client)
- collectionType = getCollectionType(selectResults);
- isStructs = collectionType.getElementType().isStructType();
+ CollectionType collectionType = getCollectionType(selectResults);
+ boolean isStructs = collectionType.getElementType().isStructType();
// Check if the Query is from CQ execution.
if (cqQuery != null) {
- // Check if the key can be sent to the client based on its version.
- sendCqResultsWithKey = sendCqResultsWithKey(servConn);
-
- if (sendCqResultsWithKey) {
- // Update the collection type to include key info.
- collectionType = new CollectionTypeImpl(Collection.class,
- new StructTypeImpl(new String[] {"key", "value"}));
- isStructs = collectionType.getElementType().isStructType();
- }
+ // Update the collection type to include key info.
+ collectionType = new CollectionTypeImpl(Collection.class,
+ new StructTypeImpl(new String[] {"key", "value"}));
+ isStructs = collectionType.getElementType().isStructType();
}
int numberOfChunks = (int) Math.ceil(selectResults.size() * 1.0 / MAXIMUM_CHUNK_SIZE);
@@ -226,11 +213,11 @@ public abstract class BaseCommandQuery extends BaseCommand {
// send it as a part of ObjectPartList
if (hasSerializedObjects) {
sendResultsAsObjectPartList(numberOfChunks, servConn, selectResults.asList(), isStructs,
- collectionType, queryString, cqQuery, sendCqResultsWithKey, sendResults,
+ collectionType, queryString, cqQuery, sendResults,
securityService);
} else {
sendResultsAsObjectArray(selectResults, numberOfChunks, servConn, isStructs,
- collectionType, queryString, cqQuery, sendCqResultsWithKey, sendResults);
+ collectionType, queryString, cqQuery, sendResults);
}
}
@@ -260,9 +247,8 @@ public abstract class BaseCommandQuery extends BaseCommand {
logger.warn(String.format("Unexpected QueryInvalidException while processing query %s",
queryString),
e);
- QueryInvalidException qie =
- new QueryInvalidException(String.format("%s : QueryString is: %s.",
- new Object[] {e.getLocalizedMessage(), queryString}));
+ QueryInvalidException qie = new QueryInvalidException(
+ String.format("%s : QueryString is: %s.", e.getLocalizedMessage(), queryString));
writeQueryResponseException(msg, qie, servConn);
return false;
} catch (DistributedSystemDisconnectedException se) {
@@ -287,13 +273,6 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
writeQueryResponseException(msg, e, servConn);
return false;
- } finally {
- // Since the query object is being shared in case of bind queries,
- // resetting the flag may cause inconsistency.
- // Also since this flag is only being set in code path executed by
- // remote query execution, resetting it is not required.
-
- // ((DefaultQuery)query).setRemoteQuery(false);
}
if (logger.isDebugEnabled()) {
@@ -304,14 +283,10 @@ public abstract class BaseCommandQuery extends BaseCommand {
return true;
}
- protected CollectionType getCollectionType(SelectResults results) {
+ protected CollectionType getCollectionType(SelectResults<?> results) {
return results.getCollectionType();
}
- private boolean sendCqResultsWithKey(ServerConnection servConn) {
- return true;
- }
-
protected void sendCqResponse(int msgType, String msgStr, int txId, Throwable e,
ServerConnection servConn) throws IOException {
ChunkedMessage cqMsg = servConn.getChunkedResponseMessage();
@@ -361,9 +336,11 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
}
- private void sendResultsAsObjectArray(SelectResults selectResults, int numberOfChunks,
- ServerConnection servConn, boolean isStructs, CollectionType collectionType,
- String queryString, ServerCQ cqQuery, boolean sendCqResultsWithKey, boolean sendResults)
+ private void sendResultsAsObjectArray(SelectResults<?> selectResults, int numberOfChunks,
+ ServerConnection servConn, boolean isStructs,
+ CollectionType collectionType,
+ String queryString, ServerCQ cqQuery,
+ boolean sendResults)
throws IOException {
int resultIndex = 0;
// For CQ only as we dont want CQEntries which have null values.
@@ -404,11 +381,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
// Add to the Results object array.
- if (sendCqResultsWithKey) {
- results[i] = e.getKeyValuePair();
- } else {
- results[i] = e.getValue();
- }
+ results[i] = e.getKeyValuePair();
} else {
// instance check added to fix bug 40516.
if (isStructs && (objs[resultIndex] instanceof Struct)) {
@@ -424,15 +397,13 @@ public abstract class BaseCommandQuery extends BaseCommand {
// of entries in the chunk does not divide evenly into the
// number of entries in the result set.
if (incompleteArray) {
- Object[] newResults;
- if (cqQuery != null) {
- newResults = new Object[cqResultIndex % MAXIMUM_CHUNK_SIZE];
- } else {
+ final Object[] newResults;
+ if (cqQuery == null) {
newResults = new Object[resultIndex % MAXIMUM_CHUNK_SIZE];
+ } else {
+ newResults = new Object[cqResultIndex % MAXIMUM_CHUNK_SIZE];
}
- for (int i = 0; i < newResults.length; i++) {
- newResults[i] = results[i];
- }
+ System.arraycopy(results, 0, newResults, 0, newResults.length);
results = newResults;
}
@@ -453,12 +424,14 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
}
- private void sendResultsAsObjectPartList(int numberOfChunks, ServerConnection servConn, List objs,
- boolean isStructs, CollectionType collectionType, String queryString, ServerCQ cqQuery,
- boolean sendCqResultsWithKey, boolean sendResults, final SecurityService securityService)
+ private void sendResultsAsObjectPartList(int numberOfChunks, ServerConnection servConn,
+ List<?> objs,
+ boolean isStructs, CollectionType collectionType,
+ String queryString, ServerCQ cqQuery,
+ boolean sendResults,
+ final SecurityService securityService)
throws IOException {
int resultIndex = 0;
- Object result = null;
for (int j = 0; j < numberOfChunks; j++) {
if (logger.isTraceEnabled()) {
logger.trace("{}: Creating chunk: {}", servConn.getName(), j);
@@ -472,6 +445,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
logger.trace("{}: Adding entry [{}] to query results: {}", servConn.getName(),
resultIndex, objs.get(resultIndex));
}
+ Object result;
if (cqQuery != null) {
CqEntry e = (CqEntry) objs.get(resultIndex);
// The value may have become null because of entry invalidation.
@@ -490,16 +464,12 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
// Add to the Results object array.
- if (sendCqResultsWithKey) {
- result = e.getKeyValuePair();
- } else {
- result = e.getValue();
- }
+ result = e.getKeyValuePair();
} else {
result = objs.get(resultIndex);
}
if (sendResults) {
- addToObjectPartList(serializedObjs, result, collectionType, false, servConn, isStructs,
+ addToObjectPartList(serializedObjs, result, isStructs,
securityService);
}
resultIndex++;
@@ -518,8 +488,7 @@ public abstract class BaseCommandQuery extends BaseCommand {
}
private void addToObjectPartList(ObjectPartList serializedObjs, Object res,
- CollectionType collectionType, boolean lastChunk, ServerConnection servConn,
- boolean isStructs, final SecurityService securityService) throws IOException {
+ boolean isStructs, final SecurityService securityService) {
if (isStructs && (res instanceof Struct)) {
Object[] values = ((Struct) res).getFieldValues();
// create another ObjectPartList for the struct
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ChunkedMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ChunkedMessage.java
index 793891d..27d2419 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ChunkedMessage.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ChunkedMessage.java
@@ -94,12 +94,9 @@ public class ChunkedMessage extends Message {
@Override
public String toString() {
- StringBuffer sb = new StringBuffer();
-
- sb.append(super.toString());
- sb.append("; chunkLength= " + chunkLength);
- sb.append("; lastChunk=" + lastChunk);
- return sb.toString();
+ return super.toString()
+ + "; chunkLength= " + chunkLength
+ + "; lastChunk=" + lastChunk;
}
/**
@@ -144,14 +141,14 @@ public class ChunkedMessage extends Message {
// if not then inform client so that it will refresh pr-meta-data
if (((b & 2) == 2)) {
- this.lastChunk |= 0x04;// setting third bit, we are okay
+ lastChunk |= 0x04;// setting third bit, we are okay
}
}
}
public void setLastChunkAndNumParts(boolean lastChunk, int numParts) {
setLastChunk(lastChunk);
- if (this.serverConnection != null) {
+ if (serverConnection != null) {
// we us e three bits for number of parts in last chunk byte
byte localLastChunk = (byte) (numParts << 5);
this.lastChunk |= localLastChunk;
@@ -159,7 +156,7 @@ public class ChunkedMessage extends Message {
}
public void setServerConnection(ServerConnection servConn) {
- if (this.serverConnection != servConn)
+ if (serverConnection != servConn)
throw new IllegalStateException("this.sc was not correctly set");
}
@@ -169,27 +166,14 @@ public class ChunkedMessage extends Message {
* @return whether this is the last chunk
*/
public boolean isLastChunk() {
- if ((this.lastChunk & 0X01) == 0X01) {
- return true;
- }
-
- return false;
- }
-
- /**
- * Returns the chunk length.
- *
- * @return the chunk length
- */
- public int getChunkLength() {
- return this.chunkLength;
+ return (lastChunk & 0X01) == 0X01;
}
/**
* Populates the header with information received via socket
*/
public void readHeader() throws IOException {
- if (this.socket != null) {
+ if (socket != null) {
final ByteBuffer cb = getCommBuffer();
synchronized (cb) {
fetchHeader();
@@ -199,16 +183,15 @@ public class ChunkedMessage extends Message {
cb.clear();
if (!MessageType.validate(type)) {
throw new IOException(
- String.format("Invalid message type %s while reading header",
- Integer.valueOf(type)));
+ String.format("Invalid message type %s while reading header", type));
}
// Set the header and payload fields only after receiving all the
// socket data, providing better message consistency in the face
// of exceptional conditions (e.g. IO problems, timeouts etc.)
- this.messageType = type;
- this.numberOfParts = numParts; // Already set in setPayloadFields via setNumberOfParts
- this.transactionId = txid;
+ messageType = type;
+ numberOfParts = numParts; // Already set in setPayloadFields via setNumberOfParts
+ transactionId = txid;
}
} else {
throw new IOException("Dead Connection");
@@ -219,7 +202,7 @@ public class ChunkedMessage extends Message {
* Reads a chunk of this message.
*/
public void receiveChunk() throws IOException {
- if (this.socket != null) {
+ if (socket != null) {
synchronized (getCommBuffer()) {
readChunk();
}
@@ -237,28 +220,27 @@ public class ChunkedMessage extends Message {
cb.clear();
int totalBytesRead = 0;
do {
- int bytesRead = 0;
- bytesRead =
+ int bytesRead =
inputStream.read(cb.array(), totalBytesRead, CHUNK_HEADER_LENGTH - totalBytesRead);
if (bytesRead == -1) {
throw new EOFException(
"Chunk read error (connection reset)");
}
totalBytesRead += bytesRead;
- if (this.messageStats != null) {
- this.messageStats.incReceivedBytes(bytesRead);
+ if (messageStats != null) {
+ messageStats.incReceivedBytes(bytesRead);
}
} while (totalBytesRead < CHUNK_HEADER_LENGTH);
cb.rewind();
// Set chunk length and last chunk
- this.chunkLength = cb.getInt();
+ chunkLength = cb.getInt();
// setLastChunk(cb.get() == 0x01);
byte lastChunk = cb.get();
setLastChunk((lastChunk & 0x01) == 0x01);
if ((lastChunk & 0x02) == 0x02) {
- this.securePart = new Part();
+ securePart = new Part();
if (logger.isDebugEnabled()) {
logger.debug("ChunkedMessage.readChunk() securePart present");
}
@@ -267,17 +249,17 @@ public class ChunkedMessage extends Message {
if ((lastChunk & 0x01) == 0x01) {
int numParts = lastChunk >> 5;
if (numParts > 0) {
- this.numberOfParts = numParts;
+ numberOfParts = numParts;
}
}
- readPayloadFields(this.numberOfParts, this.chunkLength);
+ readPayloadFields(numberOfParts, chunkLength);
}
/**
* Sends the header of this message.
*/
public void sendHeader() throws IOException {
- if (this.socket != null) {
+ if (socket != null) {
synchronized (getCommBuffer()) {
getDSCODEsForWrite();
flushBuffer();
@@ -285,8 +267,8 @@ public class ChunkedMessage extends Message {
// so I've deadcoded it for performance.
// this.os.flush();
}
- this.currentPart = 0;
- this.headerSent = true;
+ currentPart = 0;
+ headerSent = true;
} else {
throw new IOException("Dead Connection");
}
@@ -296,7 +278,7 @@ public class ChunkedMessage extends Message {
* Return true if the header for this message has already been sent.
*/
public boolean headerHasBeenSent() {
- return this.headerSent;
+ return headerSent;
}
/**
@@ -304,7 +286,7 @@ public class ChunkedMessage extends Message {
*/
public void sendChunk() throws IOException {
if (isLastChunk()) {
- this.headerSent = false;
+ headerSent = false;
}
sendBytes(true);
}
@@ -313,14 +295,14 @@ public class ChunkedMessage extends Message {
* Sends a chunk of this message.
*/
public void sendChunk(ServerConnection servConn) throws IOException {
- if (this.serverConnection != servConn)
+ if (serverConnection != servConn)
throw new IllegalStateException("this.sc was not correctly set");
sendChunk();
}
@Override
protected Part getSecurityPart() {
- if (this.isLastChunk())
+ if (isLastChunk())
return super.getSecurityPart();
else
return null;
@@ -328,7 +310,7 @@ public class ChunkedMessage extends Message {
@Override
protected int checkAndSetSecurityPart() {
- return (this.securePart != null) ? 1 : 0;
+ return (securePart != null) ? 1 : 0;
}
@Override
@@ -338,7 +320,7 @@ public class ChunkedMessage extends Message {
byte isLastChunk = 0x00;
if (isLastChunk()) {
// isLastChunk = (byte) 0x01 ;
- isLastChunk = this.lastChunk;
+ isLastChunk = lastChunk;
if (isSecurityHeader) {
isLastChunk |= 0x02;
}
@@ -353,9 +335,9 @@ public class ChunkedMessage extends Message {
protected void getDSCODEsForWrite() {
final ByteBuffer cb = getCommBuffer();
cb.clear();
- cb.putInt(this.messageType);
- cb.putInt(this.numberOfParts);
+ cb.putInt(messageType);
+ cb.putInt(numberOfParts);
- cb.putInt(this.transactionId);
+ cb.putInt(transactionId);
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
index e682805..29eec6a 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
@@ -105,7 +105,7 @@ public abstract class ServerConnection implements Runnable {
public static boolean allowInternalMessagesWithoutCredentials =
!Boolean.getBoolean(DISALLOW_INTERNAL_MESSAGES_WITHOUT_CREDENTIALS_NAME);
- private Map commands;
+ private Map<Integer, Command> commands;
protected final SecurityService securityService;
@@ -442,7 +442,7 @@ public abstract class ServerConnection implements Runnable {
}
}
- protected Map getCommands() {
+ protected Map<Integer, Command> getCommands() {
return commands;
}
@@ -839,7 +839,7 @@ public abstract class ServerConnection implements Runnable {
} else {
logger.warn(
"Failed to bind the subject of uniqueId {} for message {} with {} : Possible re-authentication required",
- uniqueId, messageType, this.getName());
+ uniqueId, messageType, getName());
throw new AuthenticationRequiredException("Failed to find the authenticated user.");
}
}
@@ -987,11 +987,11 @@ public abstract class ServerConnection implements Runnable {
void initializeCommands() {
// The commands are cached here, but are just referencing the ones stored in the
// CommandInitializer
- commands = CommandInitializer.getDefaultInstance().get(this.getClientVersion());
+ commands = CommandInitializer.getDefaultInstance().get(getClientVersion());
}
private Command getCommand(Integer messageType) {
- return (Command) commands.get(messageType);
+ return commands.get(messageType);
}
public void removeUserAuth(Message message, boolean keepAlive) {
@@ -1697,7 +1697,7 @@ public abstract class ServerConnection implements Runnable {
if (isTerminated()) {
throw new IOException("Server connection is terminated.");
}
- logger.debug("Unexpected exception {}", npe);
+ logger.debug("Unexpected exception {}", npe.toString());
}
if (uaa == null) {
throw new AuthenticationRequiredException("User authorization attributes not found.");