This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-441a7170-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit a9bd2323c5dafb9583c5df85741c01776d52193a Author: Yuhui <[email protected]> AuthorDate: Thu Sep 17 09:26:21 2026 +0800 [#13211] improvement(trino-connector): Add CatalogConnectorAdapterProvider SPI for external adapters (#13212) ### What changes were proposed in this pull request? - Add `CatalogConnectorAdapterProvider`, a `ServiceLoader` SPI that maps a Gravitino catalog provider to a `CatalogConnectorAdapter`. - `DefaultCatalogConnectorFactory` registers discovered providers after the built-in adapters, ignores a provider whose name is already registered, and skips one that fails to load. - `ContainerSuite` mounts every jar under `GRAVITINO_TRINO_CONNECTOR_EXTRA_LIBS` into the Trino container's Gravitino plugin directory, so an integration test can load an extension jar next to the connector. ### Why are the changes needed? Adapters shipped outside the connector jar can be registered without changing the connector. Fix: #13211 ### Does this PR introduce _any_ user-facing change? New SPI `org.apache.gravitino.trino.connector.catalog.CatalogConnectorAdapterProvider`. Existing behavior is unchanged when no provider is present. ### How was this patch tested? Added `TestDefaultCatalogConnectorFactory` with fake, duplicate and broken providers registered via `META-INF/services`; existing trino-connector unit tests pass. --- .../integration/test/container/ContainerSuite.java | 29 +++++++-- .../catalog/CatalogConnectorAdapterProvider.java | 44 ++++++++++++++ .../catalog/DefaultCatalogConnectorFactory.java | 40 +++++++++++++ .../BrokenCatalogConnectorAdapterProvider.java | 37 ++++++++++++ .../DuplicateCatalogConnectorAdapterProvider.java | 35 +++++++++++ .../FailingCatalogConnectorAdapterProvider.java | 37 ++++++++++++ .../FakeCatalogConnectorAdapterProvider.java | 38 ++++++++++++ .../TestDefaultCatalogConnectorFactory.java | 68 ++++++++++++++++++++++ ...nnector.catalog.CatalogConnectorAdapterProvider | 27 +++++++++ 9 files changed, 349 insertions(+), 6 deletions(-) diff --git a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java index 3b513bf800..6cb94915e0 100644 --- a/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java +++ b/integration-test-common/src/test/java/org/apache/gravitino/integration/test/container/ContainerSuite.java @@ -27,6 +27,7 @@ import com.github.dockerjava.api.model.Network.Ipam.Config; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import java.io.Closeable; +import java.io.File; import java.io.IOException; import java.net.InetAddress; import java.nio.ByteBuffer; @@ -288,6 +289,27 @@ public class ContainerSuite implements Closeable { } } + /** + * Files copied into the Gravitino plugin directory of the Trino container: the connector lib + * directory, plus every jar in the directory named by {@code + * GRAVITINO_TRINO_CONNECTOR_EXTRA_LIBS}, which lets a build add plugin jars that are not part of + * the connector itself. + */ + private static Map<String, String> trinoPluginFilesToMount(String trinoConnectorLibDir) { + ImmutableMap.Builder<String, String> files = ImmutableMap.builder(); + files.put(TrinoContainer.TRINO_CONTAINER_PLUGIN_GRAVITINO_DIR, trinoConnectorLibDir); + String extraLibDir = System.getenv("GRAVITINO_TRINO_CONNECTOR_EXTRA_LIBS"); + if (extraLibDir != null && !extraLibDir.isEmpty()) { + File[] jars = new File(extraLibDir).listFiles((dir, name) -> name.endsWith(".jar")); + for (File jar : jars == null ? new File[0] : jars) { + files.put( + TrinoContainer.TRINO_CONTAINER_PLUGIN_GRAVITINO_DIR + "/" + jar.getName(), + jar.getAbsolutePath()); + } + } + return files.build(); + } + public void startTrinoContainer( String trinoConfDir, String trinoConnectorLibDir, @@ -315,12 +337,7 @@ public class ContainerSuite implements Closeable { .put("host.docker.internal", "host-gateway") .put(HiveContainer.HOST_NAME, hiveContainerIp) .build()) - .withFilesToMount( - ImmutableMap.<String, String>builder() - .put( - TrinoContainer.TRINO_CONTAINER_PLUGIN_GRAVITINO_DIR, - trinoConnectorLibDir) - .build()) + .withFilesToMount(trinoPluginFilesToMount(trinoConnectorLibDir)) .withExposePorts(ImmutableSet.of(TrinoContainer.TRINO_PORT)) .withTrinoConfDir(trinoConfDir) .withMetalakeName(metalakeName) diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorAdapterProvider.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorAdapterProvider.java new file mode 100644 index 0000000000..ed91617f23 --- /dev/null +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorAdapterProvider.java @@ -0,0 +1,44 @@ +/* + * 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.gravitino.trino.connector.catalog; + +import org.apache.gravitino.trino.connector.GravitinoConfig; + +/** + * Service provider interface that lets a jar in the plugin directory contribute a {@link + * CatalogConnectorAdapter} for a Gravitino catalog provider. Implementations are discovered through + * {@link java.util.ServiceLoader} when the connector starts. + */ +public interface CatalogConnectorAdapterProvider { + + /** + * The Gravitino catalog provider this adapter serves, for example {@code jdbc-oracle}. + * + * @return the provider name + */ + String provider(); + + /** + * Creates the adapter for the provider. + * + * @param config the Gravitino connector configuration + * @return a new adapter + */ + CatalogConnectorAdapter createAdapter(GravitinoConfig config); +} diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/DefaultCatalogConnectorFactory.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/DefaultCatalogConnectorFactory.java index 319fe1a9f3..ca6dfd9f2e 100644 --- a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/DefaultCatalogConnectorFactory.java +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/DefaultCatalogConnectorFactory.java @@ -21,6 +21,9 @@ package org.apache.gravitino.trino.connector.catalog; import io.airlift.log.Logger; import io.trino.spi.TrinoException; import java.util.HashMap; +import java.util.Iterator; +import java.util.ServiceConfigurationError; +import java.util.ServiceLoader; import java.util.Set; import org.apache.gravitino.trino.connector.GravitinoConfig; import org.apache.gravitino.trino.connector.GravitinoErrorCode; @@ -81,9 +84,46 @@ public class DefaultCatalogConnectorFactory implements CatalogConnectorFactory { catalogBuilders.put( TRINO_CLUSTER_CONNECTOR_PROVIDER_NAME, new CatalogConnectorContext.Builder(new TrinoClusterConnectorAdapter())); + registerAdapterProviders(config); LOG.info("Start the DefaultCatalogConnectorFactory"); } + /** + * Adds the adapters contributed through {@link CatalogConnectorAdapterProvider}. A provider for a + * catalog provider name that is already registered is ignored. + */ + private void registerAdapterProviders(GravitinoConfig config) { + Iterator<CatalogConnectorAdapterProvider> iterator = + ServiceLoader.load( + CatalogConnectorAdapterProvider.class, + DefaultCatalogConnectorFactory.class.getClassLoader()) + .iterator(); + while (true) { + try { + if (!iterator.hasNext()) { + return; + } + CatalogConnectorAdapterProvider provider = iterator.next(); + String providerName = provider.provider(); + if (catalogBuilders.containsKey(providerName)) { + LOG.warn( + "Ignore catalog connector adapter provider %s for %s: already registered.", + provider.getClass().getName(), providerName); + continue; + } + catalogBuilders.put( + providerName, new CatalogConnectorContext.Builder(provider.createAdapter(config))); + LOG.info("Registered catalog connector adapter for %s", providerName); + } catch (ServiceConfigurationError | LinkageError | RuntimeException e) { + // ServiceLoader reports a missing class or a failing constructor as + // ServiceConfigurationError; a provider built against another Trino version fails with a + // LinkageError once its methods run, and a misconfigured one may throw from provider() or + // createAdapter(). Skip that entry and keep the rest. + LOG.warn(e, "Skip a catalog connector adapter provider that cannot be loaded."); + } + } + } + /** * Get supported catalog providers * diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/BrokenCatalogConnectorAdapterProvider.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/BrokenCatalogConnectorAdapterProvider.java new file mode 100644 index 0000000000..9cdcbabe7d --- /dev/null +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/BrokenCatalogConnectorAdapterProvider.java @@ -0,0 +1,37 @@ +/* + * 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.gravitino.trino.connector.catalog; + +import org.apache.gravitino.trino.connector.GravitinoConfig; + +/** Fails to link once its methods run, like a provider built against another Trino version. */ +public class BrokenCatalogConnectorAdapterProvider implements CatalogConnectorAdapterProvider { + + public static final String PROVIDER = "jdbc-broken"; + + @Override + public String provider() { + throw new NoClassDefFoundError("io/trino/spi/Missing"); + } + + @Override + public CatalogConnectorAdapter createAdapter(GravitinoConfig config) { + return null; + } +} diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/DuplicateCatalogConnectorAdapterProvider.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/DuplicateCatalogConnectorAdapterProvider.java new file mode 100644 index 0000000000..d45cd0cec4 --- /dev/null +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/DuplicateCatalogConnectorAdapterProvider.java @@ -0,0 +1,35 @@ +/* + * 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.gravitino.trino.connector.catalog; + +import org.apache.gravitino.trino.connector.GravitinoConfig; + +/** Claims a built-in provider name; the factory must keep the built-in adapter. */ +public class DuplicateCatalogConnectorAdapterProvider implements CatalogConnectorAdapterProvider { + + @Override + public String provider() { + return "jdbc-mysql"; + } + + @Override + public CatalogConnectorAdapter createAdapter(GravitinoConfig config) { + throw new AssertionError("A duplicate provider must not create an adapter"); + } +} diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/FailingCatalogConnectorAdapterProvider.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/FailingCatalogConnectorAdapterProvider.java new file mode 100644 index 0000000000..7dcd030912 --- /dev/null +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/FailingCatalogConnectorAdapterProvider.java @@ -0,0 +1,37 @@ +/* + * 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.gravitino.trino.connector.catalog; + +import org.apache.gravitino.trino.connector.GravitinoConfig; + +/** Throws while creating its adapter, like a provider that rejects the connector configuration. */ +public class FailingCatalogConnectorAdapterProvider implements CatalogConnectorAdapterProvider { + + public static final String PROVIDER = "jdbc-failing"; + + @Override + public String provider() { + return PROVIDER; + } + + @Override + public CatalogConnectorAdapter createAdapter(GravitinoConfig config) { + throw new IllegalStateException("adapter creation failed"); + } +} diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/FakeCatalogConnectorAdapterProvider.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/FakeCatalogConnectorAdapterProvider.java new file mode 100644 index 0000000000..7f1c1a4762 --- /dev/null +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/FakeCatalogConnectorAdapterProvider.java @@ -0,0 +1,38 @@ +/* + * 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.gravitino.trino.connector.catalog; + +import org.apache.gravitino.trino.connector.GravitinoConfig; +import org.apache.gravitino.trino.connector.catalog.jdbc.mysql.MySQLConnectorAdapter; + +/** Registered through META-INF/services in the test resources. */ +public class FakeCatalogConnectorAdapterProvider implements CatalogConnectorAdapterProvider { + + public static final String PROVIDER = "jdbc-testdb"; + + @Override + public String provider() { + return PROVIDER; + } + + @Override + public CatalogConnectorAdapter createAdapter(GravitinoConfig config) { + return new MySQLConnectorAdapter(); + } +} diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/TestDefaultCatalogConnectorFactory.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/TestDefaultCatalogConnectorFactory.java new file mode 100644 index 0000000000..71129ce206 --- /dev/null +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/TestDefaultCatalogConnectorFactory.java @@ -0,0 +1,68 @@ +/* + * 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.gravitino.trino.connector.catalog; + +import com.google.common.collect.ImmutableMap; +import org.apache.gravitino.trino.connector.GravitinoConfig; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class TestDefaultCatalogConnectorFactory { + + @Test + void testBuiltInProviders() { + DefaultCatalogConnectorFactory factory = new DefaultCatalogConnectorFactory(config()); + Assertions.assertTrue(factory.getSupportedCatalogProviders().contains("hive")); + Assertions.assertTrue(factory.getSupportedCatalogProviders().contains("jdbc-mysql")); + Assertions.assertTrue(factory.getSupportedCatalogProviders().contains("jdbc-postgresql")); + } + + @Test + void testAdapterProviderIsRegistered() { + DefaultCatalogConnectorFactory factory = new DefaultCatalogConnectorFactory(config()); + Assertions.assertTrue( + factory + .getSupportedCatalogProviders() + .contains(FakeCatalogConnectorAdapterProvider.PROVIDER)); + } + + @Test + void testBuiltInProviderWinsOverAdapterProvider() { + // DuplicateCatalogConnectorAdapterProvider throws if asked for an adapter. + Assertions.assertDoesNotThrow(() -> new DefaultCatalogConnectorFactory(config())); + } + + @Test + void testUnloadableAdapterProviderIsSkipped() { + DefaultCatalogConnectorFactory factory = new DefaultCatalogConnectorFactory(config()); + Assertions.assertFalse( + factory + .getSupportedCatalogProviders() + .contains(BrokenCatalogConnectorAdapterProvider.PROVIDER)); + Assertions.assertFalse( + factory + .getSupportedCatalogProviders() + .contains(FailingCatalogConnectorAdapterProvider.PROVIDER)); + } + + private static GravitinoConfig config() { + return new GravitinoConfig( + ImmutableMap.of("gravitino.uri", "http://localhost:8090";, "gravitino.metalake", "test")); + } +} diff --git a/trino-connector/trino-connector/src/test/resources/META-INF/services/org.apache.gravitino.trino.connector.catalog.CatalogConnectorAdapterProvider b/trino-connector/trino-connector/src/test/resources/META-INF/services/org.apache.gravitino.trino.connector.catalog.CatalogConnectorAdapterProvider new file mode 100644 index 0000000000..455bc27733 --- /dev/null +++ b/trino-connector/trino-connector/src/test/resources/META-INF/services/org.apache.gravitino.trino.connector.catalog.CatalogConnectorAdapterProvider @@ -0,0 +1,27 @@ +# +# 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. +# + +# A missing class exercises the ServiceConfigurationError skip path, a provider that fails to link +# the LinkageError skip path and one that throws from createAdapter the RuntimeException skip +# path; the fake below must still be registered. +org.apache.gravitino.trino.connector.catalog.MissingCatalogConnectorAdapterProvider +org.apache.gravitino.trino.connector.catalog.BrokenCatalogConnectorAdapterProvider +org.apache.gravitino.trino.connector.catalog.FailingCatalogConnectorAdapterProvider +org.apache.gravitino.trino.connector.catalog.FakeCatalogConnectorAdapterProvider +org.apache.gravitino.trino.connector.catalog.DuplicateCatalogConnectorAdapterProvider
