Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New file is not detected when renaming the sub-folder of the watched folder (Java 7 WatchService) #8659

Closed
pziobron opened this issue Jun 27, 2023 · 4 comments

Comments

@pziobron
Copy link
Contributor

pziobron commented Jun 27, 2023

In what version(s) of Spring Integration are you seeing this issue?
6.0.1.RELEASE

Describe the bug
The bug is related to the FileReadingMessageSource when using it for watching the directory with the (Java 7) WatchService feature enabled.
More specifically, the new files are not detected in the sub-directory of the watched folder after renaming it.
This issue has been discovered on Windows when using File Explorer to create a new directory ('New Folder' temporary name is used initially and user edits the name after that). In such a case 2 file events are generated.

To Reproduce

  1. Add the following logger in the logback-spring.xml to see the debug messages of the FileReadingMessageSource class
<logger name="org.springframework.integration.file" level="DEBUG" additivity="false">
   <appender-ref ref="Console" />
  <appender-ref ref="RollingFile" />
</logger>
  1. Use the following FileReadingMessageSource instance to watch the directory [C:\sandbox\test]:
FileReadingMessageSourceImproved messageSource = new FileReadingMessageSourceImproved();
messageSource.setDirectory(new File(inputDirectory));
messageSource.setUseWatchService(true);
messageSource.setWatchEvents(CREATE, MODIFY);
  1. [case A - New Folder] Create new sub-directory (PL) using Windows File Explorer in the watched folder and observe the logs:
Watch event [ENTRY_CREATE] for file [C:\sandbox\test\New folder]
registering: C:\sandbox\test\New folder for file events
Watch event [ENTRY_CREATE] for file [C:\sandbox\test\PL]
registering: C:\sandbox\test\PL for file events
  1. [case A - New Folder] Copy file (6017405.eml) into the new PL directory and observe the logs (The file is ignored !):
Watch event [ENTRY_CREATE] for file [C:\sandbox\test\New folder\6017405.eml]
A file [C:\sandbox\test\New folder\6017405.eml] for the event [ENTRY_CREATE] doesn't exist. Ignored.
  1. [case B - Renaming the folder] Now, create a new empty directory(de) in the watched folder and observe the logs:
Watch event [ENTRY_CREATE] for file [C:\sandbox\test\de]
registering: C:\sandbox\test\de for file events
  1. [case B - Renaming the folder] Rename the folder to ee and observe the logs:
Watch event [ENTRY_CREATE] for file [C:\sandbox\test\ee]
registering: C:\sandbox\test\ee for file events
  1. [case B - Renaming the folder] Copy file (6017408.eml) to the ee folder and observe the logs (The file is ignored):
Watch event [ENTRY_CREATE] for file [C:\sandbox\test\de\6017408.eml]
A file [C:\sandbox\test\de\6017408.eml] for the event [ENTRY_CREATE] doesn't exist. Ignored.

Expected behavior
The file is detected properly in the sub-directory after renaming this sub-directory in the watched folder.

Sample
//TBC

The fix that seems to solve the issue on Windows (not sure about other OS'es) - the only method changed in the FileReadingMessageSource is registerWatch:

private void registerWatch(Path dir) throws IOException {
    if (!this.pathKeys.containsKey(dir)) {
        logger.debug(() -> "registering: " + dir + " for file events");
        WatchKey watchKey = dir.register(this.watcher, this.kinds);
        if (this.pathKeys.containsValue(watchKey)) {
            File previousDir = ((Path) watchKey.watchable()).toAbsolutePath().toFile();
            logger.debug(() -> "unregistering: " + previousDir + " for file events");
            watchKey.cancel();
            watchKey = dir.register(this.watcher, this.kinds);
        }
        this.pathKeys.putIfAbsent(dir, watchKey);
    }
}

Potentially the ExtendedWatchEventModifier.FILE_TREE modifier can be used for that but it seems it is only working for Windows.

@pziobron pziobron added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Jun 27, 2023
@artembilan
Copy link
Member

I think you need to add DELETE watch event type as well.
See some explanation here: https://stackoverflow.com/questions/29368257/implementing-renaming-and-deletion-in-java-watchservice.

So, looks like when we rename the dir, a DELETE event happens and then CREATE & MODIFY, respectively.

Or do you mean the WatchKey is somehow containing a reference to the old dir and therefore doesn't see the rest of entries?
More it even doesn't not register a new watcher for that new dir name.

Is that what you try to explain?

Looking to the code I really don't see that we react properly to the DELETE event and remove and entry from this.pathKeys.
Probably that one would be as a better solution: support even DELETE by default and remove from this.pathKeys?

Thanks

@artembilan artembilan added status: waiting-for-reporter Needs a feedback from the reporter in: file and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jun 27, 2023
@pziobron
Copy link
Contributor Author

pziobron commented Jun 27, 2023

I think you need to add DELETE watch event type as well.

Yes, I will give a try. I wasn't aware that DELETE event is used for directory renaming.

See some explanation here: https://stackoverflow.com/questions/29368257/implementing-renaming-and-deletion-in-java-watchservice.

So, looks like when we rename the dir, a DELETE event happens and then CREATE & MODIFY, respectively.

Or do you mean the WatchKey is somehow containing a reference to the old dir and therefore doesn't see the rest of entries?

Yes, exactly. When You execute the code in the debugger You will notice that this operation:

WatchKey watchKey = dir.register(this.watcher, this.kinds);

returns the same watchKey object for the renamed directory as that which is already present in the pathKeys. (I assume that the renamed folder is identified internally by some identifier which stays untouched after rename operation...).

However, the path which is resolved from the event (for the file copied into the renamed directory) is incorrect! That’s why I am not sure if this problem is not related to the WhatchService ?
Nevertheless, this can be fixed here in the FileReadingMessageSource class.

More it even doesn't not register a new watcher for that new dir name.

Yes, it seems the watcher doesn't register the renamed folder internally as it seems it “thinks” it is the same folder that has been already registered - that’s why the wrong path is used in the event.

Is that what you try to explain?

Looking to the code I really don't see that we react properly to the DELETE event and remove and entry from this.pathKeys.

Yes, good point !

Probably that one would be as a better solution: support even DELETE by default and remove from this.pathKeys?

I totally agree. I simply didn't know that DELETE event is used in renaming operation.
I will try to fix that this way and let You know if it is working as expected.

Thanks

@artembilan
Copy link
Member

I will try to fix that this way and let You know if it is working as expected.

And from here we are totally opened to see a contribution via Pull Request: https://github.com/spring-projects/spring-integration/blob/main/CONTRIBUTING.adoc

@artembilan artembilan added this to the 6.2.0-M1 milestone Jun 27, 2023
pziobron added a commit to pziobron/spring-integration that referenced this issue Jun 28, 2023
…t new file after renaming the sub-folder of the watched folder (Java 7 WatchService)
@pziobron
Copy link
Contributor Author

Hello @artembilan, I've prepared a PR with a simple fix according to this thread (support DELETE event by default and remove from this.pathKeys): #8662.
This has been tested only on Windows but I believe it should work on all the platforms.

pziobron added a commit to pziobron/spring-integration that referenced this issue Jun 28, 2023
pziobron added a commit to pziobron/spring-integration that referenced this issue Jun 28, 2023
pziobron added a commit to pziobron/spring-integration that referenced this issue Jun 29, 2023
pziobron added a commit to pziobron/spring-integration that referenced this issue Jun 29, 2023
artembilan pushed a commit that referenced this issue Jun 29, 2023
Fixes #8659

* GH-8659: Updating the documentation as requested

* GH-8659: Fixes requested after coder review

**Cherry-pick to `6.1.x`, `6.0.x` & `5.5.x`**

(cherry picked from commit 0798c8d)
artembilan pushed a commit that referenced this issue Jun 29, 2023
Fixes #8659

* GH-8659: Updating the documentation as requested

* GH-8659: Fixes requested after coder review

**Cherry-pick to `6.1.x`, `6.0.x` & `5.5.x`**

(cherry picked from commit 0798c8d)

# Conflicts:
#	spring-integration-file/src/main/java/org/springframework/integration/file/FileReadingMessageSource.java
artembilan pushed a commit that referenced this issue Jun 29, 2023
Fixes #8659

* GH-8659: Updating the documentation as requested

* GH-8659: Fixes requested after coder review

**Cherry-pick to `6.1.x`, `6.0.x` & `5.5.x`**

(cherry picked from commit 0798c8d)

# Conflicts:
#	spring-integration-file/src/main/java/org/springframework/integration/file/FileReadingMessageSource.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants