CAMEL-24623: camel-infinispan - report a QUERY that has no query builder (backport to camel-4.22.x) - #26116
Conversation
Both producers ran the QUERY operation through an "if (query != null)" with no else branch, and buildQuery returns null when neither the queryBuilder option nor the CamelInfinispanQueryBuilder header is set. A misconfigured route got its own message back with no result, no exception and no log line, which is indistinguishable from a query that matched nothing. The pass-through is kept - it has been asserted by producerQueryOperationWithoutQueryBuilder since the operation was contributed in CAMEL-9624 - but it is now logged at WARN, naming the cache, the queryBuilder option and the header. The builder is also resolved before the cache is looked up, so a misconfigured remote route no longer pays a Hot Rod round trip to do nothing. The embedded producer carried the same defect and is fixed the same way. Backport of fe55a9a (apache#26111). 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:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 12 tested, 29 compile-only — current: 12 all testedMaveniverse Scalpel detected 41 affected modules (current approach: 12).
|
gnodet
left a comment
There was a problem hiding this comment.
Clean cherry-pick of #26111 to camel-4.22.x. Reviewed the full diff:
- The
resolveQueryBuilderextraction is correct — same resolution logic (header > config),InfinispanVectorQueryBuilderhandling preserved in the remote variant. - Behavioral change is strictly additive: the QUERY pass-through when no builder is set is kept (CAMEL-9624 compatibility), but now warns at WARN level and avoids the unnecessary cache/Hot Rod round-trip.
- The second null check (
if (query == null)) afterbuildQuery(builder, cache)is defensive for a builder whosebuild()returns null — acceptable. - Tests are well-designed: exercising
onQuerydirectly with an unstarted manager proves the "no cache access" property by construction. - Missing embedded producer tests, but the embedded path is simpler (no VectorQueryBuilder) and the Util method is tested transitively through the remote tests.
Note: Parent PR #26111 is still open — merge this after it lands, as stated in the description.
Suggestion: Consider setting milestone to 4.22.1.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Backport of #26111 (CAMEL-24623) to
camel-4.22.x. Clean cherry-pick, no conflicts.Both producers ran the
QUERYoperation through anif (query != null)with no else branch, so a routewith neither the
queryBuilderoption nor theCamelInfinispanQueryBuilderheader got its own message backwith no result, no exception and no log line — indistinguishable from a query that matched nothing.
The pass-through is deliberately kept:
producerQueryOperationWithoutQueryBuilderhas asserted it sincethe operation was contributed in CAMEL-9624 (2016). Only the silence goes away — both producers now log a
WARN naming the cache, the option and the header, and the builder is resolved before the cache is looked up
so a misconfigured remote route no longer pays a Hot Rod round trip to do nothing. The embedded producer
carried the same defect and is fixed the same way.
Safe for a patch release: log-only, no behaviour change, no metadata or catalog change.
Merge after #26111.
Verification
mvn clean install -DskipITsoncomponents/camel-infinispanis green on this branch — 89 embedded testsand the remote module's unit tests, including the 4 new ones in
InfinispanRemoteProducerQueryTest.The full reactor was not run here: the host was under memory pressure and killed it twice. It would add
nothing for this change — the module build regenerates nothing (
git statusshows only the fivecherry-picked files), no annotation or metadata is touched, and the only module outside the infinispan
family that depends on
camel-infinispaniscamel-langchain4j-embeddings, as a test dependency with noreference to the classes changed here.
Claude Code on behalf of oscerd
🤖 Generated with Claude Code