[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty
and @Inject do not work in RouteBuilders
URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344902446
##
File path:
extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/CamelRoutesBuilderClassBuildItem.java
##
@@ -17,20 +17,23 @@
package org.apache.camel.quarkus.core.deployment;
import io.quarkus.builder.item.MultiBuildItem;
-import io.quarkus.runtime.RuntimeValue;
import org.apache.camel.RoutesBuilder;
+import org.jboss.jandex.DotName;
/**
- * Holds the {@link RoutesBuilder} {@link RuntimeValue}.
+ * A {@link MultiBuildItem} holding class names of all {@link RoutesBuilder}
implementations found via Jandex.
*/
-public final class CamelRoutesBuilderBuildItem extends MultiBuildItem {
-private final RuntimeValue routesBuilder;
+public final class CamelRoutesBuilderClassBuildItem extends MultiBuildItem {
-public CamelRoutesBuilderBuildItem(RuntimeValue
routesBuilder) {
-this.routesBuilder = routesBuilder;
-}
+private final DotName dotName;
-public RuntimeValue getInstance() {
-return routesBuilder;
+public CamelRoutesBuilderClassBuildItem(DotName dotName) {
+super();
Review comment:
Eclipse seems to be doing that by default.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty
and @Inject do not work in RouteBuilders
URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344901192
##
File path:
extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
##
@@ -249,17 +262,55 @@ CamelRuntimeRegistryBuildItem bindRuntimeBeansToRegistry(
* disabled by setting quarkus.camel.disable-main = true
*/
public static class Main {
+
+@BuildStep
+public List
discoverRoutesBuilderClassNames(
+CombinedIndexBuildItem combinedIndex) {
+final IndexView index = combinedIndex.getIndex();
+Set allKnownImplementors = new HashSet<>();
+allKnownImplementors.addAll(
+
index.getAllKnownImplementors(DotName.createSimple(RoutesBuilder.class.getName(;
+allKnownImplementors.addAll(
+
index.getAllKnownSubclasses(DotName.createSimple(RouteBuilder.class.getName(;
+allKnownImplementors.addAll(
+
index.getAllKnownSubclasses(DotName.createSimple(AdviceWithRouteBuilder.class.getName(;
+
+return allKnownImplementors
+.stream()
+// public and non-abstract
+.filter(ci -> ((ci.flags() & (Modifier.ABSTRACT |
Modifier.PUBLIC)) == Modifier.PUBLIC))
+.map(ClassInfo::name)
+.map(CamelRoutesBuilderClassBuildItem::new)
+.collect(Collectors.toList());
+}
+
@Record(ExecutionTime.STATIC_INIT)
@BuildStep
-public List collectRoutes(
-CombinedIndexBuildItem combinedIndex,
+public List collectRoutes(
+List camelRoutesBuilders,
Review comment:
np
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty
and @Inject do not work in RouteBuilders
URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344892800
##
File path:
extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
##
@@ -274,6 +325,18 @@ void beans(BuildProducer
beanProducer) {
beanProducer.produce(AdditionalBeanBuildItem.unremovableOf(CamelMainProducers.class));
}
+@BuildStep(onlyIf = Flags.MainEnabled.class)
Review comment:
Haha, you've got me. It's a result of copy and paste. Let me figure out if I
actually want that.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty
and @Inject do not work in RouteBuilders
URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344890711
##
File path:
extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java
##
@@ -249,17 +262,55 @@ CamelRuntimeRegistryBuildItem bindRuntimeBeansToRegistry(
* disabled by setting quarkus.camel.disable-main = true
*/
public static class Main {
+
+@BuildStep
+public List
discoverRoutesBuilderClassNames(
+CombinedIndexBuildItem combinedIndex) {
+final IndexView index = combinedIndex.getIndex();
+Set allKnownImplementors = new HashSet<>();
+allKnownImplementors.addAll(
+
index.getAllKnownImplementors(DotName.createSimple(RoutesBuilder.class.getName(;
+allKnownImplementors.addAll(
+
index.getAllKnownSubclasses(DotName.createSimple(RouteBuilder.class.getName(;
+allKnownImplementors.addAll(
+
index.getAllKnownSubclasses(DotName.createSimple(AdviceWithRouteBuilder.class.getName(;
+
+return allKnownImplementors
+.stream()
+// public and non-abstract
+.filter(ci -> ((ci.flags() & (Modifier.ABSTRACT |
Modifier.PUBLIC)) == Modifier.PUBLIC))
+.map(ClassInfo::name)
+.map(CamelRoutesBuilderClassBuildItem::new)
+.collect(Collectors.toList());
+}
+
@Record(ExecutionTime.STATIC_INIT)
@BuildStep
-public List collectRoutes(
-CombinedIndexBuildItem combinedIndex,
+public List collectRoutes(
+List camelRoutesBuilders,
CamelMainRecorder recorder,
+BeanRegistrationPhaseBuildItem beanRegistrationPhase,
RecorderContext recorderContext) {
-return
CamelSupport.getRouteBuilderClasses(combinedIndex.getIndex())
-.map(recorderContext:: newInstance)
-.map(CamelRoutesBuilderBuildItem::new)
-.collect(Collectors.toList());
+final Set arcBeanClasses =
beanRegistrationPhase.getContext().get(BuildExtension.Key.BEANS)
+.stream()
+.map(BeanInfo::getImplClazz)
+.map(ClassInfo::name)
+.collect(Collectors.toSet());
+
+final List result = new
ArrayList();
+camelRoutesBuilders.stream()
+.map(CamelRoutesBuilderClassBuildItem::getDotName)
+.filter(dotName -> !arcBeanClasses.contains(dotName))
+.forEach(dotName -> {
Review comment:
Yeah, that would be possible now. The previous revisions did not allow that.
Let me refactor.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty
and @Inject do not work in RouteBuilders
URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344888604
##
File path:
extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/NativeImageProcessor.java
##
@@ -176,17 +177,15 @@ protected void addReflectiveMethod(MethodInfo mi) {
public static class Main {
@BuildStep(onlyIf = Flags.MainEnabled.class)
void process(
-CombinedIndexBuildItem combinedIndex,
+List camelRoutesBuilders,
BuildProducer reflectiveClass) {
-IndexView view = combinedIndex.getIndex();
-
//
// Register routes as reflection aware as camel-main main use
reflection
// to bind beans to the registry
//
-CamelSupport.getRouteBuilderClasses(view).forEach(name -> {
-reflectiveClass.produce(new ReflectiveClassBuildItem(true,
false, name));
+camelRoutesBuilders.stream().forEach(dotName -> {
Review comment:
+1
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty
and @Inject do not work in RouteBuilders
URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344886972
##
File path:
extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/CamelRoutesBuilderClassBuildItem.java
##
@@ -17,20 +17,23 @@
package org.apache.camel.quarkus.core.deployment;
import io.quarkus.builder.item.MultiBuildItem;
-import io.quarkus.runtime.RuntimeValue;
import org.apache.camel.RoutesBuilder;
+import org.jboss.jandex.DotName;
/**
- * Holds the {@link RoutesBuilder} {@link RuntimeValue}.
+ * A {@link MultiBuildItem} holding class names of all {@link RoutesBuilder}
implementations found via Jandex.
Review comment:
True, let me reword it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344661828 ## File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java ## @@ -232,7 +228,6 @@ CamelMainBuildItem main( CamelContextBuildItem context, CamelRoutesCollectorBuildItem routesCollector, List listeners, -List routesBuilders, Review comment: Yes, I agree. You perhaps meant CamelRoutesBuilderBuildItem rather than CamelBeanBuildItem? The ultimate aim of this PR is to make the `@Inject`, `@ConfigProperty`, etc. work in RoutesBuilders (see the tests in this PR that currently fail). I agree that the problem of double Route initialization is actually the root cause. My previous proposal to solve both problems by delegating the creation of all RouteBuilders to Arc was rejected for boot performance reasons which I found quite legitimate. I proposed an alternative approach where Arc beans are thrown away from the list provided via CamelRoutesBuilderBuildItem, but I see no feedback for that. In between, I looked how to implement that reliably, and found that the list of Arc beans could perhaps be pulled from Arc itself like this: https://github.com/ppalaga/camel-quarkus/commit/cdbfd1999b35e103944d41e570d266d73fd20995#diff-ec6205752018e84a80fcd9eaa92af189R203-R209 (once they change the visibility of some methods, I asked the Arc guys about that). What do you think about the linked solution? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [camel-quarkus] ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders
ppalaga commented on a change in pull request #358: Fix #136 @ConfigProperty and @Inject do not work in RouteBuilders URL: https://github.com/apache/camel-quarkus/pull/358#discussion_r344385789 ## File path: extensions/core/deployment/src/main/java/org/apache/camel/quarkus/core/deployment/BuildProcessor.java ## @@ -232,7 +228,6 @@ CamelMainBuildItem main( CamelContextBuildItem context, CamelRoutesCollectorBuildItem routesCollector, List listeners, -List routesBuilders, Review comment: That would re-introduce the double initialization of the routes. I was thinking of filtering by `@ApplicationScoped` that can be done well using Jandex: Builders having `@ApplicationScoped` would behave like in the current proposal and all others could go via `CamelRoutesBuilderBuildItem`. This would be more complex and thus potentially more buggy and harder to maintain. Maybe you have a better idea? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
