From fb1fdee8939dc8ba7040fd4b976587f2da678096 Mon Sep 17 00:00:00 2001 From: Nikol Georgieva Date: Wed, 9 Sep 2026 22:44:45 +0300 Subject: [PATCH] engine-java: make the 'a JavaDelegate must NOT be a @Component' rule observable (#7223) The rule that a BPMN JavaDelegate must not be annotated @Component was documented in several places but nothing in the runtime enforced, warned about, or even noticed a violation - and after #7189 the old symptom (injected fields reading null at runtime) is gone, because both the container-built singleton and the Flowable-built instance are now wired. So a developer had no signal at all: an annotated delegate silently becomes a registered bean nobody runs, a stray candidate for every List injection. Change: - ComponentContainer.createUnmanaged now emits one LOGGER.warn naming the class and the rule when the delegate type is a bean, making the violation observable at the moment it happens. A unit test asserts the warn fires. - ComponentContainerUnmanagedTest.an_unsatisfied_dependency_is_refused_and_is_not_a_rebuild_error built the container from NO classes, so its wiringErrors().isEmpty() assertion was trivially true; it now builds with a registered bean and a delegate needing an absent type, so the "not a rebuild error" half is load-bearing. - DirigibleJavaCallDelegate: the fallback instantiation error no longer advises "declare the collaborators it injects as @Component" - that branch is reached only when the class declares no injection point, so the advice can never apply; reverted to the plain message the camel sibling (DirigibleJavaInvokerImpl) already uses. Its class javadoc now lists @PostConstruct alongside constructor and @Inject field as an injection point (both are counted). - ComponentContainer / JavaLoader javadoc no longer list @Extension among the consumers - the SDK has no such annotation. Verified: mvn formatter:validate (BUILD SUCCESS, cache wiped); engine-java + engine-bpm-flowable unit suites green (129 tests, incl. the new warn test); JavaDelegateInjectionIT green (3/3); release-profile javadoc build green on both modules. Fixes #7223 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../delegate/DirigibleJavaCallDelegate.java | 10 ++--- .../java/component/ComponentContainer.java | 9 +++- .../engine/java/runtime/JavaLoader.java | 2 +- .../ComponentContainerUnmanagedTest.java | 43 ++++++++++++++++++- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/DirigibleJavaCallDelegate.java b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/DirigibleJavaCallDelegate.java index 9bc76169310..7466f2f13e7 100644 --- a/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/DirigibleJavaCallDelegate.java +++ b/components/engine/engine-bpm-flowable/src/main/java/org/eclipse/dirigible/components/engine/bpm/flowable/delegate/DirigibleJavaCallDelegate.java @@ -42,9 +42,9 @@ * value is the fully-qualified class name of a client class implementing {@link JavaDelegate}. The * class is resolved through the currently-installed {@link ClientClassLoader} (managed by * {@link ClientClassLoaderHolder}), instantiated afresh on every execution - through the client - * bean container when it declares an injection point (constructor or {@code @Inject} field), else - * via its public no-arg constructor - and invoked with the engine-provided - * {@link DelegateExecution}. + * bean container when it declares an injection point (constructor, {@code @Inject} field or + * {@code @PostConstruct} method), else via its public no-arg constructor - and invoked with the + * engine-provided {@link DelegateExecution}. * *

* For the alternative {@code flowable:class="..."} approach, see the classloader configured on the @@ -205,8 +205,8 @@ private static JavaDelegate instantiate(Class handlerClass, String fqn) { return (JavaDelegate) handlerClass.getDeclaredConstructor() .newInstance(); } catch (ReflectiveOperationException e) { - throw new BpmnRuntimeException("Failed to instantiate client Java class [" + fqn - + "]. A public no-arg constructor is required, or declare the collaborators it injects as @Component.", e); + throw new BpmnRuntimeException( + "Failed to instantiate client Java class [" + fqn + "]. A public no-arg constructor is required.", e); } } diff --git a/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/component/ComponentContainer.java b/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/component/ComponentContainer.java index 1d2ddfbcd17..626cd692a3e 100644 --- a/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/component/ComponentContainer.java +++ b/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/component/ComponentContainer.java @@ -43,8 +43,8 @@ * eagerly instantiates singletons with recursive constructor injection (plus * {@code @Inject} field injection and {@code @PostConstruct} callbacks), detecting construction * cycles. The behaviour consumers ({@code @Controller}, {@code @Scheduled}, {@code @Listener}, - * {@code @Websocket}, {@code @Extension}) then fetch the ready instances via - * {@link #instanceOf(Class)} rather than instantiating client classes themselves. + * {@code @Websocket}) then fetch the ready instances via {@link #instanceOf(Class)} rather than + * instantiating client classes themselves. * *

* Implements {@link ClientBeanFactory} and publishes itself into {@link ClientBeansHolder} so the @@ -435,6 +435,11 @@ public List getAll(Class type) { */ @Override public Optional createUnmanaged(Class type) { + if (isBean(type)) { + LOGGER.warn( + "[{}] is a JavaDelegate annotated @Component. A JavaDelegate must NOT be a @Component: Flowable instantiates the delegate itself, so the annotation additionally builds a container-managed singleton the engine never runs — a stray candidate for every List injection. Remove @Component from the delegate.", + type.getName()); + } BeanDefinition definition = new BeanDefinition(type.getName(), type); if (!declaresInjectionPoint(definition)) { // Nothing to wire: the caller's own no-arg instantiation is equivalent, so it stays on it diff --git a/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/runtime/JavaLoader.java b/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/runtime/JavaLoader.java index cd3af2f8838..0e2dd1d418e 100644 --- a/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/runtime/JavaLoader.java +++ b/components/engine/engine-java/src/main/java/org/eclipse/dirigible/engine/java/runtime/JavaLoader.java @@ -236,7 +236,7 @@ private Set applyGeneration(Map nextGeneration, Cli // Build the client bean container for the new generation BEFORE the load pass: every // @Component (and the meta-annotated @Controller / @Repository / @Scheduled / @Listener / - // @Websocket / @Extension) is instantiated here with constructor + field injection, so the + // @Websocket) is instantiated here with constructor + field injection, so the // behaviour consumers below just fetch ready instances via ComponentContainer#instanceOf. componentContainer.rebuild(nextGeneration.values()); diff --git a/components/engine/engine-java/src/test/java/org/eclipse/dirigible/engine/java/component/ComponentContainerUnmanagedTest.java b/components/engine/engine-java/src/test/java/org/eclipse/dirigible/engine/java/component/ComponentContainerUnmanagedTest.java index b2f7f12ee2a..ca2a5ad3e6d 100644 --- a/components/engine/engine-java/src/test/java/org/eclipse/dirigible/engine/java/component/ComponentContainerUnmanagedTest.java +++ b/components/engine/engine-java/src/test/java/org/eclipse/dirigible/engine/java/component/ComponentContainerUnmanagedTest.java @@ -21,7 +21,12 @@ import org.eclipse.dirigible.sdk.component.Component; import org.eclipse.dirigible.sdk.component.Inject; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; @@ -135,8 +140,11 @@ void a_post_construct_only_class_is_still_wired_so_its_callback_is_not_silently_ @Test void an_unsatisfied_dependency_is_refused_and_is_not_a_rebuild_error() { - ComponentContainer container = TestComponentContainers.of(); + // A real rebuild that registered a bean, so wiringErrors() reflects an actual generation and + // the "not a rebuild error" assertion below is load-bearing rather than trivially empty. + ComponentContainer container = TestComponentContainers.of(EnglishGreeter.class); + // The delegate needs RateProvider, which this container does not know: an unsatisfied dependency. BeanContainerException exception = assertThrows(BeanContainerException.class, () -> container.createUnmanaged(ConstructorDelegate.class)); @@ -173,6 +181,29 @@ void a_field_name_matching_a_bean_name_disambiguates() { assertEquals(GermanGreeter.class, delegate.germanGreeter.getClass()); } + @Test + void a_delegate_annotated_component_is_warned_about_because_the_rule_is_otherwise_unobservable() { + Logger logger = (Logger) LoggerFactory.getLogger(ComponentContainer.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + try { + ComponentContainer container = TestComponentContainers.of(RateProvider.class, ComponentDelegate.class); + + container.createUnmanaged(ComponentDelegate.class) + .orElseThrow(); + + assertTrue(appender.list.stream() + .anyMatch(event -> event.getLevel() == Level.WARN && event.getFormattedMessage() + .contains(ComponentDelegate.class.getName()) + && event.getFormattedMessage() + .contains("must NOT be a @Component")), + () -> "expected a WARN naming the delegate and the rule, got: " + appender.list); + } finally { + logger.detachAppender(appender); + } + } + // --- fixtures (not @Component: a delegate is never a bean) ------------------------------------ @Component @@ -241,4 +272,14 @@ static class NameHintedDelegate { @Inject Greeter germanGreeter; } + + /** The mistake the rule forbids: a delegate annotated {@code @Component}. */ + @Component + static class ComponentDelegate { + final RateProvider rates; + + ComponentDelegate(RateProvider rates) { + this.rates = rates; + } + } }