Skip to content

Commit

Permalink
Merge pull request #26996 from radcortez/fix-23807
Browse files Browse the repository at this point in the history 
Warning messages for config injections in static init classes
  • Loading branch information
@gsmet
gsmet authored Aug 17, 2022
2 parents f9f8b92 + 9933d10 commit d7e25d5
Show file tree
Hide file tree
Showing 13 changed files with 425 additions and 66 deletions.
9 changes: 5 additions & 4 deletions ...yment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.runtime.configuration.HyphenateEnumConverter;
import io.quarkus.runtime.configuration.NameIterator;
import io.quarkus.runtime.configuration.ProfileManager;
import io.quarkus.runtime.configuration.PropertiesUtil;
import io.smallrye.config.ConfigMapping;
import io.smallrye.config.ConfigMappings;
Expand Down Expand Up @@ -925,9 +924,11 @@ private SmallRyeConfig getConfigForRuntimeDefaults() {
private Map<String, String> filterActiveProfileProperties(final Map<String, String> properties) {
Set<String> propertiesToRemove = new HashSet<>();
for (String property : properties.keySet()) {
String profiledProperty = "%" + ProfileManager.getActiveProfile() + "." + property;
if (properties.containsKey(profiledProperty)) {
propertiesToRemove.add(property);
for (String profile : config.getProfiles()) {
String profiledProperty = "%" + profile + "." + property;
if (properties.containsKey(profiledProperty)) {
propertiesToRemove.add(property);
}
}
}
properties.keySet().removeAll(propertiesToRemove);
Expand Down
29 changes: 20 additions & 9 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 @@ -61,6 +61,7 @@
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.deployment.configuration.BuildTimeConfigurationReader;
import io.quarkus.deployment.configuration.RunTimeConfigurationGenerator;
import io.quarkus.deployment.pkg.steps.NativeOrNativeSourcesBuild;
import io.quarkus.deployment.recording.RecorderContext;
import io.quarkus.gizmo.ClassCreator;
import io.quarkus.gizmo.ClassOutput;
Expand All @@ -69,12 +70,12 @@
import io.quarkus.runtime.configuration.ConfigDiagnostic;
import io.quarkus.runtime.configuration.ConfigRecorder;
import io.quarkus.runtime.configuration.ConfigUtils;
import io.quarkus.runtime.configuration.ProfileManager;
import io.quarkus.runtime.configuration.QuarkusConfigValue;
import io.quarkus.runtime.configuration.RuntimeOverrideConfigSource;
import io.smallrye.config.ConfigMappings.ConfigClassWithPrefix;
import io.smallrye.config.ConfigSourceFactory;
import io.smallrye.config.PropertiesLocationConfigSourceFactory;
import io.smallrye.config.SmallRyeConfig;

public class ConfigGenerationBuildStep {

Expand Down Expand Up @@ -310,7 +311,7 @@ public void setupConfigOverride(
public void watchConfigFiles(BuildProducer<HotDeploymentWatchedFileBuildItem> watchedFiles) {
List<String> configWatchedFiles = new ArrayList<>();

Config config = ConfigProvider.getConfig();
SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
String userDir = System.getProperty("user.dir");

// Main files
Expand All @@ -320,20 +321,23 @@ public void watchConfigFiles(BuildProducer<HotDeploymentWatchedFileBuildItem> wa
configWatchedFiles.add(Paths.get(userDir, "config", "application.properties").toAbsolutePath().toString());

// Profiles
String profile = ProfileManager.getActiveProfile();
configWatchedFiles.add(String.format("application-%s.properties", profile));
configWatchedFiles.add(String.format("META-INF/microprofile-config-%s.properties", profile));
configWatchedFiles.add(Paths.get(userDir, String.format(".env-%s", profile)).toAbsolutePath().toString());
configWatchedFiles.add(
Paths.get(userDir, "config", String.format("application-%s.properties", profile)).toAbsolutePath().toString());
for (String profile : config.getProfiles()) {
configWatchedFiles.add(String.format("application-%s.properties", profile));
configWatchedFiles.add(String.format("META-INF/microprofile-config-%s.properties", profile));
configWatchedFiles.add(Paths.get(userDir, String.format(".env-%s", profile)).toAbsolutePath().toString());
configWatchedFiles.add(Paths.get(userDir, "config", String.format("application-%s.properties", profile))
.toAbsolutePath().toString());
}

Optional<List<URI>> optionalLocations = config.getOptionalValues(SMALLRYE_CONFIG_LOCATIONS, URI.class);
optionalLocations.ifPresent(locations -> {
for (URI location : locations) {
Path path = location.getScheme() != null ? Paths.get(location) : Paths.get(location.getPath());
if (!Files.isDirectory(path)) {
configWatchedFiles.add(path.toString());
configWatchedFiles.add(appendProfileToFilename(path.toString(), profile));
for (String profile : config.getProfiles()) {
configWatchedFiles.add(appendProfileToFilename(path.toString(), profile));
}
}
}
});
Expand All @@ -343,6 +347,13 @@ public void watchConfigFiles(BuildProducer<HotDeploymentWatchedFileBuildItem> wa
}
}

@BuildStep(onlyIf = NativeOrNativeSourcesBuild.class)
@Record(ExecutionTime.RUNTIME_INIT)
void warnDifferentProfileUsedBetweenBuildAndRunTime(ConfigRecorder configRecorder) {
SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
configRecorder.handleNativeProfileChange(config.getProfiles());
}

private String appendProfileToFilename(String path, String activeProfile) {
String pathWithoutExtension = FilenameUtils.removeExtension(path);
return String.format("%s-%s.%s", pathWithoutExtension, activeProfile, FilenameUtils.getExtension(path));
Expand Down
24 changes: 24 additions & 0 deletions core/runtime/src/main/java/io/quarkus/runtime/configuration/ConfigRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,30 @@ public void handleConfigChange(Map<String, ConfigValue> buildTimeRuntimeValues)
throw new IllegalStateException("Unexpected " + BuildTimeMismatchAtRuntime.class.getName() + ": "
+ configurationConfig.buildTimeMismatchAtRuntime);
}

}
}

public void handleNativeProfileChange(List<String> buildProfiles) {
SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
List<String> runtimeProfiles = config.getProfiles();

if (buildProfiles.size() != runtimeProfiles.size()) {
log.warn(
"The profile '" + buildProfiles + "' used to build the native image is different from the runtime profile '"
+ runtimeProfiles + "'. This may lead to unexpected results.");
return;
}

for (int i = 0; i < buildProfiles.size(); i++) {
String buildProfile = buildProfiles.get(i);
String runtimeProfile = runtimeProfiles.get(i);

if (!buildProfile.equals(runtimeProfile)) {
log.warn("The profile '" + buildProfile
+ "' used to build the native image is different from the runtime profile '" + runtimeProfile
+ "'. This may lead to unexpected results.");
}
}
}
}
81 changes: 76 additions & 5 deletions extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ConfigBuildStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static org.jboss.jandex.AnnotationValue.createStringValue;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -28,6 +29,7 @@
import javax.enterprise.context.Dependent;
import javax.enterprise.inject.CreationException;

import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.ConfigValue;
import org.eclipse.microprofile.config.inject.ConfigProperties;
import org.eclipse.microprofile.config.inject.ConfigProperty;
Expand All @@ -38,8 +40,10 @@
import org.jboss.jandex.DotName;
import org.jboss.jandex.FieldInfo;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.ParameterizedType;
import org.jboss.jandex.Type;
import org.jboss.jandex.Type.Kind;
import org.jboss.logging.Logger;

import io.quarkus.arc.Unremovable;
import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem.BeanConfiguratorBuildItem;
Expand All @@ -61,6 +65,7 @@
import io.quarkus.deployment.builditem.ConfigPropertiesBuildItem;
import io.quarkus.deployment.builditem.ConfigurationBuildItem;
import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
import io.quarkus.deployment.builditem.RunTimeConfigurationDefaultBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.deployment.configuration.ConfigMappingUtils;
import io.quarkus.deployment.configuration.definition.RootDefinition;
Expand All @@ -75,16 +80,21 @@
* MicroProfile Config related build steps.
*/
public class ConfigBuildStep {
private static final Logger LOGGER = Logger.getLogger(ConfigBuildStep.class.getName());

private static final DotName MP_CONFIG = DotName.createSimple(Config.class.getName());
private static final DotName MP_CONFIG_PROPERTY_NAME = DotName.createSimple(ConfigProperty.class.getName());
private static final DotName MP_CONFIG_PROPERTIES_NAME = DotName.createSimple(ConfigProperties.class.getName());
private static final DotName MP_CONFIG_VALUE_NAME = DotName.createSimple(ConfigValue.class.getName());

private static final DotName SR_CONFIG = DotName.createSimple(io.smallrye.config.SmallRyeConfig.class.getName());
private static final DotName SR_CONFIG_VALUE_NAME = DotName.createSimple(io.smallrye.config.ConfigValue.class.getName());
private static final DotName SR_WITH_CONVERTER = DotName.createSimple(WithConverter.class.getName());

private static final DotName MAP_NAME = DotName.createSimple(Map.class.getName());
private static final DotName SET_NAME = DotName.createSimple(Set.class.getName());
private static final DotName LIST_NAME = DotName.createSimple(List.class.getName());
private static final DotName SUPPLIER_NAME = DotName.createSimple(Supplier.class.getName());
private static final DotName CONFIG_VALUE_NAME = DotName.createSimple(io.smallrye.config.ConfigValue.class.getName());
private static final DotName WITH_CONVERTER = DotName.createSimple(WithConverter.class.getName());

@BuildStep
void additionalBeans(BuildProducer<AdditionalBeanBuildItem> additionalBeans) {
Expand Down Expand Up @@ -169,7 +179,7 @@ void validateConfigInjectionPoints(ValidationPhaseBuildItem validationPhase,
|| DotNames.OPTIONAL_DOUBLE.equals(injectedType.name())
|| DotNames.PROVIDER.equals(injectedType.name())
|| SUPPLIER_NAME.equals(injectedType.name())
|| CONFIG_VALUE_NAME.equals(injectedType.name())
|| SR_CONFIG_VALUE_NAME.equals(injectedType.name())
|| MP_CONFIG_VALUE_NAME.equals(injectedType.name())) {
// Never validate container objects
continue;
Expand Down Expand Up @@ -359,7 +369,7 @@ void registerConfigPropertiesBean(
void registerConfigMappingConverters(CombinedIndexBuildItem indexBuildItem,
BuildProducer<ReflectiveClassBuildItem> producer) {

String[] valueTypes = indexBuildItem.getIndex().getAnnotations(WITH_CONVERTER).stream()
String[] valueTypes = indexBuildItem.getIndex().getAnnotations(SR_WITH_CONVERTER).stream()
.map(i -> i.value().asClass().name().toString())
.toArray(String[]::new);
if (valueTypes.length > 0) {
Expand Down Expand Up @@ -459,6 +469,67 @@ void validateConfigPropertiesInjectionPoints(
toRegister.forEach(configProperties::produce);
}

@BuildStep
void warnStaticInitInjectionPoints(
CombinedIndexBuildItem indexBuildItem,
ValidationPhaseBuildItem validationPhase,
List<ConfigClassBuildItem> configClasses,
List<ConfigInjectionStaticInitBuildItem> configInjectionStaticInit,
BuildProducer<RunTimeConfigurationDefaultBuildItem> runTimeConfigurationDefault) {

// Add here annotated classes that are initialized during static init
Set<DotName> declaringClassCandidates = configInjectionStaticInit.stream()
.map(ConfigInjectionStaticInitBuildItem::getDeclaringCandidate).collect(toSet());

Set<Type> configClassesTypes = configClasses.stream().map(ConfigClassBuildItem::getTypes).flatMap(Collection::stream)
.collect(toSet());

for (InjectionPointInfo injectionPoint : validationPhase.getContext().getInjectionPoints()) {
if (injectionPoint.getType().name().equals(DotNames.INSTANCE)) {
continue;
}

Type type = Type.create(injectionPoint.getRequiredType().name(), Type.Kind.CLASS);
DotName injectionTypeName = null;
if (type.name().equals(MP_CONFIG) || type.name().equals(SR_CONFIG)) {
injectionTypeName = type.name();
} else if (injectionPoint.getRequiredQualifier(MP_CONFIG_PROPERTY_NAME) != null) {
injectionTypeName = MP_CONFIG_PROPERTY_NAME;
} else if (configClassesTypes.contains(type)) {
injectionTypeName = type.name();
}

if (injectionTypeName != null) {
AnnotationTarget target = injectionPoint.getTarget();
if (FIELD.equals(target.kind())) {
FieldInfo field = target.asField();
ClassInfo declaringClass = field.declaringClass();
Map<DotName, List<AnnotationInstance>> annotationsMap = declaringClass.annotationsMap();
for (DotName declaringClassCandidate : declaringClassCandidates) {
List<AnnotationInstance> annotationInstances = annotationsMap.get(declaringClassCandidate);
if (annotationInstances != null && annotationInstances.size() == 1) {
AnnotationInstance annotationInstance = annotationInstances.get(0);
if (annotationInstance.target().equals(declaringClass)) {
LOGGER.warn("Directly injecting a " +
injectionTypeName +
" into a " +
declaringClassCandidate +
" may lead to unexpected results. To ensure proper results, please " +
"change the type of the field to " +
ParameterizedType.create(DotNames.INSTANCE, new Type[] { type }, null) +
". Offending field is '" +
field.name() +
"' of class '" +
field.declaringClass() +
"'");
}
}
}
}
}
}
}

@BuildStep
@Record(RUNTIME_INIT)
void registerConfigClasses(
Expand Down Expand Up @@ -512,7 +583,7 @@ public static boolean isHandledByProducers(Type type) {
DotNames.BYTE.equals(type.name()) ||
DotNames.CHARACTER.equals(type.name()) ||
SUPPLIER_NAME.equals(type.name()) ||
CONFIG_VALUE_NAME.equals(type.name()) ||
SR_CONFIG_VALUE_NAME.equals(type.name()) ||
MP_CONFIG_VALUE_NAME.equals(type.name());
}

Expand Down
17 changes: 17 additions & 0 deletions ...eployment/src/main/java/io/quarkus/arc/deployment/ConfigInjectionStaticInitBuildItem.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.quarkus.arc.deployment;

import org.jboss.jandex.DotName;

import io.quarkus.builder.item.MultiBuildItem;

public final class ConfigInjectionStaticInitBuildItem extends MultiBuildItem {
private final DotName declaringCandidate;

public ConfigInjectionStaticInitBuildItem(final DotName declaringCandidate) {
this.declaringCandidate = declaringCandidate;
}

public DotName getDeclaringCandidate() {
return declaringCandidate;
}
}
9 changes: 0 additions & 9 deletions extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ConfigRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,6 @@ public void validateConfigProperties(Set<ConfigValidationMetadata> properties) {
}
}

public void registerConfigMappings(final Set<ConfigClassWithPrefix> configClasses) {
try {
SmallRyeConfig config = (SmallRyeConfig) ConfigProvider.getConfig();
ConfigMappings.registerConfigMappings(config, configClasses);
} catch (ConfigValidationException e) {
throw new DeploymentException(e.getMessage(), e);
}
}

public void registerConfigProperties(final Set<ConfigClassWithPrefix> configClasses) {
try {
SmallRyeConfig config = (SmallRyeConfig) ConfigProvider.getConfig();
Expand Down
20 changes: 12 additions & 8 deletions ...-yaml/deployment/src/main/java/io/quarkus/config/yaml/deployment/ConfigYamlProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.util.ArrayList;
import java.util.List;

import org.eclipse.microprofile.config.ConfigProvider;

import io.quarkus.config.yaml.runtime.ApplicationYamlConfigSourceLoader;
import io.quarkus.deployment.Feature;
import io.quarkus.deployment.annotations.BuildProducer;
Expand All @@ -12,7 +14,7 @@
import io.quarkus.deployment.builditem.FeatureBuildItem;
import io.quarkus.deployment.builditem.HotDeploymentWatchedFileBuildItem;
import io.quarkus.deployment.builditem.StaticInitConfigSourceProviderBuildItem;
import io.quarkus.runtime.configuration.ProfileManager;
import io.smallrye.config.SmallRyeConfig;

public final class ConfigYamlProcessor {

Expand Down Expand Up @@ -47,13 +49,15 @@ void watchYamlConfig(BuildProducer<HotDeploymentWatchedFileBuildItem> watchedFil
configWatchedFiles.add(Paths.get(userDir, "config", "application.yml").toAbsolutePath().toString());

// Profiles
String profile = ProfileManager.getActiveProfile();
configWatchedFiles.add(String.format("application-%s.yaml", profile));
configWatchedFiles.add(String.format("application-%s.yml", profile));
configWatchedFiles
.add(Paths.get(userDir, "config", String.format("application-%s.yaml", profile)).toAbsolutePath().toString());
configWatchedFiles
.add(Paths.get(userDir, "config", String.format("application-%s.yml", profile)).toAbsolutePath().toString());
SmallRyeConfig config = ConfigProvider.getConfig().unwrap(SmallRyeConfig.class);
for (String profile : config.getProfiles()) {
configWatchedFiles.add(String.format("application-%s.yaml", profile));
configWatchedFiles.add(String.format("application-%s.yml", profile));
configWatchedFiles.add(
Paths.get(userDir, "config", String.format("application-%s.yaml", profile)).toAbsolutePath().toString());
configWatchedFiles.add(
Paths.get(userDir, "config", String.format("application-%s.yml", profile)).toAbsolutePath().toString());
}

for (String configWatchedFile : configWatchedFiles) {
watchedFiles.produce(new HotDeploymentWatchedFileBuildItem(configWatchedFile));
Expand Down
Loading

0 comments on commit d7e25d5

Please sign in to comment.