This is an automated email from the ASF dual-hosted git repository.
dimas pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 15f23cae8 Make PolarisConfiguration member variables private (#2007)
15f23cae8 is described below
commit 15f23cae8e678f8a13c20c5012919565288525a0
Author: Pooja Nilangekar <poo...@umd.edu>
AuthorDate: Fri Jul 11 09:40:14 2025 -0400
Make PolarisConfiguration member variables private (#2007)
* Make PolarisConfiguration members private
* Make methods final
---
.../polaris/core/config/FeatureConfiguration.java | 4 ++--
.../polaris/core/config/PolarisConfiguration.java | 26 ++++++++++++++++------
.../core/config/PolarisConfigurationStore.java | 14 ++++++------
.../core/storage/cache/StorageCredentialCache.java | 4 ++--
.../storage/PolarisConfigurationStoreTest.java | 6 ++---
.../quarkus/config/ProductionReadinessChecks.java | 20 ++++++++---------
.../quarkus/admin/PolarisOverlappingTableTest.java | 4 ++--
.../quarkus/catalog/IcebergCatalogTest.java | 2 +-
.../service/catalog/iceberg/IcebergCatalog.java | 18 +++++++--------
.../polaris/service/catalog/io/FileIOUtil.java | 4 ++--
10 files changed, 57 insertions(+), 45 deletions(-)
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java
b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java
index 59de973be..d8ec61320 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java
@@ -55,7 +55,7 @@ public class FeatureConfiguration<T> extends
PolarisConfiguration<T> {
.getConfigurationStore()
.getConfiguration(callContext.getRealmContext(), featureConfig);
if (!enabled) {
- throw new UnsupportedOperationException("Feature not enabled: " +
featureConfig.key);
+ throw new UnsupportedOperationException("Feature not enabled: " +
featureConfig.key());
}
}
@@ -211,7 +211,7 @@ public class FeatureConfiguration<T> extends
PolarisConfiguration<T> {
.key("STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS")
.description(
"How long to store storage credentials in the local cache. This
should be less than "
- + STORAGE_CREDENTIAL_DURATION_SECONDS.key)
+ + STORAGE_CREDENTIAL_DURATION_SECONDS.key())
.defaultValue(30 * 60) // 30 minutes
.buildFeatureConfiguration();
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java
b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java
index 31ae18795..f9cf8192f 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfiguration.java
@@ -39,9 +39,9 @@ public abstract class PolarisConfiguration<T> {
private static final List<PolarisConfiguration<?>> allConfigurations = new
ArrayList<>();
- public final String key;
- public final String description;
- public final T defaultValue;
+ private final String key;
+ private final String description;
+ private final T defaultValue;
private final Optional<String> catalogConfigImpl;
private final Optional<String> catalogConfigUnsafeImpl;
private final Class<T> typ;
@@ -98,22 +98,22 @@ public abstract class PolarisConfiguration<T> {
this.typ = (Class<T>) defaultValue.getClass();
}
- public boolean hasCatalogConfig() {
+ public final boolean hasCatalogConfig() {
return catalogConfigImpl.isPresent();
}
- public String catalogConfig() {
+ public final String catalogConfig() {
return catalogConfigImpl.orElseThrow(
() ->
new IllegalStateException(
"Attempted to read a catalog config key from a configuration
that doesn't have one."));
}
- public boolean hasCatalogConfigUnsafe() {
+ public final boolean hasCatalogConfigUnsafe() {
return catalogConfigUnsafeImpl.isPresent();
}
- public String catalogConfigUnsafe() {
+ public final String catalogConfigUnsafe() {
return catalogConfigUnsafeImpl.orElseThrow(
() ->
new IllegalStateException(
@@ -124,6 +124,18 @@ public abstract class PolarisConfiguration<T> {
return this.typ.cast(value);
}
+ public final String key() {
+ return key;
+ }
+
+ public final String description() {
+ return description;
+ }
+
+ public final T defaultValue() {
+ return defaultValue;
+ }
+
public static class Builder<T> {
private String key;
private String description;
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java
b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java
index 4923d97ee..1e2a1928d 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/config/PolarisConfigurationStore.java
@@ -71,18 +71,18 @@ public interface PolarisConfigurationStore {
*/
private <T> @Nonnull T tryCast(PolarisConfiguration<T> config, Object value)
{
if (value == null) {
- return config.defaultValue;
+ return config.defaultValue();
}
- if (config.defaultValue instanceof Boolean) {
+ if (config.defaultValue() instanceof Boolean) {
return config.cast(Boolean.valueOf(String.valueOf(value)));
- } else if (config.defaultValue instanceof Integer) {
+ } else if (config.defaultValue() instanceof Integer) {
return config.cast(Integer.valueOf(String.valueOf(value)));
- } else if (config.defaultValue instanceof Long) {
+ } else if (config.defaultValue() instanceof Long) {
return config.cast(Long.valueOf(String.valueOf(value)));
- } else if (config.defaultValue instanceof Double) {
+ } else if (config.defaultValue() instanceof Double) {
return config.cast(Double.valueOf(String.valueOf(value)));
- } else if (config.defaultValue instanceof List<?>) {
+ } else if (config.defaultValue() instanceof List<?>) {
return config.cast(new ArrayList<>((List<?>) value));
} else {
return config.cast(value);
@@ -99,7 +99,7 @@ public interface PolarisConfigurationStore {
*/
default <T> @Nonnull T getConfiguration(
@Nonnull RealmContext realmContext, PolarisConfiguration<T> config) {
- T result = getConfiguration(realmContext, config.key, config.defaultValue);
+ T result = getConfiguration(realmContext, config.key(),
config.defaultValue());
return tryCast(config, result);
}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java
index 36f666db0..e3a7a4f13 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java
@@ -91,8 +91,8 @@ public class StorageCredentialCache {
throw new IllegalArgumentException(
String.format(
"%s should be less than %s",
-
FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS.key,
- FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS.key));
+
FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS.key(),
+ FeatureConfiguration.STORAGE_CREDENTIAL_DURATION_SECONDS.key()));
} else {
return cacheDurationSeconds * 1000L;
}
diff --git
a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
index 4b190d3d1..612b8716b 100644
---
a/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java
@@ -56,8 +56,8 @@ public class PolarisConfigurationStoreTest {
@Override
public <T> @Nullable T getConfiguration(@Nonnull RealmContext ctx,
String configName) {
for (PolarisConfiguration<?> c : configs) {
- if (c.key.equals(configName)) {
- return (T) String.valueOf(c.defaultValue);
+ if (c.key().equals(configName)) {
+ return (T) String.valueOf(c.defaultValue());
}
}
@@ -71,7 +71,7 @@ public class PolarisConfigurationStoreTest {
// Ensure that we can fetch all the configs and that the value is what we
expect, which
// is the config's default value based on how we've implemented
PolarisConfigurationStore above.
for (PolarisConfiguration<?> c : configs) {
- Assertions.assertEquals(c.defaultValue,
store.getConfiguration(testRealmContext, c));
+ Assertions.assertEquals(c.defaultValue(),
store.getConfiguration(testRealmContext, c));
}
}
diff --git
a/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java
b/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java
index ef2c1813e..4205ef4cf 100644
---
a/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java
+++
b/runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java
@@ -217,26 +217,26 @@ public class ProductionReadinessChecks {
var insecure = FeatureConfiguration.ALLOW_INSECURE_STORAGE_TYPES;
var errors = new ArrayList<Error>();
- if
(Boolean.parseBoolean(featureConfiguration.defaults().get(insecure.key))) {
+ if
(Boolean.parseBoolean(featureConfiguration.defaults().get(insecure.key()))) {
errors.add(
Error.ofSevere(
"Must not enable a configuration that exposes known and severe
security risks: "
- + insecure.description,
- format("polaris.features.\"%s\"", insecure.key)));
+ + insecure.description(),
+ format("polaris.features.\"%s\"", insecure.key())));
}
featureConfiguration
.realmOverrides()
.forEach(
(realmId, overrides) -> {
- if
(Boolean.parseBoolean(overrides.overrides().get(insecure.key))) {
+ if
(Boolean.parseBoolean(overrides.overrides().get(insecure.key()))) {
errors.add(
Error.ofSevere(
"Must not enable a configuration that exposes known
and severe security risks: "
- + insecure.description,
+ + insecure.description(),
format(
"polaris.features.realm-overrides.\"%s\".overrides.\"%s\"",
- realmId, insecure.key)));
+ realmId, insecure.key())));
}
});
@@ -245,7 +245,7 @@ public class ProductionReadinessChecks {
var defaults = featureConfiguration.parseDefaults(mapper);
var realmOverrides = featureConfiguration.parseRealmOverrides(mapper);
@SuppressWarnings("unchecked")
- var supported = (List<String>) defaults.getOrDefault(storageTypes.key,
List.of());
+ var supported = (List<String>) defaults.getOrDefault(storageTypes.key(),
List.of());
supported.stream()
.filter(n -> !IcebergPropertiesValidation.safeStorageType(n))
.forEach(
@@ -255,11 +255,11 @@ public class ProductionReadinessChecks {
format(
"The storage type '%s' is considered insecure and
exposes the service to severe security risks!",
t),
- format("polaris.features.\"%s\"", storageTypes.key))));
+ format("polaris.features.\"%s\"",
storageTypes.key()))));
realmOverrides.forEach(
(realmId, overrides) -> {
@SuppressWarnings("unchecked")
- var s = (List<String>) overrides.getOrDefault(storageTypes.key,
List.of());
+ var s = (List<String>) overrides.getOrDefault(storageTypes.key(),
List.of());
s.stream()
.filter(n -> !IcebergPropertiesValidation.safeStorageType(n))
.forEach(
@@ -271,7 +271,7 @@ public class ProductionReadinessChecks {
t),
format(
"polaris.features.realm-overrides.\"%s\".overrides.\"%s\"",
- realmId, storageTypes.key))));
+ realmId, storageTypes.key()))));
});
return errors.isEmpty()
? ProductionReadinessCheck.OK
diff --git
a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java
b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java
index a658b8e88..2245bb9e9 100644
---
a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java
+++
b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java
@@ -340,7 +340,7 @@ public class PolarisOverlappingTableTest {
"true",
"SUPPORTED_CATALOG_STORAGE_TYPES",
List.of("FILE", "S3"),
- OPTIMIZED_SIBLING_CHECK.key,
+ OPTIMIZED_SIBLING_CHECK.key(),
"false");
TestServices services =
@@ -369,7 +369,7 @@ public class PolarisOverlappingTableTest {
"true",
"SUPPORTED_CATALOG_STORAGE_TYPES",
List.of("FILE", "S3"),
- OPTIMIZED_SIBLING_CHECK.key,
+ OPTIMIZED_SIBLING_CHECK.key(),
"true");
Map<String, String> hashedAndOverlapButNoOptimizedCatalog =
Map.of(
diff --git
a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java
b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java
index a5f4f9080..8534e18ea 100644
---
a/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java
+++
b/runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java
@@ -1956,7 +1956,7 @@ public abstract class IcebergCatalogTest extends
CatalogTests<IcebergCatalog> {
// Attempt to drop the table:
Assertions.assertThatThrownBy(() -> noPurgeCatalog.dropTable(TABLE, true))
.isInstanceOf(ForbiddenException.class)
-
.hasMessageContaining(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key);
+
.hasMessageContaining(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key());
}
private TableMetadata createSampleTableMetadata(String tableLocation) {
diff --git
a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
index e1b317a91..5c0e6b18d 100644
---
a/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
+++
b/service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
@@ -949,8 +949,8 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
throw new IllegalStateException(
String.format(
"The configuration %s is enabled, but %s is not enabled",
-
FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key,
- FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key));
+
FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key(),
+ FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key()));
} else if (!allowTableLocationOverlap) {
// TODO consider doing this check any time ALLOW_EXTERNAL_TABLE_LOCATION
is enabled, not just
// here
@@ -961,17 +961,17 @@ public class IcebergCatalog extends
BaseMetastoreViewCatalog
+ " performed, but only within each namespace. However, %s
is enabled, which indicates"
+ " that tables may be created outside of their parent
namespace. This is not a safe"
+ " combination of configurations.",
- FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key,
- FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key,
- FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key));
+ FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key(),
+ FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key(),
+ FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key()));
} else if (!loggedPrefixOverlapWarning.getAndSet(true)) {
LOGGER.warn(
"A table is being created with {} and {} enabled, but with {}
disabled. "
+ "This is a safe combination of configurations which may
prevent table overlap, but only if the "
+ "underlying persistence actually implements %s. Exercise
caution.",
-
FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key,
- FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key,
- FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key);
+
FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key(),
+ FeatureConfiguration.OPTIMIZED_SIBLING_CHECK.key(),
+ FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP.key());
}
return buildPrefixedLocation(tableIdentifier);
} else {
@@ -2460,7 +2460,7 @@ public class IcebergCatalog extends
BaseMetastoreViewCatalog
"Unable to purge entity: %s. To enable this feature, set the
Polaris configuration %s "
+ "or the catalog configuration %s",
identifier.name(),
- FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key,
+ FeatureConfiguration.DROP_WITH_PURGE_ENABLED.key(),
FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig()));
}
}
diff --git
a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java
b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java
index 0e6211076..db3771b96 100644
---
a/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java
+++
b/service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java
@@ -88,8 +88,8 @@ public class FileIOUtil {
boolean skipCredentialSubscopingIndirection =
configurationStore.getConfiguration(
callContext.getRealmContext(),
- FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key,
-
FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue);
+ FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.key(),
+
FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION.defaultValue());
if (skipCredentialSubscopingIndirection) {
LOGGER
.atDebug()
