davsclaus commented on code in PR #26742:
URL: https://github.com/apache/camel/pull/26742#discussion_r4071439054
##########
core/camel-support/src/main/java/org/apache/camel/support/RouteWatcherReloadStrategy.java:
##########
@@ -322,6 +327,7 @@ protected void onRouteReload(Collection<Resource>
resources, boolean removeEvery
// update okay, so clear as we do not need to remember those
anymore
previousSources.clear();
+ rememberContent(sources);
Review Comment:
Looked at it closely: `removeEverything` is live,
`RouteOnDemandReloadStrategy.isRemoveEverything` passes true when the directory
has no route files left, so the path matters. But clearing the map there is the
behaviour we want, not a lost safety net: nothing runs any more and the files
are gone, so there is nothing to go back to, and keeping the content would let
a later failed reload put a deleted file back. Remembering the pre-clear
sources, as the suggestion does, is what would introduce that.
So the map is cleared in that path either way; what the suggestion correctly
points at is that it happened by accident, `retainAll` against an empty set,
rather than by decision. It is now explicit, with the reason in a comment, and
`RouteReloadRollbackTest."everything removed forgets the remembered content, a
deleted file is not put back"` covers the end to end: remove everything, then
save the file broken, and no route comes back. That test fails if the sources
are snapshotted before the clear.
--
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]
