Skip to content

Commit

Permalink
Fix #369: Calculate realistic download size (#393)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
BenHenning authored Nov 20, 2019
1 parent aa120e4 commit adb33f9
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,21 @@ import org.oppia.app.R
import org.oppia.app.fragment.FragmentScope
import org.oppia.app.model.Topic
import org.oppia.app.viewmodel.ObservableViewModel
import java.text.DecimalFormat
import javax.inject.Inject

/** [ViewModel] for showing topic overview details. */
@FragmentScope
class TopicOverviewViewModel @Inject constructor() : ObservableViewModel() {
private val decimalFormat: DecimalFormat = DecimalFormat("#.###")

val topic = ObservableField<Topic>(Topic.getDefaultInstance())

var downloadStatusIndicatorDrawableResourceId = ObservableField<Int>(R.drawable.ic_available_offline_primary_24dp)
var downloadStatusIndicatorDrawableResourceId = ObservableField(R.drawable.ic_available_offline_primary_24dp)

/** Returns the number of megabytes of disk space this topic requires, formatted for display. */
fun getTopicSizeMb(): String {
val topicSizeMb: Double = (topic.get()?.diskSizeBytes ?: 0) / (1024.0 * 1024.0)
return decimalFormat.format(topicSizeMb)
}
}
2 changes: 1 addition & 1 deletion app/src/main/res/layout/topic_overview_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@
android:layout_marginTop="12dp"
android:fontFamily="sans-serif"
android:gravity="top"
android:text="@{String.format(@string/topic_download_text, viewModel.topic.getSerializedSize())}"
android:text="@{String.format(@string/topic_download_text, viewModel.getTopicSizeMb())}"
android:textColor="@color/oppiaPrimaryText"
android:textSize="18sp"
android:textStyle="italic"
Expand Down
28 changes: 28 additions & 0 deletions domain/src/main/java/org/oppia/domain/topic/TopicController.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.oppia.domain.util.StateRetriever
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProvider
import org.oppia.util.data.DataProviders
import java.io.File
import javax.inject.Inject
import javax.inject.Singleton

Expand Down Expand Up @@ -54,6 +55,26 @@ 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(
FRACTIONS_TOPIC_ID to listOf(
"fractions_exploration0.json",
"fractions_exploration1.json",
"fractions_questions.json",
"fractions_skills.json",
"fractions_stories.json",
"fractions_topic.json"
),
RATIOS_TOPIC_ID to listOf(
"ratios_exploration0.json",
"ratios_exploration1.json",
"ratios_exploration2.json",
"ratios_exploration3.json",
"ratios_questions.json",
"ratios_skills.json",
"ratios_stories.json",
"ratios_topic.json"
)
)

private const val QUESTION_DATA_PROVIDER_ID = "QuestionDataProvider"

Expand Down Expand Up @@ -358,9 +379,16 @@ class TopicController @Inject constructor(
.addAllSkill(createSkillsFromJson(skillFileName))
.addAllStory(createStoriesFromJson(storyFileName))
.setTopicThumbnail(TOPIC_THUMBNAILS.getValue(topicId))
.setDiskSizeBytes(computeTopicSizeBytes(TOPIC_FILE_ASSOCIATIONS.getValue(topicId)))
.build()
}

private fun computeTopicSizeBytes(constituentFiles: List<String>): Long {
// TODO(#169): Compute this based on protos & the combined topic package.
// TODO(#386): Incorporate audio & image files in this computation.
return constituentFiles.map(jsonAssetRetriever::getAssetSize).map(Int::toLong).reduceRight(Long::plus)
}

/** Utility to create the skill list of a topic from its json representation. The json file is expected to have
* a key called 'skill_list' that contains an array of skill objects, each with the key 'skill'. */
private fun createSkillsFromJson(fileName: String): List<SkillSummary> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ class JsonAssetRetriever @Inject constructor(private val context: Context) {
return JSONObject(jsonContents)
}

/** 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
}

fun getStringsFromJSONArray(jsonData: JSONArray): List<String> {
val stringList = mutableListOf<String>()
for (i in 0 until jsonData.length()) {
Expand Down
3 changes: 3 additions & 0 deletions model/src/main/proto/topic.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ message Topic {

// The thumbnail corresponding to this topic.
LessonThumbnail topic_thumbnail = 6;

// The number of on-disk bytes this topic consumes.
int64 disk_size_bytes = 7;
}

// Corresponds to a concept card that can be displayed for a specific skill.
Expand Down

0 comments on commit adb33f9

Please sign in to comment.