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 <[email protected]> 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.");
