diff --git a/linter-core/src/main/java/dev/dsf/linter/output/LintingType.java b/linter-core/src/main/java/dev/dsf/linter/output/LintingType.java index 9bc9d98..659fa45 100644 --- a/linter-core/src/main/java/dev/dsf/linter/output/LintingType.java +++ b/linter-core/src/main/java/dev/dsf/linter/output/LintingType.java @@ -246,7 +246,17 @@ public enum LintingType { // ==================== PLUGIN DEFINITION - SPRING CONFIGURATIONS ==================== PLUGIN_DEFINITION_SPRING_CONFIGURATION_MISSING( "A BPMN-referenced delegate or listener class is not provided as a @Bean " - + "in any @Configuration class returned by getSpringConfigurations()."); + + "in any @Configuration class returned by getSpringConfigurations()."), + + // ==================== SPRING BEAN SCOPE ==================== + SPRING_BEAN_SCOPE_MISSING( + "BPMN-referenced bean has no @Scope annotation (defaults to singleton)."), + SPRING_BEAN_SCOPE_SINGLETON_EXPLICIT( + "BPMN-referenced bean is explicitly configured as singleton."), + SPRING_BEAN_SCOPE_PROTOTYPE( + "BPMN-referenced bean is correctly configured as prototype."), + SPRING_BEAN_SCOPE_MUTABLE_SINGLETON( + "BPMN-referenced singleton bean has mutable (non-static, non-final) instance fields."); private final String defaultMessage; diff --git a/linter-core/src/main/java/dev/dsf/linter/service/SpringConfigurationLinter.java b/linter-core/src/main/java/dev/dsf/linter/service/SpringConfigurationLinter.java index d7fd93e..0fb24b8 100644 --- a/linter-core/src/main/java/dev/dsf/linter/service/SpringConfigurationLinter.java +++ b/linter-core/src/main/java/dev/dsf/linter/service/SpringConfigurationLinter.java @@ -20,7 +20,9 @@ import java.io.File; import java.lang.annotation.Annotation; +import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -28,34 +30,74 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; /** - * Validates that every class referenced as a Camunda delegate or listener in a - * BPMN file is provided as a {@code @Bean} in at least one of the - * {@code @Configuration} classes registered via - * {@code ProcessPluginDefinition#getSpringConfigurations()}. + * Validates DSF process plugins against how Spring provides Camunda delegates + * and listeners: bean registration and (for covered beans) {@code @Scope} plus + * mutable instance fields. * - *

Background:

+ *

Background

*

* In the DSF environment the Camunda engine does not instantiate Java delegate * or listener classes directly. Spring creates those instances via {@code @Bean} - * methods declared in {@code @Configuration} classes. For those beans to be - * available at runtime, every BPMN-referenced class must have a corresponding - * {@code @Bean} method in a configuration class that is explicitly returned by - * {@code ProcessPluginDefinition#getSpringConfigurations()}. - * A missing entry typically surfaces as a {@code BeanCreationException} or - * {@code ClassNotFoundException} only at deployment time. + * methods on {@code @Configuration} classes that must be returned by + * {@code ProcessPluginDefinition#getSpringConfigurations()}. A missing + * {@code @Bean} for a BPMN-referenced class often appears only at deployment + * time as a {@code BeanCreationException} or {@code ClassNotFoundException}. *

+ *

+ * DSF best practice is prototype scope for such beans. Omitting + * {@code @Scope} defaults Spring to singleton, which is unsafe if + * the implementation ever holds mutable instance state (non-{@code static}, + * non-{@code final} fields) across concurrent process executions. + *

+ * + *

Validation pipeline (see {@link #lint})

+ *
    + *
  1. Load registered {@code @Configuration} classes from + * {@code getSpringConfigurations()}.
    2. + *
  2. Scan plugin BPMN files for {@code camunda:class} references (service + * and send tasks, throw events with message definitions, execution and + * task listeners).
    3. + *
  3. Build {@code @Bean} return types per configuration class; check each + * referenced class is covered (exact FQN or supertype assignability).
    4. + *
  4. For each covered class, find the covering {@code @Bean} method + * and inspect its {@code @Scope} and the implementation class for mutable + * instance fields.
    5. + *
* - *

Emitted lint items:

+ *

Emitted {@link LintingType} values

+ *

Bean registration

* + *

Scope and mutable state (covered classes only)

+ * */ public final class SpringConfigurationLinter { @@ -65,20 +107,37 @@ public final class SpringConfigurationLinter { /** Fully qualified name of the Spring {@code @Bean} annotation. */ private static final String BEAN_ANNOTATION = "org.springframework.context.annotation.Bean"; + /** Fully qualified name of the Spring {@code @Scope} annotation. */ + private static final String SCOPE_ANNOTATION = "org.springframework.context.annotation.Scope"; + private SpringConfigurationLinter() { } /** - * Runs the Spring-configuration validation. + * Runs Spring configuration validation: {@code @Bean} coverage for all BPMN + * delegate/listener references, then for each covered class an + * optional scope/mutability pass ({@code @Scope} on the covering + * {@code @Bean} method and mutable instance fields on the implementation + * type). * - * @param adapter the plugin adapter used to invoke - * {@code getSpringConfigurations()} reflectively. - * @param bpmnFiles BPMN files belonging to this plugin (used to collect - * delegate/listener class references). - * @param projectDir the extracted project root (used only to derive the - * location label for lint items; may be {@code null}). - * @param logger logger for diagnostic output (may be {@code null}). - * @return list of lint items; never {@code null}. + *

When {@code bpmnFiles} yields no references, returns a single success item + * and does not run registration or scope checks.

+ * + *

When there are references, emits one error per uncovered class, then for + * each covered class (if a covering {@code @Bean} method was resolved): + * prototype scope → one success item; otherwise if mutable fields → error, + * then either missing {@code @Scope} → warning or explicit non-prototype scope + * → warning. If every reference is covered by some {@code @Bean}, a summary + * success item for full registration coverage is appended.

+ * + * @param adapter the plugin adapter used to invoke + * {@code getSpringConfigurations()} reflectively + * @param bpmnFiles BPMN files belonging to this plugin (used to collect + * delegate/listener class references) + * @param projectDir the extracted project root (used only for the file + * label on lint items; may be {@code null}) + * @param logger logger for diagnostic output (may be {@code null}) + * @return ordered list of lint items; never {@code null} */ public static List lint(PluginAdapter adapter, List bpmnFiles, @@ -172,6 +231,79 @@ public static List lint(PluginAdapter adapter, )); } + // Step 5.5: For each covered class check @Scope on the covering @Bean method + // and report mutable-field hazards for effective-singleton beans. + Map> configBeanMethods = new LinkedHashMap<>(); + for (Class configClass : registered) { + if (configClass != null) { + configBeanMethods.put(configClass.getName(), + extractBeanMethodMap(configClass, logger)); + } + } + for (String refClass : referencedBpmnClasses) { + if (uncoveredClasses.contains(refClass)) { + continue; // already reported as ERROR in step 5 + } + Optional coveringMethod = findCoveringMethod(configBeanMethods, refClass, cl); + if (coveringMethod.isEmpty()) { + continue; + } + String scopeValue = getScopeValue(coveringMethod.get()); + boolean mutable = hasMutableInstanceFields(refClass, cl); + + if ("prototype".equals(scopeValue)) { + items.add(new PluginLintItem( + LinterSeverity.SUCCESS, + LintingType.SPRING_BEAN_SCOPE_PROTOTYPE, + locationFile, + refClass, + "BPMN-referenced class '" + simpleName(refClass) + "' (" + + refClass + ") is correctly configured as a prototype-scoped @Bean." + )); + continue; // prototype is safe – no further scope checks needed + } + + // Effective singleton (missing @Scope or explicit non-prototype): check mutable fields first. + if (mutable) { + items.add(new PluginLintItem( + LinterSeverity.ERROR, + LintingType.SPRING_BEAN_SCOPE_MUTABLE_SINGLETON, + locationFile, + refClass, + "BPMN-referenced class '" + simpleName(refClass) + "' (" + + refClass + ") is effectively singleton-scoped and " + + "contains mutable (non-static, non-final) instance fields. " + + "This will cause race conditions under concurrent process execution." + )); + } + + if (scopeValue == null) { + items.add(new PluginLintItem( + LinterSeverity.WARN, + LintingType.SPRING_BEAN_SCOPE_MISSING, + locationFile, + refClass, + "BPMN-referenced class '" + simpleName(refClass) + "' (" + + refClass + ") has a @Bean method without an explicit @Scope " + + "annotation. Spring defaults to singleton scope, which is risky " + + "for Camunda delegates and listeners. Consider adding " + + "@Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE)." + )); + } else { + // Explicit non-prototype scope (typically "singleton") + items.add(new PluginLintItem( + LinterSeverity.WARN, + LintingType.SPRING_BEAN_SCOPE_SINGLETON_EXPLICIT, + locationFile, + refClass, + "BPMN-referenced class '" + simpleName(refClass) + "' (" + + refClass + ") is explicitly configured with @Scope(\"" + + scopeValue + "\"). Singleton-scoped Camunda delegates and " + + "listeners must be actually completely stateless." + )); + } + } + if (uncoveredClasses.isEmpty()) { items.add(PluginLintItem.success( locationFile, @@ -340,4 +472,132 @@ private static String simpleName(String fqn) { int i = fqn.lastIndexOf('.'); return (i >= 0) ? fqn.substring(i + 1) : fqn; } + + // ==================== @Bean METHOD MAP (for scope checks) ==================== + + /** + * Returns a map from return-type name to the {@code @Bean}-annotated {@link Method} + * for all bean methods declared directly on {@code configClass}. + * Analogous to {@link #extractBeanReturnTypes} but retains the {@code Method} + * object so callers can inspect further annotations such as {@code @Scope}. + */ + private static Map extractBeanMethodMap(Class configClass, Logger logger) { + Map result = new LinkedHashMap<>(); + try { + for (Method m : configClass.getDeclaredMethods()) { + if (hasAnnotationByName(m.getAnnotations())) { + Class rt = m.getReturnType(); + if (rt != void.class && rt != Void.class) { + result.putIfAbsent(rt.getName(), m); + } + } + } + } catch (Throwable t) { + if (logger != null) { + logger.debug("Could not read @Bean methods of '" + + configClass.getName() + "' for scope check: " + t.getMessage()); + } + } + return result; + } + + /** + * Searches all registered config bean-method maps for the first {@code @Bean} + * method whose return type covers {@code referencedClass} – either by exact name + * or by assignability ({@code returnType.isAssignableFrom(referencedClass)}). + */ + private static Optional findCoveringMethod( + Map> configBeanMethods, + String referencedClass, + ClassLoader cl) { + + // 1. Exact match across all configs + for (Map methodMap : configBeanMethods.values()) { + Method m = methodMap.get(referencedClass); + if (m != null) { + return Optional.of(m); + } + } + + // 2. Assignability fallback + if (cl == null) { + return Optional.empty(); + } + Class ref; + try { + ref = Class.forName(referencedClass, false, cl); + } catch (ClassNotFoundException | LinkageError e) { + return Optional.empty(); + } + for (Map methodMap : configBeanMethods.values()) { + for (Map.Entry entry : methodMap.entrySet()) { + try { + Class returnType = Class.forName(entry.getKey(), false, cl); + if (returnType.isAssignableFrom(ref)) { + return Optional.of(entry.getValue()); + } + } catch (ClassNotFoundException | LinkageError ignored) { + // skip unresolvable types + } + } + } + return Optional.empty(); + } + + // ==================== @Scope READING ==================== + + /** + * Returns the {@code value()} of the {@code @Scope} annotation present on + * {@code beanMethod}, or {@code null} if no {@code @Scope} annotation is found. + * + *

The annotation is identified by its fully-qualified class name to tolerate + * cases where the annotation was loaded by a different {@link ClassLoader} than + * the linter's own class loader.

+ */ + private static String getScopeValue(Method beanMethod) { + for (Annotation a : beanMethod.getAnnotations()) { + if (SCOPE_ANNOTATION.equals(a.annotationType().getName())) { + try { + Object value = a.annotationType().getMethod("value").invoke(a); + if (value instanceof String s && !s.isBlank()) { + return s; + } + // scopeName() is an alias for value() in some Spring versions + Object scopeName = a.annotationType().getMethod("scopeName").invoke(a); + if (scopeName instanceof String s && !s.isBlank()) { + return s; + } + } catch (Exception ignored) { + // Annotation structure unexpected – treat as no scope + } + return null; + } + } + return null; + } + + // ==================== MUTABLE FIELD DETECTION ==================== + + /** + * Returns {@code true} when the class identified by {@code className} declares at + * least one instance field that is neither {@code static} nor {@code final}. + * Such fields are a concurrency hazard when the bean is singleton-scoped. + */ + private static boolean hasMutableInstanceFields(String className, ClassLoader cl) { + if (cl == null) { + return false; + } + try { + Class clazz = Class.forName(className, false, cl); + for (Field f : clazz.getDeclaredFields()) { + int mod = f.getModifiers(); + if (!Modifier.isStatic(mod) && !Modifier.isFinal(mod)) { + return true; + } + } + } catch (ClassNotFoundException | LinkageError ignored) { + // Cannot load class – skip mutable-field check + } + return false; + } } diff --git a/linter-core/src/test/java/dev/dsf/linter/service/SpringConfigurationLinterTest.java b/linter-core/src/test/java/dev/dsf/linter/service/SpringConfigurationLinterTest.java index cbacd1f..1e275ae 100644 --- a/linter-core/src/test/java/dev/dsf/linter/service/SpringConfigurationLinterTest.java +++ b/linter-core/src/test/java/dev/dsf/linter/service/SpringConfigurationLinterTest.java @@ -6,6 +6,9 @@ import dev.dsf.linter.output.item.AbstractLintItem; import dev.dsf.linter.plugin.PluginDefinitionDiscovery.PluginAdapter; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Scope; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -131,6 +134,78 @@ void bpmnReference_coveredByRegisteredBean_yieldsSuccess() throws IOException { } + // ==================== @Scope tests ==================== + + @Test + void beanWithoutScope_noMutableFields_yieldsWarn() throws IOException { + // ConfigNoScope provides a @Bean with no @Scope; the bean class has only + // final fields → WARN about missing scope, but no ERROR for mutable fields. + File bpmn = writeBpmn(tempProjectRoot, ConfigNoScope.ImmutableBean.class.getName()); + PluginAdapter adapter = stubAdapter(List.of(ConfigNoScope.class)); + + List items = SpringConfigurationLinter.lint( + adapter, List.of(bpmn), tempProjectRoot.toFile(), silentLogger()); + + assertTrue(items.stream().anyMatch(i -> i.getSeverity() == LinterSeverity.WARN + && i.getType() == LintingType.SPRING_BEAN_SCOPE_MISSING), + "Expected WARN for missing @Scope"); + assertFalse(items.stream().anyMatch(i -> i.getType() == LintingType.SPRING_BEAN_SCOPE_MUTABLE_SINGLETON), + "No ERROR expected when bean class has no mutable fields"); + } + + @Test + void beanWithoutScope_mutableField_yieldsWarnAndError() throws IOException { + // ConfigNoScopeMutable provides a @Bean with no @Scope; the bean class has a + // mutable field → WARN for missing scope AND ERROR for mutable singleton. + File bpmn = writeBpmn(tempProjectRoot, ConfigNoScopeMutable.MutableBean.class.getName()); + PluginAdapter adapter = stubAdapter(List.of(ConfigNoScopeMutable.class)); + + List items = SpringConfigurationLinter.lint( + adapter, List.of(bpmn), tempProjectRoot.toFile(), silentLogger()); + + assertTrue(items.stream().anyMatch(i -> i.getSeverity() == LinterSeverity.WARN + && i.getType() == LintingType.SPRING_BEAN_SCOPE_MISSING), + "Expected WARN for missing @Scope"); + assertTrue(items.stream().anyMatch(i -> i.getSeverity() == LinterSeverity.ERROR + && i.getType() == LintingType.SPRING_BEAN_SCOPE_MUTABLE_SINGLETON), + "Expected ERROR for mutable fields on effective-singleton bean"); + } + + @Test + void beanWithExplicitSingleton_mutableField_yieldsWarnAndError() throws IOException { + // ConfigExplicitSingleton uses @Scope("singleton") explicitly; the bean class + // has a mutable field → WARN for explicit singleton AND ERROR for mutable fields. + File bpmn = writeBpmn(tempProjectRoot, ConfigExplicitSingleton.MutableBean.class.getName()); + PluginAdapter adapter = stubAdapter(List.of(ConfigExplicitSingleton.class)); + + List items = SpringConfigurationLinter.lint( + adapter, List.of(bpmn), tempProjectRoot.toFile(), silentLogger()); + + assertTrue(items.stream().anyMatch(i -> i.getSeverity() == LinterSeverity.WARN + && i.getType() == LintingType.SPRING_BEAN_SCOPE_SINGLETON_EXPLICIT), + "Expected WARN for explicit singleton scope"); + assertTrue(items.stream().anyMatch(i -> i.getSeverity() == LinterSeverity.ERROR + && i.getType() == LintingType.SPRING_BEAN_SCOPE_MUTABLE_SINGLETON), + "Expected ERROR for mutable fields on explicit-singleton bean"); + } + + @Test + void beanWithPrototypeScope_yieldsSuccess_evenWithMutableFields() throws IOException { + // ConfigPrototype uses @Scope("prototype"); the bean class has a mutable field + // but prototype scope is safe → SUCCESS, no ERROR for mutable fields. + File bpmn = writeBpmn(tempProjectRoot, ConfigPrototype.AnyBean.class.getName()); + PluginAdapter adapter = stubAdapter(List.of(ConfigPrototype.class)); + + List items = SpringConfigurationLinter.lint( + adapter, List.of(bpmn), tempProjectRoot.toFile(), silentLogger()); + + assertTrue(items.stream().anyMatch(i -> i.getSeverity() == LinterSeverity.SUCCESS + && i.getType() == LintingType.SPRING_BEAN_SCOPE_PROTOTYPE), + "Expected SUCCESS for prototype-scoped bean"); + assertFalse(items.stream().anyMatch(i -> i.getType() == LintingType.SPRING_BEAN_SCOPE_MUTABLE_SINGLETON), + "No ERROR expected for prototype-scoped bean regardless of mutable fields"); + } + // ==================== Helpers ==================== private static PluginAdapter stubAdapter(List> springConfigs) { @@ -191,9 +266,63 @@ static final class ConfigWithBean { static final class BeanClass { } - @org.springframework.context.annotation.Bean + @Bean public BeanClass beanClass() { return new BeanClass(); } } + + /** Config with a @Bean and no @Scope; bean class has only final fields (immutable). */ + static final class ConfigNoScope { + @SuppressWarnings("unused") + static final class ImmutableBean { + private final int x = 0; + } + + @Bean + public ImmutableBean bean() { + return new ImmutableBean(); + } + } + + /** Config with a @Bean and no @Scope; bean class has a mutable instance field. */ + static final class ConfigNoScopeMutable { + @SuppressWarnings("unused") + static final class MutableBean { + private int counter; // mutable – not static, not final + } + + @Bean + public MutableBean bean() { + return new MutableBean(); + } + } + + /** Config with an explicit @Scope("singleton"); bean class has a mutable field. */ + static final class ConfigExplicitSingleton { + @SuppressWarnings("unused") + static final class MutableBean { + private String state; // mutable + } + + @Bean + @Scope("singleton") + public MutableBean bean() { + return new MutableBean(); + } + } + + /** Config with @Scope("prototype"); bean class has a mutable field (safe for prototype). */ + static final class ConfigPrototype { + @SuppressWarnings("unused") + static final class AnyBean { + private int x; // mutable, but prototype-scoped → no ERROR + } + + @Bean + @Scope("prototype") + public AnyBean bean() { + return new AnyBean(); + } + } }