From 9c844e626a44972909746a552cfecff852e6443e Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Tue, 12 Jan 2016 23:50:18 +0100 Subject: [PATCH] Fixed NioFileSystemIntegrationTests on windows * Streams returned from NioFolder#children, files and folders are now closed automatically after a terminal operation * Not closing them lead to a bug on windows causing directories to be not deleted after a successful Files.delete invocation --- .../cryptomator/common/AutoClosingStream.java | 173 +++++++++++++++ .../cryptomator/common/DelegatingStream.java | 208 ++++++++++++++++++ .../common/AutoClosingStreamTest.java | 59 +++++ .../cryptomator/filesystem/nio/NioFolder.java | 4 +- .../filesystem/nio/NioFolderTest.java | 10 + 5 files changed, 453 insertions(+), 1 deletion(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/AutoClosingStream.java create mode 100644 main/commons/src/main/java/org/cryptomator/common/DelegatingStream.java create mode 100644 main/commons/src/test/java/org/cryptomator/common/AutoClosingStreamTest.java diff --git a/main/commons/src/main/java/org/cryptomator/common/AutoClosingStream.java b/main/commons/src/main/java/org/cryptomator/common/AutoClosingStream.java new file mode 100644 index 000000000..02207009a --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/AutoClosingStream.java @@ -0,0 +1,173 @@ +package org.cryptomator.common; + +import java.util.Comparator; +import java.util.Optional; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; +import java.util.function.BinaryOperator; +import java.util.function.Consumer; +import java.util.function.IntFunction; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.stream.Collector; +import java.util.stream.DoubleStream; +import java.util.stream.IntStream; +import java.util.stream.LongStream; +import java.util.stream.Stream; + +/** + *

+ * A Stream which is automatically closed on after execution of a terminal operation. + *

+ * The stream keeps its auto closing behavior on invocations of intermediate operations as long as no conversion to an {@link IntStream}, {@link LongStream} or {@link DoubleStream} occurs. + * + * @author Markus Kreusch + */ +public final class AutoClosingStream extends DelegatingStream { + + public static Stream from(Stream delegate) { + return new AutoClosingStream<>(delegate); + } + + @SuppressWarnings("unchecked") + private AutoClosingStream(Stream delegate) { + super(delegate, AutoClosingStream::new); + } + + public void forEach(Consumer action) { + try { + super.forEach(action); + } finally { + close(); + } + } + + public void forEachOrdered(Consumer action) { + try { + super.forEachOrdered(action); + } finally { + close(); + } + } + + public Object[] toArray() { + try { + return super.toArray(); + } finally { + close(); + } + } + + public A[] toArray(IntFunction generator) { + try { + return super.toArray(generator); + } finally { + close(); + } + } + + public T reduce(T identity, BinaryOperator accumulator) { + try { + return super.reduce(identity, accumulator); + } finally { + close(); + } + } + + public Optional reduce(BinaryOperator accumulator) { + try { + return super.reduce(accumulator); + } finally { + close(); + } + } + + public U reduce(U identity, BiFunction accumulator, BinaryOperator combiner) { + try { + return super.reduce(identity, accumulator, combiner); + } finally { + close(); + } + } + + public R collect(Supplier supplier, BiConsumer accumulator, BiConsumer combiner) { + try { + return super.collect(supplier, accumulator, combiner); + } finally { + close(); + } + } + + public R collect(Collector collector) { + try { + return super.collect(collector); + } finally { + close(); + } + } + + public Optional min(Comparator comparator) { + try { + return super.min(comparator); + } finally { + close(); + } + } + + public Optional max(Comparator comparator) { + try { + return super.max(comparator); + } finally { + close(); + } + } + + public long count() { + try { + return super.count(); + } finally { + close(); + } + } + + public boolean anyMatch(Predicate predicate) { + try { + return super.anyMatch(predicate); + } finally { + close(); + } + } + + public boolean allMatch(Predicate predicate) { + try { + return super.allMatch(predicate); + } finally { + close(); + } + } + + public boolean noneMatch(Predicate predicate) { + try { + return super.noneMatch(predicate); + } finally { + close(); + } + } + + public Optional findFirst() { + try { + return super.findFirst(); + } finally { + close(); + } + } + + public Optional findAny() { + try { + return super.findAny(); + } finally { + close(); + } + } + +} diff --git a/main/commons/src/main/java/org/cryptomator/common/DelegatingStream.java b/main/commons/src/main/java/org/cryptomator/common/DelegatingStream.java new file mode 100644 index 000000000..e0049f260 --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/DelegatingStream.java @@ -0,0 +1,208 @@ +package org.cryptomator.common; + +import java.util.Comparator; +import java.util.Iterator; +import java.util.Optional; +import java.util.Spliterator; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; +import java.util.function.BinaryOperator; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.IntFunction; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.function.ToDoubleFunction; +import java.util.function.ToIntFunction; +import java.util.function.ToLongFunction; +import java.util.stream.Collector; +import java.util.stream.DoubleStream; +import java.util.stream.IntStream; +import java.util.stream.LongStream; +import java.util.stream.Stream; + +abstract class DelegatingStream implements Stream { + + private final Stream delegate; + private final StreamWrapper wrapper; + + protected DelegatingStream(Stream delegate, StreamWrapper wrapper) { + this.delegate = delegate; + this.wrapper = wrapper; + } + + private Stream wrapped(Stream other) { + if (getClass().isInstance(other)) { + return other; + } else { + return wrapper.wrap(other); + } + } + + public Iterator iterator() { + return delegate.iterator(); + } + + public Spliterator spliterator() { + return delegate.spliterator(); + } + + public boolean isParallel() { + return delegate.isParallel(); + } + + public Stream sequential() { + return wrapped(delegate.sequential()); + } + + public Stream parallel() { + return wrapped(delegate.parallel()); + } + + public Stream unordered() { + return wrapped(delegate.unordered()); + } + + public Stream onClose(Runnable closeHandler) { + return wrapped(delegate.onClose(closeHandler)); + } + + public void close() { + delegate.close(); + } + + public Stream filter(Predicate predicate) { + return wrapped(delegate.filter(predicate)); + } + + public Stream map(Function mapper) { + return wrapped(delegate.map(mapper)); + } + + public IntStream mapToInt(ToIntFunction mapper) { + return delegate.mapToInt(mapper); + } + + public LongStream mapToLong(ToLongFunction mapper) { + return delegate.mapToLong(mapper); + } + + public DoubleStream mapToDouble(ToDoubleFunction mapper) { + return delegate.mapToDouble(mapper); + } + + public Stream flatMap(Function> mapper) { + return wrapped(delegate.flatMap(mapper)); + } + + public IntStream flatMapToInt(Function mapper) { + return delegate.flatMapToInt(mapper); + } + + public LongStream flatMapToLong(Function mapper) { + return delegate.flatMapToLong(mapper); + } + + public DoubleStream flatMapToDouble(Function mapper) { + return delegate.flatMapToDouble(mapper); + } + + public Stream distinct() { + return wrapped(delegate.distinct()); + } + + public Stream sorted() { + return wrapped(delegate.sorted()); + } + + public Stream sorted(Comparator comparator) { + return wrapped(delegate.sorted(comparator)); + } + + public Stream peek(Consumer action) { + return wrapped(delegate.peek(action)); + } + + public Stream limit(long maxSize) { + return wrapped(delegate.limit(maxSize)); + } + + public Stream skip(long n) { + return wrapped(delegate.skip(n)); + } + + public void forEach(Consumer action) { + delegate.forEach(action); + } + + public void forEachOrdered(Consumer action) { + delegate.forEachOrdered(action); + } + + public Object[] toArray() { + return delegate.toArray(); + } + + public A[] toArray(IntFunction generator) { + return delegate.toArray(generator); + } + + public T reduce(T identity, BinaryOperator accumulator) { + return delegate.reduce(identity, accumulator); + } + + public Optional reduce(BinaryOperator accumulator) { + return delegate.reduce(accumulator); + } + + public U reduce(U identity, BiFunction accumulator, BinaryOperator combiner) { + return delegate.reduce(identity, accumulator, combiner); + } + + public R collect(Supplier supplier, BiConsumer accumulator, BiConsumer combiner) { + return delegate.collect(supplier, accumulator, combiner); + } + + public R collect(Collector collector) { + return delegate.collect(collector); + } + + public Optional min(Comparator comparator) { + return delegate.min(comparator); + } + + public Optional max(Comparator comparator) { + return delegate.max(comparator); + } + + public long count() { + return delegate.count(); + } + + public boolean anyMatch(Predicate predicate) { + return delegate.anyMatch(predicate); + } + + public boolean allMatch(Predicate predicate) { + return delegate.allMatch(predicate); + } + + public boolean noneMatch(Predicate predicate) { + return delegate.noneMatch(predicate); + } + + public Optional findFirst() { + return delegate.findFirst(); + } + + public Optional findAny() { + return delegate.findAny(); + } + + public interface StreamWrapper { + + Stream wrap(Stream other); + + } + +} diff --git a/main/commons/src/test/java/org/cryptomator/common/AutoClosingStreamTest.java b/main/commons/src/test/java/org/cryptomator/common/AutoClosingStreamTest.java new file mode 100644 index 000000000..e831ecefa --- /dev/null +++ b/main/commons/src/test/java/org/cryptomator/common/AutoClosingStreamTest.java @@ -0,0 +1,59 @@ +package org.cryptomator.common; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.function.Consumer; +import java.util.stream.Stream; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.InOrder; + +@SuppressWarnings("unchecked") +public class AutoClosingStreamTest { + + private Stream delegate; + private Stream inTest; + + @Before + public void setUp() { + delegate = mock(Stream.class); + inTest = AutoClosingStream.from(delegate); + } + + @Test + public void testSequentialReturnsNewAutoClosingStream() { + Stream newDelegate = mock(Stream.class); + when(delegate.sequential()).thenReturn(newDelegate); + + Stream result = inTest.sequential(); + + assertThat(result, is(instanceOf(AutoClosingStream.class))); + verifyDelegate(result, newDelegate); + } + + @Test + public void testForEachDelegatesToAndClosesDelegate() { + Consumer consumer = mock(Consumer.class); + + inTest.forEach(consumer); + + InOrder inOrder = inOrder(delegate); + inOrder.verify(delegate).forEach(consumer); + inOrder.verify(delegate).close(); + } + + private void verifyDelegate(Stream result, Stream newDelegate) { + result.close(); + verify(newDelegate).close(); + } + + // TODO Markus Kreusch test additional methods + +} diff --git a/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFolder.java b/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFolder.java index e11c39c30..ccf842064 100644 --- a/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFolder.java +++ b/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFolder.java @@ -9,6 +9,7 @@ import java.time.Instant; import java.util.Optional; import java.util.stream.Stream; +import org.cryptomator.common.AutoClosingStream; import org.cryptomator.common.WeakValuedCache; import org.cryptomator.filesystem.File; import org.cryptomator.filesystem.Folder; @@ -27,7 +28,7 @@ class NioFolder extends NioNode implements Folder { @Override public Stream children() throws UncheckedIOException { try { - return nioAccess.list(path).map(this::childPathToNode); + return AutoClosingStream.from(nioAccess.list(path).map(this::childPathToNode)); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -130,6 +131,7 @@ class NioFolder extends NioNode implements Folder { if (!exists()) { return; } + folders().forEach(Folder::delete); files().forEach(NioFolder::deleteFile); try { diff --git a/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFolderTest.java b/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFolderTest.java index 068b29e34..58baf7680 100644 --- a/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFolderTest.java +++ b/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFolderTest.java @@ -5,6 +5,7 @@ import static java.util.stream.Collectors.toList; import static org.cryptomator.common.test.matcher.ContainsMatcher.contains; import static org.cryptomator.filesystem.nio.ReflectiveClassMatchers.aClassThatDoesDeclareMethod; import static org.cryptomator.filesystem.nio.ReflectiveClassMatchers.aClassThatDoesNotDeclareMethod; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.theInstance; import static org.junit.Assert.assertThat; @@ -23,6 +24,7 @@ import java.util.Optional; import java.util.function.Supplier; import java.util.stream.Stream; +import org.cryptomator.common.AutoClosingStream; import org.cryptomator.filesystem.File; import org.cryptomator.filesystem.FileSystem; import org.cryptomator.filesystem.Folder; @@ -75,6 +77,14 @@ public class NioFolderTest { public class ChildrenTests { + @Test + public void testChildrenReturnsAnAutoClosingStream() throws IOException { + Stream childrenPaths = Stream.builder().build(); + when(nioAccess.list(path)).thenReturn(childrenPaths); + + assertThat(inTest.children(), is(instanceOf(AutoClosingStream.class))); + } + @Test public void testChildrenConvertsPathWhichIsADirectoryToAnNioFolderUsingTheInstanceFactory() throws IOException { Path childFolderPath = mock(Path.class);