Skip to content

Commit

Permalink
chore: Remove col from class Card
Browse files Browse the repository at this point in the history
This requires adding `col` to any function using it, recursively, until finding another function with `col` in its context.

`Reviewer.onResume` has moved to `launchWithCol`, as otherwise tests crash
with the following trace:

```
java.lang.NullPointerException
	at com.ichi2.anki.AnkiActivity.getCol(AnkiActivity.kt:151)
	at com.ichi2.anki.Reviewer.onResume(Reviewer.kt:191)
	at android.app.Instrumentation.callActivityOnResume(Instrumentation.java:1476)
	at org.robolectric.android.internal.RoboMonitoringInstrumentation.callActivityOnResume(RoboMonitoringInstrumentation.java:321)
	at android.app.Activity.performResume(Activity.java:8191)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.robolectric.shadows._Activity_$$Reflector19.performResume(Unknown Source)
	at org.robolectric.android.controller.ActivityController.lambda$resume$5(ActivityController.java:197)
	at org.robolectric.shadows.ShadowPausedLooper.runPaused(ShadowPausedLooper.java:204)
	at org.robolectric.android.controller.ComponentController.invokeWhilePaused(ComponentController.java:64)
	at org.robolectric.android.controller.ActivityController.resume(ActivityController.java:192)
	at org.robolectric.android.internal.RoboMonitoringInstrumentation.startActivitySyncInternal(RoboMonitoringInstrumentation.java:121)
	at org.robolectric.android.internal.LocalActivityInvoker.startActivity(LocalActivityInvoker.java:35)
	at org.robolectric.android.internal.LocalActivityInvoker.startActivity(LocalActivityInvoker.java:40)
	at androidx.test.core.app.ActivityScenario.launchInternal(ActivityScenario.java:362)
	at androidx.test.core.app.ActivityScenario.launch(ActivityScenario.java:202)
	at com.ichi2.anki.ReviewerTest.verifyStartupNoCollection(ReviewerTest.kt:67)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.robolectric.RobolectricTestRunner$HelperTestRunner$1.evaluate(RobolectricTestRunner.java:589)
	at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$2(SandboxTestRunner.java:290)
	at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:99)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
	Suppressed:
        org.robolectric.android.internal.AndroidTestEnvironment$UnExecutedRunnablesException:
        Main looper has queued unexecuted runnables. This might be the cause of
        the test failure. You might need a
        shadowOf(Looper.getMainLooper()).idle() call.
```

In the worst case, this slightly delays when the time starts again, which is acceptable.

----

Revived from 5f01180

Co-authored-by: Arthur Milchior <[email protected]>
  • Loading branch information
david-allison and Arthur-Milchior committed Jan 17, 2024
1 parent 274a6bc commit 65c9f91
Show file tree
Hide file tree
Showing 30 changed files with 241 additions and 215 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ class ContentProviderTest : InstrumentedTest() {
// get the first card due
// ----------------------
val card = getFirstCardFromScheduler(col)
val note = card!!.note()
val note = card!!.note(col)
val noteId = note.id

// make sure the tag is what we expect initially
Expand Down Expand Up @@ -1312,7 +1312,7 @@ class ContentProviderTest : InstrumentedTest() {
)
for (c in newNote.cards()) {
c.did = did
c.col.updateCard(c, skipUndoEntry = true)
col.updateCard(c, skipUndoEntry = true)
}
return Uri.withAppendedPath(
FlashCardsContract.Note.CONTENT_URI,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ abstract class InstrumentedTest {
this.queue = Consts.QUEUE_TYPE_REV
this.type = Consts.CARD_TYPE_REV
this.due = 0
this.col.updateCard(this, true)
col.updateCard(this, true)
}

@DuplicatedCode("This is copied from RobolectricTest. This will be refactored into a shared library later")
Expand Down
37 changes: 19 additions & 18 deletions AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ abstract class AbstractFlashcardViewer :
val card = editorCard!!
withProgress {
undoableOp {
updateNote(card.note())
updateNote(card.note(this))
}
}
onCardUpdated(card)
Expand Down Expand Up @@ -477,7 +477,7 @@ abstract class AbstractFlashcardViewer :
// despite that making no sense outside of Reviewer.kt
currentCard = withCol {
sched.card?.apply {
renderOutput()
renderOutput(this@withCol)
}
}
}
Expand Down Expand Up @@ -824,7 +824,7 @@ abstract class AbstractFlashcardViewer :
message(
text = resources.getString(
R.string.delete_note_message,
Utils.stripHTML(currentCard!!.question(true))
Utils.stripHTML(currentCard!!.question(getColUnsafe, true))
)
)
positiveButton(R.string.dialog_positive_delete) {
Expand Down Expand Up @@ -1308,15 +1308,15 @@ abstract class AbstractFlashcardViewer :
backButtonPressedToReturn = false
setInterface()
typeAnswer?.input = ""
typeAnswer?.updateInfo(currentCard!!, resources)
typeAnswer?.updateInfo(getColUnsafe, currentCard!!, resources)
if (typeAnswer?.validForEditText() == true) {
// Show text entry based on if the user wants to write the answer
answerField?.visibility = View.VISIBLE
answerField?.applyLanguageHint(typeAnswer?.languageHint)
} else {
answerField?.visibility = View.GONE
}
val content = htmlGenerator!!.generateHtml(currentCard!!, Side.FRONT)
val content = htmlGenerator!!.generateHtml(getColUnsafe, currentCard!!, Side.FRONT)
automaticAnswer.onDisplayQuestion()
updateCard(content)
hideEaseButtons()
Expand All @@ -1342,7 +1342,7 @@ abstract class AbstractFlashcardViewer :

// TODO needs testing: changing a card's model without flipping it back to the front
// (such as editing a card, then editing the card template)
typeAnswer!!.updateInfo(currentCard!!, resources)
typeAnswer!!.updateInfo(getColUnsafe, currentCard!!, resources)

// Explicitly hide the soft keyboard. It *should* be hiding itself automatically,
// but sometimes failed to do so (e.g. if an OnKeyListener is attached).
Expand All @@ -1357,7 +1357,7 @@ abstract class AbstractFlashcardViewer :
typeAnswer!!.input = answerField!!.text.toString()
}
isSelecting = false
val answerContent = htmlGenerator!!.generateHtml(currentCard!!, Side.BACK)
val answerContent = htmlGenerator!!.generateHtml(getColUnsafe, currentCard!!, Side.BACK)
automaticAnswer.onDisplayAnswer()
updateCard(answerContent)
displayAnswerBottomBar()
Expand Down Expand Up @@ -1413,7 +1413,7 @@ abstract class AbstractFlashcardViewer :
Timber.d("updateCard()")
// TODO: This doesn't need to be blocking
runBlocking {
soundPlayer.loadCardSounds(currentCard!!, if (displayAnswer) Side.BACK else Side.FRONT)
soundPlayer.loadCardSounds(getColUnsafe, currentCard!!, if (displayAnswer) Side.BACK else Side.FRONT)
}
cardContent = content.getTemplateHtml()
fillFlashcard()
Expand Down Expand Up @@ -1464,8 +1464,8 @@ abstract class AbstractFlashcardViewer :

@VisibleForTesting
fun readCardTts(side: SingleSoundSide) {
val tags = legacyGetTtsTags(currentCard!!, side, this)
tts.readCardText(tags, currentCard!!, side.toSoundSide())
val tags = legacyGetTtsTags(getColUnsafe, currentCard!!, side, this)
tts.readCardText(getColUnsafe, tags, currentCard!!, side.toSoundSide())
}

/**
Expand All @@ -1486,6 +1486,7 @@ abstract class AbstractFlashcardViewer :
protected fun showSelectTtsDialogue() {
if (ttsInitialized) {
tts.selectTts(
getColUnsafe,
this,
currentCard!!,
if (displayAnswer) SoundSide.ANSWER else SoundSide.QUESTION
Expand Down Expand Up @@ -1823,22 +1824,22 @@ abstract class AbstractFlashcardViewer :
* @see refreshIfRequired - calls through to [updateCurrentCard]
*/
private fun reloadWebViewContent() {
currentCard?.renderOutput(reload = true, browser = false)
currentCard?.renderOutput(getColUnsafe, reload = true, browser = false)
if (!isDisplayingAnswer) {
Timber.d("displayCardQuestion()")
displayAnswer = false
backButtonPressedToReturn = false
setInterface()
typeAnswer?.input = ""
typeAnswer?.updateInfo(currentCard!!, resources)
typeAnswer?.updateInfo(getColUnsafe, currentCard!!, resources)
if (typeAnswer?.validForEditText() == true) {
// Show text entry based on if the user wants to write the answer
answerField?.visibility = View.VISIBLE
answerField?.applyLanguageHint(typeAnswer?.languageHint)
} else {
answerField?.visibility = View.GONE
}
val content = htmlGenerator!!.generateHtml(currentCard!!, Side.FRONT)
val content = htmlGenerator!!.generateHtml(getColUnsafe, currentCard!!, Side.FRONT)
automaticAnswer.onDisplayQuestion()
updateCard(content)
hideEaseButtons()
Expand Down Expand Up @@ -2143,7 +2144,7 @@ abstract class AbstractFlashcardViewer :
}

protected open fun shouldDisplayMark(): Boolean {
return isMarked(currentCard!!.note())
return isMarked(currentCard!!.note(getColUnsafe))
}

val writeLock: Lock
Expand Down Expand Up @@ -2435,7 +2436,7 @@ abstract class AbstractFlashcardViewer :
@NeedsTest("14221: 'playsound' should play the sound from the start")
@BlocksSchemaUpgrade("handle TTS tags")
private suspend fun controlSound(url: String) {
val avTag = when (val tag = currentCard?.let { getAvTag(it, url) }) {
val avTag = when (val tag = currentCard?.let { getAvTag(getColUnsafe, it, url) }) {
is SoundOrVideoTag -> tag
is TTSTag -> tag
// not currently supported
Expand Down Expand Up @@ -2513,7 +2514,7 @@ abstract class AbstractFlashcardViewer :

internal fun showTagsDialog() {
val tags = ArrayList(getColUnsafe.tags.all())
val selTags = ArrayList(currentCard!!.note().tags)
val selTags = ArrayList(currentCard!!.note(getColUnsafe).tags)
val dialog = tagsDialogFactory!!.newTagsDialog()
.withArguments(TagsDialog.DialogType.EDIT_TAGS, selTags, tags)
showDialogFragment(dialog)
Expand All @@ -2525,9 +2526,9 @@ abstract class AbstractFlashcardViewer :
indeterminateTags: List<String>,
stateFilter: CardStateFilter
) {
if (currentCard!!.note().tags != selectedTags) {
if (currentCard!!.note(getColUnsafe).tags != selectedTags) {
val tagString = selectedTags.joinToString(" ")
val note = currentCard!!.note()
val note = currentCard!!.note(getColUnsafe)
note.setTagsFromStr(tagString)
note.flush()
// Reload current card to reflect tag changes
Expand Down
10 changes: 7 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.ichi2.anki.servicelayer.resetCards
import com.ichi2.anki.snackbar.setMaxLines
import com.ichi2.anki.snackbar.showSnackbar
import com.ichi2.libanki.Card
import com.ichi2.libanki.Collection
import com.ichi2.libanki.Decks
import com.ichi2.libanki.SortOrder
import com.ichi2.utils.NetworkUtils
Expand All @@ -53,6 +54,9 @@ open class AnkiDroidJsAPI(private val activity: AbstractFlashcardViewer) {
private val currentCard: Card
get() = activity.currentCard!!

private val getColUnsafe: Collection
get() = activity.getColUnsafe

/**
Javascript Interface class for calling Java function from AnkiDroid WebView
see js-api.js for available functions
Expand Down Expand Up @@ -226,7 +230,7 @@ open class AnkiDroidJsAPI(private val activity: AbstractFlashcardViewer) {
activity.launchCatchingTask { activity.resetCards(cardIds) }
convertToByteArray(apiContract, true)
}
"cardMark" -> convertToByteArray(apiContract, currentCard.note().hasTag("marked"))
"cardMark" -> convertToByteArray(apiContract, currentCard.note(getColUnsafe).hasTag("marked"))
"cardFlag" -> convertToByteArray(apiContract, currentCard.userFlag())
"cardReps" -> convertToByteArray(apiContract, currentCard.reps)
"cardInterval" -> convertToByteArray(apiContract, currentCard.ivl)
Expand Down Expand Up @@ -355,8 +359,8 @@ open class AnkiDroidJsAPI(private val activity: AbstractFlashcardViewer) {
val searchResult: MutableList<String> = ArrayList()
for (s in cards) {
val jsonObject = JSONObject()
val fieldsData = s.card.note().fields
val fieldsName = s.card.model().fieldsNames
val fieldsData = s.card.note(getColUnsafe).fields
val fieldsName = s.card.model(getColUnsafe).fieldsNames

val noteId = s.card.nid
val cardId = s.card.id
Expand Down
37 changes: 19 additions & 18 deletions AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1300,7 +1300,7 @@ open class CardBrowser :
}
val allTags = getColUnsafe.tags.all()
val selectedNotes = selectedCardIds
.map { cardId: CardId? -> getColUnsafe.getCard(cardId!!).note() }
.map { cardId: CardId? -> getColUnsafe.getCard(cardId!!).note(getColUnsafe) }
.distinct()
val checkedTags = selectedNotes
.flatMap { note: Note -> note.tags }
Expand Down Expand Up @@ -1514,7 +1514,7 @@ open class CardBrowser :
private suspend fun editSelectedCardsTags(selectedTags: List<String>, indeterminateTags: List<String>) = withProgress {
undoableOp {
val selectedNotes = selectedCardIds
.map { cardId -> getCard(cardId).note() }
.map { cardId -> getCard(cardId).note(this) }
.distinct()
.onEach { note ->
val previousTags: List<String> = note.tags
Expand Down Expand Up @@ -1554,7 +1554,7 @@ open class CardBrowser :
val card = cardBrowserCard!!
withProgress {
undoableOp {
updateNote(card.note())
updateNote(card.note(this))
}
}
updateCardInList(card)
Expand Down Expand Up @@ -1965,7 +1965,7 @@ open class CardBrowser :
6 -> R.attr.flagTurquoise
7 -> R.attr.flagPurple
else -> {
if (isMarked(card.note())) {
if (isMarked(card.note(col))) {
R.attr.markedColor
} else if (card.queue == Consts.QUEUE_TYPE_SUSPENDED) {
R.attr.suspendedColor
Expand All @@ -1980,20 +1980,20 @@ open class CardBrowser :
return when (key) {
CardBrowserColumn.FLAGS -> Integer.valueOf(card.userFlag()).toString()
CardBrowserColumn.SUSPENDED -> if (card.queue == Consts.QUEUE_TYPE_SUSPENDED) "True" else "False"
CardBrowserColumn.MARKED -> if (isMarked(card.note())) "marked" else null
CardBrowserColumn.SFLD -> card.note().sFld()
CardBrowserColumn.MARKED -> if (isMarked(card.note(col))) "marked" else null
CardBrowserColumn.SFLD -> card.note(col).sFld()
CardBrowserColumn.DECK -> col.decks.name(card.did)
CardBrowserColumn.TAGS -> card.note().stringTags()
CardBrowserColumn.CARD -> if (inCardMode) card.template().optString("name") else "${card.note().numberOfCards()}"
CardBrowserColumn.DUE -> card.dueString()
CardBrowserColumn.TAGS -> card.note(col).stringTags()
CardBrowserColumn.CARD -> if (inCardMode) card.template(col).optString("name") else "${card.note(col).numberOfCards()}"
CardBrowserColumn.DUE -> card.dueString(col)
CardBrowserColumn.EASE -> if (inCardMode) getEaseForCards() else getAvgEaseForNotes()
CardBrowserColumn.CHANGED -> LanguageUtil.getShortDateFormatFromS(if (inCardMode) card.mod else card.note().mod.toLong())
CardBrowserColumn.CHANGED -> LanguageUtil.getShortDateFormatFromS(if (inCardMode) card.mod else card.note(col).mod.toLong())
CardBrowserColumn.CREATED -> LanguageUtil.getShortDateFormatFromMs(card.nid)
CardBrowserColumn.EDITED -> LanguageUtil.getShortDateFormatFromS(card.note().mod)
CardBrowserColumn.EDITED -> LanguageUtil.getShortDateFormatFromS(card.note(col).mod)
CardBrowserColumn.INTERVAL -> if (inCardMode) queryIntervalForCards() else queryAvgIntervalForNotes()
CardBrowserColumn.LAPSES -> (if (inCardMode) card.lapses else card.totalLapsesOfNote()).toString()
CardBrowserColumn.NOTE_TYPE -> card.model().optString("name")
CardBrowserColumn.REVIEWS -> if (inCardMode) card.reps.toString() else card.totalReviewsForNote().toString()
CardBrowserColumn.LAPSES -> (if (inCardMode) card.lapses else card.totalLapsesOfNote(col)).toString()
CardBrowserColumn.NOTE_TYPE -> card.model(col).optString("name")
CardBrowserColumn.REVIEWS -> if (inCardMode) card.reps.toString() else card.totalReviewsForNote(col).toString()
CardBrowserColumn.QUESTION -> {
updateSearchItemQA()
mQa!!.first
Expand All @@ -2015,7 +2015,7 @@ open class CardBrowser :
}

private fun getAvgEaseForNotes(): String {
val avgEase = card.avgEaseOfNote()
val avgEase = card.avgEaseOfNote(col)

return if (avgEase == null) {
AnkiDroidApp.instance.getString(R.string.card_browser_interval_new_card)
Expand All @@ -2033,7 +2033,7 @@ open class CardBrowser :
}

private fun queryAvgIntervalForNotes(): String {
val avgInterval = card.avgIntervalOfNote()
val avgInterval = card.avgIntervalOfNote(col)

return if (avgInterval == null) {
"" // upstream does not display interval for notes with new or learning cards
Expand All @@ -2048,7 +2048,7 @@ open class CardBrowser :
if (reload) {
reload()
}
card.note()
card.note(col)
// First column can not be the answer. If it were to change, this code should also be changed.
if (COLUMN1_KEYS[column1Index] == CardBrowserColumn.QUESTION || arrayOf(CardBrowserColumn.QUESTION, CardBrowserColumn.ANSWER).contains(COLUMN2_KEYS[column2Index])) {
updateSearchItemQA()
Expand All @@ -2066,10 +2066,11 @@ open class CardBrowser :
return
}
// render question and answer
val qa = card.renderOutput(reload = true, browser = true)
val qa = card.renderOutput(col, reload = true, browser = true)
// Render full question / answer if the bafmt (i.e. "browser appearance") setting forced blank result
if (qa.questionText.isEmpty() || qa.answerText.isEmpty()) {
val (questionText, answerText) = card.renderOutput(
col,
reload = true,
browser = false
)
Expand Down
Loading

0 comments on commit 65c9f91

Please sign in to comment.