CAMEL-24624: camel-infinispan - catalog, lifecycle and efficiency cleanups - #26113
CAMEL-24624: camel-infinispan - catalog, lifecycle and efficiency cleanups#26113oscerd wants to merge 1 commit into
Conversation
…anups Five small items found while auditing the component. The CamelInfinispanOperationResult header is deprecated. It has had no reader or writer since the remote and embedded components were split, yet it was published in both catalogs and rendered in the docs with the description of CamelInfinispanOperationResultHeader, which is the option that actually names the result header. CamelInfinispanIgnoreReturnValues is documented as a producer header. It is read on the producer path, in InfinispanEmbeddedManager#getCache(Message, String), and its description is producer semantics, so it was in the wrong group. Both consumers now stop the handler that owns the cache listener before calling super.doStop(), the reverse of doStart(). An event arriving between the two calls used to be processed against a consumer that was already stopped. The idempotent repository uses putIfAbsent instead of containsKey followed by put. The outcome is the same, it is atomic, it leaves the lifespan of an existing entry untouched, and it halves the round trips of every check. The remote cache forces return values since CAMEL-9840, so the previous value is always reported. Two typos: "Cannot remote the listener" and "InifinispanConfiguration". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpshGTyfSmdq7hC8tuEwrZ
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Solid cleanup — five distinct improvements, all correct.
-
CamelInfinispanOperationResultdeprecation — confirmed dead since the remote/embedded split. Keeping the constant with@Deprecatedpreserves binary compatibility, and thedeprecationNoteguides users toCamelInfinispanOperationResultHeader. -
CamelInfinispanIgnoreReturnValueslabel correction — confirmed it's read on the producer path (InfinispanEmbeddedManager#getCache(Message, String)). The consumer label was always wrong. -
Consumer stop-order fix — correctly mirrors
doStart()(handler stopped beforesuper.doStop()), closing the window where events could be dispatched to a stopped consumer. Applied consistently to bothInfinispanRemoteConsumerandInfinispanEmbeddedConsumer. -
putIfAbsentoptimization — semantically equivalent to the originalcontainsKey+putpair, atomic, halves round trips. The== nulltest is sound: the remote repository usesFlag.FORCE_RETURN_VALUE(set inInfinispanRemoteIdempotentRepository.doStart()), and the embeddedCacheimplementsConcurrentMapwhich inherently returns the previous value. Also closes a subtle TOCTOU race in the original code. -
Typo fixes —
remote→remove,InifinispanConfiguration→InfinispanConfiguration.
Upgrade guide is accurate and appropriately scoped (covers API-visible changes only).
This review was generated by an AI agent, Hermès on behalf of @gnodet.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 16 tested, 24 compile-only — current: 13 all testedMaveniverse Scalpel detected 40 affected modules (current approach: 13).
|
2 participants
Five small items found while auditing
components/camel-infinispan, grouped because each is a couple oflines.
1.
CamelInfinispanOperationResultis a phantom header — deprecatedInfinispanConstants#RESULThas zero readers or writers in main or test sources, yet it is published inboth
infinispan.jsonandinfinispan-embedded.jsonand rendered throughout the docs — with thedescription of
CamelInfinispanOperationResultHeader, which is the option that actually names the resultheader. So the docs advertised two headers for one job and the more obvious-looking one did nothing.
History explains it: when the QUERY operation was contributed in CAMEL-9624 (2016) the result was read
from this header. It became dead when
setResultmoved toRESULT_HEADER/the message body.The constant is kept and marked
@Deprecatedwith adeprecationNoterather than deleted, so nothing thatimports it stops compiling. Upgrade note added for 4.23.
2.
CamelInfinispanIgnoreReturnValueswas labelled a consumer headerIt is read on the producer path —
InfinispanEmbeddedManager#getCache(Message, String)— and itsdescription ("a write operation's return value") is producer semantics. Only its group in the catalog and
the docs changes.
3. Consumer stop order
InfinispanRemoteConsumerandInfinispanEmbeddedConsumerboth calledsuper.doStop()before stoppingthe handler that owns the cache listener, the reverse of
doStart(). An event arriving between the twocalls was processed against a consumer that was already stopped. Now the listener goes first.
4. Idempotent repository: one round trip instead of two
Same outcome, atomic, and it still leaves the lifespan of an existing entry untouched, which is what the
original comment was protecting. It halves the network round trips of every idempotency check on the Hot Rod
repository. The remote cache is obtained with
Flag.FORCE_RETURN_VALUE(CAMEL-9840), so the previous valueis reported and the
== nulltest is sound there too.5. Typos
"Cannot remote the listener"→remove;"InifinispanConfiguration"→InfinispanConfiguration(thelatter is user-visible, it is in the
CamelInfinispanQueryBuilderheader description).Verification
mvn clean installoncomponents/camel-infinispanis green — 110 tests, including the Hot Rodintegration tests against a testcontainer and both Spring idempotent-repository ITs, which exercise the
putIfAbsentchange end to end. Full reactormvn clean install -DskipTests -Dquicklygreen; catalog andendpoint-DSL descriptors regenerated and committed (the DSL regeneration needs a build without
-Dquickly).Claude Code on behalf of oscerd
🤖 Generated with Claude Code