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 7b44e9396a..4923d97eea 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 @@ -24,7 +24,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.context.RealmContext; import org.apache.polaris.core.entity.CatalogEntity; import org.slf4j.Logger; @@ -37,48 +36,6 @@ public interface PolarisConfigurationStore { Logger LOGGER = LoggerFactory.getLogger(PolarisConfigurationStore.class); - /** - * Retrieve the current value for a configuration key. May be null if not set. - * - *

This function will be deprecated, it can not be called outside of active request scope, such - * as background tasks (TaskExecutor). Please use the function getConfiguration(RealmContext - * realmContext, String configName) to get the configuration value in a more robust way. TODO: - * Remove this function and replace the usage with the function takes realm. Github issue - * https://github.com/apache/polaris/issues/1775 - * - * @param ctx the current call context - * @param configName the name of the configuration key to check - * @return the current value set for the configuration key or null if not set - * @param the type of the configuration value - */ - @Deprecated - default @Nullable T getConfiguration(PolarisCallContext ctx, String configName) { - return null; - } - - /** - * Retrieve the current value for a configuration key. If not set, return the non-null default - * value. - * - *

This function will be deprecated, it can not be called outside of active request scope, such - * as background tasks (TaskExecutor). Please use the function getConfiguration(RealmContext - * realmContext, String configName, T defaultValue) to get the configuration value in a more - * robust way. TODO: Remove this function and replace the usage with the function takes realm. - * Github issue https://github.com/apache/polaris/issues/1775 - * - * @param ctx the current call context - * @param configName the name of the configuration key to check - * @param defaultValue the default value if the configuration key has no value - * @return the current value or the supplied default value - */ - @Deprecated - default @Nonnull T getConfiguration( - PolarisCallContext ctx, String configName, @Nonnull T defaultValue) { - Preconditions.checkNotNull(defaultValue, "Cannot pass null as a default value"); - T configValue = getConfiguration(ctx, configName); - return configValue != null ? configValue : defaultValue; - } - /** * Retrieve the current value for a configuration key for a given realm. May be null if not set. * @@ -102,7 +59,7 @@ public interface PolarisConfigurationStore { * @param the type of the configuration value */ default @Nonnull T getConfiguration( - RealmContext realmContext, String configName, @Nonnull T defaultValue) { + @Nonnull RealmContext realmContext, String configName, @Nonnull T defaultValue) { Preconditions.checkNotNull(defaultValue, "Cannot pass null as a default value"); T configValue = getConfiguration(realmContext, configName); return configValue != null ? configValue : defaultValue; @@ -141,7 +98,7 @@ public interface PolarisConfigurationStore { * @param the type of the configuration value */ default @Nonnull T getConfiguration( - RealmContext realmContext, PolarisConfiguration config) { + @Nonnull RealmContext realmContext, PolarisConfiguration config) { T result = getConfiguration(realmContext, config.key, config.defaultValue); return tryCast(config, result); } @@ -157,7 +114,7 @@ public interface PolarisConfigurationStore { * @param the type of the configuration value */ default @Nonnull T getConfiguration( - RealmContext realmContext, + @Nonnull RealmContext realmContext, @Nonnull CatalogEntity catalogEntity, PolarisConfiguration config) { if (config.hasCatalogConfig() || config.hasCatalogConfigUnsafe()) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 92ccc6eed5..060a908e15 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -1517,7 +1517,7 @@ private void revokeGrantRecord( callCtx .getConfigurationStore() .getConfiguration( - callCtx, + callCtx.getRealmContext(), PolarisTaskConstants.TASK_TIMEOUT_MILLIS_CONFIG, PolarisTaskConstants.TASK_TIMEOUT_MILLIS); return taskState == null diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 0224dec493..0d6111d54b 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -1956,7 +1956,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant( callCtx .getConfigurationStore() .getConfiguration( - callCtx, + callCtx.getRealmContext(), PolarisTaskConstants.TASK_TIMEOUT_MILLIS_CONFIG, PolarisTaskConstants.TASK_TIMEOUT_MILLIS); return taskState == null diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java index cd23177ad4..a4e58860de 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java @@ -102,7 +102,8 @@ public void testValidateAccessToLocationsWithWildcard() { new PolarisConfigurationStore() { @SuppressWarnings("unchecked") @Override - public @Nullable T getConfiguration(RealmContext ctx, String configName) { + public @Nullable T getConfiguration( + @Nonnull RealmContext ctx, String configName) { return (T) config.get(configName); } }, 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 f95b13f9a8..4b190d3d15 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 @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.storage; +import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import java.util.List; import java.util.Map; @@ -53,7 +54,7 @@ public void testConfigsCanBeCastedFromString() { */ @SuppressWarnings("unchecked") @Override - public @Nullable T getConfiguration(RealmContext ctx, String configName) { + public @Nullable T getConfiguration(@Nonnull RealmContext ctx, String configName) { for (PolarisConfiguration c : configs) { if (c.key.equals(configName)) { return (T) String.valueOf(c.defaultValue); @@ -84,7 +85,7 @@ public void testInvalidCastThrowsException() { new PolarisConfigurationStore() { @SuppressWarnings("unchecked") @Override - public T getConfiguration(RealmContext ctx, String configName) { + public T getConfiguration(@Nonnull RealmContext ctx, String configName) { return (T) "abc123"; } }; @@ -163,7 +164,8 @@ public void testEntityOverrides() { PolarisConfigurationStore store = new PolarisConfigurationStore() { @Override - public @Nullable T getConfiguration(RealmContext realmContext, String configName) { + public @Nullable T getConfiguration( + @Nonnull RealmContext realmContext, String configName) { //noinspection unchecked return (T) Map.of("key2", "config-value2").get(configName); } 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 5ee7f65f9c..3da4bd2950 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 @@ -20,20 +20,15 @@ import static org.assertj.core.api.Assertions.assertThat; -import io.quarkus.test.junit.QuarkusMock; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.QuarkusTestProfile; import io.quarkus.test.junit.TestProfile; import jakarta.inject.Inject; -import java.time.Clock; import java.util.Map; -import org.apache.polaris.core.PolarisCallContext; -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.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.assertj.core.api.Assertions; @@ -73,12 +68,9 @@ public Map getConfigOverrides() { } } - private PolarisCallContext polarisContext; private RealmContext realmContext; - @Inject MetaStoreManagerFactory managerFactory; @Inject PolarisConfigurationStore configurationStore; - @Inject PolarisDiagnostics diagServices; @Inject FeaturesConfiguration featuresConfiguration; @BeforeEach @@ -89,60 +81,42 @@ public void before(TestInfo testInfo) { testInfo.getTestMethod().map(java.lang.reflect.Method::getName).orElse("test"), System.nanoTime()); realmContext = () -> realmName; - polarisContext = - new PolarisCallContext( - realmContext, - managerFactory.getOrCreateSessionSupplier(realmContext).get(), - diagServices, - configurationStore, - Clock.systemDefaultZone()); - } - - @Test - public void testGetConfigurationWithNoRealmContext() { - Assertions.assertThatThrownBy( - () -> configurationStore.getConfiguration(polarisContext, "missingKeyWithoutDefault")) - .isInstanceOf(IllegalStateException.class); } @Test public void testGetConfiguration() { - QuarkusMock.installMockForType(realmContext, RealmContext.class); - Object value = configurationStore.getConfiguration(polarisContext, "missingKeyWithoutDefault"); + Object value = configurationStore.getConfiguration(realmContext, "missingKeyWithoutDefault"); assertThat(value).isNull(); Object defaultValue = - configurationStore.getConfiguration( - polarisContext, "missingKeyWithDefault", "defaultValue"); + configurationStore.getConfiguration(realmContext, "missingKeyWithDefault", "defaultValue"); assertThat(defaultValue).isEqualTo("defaultValue"); // the falseByDefaultKey is set to false for all realms in Profile.getConfigOverrides - assertThat((Boolean) configurationStore.getConfiguration(polarisContext, falseByDefaultKey)) + assertThat((Boolean) configurationStore.getConfiguration(realmContext, falseByDefaultKey)) .isFalse(); // the trueByDefaultKey is set to true for all realms in Profile.getConfigOverrides - assertThat((Boolean) configurationStore.getConfiguration(polarisContext, trueByDefaultKey)) + assertThat((Boolean) configurationStore.getConfiguration(realmContext, trueByDefaultKey)) .isTrue(); } @Test public void testGetRealmConfiguration() { // check the realmOne configuration - QuarkusMock.installMockForType(realmOneContext, RealmContext.class); // the falseByDefaultKey is set to `false` for all realms, but overwrite with value `true` for // realmOne. - assertThat((Boolean) configurationStore.getConfiguration(polarisContext, falseByDefaultKey)) + assertThat((Boolean) configurationStore.getConfiguration(realmOneContext, falseByDefaultKey)) .isTrue(); // the trueByDefaultKey is set to `false` for all realms, no overwrite for realmOne - assertThat((Boolean) configurationStore.getConfiguration(polarisContext, trueByDefaultKey)) + assertThat((Boolean) configurationStore.getConfiguration(realmOneContext, trueByDefaultKey)) .isTrue(); // check the realmTwo configuration - QuarkusMock.installMockForType(realmTwoContext, RealmContext.class); // the falseByDefaultKey is set to `false` for all realms, no overwrite for realmTwo - assertThat((Boolean) configurationStore.getConfiguration(polarisContext, falseByDefaultKey)) + assertThat((Boolean) configurationStore.getConfiguration(realmTwoContext, falseByDefaultKey)) .isFalse(); // the trueByDefaultKey is set to `false` for all realms, and overwrite with value `false` for // realmTwo - assertThat((Boolean) configurationStore.getConfiguration(polarisContext, trueByDefaultKey)) + assertThat((Boolean) configurationStore.getConfiguration(realmTwoContext, trueByDefaultKey)) .isFalse(); } @@ -167,20 +141,17 @@ void testGetConfigurationWithRealm() { @Test public void testInjectedConfigurationStore() { - QuarkusMock.installMockForType(realmContext, RealmContext.class); // the default value for trueByDefaultKey is `true` - boolean featureDefaultValue = - configurationStore.getConfiguration(polarisContext, trueByDefaultKey); + Boolean featureDefaultValue = + configurationStore.getConfiguration(realmContext, trueByDefaultKey); assertThat(featureDefaultValue).isTrue(); - QuarkusMock.installMockForType(realmTwoContext, RealmContext.class); // the value for falseByDefaultKey is `false`, and no realm override for realmTwo - boolean realmTwoValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + Boolean realmTwoValue = configurationStore.getConfiguration(realmTwoContext, falseByDefaultKey); assertThat(realmTwoValue).isFalse(); // Now, realmOne override falseByDefaultKey to `True` - QuarkusMock.installMockForType(realmOneContext, RealmContext.class); - boolean realmOneValue = configurationStore.getConfiguration(polarisContext, falseByDefaultKey); + Boolean realmOneValue = configurationStore.getConfiguration(realmOneContext, falseByDefaultKey); assertThat(realmOneValue).isTrue(); assertThat(configurationStore).isInstanceOf(DefaultConfigurationStore.class); @@ -188,7 +159,6 @@ public void testInjectedConfigurationStore() { @Test public void testInjectedFeaturesConfiguration() { - QuarkusMock.installMockForType(realmContext, RealmContext.class); assertThat(featuresConfiguration).isInstanceOf(QuarkusResolvedFeaturesConfiguration.class); assertThat(featuresConfiguration.defaults()) @@ -209,7 +179,6 @@ public void testInjectedFeaturesConfiguration() { @Test public void testRegisterAndUseFeatureConfigurations() { - QuarkusMock.installMockForType(realmContext, RealmContext.class); String prefix = "testRegisterAndUseFeatureConfigurations"; FeatureConfiguration safeConfig = diff --git a/service/common/src/main/java/org/apache/polaris/service/config/DefaultConfigurationStore.java b/service/common/src/main/java/org/apache/polaris/service/config/DefaultConfigurationStore.java index 8928291e82..02d7a2f223 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/DefaultConfigurationStore.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/DefaultConfigurationStore.java @@ -22,11 +22,9 @@ import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import jakarta.enterprise.context.ApplicationScoped; -import jakarta.enterprise.inject.Instance; import jakarta.inject.Inject; import java.util.Map; import java.util.Optional; -import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.config.PolarisConfigurationStore; import org.apache.polaris.core.context.RealmContext; import org.slf4j.Logger; @@ -38,7 +36,6 @@ public class DefaultConfigurationStore implements PolarisConfigurationStore { private final Map defaults; private final Map> realmOverrides; - @Inject private Instance realmContextInstance; // FIXME the whole PolarisConfigurationStore + PolarisConfiguration needs to be refactored // to become a proper Quarkus configuration object @@ -61,18 +58,6 @@ public DefaultConfigurationStore( return confgValue; } - @Override - public @Nullable T getConfiguration(@Nonnull PolarisCallContext ctx, String configName) { - if (realmContextInstance.isResolvable()) { - RealmContext realmContext = realmContextInstance.get(); - return getConfiguration(realmContext, configName); - } else { - LOGGER.debug( - "No RealmContext is injected when lookup value for configuration {} ", configName); - return getDefaultConfiguration(configName); - } - } - private @Nullable T getDefaultConfiguration(String configName) { @SuppressWarnings("unchecked") T confgValue = (T) defaults.get(configName);