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

Make polling frequency in PollingWatchService configurable #14

Closed
sherter opened this issue Dec 3, 2014 · 9 comments
Closed

Make polling frequency in PollingWatchService configurable #14

sherter opened this issue Dec 3, 2014 · 9 comments
Labels
fixed type=enhancement Make an existing feature better
Milestone

Comments

@sherter
Copy link

sherter commented Dec 3, 2014

The WatchService implementation in PollingWatchService uses a hard coded polling frequency (5 seconds). Currently I need to sleep relatively long in my tests in order to receive the event. It would be nice if this could be configured at file system creation time (e.g. by adding something to Configuration.builder()).

@cgdecker
Copy link
Member

cgdecker commented Dec 3, 2014

Yeah, there should be a way to configure this. I didn't make it configurable initially because I'm not sure what the best way to expose configuration for it is. It's rather fine-grained to expose as a top-level configuration option, and it's not impossible that at some point we may want to expose a way to configure that WatchService implementation at a higher level.

@cgdecker cgdecker added the type=enhancement Make an existing feature better label Dec 3, 2014
@cschoell
Copy link

Real pita here as well ... I want to use jimFS to speed up my (integration) unit tests and that's a show stopper in that regard really ;)

I solved it by making the fields accessible via reflection and setting the values straight after creating the watchservice ... but seriously - thats an ultra ugly hack ;)

Really great project otherwise though:+1:

@pvorb
Copy link

pvorb commented Jul 27, 2015

@cschoell May I ask how you did that? I also tried to use reflection to change the pollingTime of the PollingWatchService, but because FileSystem returns a new instance every time, that's no applicable approach for testing.

@cgdecker A configuration property would be much appreciated.

@cschoell
Copy link

@pvorb I basically have a single method / place where I create a new WatchService from my FileSystem. In my Unit Test I override the method which calls <FileSystem>.newWatchService() (or alternatively replace some watchsystem provider object...?) and I'm good to go....

Example code of class to test inside my unit test:

XYService underTest = new XYService() {
        @Override
        protected WatchService createWatchservice() throws IOException {
            WatchService watchservice = super.createWatchservice();
            //very ugly hack to make com.google.common.jimfs.PollingWatchService usable in a timely fashion
            //there is no straight forward way to configure the polling time (It's statically set to 5 SECONDS)
            ReflectionTestUtils.setField(watchservice, "pollingTime", 5);
            ReflectionTestUtils.setField(watchservice, "timeUnit", TimeUnit.MILLISECONDS);
            return watchservice;
        }
    };

... ReflectionTestUtils is from the Spring Test Framework btw ;)

@pvorb
Copy link

pvorb commented Jul 27, 2015

@cschoell Thank you for the inspiration.

Unfortunately it doesn't fit my particular situation. I want to test a class that directly calls FileSystem.newWatchService() (in the test case, it's a JimfsFileSystem), but I can't override that method since JimfsFileSystem is final. Even PowerMock can't do it since the class is package-private. Probably I also need to abstract some kind of WatchServiceProvider (XYService like you called it). Nonetheless, thanks for the quick reply.

@cschoell
Copy link

Happy I was able to help a bit ;)

XYService is the class which Im Testing though - which is anonymously extended in the Unit Test... And in this case my "provider" is just a method called ’createWatchservice’ which I override...

Just to clarify ;)

@sparty02
Copy link

👍 On this! In the meantime, for anybody interested, I leveraged the following workaround:

Decorator to setup the PollingWatchService with a configurable polling interval (note it has to be in the com.google.common.jimfs package due to the visibility of items in the JimfsFileSystem:

package com.google.common.jimfs;

import java.io.IOException;
import java.nio.file.FileStore;
import java.nio.file.FileSystem;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.nio.file.WatchService;
import java.nio.file.attribute.UserPrincipalLookupService;
import java.nio.file.spi.FileSystemProvider;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import org.assertj.core.util.Objects;

public class PollOnWatchFileSystemDecorator extends FileSystem
{

    private final JimfsFileSystem delegate;
    private final long pollingTime;
    private final TimeUnit timeUnit;

    public PollOnWatchFileSystemDecorator(FileSystem delegate, long pollingTime, TimeUnit timeUnit)
    {
        if (!(delegate instanceof JimfsFileSystem))
        {
            throw new RuntimeException(
                "Decorator only works with instances of the JimfsFileSystem");
        }

        this.delegate = Objects.castIfBelongsToType(delegate, JimfsFileSystem.class);
        this.pollingTime = pollingTime;
        this.timeUnit = timeUnit;
    }

    @Override
    public FileSystemProvider provider()
    {
        return delegate.provider();
    }

    @Override
    public void close() throws IOException
    {
        delegate.close();
    }

    @Override
    public boolean isOpen()
    {
        return delegate.isOpen();
    }

    @Override
    public boolean isReadOnly()
    {
        return delegate.isReadOnly();
    }

    @Override
    public String getSeparator()
    {
        return delegate.getSeparator();
    }

    @Override
    public Iterable<Path> getRootDirectories()
    {
        return delegate.getRootDirectories();
    }

    @Override
    public Iterable<FileStore> getFileStores()
    {
        return delegate.getFileStores();
    }

    @Override
    public Set<String> supportedFileAttributeViews()
    {
        return delegate.supportedFileAttributeViews();
    }

    @Override
    public Path getPath(String first, String... more)
    {
        return delegate.getPath(first, more);
    }

    @Override
    public PathMatcher getPathMatcher(String syntaxAndPattern)
    {
        return delegate.getPathMatcher(syntaxAndPattern);
    }

    @Override
    public UserPrincipalLookupService getUserPrincipalLookupService()
    {
        return delegate.getUserPrincipalLookupService();
    }

    @Override
    public WatchService newWatchService() throws IOException
    {
        return new PollingWatchService(delegate.getDefaultView(), delegate.getPathService(),
            delegate.getFileStore().state(), pollingTime, timeUnit);
    }

}

...then in the consuming class:

FileSystem unixFs = Jimfs.newFileSystem(com.google.common.jimfs.Configuration.unix());
FileSystem fileSystem = new PollOnWatchFileSystemDecorator(unixFs, 1, TimeUnit.SECONDS);

@cgdecker
Copy link
Member

cgdecker commented Jan 6, 2016

I do agree that it should be addressed. As I indicated above, the main thing that's made me hesitant about just throwing a new option into Configuration is that it effectively locks in polling as the only type of watch service that can be used. At the very least, it makes it difficult to change that in an elegant way later (if, say, we wanted to add a watch service implementation that gets change notifications pushed to it rather than polling the file system).

I've actually got something in the works for this that should address the above concern, it just obviously hasn't been a high priority for me unfortunately. I'll try to make the changes in the next couple of weeks, and then perhaps make a 1.1 release.

@cgdecker cgdecker added the fixed label Jan 12, 2016
@cgdecker cgdecker added this to the 1.1 milestone Jan 12, 2016
@pvorb
Copy link

pvorb commented Jan 12, 2016

Thank you for adding WatchServiceConfiguration!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed type=enhancement Make an existing feature better
Projects
None yet
Development

No branches or pull requests

5 participants