Skip to content

Commit 2f34d97

Browse files
CrisBarreiroCrisBarreiro
committed
Also FF remove synchronized from SyncUnavailableStore
1 parent 67439db commit 2f34d97

File tree

9 files changed

+202
-113
lines changed

9 files changed

+202
-113
lines changed

‎sync/sync-api/src/main/java/com/duckduckgo/sync/api/engine/SyncEngine.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ interface SyncEngine {
2828
* Sync Feature has been disabled / device has been removed
2929
* This is an opportunity for Features to do some local cleanup if needed
3030
*/
31-
fun onSyncDisabled()
31+
suspend fun onSyncDisabled()
3232

3333
/**
3434
* Represent each possible trigger fo

‎sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/di/SyncModule.kt

+9-1
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,16 @@ object SyncStoreModule {
156156
@SingleInstanceIn(AppScope::class)
157157
fun provideSyncPausedStore(
158158
sharedPrefsProvider: SharedPrefsProvider,
159+
@AppCoroutineScope coroutineScope: CoroutineScope,
160+
dispatcherProvider: DispatcherProvider,
161+
syncFeature: SyncFeature,
159162
): SyncUnavailableStore {
160-
return SyncUnavailableSharedPrefsStore(sharedPrefsProvider)
163+
return SyncUnavailableSharedPrefsStore(
164+
sharedPrefsProvider,
165+
coroutineScope,
166+
dispatcherProvider,
167+
syncFeature.createAsyncPreferences().isEnabled(),
168+
)
161169
}
162170

163171
private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(

‎sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/engine/RealSyncEngine.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ class RealSyncEngine @Inject constructor(
268268
}
269269
}
270270

271-
override fun onSyncDisabled() {
271+
override suspend fun onSyncDisabled() {
272272
syncStateRepository.clearAll()
273273
persisterPlugins.getPlugins().map {
274274
it.onSyncDisabled()

‎sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/engine/SyncEngineLifecycle.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ import com.duckduckgo.di.scopes.AppScope
2222
@ContributesPluginPoint(AppScope::class)
2323
interface SyncEngineLifecycle {
2424
fun onSyncEnabled()
25-
fun onSyncDisabled()
25+
suspend fun onSyncDisabled()
2626
}

‎sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/error/SyncUnavailableRepository.kt

+39-31
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import androidx.work.OneTimeWorkRequest
2424
import androidx.work.WorkManager
2525
import androidx.work.WorkerParameters
2626
import com.duckduckgo.anvil.annotations.ContributesWorker
27+
import com.duckduckgo.app.di.AppCoroutineScope
2728
import com.duckduckgo.common.utils.DispatcherProvider
2829
import com.duckduckgo.common.utils.notification.checkPermissionAndNotify
2930
import com.duckduckgo.di.scopes.AppScope
@@ -39,14 +40,16 @@ import java.time.LocalDateTime
3940
import java.time.format.DateTimeFormatter
4041
import java.util.concurrent.TimeUnit
4142
import javax.inject.Inject
43+
import kotlinx.coroutines.CoroutineScope
44+
import kotlinx.coroutines.launch
4245
import kotlinx.coroutines.withContext
4346
import timber.log.Timber
4447

4548
interface SyncUnavailableRepository {
4649
fun onServerAvailable()
4750
fun onServerUnavailable()
48-
fun isSyncUnavailable(): Boolean
49-
fun triggerNotification()
51+
suspend fun isSyncUnavailable(): Boolean
52+
suspend fun triggerNotification()
5053
}
5154

5255
@Suppress("SameParameterValue")
@@ -65,47 +68,52 @@ class RealSyncUnavailableRepository @Inject constructor(
6568
private val notificationManager: NotificationManagerCompat,
6669
private val syncNotificationBuilder: SyncNotificationBuilder,
6770
private val workManager: WorkManager,
71+
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
6872
) : SyncUnavailableRepository, SyncEngineLifecycle {
6973

70-
override fun isSyncUnavailable(): Boolean {
71-
return syncUnavailableStore.isSyncUnavailable
74+
override suspend fun isSyncUnavailable(): Boolean {
75+
return syncUnavailableStore.isSyncUnavailable()
7276
}
7377

7478
override fun onServerAvailable() {
75-
if (syncUnavailableStore.isSyncUnavailable) {
76-
Timber.d("Sync-Engine: Sync is back online - clearing data and canceling notif")
77-
syncUnavailableStore.clearError()
78-
cancelNotification()
79+
appCoroutineScope.launch {
80+
if (syncUnavailableStore.isSyncUnavailable()) {
81+
Timber.d("Sync-Engine: Sync is back online - clearing data and canceling notif")
82+
syncUnavailableStore.clearError()
83+
cancelNotification()
84+
}
7985
}
8086
}
8187

8288
override fun onServerUnavailable() {
83-
if (!syncUnavailableStore.isSyncUnavailable) {
84-
syncUnavailableStore.syncUnavailableSince = getUtcIsoLocalDate()
85-
}
86-
syncUnavailableStore.isSyncUnavailable = true
87-
syncUnavailableStore.syncErrorCount = syncUnavailableStore.syncErrorCount + 1
88-
89-
Timber.d(
90-
"Sync-Engine: server unavailable count: ${syncUnavailableStore.syncErrorCount} " +
91-
"pausedAt: ${syncUnavailableStore.syncUnavailableSince} lastNotifiedAt: ${syncUnavailableStore.userNotifiedAt}",
92-
)
93-
if (syncUnavailableStore.syncErrorCount >= ERROR_THRESHOLD_NOTIFICATION_COUNT) {
94-
Timber.d("Sync-Engine: Sync error count reached threshold")
95-
triggerNotification()
96-
} else {
97-
scheduleNotification(
98-
OneTimeWorkRequest.Builder(SchedulableErrorNotificationWorker::class.java),
99-
SYNC_ERROR_NOTIFICATION_DELAY,
100-
TimeUnit.HOURS,
101-
SYNC_ERROR_NOTIFICATION_TAG,
89+
appCoroutineScope.launch {
90+
if (!syncUnavailableStore.isSyncUnavailable()) {
91+
syncUnavailableStore.setSyncUnavailableSince(getUtcIsoLocalDate())
92+
}
93+
syncUnavailableStore.setSyncUnavailable(true)
94+
syncUnavailableStore.setSyncErrorCount(syncUnavailableStore.getSyncErrorCount() + 1)
95+
96+
Timber.d(
97+
"Sync-Engine: server unavailable count: ${syncUnavailableStore.getSyncErrorCount()} " +
98+
"pausedAt: ${syncUnavailableStore.getSyncUnavailableSince()} lastNotifiedAt: ${syncUnavailableStore.getUserNotifiedAt()}",
10299
)
100+
if (syncUnavailableStore.getSyncErrorCount() >= ERROR_THRESHOLD_NOTIFICATION_COUNT) {
101+
Timber.d("Sync-Engine: Sync error count reached threshold")
102+
triggerNotification()
103+
} else {
104+
scheduleNotification(
105+
OneTimeWorkRequest.Builder(SchedulableErrorNotificationWorker::class.java),
106+
SYNC_ERROR_NOTIFICATION_DELAY,
107+
TimeUnit.HOURS,
108+
SYNC_ERROR_NOTIFICATION_TAG,
109+
)
110+
}
103111
}
104112
}
105113

106-
override fun triggerNotification() {
114+
override suspend fun triggerNotification() {
107115
val today = LocalDateTime.now().toLocalDate()
108-
val lastNotification = syncUnavailableStore.userNotifiedAt.takeUnless { it.isEmpty() }?.let {
116+
val lastNotification = syncUnavailableStore.getUserNotifiedAt().takeUnless { it.isEmpty() }?.let {
109117
LocalDateTime.parse(it, DateTimeFormatter.ISO_LOCAL_DATE_TIME).toLocalDate()
110118
} ?: ""
111119
val userNotifiedToday = today == lastNotification
@@ -117,7 +125,7 @@ class RealSyncUnavailableRepository @Inject constructor(
117125
SYNC_ERROR_NOTIFICATION_ID,
118126
syncNotificationBuilder.buildSyncErrorNotification(context),
119127
)
120-
syncUnavailableStore.userNotifiedAt = getUtcIsoLocalDate()
128+
syncUnavailableStore.setUserNotifiedAt(getUtcIsoLocalDate())
121129
}
122130
}
123131

@@ -147,7 +155,7 @@ class RealSyncUnavailableRepository @Inject constructor(
147155
// no-op
148156
}
149157

150-
override fun onSyncDisabled() {
158+
override suspend fun onSyncDisabled() {
151159
Timber.d("Sync-Engine: Sync disabled, clearing unavailable store data")
152160
syncUnavailableStore.clearAll()
153161
cancelNotification()

‎sync/sync-impl/src/main/java/com/duckduckgo/sync/impl/ui/SyncErrorViewModel.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class SyncErrorViewModel @Inject constructor(
5858
}.flowOn(dispatcherProvider.io()).launchIn(viewModelScope)
5959
}
6060

61-
private fun getMessage(state: SyncState): Int? {
61+
private suspend fun getMessage(state: SyncState): Int? {
6262
if (state == SyncState.OFF) return null
6363

6464
if (syncErrorRepository.isSyncUnavailable()) {

‎sync/sync-impl/src/test/java/com/duckduckgo/sync/impl/error/RealSyncUnavailableRepositoryTest.kt

+29-24
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import com.duckduckgo.sync.store.SyncUnavailableSharedPrefsStore
3939
import java.time.LocalDateTime
4040
import java.time.OffsetDateTime
4141
import java.time.format.DateTimeFormatter
42+
import kotlinx.coroutines.test.runTest
4243
import org.junit.After
4344
import org.junit.Assert.*
4445
import org.junit.Before
@@ -60,6 +61,9 @@ class RealSyncUnavailableRepositoryTest {
6061
private val syncNotificationBuilder = FakeNotificationBuilder()
6162
private val syncUnavailableStore = SyncUnavailableSharedPrefsStore(
6263
sharedPrefsProv = TestSharedPrefsProvider(context),
64+
appCoroutineScope = coroutineRule.testScope,
65+
dispatcherProvider = coroutineRule.testDispatcherProvider,
66+
createAsyncPreferences = true,
6367
)
6468
private lateinit var workManager: WorkManager
6569
private lateinit var testee: RealSyncUnavailableRepository
@@ -74,6 +78,7 @@ class RealSyncUnavailableRepositoryTest {
7478
notificationManager,
7579
syncNotificationBuilder,
7680
workManager,
81+
coroutineRule.testScope,
7782
)
7883
}
7984

@@ -83,71 +88,71 @@ class RealSyncUnavailableRepositoryTest {
8388
}
8489

8590
@Test
86-
fun whenServerBecomesAvailableThenSyncAvailable() {
87-
syncUnavailableStore.isSyncUnavailable = true
91+
fun whenServerBecomesAvailableThenSyncAvailable() = runTest {
92+
syncUnavailableStore.setSyncUnavailable(true)
8893
testee.onServerAvailable()
89-
assertFalse(syncUnavailableStore.isSyncUnavailable)
90-
assertEquals("", syncUnavailableStore.syncUnavailableSince)
94+
assertFalse(syncUnavailableStore.isSyncUnavailable())
95+
assertEquals("", syncUnavailableStore.getSyncUnavailableSince())
9196
}
9297

9398
@Test
94-
fun whenServerUnavailableThenSyncUnavailable() {
99+
fun whenServerUnavailableThenSyncUnavailable() = runTest {
95100
testee.onServerUnavailable()
96-
assertTrue(syncUnavailableStore.isSyncUnavailable)
101+
assertTrue(syncUnavailableStore.isSyncUnavailable())
97102
}
98103

99104
@Test
100-
fun whenServerUnavailableThenUpdateTimestampOnce() {
105+
fun whenServerUnavailableThenUpdateTimestampOnce() = runTest {
101106
testee.onServerUnavailable()
102-
val unavailableSince = syncUnavailableStore.syncUnavailableSince
107+
val unavailableSince = syncUnavailableStore.getSyncUnavailableSince()
103108
assertTrue(unavailableSince.isNotEmpty())
104109
testee.onServerUnavailable()
105-
assertEquals(unavailableSince, syncUnavailableStore.syncUnavailableSince)
110+
assertEquals(unavailableSince, syncUnavailableStore.getSyncUnavailableSince())
106111
}
107112

108113
@Test
109-
fun whenServerUnavailableThenUpdateCounter() {
114+
fun whenServerUnavailableThenUpdateCounter() = runTest {
110115
testee.onServerUnavailable()
111-
assertTrue(syncUnavailableStore.isSyncUnavailable)
112-
assertEquals(1, syncUnavailableStore.syncErrorCount)
116+
assertTrue(syncUnavailableStore.isSyncUnavailable())
117+
assertEquals(1, syncUnavailableStore.getSyncErrorCount())
113118
testee.onServerUnavailable()
114119
testee.onServerUnavailable()
115120
testee.onServerUnavailable()
116-
assertEquals(4, syncUnavailableStore.syncErrorCount)
121+
assertEquals(4, syncUnavailableStore.getSyncErrorCount())
117122
}
118123

119124
@Test
120-
fun whenServerUnavailableThenScheduleNotification() {
125+
fun whenServerUnavailableThenScheduleNotification() = runTest {
121126
testee.onServerUnavailable()
122-
assertTrue(syncUnavailableStore.isSyncUnavailable)
127+
assertTrue(syncUnavailableStore.isSyncUnavailable())
123128
val syncErrorNotification = workManager.getWorkInfosByTag(SYNC_ERROR_NOTIFICATION_TAG).get()
124129
assertEquals(1, syncErrorNotification.size)
125130
}
126131

127132
@Test
128-
fun whenErrorCounterReachesThresholdThenTriggerNotification() {
129-
syncUnavailableStore.syncErrorCount = RealSyncUnavailableRepository.ERROR_THRESHOLD_NOTIFICATION_COUNT
133+
fun whenErrorCounterReachesThresholdThenTriggerNotification() = runTest {
134+
syncUnavailableStore.setSyncErrorCount(RealSyncUnavailableRepository.ERROR_THRESHOLD_NOTIFICATION_COUNT)
130135
testee.onServerUnavailable()
131136
notificationManager.activeNotifications
132137
.find { it.id == SYNC_ERROR_NOTIFICATION_ID } ?: fail("Notification not found")
133138
}
134139

135140
@Test
136-
fun whenUserNotifiedTodayThenDoNotTriggerNotification() {
137-
syncUnavailableStore.userNotifiedAt = OffsetDateTime.now().format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)
141+
fun whenUserNotifiedTodayThenDoNotTriggerNotification() = runTest {
142+
syncUnavailableStore.setUserNotifiedAt(OffsetDateTime.now().format(DateTimeFormatter.ISO_LOCAL_DATE_TIME))
138143
testee.triggerNotification()
139144
assertNull(notificationManager.activeNotifications.find { it.id == SYNC_ERROR_NOTIFICATION_ID })
140145
}
141146

142147
@Test
143-
fun whenUserNotifiedYesterdayThenTriggerNotificationAndUpdateNotificationTimestamp() {
144-
syncUnavailableStore.userNotifiedAt = OffsetDateTime.now().minusDays(1).format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)
148+
fun whenUserNotifiedYesterdayThenTriggerNotificationAndUpdateNotificationTimestamp() = runTest {
149+
syncUnavailableStore.setUserNotifiedAt(OffsetDateTime.now().minusDays(1).format(DateTimeFormatter.ISO_LOCAL_DATE_TIME))
145150
testee.triggerNotification()
146151
notificationManager.activeNotifications
147152
.find { it.id == SYNC_ERROR_NOTIFICATION_ID } ?: fail("Notification not found")
148153

149154
val today = LocalDateTime.now().toLocalDate()
150-
val lastNotification = LocalDateTime.parse(syncUnavailableStore.userNotifiedAt, DateTimeFormatter.ISO_LOCAL_DATE_TIME).toLocalDate()
155+
val lastNotification = LocalDateTime.parse(syncUnavailableStore.getUserNotifiedAt(), DateTimeFormatter.ISO_LOCAL_DATE_TIME).toLocalDate()
151156
assertEquals(today, lastNotification)
152157
}
153158

@@ -162,8 +167,8 @@ class RealSyncUnavailableRepositoryTest {
162167
}
163168

164169
@Test
165-
fun whenServerAvailableThenClearNotification() {
166-
syncUnavailableStore.syncErrorCount = RealSyncUnavailableRepository.ERROR_THRESHOLD_NOTIFICATION_COUNT
170+
fun whenServerAvailableThenClearNotification() = runTest {
171+
syncUnavailableStore.setSyncErrorCount(RealSyncUnavailableRepository.ERROR_THRESHOLD_NOTIFICATION_COUNT)
167172
testee.onServerUnavailable()
168173
notificationManager.activeNotifications
169174
.find { it.id == SYNC_ERROR_NOTIFICATION_ID } ?: fail("Notification not found")

0 commit comments

Comments
 (0)