Skip to content

Conversation

@mullerpeter
Copy link

@RocketChat/ReactNative

Closes #441

Fixes the bug where the avatar is never refreshed. The default behaviour of FastImage only updates the image if the url changes. But since the avatar url stays the same after changing the image, it gets never refreshed.

FastImage.cacheControl.immutable - (Default) - Only updates if url changes.
FastImage.cacheControl.web - Use headers and follow normal caching procedures.
FastImage.cacheControl.cacheOnly - Only show images from cache, do not make any network requests.

@CLAassistant
Copy link

CLAassistant commented Jul 9, 2020

CLA assistant check
All committers have signed the CLA.

@djorkaeffalexandre
Copy link
Contributor

djorkaeffalexandre commented Jul 9, 2020

Hey @mullerpeter! Thanks for your pr!
We have made this PR (User avatar cache invalidation) on our server side to generate a better way to invalidate the avatar cache, since now we're doing something like use 1 hour of cache header.
Using the cache.web should change the image after 1 hour, but it's not convenient to the user, as you can see running this PR on iOS devices, it's not changing the avatar anytime, so the web cache doesn't work as expected for us.

If you test this change on Android devices, you'll be able to see that the avatar always blink before show.
I used the OKHttp profiler from Android Studio to see if this is always requesting the avatar again when it appear at the screen.
Screen Shot 2020-07-09 at 11 25 53
As you can see, it's repeating the request for each avatar instance, that's bad and can cause an overload at the server.
We're planning to implement the new user avatar cache invalidation logic to this release.
It's a little hard to do since now we'll need to save the ETag from each user displayed on some message/room; if you want to help with this feel free to comment here, we appreciate your help.
Thanks.

@diegolmello
Copy link
Member

So cache.web doesn't work on iOS and re-executes requests on every render on Android.

@mullerpeter
Copy link
Author

Ok I see the issue. I would be happy to help on this, do you already have a plan on how to implement it?
A few questions I have:

  1. I think ETags are not supported by FastImage. Would keep using FastImage with the default cache control and then trigger the reload of the cached image by changing the url (adding the ETag as a url param).
  2. I assume that it should ideally work similar to the web client, so that the RN client also receives the updateAvatar websocket events to trigger a rerender of changed avatars while the app is running, right?
  3. How would the ETags of the changed avatars be invalidated and fetched (i.e. when you reopen the app)? Periodically checking the validity of all saved ETags?

@djorkaeffalexandre
Copy link
Contributor

djorkaeffalexandre commented Jul 9, 2020

@mullerpeter we added the ETag as a property of the user object, so when we request a message for example, it'll return the user with the ETag joined, same for subscriptions.
We'll be notified by a stream when some user change their avatar, like when someone change their status/custom status.
With the ETag we can do something like you're saying, add this as param/hash of the url.

@djorkaeffalexandre
Copy link
Contributor

Refer to #2311.
Thanks for your help @mullerpeter.
🤗

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.

Avatar images doesn't refresh

4 participants