Repository navigation
Conversation
…ants with the Camel naming convention CAMEL-23580 renamed the camel-openstack Exchange header constants to the CamelOpenstack* convention, but left four of them in SwiftConstants on their openstack4j values, on the understanding that those values are part of the Swift HTTP contract: CONTAINER_METADATA_PREFIX X-Container-Meta- -> CamelOpenstackSwiftContainerMetadata VERSIONS_LOCATION X-Versions-Location -> CamelOpenstackSwiftVersionsLocation CONTAINER_READ X-Container-Read -> CamelOpenstackSwiftContainerRead CONTAINER_WRITE X-Container-Write -> CamelOpenstackSwiftContainerWrite They are not. ContainerProducer passes the values to the openstack4j CreateUpdateContainerOptions, which builds the X-Container-Meta-, X-Versions-Location, X-Container-Read and X-Container-Write names of the Swift request itself, so the Exchange header key never reaches Swift and the request on the wire is unchanged by this rename. This commit therefore partly reverses the decision taken in CAMEL-23580, because the reason recorded for it does not hold. The Java field names are unchanged, so routes that reference the constants symbolically keep working. Routes that set one of these headers by its literal string value must move to the new value; the 4.24 upgrade guide carries the table, the note that the Swift request is unchanged, and the renamed Endpoint DSL accessors. design/headers.adoc listed Swift under the protocol-header exemption and pointed at these four constants as the worked example. It now states that the exemption applies only when the header name the route sets is the name that goes on the wire, which is not the case when a client library builds the wire name itself. ContainerProducerTest had no coverage of the container options path at all; six tests are added for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Zineb Bendhiba <bendhiba.zineb@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet-bot
left a comment
There was a problem hiding this comment.
Good cleanup — renaming the constants to the Camel naming convention is the right call. One small item on the stale description:
This review was generated by an AI agent, Hermès on behalf of @gnodet.
| // break interoperability with the Swift API. | ||
| @Metadata(label = "container", description = "Container metadata prefix.", javaType = "Map<String, String>") | ||
| public static final String CONTAINER_METADATA_PREFIX = SwiftHeaders.CONTAINER_METADATA_PREFIX; | ||
| public static final String CONTAINER_METADATA_PREFIX = "CamelOpenstackSwiftContainerMetadata"; |
There was a problem hiding this comment.
CONTAINER_METADATA_PREFIX holds the Camel Exchange header name "CamelOpenstackSwiftContainerMetadata" (a Map<String, String> header key), the @Metadata description "Container metadata prefix." is no longer accurate — the word "prefix" referred to the old wire-level HTTP header prefix (X-Container-Meta-). Consider updating it to "Container metadata." (or a more descriptive wording).
| public static final String CONTAINER_METADATA_PREFIX = "CamelOpenstackSwiftContainerMetadata"; | |
| public static final String CONTAINER_METADATA_PREFIX = "CamelOpenstackSwiftContainerMetadata"; |
davsclaus
left a comment
There was a problem hiding this comment.
Thanks Zineb, this looks good. The premise checks out: ContainerProducer.messageToCreateUpdateOptions only hands the values to openstack4j's CreateUpdateContainerOptions (metadata(...), versionsLocation(...), accessRead(...), accessWrite(...)), and in openstack4j-core 3.12 those methods put the X-Container-Meta-*, X-Versions-Location, X-Container-Read and X-Container-Write names on the request themselves. So the Exchange header key never reaches Swift and the rename does not change the wire request. Calling out explicitly that this partly reverses CAMEL-23580, and why, is appreciated.
Conventions check: the commit, branch and JIRA linkage follow the project guidelines. The 4.24 upgrade-guide entry covers what design/headers.adoc asks for on a header rename: the old -> new table, the renamed Endpoint DSL accessors, that code using the Java constants is unaffected, and that literal header strings must be updated. The generated artefacts are in the PR (component and catalog openstack-swift.json, SwiftEndpointBuilderFactory); the component doc only pulls the headers in through component-endpoint-headers.adoc, so the catalog doc mirror has nothing to change. No other references to the old values remain outside the upgrade guides and the test. The new tests stay in the module's JUnit + Mockito style, and the two that pin the values fail on the old code.
Non-blocking notes:
- gnodet-bot's open thread on
SwiftConstants.CONTAINER_METADATA_PREFIXis still valid: the descriptionContainer metadata prefix.described the oldX-Container-Meta-prefix, while the header now holds aMap<String, String>of metadata. A hint:Container metadata.(or something likeThe container metadata, as a map of name to value.). Changing it means regenerating the two JSON files and the Endpoint DSL again. - For the backports, per the backport upgrade-guide policy:
camel-4x-upgrade-guide-4_22.adoconmainhas nothing about this yet, and the 4.18 note currently points to CAMEL-25895. When the 4.22.x and 4.18.x ports land, the rename table with the exact patch version should go into both guides onmainas well.
CI: the build jobs are still running.
This review covers project rules and conventions only. It does not replace CodeRabbit, Sourcery or SonarCloud.
Claude Code on behalf of Claus Ibsen
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
CAMEL-25895: camel-openstack - align the remaining Swift header constants with the Camel naming convention
Problem
CAMEL-23580 (Camel 4.21) renamed the Exchange header constants of
camel-openstackto theCamelOpenstack*convention used across the component catalog, but left four constants inSwiftConstantson their openstack4j values:SwiftConstants.CONTAINER_METADATA_PREFIXX-Container-Meta-SwiftConstants.VERSIONS_LOCATIONX-Versions-LocationSwiftConstants.CONTAINER_READX-Container-ReadSwiftConstants.CONTAINER_WRITEX-Container-WriteThe reason recorded in the code and in the 4.21, 4.18 and 4.14 upgrade guides was that these names
"are part of the Swift HTTP protocol contract used by openstack4j" and that "renaming them would
break interoperability with the Swift API".
That reason does not hold.
ContainerProducer.messageToCreateUpdateOptionsnever puts the Exchangeheader name on the wire: it calls
CreateUpdateContainerOptions.metadata(Map),.versionsLocation(String),.accessRead(String)and.accessWrite(String), and openstack4jbuilds the
X-Container-Meta-,X-Versions-Location,X-Container-ReadandX-Container-Writenames of the Swift request itself. Verified against
openstack4j-core3.12:CreateUpdateContainerOptions.create().accessRead("r").accessWrite("w").versionsLocation("l").getOptions()returns
{X-Container-Read=r, X-Container-Write=w, X-Versions-Location=l}regardless of which Camelheader key the value came from.
So the four names bought nothing, while making
openstack-swiftthe only component in its familywhose container options are driven by header keys outside the
Camel*namespace thatDefaultHeaderFilterStrategyfilters by default.camel-aws2-s3, for comparison, already usesCamelAwsS3Acl/CamelAwsS3CannedAcl.This PR partly reverses that deliberate decision from CAMEL-23580. The decision itself was
intentional and documented; what is corrected here is its stated premise, which is factually wrong.
Per the project's AI agent guidelines, this is called out explicitly rather than left implicit.
Change
components/camel-openstack/src/main/java/.../swift/SwiftConstants.javaCONTAINER_METADATA_PREFIXX-Container-Meta-CamelOpenstackSwiftContainerMetadataVERSIONS_LOCATIONX-Versions-LocationCamelOpenstackSwiftVersionsLocationCONTAINER_READX-Container-ReadCamelOpenstackSwiftContainerReadCONTAINER_WRITEX-Container-WriteCamelOpenstackSwiftContainerWritecompiling and working. This matches the hard-break-with-stable-field-names policy in
design/headers.adocand what CAMEL-23580 did for the other constants.org.openstack4j.model.storage.object.SwiftHeadersimport are removed.ContainerProducerneeded no change: it already reads the constants, never the literal strings.design/headers.adoclisted Swift under the "protocol and standard headers" exemption andpointed at these constants as the worked example. Corrected: the exemption applies when the
header name the route sets is the name that goes on the wire, which is not the case when a client
library builds the wire name itself.
camel-4x-upgrade-guide-4_24.adocwith the value table, the note that theSwift request is unchanged, the migration for routes using literal header strings, and the
renamed Endpoint DSL accessors (
xContainerRead()->openstackSwiftContainerRead(), etc.).these names are part of the Swift HTTP contract and that renaming them would break
interoperability. Both are corrected to record that openstack4j builds the
X-Container-*wirenames itself, with a pointer to CAMEL-25895 for the releases that carry the rename. Left as a
pointer rather than a version so it stays correct across the maintenance lines.
camel-4x-upgrade-guide-4_14.adocis not touched: that line is EOL and receives no fix.openstack-swift.json, the catalog doc mirror andSwiftEndpointBuilderFactory.Tests
ContainerProducerTesthad no coverage of the container options path at all. Six tests added,in the module's existing style (JUnit 5 + Mockito via
SwiftProducerTestSupport; the class isJUnit-assertion based, so no AssertJ was introduced):
containerCreateReadsAccessControlFromCamelNamespacedHeaderscontainerUpdateReadsAccessControlFromCamelNamespacedHeaderscontainerUpdateReadsVersionsLocationAndMetadataFromCamelNamespacedHeaderscontainerUpdateIgnoresThePreviousSwiftHeaderNamescontainerUpdatePrefersTheOptionsFromTheBodyOverTheHeaderscontainerOptionHeaderNamesFollowTheCamelNamingConventioncontainerUpdateIgnoresThePreviousSwiftHeaderNamesandcontainerOptionHeaderNamesFollowTheCamelNamingConventionfail on the unfixed code(
expected: <null> but was: <CreateUpdateContainerOptions@...>andexpected: <CamelOpenstackSwiftContainerMetadata> but was: <X-Container-Meta->). They are the twothat pin the values; the three
...FromCamelNamespacedHeaderstests go through the Java constants,so they cover the producer path but not the rename itself.
containerUpdatePrefersTheOptionsFromTheBodyOverTheHeaderslocks in the existing precedence, whichthe rename does not change: a
CreateUpdateContainerOptionsin the body makes the producer skip theheaders entirely.
mvn test -pl components/camel-openstack -Dtest=ContainerProducerTest: 13 tests, 0 failures.mvn formatter:format impsort:sort -pl components/camel-openstack: nothing to change.Backports
Targets
camel-4.22.xandcamel-4.18.x.SwiftConstants.javaandContainerProducer.javaareidentical on all three lines, so the Java part is a straight cherry-pick; the generated artefacts
are regenerated per branch rather than cherry-picked. Per the project's backport upgrade-guide
policy, the matching notes go into
camel-4x-upgrade-guide-4_22.adocandcamel-4x-upgrade-guide-4_18.adoconmain.camel-4.14.xandcamel-4.10.xare EOL and getnothing.
Claude Code on behalf of zbendhiba
🤖 Generated with Claude Code