Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,10 @@
<activity
android:name=".app.testing.TextInputLayoutBindingAdaptersTestActivity"
android:theme="@style/OppiaThemeWithoutActionBar" />
<provider
android:name=".app.application.dev.ThumbnailContentProvider"
android:authorities="org.oppia.android.provider"
android:exported="false" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go into its own AndroidManifest.xml in the same package as the content provider. We will rely on the manifest merge to include this at the end. Adding it here means we need to ship the provider in production which we absolutely don't want to do.

<provider
android:name="androidx.startup.InitializationProvider"
android:authorities="${applicationId}.androidx-startup"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ kt_android_library(
"DeveloperApplicationComponent.kt",
"DeveloperBuildFlavorModule.kt",
"DeveloperOppiaApplication.kt",
"ThumbnailContentProvider.kt",
],
visibility = [
"//:oppia_binary_visibility",
Expand All @@ -25,5 +26,6 @@ kt_android_library(
"//domain/src/main/java/org/oppia/android/domain/platformparameter:debug_module",
"//utility/src/main/java/org/oppia/android/util/logging/firebase:debug_module",
"//utility/src/main/java/org/oppia/android/util/networking:debug_module",
"//utility/src/main/java/org/oppia/android/util/parser/image:dev_image_parsing_module",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ import org.oppia.android.util.networking.NetworkConnectionDebugUtilModule
import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule
import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule
import org.oppia.android.util.parser.image.GlideImageLoaderModule
import org.oppia.android.util.parser.image.ImageParsingModule
import org.oppia.android.util.parser.image.DevImageParsingModule
import org.oppia.android.util.system.OppiaClockModule
import org.oppia.android.util.threading.DispatcherModule
import javax.inject.Singleton
Expand All @@ -79,7 +79,7 @@ import javax.inject.Singleton
MultipleChoiceInputModule::class, NumberWithUnitsRuleModule::class,
NumericInputRuleModule::class, TextInputRuleModule::class, DragDropSortInputModule::class,
InteractionsModule::class, GcsResourceModule::class, GlideImageLoaderModule::class,
ImageParsingModule::class, HtmlParserEntityTypeModule::class, CachingModule::class,
DevImageParsingModule::class, HtmlParserEntityTypeModule::class, CachingModule::class,
QuestionModule::class, DebugLogReportingModule::class, AccessibilityProdModule::class,
ImageClickInputModule::class, LogStorageModule::class, IntentFactoryShimModule::class,
ViewBindingShimModule::class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package org.oppia.android.app.application.dev
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think org.oppia.android.app.utility.imageloading is probably an okay place for now. This is a new pattern so we don't have an obvious place for it to go, but the current package is meant for top-level application setup which doesn't seem like the correct place.

The new package will need to be a proper Bazel package (i.e. with a BUILD.bazel) file, and you'll need to reference that new library target as a dependency for the top-level //:oppia_dev target as one of its dependencies.


import android.content.ContentProvider
import android.content.ContentValues
import android.content.UriMatcher
import android.database.Cursor
import android.graphics.Bitmap
import android.graphics.Canvas
import android.net.Uri
import android.os.ParcelFileDescriptor
import androidx.appcompat.content.res.AppCompatResources
import org.oppia.android.app.views.R
import java.io.File
import java.io.FileOutputStream

/**
* A developer-only [ContentProvider] that serves local drawable resources as thumbnail images.
*
* In developer builds, thumbnail image loading is redirected from Google Cloud Storage to this
* content provider via the `content://org.oppia.android.provider` URI scheme. When a thumbnail
* filename (e.g., `baker.img`) is requested, this provider maps it to the corresponding local
* drawable resource (e.g., `lesson_thumbnail_graphic_baker.xml`) and returns the rendered image.
*
* This allows proto lessons with `thumbnail_filename` placeholders to successfully load thumbnails
* without needing access to GCS, which is unavailable in dev builds.
*/
class ThumbnailContentProvider : ContentProvider() {

companion object {
private const val AUTHORITY = "org.oppia.android.provider"
private const val THUMBNAIL_MATCH = 1
private val uriMatcher = UriMatcher(UriMatcher.NO_MATCH).apply {
// Match any path under the authority — the thumbnail filename is extracted from the
// last path segment of the URI.
addURI(AUTHORITY, "*/*/*/*/#", THUMBNAIL_MATCH)
addURI(AUTHORITY, "*/*/*/*", THUMBNAIL_MATCH)
addURI(AUTHORITY, "thumbnail/*", THUMBNAIL_MATCH)
addURI(AUTHORITY, "#", THUMBNAIL_MATCH)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all of these needed? It seems like we will subpath based on <entity_type>/<entity_id>/assets/<image|thumbnail>/<file.name> which seems like it only needs a single URI matcher, or am I missing something?

}

/**
* Maps thumbnail filenames (e.g., "baker.img") to their corresponding drawable resource IDs.
*
* This mapping must be kept in sync with the `thumbnail_filename` values defined in the
* textproto asset files.
*/
private val THUMBNAIL_FILENAME_TO_DRAWABLE_MAP = mapOf(
"baker.img" to R.drawable.lesson_thumbnail_graphic_baker,
"child_with_book.img" to R.drawable.lesson_thumbnail_graphic_child_with_book,
"child_with_cupcakes.img" to R.drawable.lesson_thumbnail_graphic_child_with_cupcakes,
"child_with_fractions_homework.img" to
R.drawable.lesson_thumbnail_graphic_child_with_fractions_homework,
"duck_and_chicken.img" to R.drawable.lesson_thumbnail_graphic_duck_and_chicken,
"person_with_pie_chart.img" to R.drawable.lesson_thumbnail_graphic_person_with_pie_chart
)

/** Default drawable used when a filename doesn't match any known thumbnail. */
private val DEFAULT_THUMBNAIL_DRAWABLE = R.drawable.lesson_thumbnail_graphic_baker
}

override fun onCreate(): Boolean = true

override fun query(
uri: Uri,
projection: Array<String>?,
selection: String?,
selectionArgs: Array<String>?,
sortOrder: String?
): Cursor? = null

override fun getType(uri: Uri): String = "image/png"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? Perhaps these could be SVGs, instead?


override fun insert(uri: Uri, values: ContentValues?): Uri? = null

override fun delete(uri: Uri, selection: String?, selectionArgs: Array<String>?): Int = 0

override fun update(
uri: Uri,
values: ContentValues?,
selection: String?,
selectionArgs: Array<String>?
): Int = 0

override fun openFile(uri: Uri, mode: String): ParcelFileDescriptor? {
val context = context ?: return null
val thumbnailFilename = uri.lastPathSegment ?: return null

val drawableResId = THUMBNAIL_FILENAME_TO_DRAWABLE_MAP[thumbnailFilename]
?: DEFAULT_THUMBNAIL_DRAWABLE

val drawable = AppCompatResources.getDrawable(context, drawableResId)
?: return null

// Render the drawable (which may be a vector/XML drawable) to a bitmap.
val width = if (drawable.intrinsicWidth > 0) drawable.intrinsicWidth else 192
val height = if (drawable.intrinsicHeight > 0) drawable.intrinsicHeight else 192
val bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888)
val canvas = Canvas(bitmap)
drawable.setBounds(0, 0, canvas.width, canvas.height)
drawable.draw(canvas)

// Write the bitmap to a temporary file and return a file descriptor.
val cacheFile = File(context.cacheDir, "thumbnail_${thumbnailFilename.hashCode()}.png")
FileOutputStream(cacheFile).use { outputStream ->
bitmap.compress(Bitmap.CompressFormat.PNG, 100, outputStream)
}
bitmap.recycle()

return ParcelFileDescriptor.open(cacheFile, ParcelFileDescriptor.MODE_READ_ONLY)
}
}
1 change: 1 addition & 0 deletions domain/src/main/assets/GJ2rLXRKD5hw.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ subtopic_ids: 4
is_published: true
has_practice_questions: true
topic_thumbnail {
thumbnail_filename: "child_with_fractions_homework.img"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here & elsewhere: how did you decide on which thumbnails to use? We ought to match exactly the current JSON behavior to make it an easier before-and-after check.

}
written_translations {
key: "title"
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/GJ2rLXRKD5hw_1.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ written_translation {
skill_ids: "5RM9KPfQxobH"
skill_ids: "B39yK4cbHZYI"
subtopic_thumbnail {
thumbnail_filename: "child_with_fractions_homework.img"
background_color_rgb: 16741119
}
title {
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/GJ2rLXRKD5hw_2.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ written_translation {
skill_ids: "B39yK4cbHZYI"
skill_ids: "UxTGIJqaHMLa"
subtopic_thumbnail {
thumbnail_filename: "child_with_book.img"
background_color_rgb: 16741119
}
title {
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/GJ2rLXRKD5hw_3.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ written_translation {
skill_ids: "5RM9KPfQxobH"
skill_ids: "UxTGIJqaHMLa"
subtopic_thumbnail {
thumbnail_filename: "child_with_cupcakes.img"
background_color_rgb: 16741119
}
title {
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/GJ2rLXRKD5hw_4.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ skill_ids: "B39yK4cbHZYI"
skill_ids: "5RM9KPfQxobH"
skill_ids: "UxTGIJqaHMLa"
subtopic_thumbnail {
thumbnail_filename: "baker.img"
background_color_rgb: 16741119
}
title {
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/omzF4oqgeTXd.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ subtopic_ids: 1
is_published: true
has_practice_questions: true
topic_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
}
written_translations {
key: "title"
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/omzF4oqgeTXd_1.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ written_translation {
}
skill_ids: "NGZ89uMw0IGV"
subtopic_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
background_color_rgb: 16741119
}
title {
Expand Down
4 changes: 4 additions & 0 deletions domain/src/main/assets/test_story_id_0.textproto
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
story_id: "test_story_id_0"
story_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
background_color_rgb: 11761905
}
chapters {
exploration_id: "test_exp_id_2"
chapter_thumbnail {
thumbnail_filename: "child_with_fractions_homework.img"
background_color_rgb: 14061432
}
written_translations {
Expand All @@ -29,6 +31,7 @@ chapters {
chapters {
exploration_id: "13"
chapter_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
background_color_rgb: 14061432
}
written_translations {
Expand All @@ -52,6 +55,7 @@ chapters {
chapters {
exploration_id: "test_exp_id_5"
chapter_thumbnail {
thumbnail_filename: "person_with_pie_chart.img"
background_color_rgb: 14061432
}
written_translations {
Expand Down
2 changes: 2 additions & 0 deletions domain/src/main/assets/test_story_id_2.textproto
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
story_id: "test_story_id_2"
story_thumbnail {
thumbnail_filename: "child_with_cupcakes.img"
background_color_rgb: 11761905
}
chapters {
exploration_id: "test_exp_id_4"
chapter_thumbnail {
thumbnail_filename: "baker.img"
background_color_rgb: 14061432
}
written_translations {
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/test_topic_id_0.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ subtopic_ids: 1
is_published: true
has_practice_questions: true
topic_thumbnail {
thumbnail_filename: "child_with_fractions_homework.img"
}
written_translations {
key: "title"
Expand Down
1 change: 1 addition & 0 deletions domain/src/main/assets/test_topic_id_1.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ canonical_story_ids: "test_story_id_2"
is_published: true
has_practice_questions: false
topic_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
}
written_translations {
key: "title"
Expand Down
3 changes: 3 additions & 0 deletions domain/src/main/assets/wAMdg4oOClga.textproto
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
story_id: "wAMdg4oOClga"
story_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
background_color_rgb: 11786481
}
chapters {
exploration_id: "2mzzFVDLuAj8"
chapter_thumbnail {
thumbnail_filename: "duck_and_chicken.img"
background_color_rgb: 14061432
}
written_translations {
Expand All @@ -28,6 +30,7 @@ chapters {
chapters {
exploration_id: "5NWuolNcwH6e"
chapter_thumbnail {
thumbnail_filename: "person_with_pie_chart.img"
background_color_rgb: 14088824
}
written_translations {
Expand Down
3 changes: 3 additions & 0 deletions domain/src/main/assets/wANbh4oOClga.textproto
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
story_id: "wANbh4oOClga"
story_thumbnail {
thumbnail_filename: "baker.img"
background_color_rgb: 11786481
}
chapters {
exploration_id: "umPkwp0L1M0-"
chapter_thumbnail {
thumbnail_filename: "baker.img"
background_color_rgb: 14061432
}
written_translations {
Expand All @@ -29,6 +31,7 @@ chapters {
chapters {
exploration_id: "MjZzEVOG47_1"
chapter_thumbnail {
thumbnail_filename: "child_with_fractions_homework.img"
background_color_rgb: 14055672
}
written_translations {
Expand Down
3 changes: 3 additions & 0 deletions domain/src/main/assets/xBSdg4oOClga.textproto
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
story_id: "xBSdg4oOClga"
story_thumbnail {
thumbnail_filename: "child_with_cupcakes.img"
background_color_rgb: 11792856
}
chapters {
exploration_id: "k2bQ7z5XHNbK"
chapter_thumbnail {
thumbnail_filename: "child_with_cupcakes.img"
background_color_rgb: 16158584
}
written_translations {
Expand All @@ -28,6 +30,7 @@ chapters {
chapters {
exploration_id: "tIoSb3HZFN6e"
chapter_thumbnail {
thumbnail_filename: "child_with_book.img"
background_color_rgb: 16158584
}
written_translations {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,21 @@ kt_android_library(
],
)

kt_android_library(
name = "dev_image_parsing_module",
srcs = [
"DevImageParsingModule.kt",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention in the codebase is to use 'debug' for developer dependencies, and to put it at the end of the name before module, e.g. ImageParsingDebugModule. Please make sure to also update the library target name.

],
visibility = [
"//:oppia_testing_visibility",
],
deps = [
":image_parsing_annonations",
"//:dagger",
"//third_party:javax_inject_javax_inject",
],
)

kt_android_library(
name = "image_targets",
srcs = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.oppia.android.util.parser.image

import dagger.Module
import dagger.Provides
import javax.inject.Singleton

/**
* Provides developer-only image-extraction URL dependencies.
*
* This module replaces [ImageParsingModule] in developer builds. It overrides the default GCS
* prefix with a `content://` URI scheme that routes image requests to a local
* [ContentProvider][android.content.ContentProvider] instead of Google Cloud Storage. This allows
* thumbnail images to be served from local drawable resources in dev builds where GCS assets are
* inaccessible.
*/
@Module
class DevImageParsingModule {
@Provides
@DefaultGcsPrefix
@Singleton
fun provideDefaultGcsPrefix(): String {
return "content://org.oppia.android.provider"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use content://org.oppia.android.provider/gcs so as to not pollute future content provider potential.

}

@Provides
@ImageDownloadUrlTemplate
@Singleton
fun provideImageDownloadUrlTemplate(): String {
return "%s/%s/assets/image/%s"
}

@Provides
@ThumbnailDownloadUrlTemplate
@Singleton
fun provideThumbnailDownloadUrlTemplate(): String {
return "%s/%s/assets/thumbnail/%s"
}
}
Loading