further crypto layer optimizations for WebDAV compliance

main/filesystem-api/src/main/java/org/cryptomator/filesystem/delegating/DelegatingFolder.java

@@ -83,7 +83,7 @@ public abstract class DelegatingFolder<D extends DelegatingFolder<D, F>, F exten
 			final Folder delegateDest = ((DelegatingFolder<?, ?>) destination).delegate;
 			delegate.copyTo(delegateDest);
 		} else {
-			delegate.copyTo(destination);
+			Folder.super.copyTo(destination);
 		}
 	}
 

main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/CryptoFile.java

@@ -10,8 +10,7 @@ package org.cryptomator.filesystem.crypto;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import java.io.UncheckedIOException;
-import java.time.Instant;
+import java.util.Optional;
 
 import org.cryptomator.crypto.engine.Cryptor;
 import org.cryptomator.filesystem.File;
@@ -25,25 +24,21 @@ public class CryptoFile extends CryptoNode implements File {
 	}
 
 	@Override
-	protected String encryptedName() {
-		final byte[] parentDirId = parent.getDirectoryId().getBytes(UTF_8);
-		return cryptor.getFilenameCryptor().encryptFilename(name(), parentDirId);
-	}
-
-	@Override
-	public Instant lastModified() throws UncheckedIOException {
-		return physicalFile().lastModified();
+	protected Optional<String> encryptedName() {
+		return parent().get().getDirectoryId().map(s -> s.getBytes(UTF_8)).map(parentDirId -> {
+			return cryptor.getFilenameCryptor().encryptFilename(name(), parentDirId);
+		});
 	}
 
 	@Override
 	public ReadableFile openReadable() {
 		boolean authenticate = !fileSystem().delegate().shouldSkipAuthentication(toString());
-		return new CryptoReadableFile(cryptor.getFileContentCryptor(), physicalFile().openReadable(), authenticate);
+		return new CryptoReadableFile(cryptor.getFileContentCryptor(), forceGetPhysicalFile().openReadable(), authenticate);
 	}
 
 	@Override
 	public WritableFile openWritable() {
-		return new CryptoWritableFile(cryptor.getFileContentCryptor(), physicalFile().openWritable());
+		return new CryptoWritableFile(cryptor.getFileContentCryptor(), forceGetPhysicalFile().openWritable());
 	}
 
 	@Override

main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/CryptoFileSystem.java

@@ -41,12 +41,12 @@ class CryptoFileSystem extends CryptoFolder implements FileSystem {
 	}
 
 	@Override
-	protected String getDirectoryId() {
-		return ROOT_DIRECOTRY_ID;
+	protected Optional<String> getDirectoryId() {
+		return Optional.of(ROOT_DIRECOTRY_ID);
 	}
 
 	@Override
-	protected File physicalFile() {
+	protected Optional<File> physicalFile() {
 		throw new UnsupportedOperationException("Crypto filesystem root doesn't provide a directory file, as the directory ID is fixed.");
 	}
 
@@ -62,17 +62,17 @@ class CryptoFileSystem extends CryptoFolder implements FileSystem {
 
 	@Override
 	public boolean exists() {
-		return physicalFolder().exists();
+		return forceGetPhysicalFolder().exists();
 	}
 
 	@Override
 	public Optional<Instant> creationTime() throws UncheckedIOException {
-		return physicalFolder().creationTime();
+		return forceGetPhysicalFolder().creationTime();
 	}
 
 	@Override
 	public Instant lastModified() {
-		return physicalFolder().lastModified();
+		return forceGetPhysicalFolder().lastModified();
 	}
 
 	@Override
@@ -87,7 +87,7 @@ class CryptoFileSystem extends CryptoFolder implements FileSystem {
 
 	@Override
 	public void create() {
-		physicalFolder().create();
+		forceGetPhysicalFolder().create();
 	}
 
 	@Override

main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/CryptoFolder.java

@@ -10,12 +10,13 @@ package org.cryptomator.filesystem.crypto;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.Reader;
 import java.io.UncheckedIOException;
 import java.io.Writer;
 import java.nio.channels.Channels;
-import java.time.Instant;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Predicate;
@@ -43,35 +44,47 @@ class CryptoFolder extends CryptoNode implements Folder {
 	}
 
 	@Override
-	protected String encryptedName() {
-		final byte[] parentDirId = parent().map(CryptoFolder::getDirectoryId).map(s -> s.getBytes(UTF_8)).orElse(null);
-		return cryptor.getFilenameCryptor().encryptFilename(name(), parentDirId) + DIR_SUFFIX;
+	protected Optional<String> encryptedName() {
+		if (parent().isPresent()) {
+			return parent().get().getDirectoryId().map(s -> s.getBytes(UTF_8)).map(parentDirId -> {
+				return cryptor.getFilenameCryptor().encryptFilename(name(), parentDirId) + DIR_SUFFIX;
+			});
+		} else {
+			return Optional.of(cryptor.getFilenameCryptor().encryptFilename(name()) + DIR_SUFFIX);
+		}
 	}
 
-	Folder physicalFolder() {
-		final String encryptedThenHashedDirId = cryptor.getFilenameCryptor().hashDirectoryId(getDirectoryId());
-		return physicalDataRoot().folder(encryptedThenHashedDirId.substring(0, 2)).folder(encryptedThenHashedDirId.substring(2));
+	Optional<Folder> physicalFolder() {
+		if (getDirectoryId().isPresent()) {
+			final String encryptedThenHashedDirId = cryptor.getFilenameCryptor().hashDirectoryId(getDirectoryId().get());
+			return Optional.of(physicalDataRoot().folder(encryptedThenHashedDirId.substring(0, 2)).folder(encryptedThenHashedDirId.substring(2)));
+		} else {
+			return Optional.empty();
+		}
 	}
 
-	protected String getDirectoryId() {
-		if (directoryId.get() == null) {
-			File dirFile = physicalFile();
+	Folder forceGetPhysicalFolder() {
+		return physicalFolder().orElseThrow(() -> {
+			return new UncheckedIOException(new FileNotFoundException(toString()));
+		});
+	}
+
+	protected Optional<String> getDirectoryId() {
+		if (directoryId.get() != null) {
+			return Optional.of(directoryId.get());
+		}
+		if (physicalFile().isPresent()) {
+			File dirFile = physicalFile().get();
 			if (dirFile.exists()) {
 				try (Reader reader = Channels.newReader(dirFile.openReadable(), UTF_8.newDecoder(), -1)) {
 					directoryId.set(IOUtils.toString(reader));
+					return Optional.of(directoryId.get());
 				} catch (IOException e) {
 					throw new UncheckedIOException(e);
 				}
-			} else {
-				directoryId.compareAndSet(null, UUID.randomUUID().toString());
 			}
 		}
-		return directoryId.get();
-	}
-
-	@Override
-	public Instant lastModified() {
-		return physicalFile().lastModified();
+		return Optional.empty();
 	}
 
 	@Override
@@ -81,15 +94,16 @@ class CryptoFolder extends CryptoNode implements Folder {
 
 	@Override
 	public Stream<CryptoFile> files() {
-		return physicalFolder().files().map(File::name).filter(isEncryptedFileName()).map(this::decryptChildFileName).map(this::file);
+		assert forceGetPhysicalFolder().exists();
+		return forceGetPhysicalFolder().files().map(File::name).filter(isEncryptedFileName()).map(this::decryptChildFileName).map(this::file);
 	}
 
 	private Predicate<String> isEncryptedFileName() {
-		return (String name) -> cryptor.getFilenameCryptor().isEncryptedFilename(name);
+		return (String name) -> !name.endsWith(DIR_SUFFIX) && cryptor.getFilenameCryptor().isEncryptedFilename(name);
 	}
 
 	private String decryptChildFileName(String encryptedFileName) {
-		final byte[] dirId = getDirectoryId().getBytes(UTF_8);
+		final byte[] dirId = getDirectoryId().get().getBytes(UTF_8);
 		return cryptor.getFilenameCryptor().decryptFilename(encryptedFileName, dirId);
 	}
 
@@ -104,15 +118,16 @@ class CryptoFolder extends CryptoNode implements Folder {
 
 	@Override
 	public Stream<CryptoFolder> folders() {
-		return physicalFolder().files().map(File::name).filter(isEncryptedDirectoryName()).map(this::decryptChildFolderName).map(this::folder);
+		assert forceGetPhysicalFolder().exists();
+		return forceGetPhysicalFolder().files().map(File::name).filter(isEncryptedDirectoryName()).map(this::decryptChildFolderName).map(this::folder);
 	}
 
 	private Predicate<String> isEncryptedDirectoryName() {
-		return (String name) -> name.endsWith(DIR_SUFFIX) && isEncryptedFileName().test(StringUtils.removeEnd(name, DIR_SUFFIX));
+		return (String name) -> name.endsWith(DIR_SUFFIX) && cryptor.getFilenameCryptor().isEncryptedFilename(StringUtils.removeEnd(name, DIR_SUFFIX));
 	}
 
 	private String decryptChildFolderName(String encryptedFolderName) {
-		final byte[] dirId = getDirectoryId().getBytes(UTF_8);
+		final byte[] dirId = getDirectoryId().get().getBytes(UTF_8);
 		final String ciphertext = StringUtils.removeEnd(encryptedFolderName, CryptoFolder.DIR_SUFFIX);
 		return cryptor.getFilenameCryptor().decryptFilename(ciphertext, dirId);
 	}
@@ -128,19 +143,21 @@ class CryptoFolder extends CryptoNode implements Folder {
 
 	@Override
 	public void create() {
-		final File dirFile = physicalFile();
-		final Folder dir = physicalFolder();
+		parent.create();
+		final boolean newDirIdGiven = directoryId.compareAndSet(null, UUID.randomUUID().toString());
+		final File dirFile = forceGetPhysicalFile();
+		final Folder dir = forceGetPhysicalFolder();
 		if (dirFile.exists() && dir.exists()) {
 			return;
+		} else if (!newDirIdGiven) {
+			throw new IllegalStateException("Newly created folder, that didn't exist before, already had an directoryId.");
 		}
-		parent.create();
-		final String directoryId = getDirectoryId();
 		try (Writer writer = Channels.newWriter(dirFile.openWritable(), UTF_8.newEncoder(), -1)) {
-			writer.write(directoryId);
+			writer.write(directoryId.get());
 		} catch (IOException e) {
 			throw new UncheckedIOException(e);
 		}
-		physicalFolder().create();
+		dir.create();
 	}
 
 	@Override
@@ -156,26 +173,36 @@ class CryptoFolder extends CryptoNode implements Folder {
 		if (this.isAncestorOf(target) || target.isAncestorOf(this)) {
 			throw new IllegalArgumentException("Can not move directories containing one another (src: " + this + ", dst: " + target + ")");
 		}
+		assert target.parent().isPresent() : "Target can not be root, thus has a parent";
 
-		// directoryId will be used by target, from now on we must no longer use the same id
-		// a new one will be generated on-demand.
-		final String oldDirectoryId = getDirectoryId();
+		target.parent().get().create();
+
+		// explicitly delete target, otherwise same-named folders may keep their directory ids
+		target.delete();
+
+		final File dirFile = forceGetPhysicalFile();
+		final String dirId = getDirectoryId().get();
+		boolean dirIdMovedSuccessfully = target.directoryId.compareAndSet(null, dirId);
+		if (!dirIdMovedSuccessfully) {
+			throw new IllegalStateException("Target's directoryId wasn't null, even though it has been explicitly deleted.");
+		}
+		dirFile.moveTo(target.forceGetPhysicalFile());
 		directoryId.set(null);
 
-		target.physicalFile().parent().get().create();
-		this.physicalFile().moveTo(target.physicalFile());
-
-		target.directoryId.set(oldDirectoryId);
+		assert!exists();
+		assert target.exists();
+		assert!dirFile.exists();
 	}
 
 	@Override
 	public void delete() {
 		if (!exists()) {
+			directoryId.set(null);
 			return;
 		}
 		Deleter.deleteContent(this);
-		physicalFolder().delete();
-		physicalFile().delete();
+		forceGetPhysicalFolder().delete();
+		forceGetPhysicalFile().delete();
 		directoryId.set(null);
 	}
 

main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/CryptoNode.java

@@ -8,6 +8,7 @@
  *******************************************************************************/
 package org.cryptomator.filesystem.crypto;
 
+import java.io.FileNotFoundException;
 import java.io.UncheckedIOException;
 import java.time.Instant;
 import java.util.Optional;
@@ -33,10 +34,20 @@ abstract class CryptoNode implements Node {
 		return parent.physicalDataRoot();
 	}
 
-	protected abstract String encryptedName();
+	protected abstract Optional<String> encryptedName();
 
-	protected File physicalFile() {
-		return parent.physicalFolder().file(encryptedName());
+	protected Optional<File> physicalFile() {
+		if (parent.exists() && encryptedName().isPresent()) {
+			return Optional.of(parent.forceGetPhysicalFolder().file(encryptedName().get()));
+		} else {
+			return Optional.empty();
+		}
+	}
+
+	protected File forceGetPhysicalFile() {
+		return physicalFile().orElseThrow(() -> {
+			return new UncheckedIOException(new FileNotFoundException(toString()));
+		});
 	}
 
 	@Override
@@ -56,17 +67,22 @@ abstract class CryptoNode implements Node {
 
 	@Override
 	public boolean exists() {
-		return physicalFile().exists();
+		return physicalFile().map(File::exists).orElse(false);
+	}
+
+	@Override
+	public Instant lastModified() {
+		return forceGetPhysicalFile().lastModified();
 	}
 
 	@Override
 	public Optional<Instant> creationTime() throws UncheckedIOException {
-		return physicalFile().creationTime();
+		return forceGetPhysicalFile().creationTime();
 	}
 
 	@Override
 	public void setCreationTime(Instant instant) throws UncheckedIOException {
-		physicalFile().setCreationTime(instant);
+		forceGetPhysicalFile().setCreationTime(instant);
 	}
 
 	@Override

main/filesystem-crypto/src/test/java/org/cryptomator/filesystem/crypto/CryptoFileSystemTest.java

@@ -51,6 +51,55 @@ public class CryptoFileSystemTest {
 																	// foo + bar
 	}
 
+	@Test(timeout = 2000)
+	public void testDirectoryCopyAndMove() throws UncheckedIOException, IOException {
+		// mock stuff and prepare crypto FS:
+		final Cryptor cryptor = new NoCryptor();
+		final FileSystem physicalFs = new InMemoryFileSystem();
+		final FileSystem fs = new CryptoFileSystem(physicalFs, cryptor, Mockito.mock(CryptoFileSystemDelegate.class), "foo");
+
+		// create /src/one/two/ and /dst/one:
+		final Folder src = fs.folder("src");
+		final Folder srcSub = src.folder("one");
+		final Folder srcSubSub = srcSub.folder("two");
+		final Folder dst = fs.folder("dst");
+		final Folder dstSub = dst.folder("one");
+		final Folder dstSubSub = dstSub.folder("two");
+		final Folder dst2 = fs.folder("dst2");
+
+		srcSubSub.create();
+		dstSub.create();
+		Assert.assertTrue(srcSubSub.exists());
+		Assert.assertTrue(dstSub.exists());
+		Assert.assertFalse(dstSubSub.exists());
+		Assert.assertFalse(dst2.exists());
+
+		src.copyTo(dst2);
+		Assert.assertTrue(dst2.exists());
+		Assert.assertTrue(dst2.folder("one").exists());
+		Assert.assertTrue(dst2.folder("one").folder("two").exists());
+
+		dst.delete();
+		Assert.assertFalse(dst.exists());
+		Assert.assertFalse(dst.folder("one").exists());
+		Assert.assertFalse(dst.folder("one").folder("two").exists());
+
+		dst2.moveTo(dst);
+		Assert.assertTrue(dst.exists());
+		Assert.assertTrue(dst.folder("one").exists());
+		Assert.assertTrue(dst.folder("one").folder("two").exists());
+
+		dst.folder("one").delete();
+		Assert.assertTrue(dst.exists());
+		Assert.assertFalse(dst.folder("one").exists());
+		Assert.assertFalse(dst.folder("one").folder("two").exists());
+
+		dst.copyTo(dst2);
+		Assert.assertTrue(dst2.exists());
+		Assert.assertFalse(dst2.folder("one").exists());
+		Assert.assertFalse(dst2.folder("one").folder("two").exists());
+	}
+
 	@Test(timeout = 1000)
 	public void testDirectoryMoving() throws UncheckedIOException, IOException {
 		// mock stuff and prepare crypto FS:

main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryFile.java

@@ -42,29 +42,47 @@ class InMemoryFile extends InMemoryNode implements File {
 		if (!exists()) {
 			throw new UncheckedIOException(new FileNotFoundException(this.name() + " does not exist"));
 		}
+		boolean success = false;
 		final ReadLock readLock = lock.readLock();
 		readLock.lock();
-		return new InMemoryReadableFile(this::getContent, readLock);
+		try {
+			final ReadableFile result = new InMemoryReadableFile(this::getContent, readLock);
+			success = true;
+			return result;
+		} finally {
+			if (!success) {
+				readLock.unlock();
+			}
+		}
 	}
 
 	@Override
 	public WritableFile openWritable() {
+		boolean success = false;
 		final WriteLock writeLock = lock.writeLock();
 		writeLock.lock();
-		final InMemoryFolder parent = parent().get();
-		parent.existingChildren.compute(this.name(), (k, v) -> {
-			if (v != null && v != this) {
-				// other file or folder with same name already exists.
-				throw new UncheckedIOException(new FileAlreadyExistsException(k));
-			} else {
-				if (v == null) {
-					this.creationTime = Instant.now();
+		try {
+			final InMemoryFolder parent = parent().get();
+			parent.existingChildren.compute(this.name(), (k, v) -> {
+				if (v != null && v != this) {
+					// other file or folder with same name already exists.
+					throw new UncheckedIOException(new FileAlreadyExistsException(k));
+				} else {
+					if (v == null) {
+						this.creationTime = Instant.now();
+					}
+					this.lastModified = Instant.now();
+					return this;
 				}
-				this.lastModified = Instant.now();
-				return this;
+			});
+			final WritableFile result = new InMemoryWritableFile(this::setLastModified, this::setCreationTime, this::getContent, this::setContent, this::delete, writeLock);
+			success = true;
+			return result;
+		} finally {
+			if (!success) {
+				writeLock.unlock();
 			}
-		});
-		return new InMemoryWritableFile(this::setLastModified, this::setCreationTime, this::getContent, this::setContent, this::delete, writeLock);
+		}
 	}
 
 	private void setLastModified(Instant lastModified) {
@@ -80,6 +98,8 @@ class InMemoryFile extends InMemoryNode implements File {
 	}
 
 	private void delete(Void param) {
+		content = ByteBuffer.allocate(INITIAL_SIZE);
+		content.flip();
 		final InMemoryFolder parent = parent().get();
 		parent.existingChildren.computeIfPresent(this.name(), (k, v) -> {
 			// returning null removes the entry.