From 64bb35ca8541b89f00ed0d1c7f28fba8951ca5f1 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 7 Aug 2024 04:39:27 +0900 Subject: [PATCH] Add ErrorProne Plugin for Gradle (#81) * Specify Locale.ROOT in toUpperCase method * Fix string format usage * Avoid double brace initialization in StorageCredentialCacheTest * Add ErrorProne Plugin for Gradle --- build.gradle | 6 +++ gradle/libs.versions.toml | 1 + .../storage/cache/StorageCredentialCache.java | 4 +- .../cache/StorageCredentialCacheTest.java | 38 +++++++++---------- .../service/admin/PolarisAdminService.java | 17 ++++----- .../service/catalog/BasePolarisCatalog.java | 2 +- .../catalog/IcebergCatalogAdapter.java | 2 +- .../service/types/NotificationType.java | 3 +- 8 files changed, 39 insertions(+), 34 deletions(-) diff --git a/build.gradle b/build.gradle index f69bd35f2..bdc7851a3 100644 --- a/build.gradle +++ b/build.gradle @@ -29,6 +29,7 @@ plugins { id "idea" id "eclipse" id "org.jetbrains.gradle.plugin.idea-ext" version "1.1.8" + id "net.ltgt.errorprone" version "4.0.1" } allprojects { @@ -50,10 +51,14 @@ subprojects { apply plugin: "com.diffplug.spotless" apply plugin: "jacoco-report-aggregation" apply plugin: "groovy" + apply plugin: "net.ltgt.errorprone" tasks.withType(JavaCompile).configureEach { options.compilerArgs << "-Xlint:unchecked" options.compilerArgs << "-Xlint:deprecation" + options.errorprone.disableAllWarnings = true + options.errorprone.disableWarningsInGeneratedCode = true + options.errorprone.error("StringCaseLocaleUsage") // TODO Disabled until the code is only Java 11/17, see #76 // options.release = 17 @@ -76,6 +81,7 @@ subprojects { testImplementation(libs.assertj.core) testImplementation(libs.mockito.core) + errorprone("com.google.errorprone:error_prone_core:${libs.errorprone.get().version}") testRuntimeOnly("org.junit.platform:junit-platform-launcher") } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6ede78f51..a501c1f30 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -33,6 +33,7 @@ commons-codec1 = { module = "commons-codec:commons-codec", version = "1.17.0" } commons-lang3 = { module = "org.apache.commons:commons-lang3", version = "3.14.0" } dropwizard-bom = { module = "io.dropwizard:dropwizard-bom", version = "4.0.7" } eclipselink = { module = "org.eclipse.persistence:eclipselink", version = "4.0.3" } +errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.29.2" } google-cloud-storage-bom = { module = "com.google.cloud:google-cloud-storage-bom", version = "2.40.1" } guava = { module = "com.google.guava:guava", version = "33.2.1-jre" } h2 = { module = "com.h2database:h2", version = "2.2.224" } diff --git a/polaris-core/src/main/java/io/polaris/core/storage/cache/StorageCredentialCache.java b/polaris-core/src/main/java/io/polaris/core/storage/cache/StorageCredentialCache.java index de3103b0d..04bc21905 100644 --- a/polaris-core/src/main/java/io/polaris/core/storage/cache/StorageCredentialCache.java +++ b/polaris-core/src/main/java/io/polaris/core/storage/cache/StorageCredentialCache.java @@ -138,8 +138,8 @@ public Map getOrGenerateSubScopeCreds( .addKeyValue("errorMessage", scopedCredentialsResult.getExtraInformation()) .log("Failed to get subscoped credentials"); throw new UnprocessableEntityException( - "Failed to get subscoped credentials: " - + scopedCredentialsResult.getExtraInformation()); + "Failed to get subscoped credentials: %s", + scopedCredentialsResult.getExtraInformation()); }; return cache.get(key, loader).convertToMapOfString(); } diff --git a/polaris-core/src/test/java/io/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/io/polaris/core/storage/cache/StorageCredentialCacheTest.java index c6ed422d2..0e39857ef 100644 --- a/polaris-core/src/test/java/io/polaris/core/storage/cache/StorageCredentialCacheTest.java +++ b/polaris-core/src/test/java/io/polaris/core/storage/cache/StorageCredentialCacheTest.java @@ -15,6 +15,7 @@ */ package io.polaris.core.storage.cache; +import com.google.common.collect.ImmutableMap; import io.polaris.core.PolarisCallContext; import io.polaris.core.PolarisDefaultDiagServiceImpl; import io.polaris.core.PolarisDiagnostics; @@ -382,32 +383,29 @@ private static List getFakeScop : String.valueOf(Long.MAX_VALUE); res.add( new PolarisMetaStoreManager.ScopedCredentialsResult( - new EnumMap<>(PolarisCredentialProperty.class) { - { - put(PolarisCredentialProperty.AWS_KEY_ID, "key_id_" + finalI); - put(PolarisCredentialProperty.AWS_SECRET_KEY, "key_secret_" + finalI); - put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime); - } - })); + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AWS_KEY_ID, "key_id_" + finalI) + .put(PolarisCredentialProperty.AWS_SECRET_KEY, "key_secret_" + finalI) + .put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime) + .buildOrThrow()))); if (res.size() == number) return res; res.add( new PolarisMetaStoreManager.ScopedCredentialsResult( - new EnumMap<>(PolarisCredentialProperty.class) { - { - put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_" + finalI); - put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "account_host"); - put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime); - } - })); + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_" + finalI) + .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "account_host") + .put(PolarisCredentialProperty.EXPIRATION_TIME, expireTime) + .buildOrThrow()))); if (res.size() == number) return res; res.add( new PolarisMetaStoreManager.ScopedCredentialsResult( - new EnumMap<>(PolarisCredentialProperty.class) { - { - put(PolarisCredentialProperty.GCS_ACCESS_TOKEN, "gcs_token_" + finalI); - put(PolarisCredentialProperty.GCS_ACCESS_TOKEN_EXPIRES_AT, expireTime); - } - })); + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.GCS_ACCESS_TOKEN, "gcs_token_" + finalI) + .put(PolarisCredentialProperty.GCS_ACCESS_TOKEN_EXPIRES_AT, expireTime) + .buildOrThrow()))); } return res; } diff --git a/polaris-service/src/main/java/io/polaris/service/admin/PolarisAdminService.java b/polaris-service/src/main/java/io/polaris/service/admin/PolarisAdminService.java index 609e86eea..08c23d713 100644 --- a/polaris-service/src/main/java/io/polaris/service/admin/PolarisAdminService.java +++ b/polaris-service/src/main/java/io/polaris/service/admin/PolarisAdminService.java @@ -584,13 +584,12 @@ public void deleteCatalog(String name) { if (!dropEntityResult.isSuccess()) { if (dropEntityResult.failedBecauseNotEmpty()) { throw new BadRequestException( - String.format("Catalog '%s' cannot be dropped, it is not empty", entity.getName())); + "Catalog '%s' cannot be dropped, it is not empty", entity.getName()); } else { throw new BadRequestException( - String.format( - "Catalog '%s' cannot be dropped, concurrent modification detected. Please try " - + "again", - entity.getName())); + "Catalog '%s' cannot be dropped, concurrent modification detected. Please try " + + "again", + entity.getName()); } } } @@ -701,7 +700,7 @@ private void validateUpdateCatalogDiffOrThrow( .orElseThrow( () -> new CommitFailedException( - "Concurrent modification on Catalog '%s'; retry later")); + "Concurrent modification on Catalog '%s'; retry later", name)); return returnedEntity; } @@ -826,7 +825,7 @@ public void deletePrincipal(String name) { .orElseThrow( () -> new CommitFailedException( - "Concurrent modification on Principal '%s'; retry later")); + "Concurrent modification on Principal '%s'; retry later", name)); return returnedEntity; } @@ -1004,7 +1003,7 @@ public void deletePrincipalRole(String name) { .orElseThrow( () -> new CommitFailedException( - "Concurrent modification on PrincipalRole '%s'; retry later")); + "Concurrent modification on PrincipalRole '%s'; retry later", name)); return returnedEntity; } @@ -1142,7 +1141,7 @@ public void deleteCatalogRole(String catalogName, String name) { .orElseThrow( () -> new CommitFailedException( - "Concurrent modification on CatalogRole '%s'; retry later")); + "Concurrent modification on CatalogRole '%s'; retry later", name)); return returnedEntity; } diff --git a/polaris-service/src/main/java/io/polaris/service/catalog/BasePolarisCatalog.java b/polaris-service/src/main/java/io/polaris/service/catalog/BasePolarisCatalog.java index ff3a4ac17..2f4d2423b 100644 --- a/polaris-service/src/main/java/io/polaris/service/catalog/BasePolarisCatalog.java +++ b/polaris-service/src/main/java/io/polaris/service/catalog/BasePolarisCatalog.java @@ -1570,7 +1570,7 @@ private void renameTableLike( if (existingEntitySubType == null) { // this code path is unexpected throw new AlreadyExistsException( - "Cannot rename %s to %s. Object %s already exists", from, to); + "Cannot rename %s to %s. Object already exists", from, to); } else if (existingEntitySubType == PolarisEntitySubType.TABLE) { throw new AlreadyExistsException( "Cannot rename %s to %s. Table already exists", from, to); diff --git a/polaris-service/src/main/java/io/polaris/service/catalog/IcebergCatalogAdapter.java b/polaris-service/src/main/java/io/polaris/service/catalog/IcebergCatalogAdapter.java index f2a352897..a693350eb 100644 --- a/polaris-service/src/main/java/io/polaris/service/catalog/IcebergCatalogAdapter.java +++ b/polaris-service/src/main/java/io/polaris/service/catalog/IcebergCatalogAdapter.java @@ -459,7 +459,7 @@ public Response getConfig(String warehouse, SecurityContext securityContext) { CallContext.getCurrentContext(), authenticatedPrincipal, warehouse); ResolverStatus resolverStatus = resolver.resolveAll(); if (!resolverStatus.getStatus().equals(ResolverStatus.StatusEnum.SUCCESS)) { - throw new NotFoundException("Unable to find warehouse " + warehouse); + throw new NotFoundException("Unable to find warehouse %s", warehouse); } EntityCacheEntry resolvedReferenceCatalog = resolver.getResolvedReferenceCatalog(); Map properties = diff --git a/polaris-service/src/main/java/io/polaris/service/types/NotificationType.java b/polaris-service/src/main/java/io/polaris/service/types/NotificationType.java index 245e67529..16def7278 100644 --- a/polaris-service/src/main/java/io/polaris/service/types/NotificationType.java +++ b/polaris-service/src/main/java/io/polaris/service/types/NotificationType.java @@ -16,6 +16,7 @@ package io.polaris.service.types; import java.util.Arrays; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; @@ -80,7 +81,7 @@ public static Optional lookupByName(String name) { } for (NotificationType NotificationType : NotificationType.values()) { - if (name.toUpperCase().equals(NotificationType.name())) { + if (name.toUpperCase(Locale.ROOT).equals(NotificationType.name())) { return Optional.of(NotificationType); } }