Skip to content

Commit f4b32e4

Browse files
committed
Tell system to reinitialize when it refuses to give us data we need
Another hacky workaround to fix a hacky workaround. We had talked to Google about the backup API not getting notified when essential K/V apps like @pm@ don't have new data for backup. The only option they offered is BackupMonitor which gets the information at least, but out of process on another thread. So we implemented a hacky workaround where BackupMonitor tells SnapshotCreator to extract backup data from an old snapshot. Unfortunately, it is possible that we do a backup run which includes @pm@, but encounters an error later, so the system cancels the entire backup which causes us not to have @pm@ data in a snapshots for re-use. Still, the system thinks we backed up @pm@ and doesn't give us its data.
1 parent ea10f1b commit f4b32e4

File tree

7 files changed

+67
-38
lines changed

7 files changed

+67
-38
lines changed

app/src/main/java/com/stevesoltys/seedvault/repo/RepoModule.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ val repoModule = module {
1919
val snapshotFolder = File(androidContext().filesDir, FOLDER_SNAPSHOTS)
2020
SnapshotManager(snapshotFolder, get(), get(), get())
2121
}
22-
factory { SnapshotCreatorFactory(androidContext(), get(), get(), get()) }
22+
factory { SnapshotCreatorFactory(androidContext(), get(), get(), get(), get()) }
2323
factory { Pruner(get(), get(), get(), get()) }
2424
single { Checker(get(), get(), get(), get(), get(), get()) }
2525
}

app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotCreator.kt

+39-26
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import com.stevesoltys.seedvault.proto.Snapshot
2525
import com.stevesoltys.seedvault.proto.Snapshot.Apk
2626
import com.stevesoltys.seedvault.proto.Snapshot.App
2727
import com.stevesoltys.seedvault.proto.Snapshot.Blob
28+
import com.stevesoltys.seedvault.transport.backup.BackupInitializer
2829
import com.stevesoltys.seedvault.transport.backup.PackageService
2930
import com.stevesoltys.seedvault.transport.backup.isSystemApp
3031
import io.github.oshai.kotlinlogging.KotlinLogging
@@ -41,6 +42,7 @@ internal class SnapshotCreator(
4142
private val clock: Clock,
4243
private val packageService: PackageService,
4344
private val metadataManager: MetadataManager,
45+
private val backupInitializer: BackupInitializer,
4446
) {
4547

4648
private val log = KotlinLogging.logger { }
@@ -115,44 +117,55 @@ internal class SnapshotCreator(
115117
* we try to extract data from the given [snapshot] (ideally we latest we have) and
116118
* add it to the current snapshot under construction.
117119
*
118-
* @param warnNoData log a warning, if [snapshot] had no data for the given [packageName].
120+
* @param isStopped set to true if the app had no data, because it is currently force-stopped.
119121
*/
120-
fun onNoDataInCurrentRun(snapshot: Snapshot, packageName: String, isStopped: Boolean = false) {
121-
log.info { "onKvPackageNotChanged(${snapshot.token}, $packageName)" }
122+
fun onNoDataInCurrentRun(snapshot: Snapshot?, packageName: String, isStopped: Boolean = false) {
123+
log.info { "onKvPackageNotChanged(${snapshot?.token}, $packageName)" }
122124

123125
if (appBuilderMap.containsKey(packageName)) {
124126
// the system backs up K/V apps repeatedly, e.g. @pm@
125127
log.info { " Already have data for $packageName in current snapshot, not touching it" }
126128
return
127129
}
128-
val app = snapshot.appsMap[packageName]
129-
if (app == null) {
130-
if (!isStopped) log.error {
131-
" No changed data for $packageName, but we had no data for it"
130+
if (snapshot == null) {
131+
log.error { "No latest snapshot! Initializing transport..." }
132+
// Tell the system that it needs to initialize our transport,
133+
// so it stops telling us that it has no data.
134+
// It seems crazy that we can do this mid-backup
135+
val error = { log.error { "Error initializing transport." } }
136+
backupInitializer.initialize(error) {
137+
log.info { "Success initializing transport." }
138+
}
139+
} else {
140+
val app = snapshot.appsMap[packageName]
141+
if (app == null) {
142+
if (!isStopped) log.error {
143+
" No changed data for $packageName, but we had no data for it"
144+
}
145+
return
132146
}
133-
return
134-
}
135147

136-
// get chunkIds from last snapshot
137-
val chunkIds = app.chunkIdsList.hexFromProto() +
138-
app.apk.splitsList.flatMap { it.chunkIdsList }.hexFromProto()
148+
// get chunkIds from last snapshot
149+
val chunkIds = app.chunkIdsList.hexFromProto() +
150+
app.apk.splitsList.flatMap { it.chunkIdsList }.hexFromProto()
139151

140-
// get blobs for chunkIds
141-
val blobMap = mutableMapOf<String, Blob>()
142-
chunkIds.forEach { chunkId ->
143-
val blob = snapshot.blobsMap[chunkId]
144-
if (blob == null) log.error { " No blob for $packageName chunk $chunkId" }
145-
else blobMap[chunkId] = blob
146-
}
152+
// get blobs for chunkIds
153+
val blobMap = mutableMapOf<String, Blob>()
154+
chunkIds.forEach { chunkId ->
155+
val blob = snapshot.blobsMap[chunkId]
156+
if (blob == null) log.error { " No blob for $packageName chunk $chunkId" }
157+
else blobMap[chunkId] = blob
158+
}
147159

148-
// add info to current snapshot
149-
appBuilderMap[packageName] = app.toBuilder()
150-
blobsMap.putAll(blobMap)
160+
// add info to current snapshot
161+
appBuilderMap[packageName] = app.toBuilder()
162+
blobsMap.putAll(blobMap)
151163

152-
// record local metadata if this is not a stopped app
153-
if (!isStopped) {
154-
val packageInfo = PackageInfo().apply { this.packageName = packageName }
155-
metadataManager.onPackageBackedUp(packageInfo, app.type.toBackupType(), app.size)
164+
// record local metadata if this is not a stopped app
165+
if (!isStopped) {
166+
val packageInfo = PackageInfo().apply { this.packageName = packageName }
167+
metadataManager.onPackageBackedUp(packageInfo, app.type.toBackupType(), app.size)
168+
}
156169
}
157170
}
158171

app/src/main/java/com/stevesoltys/seedvault/repo/SnapshotCreatorFactory.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package com.stevesoltys.seedvault.repo
88
import android.content.Context
99
import com.stevesoltys.seedvault.Clock
1010
import com.stevesoltys.seedvault.metadata.MetadataManager
11+
import com.stevesoltys.seedvault.transport.backup.BackupInitializer
1112
import com.stevesoltys.seedvault.transport.backup.PackageService
1213

1314
/**
@@ -18,7 +19,8 @@ internal class SnapshotCreatorFactory(
1819
private val clock: Clock,
1920
private val packageService: PackageService,
2021
private val metadataManager: MetadataManager,
22+
private val backupInitializer: BackupInitializer,
2123
) {
2224
fun createSnapshotCreator() =
23-
SnapshotCreator(context, clock, packageService, metadataManager)
25+
SnapshotCreator(context, clock, packageService, metadataManager, backupInitializer)
2426
}

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ internal class BackupCoordinator(
7575
private val state = CoordinatorState(
7676
calledInitialize = false,
7777
calledClearBackupData = false,
78-
cancelReason = UNKNOWN_ERROR
78+
cancelReason = UNKNOWN_ERROR,
7979
)
8080
private val launchableSystemApps by lazy {
8181
packageService.launchableSystemApps.map { it.activityInfo.packageName }.toSet()

app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupTransportMonitor.kt

+2-6
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ internal class BackupTransportMonitor(
3333
log.info { "sendNoDataChanged($packageName)" }
3434

3535
val snapshot = snapshotManager.latestSnapshot
36-
if (snapshot == null) {
37-
log.error { "No latest snapshot!" }
38-
} else {
39-
val snapshotCreator = appBackupManager.snapshotCreator ?: error("No SnapshotCreator")
40-
snapshotCreator.onNoDataInCurrentRun(snapshot, packageName)
41-
}
36+
val snapshotCreator = appBackupManager.snapshotCreator ?: error("No SnapshotCreator")
37+
snapshotCreator.onNoDataInCurrentRun(snapshot, packageName)
4238
}
4339
}

app/src/test/java/com/stevesoltys/seedvault/repo/SnapshotCreatorTest.kt

+14-1
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ import com.stevesoltys.seedvault.proto.SnapshotKt.app
2323
import com.stevesoltys.seedvault.proto.SnapshotKt.split
2424
import com.stevesoltys.seedvault.proto.copy
2525
import com.stevesoltys.seedvault.transport.TransportTest
26+
import com.stevesoltys.seedvault.transport.backup.BackupInitializer
2627
import com.stevesoltys.seedvault.transport.backup.PackageService
2728
import com.stevesoltys.seedvault.worker.BASE_SPLIT
2829
import io.mockk.Runs
2930
import io.mockk.every
3031
import io.mockk.just
3132
import io.mockk.mockk
33+
import io.mockk.verify
3234
import org.junit.Assert.assertEquals
3335
import org.junit.Assert.assertTrue
3436
import org.junit.Test
@@ -46,7 +48,9 @@ internal class SnapshotCreatorTest : TransportTest() {
4648

4749
private val ctx: Context = ApplicationProvider.getApplicationContext()
4850
private val packageService: PackageService = mockk()
49-
private val snapshotCreator = SnapshotCreator(ctx, clock, packageService, metadataManager)
51+
private val backupInitializer: BackupInitializer = mockk()
52+
private val snapshotCreator =
53+
SnapshotCreator(ctx, clock, packageService, metadataManager, backupInitializer)
5054

5155
init {
5256
every { packageService.launchableSystemApps } returns emptyList()
@@ -173,6 +177,15 @@ internal class SnapshotCreatorTest : TransportTest() {
173177
}
174178
}
175179

180+
@Test
181+
fun `test onNoDataInCurrentRun re-initializes if no snapshot available`() {
182+
every { backupInitializer.initialize(any(), any()) } just Runs
183+
184+
snapshotCreator.onNoDataInCurrentRun(null, MAGIC_PACKAGE_MANAGER)
185+
186+
verify { backupInitializer.initialize(any(), any()) }
187+
}
188+
176189
@Test
177190
fun `test onNoDataInCurrentRun`() {
178191
val snapshot = snapshot.copy {

app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCreationTest.kt

+7-2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,13 @@ internal class BackupCreationTest : BackupTest() {
6666
private val backupReceiver = BackupReceiver(blobCache, blobCreator, cryptoImpl)
6767
private val appBackupManager = mockk<AppBackupManager>()
6868
private val packageService = mockk<PackageService>()
69-
private val snapshotCreator =
70-
SnapshotCreator(context, clock, packageService, mockk(relaxed = true))
69+
private val snapshotCreator = SnapshotCreator(
70+
context = context,
71+
clock = clock,
72+
packageService = packageService,
73+
metadataManager = mockk(relaxed = true),
74+
backupInitializer = mockk(relaxed = true),
75+
)
7176
private val notificationManager = mockk<BackupNotificationManager>()
7277
private val db = TestKvDbManager()
7378

0 commit comments

Comments
 (0)