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

feat(image): [android] adding force-cache cache control option #47426

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mateoguzmana
Copy link
Contributor

Summary:

This PR follows up on #47182 and #47348 by adding force-cache, the final missing option to align caching controls with the existing behavior on iOS.

Local caching behavior remains unchanged: if a cached image is available locally, it will be returned; otherwise, a network request will be made.

When an image request is sent over the network, the force-cache option sent from the sent fJS side will now use the okhttp3.CacheControl.FORCE_CACHE directive.

Changelog:

[ANDROID] [ADDED] - Image force-cache caching control option

Test Plan:

New example added to the RNTester under the cache policy examples. Then inspecting that the cache control is set correctly before sending it in the okhttp3.Request builder.

FLog.w("ReactNative", "fetching uri: %s, with cacheControl: %s", uri, cacheControlBuilder.build().toString())
// fetching uri: https:...png?cacheBust=force-cache, with cacheControl: no-store, max-stale=2147483647, only-if-cached

This case was a bit more tricky to test in terms of e2e as it would involve some caching in the server as well, I'm open to suggestions to make this more complete.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 5, 2024
Comment on lines +47 to +51
ImageCacheControl.FORCE_CACHE -> {
cacheControlBuilder
.onlyIfCached()
.maxStale(Integer.MAX_VALUE, TimeUnit.SECONDS)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mateoguzmana mateoguzmana marked this pull request as ready for review November 5, 2024 16:57
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Nov 5, 2024
@facebook-github-bot
Copy link
Contributor

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants