gnodet-bot commented on code in PR #28361:
URL: https://github.com/apache/camel/pull/28361#discussion_r4360672442
##########
components/camel-file-watch/src/main/java/org/apache/camel/component/file/watch/FileWatchConsumer.java:
##########
@@ -171,12 +171,24 @@ private Exchange prepareExchange(FileEvent event) {
message.setHeader(FileWatchConstants.FILE_PARENT,
PathUtils.normalizeToString(event.getEventPath().getParent().toAbsolutePath()));
- message.setHeader(FileWatchConstants.FILE_LAST_MODIFIED,
event.getEventDate());
+ Long lastModified = lastModified(event.getEventPath());
+ if (lastModified != null) {
+ message.setHeader(FileWatchConstants.FILE_LAST_MODIFIED,
lastModified);
+ }
message.setHeader(Exchange.MESSAGE_TIMESTAMP, event.getEventDate());
return exchange;
}
+ private static Long lastModified(Path path) {
+ try {
+ return Files.getLastModifiedTime(path).toMillis();
+ } catch (IOException e) {
+ // the file no longer exists, such as for a DELETE event
Review Comment:
⚠️ **Missing `SecurityException` catch** — `Files.getLastModifiedTime()` can
throw an unchecked `SecurityException` in addition to `IOException`. If a
security manager denies access, this exception won't be caught here and will
propagate up through the consumer thread. Consider:
```suggestion
try {
return Files.getLastModifiedTime(path).toMillis();
} catch (IOException | SecurityException e) {
// the file no longer exists (IOException) or access is denied
(SecurityException)
return null;
}
```
##########
components/camel-file-watch/src/test/java/org/apache/camel/component/file/watch/FileWatchComponentTest.java:
##########
@@ -136,6 +138,24 @@ public void testCreateBatch() throws Exception {
MockEndpoint.assertIsSatisfied(context);
}
+ @Test
+ public void testLastModifiedHeaderIsTimeOfFile() throws Exception {
+ MockEndpoint watchCreate = getMockEndpoint("mock:watchCreate");
+ watchCreate.expectedMessageCount(1);
+
+ // create the file outside the watched directory with an old last
modified time, and move it in,
+ // so the last modified time of the file differs from the time of the
event
+ Path staging = Files.createDirectories(folder.resolve("staging"));
+ Path file = Files.writeString(staging.resolve("old.txt"), "Hello");
+ long lastModified =
Instant.parse("2020-01-01T00:00:00Z").toEpochMilli();
+ Files.setLastModifiedTime(file, FileTime.fromMillis(lastModified));
+ Files.move(file, Paths.get(testPath(), "old.txt"));
+
+ watchCreate.assertIsSatisfied();
+ assertEquals(lastModified,
+
watchCreate.getExchanges().get(0).getIn().getHeader(FileWatchConstants.FILE_LAST_MODIFIED,
Long.class));
Review Comment:
💡 **Consider adding a DELETE test** — this test covers only the CREATE path.
The PR changes behavior for DELETE events (`FILE_LAST_MODIFIED` is now absent
instead of set to `event.getEventDate()`). Existing consumers may depend on the
header being present for DELETE events. A test asserting the header is absent
on DELETE would guard against future regressions.
--
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]
