From ad07fc0fee7fb5fbab5f0f8a23860d5c556f5d31 Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Fri, 6 Dec 2024 02:05:39 +0400 Subject: [PATCH] Fix bookmarks widget not updating This patch fixes a bug where the bookmarks widgets were no longer being updated due to the recent refactor of bookmarks. Since the deprecated BookmarkModel was being watched for changes, and since most new code updates BookmarksDao instead, the widget never got the changes. This patch fixes it so that the widgets update whenever either get updates. --- .../model/bookmark/BookmarkModel.kt | 2 +- .../widget/BookmarksWidgetSubscriber.kt | 25 ++++- .../widget/BookmarksWidgetSubscriberTest.kt | 103 ------------------ .../mobile/bookmark/model/BookmarksDaoImpl.kt | 14 +++ .../java/com/quran/data/dao/BookmarksDao.kt | 2 + 5 files changed, 36 insertions(+), 110 deletions(-) delete mode 100644 app/src/test/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriberTest.kt diff --git a/app/src/main/java/com/quran/labs/androidquran/model/bookmark/BookmarkModel.kt b/app/src/main/java/com/quran/labs/androidquran/model/bookmark/BookmarkModel.kt index b189a54aac..54a5e0ddef 100644 --- a/app/src/main/java/com/quran/labs/androidquran/model/bookmark/BookmarkModel.kt +++ b/app/src/main/java/com/quran/labs/androidquran/model/bookmark/BookmarkModel.kt @@ -95,7 +95,7 @@ open class BookmarkModel @Inject constructor( } bookmarksDBAdapter.bulkDelete(tagsToDelete, bookmarksToDelete, untag) bookmarksPublishSubject.onNext(true) - if (tagsToDelete.size > 0) { + if (tagsToDelete.isNotEmpty()) { tagPublishSubject.onNext(true) } null diff --git a/app/src/main/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriber.kt b/app/src/main/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriber.kt index 0c7fadfcec..96b5101e6c 100644 --- a/app/src/main/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriber.kt +++ b/app/src/main/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriber.kt @@ -1,8 +1,14 @@ package com.quran.labs.androidquran.widget +import com.quran.data.dao.BookmarksDao import com.quran.labs.androidquran.model.bookmark.BookmarkModel import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import io.reactivex.rxjava3.disposables.Disposable +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.MainScope +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import javax.inject.Inject import javax.inject.Singleton @@ -12,31 +18,38 @@ import javax.inject.Singleton */ @Singleton class BookmarksWidgetSubscriber @Inject constructor( - private val bookmarkModel: BookmarkModel, - private val bookmarksWidgetUpdater: BookmarksWidgetUpdater + private val bookmarksDao: BookmarksDao, + private val bookmarkModel: BookmarkModel, + private val bookmarksWidgetUpdater: BookmarksWidgetUpdater ) { - + private var scope: CoroutineScope = MainScope() private var bookmarksWidgetDisposable: Disposable? = null fun subscribeBookmarksWidgetIfNecessary() { if (bookmarksWidgetUpdater.checkForAnyBookmarksWidgets()) { subscribeBookmarksWidget() - } + } } private fun subscribeBookmarksWidget() { bookmarksWidgetDisposable = bookmarkModel.bookmarksObservable() - .observeOn(AndroidSchedulers.mainThread()) - .subscribe { bookmarksWidgetUpdater.updateBookmarksWidget() } + .observeOn(AndroidSchedulers.mainThread()) + .subscribe { bookmarksWidgetUpdater.updateBookmarksWidget() } } fun onEnabledBookmarksWidget() { if (bookmarksWidgetDisposable == null) { subscribeBookmarksWidget() } + + scope = MainScope() + bookmarksDao.changes + .onEach { bookmarksWidgetUpdater.updateBookmarksWidget() } + .launchIn(scope) } fun onDisabledBookmarksWidget() { bookmarksWidgetDisposable?.dispose() + scope.cancel() } } diff --git a/app/src/test/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriberTest.kt b/app/src/test/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriberTest.kt deleted file mode 100644 index bfd1118638..0000000000 --- a/app/src/test/java/com/quran/labs/androidquran/widget/BookmarksWidgetSubscriberTest.kt +++ /dev/null @@ -1,103 +0,0 @@ -package com.quran.labs.androidquran.widget - -import com.quran.labs.androidquran.model.bookmark.BookmarkModel -import io.reactivex.rxjava3.android.plugins.RxAndroidPlugins -import io.reactivex.rxjava3.schedulers.TestScheduler -import io.reactivex.rxjava3.subjects.PublishSubject -import java.util.concurrent.TimeUnit -import org.junit.AfterClass -import org.junit.BeforeClass -import org.junit.Rule -import org.junit.Test -import org.mockito.InjectMocks -import org.mockito.Mock -import org.mockito.Mockito.never -import org.mockito.Mockito.verify -import org.mockito.junit.MockitoJUnit -import org.mockito.junit.MockitoRule -import org.mockito.quality.Strictness -import org.mockito.Mockito.`when` as whenever - - -class BookmarksWidgetSubscriberTest { - - @get:Rule - val mockitoRule: MockitoRule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS) - - @Mock - private lateinit var bookmarkModel: BookmarkModel - - @Mock - private lateinit var bookmarksWidgetUpdater: BookmarksWidgetUpdater - - @InjectMocks - private lateinit var bookmarksWidgetSubscriber: BookmarksWidgetSubscriber - - @Test - fun testSubscribeBookmarksWidgetIfBookmarksWidgetsExist() { - whenever(bookmarksWidgetUpdater.checkForAnyBookmarksWidgets()).thenReturn(true) - val testSubject = PublishSubject.create().toSerialized() - whenever(bookmarkModel.bookmarksObservable()).thenReturn(testSubject.hide()) - - bookmarksWidgetSubscriber.subscribeBookmarksWidgetIfNecessary() - testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - - verify(bookmarksWidgetUpdater, never()).updateBookmarksWidget() - testSubject.onNext(true) - testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - verify(bookmarksWidgetUpdater).updateBookmarksWidget() - } - - @Test - fun testDontSubscribeBookmarksWidgetIfNoBookmarksWidgetsExist() { - whenever(bookmarksWidgetUpdater.checkForAnyBookmarksWidgets()).thenReturn(false) - - bookmarksWidgetSubscriber.subscribeBookmarksWidgetIfNecessary() - - verify(bookmarkModel, never()).bookmarksObservable() - } - - @Test - fun testSubscribeBookmarksWidgetIfWidgetGetsAdded() { - whenever(bookmarksWidgetUpdater.checkForAnyBookmarksWidgets()).thenReturn(false) - val testSubject = PublishSubject.create().toSerialized() - whenever(bookmarkModel.bookmarksObservable()).thenReturn(testSubject.hide()) - - bookmarksWidgetSubscriber.subscribeBookmarksWidgetIfNecessary() - bookmarksWidgetSubscriber.onEnabledBookmarksWidget() - - testSubject.onNext(true) - testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - verify(bookmarksWidgetUpdater).updateBookmarksWidget() - } - - @Test - fun testUnsubscribeBookmarksWidgetIfAllBookmarksWidgetsGetRemoved() { - whenever(bookmarksWidgetUpdater.checkForAnyBookmarksWidgets()).thenReturn(true) - val testSubject = PublishSubject.create().toSerialized() - whenever(bookmarkModel.bookmarksObservable()).thenReturn(testSubject.hide()) - - bookmarksWidgetSubscriber.subscribeBookmarksWidgetIfNecessary() - bookmarksWidgetSubscriber.onDisabledBookmarksWidget() - testSubject.onNext(true) - testScheduler.advanceTimeBy(1, TimeUnit.SECONDS) - verify(bookmarksWidgetUpdater, never()).updateBookmarksWidget() - } - - companion object { - - private val testScheduler = TestScheduler() - - @BeforeClass - @JvmStatic - fun setup() { - RxAndroidPlugins.setMainThreadSchedulerHandler { testScheduler } - } - - @AfterClass - @JvmStatic - fun tearDown() { - RxAndroidPlugins.reset() - } - } -} diff --git a/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarksDaoImpl.kt b/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarksDaoImpl.kt index 98680f91f8..e424605fb3 100644 --- a/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarksDaoImpl.kt +++ b/common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarksDaoImpl.kt @@ -11,10 +11,14 @@ import com.quran.mobile.bookmark.mapper.Mappers import com.quran.mobile.bookmark.mapper.convergeCommonlyTagged import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext import javax.inject.Inject +import javax.inject.Singleton +@Singleton class BookmarksDaoImpl @Inject constructor( bookmarksDatabase: BookmarksDatabase ) : BookmarksDao { @@ -22,6 +26,9 @@ class BookmarksDaoImpl @Inject constructor( private val lastPageQueries = bookmarksDatabase.lastPageQueries private val bookmarkTagQueries = bookmarksDatabase.bookmarkTagQueries + private val internalChanges = MutableStateFlow(null) + override val changes: Flow = internalChanges.filterNotNull() + override suspend fun bookmarks(): List { return withContext(Dispatchers.IO) { bookmarkQueries.getBookmarksByDateAdded(Mappers.bookmarkWithTagMapper) @@ -49,6 +56,7 @@ class BookmarksDaoImpl @Inject constructor( bookmarkQueries.update(it.sura, it.ayah, it.page, it.id) } } + internalChanges.value = System.currentTimeMillis() } } @@ -58,6 +66,7 @@ class BookmarksDaoImpl @Inject constructor( if (bookmarkId != null) { bookmarkTagQueries.deleteByBookmarkIds(listOf(bookmarkId)) bookmarkQueries.deleteByIds(listOf(bookmarkId)) + internalChanges.value = System.currentTimeMillis() } } } @@ -82,9 +91,11 @@ class BookmarksDaoImpl @Inject constructor( val bookmarkId = bookmarkIds.firstOrNull() if (bookmarkId != null) { deleteBookmarkById(bookmarkId) + internalChanges.value = System.currentTimeMillis() false } else { bookmarkQueries.addBookmark(null, null, page) + internalChanges.value = System.currentTimeMillis() true } } @@ -96,9 +107,11 @@ class BookmarksDaoImpl @Inject constructor( .executeAsOneOrNull() if (bookmarkId != null) { deleteBookmarkById(bookmarkId) + internalChanges.value = System.currentTimeMillis() false } else { bookmarkQueries.addBookmark(suraAyah.sura, suraAyah.ayah, page) + internalChanges.value = System.currentTimeMillis() true } } @@ -110,6 +123,7 @@ class BookmarksDaoImpl @Inject constructor( bookmarkTagQueries.deleteByBookmarkIds(listOf(bookmarkId)) bookmarkQueries.deleteByIds(listOf(bookmarkId)) } + internalChanges.value = System.currentTimeMillis() } } diff --git a/common/data/src/main/java/com/quran/data/dao/BookmarksDao.kt b/common/data/src/main/java/com/quran/data/dao/BookmarksDao.kt index b6f1fa70ef..a822582cc3 100644 --- a/common/data/src/main/java/com/quran/data/dao/BookmarksDao.kt +++ b/common/data/src/main/java/com/quran/data/dao/BookmarksDao.kt @@ -6,6 +6,8 @@ import com.quran.data.model.bookmark.RecentPage import kotlinx.coroutines.flow.Flow interface BookmarksDao { + val changes: Flow + suspend fun bookmarks(): List fun bookmarksForPage(page: Int): Flow> fun pageBookmarksWithoutTags(): Flow>