allthingssecurity commented on PR #28321: URL: https://github.com/apache/camel/pull/28321#issuecomment-6361765168
@davsclaus thanks, addressed in 588c9abff3a2: 1. `alreadyStopped` check before the scheduled suspend (inline suggestion), with `scheduledSuspendDoesNotRecordAConsumerThatWasAlreadyStopped`, which fails without it. 2. The record is now also cleared in `onResume`. When testing it I found that the stale entry cannot come from the route controller today: a route whose consumer is not `Suspendable` does not support suspension (`DefaultRoute.supportsSuspension()` is `consumer instanceof Suspendable && consumer instanceof SuspendableService`, the same test `ServiceHelper.suspendService` uses). So `suspendRoute` calls `stopRoute` and `resumeRoute` calls `startRoute`, and `onStop`/`onStart` already clear the entry. `onResume` is only called for a route whose consumer is `Suspendable`, which the scheduled suspend suspends and never records. The new test `routeSuspendedAndResumedAfterTheScheduledSuspendKeepsNoRecordOfIt` (scheduled SUSPEND, then route controller suspend and resume, then another stop of the consumer and a scheduled RESUME: the consumer stays stopped) is therefore a control: it passes with and without the `onResume` line. I kept the line because it costs nothing and covers a future chan ge to how routes are suspended. The code comment says this. On backports: noted that `RoutePolicySupport.isResumeOrStartConsumerAllowed` (CAMEL-25018) is not on camel-4.22.x / camel-4.18.x, so a backport would need it too. Tests (offline, `-DskipITs`): `ScheduledRoutePolicyNonSuspendableConsumerTest` 6 run, 0 failures; camel-quartz unit tests 114 run, 0 failures. _Claude Code on behalf of allthingssecurity_ -- 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]
