Skip to content

Commit

Permalink
Merge pull request quarkusio#36281 from mkouba/static-init-config-val…
Browse files Browse the repository at this point in the history
…idation

Config: detect injected config value mismatch during static init
  • Loading branch information
yrodiere authored Oct 9, 2023
2 parents 39113e3 + 60c24cb commit ea956d0
Show file tree
Hide file tree
Showing 23 changed files with 525 additions and 284 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
import io.quarkus.gizmo.TryBlock;
import io.quarkus.runtime.Application;
import io.quarkus.runtime.ApplicationLifecycleManager;
import io.quarkus.runtime.ExecutionModeManager;
import io.quarkus.runtime.LaunchMode;
import io.quarkus.runtime.NativeImageRuntimePropertiesRecorder;
import io.quarkus.runtime.PreventFurtherStepsException;
Expand Down Expand Up @@ -106,6 +107,14 @@ public class MainClassBuildStep {
void.class, StartupContext.class);
public static final MethodDescriptor CONFIGURE_STEP_TIME_ENABLED = ofMethod(StepTiming.class.getName(), "configureEnabled",
void.class);
public static final MethodDescriptor RUNTIME_EXECUTION_STATIC_INIT = ofMethod(ExecutionModeManager.class.getName(),
"staticInit", void.class);
public static final MethodDescriptor RUNTIME_EXECUTION_RUNTIME_INIT = ofMethod(ExecutionModeManager.class.getName(),
"runtimeInit", void.class);
public static final MethodDescriptor RUNTIME_EXECUTION_RUNNING = ofMethod(ExecutionModeManager.class.getName(),
"running", void.class);
public static final MethodDescriptor RUNTIME_EXECUTION_UNSET = ofMethod(ExecutionModeManager.class.getName(),
"unset", void.class);
public static final MethodDescriptor CONFIGURE_STEP_TIME_START = ofMethod(StepTiming.class.getName(), "configureStart",
void.class);
private static final DotName QUARKUS_APPLICATION = DotName.createSimple(QuarkusApplication.class.getName());
Expand Down Expand Up @@ -170,6 +179,7 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,
lm);

mv.invokeStaticMethod(CONFIGURE_STEP_TIME_ENABLED);
mv.invokeStaticMethod(RUNTIME_EXECUTION_STATIC_INIT);

mv.invokeStaticMethod(ofMethod(Timing.class, "staticInitStarted", void.class, boolean.class),
mv.load(launchMode.isAuxiliaryApplication()));
Expand Down Expand Up @@ -227,6 +237,7 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,
mv.load(i.getKey()), mv.load(i.getValue()));
}
mv.invokeStaticMethod(ofMethod(NativeImageRuntimePropertiesRecorder.class, "doRuntime", void.class));
mv.invokeStaticMethod(RUNTIME_EXECUTION_RUNTIME_INIT);

// Set the SSL system properties
if (!javaLibraryPathAdditionalPaths.isEmpty()) {
Expand Down Expand Up @@ -268,6 +279,8 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,
loaders, constants, gizmoOutput, startupContext, tryBlock);
}

tryBlock.invokeStaticMethod(RUNTIME_EXECUTION_RUNNING);

// Startup log messages
List<String> featureNames = new ArrayList<>();
for (FeatureBuildItem feature : features) {
Expand Down Expand Up @@ -324,6 +337,7 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,

mv = file.getMethodCreator("doStop", void.class);
mv.setModifiers(Modifier.PROTECTED | Modifier.FINAL);
mv.invokeStaticMethod(RUNTIME_EXECUTION_UNSET);
startupContext = mv.readStaticField(scField.getFieldDescriptor());
mv.invokeVirtualMethod(ofMethod(StartupContext.class, "close", void.class), startupContext);
mv.returnValue(null);
Expand Down
30 changes: 30 additions & 0 deletions core/runtime/src/main/java/io/quarkus/runtime/ExecutionMode.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.quarkus.runtime;

/**
* The runtime execution mode.
*/
public enum ExecutionMode {

/**
* Static initializiation.
*/
STATIC_INIT,

/**
* Runtime initialization.
*/
RUNTIME_INIT,

/**
* The application is running.
*/
RUNNING,

UNSET,
;

public static ExecutionMode current() {
return ExecutionModeManager.getExecutionMode();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package io.quarkus.runtime;

public final class ExecutionModeManager {

private static volatile ExecutionMode executionMode = ExecutionMode.UNSET;

public static void staticInit() {
executionMode = ExecutionMode.STATIC_INIT;
}

public static void runtimeInit() {
executionMode = ExecutionMode.RUNTIME_INIT;
}

public static void running() {
executionMode = ExecutionMode.RUNNING;
}

public static void unset() {
executionMode = ExecutionMode.UNSET;
}

public static ExecutionMode getExecutionMode() {
return executionMode;
}
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
package io.quarkus.runtime.annotations;

import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import org.eclipse.microprofile.config.inject.ConfigProperty;

/**
* Used to mark a {@link org.eclipse.microprofile.config.spi.ConfigSource},
* {@link org.eclipse.microprofile.config.spi.ConfigSourceProvider} or {@link io.smallrye.config.ConfigSourceFactory}
* as safe to be initialized during STATIC INIT.
* Used to mark a configuration object as safe to be initialized during the STATIC INIT phase.
* <p>
* The target configuration objects include {@link org.eclipse.microprofile.config.spi.ConfigSource},
* {@link org.eclipse.microprofile.config.spi.ConfigSourceProvider}, {@link io.smallrye.config.ConfigSourceFactory} and
* {@link io.smallrye.config.ConfigMapping}. Moreover, this annotation can be used for
* {@link org.eclipse.microprofile.config.inject.ConfigProperty} injection points.
* <p>
*
* When a Quarkus application is starting up, Quarkus will execute first a static init method which contains some
* extensions actions and configurations. Example:
Expand All @@ -36,7 +44,7 @@
* previous code example and a ConfigSource that requires database access. In this case, it is impossible to properly
* initialize such ConfigSource, because the database services are not yet available so the ConfigSource in unusable.
*/
@Target(TYPE)
@Target({ TYPE, FIELD, PARAMETER })
@Retention(RUNTIME)
@Documented
public @interface StaticInitSafe {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import jakarta.enterprise.context.Dependent;
import jakarta.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 @@ -42,10 +41,8 @@
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 @@ -67,7 +64,6 @@
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.definition.RootDefinition;
import io.quarkus.deployment.recording.RecorderContext;
Expand All @@ -80,14 +76,11 @@
* 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());
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 MAP_NAME = DotName.createSimple(Map.class.getName());
Expand Down Expand Up @@ -474,67 +467,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

import io.quarkus.builder.item.MultiBuildItem;

/**
*
* @deprecated This build item is not used anymore
*/
@Deprecated(forRemoval = true)
public final class ConfigInjectionStaticInitBuildItem extends MultiBuildItem {
private final DotName declaringCandidate;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.arc.deployment;

import org.jboss.jandex.DotName;

import io.quarkus.arc.processor.AnnotationsTransformer;
import io.quarkus.arc.processor.DotNames;
import io.quarkus.arc.runtime.ConfigStaticInitCheck;
import io.quarkus.arc.runtime.ConfigStaticInitCheckInterceptor;
import io.quarkus.arc.runtime.ConfigStaticInitValues;
import io.quarkus.deployment.annotations.BuildStep;

public class ConfigStaticInitBuildSteps {

@BuildStep
AdditionalBeanBuildItem registerBeans() {
return AdditionalBeanBuildItem.builder()
.addBeanClasses(ConfigStaticInitCheckInterceptor.class, ConfigStaticInitValues.class,
ConfigStaticInitCheck.class)
.build();
}

@BuildStep
AnnotationsTransformerBuildItem transformConfigProducer() {
DotName configProducerName = DotName.createSimple("io.smallrye.config.inject.ConfigProducer");

return new AnnotationsTransformerBuildItem(AnnotationsTransformer.appliedToMethod().whenMethod(m -> {
// Apply to all producer methods declared on io.smallrye.config.inject.ConfigProducer
return m.declaringClass().name().equals(configProducerName)
&& m.hasAnnotation(DotNames.PRODUCES)
&& m.hasAnnotation(ConfigBuildStep.MP_CONFIG_PROPERTY_NAME);
}).thenTransform(t -> {
t.add(ConfigStaticInitCheck.class);
}));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.quarkus.arc.test.config.staticinit;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.context.Initialized;
import jakarta.enterprise.event.Observes;
import jakarta.inject.Singleton;

import org.eclipse.microprofile.config.inject.ConfigProperty;

@Singleton
public class StaticInitBean {

@ConfigProperty(name = "apfelstrudel")
String value;

// bean is instantiated during STATIC_INIT
void onInit(@Observes @Initialized(ApplicationScoped.class) Object event) {
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.arc.test.config.staticinit;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

import org.eclipse.microprofile.config.spi.ConfigSource;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class StaticInitConfigInjectionFailureTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot(root -> root
.addClasses(StaticInitBean.class, StaticInitEagerBean.class, UnsafeConfigSource.class)
.addAsServiceProvider(ConfigSource.class, UnsafeConfigSource.class)
// the value from application.properties should be injected during STATIC_INIT
.addAsResource(new StringAsset("apfelstrudel=jandex"), "application.properties"))
.assertException(t -> {
assertThat(t).isInstanceOf(IllegalStateException.class)
.hasMessageContainingAll(
"A runtime config property value differs from the value that was injected during the static intialization phase",
"the runtime value of 'apfelstrudel' is [gizmo] but the value [jandex] was injected into io.quarkus.arc.test.config.staticinit.StaticInitBean#value",
"the runtime value of 'apfelstrudel' is [gizmo] but the value [jandex] was injected into io.quarkus.arc.test.config.staticinit.StaticInitEagerBean#value");
});

@Test
public void test() {
fail();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.quarkus.arc.test.config.staticinit;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.context.Initialized;
import jakarta.enterprise.event.Observes;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Singleton;

import org.eclipse.microprofile.config.inject.ConfigProperty;

@Singleton
public class StaticInitEagerBean {

@ConfigProperty(name = "apfelstrudel")
Instance<String> value;

// bean is instantiated during STATIC_INIT
void onInit(@Observes @Initialized(ApplicationScoped.class) Object event) {
// this should trigger the failure
value.get();
}

}
Loading

0 comments on commit ea956d0

Please sign in to comment.