From fea135d9f7b0d2e1dcb532f4872703fed4a96213 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:20:07 -0700 Subject: [PATCH 1/3] improve null check --- .../polaris/core/config/PolarisConfigurationStore.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 acb171ce30..662a88d72d 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 @@ -118,9 +118,14 @@ public interface PolarisConfigurationStore { PolarisConfiguration config) { if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) { Map 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( From 99e2a3768eb815a10cc4d3a9081d5c27f7e349ed Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:31:02 -0700 Subject: [PATCH 2/3] add a test --- .../config/DefaultConfigurationStoreTest.java | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index e533d52142..cd72a53a24 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/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,51 @@ public void testInjectedFeaturesConfiguration() { assertThat(featuresConfiguration.realmOverrides().get(realmOne).overrides()) .containsKey(falseByDefaultKey); } + + @Test + public void testRegisterAndUseFeatureConfigurations() { + String prefix = "testRegisterAndUseFeatureConfigurations"; + + FeatureConfiguration safeConfig = FeatureConfiguration.builder() + .key(String.format("%s_safe", prefix)) + .catalogConfig(String.format("polaris.config.%s.safe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration unsafeConfig =FeatureConfiguration.builder() + .key(String.format("%s_unsafe", prefix)) + .catalogConfigUnsafe(String.format("%s.unsafe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration bothConfig = FeatureConfiguration.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(); + } } From 29082076cd38a988584b6195d1547b99f7f807ed Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 13 May 2025 19:31:05 -0700 Subject: [PATCH 3/3] autolint --- .../config/DefaultConfigurationStoreTest.java | 71 +++++++++---------- 1 file changed, 33 insertions(+), 38 deletions(-) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java index cd72a53a24..ddd1026d97 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/config/DefaultConfigurationStoreTest.java @@ -238,46 +238,41 @@ public void testInjectedFeaturesConfiguration() { public void testRegisterAndUseFeatureConfigurations() { String prefix = "testRegisterAndUseFeatureConfigurations"; - FeatureConfiguration safeConfig = FeatureConfiguration.builder() - .key(String.format("%s_safe", prefix)) - .catalogConfig(String.format("polaris.config.%s.safe", prefix)) - .defaultValue(true) - .description(prefix) - .buildFeatureConfiguration(); - - FeatureConfiguration unsafeConfig =FeatureConfiguration.builder() - .key(String.format("%s_unsafe", prefix)) - .catalogConfigUnsafe(String.format("%s.unsafe", prefix)) - .defaultValue(true) - .description(prefix) - .buildFeatureConfiguration(); - - FeatureConfiguration bothConfig = FeatureConfiguration.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(); + FeatureConfiguration safeConfig = + FeatureConfiguration.builder() + .key(String.format("%s_safe", prefix)) + .catalogConfig(String.format("polaris.config.%s.safe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration unsafeConfig = + FeatureConfiguration.builder() + .key(String.format("%s_unsafe", prefix)) + .catalogConfigUnsafe(String.format("%s.unsafe", prefix)) + .defaultValue(true) + .description(prefix) + .buildFeatureConfiguration(); + + FeatureConfiguration bothConfig = + FeatureConfiguration.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(); + Assertions.assertThat(configurationStore.getConfiguration(polarisContext, catalog, safeConfig)) + .isTrue(); + + Assertions.assertThat( + configurationStore.getConfiguration(polarisContext, catalog, unsafeConfig)) + .isTrue(); + + Assertions.assertThat(configurationStore.getConfiguration(polarisContext, catalog, bothConfig)) + .isTrue(); } }