From 48e01a8c1a553131df291f84e55ad30d11a2f60b Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 12 Aug 2020 18:27:45 -0700 Subject: [PATCH] Show a dialog when preloading assets for offline support. This does a bunch of refactoring & hacky workarounds to keep asset priming support fully isolated (just a few files now need to be deleted to clean things up). This also: 1. Fixes actual asset downloading (the GCS asset path templates have changed, so priming didn't actually work anymore). 2. Disables audio file caching since we can't play audio when offline, anyway (there's a dialog that prevents this in-player). 3. Fixes #1340 and #1341 by accounting for error cases when trying to play audio. It turns out that playing audio crashes if you didn't have internet access when going into an exploration (the no connectivity dialog only appears if you lose connectivity within a lesson). There's also some issues with existing view model code that wrongly assumed audio couldn't be in a failure state at that point. This has been fixed. 4. Moves the list of topics to cache to be in the same position as the flag enabling/disabling this functionality. The download experience isn't perfect, but it's meant to be a helper for user study coordinators so that they know when lessons can be brought offline. Note that the UI aspects of this change are really hacky. This is by design--I didn't want to overcomplicate the solution, and I wanted to keep the priming changes fully isolated to make future cleanup easier. Finally, no new tests were added. I clarified in StateFragmentTest that the edge cases fixed in this PR need to have corresponding tests, but I'm actually not sure offhand how to test that audio's playing. I think this will require additional work. I prefer to push this off to #388, but I will follow up with tests if anyone wants to push back on this. --- .../app/application/ApplicationComponent.kt | 26 +- .../player/audio/AudioFragmentPresenter.kt | 3 +- .../oppia/app/player/audio/AudioViewModel.kt | 34 +- .../app/splash/SplashActivityPresenter.kt | 6 +- .../app/player/state/StateFragmentTest.kt | 2 +- .../topic/PrimeTopicAssetsController.kt | 18 + .../topic/PrimeTopicAssetsControllerImpl.kt | 349 ++++++++++++++++++ .../topic/PrimeTopicAssetsControllerModule.kt | 29 ++ .../oppia/domain/topic/TopicListController.kt | 202 +--------- .../org/oppia/util/caching/AssetRepository.kt | 9 +- .../oppia/util/caching/CacheAssetsLocally.kt | 8 + .../oppia/util/caching/CachingAnnotations.kt | 7 - .../org/oppia/util/caching/CachingModule.kt | 14 + .../oppia/util/caching/TopicListToCache.kt | 6 + 14 files changed, 472 insertions(+), 241 deletions(-) create mode 100644 domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsController.kt create mode 100644 domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerImpl.kt create mode 100644 domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerModule.kt create mode 100644 utility/src/main/java/org/oppia/util/caching/CacheAssetsLocally.kt delete mode 100644 utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt create mode 100644 utility/src/main/java/org/oppia/util/caching/TopicListToCache.kt diff --git a/app/src/main/java/org/oppia/app/application/ApplicationComponent.kt b/app/src/main/java/org/oppia/app/application/ApplicationComponent.kt index c10c82db0be..75645c0f176 100644 --- a/app/src/main/java/org/oppia/app/application/ApplicationComponent.kt +++ b/app/src/main/java/org/oppia/app/application/ApplicationComponent.kt @@ -17,6 +17,7 @@ import org.oppia.domain.classify.rules.numericinput.NumericInputRuleModule import org.oppia.domain.classify.rules.textinput.TextInputRuleModule import org.oppia.domain.oppialogger.LogStorageModule import org.oppia.domain.question.QuestionModule +import org.oppia.domain.topic.PrimeTopicAssetsControllerModule import org.oppia.util.accessibility.AccessibilityModule import org.oppia.util.caching.CachingModule import org.oppia.util.gcsresource.GcsResourceModule @@ -29,22 +30,21 @@ import org.oppia.util.threading.DispatcherModule import javax.inject.Provider import javax.inject.Singleton -/** Root Dagger component for the application. All application-scoped modules should be included in this component. */ +/** + * Root Dagger component for the application. All application-scoped modules should be included in + * this component. + */ @Singleton @Component( modules = [ - ApplicationModule::class, DispatcherModule::class, - NetworkModule::class, LoggerModule::class, - ContinueModule::class, FractionInputModule::class, - ItemSelectionInputModule::class, MultipleChoiceInputModule::class, - NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, - TextInputRuleModule::class, DragDropSortInputModule::class, - InteractionsModule::class, GcsResourceModule::class, - GlideImageLoaderModule::class, ImageParsingModule::class, - HtmlParserEntityTypeModule::class, CachingModule::class, - QuestionModule::class, LogReportingModule::class, - AccessibilityModule::class, ImageClickInputModule::class, - LogStorageModule::class + ApplicationModule::class, DispatcherModule::class, NetworkModule::class, LoggerModule::class, + ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, + MultipleChoiceInputModule::class, NumberWithUnitsRuleModule::class, + NumericInputRuleModule::class, TextInputRuleModule::class, DragDropSortInputModule::class, + InteractionsModule::class, GcsResourceModule::class, GlideImageLoaderModule::class, + ImageParsingModule::class, HtmlParserEntityTypeModule::class, CachingModule::class, + QuestionModule::class, LogReportingModule::class, AccessibilityModule::class, + ImageClickInputModule::class, LogStorageModule::class, PrimeTopicAssetsControllerModule::class ] ) diff --git a/app/src/main/java/org/oppia/app/player/audio/AudioFragmentPresenter.kt b/app/src/main/java/org/oppia/app/player/audio/AudioFragmentPresenter.kt index 4417279c695..400aab06e77 100755 --- a/app/src/main/java/org/oppia/app/player/audio/AudioFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/app/player/audio/AudioFragmentPresenter.kt @@ -21,6 +21,7 @@ import org.oppia.app.model.CellularDataPreference import org.oppia.app.model.Profile import org.oppia.app.model.ProfileId import org.oppia.app.model.State +import org.oppia.app.player.audio.AudioViewModel.UiAudioPlayStatus import org.oppia.app.viewmodel.ViewModelProvider import org.oppia.domain.audio.CellularAudioDialogController import org.oppia.domain.profile.ProfileManagementController @@ -95,7 +96,7 @@ class AudioFragmentPresenter @Inject constructor( viewModel.playStatusLiveData.observe( fragment, Observer { - prepared = it != AudioViewModel.UiAudioPlayStatus.LOADING + prepared = it != UiAudioPlayStatus.LOADING && it != UiAudioPlayStatus.FAILED binding.sbAudioProgress.isEnabled = prepared } ) diff --git a/app/src/main/java/org/oppia/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/app/player/audio/AudioViewModel.kt index e287b4dd498..2e80c2bb4b5 100644 --- a/app/src/main/java/org/oppia/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/app/player/audio/AudioViewModel.kt @@ -143,14 +143,14 @@ class AudioViewModel @Inject constructor( } private fun processDurationResultLiveData(playProgressResult: AsyncResult): Int { - if (playProgressResult.isPending()) { + if (!playProgressResult.isSuccess()) { return 0 } return playProgressResult.getOrThrow().duration } private fun processPositionResultLiveData(playProgressResult: AsyncResult): Int { - if (playProgressResult.isPending()) { + if (!playProgressResult.isSuccess()) { return 0 } return playProgressResult.getOrThrow().position @@ -159,20 +159,22 @@ class AudioViewModel @Inject constructor( private fun processPlayStatusResultLiveData( playProgressResult: AsyncResult ): UiAudioPlayStatus { - if (playProgressResult.isPending()) return UiAudioPlayStatus.LOADING - if (playProgressResult.isFailure()) return UiAudioPlayStatus.FAILED - return when (playProgressResult.getOrThrow().type) { - PlayStatus.PREPARED -> { - if (autoPlay) audioPlayerController.play() - autoPlay = false - UiAudioPlayStatus.PREPARED - } - PlayStatus.PLAYING -> UiAudioPlayStatus.PLAYING - PlayStatus.PAUSED -> UiAudioPlayStatus.PAUSED - PlayStatus.COMPLETED -> { - if (hasFeedback) loadAudio(null, false) - hasFeedback = false - UiAudioPlayStatus.COMPLETED + return when { + playProgressResult.isPending() -> UiAudioPlayStatus.LOADING + playProgressResult.isFailure() -> UiAudioPlayStatus.FAILED + else -> when (playProgressResult.getOrThrow().type) { + PlayStatus.PREPARED -> { + if (autoPlay) audioPlayerController.play() + autoPlay = false + UiAudioPlayStatus.PREPARED + } + PlayStatus.PLAYING -> UiAudioPlayStatus.PLAYING + PlayStatus.PAUSED -> UiAudioPlayStatus.PAUSED + PlayStatus.COMPLETED -> { + if (hasFeedback) loadAudio(null, false) + hasFeedback = false + UiAudioPlayStatus.COMPLETED + } } } } diff --git a/app/src/main/java/org/oppia/app/splash/SplashActivityPresenter.kt b/app/src/main/java/org/oppia/app/splash/SplashActivityPresenter.kt index 67b71d6b68c..1296813999c 100644 --- a/app/src/main/java/org/oppia/app/splash/SplashActivityPresenter.kt +++ b/app/src/main/java/org/oppia/app/splash/SplashActivityPresenter.kt @@ -11,6 +11,7 @@ import org.oppia.app.model.OnboardingFlow import org.oppia.app.onboarding.OnboardingActivity import org.oppia.app.profile.ProfileActivity import org.oppia.domain.onboarding.OnboardingFlowController +import org.oppia.domain.topic.PrimeTopicAssetsController import org.oppia.util.data.AsyncResult import org.oppia.util.logging.ConsoleLogger import javax.inject.Inject @@ -20,7 +21,8 @@ import javax.inject.Inject class SplashActivityPresenter @Inject constructor( private val activity: AppCompatActivity, private val logger: ConsoleLogger, - private val onboardingFlowController: OnboardingFlowController + private val onboardingFlowController: OnboardingFlowController, + private val primeTopicAssetsController: PrimeTopicAssetsController ) { fun handleOnCreate() { @@ -29,6 +31,8 @@ class SplashActivityPresenter @Inject constructor( WindowManager.LayoutParams.FLAG_FULLSCREEN, WindowManager.LayoutParams.FLAG_FULLSCREEN ) + // Initiate download support before any additional processing begins. + primeTopicAssetsController.downloadAssets(R.style.AlertDialogTheme) subscribeToOnboardingFlow() } diff --git a/app/src/sharedTest/java/org/oppia/app/player/state/StateFragmentTest.kt b/app/src/sharedTest/java/org/oppia/app/player/state/StateFragmentTest.kt index 63cbe5b7d3d..2c4b44bb138 100644 --- a/app/src/sharedTest/java/org/oppia/app/player/state/StateFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/app/player/state/StateFragmentTest.kt @@ -119,7 +119,7 @@ class StateFragmentTest { // 10. Testing interactions with custom Oppia tags (including images) render correctly (when manually inspected) and are correctly functional. // 11. Update the tests to work properly on Robolectric (requires idling resource + replacing the dispatchers to leverage a coordinated test dispatcher library). // 12. Add tests for hints & solutions. - // 13. Add tests for audio states. + // 13. Add tests for audio states, including: audio playing & having an error, or no-network connectivity scenarios. See the PR introducing this comment & #1340 / #1341 for context. // TODO(#56): Add support for testing that previous/next button states are properly retained on config changes. @Test diff --git a/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsController.kt b/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsController.kt new file mode 100644 index 00000000000..ddb1a178068 --- /dev/null +++ b/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsController.kt @@ -0,0 +1,18 @@ +package org.oppia.domain.topic + +// TODO(#169): Remove this controller & download priming once downloads are properly supported. + +/** + * Controller for conditionally pre-priming assets to enable full download support. Whether + * downloading is enabled is gated by the [org.oppia.util.caching.CacheAssetsLocally] annotation. + */ +interface PrimeTopicAssetsController { + /** + * Initiates asset downloading in the background. UI affordances will be shown before and after + * priming, if it's enabled (otherwise nothing will show). + * + * @param dialogStyleResId the resource ID for the alert dialog style used for the dialog-based UI + * affordances + */ + fun downloadAssets(dialogStyleResId: Int) +} diff --git a/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerImpl.kt b/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerImpl.kt new file mode 100644 index 00000000000..fbb2ef3b754 --- /dev/null +++ b/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerImpl.kt @@ -0,0 +1,349 @@ +package org.oppia.domain.topic + +import android.annotation.SuppressLint +import android.app.Activity +import android.app.Application +import android.content.Context +import android.graphics.Typeface +import android.os.Bundle +import android.os.SystemClock +import android.text.Spannable +import android.text.style.ImageSpan +import android.util.TypedValue +import android.widget.LinearLayout +import android.widget.ProgressBar +import android.widget.TextView +import androidx.appcompat.app.AlertDialog +import androidx.appcompat.app.AppCompatActivity +import androidx.core.text.HtmlCompat +import androidx.lifecycle.MutableLiveData +import androidx.lifecycle.Observer +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.asCoroutineDispatcher +import kotlinx.coroutines.async +import kotlinx.coroutines.launch +import org.oppia.app.model.AnswerGroup +import org.oppia.app.model.Exploration +import org.oppia.app.model.Hint +import org.oppia.app.model.Interaction +import org.oppia.app.model.Outcome +import org.oppia.app.model.Solution +import org.oppia.app.model.State +import org.oppia.app.model.SubtitledHtml +import org.oppia.app.model.Voiceover +import org.oppia.app.model.VoiceoverMapping +import org.oppia.domain.exploration.ExplorationRetriever +import org.oppia.domain.util.JsonAssetRetriever +import org.oppia.util.caching.AssetRepository +import org.oppia.util.caching.TopicListToCache +import org.oppia.util.gcsresource.DefaultResourceBucketName +import org.oppia.util.logging.ConsoleLogger +import org.oppia.util.parser.DefaultGcsPrefix +import org.oppia.util.parser.ImageDownloadUrlTemplate +import java.util.concurrent.Executors +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicInteger +import javax.inject.Inject +import javax.inject.Singleton + +private const val CUSTOM_IMG_TAG = "oppia-noninteractive-image" +private const val REPLACE_IMG_TAG = "img" +private const val CUSTOM_IMG_FILE_PATH_ATTRIBUTE = "filepath-with-value" +private const val REPLACE_IMG_FILE_PATH_ATTRIBUTE = "src" + +/** + * Implementation of [PrimeTopicAssetsController] which primes assets & shows UI affordances before + * and after priming. + */ +@Singleton +class PrimeTopicAssetsControllerImpl @Inject constructor( + private val context: Context, + private val logger: ConsoleLogger, + private val assetRepository: AssetRepository, + private val topicController: TopicController, + private val jsonAssetRetriever: JsonAssetRetriever, + private val explorationRetriever: ExplorationRetriever, + @DefaultGcsPrefix private val gcsPrefix: String, + @DefaultResourceBucketName private val gcsResource: String, + @ImageDownloadUrlTemplate private val imageDownloadUrlTemplate: String, + @TopicListToCache private val topicListToCache: List + ) : PrimeTopicAssetsController { + + // NOTE TO DEVELOPERS: Don't ever do this. The application should use shared dispatchers to + // control resources & coordinate tests. This custom dispatcher is needed since priming is a + // dispatcher-intensive operation and using the shared background dispatcher ends up blocking the + // app UI, potentially in a breaking way. + private val extraDispatcher = Executors.newFixedThreadPool( + /* nThreads= */ 4 + ).asCoroutineDispatcher() + // NOTE TO DEVELOPERS: Never do this. We should never hold activity references in singleton + // objects, even as weak references. This is being done to keep priming code isolated so that it's + // easier to remove after #169 is completed. + private val extraDispatcherScope = CoroutineScope(extraDispatcher) + private val primeDownloadStatus = MutableLiveData(PrimeAssetsStatus(0, 0)) + private val currentDownloadCount = AtomicInteger() + private val failedDownloadCount = AtomicInteger() + private val dialogShown = AtomicBoolean() + private val dialogDismissed = AtomicBoolean() + + override fun downloadAssets(dialogStyleResId: Int) { + prepareUiForDownloadStatusChanges(dialogStyleResId) + + // Ensure all JSON files are available in memory for quick retrieval. + val allFiles = mutableListOf() + allFiles.add("topics.json") + val topicIdJsonArray = jsonAssetRetriever + .loadJsonFromAsset("topics.json")!! + .getJSONArray("topic_id_list") + for (i in 0 until topicIdJsonArray.length()) { + allFiles.addAll(topicController.getAssetFileNameList(topicIdJsonArray.optString(i))) + } + + val primeAssetJobs = allFiles.map { + extraDispatcherScope.async { + assetRepository.primeTextFileFromLocalAssets(it) + } + } + + // The following job encapsulates all startup loading. NB: We don't currently wait on this job + // to complete because it's fine to try to load the assets at the same time as priming the + // cache, and it's unlikely the user can get into an exploration fast enough to try to load an + // asset that would trigger a strict mode crash. + extraDispatcherScope.launch { + primeAssetJobs.forEach { it.await() } + + // Only download binary assets for one fractions lesson. The others can still be streamed. + val explorations = loadExplorations(topicListToCache) + val voiceoverUrls = listOf() + val imageUrls = collectAllImageUrls(explorations).toSet() + logger.d( + "AssetRepo", + "Downloading up to ${voiceoverUrls.size} voiceovers and ${imageUrls.size} images" + ) + val startTime = SystemClock.elapsedRealtime() + val downloadUrls = (voiceoverUrls + imageUrls).filterNot( + assetRepository::isRemoteBinarAssetDownloaded + ) + val assetDownloadCount = downloadUrls.size + primeDownloadStatus.postValue( + PrimeAssetsStatus(currentDownloadCount.get(), assetDownloadCount) + ) + downloadUrls.map { url: String -> + extraDispatcherScope.async { + try { + assetRepository.primeRemoteBinaryAsset(url) + } catch (e: Exception) { + failedDownloadCount.incrementAndGet() + logger.w("AssetRepo", "Failed to download $url because $e") + } + primeDownloadStatus.postValue( + PrimeAssetsStatus( + currentDownloadCount.incrementAndGet(), assetDownloadCount, failedDownloadCount.get() + ) + ) + } + }.forEach { it.await() } + val endTime = SystemClock.elapsedRealtime() + logger.d( + "AssetRepo", + "Finished downloading voiceovers and images in ${endTime - startTime}ms" + ) + + // Send the final count since everything should be done now. This is redundant, but it's meant + // to make sure the dialog reaches a finalized state. + primeDownloadStatus.postValue(PrimeAssetsStatus(assetDownloadCount, assetDownloadCount)) + } + } + + private fun prepareUiForDownloadStatusChanges(dialogStyleResId: Int) { + // Reference: https://stackoverflow.com/a/37713320. + val application = context.applicationContext as Application + application.registerActivityLifecycleCallbacks(object : Application.ActivityLifecycleCallbacks { + override fun onActivityPaused(activity: Activity?) {} + override fun onActivityResumed(activity: Activity?) {} + override fun onActivityStarted(activity: Activity?) {} + override fun onActivityDestroyed(activity: Activity?) {} + override fun onActivitySaveInstanceState(activity: Activity?, outState: Bundle?) {} + override fun onActivityStopped(activity: Activity?) {} + override fun onActivityCreated(activity: Activity?, savedInstanceState: Bundle?) { + if (!dialogDismissed.get()) { + activity?.let { + val appCompatActivity = it as AppCompatActivity + primeDownloadStatus.observe(appCompatActivity, object : Observer { + override fun onChanged(primeAssetsStatus: PrimeAssetsStatus?) { + primeAssetsStatus?.let { status -> + if (status.totalDownloadCount > 0 && !dialogShown.get()) { + showProgressDialog(appCompatActivity, dialogStyleResId) + } + } + } + }) + } + } + } + }) + } + + @SuppressLint("SetTextI18n") // This is a temporary, alpha-release only feature. + private fun showProgressDialog(activity: Activity, dialogStyleResId: Int) { + // Programmatically create the layout to avoid resource deps and to keep priming isolated. + val layout = LinearLayout(activity) + layout.orientation = LinearLayout.VERTICAL + val textView = TextView(activity) + layout.addView(textView) + textView.text = "Downloading assets for offline support." + val resources = activity.resources + val marginPx = TypedValue.applyDimension( + TypedValue.COMPLEX_UNIT_DIP, 16f, resources.displayMetrics + ).toInt() + (textView.layoutParams as LinearLayout.LayoutParams).setMargins( + /* left= */ marginPx, /* top= */ marginPx, /* right= */ marginPx, /* bottom= */ marginPx + ) + textView.textSize = 14f + textView.typeface = Typeface.create("sans-serif", Typeface.NORMAL) + val progressBar = ProgressBar( + activity, /* attrs= */ null, android.R.attr.progressBarStyleHorizontal + ) + layout.addView(progressBar) + (progressBar.layoutParams as LinearLayout.LayoutParams).setMargins( + /* left= */ marginPx, /* top= */ 0, /* right= */ marginPx, /* bottom= */ 0 + ) + val dialog = AlertDialog.Builder(activity, dialogStyleResId) + .setView(layout) + .setPositiveButton("Close") { dialog, _ -> + dialogDismissed.set(true) + dialog.dismiss() + }.create() + dialog.setCanceledOnTouchOutside(false) + dialog.show() + dialogShown.set(true) + primeDownloadStatus.observeForever(object : Observer { + override fun onChanged(status: PrimeAssetsStatus?) { + status?.let { + progressBar.max = it.totalDownloadCount + if (it.currentDownloadCount > progressBar.progress) { + progressBar.progress = it.currentDownloadCount + } + if (it.currentDownloadCount == it.totalDownloadCount) { + if (it.failedDownloadCount > 0) { + textView.text = "Finished downloading, but some failed to download. Please try again." + } else { + textView.text = "Finished downloading assets for offline support." + } + primeDownloadStatus.removeObserver(this) + } + } + } + }) + } + + private fun loadExplorations(explorationIds: Collection): Collection { + return explorationIds.map(explorationRetriever::loadExploration) + } + + @Suppress("unused") // Voiceovers can't be played while offline, so don't cache them for now. + private fun collectAllDesiredVoiceoverUrls( + explorations: Collection + ): Collection { + return explorations.flatMap(::collectDesiredVoiceoverUrls) + } + + private fun collectDesiredVoiceoverUrls(exploration: Exploration): Collection { + return extractDesiredVoiceovers(exploration).map { voiceover -> + getUriForVoiceover( + exploration.id, + voiceover + ) + } + } + + private fun extractDesiredVoiceovers(exploration: Exploration): Collection { + val states = exploration.statesMap.values + val mappings = states.flatMap(::getDesiredVoiceoverMapping) + return mappings.flatMap { it.voiceoverMappingMap.values } + } + + private fun getDesiredVoiceoverMapping(state: State): Collection { + val voiceoverMappings = state.recordedVoiceoversMap + val contentIds = extractDesiredContentIds(state).filter(String::isNotEmpty) + return voiceoverMappings.filterKeys(contentIds::contains).values + } + + /** Returns all collection IDs from the specified [State] that can actually be played by a user. */ + private fun extractDesiredContentIds(state: State): Collection { + val stateContentSubtitledHtml = state.content + val defaultFeedbackSubtitledHtml = state.interaction.defaultOutcome.feedback + val answerGroupOutcomes = state.interaction.answerGroupsList + .map(AnswerGroup::getOutcome) + val answerGroupsSubtitledHtml = answerGroupOutcomes.map(Outcome::getFeedback) + val targetedSubtitledHtmls = + answerGroupsSubtitledHtml + stateContentSubtitledHtml + defaultFeedbackSubtitledHtml + return targetedSubtitledHtmls.map(SubtitledHtml::getContentId) + } + + private fun collectAllImageUrls(explorations: Collection): Collection { + return explorations.flatMap(::collectImageUrls) + } + + private fun collectImageUrls(exploration: Exploration): Collection { + val subtitledHtmls = collectSubtitledHtmls(exploration) + val imageSources = subtitledHtmls.flatMap(::getImageSourcesFromHtml) + return imageSources.toSet().map { imageSource -> + getUriForImage(exploration.id, imageSource) + } + } + + private fun collectSubtitledHtmls(exploration: Exploration): Collection { + val states = exploration.statesMap.values + val stateContents = states.map(State::getContent) + val stateInteractions = states.map(State::getInteraction) + val stateSolutions = + stateInteractions.map(Interaction::getSolution).map(Solution::getExplanation) + val stateHints = stateInteractions.flatMap( + Interaction::getHintList + ).map(Hint::getHintContent) + val answerGroupOutcomes = + stateInteractions.flatMap(Interaction::getAnswerGroupsList).map(AnswerGroup::getOutcome) + val defaultOutcomes = stateInteractions.map(Interaction::getDefaultOutcome) + val outcomeFeedbacks = (answerGroupOutcomes + defaultOutcomes) + .map(Outcome::getFeedback) + val allSubtitledHtmls = stateContents + stateSolutions + stateHints + outcomeFeedbacks + return allSubtitledHtmls.filter { it != SubtitledHtml.getDefaultInstance() } + } + + private fun getImageSourcesFromHtml(subtitledHtml: SubtitledHtml): Collection { + val parsedHtml = parseHtml(replaceCustomOppiaImageTag(subtitledHtml.html)) + val imageSpans = parsedHtml.getSpans( + 0, + parsedHtml.length, + ImageSpan::class.java + ) + return imageSpans.toList().map(ImageSpan::getSource) + } + + private fun parseHtml(html: String): Spannable { + return HtmlCompat.fromHtml(html, HtmlCompat.FROM_HTML_MODE_LEGACY) as Spannable + } + + private fun replaceCustomOppiaImageTag(html: String): String { + return html.replace(CUSTOM_IMG_TAG, REPLACE_IMG_TAG) + .replace(CUSTOM_IMG_FILE_PATH_ATTRIBUTE, REPLACE_IMG_FILE_PATH_ATTRIBUTE) + .replace(""", "") + } + + private fun getUriForVoiceover(explorationId: String, voiceover: Voiceover): String { + return "https://storage.googleapis.com/${gcsResource}/exploration" + + "/$explorationId/assets/audio/${voiceover.fileName}" + } + + private fun getUriForImage(explorationId: String, imageFileName: String): String { + val downloadUrlFile = String.format( + imageDownloadUrlTemplate, "exploration", explorationId, imageFileName + ) + return "$gcsPrefix/$gcsResource/$downloadUrlFile" + } + + private data class PrimeAssetsStatus( + val currentDownloadCount: Int, val totalDownloadCount: Int, val failedDownloadCount: Int = 0 + ) +} diff --git a/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerModule.kt b/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerModule.kt new file mode 100644 index 00000000000..223c68ac0b4 --- /dev/null +++ b/domain/src/main/java/org/oppia/domain/topic/PrimeTopicAssetsControllerModule.kt @@ -0,0 +1,29 @@ +package org.oppia.domain.topic + +import dagger.Module +import dagger.Provides +import org.oppia.util.caching.CacheAssetsLocally +import javax.inject.Provider + +/** + * Module for providing a [PrimeTopicAssetsController] depending on whether asset caching is enabled + * (see [CacheAssetsLocally] for specifics). + */ +@Module +class PrimeTopicAssetsControllerModule { + @Provides + fun providePrimeTopicAssetsController( + @CacheAssetsLocally cacheAssetsLocally: Boolean, + impl: Provider + ): PrimeTopicAssetsController { + return if (cacheAssetsLocally) { + impl.get() + } else { + object : PrimeTopicAssetsController { + override fun downloadAssets(dialogStyleResId: Int) { + // Do nothing since caching is disabled. + } + } + } + } +} diff --git a/domain/src/main/java/org/oppia/domain/topic/TopicListController.kt b/domain/src/main/java/org/oppia/domain/topic/TopicListController.kt index aac1d4a3911..9cc789d52d7 100644 --- a/domain/src/main/java/org/oppia/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/domain/topic/TopicListController.kt @@ -1,50 +1,24 @@ package org.oppia.domain.topic import android.graphics.Color -import android.os.SystemClock -import android.text.Spannable -import android.text.style.ImageSpan -import androidx.core.text.HtmlCompat import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.async -import kotlinx.coroutines.launch import org.json.JSONObject -import org.oppia.app.model.AnswerGroup import org.oppia.app.model.ChapterPlayState import org.oppia.app.model.ChapterProgress import org.oppia.app.model.ChapterSummary -import org.oppia.app.model.Exploration -import org.oppia.app.model.Hint -import org.oppia.app.model.Interaction import org.oppia.app.model.LessonThumbnail import org.oppia.app.model.LessonThumbnailGraphic import org.oppia.app.model.OngoingStoryList -import org.oppia.app.model.Outcome import org.oppia.app.model.ProfileId import org.oppia.app.model.PromotedStory -import org.oppia.app.model.Solution -import org.oppia.app.model.State -import org.oppia.app.model.SubtitledHtml import org.oppia.app.model.Topic import org.oppia.app.model.TopicList import org.oppia.app.model.TopicProgress import org.oppia.app.model.TopicSummary -import org.oppia.app.model.Voiceover -import org.oppia.app.model.VoiceoverMapping -import org.oppia.domain.exploration.ExplorationRetriever import org.oppia.domain.util.JsonAssetRetriever -import org.oppia.util.caching.AssetRepository -import org.oppia.util.caching.CacheAssetsLocally import org.oppia.util.data.AsyncResult import org.oppia.util.data.DataProviders -import org.oppia.util.gcsresource.DefaultResourceBucketName -import org.oppia.util.logging.ConsoleLogger -import org.oppia.util.parser.DefaultGcsPrefix -import org.oppia.util.parser.ImageDownloadUrlTemplate -import org.oppia.util.threading.BackgroundDispatcher import java.util.Date import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -91,11 +65,6 @@ val EXPLORATION_THUMBNAILS = mapOf( TEST_EXPLORATION_ID_5 to createChapterThumbnail0() ) -private const val CUSTOM_IMG_TAG = "oppia-noninteractive-image" -private const val REPLACE_IMG_TAG = "img" -private const val CUSTOM_IMG_FILE_PATH_ATTRIBUTE = "filepath-with-value" -private const val REPLACE_IMG_FILE_PATH_ATTRIBUTE = "src" - private const val TRANSFORMED_GET_ONGOING_STORY_LIST_PROVIDER_ID = "transformed_get_ongoing_story_list_provider_id" @@ -107,73 +76,8 @@ class TopicListController @Inject constructor( private val dataProviders: DataProviders, private val jsonAssetRetriever: JsonAssetRetriever, private val topicController: TopicController, - private val storyProgressController: StoryProgressController, - private val explorationRetriever: ExplorationRetriever, - @BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher, - @CacheAssetsLocally private val cacheAssetsLocally: Boolean, - @DefaultGcsPrefix private val gcsPrefix: String, - @DefaultResourceBucketName private val gcsResource: String, - @ImageDownloadUrlTemplate private val imageDownloadUrlTemplate: String, - logger: ConsoleLogger, - assetRepository: AssetRepository + private val storyProgressController: StoryProgressController ) { - private val backgroundScope = CoroutineScope(backgroundDispatcher) - - init { - // TODO(#169): Download data reactively rather than during start-up to avoid blocking the main thread on the whole - // load operation. - if (cacheAssetsLocally) { - // Ensure all JSON files are available in memory for quick retrieval. - val allFiles = mutableListOf() - allFiles.add("topics.json") - val topicIdJsonArray = jsonAssetRetriever - .loadJsonFromAsset("topics.json")!! - .getJSONArray("topic_id_list") - for (i in 0 until topicIdJsonArray.length()) { - allFiles.addAll(topicController.getAssetFileNameList(topicIdJsonArray.optString(i))) - } - - val primeAssetJobs = allFiles.map { - backgroundScope.async { - assetRepository.primeTextFileFromLocalAssets(it) - } - } - - // The following job encapsulates all startup loading. NB: We don't currently wait on this job to complete because - // it's fine to try to load the assets at the same time as priming the cache, and it's unlikely the user can get - // into an exploration fast enough to try to load an asset that would trigger a strict mode crash. - backgroundScope.launch { - primeAssetJobs.forEach { it.await() } - - // Only download binary assets for one fractions lesson. The others can still be streamed. - val explorations = loadExplorations(listOf(FRACTIONS_EXPLORATION_ID_1)) - val voiceoverUrls = collectAllDesiredVoiceoverUrls(explorations).toSet() - val imageUrls = collectAllImageUrls(explorations).toSet() - logger.d( - "AssetRepo", - "Downloading up to ${voiceoverUrls.size} voiceovers and ${imageUrls.size} images" - ) - val startTime = SystemClock.elapsedRealtime() - val voiceoverDownloadJobs = voiceoverUrls.map { url -> - backgroundScope.async { - assetRepository.primeRemoteBinaryAsset(url) - } - } - val imageDownloadJobs = imageUrls.map { url -> - backgroundScope.async { - assetRepository.primeRemoteBinaryAsset(url) - } - } - (voiceoverDownloadJobs + imageDownloadJobs).forEach { it.await() } - val endTime = SystemClock.elapsedRealtime() - logger.d( - "AssetRepo", - "Finished downloading voiceovers and images in ${endTime - startTime}ms" - ) - } - } - } - /** * Returns the list of [TopicSummary]s currently tracked by the app, possibly up to * [EVICTION_TIME_MILLIS] old. @@ -387,110 +291,6 @@ class TopicListController @Inject constructor( } return promotedStoryBuilder.build() } - - private fun loadExplorations(explorationIds: Collection): Collection { - return explorationIds.map(explorationRetriever::loadExploration) - } - - private fun collectAllDesiredVoiceoverUrls( - explorations: Collection - ): Collection { - return explorations.flatMap(::collectDesiredVoiceoverUrls) - } - - private fun collectDesiredVoiceoverUrls(exploration: Exploration): Collection { - return extractDesiredVoiceovers(exploration).map { voiceover -> - getUriForVoiceover( - exploration.id, - voiceover - ) - } - } - - private fun extractDesiredVoiceovers(exploration: Exploration): Collection { - val states = exploration.statesMap.values - val mappings = states.flatMap(::getDesiredVoiceoverMapping) - return mappings.flatMap { it.voiceoverMappingMap.values } - } - - private fun getDesiredVoiceoverMapping(state: State): Collection { - val voiceoverMappings = state.recordedVoiceoversMap - val contentIds = extractDesiredContentIds(state).filter(String::isNotEmpty) - return voiceoverMappings.filterKeys(contentIds::contains).values - } - - /** Returns all collection IDs from the specified [State] that can actually be played by a user. */ - private fun extractDesiredContentIds(state: State): Collection { - val stateContentSubtitledHtml = state.content - val defaultFeedbackSubtitledHtml = state.interaction.defaultOutcome.feedback - val answerGroupOutcomes = state.interaction.answerGroupsList - .map(AnswerGroup::getOutcome) - val answerGroupsSubtitledHtml = answerGroupOutcomes.map(Outcome::getFeedback) - val targetedSubtitledHtmls = - answerGroupsSubtitledHtml + stateContentSubtitledHtml + defaultFeedbackSubtitledHtml - return targetedSubtitledHtmls.map(SubtitledHtml::getContentId) - } - - private fun collectAllImageUrls(explorations: Collection): Collection { - return explorations.flatMap(::collectImageUrls) - } - - private fun collectImageUrls(exploration: Exploration): Collection { - val subtitledHtmls = collectSubtitledHtmls(exploration) - val imageSources = subtitledHtmls.flatMap(::getImageSourcesFromHtml) - return imageSources.toSet().map { imageSource -> - getUriForImage(exploration.id, imageSource) - } - } - - private fun collectSubtitledHtmls(exploration: Exploration): Collection { - val states = exploration.statesMap.values - val stateContents = states.map(State::getContent) - val stateInteractions = states.map(State::getInteraction) - val stateSolutions = - stateInteractions.map(Interaction::getSolution).map(Solution::getExplanation) - val stateHints = stateInteractions.flatMap( - Interaction::getHintList - ).map(Hint::getHintContent) - val answerGroupOutcomes = - stateInteractions.flatMap(Interaction::getAnswerGroupsList).map(AnswerGroup::getOutcome) - val defaultOutcomes = stateInteractions.map(Interaction::getDefaultOutcome) - val outcomeFeedbacks = (answerGroupOutcomes + defaultOutcomes) - .map(Outcome::getFeedback) - val allSubtitledHtmls = stateContents + stateSolutions + stateHints + outcomeFeedbacks - return allSubtitledHtmls.filter { it != SubtitledHtml.getDefaultInstance() } - } - - private fun getImageSourcesFromHtml(subtitledHtml: SubtitledHtml): Collection { - val parsedHtml = parseHtml(replaceCustomOppiaImageTag(subtitledHtml.html)) - val imageSpans = parsedHtml.getSpans( - 0, - parsedHtml.length, - ImageSpan::class.java - ) - return imageSpans.toList().map(ImageSpan::getSource) - } - - private fun parseHtml(html: String): Spannable { - return HtmlCompat.fromHtml(html, HtmlCompat.FROM_HTML_MODE_LEGACY) as Spannable - } - - private fun replaceCustomOppiaImageTag(html: String): String { - return html.replace(CUSTOM_IMG_TAG, REPLACE_IMG_TAG) - .replace(CUSTOM_IMG_FILE_PATH_ATTRIBUTE, REPLACE_IMG_FILE_PATH_ATTRIBUTE) - .replace(""", "") - } - - private fun getUriForVoiceover(explorationId: String, voiceover: Voiceover): String { - return "https://storage.googleapis.com/${gcsResource}exploration" + - "/$explorationId/assets/audio/${voiceover.fileName}" - } - - private fun getUriForImage(explorationId: String, imageFileName: String): String { - return gcsPrefix + gcsResource + String.format( - imageDownloadUrlTemplate, "exploration", explorationId, imageFileName - ) - } } internal fun createTopicThumbnail(topicJsonObject: JSONObject): LessonThumbnail { diff --git a/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt b/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt index 8f3beb2c24b..b95cdbedb16 100644 --- a/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt +++ b/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt @@ -62,7 +62,7 @@ class AssetRepository @Inject constructor( /** Ensures the contents corresponding to the specified URL are available for quick retrieval. */ fun primeRemoteBinaryAsset(url: String) { - if (!getLocalCacheFile(url).exists()) { + if (!isRemoteBinarAssetDownloaded(url)) { // Otherwise, download it remotely and cache it locally. logger.d("AssetRepo", "Downloading binary asset: $url") val contents = openRemoteStream(url).use { it.readBytes() } @@ -70,6 +70,13 @@ class AssetRepository @Inject constructor( } } + /** + * Returns whether a binary asset corresponding to the specified URL has already been downloaded. + */ + fun isRemoteBinarAssetDownloaded(url: String): Boolean { + return getLocalCacheFile(url).exists() + } + private fun openRemoteStream(url: String): InputStream { return URL(url).openStream() } diff --git a/utility/src/main/java/org/oppia/util/caching/CacheAssetsLocally.kt b/utility/src/main/java/org/oppia/util/caching/CacheAssetsLocally.kt new file mode 100644 index 00000000000..8d946ec2a55 --- /dev/null +++ b/utility/src/main/java/org/oppia/util/caching/CacheAssetsLocally.kt @@ -0,0 +1,8 @@ +package org.oppia.util.caching + +import javax.inject.Qualifier + +/** + * Corresponds to an injectable boolean indicating whether lesson assets should be cached locally. + */ +@Qualifier annotation class CacheAssetsLocally diff --git a/utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt b/utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt deleted file mode 100644 index e1735ca2fa3..00000000000 --- a/utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt +++ /dev/null @@ -1,7 +0,0 @@ -// ktlint-disable filename -package org.oppia.util.caching - -import javax.inject.Qualifier - -/** Corresponds to an injectable boolean indicating whether lesson assets should be cached locally. */ -@Qualifier annotation class CacheAssetsLocally diff --git a/utility/src/main/java/org/oppia/util/caching/CachingModule.kt b/utility/src/main/java/org/oppia/util/caching/CachingModule.kt index f167b295862..940a86c3aba 100644 --- a/utility/src/main/java/org/oppia/util/caching/CachingModule.kt +++ b/utility/src/main/java/org/oppia/util/caching/CachingModule.kt @@ -3,10 +3,24 @@ package org.oppia.util.caching import dagger.Module import dagger.Provides +private const val FRACTIONS_EXPLORATION_ID_0 = "umPkwp0L1M0-" +private const val FRACTIONS_EXPLORATION_ID_1 = "MjZzEVOG47_1" +private const val RATIOS_EXPLORATION_ID_0 = "2mzzFVDLuAj8" +private const val RATIOS_EXPLORATION_ID_1 = "5NWuolNcwH6e" +private const val RATIOS_EXPLORATION_ID_2 = "k2bQ7z5XHNbK" +private const val RATIOS_EXPLORATION_ID_3 = "tIoSb3HZFN6e" + /** Provides dependencies corresponding to the app's caching policies. */ @Module class CachingModule { @Provides @CacheAssetsLocally fun provideCacheAssetsLocally(): Boolean = false + + @Provides + @TopicListToCache + fun provideTopicListToCache() = listOf( + FRACTIONS_EXPLORATION_ID_0, FRACTIONS_EXPLORATION_ID_1, RATIOS_EXPLORATION_ID_0, + RATIOS_EXPLORATION_ID_1, RATIOS_EXPLORATION_ID_2, RATIOS_EXPLORATION_ID_3 + ) } diff --git a/utility/src/main/java/org/oppia/util/caching/TopicListToCache.kt b/utility/src/main/java/org/oppia/util/caching/TopicListToCache.kt new file mode 100644 index 00000000000..1714bc9b9d8 --- /dev/null +++ b/utility/src/main/java/org/oppia/util/caching/TopicListToCache.kt @@ -0,0 +1,6 @@ +package org.oppia.util.caching + +import javax.inject.Qualifier + +/** Corresponds to an injectable list of topic IDs to cache if [CacheAssetsLocaly] is true. */ +@Qualifier annotation class TopicListToCache