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 Aug 16, 2021
1 parent 701fd3e commit fc835af
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 42 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
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 @@ -269,6 +269,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
53 changes: 53 additions & 0 deletions ...io/quarkus/jdbc/postgresql/deployment/DevServicesPostgresqlDatasourceDevModeTestCase.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package io.quarkus.jdbc.postgresql.deployment;

import java.util.logging.Level;

import javax.inject.Inject;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.agroal.api.AgroalDataSource;
import io.quarkus.test.QuarkusDevModeTest;
import io.restassured.RestAssured;

public class DevServicesPostgresqlDatasourceDevModeTestCase {

@RegisterExtension
static QuarkusDevModeTest test = new QuarkusDevModeTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClass(PgResource.class)
.addAsResource(new StringAsset(""), "application.properties"))
// Expect no warnings (in particular from Agroal)
.setLogRecordPredicate(record -> record.getLevel().intValue() >= Level.WARNING.intValue()
// There are other warnings: JDK8, TestContainers, drivers, ...
// Ignore them: we're only interested in Agroal here.
&& record.getMessage().contains("Agroal"));

@Inject
AgroalDataSource dataSource;

@Test
public void testDatasource() throws Exception {
RestAssured.get("/pg/save?name=foo&value=bar")
.then().statusCode(204);

RestAssured.get("/pg/get?name=foo")
.then().statusCode(200)
.body(Matchers.equalTo("bar"));

test.modifyResourceFile("application.properties", s -> "quarkus.datasource.devservices.properties.log=TRACE");

RestAssured.get("/pg/get?name=foo")
.then().statusCode(404);
RestAssured.get("/pg/save?name=foo&value=bar")
.then().statusCode(204);
RestAssured.get("/pg/get?name=foo")
.then().statusCode(200)
.body(Matchers.equalTo("bar"));
}
}
Loading

0 comments on commit fc835af

Please sign in to comment.