Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #111: Introduce question progress controller implementation #397

Merged
Merged
Show file tree
Hide file tree
Changes from 79 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
9594954
Initial check in for the question data controller
vinitamurthi Oct 8, 2019
6ee7aec
Removed progress controller test
vinitamurthi Oct 8, 2019
848f172
Address the renames suggested in #217.
BenHenning Oct 9, 2019
174e2f4
Merge branch 'develop' into introduce-question-progress-controller-in…
BenHenning Oct 9, 2019
4147857
Introduce thoroughly stubbed QuestionAssessmentProgressController +
BenHenning Oct 9, 2019
4596e86
Review changes
vinitamurthi Oct 9, 2019
f5c23c8
Review changes
vinitamurthi Oct 10, 2019
53d367d
Pass data providers to the assessment controller, and set caps in the…
vinitamurthi Oct 11, 2019
86630b6
Fixed merge conflicts
vinitamurthi Oct 11, 2019
a8adf93
Removed unnecessary imports/variables
vinitamurthi Oct 11, 2019
a7a10f3
Add fake question data for stubbed interfaces
vinitamurthi Oct 11, 2019
459e3e9
Remove duplicate questions while fetching in training controller
vinitamurthi Oct 11, 2019
73c49df
Comment explaining the filter function
vinitamurthi Oct 11, 2019
91cf8e6
Improve duplicate checking - check it while filtering instead of afte…
vinitamurthi Oct 11, 2019
54546d7
Add linked skill id values
vinitamurthi Oct 11, 2019
be375fd
Review changes
vinitamurthi Oct 15, 2019
64b8265
add a new module for questions constants
vinitamurthi Oct 18, 2019
75405a2
fix merge conflicts
vinitamurthi Oct 18, 2019
4878d10
Review changes
vinitamurthi Oct 18, 2019
c37f14b
add a test to verify questions were fetched properly
vinitamurthi Oct 18, 2019
d1bd4a6
reformatted code
vinitamurthi Oct 18, 2019
1c5ca42
Merge branch 'question_data' into introduce-question-progress-control…
BenHenning Oct 20, 2019
86df525
Re-add QuestionTrainingController removed in merge.
BenHenning Oct 20, 2019
54340e6
Finalize question progress controller interface.
BenHenning Oct 20, 2019
6e37b25
Implement QuestionAssessmentProgressController.
BenHenning Oct 20, 2019
aa34f14
Add initial phase of tests for the progress controller based on
BenHenning Oct 28, 2019
543b734
Merge branch 'develop' into introduce-question-progress-controller-in…
BenHenning Oct 28, 2019
73372fe
Fix typo in QuestionTrainingController.
BenHenning Oct 28, 2019
8d29dd1
Post-merge fixes and adjustments.
BenHenning Oct 28, 2019
698e71d
Fix broken ProfileManagementControllerTest.
BenHenning Oct 28, 2019
0cf1371
Merge branch 'introduce-question-progress-controller-interface' into …
BenHenning Oct 28, 2019
f2ea1d2
Merge branch 'fix-broken-profile-management-controller-test' into int…
BenHenning Oct 28, 2019
1d2460e
Post-merge fixes.
BenHenning Oct 28, 2019
7288a51
Update tests to build, but not pass.
BenHenning Oct 28, 2019
fb38ba2
Merge branch 'develop' into introduce-question-progress-controller-im…
BenHenning Nov 26, 2019
8496db0
Merge branch 'develop' into introduce-question-progress-controller-im…
BenHenning Dec 1, 2019
da371eb
Fix question session not properly being reinitialized for new sessions.
BenHenning Dec 1, 2019
4ace331
Fix index accounting for questions. Before, the index incremented upon
BenHenning Dec 1, 2019
1cf2a42
Remove support for backward navigation in questions since it isn't
BenHenning Dec 1, 2019
5f09c80
Add support for a synthetic ephemeral question to represent the end of a
BenHenning Dec 1, 2019
9e4ad96
Merge branch 'develop' into introduce-question-progress-controller-im…
BenHenning Feb 21, 2020
9d70115
Finish tests for Question{Training,AssessmentProgress}Controllers.
BenHenning Feb 28, 2020
5c8471e
Initial introduction of test coroutine dispatchers to replace Kotlin's
BenHenning Mar 16, 2020
b13c965
Introduce a new LiveData mechanism to bridge coroutines from
BenHenning Mar 16, 2020
6aa3b32
Merge branch 'develop' into introduce-test-coroutine-dispatchers
BenHenning Mar 16, 2020
9f6656b
Merge branch 'introduce-test-coroutine-dispatchers' into introduce-ne…
BenHenning Mar 16, 2020
01c2b50
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning Mar 16, 2020
14a4e11
Introduce new nested data provider that effectively allows a
BenHenning Mar 16, 2020
c30a494
Merge branch 'introduce-nested-data-provider' into introduce-question…
BenHenning Mar 16, 2020
be1872f
Early work at introducing FakeSystemClock tests (not yet complete).
BenHenning Apr 1, 2020
9a4f9c2
Merge branch 'introduce-test-coroutine-dispatchers' into introduce-ne…
BenHenning Apr 1, 2020
c70e199
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning Apr 1, 2020
9fa54e8
Merge branch 'introduce-nested-data-provider' into introduce-question…
BenHenning Apr 1, 2020
eda11d0
Merge branch 'develop' into introduce-test-coroutine-dispatchers
BenHenning Apr 1, 2020
c704ce8
Merge branch 'introduce-test-coroutine-dispatchers' into introduce-ne…
BenHenning Apr 1, 2020
d1f9b63
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning Apr 1, 2020
c9ec7e5
Merge branch 'introduce-nested-data-provider' into introduce-question…
BenHenning Apr 1, 2020
8696fca
Merge branch 'develop' into introduce-test-coroutine-dispatchers
BenHenning May 9, 2020
7110b6c
Merge branch 'introduce-test-coroutine-dispatchers' into introduce-ne…
BenHenning May 10, 2020
15d6c07
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning May 10, 2020
bc5e43d
Merge branch 'introduce-nested-data-provider' into introduce-question…
BenHenning May 10, 2020
c581a57
Remove unnecessary meta file for Mockito.
BenHenning May 10, 2020
3959c01
Minor import cleanup.
BenHenning May 10, 2020
fba02ce
Remove infeasible testing structures, add documentation, and clean up
BenHenning May 20, 2020
9ea55b5
Add notice that the dispatchers utility is temporary.
BenHenning May 20, 2020
c6dbd22
Cleanup new LiveData bridge, add tests for it, and migrate other
BenHenning May 21, 2020
3a6f452
Merge branch 'introduce-test-coroutine-dispatchers' into introduce-ne…
BenHenning May 21, 2020
1245337
Add AsyncResult tests, fix & re-enable an earlier test in PersistentC…
BenHenning May 21, 2020
8ff8805
Use ktlint to reformat TestCoroutineDispatchers per reviewer comment
BenHenning May 21, 2020
e37eaa1
Merge branch 'introduce-test-coroutine-dispatchers' into introduce-ne…
BenHenning May 21, 2020
113fdeb
Reformat files failing linter check.
BenHenning May 21, 2020
7bacc7c
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning May 21, 2020
39553a8
Reformat new DataProviders code.
BenHenning May 21, 2020
f4bdeb6
Add new tests for the NestedTransformedDataProvider.
BenHenning May 21, 2020
7611cf2
Merge branch 'develop' into introduce-new-coroutine-live-data-mechanism
BenHenning May 28, 2020
8a2820a
Address reviewer comment.
BenHenning May 28, 2020
98ff4af
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning May 28, 2020
a4ac207
Merge branch 'introduce-nested-data-provider' into introduce-question…
BenHenning May 28, 2020
7dd108f
Address reviewer comments.
BenHenning May 28, 2020
bf056ec
Merge branch 'develop' into introduce-new-coroutine-live-data-mechanism
BenHenning May 28, 2020
91d4b19
Merge branch 'introduce-new-coroutine-live-data-mechanism' into intro…
BenHenning May 28, 2020
02c1c46
Merge branch 'develop' into introduce-nested-data-provider
BenHenning May 28, 2020
a0d662f
Merge branch 'introduce-nested-data-provider' into introduce-question…
BenHenning May 28, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions data/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ dependencies {
'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.2.2',
'org.mockito:mockito-core:2.19.0',
'org.robolectric:robolectric:4.3',
project(":testing"),
)
// TODO (#59): Remove this once Bazel is introduced
// TODO (#97): Isolate retrofit-mock dependency from production
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ class PersistentCacheStore<T : MessageLite> private constructor(
@GuardedBy("failureLock") private var deferredLoadCacheFailure: Throwable? = null
private val cache = cacheFactory.create(CachePayload(state = CacheState.UNLOADED, value = initialValue))

init {
cache.observeChanges {
asyncDataSubscriptionManager.notifyChange(providerId)
}
}

override fun getId(): Any {
return providerId
}
Expand All @@ -54,7 +60,7 @@ class PersistentCacheStore<T : MessageLite> private constructor(
cache.readIfPresentAsync().await().let { cachePayload ->
// First, determine whether the current cache has been attempted to be retrieved from disk.
if (cachePayload.state == CacheState.UNLOADED) {
deferLoadFileAndNotify()
deferLoadFile()
return AsyncResult.pending()
}

Expand Down Expand Up @@ -109,7 +115,6 @@ class PersistentCacheStore<T : MessageLite> private constructor(
val deferred = cache.updateWithCustomChannelIfPresentAsync { cachePayload ->
if (cachePayload.state == CacheState.UNLOADED) {
val filePayload = loadFileCache(cachePayload)
asyncDataSubscriptionManager.notifyChange(providerId)
Pair(filePayload, filePayload.value)
} else {
Pair(cachePayload, cachePayload.value)
Expand All @@ -135,9 +140,6 @@ class PersistentCacheStore<T : MessageLite> private constructor(
*/
fun storeDataAsync(updateInMemoryCache: Boolean = true, update: (T) -> T): Deferred<Any> {
return cache.updateIfPresentAsync { cachedPayload ->
// Although it's odd to notify before the change is made, the single threaded nature of the blocking cache ensures
// nothing can read from it until this update completes.
asyncDataSubscriptionManager.notifyChange(providerId)
val updatedPayload = storeFileCache(cachedPayload, update)
if (updateInMemoryCache) updatedPayload else cachedPayload
}
Expand All @@ -146,16 +148,13 @@ class PersistentCacheStore<T : MessageLite> private constructor(
/** See [storeDataAsync]. Stores data and allows for a custom deferred result. */
fun <V> storeDataWithCustomChannelAsync(updateInMemoryCache: Boolean = true, update: (T) -> Pair<T, V>): Deferred<V> {
return cache.updateWithCustomChannelIfPresentAsync { cachedPayload ->
// Although it's odd to notify before the change is made, the single threaded nature of the blocking cache ensures
// nothing can read from it until this update completes.
asyncDataSubscriptionManager.notifyChange(providerId)
val (updatedPayload, customResult) = storeFileCacheWithCustomChannel(cachedPayload, update)
if (updateInMemoryCache) Pair(updatedPayload, customResult) else Pair(cachedPayload, customResult)
}
}

/**
* Returns a [Deferred] indicating when the cache was cleared and its on-disk file, removed. This does not notify
* Returns a [Deferred] indicating when the cache was cleared and its on-disk file, removed. This does notify
* subscribers.
*/
fun clearCacheAsync(): Deferred<Any> {
Expand All @@ -172,10 +171,8 @@ class PersistentCacheStore<T : MessageLite> private constructor(
}
}

private fun deferLoadFileAndNotify() {
// Schedule another update to the cache that actually loads the file from memory. Record any potential failures.
private fun deferLoadFile() {
cache.updateIfPresentAsync { cachePayload ->
asyncDataSubscriptionManager.notifyChange(providerId)
loadFileCache(cachePayload)
}.invokeOnCompletion {
failureLock.withLock {
Expand Down

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions domain/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@ dependencies {
'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.2.2',
'org.mockito:mockito-core:2.19.0',
'org.robolectric:robolectric:4.3',
project(":testing"),
)
kapt(
'com.google.dagger:dagger-compiler:2.24'
'com.google.dagger:dagger-compiler:2.24',
)
kaptTest(
'com.google.dagger:dagger-compiler:2.24'
'com.google.dagger:dagger-compiler:2.24',
)
// TODO(#59): Avoid needing to expose data implementations to other modules in order to make Oppia symbols
// sufficiently visible for generated Dagger code. This can be done more cleanly via Bazel since dependencies can be
Expand Down
2 changes: 1 addition & 1 deletion domain/src/main/assets/sample_questions.json
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@
"defaultOutcome": {
"dest": null,
"feedback": {
"html": "",
"html": "Incorrect. Try again.",
"content_id": "default_outcome"
},
"labelled_as_correct": false,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
package org.oppia.domain.exploration

import org.oppia.app.model.Exploration
import org.oppia.app.model.State
import org.oppia.domain.state.StateDeck
import org.oppia.domain.state.StateGraph

// TODO(#186): Use an interaction repository to retrieve whether a specific ID corresponds to a terminal interaction.
private const val TERMINAL_INTERACTION_ID = "EndExploration"
vinitamurthi marked this conversation as resolved.
Show resolved Hide resolved

/**
* Private class that encapsulates the mutable state of an exploration progress controller. This class is not
* thread-safe, so owning classes should ensure synchronized access. This class can exist across multiple exploration
* instances, but calling code is responsible for ensuring it is properly reset.
*/
internal class ExplorationProgress {
internal lateinit var currentExplorationId: String
internal lateinit var currentExploration: Exploration
internal var playStage = PlayStage.NOT_PLAYING
internal val stateGraph: StateGraph by lazy {
StateGraph(currentExploration.statesMap)
}
internal val stateDeck: StateDeck by lazy {
StateDeck(stateGraph.getState(currentExploration.initStateName), ::isTopStateTerminal)
}

/**
* Advances the current play stage to the specified stage, verifying that the transition is correct.
*
* Calling code should prevent this method from failing by checking state ahead of calling this method and providing
* more useful errors to UI calling code since errors thrown by this method will be more obscure. This method aims to
* ensure the internal state of the controller remains correct. This method is not meant to be covered in unit tests
* since none of the failures here should ever be exposed to controller callers.
*/
internal fun advancePlayStageTo(nextPlayStage: PlayStage) {
when (nextPlayStage) {
PlayStage.NOT_PLAYING -> {
// All transitions to NOT_PLAYING are valid except itself. Stopping playing can happen at any time.
check(playStage != PlayStage.NOT_PLAYING) { "Cannot transition to NOT_PLAYING from NOT_PLAYING" }
playStage = nextPlayStage
}
PlayStage.LOADING_EXPLORATION -> {
// An exploration can only be requested to be loaded from the initial NOT_PLAYING stage.
check(playStage == PlayStage.NOT_PLAYING) { "Cannot transition to LOADING_EXPLORATION from $playStage" }
playStage = nextPlayStage
}
PlayStage.VIEWING_STATE -> {
// A state can be viewed after loading an exploration, after viewing another state, or after submitting an
// answer. It cannot be viewed without a loaded exploration.
check(playStage == PlayStage.LOADING_EXPLORATION
|| playStage == PlayStage.VIEWING_STATE
|| playStage == PlayStage.SUBMITTING_ANSWER) {
"Cannot transition to VIEWING_STATE from $playStage"
}
playStage = nextPlayStage
}
PlayStage.SUBMITTING_ANSWER -> {
// An answer can only be submitted after viewing a stage.
check(playStage == PlayStage.VIEWING_STATE) { "Cannot transition to SUBMITTING_ANSWER from $playStage" }
playStage = nextPlayStage
}
}
}

companion object {
internal fun isTopStateTerminal(state: State): Boolean {
return state.interaction.id == TERMINAL_INTERACTION_ID
}
}

/** Different stages in which the progress controller can exist. */
enum class PlayStage {
/** No exploration is currently being played. */
NOT_PLAYING,

/** An exploration is being prepared to be played. */
LOADING_EXPLORATION,

/** The controller is currently viewing a State. */
VIEWING_STATE,

/** The controller is in the process of submitting an answer. */
SUBMITTING_ANSWER
}
}
Loading