gnodet-bot commented on code in PR #28202:
URL: https://github.com/apache/camel/pull/28202#discussion_r4333893772
##########
components/camel-keycloak/src/main/java/org/apache/camel/component/keycloak/KeycloakConsumer.java:
##########
@@ -164,24 +166,63 @@ private Queue<Exchange> pollEvents() throws Exception {
if (eventTime > highestEventTime) {
highestEventTime = eventTime;
}
-
- // Prevent unbounded memory growth
- if (processedEventFingerprints.size() >
MAX_FINGERPRINT_CACHE_SIZE) {
- processedEventFingerprints.clear();
- LOG.debug("Cleared fingerprint cache due to size limit");
- }
}
// Update lastEventTime if we processed events with newer timestamps
if (highestEventTime > (lastEventTime != null ? lastEventTime : 0)) {
forgetOlderFingerprints(highestEventTime);
lastEventTime = highestEventTime;
LOG.debug("Updated last event time to: {}", lastEventTime);
+ } else if (lastEventTime == null && events.isEmpty()) {
+ // no event yet (a new realm, or a filter that has not matched
yet): every event from now on is new, so
+ // the next polls read all of them page by page, and not only the
latest maxResults
+ lastEventTime = 0L;
}
+ limitFingerprintCache();
return queue;
}
+ /**
+ * Reads the events of a poll. Keycloak returns the most recent events
first, and the newest event of a poll becomes
+ * the checkpoint, so the events that do not fit into the first {@code
maxResults} (the older new ones, for example
+ * after Keycloak was unreachable for a while) would never be returned
again. Once a checkpoint is set, the
+ * following pages are read as well, until a page reaches an event older
than the checkpoint or is the last one.
+ * When new events arrive while the pages are read, the next page repeats
events of the previous one; the
+ * fingerprints skip them.
+ */
+ private <T> List<T> fetchNewEvents(BiFunction<Integer, Integer, List<T>>
fetch, ToLongFunction<T> eventTime) {
+ KeycloakConfiguration config = getEndpoint().getConfiguration();
+ int first = config.getFirst();
+ int max = config.getMaxResults();
+ List<T> page = fetch.apply(first, max);
+ if (lastEventTime == null || max <= 0 || page == null || page.size() <
max) {
+ return page;
Review Comment:
⚠️ **Null propagation risk**: when the Keycloak client returns `null` for
the first page, this returns `null` to callers that dereference it
unconditionally in a for-each loop (`for (EventRepresentation event : events)`)
and with `.isEmpty()`. The inner-loop guard breaks safely on `null`, but the
initial-page case doesn't. Suggest returning an empty list instead:
```suggestion
if (page == null || page.size() < max) {
return page != null ? page : java.util.Collections.emptyList();
}
```
This is the same null risk that existed before this PR (the base code passed
the result of `getEvents()` directly to a for-each), but `fetchNewEvents` is
the right place to consolidate the guard.
--
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]
