Better null-safety for check against unsafe catalog configs (#1584)

Better null-safety for check against unsafe catalog configs

Author: eric-maynard

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

@@ -118,9 +118,14 @@ public interface PolarisConfigurationStore {
       PolarisConfiguration<T> config) {
     if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) {
       Map<String, String> propertiesMap = catalogEntity.getPropertiesAsMap();
-      String propertyValue = propertiesMap.get(config.catalogConfig());
+      String propertyValue = null;
+      if (config.hasCatalogConfig()) {
+        propertyValue = propertiesMap.get(config.catalogConfig());
+      }
       if (propertyValue == null) {
-        propertyValue = propertiesMap.get(config.catalogConfigUnsafe());
+        if (config.hasCatalogConfigUnsafe()) {
+          propertyValue = propertiesMap.get(config.catalogConfigUnsafe());
+        }
         if (propertyValue != null) {
           LOGGER.warn(
               String.format(

quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java

@@ -30,13 +30,16 @@
 import org.apache.polaris.core.PolarisCallContext;
 import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
 import org.apache.polaris.core.PolarisDiagnostics;
+import org.apache.polaris.core.config.FeatureConfiguration;
 import org.apache.polaris.core.config.PolarisConfigurationStore;
 import org.apache.polaris.core.context.CallContext;
 import org.apache.polaris.core.context.RealmContext;
+import org.apache.polaris.core.entity.CatalogEntity;
 import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
 import org.apache.polaris.service.config.DefaultConfigurationStore;
 import org.apache.polaris.service.config.FeaturesConfiguration;
 import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory;
+import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
@@ -230,4 +233,46 @@ public void testInjectedFeaturesConfiguration() {
     assertThat(featuresConfiguration.realmOverrides().get(realmOne).overrides())
         .containsKey(falseByDefaultKey);
   }
+
+  @Test
+  public void testRegisterAndUseFeatureConfigurations() {
+    String prefix = "testRegisterAndUseFeatureConfigurations";
+
+    FeatureConfiguration<Boolean> safeConfig =
+        FeatureConfiguration.<Boolean>builder()
+            .key(String.format("%s_safe", prefix))
+            .catalogConfig(String.format("polaris.config.%s.safe", prefix))
+            .defaultValue(true)
+            .description(prefix)
+            .buildFeatureConfiguration();
+
+    FeatureConfiguration<Boolean> unsafeConfig =
+        FeatureConfiguration.<Boolean>builder()
+            .key(String.format("%s_unsafe", prefix))
+            .catalogConfigUnsafe(String.format("%s.unsafe", prefix))
+            .defaultValue(true)
+            .description(prefix)
+            .buildFeatureConfiguration();
+
+    FeatureConfiguration<Boolean> bothConfig =
+        FeatureConfiguration.<Boolean>builder()
+            .key(String.format("%s_both", prefix))
+            .catalogConfig(String.format("polaris.config.%s.both", prefix))
+            .catalogConfigUnsafe(String.format("%s.both", prefix))
+            .defaultValue(true)
+            .description(prefix)
+            .buildFeatureConfiguration();
+
+    CatalogEntity catalog = new CatalogEntity.Builder().build();
+
+    Assertions.assertThat(configurationStore.getConfiguration(polarisContext, catalog, safeConfig))
+        .isTrue();
+
+    Assertions.assertThat(
+            configurationStore.getConfiguration(polarisContext, catalog, unsafeConfig))
+        .isTrue();
+
+    Assertions.assertThat(configurationStore.getConfiguration(polarisContext, catalog, bothConfig))
+        .isTrue();
+  }
 }