fix(thumbail): generation - #17448
Conversation
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
0300da7 to
b3cfba9
Compare
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
There was a problem hiding this comment.
Pull request overview
This PR centralizes thumbnail generation across the legacy com.owncloud.android.* UI surfaces by introducing a DI-managed ThumbnailGenerator (file + folder) and migrating callers away from the large DisplayUtils.setThumbnail(...) logic and ad-hoc cache-key handling. It also standardizes cache key access via OCFile/extension helpers and persists server preview availability after upload.
Changes:
- Introduces
ThumbnailGenerator/FileThumbnailGenerator/FolderThumbnailGeneratorand migrates multiple adapters/fragments/activities to use it. - Unifies thumbnail cache key usage (small/big) via
OCFilehelpers andFileExtensionsfunctions; updates call sites and tests accordingly. - Adds preview-availability checks to avoid pointless remote preview downloads and persists
previewAvailableon upload.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/test/java/com/owncloud/android/ui/adapter/GalleryAdapterTest.kt | Updates unit test construction to pass the new ThumbnailGenerator dependency. |
| app/src/main/java/com/owncloud/android/utils/DisplayUtils.java | Removes the large thumbnail-setting implementation now replaced by the new generator classes. |
| app/src/main/java/com/owncloud/android/ui/preview/PreviewImageFragment.kt | Switches preview thumbnail and resized-image cache key usage to OCFile helpers. |
| app/src/main/java/com/owncloud/android/ui/fragment/UnifiedSearchFragment.kt | Injects ThumbnailGenerator and passes it into the unified search adapter. |
| app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java | Injects ThumbnailGenerator and wires it into file list adapter construction. |
| app/src/main/java/com/owncloud/android/ui/fragment/GalleryFragment.kt | Passes ThumbnailGenerator into GalleryAdapter. |
| app/src/main/java/com/owncloud/android/ui/fragment/FileDetailFragment.java | Uses OCFile thumbnail helpers for preview/thumbnail display. |
| app/src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.kt | Uses ThumbnailGenerator instead of DisplayUtils.setThumbnail(...). |
| app/src/main/java/com/owncloud/android/ui/adapter/uploadList/UploadListAdapter.kt | Replaces custom thumbnail generation with ThumbnailGenerator, adds async OCFile lookup. |
| app/src/main/java/com/owncloud/android/ui/adapter/UnifiedSearchListAdapter.kt | Replaces OverlayManager with ThumbnailGenerator in adapter dependencies. |
| app/src/main/java/com/owncloud/android/ui/adapter/UnifiedSearchItemViewHolder.kt | Uses ThumbnailGenerator for folder overlays and file thumbnails. |
| app/src/main/java/com/owncloud/android/ui/adapter/UnifiedSearchCurrentDirItemViewHolder.kt | Uses ThumbnailGenerator for current-directory item thumbnails. |
| app/src/main/java/com/owncloud/android/ui/adapter/TrashbinListAdapter.java | Switches trashbin thumbnail lookups to getSmallThumbnail() helper. |
| app/src/main/java/com/owncloud/android/ui/adapter/OCFileListDelegate.kt | Removes DisplayUtils thumbnail path and delegates thumbnailing to ThumbnailGenerator. |
| app/src/main/java/com/owncloud/android/ui/adapter/OCFileListAdapter.java | Plumbs ThumbnailGenerator through and routes cancellation to generator task cancellation. |
| app/src/main/java/com/owncloud/android/ui/adapter/LocalFileListAdapter.java | Uses FileExtensionsKt.getSmallThumbnail(...) for local thumbnail reads. |
| app/src/main/java/com/owncloud/android/ui/adapter/GalleryAdapter.kt | Uses ThumbnailGenerator for task cancellation and list thumbnail generation flow. |
| app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.kt | Injects and passes ThumbnailGenerator into UploadListAdapter. |
| app/src/main/java/com/owncloud/android/ui/activity/ShareActivity.kt | Uses ThumbnailGenerator for file icon/thumbnail display. |
| app/src/main/java/com/owncloud/android/ui/activity/EditorWebView.java | Uses OCFile.getSmallThumbnail() for cached thumbnail reads. |
| app/src/main/java/com/owncloud/android/providers/DocumentsStorageProvider.java | Uses getSmallThumbnailKey() for thumbnail existence checks. |
| app/src/main/java/com/owncloud/android/providers/DiskLruImageCacheFileProvider.java | Uses small/big thumbnail helpers for disk-cache lookup. |
| app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java | Persists previewAvailable from server metadata on upload completion. |
| app/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java | Switches to unified key helpers; skips remote download when server has no preview. |
| app/src/main/java/com/owncloud/android/datamodel/OCFile.java | Adds Java-level convenience methods for thumbnail keys and bitmap retrieval. |
| app/src/main/java/com/nextcloud/utils/thumbnail/ThumbnailGenerator.kt | New: single entry point choosing file vs folder vs offline thumbnail generation. |
| app/src/main/java/com/nextcloud/utils/thumbnail/FolderThumbnailGenerator.kt | Moves/renames overlay manager responsibilities into folder thumbnail generation. |
| app/src/main/java/com/nextcloud/utils/thumbnail/FileThumbnailGenerator.kt | New: centralized file thumbnail generation, task management, shimmer handling. |
| app/src/main/java/com/nextcloud/utils/ShortcutUtil.kt | Uses OCFile.smallThumbnail helper for shortcut icon generation. |
| app/src/main/java/com/nextcloud/utils/extensions/ImageViewExtensions.kt | Adds startShimmer/stopShimmer extensions used by new thumbnail code. |
| app/src/main/java/com/nextcloud/utils/extensions/FileExtensions.kt | Adds thumbnail key/bitmap helpers for ServerFileInterface and File. |
| app/src/main/java/com/nextcloud/ui/trashbinFileActions/TrashbinFileActionsBottomSheet.kt | Uses ThumbnailGenerator for bottom-sheet thumbnail display. |
| app/src/main/java/com/nextcloud/ui/fileInfo/ImageDetailInfo.kt | Uses OCFile.smallThumbnail for pin/avatar-style thumbnail composition. |
| app/src/main/java/com/nextcloud/ui/fileactions/FileActionsBottomSheet.kt | Uses ThumbnailGenerator for bottom-sheet thumbnail display. |
| app/src/main/java/com/nextcloud/client/jobs/gallery/GalleryImageGenerationJob.kt | Switches resized-image caching to bigThumbnailKey/getBigThumbnail(). |
| app/src/main/java/com/nextcloud/client/di/AppModule.java | Provides FolderThumbnailGenerator instead of the old OverlayManager. |
| app/src/androidTest/java/com/owncloud/android/ui/fragment/GalleryFragmentIT.kt | Updates instrumented test to use bigThumbnailKey and bigThumbnail helper. |
| app/src/androidTest/java/com/owncloud/android/ui/adapter/UnifiedSearchListAdapterIT.kt | Updates IT to construct and pass ThumbnailGenerator instead of OverlayManager. |
Suppressed comments (2)
app/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java:732
- In
doFileInBackground(), the disk-cache key is now prefixed viaFileExtensionsKt.getSmallThumbnailKey(file), but the code still reads/writes using the unprefixedimageKey(hashCode string). This breaks local thumbnail cache lookups via the newFileExtensionsKt.getSmallThumbnail()API (it looks up the prefixed key).
final String imageKey = Objects.requireNonNullElseGet(mImageKey, () -> String.valueOf(file.hashCode()));
// local file should always generate a thumbnail
mImageKey = FileExtensionsKt.getSmallThumbnailKey(file);
// Check disk cache in background thread
Bitmap thumbnail = getBitmapFromDiskCache(imageKey);
app/src/main/java/com/nextcloud/utils/thumbnail/FolderThumbnailGenerator.kt:6
- SPDX header doesn’t match the repository’s documented template (see AGENTS.md:43-61). New files should use “Nextcloud GmbH and Nextcloud contributors” instead of a personal attribution line.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: alperozturk96 <alper_ozturk@proton.me>
|
APK file: https://github.com/nextcloud/android/actions/runs/31163294377/artifacts/8988442890 |
|
APK file: https://github.com/nextcloud/android/actions/runs/31164847648/artifacts/8988692318 |
|
blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed. |
|
master-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/17448-IT-master-10-06 |
Issue
Thumbnail for video files were not visible on Upload and Media tab. Code was duplicated for every class and was error prone.
Changes
BitmapDisplayUtils.setThumbnailcalls with a lot of parameters. For example:Note
Reasons to unify thumbnail logic:
DisplayUtils.setThumbnailwas already covering all cases for folders and files, but it required many parameters. For each missing class, we had to inject and pass those parameters through multiple classes just to call thesetThumbnailfunction. With this approach, only one injection is needed to access that logic, for example, in the Upload screen.Reasons to unify keys:
Currently, keys are generated with long static class calls from
ThumbnailCacheManager, and later the bitmap is checked and retrieved again from the sameThumbnailCacheManager. This repeated pattern exists across many classes. Instead, we can unify this logic and simply call methods likegetSmallBitmaporgetBigBitmap.Additionally, a single incorrect usage of a key prefix can cause the cached bitmap to be missed. For example, using
remotePathorhashCodeinstead ofremoteIdfor anOCFilecached bitmap will result in a cache miss. Therefore, we should avoid hardcoding these values and copying them across the application.For local file hashCode needs to be used this change also will prevent this.
Screenshot