Skip to content

Commit 8509724

Browse files
committed
Thumbnails/Avatars: Differenciate loader by respectCacheHeaders setting
Also fix avatar for multiple accounts on same server.
1 parent 828c9fa commit 8509724

12 files changed

Lines changed: 98 additions & 39 deletions

File tree

opencloudApp/src/main/java/eu/opencloud/android/presentation/accounts/ManageAccountsAdapter.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ class ManageAccountsAdapter(
108108
try {
109109
val avatarUtils = AvatarUtils()
110110
holder.itemView.findViewTreeLifecycleOwner()?.lifecycleScope?.launch(Dispatchers.IO) {
111-
val loader = eu.opencloud.android.presentation.thumbnails.ThumbnailsRequester.getCoilImageLoader(account)
111+
val loader = eu.opencloud.android.presentation.thumbnails.ThumbnailsRequester.getRevalidatingImageLoader(account)
112112
withContext(Dispatchers.Main) {
113113
avatarUtils.loadAvatarForAccount(
114114
holder.binding.icon,

opencloudApp/src/main/java/eu/opencloud/android/presentation/avatar/AvatarUtils.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class AvatarUtils : KoinComponent {
5353
imageLoader: coil.ImageLoader? = null
5454
) {
5555
val uri = ThumbnailsRequester.getAvatarUri(account)
56-
val loader = imageLoader ?: ThumbnailsRequester.getCoilImageLoader(account)
56+
val loader = imageLoader ?: ThumbnailsRequester.getRevalidatingImageLoader(account)
5757
imageView.load(uri, loader) {
5858
placeholder(R.drawable.ic_account_circle)
5959
error(R.drawable.ic_account_circle)
@@ -67,7 +67,7 @@ class AvatarUtils : KoinComponent {
6767
@Suppress("UnusedParameter") displayRadius: Float
6868
) {
6969
val uri = ThumbnailsRequester.getAvatarUri(account)
70-
val imageLoader = ThumbnailsRequester.getCoilImageLoader(account)
70+
val imageLoader = ThumbnailsRequester.getRevalidatingImageLoader(account)
7171
val request = coil.request.ImageRequest.Builder(appContext)
7272
.data(uri)
7373
.target(

opencloudApp/src/main/java/eu/opencloud/android/presentation/files/details/FileDetailsFragment.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ class FileDetailsFragment : FileFragment() {
437437
1024,
438438
1024
439439
),
440-
ThumbnailsRequester.getCoilImageLoader(fileDetailsViewModel.getAccount())
440+
ThumbnailsRequester.getContentAddressedImageLoader(fileDetailsViewModel.getAccount())
441441
) {
442442
placeholder(MimetypeIconUtil.getFileTypeIconId(ocFile.mimeType, ocFile.fileName))
443443
error(MimetypeIconUtil.getFileTypeIconId(ocFile.mimeType, ocFile.fileName))

opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/FileListAdapter.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,8 @@ class FileListAdapter(
280280

281281
if (file.isImage) {
282282
account?.let { acc ->
283-
fileIcon.load(ThumbnailsRequester.getPreviewUriForFile(fileWithSyncInfo, acc), ThumbnailsRequester.getCoilImageLoader(acc)) {
283+
fileIcon.load(ThumbnailsRequester.getPreviewUriForFile(fileWithSyncInfo, acc),
284+
ThumbnailsRequester.getContentAddressedImageLoader(acc)) {
284285
placeholder(MimetypeIconUtil.getFileTypeIconId(file.mimeType, file.fileName))
285286
error(MimetypeIconUtil.getFileTypeIconId(file.mimeType, file.fileName))
286287
crossfade(true)

opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/MainFileListFragment.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,8 @@ class MainFileListFragment : Fragment(),
795795
val account = AccountUtils.getCurrentOpenCloudAccount(requireContext())
796796
binding.spaceHeader.spaceHeaderImage.load(
797797
ThumbnailsRequester.getPreviewUriForSpaceSpecial(spaceSpecialImage),
798-
if (account != null) ThumbnailsRequester.getCoilImageLoader(account) else ThumbnailsRequester.getCoilImageLoader()
798+
if (account != null) ThumbnailsRequester.getContentAddressedImageLoader(account)
799+
else ThumbnailsRequester.getContentAddressedImageLoader()
799800
) {
800801
placeholder(R.drawable.ic_spaces)
801802
error(R.drawable.ic_spaces)

opencloudApp/src/main/java/eu/opencloud/android/presentation/files/removefile/RemoveFilesDialogFragment.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ class RemoveFilesDialogFragment : DialogFragment() {
124124
val file = files[0]
125125
// Show the thumbnail when the file has one
126126
val account = AccountUtils.getCurrentOpenCloudAccount(requireContext())
127-
thumbnailImageView.load(ThumbnailsRequester.getPreviewUriForFile(file, account), ThumbnailsRequester.getCoilImageLoader(account)) {
127+
thumbnailImageView.load(ThumbnailsRequester.getPreviewUriForFile(file, account),
128+
ThumbnailsRequester.getContentAddressedImageLoader(account)) {
128129
placeholder(MimetypeIconUtil.getFileTypeIconId(file.mimeType, file.fileName))
129130
error(MimetypeIconUtil.getFileTypeIconId(file.mimeType, file.fileName))
130131
crossfade(true)

opencloudApp/src/main/java/eu/opencloud/android/presentation/sharing/ShareFileFragment.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ class ShareFileFragment : Fragment(), ShareUserListAdapter.ShareUserAdapterListe
242242
if (file!!.isImage) {
243243
binding.shareFileIcon.load(
244244
ThumbnailsRequester.getPreviewUriForFile(file!!, account!!),
245-
ThumbnailsRequester.getCoilImageLoader(account!!)
245+
ThumbnailsRequester.getContentAddressedImageLoader(account!!)
246246
) {
247247
placeholder(MimetypeIconUtil.getFileTypeIconId(file!!.mimeType, file!!.fileName))
248248
error(MimetypeIconUtil.getFileTypeIconId(file!!.mimeType, file!!.fileName))

opencloudApp/src/main/java/eu/opencloud/android/presentation/spaces/SpacesListAdapter.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ class SpacesListAdapter(
7676
val account = eu.opencloud.android.presentation.authentication.AccountUtils.getCurrentOpenCloudAccount(holder.itemView.context)
7777
spacesListItemImage.load(
7878
ThumbnailsRequester.getPreviewUriForSpaceSpecial(spaceSpecialImage),
79-
if (account != null) ThumbnailsRequester.getCoilImageLoader(account) else ThumbnailsRequester.getCoilImageLoader()
79+
if (account != null) ThumbnailsRequester.getContentAddressedImageLoader(account)
80+
else ThumbnailsRequester.getContentAddressedImageLoader()
8081
) {
8182
placeholder(R.drawable.ic_spaces_placeholder)
8283
error(R.drawable.ic_spaces_placeholder)

opencloudApp/src/main/java/eu/opencloud/android/presentation/thumbnails/ThumbnailsRequester.kt

Lines changed: 81 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import eu.opencloud.android.lib.common.http.HttpConstants.OC_X_REQUEST_ID
4141
import eu.opencloud.android.lib.common.http.HttpConstants.USER_AGENT_HEADER
4242
import eu.opencloud.android.lib.common.utils.RandomUtils
4343
import eu.opencloud.android.presentation.authentication.AccountUtils
44+
import okhttp3.Cache
4445
import okhttp3.Headers.Companion.toHeaders
4546
import okhttp3.Interceptor
4647
import okhttp3.Response
@@ -52,16 +53,20 @@ import java.util.Locale
5253
object ThumbnailsRequester : KoinComponent {
5354
private val clientManager: ClientManager by inject()
5455

56+
// https://docs.opencloud.eu/docs/next/dev/server/services/thumbnails/information/#thumbnail-query-string-parameters
5557
private const val SPACE_SPECIAL_PREVIEW_URI = "%s?scalingup=0&a=1&x=%d&y=%d&c=%s&preview=1"
5658
private const val FILE_PREVIEW_URI = "%s/webdav%s?x=%d&y=%d&c=%s&preview=1"
5759

58-
private const val DISK_CACHE_SIZE: Long = 1024 * 1024 * 100 // 100MB
60+
private const val THUMBNAIL_DISK_CACHE_SIZE: Long = 1024 * 1024 * 100 // 100MB
61+
private const val AVATAR_HTTP_CACHE_SIZE: Long = 10L * 1024 * 1024 // 10MB
62+
63+
private val thumbnailImageLoaders = ConcurrentHashMap<String, ImageLoader>()
64+
private val avatarImageLoaders = ConcurrentHashMap<String, ImageLoader>()
5965

60-
private val imageLoaders = ConcurrentHashMap<String, ImageLoader>()
6166
private val sharedDiskCache: DiskCache by lazy {
6267
DiskCache.Builder()
6368
.directory(appContext.cacheDir.resolve("thumbnails_coil_cache"))
64-
.maxSizeBytes(DISK_CACHE_SIZE)
69+
.maxSizeBytes(THUMBNAIL_DISK_CACHE_SIZE)
6570
.build()
6671
}
6772

@@ -71,13 +76,22 @@ object ThumbnailsRequester : KoinComponent {
7176
.build()
7277
}
7378

79+
// OkHttp's built-in HTTP cache for avatar responses. Unlike Coil's disk cache,
80+
// OkHttp's cache natively serves stale responses when the network is unavailable
81+
// (when must-revalidate is not set), giving us offline fallback for avatars.
82+
private val avatarHttpCache: Cache by lazy {
83+
Cache(appContext.cacheDir.resolve("avatar_http_cache"), AVATAR_HTTP_CACHE_SIZE)
84+
}
85+
7486
fun getAvatarUri(account: Account): String {
7587
val accountManager = AccountManager.get(appContext)
7688
val baseUrl =
7789
accountManager.getUserData(account, eu.opencloud.android.lib.common.accounts.AccountUtils.Constants.KEY_OC_BASE_URL)
7890
?.trimEnd('/')
7991
.orEmpty()
80-
return "$baseUrl/graph/v1.0/me/photo/\$value"
92+
// ?u= disambiguates the Coil cache key per account; without it two accounts
93+
// on the same server share the same URL and collide in the shared disk/memory cache.
94+
return "$baseUrl/graph/v1.0/me/photo/\$value?u=${account.name.hashCode().toString(16)}"
8195
}
8296

8397
fun getPreviewUriForFile(file: OCFile, account: Account, etag: String? = null, width: Int = 1024, height: Int = 1024): String =
@@ -100,33 +114,69 @@ object ThumbnailsRequester : KoinComponent {
100114
return String.format(Locale.US, FILE_PREVIEW_URI, baseUrl, encodedPath, width, height, etag.orEmpty())
101115
}
102116

103-
fun getCoilImageLoader(): ImageLoader {
117+
fun getContentAddressedImageLoader(): ImageLoader {
104118
val account = AccountUtils.getCurrentOpenCloudAccount(appContext)
105-
return getCoilImageLoader(account)
119+
return getContentAddressedImageLoader(account)
106120
}
107121

108-
fun getCoilImageLoader(account: Account): ImageLoader {
109-
val accountName = account.name
110-
return imageLoaders.getOrPut(accountName) {
111-
val openCloudClient = clientManager.getClientForCoilThumbnails(accountName)
122+
/**
123+
* Thumbnail URLs are content-addressed: the file etag is baked into the URL, so the
124+
* URL changes when the file changes. The disk cache entry is therefore always valid
125+
* and can be served offline without server revalidation.
126+
*
127+
* Uses Coil's disk cache with respectCacheHeaders(false).
128+
*/
129+
fun getContentAddressedImageLoader(account: Account): ImageLoader =
130+
thumbnailImageLoaders.getOrPut(account.name) {
131+
buildThumbnailImageLoader(account)
132+
}
133+
134+
/**
135+
* Avatar URLs are NOT content-addressed: the URL (/graph/v1.0/me/photo/$value) is
136+
* fixed regardless of whether the user changes their profile picture. We need server
137+
* revalidation so the app picks up avatar changes, but we also need offline fallback.
138+
*
139+
* Coil's disk cache cannot serve stale entries on network error, so we use OkHttp's
140+
* built-in HTTP cache instead (which does). Coil still handles decoding, memory cache,
141+
* and transformations (CircleCrop etc.).
142+
*/
143+
fun getRevalidatingImageLoader(account: Account): ImageLoader =
144+
avatarImageLoaders.getOrPut(account.name) {
145+
buildAvatarImageLoader(account)
146+
}
112147

113-
val coilRequestHeaderInterceptor = CoilRequestHeaderInterceptor(
114-
clientManager = clientManager,
115-
accountName = accountName
148+
private fun buildThumbnailImageLoader(account: Account): ImageLoader {
149+
val openCloudClient = clientManager.getClientForCoilThumbnails(account.name)
150+
val interceptor = CoilRequestHeaderInterceptor(clientManager, account.name)
151+
return ImageLoader(appContext).newBuilder()
152+
.okHttpClient(
153+
openCloudClient.okHttpClient.newBuilder()
154+
.addInterceptor(interceptor).build()
116155
)
156+
.logger(DebugLogger())
157+
.memoryCache { sharedMemoryCache }
158+
.diskCache { sharedDiskCache }
159+
.respectCacheHeaders(false)
160+
.build()
161+
}
117162

118-
ImageLoader(appContext).newBuilder().okHttpClient(
119-
okHttpClient = openCloudClient.okHttpClient.newBuilder()
120-
.addInterceptor(coilRequestHeaderInterceptor).build()
121-
).logger(DebugLogger())
122-
.memoryCache {
123-
sharedMemoryCache
124-
}
125-
.diskCache {
126-
sharedDiskCache
127-
}
128-
.build()
129-
}
163+
private fun buildAvatarImageLoader(account: Account): ImageLoader {
164+
val openCloudClient = clientManager.getClientForCoilThumbnails(account.name)
165+
val interceptor = CoilRequestHeaderInterceptor(clientManager, account.name)
166+
return ImageLoader(appContext).newBuilder()
167+
.okHttpClient(
168+
openCloudClient.okHttpClient.newBuilder()
169+
.addInterceptor(interceptor)
170+
.cache(avatarHttpCache)
171+
.build()
172+
)
173+
.logger(DebugLogger())
174+
.memoryCache { sharedMemoryCache }
175+
// No Coil disk cache — OkHttp's HTTP cache handles persistence
176+
// and offline fallback instead.
177+
.diskCache(null)
178+
.respectCacheHeaders(true)
179+
.build()
130180
}
131181

132182
private class CoilRequestHeaderInterceptor(
@@ -152,10 +202,15 @@ object ThumbnailsRequester : KoinComponent {
152202
var builder = response.newBuilder()
153203
var changed = false
154204

205+
// The server sends no-cache (or no Cache-Control) for avatar responses.
206+
// Rewrite to max-age=300 so OkHttp's HTTP cache caches them.
207+
// Deliberately omitting must-revalidate: without it OkHttp serves stale
208+
// responses when the network is unavailable, giving us offline fallback.
209+
// For thumbnails this rewrite is ignored (respectCacheHeaders=false).
155210
val cacheControl = response.header("Cache-Control")
156211
if (cacheControl.isNullOrEmpty() || cacheControl.contains("no-cache")) {
157212
builder.removeHeader("Cache-Control")
158-
builder.addHeader("Cache-Control", "max-age=5000, must-revalidate")
213+
builder.addHeader("Cache-Control", "max-age=300")
159214
changed = true
160215
}
161216

opencloudApp/src/main/java/eu/opencloud/android/ui/activity/DrawerActivity.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ abstract class DrawerActivity : ToolbarActivity() {
458458

459459
getDrawerCurrentAccount()?.let {
460460
lifecycleScope.launch(Dispatchers.IO) {
461-
val imageLoader = ThumbnailsRequester.getCoilImageLoader(account)
461+
val imageLoader = ThumbnailsRequester.getRevalidatingImageLoader(account)
462462
withContext(Dispatchers.Main) {
463463
AvatarUtils().loadAvatarForAccount(
464464
imageView = it,

0 commit comments

Comments
 (0)