From fed36844f186637ebe39be20a292c7fb8f37da32 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Fri, 13 Dec 2024 19:54:40 +0530 Subject: [PATCH] Re-visit previously disabled spotbugs patterns and enable them (#17560) --- codestyle/spotbugs-exclude.xml | 6 ------ .../overlord/KubernetesAndWorkerTaskRunnerConfig.java | 4 ++-- ...inatorBasicAuthenticatorMetadataStorageUpdater.java | 8 ++++---- ...ordinatorBasicAuthorizerMetadataStorageUpdater.java | 2 +- .../org/apache/druid/security/pac4j/OIDCConfig.java | 2 +- .../aggregation/histogram/ApproximateHistogram.java | 4 ++-- .../org/apache/druid/query/aggregation/Histogram.java | 10 +++++----- .../druid/query/extraction/BucketExtractionFn.java | 2 +- .../incremental/SpatialDimensionRowTransformer.java | 6 +++++- .../server/initialization/jetty/JettyServerModule.java | 2 +- 10 files changed, 22 insertions(+), 24 deletions(-) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index 9a2ba6ecf1c9..9776e554b8e7 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -137,15 +137,9 @@ - - - - - diff --git a/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesAndWorkerTaskRunnerConfig.java b/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesAndWorkerTaskRunnerConfig.java index 0ffeb0103afa..3efbfa18dce1 100644 --- a/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesAndWorkerTaskRunnerConfig.java +++ b/extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesAndWorkerTaskRunnerConfig.java @@ -31,8 +31,8 @@ public class KubernetesAndWorkerTaskRunnerConfig { - private final String RUNNER_STRATEGY_TYPE = "runnerStrategy.type"; - private final String RUNNER_STRATEGY_WORKER_TYPE = "runnerStrategy.workerType"; + private static final String RUNNER_STRATEGY_TYPE = "runnerStrategy.type"; + private static final String RUNNER_STRATEGY_WORKER_TYPE = "runnerStrategy.workerType"; /** * Select which runner type a task would run on, options are k8s or worker. */ diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java index a83c20e422d4..c0761c03ec21 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/db/updater/CoordinatorBasicAuthenticatorMetadataStorageUpdater.java @@ -74,7 +74,7 @@ public class CoordinatorBasicAuthenticatorMetadataStorageUpdater implements Basi private final BasicAuthCommonCacheConfig commonCacheConfig; private final ObjectMapper objectMapper; private final BasicAuthenticatorCacheNotifier cacheNotifier; - private final int numRetries = 5; + private static final int NUM_RETRIES = 5; private final Map cachedUserMaps; private final Set authenticatorPrefixes; @@ -294,7 +294,7 @@ private static String getPrefixedKeyColumn(String keyPrefix, String keyName) private void createUserInternal(String prefix, String userName) { int attempts = 0; - while (attempts < numRetries) { + while (attempts < NUM_RETRIES) { if (createUserOnce(prefix, userName)) { return; } else { @@ -308,7 +308,7 @@ private void createUserInternal(String prefix, String userName) private void deleteUserInternal(String prefix, String userName) { int attempts = 0; - while (attempts < numRetries) { + while (attempts < NUM_RETRIES) { if (deleteUserOnce(prefix, userName)) { return; } else { @@ -349,7 +349,7 @@ private void setUserCredentialsInternal(String prefix, String userName, BasicAut } int attempts = 0; - while (attempts < numRetries) { + while (attempts < NUM_RETRIES) { if (setUserCredentialOnce(prefix, userName, credentials)) { return; } else { diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java index 2de3ef11cc87..baff948331af 100644 --- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java +++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authorization/db/updater/CoordinatorBasicAuthorizerMetadataStorageUpdater.java @@ -91,7 +91,7 @@ public class CoordinatorBasicAuthorizerMetadataStorageUpdater implements BasicAu private final BasicAuthorizerCacheNotifier cacheNotifier; private final BasicAuthCommonCacheConfig commonCacheConfig; private final ObjectMapper objectMapper; - private final int numRetries = 5; + private static final int numRetries = 5; private final Map cachedUserMaps; private final Map cachedGroupMappingMaps; diff --git a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/OIDCConfig.java b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/OIDCConfig.java index 376181416556..50b04455dbc5 100644 --- a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/OIDCConfig.java +++ b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/OIDCConfig.java @@ -28,7 +28,7 @@ public class OIDCConfig { - private final String DEFAULT_SCOPE = "name"; + private static final String DEFAULT_SCOPE = "name"; @JsonProperty private final String clientID; diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java index fa3772518b3e..12f1afa296de 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java @@ -41,8 +41,8 @@ public class ApproximateHistogram // max size of the histogram (number of bincount/position pairs) int size; - public float[] positions; - public long[] bins; + protected float[] positions; + protected long[] bins; // used bincount int binCount; diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/Histogram.java b/processing/src/main/java/org/apache/druid/query/aggregation/Histogram.java index 708ba84e06ea..ee32ff3d9f5e 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/Histogram.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/Histogram.java @@ -27,11 +27,11 @@ public class Histogram { - public float[] breaks; - public long[] bins; - public transient long count; - public float min; - public float max; + protected float[] breaks; + protected long[] bins; + protected transient long count; + protected float min; + protected float max; public Histogram(float[] breaks) { diff --git a/processing/src/main/java/org/apache/druid/query/extraction/BucketExtractionFn.java b/processing/src/main/java/org/apache/druid/query/extraction/BucketExtractionFn.java index 970116e62169..68769007a77b 100644 --- a/processing/src/main/java/org/apache/druid/query/extraction/BucketExtractionFn.java +++ b/processing/src/main/java/org/apache/druid/query/extraction/BucketExtractionFn.java @@ -81,7 +81,7 @@ public String apply(@Nullable String value) try { return bucket(Double.parseDouble(value)); } - catch (NumberFormatException | NullPointerException ex) { + catch (NumberFormatException ex) { return null; } } diff --git a/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java b/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java index c6b40af11e54..8c4c8d4448be 100644 --- a/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java +++ b/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java @@ -213,10 +213,14 @@ private boolean isJoinedSpatialDimValValid(String dimVal) @Nullable private static Float tryParseFloat(String val) { + if (val == null) { + return null; + } + try { return Float.parseFloat(val); } - catch (NullPointerException | NumberFormatException e) { + catch (NumberFormatException e) { return null; } } diff --git a/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java b/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java index 1299e11d60c0..8ce48871c155 100644 --- a/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java +++ b/server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java @@ -597,7 +597,7 @@ public int getActiveConnections() } @VisibleForTesting - public static void setJettyServerThreadPool(QueuedThreadPool threadPool) + protected static void setJettyServerThreadPool(QueuedThreadPool threadPool) { jettyServerThreadPool = threadPool; }