code simplicifaction + lockmanager unit tests

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

@@ -14,7 +14,7 @@ import java.util.Optional;
 
 import org.cryptomator.filesystem.Node;
 
-abstract class DelegatingNode<T extends Node> implements Node {
+public abstract class DelegatingNode<T extends Node> implements Node {
 
 	protected final T delegate;
 

main/frontend-webdav/src/main/java/org/cryptomator/frontend/webdav/jackrabbitservlet/ExclusiveSharedLockManager.java

@@ -41,22 +41,22 @@ public class ExclusiveSharedLockManager implements LockManager {
 		FileSystemResourceLocator locator = resource.getLocator();
 		removedExpiredLocksInLocatorHierarchy(locator);
 
+		// look for existing locks on this resource or its ancestors:
 		ActiveLock existingExclusiveLock = getLock(lockInfo.getType(), Scope.EXCLUSIVE, resource);
 		ActiveLock existingSharedLock = getLock(lockInfo.getType(), Scope.SHARED, resource);
 		boolean hasExclusiveLock = existingExclusiveLock != null;
 		boolean hasSharedLock = existingSharedLock != null;
 		boolean isLocked = hasExclusiveLock || hasSharedLock;
 		if ((Scope.EXCLUSIVE.equals(lockInfo.getScope()) && isLocked) || (Scope.SHARED.equals(lockInfo.getScope()) && hasExclusiveLock)) {
-			throw new DavException(DavServletResponse.SC_LOCKED, "Resource already locked.");
+			throw new DavException(DavServletResponse.SC_LOCKED, "Resource (or parent resource) already locked.");
 		}
 
-		for (Entry<FileSystemResourceLocator, Map<String, ActiveLock>> entry : lockedResources.entrySet()) {
-			final FileSystemResourceLocator entryLocator = entry.getKey();
-			final Collection<ActiveLock> entryLocks = entry.getValue().values();
-			if (isAncestor(entryLocator, locator) && isAffectedByParentLocks(lockInfo, locator, entryLocks, entryLocator)) {
-				throw new DavException(DavServletResponse.SC_LOCKED, "Parent resource already locked. " + entryLocator);
-			} else if (isAncestor(locator, entryLocator) && isAffectedByChildLocks(lockInfo, locator, entryLocks, entryLocator)) {
-				throw new DavException(DavServletResponse.SC_CONFLICT, "Subresource already locked. " + entryLocator);
+		// look for locked children:
+		for (Entry<FileSystemResourceLocator, Map<String, ActiveLock>> potentialChild : lockedResources.entrySet()) {
+			final FileSystemResourceLocator childLocator = potentialChild.getKey();
+			final Collection<ActiveLock> childLocks = potentialChild.getValue().values();
+			if (isChild(locator, childLocator) && isAffectedByChildLocks(lockInfo, locator, childLocks, childLocator)) {
+				throw new DavException(DavServletResponse.SC_CONFLICT, "Subresource already locked. " + childLocator);
 			}
 		}
 
@@ -65,11 +65,12 @@ public class ExclusiveSharedLockManager implements LockManager {
 	}
 
 	private void removedExpiredLocksInLocatorHierarchy(FileSystemResourceLocator locator) {
+		Objects.requireNonNull(locator);
 		lockedResources.getOrDefault(locator, Collections.emptyMap()).values().removeIf(ActiveLock::isExpired);
 		locator.parent().ifPresent(this::removedExpiredLocksInLocatorHierarchy);
 	}
 
-	private boolean isAncestor(FileSystemResourceLocator parent, FileSystemResourceLocator child) {
+	private boolean isChild(FileSystemResourceLocator parent, FileSystemResourceLocator child) {
 		if (parent instanceof FolderLocator) {
 			FolderLocator folder = (FolderLocator) parent;
 			return folder.isAncestorOf(child);
@@ -78,18 +79,6 @@ public class ExclusiveSharedLockManager implements LockManager {
 		}
 	}
 
-	private boolean isAffectedByParentLocks(LockInfo childLockInfo, FileSystemResourceLocator childLocator, Collection<ActiveLock> parentLocks, FileSystemResourceLocator parentLocator) {
-		assert childLocator.parent().isPresent();
-		for (ActiveLock lock : parentLocks) {
-			if (Scope.SHARED.equals(childLockInfo.getScope()) && Scope.SHARED.equals(lock.getScope())) {
-				continue;
-			} else if (lock.isDeep() || childLocator.parent().get().equals(parentLocator)) {
-				return true;
-			}
-		}
-		return false;
-	}
-
 	private boolean isAffectedByChildLocks(LockInfo parentLockInfo, FileSystemResourceLocator parentLocator, Collection<ActiveLock> childLocks, FileSystemResourceLocator childLocator) {
 		assert childLocator.parent().isPresent();
 		for (ActiveLock lock : childLocks) {
@@ -107,7 +96,7 @@ public class ExclusiveSharedLockManager implements LockManager {
 		ActiveLock lock = getLock(lockInfo.getType(), lockInfo.getScope(), resource);
 		if (lock == null) {
 			throw new DavException(DavServletResponse.SC_PRECONDITION_FAILED);
-		} else if (!lock.getToken().equals(lockToken)) {
+		} else if (!lock.isLockedByToken(lockToken)) {
 			throw new DavException(DavServletResponse.SC_LOCKED);
 		}
 		lock.setTimeout(lockInfo.getTimeout());
@@ -144,24 +133,24 @@ public class ExclusiveSharedLockManager implements LockManager {
 	@Override
 	public ActiveLock getLock(Type type, Scope scope, DavResource resource) {
 		if (resource instanceof DavNode) {
-			return getLockInternal(type, scope, ((DavNode<?>) resource).getLocator());
+			return getLockInternal(type, scope, ((DavNode<?>) resource).getLocator(), 0);
 		} else {
 			throw new IllegalArgumentException("Unsupported resource type " + resource.getClass());
 		}
 	}
 
-	private ActiveLock getLockInternal(Type type, Scope scope, FileSystemResourceLocator locator) {
+	private ActiveLock getLockInternal(Type type, Scope scope, FileSystemResourceLocator locator, int depth) {
 		// try to find a lock directly on this resource:
 		if (lockedResources.containsKey(locator)) {
 			for (ActiveLock lock : lockedResources.get(locator).values()) {
-				if (type.equals(lock.getType()) && scope.equals(lock.getScope())) {
+				if (type.equals(lock.getType()) && scope.equals(lock.getScope()) && (depth == 0 || lock.isDeep())) {
 					return lock;
 				}
 			}
 		}
 		// or otherwise look for parent locks:
 		if (locator.parent().isPresent()) {
-			return getLockInternal(type, scope, locator.parent().get());
+			return getLockInternal(type, scope, locator.parent().get(), depth++);
 		} else {
 			return null;
 		}

main/frontend-webdav/src/test/java/org/cryptomator/frontend/webdav/jackrabbitservlet/ExclusiveSharedLockManagerTest.java

@@ -0,0 +1,163 @@
+package org.cryptomator.frontend.webdav.jackrabbitservlet;
+
+import java.util.Optional;
+
+import org.apache.jackrabbit.webdav.DavException;
+import org.apache.jackrabbit.webdav.DavServletResponse;
+import org.apache.jackrabbit.webdav.lock.ActiveLock;
+import org.apache.jackrabbit.webdav.lock.LockInfo;
+import org.apache.jackrabbit.webdav.lock.Scope;
+import org.apache.jackrabbit.webdav.lock.Type;
+import org.cryptomator.filesystem.jackrabbit.FileLocator;
+import org.cryptomator.filesystem.jackrabbit.FolderLocator;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class ExclusiveSharedLockManagerTest {
+
+	private FolderLocator parentLocator;
+	private DavFolder parentResource;
+	private FileLocator childLocator;
+	private DavFile childResource;
+	private LockInfo lockInfo;
+	private ExclusiveSharedLockManager lockManager;
+
+	@Before
+	public void setup() {
+		parentLocator = Mockito.mock(FolderLocator.class);
+		Mockito.when(parentLocator.name()).thenReturn("parent");
+		Mockito.when(parentLocator.parent()).thenReturn(Optional.empty());
+
+		parentResource = Mockito.mock(DavFolder.class);
+		Mockito.when(parentResource.getLocator()).thenReturn(parentLocator);
+
+		childLocator = Mockito.mock(FileLocator.class);
+		Mockito.when(childLocator.name()).thenReturn("child");
+		Mockito.when(childLocator.parent()).thenReturn(Optional.of(parentLocator));
+		Mockito.when(parentLocator.isAncestorOf(childLocator)).thenReturn(true);
+
+		childResource = Mockito.mock(DavFile.class);
+		Mockito.when(childResource.getLocator()).thenReturn(childLocator);
+
+		lockInfo = Mockito.mock(LockInfo.class);
+		Mockito.when(lockInfo.getScope()).thenReturn(Scope.EXCLUSIVE);
+		Mockito.when(lockInfo.getType()).thenReturn(Type.WRITE);
+		Mockito.when(lockInfo.getOwner()).thenReturn("test");
+		Mockito.when(lockInfo.getTimeout()).thenReturn(3600l);
+		Mockito.when(lockInfo.isDeep()).thenReturn(true);
+
+		lockManager = new ExclusiveSharedLockManager();
+	}
+
+	@Test
+	public void testLockCreation() throws DavException {
+		ActiveLock lock = lockManager.createLock(lockInfo, parentResource);
+		Assert.assertEquals(Scope.EXCLUSIVE, lock.getScope());
+		Assert.assertEquals(Type.WRITE, lock.getType());
+		Assert.assertEquals("test", lock.getOwner());
+		Assert.assertFalse(lock.isExpired());
+		Assert.assertTrue(lock.isDeep());
+	}
+
+	@Test
+	public void testLockCreationInParent() throws DavException {
+		ActiveLock lock = lockManager.createLock(lockInfo, parentResource);
+
+		Assert.assertTrue(lockManager.hasLock(lock.getToken(), parentResource));
+		Assert.assertFalse(lockManager.hasLock(lock.getToken(), childResource));
+
+		ActiveLock parentLock = lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, parentResource);
+		ActiveLock childLock = lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, childResource);
+		Assert.assertEquals(lock, parentLock);
+		Assert.assertEquals(lock, childLock);
+	}
+
+	@Test
+	public void testLockCreationInChild() throws DavException {
+		ActiveLock lock = lockManager.createLock(lockInfo, childResource);
+
+		Assert.assertTrue(lockManager.hasLock(lock.getToken(), childResource));
+		Assert.assertFalse(lockManager.hasLock(lock.getToken(), parentResource));
+
+		ActiveLock parentLock = lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, parentResource);
+		ActiveLock childLock = lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, childResource);
+		Assert.assertNull(parentLock);
+		Assert.assertEquals(lock, childLock);
+	}
+
+	@Test
+	public void testMultipleSharedLocks() throws DavException {
+		Mockito.when(lockInfo.getScope()).thenReturn(Scope.SHARED);
+
+		ActiveLock lock1 = lockManager.createLock(lockInfo, parentResource);
+		ActiveLock lock2 = lockManager.createLock(lockInfo, childResource);
+
+		Assert.assertTrue(lockManager.hasLock(lock1.getToken(), parentResource));
+		Assert.assertTrue(lockManager.hasLock(lock2.getToken(), childResource));
+
+		Assert.assertNotEquals(lock1, lock2);
+	}
+
+	@Test
+	public void testReleaseLock() throws DavException {
+		ActiveLock lock = lockManager.createLock(lockInfo, parentResource);
+		Assert.assertNotNull(lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, parentResource));
+
+		lockManager.releaseLock(lock.getToken(), parentResource);
+		Assert.assertNull(lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, parentResource));
+	}
+
+	@Test
+	public void testReleaseNonExistingLock() throws DavException {
+		lockManager.releaseLock("doesn't exist", parentResource);
+		Assert.assertNull(lockManager.getLock(Type.WRITE, Scope.EXCLUSIVE, parentResource));
+	}
+
+	@Test
+	public void testRefreshLock() throws DavException {
+		ActiveLock originalLock = lockManager.createLock(lockInfo, parentResource);
+		long originalTimeout = originalLock.getTimeout();
+
+		Mockito.when(lockInfo.getTimeout()).thenReturn(7200l);
+		ActiveLock updatedLock = lockManager.refreshLock(lockInfo, originalLock.getToken(), parentResource);
+		long updatedTimeout = updatedLock.getTimeout();
+
+		Assert.assertTrue(updatedTimeout > originalTimeout);
+	}
+
+	@Test(expected = DavException.class)
+	public void testRefreshNonExistingLock() throws DavException {
+		lockManager.refreshLock(lockInfo, "doesn't exist", parentResource);
+	}
+
+	@Test(expected = DavException.class)
+	public void testRefreshLockWithInvalidToken() throws DavException {
+		lockManager.createLock(lockInfo, parentResource);
+		lockManager.refreshLock(lockInfo, "doesn't exist", parentResource);
+	}
+
+	@Test
+	public void testLockCreationWhenParentAlreadyLocked() throws DavException {
+		lockManager.createLock(lockInfo, parentResource);
+		try {
+			lockManager.createLock(lockInfo, childResource);
+			Assert.fail("Should have thrown excpetion");
+		} catch (DavException e) {
+			Assert.assertEquals(DavServletResponse.SC_LOCKED, e.getErrorCode());
+		}
+	}
+
+	@Test
+	public void testLockCreationWhenChildAlreadyLocked() throws DavException {
+		lockManager.createLock(lockInfo, childResource);
+		try {
+			lockManager.createLock(lockInfo, parentResource);
+			Assert.fail("Should have thrown excpetion");
+		} catch (DavException e) {
+			Assert.assertEquals(DavServletResponse.SC_CONFLICT, e.getErrorCode());
+		}
+	}
+
+}