Remove old Backend.save() method and hacky WebDAV pipe code with it

grote · Dec 18, 2024 · 902c3a2 · 902c3a2
1 parent 3fbe3cf
commit 902c3a2
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 131 deletions.
diff --git a/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt b/app/src/androidTest/java/com/stevesoltys/seedvault/PluginTest.kt
@@ -17,6 +17,7 @@ import io.mockk.every
 import io.mockk.mockk
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.runBlocking
+import org.calyxos.seedvault.core.backends.BackendSaver
 import org.calyxos.seedvault.core.backends.LegacyAppBackupFile
 import org.calyxos.seedvault.core.backends.saf.SafBackend
 import org.junit.After
@@ -28,6 +29,7 @@ import org.junit.Test
 import org.junit.runner.RunWith
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
+import java.io.OutputStream
 
 @RunWith(AndroidJUnit4::class)
 @MediumTest
@@ -41,7 +43,7 @@ class PluginTest : KoinComponent {
         safStorage = settingsManager.getSafProperties() ?: error("No SAF storage"),
     )
 
-    private val backend = SafBackend(context, storage.safStorage)
+    private val backend = SafBackend(context, storage.safStorage, ".SeedvaultPluginTest")
 
     @Suppress("Deprecation")
     private val legacyStoragePlugin: LegacyStoragePlugin = DocumentsProviderLegacyPlugin(context) {
@@ -97,20 +99,17 @@ class PluginTest : KoinComponent {
         every { mockedSettingsManager.token } returnsMany listOf(token, token + 1, token + 1)
 
         // write metadata (needed for backup to be recognized)
-        backend.save(LegacyAppBackupFile.Metadata(token))
-            .writeAndClose(getRandomByteArray())
+        backend.save(LegacyAppBackupFile.Metadata(token), getSaver(getRandomByteArray()))
 
         // one backup available now
         assertEquals(1, backend.getAvailableBackupFileHandles().toList().size)
 
         // initializing again (with another restore set) does add a restore set
-        backend.save(LegacyAppBackupFile.Metadata(token + 1))
-            .writeAndClose(getRandomByteArray())
+        backend.save(LegacyAppBackupFile.Metadata(token + 1), getSaver(getRandomByteArray()))
         assertEquals(2, backend.getAvailableBackupFileHandles().toList().size)
 
         // initializing again (without new restore set) doesn't change number of restore sets
-        backend.save(LegacyAppBackupFile.Metadata(token + 1))
-            .writeAndClose(getRandomByteArray())
+        backend.save(LegacyAppBackupFile.Metadata(token + 1), getSaver(getRandomByteArray()))
         assertEquals(2, backend.getAvailableBackupFileHandles().toList().size)
     }
 
@@ -120,7 +119,7 @@ class PluginTest : KoinComponent {
 
         // write metadata
         val metadata = getRandomByteArray()
-        backend.save(LegacyAppBackupFile.Metadata(token)).writeAndClose(metadata)
+        backend.save(LegacyAppBackupFile.Metadata(token), getSaver(metadata))
 
         // get available backups, expect only one with our token and no error
         var availableBackups = backend.getAvailableBackupFileHandles().toList()
@@ -132,7 +131,7 @@ class PluginTest : KoinComponent {
         assertReadEquals(metadata, backend.load(backupHandle))
 
         // initializing again (without changing storage) keeps restore set with same token
-        backend.save(LegacyAppBackupFile.Metadata(token)).writeAndClose(metadata)
+        backend.save(LegacyAppBackupFile.Metadata(token), getSaver(metadata))
         availableBackups = backend.getAvailableBackupFileHandles().toList()
         assertEquals(1, availableBackups.size)
         backupHandle = availableBackups[0] as LegacyAppBackupFile.Metadata
@@ -149,8 +148,8 @@ class PluginTest : KoinComponent {
 
         // write random bytes as APK
         val apk1 = getRandomByteArray(1337 * 1024)
-        backend.save(LegacyAppBackupFile.Blob(token, "${packageInfo.packageName}.apk"))
-            .writeAndClose(apk1)
+        backend.save(LegacyAppBackupFile.Blob(token, "${packageInfo.packageName}.apk"),
+            getSaver(apk1))
 
         // assert that read APK bytes match what was written
         assertReadEquals(
@@ -162,8 +161,8 @@ class PluginTest : KoinComponent {
         val suffix2 = getRandomBase64(23)
         val apk2 = getRandomByteArray(23 * 1024 * 1024)
 
-        backend.save(LegacyAppBackupFile.Blob(token, "${packageInfo2.packageName}$suffix2.apk"))
-            .writeAndClose(apk2)
+        backend.save(LegacyAppBackupFile.Blob(token, "${packageInfo2.packageName}$suffix2.apk"),
+            getSaver(apk2))
 
         // assert that read APK bytes match what was written
         assertReadEquals(
@@ -182,14 +181,14 @@ class PluginTest : KoinComponent {
 
         // write full backup data
         val data = getRandomByteArray(5 * 1024 * 1024)
-        backend.save(LegacyAppBackupFile.Blob(token, name1)).writeAndClose(data)
+        backend.save(LegacyAppBackupFile.Blob(token, name1), getSaver(data))
 
         // restore data matches backed up data
         assertReadEquals(data, backend.load(LegacyAppBackupFile.Blob(token, name1)))
 
         // write and check data for second package
         val data2 = getRandomByteArray(5 * 1024 * 1024)
-        backend.save(LegacyAppBackupFile.Blob(token, name2)).writeAndClose(data2)
+        backend.save(LegacyAppBackupFile.Blob(token, name2), getSaver(data2))
         assertReadEquals(data2, backend.load(LegacyAppBackupFile.Blob(token, name2)))
 
         // remove data of first package again and ensure that no more data is found
@@ -203,4 +202,14 @@ class PluginTest : KoinComponent {
         every { mockedSettingsManager.token } returns token
     }
 
+    private fun getSaver(bytes: ByteArray) = object : BackendSaver {
+        override val size: Long = bytes.size.toLong()
+        override val sha256: String? = null
+
+        override fun save(outputStream: OutputStream): Long {
+            outputStream.write(bytes)
+            return size
+        }
+    }
+
 }
diff --git a/core/src/main/java/org/calyxos/seedvault/core/backends/Backend.kt b/core/src/main/java/org/calyxos/seedvault/core/backends/Backend.kt
@@ -7,7 +7,6 @@ package org.calyxos.seedvault.core.backends
 
 import androidx.annotation.VisibleForTesting
 import java.io.InputStream
-import java.io.OutputStream
 import kotlin.reflect.KClass
 
 public interface Backend {
@@ -25,9 +24,6 @@ public interface Backend {
      */
     public suspend fun getFreeSpace(): Long?
 
-    @Deprecated(message = "use save(FileHandle, BackendSaver) instead")
-    public suspend fun save(handle: FileHandle): OutputStream
-
     public suspend fun save(handle: FileHandle, saver: BackendSaver): Long
 
     public suspend fun load(handle: FileHandle): InputStream

diff --git a/core/src/main/java/org/calyxos/seedvault/core/backends/saf/SafBackend.kt b/core/src/main/java/org/calyxos/seedvault/core/backends/saf/SafBackend.kt
@@ -37,7 +37,6 @@ import org.calyxos.seedvault.core.backends.LegacyAppBackupFile
 import org.calyxos.seedvault.core.backends.TopLevelFolder
 import java.io.IOException
 import java.io.InputStream
-import java.io.OutputStream
 import kotlin.reflect.KClass
 
 internal const val AUTHORITY_STORAGE = "com.android.externalstorage.documents"
@@ -87,13 +86,6 @@ public class SafBackend(
         } else bytesAvailable
     }
 
-    @Deprecated("use save(FileHandle, BackendSaver) instead")
-    override suspend fun save(handle: FileHandle): OutputStream {
-        log.debugLog { "save($handle)" }
-        val file = cache.getOrCreateFile(handle)
-        return file.getOutputStream(context.contentResolver)
-    }
-
     override suspend fun save(handle: FileHandle, saver: BackendSaver): Long {
         log.debugLog { "save($handle)" }
         val file = cache.getOrCreateFile(handle)

diff --git a/.../src/main/java/org/calyxos/seedvault/core/backends/webdav/PipedCloseActionOutputStream.kt b/.../src/main/java/org/calyxos/seedvault/core/backends/webdav/PipedCloseActionOutputStream.kt
diff --git a/core/src/main/java/org/calyxos/seedvault/core/backends/webdav/WebDavBackend.kt b/core/src/main/java/org/calyxos/seedvault/core/backends/webdav/WebDavBackend.kt
@@ -13,11 +13,6 @@ import at.bitfire.dav4jvm.exception.NotFoundException
 import at.bitfire.dav4jvm.property.webdav.QuotaAvailableBytes
 import io.github.oshai.kotlinlogging.KLogger
 import io.github.oshai.kotlinlogging.KotlinLogging
-import kotlinx.coroutines.DelicateCoroutinesApi
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.GlobalScope
-import kotlinx.coroutines.async
-import kotlinx.coroutines.runBlocking
 import okhttp3.ConnectionSpec
 import okhttp3.HttpUrl
 import okhttp3.HttpUrl.Companion.toHttpUrl
@@ -45,16 +40,13 @@ import org.calyxos.seedvault.core.backends.LegacyAppBackupFile
 import org.calyxos.seedvault.core.backends.TopLevelFolder
 import java.io.IOException
 import java.io.InputStream
-import java.io.OutputStream
-import java.io.PipedInputStream
 import java.util.concurrent.TimeUnit
 import kotlin.coroutines.resume
 import kotlin.coroutines.suspendCoroutine
 import kotlin.reflect.KClass
 
 private const val DEBUG_LOG = true
 
-@OptIn(DelicateCoroutinesApi::class)
 public class WebDavBackend(
     webDavConfig: WebDavConfig,
     root: String = DIRECTORY_ROOT,
@@ -122,39 +114,6 @@ public class WebDavBackend(
         return availableBytes
     }
 
-    @Deprecated("use save(FileHandle, BackendSaver) instead")
-    override suspend fun save(handle: FileHandle): OutputStream {
-        val location = handle.toHttpUrl()
-        val davCollection = DavCollection(okHttpClient, location)
-        davCollection.ensureFoldersExist(log, folders)
-
-        val pipedInputStream = PipedInputStream()
-        val pipedOutputStream = PipedCloseActionOutputStream(pipedInputStream)
-
-        val body = object : RequestBody() {
-            override fun isOneShot(): Boolean = true
-            override fun contentType() = "application/octet-stream".toMediaType()
-            override fun writeTo(sink: BufferedSink) {
-                pipedInputStream.use { inputStream ->
-                    sink.outputStream().use { outputStream ->
-                        inputStream.copyTo(outputStream)
-                    }
-                }
-            }
-        }
-        val deferred = GlobalScope.async(Dispatchers.IO) {
-            davCollection.put(body) { response ->
-                log.debugLog { "save($location) = $response" }
-            }
-        }
-        pipedOutputStream.doOnClose {
-            runBlocking { // blocking i/o wait
-                deferred.await()
-            }
-        }
-        return pipedOutputStream
-    }
-
     override suspend fun save(handle: FileHandle, saver: BackendSaver): Long {
         val location = handle.toHttpUrl()
         val davCollection = DavCollection(okHttpClient, location)

diff --git a/storage/demo/src/main/java/de/grobox/storagebackuptester/plugin/TestSafBackend.kt b/storage/demo/src/main/java/de/grobox/storagebackuptester/plugin/TestSafBackend.kt
@@ -43,11 +43,6 @@ class TestSafBackend(
 
     override suspend fun getFreeSpace(): Long? = delegate.getFreeSpace()
 
-    override suspend fun save(handle: FileHandle): OutputStream {
-        if (getLocationUri() == null) return nullStream
-        return delegate.save(handle)
-    }
-
     override suspend fun save(handle: FileHandle, saver: BackendSaver): Long {
         if (getLocationUri() == null) return 0
         return delegate.save(handle, saver)