Skip to content

Commit

Permalink
Remove dropwizard-jackson dep from core (#470)
Browse files Browse the repository at this point in the history
This is a simplification / cleanup. The dependency does not appear to be required in `polaris-core`

Add custom code to `PolarisApplication` find classes directly listed in the `Discoverable`
service descriptor and register them with the ObjectMapper. This approach to finding sub-types
is consistent both with the java service descriptors (listed types actually implement the
service interface) and at the same time allows moving the Dropwizard `Discoverable` dependencies
to the polaris-service module that actually integrates with Dropwizard.

Move leaf metastore classes to the `Discoverable` service descriptor in their respective module.

Note: this fixes the cross-jar leak of EclipseLinkPolarisMetaStoreManagerFactory in service descriptors.
  • Loading branch information
dimas-b authored Nov 29, 2024
1 parent 6bb08d2 commit a6197bd
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeName;
import io.dropwizard.jackson.Discoverable;
import jakarta.annotation.Nonnull;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.context.RealmContext;
Expand All @@ -34,7 +35,7 @@
*/
@JsonTypeName("eclipse-link")
public class EclipseLinkPolarisMetaStoreManagerFactory
extends LocalPolarisMetaStoreManagerFactory<PolarisEclipseLinkStore> {
extends LocalPolarisMetaStoreManagerFactory<PolarisEclipseLinkStore> implements Discoverable {
@JsonProperty("conf-file")
private String confFile;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@
# under the License.
#

org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory
org.apache.polaris.extension.persistence.impl.eclipselink.EclipseLinkPolarisMetaStoreManagerFactory
4 changes: 0 additions & 4 deletions polaris-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,7 +33,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);

Expand Down
2 changes: 1 addition & 1 deletion polaris-service/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import io.dropwizard.core.Application;
import io.dropwizard.core.setup.Bootstrap;
import io.dropwizard.core.setup.Environment;
import io.dropwizard.jackson.Discoverable;
import io.dropwizard.jackson.DiscoverableSubtypeResolver;
import io.micrometer.prometheusmetrics.PrometheusConfig;
import io.micrometer.prometheusmetrics.PrometheusMeterRegistry;
import io.opentelemetry.api.OpenTelemetry;
Expand Down Expand Up @@ -63,6 +65,7 @@
import java.net.URL;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -142,6 +145,7 @@ private static void printAsciiArt() throws IOException {

@Override
public void initialize(Bootstrap<PolarisApplicationConfig> bootstrap) {
registerTypes(bootstrap.getObjectMapper());
// Enable variable substitution with environment variables
EnvironmentVariableSubstitutor substitutor = new EnvironmentVariableSubstitutor(false);
SubstitutingSourceProvider provider =
Expand All @@ -152,6 +156,18 @@ public void initialize(Bootstrap<PolarisApplicationConfig> bootstrap) {
bootstrap.addCommand(new PurgeRealmsCommand());
}

private void registerTypes(ObjectMapper mapper) {
// Reuse the DW service discovery method, but unlike the constructor of
// DiscoverableSubtypeResolver, use the first level classes from the `Discoverable`
// service descriptor and register them with the ObjectMapper.
class Discoverer extends DiscoverableSubtypeResolver {
List<Class<?>> discover() {
return discoverServices(Discoverable.class);
}
}
new Discoverer().discover().forEach(mapper::registerSubtypes);
}

@Override
public void run(PolarisApplicationConfig configuration, Environment environment) {
MetaStoreManagerFactory metaStoreManagerFactory = configuration.getMetaStoreManagerFactory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.polaris.service.persistence;

import com.fasterxml.jackson.annotation.JsonTypeName;
import io.dropwizard.jackson.Discoverable;
import jakarta.annotation.Nonnull;
import java.util.Collections;
import java.util.HashSet;
Expand All @@ -36,7 +37,7 @@

@JsonTypeName("in-memory")
public class InMemoryPolarisMetaStoreManagerFactory
extends LocalPolarisMetaStoreManagerFactory<PolarisTreeMapStore> {
extends LocalPolarisMetaStoreManagerFactory<PolarisTreeMapStore> implements Discoverable {
final Set<String> bootstrappedRealms = new HashSet<>();

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#

org.apache.polaris.service.auth.DiscoverableAuthenticator
org.apache.polaris.core.persistence.MetaStoreManagerFactory
org.apache.polaris.service.persistence.InMemoryPolarisMetaStoreManagerFactory
org.apache.polaris.service.config.OAuth2ApiService
org.apache.polaris.service.context.RealmContextResolver
org.apache.polaris.service.context.CallContextResolver
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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.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;
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<PolarisApplicationConfig> 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<PolarisApplicationConfig> app =
new DropwizardAppExtension<>(
PolarisApplication.class,
CONFIG_PATH,
RANDOM_APP_PORT,
RANDOM_ADMIN_PORT,
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())
.isInstanceOf(EclipseLinkPolarisMetaStoreManagerFactory.class)
.extracting("persistenceUnitName", "confFile")
.containsExactly("test-unit", "/test-conf-file");
}
}
}

0 comments on commit a6197bd

Please sign in to comment.