From 19f1370e57662d4ca58a8933982f83df9da8bac7 Mon Sep 17 00:00:00 2001 From: Christopher Lambert <1204398+XN137@users.noreply.github.com> Date: Fri, 23 May 2025 13:30:22 +0200 Subject: [PATCH] fix and enforce more errorprone checks enforces the following checks: https://errorprone.info/bugpattern/ObjectsHashCodePrimitive https://errorprone.info/bugpattern/OptionalMapToOptional https://errorprone.info/bugpattern/StringCharset https://errorprone.info/bugpattern/VariableNameSameAsType --- .../polaris/service/types/NotificationType.java | 7 +++---- codestyle/errorprone-rules.properties | 12 ++++++++++++ .../secrets/UnsafeInMemorySecretsManager.java | 15 +++------------ .../core/storage/PolarisStorageIntegration.java | 3 +-- .../quarkus/config/ProductionReadinessChecks.java | 2 +- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationType.java b/api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationType.java index 53d7d47779..6485d452ff 100644 --- a/api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationType.java +++ b/api/iceberg-service/src/main/java/org/apache/polaris/service/types/NotificationType.java @@ -19,7 +19,6 @@ package org.apache.polaris.service.types; import java.util.Arrays; -import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; @@ -84,9 +83,9 @@ public static Optional lookupByName(String name) { return Optional.empty(); } - for (NotificationType NotificationType : NotificationType.values()) { - if (name.toUpperCase(Locale.ROOT).equals(NotificationType.name())) { - return Optional.of(NotificationType); + for (NotificationType notificationType : NotificationType.values()) { + if (name.equalsIgnoreCase(notificationType.name())) { + return Optional.of(notificationType); } } return Optional.empty(); diff --git a/codestyle/errorprone-rules.properties b/codestyle/errorprone-rules.properties index 5000e32733..1322ce766d 100644 --- a/codestyle/errorprone-rules.properties +++ b/codestyle/errorprone-rules.properties @@ -227,6 +227,12 @@ UseCorrectAssertInTests=ERROR ConstantPatternCompile=ERROR # Variables initialized with Pattern#compile calls on constants can be constants +ObjectsHashCodePrimitive=ERROR +# Objects.hashCode(Object o) should not be passed a primitive value + +OptionalMapToOptional=ERROR +# Mapping to another Optional will yield a nested Optional. Did you mean flatMap? + PrimitiveArrayPassedToVarargsMethod=ERROR # Passing a primitive array to a varargs method is usually wrong @@ -239,6 +245,9 @@ RedundantThrows=ERROR StringCaseLocaleUsage=ERROR # Specify a `Locale` when calling `String#to{Lower,Upper}Case`. (Note: there are multiple suggested fixes; the third may be most appropriate if you're dealing with ASCII Strings.) +StringCharset=ERROR +# Prefer StandardCharsets over using string names for charsets + StronglyTypeByteString=WARN # This primitive byte array is only used to construct ByteStrings. It would be clearer to strongly type the field instead. @@ -254,6 +263,9 @@ TransientMisuse=ERROR UrlInSee=ERROR # URLs should not be used in @see tags; they are designed for Java elements which could be used with @link. +VariableNameSameAsType=ERROR +# variableName and type with the same name would refer to the static field instead of the class + #################################################################################################### # Experimental : SUGGESTION # See https://errorprone.info/bugpatterns diff --git a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java index 22e8c53786..38cf00e226 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/secrets/UnsafeInMemorySecretsManager.java @@ -19,7 +19,7 @@ package org.apache.polaris.core.secrets; import jakarta.annotation.Nonnull; -import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.security.SecureRandom; import java.util.Base64; import java.util.HashMap; @@ -54,12 +54,7 @@ public UserSecretReference writeSecret( // secret as well as the secretReferencePayload to recover the original secret, we'll use // basic XOR encryption and store the randomly generated key in the reference payload. // A production implementation will typically use a standard crypto library if applicable. - byte[] secretBytes; - try { - secretBytes = secret.getBytes("UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } + byte[] secretBytes = secret.getBytes(StandardCharsets.UTF_8); byte[] oneTimeKey = new byte[secretBytes.length]; byte[] cipherTextBytes = new byte[secretBytes.length]; @@ -143,11 +138,7 @@ public String readSecret(@Nonnull UserSecretReference secretReference) { secretBytes[i] = (byte) (cipherTextBytes[i] ^ oneTimeKey[i]); } - try { - return new String(secretBytes, "UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new RuntimeException(e); - } + return new String(secretBytes, StandardCharsets.UTF_8); } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java index 5f100a943b..e549dd1f8a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java @@ -21,7 +21,6 @@ import jakarta.annotation.Nonnull; import java.util.EnumMap; import java.util.Map; -import java.util.Objects; import java.util.Set; import org.apache.polaris.core.PolarisDiagnostics; @@ -129,7 +128,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hashCode(success); + return Boolean.hashCode(success); } @Override diff --git a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java index 2168fca556..ef2c1813e9 100644 --- a/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java +++ b/quarkus/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java @@ -151,7 +151,7 @@ public ProductionReadinessCheck checkTokenBrokers( if (config .tokenBroker() .symmetricKey() - .map(SymmetricKeyConfiguration::secret) + .flatMap(SymmetricKeyConfiguration::secret) .isPresent()) { errors.add( Error.of(