-
-
Notifications
You must be signed in to change notification settings - Fork 197
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
File watcher module #628
File watcher module #628
Conversation
Hi, I can do this today near 20:00 CET. |
Not at all, just a new module :) which I probably going to reuse in the rocker template module. Thanks. |
Sorry, I got now what you mean. I didn't wrote anything about it yet, but for now let's make sure to use 2 spaces for indent, one statement for line, final for parameters. I'm open to suggestion too. |
|
||
@Override | ||
public String toString() { | ||
return "**/*.*"; |
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.
I think it should be "**/*" since there might be files without '.'
} | ||
}; | ||
|
||
private static final WatchEvent.Kind<?>[] KINDS = {ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY }; |
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.
Needs a space after the opening bracket.
} | ||
|
||
FileEventHandler handler( | ||
final Function<Class<? extends FileEventHandler>, FileEventHandler> factory) { |
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.
4 spaces used for indent instead of 2.
Is this an exception when breaking long lines in method parameters?
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.
yes, max line characters is 100, when we couldn't fit everything in one line, we use 4 spaces.
class FileMonitor implements Runnable { | ||
|
||
/** The logging system. */ | ||
private final Logger log = LoggerFactory.getLogger(getClass()); |
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.
Why is this not static?
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.
Class is a singleton, no need to use a static logger. Also, I read somewhere that static logger cause leaks within dynamic class loaders (like the one we use with jooby:run), so unless is strictly necessary I alway keep log as final instance field.
private Set<FileEventOptions> optionList; | ||
|
||
@Inject | ||
public FileMonitor(final Injector injector, |
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.
injector, watcher and optionList fields can be final, is there a reason why are they not final while some other non-static fields final?
/** The logging system. */ | ||
private final Logger log = LoggerFactory.getLogger(getClass()); | ||
|
||
private List<CheckedFunction2<Config, Binder, FileEventOptions>> bindings = new ArrayList<>(); |
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.
In FileMonitor keys and options fields are both final and this field is very similar to them, do you have a rule when final should be used for fields?
|
||
@Override | ||
public boolean matches(final Path path) { | ||
for (PathMatcher matcher : matchers) { |
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.
As I saw you like to use Java 8 stream API, whouldn't it be better to write as matchers.stream().filter(...).findFirst()... ?
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.
Pls. check my comments, some of them apply to other places too but I don't want to put redundant reactions
@krisztiankocsis your comments are all in.. I moved to IDEA so there are some unrelated and required pom changes to make IDEA happy. Thanks |
file watcher
Watches for file system changes or event. It uses a watch service to monitor a directory for changes so that it can update its display of the list of files when files are created or deleted.
dependency
usage
Watch
mydir
and listen for create, modify or delete event on all files under it:file handler
You can specify a {@link FileEventHandler} instance or class while registering a path:
Instance:
Class reference:
Worth to mention that
MyFileEventHandler
will be provided by Guice.options
You can specify a couple of options at registration time:
Here we listen for {@link StandardWatchEventKinds#ENTRY_MODIFY} (we don't care about create or delete).
We turn off recursive watching, only direct files are detected.
We want
.java
files and we ignore any other files.configuration file
In addition to do it programmatically, you can do it via configuration properties:
The
mydirproperty
property must be present in your.conf
file.But of course, you can entirely register paths from
.conf
file:Multiple paths are supported using array notation:
Now use the module:
The FileWatcher module read the
filewatcher.register
property and setup everything.