fix(core): move the Spring beans under modules/ into the scan roots - #6639
Merged
Conversation
Three beans lived under modules/, in packages outside the two roots the platform treats as its scan convention (org.eclipse.dirigible.components and .engine - the pair DataSourceSystemConfig uses for @EnableJpaRepositories and dirigible.scan.packages). build/application never noticed, because DirigibleApplication sits in org.eclipse.dirigible and its default scan covers the whole tree; any assembly that NAMES its scan packages loses them. All three are contributor-shaped, so their absence is silent. A WebSocketConfigurer that is never registered just means the endpoint 404s; a List<ConnectionEnhancer> or List<DatabaseConfigurator> injection point just receives a shorter list. No missing-bean failure, no warning - the context comes up clean and the capability is quietly gone. HanaConnectionEnhancer -> components/data/data-sources HanaDatabaseConfigurator -> components/data/data-sources ConsoleWebsocketConfig -> components/ide/ide-logs The HANA pair moves next to the injection points that consume it (DirigibleDataSourceFactory, DataSourceInitializer) and joins its SPI siblings under components/; both are package-private, so nothing outside their package could reference them and the move costs no pom change - data-sources already depends on core-database, api-security and Hikari, and already carries a vendor-specific dependency (database-sql-h2). ConsoleWebsocketConfig joins the four other WebSocketConfigurers, every one of which already lives under components/ (ide-terminal, ide-java-lsp, ide-java-debug, data-transfer) - it was the only one stranded in modules/. ide-logs takes a dependency on commons-resources for ConsoleWebsocketHandler. The appender, handler and record deliberately stay where they are: logback configuration names ConsoleLoggingAppender by fully qualified class name, in this repo and in every downstream one, so moving that package would break configurations we do not control. Also drops the vestigial @component from DirigibleSourceProvider. Every consumer constructs it directly, so the stereotype only advertised an injection point that never existed - and its removal is behaviour-neutral. No Spring bean stereotype remains anywhere under modules/, which restores the invariant the tree is described by ("pure libraries with no Spring wiring"). The one remaining Spring reference there is ConsoleWebsocketHandler, which extends TextWebSocketHandler but is not a bean - it is constructed by the config and called statically by the appender. Registering the beans through an @autoConfiguration was the other option in the report, and was rejected: an auto-configuration class under org.eclipse.dirigible is itself covered by DirigibleApplication's default scan, so in the full assembly it would be registered twice - once by the scan and once from the imports file - giving two WebSocketConfigurers and a duplicated handler registration. Moving has no such ambiguity. Closes #6635 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6635.
Three Spring beans lived under
modules/, in packages outside the two roots the platform treats as itsscan convention (
org.eclipse.dirigible.componentsand.engine— the pairDataSourceSystemConfiguses for
@EnableJpaRepositoriesanddirigible.scan.packages).build/applicationnever noticed,because
DirigibleApplicationsits inorg.eclipse.dirigibleand its default scan covers the wholetree; any assembly that names its scan packages loses them, silently, because all three are
contributor-shaped.
HanaConnectionEnhancermodules/database/database-sql-hanacomponents/data/data-sourcesHanaDatabaseConfiguratormodules/database/database-sql-hanacomponents/data/data-sourcesConsoleWebsocketConfigmodules/commons/commons-resourcescomponents/ide/ide-logsPlus the vestigial
@Componentdropped fromDirigibleSourceProvider— every consumer constructs itdirectly, so the stereotype only advertised an injection point that never existed.
Why these homes
The HANA pair moves next to the injection points that consume it (
DirigibleDataSourceFactory,DataSourceInitializer, both indata-sources). Both classes are package-private, so nothing outsidetheir package could have referenced them, and the move needs no pom change:
data-sourcesalreadydepends on
core-database(the SPI),api-security(UserFacade) and Hikari, and already carries avendor-specific dependency (
database-sql-h2), so HANA specifics are not new there.ConsoleWebsocketConfigjoins the four otherWebSocketConfigurers —TerminalWebsocketConfig(ide-terminal),
JavaLspWebSocketConfig(ide-java-lsp),JavaDebugWebSocketConfig(ide-java-debug),DataTransferWebsocketConfig(data-transfer). Every one already lives undercomponents/; the consoleone was the only one stranded in
modules/.ide-logstakes a dependency oncommons-resourcesforConsoleWebsocketHandler.The appender, handler and log record deliberately stay put: logback configuration names
ConsoleLoggingAppenderby fully qualified class name — here incommons-resources/logback.xmlandtests-framework/logback-test.xml, and in downstream configurations we do not control — so moving thatpackage would silently break them. Only the bean moved.
Why not
@AutoConfigurationThe issue offered it as an alternative; it is a trap in this codebase. An auto-configuration class under
org.eclipse.dirigibleis itself covered byDirigibleApplication's default scan, so in the fullassembly it would be registered twice — once by the scan, once from the imports file — yielding two
WebSocketConfigurers and a duplicated handler registration on the same path. Moving has no suchambiguity.
Verification — including the part the issue could only infer
Built the full reactor, started the fat jar, and read the live bean graph from
/actuator/beans:All three reach exactly the injection points from the issue's table.
hanaConnectionEnhanceris loadedfrom
dirigible-components-data-sources.jarat the new path, confirming the relocation is what isbeing wired. The issue marked the two HANA beans "by inspection" for want of a HANA instance — this
demonstrates they reach their intake, though exercising HANA behaviour still needs a HANA server.
Websocket handshake, with controls so the check can actually fail:
Zero startup errors,
mvn -T 1C clean install -P quick-buildgreen on the whole reactor,formatter:validateclean on every touched module.Invariant restored
There is now no Spring bean stereotype anywhere under
modules/, which is what the tree isdescribed by ("pure libraries with no Spring wiring"). The single remaining Spring reference there is
ConsoleWebsocketHandler, which extendsTextWebSocketHandlerbut is not a bean — it is constructed bythe config and called statically by the appender.
Not done
The build guard the issue suggests. A blanket ban on
org.springframeworkundermodules/would notwork —
commons-resourceslegitimately still needsspring-websocketfor that handler type — so thecheck has to be "no bean stereotypes", which is a source grep rather than an enforcer rule. Happy to add
it as a follow-up if you want it; it seemed worth keeping separate from the fix.
🤖 Generated with Claude Code