From edbd5356e4095fffb844a8b66e27fb2ebe8dbe5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 27 Jul 2022 15:55:58 +0200 Subject: [PATCH 1/4] Remove support for field-based and constructor-based BuildProducers As discussed here: https://groups.google.com/g/quarkus-dev/c/JUwVPsAYVP0/m/SMcjGzu6CwAJ?utm_medium=email&utm_source=footer --- .../quarkus/deployment/ExtensionLoader.java | 93 ++----------------- 1 file changed, 9 insertions(+), 84 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java b/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java index 46501e5ff6423..720bee9cfe2e1 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java @@ -331,8 +331,6 @@ private static Consumer loadStepsFromClass(Class clazz, for (Parameter parameter : ctorParameters) { Type parameterType = parameter.getParameterizedType(); final Class parameterClass = parameter.getType(); - final boolean weak = parameter.isAnnotationPresent(Weak.class); - final boolean overridable = parameter.isAnnotationPresent(Overridable.class); if (rawTypeExtends(parameterType, SimpleBuildItem.class)) { final Class buildItemClass = rawTypeOf(parameterType) .asSubclass(SimpleBuildItem.class); @@ -349,44 +347,9 @@ private static Consumer loadStepsFromClass(Class clazz, .asSubclass(MultiBuildItem.class); stepConfig = stepConfig.andThen(bsb -> bsb.consumes(buildItemClass)); ctorParamFns.add(bc -> bc.consumeMulti(buildItemClass)); - } else if (isConsumerOf(parameterType, BuildItem.class)) { - deprecatedProducer(parameter); - final Class buildItemClass = rawTypeOfParameter(parameterType, 0) - .asSubclass(BuildItem.class); - if (overridable) { - if (weak) { - stepConfig = stepConfig - .andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE)); - } - } else { - if (weak) { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass)); - } - } - ctorParamFns.add(bc -> (Consumer) bc::produce); - } else if (isBuildProducerOf(parameterType, BuildItem.class)) { - deprecatedProducer(parameter); - final Class buildItemClass = rawTypeOfParameter(parameterType, 0) - .asSubclass(BuildItem.class); - if (overridable) { - if (weak) { - stepConfig = stepConfig - .andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE)); - } - } else { - if (weak) { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass)); - } - } - ctorParamFns.add(bc -> (BuildProducer) bc::produce); + } else if (isConsumerOf(parameterType, BuildItem.class) + || isBuildProducerOf(parameterType, BuildItem.class)) { + throw unsupportedConstructorOrFieldProducer(parameter); } else if (isOptionalOf(parameterType, SimpleBuildItem.class)) { final Class buildItemClass = rawTypeOfParameter(parameterType, 0) .asSubclass(SimpleBuildItem.class); @@ -447,8 +410,6 @@ private static Consumer loadStepsFromClass(Class clazz, // next, determine the type final Type fieldType = field.getGenericType(); final Class fieldClass = field.getType(); - final boolean weak = field.isAnnotationPresent(Weak.class); - final boolean overridable = field.isAnnotationPresent(Overridable.class); if (rawTypeExtends(fieldType, SimpleBuildItem.class)) { final Class buildItemClass = rawTypeOf(fieldType).asSubclass(SimpleBuildItem.class); stepConfig = stepConfig.andThen(bsb -> bsb.consumes(buildItemClass)); @@ -464,44 +425,9 @@ private static Consumer loadStepsFromClass(Class clazz, stepConfig = stepConfig.andThen(bsb -> bsb.consumes(buildItemClass)); stepInstanceSetup = stepInstanceSetup .andThen((bc, o) -> ReflectUtil.setFieldVal(field, o, bc.consumeMulti(buildItemClass))); - } else if (isConsumerOf(fieldType, BuildItem.class)) { - deprecatedProducer(field); - final Class buildItemClass = rawTypeOfParameter(fieldType, 0).asSubclass(BuildItem.class); - if (overridable) { - if (weak) { - stepConfig = stepConfig - .andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE)); - } - } else { - if (weak) { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass)); - } - } - stepInstanceSetup = stepInstanceSetup - .andThen((bc, o) -> ReflectUtil.setFieldVal(field, o, (Consumer) bc::produce)); - } else if (isBuildProducerOf(fieldType, BuildItem.class)) { - deprecatedProducer(field); - final Class buildItemClass = rawTypeOfParameter(fieldType, 0).asSubclass(BuildItem.class); - if (overridable) { - if (weak) { - stepConfig = stepConfig - .andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.OVERRIDABLE)); - } - } else { - if (weak) { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass, ProduceFlag.WEAK)); - } else { - stepConfig = stepConfig.andThen(bsb -> bsb.produces(buildItemClass)); - } - } - stepInstanceSetup = stepInstanceSetup.andThen( - (bc, o) -> ReflectUtil.setFieldVal(field, o, (BuildProducer) bc::produce)); + } else if (isConsumerOf(fieldType, BuildItem.class) + || isBuildProducerOf(fieldType, BuildItem.class)) { + throw unsupportedConstructorOrFieldProducer(field); } else if (isOptionalOf(fieldType, SimpleBuildItem.class)) { final Class buildItemClass = rawTypeOfParameter(fieldType, 0) .asSubclass(SimpleBuildItem.class); @@ -1042,10 +968,9 @@ private static boolean isAnEmptyBuildItemConsumer(Type parameterType) { || isConsumerOf(parameterType, EmptyBuildItem.class); } - private static void deprecatedProducer(final Object element) { - loadLog.warnf( - "Producing values from constructors and fields is no longer supported and will be removed in a future release: %s", - element); + private static IllegalArgumentException unsupportedConstructorOrFieldProducer(final AnnotatedElement element) { + return reportError(element, "Producing values from constructors or fields is no longer supported." + + " Inject the BuildProducer/Consumer through arguments of relevant @BuildStep methods instead."); } protected static List getMethods(Class clazz) { From 2422908fc2541e8e7d57c093d7a643a0e5670ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 1 Aug 2022 09:18:58 +0200 Subject: [PATCH 2/4] Remove the FIELD target from @Overridable and @Weak It should no longer be necessary since build producers can no longer be injected into fields. --- .../java/io/quarkus/deployment/annotations/Overridable.java | 2 +- .../src/main/java/io/quarkus/deployment/annotations/Weak.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/annotations/Overridable.java b/core/deployment/src/main/java/io/quarkus/deployment/annotations/Overridable.java index 43904eaef4ec3..361bd45f541a5 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/annotations/Overridable.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/annotations/Overridable.java @@ -9,6 +9,6 @@ * Indicate that the given produced item is produced only if no other steps produce that item. */ @Retention(RetentionPolicy.RUNTIME) -@Target({ ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER }) +@Target({ ElementType.METHOD, ElementType.PARAMETER }) public @interface Overridable { } diff --git a/core/deployment/src/main/java/io/quarkus/deployment/annotations/Weak.java b/core/deployment/src/main/java/io/quarkus/deployment/annotations/Weak.java index e7099e67d95ec..7b941c06a3cf7 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/annotations/Weak.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/annotations/Weak.java @@ -10,6 +10,6 @@ * by a step, the step is not included. If applied to a method, the return value of the method is considered "weak". */ @Retention(RetentionPolicy.RUNTIME) -@Target({ ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER }) +@Target({ ElementType.METHOD, ElementType.PARAMETER }) public @interface Weak { } From 95c52f7afd62882da9641265e23ecfeab1413335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 27 Jul 2022 16:29:38 +0200 Subject: [PATCH 3/4] Add @Produce(EmptyBuildItem.class) to build steps that don't produce anything To make sure all build steps get executed. --- .../hibernate/orm/deployment/HibernateOrmProcessor.java | 3 +++ .../resteasy/reactive/links/deployment/LinksProcessor.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java index b7d286deec9e3..50b8f12541032 100644 --- a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java +++ b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java @@ -77,6 +77,7 @@ import io.quarkus.arc.deployment.UnremovableBeanBuildItem.BeanTypeExclusion; import io.quarkus.arc.deployment.staticmethods.InterceptedStaticMethodsTransformersRegisteredBuildItem; import io.quarkus.arc.processor.DotNames; +import io.quarkus.builder.item.EmptyBuildItem; import io.quarkus.datasource.common.runtime.DataSourceUtil; import io.quarkus.datasource.common.runtime.DatabaseKind; import io.quarkus.deployment.Capabilities; @@ -87,6 +88,7 @@ import io.quarkus.deployment.annotations.BuildProducer; import io.quarkus.deployment.annotations.BuildStep; import io.quarkus.deployment.annotations.Consume; +import io.quarkus.deployment.annotations.Produce; import io.quarkus.deployment.annotations.Record; import io.quarkus.deployment.builditem.AdditionalApplicationArchiveMarkerBuildItem; import io.quarkus.deployment.builditem.AdditionalIndexedClassesBuildItem; @@ -197,6 +199,7 @@ void registerHibernateOrmMetadataForCoreDialects( } @BuildStep + @Produce(EmptyBuildItem.class) void checkTransactionsSupport(Capabilities capabilities) { // JTA is necessary for blocking Hibernate ORM but not necessarily for Hibernate Reactive if (capabilities.isMissing(Capability.TRANSACTIONS) diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-links/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-links/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java index 2cdbfff1adc09..a420bdaf9dd7a 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive-links/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-links/deployment/src/main/java/io/quarkus/resteasy/reactive/links/deployment/LinksProcessor.java @@ -17,12 +17,14 @@ import org.jboss.resteasy.reactive.common.util.RestMediaType; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; +import io.quarkus.builder.item.EmptyBuildItem; import io.quarkus.deployment.Capabilities; import io.quarkus.deployment.Capability; import io.quarkus.deployment.Feature; import io.quarkus.deployment.GeneratedClassGizmoAdaptor; import io.quarkus.deployment.annotations.BuildProducer; import io.quarkus.deployment.annotations.BuildStep; +import io.quarkus.deployment.annotations.Produce; import io.quarkus.deployment.annotations.Record; import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem; import io.quarkus.deployment.builditem.FeatureBuildItem; @@ -83,6 +85,7 @@ AdditionalBeanBuildItem registerRestLinksProviderProducer() { } @BuildStep + @Produce(EmptyBuildItem.class) void validateJsonNeededForHal(Capabilities capabilities, ResteasyReactiveResourceMethodEntriesBuildItem resourceMethodEntriesBuildItem) { boolean isHalSupported = capabilities.isPresent(Capability.HAL); From 9a4a38352952c83a0ba039a7e0ac8959bfc9ca76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 27 Jul 2022 16:26:58 +0200 Subject: [PATCH 4/4] Throw an exception on extension loading if a build step doesn't produce anything --- core/builder/pom.xml | 6 +++++ .../io/quarkus/builder/BuildStepBuilder.java | 9 +++++++ .../java/io/quarkus/builder/BasicTests.java | 27 +++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/core/builder/pom.xml b/core/builder/pom.xml index b2160369dbd27..d0cf9c314a729 100644 --- a/core/builder/pom.xml +++ b/core/builder/pom.xml @@ -49,6 +49,12 @@ junit-jupiter test + + + org.assertj + assertj-core + test + diff --git a/core/builder/src/main/java/io/quarkus/builder/BuildStepBuilder.java b/core/builder/src/main/java/io/quarkus/builder/BuildStepBuilder.java index 85190c18661b1..254694796a5f6 100644 --- a/core/builder/src/main/java/io/quarkus/builder/BuildStepBuilder.java +++ b/core/builder/src/main/java/io/quarkus/builder/BuildStepBuilder.java @@ -194,6 +194,15 @@ public BuildStepBuilder consumes(Class type, ConsumeFlags f */ public BuildChainBuilder build() { final BuildChainBuilder chainBuilder = this.buildChainBuilder; + if (produces.isEmpty()) { + throw new IllegalArgumentException( + "Build step '" + buildStep.getId() + + "' does not produce any build item and thus will never get executed." + + " Either change the return type of the method to a build item type," + + " add a parameter of type BuildProducer<[some build item type]>/Consumer<[some build item type]>," + + " or annotate the method with @Produces." + + " Use @Produce(EmptyBuildItem.class) if you want to always execute this step."); + } if (BuildChainBuilder.LOG_CONFLICT_CAUSING) { chainBuilder.addStep(this, new Exception().getStackTrace()); } else { diff --git a/core/builder/src/test/java/io/quarkus/builder/BasicTests.java b/core/builder/src/test/java/io/quarkus/builder/BasicTests.java index f9efe4c9a36ca..fcece83ea3625 100644 --- a/core/builder/src/test/java/io/quarkus/builder/BasicTests.java +++ b/core/builder/src/test/java/io/quarkus/builder/BasicTests.java @@ -1,5 +1,6 @@ package io.quarkus.builder; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -176,6 +177,32 @@ public void execute(final BuildContext context) { assertFalse(ran.get()); } + @Test + public void testMissingProduces() { + final BuildChainBuilder builder = BuildChain.builder(); + BuildStepBuilder stepBuilder = builder.addBuildStep(new BuildStep() { + @Override + public void execute(final BuildContext context) { + context.produce(new DummyItem()); + } + + @Override + public String getId() { + return "myBuildStepId"; + } + }); + stepBuilder.consumes(DummyItem.class); + assertThatThrownBy(stepBuilder::build) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContainingAll( + "Build step 'myBuildStepId'", + "does not produce any build item and thus will never get executed", + "change the return type of the method to a build item type", + "add a parameter of type BuildProducer<[some build item type]>/Consumer<[some build item type]>", + "annotate the method with @Produces", + "Use @Produce(EmptyBuildItem.class) if you want to always execute this step"); + } + @Test public void testCircular() { final BuildChainBuilder builder = BuildChain.builder();