You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by ed...@apache.org on 2022/08/11 14:17:49 UTC
[accumulo] branch main updated: Fix additional spotbugs errors in core (#2861)
This is an automated email from the ASF dual-hosted git repository.
edcoleman pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 6d3aeccd8e Fix additional spotbugs errors in core (#2861)
6d3aeccd8e is described below
commit 6d3aeccd8e86853bb8cf88b7590673787e127d0a
Author: EdColeman <de...@etcoleman.com>
AuthorDate: Thu Aug 11 14:17:45 2022 +0000
Fix additional spotbugs errors in core (#2861)
---
.../accumulo/core/iterators/user/RegExFilter.java | 4 +++
.../core/metadata/MetadataLocationObtainer.java | 6 ++--
.../accumulo/core/util/LocalityGroupUtil.java | 4 +++
.../java/org/apache/accumulo/fate/ZooStore.java | 2 +-
.../accumulo/core/client/ClientPropertiesTest.java | 4 +--
.../core/spi/balancer/TableLoadBalancerTest.java | 4 +--
.../core/spi/fs/SpaceAwareVolumeChooserTest.java | 37 +++++++++++-----------
.../accumulo/core/util/PreAllocatedArrayTest.java | 10 ++++--
.../org/apache/accumulo/fate/util/RetryTest.java | 2 +-
9 files changed, 43 insertions(+), 30 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/user/RegExFilter.java b/core/src/main/java/org/apache/accumulo/core/iterators/user/RegExFilter.java
index 971031ac20..0eecd82562 100644
--- a/core/src/main/java/org/apache/accumulo/core/iterators/user/RegExFilter.java
+++ b/core/src/main/java/org/apache/accumulo/core/iterators/user/RegExFilter.java
@@ -35,6 +35,8 @@ import org.apache.accumulo.core.iterators.Filter;
import org.apache.accumulo.core.iterators.IteratorEnvironment;
import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
/**
* A Filter that matches entries based on Java regular expressions.
*/
@@ -169,6 +171,8 @@ public class RegExFilter extends Filter {
}
@Override
+ @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT",
+ justification = "Pattern.compile used to validate - no matching performed")
public boolean validateOptions(Map<String,String> options) {
if (!super.validateOptions(options))
return false;
diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/MetadataLocationObtainer.java b/core/src/main/java/org/apache/accumulo/core/metadata/MetadataLocationObtainer.java
index 54631a7730..1396942eec 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/MetadataLocationObtainer.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/MetadataLocationObtainer.java
@@ -115,9 +115,9 @@ public class MetadataLocationObtainer implements TabletLocationObtainer {
range = new Range(results.lastKey().followingKey(PartialKey.ROW_COLFAM_COLQUAL_COLVIS_TIME),
true, new Key(stopRow).followingKey(PartialKey.ROW), false);
encodedResults.clear();
- more = ThriftScanner.getBatchFromServer(context, range, src.tablet_extent,
- src.tablet_location, encodedResults, locCols, serverSideIteratorList,
- serverSideIteratorOptions, Constants.SCAN_BATCH_SIZE, Authorizations.EMPTY, 0L, null);
+ ThriftScanner.getBatchFromServer(context, range, src.tablet_extent, src.tablet_location,
+ encodedResults, locCols, serverSideIteratorList, serverSideIteratorOptions,
+ Constants.SCAN_BATCH_SIZE, Authorizations.EMPTY, 0L, null);
decodeRows(encodedResults, results);
}
diff --git a/core/src/main/java/org/apache/accumulo/core/util/LocalityGroupUtil.java b/core/src/main/java/org/apache/accumulo/core/util/LocalityGroupUtil.java
index ffaba42c00..7e831c864e 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/LocalityGroupUtil.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/LocalityGroupUtil.java
@@ -54,6 +54,8 @@ import org.slf4j.LoggerFactory;
import com.google.common.base.Joiner;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
public class LocalityGroupUtil {
private static final Logger log = LoggerFactory.getLogger(LocalityGroupUtil.class);
@@ -274,6 +276,8 @@ public class LocalityGroupUtil {
}
@Override
+ @SuppressFBWarnings(value = "EQ_UNUSUAL",
+ justification = "method expected to be unused or overridden")
public boolean equals(Object o) {
throw new UnsupportedOperationException();
}
diff --git a/core/src/main/java/org/apache/accumulo/fate/ZooStore.java b/core/src/main/java/org/apache/accumulo/fate/ZooStore.java
index 7262e0f159..d78ff8306c 100644
--- a/core/src/main/java/org/apache/accumulo/fate/ZooStore.java
+++ b/core/src/main/java/org/apache/accumulo/fate/ZooStore.java
@@ -208,7 +208,7 @@ public class ZooStore<T> implements TStore<T> {
}
}
}
- } catch (Exception e) {
+ } catch (InterruptedException | KeeperException e) {
throw new RuntimeException(e);
}
}
diff --git a/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java b/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java
index cdd80fd9f9..98c2b06146 100644
--- a/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java
@@ -42,7 +42,7 @@ public class ClientPropertiesTest {
ClientProperty.validate(props1);
Properties props2 =
- Accumulo.newClientProperties().from(props1).as("user2", Paths.get("/path2")).build();
+ Accumulo.newClientProperties().from(props1).as("user2", Paths.get("./path2")).build();
// verify props1 is unchanged
assertEquals("inst1", ClientProperty.INSTANCE_NAME.getValue(props1));
@@ -56,7 +56,7 @@ public class ClientPropertiesTest {
assertEquals("zoo1", ClientProperty.INSTANCE_ZOOKEEPERS.getValue(props2));
assertEquals("user2", ClientProperty.AUTH_PRINCIPAL.getValue(props2));
assertEquals("kerberos", ClientProperty.AUTH_TYPE.getValue(props2));
- assertEquals("/path2", ClientProperty.AUTH_TOKEN.getValue(props2));
+ assertEquals("./path2", ClientProperty.AUTH_TOKEN.getValue(props2));
props2.remove(ClientProperty.AUTH_PRINCIPAL.getKey());
var e = assertThrows(IllegalArgumentException.class, () -> ClientProperty.validate(props2));
diff --git a/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java b/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java
index 89358b6937..9d856e6052 100644
--- a/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/spi/balancer/TableLoadBalancerTest.java
@@ -81,7 +81,7 @@ public class TableLoadBalancerTest {
return new TServerStatusImpl(thriftStatus);
}
- static SortedMap<TabletServerId,TServerStatus> state;
+ private static final SortedMap<TabletServerId,TServerStatus> state = new TreeMap<>();
static List<TabletStatistics> generateFakeTablets(TabletServerId tserver, TableId tableId) {
List<TabletStatistics> result = new ArrayList<>();
@@ -133,7 +133,7 @@ public class TableLoadBalancerTest {
String t1Id = TABLE_ID_MAP.get("t1"), t2Id = TABLE_ID_MAP.get("t2"),
t3Id = TABLE_ID_MAP.get("t3");
- state = new TreeMap<>();
+ state.clear();
TabletServerId svr = mkts("10.0.0.1", 1234, "0x01020304");
state.put(svr, status(t1Id, 10, t2Id, 10, t3Id, 10));
diff --git a/core/src/test/java/org/apache/accumulo/core/spi/fs/SpaceAwareVolumeChooserTest.java b/core/src/test/java/org/apache/accumulo/core/spi/fs/SpaceAwareVolumeChooserTest.java
index 7699a9e046..da4a59d431 100644
--- a/core/src/test/java/org/apache/accumulo/core/spi/fs/SpaceAwareVolumeChooserTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/spi/fs/SpaceAwareVolumeChooserTest.java
@@ -21,7 +21,6 @@ package org.apache.accumulo.core.spi.fs;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
-import java.io.IOException;
import java.util.Set;
import org.apache.accumulo.core.spi.common.ServiceEnvironment;
@@ -62,7 +61,7 @@ public class SpaceAwareVolumeChooserTest {
}
private void testSpecificSetup(long percentage1, long percentage2, String cacheDuration,
- int timesToCallPreferredVolumeChooser, boolean anyTimes) throws IOException {
+ int timesToCallPreferredVolumeChooser, boolean anyTimes) {
int max = iterations + 1;
int min = 1;
int updatePropertyMax = timesToCallPreferredVolumeChooser + iterations;
@@ -99,75 +98,75 @@ public class SpaceAwareVolumeChooserTest {
}
@Test
- public void testEvenWeightsWithCaching() throws IOException {
+ public void testEvenWeightsWithCaching() {
testSpecificSetup(10L, 10L, null, iterations, false);
makeChoices();
- assertEquals(iterations / 2, vol1Count, iterations / 10);
- assertEquals(iterations / 2, vol2Count, iterations / 10);
+ assertEquals(iterations / 2.0, vol1Count, iterations / 10.0);
+ assertEquals(iterations / 2.0, vol2Count, iterations / 10.0);
}
@Test
- public void testEvenWeightsNoCaching() throws IOException {
+ public void testEvenWeightsNoCaching() {
testSpecificSetup(10L, 10L, "0", iterations, true);
makeChoices();
- assertEquals(iterations / 2, vol1Count, iterations / 10);
- assertEquals(iterations / 2, vol2Count, iterations / 10);
+ assertEquals(iterations / 2.0, vol1Count, iterations / 10.0);
+ assertEquals(iterations / 2.0, vol2Count, iterations / 10.0);
}
@Test
- public void testNoFreeSpace() throws IOException {
+ public void testNoFreeSpace() {
testSpecificSetup(0L, 0L, null, 1, false);
assertThrows(UncheckedExecutionException.class, this::makeChoices);
}
@Test
- public void testNinetyTen() throws IOException {
+ public void testNinetyTen() {
testSpecificSetup(90L, 10L, null, iterations, false);
makeChoices();
- assertEquals(iterations * .9, vol1Count, iterations / 10);
- assertEquals(iterations * .1, vol2Count, iterations / 10);
+ assertEquals(iterations * .9, vol1Count, iterations / 10.0);
+ assertEquals(iterations * .1, vol2Count, iterations / 10.0);
}
@Test
- public void testTenNinety() throws IOException {
+ public void testTenNinety() {
testSpecificSetup(10L, 90L, null, iterations, false);
makeChoices();
- assertEquals(iterations * .1, vol1Count, iterations / 10);
- assertEquals(iterations * .9, vol2Count, iterations / 10);
+ assertEquals(iterations * .1, vol1Count, iterations / 10.0);
+ assertEquals(iterations * .9, vol2Count, iterations / 10.0);
}
@Test
- public void testWithNoCaching() throws IOException {
+ public void testWithNoCaching() {
testSpecificSetup(10L, 90L, "0", iterations, true);
makeChoices();
- assertEquals(iterations * .1, vol1Count, iterations / 10);
- assertEquals(iterations * .9, vol2Count, iterations / 10);
+ assertEquals(iterations * .1, vol1Count, iterations / 10.0);
+ assertEquals(iterations * .9, vol2Count, iterations / 10.0);
}
private void makeChoices() {
SpaceAwareVolumeChooser chooser = new SpaceAwareVolumeChooser() {
@Override
- protected double getFreeSpace(String uri) throws IOException {
+ protected double getFreeSpace(String uri) {
if (uri.equals(volumeOne))
return free1;
if (uri.equals(volumeTwo))
diff --git a/core/src/test/java/org/apache/accumulo/core/util/PreAllocatedArrayTest.java b/core/src/test/java/org/apache/accumulo/core/util/PreAllocatedArrayTest.java
index b01dc0fe6d..3249716ce6 100644
--- a/core/src/test/java/org/apache/accumulo/core/util/PreAllocatedArrayTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/util/PreAllocatedArrayTest.java
@@ -27,6 +27,10 @@ import java.util.Iterator;
import org.junit.jupiter.api.Test;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+@SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT",
+ justification = "lambda assertThrows testing exception thrown")
public class PreAllocatedArrayTest {
/**
@@ -97,14 +101,16 @@ public class PreAllocatedArrayTest {
@Test
public void testGetIndexHigh() {
PreAllocatedArray<String> strings = new PreAllocatedArray<>(3);
- strings.get(2);
+ assertNull(strings.get(2));
+ // spotbugs error suppressed at class level for lambda
assertThrows(IndexOutOfBoundsException.class, () -> strings.get(3));
}
@Test
public void testGetIndexNegative() {
PreAllocatedArray<String> strings = new PreAllocatedArray<>(3);
- strings.get(0);
+ assertNull(strings.get(0));
+ // spotbugs error suppressed at class level for lambda
assertThrows(IndexOutOfBoundsException.class, () -> strings.get(-3));
}
}
diff --git a/core/src/test/java/org/apache/accumulo/fate/util/RetryTest.java b/core/src/test/java/org/apache/accumulo/fate/util/RetryTest.java
index 01d76823ef..8e621893cc 100644
--- a/core/src/test/java/org/apache/accumulo/fate/util/RetryTest.java
+++ b/core/src/test/java/org/apache/accumulo/fate/util/RetryTest.java
@@ -81,7 +81,7 @@ public class RetryTest {
for (int i = 0; i < MAX_RETRIES; i++) {
assertEquals(i, retry.retriesCompleted());
// canRetry doesn't alter retry's state
- retry.canRetry();
+ assertTrue(retry.canRetry());
assertEquals(i, retry.retriesCompleted());
// Using the retry will increase the internal count
retry.useRetry();