Feature/clean thumbnails avatars

[zerox80]

Thumbnail Cache Refactoring and Avatar Fixes

Summary

This pull request refactors the thumbnail caching mechanism to improve performance and stability, and includes critical fixes for user avatar loading. The legacy ThumbnailsCacheManager has been replaced by an optimized ThumbnailsRequester implementation, and several thread safety issues have been resolved.

Technical Details

Thumbnail Caching

• Refactoring: Completely removed ThumbnailsCacheManager and DiskLruImageCache in favor of a modern, streamlined approach within ThumbnailsRequester.
• Thread Safety: Addressed race conditions in ThumbnailsRequester when initializing the shared disk and memory caches. Access to these caches is now properly synchronized.
• Optimization: Adjusted thumbnail sizing logic to prevent memory waste and improve loading times.
• Dead Code Removal: Cleaned up unused legacy code and resources related to the old caching system.

Avatar Loading

• Graph API Integration: Updated the user avatar loading logic to utilize the Graph API endpoint, ensuring reliable retrieval of user profile images.
• Cache Control: Fixed issues with avatar cache invalidation and loading control to ensure users always see the correct profile picture.
• Bug Fixes: Resolved specific crashes and loading failures related to avatar requests in various parts of the UI (Drawer, Toolbar, Upload List).

Other Changes

• Code Quality: Resolved extensive Detekt issues, including formatting and spacing violations.
• Stability: Fixed potential crashes caused by incorrect context usage or missing null checks in the thumbnail generation flow.

Impact

These changes significantly reduce the maintenance burden by removing complex legacy caching code and provide a smoother user experience with faster and more reliable thumbnail and avatar loading.

Testing

• Verified that file thumbnails load correctly in the file list and grid view.
• Verified that user avatars appear correctly in the navigation drawer and toolbar.
• Confirmed that removing the legacy cache classes does not introduce regressions in image loading.

[guruz]

I don't think this looks good, OpenCloud is not PHP anymore.

[guruz]

Thumbnail Caching

I haven't really looked at the code yet. Where is stuff cached? From the names it seems it was Disk(Lru) before, what is it now?

[guruz]

FYI I'm looking / testing this PR now. @zerox80

[zerox80]

Hi, ill take a look now, was busy sorry

[zerox80]

re-test please @guruz

[zerox80]

Thumbnail Caching

I haven't really looked at the code yet. Where is stuff cached? From the names it seems it was Disk(Lru) before, what is it now?

We completely moved away from the old manual DiskLruCache handling and now rely on Coil's built-in caching mechanism, which is much more efficient and modern.

It uses a two-tier caching strategy (both Memory and Disk cache), which is configured centrally in the ThumbnailsRequester.kt

Disk Cache: We use Coil's DiskCache.Builder() to store thumbnails in the app's cache directory (thumbnails_coil_cache). It is explicitly limited to 100MB to ensure it doesn't inflate the app storage endlessly. (Location: appContext.cacheDir.resolve("thumbnails_coil_cache"))

Memory Cache: We use Coil's MemoryCache.Builder() configured to use up to 25% of the available application memory (maxSizePercent(0.25)). This drastically improves scrolling performance in lists/grids because decoded images are kept directly in memory.

Both caches are shared as Singletons (sharedDiskCache and sharedMemoryCache) and passed to the ImageLoader instance, so the caching rules apply globally to all thumbnail requests handled by Coil

[guruz]

(Don't do any changes now on top as I'm working on your branch myself now)

I actually think your code is regressing the cache by not going to the cache first if we had a preview already in (disk) cache.
So it makes the behavior of the app worse by not showing images when being offline or unauthorized.

I'll check if there is a goo way to fix this..

[guruz]

Or maybe I take this back.
It seems this happened when the server sent 401 because token expired
When there was a network error (UnknownHost or whatever) it actually went succesfully to the cache

02-26 12:31:12.678 14205 14205 I RealImageLoader: 💾 Successful (DISK) - https://demo.opencloud.eu/webdav/OpenCloud-UI-Tab_Work_Web_2400px.jpg?x=1024&y=1024&c=&preview=1

Can ignore this for now then :)

[guruz]

Can ignore this for now then :)

Or can not ignore this. Because 401 is actually quite likely if access_token has low expiry time.

But maybe the actual fix is not related to here, but re-ordering how ConnectionValidator & friends behave or how token expiry is detected.

Just notes to myself..

[zerox80]

Hey @guruz, just to share some thoughts on why the cache behaves differently between UnknownHostException and 401 Unauthorized:

The difference lies in how OkHttp/Coil handles HTTP responses versus network exceptions when checking the disk cache. When Coil tries to load an image, it might need to validate a stale cache entry.

If the device is offline, OkHttp throws an IOException (like UnknownHostException). Coil/OkHttp Cache catches this and correctly falls back to returning the stale disk cache entry.
However, if the token is expired, the server responds with an HTTP 401. To OkHttp/Coil, this is a successful network transaction (no IOException is thrown). Since the server explicitly rejected the request, OkHttp doesn't fall back to the stale cache; it just returns the 401 response to Coil, which then fails the image load.
Possible solutions to consider:

Refresh on 401: Add an OkHttp Authenticator (or Interceptor) to the client used by Coil. When it catches a 401, it can synchronously refresh the token via AccountManager and retry the image request.
Mask 401 as Offline: If token refresh fails or isn't possible right away, an OkHttp Interceptor could catch the 401 and explicitly throw an IOException("Unauthorized - Token Expired"). By throwing an exception instead of returning the 401 response, it would trick Coil into treating it exactly like an offline scenario, and it will serve the stale image from the disk cache.

[zerox80]

Hey, so there was a bug that TUS Upload doesn't get an Etag? Somehow?

its fixed now, please test

[guruz]

@zerox80 Thanks for spotting/fixing! I'll try it out..

@TheOneRing CC
Seems the desktop client also has this flow where it can receive an ETag or not and then do PROPFIND. https://github.com/opencloud-eu/desktop/blob/c7c05eaadf5d9cb53c95f9c6872703a9d70ba3fb/src/libsync/propagateuploadtus.cpp#L260
Wondering why we don't always get one, is that storage specific?
old upstream discussion see here: owncloud/ocis#216 (comment)

[zerox80]

@zerox80 Thanks for spotting/fixing! I'll try it out..

@TheOneRing CC Seems the desktop client also has this flow where it can receive an ETag or not and then do PROPFIND. https://github.com/opencloud-eu/desktop/blob/c7c05eaadf5d9cb53c95f9c6872703a9d70ba3fb/src/libsync/propagateuploadtus.cpp#L260 Wondering why we don't always get one, is that storage specific? old upstream discussion see here: owncloud/ocis#216 (comment)

The Tusd library has a hook called PreFinishResponseCallback, with that REVA could calculate the etag after the last chunk and write it to the response header, but this hook is never used....

Is it atleast planned? Should we open Issue on Opencloud backend repo?