fix(doc): Transform Processor CDI Decorator into manual decorators #118

edeweerd1A · 2024-10-08T11:50:49Z

We noticed flackiness with QuarkusTest and usage of Processor decorators. It is generating randomly ClassNotFoundException.
It is happening only with Quarkus 3.8 LTS, recent versions like 3.15 or older like 3.2 do not present the issue.

The solution found is a fallback of sorts, by removing the usage of @decorator in the processor decorators. Instead they are transformed in old-school, composition-design-pattern-inspired beans with a lombok delegate. The generic type signature is removed so they can be transformed in Dependent beans. Why's that? 1. A class with generics cannot be a bean, according to compilation errors, and 2. processors returned by the supplier need to be new instances everytime. Priorities are kept, and used to resolve in order the beans, for a manual encapsulation achieved with a for loop on the list of beans.

And with that, the flackiness is gone.

Of course, those changes will not be propagated to main and the future 3.15 branch, where this flackiness is not an issue, AND the usage of CDI's Decorator can be kept.

The documentation is updated accordingly.

Fixes #117

github-actions · 2024-10-08T11:51:15Z

docs/modules/ROOT/pages/index.adoc

+.ProcessorDecorator.java for Quarkus 3.8 -> 3.10
+[source,java]
+----
+@Dependent // <1>


🚫 [vale] _{reported by reviewdog 🐶}
[RedHat.TermsErrors] Use 'concept' rather than 'notion'.

github-actions · 2024-10-08T11:51:15Z

docs/modules/ROOT/pages/index.adoc

+@Priority(150) // <2>
+public class ProcessorDecorator extends AbstractProcessorDecorator { // <3>
+    @Override
+    public void process(Record record) { // <4>


⚠️ [vale] _{reported by reviewdog 🐶}
[RedHat.Spelling] Verify the word 'instrumentated'. It is not in the American English spelling dictionary used by Vale.

edeweerd1A · 2024-10-08T12:09:32Z

Migration guide written in wiki: https://github.com/quarkiverse/quarkus-kafka-streams-processor/wiki/quarkus%E2%80%90kafka%E2%80%90streams%E2%80%90processor-2.0-migration-guide

rquinio1A · 2024-10-09T08:42:43Z

...io/quarkiverse/kafkastreamsprocessor/api/decorator/processor/AbstractProcessorDecorator.java

Does this need to be in the api module or it could stay in the impl ?
I see integration-tests new HeaderDecorator compiling on it, but not sure if it's a new capability or not.

it needs yeah, cause that type is used when discovering the beans

rquinio1A · 2024-10-09T08:46:51Z

...io/quarkiverse/kafkastreamsprocessor/api/decorator/processor/AbstractProcessorDecorator.java

+import lombok.Setter;
+import lombok.experimental.Delegate;
+
+public class AbstractProcessorDecorator implements Processor {


Class should be flagged as deprecated, if it's removed in RHBQ 3.15 ?

rquinio1A · 2024-10-09T08:50:08Z

docs/modules/ROOT/pages/index.adoc

+This happens for specific versions of Quarkus.
+Known impacted versions are 3.8.x, 3.9.x and 3.10.x.
+The 3.2 LTS and upcoming 3.15 LTS versions do not suffer from this symptom.
+The **only** solution found was to remove usage of `@Decorator` for `Processor` decorators for microservices based on Quarkus 3.8 LTS.


We could mention if/until quarkusio/quarkus#43245 is backported to 3.8 LTS ?

Changed the link to quarkusio/quarkus#41258 will seems more the origin of the "fix" in quarkus 3.11

vietk · 2024-10-11T06:21:56Z

...io/quarkiverse/kafkastreamsprocessor/api/decorator/processor/AbstractProcessorDecorator.java

+ * @deprecated It will be removed in 3.0, with the integration of Quarkus 3.15 where we will be able to go back to pure
+ *             CDI decorators.
+ */
+@Deprecated(forRemoval = true, since = "2.0")


Just born and already obsolete :D

vietk · 2024-10-11T06:29:04Z

impl/src/main/java/io/quarkiverse/kafkastreamsprocessor/impl/KStreamProcessorSupplier.java

        this.kafka3BeanInstances = kafka3BeanInstances;
        this.beanInstances = beanInstances;
        this.adapterInstances = adapterInstances;
+        this.processorDecorators = processorDecorators;
+
+        List<String> processorDecoratorNames = new ArrayList<>(processorDecorators.stream()


how the list of processorDecorators is now ordered? I can't find documentation on that

For decorators it is: the higher the priority the lower you get in the order of wrappers:
decorator A with priority 1 -> decorator B with priority 2 -> the real instance

For injecting a list of beans or the instance object (basically a stream with optional, kind of): hight priority = earlier in the list.
decorator B priority 2 -> decorator A with priority 1

--> for wrapping, perfect order, we just need to loop to wrap the real instance with B and then B with A
--> for listing the decorator names in their order, we have to reverse the list: A -> B -> ...

vietk · 2024-10-11T06:31:08Z

impl/src/main/resources/META-INF/microprofile-config.properties

@@ -19,3 +19,6 @@ kafka-streams.internal.leave.group.on.close=true
 # Deactivate exposure of metrics through JMX beans
 # It is still adding a mxBean in AppInfoParser though
 kafka-streams.auto.include.jmx.reporter=false
+# For compatibility with generic decorators and Quarkus versions prior to 3.11.0
+# TODO: remove in main branch as the problem is not reproducible with Quarkus 3.11.0 and later versions
+quarkus.test.flat-class-path=true


do you need this property since you have switched to bean injection?

@decorator

We noticed flackiness with QuarkusTest and usage of `Processor` decorators. It is generating randomly ClassNotFoundException. It is happening only with Quarkus 3.8 LTS, recent versions like 3.15 or older like 3.2 do not present the issue. The solution found is a fallback of sorts, by removing the usage of @decorator in the processor decorators. Instead they are transformed in old-school, composition-design-pattern-inspired beans with a lombok delegate. The generic type signature is removed so they can be transformed in Dependent beans. Why's that? 1. A class with generics cannot be a bean, according to compilation errors, and 2. processors returned by the supplier need to be new instances everytime. Priorities are kept, and used to resolve in order the beans, for a manual encapsulation achieved with a for loop on the list of beans. And with that, the flackiness is gone. Of course, those changes will not be propagated to main and the future 3.15 branch, where this flackiness is not an issue, AND the usage of CDI's Decorator can be kept. The documentation is updated accordingly. Fixes quarkiverse#117

Changing version to 4.0.0-SNAPSHOT to reflect the update to Quarkus 3.15 and the creation of 3.8 with the fix of Decorators. Here the Decorator fix is restored, as the flakiness can still be seen on [master](https://github.com/quarkiverse/quarkus-kafka-streams-processor/actions/runs/11296240001/job/31420666727). The original PR was [here](#118). The documentation is updated to only talk about ProcessorDecorator using the AbstractProcessorDecorator abstract class.

edeweerd1A requested a review from a team as a code owner October 8, 2024 11:50

github-actions bot reviewed Oct 8, 2024

View reviewed changes

edeweerd1A force-pushed the issue-decorator branch from ef494e9 to 41504b5 Compare October 8, 2024 11:52

edeweerd1A force-pushed the issue-decorator branch 3 times, most recently from aaf630d to 6a3f514 Compare October 8, 2024 13:33

rquinio1A reviewed Oct 9, 2024

View reviewed changes

edeweerd1A force-pushed the issue-decorator branch from 6a3f514 to 70f11a9 Compare October 9, 2024 14:27

vietk reviewed Oct 11, 2024

View reviewed changes

edeweerd1A force-pushed the issue-decorator branch from 70f11a9 to dc44045 Compare October 11, 2024 12:03

vietk approved these changes Oct 11, 2024

View reviewed changes

edeweerd1A merged commit b0b6083 into quarkiverse:quarkus-3.8 Oct 11, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(doc): Transform Processor CDI Decorator into manual decorators #118

fix(doc): Transform Processor CDI Decorator into manual decorators #118

edeweerd1A commented Oct 8, 2024

github-actions bot Oct 8, 2024

github-actions bot Oct 8, 2024

edeweerd1A commented Oct 8, 2024

rquinio1A Oct 9, 2024

edeweerd1A Oct 9, 2024

rquinio1A Oct 9, 2024

edeweerd1A Oct 10, 2024

rquinio1A Oct 9, 2024

edeweerd1A Oct 10, 2024

vietk Oct 11, 2024

vietk Oct 11, 2024

edeweerd1A Oct 11, 2024

vietk Oct 11, 2024

edeweerd1A Oct 11, 2024

fix(doc): Transform Processor CDI Decorator into manual decorators #118

fix(doc): Transform Processor CDI Decorator into manual decorators #118

Conversation

edeweerd1A commented Oct 8, 2024

github-actions bot Oct 8, 2024

Choose a reason for hiding this comment

github-actions bot Oct 8, 2024

Choose a reason for hiding this comment

edeweerd1A commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment