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; + } + } }