From e110ff740c5c291e29b1f26068c2502fd489b99d Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Tue, 1 Apr 2025 18:58:55 +0100 Subject: [PATCH 01/10] Framework for not starting dev services in augmentation, plus first implementation on Redis --- .../builditem/DevServicesResultBuildItem.java | 98 ++++++++++++++-- .../DevServicesTrackerBuildItem.java | 94 +++++++++++++++ .../deployment/builditem/Startable.java | 7 ++ .../runner/bootstrap/StartupActionImpl.java | 19 +++ .../devservices/common/LazyContainer.java | 41 +++++++ .../deployment/DevServicesProcessor.java | 7 ++ .../client/DevServicesRedisProcessor.java | 108 ++++++++---------- .../quarkus/bootstrap/app/StartupAction.java | 2 +- .../RedisInstrumentationDisabledProfile.java | 3 +- ...esRedisCustomPortReusableServiceITest.java | 2 - .../DevServicesRedisNonUniquePortITest.java | 2 - .../test/junit/NativeDevServicesHandler.java | 8 ++ 12 files changed, 318 insertions(+), 73 deletions(-) create mode 100644 core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java create mode 100644 core/deployment/src/main/java/io/quarkus/deployment/builditem/Startable.java create mode 100644 extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java index 5c59ad4e2ca26..f83c0fa55d1c7 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java @@ -2,27 +2,34 @@ import java.io.Closeable; import java.io.IOException; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; +import org.jboss.logging.Logger; + import io.quarkus.builder.item.MultiBuildItem; /** * BuildItem for running dev services. * Combines injected configs to the application with container id (if it exists). - * + *

* Processors are expected to return this build item not only when the dev service first starts, * but also if a running dev service already exists. - * + *

* {@link RunningDevService} helps to manage the lifecycle of the running dev service. */ public final class DevServicesResultBuildItem extends MultiBuildItem { + private static final Logger log = Logger.getLogger(DevServicesResultBuildItem.class); + private final String name; private final String description; private final String containerId; - private final Map config; + protected final Map config; + protected RunnableDevService runnableDevService; public DevServicesResultBuildItem(String name, String containerId, Map config) { this(name, null, containerId, config); @@ -35,6 +42,12 @@ public DevServicesResultBuildItem(String name, String description, String contai this.config = config; } + public DevServicesResultBuildItem(String name, String description, String containerId, Map config, + RunnableDevService runnableDevService) { + this(name, description, containerId, config); + this.runnableDevService = runnableDevService; + } + public String getName() { return name; } @@ -51,13 +64,22 @@ public Map getConfig() { return config; } + public void start() { + if (runnableDevService != null) { + runnableDevService.start(); + } else { + log.debugf("Not starting %s because runnable dev service is null (it is probably a running dev service.", name); + } + } + public static class RunningDevService implements Closeable { - private final String name; - private final String description; - private final String containerId; - private final Map config; - private final Closeable closeable; + protected final String name; + protected final String description; + protected final String containerId; + protected final Map config; + protected final Closeable closeable; + protected volatile boolean isRunning = true; private static Map mapOf(String key, String value) { Map map = new HashMap<>(); @@ -117,6 +139,7 @@ public boolean isOwner() { public void close() throws IOException { if (this.closeable != null) { this.closeable.close(); + isRunning = false; } } @@ -124,4 +147,63 @@ public DevServicesResultBuildItem toBuildItem() { return new DevServicesResultBuildItem(name, description, containerId, config); } } + + public static class RunnableDevService extends RunningDevService { + final DevServicesTrackerBuildItem tracker; + + private final Startable container; + + public RunnableDevService(String name, String containerId, Startable container, Map config, + DevServicesTrackerBuildItem tracker) { + super(name, containerId, container::close, config); + + this.container = container; + this.tracker = tracker; + isRunning = false; + } + + public boolean isRunning() { + return isRunning; + } + + public void start() { + // We want to do two things; find things with the same config as us to reuse them, and find things with different config to close them + // We figure out if we need to shut down existing redis containers that might have been started in previous profiles or restarts + + List matchedDevServices = tracker.getRunningServices(name, config); + // if the redis containers have already started we just return; if we wanted to be very cautious we could check the entries for an isRunningStatus, but they might be in the wrong classloader, so that's hard work + + if (matchedDevServices == null || matchedDevServices.size() > 0) { + // There isn't a running container that has the right config, we need to do work + // Let's get all the running dev services associated with this feature + Collection unusableDevServices = tracker.getAllServices(name); + if (unusableDevServices != null) { + for (Closeable closeable : unusableDevServices) { + try { + closeable.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + } + + if (container != null) { + container.start(); + // tell the tracker that we started + tracker.addRunningService(name, config, this); + isRunning = true; + } + } + } + + @Override + public void close() throws IOException { + super.close(); + tracker.removeRunningService(name, config, this); + } + + public DevServicesResultBuildItem toBuildItem() { + return new DevServicesResultBuildItem(name, description, containerId, config, this); + } + } } diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java new file mode 100644 index 0000000000000..3fda344f42a11 --- /dev/null +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java @@ -0,0 +1,94 @@ +package io.quarkus.deployment.builditem; + +import java.io.Closeable; +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import io.quarkus.builder.item.SimpleBuildItem; + +// Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type +public final class DevServicesTrackerBuildItem extends SimpleBuildItem { + + // A map of a map of a list? What?!? + // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best + private volatile static Map>> systemInstance = null; + + public DevServicesTrackerBuildItem() { + + if (systemInstance == null) { + try { + Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTrackerBuildItem.class.getName()); + systemInstance = (Map>>) s.getMethod("getBackingMap") + .invoke(null); + } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + } + + /** + * Public so it can be used across classloaders. Should not be used except by this class. + * Invoked reflectively. + */ + public static Map getBackingMap() { + if (systemInstance == null) { + systemInstance = new ConcurrentHashMap<>(); + } + return systemInstance; + } + + public List getRunningServices(String featureName, + Map config) { + Map> services = systemInstance.get(featureName); + if (services != null) { + return services.get(config); + } + return null; + } + + public Set getAllServices(String featureName) { + Map> services = systemInstance.get(featureName); + if (services == null) { + return Set.of(); + } else { + // Flatten + Set ls = new HashSet<>(); + services.values().forEach(ls::addAll); + return ls; + } + } + + public void addRunningService(String name, Map config, + DevServicesResultBuildItem.RunnableDevService service) { + Map> services = systemInstance.get(name); + + if (services == null) { + services = new HashMap<>(); + systemInstance.put(name, services); + } + + // Make a list so that we can add and remove to it + List list = new ArrayList<>(); + list.add(service); + services.put(config, list); + } + + public void removeRunningService(String name, Map config, + Closeable service) { + Map> services = systemInstance.get(name); + + if (services != null) { + List servicesForConfig = services.get(config); + if (servicesForConfig != null) { + servicesForConfig.remove(service); + } + } + } + +} diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/Startable.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/Startable.java new file mode 100644 index 0000000000000..cf94bdfc00190 --- /dev/null +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/Startable.java @@ -0,0 +1,7 @@ +package io.quarkus.deployment.builditem; + +import java.io.Closeable; + +public interface Startable extends Closeable { + void start(); +} diff --git a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java index 17104552b14ef..6c5b53ff19f72 100644 --- a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java +++ b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java @@ -33,6 +33,7 @@ import io.quarkus.deployment.builditem.ApplicationClassNameBuildItem; import io.quarkus.deployment.builditem.DevServicesLauncherConfigResultBuildItem; import io.quarkus.deployment.builditem.DevServicesNetworkIdBuildItem; +import io.quarkus.deployment.builditem.DevServicesResultBuildItem; import io.quarkus.deployment.builditem.GeneratedClassBuildItem; import io.quarkus.deployment.builditem.GeneratedResourceBuildItem; import io.quarkus.deployment.builditem.MainClassBuildItem; @@ -54,6 +55,7 @@ public class StartupActionImpl implements StartupAction { private final String mainClassName; private final String applicationClassName; private final Map devServicesProperties; + private final List devServicesResultBuildItems; private final String devServicesNetworkId; private final List runtimeApplicationShutdownBuildItems; private final List runtimeCloseTasks = new ArrayList<>(); @@ -67,6 +69,9 @@ public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buil this.devServicesNetworkId = extractDevServicesNetworkId(buildResult); this.runtimeApplicationShutdownBuildItems = buildResult.consumeMulti(RuntimeApplicationShutdownBuildItem.class); + devServicesResultBuildItems = buildResult + .consumeMulti(DevServicesResultBuildItem.class); + Map transformedClasses = extractTransformedClasses(buildResult); QuarkusClassLoader baseClassLoader = curatedApplication.getOrCreateBaseRuntimeClassLoader(); QuarkusClassLoader runtimeClassLoader; @@ -99,6 +104,8 @@ public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buil * of the JVM will exit when the app stops. */ public RunningQuarkusApplication runMainClass(String... args) throws Exception { + // Start dev services that weren't started in the augmentation phase + startDevServices(devServicesResultBuildItems); //first we hack around class loading in the fork join pool ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader); @@ -206,6 +213,9 @@ private void doClose() { @Override public int runMainClassBlocking(String... args) throws Exception { + // Start dev services that weren't started in the augmentation phase + startDevServices(devServicesResultBuildItems); + //first we hack around class loading in the fork join pool ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader); @@ -276,10 +286,19 @@ public void overrideConfig(Map config) { RuntimeOverrideConfigSource.setConfig(runtimeClassLoader, config); } + private static void startDevServices(List devServicesResultBuildItems) { + for (DevServicesResultBuildItem devServicesResultBuildItem : devServicesResultBuildItems) { + devServicesResultBuildItem.start(); + } + } + /** * Runs the application, and returns a handle that can be used to shut it down. */ public RunningQuarkusApplication run(String... args) throws Exception { + // Start dev services that weren't started in the augmentation phase + startDevServices(devServicesResultBuildItems); + //first we hack around class loading in the fork join pool ForkJoinClassLoading.setForkJoinClassLoader(runtimeClassLoader); diff --git a/extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java b/extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java new file mode 100644 index 0000000000000..d4abea09464ee --- /dev/null +++ b/extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java @@ -0,0 +1,41 @@ +package io.quarkus.devservices.common; + +import java.io.IOException; +import java.net.ServerSocket; + +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.utility.DockerImageName; + +import io.quarkus.deployment.builditem.Startable; + +public abstract class LazyContainer> extends GenericContainer implements + Startable { + + public LazyContainer(DockerImageName dockerImageName) { + super(dockerImageName); + } + + public void close() { + // Trivial, but needed to avoid ambiguous inheritance + super.close(); + } + + /** + * Testcontainers docs: Because the randomised port mapping happens during container startup, the container must + * be running at the time getMappedPort is called. You may need to ensure that the startup order of components + * in your tests caters for this. + * + * We need the port before the container starts, so invent them ourselves + */ + protected int findFreePort() { + + try (ServerSocket socket = new ServerSocket(0);) { + // Make sure the socket becomes free as soon as we start the close operation + socket.setReuseAddress(true); + return socket.getLocalPort(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + +} diff --git a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java index cfdb47b11dc27..00931301c2672 100644 --- a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java +++ b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java @@ -37,6 +37,7 @@ import com.github.dockerjava.api.model.ContainerNetworkSettings; import io.quarkus.deployment.IsDevelopment; +import io.quarkus.deployment.IsNormal; import io.quarkus.deployment.annotations.BuildProducer; import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.builditem.ConsoleCommandBuildItem; @@ -44,6 +45,7 @@ import io.quarkus.deployment.builditem.DevServicesLauncherConfigResultBuildItem; import io.quarkus.deployment.builditem.DevServicesNetworkIdBuildItem; import io.quarkus.deployment.builditem.DevServicesResultBuildItem; +import io.quarkus.deployment.builditem.DevServicesTrackerBuildItem; import io.quarkus.deployment.builditem.DockerStatusBuildItem; import io.quarkus.deployment.builditem.LaunchModeBuildItem; import io.quarkus.deployment.console.ConsoleCommand; @@ -102,6 +104,11 @@ private Optional getSharedNetworkId() { } } + @BuildStep(onlyIfNot = IsNormal.class) + DevServicesTrackerBuildItem getDevServicesTracker() { + return new DevServicesTrackerBuildItem(); + } + @BuildStep(onlyIf = { IsDevelopment.class }) public List config( DockerStatusBuildItem dockerStatusBuildItem, diff --git a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java index 61977c9604ce0..b63baa0b6456f 100644 --- a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java +++ b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java @@ -4,7 +4,6 @@ import static io.quarkus.devservices.common.Labels.QUARKUS_DEV_SERVICE; import static io.quarkus.runtime.LaunchMode.DEVELOPMENT; -import java.io.Closeable; import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; @@ -17,7 +16,6 @@ import java.util.stream.Collectors; import org.jboss.logging.Logger; -import org.testcontainers.containers.GenericContainer; import org.testcontainers.utility.DockerImageName; import io.quarkus.deployment.Feature; @@ -27,8 +25,10 @@ import io.quarkus.deployment.builditem.CuratedApplicationShutdownBuildItem; import io.quarkus.deployment.builditem.DevServicesComposeProjectBuildItem; import io.quarkus.deployment.builditem.DevServicesResultBuildItem; +import io.quarkus.deployment.builditem.DevServicesResultBuildItem.RunnableDevService; import io.quarkus.deployment.builditem.DevServicesResultBuildItem.RunningDevService; import io.quarkus.deployment.builditem.DevServicesSharedNetworkBuildItem; +import io.quarkus.deployment.builditem.DevServicesTrackerBuildItem; import io.quarkus.deployment.builditem.DockerStatusBuildItem; import io.quarkus.deployment.builditem.LaunchModeBuildItem; import io.quarkus.deployment.console.ConsoleInstalledBuildItem; @@ -38,6 +38,7 @@ import io.quarkus.devservices.common.ComposeLocator; import io.quarkus.devservices.common.ConfigureUtil; import io.quarkus.devservices.common.ContainerLocator; +import io.quarkus.devservices.common.LazyContainer; import io.quarkus.redis.deployment.client.RedisBuildTimeConfig.DevServiceConfiguration; import io.quarkus.redis.runtime.client.config.RedisConfig; import io.quarkus.runtime.LaunchMode; @@ -46,6 +47,9 @@ @BuildSteps(onlyIfNot = IsNormal.class, onlyIf = { DevServicesConfig.Enabled.class }) public class DevServicesRedisProcessor { private static final Logger log = Logger.getLogger(DevServicesRedisProcessor.class); + + private static final String IMAGE_NAME_KEY = "quarkus.redis.devservices.image-name"; + private static final String REDIS_IMAGE = "docker.io/redis:7"; private static final int REDIS_EXPOSED_PORT = 6379; private static final String REDIS_SCHEME = "redis://"; @@ -61,9 +65,6 @@ public class DevServicesRedisProcessor { private static final String QUARKUS = "quarkus."; private static final String DOT = "."; - private static volatile List devServices; - private static volatile Map capturedDevServicesConfiguration; - private static volatile boolean first = true; @BuildStep public List startRedisContainers(LaunchModeBuildItem launchMode, @@ -71,6 +72,7 @@ public List startRedisContainers(LaunchModeBuildItem DevServicesComposeProjectBuildItem composeProjectBuildItem, List devServicesSharedNetworkBuildItem, RedisBuildTimeConfig config, + DevServicesTrackerBuildItem tracker, Optional consoleInstalledBuildItem, CuratedApplicationShutdownBuildItem closeBuildItem, LoggingSetupBuildItem loggingSetupBuildItem, @@ -79,25 +81,10 @@ public List startRedisContainers(LaunchModeBuildItem Map currentDevServicesConfiguration = new HashMap<>(config.additionalDevServices()); currentDevServicesConfiguration.put(RedisConfig.DEFAULT_CLIENT_NAME, config.defaultDevService()); - // figure out if we need to shut down and restart existing redis containers - // if not and the redis containers have already started we just return - if (devServices != null) { - boolean restartRequired = !currentDevServicesConfiguration.equals(capturedDevServicesConfiguration); - if (!restartRequired) { - return devServices.stream().map(RunningDevService::toBuildItem).collect(Collectors.toList()); - } - for (Closeable closeable : devServices) { - try { - closeable.close(); - } catch (Throwable e) { - log.error("Failed to stop redis container", e); - } - } - devServices = null; - capturedDevServicesConfiguration = null; - } + // We cannot assume an augmentation order, so we cannot just check and reuse previous dev services. + // We *could* get the services from the tracker, and short circuit some work. But that short circuit has some risk. + // If the matching RunningDevService was in a different classloader, we'd get a ClassCastException. - capturedDevServicesConfiguration = currentDevServicesConfiguration; List newDevServices = new ArrayList<>(); StartupLogCompressor compressor = new StartupLogCompressor( @@ -108,14 +95,16 @@ public List startRedisContainers(LaunchModeBuildItem String connectionName = entry.getKey(); boolean useSharedNetwork = DevServicesSharedNetworkBuildItem.isSharedNetworkRequired(devServicesConfig, devServicesSharedNetworkBuildItem); - RunningDevService devService = startContainer(dockerStatusBuildItem, composeProjectBuildItem, + + RunningDevService devService = createContainer(dockerStatusBuildItem, composeProjectBuildItem, connectionName, entry.getValue().devservices(), launchMode.getLaunchMode(), - useSharedNetwork, devServicesConfig.timeout()); + useSharedNetwork, devServicesConfig.timeout(), tracker); if (devService == null) { continue; } + newDevServices.add(devService); String configKey = getConfigPrefix(connectionName) + RedisConfig.HOSTS_CONFIG_NAME; log.infof("The %s redis server is ready to accept connections on %s", connectionName, @@ -131,34 +120,15 @@ public List startRedisContainers(LaunchModeBuildItem throw new RuntimeException(t); } - devServices = newDevServices; - - if (first) { - first = false; - Runnable closeTask = () -> { - if (devServices != null) { - for (Closeable closeable : devServices) { - try { - closeable.close(); - } catch (Throwable t) { - log.error("Failed to stop database", t); - } - } - } - first = true; - devServices = null; - capturedDevServicesConfiguration = null; - }; - closeBuildItem.addCloseTask(closeTask, true); - } - return devServices.stream().map(RunningDevService::toBuildItem).collect(Collectors.toList()); + return newDevServices.stream().map(RunningDevService::toBuildItem).collect(Collectors.toList()); } - private RunningDevService startContainer(DockerStatusBuildItem dockerStatusBuildItem, + private RunningDevService createContainer(DockerStatusBuildItem dockerStatusBuildItem, DevServicesComposeProjectBuildItem composeProjectBuildItem, String name, io.quarkus.redis.deployment.client.DevServicesConfig devServicesConfig, LaunchMode launchMode, - boolean useSharedNetwork, Optional timeout) { + boolean useSharedNetwork, Optional timeout, DevServicesTrackerBuildItem tracker) { + if (!devServicesConfig.enabled()) { // explicitly disabled log.debug("Not starting devservices for " + (RedisConfig.isDefaultClient(name) ? "default redis client" : name) @@ -170,7 +140,7 @@ private RunningDevService startContainer(DockerStatusBuildItem dockerStatusBuild boolean needToStart = !ConfigUtils.isPropertyNonEmpty(configPrefix + RedisConfig.HOSTS_CONFIG_NAME); if (!needToStart) { - log.debug("Not starting devservices for " + (RedisConfig.isDefaultClient(name) ? "default redis client" : name) + log.debug("Not starting dev services for " + (RedisConfig.isDefaultClient(name) ? "default redis client" : name) + " as hosts have been provided"); return null; } @@ -186,16 +156,30 @@ private RunningDevService startContainer(DockerStatusBuildItem dockerStatusBuild .asCompatibleSubstituteFor(REDIS_IMAGE); Supplier defaultRedisServerSupplier = () -> { - QuarkusPortRedisContainer redisContainer = new QuarkusPortRedisContainer(dockerImageName, devServicesConfig.port(), - launchMode == DEVELOPMENT ? devServicesConfig.serviceName() : null, - composeProjectBuildItem.getDefaultNetworkId(), + OptionalInt fixedExposedPort = devServicesConfig.port(); + String serviceName = launchMode == DEVELOPMENT ? devServicesConfig.serviceName() : null; + String defaultNetworkId = composeProjectBuildItem.getDefaultNetworkId(); + QuarkusPortRedisContainer redisContainer = new QuarkusPortRedisContainer(dockerImageName, fixedExposedPort, + serviceName, + defaultNetworkId, useSharedNetwork); timeout.ifPresent(redisContainer::withStartupTimeout); redisContainer.withEnv(devServicesConfig.containerEnv()); - redisContainer.start(); String redisHost = REDIS_SCHEME + redisContainer.getHost() + ":" + redisContainer.getPort(); - return new RunningDevService(Feature.REDIS_CLIENT.getName(), redisContainer.getContainerId(), - redisContainer::close, configPrefix + RedisConfig.HOSTS_CONFIG_NAME, redisHost); + + // This config map is what we use for deciding if a container from another profile can be re-used + // TODO ideally the container properties would get put into it in a centralised way, but the RunnableDevService object doesn't get passed detailed information about the container + Map config = new HashMap(); + config.put(configPrefix + RedisConfig.HOSTS_CONFIG_NAME, redisHost); + config.put(IMAGE_NAME_KEY, dockerImageName.asCanonicalNameString()); + + RunnableDevService answer = new RunnableDevService( + Feature.REDIS_CLIENT.getName(), + redisContainer.getContainerId(), + redisContainer, config, tracker); + + return answer; + }; return redisContainerLocator.locateContainer(devServicesConfig.serviceName(), devServicesConfig.shared(), launchMode) @@ -204,7 +188,9 @@ private RunningDevService startContainer(DockerStatusBuildItem dockerStatusBuild REDIS_EXPOSED_PORT, launchMode, useSharedNetwork)) .map(containerAddress -> { String redisUrl = REDIS_SCHEME + containerAddress.getUrl(); - return new RunningDevService(Feature.REDIS_CLIENT.getName(), containerAddress.getId(), + // If there's a pre-existing container, it's already running, so create a running container, not a runnable one + return new RunningDevService(Feature.REDIS_CLIENT.getName(), + containerAddress.getId(), null, configPrefix + RedisConfig.HOSTS_CONFIG_NAME, redisUrl); }) .orElseGet(defaultRedisServerSupplier); @@ -218,7 +204,7 @@ private String getConfigPrefix(String name) { return configPrefix; } - private static class QuarkusPortRedisContainer extends GenericContainer { + private static class QuarkusPortRedisContainer extends LazyContainer { private final OptionalInt fixedExposedPort; private final boolean useSharedNetwork; @@ -227,7 +213,13 @@ private static class QuarkusPortRedisContainer extends GenericContainer * Before this method is called an appropriate exit handler will likely need to * be set in {@link io.quarkus.runtime.ApplicationLifecycleManager#setDefaultExitCodeHandler(Consumer)} * of the JVM will exit when the app stops. diff --git a/integration-tests/opentelemetry-redis-instrumentation/src/test/java/io/quarkus/it/opentelemetry/profile/RedisInstrumentationDisabledProfile.java b/integration-tests/opentelemetry-redis-instrumentation/src/test/java/io/quarkus/it/opentelemetry/profile/RedisInstrumentationDisabledProfile.java index 89071bfc91e8e..1c0a9ec41fd7f 100644 --- a/integration-tests/opentelemetry-redis-instrumentation/src/test/java/io/quarkus/it/opentelemetry/profile/RedisInstrumentationDisabledProfile.java +++ b/integration-tests/opentelemetry-redis-instrumentation/src/test/java/io/quarkus/it/opentelemetry/profile/RedisInstrumentationDisabledProfile.java @@ -9,9 +9,8 @@ public class RedisInstrumentationDisabledProfile implements QuarkusTestProfile { @Override public Map getConfigOverrides() { - Map overrides = new HashMap(); + Map overrides = new HashMap(); overrides.put("quarkus.otel.instrument.vertx-redis-client", "false"); - overrides.put("quarkus.redis.devservices.port", "4001"); // Workaround for #45785; the dev services for distinct dev services cannot share a port return overrides; } diff --git a/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisCustomPortReusableServiceITest.java b/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisCustomPortReusableServiceITest.java index d81ce499d5136..953970471e572 100644 --- a/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisCustomPortReusableServiceITest.java +++ b/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisCustomPortReusableServiceITest.java @@ -1,7 +1,6 @@ package io.quarkus.redis.devservices.it; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -10,7 +9,6 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; -@Disabled("Tracked by https://github.com/quarkusio/quarkus/issues/45785") @QuarkusTest @TestProfile(DevServicesCustomPortReusableServiceProfile.class) public class DevServicesRedisCustomPortReusableServiceITest { diff --git a/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisNonUniquePortITest.java b/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisNonUniquePortITest.java index bd716219ca0ec..91cd86ea51620 100644 --- a/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisNonUniquePortITest.java +++ b/integration-tests/redis-devservices/src/test/java/io/quarkus/redis/devservices/it/DevServicesRedisNonUniquePortITest.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -15,7 +14,6 @@ import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; -@Disabled("Tracked by https://github.com/quarkusio/quarkus/issues/45785") @QuarkusTest @TestProfile(DevServicesNonUniquePortProfile.class) public class DevServicesRedisNonUniquePortITest { diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java index 8004e26e566fe..01509b369e595 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java @@ -1,10 +1,12 @@ package io.quarkus.test.junit; +import java.util.List; import java.util.function.BiConsumer; import io.quarkus.builder.BuildResult; import io.quarkus.deployment.builditem.DevServicesLauncherConfigResultBuildItem; import io.quarkus.deployment.builditem.DevServicesNetworkIdBuildItem; +import io.quarkus.deployment.builditem.DevServicesResultBuildItem; public class NativeDevServicesHandler implements BiConsumer { @Override @@ -20,5 +22,11 @@ public void accept(Object o, BuildResult buildResult) { if (compose != null && compose.getNetworkId() != null) { propertyConsumer.accept("quarkus.test.container.network", compose.getNetworkId()); } + + List devServicesResultBuildItems = buildResult + .consumeMulti(DevServicesResultBuildItem.class); + for (DevServicesResultBuildItem devServicesResultBuildItem : devServicesResultBuildItems) { + devServicesResultBuildItem.start(); + } } } From d4c996704ca0ef5d08c6ce3770b3f9b6e1d35489 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Wed, 30 Apr 2025 12:05:32 +0100 Subject: [PATCH 02/10] Drop unused argument --- .../redis/deployment/client/DevServicesRedisProcessor.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java index b63baa0b6456f..df01081b95064 100644 --- a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java +++ b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java @@ -22,7 +22,6 @@ import io.quarkus.deployment.IsNormal; import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.annotations.BuildSteps; -import io.quarkus.deployment.builditem.CuratedApplicationShutdownBuildItem; import io.quarkus.deployment.builditem.DevServicesComposeProjectBuildItem; import io.quarkus.deployment.builditem.DevServicesResultBuildItem; import io.quarkus.deployment.builditem.DevServicesResultBuildItem.RunnableDevService; @@ -74,7 +73,6 @@ public List startRedisContainers(LaunchModeBuildItem RedisBuildTimeConfig config, DevServicesTrackerBuildItem tracker, Optional consoleInstalledBuildItem, - CuratedApplicationShutdownBuildItem closeBuildItem, LoggingSetupBuildItem loggingSetupBuildItem, DevServicesConfig devServicesConfig) { From 4bdfde5d5d9333c4ce909421500117cf429b5a89 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Wed, 30 Apr 2025 17:33:38 +0100 Subject: [PATCH 03/10] Switch to using ConfigSource for lazy config --- bom/application/pom.xml | 5 ++ .../builditem/DevServicesResultBuildItem.java | 20 +++++- .../DevServicesTrackerBuildItem.java | 35 +++------- .../quarkus/runtime/DevServicesTracker.java | 66 ++++++++++++++++++ .../devservices/common/LazyContainer.java | 41 ------------ extensions/devservices/deployment/pom.xml | 4 ++ .../deployment/DevServicesProcessor.java | 11 +++ extensions/devservices/pom.xml | 1 + extensions/devservices/runtime/pom.xml | 67 +++++++++++++++++++ .../deployment/DevServicesConfigBuilder.java | 20 ++++++ .../deployment/DevServicesConfigSource.java | 40 +++++++++++ .../client/DevServicesRedisProcessor.java | 32 +++++---- extensions/redis-client/runtime/pom.xml | 4 ++ .../menu/ConfigurationProcessor.java | 1 + 14 files changed, 264 insertions(+), 83 deletions(-) create mode 100644 core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java delete mode 100644 extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java create mode 100644 extensions/devservices/runtime/pom.xml create mode 100644 extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java create mode 100644 extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java diff --git a/bom/application/pom.xml b/bom/application/pom.xml index 1f71bd99bcce5..6a5ad545c6f9d 100644 --- a/bom/application/pom.xml +++ b/bom/application/pom.xml @@ -839,6 +839,11 @@ quarkus-devservices-common ${project.version} + + io.quarkus + quarkus-devservices + ${project.version} + io.quarkus quarkus-devservices-deployment diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java index f83c0fa55d1c7..303b258ab7f26 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java @@ -7,6 +7,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import org.jboss.logging.Logger; @@ -148,18 +149,20 @@ public DevServicesResultBuildItem toBuildItem() { } } - public static class RunnableDevService extends RunningDevService { + public static class RunnableDevService extends RunningDevService implements Supplier> { final DevServicesTrackerBuildItem tracker; private final Startable container; + private Map lazyConfig; public RunnableDevService(String name, String containerId, Startable container, Map config, - DevServicesTrackerBuildItem tracker) { + Map lazyConfig, DevServicesTrackerBuildItem tracker) { super(name, containerId, container::close, config); this.container = container; this.tracker = tracker; isRunning = false; + this.lazyConfig = lazyConfig; } public boolean isRunning() { @@ -175,7 +178,7 @@ public void start() { if (matchedDevServices == null || matchedDevServices.size() > 0) { // There isn't a running container that has the right config, we need to do work - // Let's get all the running dev services associated with this feature + // Let's get all the running dev services associated with this feature, so we can close them Collection unusableDevServices = tracker.getAllServices(name); if (unusableDevServices != null) { for (Closeable closeable : unusableDevServices) { @@ -205,5 +208,16 @@ public void close() throws IOException { public DevServicesResultBuildItem toBuildItem() { return new DevServicesResultBuildItem(name, description, containerId, config, this); } + + @Override + public Map get() { + // TODO printlns show this gets called way too often - does specifying the properties cut that down? + Map config = getConfig(); + Map newConfig = new HashMap<>(config); + for (Map.Entry entry : lazyConfig.entrySet()) { + newConfig.put(entry.getKey(), entry.getValue().get()); + } + return newConfig; + } } } diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java index 3fda344f42a11..19055ccd24091 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java @@ -1,46 +1,27 @@ package io.quarkus.deployment.builditem; import java.io.Closeable; -import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import io.quarkus.builder.item.SimpleBuildItem; +import io.quarkus.runtime.DevServicesTracker; // Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type +// Needs to be in core because DevServicesResultBuildItem is in core public final class DevServicesTrackerBuildItem extends SimpleBuildItem { // A map of a map of a list? What?!? // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best - private volatile static Map>> systemInstance = null; + private final Map>> systemInstance; public DevServicesTrackerBuildItem() { - if (systemInstance == null) { - try { - Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTrackerBuildItem.class.getName()); - systemInstance = (Map>>) s.getMethod("getBackingMap") - .invoke(null); - } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - } - - /** - * Public so it can be used across classloaders. Should not be used except by this class. - * Invoked reflectively. - */ - public static Map getBackingMap() { - if (systemInstance == null) { - systemInstance = new ConcurrentHashMap<>(); - } - return systemInstance; + systemInstance = new DevServicesTracker().getBackingMap(); } public List getRunningServices(String featureName, @@ -64,7 +45,7 @@ public Set getAllServices(String featureName) { } } - public void addRunningService(String name, Map config, + public void addRunningService(String name, Map identifyingConfig, DevServicesResultBuildItem.RunnableDevService service) { Map> services = systemInstance.get(name); @@ -76,15 +57,15 @@ public void addRunningService(String name, Map config, // Make a list so that we can add and remove to it List list = new ArrayList<>(); list.add(service); - services.put(config, list); + services.put(identifyingConfig, list); } - public void removeRunningService(String name, Map config, + public void removeRunningService(String name, Map identifyingConfig, Closeable service) { Map> services = systemInstance.get(name); if (services != null) { - List servicesForConfig = services.get(config); + List servicesForConfig = services.get(identifyingConfig); if (servicesForConfig != null) { servicesForConfig.remove(service); } diff --git a/core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java b/core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java new file mode 100644 index 0000000000000..0b2cc1d163a80 --- /dev/null +++ b/core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java @@ -0,0 +1,66 @@ +package io.quarkus.runtime; + +import java.io.Closeable; +import java.lang.reflect.InvocationTargetException; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; + +// Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type +// Needs to be in core because DevServicesResultBuildItem is in core +public final class DevServicesTracker { + + // HACK! We share a backing map with the build item + + // A map of a map of a list? What?!? + // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best + private volatile static Map>> systemInstance = null; + + public DevServicesTracker() { + + if (systemInstance == null) { + try { + Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTracker.class.getName()); + systemInstance = (Map>>) s.getMethod("getBackingMap") + .invoke(null); + } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + } + + /** + * Public so it can be used across classloaders. Should not be used except by this class, and by the + * DevServicesTrackerBuildItem, which shares this map. + * Invoked reflectively. + */ + public static Map getBackingMap() { + if (systemInstance == null) { + systemInstance = new ConcurrentHashMap<>(); + } + return systemInstance; + } + + public Set> getAllRunningServices() { + Collection>> services = systemInstance.values(); + if (services == null) { + return Set.of(); + } else { + Set> set = new HashSet<>(); + for (Map> service : services) { + // Flatten and recklessly cast, because the types needed for config do not match those needed for lifecycle management + for (List list : service.values()) { + for (Closeable o : list) { + set.add((Supplier) o); + } + } + } + return set; + + } + } +} diff --git a/extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java b/extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java deleted file mode 100644 index d4abea09464ee..0000000000000 --- a/extensions/devservices/common/src/main/java/io/quarkus/devservices/common/LazyContainer.java +++ /dev/null @@ -1,41 +0,0 @@ -package io.quarkus.devservices.common; - -import java.io.IOException; -import java.net.ServerSocket; - -import org.testcontainers.containers.GenericContainer; -import org.testcontainers.utility.DockerImageName; - -import io.quarkus.deployment.builditem.Startable; - -public abstract class LazyContainer> extends GenericContainer implements - Startable { - - public LazyContainer(DockerImageName dockerImageName) { - super(dockerImageName); - } - - public void close() { - // Trivial, but needed to avoid ambiguous inheritance - super.close(); - } - - /** - * Testcontainers docs: Because the randomised port mapping happens during container startup, the container must - * be running at the time getMappedPort is called. You may need to ensure that the startup order of components - * in your tests caters for this. - * - * We need the port before the container starts, so invent them ourselves - */ - protected int findFreePort() { - - try (ServerSocket socket = new ServerSocket(0);) { - // Make sure the socket becomes free as soon as we start the close operation - socket.setReuseAddress(true); - return socket.getLocalPort(); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - -} diff --git a/extensions/devservices/deployment/pom.xml b/extensions/devservices/deployment/pom.xml index 64f2514c45562..6938d372cb209 100644 --- a/extensions/devservices/deployment/pom.xml +++ b/extensions/devservices/deployment/pom.xml @@ -13,6 +13,10 @@ Quarkus - DevServices - Deployment + + io.quarkus + quarkus-devservices + io.quarkus quarkus-devservices-common diff --git a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java index 00931301c2672..60e66a1d88495 100644 --- a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java +++ b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java @@ -48,6 +48,7 @@ import io.quarkus.deployment.builditem.DevServicesTrackerBuildItem; import io.quarkus.deployment.builditem.DockerStatusBuildItem; import io.quarkus.deployment.builditem.LaunchModeBuildItem; +import io.quarkus.deployment.builditem.RunTimeConfigBuilderBuildItem; import io.quarkus.deployment.console.ConsoleCommand; import io.quarkus.deployment.console.ConsoleStateManager; import io.quarkus.deployment.dev.devservices.ContainerInfo; @@ -109,6 +110,16 @@ DevServicesTrackerBuildItem getDevServicesTracker() { return new DevServicesTrackerBuildItem(); } + @BuildStep + public RunTimeConfigBuilderBuildItem registerDevResourcesConfigSource( + List devServicesResults) { + // We want to track two kinds of things; + // one is dev services from a previous run that a processor might want to re - use, and the other is dev services from a previous run that a processor might want to close + // The getting is different, but the setting can be the same + + return new RunTimeConfigBuilderBuildItem(DevServicesConfigBuilder.class); + } + @BuildStep(onlyIf = { IsDevelopment.class }) public List config( DockerStatusBuildItem dockerStatusBuildItem, diff --git a/extensions/devservices/pom.xml b/extensions/devservices/pom.xml index fc3e4fae0d123..53608f49bc997 100644 --- a/extensions/devservices/pom.xml +++ b/extensions/devservices/pom.xml @@ -28,6 +28,7 @@ oracle common deployment + runtime keycloak oidc diff --git a/extensions/devservices/runtime/pom.xml b/extensions/devservices/runtime/pom.xml new file mode 100644 index 0000000000000..e8d725ea20959 --- /dev/null +++ b/extensions/devservices/runtime/pom.xml @@ -0,0 +1,67 @@ + + + 4.0.0 + + quarkus-devservices-parent + io.quarkus + 999-SNAPSHOT + + quarkus-devservices + Quarkus - DevServices + + Support for Dev Services + + + + io.quarkus + quarkus-arc + + + io.quarkus + quarkus-core + + + + + + + + + + io.quarkus + quarkus-extension-maven-plugin + + + generate-extension-descriptor + + extension-descriptor + + process-resources + + ${project.groupId}:${project.artifactId}-deployment:${project.version} + + + + + + maven-compiler-plugin + + + default-compile + + + + io.quarkus + quarkus-extension-processor + ${project.version} + + + + + + + + + diff --git a/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java b/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java new file mode 100644 index 0000000000000..d27916713bd88 --- /dev/null +++ b/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java @@ -0,0 +1,20 @@ +package io.quarkus.devservices.deployment; + +import io.quarkus.runtime.configuration.ConfigBuilder; +import io.smallrye.config.SmallRyeConfigBuilder; + +public class DevServicesConfigBuilder implements ConfigBuilder { + + @Override + public SmallRyeConfigBuilder configBuilder(SmallRyeConfigBuilder builder) { + return builder.withSources(new io.quarkus.devservices.deployment.DevServicesConfigSource()); + } + + @Override + public int priority() { + // We feel more important than config in environment variables and application.properties files, but dev services should be looking at those sources and not doing anything if there's existing config, + // so a very low priority is also arguably correct. + + return 500; + } +} diff --git a/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java b/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java new file mode 100644 index 0000000000000..263faf42bf0b5 --- /dev/null +++ b/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java @@ -0,0 +1,40 @@ +package io.quarkus.devservices.deployment; + +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; + +import org.eclipse.microprofile.config.spi.ConfigSource; + +import io.quarkus.runtime.DevServicesTracker; + +public class DevServicesConfigSource implements ConfigSource { + + DevServicesTracker devServiceTrackerBuildItem = new DevServicesTracker(); + + @Override + public Set getPropertyNames() { + return Set.of(); + } + + @Override + public String getValue(String propertyName) { + for (Object o : devServiceTrackerBuildItem.getAllRunningServices()) { + Map config = (Map) ((Supplier) o).get(); + return (String) config.get(propertyName); + + } + return null; + } + + @Override + public String getName() { + return "DevServicesConfigSource"; + } + + @Override + public int getOrdinal() { + // greater than any default Microprofile ConfigSource + return 500; + } +} diff --git a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java index df01081b95064..d1dcaf94d623d 100644 --- a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java +++ b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java @@ -16,6 +16,7 @@ import java.util.stream.Collectors; import org.jboss.logging.Logger; +import org.testcontainers.containers.GenericContainer; import org.testcontainers.utility.DockerImageName; import io.quarkus.deployment.Feature; @@ -30,6 +31,7 @@ import io.quarkus.deployment.builditem.DevServicesTrackerBuildItem; import io.quarkus.deployment.builditem.DockerStatusBuildItem; import io.quarkus.deployment.builditem.LaunchModeBuildItem; +import io.quarkus.deployment.builditem.Startable; import io.quarkus.deployment.console.ConsoleInstalledBuildItem; import io.quarkus.deployment.console.StartupLogCompressor; import io.quarkus.deployment.dev.devservices.DevServicesConfig; @@ -37,7 +39,6 @@ import io.quarkus.devservices.common.ComposeLocator; import io.quarkus.devservices.common.ConfigureUtil; import io.quarkus.devservices.common.ContainerLocator; -import io.quarkus.devservices.common.LazyContainer; import io.quarkus.redis.deployment.client.RedisBuildTimeConfig.DevServiceConfiguration; import io.quarkus.redis.runtime.client.config.RedisConfig; import io.quarkus.runtime.LaunchMode; @@ -163,18 +164,27 @@ private RunningDevService createContainer(DockerStatusBuildItem dockerStatusBuil useSharedNetwork); timeout.ifPresent(redisContainer::withStartupTimeout); redisContainer.withEnv(devServicesConfig.containerEnv()); - String redisHost = REDIS_SCHEME + redisContainer.getHost() + ":" + redisContainer.getPort(); + String redisHost = fixedExposedPort.isPresent() + ? REDIS_SCHEME + redisContainer.getHost() + ":" + redisContainer.getPort() + : null; // This config map is what we use for deciding if a container from another profile can be re-used // TODO ideally the container properties would get put into it in a centralised way, but the RunnableDevService object doesn't get passed detailed information about the container Map config = new HashMap(); - config.put(configPrefix + RedisConfig.HOSTS_CONFIG_NAME, redisHost); + if (fixedExposedPort.isPresent()) { + config.put(configPrefix + RedisConfig.HOSTS_CONFIG_NAME, redisHost); + } config.put(IMAGE_NAME_KEY, dockerImageName.asCanonicalNameString()); + Map dynamicConfig = new HashMap(); + Supplier hoster = () -> REDIS_SCHEME + redisContainer.getHost() + ":" + redisContainer.getPort(); + dynamicConfig.put(configPrefix + RedisConfig.HOSTS_CONFIG_NAME, hoster); + RunnableDevService answer = new RunnableDevService( Feature.REDIS_CLIENT.getName(), redisContainer.getContainerId(), - redisContainer, config, tracker); + (Startable) redisContainer, config, dynamicConfig, tracker); + // TODO is this case right? or make everyone implement it? Or have an interface of our own? return answer; @@ -202,7 +212,7 @@ private String getConfigPrefix(String name) { return configPrefix; } - private static class QuarkusPortRedisContainer extends LazyContainer { + private static class QuarkusPortRedisContainer extends GenericContainer implements Startable { private final OptionalInt fixedExposedPort; private final boolean useSharedNetwork; @@ -211,13 +221,7 @@ private static class QuarkusPortRedisContainer extends LazyContainerio.smallrye.reactive smallrye-mutiny-vertx-redis-client + + io.quarkus + quarkus-devservices + io.quarkus diff --git a/extensions/vertx-http/deployment/src/main/java/io/quarkus/devui/deployment/menu/ConfigurationProcessor.java b/extensions/vertx-http/deployment/src/main/java/io/quarkus/devui/deployment/menu/ConfigurationProcessor.java index 7174c9abd7d04..4c29ed564540d 100644 --- a/extensions/vertx-http/deployment/src/main/java/io/quarkus/devui/deployment/menu/ConfigurationProcessor.java +++ b/extensions/vertx-http/deployment/src/main/java/io/quarkus/devui/deployment/menu/ConfigurationProcessor.java @@ -180,6 +180,7 @@ private static String cleanUpAsciiDocIfNecessary(String docs) { .replace("\n", "
"); } + // TODO does this need updating to the config source? Or just deleting? Or consolidating? private static boolean isSetByDevServices(Optional devServicesLauncherConfig, String propertyName) { if (devServicesLauncherConfig.isPresent()) { From ff0ec4dcfce354b52b832cc7b71c17a231eb322e Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Wed, 30 Apr 2025 21:38:05 +0100 Subject: [PATCH 04/10] Abandon devservices/runtime module, clean up ConfigSource implementation --- .../DevServicesTrackerBuildItem.java | 37 ++++++++-- .../config}/DevServicesConfigBuilder.java | 5 +- .../config}/DevServicesConfigSource.java | 7 +- .../runtime/DevServicesConfigTracker.java | 51 ++++++++++++++ .../quarkus/runtime/DevServicesTracker.java | 66 ------------------ extensions/devservices/deployment/pom.xml | 4 -- .../deployment/DevServicesProcessor.java | 1 + extensions/devservices/pom.xml | 1 - extensions/devservices/runtime/pom.xml | 67 ------------------- extensions/redis-client/runtime/pom.xml | 4 -- 10 files changed, 92 insertions(+), 151 deletions(-) rename {extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment => core/runtime/src/main/java/io/quarkus/devservice/runtime/config}/DevServicesConfigBuilder.java (72%) rename {extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment => core/runtime/src/main/java/io/quarkus/devservice/runtime/config}/DevServicesConfigSource.java (72%) create mode 100644 core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java delete mode 100644 core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java delete mode 100644 extensions/devservices/runtime/pom.xml diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java index 19055ccd24091..978070b4f458f 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java @@ -1,15 +1,17 @@ package io.quarkus.deployment.builditem; import java.io.Closeable; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import io.quarkus.builder.item.SimpleBuildItem; -import io.quarkus.runtime.DevServicesTracker; +import io.quarkus.runtime.DevServicesConfigTracker; // Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type // Needs to be in core because DevServicesResultBuildItem is in core @@ -17,11 +19,33 @@ public final class DevServicesTrackerBuildItem extends SimpleBuildItem { // A map of a map of a list? What?!? // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best - private final Map>> systemInstance; + private static Map>> systemInstance = null; + private final DevServicesConfigTracker configTracker; public DevServicesTrackerBuildItem() { - systemInstance = new DevServicesTracker().getBackingMap(); + if (systemInstance == null) { + try { + Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTrackerBuildItem.class.getName()); + systemInstance = (Map>>) s.getMethod("getBackingMap") + .invoke(null); + } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + configTracker = new DevServicesConfigTracker(); + + } + + /** + * Public so it can be used across classloaders. Should not be used except by this class. + * Invoked reflectively. + */ + public static Map getBackingMap() { + if (systemInstance == null) { + systemInstance = new ConcurrentHashMap<>(); + } + return systemInstance; } public List getRunningServices(String featureName, @@ -58,10 +82,12 @@ public void addRunningService(String name, Map identifyingConfig List list = new ArrayList<>(); list.add(service); services.put(identifyingConfig, list); + + configTracker.addRunningService(service); } public void removeRunningService(String name, Map identifyingConfig, - Closeable service) { + DevServicesResultBuildItem.RunnableDevService service) { Map> services = systemInstance.get(name); if (services != null) { @@ -70,6 +96,9 @@ public void removeRunningService(String name, Map identifyingCon servicesForConfig.remove(service); } } + + configTracker.removeRunningService(service); + } } diff --git a/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java similarity index 72% rename from extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java rename to core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java index d27916713bd88..f874426670b9e 100644 --- a/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigBuilder.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java @@ -1,13 +1,14 @@ -package io.quarkus.devservices.deployment; +package io.quarkus.devservice.runtime.config; import io.quarkus.runtime.configuration.ConfigBuilder; import io.smallrye.config.SmallRyeConfigBuilder; +// This should live in the devservices/runtime module, but that module doesn't exist, and adding it is a breaking change public class DevServicesConfigBuilder implements ConfigBuilder { @Override public SmallRyeConfigBuilder configBuilder(SmallRyeConfigBuilder builder) { - return builder.withSources(new io.quarkus.devservices.deployment.DevServicesConfigSource()); + return builder.withSources(new DevServicesConfigSource()); } @Override diff --git a/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java similarity index 72% rename from extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java rename to core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java index 263faf42bf0b5..c0c5562e0b7e8 100644 --- a/extensions/devservices/runtime/src/main/java/io/quarkus/devservices/deployment/DevServicesConfigSource.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java @@ -1,4 +1,4 @@ -package io.quarkus.devservices.deployment; +package io.quarkus.devservice.runtime.config; import java.util.Map; import java.util.Set; @@ -6,11 +6,12 @@ import org.eclipse.microprofile.config.spi.ConfigSource; -import io.quarkus.runtime.DevServicesTracker; +import io.quarkus.runtime.DevServicesConfigTracker; +// This should live in the devservices/runtime module, but that module doesn't exist, and adding it is a breaking change public class DevServicesConfigSource implements ConfigSource { - DevServicesTracker devServiceTrackerBuildItem = new DevServicesTracker(); + DevServicesConfigTracker devServiceTrackerBuildItem = new DevServicesConfigTracker(); @Override public Set getPropertyNames() { diff --git a/core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java b/core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java new file mode 100644 index 0000000000000..24ea425727b1f --- /dev/null +++ b/core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java @@ -0,0 +1,51 @@ +package io.quarkus.runtime; + +import java.lang.reflect.InvocationTargetException; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; + +// Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type +// Needs to be in core because DevServicesResultBuildItem is in core +public final class DevServicesConfigTracker { + + private static Set> systemInstance = null; + + public DevServicesConfigTracker() { + + if (systemInstance == null) { + try { + Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesConfigTracker.class.getName()); + systemInstance = (Set>) s.getMethod("getBackingSet") + .invoke(null); + } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + } + + /** + * Public so it can be used across classloaders. Should not be used except by this class. + * Invoked reflectively. + */ + public static Set getBackingSet() { + if (systemInstance == null) { + systemInstance = new HashSet<>(); + } + return systemInstance; + } + + public Set> getAllRunningServices() { + return Collections.unmodifiableSet(systemInstance); + } + + public void addRunningService(Supplier service) { + systemInstance.add(service); + } + + public void removeRunningService(Supplier service) { + systemInstance.remove(service); + } +} diff --git a/core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java b/core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java deleted file mode 100644 index 0b2cc1d163a80..0000000000000 --- a/core/runtime/src/main/java/io/quarkus/runtime/DevServicesTracker.java +++ /dev/null @@ -1,66 +0,0 @@ -package io.quarkus.runtime; - -import java.io.Closeable; -import java.lang.reflect.InvocationTargetException; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Supplier; - -// Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type -// Needs to be in core because DevServicesResultBuildItem is in core -public final class DevServicesTracker { - - // HACK! We share a backing map with the build item - - // A map of a map of a list? What?!? - // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best - private volatile static Map>> systemInstance = null; - - public DevServicesTracker() { - - if (systemInstance == null) { - try { - Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTracker.class.getName()); - systemInstance = (Map>>) s.getMethod("getBackingMap") - .invoke(null); - } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - } - - /** - * Public so it can be used across classloaders. Should not be used except by this class, and by the - * DevServicesTrackerBuildItem, which shares this map. - * Invoked reflectively. - */ - public static Map getBackingMap() { - if (systemInstance == null) { - systemInstance = new ConcurrentHashMap<>(); - } - return systemInstance; - } - - public Set> getAllRunningServices() { - Collection>> services = systemInstance.values(); - if (services == null) { - return Set.of(); - } else { - Set> set = new HashSet<>(); - for (Map> service : services) { - // Flatten and recklessly cast, because the types needed for config do not match those needed for lifecycle management - for (List list : service.values()) { - for (Closeable o : list) { - set.add((Supplier) o); - } - } - } - return set; - - } - } -} diff --git a/extensions/devservices/deployment/pom.xml b/extensions/devservices/deployment/pom.xml index 6938d372cb209..64f2514c45562 100644 --- a/extensions/devservices/deployment/pom.xml +++ b/extensions/devservices/deployment/pom.xml @@ -13,10 +13,6 @@ Quarkus - DevServices - Deployment - - io.quarkus - quarkus-devservices - io.quarkus quarkus-devservices-common diff --git a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java index 60e66a1d88495..fe42096119660 100644 --- a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java +++ b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java @@ -56,6 +56,7 @@ import io.quarkus.deployment.util.ContainerRuntimeUtil; import io.quarkus.deployment.util.ContainerRuntimeUtil.ContainerRuntime; import io.quarkus.dev.spi.DevModeType; +import io.quarkus.devservice.runtime.config.DevServicesConfigBuilder; import io.quarkus.devservices.common.ContainerUtil; import io.quarkus.devui.spi.buildtime.FooterLogBuildItem; diff --git a/extensions/devservices/pom.xml b/extensions/devservices/pom.xml index 53608f49bc997..fc3e4fae0d123 100644 --- a/extensions/devservices/pom.xml +++ b/extensions/devservices/pom.xml @@ -28,7 +28,6 @@ oracle common deployment - runtime keycloak oidc diff --git a/extensions/devservices/runtime/pom.xml b/extensions/devservices/runtime/pom.xml deleted file mode 100644 index e8d725ea20959..0000000000000 --- a/extensions/devservices/runtime/pom.xml +++ /dev/null @@ -1,67 +0,0 @@ - - - 4.0.0 - - quarkus-devservices-parent - io.quarkus - 999-SNAPSHOT - - quarkus-devservices - Quarkus - DevServices - - Support for Dev Services - - - - io.quarkus - quarkus-arc - - - io.quarkus - quarkus-core - - - - - - - - - - io.quarkus - quarkus-extension-maven-plugin - - - generate-extension-descriptor - - extension-descriptor - - process-resources - - ${project.groupId}:${project.artifactId}-deployment:${project.version} - - - - - - maven-compiler-plugin - - - default-compile - - - - io.quarkus - quarkus-extension-processor - ${project.version} - - - - - - - - - diff --git a/extensions/redis-client/runtime/pom.xml b/extensions/redis-client/runtime/pom.xml index b1b01fb51388e..c010999601a2f 100644 --- a/extensions/redis-client/runtime/pom.xml +++ b/extensions/redis-client/runtime/pom.xml @@ -31,10 +31,6 @@ io.smallrye.reactive smallrye-mutiny-vertx-redis-client - - io.quarkus - quarkus-devservices - io.quarkus From 98c526ad9c74dba2880ca9e0eb4f39589fa756ac Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Wed, 30 Apr 2025 21:42:25 +0100 Subject: [PATCH 05/10] Add comment --- .../deployment/builditem/DevServicesResultBuildItem.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java index 303b258ab7f26..82d85afbb8383 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java @@ -132,6 +132,9 @@ public Closeable getCloseable() { return closeable; } + // This method should be on RunningDevService, but not on RunnableDevService, where we use different logic to + // decide when it's time to close a container. For now, leave it where it is and hope it doesn't get called when it shouldn't. + // We can either make a common parent class or throw unsupported when this is called from Runnable. public boolean isOwner() { return closeable != null; } From 18f4f711bd89189eb798d09bea407ef40087ae53 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Wed, 30 Apr 2025 22:29:34 +0100 Subject: [PATCH 06/10] Tolerate cases where the system classloader does not have access to all quarkus classes --- .../builditem/DevServicesTrackerBuildItem.java | 2 +- .../runtime/config/DevServicesConfigSource.java | 2 +- .../runtime/DevServicesConfigTracker.java | 12 ++++++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) rename core/runtime/src/main/java/io/quarkus/{ => devservices}/runtime/DevServicesConfigTracker.java (80%) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java index 978070b4f458f..db5dcfb175cdd 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java @@ -11,7 +11,7 @@ import java.util.concurrent.ConcurrentHashMap; import io.quarkus.builder.item.SimpleBuildItem; -import io.quarkus.runtime.DevServicesConfigTracker; +import io.quarkus.devservices.runtime.DevServicesConfigTracker; // Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type // Needs to be in core because DevServicesResultBuildItem is in core diff --git a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java index c0c5562e0b7e8..2c71769908c34 100644 --- a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java @@ -6,7 +6,7 @@ import org.eclipse.microprofile.config.spi.ConfigSource; -import io.quarkus.runtime.DevServicesConfigTracker; +import io.quarkus.devservices.runtime.DevServicesConfigTracker; // This should live in the devservices/runtime module, but that module doesn't exist, and adding it is a breaking change public class DevServicesConfigSource implements ConfigSource { diff --git a/core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java b/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java similarity index 80% rename from core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java rename to core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java index 24ea425727b1f..be2c45924fe2e 100644 --- a/core/runtime/src/main/java/io/quarkus/runtime/DevServicesConfigTracker.java +++ b/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java @@ -1,4 +1,4 @@ -package io.quarkus.runtime; +package io.quarkus.devservices.runtime; import java.lang.reflect.InvocationTargetException; import java.util.Collections; @@ -7,10 +7,16 @@ import java.util.Set; import java.util.function.Supplier; +import org.jboss.logging.Logger; + +import io.quarkus.runtime.shutdown.ShutdownRecorder; + // Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type // Needs to be in core because DevServicesResultBuildItem is in core public final class DevServicesConfigTracker { + private static final Logger log = Logger.getLogger(ShutdownRecorder.class); + private static Set> systemInstance = null; public DevServicesConfigTracker() { @@ -21,7 +27,9 @@ public DevServicesConfigTracker() { systemInstance = (Set>) s.getMethod("getBackingSet") .invoke(null); } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw new RuntimeException(e); + // If this happens we can't do cross-profile config tracking, but it's also unlikely to be needed + log.debug(e); + systemInstance = getBackingSet(); } } } From 66f0ee55f9828a2f17bd67607e73100ef4cbfc6f Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Thu, 1 May 2025 12:11:56 +0100 Subject: [PATCH 07/10] Drop priority --- .../deployment/builditem/DevServicesResultBuildItem.java | 3 ++- .../devservice/runtime/config/DevServicesConfigBuilder.java | 5 +++-- .../devservice/runtime/config/DevServicesConfigSource.java | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java index 82d85afbb8383..f08e83aeb0435 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java @@ -176,7 +176,8 @@ public void start() { // We want to do two things; find things with the same config as us to reuse them, and find things with different config to close them // We figure out if we need to shut down existing redis containers that might have been started in previous profiles or restarts - List matchedDevServices = tracker.getRunningServices(name, config); + // These RunnableDevService classes could be from another classloader, so don't make assumptions about the class + List matchedDevServices = tracker.getRunningServices(name, config); // if the redis containers have already started we just return; if we wanted to be very cautious we could check the entries for an isRunningStatus, but they might be in the wrong classloader, so that's hard work if (matchedDevServices == null || matchedDevServices.size() > 0) { diff --git a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java index f874426670b9e..2a518bfa7df48 100644 --- a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java @@ -13,9 +13,10 @@ public SmallRyeConfigBuilder configBuilder(SmallRyeConfigBuilder builder) { @Override public int priority() { - // We feel more important than config in environment variables and application.properties files, but dev services should be looking at those sources and not doing anything if there's existing config, + // What's the right priority? This is a cheeky dynamic override, so a high priority seems correct, but dev services are supposed to fill in gaps in existing information. + // Dev services should be looking at those sources and not doing anything if there's existing config, // so a very low priority is also arguably correct. - return 500; + return 10; } } diff --git a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java index 2c71769908c34..191519416a382 100644 --- a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java @@ -35,7 +35,7 @@ public String getName() { @Override public int getOrdinal() { - // greater than any default Microprofile ConfigSource - return 500; + // See discussion on DevServicesConfigBuilder about what the right value here is + return 10; } } From 2b57db70c7a8466e3e84b9fbcb0631faf87306ff Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Thu, 1 May 2025 23:39:31 +0100 Subject: [PATCH 08/10] Sort out ConfigSource alternative for native builds, half-do Ozan suggestion for avoiding reflection --- .../builditem/DevServicesResultBuildItem.java | 9 ++++++ .../config/DevServicesConfigBuilder.java | 1 + .../config/DevServicesConfigSource.java | 17 +++++++--- .../runtime/DevServicesConfigTracker.java | 31 ++----------------- .../deployment/DevServicesProcessor.java | 8 ++--- .../client/DevServicesRedisProcessor.java | 3 +- .../classloading/QuarkusClassLoader.java | 5 ++- .../test/junit/NativeDevServicesHandler.java | 8 +++++ 8 files changed, 42 insertions(+), 40 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java index f08e83aeb0435..c4d5ada4faf7f 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesResultBuildItem.java @@ -73,6 +73,15 @@ public void start() { } } + // Ideally everyone would use the config source, but if people need to ask for config directly, make it possible + public Map getDynamicConfig() { + if (runnableDevService != null && runnableDevService.isRunning()) { + return runnableDevService.get(); + } else { + return Collections.emptyMap(); + } + } + public static class RunningDevService implements Closeable { protected final String name; diff --git a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java index 2a518bfa7df48..dfd0dc1517a1c 100644 --- a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigBuilder.java @@ -16,6 +16,7 @@ public int priority() { // What's the right priority? This is a cheeky dynamic override, so a high priority seems correct, but dev services are supposed to fill in gaps in existing information. // Dev services should be looking at those sources and not doing anything if there's existing config, // so a very low priority is also arguably correct. + // In principle the priority actually shouldn't matter much, but in practice it needs to not be higher than Arquillian config overrides or some tests fail return 10; } diff --git a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java index 191519416a382..a24ce35c031ce 100644 --- a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java @@ -1,5 +1,6 @@ package io.quarkus.devservice.runtime.config; +import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.function.Supplier; @@ -11,19 +12,25 @@ // This should live in the devservices/runtime module, but that module doesn't exist, and adding it is a breaking change public class DevServicesConfigSource implements ConfigSource { - DevServicesConfigTracker devServiceTrackerBuildItem = new DevServicesConfigTracker(); + DevServicesConfigTracker tracker = new DevServicesConfigTracker(); @Override public Set getPropertyNames() { - return Set.of(); + // We could make this more efficient by not invoking the supplier on the other end, but it would need a more complex interface + Set names = new HashSet<>(); + + for (Supplier o : tracker.getAllRunningServices()) { + Map config = o.get(); + names.addAll(config.keySet()); + } + return names; } @Override public String getValue(String propertyName) { - for (Object o : devServiceTrackerBuildItem.getAllRunningServices()) { - Map config = (Map) ((Supplier) o).get(); + for (Supplier o : tracker.getAllRunningServices()) { + Map config = o.get(); return (String) config.get(propertyName); - } return null; } diff --git a/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java b/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java index be2c45924fe2e..8650aa8f4d4b8 100644 --- a/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java +++ b/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java @@ -1,50 +1,25 @@ package io.quarkus.devservices.runtime; -import java.lang.reflect.InvocationTargetException; import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.function.Supplier; -import org.jboss.logging.Logger; - -import io.quarkus.runtime.shutdown.ShutdownRecorder; - // Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type // Needs to be in core because DevServicesResultBuildItem is in core public final class DevServicesConfigTracker { - private static final Logger log = Logger.getLogger(ShutdownRecorder.class); - - private static Set> systemInstance = null; + private static volatile Set> systemInstance = null; public DevServicesConfigTracker() { - - if (systemInstance == null) { - try { - Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesConfigTracker.class.getName()); - systemInstance = (Set>) s.getMethod("getBackingSet") - .invoke(null); - } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - // If this happens we can't do cross-profile config tracking, but it's also unlikely to be needed - log.debug(e); - systemInstance = getBackingSet(); - } - } - } - - /** - * Public so it can be used across classloaders. Should not be used except by this class. - * Invoked reflectively. - */ - public static Set getBackingSet() { + //This needs to work across classloaders, but the QuarkusClassLoader will load us parent first if (systemInstance == null) { systemInstance = new HashSet<>(); } - return systemInstance; } + // This gets called an awful lot. Should we cache it? public Set> getAllRunningServices() { return Collections.unmodifiableSet(systemInstance); } diff --git a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java index fe42096119660..242e9e9dc5307 100644 --- a/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java +++ b/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java @@ -108,16 +108,16 @@ private Optional getSharedNetworkId() { @BuildStep(onlyIfNot = IsNormal.class) DevServicesTrackerBuildItem getDevServicesTracker() { + // We want to track two kinds of things; + // one is dev services from a previous run that a processor might want to re - use, and the other is dev services from a previous run that a processor might want to close + // The getting is different, but the setting can be the same return new DevServicesTrackerBuildItem(); } @BuildStep public RunTimeConfigBuilderBuildItem registerDevResourcesConfigSource( List devServicesResults) { - // We want to track two kinds of things; - // one is dev services from a previous run that a processor might want to re - use, and the other is dev services from a previous run that a processor might want to close - // The getting is different, but the setting can be the same - + // Once all the dev services are registered, we can share config return new RunTimeConfigBuilderBuildItem(DevServicesConfigBuilder.class); } diff --git a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java index d1dcaf94d623d..3ba559085a737 100644 --- a/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java +++ b/extensions/redis-client/deployment/src/main/java/io/quarkus/redis/deployment/client/DevServicesRedisProcessor.java @@ -183,8 +183,7 @@ private RunningDevService createContainer(DockerStatusBuildItem dockerStatusBuil RunnableDevService answer = new RunnableDevService( Feature.REDIS_CLIENT.getName(), redisContainer.getContainerId(), - (Startable) redisContainer, config, dynamicConfig, tracker); - // TODO is this case right? or make everyone implement it? Or have an interface of our own? + redisContainer, config, dynamicConfig, tracker); return answer; diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index d6becf2f0c108..a6b0f2f4938ff 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -222,7 +222,10 @@ private String sanitizeName(String name) { } private boolean parentFirst(String name, ClassPathResourceIndex classPathResourceIndex) { - return parentFirst || classPathResourceIndex.isParentFirst(name); + // TODO ugly hardcoding + return parentFirst || name.matches("io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.class") + || name.matches("io/quarkus/devservices/runtime/DevServicesConfigTracker.class") + || classPathResourceIndex.isParentFirst(name); } public void reset(Map generatedResources, Map transformedClasses) { diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java index 01509b369e595..55cd30fd65044 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/NativeDevServicesHandler.java @@ -1,6 +1,7 @@ package io.quarkus.test.junit; import java.util.List; +import java.util.Map; import java.util.function.BiConsumer; import io.quarkus.builder.BuildResult; @@ -27,6 +28,13 @@ public void accept(Object o, BuildResult buildResult) { .consumeMulti(DevServicesResultBuildItem.class); for (DevServicesResultBuildItem devServicesResultBuildItem : devServicesResultBuildItems) { devServicesResultBuildItem.start(); + + // It would be nice to use the config source, but since we have the build item right there and this is a one-shot operation, just ask it instead + Map dynamicConfig = devServicesResultBuildItem.getDynamicConfig(); + for (Map.Entry entry : dynamicConfig.entrySet()) { + propertyConsumer.accept(entry.getKey(), entry.getValue()); + + } } } } From 621eae7fb86fe8b7530adb8c8c45a955b6109d16 Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Fri, 2 May 2025 10:15:33 +0100 Subject: [PATCH 09/10] Correct classloading problem --- .../io/quarkus/bootstrap/classloading/QuarkusClassLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index a6b0f2f4938ff..7df6c9e64ad04 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -223,7 +223,7 @@ private String sanitizeName(String name) { private boolean parentFirst(String name, ClassPathResourceIndex classPathResourceIndex) { // TODO ugly hardcoding - return parentFirst || name.matches("io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.class") + return parentFirst || name.matches("io/quarkus/deployment/builditem/DevServicesTrackerHolder.class") || name.matches("io/quarkus/devservices/runtime/DevServicesConfigTracker.class") || classPathResourceIndex.isParentFirst(name); } From ac1a817d92ed5a2c22ef04d05845c895cb236e0b Mon Sep 17 00:00:00 2001 From: Holly Cummins Date: Fri, 2 May 2025 14:07:12 +0100 Subject: [PATCH 10/10] Finish Ozan's suggestion about classloading --- .../DevServicesTrackerBuildItem.java | 82 ++------------- .../config/DevServicesConfigSource.java | 8 +- .../runtime/RunningDevServicesTracker.java | 99 +++++++++++++++++++ .../runtime/DevServicesConfigTracker.java | 34 ------- .../classloading/QuarkusClassLoader.java | 4 +- 5 files changed, 114 insertions(+), 113 deletions(-) create mode 100644 core/runtime/src/main/java/io/quarkus/devservices/crossclassloader/runtime/RunningDevServicesTracker.java delete mode 100644 core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java index db5dcfb175cdd..086abfeb3794d 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/DevServicesTrackerBuildItem.java @@ -1,104 +1,42 @@ package io.quarkus.deployment.builditem; import java.io.Closeable; -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import io.quarkus.builder.item.SimpleBuildItem; -import io.quarkus.devservices.runtime.DevServicesConfigTracker; +import io.quarkus.devservices.crossclassloader.runtime.RunningDevServicesTracker; // Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type // Needs to be in core because DevServicesResultBuildItem is in core public final class DevServicesTrackerBuildItem extends SimpleBuildItem { - // A map of a map of a list? What?!? - // The reason this is like this, rather than being broken out into types, is because this map gets shared between classloaders, so language-level constructs work best - private static Map>> systemInstance = null; - private final DevServicesConfigTracker configTracker; + // This is a fairly thin wrapper around the tracker, so the tracker can be loaded with the system classloader + // The QuarkusClassLoader takes care of loading the tracker with the right classloader + private final RunningDevServicesTracker tracker; public DevServicesTrackerBuildItem() { - - if (systemInstance == null) { - try { - Class s = ClassLoader.getSystemClassLoader().loadClass(DevServicesTrackerBuildItem.class.getName()); - systemInstance = (Map>>) s.getMethod("getBackingMap") - .invoke(null); - } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - configTracker = new DevServicesConfigTracker(); - - } - - /** - * Public so it can be used across classloaders. Should not be used except by this class. - * Invoked reflectively. - */ - public static Map getBackingMap() { - if (systemInstance == null) { - systemInstance = new ConcurrentHashMap<>(); - } - return systemInstance; + tracker = new RunningDevServicesTracker(); } public List getRunningServices(String featureName, - Map config) { - Map> services = systemInstance.get(featureName); - if (services != null) { - return services.get(config); - } - return null; + Map identifyingConfig) { + return tracker.getRunningServices(featureName, identifyingConfig); } public Set getAllServices(String featureName) { - Map> services = systemInstance.get(featureName); - if (services == null) { - return Set.of(); - } else { - // Flatten - Set ls = new HashSet<>(); - services.values().forEach(ls::addAll); - return ls; - } + return tracker.getAllServices(featureName); } public void addRunningService(String name, Map identifyingConfig, DevServicesResultBuildItem.RunnableDevService service) { - Map> services = systemInstance.get(name); - - if (services == null) { - services = new HashMap<>(); - systemInstance.put(name, services); - } - - // Make a list so that we can add and remove to it - List list = new ArrayList<>(); - list.add(service); - services.put(identifyingConfig, list); - - configTracker.addRunningService(service); + tracker.addRunningService(name, identifyingConfig, service); } public void removeRunningService(String name, Map identifyingConfig, DevServicesResultBuildItem.RunnableDevService service) { - Map> services = systemInstance.get(name); - - if (services != null) { - List servicesForConfig = services.get(identifyingConfig); - if (servicesForConfig != null) { - servicesForConfig.remove(service); - } - } - - configTracker.removeRunningService(service); - + tracker.removeRunningService(name, identifyingConfig, service); } } diff --git a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java index a24ce35c031ce..de95268bec2c1 100644 --- a/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java +++ b/core/runtime/src/main/java/io/quarkus/devservice/runtime/config/DevServicesConfigSource.java @@ -7,19 +7,19 @@ import org.eclipse.microprofile.config.spi.ConfigSource; -import io.quarkus.devservices.runtime.DevServicesConfigTracker; +import io.quarkus.devservices.crossclassloader.runtime.RunningDevServicesTracker; // This should live in the devservices/runtime module, but that module doesn't exist, and adding it is a breaking change public class DevServicesConfigSource implements ConfigSource { - DevServicesConfigTracker tracker = new DevServicesConfigTracker(); + RunningDevServicesTracker tracker = new RunningDevServicesTracker(); @Override public Set getPropertyNames() { // We could make this more efficient by not invoking the supplier on the other end, but it would need a more complex interface Set names = new HashSet<>(); - for (Supplier o : tracker.getAllRunningServices()) { + for (Supplier o : tracker.getConfigForAllRunningServices()) { Map config = o.get(); names.addAll(config.keySet()); } @@ -28,7 +28,7 @@ public Set getPropertyNames() { @Override public String getValue(String propertyName) { - for (Supplier o : tracker.getAllRunningServices()) { + for (Supplier o : tracker.getConfigForAllRunningServices()) { Map config = o.get(); return (String) config.get(propertyName); } diff --git a/core/runtime/src/main/java/io/quarkus/devservices/crossclassloader/runtime/RunningDevServicesTracker.java b/core/runtime/src/main/java/io/quarkus/devservices/crossclassloader/runtime/RunningDevServicesTracker.java new file mode 100644 index 0000000000000..3a5409e87b408 --- /dev/null +++ b/core/runtime/src/main/java/io/quarkus/devservices/crossclassloader/runtime/RunningDevServicesTracker.java @@ -0,0 +1,99 @@ +package io.quarkus.devservices.crossclassloader.runtime; + +import java.io.Closeable; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; + +/** + * * Note: This class should only use language-level classes and classes defined in this same package. + * Other Quarkus classes might be in a different classloader. + */ +public class RunningDevServicesTracker { + private static volatile Set> configTracker = null; + + private static Map servicesIndexedByFeature = null; + + public RunningDevServicesTracker() { + //This needs to work across classloaders, but the QuarkusClassLoader will load us parent first + if (configTracker == null) { + configTracker = new HashSet<>(); + } + if (servicesIndexedByFeature == null) { + servicesIndexedByFeature = new HashMap<>(); + } + } + + // This gets called an awful lot. Should we cache it? + public Set> getConfigForAllRunningServices() { + return Collections.unmodifiableSet(configTracker); + } + + public List getRunningServices(String featureName, + Map identifyingConfig) { + DevServicesIndexedByConfig services = servicesIndexedByFeature.get(featureName); + if (services != null) { + return services.get(identifyingConfig); + } + return null; + } + + public Set getAllServices(String featureName) { + DevServicesIndexedByConfig services = servicesIndexedByFeature.get(featureName); + if (services == null) { + return Set.of(); + } else { + // Flatten + Set ls = new HashSet<>(); + services.values().forEach(ls::addAll); + return ls; + } + } + + public void addRunningService(String name, Map identifyingConfig, + Closeable service) { + DevServicesIndexedByConfig services = servicesIndexedByFeature.get(name); + + if (services == null) { + services = new DevServicesIndexedByConfig(); + servicesIndexedByFeature.put(name, services); + } + + // Make a list so that we can add and remove to it + List list = new ArrayList<>(); + list.add(service); + services.put(identifyingConfig, list); + + configTracker.add((Supplier) service); + } + + public void removeRunningService(String name, Map identifyingConfig, + Closeable service) { + DevServicesIndexedByConfig services = servicesIndexedByFeature.get(name); + + if (services != null) { + List servicesForConfig = services.get(identifyingConfig); + if (servicesForConfig != null) { + servicesForConfig.remove(service); + } + } + + configTracker.remove(service); + } + + /** + * Type to give a bit of clarity of intent and avoid some of the thicket of angle brackets. + * The key is a map of identifying config, and the value is a List of RunningDevService objects ... only they might be in a + * different classloader, so we don't call them that. + */ + private static class DevServicesIndexedByConfig extends HashMap> { + public DevServicesIndexedByConfig() { + super(); + } + } +} diff --git a/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java b/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java deleted file mode 100644 index 8650aa8f4d4b8..0000000000000 --- a/core/runtime/src/main/java/io/quarkus/devservices/runtime/DevServicesConfigTracker.java +++ /dev/null @@ -1,34 +0,0 @@ -package io.quarkus.devservices.runtime; - -import java.util.Collections; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; -import java.util.function.Supplier; - -// Ideally we would have a unique build item for each processor/feature, but that would need a new KeyedBuildItem or FeatureBuildItem type -// Needs to be in core because DevServicesResultBuildItem is in core -public final class DevServicesConfigTracker { - - private static volatile Set> systemInstance = null; - - public DevServicesConfigTracker() { - //This needs to work across classloaders, but the QuarkusClassLoader will load us parent first - if (systemInstance == null) { - systemInstance = new HashSet<>(); - } - } - - // This gets called an awful lot. Should we cache it? - public Set> getAllRunningServices() { - return Collections.unmodifiableSet(systemInstance); - } - - public void addRunningService(Supplier service) { - systemInstance.add(service); - } - - public void removeRunningService(Supplier service) { - systemInstance.remove(service); - } -} diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index 7df6c9e64ad04..8dafef075a428 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -222,9 +222,7 @@ private String sanitizeName(String name) { } private boolean parentFirst(String name, ClassPathResourceIndex classPathResourceIndex) { - // TODO ugly hardcoding - return parentFirst || name.matches("io/quarkus/deployment/builditem/DevServicesTrackerHolder.class") - || name.matches("io/quarkus/devservices/runtime/DevServicesConfigTracker.class") + return parentFirst || name.startsWith("io/quarkus/devservices/crossclassloader") || classPathResourceIndex.isParentFirst(name); }