gnodet-bot commented on code in PR #29805:
URL: https://github.com/apache/camel/pull/29805#discussion_r4568973583
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/PluginHelper.java:
##########
@@ -813,33 +852,81 @@ public static JsonObject createPluginConfig(Path
configPath) {
if (parent != null) {
Files.createDirectories(parent);
}
- Files.writeString(f, config.toJson(),
- StandardOpenOption.CREATE,
- StandardOpenOption.WRITE,
- StandardOpenOption.TRUNCATE_EXISTING);
+ atomicWriteJson(f, config.toJson());
} catch (IOException e) {
throw new RuntimeCamelException("Failed to create plugin
configuration", e);
}
return config;
}
+ /**
+ * Writes {@code content} to {@code target} atomically: writes to a
sibling temp file first, then moves it into
+ * place with {@link StandardCopyOption#REPLACE_EXISTING}. When atomic
move is not supported by the underlying
+ * filesystem, falls back to a plain overwriting move (rename(2)), which
is still safe — a reader never observes a
+ * half-written file.
+ */
+ private static void atomicWriteJson(Path target, String content) throws
IOException {
+ // Resolve symlinks so the move replaces the target file, not the
symlink itself.
+ Path realTarget;
+ try {
+ realTarget = target.toRealPath();
+ } catch (IOException e) {
+ // file does not exist yet — no symlink to resolve
+ realTarget = target;
+ }
+ Path dir = realTarget.getParent();
+ if (dir != null) {
+ Files.createDirectories(dir);
+ }
+ Path tmp = dir != null
+ ? Files.createTempFile(dir, "." + realTarget.getFileName(),
".tmp")
+ : Files.createTempFile("." + realTarget.getFileName(), ".tmp");
+ try {
+ Files.writeString(tmp, content, StandardOpenOption.WRITE,
StandardOpenOption.TRUNCATE_EXISTING);
+ try {
+ Files.move(tmp, realTarget,
StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
+ tmp = null; // moved successfully, nothing to delete
+ } catch (AtomicMoveNotSupportedException e) {
+ // fall back: non-atomic replace — still better than
truncate-then-write
+ Files.move(tmp, realTarget,
StandardCopyOption.REPLACE_EXISTING);
+ tmp = null;
+ }
+ } finally {
+ if (tmp != null) {
+ try {
+ Files.deleteIfExists(tmp);
+ } catch (IOException ignored) {
+ // best effort
+ }
+ }
+ }
+ }
+
public static void savePluginConfig(JsonObject plugins) {
savePluginConfig(plugins, null);
}
public static void savePluginConfig(JsonObject plugins, Path configPath) {
Path f = getCustomPluginConfigPath(configPath);
try {
- Files.writeString(f, plugins.toJson(),
- StandardOpenOption.CREATE,
- StandardOpenOption.WRITE,
- StandardOpenOption.TRUNCATE_EXISTING);
+ atomicWriteJson(f, plugins.toJson());
} catch (IOException e) {
throw new RuntimeCamelException("Failed to save plugin
configuration", e);
}
}
+ /**
+ * Warns about a corrupt or unreadable plugin configuration file, once per
path per JVM run, so multiple callers
+ * (e.g. {@code getActivePlugins} and {@code printAvailablePlugins}) do
not repeat the message.
+ */
+ public static void warnCorruptPluginConfig(Printer printer, Exception e) {
+ String key = e.getMessage() != null ? e.getMessage() :
"unknown-corrupt-plugin-config";
Review Comment:
💡 **Minor:** `WARNED_PLUGIN_PATHS` uses `e.getMessage()` as the dedup key,
while the sibling `CommandLineHelper.WARNED_PATHS` uses the file's absolute
path. The message works here because `getPluginConfig()` embeds the file path
in the `RuntimeCamelException` — but it's fragile (varying parse-error details
for the same file could produce different keys). Consider passing the path
explicitly and using it as the key for consistency:
```suggestion
public static void warnCorruptPluginConfig(Printer printer, String
configPath, Exception e) {
if (WARNED_PLUGIN_PATHS.add(configPath)) {
printer.printErr("Warning: " + e.getMessage() + ". Fix the JSON
by hand or delete the file.");
}
}
```
(callers would pass
`getCustomPluginConfigPath(configPath).toAbsolutePath().toString()`)
--
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]
