Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #386: Add support for local caching of audio & image assets #399

Merged
merged 14 commits into from
Nov 20, 2019

Conversation

BenHenning
Copy link
Member

Fix #386.

This PR introduces an AssetRepository that supports caching both JSON and and binary assets (for images and audio files). The repository caches JSON files in memory for faster retrieval, and caches binary files in the app's cache directory.

This PR also updates TopicListController to prime the repository with audio and image files from the 'meaning of equal parts' lesson. This will download all binary assets corresponding to that exploration upon opening the app. This takes a few minutes depending on the device's connection strength and consumes about ~80mb of cache space.

By default, the binary asset caching is disabled using a Dagger flag.

This PR does not introduce any tests because this local caching behavior is temporary until #169 is completed.

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.
@BenHenning BenHenning changed the base branch from develop to calculate-realistic-download-size November 18, 2019 01:09
@BenHenning BenHenning changed the title Fix #386: Add support for local caching of audio & image assets Fix #386: Add support for local caching of audio & image assets [Blocked: #393] Nov 18, 2019
@BenHenning
Copy link
Member Author

This is blocked on #393.

Copy link
Contributor

@vinitamurthi vinitamurthi left a comment

Choose a reason for hiding this comment

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

I dont have much context on how it should work but code wise LGTM!

@vinitamurthi vinitamurthi removed their assignment Nov 19, 2019
Copy link
Contributor

@jamesxu0 jamesxu0 left a comment

Choose a reason for hiding this comment

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

Really good implementation! I need to do a deeper look at AssetRepository to understand all functionality but in general LGTM.

@jamesxu0 jamesxu0 assigned BenHenning and unassigned jamesxu0 Nov 19, 2019
Conflicts:
	domain/src/main/java/org/oppia/domain/topic/TopicController.kt
Conflicts:
	domain/src/main/java/org/oppia/domain/topic/TopicController.kt
@BenHenning BenHenning changed the title Fix #386: Add support for local caching of audio & image assets [Blocked: #393] Fix #386: Add support for local caching of audio & image assets Nov 20, 2019
…repo

Conflicts:
	utility/src/main/java/org/oppia/util/parser/UrlImageParser.kt
Conflicts:
	domain/src/main/java/org/oppia/domain/topic/TopicController.kt
@BenHenning BenHenning changed the base branch from calculate-realistic-download-size to develop November 20, 2019 00:47
@BenHenning
Copy link
Member Author

Thanks!

@BenHenning BenHenning merged commit b8100ad into develop Nov 20, 2019
@BenHenning BenHenning deleted the asset-download-repo branch November 20, 2019 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add temporary offline support for audio & image streaming
3 participants