From 3f5bcddb4fecc0f0c5ffe4f5a6013721a61af59f Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Thu, 7 Nov 2024 15:31:21 -0500 Subject: [PATCH 1/4] Remove dropwizard-jackson dep from core This is a simplification / cleanup. The dependency does not appear to be required in `polaris-core` --- polaris-core/build.gradle.kts | 4 ---- .../polaris/core/persistence/MetaStoreManagerFactory.java | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/polaris-core/build.gradle.kts b/polaris-core/build.gradle.kts index ec5b2bd3f8..bf5a60278e 100644 --- a/polaris-core/build.gradle.kts +++ b/polaris-core/build.gradle.kts @@ -33,10 +33,6 @@ dependencies { constraints { implementation("io.airlift:aircompressor:0.27") { because("Vulnerability detected in 0.25") } } - // TODO - this is only here for the Discoverable interface - // We should use a different mechanism to discover the plugin implementations - implementation(platform(libs.dropwizard.bom)) - implementation("io.dropwizard:dropwizard-jackson") implementation(platform(libs.jackson.bom)) implementation("com.fasterxml.jackson.core:jackson-annotations") diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/MetaStoreManagerFactory.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/MetaStoreManagerFactory.java index 643294dc83..e8a4ad1144 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/MetaStoreManagerFactory.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/MetaStoreManagerFactory.java @@ -19,7 +19,6 @@ package org.apache.polaris.core.persistence; import com.fasterxml.jackson.annotation.JsonTypeInfo; -import io.dropwizard.jackson.Discoverable; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -33,7 +32,7 @@ * configuration */ @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type") -public interface MetaStoreManagerFactory extends Discoverable { +public interface MetaStoreManagerFactory { PolarisMetaStoreManager getOrCreateMetaStoreManager(RealmContext realmContext); From c7f73ea3081e4770156bcf4f11578c6939e2ff1c Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Thu, 7 Nov 2024 20:00:30 -0500 Subject: [PATCH 2/4] review: add PolarisApplicationConfigurationTest --- .../PolarisApplicationConfigurationTest.java | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java diff --git a/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java b/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java new file mode 100644 index 0000000000..d0ff17e34e --- /dev/null +++ b/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.dropwizard.testing.ConfigOverride; +import io.dropwizard.testing.ResourceHelpers; +import io.dropwizard.testing.junit5.DropwizardAppExtension; +import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import org.apache.polaris.service.config.PolarisApplicationConfig; +import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(DropwizardExtensionsSupport.class) +public class PolarisApplicationConfigurationTest { + + public static final String CONFIG_PATH = + ResourceHelpers.resourceFilePath("polaris-server-integrationtest.yml"); + // Bind to random ports to support parallelism + public static final ConfigOverride RANDOM_APP_PORT = + ConfigOverride.config("server.applicationConnectors[0].port", "0"); + public static final ConfigOverride RANDOM_ADMIN_PORT = + ConfigOverride.config("server.adminConnectors[0].port", "0"); + + @Nested + class DefaultMetastore { + private final DropwizardAppExtension app = + new DropwizardAppExtension<>( + PolarisApplication.class, CONFIG_PATH, RANDOM_APP_PORT, RANDOM_ADMIN_PORT); + + @Test + void testMetastoreType() { + assertThat(app.getConfiguration().getMetaStoreManagerFactory()) + .isInstanceOf(InMemoryPolarisMetaStoreManagerFactory.class); + } + } + + @Nested + class EclipseLinkMetastore { + private final DropwizardAppExtension app = + new DropwizardAppExtension<>( + PolarisApplication.class, + CONFIG_PATH, + RANDOM_APP_PORT, + RANDOM_ADMIN_PORT, + ConfigOverride.config("metaStoreManager.type", "eclipse-link")); + + @Test + void testMetastoreType() { + assertThat(app.getConfiguration().getMetaStoreManagerFactory()) + .extracting(Object::getClass) + .extracting(Class::getSimpleName) + .isEqualTo("EclipseLinkPolarisMetaStoreManagerFactory"); + } + } +} From a249d99ee81efd60377f6ec9c7c3a4aa83b5ebe8 Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Tue, 12 Nov 2024 19:39:07 -0500 Subject: [PATCH 3/4] Add coherent service descriptors for discoverable metastore classes --- .../DiscoverableMetaStoreManagerFactory.java | 30 +++++++++++++++++++ ...pseLinkPolarisMetaStoreManagerFactory.java | 3 +- .../io.dropwizard.jackson.Discoverable | 20 +++++++++++++ ...selink.DiscoverableMetaStoreManagerFactory | 1 - .../DiscoverableMetaStoreManagerFactory.java | 30 +++++++++++++++++++ ...nMemoryPolarisMetaStoreManagerFactory.java | 3 +- .../io.dropwizard.jackson.Discoverable | 2 +- ...stence.DiscoverableMetaStoreManagerFactory | 20 +++++++++++++ 8 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/DiscoverableMetaStoreManagerFactory.java create mode 100644 extension/persistence/eclipselink/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable rename polaris-service/src/main/resources/META-INF/services/org.apache.polaris.core.persistence.MetaStoreManagerFactory => extension/persistence/eclipselink/src/main/resources/META-INF/services/org.apache.polaris.extension.persistence.impl.eclipselink.DiscoverableMetaStoreManagerFactory (91%) create mode 100644 polaris-service/src/main/java/org/apache/polaris/service/persistence/DiscoverableMetaStoreManagerFactory.java create mode 100644 polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.persistence.DiscoverableMetaStoreManagerFactory diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/DiscoverableMetaStoreManagerFactory.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/DiscoverableMetaStoreManagerFactory.java new file mode 100644 index 0000000000..fc6886ba80 --- /dev/null +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/DiscoverableMetaStoreManagerFactory.java @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.extension.persistence.impl.eclipselink; + +import io.dropwizard.jackson.Discoverable; +import io.dropwizard.jackson.DiscoverableSubtypeResolver; + +/** + * Discoverability aid for Dropwizard. Leaf classes cannot implement {@link Discoverable} directly, + * they have to implement another interface, which implements {@link Discoverable}. + * + * @see DiscoverableSubtypeResolver + */ +public interface DiscoverableMetaStoreManagerFactory extends Discoverable {} diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java index 0415c0d5f9..9b38c895c1 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/EclipseLinkPolarisMetaStoreManagerFactory.java @@ -34,7 +34,8 @@ */ @JsonTypeName("eclipse-link") public class EclipseLinkPolarisMetaStoreManagerFactory - extends LocalPolarisMetaStoreManagerFactory { + extends LocalPolarisMetaStoreManagerFactory + implements DiscoverableMetaStoreManagerFactory { @JsonProperty("conf-file") private String confFile; diff --git a/extension/persistence/eclipselink/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable b/extension/persistence/eclipselink/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable new file mode 100644 index 0000000000..12978f2bde --- /dev/null +++ b/extension/persistence/eclipselink/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable @@ -0,0 +1,20 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +org.apache.polaris.extension.persistence.impl.eclipselink.DiscoverableMetaStoreManagerFactory diff --git a/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.core.persistence.MetaStoreManagerFactory b/extension/persistence/eclipselink/src/main/resources/META-INF/services/org.apache.polaris.extension.persistence.impl.eclipselink.DiscoverableMetaStoreManagerFactory similarity index 91% rename from polaris-service/src/main/resources/META-INF/services/org.apache.polaris.core.persistence.MetaStoreManagerFactory rename to extension/persistence/eclipselink/src/main/resources/META-INF/services/org.apache.polaris.extension.persistence.impl.eclipselink.DiscoverableMetaStoreManagerFactory index 85ae92caf8..58f8891eb8 100644 --- a/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.core.persistence.MetaStoreManagerFactory +++ b/extension/persistence/eclipselink/src/main/resources/META-INF/services/org.apache.polaris.extension.persistence.impl.eclipselink.DiscoverableMetaStoreManagerFactory @@ -17,5 +17,4 @@ # under the License. # -org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory org.apache.polaris.extension.persistence.impl.eclipselink.EclipseLinkPolarisMetaStoreManagerFactory diff --git a/polaris-service/src/main/java/org/apache/polaris/service/persistence/DiscoverableMetaStoreManagerFactory.java b/polaris-service/src/main/java/org/apache/polaris/service/persistence/DiscoverableMetaStoreManagerFactory.java new file mode 100644 index 0000000000..2c2c826c14 --- /dev/null +++ b/polaris-service/src/main/java/org/apache/polaris/service/persistence/DiscoverableMetaStoreManagerFactory.java @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.persistence; + +import io.dropwizard.jackson.Discoverable; +import io.dropwizard.jackson.DiscoverableSubtypeResolver; + +/** + * Discoverability aid for Dropwizard. Leaf classes cannot implement {@link Discoverable} directly, + * they have to implement another interface, which implements {@link Discoverable}. + * + * @see DiscoverableSubtypeResolver + */ +public interface DiscoverableMetaStoreManagerFactory extends Discoverable {} diff --git a/polaris-service/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java b/polaris-service/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java index 1d58a1eb58..e23bb935c9 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/persistence/InMemoryPolarisMetaStoreManagerFactory.java @@ -35,7 +35,8 @@ @JsonTypeName("in-memory") public class InMemoryPolarisMetaStoreManagerFactory - extends LocalPolarisMetaStoreManagerFactory { + extends LocalPolarisMetaStoreManagerFactory + implements DiscoverableMetaStoreManagerFactory { final Set bootstrappedRealms = new HashSet<>(); @Override diff --git a/polaris-service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable b/polaris-service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable index 95d1f8ec7a..b37f6168e7 100644 --- a/polaris-service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable +++ b/polaris-service/src/main/resources/META-INF/services/io.dropwizard.jackson.Discoverable @@ -18,7 +18,7 @@ # org.apache.polaris.service.auth.DiscoverableAuthenticator -org.apache.polaris.core.persistence.MetaStoreManagerFactory +org.apache.polaris.service.persistence.DiscoverableMetaStoreManagerFactory org.apache.polaris.service.config.OAuth2ApiService org.apache.polaris.service.context.RealmContextResolver org.apache.polaris.service.context.CallContextResolver diff --git a/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.persistence.DiscoverableMetaStoreManagerFactory b/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.persistence.DiscoverableMetaStoreManagerFactory new file mode 100644 index 0000000000..611f985cef --- /dev/null +++ b/polaris-service/src/main/resources/META-INF/services/org.apache.polaris.service.persistence.DiscoverableMetaStoreManagerFactory @@ -0,0 +1,20 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory From c343504ef7e94561dc8f7cb00583df1ebea86f8c Mon Sep 17 00:00:00 2001 From: Dmitri Bourlatchkov Date: Thu, 14 Nov 2024 21:54:07 -0500 Subject: [PATCH 4/4] review: add EclipseLink-specific config to tests --- polaris-service/build.gradle.kts | 2 +- .../service/PolarisApplicationConfigurationTest.java | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/polaris-service/build.gradle.kts b/polaris-service/build.gradle.kts index bc7a4a4f51..d45ac66632 100644 --- a/polaris-service/build.gradle.kts +++ b/polaris-service/build.gradle.kts @@ -119,7 +119,7 @@ dependencies { testImplementation(libs.mockito.core) testRuntimeOnly("org.junit.platform:junit-platform-launcher") - testRuntimeOnly(project(":polaris-eclipselink")) + testImplementation(project(":polaris-eclipselink")) } if (project.properties.get("eclipseLink") == "true") { diff --git a/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java b/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java index d0ff17e34e..74442e0409 100644 --- a/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java +++ b/polaris-service/src/test/java/org/apache/polaris/service/PolarisApplicationConfigurationTest.java @@ -24,6 +24,7 @@ import io.dropwizard.testing.ResourceHelpers; import io.dropwizard.testing.junit5.DropwizardAppExtension; import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; +import org.apache.polaris.extension.persistence.impl.eclipselink.EclipseLinkPolarisMetaStoreManagerFactory; import org.apache.polaris.service.config.PolarisApplicationConfig; import org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory; import org.junit.jupiter.api.Nested; @@ -62,14 +63,16 @@ class EclipseLinkMetastore { CONFIG_PATH, RANDOM_APP_PORT, RANDOM_ADMIN_PORT, - ConfigOverride.config("metaStoreManager.type", "eclipse-link")); + ConfigOverride.config("metaStoreManager.type", "eclipse-link"), + ConfigOverride.config("metaStoreManager.persistence-unit", "test-unit"), + ConfigOverride.config("metaStoreManager.conf-file", "/test-conf-file")); @Test void testMetastoreType() { assertThat(app.getConfiguration().getMetaStoreManagerFactory()) - .extracting(Object::getClass) - .extracting(Class::getSimpleName) - .isEqualTo("EclipseLinkPolarisMetaStoreManagerFactory"); + .isInstanceOf(EclipseLinkPolarisMetaStoreManagerFactory.class) + .extracting("persistenceUnitName", "confFile") + .containsExactly("test-unit", "/test-conf-file"); } } }