patka / cassandra-migration

Schema migration library for Cassandra
MIT License
153 stars 47 forks source link

java.nio.file.FileSystemAlreadyExistsException when executing a uber jar #9

Closed mariogarcia closed 7 years ago

mariogarcia commented 7 years ago

I'm using cassandra-migration in a Ratpack application. The application is executed as a uber-jar just using java -jar myapplication.jar. When boostrapping the application the cassandra migration is executed causing the following exception:

Caused by: java.nio.file.FileSystemAlreadyExistsException
        at com.sun.nio.zipfs.ZipFileSystemProvider.newFileSystem(ZipFileSystemProvider.java:113)
        at java.nio.file.FileSystems.newFileSystem(FileSystems.java:326)
        at java.nio.file.FileSystems.newFileSystem(FileSystems.java:276)
        at org.cognitor.cassandra.migration.scanner.JarLocationScanner.findResourceNames(JarLocationScanner.java:35)
        at org.cognitor.cassandra.migration.MigrationRepository.scanForScripts(MigrationRepository.java:164)
        at org.cognitor.cassandra.migration.MigrationRepository.<init>(MigrationRepository.java:118)
        at org.cognitor.cassandra.migration.MigrationRepository.<init>(MigrationRepository.java:101)
        at org.cognitor.cassandra.migration.MigrationRepository.<init>(MigrationRepository.java:88)

Apparently googling a little bit, sometimes it may be necessary to add a check to make sure the file system has not been created already.

I've cloned the code and did a local snapshot changing the following at JarLocationScanner:

    @Override
    public Set<String> findResourceNames(String location, URI locationUri) throws IOException {
        notNullOrEmpty(location, "location");
        notNull(locationUri, "locationUri");
        LOGGER.debug("Scanning in jar {} in location {}", locationUri, location);
        try(FileSystem fileSystem = getFileSystem(locationUri)) {
            final Path systemPath = fileSystem.getPath(location);
            return Files.walk(systemPath)
                    .filter(Files::isRegularFile)
                    .map(path -> normalizePath(path.toString()))
                    .collect(toSet());
        }
    }

    private FileSystem getFileSystem(URI locationUri) throws IOException {
        FileSystem fileSystem = FileSystems.getFileSystem(locationUri);
        if (fileSystem != null) {
            return fileSystem;
        }
        return FileSystems.newFileSystem(locationUri, emptyMap());
    }

And it fixes the problem.

patka commented 7 years ago

Hi Mario,

thanks a lot for investigating this. Sounds like an interesting bug :) I will try to create a test to reproduce this issue and include your change in the code.

Cheers Patrick

mariogarcia commented 7 years ago

Glad to help :)

If you need any more info, just let me know!

patka commented 7 years ago

Hi Mario,

I put your fix in the master branch but I had to change it a little bit. Actually I was not able to check for null on the getFileSytem() return value as the method was thrown a "FileSystemNotFound" exception. Can you quickly check this on your side if it is still working as expected. I have in general the feeling that the contract of FileSystemProvider is not very strict and you have to deal with all kinds of funny situations, depending on the provider ;-)

Cheers Patrick

mariogarcia commented 7 years ago

I'll try to save some time between today and tomorrow to do the checking. I'll get back to you asap. Thanks Patrick

patka commented 7 years ago

Hi Mario,

I am back from vacation and would like to create the release to fix your issues :) Have you had the time to look into this?

Cheers Patrick

mariogarcia commented 7 years ago

Sorry, I did the checking but I didn't come back with the answer.

It's true that I did a jitpack dependency from my forked repository and it worked nicely. But to make sure it would work with your master branch, I've created another jitpack dependency (https://jitpack.io/#patka/cassandra-migration/master-SNAPSHOT) and again, it worked as expected.

I've considered two cases:

It worked as expected in both cases:+1: