-
Notifications
You must be signed in to change notification settings - Fork 1.1k
GH-7971: Add FileReadingMessageSource.directoryExpression
#10486
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
Conversation
…sion` Fixes: spring-projects#7971 Extend a `FileReadingMessageSource.directory` logic to the SpEL expression to evaluate on each scan * Introduce an internal `record DirFile` to keep the `File` and scanned directory per file * Propagate a new directory down to the `WatchServiceDirectoryScanner` and restart it * Calculate a root directory from the failed `Message` we offer back to the queue * Fix tests for a new `FileReadingMessageSource.directoryExpression` property * Expose a `Supplier<File>` configuration for Java DSL * Modify `FileTests.MyService.pollDirectories()` for a `Supplier<File>` configuration * Document this new feature
cppwfs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
My usual annoying nitpicks ;-)
| } | ||
|
|
||
| /** | ||
| * Specify a SpEL expression for an input directory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick. But isn't it an implementation of Expression instead of a single implementation of SpELExpression?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The framework is called SpEL and because of many Expression classes in the wild it would be better to be specific in our JavaDocs of what exactly framework an Expression instance is expected for this setter.
We also use SpEL term everywhere else, including docs, so it should not be confusing what is expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! That makes sense. Just wanted to make sure.
| StandardIntegrationFlow integrationFlow = IntegrationFlow | ||
| .from(Files.inboundAdapter(() -> directories[index.getAndIncrement() % directories.length]) | ||
| .recursive(true), | ||
| e -> e.poller(p -> p.fixedDelay(100)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, Was the drop to a delay from 1000 milli to 100 milli on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I'm very suspicious when test is slower than it should be.
Like in this case, we've got before a delay of one second in between reading from those directories we iterate in the supplier.
Such a delay is not justified for this test goal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Just wanted to make sure!
cppwfs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM .
I love the addition!
Fixes: #7971
Extend a
FileReadingMessageSource.directorylogic to the SpEL expression to evaluate on each scanrecord DirFileto keep theFileand scanned directory per fileWatchServiceDirectoryScannerand restart itMessagewe offer back to the queueFileReadingMessageSource.directoryExpressionpropertySupplier<File>configuration for Java DSLFileTests.MyService.pollDirectories()for aSupplier<File>configuration