Introduce component decorators #2537

CodeDrivenMitch · 2022-12-30T12:37:01Z

You can now decorate Component instances by calling Configurer.registerComponentDecorator. Upon initialization of the component, the original component will be constructed and the decorators will be called in order. Decorators can return the existing component, a modified version of it or a completely different implementation, as long as it's the same type.

smcvb

Just a couple of small remarks, mainly documentation. Other than that, this looks good to me.

config/src/main/java/org/axonframework/config/Component.java

smcvb · 2023-01-02T11:51:02Z

config/src/main/java/org/axonframework/config/ComponentDecorator.java

+@FunctionalInterface
+public interface ComponentDecorator<T> {
+
+    T decorate(Configuration configuration, T component);


This public method deserves documentation.

I assume these changes are still pending?

config/src/main/java/org/axonframework/config/ComponentDecorator.java

config/src/main/java/org/axonframework/config/Configurer.java

config/src/main/java/org/axonframework/config/DefaultConfigurer.java

messaging/src/main/java/org/axonframework/tracing/SpanFactory.java

You can now decorate `Component` instances by calling `Configurer.registerComponentDecorator`. Upon initialization of the component, the original component will be constructed and the decorators will be called in order. Decorators can return the existing component, a modified version of it or a completely different implementation, as long as it's the same type.

sonarcloud · 2023-01-09T09:22:52Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

87.5% Coverage
0.0% Duplication

config/src/main/java/org/axonframework/config/Configurer.java

abuijze

Looks pretty good. Have a few recommendations for changes, though

abuijze · 2023-01-09T19:41:16Z

config/src/main/java/org/axonframework/config/Component.java

+        Function<Configuration, ? extends B> previous = builderFunction;
+        this.update((configuration) -> {
+            B wrappedComponent = previous.apply(configuration);
+            LifecycleHandlerInspector.registerLifecycleHandlers(configuration, wrappedComponent);


This line makes me wonder: should we always call this, or should we rely on the decorator to do this? In the latter case, a decorator could influence the startup process. For example to implement some lazy initialization.

Do we want a decorator to have that much power? I am afraid of abuse. In addition, I feel a decorator is there to enhance a component as-is, without modifying the original functionality.
It does open up some possibilities though, like you say! But it's already easy to miss

The Configurer.registerComponentDecorator takes as an argument whether the user wants the lifecycle handlers called. If the boolean is not supplied, they are (by an overloaded method)

config/src/main/java/org/axonframework/config/ComponentDecorator.java

config/src/main/java/org/axonframework/config/Configurer.java

config/src/main/java/org/axonframework/config/DefaultConfigurer.java

…omponent when initializing

smcvb · 2023-01-20T10:15:40Z

Moving this pull request to release 4.8.0, so we can implement a workable integration with Spring for decorators on Axon Framework's infrastructure components.

abuijze

I have some concerns about the lifecycle handler registration proposal. I'm afraid this implementation could lead to excessive registration of default handlers.

EDIT - Should have checked a second time. Because the original instance is swapped with the decorated one, the lifecycle handlers will only be called once. 🤦

abuijze · 2023-02-13T09:56:50Z

config/src/main/java/org/axonframework/config/Component.java

     *
     * @return the initialized component contained in this instance
     */
    public B get() {
        if (instance == null) {
            Configuration configuration = configSupplier.get();
            instance = builderFunction.apply(configuration);
+            List<ComponentDecorator> decorators = configuration != null ? configuration.getDecorators() : Collections.emptyList();


I'm curious why the null-check is here. It really shouldn't be null, as the configuration is passed as a parameter to the builder function. A null configuration at this point should be a configuration problem.

abuijze · 2023-02-13T09:59:26Z

config/src/main/java/org/axonframework/config/Configuration.java

@@ -412,6 +412,10 @@ default ScopeAwareProvider scopeAwareProvider() {
     */
    EventUpcasterChain upcasterChain();

+    default List<ComponentDecorator> getDecorators() {
+        throw new AxonConfigurationException("The getDecorators has not been implemented by the Configuration!");


If it's not implemented, wouldn't the default behavior of returning an empty list suffice?

abuijze · 2023-02-13T12:21:46Z

config/src/main/java/org/axonframework/config/Configurer.java

+                                                      @Nonnull BiFunction<Configuration, C, C> decoratorFunction,
+                                                      boolean registerOriginalLifeCycleHandlers
+    ) {
+        throw new AxonConfigurationException(


Since this API isn't designed to be implemented by others, but rather for the abstraction of the implementation, I suggest we don't provide a default implementation of this method.

abuijze · 2023-02-13T12:28:54Z

config/src/main/java/org/axonframework/config/ComponentTypeSafeDecorator.java

+    public Object decorate(Configuration configuration, Object component) {
+        if (componentClass.isAssignableFrom(component.getClass())) {
+            T result = decorator.apply(configuration, (T) component);
+            if (registerOriginalLifeCycleHandlers) {


~~What if a component is decorated more than once? In that case the registerLifecycleHandlers method would be invoked multiple times, and the component would perhaps be initialized multiple times.~~
~~Shouldn't be the decorator's responsibility to also decorate calls to Lifecycle.registerLifecycleHandlers?~~

abuijze · 2023-02-13T12:29:36Z

config/src/main/java/org/axonframework/config/Configurer.java

+     * @param <C>                               The declared type of component
+     * @return the current instance of the Configurer, for chaining purposes
+     */
+    default <C> Configurer registerComponentDecorator(@Nonnull Class<C> componentType,


I noticed the order of parameters here is different than for the constructor of the ComponentTypeSafeDecorator. The boolean is probably last here as it is an optional parameter?

abuijze · 2023-02-13T12:31:49Z

config/src/main/java/org/axonframework/config/DefaultConfigurer.java

+
+        @Override
+        public List<ComponentDecorator> getDecorators() {
+            return decorators;


shouldn't we return a Collections.unmodifiableList to make sure the list isn't modified externally?

smcvb · 2023-06-15T10:40:02Z

Time doesn't allow us to introduce this change into 4.8.0. Hence, I've removed the milestone.

CodeDrivenMitch requested review from a team, gklijs and smcvb and removed request for a team December 30, 2022 12:37

CodeDrivenMitch self-assigned this Dec 30, 2022

CodeDrivenMitch added this to the Release 4.7.0 milestone Dec 30, 2022

smcvb approved these changes Jan 2, 2023

View reviewed changes

CodeDrivenMitch force-pushed the feature/component-decorators branch from c7b0fa5 to 48e5e0b Compare January 9, 2023 08:56

Improve javadoc for Component decorators

c838eba

smcvb reviewed Jan 9, 2023

View reviewed changes

config/src/main/java/org/axonframework/config/Configurer.java Outdated Show resolved Hide resolved

abuijze requested changes Jan 9, 2023

View reviewed changes

CodeDrivenMitch added 2 commits January 13, 2023 16:24

Improve the decorator pattern by calling the Configuration from the c…

2e89ec3

…omponent when initializing

Fix typo in javadoc of the Configurer

7ec3701

CodeDrivenMitch requested review from abuijze and smcvb January 13, 2023 15:31

smcvb modified the milestones: Release 4.7.0, Release 4.8.0 Jan 20, 2023

abuijze requested changes Feb 13, 2023

View reviewed changes

smcvb removed this from the Release 4.8.0 milestone Jun 15, 2023

smcvb added Status: On Hold Use to signal that work on this issue is paused. The description should clarify why. and removed Status: In Progress Use to signal this issue is actively worked on. labels Jun 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce component decorators #2537

Introduce component decorators #2537

CodeDrivenMitch commented Dec 30, 2022 •

edited

smcvb left a comment

smcvb Jan 2, 2023

smcvb Jan 9, 2023

CodeDrivenMitch Jan 13, 2023

sonarcloud bot commented Jan 9, 2023

abuijze left a comment

abuijze Jan 9, 2023

CodeDrivenMitch Jan 13, 2023

CodeDrivenMitch Jan 13, 2023

smcvb commented Jan 20, 2023

abuijze left a comment •

edited

abuijze Feb 13, 2023

abuijze Feb 13, 2023

abuijze Feb 13, 2023

abuijze Feb 13, 2023 •

edited

abuijze Feb 13, 2023

abuijze Feb 13, 2023

smcvb commented Jun 15, 2023

Introduce component decorators #2537

Are you sure you want to change the base?

Introduce component decorators #2537

Conversation

CodeDrivenMitch commented Dec 30, 2022 • edited

smcvb left a 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

sonarcloud bot commented Jan 9, 2023

abuijze left a 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

smcvb commented Jan 20, 2023

abuijze left a comment • edited

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

abuijze Feb 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smcvb commented Jun 15, 2023

CodeDrivenMitch commented Dec 30, 2022 •

edited

abuijze left a comment •

edited

abuijze Feb 13, 2023 •

edited