CAMEL-24359: camel-atmosphere-websocket - align Exchange header constant names with Camel naming convention - #25366
Merged
Merged
Conversation
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 567 tested, 26 compile-only — current: 564 all testedMaveniverse Scalpel detected 593 affected modules (current approach: 564).
|
davsclaus
approved these changes
Aug 6, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
LGTM — clean header-alignment change, thorough upgrade-guide entry, and CI is green.
One minor convention note (non-blocking):
- The new
WebsocketConstantsTestuses JUnit assertions (assertEquals,assertTrue). Per project conventions, new test code should prefer AssertJ (assertThat(...)) for consistency. This can be addressed in a follow-up if desired.
Everything else checks out:
- Header values follow
design/headers.adoc("Camel" + ComponentName + Feature, PascalCase) SEND_TO_ALLnot being read by the component is confirmed — rename is for internal consistency only- The
HeaderFilterStrategybehaviour change is well-documented in the upgrade guide - Generated files are properly regenerated
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of davsclaus
| import org.apache.camel.spi.HeaderFilterStrategy; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; |
Contributor
There was a problem hiding this comment.
Non-blocking: per project conventions, new test code should prefer AssertJ assertions (assertThat(...)) over JUnit assertions (assertEquals, assertTrue). For example:
Suggested change
| import static org.junit.jupiter.api.Assertions.assertEquals; | |
| import static org.assertj.core.api.Assertions.assertThat; |
And then assertThat(WebsocketConstants.CONNECTION_KEY).isEqualTo("CamelAtmosphereWebsocketConnectionKey") etc.
oscerd
added a commit
to oscerd/camel
that referenced
this pull request
Aug 6, 2026
Addresses review feedback on apache#25366. The project convention is to prefer AssertJ over JUnit assertions in new test code. assertj-core was not on the camel-atmosphere-websocket test classpath, so it is declared here as a test dependency; the version comes from the parent dependencyManagement, matching how other modules declare it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Contributor
Author
|
Thanks for the review. Addressed in d12c0e7 — the new One thing worth flagging, since the suggestion assumed AssertJ was already usable here: So the change also declares it as a test dependency. The version comes from the parent <dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>Flagging it explicitly because Module build is green: 14 tests pass, and the full reactor build from root is clean with no regenerated-artifact drift. Claude Code on behalf of oscerd |
…ant names with Camel naming convention Rename the Exchange header string values in WebsocketConstants from the dotted websocket.* prefix to the project-wide Camel<Component><Feature> convention documented in design/headers.adoc: CONNECTION_KEY -> CamelAtmosphereWebsocketConnectionKey CONNECTION_KEY_LIST -> CamelAtmosphereWebsocketConnectionKeyList SEND_TO_ALL -> CamelAtmosphereWebsocketSendToAll EVENT_TYPE -> CamelAtmosphereWebsocketEventType ERROR_TYPE -> CamelAtmosphereWebsocketErrorType The Java field names are unchanged, so routes and code that reference the constants symbolically continue to work without changes. Routes that set the headers by their literal string value must be updated. The component was already in scope for CAMEL-23532, but that change only applied the inherited HeaderFilterStrategy to the consumer's WebSocket query parameters and left the constants on the legacy websocket. prefix, unlike its camel-vertx-websocket sibling. Because WebsocketEndpoint extends ServletEndpoint, the inherited HttpHeaderFilterStrategy filters only the Camel / camel prefixes, so the dotted names sat outside the filtered namespace in both directions. Adds WebsocketConstantsTest covering the new values and asserting that all five headers are filtered by the inherited HttpHeaderFilterStrategy in both directions, adds a 4.22 upgrade-guide entry mirroring the CAMEL-23574 and CAMEL-23532 entries, and regenerates the component metadata, catalog, important headers list and endpoint DSL factory. Note that WebsocketConstants.SEND_TO_ALL is not read by this component (broadcast is selected through the sendToAll endpoint option); it is renamed only to keep the class internally consistent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
Addresses review feedback on apache#25366. The project convention is to prefer AssertJ over JUnit assertions in new test code. assertj-core was not on the camel-atmosphere-websocket test classpath, so it is declared here as a test dependency; the version comes from the parent dependencyManagement, matching how other modules declare it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
davsclaus
force-pushed
the
fix/CAMEL-24359
branch
from
August 6, 2026 10:57
d12c0e7 to
ebaf733
Compare
This was referenced Aug 6, 2026
davsclaus
added a commit
that referenced
this pull request
Aug 6, 2026
…gn Exchange header constant names with Camel naming convention (#25382) CAMEL-24359: camel-atmosphere-websocket - align Exchange header constant names with Camel naming convention Rename the Exchange header string values in WebsocketConstants from the dotted websocket.* prefix to the project-wide Camel<Component><Feature> convention documented in design/headers.adoc. The Java field names are unchanged, so code referencing the constants symbolically keeps working. Adds WebsocketConstantsTest covering the new values and asserting all five headers are filtered by the inherited HttpHeaderFilterStrategy. Includes a 4.22 upgrade-guide entry and regenerated catalog/DSL metadata. Closes #25366 Co-authored-by: Andrea Cosentino <ancosen@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
davsclaus
added a commit
that referenced
this pull request
Aug 6, 2026
…gn Exchange header constant names with Camel naming convention (#25383) CAMEL-24359: camel-atmosphere-websocket - align Exchange header constant names with Camel naming convention Rename the Exchange header string values in WebsocketConstants from the dotted websocket.* prefix to the project-wide Camel<Component><Feature> convention documented in design/headers.adoc. The Java field names are unchanged, so code referencing the constants symbolically keeps working. Adds WebsocketConstantsTest covering the new values and asserting all five headers are filtered by the inherited HttpHeaderFilterStrategy. Includes a 4.22 upgrade-guide entry and regenerated catalog/DSL metadata. Closes #25366 Co-authored-by: Andrea Cosentino <ancosen@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <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
2 participants
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.
Description
Renames the Exchange header string values in
WebsocketConstantsfrom the dottedwebsocket.*prefix to the project-wideCamel<Component><Feature>convention documented indesign/headers.adoc:CONNECTION_KEYwebsocket.connectionKeyCamelAtmosphereWebsocketConnectionKeyCONNECTION_KEY_LISTwebsocket.connectionKey.listCamelAtmosphereWebsocketConnectionKeyListSEND_TO_ALLwebsocket.sendToAllCamelAtmosphereWebsocketSendToAllEVENT_TYPEwebsocket.eventTypeCamelAtmosphereWebsocketEventTypeERROR_TYPEwebsocket.errorTypeCamelAtmosphereWebsocketErrorTypeThe Java field names are unchanged, so code referencing the constants symbolically keeps working. Code using the literal strings must be updated.
Background
camel-atmosphere-websocketwas in scope for CAMEL-23532, but that change only applied the inheritedHeaderFilterStrategyto the consumer's WebSocket query parameters. The constants themselves stayed on the legacywebsocket.prefix, unlike thecamel-vertx-websocketsibling which was renamed in the same ticket. SinceWebsocketEndpoint extends ServletEndpoint, the inheritedHttpHeaderFilterStrategyfilters only theCamel/camelprefixes, so the dotted names sat outside the filtered namespace in both directions.This continues the CAMEL-23577 alignment sweep (CAMEL-23574, CAMEL-23584, CAMEL-23588, CAMEL-23716).
Note on the spelling
The values use the concatenated form mandated by
design/headers.adoc("Camel" + ComponentName + Feature, PascalCase, no separators), matching what the rest of the sweep shipped:CamelDnsServer,CamelJiraIssueKey,CamelIrcSendTo. The dottedCamelVertxWebsocket.*form used by the vertx sibling is one of only 37 dotted header values repo-wide against 2313 concatenated, and is not copied here.SEND_TO_ALLis not read by this component at all: broadcast is selected through thesendToAllendpoint option, not the header. It is renamed only to keep the class internally consistent.Changes
WebsocketConstants- the five header valuesWebsocketConstantsTest- locks the values and asserts all five are filtered by the inheritedHttpHeaderFilterStrategyin both directionsimportant-headers.json/ImportantHeaderUtils, and the endpoint DSL factoryTesting
camel-atmosphere-websocketmodule build green: 14 tests pass (12 existing + 2 new)mvn clean install -DskipTests) green, all regenerated artifacts committedBackport
The constants and the producer/consumer reads are identical on
main,camel-4.18.xandcamel-4.14.x, so this should be backported to both LTS lines with the matching 4.18 / 4.14 upgrade-guide entries (and those entries doc-synced back tomainper the backport policy).Review feedback (d12c0e7)
WebsocketConstantsTestnow uses AssertJ (assertThat(...)) instead of JUnit assertions, per project convention.assertj-corewas not on thecamel-atmosphere-websockettest classpath, so it is also declared as a test dependency (version-less, supplied by the parentdependencyManagement). Flagged in the PR thread since the project standards ask that new dependencies be justified.Claude Code on behalf of oscerd