Skip to content

Commit

Permalink
RuntimeConfigDefault changes ignored on restart
Browse files Browse the repository at this point in the history 
Also makes sure datasources restart if devservices properties are
changed.

Fixes #17069
  • Loading branch information
@stuartwdouglas
stuartwdouglas committed Sep 9, 2021
1 parent 0f810e1 commit 554e1c2
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 67 deletions.
44 changes: 15 additions & 29 deletions ...ment/src/main/java/io/quarkus/deployment/configuration/RunTimeConfigurationGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ private RunTimeConfigurationGenerator() {
}

public static final class GenerateOperation implements AutoCloseable {
final boolean devMode;
final boolean liveReloadPossible;
final LaunchMode launchMode;
final AccessorFinder accessorFinder;
final ClassOutput classOutput;
Expand All @@ -283,8 +283,6 @@ public static final class GenerateOperation implements AutoCloseable {
// default values given in the build configuration
final Map<String, String> specifiedRunTimeDefaultValues;
final Map<String, String> buildTimeRunTimeVisibleValues;
// default values produced by extensions via build item
final Map<String, String> runTimeDefaults;
final Map<Container, MethodDescriptor> enclosingMemberMethods = new HashMap<>();
final Map<Class<?>, MethodDescriptor> groupInitMethods = new HashMap<>();
final Map<Class<?>, FieldDescriptor> configRootsByType = new HashMap<>();
Expand Down Expand Up @@ -317,7 +315,7 @@ public static final class GenerateOperation implements AutoCloseable {

GenerateOperation(Builder builder) {
this.launchMode = builder.launchMode;
this.devMode = builder.launchMode == LaunchMode.DEVELOPMENT;
this.liveReloadPossible = builder.liveReloadPossible;
final BuildTimeConfigurationReader.ReadResult buildTimeReadResult = builder.buildTimeReadResult;
buildTimeConfigResult = Assert.checkNotNullParam("buildTimeReadResult", buildTimeReadResult);
specifiedRunTimeDefaultValues = Assert.checkNotNullParam("specifiedRunTimeDefaultValues",
Expand All @@ -326,7 +324,6 @@ public static final class GenerateOperation implements AutoCloseable {
buildTimeReadResult.getBuildTimeRunTimeVisibleValues());
classOutput = Assert.checkNotNullParam("classOutput", builder.getClassOutput());
roots = Assert.checkNotNullParam("builder.roots", builder.getBuildTimeReadResult().getAllRoots());
runTimeDefaults = Assert.checkNotNullParam("runTimeDefaults", builder.getRunTimeDefaults());
additionalTypes = Assert.checkNotNullParam("additionalTypes", builder.getAdditionalTypes());
additionalBootstrapConfigSourceProviders = builder.getAdditionalBootstrapConfigSourceProviders();
staticConfigSources = builder.getStaticConfigSources();
Expand All @@ -343,7 +340,7 @@ public static final class GenerateOperation implements AutoCloseable {
mc.invokeSpecialMethod(MethodDescriptor.ofConstructor(Object.class), mc.getThis());
mc.returnValue(null);
}
if (devMode) {
if (liveReloadPossible) {
reinit = cc.getMethodCreator(REINIT);
reinit.setModifiers(Opcodes.ACC_STATIC | Opcodes.ACC_PUBLIC);
} else {
Expand Down Expand Up @@ -460,7 +457,7 @@ public void run() {
// make the build time config global until we read the run time config -
// at run time (when we're ready) we update the factory and then release the build time config
installConfiguration(clinitConfig, clinit);
if (devMode) {
if (liveReloadPossible) {
final ResultHandle buildTimeRunTimeDefaultValuesConfigSource = reinit
.readStaticField(C_BUILD_TIME_RUN_TIME_DEFAULTS_CONFIG_SOURCE);
// create the map for build time config source
Expand Down Expand Up @@ -540,7 +537,7 @@ public void run() {

// create the map for run time specified values config source
final ResultHandle specifiedRunTimeValues = clinit.newInstance(HM_NEW);
if (!devMode) {
if (!liveReloadPossible) {
//we don't need these in devmode
//including it would just cache the first values
//but these can already just be read directly, as we are in the same JVM
Expand All @@ -549,17 +546,10 @@ public void run() {
clinit.load(entry.getValue()));
}
}
for (Map.Entry<String, String> entry : runTimeDefaults.entrySet()) {
if (!specifiedRunTimeDefaultValues.containsKey(entry.getKey())) {
// only add entry if the user didn't override it
clinit.invokeVirtualMethod(HM_PUT, specifiedRunTimeValues, clinit.load(entry.getKey()),
clinit.load(entry.getValue()));
}
}
final ResultHandle specifiedRunTimeSource = clinit.newInstance(PCS_NEW, specifiedRunTimeValues,
clinit.load("Specified default values"), clinit.load(Integer.MIN_VALUE + 100));
cc.getFieldCreator(C_SPECIFIED_RUN_TIME_CONFIG_SOURCE)
.setModifiers(Opcodes.ACC_STATIC | (devMode ? Opcodes.ACC_VOLATILE : Opcodes.ACC_FINAL));
.setModifiers(Opcodes.ACC_STATIC | (liveReloadPossible ? Opcodes.ACC_VOLATILE : Opcodes.ACC_FINAL));
clinit.writeStaticField(C_SPECIFIED_RUN_TIME_CONFIG_SOURCE, specifiedRunTimeSource);

// add in the custom sources that bootstrap config needs
Expand Down Expand Up @@ -706,7 +696,7 @@ public void run() {
// config root field is volatile in dev mode, final otherwise; we initialize it from clinit, and readConfig in dev mode
cc.getFieldCreator(rootFieldDescriptor)
.setModifiers(Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC
| (devMode ? Opcodes.ACC_VOLATILE : Opcodes.ACC_FINAL));
| (liveReloadPossible ? Opcodes.ACC_VOLATILE : Opcodes.ACC_FINAL));

// construct instance in <clinit>
ResultHandle instance;
Expand All @@ -726,7 +716,7 @@ public void run() {
}
clinit.invokeStaticMethod(initGroup, clinitConfig, clinitNameBuilder, instance);
clinit.invokeVirtualMethod(SB_SET_LENGTH, clinitNameBuilder, clInitOldLen);
if (devMode) {
if (liveReloadPossible) {
instance = readConfig.readStaticField(rootFieldDescriptor);
if (!rootName.isEmpty()) {
readConfig.invokeVirtualMethod(SB_APPEND_CHAR, readConfigNameBuilder, readConfig.load('.'));
Expand Down Expand Up @@ -796,7 +786,7 @@ public void run() {
clinit.invokeStaticMethod(CD_UNKNOWN_PROPERTIES,
clinit.readStaticField(FieldDescriptor.of(cc.getClassName(), "unused", List.class)));

if (devMode) {
if (liveReloadPossible) {
configSweepLoop(siParserBody, readConfig, runTimeConfig);
}
// generate sweep for run time
Expand Down Expand Up @@ -1673,10 +1663,10 @@ public static Builder builder() {
}

public static final class Builder {
public boolean liveReloadPossible;
private LaunchMode launchMode;
private ClassOutput classOutput;
private BuildTimeConfigurationReader.ReadResult buildTimeReadResult;
private Map<String, String> runTimeDefaults;
private List<Class<?>> additionalTypes;
private List<String> additionalBootstrapConfigSourceProviders;

Expand All @@ -1699,6 +1689,11 @@ public Builder setClassOutput(final ClassOutput classOutput) {
return this;
}

public Builder setLiveReloadPossible(boolean liveReloadPossible) {
this.liveReloadPossible = liveReloadPossible;
return this;
}

BuildTimeConfigurationReader.ReadResult getBuildTimeReadResult() {
return buildTimeReadResult;
}
Expand All @@ -1708,15 +1703,6 @@ public Builder setBuildTimeReadResult(final BuildTimeConfigurationReader.ReadRes
return this;
}

Map<String, String> getRunTimeDefaults() {
return runTimeDefaults;
}

public Builder setRunTimeDefaults(final Map<String, String> runTimeDefaults) {
this.runTimeDefaults = runTimeDefaults;
return this;
}

List<Class<?>> getAdditionalTypes() {
return additionalTypes;
}
Expand Down
29 changes: 18 additions & 11 deletions core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import io.quarkus.deployment.builditem.LaunchModeBuildItem;
import io.quarkus.deployment.builditem.LiveReloadBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigurationSourceValueBuildItem;
import io.quarkus.deployment.builditem.StaticInitConfigSourceFactoryBuildItem;
import io.quarkus.deployment.builditem.StaticInitConfigSourceProviderBuildItem;
import io.quarkus.deployment.builditem.SuppressNonRuntimeConfigChangedWarningBuildItem;
Expand All @@ -46,9 +47,10 @@
import io.quarkus.deployment.logging.LoggingSetupBuildItem;
import io.quarkus.gizmo.ClassCreator;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.runtime.LaunchMode;
import io.quarkus.runtime.annotations.ConfigPhase;
import io.quarkus.runtime.annotations.StaticInitSafe;
import io.quarkus.runtime.configuration.ConfigChangeRecorder;
import io.quarkus.runtime.configuration.ConfigRecorder;
import io.quarkus.runtime.configuration.ConfigurationRuntimeConfig;
import io.quarkus.runtime.configuration.RuntimeOverrideConfigSource;
import io.smallrye.config.ConfigSourceFactory;
Expand All @@ -74,13 +76,24 @@ void staticInitSources(
PropertiesLocationConfigSourceFactory.class.getName()));
}

@BuildStep
@Record(ExecutionTime.RUNTIME_INIT)
RunTimeConfigurationSourceValueBuildItem devTestConfig(List<RunTimeConfigurationDefaultBuildItem> runTimeDefaults,
ConfigRecorder recorder) {
if (runTimeDefaults.isEmpty()) {
return null;
}
return new RunTimeConfigurationSourceValueBuildItem(recorder.config(runTimeDefaults.stream().collect(
Collectors.toMap(RunTimeConfigurationDefaultBuildItem::getKey, RunTimeConfigurationDefaultBuildItem::getValue)),
Integer.MIN_VALUE + 50));
}

/**
* Generate the Config class that instantiates MP Config and holds all the config objects
*/
@BuildStep
void generateConfigClass(
ConfigurationBuildItem configItem,
List<RunTimeConfigurationDefaultBuildItem> runTimeDefaults,
List<ConfigurationTypeBuildItem> typeItems,
LaunchModeBuildItem launchModeBuildItem,
BuildProducer<GeneratedClassBuildItem> generatedClass,
Expand All @@ -95,13 +108,6 @@ void generateConfigClass(
return;
}

Map<String, String> defaults = new HashMap<>();
for (RunTimeConfigurationDefaultBuildItem item : runTimeDefaults) {
if (defaults.putIfAbsent(item.getKey(), item.getValue()) != null) {
throw new IllegalStateException("More than one default value for " + item.getKey() + " was produced");
}
}

Set<String> discoveredConfigSources = discoverService(ConfigSource.class, reflectiveClass);
Set<String> discoveredConfigSourceProviders = discoverService(ConfigSourceProvider.class, reflectiveClass);
Set<String> discoveredConfigSourceFactories = discoverService(ConfigSourceFactory.class, reflectiveClass);
Expand All @@ -120,7 +126,8 @@ void generateConfigClass(
.setBuildTimeReadResult(configItem.getReadResult())
.setClassOutput(new GeneratedClassGizmoAdaptor(generatedClass, false))
.setLaunchMode(launchModeBuildItem.getLaunchMode())
.setRunTimeDefaults(defaults)
.setLiveReloadPossible(launchModeBuildItem.getLaunchMode() == LaunchMode.DEVELOPMENT
|| launchModeBuildItem.isAuxiliaryApplication())
.setAdditionalTypes(typeItems.stream().map(ConfigurationTypeBuildItem::getValueType).collect(toList()))
.setAdditionalBootstrapConfigSourceProviders(
getAdditionalBootstrapConfigSourceProviders(additionalBootstrapConfigSourceProviders))
Expand Down Expand Up @@ -157,7 +164,7 @@ public SuppressNonRuntimeConfigChangedWarningBuildItem ignoreQuarkusProfileChang
@BuildStep
@Record(ExecutionTime.RUNTIME_INIT)
public void checkForBuildTimeConfigChange(
ConfigChangeRecorder recorder, ConfigurationBuildItem configItem, LoggingSetupBuildItem loggingSetupBuildItem,
ConfigRecorder recorder, ConfigurationBuildItem configItem, LoggingSetupBuildItem loggingSetupBuildItem,
ConfigurationRuntimeConfig configurationConfig,
List<SuppressNonRuntimeConfigChangedWarningBuildItem> suppressNonRuntimeConfigChangedWarningItems) {
BuildTimeConfigurationReader.ReadResult readResult = configItem.getReadResult();
Expand Down
20 changes: 5 additions & 15 deletions core/deployment/src/main/java/io/quarkus/deployment/steps/DevServicesConfigBuildStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
import io.quarkus.deployment.builditem.DevServicesConfigResultBuildItem;
import io.quarkus.deployment.builditem.DevServicesLauncherConfigResultBuildItem;
import io.quarkus.deployment.builditem.DevServicesNativeConfigResultBuildItem;
import io.quarkus.deployment.builditem.LiveReloadBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem;
import io.quarkus.deployment.builditem.ServiceStartBuildItem;

class DevServicesConfigBuildStep {
static volatile Map<String, String> oldConfig;

@BuildStep
List<DevServicesConfigResultBuildItem> deprecated(List<DevServicesNativeConfigResultBuildItem> items) {
Expand All @@ -30,18 +30,16 @@ List<DevServicesConfigResultBuildItem> deprecated(List<DevServicesNativeConfigRe
@BuildStep
@Produce(ServiceStartBuildItem.class)
DevServicesLauncherConfigResultBuildItem setup(BuildProducer<RunTimeConfigurationDefaultBuildItem> runtimeConfig,
List<DevServicesConfigResultBuildItem> devServicesConfigResultBuildItems,
LiveReloadBuildItem liveReloadBuildItem) {
List<DevServicesConfigResultBuildItem> devServicesConfigResultBuildItems) {
Map<String, String> newProperties = new HashMap<>(devServicesConfigResultBuildItems.stream().collect(
Collectors.toMap(DevServicesConfigResultBuildItem::getKey, DevServicesConfigResultBuildItem::getValue)));
Config config = ConfigProvider.getConfig();
PreviousConfig oldProperties = liveReloadBuildItem.getContextObject(PreviousConfig.class);
//check if there are existing already started dev services
//if there were no changes to the processors they don't produce config
//so we merge existing config from previous runs
//we also check the current config, as the dev service may have been disabled by explicit config
if (oldProperties != null) {
for (Map.Entry<String, String> entry : oldProperties.config.entrySet()) {
if (oldConfig != null) {
for (Map.Entry<String, String> entry : oldConfig.entrySet()) {
if (!newProperties.containsKey(entry.getKey())
&& config.getOptionalValue(entry.getKey(), String.class).isEmpty()) {
newProperties.put(entry.getKey(), entry.getValue());
Expand All @@ -51,15 +49,7 @@ DevServicesLauncherConfigResultBuildItem setup(BuildProducer<RunTimeConfiguratio
for (Map.Entry<String, String> entry : newProperties.entrySet()) {
runtimeConfig.produce(new RunTimeConfigurationDefaultBuildItem(entry.getKey(), entry.getValue()));
}
liveReloadBuildItem.setContextObject(PreviousConfig.class, new PreviousConfig(newProperties));
oldConfig = newProperties;
return new DevServicesLauncherConfigResultBuildItem(Collections.unmodifiableMap(newProperties));
}

static class PreviousConfig {
final Map<String, String> config;

public PreviousConfig(Map<String, String> config) {
this.config = config;
}
}
}
19 changes: 17 additions & 2 deletions ...e/configuration/ConfigChangeRecorder.java → ...runtime/configuration/ConfigRecorder.java
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,27 @@
package io.quarkus.runtime.configuration;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigProvider;
import org.eclipse.microprofile.config.spi.ConfigSource;
import org.eclipse.microprofile.config.spi.ConfigSourceProvider;
import org.jboss.logging.Logger;

import io.quarkus.runtime.RuntimeValue;
import io.quarkus.runtime.annotations.Recorder;
import io.quarkus.runtime.configuration.ConfigurationRuntimeConfig.BuildTimeMismatchAtRuntime;
import io.smallrye.config.common.MapBackedConfigSource;

@Recorder
public class ConfigChangeRecorder {
public class ConfigRecorder {

private static final Logger log = Logger.getLogger(ConfigChangeRecorder.class);
private static final Logger log = Logger.getLogger(ConfigRecorder.class);

public void handleConfigChange(ConfigurationRuntimeConfig configurationConfig, Map<String, String> buildTimeConfig) {
Config configProvider = ConfigProvider.getConfig();
Expand Down Expand Up @@ -50,4 +55,14 @@ public void handleConfigChange(ConfigurationRuntimeConfig configurationConfig, M

}
}

public RuntimeValue<ConfigSourceProvider> config(Map<String, String> values, int priority) {
return new RuntimeValue<>(new ConfigSourceProvider() {
@Override
public Iterable<ConfigSource> getConfigSources(ClassLoader forClassLoader) {
return Collections.singleton(new MapBackedConfigSource("Runtime Default Values", values, priority) {
});
}
});
}
}
7 changes: 7 additions & 0 deletions ...ain/java/io/quarkus/datasource/deployment/devservices/DevServicesDatasourceProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,13 @@ private DevServicesDatasourceResultBuildItem.DbResult startDevDb(String dbName,
if (datasource.getPassword() != null) {
devDebProperties.put(prefix + "password", datasource.getPassword());
}

String devservices = prefix + "devservices.";
for (var name : ConfigProvider.getConfig().getPropertyNames()) {
if (name.startsWith(devservices)) {
devDebProperties.put(name, ConfigProvider.getConfig().getValue(name, String.class));
}
}
return new DevServicesDatasourceResultBuildItem.DbResult(defaultDbKind.get(), devDebProperties);
}
}
10 changes: 10 additions & 0 deletions extensions/jdbc/jdbc-postgresql/deployment/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-resteasy-reactive-deployment</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.rest-assured</groupId>
<artifactId>rest-assured</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Loading

0 comments on commit 554e1c2

Please sign in to comment.