From e88e58b422491db5e9dfd6a0cb3ba5b548f2ab97 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 12:35:18 +0530 Subject: [PATCH 1/7] Add DCN_NULLPOINTER_EXCEPTION check --- codestyle/spotbugs-exclude.xml | 5 ++--- .../apache/druid/query/extraction/BucketExtractionFn.java | 2 +- .../segment/incremental/SpatialDimensionRowTransformer.java | 6 +++++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index 9a2ba6ecf1c9..e8765631be30 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -137,11 +137,10 @@ - - + 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; } } From 45472f4b11437709d82a6e2df061ef378e888156 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 12:56:57 +0530 Subject: [PATCH 2/7] Add PA_PUBLIC_PRIMITIVE_ATTRIBUTE check --- codestyle/spotbugs-exclude.xml | 5 ++--- .../aggregation/histogram/ApproximateHistogram.java | 4 ++-- .../org/apache/druid/query/aggregation/Histogram.java | 10 +++++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index e8765631be30..23b4e61158fd 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -139,11 +139,10 @@ - - + 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..e4b3323c4c2c 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; + private 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..769276e421c9 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; + private float[] breaks; + protected long[] bins; + protected transient long count; + private float min; + private float max; public Histogram(float[] breaks) { From ec74fa934c66722aa31d38a6ac281ed66925cf54 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 13:02:12 +0530 Subject: [PATCH 3/7] Add EI_EXPOSE_STATIC_REP2 check --- codestyle/spotbugs-exclude.xml | 1 - .../druid/server/initialization/jetty/JettyServerModule.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index 23b4e61158fd..3bc677a0a8c6 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -143,7 +143,6 @@ - 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; } From aff463aa3e1c0e081cf4e21e729b022a90aeb4fd Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 13:09:36 +0530 Subject: [PATCH 4/7] Add SS_SHOULD_BE_STATIC check --- codestyle/spotbugs-exclude.xml | 1 - .../k8s/overlord/KubernetesAndWorkerTaskRunnerConfig.java | 4 ++-- .../CoordinatorBasicAuthenticatorMetadataStorageUpdater.java | 2 +- .../CoordinatorBasicAuthorizerMetadataStorageUpdater.java | 2 +- .../main/java/org/apache/druid/security/pac4j/OIDCConfig.java | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index 3bc677a0a8c6..a5b9e0a0cad1 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -143,6 +143,5 @@ - 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..3650b7599490 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 numRetries = 5; private final Map cachedUserMaps; private final Set authenticatorPrefixes; 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; From 349d7c539f0b1bed8cb179196cd515bfe1a33c8d Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 13:14:24 +0530 Subject: [PATCH 5/7] Remove the comment --- codestyle/spotbugs-exclude.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/codestyle/spotbugs-exclude.xml b/codestyle/spotbugs-exclude.xml index a5b9e0a0cad1..9776e554b8e7 100644 --- a/codestyle/spotbugs-exclude.xml +++ b/codestyle/spotbugs-exclude.xml @@ -141,7 +141,5 @@ - From 1c33ae9e017578c9f5d9c212825322d06ebce20c Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 14:18:20 +0530 Subject: [PATCH 6/7] Fix checkstyle --- ...ordinatorBasicAuthenticatorMetadataStorageUpdater.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 3650b7599490..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 static 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 { From 5802147cd9307701a10e5b928afd08a669161114 Mon Sep 17 00:00:00 2001 From: Akshat Jain Date: Thu, 12 Dec 2024 15:56:00 +0530 Subject: [PATCH 7/7] Address review comments --- .../query/aggregation/histogram/ApproximateHistogram.java | 2 +- .../java/org/apache/druid/query/aggregation/Histogram.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) 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 e4b3323c4c2c..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 @@ -42,7 +42,7 @@ public class ApproximateHistogram int size; protected float[] positions; - private long[] bins; + 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 769276e421c9..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 { - private float[] breaks; + protected float[] breaks; protected long[] bins; protected transient long count; - private float min; - private float max; + protected float min; + protected float max; public Histogram(float[] breaks) {