Add tests for create dir/file/link that already exists

Fix implementations to work as expected
(that createDirectory/File/Link fails if an entry with that name
already exists).

8 files changed, 91 insertions, 4 deletions

diff --git a/libs/documents/src/androidTest/java/org/the_jk/cleversync/documents/DocumentTreeAndroidTest.kt b/libs/documents/src/androidTest/java/org/the_jk/cleversync/documents/DocumentTreeAndroidTest.kt
index 8510ef1..c9e0782 100644
--- a/libs/documents/src/androidTest/java/org/the_jk/cleversync/documents/DocumentTreeAndroidTest.kt
+++ b/libs/documents/src/androidTest/java/org/the_jk/cleversync/documents/DocumentTreeAndroidTest.kt
@@ -68,6 +68,11 @@ class DocumentTreeAndroidTest : TreeAbstractTest() {
         runTest { super.createDirectory() }
     }
 
+    @Test
+    override fun createDirectoryAlreadyExists() {
+        runTest { super.createDirectoryAlreadyExists() }
+    }
+
     @Test(timeout = 30000)
     override fun observeCreateDirectory() {
         runTest { super.observeCreateDirectory() }
@@ -79,6 +84,11 @@ class DocumentTreeAndroidTest : TreeAbstractTest() {
     }
 
     @Test
+    override fun createFileAlreadyExists() {
+        runTest { super.createFileAlreadyExists() }
+    }
+
+    @Test
     override fun overwriteFile() {
         runTest { super.overwriteFile() }
     }
diff --git a/libs/documents/src/main/java/org/the_jk/cleversync/io/documents/DocumentDirectory.kt b/libs/documents/src/main/java/org/the_jk/cleversync/io/documents/DocumentDirectory.kt
index 258d5b6..e8b94aa 100644
--- a/libs/documents/src/main/java/org/the_jk/cleversync/io/documents/DocumentDirectory.kt
+++ b/libs/documents/src/main/java/org/the_jk/cleversync/io/documents/DocumentDirectory.kt
@@ -17,6 +17,7 @@ import org.the_jk.cleversync.io.ModifiableDirectory
 import org.the_jk.cleversync.io.ModifiableFile
 import org.the_jk.cleversync.io.ModifiableLink
 import java.io.IOException
+import java.nio.file.FileAlreadyExistsException
 import kotlin.time.Duration
 
 internal open class DocumentDirectory(
@@ -103,6 +104,7 @@ internal open class DocumentDirectory(
 
     override fun createDirectory(name: String): ModifiableDirectory {
         if (!metadata.supportsCreate()) throw IOException("Directory doesn't support creation")
+        if (findChild(name) != null) throw FileAlreadyExistsException(name)
         val documentUri = DocumentsContract.createDocument(
             contentResolver,
             DocumentsContract.buildDocumentUriUsingTree(treeUri, metadata.documentId),
@@ -118,6 +120,7 @@ internal open class DocumentDirectory(
 
     override fun createFile(name: String): ModifiableFile {
         if (!metadata.supportsCreate()) throw IOException("Directory doesn't support creation")
+        if (findChild(name) != null) throw FileAlreadyExistsException(name)
         // TODO: Handle .tar.gz and such? Is it worth it (will android find them anyway)
         val dotIndex = name.lastIndexOf('.')
         val mimeType = if (dotIndex > 0) {
@@ -284,9 +287,16 @@ internal open class DocumentDirectory(
         val queryArgs = Bundle()
         queryArgs.putString(DocumentsContract.QUERY_ARG_DISPLAY_NAME, displayName)
         return contentResolver.getAllMetadata(
-            DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, metadata.documentId),
+            DocumentsContract.buildChildDocumentsUriUsingTree(
+                treeUri,
+                metadata.documentId
+            ),
             queryArgs = queryArgs,
-        ).singleOrNull()
+        ).singleOrNull {
+            // Not all document providers use QUERY_ARG_DISPLAY_NAME and the ones
+            // that do can still return partial matches and case insensitive matches
+            it.displayName == displayName
+        }
     }
 
     private companion object {
diff --git a/libs/local/src/main/java/org/the_jk/cleversync/io/local/PathDirectory.kt b/libs/local/src/main/java/org/the_jk/cleversync/io/local/PathDirectory.kt
index 9899f02..516d5ed 100644
--- a/libs/local/src/main/java/org/the_jk/cleversync/io/local/PathDirectory.kt
+++ b/libs/local/src/main/java/org/the_jk/cleversync/io/local/PathDirectory.kt
@@ -9,6 +9,7 @@ import org.the_jk.cleversync.io.File
 import org.the_jk.cleversync.io.ModifiableDirectory
 import org.the_jk.cleversync.io.ModifiableFile
 import org.the_jk.cleversync.io.ModifiableLink
+import java.nio.file.FileAlreadyExistsException
 import java.nio.file.LinkOption
 import java.nio.file.NoSuchFileException
 import java.nio.file.Path
@@ -17,6 +18,7 @@ import kotlin.io.path.createDirectory
 import kotlin.io.path.createSymbolicLinkPointingTo
 import kotlin.io.path.deleteIfExists
 import kotlin.io.path.deleteRecursively
+import kotlin.io.path.exists
 import kotlin.io.path.isDirectory
 import kotlin.io.path.isRegularFile
 import kotlin.io.path.isSymbolicLink
@@ -82,6 +84,7 @@ internal open class PathDirectory(
 
     override fun createFile(name: String): ModifiableFile {
         val path = path.resolve(name)
+        if (path.exists(options = arrayOf(LinkOption.NOFOLLOW_LINKS))) throw FileAlreadyExistsException(name)
         return PathFile(path)
     }
 
diff --git a/libs/samba/src/main/java/org/the_jk/cleversync/io/samba/SambaDirectory.kt b/libs/samba/src/main/java/org/the_jk/cleversync/io/samba/SambaDirectory.kt
index 4865d7f..7ca4732 100644
--- a/libs/samba/src/main/java/org/the_jk/cleversync/io/samba/SambaDirectory.kt
+++ b/libs/samba/src/main/java/org/the_jk/cleversync/io/samba/SambaDirectory.kt
@@ -12,6 +12,7 @@ import org.the_jk.cleversync.io.ModifiableDirectory
 import org.the_jk.cleversync.io.ModifiableFile
 import org.the_jk.cleversync.io.ModifiableLink
 import java.io.IOException
+import java.nio.file.FileAlreadyExistsException
 import java.time.Instant
 import kotlin.time.Duration.Companion.seconds
 
@@ -105,6 +106,7 @@ internal open class SambaDirectory(
 
     override fun createFile(name: String): ModifiableFile {
         val newPath = PathUtils.join(path, name)
+        if (conn.entry(newPath) != null) throw FileAlreadyExistsException(name)
         return SambaFile(conn, newPath, name, 0UL, Instant.EPOCH, Instant.EPOCH)
     }
 
diff --git a/libs/sftp/src/main/cpp/sftp.cpp b/libs/sftp/src/main/cpp/sftp.cpp
index 0feafb1..1bc4fcb 100644
--- a/libs/sftp/src/main/cpp/sftp.cpp
+++ b/libs/sftp/src/main/cpp/sftp.cpp
@@ -218,8 +218,6 @@ class SftpSession {
   }
 
   bool Symlink(const std::string &target, const std::string &path) {
-    // symlink fails is path already exists, so remove any existing entry first.
-    libssh2_sftp_unlink_ex(sftp_->get(), path.data(), path.size());
     // The argument order does not seem to match the documentation, so this
     // might change?
     return libssh2_sftp_symlink_ex(sftp_->get(), target.data(), target.size(),
diff --git a/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpDirectory.kt b/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpDirectory.kt
index 90a3127..f1ae513 100644
--- a/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpDirectory.kt
+++ b/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpDirectory.kt
@@ -12,6 +12,7 @@ import org.the_jk.cleversync.io.ModifiableDirectory
 import org.the_jk.cleversync.io.ModifiableFile
 import org.the_jk.cleversync.io.ModifiableLink
 import java.io.IOException
+import java.nio.file.FileAlreadyExistsException
 import java.time.Instant
 import kotlin.time.Duration.Companion.seconds
 
@@ -111,6 +112,8 @@ internal open class SftpDirectory(
 
     override fun createFile(name: String): ModifiableFile {
         val newPath = PathUtils.join(path, name)
+        val entry = conn.entry(newPath, followLink = false)
+        if (entry != null) throw FileAlreadyExistsException(name)
         return SftpFile(conn, newPath, name, 0UL, Instant.EPOCH, Instant.EPOCH)
     }
 
diff --git a/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpLink.kt b/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpLink.kt
index a922e26..0766f1a 100644
--- a/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpLink.kt
+++ b/libs/sftp/src/main/java/org/the_jk/cleversync/io/sftp/SftpLink.kt
@@ -40,6 +40,8 @@ internal class SftpLink(
     }
 
     private fun target(name: String, rawTarget: Boolean) {
+        // conn.symlink fails if path already exists
+        conn.unlink(path)
         if (!conn.symlink(name, rawTarget, path)) throw IOException(conn.error)
     }
 
diff --git a/libs/test-utils/src/main/java/org/the_jk/cleversync/TreeAbstractTest.kt b/libs/test-utils/src/main/java/org/the_jk/cleversync/TreeAbstractTest.kt
index 2555195..84f4ed6 100644
--- a/libs/test-utils/src/main/java/org/the_jk/cleversync/TreeAbstractTest.kt
+++ b/libs/test-utils/src/main/java/org/the_jk/cleversync/TreeAbstractTest.kt
@@ -8,6 +8,7 @@ import org.the_jk.cleversync.io.Directory
 import org.the_jk.cleversync.io.Link
 import org.the_jk.cleversync.io.ModifiableLink
 import org.the_jk.cleversync.io.ModifiableTree
+import java.io.IOException
 
 abstract class TreeAbstractTest {
     protected lateinit var tree: ModifiableTree
@@ -45,6 +46,26 @@ abstract class TreeAbstractTest {
         assertThat(content.links).isEmpty()
     }
 
+    @Test
+    open fun createDirectoryAlreadyExists() {
+        tree.createDirectory("foo")
+        tree.createFile("bar").write().use { it.write(0) }
+        if (supportSymlinks()) {
+            tree.createLink("fum", "does-not-exist")
+        }
+        Assert.assertThrows(IOException::class.java) {
+            tree.createDirectory("foo")
+        }
+        Assert.assertThrows(IOException::class.java) {
+            tree.createDirectory("bar")
+        }
+        if (supportSymlinks()) {
+            Assert.assertThrows(IOException::class.java) {
+                tree.createDirectory("fum")
+            }
+        }
+    }
+
     @Test(timeout = 10000)
     open fun observeCreateDirectory() {
         val content = tree.liveList()
@@ -80,6 +101,26 @@ abstract class TreeAbstractTest {
     }
 
     @Test
+    open fun createFileAlreadyExists() {
+        tree.createDirectory("foo")
+        tree.createFile("bar").write().use { it.write(0) }
+        if (supportSymlinks()) {
+            tree.createLink("fum", "does-not-exist")
+        }
+        Assert.assertThrows(IOException::class.java) {
+            tree.createFile("foo")
+        }
+        Assert.assertThrows(IOException::class.java) {
+            tree.createFile("bar")
+        }
+        if (supportSymlinks()) {
+            Assert.assertThrows(IOException::class.java) {
+                tree.createFile("fum")
+            }
+        }
+    }
+
+    @Test
     open fun overwriteFile() {
         val foo = tree.createFile("foo")
         foo.write().use { os ->
@@ -159,6 +200,24 @@ abstract class TreeAbstractTest {
     }
 
     @Test
+    open fun createLinkAlreadyExists() {
+        Assume.assumeTrue(supportSymlinks())
+
+        tree.createDirectory("foo")
+        tree.createFile("bar").write().use { it.write(0) }
+        tree.createLink("fum", "does-not-exist")
+        Assert.assertThrows(IOException::class.java) {
+            tree.createLink("foo", "test")
+        }
+        Assert.assertThrows(IOException::class.java) {
+            tree.createLink("bar", "test")
+        }
+        Assert.assertThrows(IOException::class.java) {
+            tree.createLink("fum", "test")
+        }
+    }
+
+    @Test
     open fun createLinkSubdir() {
         Assume.assumeTrue(supportSymlinks())