From 19a4f6109720b4e86203f1183965d4c4141fbe80 Mon Sep 17 00:00:00 2001 From: Brayan Oliveira <69634269+brayandso@users.noreply.github.com> Date: Sun, 26 May 2024 09:28:47 -0300 Subject: [PATCH] feat(new reviewer): handle custom scheduling ReviewerFragmentTest is basically a copy of ReviewerTest --- .../com/ichi2/anki/ReviewerFragmentTest.kt | 184 ++++++++++++++++++ .../anki/previewer/CardViewerFragment.kt | 7 + .../ui/windows/reviewer/ReviewerFragment.kt | 6 + .../ui/windows/reviewer/ReviewerViewModel.kt | 89 ++++++++- 4 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt new file mode 100644 index 000000000000..34f653feeaa1 --- /dev/null +++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt @@ -0,0 +1,184 @@ +/* + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 3 of the License, or (at your option) any later + * version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A + * PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ +package com.ichi2.anki + +import androidx.core.content.edit +import androidx.recyclerview.widget.RecyclerView +import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.action.ViewActions.click +import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.contrib.RecyclerViewActions +import androidx.test.espresso.matcher.ViewMatchers.hasDescendant +import androidx.test.espresso.matcher.ViewMatchers.isDisplayed +import androidx.test.espresso.matcher.ViewMatchers.withId +import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.ext.junit.rules.ActivityScenarioRule +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.ichi2.anki.preferences.sharedPrefs +import com.ichi2.anki.tests.InstrumentedTest +import com.ichi2.anki.testutil.GrantStoragePermission.storagePermission +import com.ichi2.anki.testutil.grantPermissions +import com.ichi2.anki.testutil.notificationPermission +import com.ichi2.libanki.Collection +import com.ichi2.testutils.Flaky +import com.ichi2.testutils.OS +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import java.util.concurrent.TimeUnit + +@RunWith(AndroidJUnit4::class) +class ReviewerFragmentTest : InstrumentedTest() { + + // Launch IntroductionActivity instead of DeckPicker activity because in CI + // builds, it seems to create IntroductionActivity after the DeckPicker, + // causing the DeckPicker activity to be destroyed. As a consequence, this + // will throw RootViewWithoutFocusException when Espresso tries to interact + // with an already destroyed activity. By launching IntroductionActivity, we + // ensure that IntroductionActivity is launched first and navigate to the + // DeckPicker -> Reviewer activities + @get:Rule + val activityScenarioRule = ActivityScenarioRule(IntroductionActivity::class.java) + + @get:Rule + val runtimePermissionRule = grantPermissions(storagePermission, notificationPermission) + + @Test + @Flaky(os = OS.ALL, "Fails on CI with timing issues frequently") + fun testCustomSchedulerWithCustomData() { + setNewReviewer() + col.cardStateCustomizer = + """ + states.good.normal.review.easeFactor = 3.0; + states.good.normal.review.scheduledDays = 123; + customData.good.c += 1; + """ + val note = addNoteUsingBasicModel("foo", "bar") + val card = note.firstCard(col) + val deck = col.decks.get(note.notetype.did)!! + card.moveToReviewQueue() + col.backend.updateCards( + listOf( + card.toBackendCard().toBuilder().setCustomData("""{"c":1}""").build() + ), + true + ) + + closeGetStartedScreenIfExists() + closeBackupCollectionDialogIfExists() + reviewDeckWithName(deck.name) + + var cardFromDb = col.getCard(card.id).toBackendCard() + assertThat(cardFromDb.easeFactor, equalTo(card.factor)) + assertThat(cardFromDb.interval, equalTo(card.ivl)) + assertThat(cardFromDb.customData, equalTo("""{"c":1}""")) + + clickShowAnswerAndAnswerGood() + + cardFromDb = col.getCard(card.id).toBackendCard() + assertThat(cardFromDb.easeFactor, equalTo(3000)) + assertThat(cardFromDb.interval, equalTo(123)) + assertThat(cardFromDb.customData, equalTo("""{"c":2}""")) + } + + @Test + @Flaky(os = OS.ALL, "Fails on CI with timing issues frequently") + fun testCustomSchedulerWithRuntimeError() { + setNewReviewer() + // Issue 15035 - runtime errors weren't handled + col.cardStateCustomizer = "states.this_is_not_defined.normal.review = 12;" + addNoteUsingBasicModel() + + closeGetStartedScreenIfExists() + closeBackupCollectionDialogIfExists() + reviewDeckWithName("Default") + + clickShowAnswer() + + ensureAnswerButtonsAreDisplayed() + } + + private fun closeGetStartedScreenIfExists() { + onView(withId(R.id.get_started)).withFailureHandler { _, _ -> }.perform(click()) + } + + private fun closeBackupCollectionDialogIfExists() { + onView(withText(R.string.button_backup_later)) + .withFailureHandler { _, _ -> } + .perform(click()) + } + + private fun clickOnDeckWithName(deckName: String) { + onView(withId(R.id.files)).checkWithTimeout(matches(hasDescendant(withText(deckName)))) + onView(withId(R.id.files)).perform( + RecyclerViewActions.actionOnItem( + hasDescendant(withText(deckName)), + click() + ) + ) + } + + private fun clickOnStudyButtonIfExists() { + onView(withId(R.id.studyoptions_start)) + .withFailureHandler { _, _ -> } + .perform(click()) + } + + private fun reviewDeckWithName(deckName: String) { + clickOnDeckWithName(deckName) + // Adding cards directly to the database while in the Deck Picker screen + // will not update the page with correct card counts. Hence, clicking + // on the deck will bring us to the study options page where we need to + // click on the Study button. If we have added cards to the database + // before the Deck Picker screen has fully loaded, then we skip clicking + // the Study button + clickOnStudyButtonIfExists() + } + + private fun clickShowAnswerAndAnswerGood() { + clickShowAnswer() + ensureAnswerButtonsAreDisplayed() + onView(withId(R.id.good_button)).perform(click()) + } + + private fun clickShowAnswer() { + onView(withId(R.id.show_answer)).perform(click()) + } + + private fun ensureAnswerButtonsAreDisplayed() { + // We need to wait for the card to fully load to allow enough time for + // the messages to be passed in and out of the WebView when evaluating + // the custom JS scheduler code. The ease buttons are hidden until the + // custom scheduler has finished running + onView(withId(R.id.good_button)).checkWithTimeout( + matches(isDisplayed()), + 100, + // Increase to a max of 30 seconds because CI builds can be very + // slow + TimeUnit.SECONDS.toMillis(30) + ) + } + + private fun setNewReviewer() { + testContext.sharedPrefs().edit { + putBoolean("newReviewer", true) + } + } +} + +private var Collection.cardStateCustomizer: String? + get() = config.get("cardStateCustomizer") + set(value) { config.set("cardStateCustomizer", value) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/previewer/CardViewerFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/previewer/CardViewerFragment.kt index c69ce62c9334..58bee5db325d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/previewer/CardViewerFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/previewer/CardViewerFragment.kt @@ -16,6 +16,7 @@ package com.ichi2.anki.previewer import android.content.Intent +import android.graphics.Bitmap import android.net.Uri import android.os.Bundle import android.view.View @@ -133,6 +134,12 @@ abstract class CardViewerFragment(@LayoutRes layout: Int) : Fragment(layout) { return resourceHandler.shouldInterceptRequest(request) } + override fun onPageStarted(view: WebView?, url: String?, favicon: Bitmap?) { + super.onPageStarted(view, url, favicon) + // TODO remove this after the backend is upgraded to v0.1.39 + view?.evaluateJavascript("globalThis.ankidroid = globalThis.ankidroid || {}; ankidroid.postBaseUrl = ``", null) + } + override fun onPageFinished(view: WebView?, url: String?) { viewModel.onPageFinished(isAfterRecreation = savedInstanceState != null) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt index 4a30002e34c4..849f4001ad61 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt @@ -88,6 +88,12 @@ class ReviewerFragment : } } } + + viewModel.statesMutationEval.collectIn(lifecycleScope) { eval -> + webView.evaluateJavascript(eval) { + viewModel.onStateMutationCallback() + } + } } // TODO diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt index f22ffcd4de6d..025a6656c938 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt @@ -18,20 +18,27 @@ package com.ichi2.anki.ui.windows.reviewer import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewmodel.initializer import androidx.lifecycle.viewmodel.viewModelFactory +import anki.frontend.SetSchedulingStatesRequest import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.Ease import com.ichi2.anki.asyncIO import com.ichi2.anki.cardviewer.CardMediaPlayer import com.ichi2.anki.launchCatchingIO +import com.ichi2.anki.pages.AnkiServer +import com.ichi2.anki.pages.PostRequestHandler import com.ichi2.anki.previewer.CardViewerViewModel import com.ichi2.anki.reviewer.CardSide import com.ichi2.libanki.sched.CurrentQueueState import com.ichi2.libanki.undoableOp +import com.ichi2.libanki.utils.TimeManager import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Deferred +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableSharedFlow -class ReviewerViewModel(cardMediaPlayer: CardMediaPlayer) : CardViewerViewModel(cardMediaPlayer) { +class ReviewerViewModel(cardMediaPlayer: CardMediaPlayer) : + CardViewerViewModel(cardMediaPlayer), + PostRequestHandler { private var queueState: Deferred = asyncIO { // this assumes that the Reviewer won't be launched if there isn't a queueState @@ -42,6 +49,25 @@ class ReviewerViewModel(cardMediaPlayer: CardMediaPlayer) : CardViewerViewModel( } var isQueueFinishedFlow = MutableSharedFlow() + private val server = AnkiServer(this).also { it.start() } + private val stateMutationKey = TimeManager.time.intTimeMS().toString() + val statesMutationEval = MutableSharedFlow() + + /** + * A flag that determines if the SchedulingStates in CurrentQueueState are + * safe to persist in the database when answering a card. This is used to + * ensure that the custom JS scheduler has persisted its SchedulingStates + * back to the Reviewer before we save it to the database. If the custom + * scheduler has not been configured, then it is safe to immediately set + * this to true. + * + * This flag should be set to false when we show the front of the card + * and only set to true once we know the custom scheduler has finished its + * execution, or set to true immediately if the custom scheduler has not + * been configured. + */ + private var statesMutated = true + /* ********************************************************************************************* ************************ Public methods: meant to be used by the View ************************** ********************************************************************************************* */ @@ -59,8 +85,13 @@ class ReviewerViewModel(cardMediaPlayer: CardMediaPlayer) : CardViewerViewModel( } } + override fun baseUrl(): String = server.baseUrl() + fun showAnswer() { launchCatchingIO { + while (!statesMutated) { + delay(50) + } showAnswerInternal() loadAndPlaySounds(CardSide.ANSWER) } @@ -71,10 +102,66 @@ class ReviewerViewModel(cardMediaPlayer: CardMediaPlayer) : CardViewerViewModel( fun answerGood() = answerCard(Ease.GOOD) fun answerEasy() = answerCard(Ease.EASY) + fun onStateMutationCallback() { + statesMutated = true + } + /* ********************************************************************************************* *************************************** Internal methods *************************************** ********************************************************************************************* */ + override suspend fun handlePostRequest(uri: String, bytes: ByteArray): ByteArray { + return if (uri.startsWith(AnkiServer.ANKI_PREFIX)) { + when (uri.substring(AnkiServer.ANKI_PREFIX.length)) { + "getSchedulingStatesWithContext" -> getSchedulingStatesWithContext() + "setSchedulingStates" -> setSchedulingStates(bytes) + else -> byteArrayOf() + } + } else { + byteArrayOf() + } + } + + override suspend fun showQuestion() { + super.showQuestion() + runStateMutationHook() + } + + private suspend fun runStateMutationHook() { + val state = queueState.await() ?: return + val js = state.customSchedulingJs + if (js.isEmpty()) { + statesMutated = true + return + } + statesMutated = false + statesMutationEval.emit( + "anki.mutateNextCardStates('$stateMutationKey', async (states, customData, ctx) => { $js });" + ) + } + + private suspend fun getSchedulingStatesWithContext(): ByteArray { + val state = queueState.await() ?: return ByteArray(0) + return state.schedulingStatesWithContext().toBuilder() + .mergeStates( + state.states.toBuilder().mergeCurrent( + state.states.current.toBuilder() + .setCustomData(state.topCard.toBackendCard().customData).build() + ).build() + ) + .build() + .toByteArray() + } + + private suspend fun setSchedulingStates(bytes: ByteArray): ByteArray { + val state = queueState.await() ?: return ByteArray(0) + val req = SetSchedulingStatesRequest.parseFrom(bytes) + if (req.key == stateMutationKey) { + state.states = req.states + } + return ByteArray(0) + } + private fun answerCard(ease: Ease) { launchCatchingIO { queueState.await()?.let {