diff --git a/app/src/main/java/com/stevesoltys/seedvault/backend/BackendExt.kt b/app/src/main/java/com/stevesoltys/seedvault/backend/BackendExt.kt index 0387bc7c..f5f6fc21 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/backend/BackendExt.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/backend/BackendExt.kt @@ -11,11 +11,6 @@ import org.calyxos.seedvault.core.backends.Backend import org.calyxos.seedvault.core.backends.FileHandle import org.calyxos.seedvault.core.backends.LegacyAppBackupFile import java.io.IOException -import java.io.OutputStream - -suspend fun Backend.getMetadataOutputStream(token: Long): OutputStream { - return save(LegacyAppBackupFile.Metadata(token)) -} suspend fun Backend.getAvailableBackupFileHandles(): List { // v1 get all restore set tokens in root folder that have a metadata file diff --git a/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt b/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt index ec8a4469..ca5dcdfb 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/metadata/MetadataManager.kt @@ -100,7 +100,6 @@ internal class MetadataManager( * Call this after a package's APK has been backed up successfully. * * It updates the packages' metadata to the internal cache. - * You still need to call [uploadMetadata] to persist all local modifications. */ @Synchronized @Throws(IOException::class) @@ -188,11 +187,10 @@ internal class MetadataManager( internal fun onPackageBackupError( packageInfo: PackageInfo, packageState: PackageState, - metadataOutputStream: OutputStream, backupType: BackupType? = null, ) { check(packageState != APK_AND_DATA) { "Backup Error with non-error package state." } - modifyMetadata(metadataOutputStream) { + modifyCachedMetadata { metadata.packageMetadataMap.getOrPut(packageInfo.packageName) { val isSystemApp = packageInfo.isSystemApp() PackageMetadata( @@ -212,7 +210,6 @@ internal class MetadataManager( * Call this for all packages we can not back up for some reason. * * It updates the packages' local metadata. - * You still need to call [uploadMetadata] to persist all local modifications. */ @Synchronized @Throws(IOException::class) @@ -239,15 +236,6 @@ internal class MetadataManager( } } - /** - * Uploads metadata to given [metadataOutputStream] after performing local modifications. - */ - @Synchronized - @Throws(IOException::class) - fun uploadMetadata(metadataOutputStream: OutputStream) { - metadataWriter.write(metadata, metadataOutputStream) - } - @Throws(IOException::class) private fun modifyCachedMetadata(modFun: () -> Unit) { val oldMetadata = metadata.copy( // copy map, otherwise it will re-use same reference @@ -262,24 +250,7 @@ internal class MetadataManager( metadata = oldMetadata throw IOException(e) } - } - - @Throws(IOException::class) - private fun modifyMetadata(metadataOutputStream: OutputStream, modFun: () -> Unit) { - val oldMetadata = metadata.copy( // copy map, otherwise it will re-use same reference - packageMetadataMap = PackageMetadataMap(metadata.packageMetadataMap), - ) - try { - modFun.invoke() - metadataWriter.write(metadata, metadataOutputStream) - writeMetadataToCache() - } catch (e: IOException) { - Log.w(TAG, "Error writing metadata to storage", e) - // revert metadata and do not write it to cache - metadata = oldMetadata - throw IOException(e) - } - mLastBackupTime.postValue(metadata.time) + mLastBackupTime.postValue(metadata.time) // TODO only do after snapshot was written } /** diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index d9e7733b..b5a6d5e6 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -23,7 +23,6 @@ import android.util.Log import androidx.annotation.WorkerThread import com.stevesoltys.seedvault.Clock import com.stevesoltys.seedvault.backend.BackendManager -import com.stevesoltys.seedvault.backend.getMetadataOutputStream import com.stevesoltys.seedvault.backend.isOutOfSpace import com.stevesoltys.seedvault.metadata.BackupType import com.stevesoltys.seedvault.metadata.MetadataManager @@ -386,13 +385,10 @@ internal class BackupCoordinator( } // TODO is this only nice to have info, or do we need to do more? - private suspend fun onPackageBackupError(packageInfo: PackageInfo, type: BackupType) { + private fun onPackageBackupError(packageInfo: PackageInfo, type: BackupType) { val packageName = packageInfo.packageName try { - val token = settingsManager.getToken() ?: error("no token") - backend.getMetadataOutputStream(token).use { - metadataManager.onPackageBackupError(packageInfo, state.cancelReason, it, type) - } + metadataManager.onPackageBackupError(packageInfo, state.cancelReason, type) } catch (e: IOException) { Log.e(TAG, "Error while writing metadata for $packageName", e) } diff --git a/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt b/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt index cc36df37..0ebeb6a1 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/metadata/MetadataManagerTest.kt @@ -473,7 +473,7 @@ class MetadataManagerTest { expectReadFromCache() expectModifyMetadata(updatedMetadata) - manager.onPackageBackupError(packageInfo, NO_DATA, storageOutputStream, BackupType.KV) + manager.onPackageBackupError(packageInfo, NO_DATA, BackupType.KV) } @Test @@ -486,22 +486,7 @@ class MetadataManagerTest { expectReadFromCache() expectModifyMetadata(updatedMetadata) - manager.onPackageBackupError(packageInfo, WAS_STOPPED, storageOutputStream) - } - - @Test - fun `test uploadMetadata() uploads`() { - expectReadFromCache() - every { metadataWriter.write(initialMetadata, storageOutputStream) } just Runs - - manager.uploadMetadata(storageOutputStream) - } - - @Test - fun `test getBackupToken() on first run`() { - every { context.openFileInput(METADATA_CACHE_FILE) } throws FileNotFoundException() - - assertEquals(0L, manager.getBackupToken()) + manager.onPackageBackupError(packageInfo, WAS_STOPPED) } @Test diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt index cf091242..a0b53a04 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt @@ -32,12 +32,10 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.runBlocking import org.calyxos.seedvault.core.backends.Backend -import org.calyxos.seedvault.core.backends.LegacyAppBackupFile import org.calyxos.seedvault.core.backends.saf.SafProperties import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test import java.io.IOException -import java.io.OutputStream import kotlin.random.Random internal class BackupCoordinatorTest : BackupTest() { @@ -64,7 +62,6 @@ internal class BackupCoordinatorTest : BackupTest() { ) private val backend = mockk() - private val metadataOutputStream = mockk() private val fileDescriptor: ParcelFileDescriptor = mockk() private val packageMetadata: PackageMetadata = mockk() private val safProperties = SafProperties( @@ -211,12 +208,9 @@ internal class BackupCoordinatorTest : BackupTest() { metadataManager.onPackageBackupError( packageInfo, UNKNOWN_ERROR, - metadataOutputStream, BackupType.KV, ) } just Runs - coEvery { backend.save(LegacyAppBackupFile.Metadata(token)) } returns metadataOutputStream - every { metadataOutputStream.close() } just Runs assertEquals(TRANSPORT_PACKAGE_REJECTED, backup.finishBackup()) } @@ -270,14 +264,12 @@ internal class BackupCoordinatorTest : BackupTest() { metadataManager.onPackageBackupError( packageInfo, QUOTA_EXCEEDED, - metadataOutputStream, BackupType.FULL ) } just Runs coEvery { full.cancelFullBackup() } just Runs every { backendManager.backendProperties } returns safProperties every { settingsManager.useMeteredNetwork } returns false - every { metadataOutputStream.close() } just Runs assertEquals( TRANSPORT_OK, @@ -298,11 +290,9 @@ internal class BackupCoordinatorTest : BackupTest() { metadataManager.onPackageBackupError( packageInfo, QUOTA_EXCEEDED, - metadataOutputStream, BackupType.FULL ) } - verify { metadataOutputStream.close() } } @Test @@ -318,14 +308,12 @@ internal class BackupCoordinatorTest : BackupTest() { metadataManager.onPackageBackupError( packageInfo, NO_DATA, - metadataOutputStream, BackupType.FULL ) } just Runs coEvery { full.cancelFullBackup() } just Runs every { backendManager.backendProperties } returns safProperties every { settingsManager.useMeteredNetwork } returns false - every { metadataOutputStream.close() } just Runs assertEquals( TRANSPORT_OK, @@ -343,11 +331,9 @@ internal class BackupCoordinatorTest : BackupTest() { metadataManager.onPackageBackupError( packageInfo, NO_DATA, - metadataOutputStream, BackupType.FULL ) } - verify { metadataOutputStream.close() } } @Test @@ -357,7 +343,6 @@ internal class BackupCoordinatorTest : BackupTest() { private fun expectApkBackupAndMetadataWrite() { coEvery { apkBackup.backupApkIfNecessary(packageInfo) } just Runs every { settingsManager.getToken() } returns token - coEvery { backend.save(LegacyAppBackupFile.Metadata(token)) } returns metadataOutputStream every { metadataManager.onApkBackedUp(any(), packageMetadata) } just Runs }