CAMEL-24359: camel-atmosphere-websocket - align Exchange header constant names with Camel naming convention - #25366
Open
oscerd wants to merge 2 commits into
Open
CAMEL-24359: camel-atmosphere-websocket - align Exchange header constant names with Camel naming convention#25366oscerd wants to merge 2 commits into
oscerd wants to merge 2 commits into
Conversation
…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>
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: 566 tested, 26 compile-only — current: 563 all testedMaveniverse Scalpel detected 592 affected modules (current approach: 563).
|
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.
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>
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).Claude Code on behalf of oscerd