From b8100ad90a64fac50cbd6cdfda47c231f07531af Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 19 Nov 2019 16:50:23 -0800 Subject: [PATCH] Fix #386: Add support for local caching of audio & image assets (#399) * Ensured progress, thumbnails, and stats are consistent. This change ensures that completed progress is consistent and correct everywhere for fractions and ratios topics. It also ensures that there are thumbnails defined for chapters, and that all thumbnails are consistent regardless of which screen topics, stories, or chapters are viewed from. It updates the reported lesson count to be correct. * Use a different color for the fractions topic vs. ratios. * Remove dev comment. * Compute actual JSON disk size requirement for each topic. * Introduce AssetRepository to prefetch JSON files, and support offline caching for audio files. * Introduce custom local image caching. * Add module to disable local asset caching by default, and ensure tests pass. * Post-merge fix. --- .../app/application/ApplicationComponent.kt | 3 +- domain/src/main/assets/about_oppia.json | 1 + domain/src/main/assets/oppia_exploration.json | 1 + .../main/assets/prototype_exploration.json | 1 + domain/src/main/assets/welcome.json | 1 + .../domain/audio/AudioPlayerController.kt | 40 +++- .../exploration/ExplorationRetriever.kt | 13 +- .../org/oppia/domain/topic/TopicController.kt | 2 +- .../oppia/domain/topic/TopicListController.kt | 176 +++++++++++++++++- .../oppia/domain/util/JsonAssetRetriever.kt | 11 +- .../domain/audio/AudioPlayerControllerTest.kt | 5 + .../ExplorationProgressControllerTest.kt | 16 ++ .../topic/StoryProgressControllerTest.kt | 47 +++++ .../oppia/domain/topic/TopicControllerTest.kt | 23 +++ .../domain/topic/TopicListControllerTest.kt | 45 +++++ utility/build.gradle | 1 + .../org/oppia/util/caching/AssetRepository.kt | 130 +++++++++++++ .../oppia/util/caching/CachingAnnotations.kt | 6 + .../org/oppia/util/caching/CachingModule.kt | 12 ++ .../org/oppia/util/parser/GlideImageLoader.kt | 17 +- .../oppia/util/parser/ImageAssetFetcher.kt | 10 + .../util/parser/RepositoryGlideModule.kt | 17 ++ .../util/parser/RepositoryModelLoader.kt | 51 +++++ .../org/oppia/util/parser/UrlImageParser.kt | 2 +- 24 files changed, 610 insertions(+), 21 deletions(-) create mode 100644 utility/src/main/java/org/oppia/util/caching/AssetRepository.kt create mode 100644 utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt create mode 100644 utility/src/main/java/org/oppia/util/caching/CachingModule.kt create mode 100644 utility/src/main/java/org/oppia/util/parser/ImageAssetFetcher.kt create mode 100644 utility/src/main/java/org/oppia/util/parser/RepositoryGlideModule.kt create mode 100644 utility/src/main/java/org/oppia/util/parser/RepositoryModelLoader.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 b81f7e60780..32b64f4e689 100644 --- a/app/src/main/java/org/oppia/app/application/ApplicationComponent.kt +++ b/app/src/main/java/org/oppia/app/application/ApplicationComponent.kt @@ -13,6 +13,7 @@ import org.oppia.domain.classify.rules.multiplechoiceinput.MultipleChoiceInputMo import org.oppia.domain.classify.rules.numberwithunits.NumberWithUnitsRuleModule import org.oppia.domain.classify.rules.numericinput.NumericInputRuleModule import org.oppia.domain.classify.rules.textinput.TextInputRuleModule +import org.oppia.util.caching.CachingModule import org.oppia.util.gcsresource.GcsResourceModule import org.oppia.util.logging.LoggerModule import org.oppia.util.parser.GlideImageLoaderModule @@ -29,7 +30,7 @@ import javax.inject.Singleton ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, MultipleChoiceInputModule::class, NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, TextInputRuleModule::class, InteractionsModule::class, GcsResourceModule::class, GlideImageLoaderModule::class, ImageParsingModule::class, - HtmlParserEntityTypeModule::class + HtmlParserEntityTypeModule::class, CachingModule::class ]) interface ApplicationComponent { @Component.Builder diff --git a/domain/src/main/assets/about_oppia.json b/domain/src/main/assets/about_oppia.json index 1202f9a506e..b195f7a461c 100644 --- a/domain/src/main/assets/about_oppia.json +++ b/domain/src/main/assets/about_oppia.json @@ -1,4 +1,5 @@ { + "exploration_id": "1", "author_notes": "", "blurb": "", "category": "Welcome", diff --git a/domain/src/main/assets/oppia_exploration.json b/domain/src/main/assets/oppia_exploration.json index ebf01b482d4..d569575d418 100755 --- a/domain/src/main/assets/oppia_exploration.json +++ b/domain/src/main/assets/oppia_exploration.json @@ -1,4 +1,5 @@ { + "exploration_id": "3", "author_notes": "", "blurb": "", "category": "Welcome", diff --git a/domain/src/main/assets/prototype_exploration.json b/domain/src/main/assets/prototype_exploration.json index aa077230c3d..42a39890f4d 100644 --- a/domain/src/main/assets/prototype_exploration.json +++ b/domain/src/main/assets/prototype_exploration.json @@ -1,4 +1,5 @@ { + "exploration_id": "2", "language_code": "en", "param_specs": {}, "param_changes": [], diff --git a/domain/src/main/assets/welcome.json b/domain/src/main/assets/welcome.json index 7030ee04c9e..992af67d990 100644 --- a/domain/src/main/assets/welcome.json +++ b/domain/src/main/assets/welcome.json @@ -1,4 +1,5 @@ { + "exploration_id": "0", "author_notes": "", "blurb": "", "category": "Welcome", diff --git a/domain/src/main/java/org/oppia/domain/audio/AudioPlayerController.kt b/domain/src/main/java/org/oppia/domain/audio/AudioPlayerController.kt index bb396f5aaa5..eae9fcba0e4 100644 --- a/domain/src/main/java/org/oppia/domain/audio/AudioPlayerController.kt +++ b/domain/src/main/java/org/oppia/domain/audio/AudioPlayerController.kt @@ -1,6 +1,8 @@ package org.oppia.domain.audio +import android.media.MediaDataSource import android.media.MediaPlayer +import android.os.Build import androidx.annotation.VisibleForTesting import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData @@ -9,6 +11,8 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import org.oppia.util.caching.AssetRepository +import org.oppia.util.caching.CacheAssetsLocally import org.oppia.util.data.AsyncResult import org.oppia.util.logging.Logger import org.oppia.util.threading.BackgroundDispatcher @@ -28,7 +32,9 @@ import kotlin.concurrent.withLock @Singleton class AudioPlayerController @Inject constructor( private val logger: Logger, - @BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher + private val assetRepository: AssetRepository, + @BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher, + @CacheAssetsLocally private val cacheAssetsLocally: Boolean ) { inner class AudioMutableLiveData : MutableLiveData>() { @@ -125,7 +131,37 @@ class AudioPlayerController @Inject constructor( private fun prepareDataSource(url: String) { try { - mediaPlayer.setDataSource(url) + if (cacheAssetsLocally && Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + val mediaDataSource: MediaDataSource = object : MediaDataSource() { + private val audioFileBuffer: ByteArray by lazy { + // Ensure that the download occurs off the main thread to avoid strict mode violations for + // cases when we need to stream audio. + assetRepository.loadRemoteBinaryAsset(url)() + } + + // https://medium.com/@jacks205/implementing-your-own-android-mediadatasource-e67adb070731. + override fun readAt(position: Long, buffer: ByteArray?, offset: Int, size: Int): Int { + checkNotNull(buffer) + val intPosition = position.toInt() + if (intPosition >= audioFileBuffer.size) { + return -1 + } + val availableData = audioFileBuffer.size - intPosition + val adjustedSize = size.coerceIn(0 until availableData) + audioFileBuffer.copyInto(buffer, offset, intPosition, intPosition + adjustedSize) + return adjustedSize + } + + override fun getSize(): Long { + return audioFileBuffer.size.toLong() + } + + override fun close() {} + } + mediaPlayer.setDataSource(mediaDataSource) + } else { + mediaPlayer.setDataSource(url) + } mediaPlayer.prepareAsync() } catch (e: IOException) { logger.e("AudioPlayerController", "Failed to set data source for media player", e) diff --git a/domain/src/main/java/org/oppia/domain/exploration/ExplorationRetriever.kt b/domain/src/main/java/org/oppia/domain/exploration/ExplorationRetriever.kt index 0e501d6ca5c..c9427baa825 100644 --- a/domain/src/main/java/org/oppia/domain/exploration/ExplorationRetriever.kt +++ b/domain/src/main/java/org/oppia/domain/exploration/ExplorationRetriever.kt @@ -14,10 +14,10 @@ import org.oppia.domain.util.StateRetriever import java.io.IOException import javax.inject.Inject -const val TEST_EXPLORATION_ID_5 = "DIWZiVgs0km-" -const val TEST_EXPLORATION_ID_6 = "test_exp_id_6" -const val TEST_EXPLORATION_ID_30 = "30" -const val TEST_EXPLORATION_ID_7 = "test_exp_id_7" +const val TEST_EXPLORATION_ID_5 = "0" +const val TEST_EXPLORATION_ID_6 = "1" +const val TEST_EXPLORATION_ID_30 = "2" +const val TEST_EXPLORATION_ID_7 = "3" // TODO(#59): Make this class inaccessible outside of the domain package except for tests. UI code should not be allowed // to depend on this utility. @@ -27,9 +27,9 @@ class ExplorationRetriever @Inject constructor( private val jsonAssetRetriever: JsonAssetRetriever, private val stateRetriever: StateRetriever ) { + // TODO(#169): Force callers of this method on a background thread. /** Loads and returns an exploration for the specified exploration ID, or fails. */ - @Suppress("RedundantSuspendModifier") // Force callers to call this on a background thread. - internal suspend fun loadExploration(explorationId: String): Exploration { + internal fun loadExploration(explorationId: String): Exploration { return when (explorationId) { TEST_EXPLORATION_ID_5 -> loadExplorationFromAsset("welcome.json") TEST_EXPLORATION_ID_6 -> loadExplorationFromAsset("about_oppia.json") @@ -50,6 +50,7 @@ class ExplorationRetriever @Inject constructor( try { val explorationObject = jsonAssetRetriever.loadJsonFromAsset(assetName) ?: return Exploration.getDefaultInstance() return Exploration.newBuilder() + .setId(explorationObject.getString("exploration_id")) .setTitle(explorationObject.getString("title")) .setLanguageCode(explorationObject.getString("language_code")) .setInitStateName(explorationObject.getString("init_state_name")) diff --git a/domain/src/main/java/org/oppia/domain/topic/TopicController.kt b/domain/src/main/java/org/oppia/domain/topic/TopicController.kt index 9f88544d517..0a123ca1fc4 100644 --- a/domain/src/main/java/org/oppia/domain/topic/TopicController.kt +++ b/domain/src/main/java/org/oppia/domain/topic/TopicController.kt @@ -55,7 +55,7 @@ const val FRACTIONS_QUESTION_ID_8 = "AciwQAtcvZfI" const val FRACTIONS_QUESTION_ID_9 = "YQwbX2r6p3Xj" const val FRACTIONS_QUESTION_ID_10 = "NNuVGmbJpnj5" const val RATIOS_QUESTION_ID_0 = "QiKxvAXpvUbb" -private val TOPIC_FILE_ASSOCIATIONS = mapOf( +val TOPIC_FILE_ASSOCIATIONS = mapOf( FRACTIONS_TOPIC_ID to listOf( "fractions_exploration0.json", "fractions_exploration1.json", 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 2b6375028a1..d824d164c03 100644 --- a/domain/src/main/java/org/oppia/domain/topic/TopicListController.kt +++ b/domain/src/main/java/org/oppia/domain/topic/TopicListController.kt @@ -1,19 +1,46 @@ package org.oppia.domain.topic +import android.os.SystemClock +import android.text.Html +import android.text.Spannable +import android.text.style.ImageSpan import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +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.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.PromotedStory +import org.oppia.app.model.Solution +import org.oppia.app.model.State import org.oppia.app.model.StorySummary +import org.oppia.app.model.SubtitledHtml import org.oppia.app.model.Topic import org.oppia.app.model.TopicList 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.util.caching.AssetRepository import org.oppia.domain.util.JsonAssetRetriever +import org.oppia.util.caching.CacheAssetsLocally import org.oppia.util.data.AsyncResult +import org.oppia.util.logging.Logger +import org.oppia.util.parser.DefaultGcsPrefix +import org.oppia.util.parser.DefaultGcsResource +import org.oppia.util.parser.ImageDownloadUrlTemplate +import org.oppia.util.threading.BackgroundDispatcher import java.util.concurrent.TimeUnit import javax.inject.Inject import javax.inject.Singleton @@ -45,6 +72,11 @@ val TOPIC_SKILL_ASSOCIATIONS = mapOf( RATIOS_TOPIC_ID to listOf(RATIOS_SKILL_ID_0) ) +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 val EVICTION_TIME_MILLIS = TimeUnit.DAYS.toMillis(1) /** Controller for retrieving the list of topics available to the learner to play. */ @@ -52,8 +84,61 @@ private val EVICTION_TIME_MILLIS = TimeUnit.DAYS.toMillis(1) class TopicListController @Inject constructor( private val jsonAssetRetriever: JsonAssetRetriever, private val topicController: TopicController, - private val storyProgressController: StoryProgressController + private val storyProgressController: StoryProgressController, + private val explorationRetriever: ExplorationRetriever, + @BackgroundDispatcher private val backgroundDispatcher: CoroutineDispatcher, + @CacheAssetsLocally private val cacheAssetsLocally: Boolean, + @DefaultGcsPrefix private val gcsPrefix: String, + @DefaultGcsResource private val gcsResource: String, + @ImageDownloadUrlTemplate private val imageDownloadUrlTemplate: String, + logger: Logger, + assetRepository: AssetRepository ) { + 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 = TOPIC_FILE_ASSOCIATIONS.values.flatten() + 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. @@ -186,6 +271,95 @@ 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.map(Interaction::getHint).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 if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.N) { + Html.fromHtml(html, Html.FROM_HTML_MODE_LEGACY) as Spannable + } else { + Html.fromHtml(html) 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 createTopicThumbnail0(): LessonThumbnail { diff --git a/domain/src/main/java/org/oppia/domain/util/JsonAssetRetriever.kt b/domain/src/main/java/org/oppia/domain/util/JsonAssetRetriever.kt index acd869ded28..1bd48d609ce 100644 --- a/domain/src/main/java/org/oppia/domain/util/JsonAssetRetriever.kt +++ b/domain/src/main/java/org/oppia/domain/util/JsonAssetRetriever.kt @@ -1,26 +1,23 @@ package org.oppia.domain.util -import android.content.Context import org.json.JSONArray import org.json.JSONObject +import org.oppia.util.caching.AssetRepository import javax.inject.Inject /** Utility that retrieves JSON assets and converts them to JSON objects. */ -class JsonAssetRetriever @Inject constructor(private val context: Context) { +class JsonAssetRetriever @Inject constructor(private val assetRepository: AssetRepository) { /** Loads the JSON string from an asset and converts it to a JSONObject */ fun loadJsonFromAsset(assetName: String): JSONObject? { - val assetManager = context.assets - val jsonContents = assetManager.open(assetName).bufferedReader().use { it.readText() } - return JSONObject(jsonContents) + return JSONObject(assetRepository.loadTextFileFromLocalAssets(assetName)) } /** Returns the on-disk size of the specified asset, in bytes. */ fun getAssetSize(assetName: String): Int { // Unfortunately, the entire file needs to be read to retrieve the asset size since JSON files are compressed in the // apk. See: https://stackoverflow.com/a/6187097. - // TODO(#386): Use an asset retriever to prefetch and cache these to avoid needing to keep re-reading them. - return context.assets.open(assetName).use { it.readBytes() }.size + return assetRepository.loadTextFileFromLocalAssets(assetName).toByteArray(Charsets.UTF_8).size } fun getStringsFromJSONArray(jsonData: JSONArray): List { diff --git a/domain/src/test/java/org/oppia/domain/audio/AudioPlayerControllerTest.kt b/domain/src/test/java/org/oppia/domain/audio/AudioPlayerControllerTest.kt index b7cf589819c..54689476dc3 100644 --- a/domain/src/test/java/org/oppia/domain/audio/AudioPlayerControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/audio/AudioPlayerControllerTest.kt @@ -27,6 +27,7 @@ import org.mockito.Mockito.verify import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoRule import org.oppia.domain.audio.AudioPlayerController.PlayStatus +import org.oppia.util.caching.CacheAssetsLocally import org.oppia.util.data.AsyncResult import org.oppia.util.logging.EnableConsoleLog import org.oppia.util.logging.EnableFileLog @@ -476,6 +477,10 @@ class AudioPlayerControllerTest { @GlobalLogLevel @Provides fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE + + @CacheAssetsLocally + @Provides + fun provideCacheAssetsLocally(): Boolean = false } // TODO(#89): Move this to a common test application component. diff --git a/domain/src/test/java/org/oppia/domain/exploration/ExplorationProgressControllerTest.kt b/domain/src/test/java/org/oppia/domain/exploration/ExplorationProgressControllerTest.kt index 317aae070e8..7b76696af1c 100644 --- a/domain/src/test/java/org/oppia/domain/exploration/ExplorationProgressControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/exploration/ExplorationProgressControllerTest.kt @@ -42,6 +42,10 @@ import org.oppia.domain.classify.rules.numberwithunits.NumberWithUnitsRuleModule import org.oppia.domain.classify.rules.numericinput.NumericInputRuleModule import org.oppia.domain.classify.rules.textinput.TextInputRuleModule import org.oppia.util.data.AsyncResult +import org.oppia.util.logging.EnableConsoleLog +import org.oppia.util.logging.EnableFileLog +import org.oppia.util.logging.GlobalLogLevel +import org.oppia.util.logging.LogLevel import org.oppia.util.threading.BackgroundDispatcher import org.oppia.util.threading.BlockingDispatcher import org.robolectric.annotation.Config @@ -1248,6 +1252,18 @@ class ExplorationProgressControllerTest { fun provideBlockingDispatcher(@TestDispatcher testDispatcher: TestCoroutineDispatcher): CoroutineDispatcher { return testDispatcher } + + @EnableConsoleLog + @Provides + fun provideEnableConsoleLog(): Boolean = true + + @EnableFileLog + @Provides + fun provideEnableFileLog(): Boolean = false + + @GlobalLogLevel + @Provides + fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE } // TODO(#89): Move this to a common test application component. diff --git a/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt b/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt index 91b24330188..459ea9ddc75 100644 --- a/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/topic/StoryProgressControllerTest.kt @@ -9,14 +9,24 @@ import dagger.BindsInstance import dagger.Component import dagger.Module import dagger.Provides +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher 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.oppia.util.logging.EnableConsoleLog +import org.oppia.util.logging.EnableFileLog +import org.oppia.util.logging.GlobalLogLevel +import org.oppia.util.logging.LogLevel +import org.oppia.util.threading.BackgroundDispatcher +import org.oppia.util.threading.BlockingDispatcher import org.robolectric.annotation.Config import javax.inject.Inject +import javax.inject.Qualifier import javax.inject.Singleton /** Tests for [StoryProgressController]. */ @@ -245,6 +255,9 @@ class StoryProgressControllerTest { .inject(this) } + @Qualifier + annotation class TestDispatcher + // TODO(#89): Move this to a common test application component. @Module class TestModule { @@ -253,6 +266,40 @@ class StoryProgressControllerTest { fun provideContext(application: Application): Context { return application } + + @ExperimentalCoroutinesApi + @Singleton + @Provides + @TestDispatcher + fun provideTestDispatcher(): CoroutineDispatcher { + return TestCoroutineDispatcher() + } + + @Singleton + @Provides + @BackgroundDispatcher + fun provideBackgroundDispatcher(@TestDispatcher testDispatcher: CoroutineDispatcher): CoroutineDispatcher { + return testDispatcher + } + + @Singleton + @Provides + @BlockingDispatcher + fun provideBlockingDispatcher(@TestDispatcher testDispatcher: CoroutineDispatcher): CoroutineDispatcher { + return testDispatcher + } + + @EnableConsoleLog + @Provides + fun provideEnableConsoleLog(): Boolean = true + + @EnableFileLog + @Provides + fun provideEnableFileLog(): Boolean = false + + @GlobalLogLevel + @Provides + fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE } // TODO(#89): Move this to a common test application component. diff --git a/domain/src/test/java/org/oppia/domain/topic/TopicControllerTest.kt b/domain/src/test/java/org/oppia/domain/topic/TopicControllerTest.kt index 4404a1ea41c..35499ca42d7 100644 --- a/domain/src/test/java/org/oppia/domain/topic/TopicControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/topic/TopicControllerTest.kt @@ -39,8 +39,13 @@ import org.oppia.app.model.SkillSummary import org.oppia.app.model.StorySummary import org.oppia.app.model.Topic import org.oppia.domain.exploration.TEST_EXPLORATION_ID_30 +import org.oppia.util.caching.CacheAssetsLocally import org.oppia.util.data.AsyncResult import org.oppia.util.data.DataProviders +import org.oppia.util.logging.EnableConsoleLog +import org.oppia.util.logging.EnableFileLog +import org.oppia.util.logging.GlobalLogLevel +import org.oppia.util.logging.LogLevel import org.oppia.util.threading.BackgroundDispatcher import org.oppia.util.threading.BlockingDispatcher import org.robolectric.annotation.Config @@ -762,6 +767,24 @@ class TopicControllerTest { fun provideBlockingDispatcher(@TestDispatcher testDispatcher: CoroutineDispatcher): CoroutineDispatcher { return testDispatcher } + + // TODO(#59): Either isolate these to their own shared test module, or use the real logging + // module in tests to avoid needing to specify these settings for tests. + @EnableConsoleLog + @Provides + fun provideEnableConsoleLog(): Boolean = true + + @EnableFileLog + @Provides + fun provideEnableFileLog(): Boolean = false + + @GlobalLogLevel + @Provides + fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE + + @CacheAssetsLocally + @Provides + fun provideCacheAssetsLocally(): Boolean = false } // TODO(#89): Move this to a common test application component. diff --git a/domain/src/test/java/org/oppia/domain/topic/TopicListControllerTest.kt b/domain/src/test/java/org/oppia/domain/topic/TopicListControllerTest.kt index 5dd4cd37109..e532ef9ab7d 100644 --- a/domain/src/test/java/org/oppia/domain/topic/TopicListControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/topic/TopicListControllerTest.kt @@ -16,6 +16,14 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.oppia.app.model.LessonThumbnailGraphic +import org.oppia.util.caching.CacheAssetsLocally +import org.oppia.util.logging.EnableConsoleLog +import org.oppia.util.logging.EnableFileLog +import org.oppia.util.logging.GlobalLogLevel +import org.oppia.util.logging.LogLevel +import org.oppia.util.parser.DefaultGcsPrefix +import org.oppia.util.parser.DefaultGcsResource +import org.oppia.util.parser.ImageDownloadUrlTemplate import org.oppia.util.threading.BackgroundDispatcher import org.oppia.util.threading.BlockingDispatcher import org.robolectric.annotation.Config @@ -304,6 +312,43 @@ class TopicListControllerTest { fun provideBlockingDispatcher(@TestDispatcher testDispatcher: CoroutineDispatcher): CoroutineDispatcher { return testDispatcher } + + @CacheAssetsLocally + @Provides + fun provideCacheAssetsLocally(): Boolean = false + + @Provides + @DefaultGcsPrefix + @Singleton + fun provideDefaultGcsPrefix(): String { + return "https://storage.googleapis.com/" + } + + @Provides + @DefaultGcsResource + @Singleton + fun provideDefaultGcsResource(): String { + return "oppiaserver-resources/" + } + + @Provides + @ImageDownloadUrlTemplate + @Singleton + fun provideImageDownloadUrlTemplate(): String { + return "%s/%s/assets/image/%s" + } + + @EnableConsoleLog + @Provides + fun provideEnableConsoleLog(): Boolean = true + + @EnableFileLog + @Provides + fun provideEnableFileLog(): Boolean = false + + @GlobalLogLevel + @Provides + fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE } // TODO(#89): Move this to a common test application component. diff --git a/utility/build.gradle b/utility/build.gradle index d660073493e..e8e8d65f4bd 100644 --- a/utility/build.gradle +++ b/utility/build.gradle @@ -58,6 +58,7 @@ dependencies { 'org.robolectric:robolectric:4.3', ) kapt( + 'com.github.bumptech.glide:compiler:4.9.0', 'com.google.dagger:dagger-compiler:2.24' ) kaptTest( diff --git a/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt b/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt new file mode 100644 index 00000000000..846f3f7a9dc --- /dev/null +++ b/utility/src/main/java/org/oppia/util/caching/AssetRepository.kt @@ -0,0 +1,130 @@ +package org.oppia.util.caching + +import android.content.Context +import org.oppia.util.logging.Logger +import java.io.File +import java.io.InputStream +import java.io.OutputStream +import java.net.URL +import java.util.concurrent.locks.ReentrantLock +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.concurrent.withLock + +// TODO(#169): Leverage this repository or a version of it for caching all topic contents in a proto. It may also be +// worth keeping a version of this repository for caching audio files within certain size limits for buffering during +// an exploration. +/** + * A generic repository for access local APK asset files, and downloading remote binary files. This repository aims to + * centralize caching management of external asset files to simplify downstream code, and allow assets to be retrieved + * quickly and synchronously. + */ +@Singleton +class AssetRepository @Inject constructor(private val context: Context, private val logger: Logger) { + private val repositoryLock = ReentrantLock() + + /** Map of asset names to file contents for text file assets. */ + private val textFileAssets = mutableMapOf() + + /** Returns the whole text contents of the file corresponding to the specified asset name. */ + fun loadTextFileFromLocalAssets(assetName: String): String { + repositoryLock.withLock { + primeTextFileFromLocalAssets(assetName) + return textFileAssets.getValue(assetName) + } + } + + /** Ensures the contents corresponding to the specified asset are available for quick retrieval. */ + fun primeTextFileFromLocalAssets(assetName: String) { + repositoryLock.withLock { + if (assetName !in textFileAssets) { + logger.d("AssetRepo", "Caching local text asset: $assetName") + textFileAssets[assetName] = context.assets.open(assetName).bufferedReader().use { it.readText() } + } + } + } + + /** + * Returns a function to retrieve the stream of the binary asset corresponding to the specified URL, to be called on a + * background thread. + */ + fun loadRemoteBinaryAsset(url: String): () -> ByteArray { + return { + val stream = openLocalCacheFileForRead(url) ?: openCachingStreamToRemoteFile(url) + stream.use { it.readBytes() } + } + } + + /** Ensures the contents corresponding to the specified URL are available for quick retrieval. */ + fun primeRemoteBinaryAsset(url: String) { + if (!getLocalCacheFile(url).exists()) { + // Otherwise, download it remotely and cache it locally. + logger.d("AssetRepo", "Downloading binary asset: $url") + val contents = openRemoteStream(url).use { it.readBytes() } + saveLocalCacheFile(url, contents) + } + } + + private fun openRemoteStream(url: String): InputStream { + return URL(url).openStream() + } + + /** Returns an [InputStream] that also saves its results to a local file. */ + private fun openCachingStreamToRemoteFile(url: String): InputStream { + val urlInStream = openRemoteStream(url) + val fileOutStream = openLocalCacheFileForWrite(url) + return object: InputStream() { + override fun available(): Int { + return urlInStream.available() + } + + override fun read(): Int { + val byte = urlInStream.read() + if (byte != -1) { + fileOutStream.write(byte) + } + return byte + } + + override fun read(b: ByteArray?): Int { + return read(b, 0, b!!.size) + } + + override fun read(b: ByteArray?, off: Int, len: Int): Int { + val count = urlInStream.read(b, off, len) + if (count > -1) { + fileOutStream.write(b, off, count) + } + return count + } + + override fun close() { + super.close() + fileOutStream.flush() + fileOutStream.close() + urlInStream.close() + } + } + } + + private fun openLocalCacheFileForRead(identifier: String): InputStream? { + val cacheFile = getLocalCacheFile(identifier) + return if (cacheFile.exists()) cacheFile.inputStream() else null + } + + private fun saveLocalCacheFile(identifier: String, contents: ByteArray) { + getLocalCacheFile(identifier).writeBytes(contents) + } + + private fun openLocalCacheFileForWrite(identifier: String): OutputStream { + return getLocalCacheFile(identifier).outputStream() + } + + private fun getLocalCacheFile(identifier: String): File { + return File(context.cacheDir, convertIdentifierToCacheFileName(identifier)) + } + + private fun convertIdentifierToCacheFileName(identifier: String): String { + return "${identifier.hashCode()}.cache" + } +} diff --git a/utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt b/utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt new file mode 100644 index 00000000000..c9f1232867b --- /dev/null +++ b/utility/src/main/java/org/oppia/util/caching/CachingAnnotations.kt @@ -0,0 +1,6 @@ +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 \ No newline at end of file diff --git a/utility/src/main/java/org/oppia/util/caching/CachingModule.kt b/utility/src/main/java/org/oppia/util/caching/CachingModule.kt new file mode 100644 index 00000000000..f167b295862 --- /dev/null +++ b/utility/src/main/java/org/oppia/util/caching/CachingModule.kt @@ -0,0 +1,12 @@ +package org.oppia.util.caching + +import dagger.Module +import dagger.Provides + +/** Provides dependencies corresponding to the app's caching policies. */ +@Module +class CachingModule { + @Provides + @CacheAssetsLocally + fun provideCacheAssetsLocally(): Boolean = false +} diff --git a/utility/src/main/java/org/oppia/util/parser/GlideImageLoader.kt b/utility/src/main/java/org/oppia/util/parser/GlideImageLoader.kt index 602b26c560e..32c362d4fa2 100644 --- a/utility/src/main/java/org/oppia/util/parser/GlideImageLoader.kt +++ b/utility/src/main/java/org/oppia/util/parser/GlideImageLoader.kt @@ -4,14 +4,27 @@ import android.content.Context import android.graphics.Bitmap import com.bumptech.glide.Glide import com.bumptech.glide.request.target.CustomTarget +import org.oppia.util.caching.AssetRepository +import org.oppia.util.caching.CacheAssetsLocally import javax.inject.Inject /** An [ImageLoader] that uses Glide. */ -class GlideImageLoader @Inject constructor(private val context: Context) : ImageLoader { +class GlideImageLoader @Inject constructor( + private val context: Context, + @CacheAssetsLocally private val cacheAssetsLocally: Boolean, + private val assetRepository: AssetRepository +) : ImageLoader { override fun load(imageUrl: String, target: CustomTarget) { + val model: Any = if (cacheAssetsLocally) { + object : ImageAssetFetcher { + override fun fetchImage(): ByteArray = assetRepository.loadRemoteBinaryAsset(imageUrl)() + + override fun getImageIdentifier(): String = imageUrl + } + } else imageUrl Glide.with(context) .asBitmap() - .load(imageUrl) + .load(model) .into(target) } } diff --git a/utility/src/main/java/org/oppia/util/parser/ImageAssetFetcher.kt b/utility/src/main/java/org/oppia/util/parser/ImageAssetFetcher.kt new file mode 100644 index 00000000000..1c21e0ee02c --- /dev/null +++ b/utility/src/main/java/org/oppia/util/parser/ImageAssetFetcher.kt @@ -0,0 +1,10 @@ +package org.oppia.util.parser + +/** Fetcher for image assets from the app's local asset repository. */ +internal interface ImageAssetFetcher { + /** Fetches an image asset. Must be called on a background thread. */ + fun fetchImage(): ByteArray + + /** Returns the identifier corresponding to this image. */ + fun getImageIdentifier(): String +} diff --git a/utility/src/main/java/org/oppia/util/parser/RepositoryGlideModule.kt b/utility/src/main/java/org/oppia/util/parser/RepositoryGlideModule.kt new file mode 100644 index 00000000000..a4cb6651bfc --- /dev/null +++ b/utility/src/main/java/org/oppia/util/parser/RepositoryGlideModule.kt @@ -0,0 +1,17 @@ +package org.oppia.util.parser + +import android.content.Context +import com.bumptech.glide.Glide +import com.bumptech.glide.Registry +import com.bumptech.glide.annotation.GlideModule +import com.bumptech.glide.module.AppGlideModule +import org.oppia.util.caching.AssetRepository +import java.io.InputStream + +/** Custom [AppGlideModule] to enable loading images from [AssetRepository] via Glide. */ +@GlideModule +class RepositoryGlideModule: AppGlideModule() { + override fun registerComponents(context: Context, glide: Glide, registry: Registry) { + registry.prepend(ImageAssetFetcher::class.java, InputStream::class.java, RepositoryModelLoader.Factory()) + } +} diff --git a/utility/src/main/java/org/oppia/util/parser/RepositoryModelLoader.kt b/utility/src/main/java/org/oppia/util/parser/RepositoryModelLoader.kt new file mode 100644 index 00000000000..8b393cbdfb7 --- /dev/null +++ b/utility/src/main/java/org/oppia/util/parser/RepositoryModelLoader.kt @@ -0,0 +1,51 @@ +package org.oppia.util.parser + +import com.bumptech.glide.Priority +import com.bumptech.glide.load.DataSource +import com.bumptech.glide.load.Options +import com.bumptech.glide.load.data.DataFetcher +import com.bumptech.glide.load.model.ModelLoader +import com.bumptech.glide.load.model.ModelLoaderFactory +import com.bumptech.glide.load.model.MultiModelLoaderFactory +import com.bumptech.glide.signature.ObjectKey +import java.io.ByteArrayInputStream +import java.io.InputStream + +// https://bumptech.github.io/glide/tut/custom-modelloader.html#an-empty-implementation. +/** ModelLoader for loading assets from the app's local asset repository. */ +internal class RepositoryModelLoader : ModelLoader { + override fun buildLoadData( + model: ImageAssetFetcher, + width: Int, + height: Int, + options: Options + ): ModelLoader.LoadData? { + return ModelLoader.LoadData(ObjectKey(model.getImageIdentifier()), RepositoryDataFetcher(model)) + } + + override fun handles(model: ImageAssetFetcher): Boolean = true + + private class RepositoryDataFetcher(private val fetcher: ImageAssetFetcher): DataFetcher { + override fun getDataClass(): Class = InputStream::class.java + + override fun cleanup() {} + + override fun getDataSource(): DataSource = DataSource.REMOTE + + override fun cancel() {} + + override fun loadData(priority: Priority, callback: DataFetcher.DataCallback) { + val imageData = fetcher.fetchImage() + callback.onDataReady(ByteArrayInputStream(imageData)) + } + } + + /** [ModelLoaderFactory] for creating new [RepositoryModelLoader]s. */ + internal class Factory: ModelLoaderFactory { + override fun build(multiFactory: MultiModelLoaderFactory): ModelLoader { + return RepositoryModelLoader() + } + + override fun teardown() {} + } +} diff --git a/utility/src/main/java/org/oppia/util/parser/UrlImageParser.kt b/utility/src/main/java/org/oppia/util/parser/UrlImageParser.kt index 1a40e326900..b5d2c92682a 100644 --- a/utility/src/main/java/org/oppia/util/parser/UrlImageParser.kt +++ b/utility/src/main/java/org/oppia/util/parser/UrlImageParser.kt @@ -48,7 +48,7 @@ class UrlImageParser private constructor( } override fun onResourceReady(resource: Bitmap, transition: Transition?) { - val drawable = BitmapDrawable(context.resources, resource) + val drawable = BitmapDrawable(context.resources, resource) htmlContentTextView.post { val drawableHeight = drawable.intrinsicHeight val drawableWidth = drawable.intrinsicWidth