From c00639253837645b90b01717d9e1fde868a9e75f Mon Sep 17 00:00:00 2001 From: Donal Evans Date: Mon, 10 Aug 2020 17:25:02 -0700 Subject: [PATCH] GEODE-7864: Fix most "Statement has empty body" IDE warnings Authored-by: Donal Evans --- ...essionReplicationIntegrationJUnitTest.java | 5 +- .../session/bootstrap/AbstractCache.java | 1 + .../util/ResourceManagerValidator.java | 8 +- .../web/controllers/AddFreeItemToOrders.java | 20 +- .../cache/query/dunit/PdxQueryDUnitTest.java | 16 +- .../query/dunit/SelectStarQueryDUnitTest.java | 82 +++------ ...eStatisticsPartitionedRegionDUnitTest.java | 6 +- .../cache30/ClearMultiVmCallBkDUnitTest.java | 8 +- .../cache30/DistAckMapMethodsDUnitTest.java | 7 +- .../ClientServerTransactionDUnitTest.java | 5 +- .../cache/RemoteTransactionDUnitTest.java | 8 +- .../cache/execute/PRTransactionDUnitTest.java | 6 +- ...stentPartitionedRegionDistributedTest.java | 3 - .../jta/dunit/MaxPoolSizeDUnitTest.java | 5 +- .../management/RegionManagementDUnitTest.java | 16 +- .../apache/geode/cache/ProxyJUnitTest.java | 5 +- .../functional/NestedQueryJUnitTest.java | 7 +- .../FilterProfileIntegrationJUnitTest.java | 57 +++--- .../offheap/OffHeapValidationJUnitTest.java | 32 ++-- .../internal/ra/spi/JCALocalTransaction.java | 8 +- .../java/org/apache/geode/CopyHelper.java | 5 +- .../admin/jmx/internal/AgentLauncher.java | 13 +- .../geode/admin/jmx/internal/MBeanUtil.java | 4 +- .../internal/SerialAsyncEventQueueImpl.java | 5 +- .../geode/cache/client/internal/PutAllOp.java | 31 ++-- .../cache/client/internal/RemoveAllOp.java | 30 +-- .../pooling/ConnectionManagerImpl.java | 30 +-- .../cache/query/internal/DerivedInfo.java | 18 +- .../query/internal/index/HashIndexSet.java | 5 +- .../query/internal/index/IndexManager.java | 4 +- .../query/internal/index/RangeIndex.java | 18 +- .../internal/ClusterDistributionManager.java | 12 +- .../distributed/internal/ReplyMessage.java | 6 +- .../internal/StartupResponseMessage.java | 4 +- .../internal/locks/DLockGrantor.java | 51 +++--- .../geode/internal/DistributionLocator.java | 2 - .../internal/InternalDataSerializer.java | 102 +++++------ .../admin/remote/DistributionLocatorId.java | 15 +- .../admin/remote/RemoteTransportConfig.java | 3 - .../internal/cache/AbstractRegionMap.java | 7 +- .../geode/internal/cache/BucketRegion.java | 3 - .../geode/internal/cache/DiskInitFile.java | 6 +- .../geode/internal/cache/DiskRegion.java | 3 +- .../cache/DiskWriteAttributesImpl.java | 6 - .../cache/DistributedCacheOperation.java | 9 +- .../geode/internal/cache/EntryEventImpl.java | 4 +- .../apache/geode/internal/cache/HARegion.java | 2 - .../internal/cache/NonLocalRegionEntry.java | 5 +- .../apache/geode/internal/cache/Oplog.java | 5 +- .../cache/PRHARedundancyProvider.java | 13 +- .../internal/cache/PartitionedRegion.java | 27 +-- .../geode/internal/cache/QueuedOperation.java | 4 +- .../cache/SearchLoadAndWriteProcessor.java | 6 +- .../internal/cache/entries/DiskEntry.java | 66 ++++--- .../internal/cache/ha/HARegionQueue.java | 12 +- .../cache/partitioned/IndexCreationMsg.java | 11 +- .../cache/tier/sockets/ClientUserAuths.java | 2 - .../cache/tier/sockets/command/PutAll70.java | 9 - .../cache/tier/sockets/command/PutAll80.java | 9 - .../sockets/command/PutUserCredentials.java | 4 - .../parallel/ParallelGatewaySenderQueue.java | 19 +- .../SerialGatewaySenderEventProcessor.java | 10 +- .../cache/xmlcache/CacheXmlParser.java | 173 +++++++----------- .../xmlcache/CacheXmlPropertyResolver.java | 4 +- .../cache/xmlcache/XmlGeneratorUtils.java | 5 +- .../concurrent/CompactConcurrentHashSet2.java | 5 +- .../FacetsJCAConnectionManagerImpl.java | 2 - .../internal/jta/TransactionManagerImpl.java | 5 +- .../geode/internal/net/SocketCloser.java | 4 +- .../statistics/StatArchiveHandler.java | 20 +- .../statistics/StatisticsMonitor.java | 4 +- .../platform/LinuxProcFsStatistics.java | 6 +- .../geode/internal/stats50/VMStats50.java | 3 +- .../geode/pdx/internal/PdxInstanceImpl.java | 21 ++- .../org/apache/geode/cache30/TestHeapLRU.java | 11 -- .../cache/RemoteCQTransactionDUnitTest.java | 4 +- .../security/FilterPostAuthorization.java | 2 - .../internal/index/IndexTrackingTestHook.java | 9 +- .../experimental/driver/ProtobufDriver.java | 8 +- .../codeAnalysis/CompiledClassUtils.java | 5 +- .../test/concurrent/CPUContentionService.java | 1 + .../geode/test/junit/rules/RetryRule.java | 3 - .../membership/gms/GMSMembership.java | 19 +- .../membership/gms/MemberIdentifierImpl.java | 20 +- .../gms/membership/GMSJoinLeave.java | 15 +- .../pulse/tests/junit/BaseServiceTest.java | 5 - .../geode/redis/session/SessionDUnitTest.java | 5 +- .../redis/internal/GeodeServerRunTest.java | 5 +- .../wan/misc/WANConfigurationJUnitTest.java | 23 +-- .../GatewaySenderEventRemoteDispatcher.java | 36 ++-- .../wan/serial/SerialGatewaySenderImpl.java | 5 +- 91 files changed, 523 insertions(+), 810 deletions(-) diff --git a/extensions/geode-modules-session/src/integrationTest/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java b/extensions/geode-modules-session/src/integrationTest/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java index 91fc19f32820..00f3b21a2bcb 100644 --- a/extensions/geode-modules-session/src/integrationTest/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java +++ b/extensions/geode-modules-session/src/integrationTest/java/org/apache/geode/modules/session/internal/filter/SessionReplicationIntegrationJUnitTest.java @@ -1097,9 +1097,8 @@ private List getCookies(HttpTester.Response response) { c.setPath(value); } else if ("secure".equalsIgnoreCase(param)) { c.setSecure(true); - } else if ("httponly".equalsIgnoreCase(param)) { - // Ignored?? - } else { + // Ignore the case where param == "httponly"? + } else if (!"httponly".equalsIgnoreCase(param)) { if (c == null) { c = new Cookie(param, value); } else { diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/bootstrap/AbstractCache.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/bootstrap/AbstractCache.java index 44812ca93025..a8feb5becc76 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/bootstrap/AbstractCache.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/session/bootstrap/AbstractCache.java @@ -111,6 +111,7 @@ public boolean isStarted() { public void close() { getCache().close(); while (!getCache().isClosed()) { + // Intentionally empty block } diff --git a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/ResourceManagerValidator.java b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/ResourceManagerValidator.java index c67c5ec9f1cd..4428c273d591 100644 --- a/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/ResourceManagerValidator.java +++ b/extensions/geode-modules/src/main/java/org/apache/geode/modules/util/ResourceManagerValidator.java @@ -42,13 +42,9 @@ public static void validateJavaStartupParameters(GemFireCache cache) { if (vmVendor.startsWith("Sun") || vmVendor.startsWith("Apple")) { // java.vm.vendor = Sun Microsystems Inc. || java.vm.vendor = Apple Inc. validateSunArguments(cache, rm, inputArguments); - } else if (vmVendor.startsWith("IBM")) { - // java.vm.vendor = IBM Corporation - // TODO validate IBM input arguments - } else if (vmVendor.startsWith("BEA")) { - // java.vm.vendor = BEA Systems, Inc. - // TODO validate JRockit input arguments } + // TODO validate IBM input arguments (vmVendor.startsWith("IBM")) + // TODO validate JRockit input arguments (vmVendor.startsWith("BEA")) } private static void validateSunArguments(GemFireCache cache, ResourceManager rm, diff --git a/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java b/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java index 1ec38485ac20..2d8dee736f36 100644 --- a/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java +++ b/geode-assembly/geode-assembly-test/src/main/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java @@ -45,21 +45,17 @@ public class AddFreeItemToOrders implements Function { @Override public void execute(FunctionContext context) { Region region = null; - List vals = new ArrayList(); - List keys = new ArrayList(); - List argsList = new ArrayList(); - Object[] argsArray = null; + List vals = new ArrayList<>(); + List keys = new ArrayList<>(); + List argsList = new ArrayList<>(); + Object[] argsArray; - if (context.getArguments() instanceof Boolean) { - - } else if (context.getArguments() instanceof String) { - String arg = (String) context.getArguments(); - } else if (context.getArguments() instanceof Vector) { - - } else if (context.getArguments() instanceof Object[]) { + if (context.getArguments() instanceof Object[]) { argsArray = (Object[]) context.getArguments(); argsList = Arrays.asList(argsArray); - } else { + } else if (!((context.getArguments() instanceof Boolean) + || (context.getArguments() instanceof Vector) + || (context.getArguments() instanceof String))) { System.out.println("AddFreeItemToOrders : Invalid Arguments"); } diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java index 19bf8f8608f3..881379b2d5bf 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/PdxQueryDUnitTest.java @@ -3062,9 +3062,7 @@ public Object call() throws Exception { assertEquals(5, sr.size()); if (i == 1) { for (Object o : sr) { - if (o == null) { - } else if (o instanceof String) { - } else { + if (o != null && !(o instanceof String)) { fail("Result should be either null or String and not " + o.getClass()); } } @@ -3095,9 +3093,7 @@ public Object call() throws Exception { assertEquals(5, sr.size()); if (i == 1) { for (Object o : sr) { - if (o == null) { - } else if (o instanceof String) { - } else { + if (o != null && !(o instanceof String)) { fail("Result should be either null or String and not " + o.getClass()); } } @@ -3129,9 +3125,7 @@ public Object call() throws Exception { assertEquals(5, sr.size()); if (i == 1) { for (Object o : sr) { - if (o == null) { - } else if (o instanceof String) { - } else { + if (o != null && !(o instanceof String)) { fail("Result should be either null or String and not " + o.getClass()); } } @@ -3302,9 +3296,7 @@ public Object call() throws Exception { assertEquals(5, sr.size()); if (i == 1) { for (Object o : sr) { - if (o == null) { - } else if (o instanceof String) { - } else { + if (o != null && !(o instanceof String)) { fail("Result should be either null or String and not " + o.getClass()); } } diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/SelectStarQueryDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/SelectStarQueryDUnitTest.java index 45b9421ecbc8..860d84db6037 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/SelectStarQueryDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/cache/query/dunit/SelectStarQueryDUnitTest.java @@ -320,14 +320,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -377,14 +375,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -486,14 +482,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -545,14 +539,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1006,14 +998,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1076,14 +1066,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + multipleRegionQueries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1174,14 +1162,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1232,14 +1218,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1285,14 +1269,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PdxInstance) { - } else { + if (!(obj instanceof PdxInstance)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PdxInstance and not " + obj.getClass()); } } - } else if (rs instanceof PdxInstance) { - } else { + } else if (!(rs instanceof PdxInstance)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PdxInstance and not " + rs.getClass()); } @@ -1383,16 +1365,13 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx or PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1443,16 +1422,13 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx or PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } @@ -1496,15 +1472,13 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else if (obj instanceof PdxInstance) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx) + && !(obj instanceof PdxInstance)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PdxInstance and not " + obj.getClass()); } } - } else if (rs instanceof PdxInstance || rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PdxInstance) && !(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PdxInstance and not " + rs.getClass()); } @@ -1589,14 +1563,12 @@ public Object call() throws Exception { for (Object rs : res) { if (rs instanceof StructImpl) { for (Object obj : ((StructImpl) rs).getFieldValues()) { - if (obj instanceof PortfolioPdx || obj instanceof PositionPdx) { - } else { + if (!(obj instanceof PortfolioPdx) && !(obj instanceof PositionPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + obj.getClass()); } } - } else if (rs instanceof PortfolioPdx) { - } else { + } else if (!(rs instanceof PortfolioPdx)) { fail("Result objects for remote client query: " + queries[i] + " should be instance of PortfolioPdx and not " + rs.getClass()); } diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache30/CacheStatisticsPartitionedRegionDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache30/CacheStatisticsPartitionedRegionDUnitTest.java index 743388a60d5e..bf7f85cd1e87 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/cache30/CacheStatisticsPartitionedRegionDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/cache30/CacheStatisticsPartitionedRegionDUnitTest.java @@ -14,6 +14,8 @@ */ package org.apache.geode.cache30; +import static org.apache.geode.test.awaitility.GeodeAwaitility.await; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -249,8 +251,8 @@ private void waitForClockToChange(Region region) { */ private void waitForClockToChange(Region region, long millisecs) { long time = ((InternalRegion) region).cacheTimeMillis(); - while (time >= ((InternalRegion) region).cacheTimeMillis() + millisecs) { - } + await().untilAsserted(() -> assertThat(((InternalRegion) region).cacheTimeMillis() + millisecs) + .isGreaterThanOrEqualTo(time)); } diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClearMultiVmCallBkDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClearMultiVmCallBkDUnitTest.java index 1bc5db725f50..fa924e8d619b 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClearMultiVmCallBkDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/cache30/ClearMultiVmCallBkDUnitTest.java @@ -20,8 +20,10 @@ */ package org.apache.geode.cache30; +import static org.apache.geode.test.awaitility.GeodeAwaitility.await; import static org.apache.geode.test.dunit.Assert.assertEquals; import static org.apache.geode.test.dunit.Assert.fail; +import static org.junit.Assert.assertFalse; import java.util.Properties; @@ -117,8 +119,7 @@ public void testClearSingleVM() { vm0.invoke(() -> ClearMultiVmCallBkDUnitTest.clearMethod()); LogWriterUtils.getLogWriter().fine("Did clear successfully"); - while (afterClear) { - } + await().untilAsserted(() -> assertFalse(afterClear)); int Regsize = vm0.invoke(() -> ClearMultiVmCallBkDUnitTest.sizeMethod()); assertEquals(1, Regsize); @@ -144,8 +145,7 @@ public void testClearMultiVM() { vm1.invoke(() -> ClearMultiVmCallBkDUnitTest.clearMethod()); LogWriterUtils.getLogWriter().fine("Did clear successfully"); - while (afterClear) { - } + await().untilAsserted(() -> assertFalse(afterClear)); int Regsize = vm0.invoke(() -> ClearMultiVmCallBkDUnitTest.sizeMethod()); assertEquals(1, Regsize); diff --git a/geode-core/src/distributedTest/java/org/apache/geode/cache30/DistAckMapMethodsDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/cache30/DistAckMapMethodsDUnitTest.java index 3a00c9e56753..7a78970ce143 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/cache30/DistAckMapMethodsDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/cache30/DistAckMapMethodsDUnitTest.java @@ -20,6 +20,7 @@ */ package org.apache.geode.cache30; +import static org.apache.geode.test.awaitility.GeodeAwaitility.await; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; @@ -562,9 +563,9 @@ public static void removeMethodDetails() { inOb2 = new Integer(1); assertEquals("beforeDestroy", remRegion.get(inOb2).toString()); - // wait till listeber switches afterDestroy to true - while (!afterDestroy) { - } + // wait till listener switches afterDestroy to true + await().until(() -> afterDestroy); + // to test cacheListener inOb2 = new Integer(3); assertEquals("afterDestroy", remRegion.get(inOb2).toString()); diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java index 5f46a564d207..310c6e7faa6c 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/ClientServerTransactionDUnitTest.java @@ -4354,9 +4354,8 @@ private void doJTATx1(String regionName, CountDownLatch latch1, CountDownLatch l utx.commit(); fail("Do not get expected RollbackException"); } catch (Exception e) { - if (e instanceof RollbackException) { - // expected exception. - } else { + // RollbackException is expected and can be ignored. + if (!(e instanceof RollbackException)) { Assert.fail("Unexpected exception while doing JTA Transaction1 ", e); } } finally { diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/RemoteTransactionDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/RemoteTransactionDUnitTest.java index 74bda5d78c4b..89b939cdcdf1 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/RemoteTransactionDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/RemoteTransactionDUnitTest.java @@ -1622,8 +1622,6 @@ private void throwListenerException(Region r) throws Exception { CacheListener listener = r.getAttributes().getCacheListeners()[0]; if (listener instanceof TestCacheListener) { e = ((TestCacheListener) listener).ex; - } else { - // e = ((ClientListener)listener).??? } if (e != null) { throw e; @@ -1635,8 +1633,6 @@ private void throwWriterException(Region r) throws Exception { CacheListener listener = r.getAttributes().getCacheListeners()[0]; if (listener instanceof TestCacheListener) { e = ((TestCacheListener) listener).ex; - } else { - // e = ((ClientListener)listener).??? } if (e != null) { throw e; @@ -3693,9 +3689,7 @@ class OneDestroyAndThenOneCreateCacheWriter extends CacheWriterAdapter { private boolean oneCreate; public void checkSuccess() throws Exception { - if (oneDestroy && oneCreate) { - // chill - } else { + if (!(oneDestroy && oneCreate)) { fail("Didn't get both events. oneDestroy=" + oneDestroy + " oneCreate=" + oneCreate); } } diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java index 21cc680f32d7..cb8fe8c40bb7 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java @@ -180,9 +180,8 @@ private void runFunction(final Region pr, int cust, boolean isFirstFunc) { assertTrue("Expected exception was not thrown", isFirstFunc); } catch (Exception exp) { if (!isFirstFunc) { - if (exp instanceof TransactionException && exp.getMessage() + if (!(exp instanceof TransactionException) || !exp.getMessage() .startsWith("Function execution is not colocated with transaction.")) { - } else { logger.info("Expected to catch TransactionException but caught exception " + exp, exp); Assert.fail("Expected to catch TransactionException but caught exception ", exp); } @@ -253,8 +252,7 @@ public Object call() { fail("Expected exception was not thrown"); } catch (FunctionException fe) { LogWriterUtils.getLogWriter().info("Caught Expected exception"); - if (fe.getCause() instanceof TransactionDataNotColocatedException) { - } else { + if (!(fe.getCause() instanceof TransactionDataNotColocatedException)) { throw new TestException( "Expected to catch FunctionException with cause TransactionDataNotColocatedException" + " but cause is " + fe.getCause(), diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDistributedTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDistributedTest.java index 81777a1600e4..71019182571d 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDistributedTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDistributedTest.java @@ -1534,21 +1534,18 @@ private void checkReadWriteOperationsWithOfflineMember(final int bucketOnVM0, Set keys = region.keySet(); assertThatThrownBy(() -> { for (Object key : keys) { - // nothing } }).isInstanceOf(PartitionOfflineException.class); Collection values = region.values(); assertThatThrownBy(() -> { for (Object value : values) { - // nothing } }).isInstanceOf(PartitionOfflineException.class); Set entries = region.entrySet(); assertThatThrownBy(() -> { for (Object entry : entries) { - // nothing } }).isInstanceOf(PartitionOfflineException.class); diff --git a/geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/MaxPoolSizeDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/MaxPoolSizeDUnitTest.java index 71743fdbb8e2..3a076d24edbd 100755 --- a/geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/MaxPoolSizeDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/MaxPoolSizeDUnitTest.java @@ -182,12 +182,11 @@ public static void destroyTable() throws NamingException, SQLException { sm.execute(sql); conn.close(); } catch (SQLException se) { + // If we see the message below, disregard - this happens sometimes on unit test runs on slower + // machines if (!se.getMessage().contains("A lock could not be obtained within the time requested")) { LogWriterUtils.getLogWriter().fine("destroy table sql exception: " + se); throw se; - } else { - // disregard - this happens sometimes on unit test runs on slower - // machines } } closeCache(); diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/RegionManagementDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/RegionManagementDUnitTest.java index a4cf795bdb22..fbb981918f0c 100644 --- a/geode-core/src/distributedTest/java/org/apache/geode/management/RegionManagementDUnitTest.java +++ b/geode-core/src/distributedTest/java/org/apache/geode/management/RegionManagementDUnitTest.java @@ -429,11 +429,9 @@ private void createFixedPartitionRegion(final String regionName, assertThat(fixedPartitionAttributesData).isNotNull(); assertThat(fixedPartitionAttributesData).hasSize(3); - for (FixedPartitionAttributesData aFixedPartitionAttributesData : fixedPartitionAttributesData) { - // TODO: add real assertions for FixedPartitionAttributesData - // LogWriterUtils.getLogWriter().info(" Fixed PR Data is " + - // fixedPartitionAttributesData[i] + " "); - } + // TODO: add real assertions for FixedPartitionAttributesData + // LogWriterUtils.getLogWriter().info(" Fixed PR Data is " + + // fixedPartitionAttributesData[i] + " "); } private List createFixedPartitionList(final int primaryIndex) { @@ -1034,11 +1032,9 @@ private void verifyRemoteFixedPartitionRegion(final String regionName) { assertThat(fixedPrData).isNotNull(); assertThat(fixedPrData).hasSize(3); - for (FixedPartitionAttributesData aFixedPrData : fixedPrData) { - // TODO: add real assertions for FixedPartitionAttributesData - // LogWriterUtils.getLogWriter().info(" Remote PR Data is " + - // fixedPrData[i] + " "); - } + // TODO: add real assertions for FixedPartitionAttributesData + // LogWriterUtils.getLogWriter().info(" Remote PR Data is " + + // fixedPrData[i] + " "); } } diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java index 229dd09774da..088fa4ec0223 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java @@ -17,6 +17,7 @@ import static org.apache.geode.cache.Region.SEPARATOR; import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS; import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; +import static org.apache.geode.test.awaitility.GeodeAwaitility.await; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -1122,10 +1123,10 @@ public void testCacheStatisticsOnProxy() throws Exception { } /** - * Waits (hot) until the system time changes. + * Waits until the system time changes. */ private void waitForSystemTimeChange() { long start = System.currentTimeMillis(); - while (System.currentTimeMillis() == start); + await().until(() -> System.currentTimeMillis() != start); } } diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/NestedQueryJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/NestedQueryJUnitTest.java index 3d9d35c7204c..88820c0d7ab0 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/NestedQueryJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/query/functional/NestedQueryJUnitTest.java @@ -221,15 +221,14 @@ public void testNestedQueriesEvaluation() throws Exception { SEPARATOR + "Portfolios.values['0'].positions.values"); for (int i = 0; i < queries.length; i++) { - Query q = null; + Query q; try { q = CacheUtils.getQueryService().newQuery(queries[i]); QueryObserverImpl observer2 = new QueryObserverImpl(); QueryObserverHolder.setInstance(observer2); r[i][1] = (SelectResults) q.execute(); - if (observer2.isIndexesUsed == true) { - } else if (i != 3) { + if (!observer2.isIndexesUsed && i != 3) { fail("Index not used"); } resType2 = (r[i][1]).getCollectionType().getElementType(); @@ -237,7 +236,7 @@ public void testNestedQueriesEvaluation() throws Exception { set2 = ((r[i][1]).asSet()); } catch (Exception e) { - throw new AssertionError(q.getQueryString(), e); + throw new AssertionError(queries[i], e); } } for (int j = 0; j <= 1; j++) { diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/tier/sockets/FilterProfileIntegrationJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/tier/sockets/FilterProfileIntegrationJUnitTest.java index 240173118fc0..23ab214cff20 100755 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/tier/sockets/FilterProfileIntegrationJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/tier/sockets/FilterProfileIntegrationJUnitTest.java @@ -14,19 +14,19 @@ */ package org.apache.geode.internal.cache.tier.sockets; -import java.text.ParseException; +import static org.apache.geode.test.awaitility.GeodeAwaitility.await; + import java.util.concurrent.CountDownLatch; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.apache.geode.cache.AttributesFactory; -import org.apache.geode.cache.Cache; +import org.apache.geode.cache.CacheFactory; import org.apache.geode.cache.DataPolicy; -import org.apache.geode.cache.Region; -import org.apache.geode.cache.RegionAttributes; -import org.apache.geode.cache.query.CacheUtils; import org.apache.geode.internal.cache.FilterProfile; +import org.apache.geode.internal.cache.InternalCache; import org.apache.geode.internal.cache.LocalRegion; import org.apache.geode.internal.cache.tier.InterestType; import org.apache.geode.test.junit.categories.ClientSubscriptionTest; @@ -34,14 +34,23 @@ @Category({ClientSubscriptionTest.class}) public class FilterProfileIntegrationJUnitTest { - private static String regionName = "test"; - private static int numElem = 120; + private InternalCache cache; + + @Before + public void setUp() throws Exception { + cache = (InternalCache) new CacheFactory().create(); + } + + @After + public void tearDown() throws Exception { + cache.close(); + } @Test - public void testFilterProfile() throws Exception { - Cache cache = CacheUtils.getCache(); + public void testFilterProfile() { try { - createLocalRegion(); + String regionName = "test"; + cache.createRegionFactory().setDataPolicy(DataPolicy.NORMAL).create(regionName); LocalRegion region = (LocalRegion) cache.getRegion(regionName); final FilterProfile filterProfile = new FilterProfile(region); filterProfile.registerClientInterest("clientId", ".*", InterestType.REGULAR_EXPRESSION, @@ -50,20 +59,13 @@ public void testFilterProfile() throws Exception { final FilterProfileTestHook hook = new FilterProfileTestHook(); FilterProfile.testHook = hook; - new Thread(new Runnable() { - @Override - public void run() { - while (hook.getCount() != 1) { - - } - filterProfile.unregisterClientInterest("clientId", ".*", InterestType.REGULAR_EXPRESSION); - - } + new Thread(() -> { + await().until(() -> hook.getCount() == 1); + filterProfile.unregisterClientInterest("clientId", ".*", InterestType.REGULAR_EXPRESSION); }).start(); filterProfile.hasAllKeysInterestFor("clientId"); } finally { cache.getDistributedSystem().disconnect(); - cache.close(); } } @@ -93,18 +95,5 @@ public void release() { latch.countDown(); } - }; - - /** - * Helper Methods - */ - - private void createLocalRegion() throws ParseException { - Cache cache = CacheUtils.getCache(); - AttributesFactory attributesFactory = new AttributesFactory(); - attributesFactory.setDataPolicy(DataPolicy.NORMAL); - RegionAttributes regionAttributes = attributesFactory.create(); - Region region = cache.createRegion(regionName, regionAttributes); } - } diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/offheap/OffHeapValidationJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/offheap/OffHeapValidationJUnitTest.java index dba0ab1a83db..f6345eefcbfe 100755 --- a/geode-core/src/integrationTest/java/org/apache/geode/internal/offheap/OffHeapValidationJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/offheap/OffHeapValidationJUnitTest.java @@ -16,6 +16,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS; import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT; +import static org.apache.geode.internal.offheap.MemoryAllocatorImpl.FREE_OFF_HEAP_MEMORY_PROPERTY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -48,7 +49,9 @@ import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; import org.junit.experimental.categories.Category; import org.apache.geode.DataSerializer; @@ -70,10 +73,14 @@ @Category({OffHeapTest.class}) public class OffHeapValidationJUnitTest { + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + private GemFireCacheImpl cache; @Before public void setUp() throws Exception { + System.setProperty(FREE_OFF_HEAP_MEMORY_PROPERTY, "true"); this.cache = createCache(); } @@ -223,18 +230,21 @@ public int compare(ExpectedValues o1, ExpectedValues o2) { + Arrays.toString((byte[]) values.dataValue) + " but was " + Arrays.toString((byte[]) block.getDataValue()), Arrays.equals((byte[]) values.dataValue, (byte[]) block.getDataValue())); - } else if (values.dataType.contains("[")) { // TODO: multiple dimension arrays or non-byte arrays - } else if (values.dataValue instanceof Collection) { - int diff = joint((Collection) values.dataValue, (Collection) block.getDataValue()); - assertEquals(i + ":" + values.dataType, 0, diff); - } else if (values.dataValue instanceof IdentityHashMap) { - // TODO - } else if (values.dataValue instanceof Map) { - int diff = joint((Map) values.dataValue, (Map) block.getDataValue()); - assertEquals(i + ":" + values.dataType, 0, diff); - } else { - assertEquals(i + ":" + values.dataType, values.dataValue, block.getDataValue()); + } else if (!values.dataType.contains("[")) { + if (values.dataValue instanceof Collection) { + int diff = + joint((Collection) values.dataValue, (Collection) block.getDataValue()); + assertEquals(i + ":" + values.dataType, 0, diff); + // TODO IdentityHashMap + } else if (!(values.dataValue instanceof IdentityHashMap)) { + if (values.dataValue instanceof Map) { + int diff = joint((Map) values.dataValue, (Map) block.getDataValue()); + assertEquals(i + ":" + values.dataType, 0, diff); + } else { + assertEquals(i + ":" + values.dataType, values.dataValue, block.getDataValue()); + } + } } block = block.getNextBlock(); i++; diff --git a/geode-core/src/jca/java/org/apache/geode/internal/ra/spi/JCALocalTransaction.java b/geode-core/src/jca/java/org/apache/geode/internal/ra/spi/JCALocalTransaction.java index 5aab51e0bded..d9ce62e77401 100644 --- a/geode-core/src/jca/java/org/apache/geode/internal/ra/spi/JCALocalTransaction.java +++ b/geode-core/src/jca/java/org/apache/geode/internal/ra/spi/JCALocalTransaction.java @@ -124,11 +124,9 @@ public void rollback() throws ResourceException { try { this.gfTxMgr.rollback(); } catch (IllegalStateException ise) { - // It is possible that the GFE transaction has already been rolled back. - if (ise.getMessage() - .equals("Thread does not have an active transaction")) { - // ignore - } else { + // It is possible that the GFE transaction has already been rolled back. In that case, ignore + // the IllegalStateException + if (!ise.getMessage().equals("Thread does not have an active transaction")) { throw new ResourceException(ise); } } catch (RuntimeException e) { diff --git a/geode-core/src/main/java/org/apache/geode/CopyHelper.java b/geode-core/src/main/java/org/apache/geode/CopyHelper.java index cfd0b9ba0194..c32529e18a6d 100644 --- a/geode-core/src/main/java/org/apache/geode/CopyHelper.java +++ b/geode-core/src/main/java/org/apache/geode/CopyHelper.java @@ -175,9 +175,8 @@ public static T copy(T o) { // try using Serialization } catch (InvocationTargetException ex) { Throwable cause = ex.getTargetException(); - if (cause instanceof CloneNotSupportedException) { - // try using Serialization - } else { + // If the cause is a CloneNotSupportedException, ignore and try with serialization + if (!(cause instanceof CloneNotSupportedException)) { throw new CopyException("Clone failed.", cause != null ? cause : ex); } diff --git a/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/AgentLauncher.java b/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/AgentLauncher.java index b18dedacf51c..8cad4799f1ae 100644 --- a/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/AgentLauncher.java +++ b/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/AgentLauncher.java @@ -475,13 +475,12 @@ protected Map getStopOptions(final String[] args) throws Excepti options.put(DIR, IOUtils.tryGetCanonicalFileElseGetAbsoluteFile(new File("."))); for (final String arg : args) { - if (arg.equals("stop") || arg.equals("status")) { - // expected - } else if (arg.startsWith("-dir=")) { - processDirOption(options, arg.substring("-dir=".length())); - } else { - throw new Exception( - String.format("Unknown argument: %s", arg)); + if (!arg.equals("stop") && !arg.equals("status")) { + if (arg.startsWith("-dir=")) { + processDirOption(options, arg.substring("-dir=".length())); + } else { + throw new Exception(String.format("Unknown argument: %s", arg)); + } } } diff --git a/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/MBeanUtil.java b/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/MBeanUtil.java index 63eb64d72ea4..f96c4aa710c1 100755 --- a/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/MBeanUtil.java +++ b/geode-core/src/main/java/org/apache/geode/admin/jmx/internal/MBeanUtil.java @@ -373,7 +373,7 @@ static void registerRefreshNotification(NotificationListener client, Object user notifications.put(type, null); } } - + // refreshInterval must be over 0 to do anything... if (refreshInterval > 0) { // add notification to the refresh timer... timerNotificationId = refreshTimer.addNotification(type.getType(), // type @@ -384,8 +384,6 @@ static void registerRefreshNotification(NotificationListener client, Object user // put an entry into the map for the listener... notifications.put(type, timerNotificationId); - } else { - // do nothing! refreshInterval must be over 0 to do anything... } } catch (java.lang.RuntimeException e) { logStackTrace(Level.WARN, e); diff --git a/geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/SerialAsyncEventQueueImpl.java b/geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/SerialAsyncEventQueueImpl.java index 98693a9f4d98..cb6c503f7820 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/SerialAsyncEventQueueImpl.java +++ b/geode-core/src/main/java/org/apache/geode/cache/asyncqueue/internal/SerialAsyncEventQueueImpl.java @@ -249,9 +249,8 @@ public void setModifiedEventId(EntryEventImpl clonedEvent) { EventID originalEventId = clonedEvent.getEventId(); long originalThreadId = originalEventId.getThreadID(); long newThreadId = originalThreadId; - if (ThreadIdentifier.isWanTypeThreadID(newThreadId)) { - // This thread id has already been converted. Do nothing. - } else { + // If isWanTypeThreadID() returns true, this thread id has already been converted. Do nothing. + if (!ThreadIdentifier.isWanTypeThreadID(newThreadId)) { newThreadId = ThreadIdentifier.createFakeThreadIDForParallelGSPrimaryBucket(0, originalThreadId, getEventIdIndex()); } diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutAllOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutAllOp.java index d67d27c26740..044a305def37 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutAllOp.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutAllOp.java @@ -348,28 +348,29 @@ public void handle(ChunkedMessage cm) throws Exception { if (isDebugEnabled) { logger.debug("part({}) contained {}", partNo, o); } - if (o == null) { - // no response is an okay response - } else if (o instanceof byte[]) { - if (prSingleHopEnabled) { - byte[] bytesReceived = part.getSerializedForm(); - if (bytesReceived[0] != ClientMetadataService.INITIAL_VERSION) { // nw hop - if (region != null) { + // no response is an okay response, so do nothing if o is null + if (o != null) { + if (o instanceof byte[]) { + if (prSingleHopEnabled) { + byte[] bytesReceived = part.getSerializedForm(); + // nw hop + if (bytesReceived[0] != ClientMetadataService.INITIAL_VERSION + && region != null) { try { ClientMetadataService cms = region.getCache().getClientMetadataService(); cms.scheduleGetPRMetaData(region, false, bytesReceived[1]); - } catch (CacheClosedException e) { + } catch (CacheClosedException ignore) { } } } + } else if (o instanceof Throwable) { + String s = "While performing a remote putAll"; + exceptionRef[0] = new ServerOperationException(s, (Throwable) o); + } else { + VersionedObjectList chunk = (VersionedObjectList) o; + chunk.replaceNullIDs(con.getEndpoint().getMemberId()); + result.addAll(chunk); } - } else if (o instanceof Throwable) { - String s = "While performing a remote putAll"; - exceptionRef[0] = new ServerOperationException(s, (Throwable) o); - } else { - VersionedObjectList chunk = (VersionedObjectList) o; - chunk.replaceNullIDs(con.getEndpoint().getMemberId()); - result.addAll(chunk); } } catch (Exception e) { exceptionRef[0] = new ServerOperationException("Unable to deserialize value", e); diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RemoveAllOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RemoveAllOp.java index c653e1cf1556..ce2eec17c0e1 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/RemoveAllOp.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/RemoveAllOp.java @@ -303,28 +303,28 @@ public void handle(ChunkedMessage cm) throws Exception { if (isDebugEnabled) { logger.debug("part({}) contained {}", partNo, o); } - if (o == null) { - // no response is an okay response - } else if (o instanceof byte[]) { - if (prSingleHopEnabled) { - byte[] bytesReceived = part.getSerializedForm(); - if (bytesReceived[0] != ClientMetadataService.INITIAL_VERSION) { - if (region != null) { + // no response is an okay response, so do nothing if o is null + if (o != null) { + if (o instanceof byte[]) { + if (prSingleHopEnabled) { + byte[] bytesReceived = part.getSerializedForm(); + if (bytesReceived[0] != ClientMetadataService.INITIAL_VERSION + && region != null) { try { ClientMetadataService cms = region.getCache().getClientMetadataService(); cms.scheduleGetPRMetaData(region, false, bytesReceived[1]); - } catch (CacheClosedException e) { + } catch (CacheClosedException ignore) { } } } + } else if (o instanceof Throwable) { + String s = "While performing a remote removeAll"; + exceptionRef[0] = new ServerOperationException(s, (Throwable) o); + } else { + VersionedObjectList chunk = (VersionedObjectList) o; + chunk.replaceNullIDs(con.getEndpoint().getMemberId()); + result.addAll(chunk); } - } else if (o instanceof Throwable) { - String s = "While performing a remote removeAll"; - exceptionRef[0] = new ServerOperationException(s, (Throwable) o); - } else { - VersionedObjectList chunk = (VersionedObjectList) o; - chunk.replaceNullIDs(con.getEndpoint().getMemberId()); - result.addAll(chunk); } } catch (Exception e) { exceptionRef[0] = new ServerOperationException("Unable to deserialize value", e); diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerImpl.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerImpl.java index ae48659340c8..b3d0bae21878 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerImpl.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerImpl.java @@ -965,14 +965,11 @@ synchronized boolean hasExpiredCnxToServer(ServerLocation currentServer) { if (!allConnections.isEmpty()) { final long now = System.nanoTime(); for (PooledConnection pc : allConnections) { - if (pc.shouldDestroy()) { - // this con has already been destroyed so ignore it - } else if (currentServer.equals(pc.getServer())) { - { - long life = pc.remainingLife(now, lifetimeTimeoutNanos); - if (life <= 0) { - return true; - } + // if this con has already been destroyed, ignore it + if (!pc.shouldDestroy() && currentServer.equals(pc.getServer())) { + long life = pc.remainingLife(now, lifetimeTimeoutNanos); + if (life <= 0) { + return true; } } } @@ -988,13 +985,8 @@ synchronized boolean checkForReschedule(boolean rescheduleOk) { if (!allConnections.isEmpty()) { final long now = System.nanoTime(); for (PooledConnection pc : allConnections) { - if (pc.hasIdleExpired(now, idleTimeoutNanos)) { - // this con has already idle expired so ignore it - continue; - } else if (pc.shouldDestroy()) { - // this con has already been destroyed so ignore it - continue; - } else { + // if this con has already idle expired or been destroyed, ignore it + if (!(pc.hasIdleExpired(now, idleTimeoutNanos) || pc.shouldDestroy())) { long life = pc.remainingLife(now, lifetimeTimeoutNanos); if (life > 0) { if (rescheduleOk) { @@ -1023,12 +1015,8 @@ synchronized void extendLifeOfCnxToServer(ServerLocation sl) { if (pc.remainingLife(now, lifetimeTimeoutNanos) > 0) { // no more connections whose lifetime could have expired break; - // note don't ignore idle connections because they are still connected - // } else if (pc.remainingIdle(now, idleTimeoutNanos) <= 0) { - // // this con has already idle expired so ignore it - } else if (pc.shouldDestroy()) { - // this con has already been destroyed so ignore it - } else if (sl.equals(pc.getEndpoint().getLocation())) { + // if this connection has already been destroyed, ignore it + } else if (!pc.shouldDestroy() && sl.equals(pc.getEndpoint().getLocation())) { // we found a connection to whose lifetime we can extend it.remove(); pc.setBirthDate(now); diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DerivedInfo.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DerivedInfo.java index c286ff89eb8e..ce87eb1651d4 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DerivedInfo.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DerivedInfo.java @@ -255,19 +255,13 @@ private void populateDerivedResultsFromDerivedJoin(ExecutionContext context, context.cachePut(CompiledValue.CAN_APPLY_LIMIT_AT_INDEX, originalCanApplyLimit); context.cachePut(CompiledValue.CAN_APPLY_ORDER_BY_AT_INDEX, originalCanApplyOrderBy); ObjectType ot = indexInfo._index.getResultSetType(); - // The following if block is not currently used other than the else - // This would be needed once we figure out how to handle nested object indexes (range, map, etc) + // Once we figure out how to handle nested object indexes (range, map, etc), this should be + // expanded to an if/else block with individual cases for ot.isStructType(), ot.isMapType() and + // ot.isCollectionType(). // The issue we have right now with these indexes is the results will come back as a tuple, if - // we use those as is, we end up - // reusing the evaluated values even if they did not come from the top level object leading to - // duplicate results or incorrect tupling - if (ot.isStructType()) { - // createObjectResultsFromStructResults(indexInfo, sr); - } else if (ot.isMapType()) { - - } else if (ot.isCollectionType()) { - - } else { + // we use those as is, we end up reusing the evaluated values even if they did not come from the + // top level object leading to duplicate results or incorrect tupling + if (!ot.isStructType() && !ot.isMapType() && !ot.isCollectionType()) { this.addDerivedResults(dcc.getIndexInfo(context)[0], sr); } } diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/HashIndexSet.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/HashIndexSet.java index 35edd17b7fcf..0be8f6b36a8c 100755 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/HashIndexSet.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/HashIndexSet.java @@ -734,9 +734,8 @@ public boolean hasNext() { if (keysToRemove != null) { while (pos < objects.length) { current = objects[pos]; - if (current == null || current.equals(REMOVED)) { - // continue searching - } else if (notMatchingAnyKeyToRemove(keysToRemove, current)) { + if (current != null && !current.equals(REMOVED) && notMatchingAnyKeyToRemove(keysToRemove, + current)) { return true; } setPos(pos + 1); diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java index 5b2867b97079..13d71e075f32 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexManager.java @@ -463,9 +463,7 @@ private boolean shouldCreateCompactIndex(FunctionalIndexCreationHelper helper) { if (nodeType != OQLLexerTokenTypes.Identifier && nodeType != OQLLexerTokenTypes.METHOD_INV) { if (nodeType == OQLLexerTokenTypes.TOK_LBRACK && !helper.isMapTypeIndex() && helper.modifiedIndexExpr instanceof MapIndexable) { - if (((MapIndexable) helper.modifiedIndexExpr).getIndexingKeys().size() == 1) { - - } else { + if (((MapIndexable) helper.modifiedIndexExpr).getIndexingKeys().size() != 1) { return false; } } else { diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/RangeIndex.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/RangeIndex.java index 776cd7064868..55637f757ff4 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/RangeIndex.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/index/RangeIndex.java @@ -157,11 +157,10 @@ void saveMapping(Object key, Object indxResultSet, RegionEntry entry) { } if (indxResultSet != null) { - if (indxResultSet.getClass().getName().startsWith("org.apache.geode.internal.cache.Token$") - || indxResultSet == QueryService.UNDEFINED) { - // do nothing, Entries are either removed or invalidated or destroyed - // by other thread. - } else { + // If the below evaluates to false, do nothing. Entries are either removed or invalidated or + // destroyed by other thread. + if (!indxResultSet.getClass().getName().startsWith("org.apache.geode.internal.cache.Token$") + && indxResultSet != QueryService.UNDEFINED) { undefinedSet.add(indxResultSet); } } @@ -464,11 +463,10 @@ void addMapping(Object key, Object value, RegionEntry entry) throws IMQException this.internalIndexStats.incNumValues(1); } else if (key == QueryService.UNDEFINED) { if (value != null) { - if (value.getClass().getName().startsWith("org.apache.geode.internal.cache.Token$") - || value == QueryService.UNDEFINED) { - // do nothing, Entries are either removed or invalidated or destroyed - // by other thread. - } else { + // If the below evaluates to false, do nothing. Entries are either removed or invalidated or + // destroyed by other thread. + if (!value.getClass().getName().startsWith("org.apache.geode.internal.cache.Token$") + && value != QueryService.UNDEFINED) { undefinedMappedEntries.add(entry, value); this.internalIndexStats.incNumValues(1); } diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java index 1f583440cdf5..65c029923d46 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java @@ -1344,14 +1344,10 @@ public void removeAllMembershipListener(MembershipListener l) { synchronized (allMembershipListenersLock) { Set newAllMembershipListeners = new HashSet<>(allMembershipListeners); - if (!newAllMembershipListeners.remove(l)) { - // There seems to be a race condition in which - // multiple departure events can be registered - // on the same peer. We regard this as benign. - // FIXME when membership events become sane again - // String s = "MembershipListener was never registered"; - // throw new IllegalArgumentException(s); - } + // There seems to be a race condition in which + // multiple departure events can be registered + // on the same peer. We regard this as benign. + newAllMembershipListeners.remove(l); allMembershipListeners = newAllMembershipListeners; } } diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/ReplyMessage.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/ReplyMessage.java index 2f59608177e7..3eed1652e68d 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/ReplyMessage.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/ReplyMessage.java @@ -105,9 +105,9 @@ public static void send(InternalDistributedMember recipient, int processorId, logger.debug("Replying with entry-not-found: {}", exception.getCause().getMessage()); } else if (cause instanceof ConcurrentCacheModificationException) { logger.debug("Replying with concurrent-modification-exception"); - } else if (cause instanceof CancelException) { - // no need to log this - it will show up in normal debug-level logs when the reply is sent - } else { + // no need to log CancelException - it will show up in normal debug-level logs when the + // reply is sent + } else if (!(cause instanceof CancelException)) { logger.debug("Replying with exception: " + m, exception); } } diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java index 6b6607fcc901..45f80affb2c8 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/StartupResponseMessage.java @@ -140,9 +140,7 @@ public boolean sendViaUDP() { @Override protected void process(ClusterDistributionManager dm) { - if (this.interfaces == null || this.interfaces.size() == 0) { - // this.rejectionMessage = "Peer " + getSender() + " has no network interfaces"; - } else { + if (this.interfaces != null && this.interfaces.size() != 0) { dm.setEquivalentHosts(this.interfaces); } dm.setDistributedSystemId(this.distributedSystemId); diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockGrantor.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockGrantor.java index f019b0f1b479..510d202841f4 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockGrantor.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockGrantor.java @@ -794,10 +794,10 @@ void handleLockRequest(DLockRequestMessage request) throws InterruptedException dLockLessorDepartureHandler.waitForInProcessDepartures(); } checkDestroyed(); + // If this evaluates to false, the request has been added to suspendQueue for deferred + // handling if (acquireLockPermission(request)) { handlePermittedLockRequest(request); - } else { - // request has been added to suspendQueue for deferred handling } } finally { releaseDestroyReadLock(); @@ -822,30 +822,24 @@ private void handlePermittedLockRequest(final DLockRequestMessage request) { try { // try to grant immediately if not currently granted... - if (grant.grantLockToRequest(request)) { - // do nothing - } - - // if request was local and then interrupted/released... - else if (request.responded()) { - // do nothing - } - - // if request was a failed try-lock... - else if (request.isTryLock()) { - cleanupSuspendState(request); - request.respondWithTryLockFailed(request.getObjectName()); - } - - // if request has timed out... - else if (request.checkForTimeout()) { - cleanupSuspendState(request); - } - - // schedule into waiting queue for eventual granting... - else { - grant.schedule(request); - this.thread.checkTimeToWait(calcWaitMillisFromNow(request), false); + if (!grant.grantLockToRequest(request)) { + // if request was local and then interrupted/released... + if (!request.responded()) { + // if request was a failed try-lock... + if (request.isTryLock()) { + cleanupSuspendState(request); + request.respondWithTryLockFailed(request.getObjectName()); + } + // if request has timed out... + else if (request.checkForTimeout()) { + cleanupSuspendState(request); + } + // schedule into waiting queue for eventual granting... + else { + grant.schedule(request); + this.thread.checkTimeToWait(calcWaitMillisFromNow(request), false); + } + } } } finally { grant.decAccess(); @@ -3543,9 +3537,8 @@ public void run() { // Not necessary to reset the interrupt bit, we're going to go // away of our own accord. - if (this.shutdown) { - // ok to ignore since this thread will now shutdown - } else { + // ok to ignore if this thread is shutting down, otherwise log a warning + if (!this.shutdown) { logger.warn("DLockGrantorThread was unexpectedly interrupted", e); // do not set interrupt flag since this thread needs to resume diff --git a/geode-core/src/main/java/org/apache/geode/internal/DistributionLocator.java b/geode-core/src/main/java/org/apache/geode/internal/DistributionLocator.java index 731459cd329c..b48a1673f921 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/DistributionLocator.java +++ b/geode-core/src/main/java/org/apache/geode/internal/DistributionLocator.java @@ -122,8 +122,6 @@ public static void main(String args[]) { ExitCode.FATAL.doSystemExit(); } address = InetAddress.getByName(args[1]); - } else { - // address = null; // was InetAddress.getLocalHost(); (redundant assignment) } if (args.length > 2) { peerLocator = "true".equalsIgnoreCase(args[2]); diff --git a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java index 733f72b62a14..070e62e87d5a 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java +++ b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java @@ -2002,60 +2002,60 @@ public static void basicWriteObject(Object o, DataOutput out, boolean ensurePdxC checkPdxCompatible(o, ensurePdxCompatibility); BasicSerializable bs = (BasicSerializable) o; dsfidSerializer.write(bs, out); - } else if (autoSerialized(o, out)) { - // all done - } else if (o instanceof DataSerializable.Replaceable) { - // do this first to fix bug 31609 - // do this before DataSerializable - Object replacement = ((DataSerializable.Replaceable) o).replace(); - basicWriteObject(replacement, out, ensurePdxCompatibility); - - } else if (o instanceof PdxSerializable) { - writePdx(out, GemFireCacheImpl - .getForPdx("PDX registry is unavailable because the Cache has been closed."), o, null); - } else if (o instanceof DataSerializable) { - if (isDebugEnabled_SERIALIZER) { - logger.trace(LogMarker.SERIALIZER_VERBOSE, "Writing DataSerializable: {}", o); - } - checkPdxCompatible(o, ensurePdxCompatibility); + // If autoSerialized() returns true, we don't need to continue + } else if (!autoSerialized(o, out)) { + if (o instanceof DataSerializable.Replaceable) { + // do this first to fix bug 31609 + // do this before DataSerializable + Object replacement = ((DataSerializable.Replaceable) o).replace(); + basicWriteObject(replacement, out, ensurePdxCompatibility); + + } else if (o instanceof PdxSerializable) { + writePdx(out, GemFireCacheImpl + .getForPdx("PDX registry is unavailable because the Cache has been closed."), o, null); + } else if (o instanceof DataSerializable) { + if (isDebugEnabled_SERIALIZER) { + logger.trace(LogMarker.SERIALIZER_VERBOSE, "Writing DataSerializable: {}", o); + } + checkPdxCompatible(o, ensurePdxCompatibility); - Class c = o.getClass(); - // Is "c" a user class registered with an Instantiator? - int classId = InternalInstantiator.getClassId(c); - if (classId != 0) { - writeUserDataSerializableHeader(classId, out); - } else { - out.writeByte(DSCODE.DATA_SERIALIZABLE.toByte()); - DataSerializer.writeClass(c, out); - } - invokeToData(o, out); - } else if (o instanceof Sendable) { - if (!(o instanceof PdxInstance) || o instanceof PdxInstanceEnum) { + Class c = o.getClass(); + // Is "c" a user class registered with an Instantiator? + int classId = InternalInstantiator.getClassId(c); + if (classId != 0) { + writeUserDataSerializableHeader(classId, out); + } else { + out.writeByte(DSCODE.DATA_SERIALIZABLE.toByte()); + DataSerializer.writeClass(c, out); + } + invokeToData(o, out); + } else if (o instanceof Sendable) { + if (!(o instanceof PdxInstance) || o instanceof PdxInstanceEnum) { + checkPdxCompatible(o, ensurePdxCompatibility); + } + ((Sendable) o).sendTo(out); + // If writeWellKnownObject() returns true, we don't need to continue + } else if (!writeWellKnownObject(o, out, ensurePdxCompatibility)) { checkPdxCompatible(o, ensurePdxCompatibility); - } - ((Sendable) o).sendTo(out); - } else if (writeWellKnownObject(o, out, ensurePdxCompatibility)) { - // Nothing more to do... - } else { - checkPdxCompatible(o, ensurePdxCompatibility); - if (logger.isTraceEnabled(LogMarker.SERIALIZER_ANNOUNCE_TYPE_WRITTEN_VERBOSE)) { - logger.trace(LogMarker.SERIALIZER_ANNOUNCE_TYPE_WRITTEN_VERBOSE, - "DataSerializer Serializing an instance of {}", o.getClass().getName()); - } - - /* - * If the (internally known) ThreadLocal named "DataSerializer.DISALLOW_JAVA_SERIALIZATION" is - * set, then an exception will be thrown if we try to do standard Java Serialization. This is - * used to catch Java serialization early for the case where the data is being sent to a - * non-Java client - */ - if (disallowJavaSerialization() && o instanceof Serializable) { - throw new NotSerializableException( - String.format("%s is not DataSerializable and Java Serialization is disallowed", - o.getClass().getName())); - } + if (logger.isTraceEnabled(LogMarker.SERIALIZER_ANNOUNCE_TYPE_WRITTEN_VERBOSE)) { + logger.trace(LogMarker.SERIALIZER_ANNOUNCE_TYPE_WRITTEN_VERBOSE, + "DataSerializer Serializing an instance of {}", o.getClass().getName()); + } - writeSerializableObject(o, out); + /* + * If the (internally known) ThreadLocal named "DataSerializer.DISALLOW_JAVA_SERIALIZATION" + * is set, then an exception will be thrown if we try to do standard Java Serialization. + * This is used to catch Java serialization early for the case where the data is being sent + * to a non-Java client + */ + if (disallowJavaSerialization() && o instanceof Serializable) { + throw new NotSerializableException( + String.format("%s is not DataSerializable and Java Serialization is disallowed", + o.getClass().getName())); + } + + writeSerializableObject(o, out); + } } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java b/geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java index 2ede0a10653c..38349736b380 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java +++ b/geode-core/src/main/java/org/apache/geode/internal/admin/remote/DistributionLocatorId.java @@ -151,16 +151,11 @@ public DistributionLocatorId(String marshalled) { int optionsIndex = marshalled.indexOf(','); if (optionsIndex > 0) { String[] options = marshalled.substring(optionsIndex).split(","); - for (int i = 0; i < options.length; i++) { - String[] optionFields = options[i].split("="); - if (optionFields.length == 2) { - if (optionFields[0].equalsIgnoreCase("peer")) { - // this setting is deprecated - // this.peerLocator = Boolean.valueOf(optionFields[1]).booleanValue(); - } else if (optionFields[0].equalsIgnoreCase("server")) { - // this setting is deprecated - // this.serverLocator = Boolean.valueOf(optionFields[1]).booleanValue(); - } else if (optionFields[0].equalsIgnoreCase("hostname-for-clients")) { + for (String option : options) { + String[] optionFields = option.split("="); + if (optionFields.length == 2 && !optionFields[0].equalsIgnoreCase("peer") + && !optionFields[0].equalsIgnoreCase("server")) { + if (optionFields[0].equalsIgnoreCase("hostname-for-clients")) { this.hostnameForClients = optionFields[1]; } else { throw new IllegalArgumentException(marshalled + " invalid option " + optionFields[0] diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java b/geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java index 42aa30696f57..0347fadcb564 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java +++ b/geode-core/src/main/java/org/apache/geode/internal/admin/remote/RemoteTransportConfig.java @@ -151,9 +151,6 @@ public RemoteTransportConfig(boolean isMcastEnabled, boolean isTcpDisabled, DistributionLocatorId id = (DistributionLocatorId) it.next(); if (id.isMcastId()) { mid = id; - // System.out.println("mcast id: " + id); - } else { - // System.out.println("non-mcast id: " + id); } } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java index f1f765e39ba7..c832ef26dd00 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java @@ -1619,10 +1619,9 @@ public boolean invalidate(EntryEventImpl event, boolean invokeCallbacks, boolean } } // synchronized re } // re != null - else { - // At this point, either it's not in GII mode, or the placeholder - // is in region, do nothing - } + // At this point, either it's not in GII mode, or the placeholder + // is in region, do nothing + if (!entryExisted) { owner.checkEntryNotFound(event.getKey()); } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java index d91786f9c615..cee9f75c4ef9 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java @@ -695,9 +695,6 @@ public long basicPutPart2(EntryEventImpl event, RegionEntry entry, boolean isIni } finally { partitionedRegion.getPrStats().endSendReplication(start); } - } else { - // consolidate the UpdateOperation for each entry into a PutAllMessage - // basicPutPart3 takes care of this } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java index 4b31bc239517..7c9a03298e3f 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskInitFile.java @@ -1959,10 +1959,9 @@ void clearRegion(DiskRegionView dr, long clearOplogEntryId) { try { if (clearOplogEntryId != DiskStoreImpl.INVALID_ID) { this.ifTotalRecordCount++; + // If this evaluates to false, we now have one record to gc (the previous clear). if (dr.getClearOplogEntryId() == DiskStoreImpl.INVALID_ID) { this.ifLiveRecordCount++; - } else { - // we now have one record to gc (the previous clear). } dr.setClearOplogEntryId(clearOplogEntryId); if (clearOplogEntryId > clearOplogEntryIdHWM) { @@ -1982,10 +1981,9 @@ void clearRegion(DiskRegion dr, RegionVersionVector rvv) { lock(true); try { this.ifTotalRecordCount++; + // If this evaluates to false, we now have one record to gc (the previous clear). if (dr.getClearRVV() == null) { this.ifLiveRecordCount++; - } else { - // we now have one record to gc (the previous clear). } dr.setClearRVV(rvv); writeClearRecord(dr, rvv); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskRegion.java index 1e7e26ba0db1..99713f893b11 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskRegion.java @@ -244,9 +244,8 @@ void finishInitializeOwner(LocalRegion drs, GIIStatus giiStatus) { // this.scheduleCompaction(); if (GIIStatus.didFullGII(giiStatus)) { destroyRemainingRecoveredEntries(drs); - } else if (GIIStatus.didDeltaGII(giiStatus)) { // TODO: not sure if we should destroy old tombstones for deltaGII - } else if (getRegionVersionVector() != null) { + } else if (!GIIStatus.didDeltaGII(giiStatus) && getRegionVersionVector() != null) { destroyOldTomstones(drs); } releaseRecoveryData(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskWriteAttributesImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskWriteAttributesImpl.java index b5e679d6bb19..8e6a4ace51d6 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DiskWriteAttributesImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DiskWriteAttributesImpl.java @@ -165,9 +165,6 @@ public DiskWriteAttributesImpl(Properties properties) { String bytesThresholdString = properties.getProperty(CacheXml.BYTES_THRESHOLD); if (bytesThresholdString != null) { - if (this.isSynchronous) { - // log warning, no use setting time if is synchronous - } this.bytesThreshold = verifyLongInString(bytesThresholdString, CacheXml.BYTES_THRESHOLD); } else { this.bytesThreshold = 0L; @@ -175,9 +172,6 @@ public DiskWriteAttributesImpl(Properties properties) { String timeIntervalString = properties.getProperty(CacheXml.TIME_INTERVAL); if (timeIntervalString != null) { - if (this.isSynchronous) { - // log warning, no use setting time if is synchronous - } this.timeInterval = verifyLongInString(timeIntervalString, CacheXml.TIME_INTERVAL); } else { if (!this.isSynchronous && this.bytesThreshold == 0) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedCacheOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedCacheOperation.java index 3e55045fe192..ce44f6f98afb 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedCacheOperation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedCacheOperation.java @@ -436,9 +436,7 @@ protected void _distribute() { } } } - if (!reliableOp || region.isNoDistributionOk()) { - // nothing needs be done in this case - } else { + if (reliableOp && !region.isNoDistributionOk()) { region.handleReliableDistribution(Collections.emptySet()); } @@ -1267,9 +1265,8 @@ protected void basicProcess(ClusterDistributionManager dm, LocalRegion lclRgn) { public void sendReply(InternalDistributedMember recipient, int pId, ReplyException rex, ReplySender dm) { - if (pId == 0 && (dm instanceof DistributionManager) && !this.directAck) {// Fix for #41871 - // distributed-no-ack message. Don't respond - } else { + // Don't respond to distributed-no-ack messages. + if (pId != 0 || (!(dm instanceof DistributionManager)) || this.directAck) { ReplyMessage.send(recipient, pId, rex, dm, !this.appliedOperation, this.closed, false, isInternal()); } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java index 416854116e1a..9f303f1bf5fe 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/EntryEventImpl.java @@ -1883,9 +1883,7 @@ void setTXEntryOldValue(Object oldVal, boolean mustBeAvailable) { if (Token.isInvalidOrRemoved(oldVal)) { oldVal = null; } else { - if (mustBeAvailable || oldVal == null || areOldValuesEnabled()) { - // set oldValue to oldVal - } else { + if (!mustBeAvailable && oldVal != null && !areOldValuesEnabled()) { oldVal = Token.NOT_AVAILABLE; } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/HARegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/HARegion.java index 98f6fb5146da..dda2d6683271 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/HARegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/HARegion.java @@ -392,8 +392,6 @@ Object findObjectInSystem(KeyInfo keyInfo, boolean isCreate, TXStateInterface tx } finally { event.release(); } - if (txState == null) { - } } catch (CacheWriterException cwe) { // @todo smenon Log the exception } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java index 0ab1182e1f91..68f043c9be9b 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/NonLocalRegionEntry.java @@ -94,12 +94,11 @@ public NonLocalRegionEntry(Region.Entry re, InternalRegion br) { } Assert.assertTrue(this.value != Token.NOT_AVAILABLE, "getEntry did not fault value in from disk"); - this.lastModified = 0l;// re.getStatistics().getLastModifiedTime(); + this.lastModified = 0L;// re.getStatistics().getLastModifiedTime(); this.isRemoved = Token.isRemoved(value); + // TODO need to get version information from transaction entries if re is not an EntrySnapshot if (re instanceof EntrySnapshot) { this.versionTag = ((EntrySnapshot) re).getVersionTag(); - } else { - // TODO need to get version information from transaction entries } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java index 4cfe591f202d..7c5d4aad4d22 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java @@ -959,9 +959,8 @@ void initAfterRecovery(boolean offline) { getOplogSet().drfDelete(this.oplogId); deleteFile(this.drf); } - } else if (needsCompaction()) { - // just leave it in the list it is already in - } else { + // just leave it in the list it is already in if needsCompaction() returns true + } else if (!needsCompaction()) { // remove it from the compactable list getOplogSet().removeOplog(getOplogId(), true/* diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java index e07ab55a395b..40f04335bf05 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PRHARedundancyProvider.java @@ -1069,12 +1069,9 @@ public boolean createBackupBucketOnMember(int bucketId, InternalDistributedMembe // error condition, so you also need to check to see if the JVM // is still usable: SystemFailure.checkFailure(); - if (e instanceof ForceReattemptException) { - // no log needed - } else if (e instanceof CancelException - || e.getCause() != null && e.getCause() instanceof CancelException) { - // no need to log exceptions caused by cache closure - } else { + // no need to log exceptions caused by cache closure or ForceReattemptException + if (!(e instanceof ForceReattemptException) && !(e instanceof CancelException) + && (e.getCause() == null || !(e.getCause() instanceof CancelException))) { logger.warn("Exception creating partition on {}", targetMember, e); } return false; @@ -1168,9 +1165,7 @@ private ManageBucketRsp createBucketOnMember(int bucketId, InternalDistributedMe // no need to log exceptions caused by cache closure return ManageBucketRsp.CLOSED; } - if (e instanceof ForceReattemptException) { - // no log needed - } else { + if (!(e instanceof ForceReattemptException)) { logger.warn("Exception creating partition on {}", targetMember, e); } return ManageBucketRsp.NO; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java index 317889395582..b66217095ee6 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java @@ -2124,22 +2124,17 @@ private Object doExecuteQuery(final DefaultQuery query, // Drop Duplicates if this is a DISTINCT query boolean allowsDuplicates = results.getCollectionType().allowsDuplicates(); - // No need to apply the limit to the SelectResults. - // We know that even if we do not apply the limit, - // the results will satisfy the limit - // as it has been evaluated in the iteration of List to - // populate the SelectsResuts - // So if the results is instance of ResultsBag or is a StructSet or - // a ResultsSet, if the limit exists, the data set size will - // be exactly matching the limit + // No need to apply the limit to the SelectResults. We know that even if we do not apply the + // limit, the results will satisfy the limit as it has been evaluated in the iteration of List + // to populate the SelectsResuts. So if the results is instance of ResultsBag or is a StructSet + // or a ResultsSet, if the limit exists, the data set size will be exactly matching the limit if (selectExpr.isDistinct()) { - // don't just convert to a ResultsSet (or StructSet), since - // the bags can convert themselves to a Set more efficiently + // don't just convert to a ResultsSet (or StructSet), since the bags can convert themselves to + // a Set more efficiently ObjectType elementType = results.getCollectionType().getElementType(); - if (selectExpr.getOrderByAttrs() != null) { - // Set limit also, its not applied while building the final result set as order by is - // involved. - } else if (allowsDuplicates) { + // Set limit also, its not applied while building the final result set as order by is + // involved. + if (selectExpr.getOrderByAttrs() == null && allowsDuplicates) { results = new ResultsCollectionWrapper(elementType, results.asSet()); } if (selectExpr.isCount() && (results.isEmpty() || selectExpr.isDistinct())) { @@ -9478,9 +9473,7 @@ public List getSortedBuckets() { HeapEvictor.BUCKET_SORTING_INTERVAL); } } - if (!bucketSortedOnce.get()) { - while (bucketSortedOnce.get() == false); - } + while (!bucketSortedOnce.get()); List bucketList = new ArrayList<>(this.sortedBuckets); return bucketList; } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/QueuedOperation.java b/geode-core/src/main/java/org/apache/geode/internal/cache/QueuedOperation.java index 981d0129dd4a..9e2610ba6b75 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/QueuedOperation.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/QueuedOperation.java @@ -93,9 +93,7 @@ public void process(LocalRegion lr, DistributedMember src, long lastMod) { if (ee.getVersionTag() != null) { time = ee.getVersionTag().getVersionTimeStamp(); } - if (AbstractUpdateOperation.doPutOrCreate(lr, ee, time)) { - // am I done? - } + AbstractUpdateOperation.doPutOrCreate(lr, ee, time); } catch (ConcurrentCacheModificationException e) { // operation was rejected by the cache's concurrency control mechanism as being old } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java b/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java index 1a5e3de64339..fa956bae0dac 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java @@ -1884,10 +1884,8 @@ void doGet(ClusterDistributionManager dm) { } Object eov = region.getNoLRU(this.key, false, true, true); // OFFHEAP: incrc, copy // bytes, decrc - if (eov != null) { - if (eov == Token.INVALID || eov == Token.LOCAL_INVALID) { - // nothing? - } else if (dm.cacheTimeMillis() - startTime < timeoutMs) { + if (eov != null && eov != Token.INVALID && eov != Token.LOCAL_INVALID) { + if (dm.cacheTimeMillis() - startTime < timeoutMs) { if (!region.isExpiredWithRegardTo(this.key, this.ttl, this.idleTime)) { lastModifiedCacheTime = entry.getLastModified(); if (eov instanceof CachedDeserializable) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/DiskEntry.java b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/DiskEntry.java index 7b33d65927c1..83030c56e3c2 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/entries/DiskEntry.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/entries/DiskEntry.java @@ -908,9 +908,7 @@ private static AsyncDiskEntry basicUpdate(DiskEntry entry, InternalRegion region caughtCacheClosed = true; throw e; } finally { - if (caughtCacheClosed) { - // 47616: not to set the value to be removedFromDisk since it failed to persist - } else { + if (!caughtCacheClosed) { // Ensure that the value is rightly set despite clear so // that it can be distributed correctly entry.setValueWithContext(region, newValue); // OFFHEAP newValue was already @@ -1002,17 +1000,15 @@ private static AsyncDiskEntry basicUpdate(DiskEntry entry, InternalRegion region updateStats(dr, region, 0/* InVM */, -1/* OnDisk */, -oldValueLength); } else if (!Token.isInvalidOrRemoved(oldValue)) { updateStats(dr, region, -1/* InVM */, 0/* OnDisk */, 0); - } else { - // oldValue was also a token which is neither in vm or on disk. - } + } // otherwise, oldValue was also a token which is neither in vm or on disk. + } else { // we have a value to put in the vm if (oldValue == null) { updateStats(dr, region, 1/* InVM */, -1/* OnDisk */, -oldValueLength); } else if (Token.isInvalidOrRemoved(oldValue)) { updateStats(dr, region, 1/* InVM */, 0/* OnDisk */, 0/* overflowBytesOnDisk */); - } else { - // old value was also in vm so no change - } + } // otherwise, old value was also in vm so no change + } } if (entry instanceof EvictableEntry) { @@ -1372,10 +1368,9 @@ public static int overflowToDisk(DiskEntry entry, InternalRegion region, } else if (!wasAlreadyPendingAsync) { scheduledAsyncHere = true; did.setPendingAsync(true); - } else { - // it may have been scheduled to be written (isBackup==true) - // and now we are faulting it out - } + } // otherwise, it may have been scheduled to be written (isBackup==true) and now we are + // faulting it out + } // If async then if it does not need to be written (because it already was) @@ -1481,30 +1476,31 @@ private static void writeEntryToDisk(DiskEntry entry, InternalRegion region, Ver if (dr.isBackup()) { did.setKeyId(DiskRegion.INVALID_ID); // fix for bug 41340 } - } else if ((Token.isInvalid(entryVal) || entryVal == Token.TOMBSTONE) - && !dr.isBackup()) { // no need to write invalid or tombstones to disk if overflow only - } else if (entryVal != null) { - writeToDisk(entry, region, true); - assert !dr.isSync(); - // Only setValue to null if this was an evict. - // We could just be a backup that is writing async. - if (!Token.isInvalid(entryVal) && (entryVal != Token.TOMBSTONE) - && entry instanceof EvictableEntry - && ((EvictableEntry) entry).isEvicted()) { - // Moved this here to fix bug 40116. - region.updateSizeOnEvict(entry.getKey(), entryValSize); - updateStats(dr, region, -1/* InVM */, 1/* OnDisk */, did.getValueLength()); - entry.handleValueOverflow(region); - entry.setValueWithContext(region, null); - } - } else { - // if we have a version tag we need to record the operation - // to update the RVV - if (tag != null) { - DiskEntry.Helper.doAsyncFlush(tag, region); + } else if ((!Token.isInvalid(entryVal) && entryVal != Token.TOMBSTONE) + || dr.isBackup()) { + if (entryVal != null) { + writeToDisk(entry, region, true); + assert !dr.isSync(); + // Only setValue to null if this was an evict. + // We could just be a backup that is writing async. + if (!Token.isInvalid(entryVal) && (entryVal != Token.TOMBSTONE) + && entry instanceof EvictableEntry + && ((EvictableEntry) entry).isEvicted()) { + // Moved this here to fix bug 40116. + region.updateSizeOnEvict(entry.getKey(), entryValSize); + updateStats(dr, region, -1/* InVM */, 1/* OnDisk */, did.getValueLength()); + entry.handleValueOverflow(region); + entry.setValueWithContext(region, null); + } + } else { + // if we have a version tag we need to record the operation + // to update the RVV + if (tag != null) { + Helper.doAsyncFlush(tag, region); + } + return; } - return; } } catch (RegionClearedException ignore) { // no need to do the op since it was clobbered by a region clear diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java index 13475dfbee91..201390df7605 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ha/HARegionQueue.java @@ -1000,13 +1000,10 @@ void expireTheEventOrThreadIdentifier(EntryEvent event) throws CacheException { } Object key = event.getKey(); if (key instanceof ThreadIdentifier) { - // Check if the sequenceID present as value against this key is same - // as - // the last dispatched sequence & the size of set containing the - // counters - // is 0. If yes the Dace should be removed - // Get DACE + // Check if the sequenceID present as value against this key is same as the last dispatched + // sequence & the size of set containing the counters is 0. If yes the Dace should be removed + // Get DACE DispatchedAndCurrentEvents dace = (DispatchedAndCurrentEvents) HARegionQueue.this.eventsMap.get(key); Assert.assertTrue(dace != null); @@ -1032,9 +1029,6 @@ void expireTheEventOrThreadIdentifier(EntryEvent event) throws CacheException { } else { dace.destroy((Long) key); } - // } - } else { - // unexpected condition, throw exception? } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java index 6b9534f6f846..0d59aa3b6319 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/IndexCreationMsg.java @@ -481,12 +481,11 @@ public IndexCreationResult waitForResult() throws CacheException, ForceReattempt waitForCacheException(); } catch (RuntimeException re) { if (re instanceof PartitionedRegionException) { - if (re.getCause() instanceof RegionNotFoundException) { - // Region may not be available at the receiver. - // ignore the exception. - // This will happen when the region on the remote end is still in - // initialization mode and is not yet created the region ID. - } else { + // Region may not be available at the receiver. + // ignore the RegionNotFoundException. + // This will happen when the region on the remote end is still in + // initialization mode and is not yet created the region ID. + if (!(re.getCause() instanceof RegionNotFoundException)) { throw re; } } else { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUserAuths.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUserAuths.java index d6c4012b4ed5..f0aa08efc48d 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUserAuths.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUserAuths.java @@ -120,8 +120,6 @@ public void setUserAuthAttributesForCq(String cqName, long uniqueId, boolean isD this.cleanUserAuth(oldUaa); // add durable(increment) uaa.setDurable(); - } else { - // if looks extra call from client } } else { uaa.setDurable(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll70.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll70.java index e2ca674e4942..59b98f3c4dda 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll70.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll70.java @@ -220,15 +220,6 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve map = ((UpdateOnlyMap) map).getInternalMap(); } } - } else { - // no auth, so update the map based on isObjectMap here - /* - * Collection entries = map.entrySet(); Iterator iterator = entries.iterator(); Map.Entry - * mapEntry = null; while (iterator.hasNext()) { mapEntry = (Map.Entry)iterator.next(); - * Object currkey = mapEntry.getKey(); byte[] serializedValue = (byte[])mapEntry.getValue(); - * boolean isObject = ((Boolean)isObjectMap.get(currkey)).booleanValue(); if (isObject) { - * map.put(currkey, CachedDeserializableFactory.create(serializedValue)); } } - */ } if (logger.isDebugEnabled()) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll80.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll80.java index 247c95a70ab1..b3af5111149a 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll80.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutAll80.java @@ -258,15 +258,6 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve } callbackArg = putAllContext.getCallbackArg(); } - } else { - // no auth, so update the map based on isObjectMap here - /* - * Collection entries = map.entrySet(); Iterator iterator = entries.iterator(); Map.Entry - * mapEntry = null; while (iterator.hasNext()) { mapEntry = (Map.Entry)iterator.next(); - * Object currkey = mapEntry.getKey(); byte[] serializedValue = (byte[])mapEntry.getValue(); - * boolean isObject = ((Boolean)isObjectMap.get(currkey)).booleanValue(); if (isObject) { - * map.put(currkey, CachedDeserializableFactory.create(serializedValue)); } } - */ } response = region.basicBridgePutAll(map, retryVersions, serverConnection.getProxyID(), diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutUserCredentials.java b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutUserCredentials.java index f8bfab69599a..26c77b5304f3 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutUserCredentials.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/PutUserCredentials.java @@ -72,11 +72,7 @@ public void cmdExecute(final Message clientMessage, final ServerConnection serve serverConnection.setAsTrue(RESPONDED); } - } else { - // need to throw some exeception } - } else { - // need to throw exception } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java index df531ce60d32..56afce7a85bb 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/parallel/ParallelGatewaySenderQueue.java @@ -765,18 +765,14 @@ public boolean put(Object object) throws InterruptedException, CacheException { } finally { brq.getInitializationLock().readLock().unlock(); } - } else if (isDREvent) { - // in case of DR with PGS, if shadow bucket is not found event after - // above search then it means that bucket is not intended for this - // node. So lets not add this event in temp queue event as we are - // doing it for PRevent + // in case of DR with PGS, if shadow bucket is not found event after above search then + // it means that bucket is not intended for this node. So lets not add this event in + // temp queue event as we are doing it for PRevent // does not put onto the queue - } else { - // We have to handle the case where brq is null because the - // colocation - // chain is getting destroyed one by one starting from child region - // i.e this bucket due to moveBucket operation - // In that case we don't want to store this event. + } else if (!isDREvent) { + // We have to handle the case where brq is null because the colocation chain is getting + // destroyed one by one starting from child region i.e this bucket due to moveBucket + // operation In that case we don't want to store this event. if (prQ.getColocatedWithRegion().getRegionAdvisor().getBucketAdvisor(bucketId) .isShadowBucketDestroyed(bucketFullPath)) { if (isDebugEnabled) { @@ -823,7 +819,6 @@ public boolean put(Object object) throws InterruptedException, CacheException { } } } - } finally { LocalRegion.setThreadInitLevelRequirement(oldLevel); } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java index daf97e747d00..5b023fb4f98f 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderEventProcessor.java @@ -660,11 +660,9 @@ protected void basicHandlePrimaryEvent(final GatewaySenderEventImpl gatewayEvent Long mapValue = Long.valueOf(System.currentTimeMillis() + AbstractGatewaySender.TOKEN_TIMEOUT); Long oldv = this.unprocessedTokens.put(gatewayEvent.getEventId(), mapValue); + // TODO add an assertion that oldv is non-null if (oldv == null) { statistics.incUnprocessedTokensAddedByPrimary(); - } else { - // its ok for oldv to be non-null - // this shouldn't happen anymore @todo add an assertion here } } } else { @@ -772,9 +770,6 @@ private void reapOld(final GatewaySenderStats statistics, boolean forceEventReap break; } } - if (count > 0) { - // statistics.incUnprocessedTokensRemovedByTimeout(count); - } } if (forceEventReap || this.unprocessedEvents.size() > REAP_THRESHOLD) { Iterator> it = @@ -794,9 +789,6 @@ private void reapOld(final GatewaySenderStats statistics, boolean forceEventReap break; } } - if (count > 0) { - // statistics.incUnprocessedEventsRemovedByTimeout(count); - } } } else { uncheckedCount++; diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java index c775086a920c..fb54477ecb00 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java @@ -1073,35 +1073,36 @@ private void endTransactionWriter() { private void startRegionAttributes(Attributes atts) { RegionAttributesCreation attrs = new RegionAttributesCreation(this.cache); String scope = atts.getValue(SCOPE); - if (scope == null) { - } else if (scope.equals(LOCAL)) { - attrs.setScope(Scope.LOCAL); - } else if (scope.equals(DISTRIBUTED_NO_ACK)) { - attrs.setScope(Scope.DISTRIBUTED_NO_ACK); - } else if (scope.equals(DISTRIBUTED_ACK)) { - attrs.setScope(Scope.DISTRIBUTED_ACK); - } else if (scope.equals(GLOBAL)) { - attrs.setScope(Scope.GLOBAL); - } else { - throw new InternalGemFireException( - String.format("Unknown scope: %s", scope)); + if (scope != null) { + if (scope.equals(LOCAL)) { + attrs.setScope(Scope.LOCAL); + } else if (scope.equals(DISTRIBUTED_NO_ACK)) { + attrs.setScope(Scope.DISTRIBUTED_NO_ACK); + } else if (scope.equals(DISTRIBUTED_ACK)) { + attrs.setScope(Scope.DISTRIBUTED_ACK); + } else if (scope.equals(GLOBAL)) { + attrs.setScope(Scope.GLOBAL); + } else { + throw new InternalGemFireException( + String.format("Unknown scope: %s", scope)); + } } String mirror = atts.getValue(MIRROR_TYPE); - if (mirror == null) { - } else if (mirror.equals(NONE)) { - attrs.setMirrorType(MirrorType.NONE); - } else if (mirror.equals(KEYS)) { - attrs.setMirrorType(MirrorType.KEYS); - } else if (mirror.equals(KEYS_VALUES)) { - attrs.setMirrorType(MirrorType.KEYS_VALUES); - } else { - throw new InternalGemFireException( - String.format("Unknown mirror type: %s", mirror)); + if (mirror != null) { + if (mirror.equals(NONE)) { + attrs.setMirrorType(MirrorType.NONE); + } else if (mirror.equals(KEYS)) { + attrs.setMirrorType(MirrorType.KEYS); + } else if (mirror.equals(KEYS_VALUES)) { + attrs.setMirrorType(MirrorType.KEYS_VALUES); + } else { + throw new InternalGemFireException( + String.format("Unknown mirror type: %s", mirror)); + } } - { - String dp = atts.getValue(DATA_POLICY); - if (dp == null) { - } else if (dp.equals(NORMAL_DP)) { + String dp = atts.getValue(DATA_POLICY); + if (dp != null) { + if (dp.equals(NORMAL_DP)) { attrs.setDataPolicy(DataPolicy.NORMAL); } else if (dp.equals(PRELOADED_DP)) { attrs.setDataPolicy(DataPolicy.PRELOADED); @@ -1120,7 +1121,6 @@ private void startRegionAttributes(Attributes atts) { String.format("Unknown data policy: %s", dp)); } } - String initialCapacity = atts.getValue(INITIAL_CAPACITY); if (initialCapacity != null) { attrs.setInitialCapacity(parseInt(initialCapacity)); @@ -1559,9 +1559,6 @@ private void endDiskWriteAttributes() { private void endDiskDir() { StringBuffer dirName = (StringBuffer) stack.pop(); File dir = new File(dirName.toString().trim()); - if (!dir.exists()) { - - } stack.push(dir); } @@ -2658,7 +2655,6 @@ public void startElement(String namespaceURI, String localName, String qName, At startCacheServer(atts); } else if (qName.equals(CACHE_SERVER)) { startCacheServer(atts); - } else if (qName.equals(LOAD_PROBE)) { } else if (qName.equals(CONNECTION_POOL)) { startPool(atts); } else if (qName.equals(CLIENT_SUBSCRIPTION)) { @@ -2669,13 +2665,8 @@ public void startElement(String namespaceURI, String localName, String qName, At startGatewaySender(atts); } else if (qName.equals(GATEWAY_RECEIVER)) { startGatewayReceiver(atts); - } else if (qName.equals(GATEWAY_EVENT_FILTER)) { - } else if (qName.equals(GATEWAY_TRANSPORT_FILTER)) { - } else if (qName.equals(GATEWAY_EVENT_LISTENER)) { - } else if (qName.equals(GATEWAY_EVENT_SUBSTITUTION_FILTER)) { } else if (qName.equals(ASYNC_EVENT_QUEUE)) { startAsyncEventQueue(atts); - } else if (qName.equals(GATEWAY_CONFLICT_RESOLVER)) { } else if (qName.equals(LOCATOR)) { doLocator(atts); } else if (qName.equals(REGION)) { @@ -2686,22 +2677,12 @@ public void startElement(String namespaceURI, String localName, String qName, At startRegionAttributes(atts); } else if (qName.equals(DISK_STORE)) { startDiskStore(atts); - } else if (qName.equals(KEY_CONSTRAINT)) { - } else if (qName.equals(VALUE_CONSTRAINT)) { - } else if (qName.equals(INDEX_UPDATE_TYPE)) { - } else if (qName.equals(REGION_TIME_TO_LIVE)) { - } else if (qName.equals(REGION_IDLE_TIME)) { - } else if (qName.equals(ENTRY_TIME_TO_LIVE)) { - } else if (qName.equals(ENTRY_IDLE_TIME)) { } else if (qName.equals(EXPIRATION_ATTRIBUTES)) { startExpirationAttributes(atts); } else if (qName.equals(SERVER)) { doServer(atts); - } else if (qName.equals(CUSTOM_EXPIRY)) { } else if (qName.equals(SUBSCRIPTION_ATTRIBUTES)) { startSubscriptionAttributes(atts); - } else if (qName.equals(ENTRY)) { - } else if (qName.equals(CLASS_NAME)) { } else if (qName.equals(PARAMETER)) { startParameter(atts); } else if (qName.equals(DISK_WRITE_ATTRIBUTES)) { @@ -2710,10 +2691,8 @@ public void startElement(String namespaceURI, String localName, String qName, At startSynchronousWrites(); } else if (qName.equals(ASYNCHRONOUS_WRITES)) { startAsynchronousWrites(atts); - } else if (qName.equals(DISK_DIRS)) { } else if (qName.equals(DISK_DIR)) { startDiskDir(atts); - } else if (qName.equals(GROUP)) { } else if (qName.equals(PARTITION_ATTRIBUTES)) { startPartitionAttributes(atts); } else if (qName.equals(FIXED_PARTITION_ATTRIBUTES)) { @@ -2726,9 +2705,6 @@ public void startElement(String namespaceURI, String localName, String qName, At startPartitionProperties(atts, LOCAL_PROPERTIES); } else if (qName.equals(GLOBAL_PROPERTIES)) { startPartitionProperties(atts, GLOBAL_PROPERTIES); - } else if (qName.equals(CACHE_LOADER)) { - } else if (qName.equals(CACHE_WRITER)) { - } else if (qName.equals(EVICTION_ATTRIBUTES)) { } else if (qName.equals(LRU_ENTRY_COUNT)) { startLRUEntryCount(atts); // internal to eviction-attributes } else if (qName.equals(LRU_MEMORY_SIZE)) { @@ -2738,12 +2714,6 @@ public void startElement(String namespaceURI, String localName, String qName, At startLRUMemorySize(atts); } else if (qName.equals(LRU_HEAP_PERCENTAGE)) { startLRUHeapPercentage(atts); // internal to eviction-attributes - } else if (qName.equals(CACHE_LISTENER)) { - } else if (qName.equals(ASYNC_EVENT_LISTENER)) { - } else if (qName.equals(KEY)) { - } else if (qName.equals(VALUE)) { - } else if (qName.equals(STRING)) { - } else if (qName.equals(DECLARABLE)) { } else if (qName.equals(INDEX)) { // Asif: Create an object of type IndexCreationData & // push it in stack @@ -2755,9 +2725,6 @@ public void startElement(String namespaceURI, String localName, String qName, At startPrimaryKeyIndex(atts); } else if (qName.equals(TRANSACTION_MANAGER)) { startCacheTransactionManager(); - } else if (qName.equals(TRANSACTION_LISTENER)) { - } else if (qName.equals(TRANSACTION_WRITER)) { - } else if (qName.equals(JNDI_BINDINGS)) { // added by Nand Kishor } else if (qName.equals(JNDI_BINDING)) { // added by Nand Kishor // Asif: Push the BindingCreation object in the stack Map gfSpecific = new HashMap(); @@ -2771,43 +2738,52 @@ public void startElement(String namespaceURI, String localName, String qName, At List vendorSpecific = bc.getVendorSpecificList(); // Rohit: Add a ConfigProperty Data Object to the list. vendorSpecific.add(new ConfigProperty()); - } else if (qName.equals(CONFIG_PROPERTY_NAME)) { - } else if (qName.equals(CONFIG_PROPERTY_VALUE)) { - } else if (qName.equals(CONFIG_PROPERTY_TYPE)) { - } else if (qName.equals(PARTITION_RESOLVER)) { - } else if (qName.equals(PARTITION_LISTENER)) { } else if (qName.equals(FUNCTION_SERVICE)) { startFunctionService(); - } else if (qName.equals(FUNCTION)) { } else if (qName.equals(TOP_SERIALIZER_REGISTRATION)) { startSerializerRegistration(); } else if (qName.equals(INITIALIZER)) { startInitializer(); } else if (qName.equals(INSTANTIATOR_REGISTRATION)) { startInstantiator(atts); - } else if (qName.equals(SERIALIZER_REGISTRATION)) { - // do nothing } else if (qName.equals(RESOURCE_MANAGER)) { startResourceManager(atts); - } else if (qName.equals(BACKUP)) { - // do nothing } else if (qName.equals(PDX)) { startPdx(atts); - } else if (qName.equals(PDX_SERIALIZER)) { - // do nothing - } else if (qName.equals(COMPRESSOR)) { - } else if (qName.equals(SOCKET_FACTORY)) { - } else { + } else if (!isXmlElementRequiringNoStartAction(qName)) { final XmlParser delegate = getDelegate(namespaceURI); if (null == delegate) { throw new CacheXmlException( String.format("Unknown XML element %s", qName)); } - delegate.startElement(namespaceURI, localName, qName, atts); } } + // This method checks if the given qName is any of the CacheXml elements for which no action is + // taken during parsing for startElement() + private boolean isXmlElementRequiringNoStartAction(String qName) { + return qName.equals(ASYNC_EVENT_LISTENER) || qName.equals(BACKUP) + || qName.equals(CACHE_LISTENER) || qName.equals(CACHE_LOADER) || qName.equals(CACHE_WRITER) + || qName.equals(CLASS_NAME) || qName.equals(COMPRESSOR) + || qName.equals(CONFIG_PROPERTY_NAME) || qName.equals(CONFIG_PROPERTY_TYPE) + || qName.equals(CONFIG_PROPERTY_VALUE) || qName.equals(CUSTOM_EXPIRY) + || qName.equals(DECLARABLE) || qName.equals(DISK_DIRS) || qName.equals(ENTRY) + || qName.equals(ENTRY_IDLE_TIME) || qName.equals(ENTRY_TIME_TO_LIVE) + || qName.equals(EVICTION_ATTRIBUTES) || qName.equals(FUNCTION) + || qName.equals(GATEWAY_CONFLICT_RESOLVER) || qName.equals(GATEWAY_EVENT_FILTER) + || qName.equals(GATEWAY_EVENT_LISTENER) || qName.equals(GATEWAY_EVENT_SUBSTITUTION_FILTER) + || qName.equals(GATEWAY_TRANSPORT_FILTER) || qName.equals(GROUP) + || qName.equals(INDEX_UPDATE_TYPE) || qName.equals(JNDI_BINDINGS) || qName.equals(KEY) + || qName.equals(KEY_CONSTRAINT) || qName.equals(LOAD_PROBE) + || qName.equals(PARTITION_LISTENER) || qName.equals(PARTITION_RESOLVER) + || qName.equals(PDX_SERIALIZER) || qName.equals(REGION_IDLE_TIME) + || qName.equals(REGION_TIME_TO_LIVE) || qName.equals(SERIALIZER_REGISTRATION) + || qName.equals(SOCKET_FACTORY) || qName.equals(STRING) + || qName.equals(TRANSACTION_LISTENER) || qName.equals(TRANSACTION_WRITER) + || qName.equals(VALUE) || qName.equals(VALUE_CONSTRAINT); + } + /** * Get delegate {@link XmlParser} for the given namespaceUri * @@ -2995,8 +2971,6 @@ public void endElement(String namespaceURI, String localName, String qName) thro endCustomExpiry(); } else if (qName.equals(DISK_WRITE_ATTRIBUTES)) { endDiskWriteAttributes(); - } else if (qName.equals(SYNCHRONOUS_WRITES)) { - } else if (qName.equals(ASYNCHRONOUS_WRITES)) { } else if (qName.equals(DISK_DIRS)) { endDiskDirs(); } else if (qName.equals(DISK_DIR)) { @@ -3015,10 +2989,6 @@ public void endElement(String namespaceURI, String localName, String qName) thro endMembershipAttributes(); } else if (qName.equals(REQUIRED_ROLE)) { endRequiredRole(); - } else if (qName.equals(EXPIRATION_ATTRIBUTES)) { - } else if (qName.equals(CUSTOM_EXPIRY)) { - endCustomExpiry(); - } else if (qName.equals(SUBSCRIPTION_ATTRIBUTES)) { } else if (qName.equals(ENTRY)) { endEntry(); } else if (qName.equals(CLASS_NAME)) { @@ -3029,9 +2999,6 @@ public void endElement(String namespaceURI, String localName, String qName) thro endCacheLoader(); } else if (qName.equals(CACHE_WRITER)) { endCacheWriter(); - } else if (qName.equals(EVICTION_ATTRIBUTES)) { - } else if (qName.equals(LRU_ENTRY_COUNT)) { - // internal to eviction-attributes } else if (qName.equals(LRU_MEMORY_SIZE)) { endLRUMemorySize(); // internal to eviction-attributes } else if (qName.equals(LRU_HEAP_PERCENTAGE)) { @@ -3040,23 +3007,18 @@ public void endElement(String namespaceURI, String localName, String qName) thro endCacheListener(); } else if (qName.equals(ASYNC_EVENT_LISTENER)) { endAsyncEventListener(); - } else if (qName.equals(KEY)) { - } else if (qName.equals(VALUE)) { } else if (qName.equals(STRING)) { endString(); } else if (qName.equals(DECLARABLE)) { endDeclarable(); - } else if (qName.equals(FUNCTIONAL)) { } else if (qName.equals(INDEX)) { endIndex(); - } else if (qName.equals(PRIMARY_KEY)) { } else if (qName.equals(TRANSACTION_MANAGER)) { endCacheTransactionManager(); } else if (qName.equals(TRANSACTION_LISTENER)) { endTransactionListener(); } else if (qName.equals(TRANSACTION_WRITER)) { endTransactionWriter(); - } else if (qName.equals(JNDI_BINDINGS)) { } else if (qName.equals(JNDI_BINDING)) { BindingCreation bc = (BindingCreation) this.stack.pop(); Map map = bc.getGFSpecificMap(); @@ -3068,12 +3030,12 @@ public void endElement(String namespaceURI, String localName, String qName) thro logger.warn("jndi-binding creation of {} failed with: {}", map.get("jndi-name"), ex); } } - } else if (qName.equals(CONFIG_PROPERTY_BINDING)) { } else if (qName.equals(CONFIG_PROPERTY_NAME)) { String name = null; - if (this.stack.peek() instanceof StringBuffer) + if (this.stack.peek() instanceof StringBuffer) { // Pop the config-property-name element value from the stack. name = ((StringBuffer) this.stack.pop()).toString(); + } BindingCreation bc = (BindingCreation) this.stack.peek(); List vsList = bc.getVendorSpecificList(); ConfigProperty cp = (ConfigProperty) vsList.get(vsList.size() - 1); @@ -3089,8 +3051,9 @@ public void endElement(String namespaceURI, String localName, String qName) thro } else if (qName.equals(CONFIG_PROPERTY_VALUE)) { String value = null; // Pop the config-property-value element value from the stack. - if (this.stack.peek() instanceof StringBuffer) + if (this.stack.peek() instanceof StringBuffer) { value = ((StringBuffer) this.stack.pop()).toString(); + } BindingCreation bc = (BindingCreation) this.stack.peek(); List vsList = bc.getVendorSpecificList(); ConfigProperty cp = (ConfigProperty) vsList.get(vsList.size() - 1); @@ -3098,8 +3061,9 @@ public void endElement(String namespaceURI, String localName, String qName) thro cp.setValue(value); } else if (qName.equals(CONFIG_PROPERTY_TYPE)) { String type = null; - if (this.stack.peek() instanceof StringBuffer) + if (this.stack.peek() instanceof StringBuffer) { type = ((StringBuffer) this.stack.pop()).toString(); + } BindingCreation bc = (BindingCreation) this.stack.peek(); List vsList = bc.getVendorSpecificList(); ConfigProperty cp = (ConfigProperty) vsList.get(vsList.size() - 1); @@ -3111,14 +3075,6 @@ public void endElement(String namespaceURI, String localName, String qName) thro } else { cp.setType(type); } - } else if (qName.equals(LRU_MEMORY_SIZE)) { // internal to eviction-attributes - // Visit startLRUMemorySize() to know the begining - // of lru-memory-size eviction configuration - endLRUMemorySize(); - } else if (qName.equals(LOCATOR)) { - // nothing needed - } else if (qName.equals(SERVER)) { - // nothing needed } else if (qName.equals(PARTITION_RESOLVER)) { endPartitionResolver(); } else if (qName.equals(PARTITION_LISTENER)) { @@ -3139,15 +3095,13 @@ public void endElement(String namespaceURI, String localName, String qName) thro endResourceManager(); } else if (qName.equals(BACKUP)) { endBackup(); - } else if (qName.equals(PDX)) { - // nothing needed } else if (qName.equals(PDX_SERIALIZER)) { endPdxSerializer(); } else if (qName.equals(COMPRESSOR)) { endCompressor(); } else if (qName.equals(SOCKET_FACTORY)) { endSocketFactory(); - } else { + } else if (!isXmlElementRequiringNoEndAction(qName)) { final XmlParser delegate = getDelegate(namespaceURI); if (null == delegate) { throw new CacheXmlException( @@ -3163,6 +3117,15 @@ public void endElement(String namespaceURI, String localName, String qName) thro } } + private boolean isXmlElementRequiringNoEndAction(String qName) { + return qName.equals(ASYNCHRONOUS_WRITES) || qName.equals(CONFIG_PROPERTY_BINDING) + || qName.equals(EVICTION_ATTRIBUTES) || qName.equals(EXPIRATION_ATTRIBUTES) + || qName.equals(FUNCTIONAL) || qName.equals(JNDI_BINDINGS) || qName.equals(KEY) + || qName.equals(VALUE) || qName.equals(LOCATOR) || qName.equals(LRU_ENTRY_COUNT) + || qName.equals(PDX) || qName.equals(PRIMARY_KEY) || qName.equals(SERVER) + || qName.equals(SUBSCRIPTION_ATTRIBUTES) || qName.equals(SYNCHRONOUS_WRITES); + } + private void endSocketFactory() { Declarable d = createDeclarable(); if (!(d instanceof SocketFactory)) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlPropertyResolver.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlPropertyResolver.java index 6c8c2a092801..2359357202db 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlPropertyResolver.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlPropertyResolver.java @@ -119,9 +119,7 @@ public String processUnresolvableString(String stringWithPrefixAndSuffix, String resolvedString = helper.parseResolvablePropString(stringWithPrefixAndSuffix, this, new HashSet()); } catch (IllegalArgumentException e) { - if (ignoreUnresolvedProperties) { - // Do Nothing - } else { + if (!ignoreUnresolvedProperties) { logger.error("Format of the string {} used for perameterization is unresolvable", stringWithPrefixAndSuffix); } diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/XmlGeneratorUtils.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/XmlGeneratorUtils.java index d16e267dfebc..145d3c7ea61a 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/XmlGeneratorUtils.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/XmlGeneratorUtils.java @@ -166,19 +166,16 @@ public static void addDeclarable(final ContentHandler handler, Declarable declar handler.startElement("", PARAMETER, PARAMETER, atts); + // Ignore the value if it's not a String or Declarable if (value instanceof String) { String sValue = (String) value; handler.startElement("", STRING, STRING, EMPTY); handler.characters(sValue.toCharArray(), 0, sValue.length()); handler.endElement("", STRING, STRING); - } else if (value instanceof Declarable) { handler.startElement("", DECLARABLE, DECLARABLE, EMPTY); addDeclarable(handler, (Declarable) value); handler.endElement("", DECLARABLE, DECLARABLE); - - } else { - // Ignore it } handler.endElement("", PARAMETER, PARAMETER); diff --git a/geode-core/src/main/java/org/apache/geode/internal/concurrent/CompactConcurrentHashSet2.java b/geode-core/src/main/java/org/apache/geode/internal/concurrent/CompactConcurrentHashSet2.java index 287e6f45e3db..2f4d84cc2dbd 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/concurrent/CompactConcurrentHashSet2.java +++ b/geode-core/src/main/java/org/apache/geode/internal/concurrent/CompactConcurrentHashSet2.java @@ -1587,8 +1587,9 @@ Node find(int h, Object k) { Thread w; int ls; - do { - } while (!U.compareAndSwapInt(this, LOCKSTATE, ls = lockState, ls - READER)); + while (!U.compareAndSwapInt(this, LOCKSTATE, ls = lockState, ls - READER)) { + // Intentionally empty block + } if (ls == (READER | WAITER) && (w = waiter) != null) LockSupport.unpark(w); } diff --git a/geode-core/src/main/java/org/apache/geode/internal/datasource/FacetsJCAConnectionManagerImpl.java b/geode-core/src/main/java/org/apache/geode/internal/datasource/FacetsJCAConnectionManagerImpl.java index 1f78623d3fd9..7d62c4b70614 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/datasource/FacetsJCAConnectionManagerImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/datasource/FacetsJCAConnectionManagerImpl.java @@ -172,8 +172,6 @@ public void connectionErrorOccurred(ConnectionEvent event) { Transaction txn = transManager.getTransaction(); if (txn == null) { mannPoolCache.returnPooledConnectionToPool(conn); - } else { - // do nothing. } } catch (Exception se) { se.printStackTrace(); diff --git a/geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java b/geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java index f1a66c50eaf7..d0d67e54636a 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java +++ b/geode-core/src/main/java/org/apache/geode/internal/jta/TransactionManagerImpl.java @@ -513,9 +513,8 @@ public void setRollbackOnly() throws IllegalStateException, SystemException { gtx.setRollbackOnly(); else if (status == Status.STATUS_COMMITTING) gtx.setStatus(Status.STATUS_ROLLING_BACK); - else if (status == Status.STATUS_ROLLING_BACK) - ; // Dont do anything - else { + // Dont do anything if status == STATUS_ROLLING_BACK + else if (status != Status.STATUS_ROLLING_BACK) { String exception = String.format("Transaction cannot be marked for rollback. Transcation status, %s", Integer.valueOf(status)); diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java index 15a129ecc719..3d317a225a51 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java +++ b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCloser.java @@ -266,9 +266,7 @@ private static void inlineClose(final Socket sock) { // Sun's NIO implementation has been known to throw Errors // that are caused by IOExceptions. If this is the case, it's // okay. - if (e.getCause() instanceof IOException) { - // okay... - } else { + if (!(e.getCause() instanceof IOException)) { throw e; } } diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java index 47c5287b8b68..77c351b7e599 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java +++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveHandler.java @@ -429,9 +429,8 @@ private void changeArchiveFile(File newFile, boolean resetHandler, long nanosTim * archiveId to the name for supporting file rolling */ File getRollingArchiveName(File archive, boolean archiveClosed) { - if (mainArchiveId != -1) { - // leave mainArchiveId as is. Bump archiveId. - } else { + // If the below evaluates to false, leave mainArchiveId as is. Bump archiveId. + if (mainArchiveId == -1) { archiveDir = archive.getAbsoluteFile().getParentFile(); boolean mainArchiveIdCalculated = false; if (config.getLogFile().isPresent()) { @@ -487,9 +486,8 @@ File getRollingArchiveName(File archive, boolean archiveClosed) { .append(this.rollingFileHandler.formatId(0)).append(".marker"); File marker = new File(buf.toString()); if (marker.exists()) { - if (!marker.delete()) { - // could not delete it; nothing to be done - } + // If we could not delete it; nothing to be done + marker.delete(); } } if (!archiveClosed) { @@ -509,9 +507,8 @@ File getRollingArchiveName(File archive, boolean archiveClosed) { File marker = new File(buf.toString()); if (!marker.exists()) { try { - if (!marker.createNewFile()) { - // could not create it; that is ok - } + // If we could not create it; that is ok + marker.createNewFile(); } catch (IOException ignore) { // If we can't create the marker that is ok } @@ -561,9 +558,8 @@ void initMainArchiveId(File archive) { File marker = new File(buf.toString()); if (!marker.exists()) { try { - if (!marker.createNewFile()) { - // could not create it; that is ok - } + // if we could not create it; that is ok + marker.createNewFile(); } catch (IOException ignore) { // If we can't create the marker that is ok } diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java index 26f2841edc4a..b7475c30aa3e 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java +++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/StatisticsMonitor.java @@ -99,9 +99,7 @@ protected void monitor(final long millisTimeStamp, private void monitorStatisticIds(final long millisTimeStamp, final List resourceInstances) { - if (!this.statisticIds.isEmpty()) { - // TODO: - } + // TODO } protected void notifyListeners(final StatisticsNotification notification) { diff --git a/geode-core/src/main/java/org/apache/geode/internal/statistics/platform/LinuxProcFsStatistics.java b/geode-core/src/main/java/org/apache/geode/internal/statistics/platform/LinuxProcFsStatistics.java index bd60ac3b161b..5f1e986e2b36 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/statistics/platform/LinuxProcFsStatistics.java +++ b/geode-core/src/main/java/org/apache/geode/internal/statistics/platform/LinuxProcFsStatistics.java @@ -445,9 +445,7 @@ private static void getNetStats(LocalStatisticsImpl stats) { other_recv_errs += recv_errs; other_recv_drop += recv_drop; - if (isloopback) { - /* loopback_xmit_packets = xmit_packets; */ - } else { + if (!isloopback) { other_xmit_packets += xmit_packets; other_xmit_bytes += xmit_bytes; } @@ -861,7 +859,7 @@ protected static long parseAsLong(String number) { long l = 0L; try { l = Long.parseLong(number); - } catch (NumberFormatException nfe) { + } catch (NumberFormatException ignore) { } return l; } diff --git a/geode-core/src/main/java/org/apache/geode/internal/stats50/VMStats50.java b/geode-core/src/main/java/org/apache/geode/internal/stats50/VMStats50.java index 83febd17e4fe..51356a0b8c3d 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/stats50/VMStats50.java +++ b/geode-core/src/main/java/org/apache/geode/internal/stats50/VMStats50.java @@ -159,12 +159,11 @@ public class VMStats50 implements VMStatsContract { try { Class c = ClassPathLoader.getLatest().forName("com.sun.management.UnixOperatingSystemMXBean"); + // leave them null if the below is false if (c.isInstance(osBean)) { m1 = c.getMethod("getMaxFileDescriptorCount", new Class[] {}); m2 = c.getMethod("getOpenFileDescriptorCount", new Class[] {}); bean = osBean; - } else { - // leave them null } // Always set ProcessCpuTime m3 = osBean.getClass().getMethod("getProcessCpuTime", new Class[] {}); diff --git a/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java b/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java index 24f07c7c3d85..9f026a7bba73 100644 --- a/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java +++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java @@ -298,18 +298,19 @@ public int hashCode() { } case OBJECT: { Object objectValue = ur.readObject(ft); - if (objectValue == null) { - // default value of null does not modify hashCode. - } else if (objectValue.getClass().isArray()) { - Class myComponentType = objectValue.getClass().getComponentType(); - if (myComponentType.isPrimitive()) { - ByteSource buffer = getRaw(ft); - hashCode = hashCode * 31 + buffer.hashCode(); + // default value of null does not modify hashCode. + if (objectValue != null) { + if (!objectValue.getClass().isArray()) { + hashCode = hashCode * 31 + objectValue.hashCode(); } else { - hashCode = hashCode * 31 + Arrays.deepHashCode((Object[]) objectValue); + Class myComponentType = objectValue.getClass().getComponentType(); + if (myComponentType.isPrimitive()) { + ByteSource buffer = getRaw(ft); + hashCode = hashCode * 31 + buffer.hashCode(); + } else { + hashCode = hashCode * 31 + Arrays.deepHashCode((Object[]) objectValue); + } } - } else { - hashCode = hashCode * 31 + objectValue.hashCode(); } break; } diff --git a/geode-core/src/test/java/org/apache/geode/cache30/TestHeapLRU.java b/geode-core/src/test/java/org/apache/geode/cache30/TestHeapLRU.java index 0c3eef46f41c..7d9a4edb3830 100644 --- a/geode-core/src/test/java/org/apache/geode/cache30/TestHeapLRU.java +++ b/geode-core/src/test/java/org/apache/geode/cache30/TestHeapLRU.java @@ -61,21 +61,10 @@ public static void main(String[] args) throws Exception { System.err.println("Interrupted"); // FIXME should throw } }); - // thread.start(); - - // ArrayList list = new ArrayList(); for (int i = 0; i < Integer.MAX_VALUE; i++) { - if (i % 1000 == 0) { - // System.out.println("i = " + i); - // list = new ArrayList(); - - } else { - // list.add(new Integer(i)); - } Integer key = new Integer(i % 10000); long[] value = new long[2000]; - // System.out.println("Put " + key + " -> " + value); region.put(key, value); } } diff --git a/geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/RemoteCQTransactionDUnitTest.java b/geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/RemoteCQTransactionDUnitTest.java index f551bb6151fa..e9b29a581f75 100755 --- a/geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/RemoteCQTransactionDUnitTest.java +++ b/geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/RemoteCQTransactionDUnitTest.java @@ -702,9 +702,7 @@ static class OneDestroyAndThenOneCreateCacheWriter extends CacheWriterAdapter { private boolean oneCreate; public void checkSuccess() throws Exception { - if (oneDestroy && oneCreate) { - // chill - } else { + if (!(oneDestroy && oneCreate)) { fail("Didn't get both events. oneDestroy=" + oneDestroy + " oneCreate=" + oneCreate); } } diff --git a/geode-cq/src/distributedTest/java/org/apache/geode/security/FilterPostAuthorization.java b/geode-cq/src/distributedTest/java/org/apache/geode/security/FilterPostAuthorization.java index 1e08045e3d75..ea53bf088bc6 100644 --- a/geode-cq/src/distributedTest/java/org/apache/geode/security/FilterPostAuthorization.java +++ b/geode-cq/src/distributedTest/java/org/apache/geode/security/FilterPostAuthorization.java @@ -181,8 +181,6 @@ public boolean authorizeOperation(String regionName, OperationContext context) { putContext.setSerializedValue(serializedValue, isObject); return true; } - } else if (opCode.equals(OperationCode.PUTALL)) { - // no need for now } else if (opCode.isQuery() || opCode.isExecuteCQ()) { QueryOperationContext queryContext = (QueryOperationContext) context; Object value = queryContext.getQueryResult(); diff --git a/geode-dunit/src/main/java/org/apache/geode/cache/query/internal/index/IndexTrackingTestHook.java b/geode-dunit/src/main/java/org/apache/geode/cache/query/internal/index/IndexTrackingTestHook.java index 6aef3c7c4ec8..dd215684ae52 100644 --- a/geode-dunit/src/main/java/org/apache/geode/cache/query/internal/index/IndexTrackingTestHook.java +++ b/geode-dunit/src/main/java/org/apache/geode/cache/query/internal/index/IndexTrackingTestHook.java @@ -53,10 +53,11 @@ public void hook(int spot) throws RuntimeException { assertTrue(observer instanceof IndexTrackingQueryObserver); IndexTrackingQueryObserver gfObserver = (IndexTrackingQueryObserver) observer; - if (spot == 1) { // before index lookup - } else if (spot == 2) { // before key range index lookup - } else if (spot == 3) { // End of afterIndexLookup call - } else if (spot == 4) { // Before resetting indexInfoMap + // 1 = before key range index lookup + // 2 = End of afterIndexLookup call + // 3 = before index lookup + // 4 = Before resetting indexInfoMap + if (spot == 4) { Map map = gfObserver.getUsedIndexes(); assertEquals(1, map.size()); diff --git a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java index 55563c5a2a9d..4482fa4910ba 100644 --- a/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java +++ b/geode-experimental-driver/src/main/java/org/apache/geode/experimental/driver/ProtobufDriver.java @@ -18,7 +18,6 @@ import java.net.InetSocketAddress; import java.security.GeneralSecurityException; import java.util.HashSet; -import java.util.Objects; import java.util.Set; import org.apache.geode.annotations.Experimental; @@ -100,12 +99,7 @@ public void close() { .setDisconnectClientRequest( ConnectionAPI.DisconnectClientRequest.newBuilder().setReason("Driver closed")) .build(); - final ConnectionAPI.DisconnectClientResponse disconnectClientResponse = - channel.sendRequest(disconnectClientRequest, MessageTypeCase.DISCONNECTCLIENTRESPONSE) - .getDisconnectClientResponse(); - if (Objects.isNull(disconnectClientResponse)) { - // The server did not acknowledge the disconnect request; ignore for now. - } + channel.sendRequest(disconnectClientRequest, MessageTypeCase.DISCONNECTCLIENTRESPONSE); } catch (IOException ioe) { // NOP } finally { diff --git a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java index c7e2d27cb7e4..fe71e971ccb8 100644 --- a/geode-junit/src/main/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java +++ b/geode-junit/src/main/java/org/apache/geode/codeAnalysis/CompiledClassUtils.java @@ -301,9 +301,8 @@ public static List loadClassesAndVariables(InputStream String line; while ((line = in.readLine()) != null) { line = line.trim(); - if (line.startsWith("#") || line.startsWith("//")) { - // comment line - } else { + // Not a comment line + if (!line.startsWith("#") && !line.startsWith("//")) { result.add(new ClassAndVariableDetails(line)); } } diff --git a/geode-junit/src/main/java/org/apache/geode/test/concurrent/CPUContentionService.java b/geode-junit/src/main/java/org/apache/geode/test/concurrent/CPUContentionService.java index c4590840c085..d7c7f0e61fc2 100644 --- a/geode-junit/src/main/java/org/apache/geode/test/concurrent/CPUContentionService.java +++ b/geode-junit/src/main/java/org/apache/geode/test/concurrent/CPUContentionService.java @@ -149,6 +149,7 @@ private static Callable generateProbablePrimes() { private Runnable watchForStop(BooleanSupplier stopCondition) { return () -> { while (!Thread.currentThread().isInterrupted() && !stopCondition.getAsBoolean()) { + // Intentionally empty block } stop(); }; diff --git a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RetryRule.java b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RetryRule.java index d8ee07dd3c27..1c67da763e20 100755 --- a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RetryRule.java +++ b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/RetryRule.java @@ -83,9 +83,6 @@ protected AbstractRetryRule() {} protected void evaluate(final Statement base, final Description description, final int retryCount) throws Throwable { - if (retryCount == 0) { - - } Throwable caughtThrowable = null; for (int count = 0; count < retryCount; count++) { diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java index db2ecaeb4d31..6fcba4e5bd2c 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java @@ -1705,16 +1705,15 @@ public boolean waitForNewMember(ID remoteId) { // remoteId != null latestViewWriteLock.lock(); try { - if (latestView == null) { - // Not sure how this would happen, but see bug 38460. - // No view?? Not found! - } else if (latestView.contains(remoteId)) { - // ARB: check if remoteId is already in membership view. - // If not, then create a latch if needed and wait for the latch to open. - foundRemoteId = true; - } else if ((currentLatch = this.memberLatch.get(remoteId)) == null) { - currentLatch = new CountDownLatch(1); - this.memberLatch.put(remoteId, currentLatch); + if (latestView != null) { + if (latestView.contains(remoteId)) { + // ARB: check if remoteId is already in membership view. + // If not, then create a latch if needed and wait for the latch to open. + foundRemoteId = true; + } else if ((currentLatch = this.memberLatch.get(remoteId)) == null) { + currentLatch = new CountDownLatch(1); + this.memberLatch.put(remoteId, currentLatch); + } } } finally { latestViewWriteLock.unlock(); diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java index b815b48e1b0d..b6735ec01639 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/MemberIdentifierImpl.java @@ -278,16 +278,16 @@ public int compareTo(MemberIdentifier other, boolean compareMemberData, String myName = getName(); String otherName = other.getName(); if (!(other.isPartial() || this.isPartial())) { - if (myName == null && otherName == null) { - // do nothing - } else if (myName == null) { - return -1; - } else if (otherName == null) { - return 1; - } else { - int i = myName.compareTo(otherName); - if (i != 0) { - return i; + if (!(myName == null && otherName == null)) { + if (myName == null) { + return -1; + } else if (otherName == null) { + return 1; + } else { + int i = myName.compareTo(otherName); + if (i != 0) { + return i; + } } } } diff --git a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java index 087533d3520f..da92cd900b33 100644 --- a/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java +++ b/geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java @@ -1043,10 +1043,10 @@ void processMessage(final InstallViewMessage m) { if (m.isPreparing()) { if (this.preparedView != null && this.preparedView.getViewId() >= view.getViewId()) { - if (this.preparedView.getViewId() == view.getViewId() && - this.preparedView.getCreator().equals(view.getCreator())) { - // this can happen if we received two prepares during auto-reconnect - } else { + // The inverse of this case can happen if we received two prepares during auto-reconnect, so + // in that case, do nothing + if (this.preparedView.getViewId() != view.getViewId() || + !this.preparedView.getCreator().equals(view.getCreator())) { // send the conflicting view to the creator of this new view services.getMessenger() .send(new ViewAckMessage<>(view.getViewId(), m.getSender(), this.preparedView)); @@ -1358,10 +1358,9 @@ void processMessage(JoinResponseMessage rsp) { if (rsp.getRejectionMessage() != null) { joinResponse[0] = rsp; joinResponse.notifyAll(); - } else if (rsp.getCurrentView() != null) { - // ignore - we get to join when we receive a view. Joining earlier may - // confuse other members if we've reused an old address - } else { + // If the current view is not null, ignore - we get to join when we receive a view. + // Joining earlier may confuse other members if we've reused an old address + } else if (rsp.getCurrentView() == null) { // we got secret key lets add it services.getMessenger().setClusterSecretKey(rsp.getSecretPk()); } diff --git a/geode-pulse/src/uiTest/java/org/apache/geode/tools/pulse/tests/junit/BaseServiceTest.java b/geode-pulse/src/uiTest/java/org/apache/geode/tools/pulse/tests/junit/BaseServiceTest.java index 1579c125e2df..d1725628e6d3 100644 --- a/geode-pulse/src/uiTest/java/org/apache/geode/tools/pulse/tests/junit/BaseServiceTest.java +++ b/geode-pulse/src/uiTest/java/org/apache/geode/tools/pulse/tests/junit/BaseServiceTest.java @@ -154,11 +154,6 @@ protected static void doLogin() throws Exception { .println("BaseServiceTest :: HTTP request status : " + loginResponse.getStatusLine()); List cookies = cookieStore.getCookies(); - if (cookies.isEmpty()) { - } else { - for (int i = 0; i < cookies.size(); i++) { - } - } } finally { if (loginResponse != null) loginResponse.close(); diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionDUnitTest.java index 447e1a87cdbd..385df3e6fc00 100644 --- a/geode-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionDUnitTest.java +++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/session/SessionDUnitTest.java @@ -165,9 +165,8 @@ protected String[] getSessionNotes(int sessionApp, String sessionCookie) { .getBody(); sesssionObtained = true; } catch (HttpServerErrorException e) { - if (e.getMessage().contains("Internal Server Error")) { - // retry - } else { + // retry if not an Internal Server Error + if (e.getMessage() == null || !e.getMessage().contains("Internal Server Error")) { throw e; } } diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/GeodeServerRunTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/GeodeServerRunTest.java index 1ba35a1bdf72..e9e9d537880b 100755 --- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/GeodeServerRunTest.java +++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/GeodeServerRunTest.java @@ -14,6 +14,8 @@ */ package org.apache.geode.redis.internal; +import static org.apache.geode.test.awaitility.GeodeAwaitility.await; + import org.junit.ClassRule; import org.junit.Ignore; import org.junit.Test; @@ -32,7 +34,6 @@ public class GeodeServerRunTest { @Ignore("This is a no-op test to conveniently run redis api for geode server for local development/testing purposes") public void runGeodeServer() { LogService.getLogger().warn("Server running on port: " + server.getPort()); - while (true) { - } + await().until(() -> false); } } diff --git a/geode-wan/src/integrationTest/java/org/apache/geode/internal/cache/wan/misc/WANConfigurationJUnitTest.java b/geode-wan/src/integrationTest/java/org/apache/geode/internal/cache/wan/misc/WANConfigurationJUnitTest.java index 05d08ad24573..0e9b52e1778a 100644 --- a/geode-wan/src/integrationTest/java/org/apache/geode/internal/cache/wan/misc/WANConfigurationJUnitTest.java +++ b/geode-wan/src/integrationTest/java/org/apache/geode/internal/cache/wan/misc/WANConfigurationJUnitTest.java @@ -74,9 +74,8 @@ public void test_GatewaySender_without_Locator() { sender1.start(); fail("Expected IllegalStateException but not thrown"); } catch (Exception e) { - if ((e instanceof IllegalStateException && e.getMessage().startsWith( + if (!(e instanceof IllegalStateException && e.getMessage().startsWith( "Locators must be configured before starting gateway-sender."))) { - } else { fail("Expected IllegalStateException but received :" + e); } } @@ -121,10 +120,8 @@ public void test_SameGatewaySenderCreatedTwice() { fact.create("NYSender", 2); fail("Expected IllegalStateException but not thrown"); } catch (Exception e) { - if (e instanceof IllegalStateException - && e.getMessage().contains("A GatewaySender with id")) { - - } else { + if (!(e instanceof IllegalStateException && e.getMessage() + .contains("A GatewaySender with id"))) { fail("Expected IllegalStateException but received :" + e); } } @@ -146,9 +143,8 @@ public void test_SameGatewaySenderIdAddedTwice() { factory.addGatewaySenderId(sender1.getId()); fail("Expected IllegalArgumentException but not thrown"); } catch (Exception e) { - if (e instanceof IllegalArgumentException && e.getMessage().contains("is already added")) { - - } else { + if (!(e instanceof IllegalArgumentException && e.getMessage() + .contains("is already added"))) { fail("Expected IllegalStateException but received :" + e); } } @@ -216,8 +212,7 @@ public void test_GatewaySender_Serial_ZERO_DispatcherThread() { GatewaySender sender1 = fact.create("NYSender", 2); fail("Expected GatewaySenderException but not thrown"); } catch (GatewaySenderException e) { - if (e.getMessage().contains("can not be created with dispatcher threads less than 1")) { - } else { + if (!e.getMessage().contains("can not be created with dispatcher threads less than 1")) { fail("Expected IllegalStateException but received :" + e); } } @@ -395,10 +390,8 @@ public void test_GatewaySenderWithGatewaySenderEventListener1() { fail( "Expected GatewaySenderException. When a sender is added , remoteDSId should not be provided."); } catch (Exception e) { - if (e instanceof GatewaySenderException && e.getMessage().contains( - "cannot define a remote site because at least AsyncEventListener is already added.")) { - - } else { + if (!(e instanceof GatewaySenderException && e.getMessage().contains( + "cannot define a remote site because at least AsyncEventListener is already added."))) { fail("Expected GatewaySenderException but received :" + e); } } diff --git a/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java b/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java index 1f14d26184ca..60d5db7d3514 100644 --- a/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java +++ b/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java @@ -132,12 +132,13 @@ protected GatewayAck readAcknowledgement() { // it destroyConnection(); } - if (this.sender.getProxy() == null || this.sender.getProxy().isDestroyed()) { - // if our pool is shutdown then just be silent - } else if (RecoverableExceptionPredicates.isRecoverableWhenReadingAck(ex)) { - sleepBeforeRetry(); - } else { - logAndStopProcessor(ex); + // if our pool is shutdown then just be silent + if (this.sender.getProxy() != null && !this.sender.getProxy().isDestroyed()) { + if (RecoverableExceptionPredicates.isRecoverableWhenReadingAck(ex)) { + sleepBeforeRetry(); + } else { + logAndStopProcessor(ex); + } } } finally { messageProcessingAttempted.accept(true); @@ -158,18 +159,19 @@ public boolean dispatchBatch(List events, boolean removeFromQueueOnException, bo } } catch (GatewaySenderException ge) { Throwable t = ge.getCause(); - if (this.sender.getProxy() == null || this.sender.getProxy().isDestroyed()) { - // if our pool is shutdown then just be silent - } else if (RecoverableExceptionPredicates.isRecoverableWhenDispatchingBatch(t)) { - this.processor.handleException(); - sleepBeforeRetry(); - if (logger.isDebugEnabled()) { - logger.debug( - "Failed to dispatch a batch with id {} due to non-fatal exception {}. Retrying in {} ms", - this.processor.getBatchId(), t, RETRY_WAIT_TIME); + // if our pool is shutdown then just be silent + if (this.sender.getProxy() != null && !this.sender.getProxy().isDestroyed()) { + if (RecoverableExceptionPredicates.isRecoverableWhenDispatchingBatch(t)) { + this.processor.handleException(); + sleepBeforeRetry(); + if (logger.isDebugEnabled()) { + logger.debug( + "Failed to dispatch a batch with id {} due to non-fatal exception {}. Retrying in {} ms", + this.processor.getBatchId(), t, RETRY_WAIT_TIME); + } + } else { + logAndStopProcessor(ge); } - } else { - logAndStopProcessor(ge); } } catch (CancelException e) { logAndStopProcessor(e); diff --git a/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderImpl.java b/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderImpl.java index 1ca5d9ae8c25..038af4f50804 100644 --- a/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderImpl.java +++ b/geode-wan/src/main/java/org/apache/geode/internal/cache/wan/serial/SerialGatewaySenderImpl.java @@ -235,9 +235,8 @@ public void setModifiedEventId(EntryEventImpl clonedEvent) { EventID originalEventId = clonedEvent.getEventId(); long originalThreadId = originalEventId.getThreadID(); long newThreadId = originalThreadId; - if (ThreadIdentifier.isWanTypeThreadID(newThreadId)) { - // This thread id has already been converted. Do nothing. - } else { + // If isWanTypeThreadID() returns true, this thread id has already been converted. Do nothing. + if (!ThreadIdentifier.isWanTypeThreadID(newThreadId)) { newThreadId = ThreadIdentifier.createFakeThreadIDForParallelGSPrimaryBucket(0, originalThreadId, getEventIdIndex()); }