Make PolarisConfiguration member variables private

polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java

@@ -55,7 +55,7 @@ public static void enforceFeatureEnabledOrThrow(
             .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 static void enforceFeatureEnabledOrThrow(
           .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();
 

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 @@ protected PolarisConfiguration(
     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 @@ T cast(Object value) {
     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;

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);
   }
 

polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java

@@ -91,8 +91,8 @@ private long maxCacheDurationMs() {
       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;
     }

polaris-core/src/test/java/org/apache/polaris/service/storage/PolarisConfigurationStoreTest.java

@@ -56,8 +56,8 @@ public void testConfigsCanBeCastedFromString() {
           @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 void testConfigsCanBeCastedFromString() {
     // 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));
     }
   }
 

runtime/service/src/main/java/org/apache/polaris/service/quarkus/config/ProductionReadinessChecks.java

@@ -217,26 +217,26 @@ public ProductionReadinessCheck checkInsecureStorageSettings(
     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 ProductionReadinessCheck checkInsecureStorageSettings(
     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 ProductionReadinessCheck checkInsecureStorageSettings(
                         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 ProductionReadinessCheck checkInsecureStorageSettings(
                                   t),
                               format(
                                   "polaris.features.realm-overrides.\"%s\".overrides.\"%s\"",
-                                  realmId, storageTypes.key))));
+                                  realmId, storageTypes.key()))));
         });
     return errors.isEmpty()
         ? ProductionReadinessCheck.OK

runtime/service/src/test/java/org/apache/polaris/service/quarkus/admin/PolarisOverlappingTableTest.java

@@ -340,7 +340,7 @@ void testInvalidSetupsForObjectStorageLocation(
             "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 void testHashedTableLocations(@TempDir Path tempDir) {
             "true",
             "SUPPORTED_CATALOG_STORAGE_TYPES",
             List.of("FILE", "S3"),
-            OPTIMIZED_SIBLING_CHECK.key,
+            OPTIMIZED_SIBLING_CHECK.key(),
             "true");
     Map<String, String> hashedAndOverlapButNoOptimizedCatalog =
         Map.of(

runtime/service/src/test/java/org/apache/polaris/service/quarkus/catalog/IcebergCatalogTest.java

@@ -1956,7 +1956,7 @@ public void testDropTableWithPurgeDisabled() {
     // 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) {

service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

@@ -949,8 +949,8 @@ private String applyDefaultLocationObjectStoragePrefix(
       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 @@ private String applyDefaultLocationObjectStoragePrefix(
                     + " 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 @@ private void updateTableLike(TableIdentifier identifier, PolarisEntity entity) {
                 "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()));
       }
     }

service/common/src/main/java/org/apache/polaris/service/catalog/io/FileIOUtil.java

@@ -88,8 +88,8 @@ public static AccessConfig refreshAccessConfig(
     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()