CAMEL-24623: camel-infinispan - report a QUERY that has no query builder - #26111
CAMEL-24623: camel-infinispan - report a QUERY that has no query builder#26111oscerd wants to merge 1 commit into
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. 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
added
port/camel-4.22.x
🔄 Backport BotThis bugfix targets
Labels Port PRs will be created automatically when this PR is merged. Comment ℹ️ If you push additional commits after |
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 12 tested, 27 compile-only — current: 12 all testedMaveniverse Scalpel detected 39 affected modules (current approach: 12).
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Review summary: Solid, well-motivated fix. The refactoring correctly extracts builder resolution ahead of the cache lookup, preserving the decade-old pass-through contract while adding a useful WARN log. The InfinispanVectorQueryBuilder special handling is properly preserved. Tests cover the key resolution paths.
Two minor observations below.
Metadata: No milestone set — consider assigning to the appropriate release milestone.
This review was generated by an AI agent, Hermès, on behalf of @gnodet.
| warnNoQueryBuilder(); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
[low] After the builder != null check on line 58, buildQuery(builder, cache) delegates to queryBuilder != null ? queryBuilder.build(cache) : null — the null branch is dead code since we already filtered out null builders. If build() can genuinely return null, the warning message here is misleading: it says "Set a query builder" when the builder IS set but its build() produced no query.
Two options:
- Remove the second null check entirely (trust that
build()returns non-null) and callsetResultunconditionally afterbuildQuery. - Keep the defensive check but use a different message, e.g. "Query builder returned no query for cache {}".
Same applies to the embedded producer.
| return; | ||
| } | ||
|
|
||
| setResult(message, query.execute().list()); |
There was a problem hiding this comment.
[low] warnNoQueryBuilder() is duplicated verbatim in InfinispanEmbeddedProducer. Since both producers extend InfinispanProducer, this method could live in the parent class to avoid the duplication.
2 participants
Found by a source audit of
components/camel-infinispan.The problem
Both producers run the
QUERYoperation through an unguardedif:and
buildQueryreturnsnullwhen neither thequeryBuilderendpoint option nor theCamelInfinispanQueryBuilderheader is set. A route runningoperation=QUERYwithout a builder thereforegets its own message back — no result, no exception, no log line — which is indistinguishable from a query
that legitimately matched nothing.
The change
The pass-through is kept.
producerQueryOperationWithoutQueryBuilderhas asserted "no exception, noresult" since the operation was contributed in CAMEL-9624 (2016), and the same test exists on the embedded
side, so failing the exchange would revert a deliberate, decade-old contract and would need an
upgrade-guide entry — disproportionate for what is a route misconfiguration.
What changes is only the silence:
queryBuilderoption and theCamelInfinispanQueryBuilderheader;
Hot Rod round trip to do nothing;
InfinispanEmbeddedProducer.onQuerycarried the identical defect — the audit only cited the remote one —and is fixed the same way.
If you would rather have it throw, that is a one-line change plus those two tests and an upgrade note; say
so and I will follow up.
Tests
InfinispanRemoteProducerQueryTest(new, 4 tests): the pass-through still happens and reaches no cache (themanager it runs against was never started, so any remote call would fail), and the header-vs-option
precedence of the builder, which had no coverage at all.
mvn teston both modules is green — 89 embedded tests, including the 2016producerQueryOperationWithoutQueryBuilderagainst a real embedded cache, and 13 remote ones. Full reactor
mvn clean install -DskipTests -Dquicklygreen.Claude Code on behalf of oscerd
🤖 Generated with Claude Code