[android] migrate OkHttp cache to application cache directory #11600
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ide
reviewed
Jan 12, 2021
android/expoview/src/main/java/host/exp/exponent/network/ExponentNetwork.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/network/ExponentNetwork.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/ReactNativeStaticHelpers.java
Show resolved
Hide resolved
esamelson
force-pushed
the
@eric/android-cache-control
branch
from
26c7232
to
1de7a58
Compare
ide
reviewed
Jan 13, 2021
| @@ -56,6 +56,7 @@ | |||
| public static final String SAFE_MANIFEST_KEY = "safe_manifest"; | |||
| public static final String EXPO_AUTH_SESSION = "expo_auth_session"; | |||
| public static final String EXPO_AUTH_SESSION_SECRET_KEY = "sessionSecret"; | |||
| public static final String HAS_CLEARED_LEGACY_OKHTTP_CACHE_KEY = "has_cleared_legacy_okhttp_cache"; | |||
There was a problem hiding this comment.
I'm a bit hesitant to put a key called has_cleared_legacy_okhttp_cache on devices with newly installed apps that aren't affected at all by this cache transition. A more forward-looking approach would be to have a key like okhttp_cache_version with the value 1 (no key = we need to try to clear the cache). Then if we were to make cache changes in the future, we could bump this key from 1 to 2.
ide
approved these changes
Jan 13, 2021
This was referenced Jan 15, 2021
tsapeta
pushed a commit
that referenced
this pull request
Jan 16, 2021
# Why Follow-up to #11599. Since OTA update assets are stored in a separate location, it's better to keep the OkHttp cache in the application cache directory, rather than the files directory -- this allows the OS to clean up files if it needs to, and reduces the chance that developers' application code will unexpectedly interfere with the OkHttp cache. # How - Moved OkHttp cache to `Context.getCacheDir`, and renamed it to match the name of bare RN OkHttp caches. - Added some deletion logic for the old cache in `Context.getFilesDir`. After deleting we set a key in SharedPreferences so we don't try to delete again. - Also increased the maximum cache size to 50Mb -- this is [recommended in the OkHttp docs](https://square.github.io/okhttp/caching/). I checked [`StorageManager.getCacheQuotaBytes`](https://developer.android.com/reference/android/os/storage/StorageManager#getCacheQuotaBytes(java.util.UUID)) for the cache directory of a fresh installation of the Expo Go client on my Pixel 2, and got 64Mb. Since the cache directory also includes a few other caches (image, picasso, home experience data) 50Mb seems like a reasonable maximum size for the OkHttp cache. - Finally, removed some extraneous logic around the OkHttpClient builder that was specifically for ExpoKit apps. # Test Plan Ran the same tests from #11599 again.
This was referenced Aug 29, 2021
This was referenced Dec 16, 2021
This was referenced Jan 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Follow-up to #11599. Since OTA update assets are stored in a separate location, it's better to keep the OkHttp cache in the application cache directory, rather than the files directory -- this allows the OS to clean up files if it needs to, and reduces the chance that developers' application code will unexpectedly interfere with the OkHttp cache.
How
Context.getCacheDir, and renamed it to match the name of bare RN OkHttp caches.Context.getFilesDir. After deleting we set a key in SharedPreferences so we don't try to delete again.StorageManager.getCacheQuotaBytesfor the cache directory of a fresh installation of the Expo Go client on my Pixel 2, and got 64Mb. Since the cache directory also includes a few other caches (image, picasso, home experience data) 50Mb seems like a reasonable maximum size for the OkHttp cache.Test Plan
Ran the same tests from #11599 again.