gnodet commented on code in PR #26111:
URL: https://github.com/apache/camel/pull/26111#discussion_r3932492822
##########
components/camel-infinispan/camel-infinispan/src/main/java/org/apache/camel/component/infinispan/remote/InfinispanRemoteProducer.java:
##########
@@ -49,11 +56,26 @@ public void onStats(Message message) {
@SuppressWarnings("unchecked")
@InvokeOnHeader("QUERY")
public void onQuery(Message message) {
- final RemoteCache<Object, Object> cache =
getManager().getCache(message, getCacheName(), RemoteCache.class);
- final Query<?> query =
InfinispanRemoteUtil.buildQuery(getConfiguration(), cache, message);
+ // resolved before the cache is opened, so a misconfigured route does
not pay a remote call
+ final InfinispanQueryBuilder builder =
InfinispanRemoteUtil.resolveQueryBuilder(getConfiguration(), message);
+ if (builder == null) {
+ warnNoQueryBuilder();
+ return;
+ }
Review 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:
1. Remove the second null check entirely (trust that `build()` returns
non-null) and call `setResult` unconditionally after `buildQuery`.
2. Keep the defensive check but use a different message, e.g. "Query builder
returned no query for cache {}".
Same applies to the embedded producer.
##########
components/camel-infinispan/camel-infinispan/src/main/java/org/apache/camel/component/infinispan/remote/InfinispanRemoteProducer.java:
##########
@@ -49,11 +56,26 @@ public void onStats(Message message) {
@SuppressWarnings("unchecked")
@InvokeOnHeader("QUERY")
public void onQuery(Message message) {
- final RemoteCache<Object, Object> cache =
getManager().getCache(message, getCacheName(), RemoteCache.class);
- final Query<?> query =
InfinispanRemoteUtil.buildQuery(getConfiguration(), cache, message);
+ // resolved before the cache is opened, so a misconfigured route does
not pay a remote call
+ final InfinispanQueryBuilder builder =
InfinispanRemoteUtil.resolveQueryBuilder(getConfiguration(), message);
+ if (builder == null) {
+ warnNoQueryBuilder();
+ return;
+ }
- if (query != null) {
- setResult(message, query.execute().list());
+ final RemoteCache<Object, Object> cache =
getManager().getCache(message, getCacheName(), RemoteCache.class);
+ final Query<?> query = InfinispanRemoteUtil.buildQuery(builder, cache);
+ if (query == null) {
+ warnNoQueryBuilder();
+ return;
}
+
+ setResult(message, query.execute().list());
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
