From e631781318706195955638663356481df08b0502 Mon Sep 17 00:00:00 2001 From: Paul Rogers Date: Sat, 23 Jul 2022 14:17:17 -0700 Subject: [PATCH 1/4] Refactor Guice initialization Builders for various module collections Revise the extensions loader Injector builders for server startup Move Hadoop init to indexer Clean up server node role filtering Calcite test injector builder --- .../druid/initialization/DruidModule.java | 7 + indexing-service/pom.xml | 4 + .../indexing/common/task/HadoopTask.java | 30 +- .../indexing/common/task/Initialization.java | 62 ++ .../parallel/ParallelIndexSupervisorTask.java | 2 +- .../common/task/InitializationTest.java | 95 +++ ...stractParallelIndexSupervisorTaskTest.java | 1 - ...ultiPhaseParallelIndexingRowStatsTest.java | 3 +- .../druid/guice/BaseInjectorBuilder.java | 68 ++ .../guice}/ExtensionFirstClassLoader.java | 2 +- .../apache/druid/guice/ExtensionsLoader.java | 328 +++++++++ .../apache/druid/guice/ExtensionsModule.java | 64 ++ .../apache/druid/guice/GuiceInjectors.java | 40 +- .../druid/guice/StartupInjectorBuilder.java | 85 +++ .../query/aggregation/AggregatorFactory.java | 1 - .../GroupByMergingQueryRunnerV2.java | 1 - .../druid/guice/ExtensionsLoaderTest.java | 332 +++++++++ server/pom.xml | 6 +- .../druid/guice/DruidInjectorBuilder.java | 203 ++++++ .../apache/druid/guice/ExpressionModule.java | 11 +- .../initialization/CoreInjectorBuilder.java | 142 ++++ .../ExtensionInjectorBuilder.java | 51 ++ .../druid/initialization/Initialization.java | 524 +-------------- .../initialization/ServerInjectorBuilder.java | 128 ++++ .../ServiceInjectorBuilder.java | 63 ++ .../apache/druid/server/StatusResource.java | 13 +- .../druid/client/DruidServerConfigTest.java | 2 - .../druid/curator/CuratorModuleTest.java | 19 +- .../initialization/InitializationTest.java | 631 ------------------ .../ServerInjectorBuilderTest.java | 285 ++++++++ .../query/lookup/LookupSerdeModuleTest.java | 26 +- .../druid/server/StatusResourceTest.java | 4 +- ...rg.apache.druid.initialization.DruidModule | 2 +- services/pom.xml | 1 + .../apache/druid/cli/CliHadoopIndexer.java | 17 +- .../org/apache/druid/cli/GuiceRunnable.java | 38 +- .../main/java/org/apache/druid/cli/Main.java | 14 +- .../java/org/apache/druid/cli/Version.java | 13 +- .../cli/validate/DruidJsonValidator.java | 6 +- .../guice/AbstractDruidServiceModule.java | 7 +- .../cli/DiscoverySideEffectsProviderTest.java | 16 +- .../util/CalciteTestInjectorBuilder.java | 106 +++ .../druid/sql/calcite/util/CalciteTests.java | 47 +- 43 files changed, 2129 insertions(+), 1371 deletions(-) create mode 100644 indexing-service/src/main/java/org/apache/druid/indexing/common/task/Initialization.java create mode 100644 indexing-service/src/test/java/org/apache/druid/indexing/common/task/InitializationTest.java create mode 100644 processing/src/main/java/org/apache/druid/guice/BaseInjectorBuilder.java rename {server/src/main/java/org/apache/druid/initialization => processing/src/main/java/org/apache/druid/guice}/ExtensionFirstClassLoader.java (98%) create mode 100644 processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java create mode 100644 processing/src/main/java/org/apache/druid/guice/ExtensionsModule.java create mode 100644 processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java create mode 100644 processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java create mode 100644 server/src/main/java/org/apache/druid/guice/DruidInjectorBuilder.java create mode 100644 server/src/main/java/org/apache/druid/initialization/CoreInjectorBuilder.java create mode 100644 server/src/main/java/org/apache/druid/initialization/ExtensionInjectorBuilder.java create mode 100644 server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java create mode 100644 server/src/main/java/org/apache/druid/initialization/ServiceInjectorBuilder.java delete mode 100644 server/src/test/java/org/apache/druid/initialization/InitializationTest.java create mode 100644 server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java create mode 100644 sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTestInjectorBuilder.java diff --git a/core/src/main/java/org/apache/druid/initialization/DruidModule.java b/core/src/main/java/org/apache/druid/initialization/DruidModule.java index ba298c9be6fa..b2a42f41e9cd 100644 --- a/core/src/main/java/org/apache/druid/initialization/DruidModule.java +++ b/core/src/main/java/org/apache/druid/initialization/DruidModule.java @@ -25,6 +25,13 @@ import java.util.List; /** + * A Guice module which also provides Jackson modules. + * There is generally no need to implement this interface unless + * you want to define Jackson modules: use the Guice + * {@code Module} otherwise. + *

+ * Extension modules extend this interface rather than from + * the Guice {@code Module} interface. */ @ExtensionPoint public interface DruidModule extends com.google.inject.Module diff --git a/indexing-service/pom.xml b/indexing-service/pom.xml index da36730730b5..b78de7e25f36 100644 --- a/indexing-service/pom.xml +++ b/indexing-service/pom.xml @@ -212,6 +212,10 @@ commons-collections4 provided + + org.eclipse.aether + aether-api + junit diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java index 39d74537fac2..0de13add2113 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java @@ -25,13 +25,14 @@ import com.google.common.collect.Lists; import com.google.inject.Injector; import org.apache.druid.guice.ExtensionsConfig; -import org.apache.druid.guice.GuiceInjectors; +import org.apache.druid.guice.ExtensionsLoader; +import org.apache.druid.guice.StartupInjectorBuilder; import org.apache.druid.indexing.common.TaskToolbox; -import org.apache.druid.initialization.Initialization; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.utils.JvmUtils; import javax.annotation.Nullable; + import java.io.File; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -48,13 +49,11 @@ public abstract class HadoopTask extends AbstractBatchIndexTask { private static final Logger log = new Logger(HadoopTask.class); - private static final ExtensionsConfig EXTENSIONS_CONFIG; - - static final Injector INJECTOR = GuiceInjectors.makeStartupInjector(); - static { - EXTENSIONS_CONFIG = INJECTOR.getInstance(ExtensionsConfig.class); - } + // Note: static variables mean that this task cannot run in a shared JVM, + // such as the Indexer. + static final Injector INJECTOR = new StartupInjectorBuilder().withExtensions().build(); + private static final ExtensionsLoader EXTENSIONS_LOADER = ExtensionsLoader.instance(INJECTOR); private final List hadoopDependencyCoordinates; @@ -152,8 +151,8 @@ public static ClassLoader buildClassLoader(final List hadoopDependencyCo } final List extensionURLs = new ArrayList<>(); - for (final File extension : Initialization.getExtensionFilesToLoad(EXTENSIONS_CONFIG)) { - final URLClassLoader extensionLoader = Initialization.getClassLoaderForExtension(extension, false); + for (final File extension : EXTENSIONS_LOADER.getExtensionFilesToLoad()) { + final URLClassLoader extensionLoader = EXTENSIONS_LOADER.getClassLoaderForExtension(extension, false); extensionURLs.addAll(Arrays.asList(extensionLoader.getURLs())); } @@ -165,9 +164,9 @@ public static ClassLoader buildClassLoader(final List hadoopDependencyCo for (final File hadoopDependency : Initialization.getHadoopDependencyFilesToLoad( finalHadoopDependencyCoordinates, - EXTENSIONS_CONFIG + EXTENSIONS_LOADER.config() )) { - final URLClassLoader hadoopLoader = Initialization.getClassLoaderForExtension(hadoopDependency, false); + final URLClassLoader hadoopLoader = EXTENSIONS_LOADER.getClassLoaderForExtension(hadoopDependency, false); localClassLoaderURLs.addAll(Arrays.asList(hadoopLoader.getURLs())); } @@ -187,15 +186,16 @@ public static ClassLoader buildClassLoader(final List hadoopDependencyCo ); final String hadoopContainerDruidClasspathJars; - if (EXTENSIONS_CONFIG.getHadoopContainerDruidClasspath() == null) { + ExtensionsConfig extnConfig = EXTENSIONS_LOADER.config(); + if (extnConfig.getHadoopContainerDruidClasspath() == null) { hadoopContainerDruidClasspathJars = Joiner.on(File.pathSeparator).join(jobURLs); } else { List hadoopContainerURLs = Lists.newArrayList( - Initialization.getURLsForClasspath(EXTENSIONS_CONFIG.getHadoopContainerDruidClasspath()) + ExtensionsLoader.getURLsForClasspath(extnConfig.getHadoopContainerDruidClasspath()) ); - if (EXTENSIONS_CONFIG.getAddExtensionsToHadoopContainer()) { + if (extnConfig.getAddExtensionsToHadoopContainer()) { hadoopContainerURLs.addAll(extensionURLs); } diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Initialization.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Initialization.java new file mode 100644 index 000000000000..df0312127ac9 --- /dev/null +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Initialization.java @@ -0,0 +1,62 @@ +/* + * 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.druid.indexing.common.task; + +import org.apache.druid.guice.ExtensionsConfig; +import org.apache.druid.java.util.common.ISE; +import org.eclipse.aether.artifact.DefaultArtifact; + +import java.io.File; +import java.util.List; + +public class Initialization +{ + /** + * Find all the Hadoop dependencies that should be loaded by Druid. + * + * @param hadoopDependencyCoordinates e.g.["org.apache.hadoop:hadoop-client:2.3.0"] + * @param extensionsConfig ExtensionsConfig configured by druid.extensions.xxx + * + * @return an array of Hadoop dependency files that will be loaded by the Druid process. + */ + public static File[] getHadoopDependencyFilesToLoad( + List hadoopDependencyCoordinates, + ExtensionsConfig extensionsConfig + ) + { + final File rootHadoopDependenciesDir = new File(extensionsConfig.getHadoopDependenciesDir()); + if (rootHadoopDependenciesDir.exists() && !rootHadoopDependenciesDir.isDirectory()) { + throw new ISE("Root Hadoop dependencies directory [%s] is not a directory!?", rootHadoopDependenciesDir); + } + final File[] hadoopDependenciesToLoad = new File[hadoopDependencyCoordinates.size()]; + int i = 0; + for (final String coordinate : hadoopDependencyCoordinates) { + final DefaultArtifact artifact = new DefaultArtifact(coordinate); + final File hadoopDependencyDir = new File(rootHadoopDependenciesDir, artifact.getArtifactId()); + final File versionDir = new File(hadoopDependencyDir, artifact.getVersion()); + // find the hadoop dependency with the version specified in coordinate + if (!hadoopDependencyDir.isDirectory() || !versionDir.isDirectory()) { + throw new ISE("Hadoop dependency [%s] didn't exist!?", versionDir.getAbsolutePath()); + } + hadoopDependenciesToLoad[i++] = versionDir; + } + return hadoopDependenciesToLoad; + } +} diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java index f8381ffafe8f..434bbcdff639 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/batch/parallel/ParallelIndexSupervisorTask.java @@ -140,7 +140,7 @@ public class ParallelIndexSupervisorTask extends AbstractBatchIndexTask implemen // reproduce but looking at the code around where the following constant is used one // possibility is that the sketch's estimate is negative. If that case happens // code has been added to log it and to set the estimate to the value of the - // following constant. It is not necessary to parametize this value since if this + // following constant. It is not necessary to parameterize this value since if this // happens it is a bug and the new logging may now provide some evidence to reproduce // and fix private static final long DEFAULT_NUM_SHARDS_WHEN_ESTIMATE_GOES_NEGATIVE = 7L; diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/InitializationTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/InitializationTest.java new file mode 100644 index 000000000000..f5ad498ba76e --- /dev/null +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/InitializationTest.java @@ -0,0 +1,95 @@ +/* + * 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.druid.indexing.common.task; + +import com.google.common.collect.ImmutableList; +import org.apache.druid.guice.ExtensionsConfig; +import org.apache.druid.java.util.common.ISE; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; + +public class InitializationTest +{ + @Rule + public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Test(expected = ISE.class) + public void testGetHadoopDependencyFilesToLoad_wrong_type_root_hadoop_depenencies_dir() throws IOException + { + final File rootHadoopDependenciesDir = temporaryFolder.newFile(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public String getHadoopDependenciesDir() + { + return rootHadoopDependenciesDir.getAbsolutePath(); + } + }; + Initialization.getHadoopDependencyFilesToLoad(ImmutableList.of(), config); + } + + @Test(expected = ISE.class) + public void testGetHadoopDependencyFilesToLoad_non_exist_version_dir() throws IOException + { + final File rootHadoopDependenciesDir = temporaryFolder.newFolder(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public String getHadoopDependenciesDir() + { + return rootHadoopDependenciesDir.getAbsolutePath(); + } + }; + final File hadoopClient = new File(rootHadoopDependenciesDir, "hadoop-client"); + hadoopClient.mkdir(); + Initialization.getHadoopDependencyFilesToLoad(ImmutableList.of("org.apache.hadoop:hadoop-client:2.3.0"), config); + } + + + @Test + public void testGetHadoopDependencyFilesToLoad_with_hadoop_coordinates() throws IOException + { + final File rootHadoopDependenciesDir = temporaryFolder.newFolder(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public String getHadoopDependenciesDir() + { + return rootHadoopDependenciesDir.getAbsolutePath(); + } + }; + final File hadoopClient = new File(rootHadoopDependenciesDir, "hadoop-client"); + final File versionDir = new File(hadoopClient, "2.3.0"); + hadoopClient.mkdir(); + versionDir.mkdir(); + final File[] expectedFileList = new File[]{versionDir}; + final File[] actualFileList = Initialization.getHadoopDependencyFilesToLoad( + ImmutableList.of( + "org.apache.hadoop:hadoop-client:2.3.0" + ), config + ); + Assert.assertArrayEquals(expectedFileList, actualFileList); + } +} diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java index ecb00917a9b3..b72e39a657d9 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java @@ -923,7 +923,6 @@ static class LocalParallelIndexTaskClientProvider implements ParallelIndexSuperv this.transientApiCallFailureRate = transientApiCallFailureRate; } - @Override public ParallelIndexSupervisorTaskClient build(String supervisorTaskId, Duration httpTimeout, long numRetries) { diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java index 06aced68eeb3..0b8d38e947f8 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java @@ -33,6 +33,7 @@ import org.apache.druid.segment.incremental.RowIngestionMetersTotals; import org.joda.time.Interval; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import java.io.File; @@ -107,6 +108,7 @@ public void testHashPartitionRowStats() } @Test + @Ignore("assumes record rates, to be fixed in separate PR") public void testHashPartitionRowStats_concurrentSubTasks_1() { testHashPartitionRowStats(1); @@ -175,5 +177,4 @@ public void testRangePartitionRowStats() Map actualReports = runTaskAndGetReports(task, TaskState.SUCCESS); compareTaskReports(expectedReports, actualReports); } - } diff --git a/processing/src/main/java/org/apache/druid/guice/BaseInjectorBuilder.java b/processing/src/main/java/org/apache/druid/guice/BaseInjectorBuilder.java new file mode 100644 index 000000000000..40bbc5a0f8ec --- /dev/null +++ b/processing/src/main/java/org/apache/druid/guice/BaseInjectorBuilder.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.druid.guice; + +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Module; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Utilities for building a Guice injector. Defined as a parameterized + * type so that this class can be used in derived fluent builders. + */ +public class BaseInjectorBuilder> +{ + private final List modules = new ArrayList<>(); + + @SuppressWarnings("unchecked") + public T add(Module... modules) + { + // Done this way because IntelliJ inspections complains if we + // try to iterate, because it thinks addAll() accepts an array, + // which it does not. + this.modules.addAll(Arrays.asList(modules)); + return (T) this; + } + + @SuppressWarnings("unchecked") + public T addAll(List modules) + { + this.modules.addAll(modules); + return (T) this; + } + + @SuppressWarnings("unchecked") + public T addAll(Iterable modules) + { + for (Module m : modules) { + this.modules.add(m); + } + return (T) this; + } + + public Injector build() + { + return Guice.createInjector(modules); + } +} diff --git a/server/src/main/java/org/apache/druid/initialization/ExtensionFirstClassLoader.java b/processing/src/main/java/org/apache/druid/guice/ExtensionFirstClassLoader.java similarity index 98% rename from server/src/main/java/org/apache/druid/initialization/ExtensionFirstClassLoader.java rename to processing/src/main/java/org/apache/druid/guice/ExtensionFirstClassLoader.java index 2c460e242aa8..1a2944b2bdb0 100644 --- a/server/src/main/java/org/apache/druid/initialization/ExtensionFirstClassLoader.java +++ b/processing/src/main/java/org/apache/druid/guice/ExtensionFirstClassLoader.java @@ -17,7 +17,7 @@ * under the License. */ -package org.apache.druid.initialization; +package org.apache.druid.guice; import com.google.common.base.Preconditions; import com.google.common.collect.Iterators; diff --git a/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java b/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java new file mode 100644 index 000000000000..2d0d933513e8 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java @@ -0,0 +1,328 @@ +/* + * 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.druid.guice; + +import com.google.common.annotations.VisibleForTesting; +import com.google.inject.Injector; +import org.apache.commons.io.FileUtils; +import org.apache.druid.initialization.DruidModule; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.logger.Logger; + +import javax.inject.Inject; + +import java.io.File; +import java.io.FilenameFilter; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.ServiceLoader; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; + +public class ExtensionsLoader +{ + private static final Logger log = new Logger(ExtensionsLoader.class); + + private final ExtensionsConfig extensionsConfig; + private final ConcurrentHashMap loaders = new ConcurrentHashMap<>(); + + /** + * Map of loaded extensions, keyed by class (or interface). + */ + private final ConcurrentHashMap, Collection> extensions = new ConcurrentHashMap<>(); + + @Inject + public ExtensionsLoader(ExtensionsConfig config) + { + this.extensionsConfig = config; + } + + public static ExtensionsLoader instance(Injector injector) + { + return injector.getInstance(ExtensionsLoader.class); + } + + public ExtensionsConfig config() + { + return extensionsConfig; + } + + /** + * @param clazz service class + * @param the service type + * + * @return Returns a collection of implementations loaded. + */ + public Collection getLoadedImplementations(Class clazz) + { + @SuppressWarnings("unchecked") + Collection retVal = (Collection) extensions.get(clazz); + if (retVal == null) { + return new HashSet<>(); + } + return retVal; + } + + /** + * @return Returns a collection of implementations loaded. + */ + public Collection getLoadedModules() + { + return getLoadedImplementations(DruidModule.class); + } + + @VisibleForTesting + public Map getLoadersMap() + { + return loaders; + } + + /** + * Look for implementations for the given class from both classpath and extensions directory, using {@link + * ServiceLoader}. A user should never put the same two extensions in classpath and extensions directory, if he/she + * does that, the one that is in the classpath will be loaded, the other will be ignored. + * + * @param serviceClass The class to look the implementations of (e.g., DruidModule) + * + * @return A collection that contains implementations (of distinct concrete classes) of the given class. The order of + * elements in the returned collection is not specified and not guaranteed to be the same for different calls to + * getFromExtensions(). + */ + @SuppressWarnings("unchecked") + public Collection getFromExtensions(Class serviceClass) + { + // Classes classes are loaded once upon first request. Since the class path does + // not change during a run, the set of extension classes cannot change once + // computed. + // + // In practice, it appears the only place this matters is with DruidModule: + // initialization gets the list of extensions, and two REST API calls later + // ask for the same list. + Collection modules = extensions.computeIfAbsent( + serviceClass, + serviceC -> new ServiceLoadingFromExtensions<>(serviceC).implsToLoad + ); + //noinspection unchecked + return (Collection) modules; + } + + public Collection getModules() + { + return getFromExtensions(DruidModule.class); + } + + /** + * Find all the extension files that should be loaded by druid. + *

+ * If user explicitly specifies druid.extensions.loadList, then it will look for those extensions under root + * extensions directory. If one of them is not found, druid will fail loudly. + *

+ * If user doesn't specify druid.extension.toLoad (or its value is empty), druid will load all the extensions + * under the root extensions directory. + * + * @return an array of druid extension files that will be loaded by druid process + */ + public File[] getExtensionFilesToLoad() + { + final File rootExtensionsDir = new File(extensionsConfig.getDirectory()); + if (rootExtensionsDir.exists() && !rootExtensionsDir.isDirectory()) { + throw new ISE("Root extensions directory [%s] is not a directory!?", rootExtensionsDir); + } + File[] extensionsToLoad; + final LinkedHashSet toLoad = extensionsConfig.getLoadList(); + if (toLoad == null) { + extensionsToLoad = rootExtensionsDir.listFiles(); + } else { + int i = 0; + extensionsToLoad = new File[toLoad.size()]; + for (final String extensionName : toLoad) { + File extensionDir = new File(extensionName); + if (!extensionDir.isAbsolute()) { + extensionDir = new File(rootExtensionsDir, extensionName); + } + + if (!extensionDir.isDirectory()) { + throw new ISE( + "Extension [%s] specified in \"druid.extensions.loadList\" didn't exist!?", + extensionDir.getAbsolutePath() + ); + } + extensionsToLoad[i++] = extensionDir; + } + } + return extensionsToLoad == null ? new File[]{} : extensionsToLoad; + } + + /** + * @param extension The File instance of the extension we want to load + * + * @return a URLClassLoader that loads all the jars on which the extension is dependent + */ + public URLClassLoader getClassLoaderForExtension(File extension, boolean useExtensionClassloaderFirst) + { + return loaders.computeIfAbsent( + extension, + theExtension -> makeClassLoaderForExtension(theExtension, useExtensionClassloaderFirst) + ); + } + + private static URLClassLoader makeClassLoaderForExtension( + final File extension, + final boolean useExtensionClassloaderFirst + ) + { + final Collection jars = FileUtils.listFiles(extension, new String[]{"jar"}, false); + final URL[] urls = new URL[jars.size()]; + + try { + int i = 0; + for (File jar : jars) { + final URL url = jar.toURI().toURL(); + log.debug("added URL[%s] for extension[%s]", url, extension.getName()); + urls[i++] = url; + } + } + catch (MalformedURLException e) { + throw new RuntimeException(e); + } + + if (useExtensionClassloaderFirst) { + return new ExtensionFirstClassLoader(urls, ExtensionsLoader.class.getClassLoader()); + } else { + return new URLClassLoader(urls, ExtensionsLoader.class.getClassLoader()); + } + } + + public static List getURLsForClasspath(String cp) + { + try { + String[] paths = cp.split(File.pathSeparator); + + List urls = new ArrayList<>(); + for (String path : paths) { + File f = new File(path); + if ("*".equals(f.getName())) { + File parentDir = f.getParentFile(); + if (parentDir.isDirectory()) { + File[] jars = parentDir.listFiles( + new FilenameFilter() + { + @Override + public boolean accept(File dir, String name) + { + return name != null && (name.endsWith(".jar") || name.endsWith(".JAR")); + } + } + ); + for (File jar : jars) { + urls.add(jar.toURI().toURL()); + } + } + } else { + urls.add(new File(path).toURI().toURL()); + } + } + return urls; + } + catch (IOException ex) { + throw new RuntimeException(ex); + } + } + + private class ServiceLoadingFromExtensions + { + private final Class serviceClass; + private final List implsToLoad = new ArrayList<>(); + private final Set implClassNamesToLoad = new HashSet<>(); + + private ServiceLoadingFromExtensions(Class serviceClass) + { + this.serviceClass = serviceClass; + if (extensionsConfig.searchCurrentClassloader()) { + addAllFromCurrentClassLoader(); + } + addAllFromFileSystem(); + } + + private void addAllFromCurrentClassLoader() + { + ServiceLoader + .load(serviceClass, Thread.currentThread().getContextClassLoader()) + .forEach(impl -> tryAdd(impl, "classpath")); + } + + private void addAllFromFileSystem() + { + for (File extension : getExtensionFilesToLoad()) { + log.debug("Loading extension [%s] for class [%s]", extension.getName(), serviceClass); + try { + final URLClassLoader loader = getClassLoaderForExtension( + extension, + extensionsConfig.isUseExtensionClassloaderFirst() + ); + + log.info( + "Loading extension [%s], jars: %s", + extension.getName(), + Arrays.stream(loader.getURLs()) + .map(u -> new File(u.getPath()).getName()) + .collect(Collectors.joining(", ")) + ); + + ServiceLoader.load(serviceClass, loader).forEach(impl -> tryAdd(impl, "local file system")); + } + catch (Exception e) { + throw new RuntimeException(e); + } + } + } + + private void tryAdd(T serviceImpl, String extensionType) + { + final String serviceImplName = serviceImpl.getClass().getName(); + if (serviceImplName == null) { + log.warn( + "Implementation [%s] was ignored because it doesn't have a canonical name, " + + "is it a local or anonymous class?", + serviceImpl.getClass().getName() + ); + } else if (!implClassNamesToLoad.contains(serviceImplName)) { + log.debug( + "Adding implementation [%s] for class [%s] from %s extension", + serviceImplName, + serviceClass, + extensionType + ); + implClassNamesToLoad.add(serviceImplName); + implsToLoad.add(serviceImpl); + } + } + } +} diff --git a/processing/src/main/java/org/apache/druid/guice/ExtensionsModule.java b/processing/src/main/java/org/apache/druid/guice/ExtensionsModule.java new file mode 100644 index 000000000000..070bbc152ebf --- /dev/null +++ b/processing/src/main/java/org/apache/druid/guice/ExtensionsModule.java @@ -0,0 +1,64 @@ +/* + * 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.druid.guice; + +import com.google.inject.Binder; +import com.google.inject.Module; + +import javax.inject.Inject; + +/** + * Module for the extensions loader. Add to the startup injector + * for Druid servers. Not visible to the {@link StartupInjectorBuilder}, + * so must be added by servers explicitly. + */ +public class ExtensionsModule implements Module +{ + @Override + public void configure(Binder binder) + { + binder.bind(ExtensionsLoader.class).in(LazySingleton.class); + JsonConfigProvider.bind(binder, "druid.extensions", ExtensionsConfig.class); + JsonConfigProvider.bind(binder, "druid.modules", ModulesConfig.class); + } + + /** + * Transfers the now-populated extension loader instance from the + * startup to the main injector. Not done in {@code DruidSecondaryModule} + * because extensions are loaded only in the server, but + * {@code DruidSecondaryModule} is used for tests and clients also. + */ + public static class SecondaryModule implements Module + { + private final ExtensionsLoader extnLoader; + + @Inject + public SecondaryModule(final ExtensionsLoader extnLoader) + { + this.extnLoader = extnLoader; + } + + @Override + public void configure(Binder binder) + { + binder.bind(ExtensionsLoader.class).toInstance(extnLoader); + } + } +} diff --git a/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java b/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java index 7ee10df46e68..b502cfcd48b4 100644 --- a/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java +++ b/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java @@ -19,51 +19,27 @@ package org.apache.druid.guice; -import com.google.common.collect.ImmutableList; -import com.google.inject.Guice; import com.google.inject.Injector; import com.google.inject.Module; -import org.apache.druid.jackson.JacksonModule; -import org.apache.druid.math.expr.ExpressionProcessingModule; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; +import java.util.Collections; /** + * Creates the startup injector. Regained for backward compatibility. + * New code should prefer using {@link StartupInjectorBuilder} */ public class GuiceInjectors { - public static Collection makeDefaultStartupModules() - { - return ImmutableList.of( - new DruidGuiceExtensions(), - new JacksonModule(), - new PropertiesModule(Arrays.asList("common.runtime.properties", "runtime.properties")), - new RuntimeInfoModule(), - new ConfigModule(), - new NullHandlingModule(), - new ExpressionProcessingModule(), - binder -> { - binder.bind(DruidSecondaryModule.class); - JsonConfigProvider.bind(binder, "druid.extensions", ExtensionsConfig.class); - JsonConfigProvider.bind(binder, "druid.modules", ModulesConfig.class); - } - ); - } - public static Injector makeStartupInjector() { - return Guice.createInjector(makeDefaultStartupModules()); + return makeStartupInjectorWithModules(Collections.emptyList()); } public static Injector makeStartupInjectorWithModules(Iterable modules) { - List theModules = new ArrayList<>(makeDefaultStartupModules()); - for (Module theModule : modules) { - theModules.add(theModule); - } - return Guice.createInjector(theModules); + return new StartupInjectorBuilder() + .forServer() + .addAll(modules) + .build(); } } diff --git a/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java b/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java new file mode 100644 index 000000000000..838345f58d01 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/guice/StartupInjectorBuilder.java @@ -0,0 +1,85 @@ +/* + * 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.druid.guice; + +import org.apache.druid.jackson.JacksonModule; +import org.apache.druid.math.expr.ExpressionProcessingModule; + +import java.util.Arrays; +import java.util.Properties; + +/** + * Create the startup injector used to "prime" the modules for the + * main injector. + *

+ * Servers call the {@link #forServer()} method to configure server-style + * properties and the server metrics. Servers must also add + * {@code org.apache.druid.initialization.ExtensionsModule} which is + * not visible here, and can't be added in the {@link #forServer()} + * method. + *

+ * Tests and clients must provide + * properties via another mechanism. + */ +public class StartupInjectorBuilder extends BaseInjectorBuilder +{ + public StartupInjectorBuilder() + { + add( + new DruidGuiceExtensions(), + new JacksonModule(), + new ConfigModule(), + new NullHandlingModule(), + new ExpressionProcessingModule(), + binder -> { + binder.bind(DruidSecondaryModule.class); + } + ); + } + + public StartupInjectorBuilder withProperties(Properties properties) + { + add(binder -> { + binder.bind(Properties.class).toInstance(properties); + }); + return this; + } + + public StartupInjectorBuilder withEmptyProperties() + { + return withProperties(new Properties()); + } + + public StartupInjectorBuilder withExtensions() + { + add(new ExtensionsModule()); + return this; + } + + public StartupInjectorBuilder forServer() + { + withExtensions(); + add( + new PropertiesModule(Arrays.asList("common.runtime.properties", "runtime.properties")), + new RuntimeInfoModule() + ); + return this; + } +} diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorFactory.java b/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorFactory.java index 9056423832d6..12d7fbe1c1a5 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/AggregatorFactory.java @@ -267,7 +267,6 @@ public ColumnType getResultType() return ColumnTypeFactory.ofValueType(finalized); } - /** * This method is deprecated and will be removed soon. Use {@link #getIntermediateType()} instead. Do not call this * method, it will likely produce incorrect results, it exists for backwards compatibility. diff --git a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByMergingQueryRunnerV2.java b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByMergingQueryRunnerV2.java index 16e3dd3ac7ce..4f3bb6a645a2 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByMergingQueryRunnerV2.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByMergingQueryRunnerV2.java @@ -390,5 +390,4 @@ private void waitForFutureCompletion( throw new RuntimeException(e); } } - } diff --git a/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java b/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java new file mode 100644 index 000000000000..ca9fc1cc586e --- /dev/null +++ b/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java @@ -0,0 +1,332 @@ +/* + * 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.druid.guice; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Sets; +import com.google.inject.Injector; +import org.apache.druid.initialization.DruidModule; +import org.apache.druid.java.util.common.ISE; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Arrays; +import java.util.Collection; +import java.util.Comparator; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +public class ExtensionsLoaderTest +{ + @Rule + public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private Injector startupInjector() + { + return new StartupInjectorBuilder() + .withEmptyProperties() + .withExtensions() + .build(); + } + + @Test + public void test02MakeStartupInjector() + { + Injector startupInjector = startupInjector(); + Assert.assertNotNull(startupInjector); + Assert.assertNotNull(startupInjector.getInstance(ObjectMapper.class)); + ExtensionsLoader extnLoader = ExtensionsLoader.instance(startupInjector); + Assert.assertNotNull(extnLoader); + Assert.assertSame(extnLoader, ExtensionsLoader.instance(startupInjector)); + } + + + @Test + public void test04DuplicateClassLoaderExtensions() throws Exception + { + final File extensionDir = temporaryFolder.newFolder(); + Injector startupInjector = startupInjector(); + ExtensionsLoader extnLoader = ExtensionsLoader.instance(startupInjector); + + extnLoader.getLoadersMap() + .put(extensionDir, new URLClassLoader(new URL[]{}, ExtensionsLoader.class.getClassLoader())); + + Collection modules = extnLoader.getFromExtensions(DruidModule.class); + + Set loadedModuleNames = new HashSet<>(); + for (DruidModule module : modules) { + Assert.assertFalse("Duplicate extensions are loaded", loadedModuleNames.contains(module.getClass().getName())); + loadedModuleNames.add(module.getClass().getName()); + } + } + + @Test + public void test06GetClassLoaderForExtension() throws IOException + { + final ExtensionsLoader extnLoader = new ExtensionsLoader(new ExtensionsConfig()); + + final File some_extension_dir = temporaryFolder.newFolder(); + final File a_jar = new File(some_extension_dir, "a.jar"); + final File b_jar = new File(some_extension_dir, "b.jar"); + final File c_jar = new File(some_extension_dir, "c.jar"); + a_jar.createNewFile(); + b_jar.createNewFile(); + c_jar.createNewFile(); + final URLClassLoader loader = extnLoader.getClassLoaderForExtension(some_extension_dir, false); + final URL[] expectedURLs = new URL[]{a_jar.toURI().toURL(), b_jar.toURI().toURL(), c_jar.toURI().toURL()}; + final URL[] actualURLs = loader.getURLs(); + Arrays.sort(actualURLs, Comparator.comparing(URL::getPath)); + Assert.assertArrayEquals(expectedURLs, actualURLs); + } + + @Test + public void testGetLoadedModules() + { + final ExtensionsLoader extnLoader = new ExtensionsLoader(new ExtensionsConfig()); + Collection modules = extnLoader.getModules(); + HashSet moduleSet = new HashSet<>(modules); + + Collection loadedModules = extnLoader.getModules(); + Assert.assertEquals("Set from loaded modules #1 should be same!", modules.size(), loadedModules.size()); + Assert.assertEquals("Set from loaded modules #1 should be same!", moduleSet, new HashSet<>(loadedModules)); + + Collection loadedModules2 = extnLoader.getModules(); + Assert.assertEquals("Set from loaded modules #2 should be same!", modules.size(), loadedModules2.size()); + Assert.assertEquals("Set from loaded modules #2 should be same!", moduleSet, new HashSet<>(loadedModules2)); + } + + @Test + public void testGetExtensionFilesToLoad_non_exist_extensions_dir() throws IOException + { + final File tmpDir = temporaryFolder.newFolder(); + Assert.assertTrue("could not create missing folder", !tmpDir.exists() || tmpDir.delete()); + final ExtensionsLoader extnLoader = new ExtensionsLoader(new ExtensionsConfig() + { + @Override + public String getDirectory() + { + return tmpDir.getAbsolutePath(); + } + }); + Assert.assertArrayEquals( + "Non-exist root extensionsDir should return an empty array of File", + new File[]{}, + extnLoader.getExtensionFilesToLoad() + ); + } + + + @Test(expected = ISE.class) + public void testGetExtensionFilesToLoad_wrong_type_extensions_dir() throws IOException + { + final File extensionsDir = temporaryFolder.newFile(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public String getDirectory() + { + return extensionsDir.getAbsolutePath(); + } + }; + final ExtensionsLoader extnLoader = new ExtensionsLoader(config); + extnLoader.getExtensionFilesToLoad(); + } + + @Test + public void testGetExtensionFilesToLoad_empty_extensions_dir() throws IOException + { + final File extensionsDir = temporaryFolder.newFolder(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public String getDirectory() + { + return extensionsDir.getAbsolutePath(); + } + }; + + final ExtensionsLoader extnLoader = new ExtensionsLoader(config); + Assert.assertArrayEquals( + "Empty root extensionsDir should return an empty array of File", + new File[]{}, + extnLoader.getExtensionFilesToLoad() + ); + } + + /** + * If druid.extension.load is not specified, Initialization.getExtensionFilesToLoad is supposed to return all the + * extension folders under root extensions directory. + */ + @Test + public void testGetExtensionFilesToLoad_null_load_list() throws IOException + { + final File extensionsDir = temporaryFolder.newFolder(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public String getDirectory() + { + return extensionsDir.getAbsolutePath(); + } + }; + final ExtensionsLoader extnLoader = new ExtensionsLoader(config); + final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage"); + mysql_metadata_storage.mkdir(); + + final File[] expectedFileList = new File[]{mysql_metadata_storage}; + final File[] actualFileList = extnLoader.getExtensionFilesToLoad(); + Arrays.sort(actualFileList); + Assert.assertArrayEquals(expectedFileList, actualFileList); + } + + /** + * druid.extension.load is specified, Initialization.getExtensionFilesToLoad is supposed to return all the extension + * folders appeared in the load list. + */ + @Test + public void testGetExtensionFilesToLoad_with_load_list() throws IOException + { + final File extensionsDir = temporaryFolder.newFolder(); + + final File absolutePathExtension = temporaryFolder.newFolder(); + + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public LinkedHashSet getLoadList() + { + return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath())); + } + + @Override + public String getDirectory() + { + return extensionsDir.getAbsolutePath(); + } + }; + final ExtensionsLoader extnLoader = new ExtensionsLoader(config); + final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage"); + final File random_extension = new File(extensionsDir, "random-extensions"); + + mysql_metadata_storage.mkdir(); + random_extension.mkdir(); + + final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension}; + final File[] actualFileList = extnLoader.getExtensionFilesToLoad(); + Assert.assertArrayEquals(expectedFileList, actualFileList); + } + + /** + * druid.extension.load is specified, but contains an extension that is not prepared under root extension directory. + * Initialization.getExtensionFilesToLoad is supposed to throw ISE. + */ + @Test(expected = ISE.class) + public void testGetExtensionFilesToLoad_with_non_exist_item_in_load_list() throws IOException + { + final File extensionsDir = temporaryFolder.newFolder(); + final ExtensionsConfig config = new ExtensionsConfig() + { + @Override + public LinkedHashSet getLoadList() + { + return Sets.newLinkedHashSet(ImmutableList.of("mysql-metadata-storage")); + } + + @Override + public String getDirectory() + { + return extensionsDir.getAbsolutePath(); + } + }; + final File random_extension = new File(extensionsDir, "random-extensions"); + random_extension.mkdir(); + final ExtensionsLoader extnLoader = new ExtensionsLoader(config); + extnLoader.getExtensionFilesToLoad(); + } + + @Test + public void testGetURLsForClasspath() throws Exception + { + File tmpDir1 = temporaryFolder.newFolder(); + File tmpDir2 = temporaryFolder.newFolder(); + File tmpDir3 = temporaryFolder.newFolder(); + + File tmpDir1a = new File(tmpDir1, "a.jar"); + tmpDir1a.createNewFile(); + File tmpDir1b = new File(tmpDir1, "b.jar"); + tmpDir1b.createNewFile(); + new File(tmpDir1, "note1.txt").createNewFile(); + + File tmpDir2c = new File(tmpDir2, "c.jar"); + tmpDir2c.createNewFile(); + File tmpDir2d = new File(tmpDir2, "d.jar"); + tmpDir2d.createNewFile(); + File tmpDir2e = new File(tmpDir2, "e.JAR"); + tmpDir2e.createNewFile(); + new File(tmpDir2, "note2.txt").createNewFile(); + + String cp = tmpDir1.getAbsolutePath() + File.separator + "*" + + File.pathSeparator + + tmpDir3.getAbsolutePath() + + File.pathSeparator + + tmpDir2.getAbsolutePath() + File.separator + "*"; + + // getURLsForClasspath uses listFiles which does NOT guarantee any ordering for the name strings. + List urLsForClasspath = ExtensionsLoader.getURLsForClasspath(cp); + Assert.assertEquals(Sets.newHashSet(tmpDir1a.toURI().toURL(), tmpDir1b.toURI().toURL()), + Sets.newHashSet(urLsForClasspath.subList(0, 2))); + Assert.assertEquals(tmpDir3.toURI().toURL(), urLsForClasspath.get(2)); + Assert.assertEquals(Sets.newHashSet(tmpDir2c.toURI().toURL(), tmpDir2d.toURI().toURL(), tmpDir2e.toURI().toURL()), + Sets.newHashSet(urLsForClasspath.subList(3, 6))); + } + + @Test + public void testExtensionsWithSameDirName() throws Exception + { + final String extensionName = "some_extension"; + final File tmpDir1 = temporaryFolder.newFolder(); + final File tmpDir2 = temporaryFolder.newFolder(); + final File extension1 = new File(tmpDir1, extensionName); + final File extension2 = new File(tmpDir2, extensionName); + Assert.assertTrue(extension1.mkdir()); + Assert.assertTrue(extension2.mkdir()); + final File jar1 = new File(extension1, "jar1.jar"); + final File jar2 = new File(extension2, "jar2.jar"); + + Assert.assertTrue(jar1.createNewFile()); + Assert.assertTrue(jar2.createNewFile()); + + final ExtensionsLoader extnLoader = new ExtensionsLoader(new ExtensionsConfig()); + final ClassLoader classLoader1 = extnLoader.getClassLoaderForExtension(extension1, false); + final ClassLoader classLoader2 = extnLoader.getClassLoaderForExtension(extension2, false); + + Assert.assertArrayEquals(new URL[]{jar1.toURI().toURL()}, ((URLClassLoader) classLoader1).getURLs()); + Assert.assertArrayEquals(new URL[]{jar2.toURI().toURL()}, ((URLClassLoader) classLoader2).getURLs()); + } +} diff --git a/server/pom.xml b/server/pom.xml index febee7035d3f..6615931ef5c5 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -48,7 +48,7 @@ druid-gcp-common ${project.parent.version} runtime - + org.apache.druid @@ -304,10 +304,6 @@ commons-lang commons-lang - - org.eclipse.aether - aether-api - org.slf4j slf4j-api diff --git a/server/src/main/java/org/apache/druid/guice/DruidInjectorBuilder.java b/server/src/main/java/org/apache/druid/guice/DruidInjectorBuilder.java new file mode 100644 index 000000000000..b9dcb7f30531 --- /dev/null +++ b/server/src/main/java/org/apache/druid/guice/DruidInjectorBuilder.java @@ -0,0 +1,203 @@ +/* + * 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.druid.guice; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.Module; +import org.apache.druid.discovery.NodeRole; +import org.apache.druid.guice.annotations.Json; +import org.apache.druid.guice.annotations.LoadScope; +import org.apache.druid.guice.annotations.Smile; +import org.apache.druid.initialization.DruidModule; +import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.logger.Logger; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Druid-enabled injector builder which supports {@link DruidModule}s, module classes + * created from the base injector, and filtering based on properties and {@link LoadScope} + * annotations. + *

+ * Can be used in clients and tests, in which case no module filtering is done. + * Presumably, the test or client has already selected the modules that it needs. + *

+ * Druid injector builders can be chained with an earlier builder providing a set of + * modules which a later builder overrides. Again, this is typically used only in the + * server, not in clients or tests. + */ +public class DruidInjectorBuilder +{ + private static final Logger log = new Logger(DruidInjectorBuilder.class); + + private final List modules = new ArrayList<>(); + protected final Injector baseInjector; + private final ObjectMapper jsonMapper; + private final ObjectMapper smileMapper; + private final Set nodeRoles; + private final ModulesConfig modulesConfig; + + public DruidInjectorBuilder(final Injector baseInjector) + { + this(baseInjector, Collections.emptySet()); + } + + public DruidInjectorBuilder(final Injector baseInjector, final Set nodeRoles) + { + this.baseInjector = baseInjector; + this.nodeRoles = nodeRoles; + this.modulesConfig = baseInjector.getInstance(ModulesConfig.class); + this.jsonMapper = baseInjector.getInstance(Key.get(ObjectMapper.class, Json.class)); + this.smileMapper = baseInjector.getInstance(Key.get(ObjectMapper.class, Smile.class)); + } + + public DruidInjectorBuilder(final DruidInjectorBuilder from) + { + this.baseInjector = from.baseInjector; + this.nodeRoles = from.nodeRoles; + this.modulesConfig = from.modulesConfig; + this.jsonMapper = from.jsonMapper; + this.smileMapper = from.smileMapper; + } + + /** + * Add an arbitrary set of modules. + * + * @see #add(Object) + */ + public DruidInjectorBuilder add(Object...input) + { + for (Object o : input) { + add(o); + } + return this; + } + + /** + * Add an arbitrary {@link Module}, {@link DruidModule} instance, + * or a subclass of these classes. If a class is provided, it is instantiated + * using the base injector to allow dependency injection. If a module + * instance is provided, its members are injected. Note that such + * modules have visibility only to objects defined in the base + * injector, but not to objects defined in the injector being built. + */ + public DruidInjectorBuilder add(Object input) + { + if (input instanceof DruidModule) { + return addDruidModule((DruidModule) input); + } else if (input instanceof Module) { + return addModule((Module) input); + } else if (input instanceof Class) { + return addClass((Class) input); + } else { + throw new ISE("Unknown module type[%s]", input.getClass()); + } + } + + public DruidInjectorBuilder addDruidModule(DruidModule module) + { + if (!acceptModule(module.getClass())) { + return this; + } + baseInjector.injectMembers(module); + registerJacksonModules(module); + modules.add(module); + return this; + } + + public DruidInjectorBuilder addModule(Module module) + { + if (!acceptModule(module.getClass())) { + return this; + } + baseInjector.injectMembers(module); + modules.add(module); + return this; + } + + public DruidInjectorBuilder addClass(Class input) + { + if (!acceptModule((Class) input)) { + return this; + } + if (DruidModule.class.isAssignableFrom(input)) { + @SuppressWarnings("unchecked") + DruidModule module = baseInjector.getInstance((Class) input); + registerJacksonModules(module); + modules.add(module); + } else if (Module.class.isAssignableFrom(input)) { + @SuppressWarnings("unchecked") + Module module = baseInjector.getInstance((Class) input); + modules.add(module); + } else { + throw new ISE("Class [%s] does not implement %s", input, Module.class); + } + return this; + } + + /** + * Filter module classes based on the (optional) module exclude list and + * (optional) set of known node roles. + */ + private boolean acceptModule(Class moduleClass) + { + // Modules config is optional: it won't be present in tests or clients. + String moduleClassName = moduleClass.getName(); + if (moduleClassName != null && modulesConfig.getExcludeList().contains(moduleClassName)) { + log.info("Not loading module [%s] because it is present in excludeList", moduleClassName); + return false; + } + LoadScope loadScope = moduleClass.getAnnotation(LoadScope.class); + if (loadScope == null) { + // always load if annotation is not specified + return true; + } + Set rolesPredicate = Arrays.stream(loadScope.roles()) + .map(NodeRole::fromJsonName) + .collect(Collectors.toSet()); + return rolesPredicate.stream().anyMatch(nodeRoles::contains); + } + + private void registerJacksonModules(DruidModule module) + { + for (com.fasterxml.jackson.databind.Module jacksonModule : module.getJacksonModules()) { + jsonMapper.registerModule(jacksonModule); + smileMapper.registerModule(jacksonModule); + } + } + + public List modules() + { + return modules; + } + + public Injector build() + { + return Guice.createInjector(modules); + } +} diff --git a/server/src/main/java/org/apache/druid/guice/ExpressionModule.java b/server/src/main/java/org/apache/druid/guice/ExpressionModule.java index fa096de010bd..ffc98c99c291 100644 --- a/server/src/main/java/org/apache/druid/guice/ExpressionModule.java +++ b/server/src/main/java/org/apache/druid/guice/ExpressionModule.java @@ -19,11 +19,10 @@ package org.apache.druid.guice; -import com.fasterxml.jackson.databind.Module; import com.google.common.collect.ImmutableList; import com.google.inject.Binder; +import com.google.inject.Module; import com.google.inject.multibindings.Multibinder; -import org.apache.druid.initialization.DruidModule; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.expression.CaseInsensitiveContainsExprMacro; import org.apache.druid.query.expression.ContainsExprMacro; @@ -46,7 +45,7 @@ import java.util.List; -public class ExpressionModule implements DruidModule +public class ExpressionModule implements Module { public static final List> EXPR_MACROS = ImmutableList.>builder() @@ -94,12 +93,6 @@ public void configure(Binder binder) } } - @Override - public List getJacksonModules() - { - return ImmutableList.of(); - } - public static void addExprMacro(final Binder binder, final Class clazz) { Multibinder.newSetBinder(binder, ExprMacroTable.ExprMacro.class) diff --git a/server/src/main/java/org/apache/druid/initialization/CoreInjectorBuilder.java b/server/src/main/java/org/apache/druid/initialization/CoreInjectorBuilder.java new file mode 100644 index 000000000000..0497229b3b95 --- /dev/null +++ b/server/src/main/java/org/apache/druid/initialization/CoreInjectorBuilder.java @@ -0,0 +1,142 @@ +/* + * 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.druid.initialization; + +import com.google.inject.Injector; +import org.apache.druid.curator.CuratorModule; +import org.apache.druid.curator.discovery.DiscoveryModule; +import org.apache.druid.discovery.NodeRole; +import org.apache.druid.guice.AnnouncerModule; +import org.apache.druid.guice.CoordinatorDiscoveryModule; +import org.apache.druid.guice.DruidInjectorBuilder; +import org.apache.druid.guice.DruidProcessingConfigModule; +import org.apache.druid.guice.DruidSecondaryModule; +import org.apache.druid.guice.ExpressionModule; +import org.apache.druid.guice.ExtensionsModule; +import org.apache.druid.guice.FirehoseModule; +import org.apache.druid.guice.IndexingServiceDiscoveryModule; +import org.apache.druid.guice.JacksonConfigManagerModule; +import org.apache.druid.guice.JavaScriptModule; +import org.apache.druid.guice.LifecycleModule; +import org.apache.druid.guice.LocalDataStorageDruidModule; +import org.apache.druid.guice.MetadataConfigModule; +import org.apache.druid.guice.NestedDataModule; +import org.apache.druid.guice.ServerModule; +import org.apache.druid.guice.ServerViewModule; +import org.apache.druid.guice.StartupLoggingModule; +import org.apache.druid.guice.StorageNodeModule; +import org.apache.druid.guice.annotations.Client; +import org.apache.druid.guice.annotations.EscalatedClient; +import org.apache.druid.guice.http.HttpClientModule; +import org.apache.druid.guice.security.AuthenticatorModule; +import org.apache.druid.guice.security.AuthorizerModule; +import org.apache.druid.guice.security.DruidAuthModule; +import org.apache.druid.guice.security.EscalatorModule; +import org.apache.druid.metadata.storage.derby.DerbyMetadataStorageDruidModule; +import org.apache.druid.rpc.guice.ServiceClientModule; +import org.apache.druid.segment.writeout.SegmentWriteOutMediumModule; +import org.apache.druid.server.emitter.EmitterModule; +import org.apache.druid.server.initialization.AuthenticatorMapperModule; +import org.apache.druid.server.initialization.AuthorizerMapperModule; +import org.apache.druid.server.initialization.ExternalStorageAccessSecurityModule; +import org.apache.druid.server.initialization.jetty.JettyServerModule; +import org.apache.druid.server.metrics.MetricsModule; +import org.apache.druid.server.security.TLSCertificateCheckerModule; + +import java.util.Collections; +import java.util.Set; + +/** + * Builds the core (common) set of modules used by all Druid services and + * commands. The basic injector just adds logging and the Druid lifecycle. + * Call {@link #forServer()} to add the server-specific modules. + */ +public class CoreInjectorBuilder extends DruidInjectorBuilder +{ + public CoreInjectorBuilder(final Injector baseInjector) + { + this(baseInjector, Collections.emptySet()); + } + + public CoreInjectorBuilder(final Injector baseInjector, final Set nodeRoles) + { + super(baseInjector, nodeRoles); + add(DruidSecondaryModule.class); + } + + public CoreInjectorBuilder withLogging() + { + // New modules should be added after Log4jShutterDownerModule + add(new Log4jShutterDownerModule()); + return this; + } + + public CoreInjectorBuilder withLifecycle() + { + add(new LifecycleModule()); + return this; + } + + public CoreInjectorBuilder forServer() + { + withLogging(); + withLifecycle(); + add( + ExtensionsModule.SecondaryModule.class, + new DruidAuthModule(), + TLSCertificateCheckerModule.class, + EmitterModule.class, + HttpClientModule.global(), + HttpClientModule.escalatedGlobal(), + new HttpClientModule("druid.broker.http", Client.class), + new HttpClientModule("druid.broker.http", EscalatedClient.class), + new CuratorModule(), + new AnnouncerModule(), + new MetricsModule(), + new SegmentWriteOutMediumModule(), + new ServerModule(), + new DruidProcessingConfigModule(), + new StorageNodeModule(), + new JettyServerModule(), + new ExpressionModule(), + new NestedDataModule(), + new DiscoveryModule(), + new ServerViewModule(), + new MetadataConfigModule(), + new DerbyMetadataStorageDruidModule(), + new JacksonConfigManagerModule(), + new IndexingServiceDiscoveryModule(), + new CoordinatorDiscoveryModule(), + new LocalDataStorageDruidModule(), + new TombstoneDataStorageModule(), + new FirehoseModule(), + new JavaScriptModule(), + new AuthenticatorModule(), + new AuthenticatorMapperModule(), + new EscalatorModule(), + new AuthorizerModule(), + new AuthorizerMapperModule(), + new StartupLoggingModule(), + new ExternalStorageAccessSecurityModule(), + new ServiceClientModule() + ); + return this; + } +} diff --git a/server/src/main/java/org/apache/druid/initialization/ExtensionInjectorBuilder.java b/server/src/main/java/org/apache/druid/initialization/ExtensionInjectorBuilder.java new file mode 100644 index 000000000000..f030f62bc723 --- /dev/null +++ b/server/src/main/java/org/apache/druid/initialization/ExtensionInjectorBuilder.java @@ -0,0 +1,51 @@ +/* + * 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.druid.initialization; + +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.util.Modules; +import org.apache.druid.guice.DruidInjectorBuilder; +import org.apache.druid.guice.ExtensionsLoader; + +/** + * Injector builder which overrides service modules with extension + * modules. Used only in the server, not in clients or tests. + */ +public class ExtensionInjectorBuilder extends DruidInjectorBuilder +{ + private final ServiceInjectorBuilder serviceBuilder; + + public ExtensionInjectorBuilder(ServiceInjectorBuilder serviceBuilder) + { + super(serviceBuilder); + this.serviceBuilder = serviceBuilder; + ExtensionsLoader extnLoader = ExtensionsLoader.instance(baseInjector); + for (DruidModule module : extnLoader.getFromExtensions(DruidModule.class)) { + addDruidModule(module); + } + } + + @Override + public Injector build() + { + return Guice.createInjector(Modules.override(serviceBuilder.merge()).with(modules())); + } +} diff --git a/server/src/main/java/org/apache/druid/initialization/Initialization.java b/server/src/main/java/org/apache/druid/initialization/Initialization.java index 3c0dac061ca7..bd54efa7fbb2 100644 --- a/server/src/main/java/org/apache/druid/initialization/Initialization.java +++ b/server/src/main/java/org/apache/druid/initialization/Initialization.java @@ -19,536 +19,24 @@ package org.apache.druid.initialization; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableSet; -import com.google.inject.Guice; import com.google.inject.Injector; -import com.google.inject.Key; import com.google.inject.Module; -import com.google.inject.util.Modules; -import org.apache.commons.io.FileUtils; -import org.apache.druid.curator.CuratorModule; -import org.apache.druid.curator.discovery.DiscoveryModule; -import org.apache.druid.discovery.NodeRole; -import org.apache.druid.guice.AnnouncerModule; -import org.apache.druid.guice.CoordinatorDiscoveryModule; -import org.apache.druid.guice.DruidProcessingConfigModule; -import org.apache.druid.guice.DruidSecondaryModule; -import org.apache.druid.guice.ExpressionModule; -import org.apache.druid.guice.ExtensionsConfig; -import org.apache.druid.guice.FirehoseModule; -import org.apache.druid.guice.IndexingServiceDiscoveryModule; -import org.apache.druid.guice.JacksonConfigManagerModule; -import org.apache.druid.guice.JavaScriptModule; -import org.apache.druid.guice.LifecycleModule; -import org.apache.druid.guice.LocalDataStorageDruidModule; -import org.apache.druid.guice.MetadataConfigModule; -import org.apache.druid.guice.ModulesConfig; -import org.apache.druid.guice.NestedDataModule; -import org.apache.druid.guice.ServerModule; -import org.apache.druid.guice.ServerViewModule; -import org.apache.druid.guice.StartupLoggingModule; -import org.apache.druid.guice.StorageNodeModule; -import org.apache.druid.guice.annotations.Client; -import org.apache.druid.guice.annotations.EscalatedClient; -import org.apache.druid.guice.annotations.Json; -import org.apache.druid.guice.annotations.LoadScope; -import org.apache.druid.guice.annotations.Smile; -import org.apache.druid.guice.http.HttpClientModule; -import org.apache.druid.guice.security.AuthenticatorModule; -import org.apache.druid.guice.security.AuthorizerModule; -import org.apache.druid.guice.security.DruidAuthModule; -import org.apache.druid.guice.security.EscalatorModule; -import org.apache.druid.java.util.common.ISE; -import org.apache.druid.java.util.common.logger.Logger; -import org.apache.druid.metadata.storage.derby.DerbyMetadataStorageDruidModule; -import org.apache.druid.rpc.guice.ServiceClientModule; -import org.apache.druid.segment.writeout.SegmentWriteOutMediumModule; -import org.apache.druid.server.emitter.EmitterModule; -import org.apache.druid.server.initialization.AuthenticatorMapperModule; -import org.apache.druid.server.initialization.AuthorizerMapperModule; -import org.apache.druid.server.initialization.ExternalStorageAccessSecurityModule; -import org.apache.druid.server.initialization.jetty.JettyServerModule; -import org.apache.druid.server.metrics.MetricsModule; -import org.apache.druid.server.security.TLSCertificateCheckerModule; -import org.eclipse.aether.artifact.DefaultArtifact; - -import java.io.File; -import java.io.FilenameFilter; -import java.io.IOException; -import java.net.MalformedURLException; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.ServiceLoader; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Collectors; /** - * + * Initialize Guice for a server. Clients and tests should use + * the individual builders to create a non-server environment. */ public class Initialization { - private static final Logger log = new Logger(Initialization.class); - private static final ConcurrentHashMap LOADERS_MAP = new ConcurrentHashMap<>(); - - private static final ConcurrentHashMap, Collection> EXTENSIONS_MAP = new ConcurrentHashMap<>(); - - /** - * @param clazz service class - * @param the service type - * - * @return Returns a collection of implementations loaded. - */ - public static Collection getLoadedImplementations(Class clazz) - { - @SuppressWarnings("unchecked") - Collection retVal = (Collection) EXTENSIONS_MAP.get(clazz); - if (retVal == null) { - return new HashSet<>(); - } - return retVal; - } - - @VisibleForTesting - static void clearLoadedImplementations() - { - EXTENSIONS_MAP.clear(); - } - - @VisibleForTesting - static Map getLoadersMap() - { - return LOADERS_MAP; - } - - /** - * Look for implementations for the given class from both classpath and extensions directory, using {@link - * ServiceLoader}. A user should never put the same two extensions in classpath and extensions directory, if he/she - * does that, the one that is in the classpath will be loaded, the other will be ignored. - * - * @param config Extensions configuration - * @param serviceClass The class to look the implementations of (e.g., DruidModule) - * - * @return A collection that contains implementations (of distinct concrete classes) of the given class. The order of - * elements in the returned collection is not specified and not guaranteed to be the same for different calls to - * getFromExtensions(). - */ - public static Collection getFromExtensions(ExtensionsConfig config, Class serviceClass) - { - // It's not clear whether we should recompute modules even if they have been computed already for the serviceClass, - // but that's how it used to be an preserving the old behaviour here. - Collection modules = EXTENSIONS_MAP.compute( - serviceClass, - (serviceC, ignored) -> new ServiceLoadingFromExtensions<>(config, serviceC).implsToLoad - ); - //noinspection unchecked - return (Collection) modules; - } - - private static class ServiceLoadingFromExtensions - { - private final ExtensionsConfig extensionsConfig; - private final Class serviceClass; - private final List implsToLoad = new ArrayList<>(); - private final Set implClassNamesToLoad = new HashSet<>(); - - private ServiceLoadingFromExtensions(ExtensionsConfig extensionsConfig, Class serviceClass) - { - this.extensionsConfig = extensionsConfig; - this.serviceClass = serviceClass; - if (extensionsConfig.searchCurrentClassloader()) { - addAllFromCurrentClassLoader(); - } - addAllFromFileSystem(); - } - - private void addAllFromCurrentClassLoader() - { - ServiceLoader - .load(serviceClass, Thread.currentThread().getContextClassLoader()) - .forEach(impl -> tryAdd(impl, "classpath")); - } - - private void addAllFromFileSystem() - { - for (File extension : getExtensionFilesToLoad(extensionsConfig)) { - log.debug("Loading extension [%s] for class [%s]", extension.getName(), serviceClass); - try { - final URLClassLoader loader = getClassLoaderForExtension( - extension, - extensionsConfig.isUseExtensionClassloaderFirst() - ); - - log.info( - "Loading extension [%s], jars: %s", - extension.getName(), - Arrays.stream(loader.getURLs()) - .map(u -> new File(u.getPath()).getName()) - .collect(Collectors.joining(", ")) - ); - - ServiceLoader.load(serviceClass, loader).forEach(impl -> tryAdd(impl, "local file system")); - } - catch (Exception e) { - throw new RuntimeException(e); - } - } - } - - private void tryAdd(T serviceImpl, String extensionType) - { - final String serviceImplName = serviceImpl.getClass().getName(); - if (serviceImplName == null) { - log.warn( - "Implementation [%s] was ignored because it doesn't have a canonical name, " - + "is it a local or anonymous class?", - serviceImpl.getClass().getName() - ); - } else if (!implClassNamesToLoad.contains(serviceImplName)) { - log.debug( - "Adding implementation [%s] for class [%s] from %s extension", - serviceImplName, - serviceClass, - extensionType - ); - implClassNamesToLoad.add(serviceImplName); - implsToLoad.add(serviceImpl); - } - } - } - - /** - * Find all the extension files that should be loaded by druid. - *

- * If user explicitly specifies druid.extensions.loadList, then it will look for those extensions under root - * extensions directory. If one of them is not found, druid will fail loudly. - *

- * If user doesn't specify druid.extension.toLoad (or its value is empty), druid will load all the extensions - * under the root extensions directory. - * - * @param config ExtensionsConfig configured by druid.extensions.xxx - * - * @return an array of druid extension files that will be loaded by druid process - */ - public static File[] getExtensionFilesToLoad(ExtensionsConfig config) - { - final File rootExtensionsDir = new File(config.getDirectory()); - if (rootExtensionsDir.exists() && !rootExtensionsDir.isDirectory()) { - throw new ISE("Root extensions directory [%s] is not a directory!?", rootExtensionsDir); - } - File[] extensionsToLoad; - final LinkedHashSet toLoad = config.getLoadList(); - if (toLoad == null) { - extensionsToLoad = rootExtensionsDir.listFiles(); - } else { - int i = 0; - extensionsToLoad = new File[toLoad.size()]; - for (final String extensionName : toLoad) { - File extensionDir = new File(extensionName); - if (!extensionDir.isAbsolute()) { - extensionDir = new File(rootExtensionsDir, extensionName); - } - - if (!extensionDir.isDirectory()) { - throw new ISE( - "Extension [%s] specified in \"druid.extensions.loadList\" didn't exist!?", - extensionDir.getAbsolutePath() - ); - } - extensionsToLoad[i++] = extensionDir; - } - } - return extensionsToLoad == null ? new File[]{} : extensionsToLoad; - } - - /** - * Find all the hadoop dependencies that should be loaded by druid - * - * @param hadoopDependencyCoordinates e.g.["org.apache.hadoop:hadoop-client:2.3.0"] - * @param extensionsConfig ExtensionsConfig configured by druid.extensions.xxx - * - * @return an array of hadoop dependency files that will be loaded by druid process - */ - public static File[] getHadoopDependencyFilesToLoad( - List hadoopDependencyCoordinates, - ExtensionsConfig extensionsConfig - ) - { - final File rootHadoopDependenciesDir = new File(extensionsConfig.getHadoopDependenciesDir()); - if (rootHadoopDependenciesDir.exists() && !rootHadoopDependenciesDir.isDirectory()) { - throw new ISE("Root Hadoop dependencies directory [%s] is not a directory!?", rootHadoopDependenciesDir); - } - final File[] hadoopDependenciesToLoad = new File[hadoopDependencyCoordinates.size()]; - int i = 0; - for (final String coordinate : hadoopDependencyCoordinates) { - final DefaultArtifact artifact = new DefaultArtifact(coordinate); - final File hadoopDependencyDir = new File(rootHadoopDependenciesDir, artifact.getArtifactId()); - final File versionDir = new File(hadoopDependencyDir, artifact.getVersion()); - // find the hadoop dependency with the version specified in coordinate - if (!hadoopDependencyDir.isDirectory() || !versionDir.isDirectory()) { - throw new ISE("Hadoop dependency [%s] didn't exist!?", versionDir.getAbsolutePath()); - } - hadoopDependenciesToLoad[i++] = versionDir; - } - return hadoopDependenciesToLoad; - } - - /** - * @param extension The File instance of the extension we want to load - * - * @return a URLClassLoader that loads all the jars on which the extension is dependent - */ - public static URLClassLoader getClassLoaderForExtension(File extension, boolean useExtensionClassloaderFirst) - { - return LOADERS_MAP.computeIfAbsent( - extension, - theExtension -> makeClassLoaderForExtension(theExtension, useExtensionClassloaderFirst) - ); - } - - private static URLClassLoader makeClassLoaderForExtension( - final File extension, - final boolean useExtensionClassloaderFirst - ) - { - final Collection jars = FileUtils.listFiles(extension, new String[]{"jar"}, false); - final URL[] urls = new URL[jars.size()]; - - try { - int i = 0; - for (File jar : jars) { - final URL url = jar.toURI().toURL(); - log.debug("added URL[%s] for extension[%s]", url, extension.getName()); - urls[i++] = url; - } - } - catch (MalformedURLException e) { - throw new RuntimeException(e); - } - - if (useExtensionClassloaderFirst) { - return new ExtensionFirstClassLoader(urls, Initialization.class.getClassLoader()); - } else { - return new URLClassLoader(urls, Initialization.class.getClassLoader()); - } - } - - public static List getURLsForClasspath(String cp) - { - try { - String[] paths = cp.split(File.pathSeparator); - - List urls = new ArrayList<>(); - for (String path : paths) { - File f = new File(path); - if ("*".equals(f.getName())) { - File parentDir = f.getParentFile(); - if (parentDir.isDirectory()) { - File[] jars = parentDir.listFiles( - new FilenameFilter() - { - @Override - public boolean accept(File dir, String name) - { - return name != null && (name.endsWith(".jar") || name.endsWith(".JAR")); - } - } - ); - for (File jar : jars) { - urls.add(jar.toURI().toURL()); - } - } - } else { - urls.add(new File(path).toURI().toURL()); - } - } - return urls; - } - catch (IOException ex) { - throw new RuntimeException(ex); - } - } - - public static Injector makeInjectorWithModules( - final Injector baseInjector, - final Iterable modules - ) - { - return makeInjectorWithModules(ImmutableSet.of(), baseInjector, modules); - } - + // Use individual builders for testing: this method brings in + // server-only dependencies, which is generally not desired. + @Deprecated public static Injector makeInjectorWithModules( - final Set nodeRoles, final Injector baseInjector, final Iterable modules ) { - final ModuleList defaultModules = new ModuleList(baseInjector, nodeRoles); - defaultModules.addModules( - // New modules should be added after Log4jShutterDownerModule - new Log4jShutterDownerModule(), - new DruidAuthModule(), - new LifecycleModule(), - TLSCertificateCheckerModule.class, - EmitterModule.class, - HttpClientModule.global(), - HttpClientModule.escalatedGlobal(), - new HttpClientModule("druid.broker.http", Client.class), - new HttpClientModule("druid.broker.http", EscalatedClient.class), - new CuratorModule(), - new AnnouncerModule(), - new MetricsModule(), - new SegmentWriteOutMediumModule(), - new ServerModule(), - new DruidProcessingConfigModule(), - new StorageNodeModule(), - new JettyServerModule(), - new ExpressionModule(), - new NestedDataModule(), - new DiscoveryModule(), - new ServerViewModule(), - new MetadataConfigModule(), - new DerbyMetadataStorageDruidModule(), - new JacksonConfigManagerModule(), - new IndexingServiceDiscoveryModule(), - new CoordinatorDiscoveryModule(), - new LocalDataStorageDruidModule(), - new TombstoneDataStorageModule(), - new FirehoseModule(), - new JavaScriptModule(), - new AuthenticatorModule(), - new AuthenticatorMapperModule(), - new EscalatorModule(), - new AuthorizerModule(), - new AuthorizerMapperModule(), - new StartupLoggingModule(), - new ExternalStorageAccessSecurityModule(), - new ServiceClientModule() - ); - - ModuleList actualModules = new ModuleList(baseInjector, nodeRoles); - actualModules.addModule(DruidSecondaryModule.class); - for (Object module : modules) { - actualModules.addModule(module); - } - - Module intermediateModules = Modules.override(defaultModules.getModules()).with(actualModules.getModules()); - - ModuleList extensionModules = new ModuleList(baseInjector, nodeRoles); - final ExtensionsConfig config = baseInjector.getInstance(ExtensionsConfig.class); - for (DruidModule module : Initialization.getFromExtensions(config, DruidModule.class)) { - extensionModules.addModule(module); - } - - return Guice.createInjector(Modules.override(intermediateModules).with(extensionModules.getModules())); - } - - public static class ModuleList - { - private final Injector baseInjector; - private final Set nodeRoles; - private final ModulesConfig modulesConfig; - private final ObjectMapper jsonMapper; - private final ObjectMapper smileMapper; - private final List modules; - - public ModuleList(Injector baseInjector, Set nodeRoles) - { - this.baseInjector = baseInjector; - this.nodeRoles = nodeRoles; - this.modulesConfig = baseInjector.getInstance(ModulesConfig.class); - this.jsonMapper = baseInjector.getInstance(Key.get(ObjectMapper.class, Json.class)); - this.smileMapper = baseInjector.getInstance(Key.get(ObjectMapper.class, Smile.class)); - this.modules = new ArrayList<>(); - } - - public List getModules() - { - return Collections.unmodifiableList(modules); - } - - public void addModule(Object input) - { - if (!shouldLoadOnCurrentNodeType(input)) { - return; - } - - if (input instanceof DruidModule) { - if (!checkModuleClass(input.getClass())) { - return; - } - baseInjector.injectMembers(input); - modules.add(registerJacksonModules(((DruidModule) input))); - } else if (input instanceof Module) { - if (!checkModuleClass(input.getClass())) { - return; - } - baseInjector.injectMembers(input); - modules.add((Module) input); - } else if (input instanceof Class) { - if (!checkModuleClass((Class) input)) { - return; - } - if (DruidModule.class.isAssignableFrom((Class) input)) { - modules.add(registerJacksonModules(baseInjector.getInstance((Class) input))); - } else if (Module.class.isAssignableFrom((Class) input)) { - modules.add(baseInjector.getInstance((Class) input)); - return; - } else { - throw new ISE("Class[%s] does not implement %s", input.getClass(), Module.class); - } - } else { - throw new ISE("Unknown module type[%s]", input.getClass()); - } - } - - private boolean shouldLoadOnCurrentNodeType(Object object) - { - LoadScope loadScope = object.getClass().getAnnotation(LoadScope.class); - if (loadScope == null) { - // always load if annotation is not specified - return true; - } - Set rolesPredicate = Arrays.stream(loadScope.roles()) - .map(NodeRole::fromJsonName) - .collect(Collectors.toSet()); - return rolesPredicate.stream().anyMatch(nodeRoles::contains); - } - - private boolean checkModuleClass(Class moduleClass) - { - String moduleClassName = moduleClass.getName(); - if (moduleClassName != null && modulesConfig.getExcludeList().contains(moduleClassName)) { - log.info("Not loading module [%s] because it is present in excludeList", moduleClassName); - return false; - } - return true; - } - - public void addModules(Object... object) - { - for (Object o : object) { - addModule(o); - } - } - - private DruidModule registerJacksonModules(DruidModule module) - { - for (com.fasterxml.jackson.databind.Module jacksonModule : module.getJacksonModules()) { - jsonMapper.registerModule(jacksonModule); - smileMapper.registerModule(jacksonModule); - } - return module; - } + return ServerInjectorBuilder.makeServerInjector(baseInjector, ImmutableSet.of(), modules); } } diff --git a/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java b/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java new file mode 100644 index 000000000000..f8f0875008b8 --- /dev/null +++ b/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java @@ -0,0 +1,128 @@ +/* + * 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.druid.initialization; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; +import com.google.inject.Injector; +import com.google.inject.Module; +import com.google.inject.TypeLiteral; +import com.google.inject.multibindings.MapBinder; +import com.google.inject.multibindings.Multibinder; +import org.apache.druid.discovery.DruidService; +import org.apache.druid.discovery.NodeRole; +import org.apache.druid.guice.annotations.Self; + +import java.util.Set; + +/** + * Initialize Guice for a server. Clients and tests should use + * the individual builders to create a non-server environment. + */ +public class ServerInjectorBuilder +{ + private Injector baseInjector; + private Set nodeRoles; + private Iterable modules; + + /** + * Create a server injector. Located here for testing. Should only be + * used by {@code GuiceRunnable} (and tests). + * + * @param nodeRoles the roles which this server provides + * @param baseInjector the startup injector + * @param modules modules for this server + * @return the injector for the server + */ + // TODO: Move to the CLI package, perhaps GuiceRunnable, once + // tests are cleaned up. + @VisibleForTesting + public static Injector makeServerInjector( + final Injector baseInjector, + final Set nodeRoles, + final Iterable modules + ) + { + return new ServerInjectorBuilder(baseInjector) + .nodeRoles(nodeRoles) + .serviceModules(modules) + .build(); + } + + public ServerInjectorBuilder(Injector baseInjector) + { + this.baseInjector = baseInjector; + } + + public ServerInjectorBuilder nodeRoles(final Set nodeRoles) + { + this.nodeRoles = nodeRoles; + return this; + } + + public ServerInjectorBuilder serviceModules(final Iterable modules) + { + this.modules = modules; + return this; + } + + public Injector build() + { + Module registerNodeRoleModule = registerNodeRoleModule(nodeRoles); + + // Child injector, with the registered node roles + Injector childInjector = baseInjector.createChildInjector(registerNodeRoleModule); + + // Create the core set of modules shared by all services. + // Here and below, the modules are filtered by the load modules list and + // the set of roles which this server provides. + CoreInjectorBuilder coreBuilder = new CoreInjectorBuilder(childInjector, nodeRoles).forServer(); + + // Override with the per-service modules. + ServiceInjectorBuilder serviceBuilder = new ServiceInjectorBuilder(coreBuilder).addAll( + Iterables.concat( + // bind nodeRoles for the new injector as well + ImmutableList.of(registerNodeRoleModule), + modules + ) + ); + + // Override again with extensions. + return new ExtensionInjectorBuilder(serviceBuilder).build(); + } + + public static Module registerNodeRoleModule(Set nodeRoles) + { + if (nodeRoles.isEmpty()) { + return binder -> {}; + } + return binder -> { + Multibinder selfBinder = Multibinder.newSetBinder(binder, NodeRole.class, Self.class); + nodeRoles.forEach(nodeRole -> selfBinder.addBinding().toInstance(nodeRole)); + + MapBinder.newMapBinder( + binder, + new TypeLiteral(){}, + new TypeLiteral>>(){} + ); + }; + } +} diff --git a/server/src/main/java/org/apache/druid/initialization/ServiceInjectorBuilder.java b/server/src/main/java/org/apache/druid/initialization/ServiceInjectorBuilder.java new file mode 100644 index 000000000000..0b21f1031b44 --- /dev/null +++ b/server/src/main/java/org/apache/druid/initialization/ServiceInjectorBuilder.java @@ -0,0 +1,63 @@ +/* + * 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.druid.initialization; + +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Module; +import com.google.inject.util.Modules; +import org.apache.druid.guice.DruidInjectorBuilder; + +/** + * Injector builder for a service within a server. In the server, this builder + * is input to the {@link ExtensionInjectorBuilder}. Also used to build clients + * or tests, without extensions, where this builder itself builds the injector. + */ +public class ServiceInjectorBuilder extends DruidInjectorBuilder +{ + private final CoreInjectorBuilder coreBuilder; + + public ServiceInjectorBuilder( + final CoreInjectorBuilder coreBuilder + ) + { + super(coreBuilder); + this.coreBuilder = coreBuilder; + } + + public Module merge() + { + return Modules.override(coreBuilder.modules()).with(modules()); + } + + @Override + public Injector build() + { + return Guice.createInjector(merge()); + } + + public ServiceInjectorBuilder addAll(Iterable modules) + { + for (Module module : modules) { + add(module); + } + return this; + } +} diff --git a/server/src/main/java/org/apache/druid/server/StatusResource.java b/server/src/main/java/org/apache/druid/server/StatusResource.java index cf457f14ee3b..ed4c626a37ea 100644 --- a/server/src/main/java/org/apache/druid/server/StatusResource.java +++ b/server/src/main/java/org/apache/druid/server/StatusResource.java @@ -24,8 +24,8 @@ import com.google.common.collect.Maps; import com.sun.jersey.spi.container.ResourceFilters; import org.apache.druid.client.DruidServerConfig; +import org.apache.druid.guice.ExtensionsLoader; import org.apache.druid.initialization.DruidModule; -import org.apache.druid.initialization.Initialization; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.server.http.security.ConfigResourceFilter; import org.apache.druid.server.http.security.StateResourceFilter; @@ -39,6 +39,7 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.Context; import javax.ws.rs.core.MediaType; + import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -52,14 +53,18 @@ public class StatusResource { private final Properties properties; - private final DruidServerConfig druidServerConfig; + private final ExtensionsLoader extnLoader; @Inject - public StatusResource(Properties properties, DruidServerConfig druidServerConfig) + public StatusResource( + final Properties properties, + final DruidServerConfig druidServerConfig, + final ExtensionsLoader extnLoader) { this.properties = properties; this.druidServerConfig = druidServerConfig; + this.extnLoader = extnLoader; } @GET @@ -80,7 +85,7 @@ public Status doGet( @Context final HttpServletRequest req ) { - return new Status(Initialization.getLoadedImplementations(DruidModule.class)); + return new Status(extnLoader.getLoadedModules()); } /** diff --git a/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java b/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java index b3d962b4812e..fd022109ea3f 100644 --- a/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java +++ b/server/src/test/java/org/apache/druid/client/DruidServerConfigTest.java @@ -61,7 +61,6 @@ public void setUp() throws Exception { testSegmentCacheDir1 = tmpFolder.newFolder("segment_cache_folder1"); testSegmentCacheDir2 = tmpFolder.newFolder("segment_cache_folder2"); - } @Test @@ -74,7 +73,6 @@ public void testBasicInjection() Assert.assertNotNull(druidServerConfig); Assert.assertEquals(DruidServerConfig.class, druidServerConfig.getClass()); - } @Test diff --git a/server/src/test/java/org/apache/druid/curator/CuratorModuleTest.java b/server/src/test/java/org/apache/druid/curator/CuratorModuleTest.java index 461416de020e..24264b219d0d 100644 --- a/server/src/test/java/org/apache/druid/curator/CuratorModuleTest.java +++ b/server/src/test/java/org/apache/druid/curator/CuratorModuleTest.java @@ -19,11 +19,7 @@ package org.apache.druid.curator; -import com.google.common.collect.ImmutableList; -import com.google.inject.Guice; import com.google.inject.Injector; -import com.google.inject.Module; -import com.google.inject.util.Modules; import org.apache.curator.RetryPolicy; import org.apache.curator.ensemble.EnsembleProvider; import org.apache.curator.ensemble.exhibitor.ExhibitorEnsembleProvider; @@ -31,8 +27,8 @@ import org.apache.curator.framework.CuratorFramework; import org.apache.curator.retry.BoundedExponentialBackoffRetry; import org.apache.curator.retry.ExponentialBackoffRetry; -import org.apache.druid.guice.GuiceInjectors; import org.apache.druid.guice.LifecycleModule; +import org.apache.druid.guice.StartupInjectorBuilder; import org.apache.druid.testing.junit.LoggerCaptureRule; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.LogEvent; @@ -178,14 +174,13 @@ public void ignoresDeprecatedCuratorConfigProperties() private Injector newInjector(final Properties props) { - List modules = ImmutableList.builder() - .addAll(GuiceInjectors.makeDefaultStartupModules()) - .add(new LifecycleModule()) - .add(new CuratorModule()) + return new StartupInjectorBuilder() + .add( + new LifecycleModule(), + new CuratorModule(), + binder -> binder.bind(Properties.class).toInstance(props) + ) .build(); - return Guice.createInjector( - Modules.override(modules).with(binder -> binder.bind(Properties.class).toInstance(props)) - ); } private static CuratorFramework createCuratorFramework(Injector injector, int maxRetries) diff --git a/server/src/test/java/org/apache/druid/initialization/InitializationTest.java b/server/src/test/java/org/apache/druid/initialization/InitializationTest.java deleted file mode 100644 index 13541a95226e..000000000000 --- a/server/src/test/java/org/apache/druid/initialization/InitializationTest.java +++ /dev/null @@ -1,631 +0,0 @@ -/* - * 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.druid.initialization; - -import com.fasterxml.jackson.databind.Module; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Function; -import com.google.common.collect.Collections2; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; -import com.google.inject.Binder; -import com.google.inject.ConfigurationException; -import com.google.inject.Inject; -import com.google.inject.Injector; -import com.google.inject.Key; -import com.google.inject.multibindings.Multibinder; -import com.google.inject.name.Names; -import org.apache.druid.discovery.NodeRole; -import org.apache.druid.guice.ExtensionsConfig; -import org.apache.druid.guice.GuiceInjectors; -import org.apache.druid.guice.JsonConfigProvider; -import org.apache.druid.guice.annotations.LoadScope; -import org.apache.druid.guice.annotations.Self; -import org.apache.druid.java.util.common.ISE; -import org.apache.druid.server.DruidNode; -import org.junit.Assert; -import org.junit.FixMethodOrder; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.junit.runners.MethodSorters; - -import javax.annotation.Nullable; -import java.io.File; -import java.io.IOException; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.Arrays; -import java.util.Collection; -import java.util.Comparator; -import java.util.HashSet; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; - -@FixMethodOrder(MethodSorters.NAME_ASCENDING) -public class InitializationTest -{ - @Rule - public final TemporaryFolder temporaryFolder = new TemporaryFolder(); - - @Test - public void test01InitialModulesEmpty() - { - Initialization.clearLoadedImplementations(); - Assert.assertEquals( - "Initial set of loaded modules must be empty", - 0, - Initialization.getLoadedImplementations(DruidModule.class).size() - ); - } - - @Test - public void test02MakeStartupInjector() - { - Injector startupInjector = GuiceInjectors.makeStartupInjector(); - Assert.assertNotNull(startupInjector); - Assert.assertNotNull(startupInjector.getInstance(ObjectMapper.class)); - } - - @Test - public void test03ClassLoaderExtensionsLoading() - { - Injector startupInjector = GuiceInjectors.makeStartupInjector(); - - Function fnClassName = new Function() - { - @Nullable - @Override - public String apply(@Nullable DruidModule input) - { - return input.getClass().getName(); - } - }; - - Assert.assertFalse( - "modules does not contain TestDruidModule", - Collections2.transform(Initialization.getLoadedImplementations(DruidModule.class), fnClassName) - .contains("org.apache.druid.initialization.InitializationTest.TestDruidModule") - ); - - Collection modules = Initialization.getFromExtensions( - startupInjector.getInstance(ExtensionsConfig.class), - DruidModule.class - ); - - Assert.assertTrue( - "modules contains TestDruidModule", - Collections2.transform(modules, fnClassName).contains(TestDruidModule.class.getName()) - ); - } - - @Test - public void test04DuplicateClassLoaderExtensions() throws Exception - { - final File extensionDir = temporaryFolder.newFolder(); - Initialization.getLoadersMap() - .put(extensionDir, new URLClassLoader(new URL[]{}, Initialization.class.getClassLoader())); - - Collection modules = Initialization.getFromExtensions(new ExtensionsConfig(), DruidModule.class); - - Set loadedModuleNames = new HashSet<>(); - for (DruidModule module : modules) { - Assert.assertFalse("Duplicate extensions are loaded", loadedModuleNames.contains(module.getClass().getName())); - loadedModuleNames.add(module.getClass().getName()); - } - - Initialization.getLoadersMap().clear(); - } - - @Test - public void test05MakeInjectorWithModules() - { - Injector startupInjector = GuiceInjectors.makeStartupInjector(); - Injector injector = Initialization.makeInjectorWithModules( - startupInjector, - ImmutableList.of( - new com.google.inject.Module() - { - @Override - public void configure(Binder binder) - { - JsonConfigProvider.bindInstance( - binder, - Key.get(DruidNode.class, Self.class), - new DruidNode("test-inject", null, false, null, null, true, false) - ); - } - } - ) - ); - Assert.assertNotNull(injector); - } - - @Test - public void test06GetClassLoaderForExtension() throws IOException - { - final File some_extension_dir = temporaryFolder.newFolder(); - final File a_jar = new File(some_extension_dir, "a.jar"); - final File b_jar = new File(some_extension_dir, "b.jar"); - final File c_jar = new File(some_extension_dir, "c.jar"); - a_jar.createNewFile(); - b_jar.createNewFile(); - c_jar.createNewFile(); - final URLClassLoader loader = Initialization.getClassLoaderForExtension(some_extension_dir, false); - final URL[] expectedURLs = new URL[]{a_jar.toURI().toURL(), b_jar.toURI().toURL(), c_jar.toURI().toURL()}; - final URL[] actualURLs = loader.getURLs(); - Arrays.sort(actualURLs, Comparator.comparing(URL::getPath)); - Assert.assertArrayEquals(expectedURLs, actualURLs); - } - - @Test - public void testGetLoadedModules() - { - - Collection modules = Initialization.getLoadedImplementations(DruidModule.class); - HashSet moduleSet = new HashSet<>(modules); - - Collection loadedModules = Initialization.getLoadedImplementations(DruidModule.class); - Assert.assertEquals("Set from loaded modules #1 should be same!", modules.size(), loadedModules.size()); - Assert.assertEquals("Set from loaded modules #1 should be same!", moduleSet, new HashSet<>(loadedModules)); - - Collection loadedModules2 = Initialization.getLoadedImplementations(DruidModule.class); - Assert.assertEquals("Set from loaded modules #2 should be same!", modules.size(), loadedModules2.size()); - Assert.assertEquals("Set from loaded modules #2 should be same!", moduleSet, new HashSet<>(loadedModules2)); - } - - @Test - public void testGetExtensionFilesToLoad_non_exist_extensions_dir() throws IOException - { - final File tmpDir = temporaryFolder.newFolder(); - Assert.assertTrue("could not create missing folder", !tmpDir.exists() || tmpDir.delete()); - Assert.assertArrayEquals( - "Non-exist root extensionsDir should return an empty array of File", - new File[]{}, - Initialization.getExtensionFilesToLoad(new ExtensionsConfig() - { - @Override - public String getDirectory() - { - return tmpDir.getAbsolutePath(); - } - }) - ); - } - - @Test(expected = ISE.class) - public void testGetExtensionFilesToLoad_wrong_type_extensions_dir() throws IOException - { - final File extensionsDir = temporaryFolder.newFile(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public String getDirectory() - { - return extensionsDir.getAbsolutePath(); - } - }; - Initialization.getExtensionFilesToLoad(config); - } - - @Test - public void testGetExtensionFilesToLoad_empty_extensions_dir() throws IOException - { - final File extensionsDir = temporaryFolder.newFolder(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public String getDirectory() - { - return extensionsDir.getAbsolutePath(); - } - }; - - Assert.assertArrayEquals( - "Empty root extensionsDir should return an empty array of File", - new File[]{}, - Initialization.getExtensionFilesToLoad(config) - ); - } - - /** - * If druid.extension.load is not specified, Initialization.getExtensionFilesToLoad is supposed to return all the - * extension folders under root extensions directory. - */ - @Test - public void testGetExtensionFilesToLoad_null_load_list() throws IOException - { - final File extensionsDir = temporaryFolder.newFolder(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public String getDirectory() - { - return extensionsDir.getAbsolutePath(); - } - }; - final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage"); - mysql_metadata_storage.mkdir(); - - final File[] expectedFileList = new File[]{mysql_metadata_storage}; - final File[] actualFileList = Initialization.getExtensionFilesToLoad(config); - Arrays.sort(actualFileList); - Assert.assertArrayEquals(expectedFileList, actualFileList); - } - - /** - * druid.extension.load is specified, Initialization.getExtensionFilesToLoad is supposed to return all the extension - * folders appeared in the load list. - */ - @Test - public void testGetExtensionFilesToLoad_with_load_list() throws IOException - { - final File extensionsDir = temporaryFolder.newFolder(); - - final File absolutePathExtension = temporaryFolder.newFolder(); - - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public LinkedHashSet getLoadList() - { - return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath())); - } - - @Override - public String getDirectory() - { - return extensionsDir.getAbsolutePath(); - } - }; - final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage"); - final File random_extension = new File(extensionsDir, "random-extensions"); - - mysql_metadata_storage.mkdir(); - random_extension.mkdir(); - - final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension}; - final File[] actualFileList = Initialization.getExtensionFilesToLoad(config); - Assert.assertArrayEquals(expectedFileList, actualFileList); - } - - /** - * druid.extension.load is specified, but contains an extension that is not prepared under root extension directory. - * Initialization.getExtensionFilesToLoad is supposed to throw ISE. - */ - @Test(expected = ISE.class) - public void testGetExtensionFilesToLoad_with_non_exist_item_in_load_list() throws IOException - { - final File extensionsDir = temporaryFolder.newFolder(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public LinkedHashSet getLoadList() - { - return Sets.newLinkedHashSet(ImmutableList.of("mysql-metadata-storage")); - } - - @Override - public String getDirectory() - { - return extensionsDir.getAbsolutePath(); - } - }; - final File random_extension = new File(extensionsDir, "random-extensions"); - random_extension.mkdir(); - Initialization.getExtensionFilesToLoad(config); - } - - @Test(expected = ISE.class) - public void testGetHadoopDependencyFilesToLoad_wrong_type_root_hadoop_depenencies_dir() throws IOException - { - final File rootHadoopDependenciesDir = temporaryFolder.newFile(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public String getHadoopDependenciesDir() - { - return rootHadoopDependenciesDir.getAbsolutePath(); - } - }; - Initialization.getHadoopDependencyFilesToLoad(ImmutableList.of(), config); - } - - @Test(expected = ISE.class) - public void testGetHadoopDependencyFilesToLoad_non_exist_version_dir() throws IOException - { - final File rootHadoopDependenciesDir = temporaryFolder.newFolder(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public String getHadoopDependenciesDir() - { - return rootHadoopDependenciesDir.getAbsolutePath(); - } - }; - final File hadoopClient = new File(rootHadoopDependenciesDir, "hadoop-client"); - hadoopClient.mkdir(); - Initialization.getHadoopDependencyFilesToLoad(ImmutableList.of("org.apache.hadoop:hadoop-client:2.3.0"), config); - } - - @Test - public void testGetHadoopDependencyFilesToLoad_with_hadoop_coordinates() throws IOException - { - final File rootHadoopDependenciesDir = temporaryFolder.newFolder(); - final ExtensionsConfig config = new ExtensionsConfig() - { - @Override - public String getHadoopDependenciesDir() - { - return rootHadoopDependenciesDir.getAbsolutePath(); - } - }; - final File hadoopClient = new File(rootHadoopDependenciesDir, "hadoop-client"); - final File versionDir = new File(hadoopClient, "2.3.0"); - hadoopClient.mkdir(); - versionDir.mkdir(); - final File[] expectedFileList = new File[]{versionDir}; - final File[] actualFileList = Initialization.getHadoopDependencyFilesToLoad( - ImmutableList.of( - "org.apache.hadoop:hadoop-client:2.3.0" - ), config - ); - Assert.assertArrayEquals(expectedFileList, actualFileList); - } - - @Test - public void testGetURLsForClasspath() throws Exception - { - File tmpDir1 = temporaryFolder.newFolder(); - File tmpDir2 = temporaryFolder.newFolder(); - File tmpDir3 = temporaryFolder.newFolder(); - - File tmpDir1a = new File(tmpDir1, "a.jar"); - tmpDir1a.createNewFile(); - File tmpDir1b = new File(tmpDir1, "b.jar"); - tmpDir1b.createNewFile(); - new File(tmpDir1, "note1.txt").createNewFile(); - - File tmpDir2c = new File(tmpDir2, "c.jar"); - tmpDir2c.createNewFile(); - File tmpDir2d = new File(tmpDir2, "d.jar"); - tmpDir2d.createNewFile(); - File tmpDir2e = new File(tmpDir2, "e.JAR"); - tmpDir2e.createNewFile(); - new File(tmpDir2, "note2.txt").createNewFile(); - - String cp = tmpDir1.getAbsolutePath() + File.separator + "*" - + File.pathSeparator - + tmpDir3.getAbsolutePath() - + File.pathSeparator - + tmpDir2.getAbsolutePath() + File.separator + "*"; - - // getURLsForClasspath uses listFiles which does NOT guarantee any ordering for the name strings. - List urLsForClasspath = Initialization.getURLsForClasspath(cp); - Assert.assertEquals(Sets.newHashSet(tmpDir1a.toURI().toURL(), tmpDir1b.toURI().toURL()), - Sets.newHashSet(urLsForClasspath.subList(0, 2))); - Assert.assertEquals(tmpDir3.toURI().toURL(), urLsForClasspath.get(2)); - Assert.assertEquals(Sets.newHashSet(tmpDir2c.toURI().toURL(), tmpDir2d.toURI().toURL(), tmpDir2e.toURI().toURL()), - Sets.newHashSet(urLsForClasspath.subList(3, 6))); - - - } - - @Test - public void testExtensionsWithSameDirName() throws Exception - { - final String extensionName = "some_extension"; - final File tmpDir1 = temporaryFolder.newFolder(); - final File tmpDir2 = temporaryFolder.newFolder(); - final File extension1 = new File(tmpDir1, extensionName); - final File extension2 = new File(tmpDir2, extensionName); - Assert.assertTrue(extension1.mkdir()); - Assert.assertTrue(extension2.mkdir()); - final File jar1 = new File(extension1, "jar1.jar"); - final File jar2 = new File(extension2, "jar2.jar"); - - Assert.assertTrue(jar1.createNewFile()); - Assert.assertTrue(jar2.createNewFile()); - - final ClassLoader classLoader1 = Initialization.getClassLoaderForExtension(extension1, false); - final ClassLoader classLoader2 = Initialization.getClassLoaderForExtension(extension2, false); - - Assert.assertArrayEquals(new URL[]{jar1.toURI().toURL()}, ((URLClassLoader) classLoader1).getURLs()); - Assert.assertArrayEquals(new URL[]{jar2.toURI().toURL()}, ((URLClassLoader) classLoader2).getURLs()); - } - - public static class TestDruidModule implements DruidModule - { - @Override - public List getJacksonModules() - { - return ImmutableList.of(); - } - - @Override - public void configure(Binder binder) - { - // Do nothing - } - } - - @Test - public void testCreateInjectorWithNodeRoles() - { - final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); - Injector startupInjector = GuiceInjectors.makeStartupInjector(); - Injector injector = Initialization.makeInjectorWithModules( - ImmutableSet.of(new NodeRole("role1"), new NodeRole("role2")), - startupInjector, - ImmutableList.of( - binder -> JsonConfigProvider.bindInstance( - binder, - Key.get(DruidNode.class, Self.class), - expected - ) - ) - ); - Assert.assertNotNull(injector); - Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); - } - - @Test - public void testCreateInjectorWithNodeRoleFilter_moduleNotLoaded() - { - final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); - Injector startupInjector = GuiceInjectors.makeStartupInjector(); - Injector injector = Initialization.makeInjectorWithModules( - ImmutableSet.of(new NodeRole("role1"), new NodeRole("role2")), - startupInjector, - ImmutableList.of( - (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( - binder, - Key.get(DruidNode.class, Self.class), - expected - ), - new LoadOnAnnotationTestModule() - ) - ); - Assert.assertNotNull(injector); - Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); - Assert.assertThrows( - "Guice configuration errors", - ConfigurationException.class, - () -> injector.getInstance(Key.get(String.class, Names.named("emperor"))) - ); - } - - @Test - public void testCreateInjectorWithNodeRoleFilterUsingAnnotation_moduleLoaded() - { - final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); - Injector startupInjector = GuiceInjectors.makeStartupInjector(); - Injector injector = Initialization.makeInjectorWithModules( - ImmutableSet.of(new NodeRole("role1"), new NodeRole("druid")), - startupInjector, - ImmutableList.of( - (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( - binder, - Key.get(DruidNode.class, Self.class), - expected - ), - new LoadOnAnnotationTestModule() - ) - ); - Assert.assertNotNull(injector); - Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); - Assert.assertEquals("I am Druid", injector.getInstance(Key.get(String.class, Names.named("emperor")))); - } - - @LoadScope(roles = {"emperor", "druid"}) - private static class LoadOnAnnotationTestModule implements com.google.inject.Module - { - @Override - public void configure(Binder binder) - { - binder.bind(String.class).annotatedWith(Names.named("emperor")).toInstance("I am Druid"); - } - } - - @Test - public void testCreateInjectorWithNodeRoleFilterUsingInject_moduleNotLoaded() - { - final Set nodeRoles = ImmutableSet.of(new NodeRole("role1"), new NodeRole("role2")); - final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); - Injector startupInjector = GuiceInjectors.makeStartupInjectorWithModules( - ImmutableList.of( - binder -> { - Multibinder selfBinder = Multibinder.newSetBinder(binder, NodeRole.class, Self.class); - nodeRoles.forEach(nodeRole -> selfBinder.addBinding().toInstance(nodeRole)); - } - ) - ); - Injector injector = Initialization.makeInjectorWithModules( - nodeRoles, - startupInjector, - ImmutableList.of( - (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( - binder, - Key.get(DruidNode.class, Self.class), - expected - ), - new NodeRolesInjectTestModule() - ) - ); - Assert.assertNotNull(injector); - Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); - Assert.assertThrows( - "Guice configuration errors", - ConfigurationException.class, - () -> injector.getInstance(Key.get(String.class, Names.named("emperor"))) - ); - } - - @Test - public void testCreateInjectorWithNodeRoleFilterUsingInject_moduleLoaded() - { - final Set nodeRoles = ImmutableSet.of(new NodeRole("role1"), new NodeRole("druid")); - final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); - Injector startupInjector = GuiceInjectors.makeStartupInjectorWithModules( - ImmutableList.of( - binder -> { - Multibinder selfBinder = Multibinder.newSetBinder(binder, NodeRole.class, Self.class); - nodeRoles.forEach(nodeRole -> selfBinder.addBinding().toInstance(nodeRole)); - } - ) - ); - Injector injector = Initialization.makeInjectorWithModules( - nodeRoles, - startupInjector, - ImmutableList.of( - (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( - binder, - Key.get(DruidNode.class, Self.class), - expected - ), - new NodeRolesInjectTestModule() - ) - ); - Assert.assertNotNull(injector); - Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); - Assert.assertEquals("I am Druid", injector.getInstance(Key.get(String.class, Names.named("emperor")))); - } - - private static class NodeRolesInjectTestModule implements com.google.inject.Module - { - private Set nodeRoles; - - @Inject - public void init(@Self Set nodeRoles) - { - this.nodeRoles = nodeRoles; - } - - @Override - public void configure(Binder binder) - { - if (nodeRoles.contains(new NodeRole("emperor")) || nodeRoles.contains(new NodeRole("druid"))) { - binder.bind(String.class).annotatedWith(Names.named("emperor")).toInstance("I am Druid"); - } - } - } -} diff --git a/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java b/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java new file mode 100644 index 000000000000..0bce3d7fac35 --- /dev/null +++ b/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java @@ -0,0 +1,285 @@ +/* + * 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.druid.initialization; + +import com.fasterxml.jackson.databind.Module; +import com.google.common.base.Function; +import com.google.common.collect.Collections2; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.inject.Binder; +import com.google.inject.ConfigurationException; +import com.google.inject.Inject; +import com.google.inject.Injector; +import com.google.inject.Key; +import com.google.inject.name.Names; +import org.apache.druid.discovery.NodeRole; +import org.apache.druid.guice.ExtensionsLoader; +import org.apache.druid.guice.JsonConfigProvider; +import org.apache.druid.guice.StartupInjectorBuilder; +import org.apache.druid.guice.annotations.LoadScope; +import org.apache.druid.guice.annotations.Self; +import org.apache.druid.server.DruidNode; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import javax.annotation.Nullable; + +import java.util.Collection; +import java.util.List; +import java.util.Set; + +// See ExtensionsLoaderTest for tests formerly in this file related +// to extension loading itself. +public class ServerInjectorBuilderTest +{ + @Rule + public final TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private Injector startupInjector() + { + return new StartupInjectorBuilder() + .withEmptyProperties() + .withExtensions() + .build(); + } + + @Test + public void test03ClassLoaderExtensionsLoading() + { + Injector startupInjector = startupInjector(); + ExtensionsLoader extnLoader = ExtensionsLoader.instance(startupInjector); + + Function fnClassName = new Function() + { + @Nullable + @Override + public String apply(@Nullable DruidModule input) + { + return input.getClass().getName(); + } + }; + + Collection modules = extnLoader.getFromExtensions( + DruidModule.class + ); + + Assert.assertTrue( + "modules contains TestDruidModule", + Collections2.transform(modules, fnClassName).contains(TestDruidModule.class.getName()) + ); + } + + @Test + public void test05MakeInjectorWithModules() + { + Injector startupInjector = startupInjector(); + ExtensionsLoader extnLoader = ExtensionsLoader.instance(startupInjector); + Injector injector = ServerInjectorBuilder.makeServerInjector( + startupInjector, + ImmutableSet.of(), + ImmutableList.of( + new com.google.inject.Module() + { + @Override + public void configure(Binder binder) + { + JsonConfigProvider.bindInstance( + binder, + Key.get(DruidNode.class, Self.class), + new DruidNode("test-inject", null, false, null, null, true, false) + ); + } + } + ) + ); + Assert.assertNotNull(injector); + Assert.assertNotNull(ExtensionsLoader.instance(injector)); + Assert.assertSame(extnLoader, ExtensionsLoader.instance(injector)); + } + + // Note: this name is referenced in + // src/test/resources/META-INF/services/org.apache.druid.initialization.DruidModule + // which impacts many tests, not just those here. + // This is likely a bug, not a feature. + public static class TestDruidModule implements DruidModule + { + @Override + public List getJacksonModules() + { + return ImmutableList.of(); + } + + @Override + public void configure(Binder binder) + { + // Do nothing + } + } + + @Test + public void testCreateInjectorWithNodeRoles() + { + final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); + Injector startupInjector = startupInjector(); + Injector injector = ServerInjectorBuilder.makeServerInjector( + startupInjector, + ImmutableSet.of(new NodeRole("role1"), new NodeRole("role2")), + ImmutableList.of( + binder -> JsonConfigProvider.bindInstance( + binder, + Key.get(DruidNode.class, Self.class), + expected + ) + ) + ); + Assert.assertNotNull(injector); + Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); + } + + @Test + public void testCreateInjectorWithNodeRoleFilter_moduleNotLoaded() + { + final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); + Injector startupInjector = startupInjector(); + Injector injector = ServerInjectorBuilder.makeServerInjector( + startupInjector, + ImmutableSet.of(new NodeRole("role1"), new NodeRole("role2")), + ImmutableList.of( + (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( + binder, + Key.get(DruidNode.class, Self.class), + expected + ), + new LoadOnAnnotationTestModule() + ) + ); + Assert.assertNotNull(injector); + Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); + Assert.assertThrows( + "Guice configuration errors", + ConfigurationException.class, + () -> injector.getInstance(Key.get(String.class, Names.named("emperor"))) + ); + } + + @Test + public void testCreateInjectorWithNodeRoleFilterUsingAnnotation_moduleLoaded() + { + final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); + Injector startupInjector = startupInjector(); + Injector injector = ServerInjectorBuilder.makeServerInjector( + startupInjector, + ImmutableSet.of(new NodeRole("role1"), new NodeRole("druid")), + ImmutableList.of( + (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( + binder, + Key.get(DruidNode.class, Self.class), + expected + ), + new LoadOnAnnotationTestModule() + ) + ); + Assert.assertNotNull(injector); + Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); + Assert.assertEquals("I am Druid", injector.getInstance(Key.get(String.class, Names.named("emperor")))); + } + + @LoadScope(roles = {"emperor", "druid"}) + private static class LoadOnAnnotationTestModule implements com.google.inject.Module + { + @Override + public void configure(Binder binder) + { + binder.bind(String.class).annotatedWith(Names.named("emperor")).toInstance("I am Druid"); + } + } + + @Test + public void testCreateInjectorWithNodeRoleFilterUsingInject_moduleNotLoaded() + { + final Set nodeRoles = ImmutableSet.of(new NodeRole("role1"), new NodeRole("role2")); + final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); + Injector startupInjector = startupInjector(); + Injector injector = ServerInjectorBuilder.makeServerInjector( + startupInjector, + nodeRoles, + ImmutableList.of( + (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( + binder, + Key.get(DruidNode.class, Self.class), + expected + ), + new NodeRolesInjectTestModule() + ) + ); + Assert.assertNotNull(injector); + Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); + Assert.assertThrows( + "Guice configuration errors", + ConfigurationException.class, + () -> injector.getInstance(Key.get(String.class, Names.named("emperor"))) + ); + } + + @Test + public void testCreateInjectorWithNodeRoleFilterUsingInject_moduleLoaded() + { + final Set nodeRoles = ImmutableSet.of(new NodeRole("role1"), new NodeRole("druid")); + final DruidNode expected = new DruidNode("test-inject", null, false, null, null, true, false); + Injector startupInjector = startupInjector(); + Injector injector = ServerInjectorBuilder.makeServerInjector( + startupInjector, + nodeRoles, + ImmutableList.of( + (com.google.inject.Module) binder -> JsonConfigProvider.bindInstance( + binder, + Key.get(DruidNode.class, Self.class), + expected + ), + new NodeRolesInjectTestModule() + ) + ); + Assert.assertNotNull(injector); + Assert.assertEquals(expected, injector.getInstance(Key.get(DruidNode.class, Self.class))); + Assert.assertEquals("I am Druid", injector.getInstance(Key.get(String.class, Names.named("emperor")))); + } + + private static class NodeRolesInjectTestModule implements com.google.inject.Module + { + private Set nodeRoles; + + @Inject + public void init(@Self Set nodeRoles) + { + this.nodeRoles = nodeRoles; + } + + @Override + public void configure(Binder binder) + { + if (nodeRoles.contains(new NodeRole("emperor")) || nodeRoles.contains(new NodeRole("druid"))) { + binder.bind(String.class).annotatedWith(Names.named("emperor")).toInstance("I am Druid"); + } + } + } +} diff --git a/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java b/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java index 5ef82e6ecb42..dfa98534fede 100644 --- a/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java +++ b/server/src/test/java/org/apache/druid/query/lookup/LookupSerdeModuleTest.java @@ -19,15 +19,13 @@ package org.apache.druid.query.lookup; -import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableList; import com.google.inject.Injector; import com.google.inject.Key; import org.apache.druid.guice.ExpressionModule; -import org.apache.druid.guice.GuiceInjectors; +import org.apache.druid.guice.StartupInjectorBuilder; import org.apache.druid.guice.annotations.Json; -import org.apache.druid.initialization.DruidModule; +import org.apache.druid.initialization.CoreInjectorBuilder; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.dimension.DimensionSpec; import org.apache.druid.query.dimension.ExtractionDimensionSpec; @@ -49,22 +47,14 @@ public class LookupSerdeModuleTest @Before public void setUp() { - final ImmutableList modules = ImmutableList.of( - new ExpressionModule(), - new LookupSerdeModule() - ); + injector = new CoreInjectorBuilder(new StartupInjectorBuilder().build()) + .add( + new ExpressionModule(), + new LookupSerdeModule() + ) + .build(); - injector = GuiceInjectors.makeStartupInjectorWithModules(modules); objectMapper = injector.getInstance(Key.get(ObjectMapper.class, Json.class)); - objectMapper.setInjectableValues( - new InjectableValues.Std() - .addValue(ExprMacroTable.class, injector.getInstance(ExprMacroTable.class)) - .addValue( - LookupExtractorFactoryContainerProvider.class, - injector.getInstance(LookupExtractorFactoryContainerProvider.class) - ) - ); - modules.stream().flatMap(module -> module.getJacksonModules().stream()).forEach(objectMapper::registerModule); } @Test diff --git a/server/src/test/java/org/apache/druid/server/StatusResourceTest.java b/server/src/test/java/org/apache/druid/server/StatusResourceTest.java index c0750e14ba22..0a1de9d01ffe 100644 --- a/server/src/test/java/org/apache/druid/server/StatusResourceTest.java +++ b/server/src/test/java/org/apache/druid/server/StatusResourceTest.java @@ -25,7 +25,7 @@ import com.google.inject.Injector; import org.apache.druid.guice.PropertiesModule; import org.apache.druid.initialization.DruidModule; -import org.apache.druid.initialization.InitializationTest; +import org.apache.druid.initialization.ServerInjectorBuilderTest; import org.junit.Assert; import org.junit.Test; @@ -42,7 +42,7 @@ public class StatusResourceTest public void testLoadedModules() { - Collection modules = ImmutableList.of(new InitializationTest.TestDruidModule()); + Collection modules = ImmutableList.of(new ServerInjectorBuilderTest.TestDruidModule()); List statusResourceModuleList = new StatusResource.Status(modules).getModules(); Assert.assertEquals("Status should have all modules loaded!", modules.size(), statusResourceModuleList.size()); diff --git a/server/src/test/resources/META-INF/services/org.apache.druid.initialization.DruidModule b/server/src/test/resources/META-INF/services/org.apache.druid.initialization.DruidModule index fa5a533558a9..78a20ef86ec9 100644 --- a/server/src/test/resources/META-INF/services/org.apache.druid.initialization.DruidModule +++ b/server/src/test/resources/META-INF/services/org.apache.druid.initialization.DruidModule @@ -13,4 +13,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -org.apache.druid.initialization.InitializationTest$TestDruidModule +org.apache.druid.initialization.ServerInjectorBuilderTest$TestDruidModule diff --git a/services/pom.xml b/services/pom.xml index 32496fd0ed92..31253f4abea0 100644 --- a/services/pom.xml +++ b/services/pom.xml @@ -324,6 +324,7 @@ jakarta.xml.bind:jakarta.xml.bind-api + jakarta.inject:jakarta.inject-api diff --git a/services/src/main/java/org/apache/druid/cli/CliHadoopIndexer.java b/services/src/main/java/org/apache/druid/cli/CliHadoopIndexer.java index 2b3b0389b942..30534ccafff7 100644 --- a/services/src/main/java/org/apache/druid/cli/CliHadoopIndexer.java +++ b/services/src/main/java/org/apache/druid/cli/CliHadoopIndexer.java @@ -25,9 +25,9 @@ import com.github.rvesse.airline.annotations.restrictions.Required; import com.google.common.base.Joiner; import com.google.inject.Inject; -import org.apache.druid.guice.ExtensionsConfig; +import org.apache.druid.guice.ExtensionsLoader; import org.apache.druid.indexing.common.config.TaskConfig; -import org.apache.druid.initialization.Initialization; +import org.apache.druid.indexing.common.task.Initialization; import org.apache.druid.java.util.common.logger.Logger; import java.io.File; @@ -46,7 +46,6 @@ ) public class CliHadoopIndexer implements Runnable { - private static final List DEFAULT_HADOOP_COORDINATES = TaskConfig.DEFAULT_DEFAULT_HADOOP_COORDINATES; private static final Logger log = new Logger(CliHadoopIndexer.class); @@ -65,10 +64,9 @@ public class CliHadoopIndexer implements Runnable public boolean noDefaultHadoop; @Inject - private ExtensionsConfig extensionsConfig = null; + private ExtensionsLoader extnLoader; @Override - @SuppressWarnings("unchecked") public void run() { try { @@ -81,8 +79,8 @@ public void run() } final List extensionURLs = new ArrayList<>(); - for (final File extension : Initialization.getExtensionFilesToLoad(extensionsConfig)) { - final URLClassLoader extensionLoader = Initialization.getClassLoaderForExtension(extension, false); + for (final File extension : extnLoader.getExtensionFilesToLoad()) { + final URLClassLoader extensionLoader = extnLoader.getClassLoaderForExtension(extension, false); extensionURLs.addAll(Arrays.asList(extensionLoader.getURLs())); } @@ -92,8 +90,8 @@ public void run() final List driverURLs = new ArrayList<>(nonHadoopURLs); // put hadoop dependencies last to avoid jets3t & apache.httpcore version conflicts - for (File hadoopDependency : Initialization.getHadoopDependencyFilesToLoad(allCoordinates, extensionsConfig)) { - final URLClassLoader hadoopLoader = Initialization.getClassLoaderForExtension(hadoopDependency, false); + for (File hadoopDependency : Initialization.getHadoopDependencyFilesToLoad(allCoordinates, extnLoader.config())) { + final URLClassLoader hadoopLoader = extnLoader.getClassLoaderForExtension(hadoopDependency, false); driverURLs.addAll(Arrays.asList(hadoopLoader.getURLs())); } @@ -122,5 +120,4 @@ public void run() System.exit(1); } } - } diff --git a/services/src/main/java/org/apache/druid/cli/GuiceRunnable.java b/services/src/main/java/org/apache/druid/cli/GuiceRunnable.java index a790bf0967ee..9f694e45755f 100644 --- a/services/src/main/java/org/apache/druid/cli/GuiceRunnable.java +++ b/services/src/main/java/org/apache/druid/cli/GuiceRunnable.java @@ -19,21 +19,14 @@ package org.apache.druid.cli; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Module; -import com.google.inject.TypeLiteral; -import com.google.inject.multibindings.MapBinder; -import com.google.inject.multibindings.Multibinder; -import org.apache.druid.discovery.DruidService; import org.apache.druid.discovery.NodeRole; -import org.apache.druid.guice.annotations.Self; -import org.apache.druid.initialization.Initialization; +import org.apache.druid.initialization.ServerInjectorBuilder; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.lifecycle.Lifecycle; import org.apache.druid.java.util.common.logger.Logger; @@ -88,41 +81,14 @@ public Injector makeInjector() public Injector makeInjector(Set nodeRoles) { - Module registerNodeRoleModule = registerNodeRoleModule(nodeRoles); try { - return Initialization.makeInjectorWithModules( - nodeRoles, - baseInjector.createChildInjector(registerNodeRoleModule), - Iterables.concat( - // bind nodeRoles for the new injector as well - ImmutableList.of(registerNodeRoleModule), - getModules() - ) - ); + return ServerInjectorBuilder.makeServerInjector(baseInjector, nodeRoles, getModules()); } catch (Exception e) { throw new RuntimeException(e); } } - static Module registerNodeRoleModule(Set nodeRoles) - { - if (nodeRoles.isEmpty()) { - return binder -> {}; - } else { - return binder -> { - Multibinder selfBinder = Multibinder.newSetBinder(binder, NodeRole.class, Self.class); - nodeRoles.forEach(nodeRole -> selfBinder.addBinding().toInstance(nodeRole)); - - MapBinder.newMapBinder( - binder, - new TypeLiteral(){}, - new TypeLiteral>>(){} - ); - }; - } - } - public Lifecycle initLifecycle(Injector injector) { return initLifecycle(injector, log); diff --git a/services/src/main/java/org/apache/druid/cli/Main.java b/services/src/main/java/org/apache/druid/cli/Main.java index d38e58253785..ffd60410b07a 100644 --- a/services/src/main/java/org/apache/druid/cli/Main.java +++ b/services/src/main/java/org/apache/druid/cli/Main.java @@ -26,9 +26,8 @@ import com.google.inject.Injector; import io.netty.util.SuppressForbidden; import org.apache.druid.cli.validate.DruidJsonValidator; -import org.apache.druid.guice.ExtensionsConfig; -import org.apache.druid.guice.GuiceInjectors; -import org.apache.druid.initialization.Initialization; +import org.apache.druid.guice.ExtensionsLoader; +import org.apache.druid.guice.StartupInjectorBuilder; import java.util.Arrays; import java.util.Collection; @@ -94,11 +93,10 @@ public static void main(String[] args) .withDefaultCommand(Help.class) .withCommands(CliPeon.class, CliInternalHadoopIndexer.class); - final Injector injector = GuiceInjectors.makeStartupInjector(); - final ExtensionsConfig config = injector.getInstance(ExtensionsConfig.class); - final Collection extensionCommands = Initialization.getFromExtensions( - config, - CliCommandCreator.class + final Injector injector = new StartupInjectorBuilder().forServer().build(); + ExtensionsLoader extnLoader = ExtensionsLoader.instance(injector); + final Collection extensionCommands = extnLoader.getFromExtensions( + CliCommandCreator.class ); for (CliCommandCreator creator : extensionCommands) { diff --git a/services/src/main/java/org/apache/druid/cli/Version.java b/services/src/main/java/org/apache/druid/cli/Version.java index d6da236bd430..b5355b7f3ca7 100644 --- a/services/src/main/java/org/apache/druid/cli/Version.java +++ b/services/src/main/java/org/apache/druid/cli/Version.java @@ -21,20 +21,27 @@ import com.github.rvesse.airline.annotations.Command; import io.netty.util.SuppressForbidden; -import org.apache.druid.initialization.DruidModule; -import org.apache.druid.initialization.Initialization; +import org.apache.druid.guice.ExtensionsLoader; import org.apache.druid.server.StatusResource; +import javax.inject.Inject; + @Command( name = "version", description = "Returns Druid version information" ) public class Version implements Runnable { + @Inject + private ExtensionsLoader extnLoader; + @Override @SuppressForbidden(reason = "System#out") public void run() { - System.out.println(new StatusResource.Status(Initialization.getLoadedImplementations(DruidModule.class))); + // Since Version is a command, we don't go through the server + // process to load modules and they are thus not already loaded. + // Explicitly load them here. + System.out.println(new StatusResource.Status(extnLoader.getModules())); } } diff --git a/services/src/main/java/org/apache/druid/cli/validate/DruidJsonValidator.java b/services/src/main/java/org/apache/druid/cli/validate/DruidJsonValidator.java index 063b164711be..0518c7259e63 100644 --- a/services/src/main/java/org/apache/druid/cli/validate/DruidJsonValidator.java +++ b/services/src/main/java/org/apache/druid/cli/validate/DruidJsonValidator.java @@ -39,7 +39,7 @@ import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.impl.StringInputRowParser; import org.apache.druid.guice.DruidProcessingModule; -import org.apache.druid.guice.ExtensionsConfig; +import org.apache.druid.guice.ExtensionsLoader; import org.apache.druid.guice.FirehoseModule; import org.apache.druid.guice.IndexingServiceFirehoseModule; import org.apache.druid.guice.IndexingServiceInputSourceModule; @@ -50,7 +50,6 @@ import org.apache.druid.indexer.IndexingHadoopModule; import org.apache.druid.indexing.common.task.Task; import org.apache.druid.initialization.DruidModule; -import org.apache.druid.initialization.Initialization; import org.apache.druid.java.util.common.UOE; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.query.Query; @@ -124,11 +123,12 @@ public void run() final Injector injector = makeInjector(); final ObjectMapper jsonMapper = injector.getInstance(ObjectMapper.class); + ExtensionsLoader extnLoader = injector.getInstance(ExtensionsLoader.class); registerModules( jsonMapper, Iterables.concat( - Initialization.getFromExtensions(injector.getInstance(ExtensionsConfig.class), DruidModule.class), + extnLoader.getModules(), Arrays.asList( new FirehoseModule(), new IndexingHadoopModule(), diff --git a/services/src/main/java/org/apache/druid/guice/AbstractDruidServiceModule.java b/services/src/main/java/org/apache/druid/guice/AbstractDruidServiceModule.java index c0579d359b6f..fb69005675f1 100644 --- a/services/src/main/java/org/apache/druid/guice/AbstractDruidServiceModule.java +++ b/services/src/main/java/org/apache/druid/guice/AbstractDruidServiceModule.java @@ -33,8 +33,9 @@ /** * An abstract module for dynamic registration of {@link DruidService}. * DruidServices are bound to a set which is mapped to a certain {@link NodeRole}. - * See {@link org.apache.druid.cli.GuiceRunnable#registerNodeRoleModule} for how the map is bound. - * + * See {@link org.apache.druid.initialization.ServerInjectorBuilder#registerNodeRoleModule} + * for how the map is bound. + *

* To register a DruidService, create a class something like below: * *

@@ -63,7 +64,7 @@ public void configure(Binder binder)
   }
 
   /**
-   * A helper method for extensions which do not implement Module directly. 
+   * A helper method for extensions which do not implement Module directly.
    */
   public static void configure(Binder binder, NodeRole role)
   {
diff --git a/services/src/test/java/org/apache/druid/cli/DiscoverySideEffectsProviderTest.java b/services/src/test/java/org/apache/druid/cli/DiscoverySideEffectsProviderTest.java
index 4e8df8c8c6d2..09ad6af85e2c 100644
--- a/services/src/test/java/org/apache/druid/cli/DiscoverySideEffectsProviderTest.java
+++ b/services/src/test/java/org/apache/druid/cli/DiscoverySideEffectsProviderTest.java
@@ -32,8 +32,9 @@
 import org.apache.druid.discovery.DruidService;
 import org.apache.druid.discovery.NodeRole;
 import org.apache.druid.guice.AbstractDruidServiceModule;
-import org.apache.druid.guice.GuiceInjectors;
+import org.apache.druid.guice.StartupInjectorBuilder;
 import org.apache.druid.guice.annotations.Self;
+import org.apache.druid.initialization.ServerInjectorBuilder;
 import org.apache.druid.java.util.common.lifecycle.Lifecycle;
 import org.apache.druid.server.DruidNode;
 import org.junit.Assert;
@@ -180,18 +181,17 @@ protected NodeRole getNodeRoleKey()
 
   private Injector createInjector(List modules)
   {
-    List finalModules = new ArrayList<>(modules);
-    finalModules.addAll(
-        ImmutableList.of(
-            GuiceRunnable.registerNodeRoleModule(ImmutableSet.of(nodeRole)),
+    return new StartupInjectorBuilder()
+        .add(
+            ServerInjectorBuilder.registerNodeRoleModule(ImmutableSet.of(nodeRole)),
             binder -> {
               binder.bind(DruidNodeAnnouncer.class).toInstance(discoverableOnlyAnnouncer);
               binder.bind(DruidNode.class).annotatedWith(Self.class).toInstance(druidNode);
               binder.bind(ServiceAnnouncer.class).toInstance(legacyAnnouncer);
               binder.bind(Lifecycle.class).toInstance(lifecycle);
             }
-        )
-    );
-    return GuiceInjectors.makeStartupInjectorWithModules(finalModules);
+         )
+        .addAll(modules)
+        .build();
   }
 }
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTestInjectorBuilder.java b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTestInjectorBuilder.java
new file mode 100644
index 000000000000..0f9295c87b7f
--- /dev/null
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTestInjectorBuilder.java
@@ -0,0 +1,106 @@
+/*
+ * 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.druid.sql.calcite.util;
+
+import com.fasterxml.jackson.databind.Module;
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Binder;
+import com.google.inject.Injector;
+import org.apache.druid.guice.StartupInjectorBuilder;
+import org.apache.druid.initialization.CoreInjectorBuilder;
+import org.apache.druid.initialization.DruidModule;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.expression.LookupEnabledTestExprMacroTable;
+import org.apache.druid.query.expression.TestExprMacroTable;
+import org.apache.druid.query.lookup.LookupExtractorFactoryContainerProvider;
+import org.apache.druid.query.lookup.LookupSerdeModule;
+import org.apache.druid.sql.calcite.aggregation.SqlAggregationModule;
+import org.apache.druid.sql.calcite.expression.builtin.QueryLookupOperatorConversion;
+import org.apache.druid.sql.calcite.external.ExternalOperatorConversion;
+import org.apache.druid.sql.guice.SqlBindings;
+import org.apache.druid.timeline.DataSegment;
+
+import java.util.List;
+
+/**
+ * Create the injector used for {@link CalciteTests#INJECTOR}, but in a way
+ * that is extensible.
+ */
+public class CalciteTestInjectorBuilder extends CoreInjectorBuilder
+{
+  public CalciteTestInjectorBuilder()
+  {
+    super(new StartupInjectorBuilder()
+        .withEmptyProperties()
+        .build());
+    add(
+        new BasicTestModule(),
+        new SqlAggregationModule()
+    );
+  }
+
+  @Override
+  public Injector build()
+  {
+    try {
+      return super.build();
+    }
+    catch (Exception e) {
+      // Catches failures when used as a static initializer.
+      e.printStackTrace();
+      System.exit(1);
+      throw e;
+    }
+  }
+
+  private static class BasicTestModule implements DruidModule
+  {
+    @Override
+    public void configure(Binder binder)
+    {
+      final LookupExtractorFactoryContainerProvider lookupProvider =
+          LookupEnabledTestExprMacroTable.createTestLookupProvider(
+              ImmutableMap.of(
+                  "a", "xa",
+                  "abc", "xabc",
+                  "nosuchkey", "mysteryvalue",
+                  "6", "x6"
+              )
+          );
+
+      binder.bind(ExprMacroTable.class).toInstance(TestExprMacroTable.INSTANCE);
+      binder.bind(DataSegment.PruneSpecsHolder.class).toInstance(DataSegment.PruneSpecsHolder.DEFAULT);
+      binder.bind(LookupExtractorFactoryContainerProvider.class).toInstance(lookupProvider);
+
+      // This Module is just to get a LookupExtractorFactoryContainerProvider with a usable "lookyloo" lookup.
+      binder.bind(LookupExtractorFactoryContainerProvider.class).toInstance(lookupProvider);
+      SqlBindings.addOperatorConversion(binder, QueryLookupOperatorConversion.class);
+
+      // Add "EXTERN" table macro, for CalciteInsertDmlTest.
+      SqlBindings.addOperatorConversion(binder, ExternalOperatorConversion.class);
+    }
+
+    @Override
+    public List getJacksonModules()
+    {
+      return new LookupSerdeModule().getJacksonModules();
+    }
+  }
+}
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java
index 2a67706b3b79..29de594350a6 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java
@@ -19,7 +19,6 @@
 
 package org.apache.druid.sql.calcite.util;
 
-import com.fasterxml.jackson.databind.InjectableValues;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Predicate;
 import com.google.common.base.Suppliers;
@@ -28,7 +27,6 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ListenableFuture;
-import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Key;
 import org.apache.calcite.jdbc.CalciteSchema;
@@ -81,14 +79,10 @@
 import org.apache.druid.query.aggregation.FloatSumAggregatorFactory;
 import org.apache.druid.query.aggregation.LongSumAggregatorFactory;
 import org.apache.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory;
-import org.apache.druid.query.expression.LookupEnabledTestExprMacroTable;
 import org.apache.druid.query.expression.LookupExprMacro;
-import org.apache.druid.query.expression.TestExprMacroTable;
 import org.apache.druid.query.lookup.LookupExtractorFactoryContainerProvider;
-import org.apache.druid.query.lookup.LookupSerdeModule;
 import org.apache.druid.segment.IndexBuilder;
 import org.apache.druid.segment.QueryableIndex;
-import org.apache.druid.segment.TestHelper;
 import org.apache.druid.segment.column.ColumnType;
 import org.apache.druid.segment.column.RowSignature;
 import org.apache.druid.segment.incremental.IncrementalIndexSchema;
@@ -119,9 +113,6 @@
 import org.apache.druid.server.security.NoopEscalator;
 import org.apache.druid.server.security.ResourceType;
 import org.apache.druid.sql.SqlLifecycleFactory;
-import org.apache.druid.sql.calcite.aggregation.SqlAggregationModule;
-import org.apache.druid.sql.calcite.expression.builtin.QueryLookupOperatorConversion;
-import org.apache.druid.sql.calcite.external.ExternalOperatorConversion;
 import org.apache.druid.sql.calcite.planner.DruidOperatorTable;
 import org.apache.druid.sql.calcite.planner.PlannerConfig;
 import org.apache.druid.sql.calcite.planner.PlannerFactory;
@@ -143,7 +134,6 @@
 import org.apache.druid.sql.calcite.schema.ViewSchema;
 import org.apache.druid.sql.calcite.view.DruidViewMacroFactory;
 import org.apache.druid.sql.calcite.view.ViewManager;
-import org.apache.druid.sql.guice.SqlBindings;
 import org.apache.druid.timeline.DataSegment;
 import org.apache.druid.timeline.partition.LinearShardSpec;
 import org.easymock.EasyMock;
@@ -152,6 +142,7 @@
 import org.joda.time.chrono.ISOChronology;
 
 import javax.annotation.Nullable;
+
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -263,41 +254,7 @@ public AuthenticationResult createEscalatedAuthenticationResult()
 
   private static final String TIMESTAMP_COLUMN = "t";
 
-  public static final Injector INJECTOR = Guice.createInjector(
-      binder -> {
-        final LookupExtractorFactoryContainerProvider lookupProvider =
-            LookupEnabledTestExprMacroTable.createTestLookupProvider(
-                ImmutableMap.of(
-                    "a", "xa",
-                    "abc", "xabc",
-                    "nosuchkey", "mysteryvalue",
-                    "6", "x6"
-                )
-            );
-
-        ObjectMapper mapper = TestHelper.makeJsonMapper().registerModules(
-            new LookupSerdeModule().getJacksonModules()
-        );
-        mapper.setInjectableValues(
-            new InjectableValues.Std()
-                .addValue(ExprMacroTable.class.getName(), TestExprMacroTable.INSTANCE)
-                .addValue(ObjectMapper.class.getName(), mapper)
-                .addValue(DataSegment.PruneSpecsHolder.class, DataSegment.PruneSpecsHolder.DEFAULT)
-                .addValue(LookupExtractorFactoryContainerProvider.class.getName(), lookupProvider)
-        );
-        binder.bind(Key.get(ObjectMapper.class, Json.class)).toInstance(
-            mapper
-        );
-
-        // This Module is just to get a LookupExtractorFactoryContainerProvider with a usable "lookyloo" lookup.
-        binder.bind(LookupExtractorFactoryContainerProvider.class).toInstance(lookupProvider);
-        SqlBindings.addOperatorConversion(binder, QueryLookupOperatorConversion.class);
-
-        // Add "EXTERN" table macro, for CalciteInsertDmlTest.
-        SqlBindings.addOperatorConversion(binder, ExternalOperatorConversion.class);
-      },
-      new SqlAggregationModule()
-  );
+  public static final Injector INJECTOR = new CalciteTestInjectorBuilder().build();
 
   private static final InputRowParser> PARSER = new MapInputRowParser(
       new TimeAndDimsParseSpec(

From 8a8620c26048d2238a5e43352b8eb986d7fc703d Mon Sep 17 00:00:00 2001
From: Paul Rogers 
Date: Mon, 1 Aug 2022 18:22:35 -0700
Subject: [PATCH 2/4] Revisions from review comments

---
 .../apache/druid/common/aws/AWSModule.java    | 10 --------
 .../apache/druid/common/gcp/GcpModule.java    | 10 --------
 .../druid/common/gcp/GcpMockModule.java       | 12 ---------
 .../druid/initialization/DruidModule.java     | 17 +++++++------
 .../data/input/s3/S3InputSourceTest.java      |  7 ------
 .../apache/druid/https/SSLContextModule.java  | 10 --------
 .../org/apache/druid/guice/SleepModule.java   | 10 --------
 .../guice/ITTLSCertificateCheckerModule.java  | 11 --------
 .../apache/druid/guice/ExtensionsLoader.java  | 18 +++++++++----
 .../apache/druid/guice/GuiceInjectors.java    |  2 +-
 .../druid/guice/security/DruidAuthModule.java | 10 --------
 .../druid/initialization/Initialization.java  | 25 ++++++++++++++++++-
 .../initialization/ServerInjectorBuilder.java |  8 ++++--
 .../druid/rpc/guice/ServiceClientModule.java  |  9 -------
 .../emitter/ComposingEmitterModule.java       |  8 ------
 .../AuthenticatorMapperModule.java            | 10 --------
 .../AuthorizerMapperModule.java               |  8 ------
 .../ExternalStorageAccessSecurityModule.java  | 11 --------
 .../ServerInjectorBuilderTest.java            |  2 --
 .../druid/guice/QueryablePeonModule.java      | 10 --------
 .../apache/druid/sql/guice/SqlModuleTest.java | 15 ++---------
 21 files changed, 56 insertions(+), 167 deletions(-)

diff --git a/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSModule.java b/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSModule.java
index 79b60c956c34..ba305e4e9dd9 100644
--- a/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSModule.java
+++ b/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSModule.java
@@ -22,16 +22,12 @@
 import com.amazonaws.auth.AWSCredentialsProvider;
 import com.amazonaws.services.ec2.AmazonEC2;
 import com.amazonaws.services.ec2.AmazonEC2Client;
-import com.fasterxml.jackson.databind.Module;
 import com.google.inject.Binder;
 import com.google.inject.Provides;
 import org.apache.druid.guice.JsonConfigProvider;
 import org.apache.druid.guice.LazySingleton;
 import org.apache.druid.initialization.DruidModule;
 
-import java.util.Collections;
-import java.util.List;
-
 public class AWSModule implements DruidModule
 {
   @Override
@@ -56,10 +52,4 @@ public AmazonEC2 getEc2Client(AWSCredentialsProvider credentials)
   {
     return new AmazonEC2Client(credentials);
   }
-
-  @Override
-  public List getJacksonModules()
-  {
-    return Collections.emptyList();
-  }
 }
diff --git a/cloud/gcp-common/src/main/java/org/apache/druid/common/gcp/GcpModule.java b/cloud/gcp-common/src/main/java/org/apache/druid/common/gcp/GcpModule.java
index 56458ee75bcc..f261cc16703b 100644
--- a/cloud/gcp-common/src/main/java/org/apache/druid/common/gcp/GcpModule.java
+++ b/cloud/gcp-common/src/main/java/org/apache/druid/common/gcp/GcpModule.java
@@ -19,14 +19,12 @@
 
 package org.apache.druid.common.gcp;
 
-import com.fasterxml.jackson.databind.Module;
 import com.google.api.client.googleapis.auth.oauth2.GoogleCredential;
 import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport;
 import com.google.api.client.http.HttpRequestInitializer;
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.jackson2.JacksonFactory;
-import com.google.common.collect.ImmutableList;
 import com.google.inject.Binder;
 import com.google.inject.Provides;
 import org.apache.druid.guice.LazySingleton;
@@ -35,23 +33,15 @@
 import java.io.IOException;
 import java.security.GeneralSecurityException;
 import java.util.Collections;
-import java.util.List;
 
 public class GcpModule implements DruidModule
 {
-  @Override
-  public List getJacksonModules()
-  {
-    return ImmutableList.of();
-  }
-
   @Override
   public void configure(Binder binder)
   {
     // Nothing to proactively bind
   }
 
-
   @Provides
   @LazySingleton
   public HttpRequestInitializer getHttpRequestInitializer(HttpTransport transport, JsonFactory factory)
diff --git a/cloud/gcp-common/src/test/java/org/apache/druid/common/gcp/GcpMockModule.java b/cloud/gcp-common/src/test/java/org/apache/druid/common/gcp/GcpMockModule.java
index c1f575416153..b8d6048b4ae7 100644
--- a/cloud/gcp-common/src/test/java/org/apache/druid/common/gcp/GcpMockModule.java
+++ b/cloud/gcp-common/src/test/java/org/apache/druid/common/gcp/GcpMockModule.java
@@ -19,36 +19,24 @@
 
 package org.apache.druid.common.gcp;
 
-import com.fasterxml.jackson.databind.Module;
 import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential;
 import com.google.api.client.http.HttpRequestInitializer;
 import com.google.api.client.http.HttpTransport;
 import com.google.api.client.json.JsonFactory;
 import com.google.api.client.json.jackson2.JacksonFactory;
 import com.google.api.client.testing.http.MockHttpTransport;
-import com.google.common.collect.ImmutableList;
 import com.google.inject.Binder;
 import com.google.inject.Provides;
 import org.apache.druid.guice.LazySingleton;
 import org.apache.druid.initialization.DruidModule;
 
-import java.util.List;
-
 public class GcpMockModule implements DruidModule
 {
-  @Override
-  public List getJacksonModules()
-  {
-    return ImmutableList.of();
-  }
-
   @Override
   public void configure(Binder binder)
   {
-
   }
 
-
   @Provides
   @LazySingleton
   public HttpRequestInitializer mockRequestInitializer(
diff --git a/core/src/main/java/org/apache/druid/initialization/DruidModule.java b/core/src/main/java/org/apache/druid/initialization/DruidModule.java
index b2a42f41e9cd..45d663cf5ce3 100644
--- a/core/src/main/java/org/apache/druid/initialization/DruidModule.java
+++ b/core/src/main/java/org/apache/druid/initialization/DruidModule.java
@@ -22,19 +22,22 @@
 import com.fasterxml.jackson.databind.Module;
 import org.apache.druid.guice.annotations.ExtensionPoint;
 
+import java.util.Collections;
 import java.util.List;
 
 /**
  * A Guice module which also provides Jackson modules.
- * There is generally no need to implement this interface unless
- * you want to define Jackson modules: use the Guice
- * {@code Module} otherwise.
- * 

- * Extension modules extend this interface rather than from - * the Guice {@code Module} interface. + * Extension modules must implement this interface. + * (Enforced in {@code ExtensionInjectorBuilder}). + * Built-in implementations that do not provide Jackson modules can + * implement the simpler {@link com.google.inject.Module Guice Module} + * interface instead. */ @ExtensionPoint public interface DruidModule extends com.google.inject.Module { - List getJacksonModules(); + default List getJacksonModules() + { + return Collections.emptyList(); + } } diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java index 359de18039bd..afca41d61f7e 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java +++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java @@ -1172,16 +1172,9 @@ public AWSCredentialsProvider getAWSCredentialsProvider() return AWSCredentialsUtils.defaultAWSCredentialsProviderChain(null); } - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Override public void configure(Binder binder) { - } } ); diff --git a/extensions-core/simple-client-sslcontext/src/main/java/org/apache/druid/https/SSLContextModule.java b/extensions-core/simple-client-sslcontext/src/main/java/org/apache/druid/https/SSLContextModule.java index e58f483e4c4c..baa91fc4142d 100644 --- a/extensions-core/simple-client-sslcontext/src/main/java/org/apache/druid/https/SSLContextModule.java +++ b/extensions-core/simple-client-sslcontext/src/main/java/org/apache/druid/https/SSLContextModule.java @@ -19,8 +19,6 @@ package org.apache.druid.https; -import com.fasterxml.jackson.databind.Module; -import com.google.common.collect.ImmutableList; import com.google.inject.Binder; import org.apache.druid.guice.JsonConfigProvider; import org.apache.druid.guice.annotations.Client; @@ -31,17 +29,9 @@ import org.apache.druid.server.router.Router; import javax.net.ssl.SSLContext; -import java.util.List; public class SSLContextModule implements DruidModule { - - @Override - public List getJacksonModules() - { - return ImmutableList.of(); - } - @Override public void configure(Binder binder) { diff --git a/extensions-core/testing-tools/src/main/java/org/apache/druid/guice/SleepModule.java b/extensions-core/testing-tools/src/main/java/org/apache/druid/guice/SleepModule.java index ec8231981401..2a861dfa90e9 100644 --- a/extensions-core/testing-tools/src/main/java/org/apache/druid/guice/SleepModule.java +++ b/extensions-core/testing-tools/src/main/java/org/apache/druid/guice/SleepModule.java @@ -19,24 +19,14 @@ package org.apache.druid.guice; -import com.fasterxml.jackson.databind.Module; import com.google.inject.Binder; import org.apache.druid.initialization.DruidModule; import org.apache.druid.query.expressions.SleepExprMacro; import org.apache.druid.query.sql.SleepOperatorConversion; import org.apache.druid.sql.guice.SqlBindings; -import java.util.Collections; -import java.util.List; - public class SleepModule implements DruidModule { - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Override public void configure(Binder binder) { diff --git a/integration-tests/src/main/java/org/apache/druid/testing/guice/ITTLSCertificateCheckerModule.java b/integration-tests/src/main/java/org/apache/druid/testing/guice/ITTLSCertificateCheckerModule.java index 5d2132322290..e992d8437430 100644 --- a/integration-tests/src/main/java/org/apache/druid/testing/guice/ITTLSCertificateCheckerModule.java +++ b/integration-tests/src/main/java/org/apache/druid/testing/guice/ITTLSCertificateCheckerModule.java @@ -19,16 +19,12 @@ package org.apache.druid.testing.guice; -import com.fasterxml.jackson.databind.Module; import com.google.inject.Binder; import com.google.inject.name.Names; import org.apache.druid.initialization.DruidModule; import org.apache.druid.server.security.TLSCertificateChecker; import org.apache.druid.testing.utils.ITTLSCertificateChecker; -import java.util.Collections; -import java.util.List; - public class ITTLSCertificateCheckerModule implements DruidModule { private final ITTLSCertificateChecker INSTANCE = new ITTLSCertificateChecker(); @@ -42,11 +38,4 @@ public void configure(Binder binder) .annotatedWith(Names.named(IT_CHECKER_TYPE)) .toInstance(INSTANCE); } - - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } } - diff --git a/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java b/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java index 2d0d933513e8..fc7a872751f5 100644 --- a/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java +++ b/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java @@ -37,6 +37,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -46,6 +47,13 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; +/** + * Manages the loading of Druid extensions. Used in two cases: for + * CLI extensions from {@code Main}, and for {@code DruidModule} + * extensions during initialization. The design, however, should support + * any kind of extension that may be needed in the future. + * The extensions are cached so that they can be reported by various REST APIs. + */ public class ExtensionsLoader { private static final Logger log = new Logger(ExtensionsLoader.class); @@ -75,23 +83,23 @@ public ExtensionsConfig config() } /** + * Returns a collection of implementations loaded. + * * @param clazz service class * @param the service type - * - * @return Returns a collection of implementations loaded. */ public Collection getLoadedImplementations(Class clazz) { @SuppressWarnings("unchecked") Collection retVal = (Collection) extensions.get(clazz); if (retVal == null) { - return new HashSet<>(); + return Collections.emptySet(); } return retVal; } /** - * @return Returns a collection of implementations loaded. + * @return a collection of implementations loaded. */ public Collection getLoadedModules() { @@ -118,7 +126,7 @@ public Map getLoadersMap() @SuppressWarnings("unchecked") public Collection getFromExtensions(Class serviceClass) { - // Classes classes are loaded once upon first request. Since the class path does + // Classes are loaded once upon first request. Since the class path does // not change during a run, the set of extension classes cannot change once // computed. // diff --git a/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java b/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java index b502cfcd48b4..d0788cbf69e2 100644 --- a/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java +++ b/processing/src/main/java/org/apache/druid/guice/GuiceInjectors.java @@ -25,7 +25,7 @@ import java.util.Collections; /** - * Creates the startup injector. Regained for backward compatibility. + * Creates the startup injector. Retained for backward compatibility. * New code should prefer using {@link StartupInjectorBuilder} */ public class GuiceInjectors diff --git a/server/src/main/java/org/apache/druid/guice/security/DruidAuthModule.java b/server/src/main/java/org/apache/druid/guice/security/DruidAuthModule.java index e9e8fba0f977..5d5430287a28 100644 --- a/server/src/main/java/org/apache/druid/guice/security/DruidAuthModule.java +++ b/server/src/main/java/org/apache/druid/guice/security/DruidAuthModule.java @@ -19,23 +19,13 @@ package org.apache.druid.guice.security; -import com.fasterxml.jackson.databind.Module; import com.google.inject.Binder; import org.apache.druid.guice.JsonConfigProvider; import org.apache.druid.initialization.DruidModule; import org.apache.druid.server.security.AuthConfig; -import java.util.Collections; -import java.util.List; - public class DruidAuthModule implements DruidModule { - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Override public void configure(Binder binder) { diff --git a/server/src/main/java/org/apache/druid/initialization/Initialization.java b/server/src/main/java/org/apache/druid/initialization/Initialization.java index bd54efa7fbb2..1c65e393d2a1 100644 --- a/server/src/main/java/org/apache/druid/initialization/Initialization.java +++ b/server/src/main/java/org/apache/druid/initialization/Initialization.java @@ -24,13 +24,36 @@ import com.google.inject.Module; /** - * Initialize Guice for a server. Clients and tests should use + * Initialize Guice for a server. This is a legacy version, kept for + * compatibility with existing tests. Clients and tests should use * the individual builders to create a non-server environment. + * Clients (and tests) never load extensions, and so do not need + * (and, in fact, should not use) the + * {@link ExtensionInjectorBuilder}. Instead, simple tests can use + * {@link org.apache.druid.guice.StartupInjectorBuilder + * StartupInjectorBuilder} directly, passing in any needed modules. + *

+ * Some tests use modules that rely on the "startup injector" to + * inject values into a module. In that case, tests should use two + * builders: the {@link StartupInjectorBuilder} followed by + * the {@link CoreInjectorBuilder} class to hold extra modules. + * Look for references to {@link CoreInjectorBuilder} to find examples + * of this pattern. + *

+ * In both cases, the injector builders have options to add the full + * set of server modules. Tests should not load those modules. Instead, + * let the injector builders provide just the required set, and then + * explicitly list the (small subset) of modules needed by any given test. + *

+ * The server initialization formerly done here is now done in + * {@link org.apache.druid.cli.GuiceRunnable GuiceRunnable} by way of + * the {@link ServerInjectorBuilder}. */ public class Initialization { // Use individual builders for testing: this method brings in // server-only dependencies, which is generally not desired. + // See class comment for more information. @Deprecated public static Injector makeInjectorWithModules( final Injector baseInjector, diff --git a/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java b/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java index f8f0875008b8..55fba0b08cf0 100644 --- a/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java +++ b/server/src/main/java/org/apache/druid/initialization/ServerInjectorBuilder.java @@ -36,6 +36,12 @@ /** * Initialize Guice for a server. Clients and tests should use * the individual builders to create a non-server environment. + *

+ * This class is in this package for historical reasons. The proper + * place is in the same module as {@code GuiceRunnable} since this + * class should only ever be used by servers. It is here until + * tests are converted to use the builders, and @{link Initialization} + * is deleted. */ public class ServerInjectorBuilder { @@ -52,8 +58,6 @@ public class ServerInjectorBuilder * @param modules modules for this server * @return the injector for the server */ - // TODO: Move to the CLI package, perhaps GuiceRunnable, once - // tests are cleaned up. @VisibleForTesting public static Injector makeServerInjector( final Injector baseInjector, diff --git a/server/src/main/java/org/apache/druid/rpc/guice/ServiceClientModule.java b/server/src/main/java/org/apache/druid/rpc/guice/ServiceClientModule.java index 1f7bb3b654ef..7ad9f26526ee 100644 --- a/server/src/main/java/org/apache/druid/rpc/guice/ServiceClientModule.java +++ b/server/src/main/java/org/apache/druid/rpc/guice/ServiceClientModule.java @@ -19,7 +19,6 @@ package org.apache.druid.rpc.guice; -import com.fasterxml.jackson.databind.Module; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.inject.Binder; import com.google.inject.Provides; @@ -41,8 +40,6 @@ import org.apache.druid.rpc.indexing.OverlordClient; import org.apache.druid.rpc.indexing.OverlordClientImpl; -import java.util.Collections; -import java.util.List; import java.util.concurrent.ScheduledExecutorService; public class ServiceClientModule implements DruidModule @@ -50,12 +47,6 @@ public class ServiceClientModule implements DruidModule private static final int CONNECT_EXEC_THREADS = 4; private static final int OVERLORD_ATTEMPTS = 6; - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Override public void configure(Binder binder) { diff --git a/server/src/main/java/org/apache/druid/server/emitter/ComposingEmitterModule.java b/server/src/main/java/org/apache/druid/server/emitter/ComposingEmitterModule.java index e466319a3b07..dd06e855290b 100644 --- a/server/src/main/java/org/apache/druid/server/emitter/ComposingEmitterModule.java +++ b/server/src/main/java/org/apache/druid/server/emitter/ComposingEmitterModule.java @@ -19,7 +19,6 @@ package org.apache.druid.server.emitter; -import com.fasterxml.jackson.databind.Module; import com.google.common.base.Function; import com.google.common.collect.Lists; import com.google.inject.Binder; @@ -35,7 +34,6 @@ import org.apache.druid.java.util.emitter.core.ComposingEmitter; import org.apache.druid.java.util.emitter.core.Emitter; -import java.util.Collections; import java.util.List; /** @@ -50,12 +48,6 @@ public void configure(Binder binder) JsonConfigProvider.bind(binder, "druid.emitter.composing", ComposingEmitterConfig.class); } - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Provides @ManageLifecycle @Named("composing") diff --git a/server/src/main/java/org/apache/druid/server/initialization/AuthenticatorMapperModule.java b/server/src/main/java/org/apache/druid/server/initialization/AuthenticatorMapperModule.java index f2f2e990fe20..56c17218e94c 100644 --- a/server/src/main/java/org/apache/druid/server/initialization/AuthenticatorMapperModule.java +++ b/server/src/main/java/org/apache/druid/server/initialization/AuthenticatorMapperModule.java @@ -19,7 +19,6 @@ package org.apache.druid.server.initialization; -import com.fasterxml.jackson.databind.Module; import com.google.common.base.Supplier; import com.google.common.collect.Maps; import com.google.inject.Binder; @@ -34,13 +33,11 @@ import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; -import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.server.security.AllowAllAuthenticator; import org.apache.druid.server.security.AuthConfig; import org.apache.druid.server.security.Authenticator; import org.apache.druid.server.security.AuthenticatorMapper; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -50,7 +47,6 @@ public class AuthenticatorMapperModule implements DruidModule { private static final String AUTHENTICATOR_PROPERTIES_FORMAT_STRING = "druid.auth.authenticator.%s"; - private static Logger log = new Logger(AuthenticatorMapperModule.class); @Override public void configure(Binder binder) @@ -62,12 +58,6 @@ public void configure(Binder binder) LifecycleModule.register(binder, AuthenticatorMapper.class); } - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - private static class AuthenticatorMapperProvider implements Provider { private AuthConfig authConfig; diff --git a/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java b/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java index f547d14c5f33..750df5eb7d40 100644 --- a/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java +++ b/server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java @@ -19,7 +19,6 @@ package org.apache.druid.server.initialization; -import com.fasterxml.jackson.databind.Module; import com.google.common.base.Supplier; import com.google.inject.Binder; import com.google.inject.Inject; @@ -39,7 +38,6 @@ import org.apache.druid.server.security.Authorizer; import org.apache.druid.server.security.AuthorizerMapper; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -62,12 +60,6 @@ public void configure(Binder binder) LifecycleModule.register(binder, AuthorizerMapper.class); } - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - private static class AuthorizerMapperProvider implements Provider { private AuthConfig authConfig; diff --git a/server/src/main/java/org/apache/druid/server/initialization/ExternalStorageAccessSecurityModule.java b/server/src/main/java/org/apache/druid/server/initialization/ExternalStorageAccessSecurityModule.java index eda21ed1d30d..6ea774a575dc 100644 --- a/server/src/main/java/org/apache/druid/server/initialization/ExternalStorageAccessSecurityModule.java +++ b/server/src/main/java/org/apache/druid/server/initialization/ExternalStorageAccessSecurityModule.java @@ -19,23 +19,12 @@ package org.apache.druid.server.initialization; -import com.fasterxml.jackson.databind.Module; -import com.google.common.collect.ImmutableList; import com.google.inject.Binder; import org.apache.druid.guice.JsonConfigProvider; import org.apache.druid.initialization.DruidModule; -import java.util.List; - public class ExternalStorageAccessSecurityModule implements DruidModule { - - @Override - public List getJacksonModules() - { - return ImmutableList.of(); - } - @Override public void configure(Binder binder) { diff --git a/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java b/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java index 0bce3d7fac35..736d0604a6b9 100644 --- a/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java +++ b/server/src/test/java/org/apache/druid/initialization/ServerInjectorBuilderTest.java @@ -48,8 +48,6 @@ import java.util.List; import java.util.Set; -// See ExtensionsLoaderTest for tests formerly in this file related -// to extension loading itself. public class ServerInjectorBuilderTest { @Rule diff --git a/services/src/main/java/org/apache/druid/guice/QueryablePeonModule.java b/services/src/main/java/org/apache/druid/guice/QueryablePeonModule.java index 408cbc5bcdd3..0d9c316b3ecf 100644 --- a/services/src/main/java/org/apache/druid/guice/QueryablePeonModule.java +++ b/services/src/main/java/org/apache/druid/guice/QueryablePeonModule.java @@ -19,23 +19,13 @@ package org.apache.druid.guice; -import com.fasterxml.jackson.databind.Module; import com.google.inject.Binder; import org.apache.druid.initialization.DruidModule; import org.apache.druid.server.QueryResource; import org.apache.druid.server.metrics.QueryCountStatsProvider; -import java.util.Collections; -import java.util.List; - public class QueryablePeonModule implements DruidModule { - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Override public void configure(Binder binder) { diff --git a/sql/src/test/java/org/apache/druid/sql/guice/SqlModuleTest.java b/sql/src/test/java/org/apache/druid/sql/guice/SqlModuleTest.java index 4da433e46e6a..640584e71301 100644 --- a/sql/src/test/java/org/apache/druid/sql/guice/SqlModuleTest.java +++ b/sql/src/test/java/org/apache/druid/sql/guice/SqlModuleTest.java @@ -73,8 +73,7 @@ import javax.validation.Validation; import javax.validation.Validator; -import java.util.Collections; -import java.util.List; + import java.util.Map; import java.util.Properties; @@ -148,7 +147,7 @@ public void testDefaultViewManagerBind() Assert.assertNotNull(viewManager); Assert.assertTrue(viewManager instanceof NoopViewManager); } - + @Test public void testNonDefaultViewManagerBind() { @@ -207,12 +206,6 @@ private Injector makeInjectorWithProperties(final Properties props) private static class TestViewManagerModule implements DruidModule { - @Override - public List getJacksonModules() - { - return Collections.emptyList(); - } - @Override public void configure(Binder binder) { @@ -225,7 +218,6 @@ public void configure(Binder binder) private static class BindTestViewManager implements ViewManager { - @Override public void createView( PlannerFactory plannerFactory, @@ -233,7 +225,6 @@ public void createView( String viewSql ) { - } @Override @@ -243,13 +234,11 @@ public void alterView( String viewSql ) { - } @Override public void dropView(String viewName) { - } @Override From 9d951e13a378077bc18ebc7561ff49e935db82f5 Mon Sep 17 00:00:00 2001 From: Paul Rogers Date: Mon, 1 Aug 2022 22:50:35 -0700 Subject: [PATCH 3/4] Build fixes --- cloud/gcp-common/pom.xml | 8 -------- .../org/apache/druid/initialization/Initialization.java | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/cloud/gcp-common/pom.xml b/cloud/gcp-common/pom.xml index e012072786e5..a410336e0218 100644 --- a/cloud/gcp-common/pom.xml +++ b/cloud/gcp-common/pom.xml @@ -52,18 +52,10 @@ jackson-module-guice runtime - - com.google.guava - guava - com.google.inject guice - - com.fasterxml.jackson.core - jackson-databind - com.google.http-client google-http-client diff --git a/server/src/main/java/org/apache/druid/initialization/Initialization.java b/server/src/main/java/org/apache/druid/initialization/Initialization.java index 1c65e393d2a1..adf3136d3b1d 100644 --- a/server/src/main/java/org/apache/druid/initialization/Initialization.java +++ b/server/src/main/java/org/apache/druid/initialization/Initialization.java @@ -35,7 +35,7 @@ *

* Some tests use modules that rely on the "startup injector" to * inject values into a module. In that case, tests should use two - * builders: the {@link StartupInjectorBuilder} followed by + * builders: the {@code StartupInjectorBuilder} followed by * the {@link CoreInjectorBuilder} class to hold extra modules. * Look for references to {@link CoreInjectorBuilder} to find examples * of this pattern. From fc0893b5e159b238c790b609d4399944aa9c639b Mon Sep 17 00:00:00 2001 From: Paul Rogers Date: Wed, 3 Aug 2022 10:12:12 -0700 Subject: [PATCH 4/4] Revisions from review comments --- .../apache/druid/indexing/common/task/HadoopTask.java | 2 -- .../MultiPhaseParallelIndexingRowStatsTest.java | 2 +- .../java/org/apache/druid/guice/ExtensionsLoader.java | 11 ++++++----- .../org/apache/druid/guice/ExtensionsLoaderTest.java | 5 +++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java index 0de13add2113..d07627e1026a 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java @@ -50,8 +50,6 @@ public abstract class HadoopTask extends AbstractBatchIndexTask { private static final Logger log = new Logger(HadoopTask.class); - // Note: static variables mean that this task cannot run in a shared JVM, - // such as the Indexer. static final Injector INJECTOR = new StartupInjectorBuilder().withExtensions().build(); private static final ExtensionsLoader EXTENSIONS_LOADER = ExtensionsLoader.instance(INJECTOR); diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java index 0b8d38e947f8..3c1a7fb5e536 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/MultiPhaseParallelIndexingRowStatsTest.java @@ -108,7 +108,7 @@ public void testHashPartitionRowStats() } @Test - @Ignore("assumes record rates, to be fixed in separate PR") + @Ignore("assumes record rates, to be fixed PR #12852") public void testHashPartitionRowStats_concurrentSubTasks_1() { testHashPartitionRowStats(1); diff --git a/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java b/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java index fc7a872751f5..b76bcb9a2cb4 100644 --- a/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java +++ b/processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java @@ -24,6 +24,7 @@ import org.apache.commons.io.FileUtils; import org.apache.druid.initialization.DruidModule; import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.Pair; import org.apache.druid.java.util.common.logger.Logger; import javax.inject.Inject; @@ -59,7 +60,7 @@ public class ExtensionsLoader private static final Logger log = new Logger(ExtensionsLoader.class); private final ExtensionsConfig extensionsConfig; - private final ConcurrentHashMap loaders = new ConcurrentHashMap<>(); + private final ConcurrentHashMap, URLClassLoader> loaders = new ConcurrentHashMap<>(); /** * Map of loaded extensions, keyed by class (or interface). @@ -107,7 +108,7 @@ public Collection getLoadedModules() } @VisibleForTesting - public Map getLoadersMap() + public Map, URLClassLoader> getLoadersMap() { return loaders; } @@ -196,8 +197,8 @@ public File[] getExtensionFilesToLoad() public URLClassLoader getClassLoaderForExtension(File extension, boolean useExtensionClassloaderFirst) { return loaders.computeIfAbsent( - extension, - theExtension -> makeClassLoaderForExtension(theExtension, useExtensionClassloaderFirst) + Pair.of(extension, useExtensionClassloaderFirst), + k -> makeClassLoaderForExtension(k.lhs, k.rhs) ); } @@ -213,7 +214,7 @@ private static URLClassLoader makeClassLoaderForExtension( int i = 0; for (File jar : jars) { final URL url = jar.toURI().toURL(); - log.debug("added URL[%s] for extension[%s]", url, extension.getName()); + log.debug("added URL [%s] for extension [%s]", url, extension.getName()); urls[i++] = url; } } diff --git a/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java b/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java index ca9fc1cc586e..44b7f06fb3d8 100644 --- a/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java +++ b/processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java @@ -25,6 +25,7 @@ import com.google.inject.Injector; import org.apache.druid.initialization.DruidModule; import org.apache.druid.java.util.common.ISE; +import org.apache.druid.java.util.common.Pair; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -66,7 +67,6 @@ public void test02MakeStartupInjector() Assert.assertSame(extnLoader, ExtensionsLoader.instance(startupInjector)); } - @Test public void test04DuplicateClassLoaderExtensions() throws Exception { @@ -74,8 +74,9 @@ public void test04DuplicateClassLoaderExtensions() throws Exception Injector startupInjector = startupInjector(); ExtensionsLoader extnLoader = ExtensionsLoader.instance(startupInjector); + Pair key = Pair.of(extensionDir, true); extnLoader.getLoadersMap() - .put(extensionDir, new URLClassLoader(new URL[]{}, ExtensionsLoader.class.getClassLoader())); + .put(key, new URLClassLoader(new URL[]{}, ExtensionsLoader.class.getClassLoader())); Collection modules = extnLoader.getFromExtensions(DruidModule.class);