This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-25834 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 4df402aa6c71abdf99be61691e2c7de7acaf302a Author: Claus Ibsen <[email protected]> AuthorDate: Mon Oct 19 19:52:54 2026 +0200 CAMEL-25834: camel-core - dev mode: a properties file that fails in a batch reload is reported, as when it changed alone The files of one scan of the folder are reloaded together. When a properties file of such a batch failed, the failure was only logged at DEBUG: the batch was reloaded again one file at a time, and by then the properties component had the new values of the file, so the file had no changes left and nothing failed. Each properties or Groovy file of a batch is now reloaded on its own, and a failure is reported as when the file changed alone: a WARN, the failed counter, the last error and the reload failure event. The other files still reload. Reloading one at a time stays for the route files, where it finds the file that is wrong. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m --- .../RouteWatcherReloadBatchFailureTest.java | 193 +++++++++++++++++++++ .../support/FileWatcherResourceReloadStrategy.java | 26 ++- .../camel/support/RouteWatcherReloadStrategy.java | 46 +++-- 3 files changed, 238 insertions(+), 27 deletions(-) diff --git a/core/camel-core/src/test/java/org/apache/camel/support/RouteWatcherReloadBatchFailureTest.java b/core/camel-core/src/test/java/org/apache/camel/support/RouteWatcherReloadBatchFailureTest.java new file mode 100644 index 000000000000..1e6c1d9f1c80 --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/support/RouteWatcherReloadBatchFailureTest.java @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.support; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Properties; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.apache.camel.CamelContext; +import org.apache.camel.ContextTestSupport; +import org.apache.camel.impl.engine.DefaultRoutesLoader; +import org.apache.camel.spi.CamelEvent; +import org.apache.camel.spi.PropertiesReload; +import org.apache.camel.spi.Resource; +import org.apache.camel.spi.RoutesLoader; +import org.apache.camel.support.service.ServiceSupport; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.Appender; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Property; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * CAMEL-25834: the files of one scan of the folder are reloaded together. When a properties file of such a batch failed + * to reload, the failure was only logged at DEBUG: the batch was reloaded again one file at a time, and by then the + * properties component had the new values of the file, so the file had no changes left and nothing failed. + */ +public class RouteWatcherReloadBatchFailureTest extends ContextTestSupport { + + private static final String WRONG = "the configuration in the file is wrong"; + + @TempDir + Path dir; + + private final List<String> applied = new CopyOnWriteArrayList<>(); + private final List<CamelEvent.CamelContextReloadFailureEvent> failures = new CopyOnWriteArrayList<>(); + private final List<String> warnings = new CopyOnWriteArrayList<>(); + private Path wrong; + private Path other; + private Appender appender; + private RecordingRoutesLoader loader; + private RouteWatcherReloadStrategy strategy; + + @Override + protected CamelContext createCamelContext() throws Exception { + CamelContext context = super.createCamelContext(); + wrong = Files.writeString(dir.resolve("app.properties"), "greeting = hello\n"); + other = Files.writeString(dir.resolve("other.properties"), "name = world\n"); + context.getPropertiesComponent().setLocation("file:" + wrong + ",file:" + other); + context.addService(new FailingPropertiesReload("file:" + wrong, applied)); + context.getManagementStrategy().addEventNotifier(new EventNotifierSupport() { + @Override + public void notify(CamelEvent event) { + if (event instanceof CamelEvent.CamelContextReloadFailureEvent failure) { + failures.add(failure); + } + } + }); + return context; + } + + @Override + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + loader = new RecordingRoutesLoader(); + loader.setCamelContext(context); + context.getCamelContextExtension().addContextPlugin(RoutesLoader.class, loader); + // started without a folder: it has the listener that reloads one file, and watches nothing + strategy = new RouteWatcherReloadStrategy(); + strategy.setCamelContext(context); + strategy.start(); + + appender = new AbstractAppender("reload-warnings", null, null, true, Property.EMPTY_ARRAY) { + @Override + public void append(LogEvent event) { + if (event.getLevel().isMoreSpecificThan(Level.WARN)) { + warnings.add(event.getMessage().getFormattedMessage()); + } + } + }; + appender.start(); + LoggerContext logging = (LoggerContext) LogManager.getContext(false); + logging.getConfiguration().getRootLogger().addAppender(appender, Level.ALL, null); + logging.updateLoggers(); + } + + @AfterEach + public void stopCapture() { + strategy.stop(); + LoggerContext logging = (LoggerContext) LogManager.getContext(false); + logging.getConfiguration().getRootLogger().removeAppender(appender.getName()); + logging.updateLoggers(); + appender.stop(); + } + + @Test + public void aPropertiesFileThatFailsIsReportedAndTheRouteStillReloads() throws Exception { + Files.writeString(wrong, "greeting = hello\ncamel.beans.faker = #class:com.github.javafaker.Faker\n"); + File route = Files.writeString(dir.resolve("route.yaml"), "- route: {}\n").toFile(); + + strategy.onReloadBatch(List.of(wrong.toFile(), route)); + + assertReported(); + // the route of the batch reloads once, and counts as reloaded + assertEquals(List.of("file:" + route.getPath()), loader.loaded); + assertEquals(1, strategy.getReloadCounter()); + } + + @Test + public void aPropertiesFileThatFailsDoesNotStopTheOtherPropertiesFile() throws Exception { + Files.writeString(wrong, "greeting = hello\ncamel.beans.faker = #class:com.github.javafaker.Faker\n"); + Files.writeString(other, "name = world\ncamel.component.log.level = WARN\n"); + + strategy.onReloadBatch(List.of(wrong.toFile(), other.toFile())); + + assertReported(); + assertEquals(List.of("file:" + other + ": camel.component.log.level"), applied); + assertEquals(1, strategy.getReloadCounter()); + } + + // reported as when the file is reloaded on its own: a WARN, the failed counter, the last error and the event + private void assertReported() { + assertTrue(warnings.stream().anyMatch(w -> w.contains(wrong.toString()) && w.contains(WRONG)), + "A WARN names the file and why it failed: " + warnings); + assertEquals(1, strategy.getFailedCounter()); + assertEquals(WRONG, strategy.getLastError().getMessage()); + assertEquals(1, failures.size()); + assertEquals(WRONG, failures.get(0).getCause().getMessage()); + } + + // the listener that applies the configuration of a properties file, as camel-main does: it fails on one file + private static class FailingPropertiesReload extends ServiceSupport implements PropertiesReload { + + private final String wrong; + private final List<String> applied; + + FailingPropertiesReload(String wrong, List<String> applied) { + this.wrong = wrong; + this.applied = applied; + } + + @Override + public void onReload(String name, Properties properties) { + if (wrong.equals(name)) { + throw new IllegalArgumentException(WRONG); + } + applied.add(name + ": " + String.join(", ", properties.stringPropertyNames())); + } + } + + // loads no routes: it records the files + private static class RecordingRoutesLoader extends DefaultRoutesLoader { + private final List<String> loaded = new ArrayList<>(); + + @Override + public Set<String> updateRoutes(Collection<Resource> resources) { + for (Resource resource : resources) { + loaded.add(resource.getLocation()); + } + return Set.of(); + } + } +} diff --git a/core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java b/core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java index 9e92185c506a..5739f843dd57 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/FileWatcherResourceReloadStrategy.java @@ -415,16 +415,24 @@ public class FileWatcherResourceReloadStrategy extends ResourceReloadStrategySup getResourceReload().onReload(name, resource); incSucceededCounter(); } catch (Exception e) { - setLastError(e); - incFailedCounter(); - // the same event a failed context reload emits, so a listener can act on the file - EventHelper.notifyContextReloadFailure(getCamelContext(), name, e); - String msg = e.getMessage(); - if (msg != null && msg.endsWith(".")) { - msg = msg.substring(0, msg.length() - 1); - } - LOG.warn("Error reloading routes from file: {} due to: {}. This exception is ignored.", name, msg, e); + reloadFailed(name, e); + } + } + + /** + * Counts and reports a file that failed to reload: the last error, the failed counter, the event of a failed + * context reload, and a WARN. + */ + void reloadFailed(String name, Exception e) { + setLastError(e); + incFailedCounter(); + // the same event a failed context reload emits, so a listener can act on the file + EventHelper.notifyContextReloadFailure(getCamelContext(), name, e); + String msg = e.getMessage(); + if (msg != null && msg.endsWith(".")) { + msg = msg.substring(0, msg.length() - 1); } + LOG.warn("Error reloading routes from file: {} due to: {}. This exception is ignored.", name, msg, e); } } diff --git a/core/camel-support/src/main/java/org/apache/camel/support/RouteWatcherReloadStrategy.java b/core/camel-support/src/main/java/org/apache/camel/support/RouteWatcherReloadStrategy.java index bb9d767da998..610a21e34a6c 100644 --- a/core/camel-support/src/main/java/org/apache/camel/support/RouteWatcherReloadStrategy.java +++ b/core/camel-support/src/main/java/org/apache/camel/support/RouteWatcherReloadStrategy.java @@ -446,8 +446,12 @@ public class RouteWatcherReloadStrategy extends FileWatcherResourceReloadStrateg * saved together with a property it uses is therefore built with that property already in place, instead of failing * on it and being restored (CAMEL-25032). * <p/> - * If the batch fails, the files are reloaded one at a time: the reload then fails on the one file that is wrong and - * says which it was, so a mistake in one file does not hide behind the others (CAMEL-24860). + * A properties or Groovy file that fails is reported at once, as when it changed alone, and the other files still + * reload. A properties file is not reloaded again: the properties component has its new values already, so the file + * would have no changes left, and its failure would be lost (CAMEL-25834). + * <p/> + * If the routes fail, the route files are reloaded one at a time: the reload then fails on the one file that is + * wrong and says which it was, so a mistake in one file does not hide behind the others (CAMEL-24860). */ @Override protected void onReloadBatch(List<File> changed) { @@ -456,21 +460,27 @@ public class RouteWatcherReloadStrategy extends FileWatcherResourceReloadStrateg super.onReloadBatch(changed); return; } + setLastError(null); + List<File> routeFiles = new ArrayList<>(); List<Resource> routes = new ArrayList<>(); boolean others = false; - try { - setLastError(null); - for (File file : changed) { - String name = FileUtil.compactPath(file.getPath()); - Resource resource = PluginHelper.getResourceLoader(getCamelContext()).resolveResource("file:" + name); - if (name.endsWith(".properties")) { - others |= onPropertiesReload(resource, false); - } else if (name.endsWith(".groovy")) { - others |= onGroovyReload(resource, false); - } else { - routes.add(resource); + for (File file : changed) { + String name = FileUtil.compactPath(file.getPath()); + Resource resource = PluginHelper.getResourceLoader(getCamelContext()).resolveResource("file:" + name); + if (name.endsWith(".properties") || name.endsWith(".groovy")) { + try { + others |= name.endsWith(".properties") + ? onPropertiesReload(resource, false) : onGroovyReload(resource, false); + incSucceededCounter(); + } catch (Exception e) { + reloadFailed(name, e); } + } else { + routeFiles.add(file); + routes.add(resource); } + } + try { if (!routes.isEmpty()) { onRouteReload(routes, false); } else if (others) { @@ -478,14 +488,14 @@ public class RouteWatcherReloadStrategy extends FileWatcherResourceReloadStrateg retryFailedOrReloadAll(); } // the counter is files reloaded, as it is when they are reloaded one at a time - for (int i = 0; i < changed.size(); i++) { + for (int i = 0; i < routes.size(); i++) { incSucceededCounter(); } } catch (Exception e) { - LOG.debug("Reloading {} changed file(s) together failed, reloading them one at a time to find the file" - + " that is wrong: {}", - changed.size(), e.getMessage(), e); - super.onReloadBatch(changed); + LOG.debug("Reloading {} changed route file(s) together failed, reloading them one at a time to find the" + + " file that is wrong: {}", + routes.size(), e.getMessage(), e); + super.onReloadBatch(routeFiles); } }
