From 4e0d5777360cb4abdbb3a2c12cfae0cf70b7f8fd Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 24 Sep 2019 00:28:44 -0700 Subject: [PATCH] Fix #119: Introduce interface for StoryProgressController (#177) * Initial introduction of StoryProgressController & tests. * Address review comments. --- .../domain/topic/StoryProgressController.kt | 170 +++++++++++++ .../topic/StoryProgressControllerTest.kt | 233 ++++++++++++++++++ model/src/main/proto/topic.proto | 37 +++ 3 files changed, 440 insertions(+) create mode 100644 domain/src/main/java/org/oppia/domain/topic/StoryProgressController.kt create mode 100644 domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt create mode 100644 model/src/main/proto/topic.proto diff --git a/domain/src/main/java/org/oppia/domain/topic/StoryProgressController.kt b/domain/src/main/java/org/oppia/domain/topic/StoryProgressController.kt new file mode 100644 index 00000000000..175bd2aceca --- /dev/null +++ b/domain/src/main/java/org/oppia/domain/topic/StoryProgressController.kt @@ -0,0 +1,170 @@ +package org.oppia.domain.topic + +import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData +import javax.inject.Inject +import javax.inject.Singleton +import org.oppia.app.model.ChapterPlayState +import org.oppia.app.model.ChapterProgress +import org.oppia.app.model.StoryProgress +import org.oppia.util.data.AsyncResult + +const val TEST_STORY_ID_0 = "test_story_id_0" +const val TEST_STORY_ID_1 = "test_story_id_1" +const val TEST_STORY_ID_2 = "test_story_id_2" +const val TEST_EXPLORATION_ID_0 = "test_exp_id_0" +const val TEST_EXPLORATION_ID_1 = "test_exp_id_1" +const val TEST_EXPLORATION_ID_2 = "test_exp_id_2" +const val TEST_EXPLORATION_ID_3 = "test_exp_id_3" +const val TEST_EXPLORATION_ID_4 = "test_exp_id_4" + +/** Controller that records and provides completion statuses of chapters within the context of a story. */ +@Singleton +class StoryProgressController @Inject constructor() { + // TODO(#21): Determine whether chapters can have missing prerequisites in the initial prototype, or if that just + // indicates that they can't be started due to previous chapter not yet being completed. + + private val trackedStoriesProgress: Map by lazy { createInitialStoryProgressState() } + + /** + * Records the specified chapter completed within the context of the specified story. Returns a [LiveData] that + * provides exactly one [AsyncResult] to indicate whether this operation has succeeded. This method will never return + * a pending result. + */ + fun recordCompletedChapter(storyId: String, explorationId: String): LiveData> { + return try { + trackCompletedChapter(storyId, explorationId) + MutableLiveData(AsyncResult.success(null)) + } catch (e: Exception) { + MutableLiveData(AsyncResult.failed(e)) + } + } + + // TODO(#21): Implement notifying story progress changes when a chapter is recorded as complete, and add tests for + // this case. + + /** + * Returns a [LiveData] corresponding to the story progress of the specified story, or a failure if no such story can + * be identified. This [LiveData] will update as the story's progress changes. + */ + fun getStoryProgress(storyId: String): LiveData> { + return try { + MutableLiveData(AsyncResult.success(createStoryProgressSnapshot(storyId))) + } catch (e: Exception) { + MutableLiveData(AsyncResult.failed(e)) + } + } + + private fun trackCompletedChapter(storyId: String, explorationId: String) { + check(storyId in trackedStoriesProgress) { "No story found with ID: $storyId" } + trackedStoriesProgress.getValue(storyId).markChapterCompleted(explorationId) + } + + private fun createStoryProgressSnapshot(storyId: String): StoryProgress { + check(storyId in trackedStoriesProgress) { "No story found with ID: $storyId" } + return trackedStoriesProgress.getValue(storyId).toStoryProgress() + } + + private fun createInitialStoryProgressState(): Map { + return mapOf( + TEST_STORY_ID_0 to createStoryProgress0(), + TEST_STORY_ID_1 to createStoryProgress1(), + TEST_STORY_ID_2 to createStoryProgress2() + ) + } + + private fun createStoryProgress0(): TrackedStoryProgress { + return TrackedStoryProgress( + chapterList = listOf(TEST_EXPLORATION_ID_0), + completedChapters = setOf(TEST_EXPLORATION_ID_0) + ) + } + + private fun createStoryProgress1(): TrackedStoryProgress { + return TrackedStoryProgress( + chapterList = listOf(TEST_EXPLORATION_ID_1, TEST_EXPLORATION_ID_2, TEST_EXPLORATION_ID_3), + completedChapters = setOf(TEST_EXPLORATION_ID_1) + ) + } + + private fun createStoryProgress2(): TrackedStoryProgress { + return TrackedStoryProgress( + chapterList = listOf(TEST_EXPLORATION_ID_4), + completedChapters = setOf() + ) + } + + /** + * Mutable container for [StoryProgress] that provides support for determining whether a specific chapter can be + * played in the context of this story, marking a chapter as played, and converting to a [StoryProgress] object for + * reporting to the UI. + */ + private class TrackedStoryProgress(private val chapterList: List, completedChapters: Set) { + private val trackedCompletedChapters: MutableSet = completedChapters.toMutableSet() + + // TODO(#21): Implement tests for the following invariant checking logic, if possible. + init { + // Verify that the progress object is well-defined by ensuring that the invariant where lessons must be played in + // order holds. + var expectedCompleted: Boolean? = null + chapterList.reversed().forEach { explorationId -> + val completedChapter = explorationId in trackedCompletedChapters + val expectedCompletedSnapshot = expectedCompleted + if (expectedCompletedSnapshot == null) { + // This should always be initialized for the last lesson. If it's completed, all previous lessons must be + // completed. If it's not, then previous lessons may be completed or incomplete. + expectedCompleted = completedChapter + } else if (completedChapter != expectedCompletedSnapshot) { + // There's exactly one case where the expectation can change: if the next lesson is not completed. This means + // the current lesson is the most recent one completed in the list, and all previous lessons must also be + // completed. + check(!expectedCompletedSnapshot) { + "Expected lessons to be completed in order with no holes between them, and starting from the beginning " + + "of the story. Encountered uncompleted chapter right before a completed chapter: $explorationId" + } + // This is the first lesson that was completed after encountering one or more lessons that are not completed. + // All previous lessons in the list (the lessons next to be iterated) must be completed in order for the + // in-order invariant to hold. + expectedCompleted = true + } + // Otherwise, the invariant holds. Continue on to the previous lesson. + } + } + + /** + * Returns whether the specified exploration ID can be played, or if it's missing prerequisites. Fails if the + * specified exploration ID is not contained in this story. + */ + fun canPlayChapter(explorationId: String): Boolean { + // The chapter can be played only if it's the first one, or the chapter before it has been completed. + check(explorationId in chapterList) { "Chapter not found in story: $explorationId" } + val chapterIndex = chapterList.indexOf(explorationId) + return if (chapterIndex == 0) true else chapterList[chapterIndex - 1] in trackedCompletedChapters + } + + /** Marks the specified exploration ID as completed, or fails if the exploration is not contained in this story. */ + fun markChapterCompleted(explorationId: String) { + check(canPlayChapter(explorationId)) { "Cannot mark chapter as completed, missing prerequisites: $explorationId" } + trackedCompletedChapters.add(explorationId) + } + + /** Returns an immutable [StoryProgress] representation of this progress object. */ + fun toStoryProgress(): StoryProgress { + return StoryProgress.newBuilder() + .addAllChapterProgress(chapterList.map(this::buildChapterProgress)) + .build() + } + + private fun buildChapterProgress(explorationId: String): ChapterProgress { + val chapterPlayState = when { + explorationId in trackedCompletedChapters -> ChapterPlayState.COMPLETED + canPlayChapter(explorationId) -> ChapterPlayState.NOT_STARTED + else -> ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES /* Assume only reason is missing prerequisites. */ + } + return ChapterProgress.newBuilder() + .setExplorationId(explorationId) + .setPlayState(chapterPlayState) + .build() + } + } +} diff --git a/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt b/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt new file mode 100644 index 00000000000..f897b7672c2 --- /dev/null +++ b/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt @@ -0,0 +1,233 @@ +package org.oppia.domain.topic + +import android.app.Application +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import dagger.BindsInstance +import dagger.Component +import dagger.Module +import dagger.Provides +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.oppia.app.model.ChapterPlayState.COMPLETED +import org.oppia.app.model.ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES +import org.oppia.app.model.ChapterPlayState.NOT_STARTED +import org.robolectric.annotation.Config +import javax.inject.Inject +import javax.inject.Singleton + +/** Tests for [StoryProgressController]. */ +@RunWith(AndroidJUnit4::class) +@Config(manifest = Config.NONE) +class StoryProgressControllerTest { + @Inject + lateinit var storyProgressController: StoryProgressController + + @Before + fun setUp() { + setUpTestApplicationComponent() + } + + @Test + fun testGetStoryProgress_validStory_isSuccessful() { + val storyProgressLiveData = storyProgressController.getStoryProgress(TEST_STORY_ID_0) + + val storyProgressResult = storyProgressLiveData.value + assertThat(storyProgressResult).isNotNull() + assertThat(storyProgressResult!!.isSuccess()).isTrue() + } + + @Test + fun testGetStoryProgress_validStory_providesCorrectChapterProgress() { + val storyProgressLiveData = storyProgressController.getStoryProgress(TEST_STORY_ID_0) + + val storyProgress = storyProgressLiveData.value!!.getOrThrow() + assertThat(storyProgress.chapterProgressCount).isEqualTo(1) + assertThat(storyProgress.getChapterProgress(0).explorationId).isEqualTo(TEST_EXPLORATION_ID_0) + assertThat(storyProgress.getChapterProgress(0).playState).isEqualTo(COMPLETED) + } + + @Test + fun testGetStoryProgress_validSecondStory_providesCorrectChapterProgress() { + val storyProgressLiveData = storyProgressController.getStoryProgress(TEST_STORY_ID_1) + + // The third chapter should be missing prerequisites since chapter prior to it has yet to be completed. + val storyProgress = storyProgressLiveData.value!!.getOrThrow() + assertThat(storyProgress.chapterProgressCount).isEqualTo(3) + assertThat(storyProgress.getChapterProgress(0).explorationId).isEqualTo(TEST_EXPLORATION_ID_1) + assertThat(storyProgress.getChapterProgress(0).playState).isEqualTo(COMPLETED) + assertThat(storyProgress.getChapterProgress(1).explorationId).isEqualTo(TEST_EXPLORATION_ID_2) + assertThat(storyProgress.getChapterProgress(1).playState).isEqualTo(NOT_STARTED) + assertThat(storyProgress.getChapterProgress(2).explorationId).isEqualTo(TEST_EXPLORATION_ID_3) + assertThat(storyProgress.getChapterProgress(2).playState).isEqualTo(NOT_PLAYABLE_MISSING_PREREQUISITES) + } + + @Test + fun testGetStoryProgress_validThirdStory_providesCorrectChapterProgress() { + val storyProgressLiveData = storyProgressController.getStoryProgress(TEST_STORY_ID_2) + + val storyProgress = storyProgressLiveData.value!!.getOrThrow() + assertThat(storyProgress.chapterProgressCount).isEqualTo(1) + assertThat(storyProgress.getChapterProgress(0).explorationId).isEqualTo(TEST_EXPLORATION_ID_4) + assertThat(storyProgress.getChapterProgress(0).playState).isEqualTo(NOT_STARTED) + } + + @Test + fun testGetStoryProgress_invalidStory_providesError() { + val storyProgressLiveData = storyProgressController.getStoryProgress("invalid_story_id") + + val storyProgressResult = storyProgressLiveData.value + assertThat(storyProgressResult).isNotNull() + assertThat(storyProgressResult!!.isFailure()).isTrue() + assertThat(storyProgressResult.getErrorOrNull()) + .hasMessageThat() + .contains("No story found with ID: invalid_story_id") + } + + @Test + fun testRecordCompletedChapter_validStory_validChapter_alreadyCompleted_succeeds() { + val recordProgressLiveData = storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, TEST_EXPLORATION_ID_1) + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isSuccess()).isTrue() + } + + @Test + fun testRecordCompletedChapter_validStory_validChapter_alreadyCompleted_keepsChapterAsCompleted() { + storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, TEST_EXPLORATION_ID_1) + + val storyProgress = storyProgressController.getStoryProgress(TEST_STORY_ID_1).value!!.getOrThrow() + assertThat(storyProgress.getChapterProgress(0).explorationId).isEqualTo(TEST_EXPLORATION_ID_1) + assertThat(storyProgress.getChapterProgress(0).playState).isEqualTo(COMPLETED) + } + + @Test + fun testRecordCompletedChapter_validStory_validChapter_notYetCompleted_succeeds() { + val recordProgressLiveData = storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, TEST_EXPLORATION_ID_2) + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isSuccess()).isTrue() + } + + @Test + fun testRecordCompletedChapter_validStory_validChapter_notYetCompleted_marksChapterAsCompleted() { + storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, TEST_EXPLORATION_ID_2) + + val storyProgress = storyProgressController.getStoryProgress(TEST_STORY_ID_1).value!!.getOrThrow() + assertThat(storyProgress.getChapterProgress(1).explorationId).isEqualTo(TEST_EXPLORATION_ID_2) + assertThat(storyProgress.getChapterProgress(1).playState).isEqualTo(COMPLETED) + } + + @Test + fun testRecordCompletedChapter_validStory_validChapter_missingPrereqs_fails() { + val recordProgressLiveData = storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, TEST_EXPLORATION_ID_3) + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isFailure()).isTrue() + assertThat(recordProgressResult.getErrorOrNull()) + .hasMessageThat() + .contains("Cannot mark chapter as completed, missing prerequisites: $TEST_EXPLORATION_ID_3") + } + + @Test + fun testRecordCompletedChapter_validStory_validChapter_missingPrereqs_keepsChapterMissingPrereqs() { + storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, TEST_EXPLORATION_ID_3) + + val storyProgress = storyProgressController.getStoryProgress(TEST_STORY_ID_1).value!!.getOrThrow() + assertThat(storyProgress.getChapterProgress(2).explorationId).isEqualTo(TEST_EXPLORATION_ID_3) + assertThat(storyProgress.getChapterProgress(2).playState).isEqualTo(NOT_PLAYABLE_MISSING_PREREQUISITES) + } + + @Test + fun testRecordCompletedChapter_validStory_invalidChapter_fails() { + val recordProgressLiveData = storyProgressController.recordCompletedChapter(TEST_STORY_ID_1, "invalid_exp_id") + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isFailure()).isTrue() + assertThat(recordProgressResult.getErrorOrNull()) + .hasMessageThat() + .contains("Chapter not found in story: invalid_exp_id") + } + + @Test + fun testRecordCompletedChapter_validSecondStory_validChapter_notYetCompleted_succeeds() { + val recordProgressLiveData = storyProgressController.recordCompletedChapter(TEST_STORY_ID_2, TEST_EXPLORATION_ID_4) + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isSuccess()).isTrue() + } + + @Test + fun testRecordCompletedChapter_validSecondStory_validChapter_notYetCompleted_marksChapterAsCompleted() { + storyProgressController.recordCompletedChapter(TEST_STORY_ID_2, TEST_EXPLORATION_ID_4) + + val storyProgress = storyProgressController.getStoryProgress(TEST_STORY_ID_2).value!!.getOrThrow() + assertThat(storyProgress.getChapterProgress(0).explorationId).isEqualTo(TEST_EXPLORATION_ID_4) + assertThat(storyProgress.getChapterProgress(0).playState).isEqualTo(COMPLETED) + } + + @Test + fun testRecordCompletedChapter_validSecondStory_validChapterInOtherStory_fails() { + val recordProgressLiveData = storyProgressController.recordCompletedChapter(TEST_STORY_ID_2, TEST_EXPLORATION_ID_3) + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isFailure()).isTrue() + assertThat(recordProgressResult.getErrorOrNull()) + .hasMessageThat() + .contains("Chapter not found in story: $TEST_EXPLORATION_ID_3") + } + + @Test + fun testRecordCompletedChapter_invalidStory_fails() { + val recordProgressLiveData = + storyProgressController.recordCompletedChapter("invalid_story_id", TEST_EXPLORATION_ID_0) + + val recordProgressResult = recordProgressLiveData.value + assertThat(recordProgressResult).isNotNull() + assertThat(recordProgressResult!!.isFailure()).isTrue() + assertThat(recordProgressResult.getErrorOrNull()) + .hasMessageThat() + .contains("No story found with ID: invalid_story_id") + } + + private fun setUpTestApplicationComponent() { + DaggerStoryProgressControllerTest_TestApplicationComponent.builder() + .setApplication(ApplicationProvider.getApplicationContext()) + .build() + .inject(this) + } + + // TODO(#89): Move this to a common test application component. + @Module + class TestModule { + @Provides + @Singleton + fun provideContext(application: Application): Context { + return application + } + } + + // TODO(#89): Move this to a common test application component. + @Singleton + @Component(modules = [TestModule::class]) + interface TestApplicationComponent { + @Component.Builder + interface Builder { + @BindsInstance + fun setApplication(application: Application): Builder + + fun build(): TestApplicationComponent + } + + fun inject(storyProgressControllerTest: StoryProgressControllerTest) + } +} diff --git a/model/src/main/proto/topic.proto b/model/src/main/proto/topic.proto new file mode 100644 index 00000000000..d1e1d626136 --- /dev/null +++ b/model/src/main/proto/topic.proto @@ -0,0 +1,37 @@ +syntax = "proto3"; + +package model; + +option java_package = "org.oppia.app.model"; +option java_multiple_files = true; + +// Represents the play state of a single chapter. +enum ChapterPlayState { + // The completion status is unknown. + COMPLETION_STATUS_UNSPECIFIED = 0; + + // The chapter has not yet been started, but can be started by the player. + NOT_STARTED = 1; + + // The chapter has not yet been started, and cannot be started since the player is missing prerequisites. + NOT_PLAYABLE_MISSING_PREREQUISITES = 2; + + // The chapter has been completed by the player. + COMPLETED = 3; +} + +// Represents the progress a player has made for a story. +message StoryProgress { + // Represents the progress a learner has made on a single chapter. These chapters are kept in the same order they + // should be completed within the story. + repeated ChapterProgress chapter_progress = 1; +} + +// Represents the progress a player has made on a single chapter. +message ChapterProgress { + // The exploration ID of the chapter with possible progress made. + string exploration_id = 1; + + // Corresponds to whether this chapter is playable or has been started. + ChapterPlayState play_state = 2; +}