Remove hasData() method from StoragePlugin
because it is one extra request for packages that do have data and from the looks of it not really needed.
This commit is contained in:
parent
27eb95f768
commit
099e0ba6d5
8 changed files with 29 additions and 131 deletions
|
@ -199,37 +199,23 @@ class PluginTest : KoinComponent {
|
||||||
val name1 = getRandomBase64()
|
val name1 = getRandomBase64()
|
||||||
val name2 = getRandomBase64()
|
val name2 = getRandomBase64()
|
||||||
|
|
||||||
// no data available initially
|
|
||||||
assertFalse(storagePlugin.hasData(token, name1))
|
|
||||||
assertFalse(storagePlugin.hasData(token, name2))
|
|
||||||
|
|
||||||
// write full backup data
|
// write full backup data
|
||||||
val data = getRandomByteArray(5 * 1024 * 1024)
|
val data = getRandomByteArray(5 * 1024 * 1024)
|
||||||
storagePlugin.getOutputStream(token, name1).writeAndClose(data)
|
storagePlugin.getOutputStream(token, name1).writeAndClose(data)
|
||||||
|
|
||||||
// data is available now, but only this token
|
|
||||||
assertTrue(storagePlugin.hasData(token, name1))
|
|
||||||
assertFalse(storagePlugin.hasData(token + 1, name1))
|
|
||||||
|
|
||||||
// restore data matches backed up data
|
// restore data matches backed up data
|
||||||
assertReadEquals(data, storagePlugin.getInputStream(token, name1))
|
assertReadEquals(data, storagePlugin.getInputStream(token, name1))
|
||||||
|
|
||||||
// write and check data for second package
|
// write and check data for second package
|
||||||
val data2 = getRandomByteArray(5 * 1024 * 1024)
|
val data2 = getRandomByteArray(5 * 1024 * 1024)
|
||||||
storagePlugin.getOutputStream(token, name2).writeAndClose(data2)
|
storagePlugin.getOutputStream(token, name2).writeAndClose(data2)
|
||||||
assertTrue(storagePlugin.hasData(token, name2))
|
|
||||||
assertReadEquals(data2, storagePlugin.getInputStream(token, name2))
|
assertReadEquals(data2, storagePlugin.getInputStream(token, name2))
|
||||||
|
|
||||||
// remove data of first package again and ensure that no more data is found
|
// remove data of first package again and ensure that no more data is found
|
||||||
storagePlugin.removeData(token, name1)
|
storagePlugin.removeData(token, name1)
|
||||||
assertFalse(storagePlugin.hasData(token, name1))
|
|
||||||
|
|
||||||
// second package is still there
|
|
||||||
assertTrue(storagePlugin.hasData(token, name2))
|
|
||||||
|
|
||||||
// ensure that it gets deleted as well
|
// ensure that it gets deleted as well
|
||||||
storagePlugin.removeData(token, name2)
|
storagePlugin.removeData(token, name2)
|
||||||
assertFalse(storagePlugin.hasData(token, name2))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun initStorage(token: Long) = runBlocking {
|
private fun initStorage(token: Long) = runBlocking {
|
||||||
|
|
|
@ -39,12 +39,6 @@ interface StoragePlugin<T> {
|
||||||
@Throws(IOException::class)
|
@Throws(IOException::class)
|
||||||
suspend fun initializeDevice()
|
suspend fun initializeDevice()
|
||||||
|
|
||||||
/**
|
|
||||||
* Return true if there is data stored for the given name.
|
|
||||||
*/
|
|
||||||
@Throws(IOException::class)
|
|
||||||
suspend fun hasData(token: Long, name: String): Boolean
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return a raw byte stream for writing data for the given name.
|
* Return a raw byte stream for writing data for the given name.
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -87,12 +87,6 @@ internal class DocumentsProviderStoragePlugin(
|
||||||
storage.reset(null)
|
storage.reset(null)
|
||||||
}
|
}
|
||||||
|
|
||||||
@Throws(IOException::class)
|
|
||||||
override suspend fun hasData(token: Long, name: String): Boolean {
|
|
||||||
val setDir = storage.getSetDir(token) ?: return false
|
|
||||||
return setDir.findFileBlocking(context, name) != null
|
|
||||||
}
|
|
||||||
|
|
||||||
@Throws(IOException::class)
|
@Throws(IOException::class)
|
||||||
override suspend fun getOutputStream(token: Long, name: String): OutputStream {
|
override suspend fun getOutputStream(token: Long, name: String): OutputStream {
|
||||||
val setDir = storage.getSetDir(token) ?: throw IOException()
|
val setDir = storage.getSetDir(token) ?: throw IOException()
|
||||||
|
|
|
@ -98,28 +98,6 @@ internal class WebDavStoragePlugin(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Throws(IOException::class)
|
|
||||||
override suspend fun hasData(token: Long, name: String): Boolean {
|
|
||||||
val location = "$url/$token/$name".toHttpUrl()
|
|
||||||
val davCollection = DavCollection(okHttpClient, location)
|
|
||||||
|
|
||||||
return try {
|
|
||||||
val response = suspendCoroutine { cont ->
|
|
||||||
davCollection.head { response ->
|
|
||||||
cont.resume(response)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
debugLog { "hasData($token, $name) = $response" }
|
|
||||||
response.isSuccessful
|
|
||||||
} catch (e: NotFoundException) {
|
|
||||||
debugLog { "hasData($token, $name) = $e" }
|
|
||||||
false
|
|
||||||
} catch (e: Exception) {
|
|
||||||
if (e is IOException) throw e
|
|
||||||
else throw IOException(e)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
@Throws(IOException::class)
|
@Throws(IOException::class)
|
||||||
override suspend fun getOutputStream(token: Long, name: String): OutputStream {
|
override suspend fun getOutputStream(token: Long, name: String): OutputStream {
|
||||||
val location = "$url/$token/$name".toHttpUrl()
|
val location = "$url/$token/$name".toHttpUrl()
|
||||||
|
|
|
@ -234,48 +234,36 @@ internal class RestoreCoordinator(
|
||||||
if (version == 0.toByte()) return nextRestorePackageV0(state, packageInfo)
|
if (version == 0.toByte()) return nextRestorePackageV0(state, packageInfo)
|
||||||
|
|
||||||
val packageName = packageInfo.packageName
|
val packageName = packageInfo.packageName
|
||||||
val type = try {
|
val type = when (state.backupMetadata.packageMetadataMap[packageName]?.backupType) {
|
||||||
when (state.backupMetadata.packageMetadataMap[packageName]?.backupType) {
|
BackupType.KV -> {
|
||||||
BackupType.KV -> {
|
val name = crypto.getNameForPackage(state.backupMetadata.salt, packageName)
|
||||||
val name = crypto.getNameForPackage(state.backupMetadata.salt, packageName)
|
kv.initializeState(
|
||||||
if (plugin.hasData(state.token, name)) {
|
version = version,
|
||||||
Log.i(TAG, "Found K/V data for $packageName.")
|
token = state.token,
|
||||||
kv.initializeState(
|
name = name,
|
||||||
version = version,
|
packageInfo = packageInfo,
|
||||||
token = state.token,
|
autoRestorePackageInfo = state.autoRestorePackageInfo
|
||||||
name = name,
|
)
|
||||||
packageInfo = packageInfo,
|
state.currentPackage = packageName
|
||||||
autoRestorePackageInfo = state.autoRestorePackageInfo
|
TYPE_KEY_VALUE
|
||||||
)
|
}
|
||||||
state.currentPackage = packageName
|
|
||||||
TYPE_KEY_VALUE
|
BackupType.FULL -> {
|
||||||
} else throw IOException("No data found for $packageName. Skipping.")
|
val name = crypto.getNameForPackage(state.backupMetadata.salt, packageName)
|
||||||
}
|
full.initializeState(version, state.token, name, packageInfo)
|
||||||
|
state.currentPackage = packageName
|
||||||
BackupType.FULL -> {
|
TYPE_FULL_STREAM
|
||||||
val name = crypto.getNameForPackage(state.backupMetadata.salt, packageName)
|
}
|
||||||
if (plugin.hasData(state.token, name)) {
|
|
||||||
Log.i(TAG, "Found full backup data for $packageName.")
|
null -> {
|
||||||
full.initializeState(version, state.token, name, packageInfo)
|
Log.i(TAG, "No backup type found for $packageName. Skipping...")
|
||||||
state.currentPackage = packageName
|
state.backupMetadata.packageMetadataMap[packageName]?.backupType?.let { s ->
|
||||||
TYPE_FULL_STREAM
|
Log.w(TAG, "State was ${s.name}")
|
||||||
} else throw IOException("No data found for $packageName. Skipping...")
|
}
|
||||||
}
|
failedPackages.add(packageName)
|
||||||
|
// don't return null and cause abort here, but try next package
|
||||||
null -> {
|
return nextRestorePackage()
|
||||||
Log.i(TAG, "No backup type found for $packageName. Skipping...")
|
|
||||||
state.backupMetadata.packageMetadataMap[packageName]?.backupType?.let { s ->
|
|
||||||
Log.w(TAG, "State was ${s.name}")
|
|
||||||
}
|
|
||||||
failedPackages.add(packageName)
|
|
||||||
return nextRestorePackage()
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
} catch (e: IOException) {
|
|
||||||
Log.e(TAG, "Error finding restore data for $packageName.", e)
|
|
||||||
failedPackages.add(packageName)
|
|
||||||
// don't return null and cause abort here, but try next package
|
|
||||||
return nextRestorePackage()
|
|
||||||
}
|
}
|
||||||
return RestoreDescription(packageName, type)
|
return RestoreDescription(packageName, type)
|
||||||
}
|
}
|
||||||
|
|
|
@ -64,17 +64,11 @@ internal class WebDavStoragePluginTest : TransportTest() {
|
||||||
// initially, we don't have any backups
|
// initially, we don't have any backups
|
||||||
assertEquals(emptySet<EncryptedMetadata>(), plugin.getAvailableBackups()?.toSet())
|
assertEquals(emptySet<EncryptedMetadata>(), plugin.getAvailableBackups()?.toSet())
|
||||||
|
|
||||||
// and no data
|
|
||||||
assertFalse(plugin.hasData(token, FILE_BACKUP_METADATA))
|
|
||||||
|
|
||||||
// write out the metadata file
|
// write out the metadata file
|
||||||
plugin.getOutputStream(token, FILE_BACKUP_METADATA).use {
|
plugin.getOutputStream(token, FILE_BACKUP_METADATA).use {
|
||||||
it.write(metadata)
|
it.write(metadata)
|
||||||
}
|
}
|
||||||
|
|
||||||
// now we have data
|
|
||||||
assertTrue(plugin.hasData(token, FILE_BACKUP_METADATA))
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// now we have one backup matching our token
|
// now we have one backup matching our token
|
||||||
val backups = plugin.getAvailableBackups()?.toSet() ?: fail()
|
val backups = plugin.getAvailableBackups()?.toSet() ?: fail()
|
||||||
|
@ -86,9 +80,6 @@ internal class WebDavStoragePluginTest : TransportTest() {
|
||||||
metadata,
|
metadata,
|
||||||
plugin.getInputStream(token, FILE_BACKUP_METADATA).use { it.readAllBytes() },
|
plugin.getInputStream(token, FILE_BACKUP_METADATA).use { it.readAllBytes() },
|
||||||
)
|
)
|
||||||
|
|
||||||
// it has data now
|
|
||||||
assertTrue(plugin.hasData(token, FILE_BACKUP_METADATA))
|
|
||||||
} finally {
|
} finally {
|
||||||
// remove data at the end, so consecutive test runs pass
|
// remove data at the end, so consecutive test runs pass
|
||||||
plugin.removeData(token, FILE_BACKUP_METADATA)
|
plugin.removeData(token, FILE_BACKUP_METADATA)
|
||||||
|
|
|
@ -190,7 +190,6 @@ internal class CoordinatorIntegrationTest : TransportTest() {
|
||||||
|
|
||||||
// find data for K/V backup
|
// find data for K/V backup
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageInfo.packageName) } returns name
|
every { crypto.getNameForPackage(metadata.salt, packageInfo.packageName) } returns name
|
||||||
coEvery { backupPlugin.hasData(token, name) } returns true
|
|
||||||
|
|
||||||
val restoreDescription = restore.nextRestorePackage() ?: fail()
|
val restoreDescription = restore.nextRestorePackage() ?: fail()
|
||||||
assertEquals(packageInfo.packageName, restoreDescription.packageName)
|
assertEquals(packageInfo.packageName, restoreDescription.packageName)
|
||||||
|
@ -264,7 +263,6 @@ internal class CoordinatorIntegrationTest : TransportTest() {
|
||||||
|
|
||||||
// find data for K/V backup
|
// find data for K/V backup
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageInfo.packageName) } returns name
|
every { crypto.getNameForPackage(metadata.salt, packageInfo.packageName) } returns name
|
||||||
coEvery { backupPlugin.hasData(token, name) } returns true
|
|
||||||
|
|
||||||
val restoreDescription = restore.nextRestorePackage() ?: fail()
|
val restoreDescription = restore.nextRestorePackage() ?: fail()
|
||||||
assertEquals(packageInfo.packageName, restoreDescription.packageName)
|
assertEquals(packageInfo.packageName, restoreDescription.packageName)
|
||||||
|
@ -327,7 +325,6 @@ internal class CoordinatorIntegrationTest : TransportTest() {
|
||||||
|
|
||||||
// finds data for full backup
|
// finds data for full backup
|
||||||
every { crypto.getNameForPackage(salt, packageInfo.packageName) } returns name
|
every { crypto.getNameForPackage(salt, packageInfo.packageName) } returns name
|
||||||
coEvery { backupPlugin.hasData(token, name) } returns true
|
|
||||||
|
|
||||||
val restoreDescription = restore.nextRestorePackage() ?: fail()
|
val restoreDescription = restore.nextRestorePackage() ?: fail()
|
||||||
assertEquals(packageInfo.packageName, restoreDescription.packageName)
|
assertEquals(packageInfo.packageName, restoreDescription.packageName)
|
||||||
|
|
|
@ -239,7 +239,6 @@ internal class RestoreCoordinatorTest : TransportTest() {
|
||||||
restore.startRestore(token, packageInfoArray)
|
restore.startRestore(token, packageInfoArray)
|
||||||
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageName) } returns name
|
every { crypto.getNameForPackage(metadata.salt, packageName) } returns name
|
||||||
coEvery { plugin.hasData(token, name) } returns true
|
|
||||||
every { kv.initializeState(VERSION, token, name, packageInfo) } just Runs
|
every { kv.initializeState(VERSION, token, name, packageInfo) } just Runs
|
||||||
|
|
||||||
val expected = RestoreDescription(packageName, TYPE_KEY_VALUE)
|
val expected = RestoreDescription(packageName, TYPE_KEY_VALUE)
|
||||||
|
@ -273,19 +272,6 @@ internal class RestoreCoordinatorTest : TransportTest() {
|
||||||
assertEquals(expected, restore.nextRestorePackage())
|
assertEquals(expected, restore.nextRestorePackage())
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
|
||||||
fun `nextRestorePackage() returns NO_MORE_PACKAGES if data not found`() = runBlocking {
|
|
||||||
restore.beforeStartRestore(metadata)
|
|
||||||
restore.startRestore(token, packageInfoArray2)
|
|
||||||
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageName) } returns name
|
|
||||||
coEvery { plugin.hasData(token, name) } returns false
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageInfo2.packageName) } returns name2
|
|
||||||
coEvery { plugin.hasData(token, name2) } returns false
|
|
||||||
|
|
||||||
assertEquals(NO_MORE_PACKAGES, restore.nextRestorePackage())
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `nextRestorePackage() tries next package if one has no backup type()`() = runBlocking {
|
fun `nextRestorePackage() tries next package if one has no backup type()`() = runBlocking {
|
||||||
metadata.packageMetadataMap[packageName] =
|
metadata.packageMetadataMap[packageName] =
|
||||||
|
@ -294,7 +280,6 @@ internal class RestoreCoordinatorTest : TransportTest() {
|
||||||
restore.startRestore(token, packageInfoArray2)
|
restore.startRestore(token, packageInfoArray2)
|
||||||
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageInfo2.packageName) } returns name2
|
every { crypto.getNameForPackage(metadata.salt, packageInfo2.packageName) } returns name2
|
||||||
coEvery { plugin.hasData(token, name2) } returns true
|
|
||||||
every { full.initializeState(VERSION, token, name2, packageInfo2) } just Runs
|
every { full.initializeState(VERSION, token, name2, packageInfo2) } just Runs
|
||||||
|
|
||||||
val expected = RestoreDescription(packageInfo2.packageName, TYPE_FULL_STREAM)
|
val expected = RestoreDescription(packageInfo2.packageName, TYPE_FULL_STREAM)
|
||||||
|
@ -309,14 +294,12 @@ internal class RestoreCoordinatorTest : TransportTest() {
|
||||||
restore.startRestore(token, packageInfoArray2)
|
restore.startRestore(token, packageInfoArray2)
|
||||||
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageName) } returns name
|
every { crypto.getNameForPackage(metadata.salt, packageName) } returns name
|
||||||
coEvery { plugin.hasData(token, name) } returns true
|
|
||||||
every { kv.initializeState(VERSION, token, name, packageInfo) } just Runs
|
every { kv.initializeState(VERSION, token, name, packageInfo) } just Runs
|
||||||
|
|
||||||
val expected = RestoreDescription(packageInfo.packageName, TYPE_KEY_VALUE)
|
val expected = RestoreDescription(packageInfo.packageName, TYPE_KEY_VALUE)
|
||||||
assertEquals(expected, restore.nextRestorePackage())
|
assertEquals(expected, restore.nextRestorePackage())
|
||||||
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageInfo2.packageName) } returns name2
|
every { crypto.getNameForPackage(metadata.salt, packageInfo2.packageName) } returns name2
|
||||||
coEvery { plugin.hasData(token, name2) } returns true
|
|
||||||
every { full.initializeState(VERSION, token, name2, packageInfo2) } just Runs
|
every { full.initializeState(VERSION, token, name2, packageInfo2) } just Runs
|
||||||
|
|
||||||
val expected2 =
|
val expected2 =
|
||||||
|
@ -359,19 +342,6 @@ internal class RestoreCoordinatorTest : TransportTest() {
|
||||||
assertEquals(NO_MORE_PACKAGES, restore.nextRestorePackage())
|
assertEquals(NO_MORE_PACKAGES, restore.nextRestorePackage())
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
|
||||||
fun `when plugin#hasData() throws, it tries next package`() = runBlocking {
|
|
||||||
restore.beforeStartRestore(metadata)
|
|
||||||
restore.startRestore(token, packageInfoArray2)
|
|
||||||
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageName) } returns name
|
|
||||||
coEvery { plugin.hasData(token, name) } returns false
|
|
||||||
every { crypto.getNameForPackage(metadata.salt, packageInfo2.packageName) } returns name2
|
|
||||||
coEvery { plugin.hasData(token, name2) } throws IOException()
|
|
||||||
|
|
||||||
assertEquals(NO_MORE_PACKAGES, restore.nextRestorePackage())
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@Suppress("deprecation")
|
@Suppress("deprecation")
|
||||||
fun `v0 when full#hasDataForPackage() throws, it tries next package`() = runBlocking {
|
fun `v0 when full#hasDataForPackage() throws, it tries next package`() = runBlocking {
|
||||||
|
|
Loading…
Reference in a new issue