Repository navigation
CAMEL-25918: camel-as2 - consumers sharing a port use their own asynchronous MDN settings - #28274
Conversation
…hronous MDN settings AS2 consumers that share a serverPortNumber delivered their asynchronous MDNs with the mdnUserName, mdnPassword, mdnAccessToken and asyncMdnAllowedHosts of the first endpoint created on the port: the shared AS2ServerConnection kept those values as connection-wide fields, while the signing material was already selected per request path. The MDN settings are now part of the per-path AS2ConsumerConfiguration. Each consumer registers its own values for its own requestUriPattern, also for the default "/" pattern, which no longer inherits the fallback, and the asynchronous MDN of a request uses the settings of the matched path. The fallback configuration of the root path carries the values given to the connection's constructor, so direct users of AS2ServerConnection behave as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
✅ Clean, well-scoped fix. The move of async MDN settings (username, password, access token, allowed hosts) from connection-level fields into AS2ConsumerConfiguration is the right design — it allows consumers sharing a port to each have their own async MDN configuration.
Implementation looks correct:
doStopproperly cleans up viaunlisten→consumerConfigurations.removedoStartunconditionally re-registers, which is correct for restart scenariossynchronizedMapprovides adequate per-operation thread safety for theConcurrentHashMap-backed structure- All 6 integration tests are confirmed to cover the multi-consumer scenarios this fixes
💡 Minor nit: The new 9-arg AS2ConsumerConfiguration constructor (with mdnUserName, mdnPassword, mdnAccessToken, asyncMdnAllowedHosts) is missing a Javadoc comment, while the 5-arg constructor has one. Consider adding a /** ... */ block to keep the API documented consistently.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks Andrea,
The change looks good to me. The asynchronous MDN settings are now picked from the configuration matched for the request path, the same way the signing material already is. AS2Consumer.doStart now always registers the consumer's own configuration, and that also covers the consumer on / that used to keep the fallback built from the first endpoint. Registering the same pattern again also keeps the configuration in step with the handler: listen already used the last consumer, while the configuration used to stay with the first one. The old 5-argument AS2ConsumerConfiguration constructor is kept, so direct users of camel-as2-api are not affected. The tests cover each case from the JIRA.
Two optional doc points inline: the settings that are still per port (shared AS2ServerConnection), and the asyncMdnAllowedHosts change in the upgrade guide.
Claude Code on behalf of @davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 CI tested the following changed modules:
🔁 1 test passed only after a retry on JDK 21 (1 retried attempt) Recovered flaky tests on JDK 21 (1)
🔬 Scalpel shadow comparison — Scalpel: 9 of 707 tested, 24 compile-only — current: 9 all testedMaveniverse Scalpel detected 9 affected modules (current approach: 9). Skip-tests mode would test 9 modules (5 direct + 7 downstream), skip tests for 24 (generated code, meta-modules) Modules Scalpel would test (9)
Modules with tests skipped (24)
All tested modules (36 modules, 6m 10s total)Total reactor time: 6m 10s
Top 20 slowest modules:
|
… and the asyncMdnAllowedHosts change, javadoc for the MDN constructor The component page says which settings still come from the first endpoint created on a shared port (the TLS context, mdnMessageTemplate, server, serverFqdn and as2Version), the upgrade-guide note says what changes for a consumer without its own asyncMdnAllowedHosts, and the AS2ConsumerConfiguration constructor that carries the asynchronous MDN settings has a javadoc like the one without them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
Pushed one more commit for the review: "CAMEL-25918: Address review feedback - document the per-port settings and the asyncMdnAllowedHosts change, javadoc for the MDN constructor".
Documentation and javadoc only; no code change. Re-requested the review since the approval was given on the previous head. Claude Code on behalf of oscerd |
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Previous finding addressed. LGTM.
The only finding from my prior review — the missing Javadoc on the 9-argument AS2ConsumerConfiguration constructor — is now documented with a clear description of what the MDN credentials and asyncMdnAllowedHosts do. The 5-argument constructor got a matching doc block too. The component page now explicitly calls out which settings are per-consumer vs per-port, and the upgrade guide documents the asyncMdnAllowedHosts behavior change that davsclaus flagged.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks Andrea, the new commit addresses both doc points:
- The component page (and its catalog copy, which is identical) lists the settings that still come from the first endpoint on the port.
- The upgrade guide covers
asyncMdnAllowedHosts. The text matchesAS2AsynchronousMDNManager:isAllowedHostreturns false when no list is set, so the MDN goes to the address the sender gives, without theAuthorizationheader.
The new constructor javadoc also covers gnodet-bot's nit. This commit only changes docs and javadoc.
One heads-up: #28265 (AS2 HTTP proxy) changes the same parts of AS2Configuration.java, as2.json, the endpoint DSL and the end of as2-component.adoc. Whichever PR merges second will need a rebase and the generated files regenerated.
LGTM once CI is green.
Claude Code on behalf of @davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
davsclaus
added
port/camel-4.18.x
|
/port |
🔄 Backport BotConflicts (🤖 agent dispatched to resolve):
ℹ️ If you push additional commits after |
|
✅ Backport to |
|
✅ Backport to |
Jira: https://issues.apache.org/jira/browse/CAMEL-25918
Problem
AS2ConnectionHelperkeeps oneAS2ServerConnectionperserverPortNumber, built from the first endpoint created on that port. Since CAMEL-22557 the signing and decrypting material is selected per request path (AS2ServerConnection.AS2ConsumerConfiguration, registered byAS2Consumer.doStartfor the consumer'srequestUriPattern), but the asynchronous MDN settings (mdnUserName,mdnPasswordandmdnAccessToken, added by CAMEL-21875, andasyncMdnAllowedHosts) stayed connection-wide fields taken from the first endpoint only.When several
as2://server/listenconsumers share a port with differentrequestUriPatternvalues:/pattern never registered its own configuration when another endpoint had created the connection (doStartonly registered when no configuration matched, and the constructor's/fallback always matched);unlistenremoves only the path's handler and configuration.A single consumer per port is not affected. The four options are documented as endpoint options, so each consumer is expected to apply its own values.
Change
camel-as2-api,AS2ServerConnection:AS2ConsumerConfigurationcarriesmdnUserName,mdnPassword,mdnAccessTokenandasyncMdnAllowedHoststhrough a new constructor; the existing five-argument constructor stays and delegates with null values.handleAsynchronousMDNbuilds theAS2AsynchronousMDNManagerfrom the matched path's configuration instead of the connection-level fields, which are removed. The/fallback registered by the constructor carries the constructor's values, so direct users ofAS2ServerConnectionand single-consumer deployments behave as before. The javadoc ofregisterConsumerConfigurationstates that each consumer registers its own configuration for its own pattern.camel-as2-component,AS2Consumer.doStart: registers the consumer's own configuration (keys, certificates and MDN settings) for its ownrequestUriPattern, replacing the/fallback when the pattern is/.AS2Configuration: the javadoc of the four options states that they apply to the consumer endpoint they are set on;as2.json, the catalog mirror andAS2EndpointBuilderFactoryare regenerated.as2-component.adoc: a note under Usage that consumers sharing aserverPortNumbereach apply their own keys and asynchronous MDN settings.main.Tests
AS2ServerTwoConsumerAsyncMdnSettingsTest(camel-as2-component): four consumers on one port (a first consumer with basic authentication and an allowed-host list, a second with a bearer token and its own list, one without settings, one on the default/pattern) and local receipt endpoints. The asynchronous MDN for the second consumer carries its own bearer token and follows its own allowed-host list; a consumer without settings sends noAuthorizationheader; the consumer on/uses its own token; the first consumer is unchanged; after the first consumer's route stops, the remaining consumer's settings are used. Without the change, the five tests about the other consumers fail (the MDN carried the first consumer's basic authentication, or followed its allowed-host list).AS2ConsumerConfigurationMdnSettingsTest(camel-as2-api): the per-path configuration carries the settings, the/fallback carries the constructor's values, and registering a path again replaces its configuration.AS2AsyncMdnBasicAuthHeaderTest,AS2AsyncMdnTokenAuthHeaderTest,AS2AsyncMdnContextReuseTest,AS2AsynchronousMDNManagerDeliveryAddressTest, the camel-as2-api and camel-as2-component unit tests, andAS2ServerTwoConsumerSecEncryptedIT.Claude Code on behalf of oscerd
🤖 Generated with Claude Code