Skip to content

Commit

Permalink
Merge branch 'integrate-multiple-classrooms-support' into introduce-a…
Browse files Browse the repository at this point in the history
…sset-download-script

Conflicts:
	WORKSPACE
	domain/BUILD.bazel
	domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt
	scripts/src/java/org/oppia/android/scripts/assets/DownloadLessons.kt
	scripts/src/java/org/oppia/android/scripts/assets/DtoProtoToLegacyProtoConverter.kt
	scripts/src/java/org/oppia/android/scripts/gae/GaeAndroidEndpointJsonImpl.kt
	scripts/src/java/org/oppia/android/scripts/gae/compat/TopicPackRepository.kt
	scripts/src/java/org/oppia/android/scripts/gae/proto/JsonToProtoConverter.kt
	scripts/src/java/org/oppia/android/scripts/gae/proto/LocalizationTracker.kt
  • Loading branch information
BenHenning committed Jun 21, 2024
2 parents 6b1d81c + 23b36d9 commit 6e87842
Show file tree
Hide file tree
Showing 13 changed files with 306 additions and 107 deletions.
6 changes: 3 additions & 3 deletions domain/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ MIGRATED_PROD_FILES = glob([

DOMAIN_ASSETS = generate_assets_list_from_text_protos(
name = "domain_assets",
classroom_list_file_names = [
"classrooms",
],
exploration_file_names = [
"test_exp_id_2",
"13",
Expand Down Expand Up @@ -99,6 +96,9 @@ DOMAIN_ASSETS = generate_assets_list_from_text_protos(
"GJ2rLXRKD5hw",
"omzF4oqgeTXd",
],
classroom_list_file_names = [
"classrooms",
],
)

kt_android_library(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.oppia.android.app.model.StoryRecord
import org.oppia.android.app.model.StorySummary
import org.oppia.android.app.model.SubtitledHtml
import org.oppia.android.app.model.Topic
import org.oppia.android.app.model.ClassroomList
import org.oppia.android.app.model.TopicList
import org.oppia.android.app.model.TopicPlayAvailability
import org.oppia.android.app.model.TopicPlayAvailability.AvailabilityCase.AVAILABLE_TO_PLAY_IN_FUTURE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.oppia.android.scripts.assets.DtoProtoToLegacyProtoConverter.convertTo
import org.oppia.android.scripts.assets.DtoProtoToLegacyProtoConverter.convertToExploration
import org.oppia.android.scripts.assets.DtoProtoToLegacyProtoConverter.convertToStoryRecord
import org.oppia.android.scripts.assets.DtoProtoToLegacyProtoConverter.convertToSubtopicRecord
import org.oppia.android.scripts.assets.DtoProtoToLegacyProtoConverter.convertToClassroomList
import org.oppia.android.scripts.assets.DtoProtoToLegacyProtoConverter.convertToTopicRecord
import org.oppia.android.scripts.gae.GaeAndroidEndpoint
import org.oppia.android.scripts.gae.GaeAndroidEndpointJsonImpl
Expand Down Expand Up @@ -100,6 +101,38 @@ import org.oppia.proto.v1.api.DownloadRequestStructureIdentifierDto.Builder as D
import org.oppia.proto.v1.structure.DragAndDropSortInputInstanceDto.RuleSpecDto as DragDropSortRuleSpecDto
import org.oppia.proto.v1.structure.ItemSelectionInputInstanceDto.RuleSpecDto as ItemSelRuleSpecDto

/*
TODO: Thoughts for improving the script:
- Introduce args to allow enabling/disabling specific checks.
- Change output structure such that just the top-level asset dir is provided and then the following are always generated:
- prod_assets/{images,*.pb}
- pipeline/
- images_{conversions,renames}/
- protov{1,2}/{textproto,binary}/
- proto_conversion_info.log
- classrooms_cache/ (full picture isn't presented, but the idea is to show all versions downloaded & analyses from them, plus data hierarchy)
- {separate_classroom_name}_topics/
- classroom_{name}_download_info.log
- {topic_id}_v{version_name}/
- topic_{id}_summary_download_info.log
- topic_summary.json
...
- Change cache_mode to just be lazy/force (with lazy being default). 'none' isn't necessary.
- Generate download_info.log files for the cache that include:
- The specific URL used to download each file.
- Any specific irrecoverable failures encountered during import (prior to conversion to proto).
- All optional & non-optional failures from json to protov2.
- Generate a proto_conversion_info.log that includes:
- Any issues or warnings from protov2 to protov1.
- Broadly, reduce all output such that the script is leaning much more heavily on files (where it also should output much more diagnostic information than done today). This keeps the script output lean to pass/fail & progress indicators, and the generated output logs to enough context to investigate both recoverable and irrecoverable failures.
- NOTE: Enabling addAllRequiredAdditionalLanguages below forces large batches of version requests (which leads to significant performance issues on Oppia web). Things to do here:
- 1. Introduce a batch fetcher for merging different structure versions & IDs (and maybe types) since the endpoint can handle this. It's fewer items to pass back and forth.
- 2. Make web more fault tolerant; it's fine if it can't fetch everything. Provide enough context to the client to retry failing requests (though we need to pass *why* it fails). Consider also avoiding using memcache in some cases since it doesn't need to cache old versions of structures.
- 3. Ensure the fetcher utilizes the retry functionality from (2).
- 4. Once the fetch succeeds again, look into fixing the compatibility problems that lead to the version explosion. Make sure on-disk caching works correctly.
- Consider looking into Firebase auth now vs. later since we need to make backend changes, anyway.
*/

// TODO: hook up to language configs for prod/dev language restrictions.
// TODO: Consider using better argument parser so that dev env vals can be defaulted.
fun main(vararg args: String) {
Expand Down Expand Up @@ -183,7 +216,6 @@ class LessonDownloader(
cacheDir,
forceCacheLoad,
coroutineDispatcher,
topicDependencies = topicDependenciesTable,
imageDownloader,
forcedVersions = downloadListVersions
)
Expand Down Expand Up @@ -226,7 +258,7 @@ class LessonDownloader(
compatibilityContext = ProtoVersionProvider.createCompatibilityContext()
// No structures are considered already downloaded. TODO: Integrate with local files cache?
requestedDefaultLanguage = defaultLanguage
// addAllRequiredAdditionalLanguages(requestedLanguages)
// addAllRequiredAdditionalLanguages(requestedLanguages)
addAllSupportedAdditionalLanguages(requestedLanguages)
}.build()

Expand Down Expand Up @@ -257,6 +289,14 @@ class LessonDownloader(
" ${downloadableTopics.size} are downloadable, IDs: $downloadableTopicIds." +
" ${futureTopicIds.size} topics will later be available, IDs: $futureTopicIds."
)
println("${listResponse.classroomsCount} classrooms are available:")
listResponse.classroomsList.forEach { classroom ->
val defaultLocalization = classroom.localizations.defaultMapping
val nameContentId = classroom.name.contentId
val textMapping = defaultLocalization.localizableTextContentMappingMap[nameContentId]
val classroomName = textMapping?.singleLocalizableText?.text?.takeIf { it.isNotEmpty() } ?: "<unknown>"
println("- ${classroom.id}: $classroomName (has ${classroom.topicIdsCount} topics)")
}

println()
val contentRequest =
Expand Down Expand Up @@ -545,6 +585,9 @@ class LessonDownloader(
val conceptCardImageReplacements = conceptCards.associate { dto ->
dto.skillId to images.computeReplacements(ImageContainerType.SKILL, dto.skillId)
}
val classroomImageReplacements = listResponse.classroomsList.associate { classroom ->
classroom.id to images.computeReplacements(ImageContainerType.CLASSROOM, classroom.id)
}
val writeProtoV1AsyncResults = topicSummaries.map { (topicId, topicSummary) ->
val imageReplacements = images.computeReplacements(ImageContainerType.TOPIC, topicId)
writeProtosAsync(protoV1Dir, topicId, topicSummary.convertToTopicRecord(imageReplacements))
Expand Down Expand Up @@ -582,10 +625,15 @@ class LessonDownloader(
)
) + explorations.map { exp ->
val imageReplacements = images.computeReplacements(ImageContainerType.EXPLORATION, exp.id)
val packs = explorationPacks.getValue(exp.id)
// TODO: The listOf() default here allows some explorations to have no translations.
val packs = explorationPacks[exp.id] ?: listOf()
writeProtosAsync(protoV1Dir, exp.id, exp.convertToExploration(imageReplacements, packs))
} + writeProtosAsync(
protoV1Dir, baseName = "classrooms", topicSummaries.values.convertToClassroomList()
protoV1Dir,
baseName = "classrooms",
listResponse.classroomsList.convertToClassroomList(
topicSummaries.values, classroomImageReplacements
)
)

// Wait for all proto writes to finish.
Expand All @@ -596,6 +644,7 @@ class LessonDownloader(
println("- Proto v1 text protos can be found in: ${textProtoV1Dir.path}")
println("- Proto v1 binary protos can be found in: ${binaryProtoV1Dir.path}")

// TODO: Reenable analysis for all structures when non-explorations are expected to be fully translated (or a version can be downloaded by the script).
val analyzer = CompatibilityAnalyzer(requestedLanguages + setOf(defaultLanguage))
topicSummaries.values.forEach(analyzer::track)
upcomingTopics.forEach(analyzer::track)
Expand Down Expand Up @@ -2206,41 +2255,6 @@ class LessonDownloader(
}
}

private const val PLACE_VALUES_ID = "iX9kYCjnouWN"
private const val ADDITION_AND_SUBTRACTION_ID = "sWBXKH4PZcK6"
private const val MULTIPLICATION_ID = "C4fqwrvqWpRm"
private const val DIVISION_ID = "qW12maD4hiA8"
private const val EXPRESSIONS_AND_EQUATIONS_ID = "dLmjjMDbCcrf"
private const val FRACTIONS_ID = "0abdeaJhmfPm"
private const val RATIOS_ID = "5g0nxGUmx5J5"

private val fractionsDependencies by lazy {
setOf(ADDITION_AND_SUBTRACTION_ID, MULTIPLICATION_ID, DIVISION_ID)
}
private val ratiosDependencies by lazy {
setOf(ADDITION_AND_SUBTRACTION_ID, MULTIPLICATION_ID, DIVISION_ID)
}
private val additionAndSubtractionDependencies by lazy { setOf(PLACE_VALUES_ID) }
private val multiplicationDependencies by lazy { setOf(ADDITION_AND_SUBTRACTION_ID) }
private val divisionDependencies by lazy { setOf(MULTIPLICATION_ID) }
private val placeValuesDependencies by lazy { setOf<String>() }
private val expressionsAndEquationsDependencies by lazy {
setOf(ADDITION_AND_SUBTRACTION_ID, MULTIPLICATION_ID, DIVISION_ID)
}

// TODO: Document that this exists since Oppia web doesn't yet provide signals on order.
private val topicDependenciesTable by lazy {
mapOf(
FRACTIONS_ID to fractionsDependencies,
RATIOS_ID to ratiosDependencies,
ADDITION_AND_SUBTRACTION_ID to additionAndSubtractionDependencies,
MULTIPLICATION_ID to multiplicationDependencies,
DIVISION_ID to divisionDependencies,
PLACE_VALUES_ID to placeValuesDependencies,
EXPRESSIONS_AND_EQUATIONS_ID to expressionsAndEquationsDependencies,
)
}

private fun createSubtopicId(topicId: String, subtopicIndex: Int): SubtopicPageIdDto {
return SubtopicPageIdDto.newBuilder().apply {
this.topicId = topicId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import org.oppia.android.app.model.StoryRecord
import org.oppia.android.app.model.SubtitledHtml
import org.oppia.android.app.model.SubtitledUnicode
import org.oppia.android.app.model.SubtopicRecord
import org.oppia.android.app.model.ClassroomList
import org.oppia.android.app.model.ClassroomRecord
import org.oppia.android.app.model.TopicRecord
import org.oppia.android.app.model.TranslatableHtmlContentId
import org.oppia.android.app.model.TranslatableSetOfNormalizedString
Expand All @@ -46,6 +48,7 @@ import org.oppia.proto.v1.structure.ContentLocalizationDto
import org.oppia.proto.v1.structure.ContentLocalizationsDto
import org.oppia.proto.v1.structure.ContinueInstanceDto
import org.oppia.proto.v1.structure.DownloadableTopicSummaryDto
import org.oppia.proto.v1.structure.ClassroomDto
import org.oppia.proto.v1.structure.DragAndDropSortInputInstanceDto
import org.oppia.proto.v1.structure.DragAndDropSortInputInstanceDto.RuleSpecDto.HasElementXBeforeElementYSpecDto
import org.oppia.proto.v1.structure.DragAndDropSortInputInstanceDto.RuleSpecDto.IsEqualToOrderingWithOneItemAtIncorrectPositionSpecDto
Expand Down Expand Up @@ -125,13 +128,21 @@ private typealias XlatableContentIdsSet = SetOfTranslatableHtmlContentIds
private typealias XlatableContentIdsSetList = ListOfSetsOfTranslatableHtmlContentIds

object DtoProtoToLegacyProtoConverter {
fun Iterable<DownloadableTopicSummaryDto>.convertToClassroomList(): ClassroomList {
// TODO: Finish this.
// val dtos = this
// return TopicIdList.newBuilder().apply {
// addAllTopicIds(dtos.map { it.id })
// }.build()
return ClassroomList.getDefaultInstance()
fun Iterable<ClassroomDto>.convertToClassroomList(
topicSummaries: Iterable<DownloadableTopicSummaryDto>,
allImageReferenceReplacements: Map<String, Map<String, String>>
): ClassroomList {
val dtos = this
val topicSummaryMap = topicSummaries.associateBy { it.id }
return ClassroomList.newBuilder().apply {
addAllClassrooms(
dtos.map {
it.convertToClassroomRecord(
topicSummaryMap, allImageReferenceReplacements.getValue(it.id)
)
}
)
}.build()
}

fun DownloadableTopicSummaryDto.convertToTopicRecord(
Expand Down Expand Up @@ -237,6 +248,25 @@ object DtoProtoToLegacyProtoConverter {
}.build()
}

private fun ClassroomDto.convertToClassroomRecord(
topicSummaryMap: Map<String, DownloadableTopicSummaryDto>,
imageReferenceReplacements: Map<String, String>
): ClassroomRecord {
val dto = this
return ClassroomRecord.newBuilder().apply {
this.id = dto.id
this.classroomThumbnail =
dto.localizations.extractDefaultThumbnail(imageReferenceReplacements)
putAllWrittenTranslations(dto.localizations.toTranslationMappings(imageReferenceReplacements))
this.translatableTitle = dto.localizations.extractDefaultSubtitledHtml(dto.name)
putAllTopicPrerequisites(dto.topicIdsList.associateWith { topicId ->
ClassroomRecord.TopicIdList.newBuilder().apply {
addAllTopicIds(topicSummaryMap.getValue(topicId).prerequisiteTopicIdsList)
}.build()
})
}.build()
}

private fun ChapterSummaryDto.convertToChapterRecord(
imageReferenceReplacements: Map<String, String>
): ChapterRecord {
Expand Down
Loading

0 comments on commit 6e87842

Please sign in to comment.