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

Thumbnail support #533

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
9ab203c
Thumbail Generation study - inconsistent changes
JustFanta01 Apr 11, 2024
e6f673c
Merge branch 'cryptomator:develop' into feature/thumbnail-main
JustFanta01 Apr 11, 2024
9fa1f83
Added thumbnail File in CryptoFile and logic for storing and retrievi…
JustFanta01 Apr 13, 2024
f158359
Remove thumbnail from cache, add check if thumbnail exists only for i…
taglioIsCoding Apr 13, 2024
a5da6b1
Generate thumbnails only for images
taglioIsCoding Apr 14, 2024
4bbd29c
Removed unused imports
taglioIsCoding Apr 19, 2024
46fcb27
Add local LRU cache and first refactor
taglioIsCoding Apr 21, 2024
7c95cad
Removed unused imports + fix typo
JustFanta01 Apr 21, 2024
6f5d2da
changed names for retrieving cloud-related DiskLruCache
JustFanta01 Apr 28, 2024
91c1537
modified the cachekey for DiskLruCache
JustFanta01 Apr 28, 2024
54499a0
Added also in FormatPre7 the fetch of the thumbnail (not tested yet)
JustFanta01 Apr 28, 2024
bba7877
Add enum Thumbnail option
taglioIsCoding Apr 29, 2024
9e25e04
Merge branch 'feature/thumbnail-playground' of https://github.com/Jus…
JustFanta01 Apr 29, 2024
f6ded14
minor changes
JustFanta01 Apr 29, 2024
4fa8670
Change cachekey for thumbnails
JustFanta01 May 7, 2024
e67edf8
Use onEach and added the delete operation in the FormatPre7
JustFanta01 May 7, 2024
23f4f83
Added a separate thread to acquire the bitmap of the image and genera…
JustFanta01 Apr 28, 2024
fcdc306
Add thumbanil generator thread pool executor and subsample image stream
JustFanta01 May 1, 2024
8a2b3d6
Cleanup
taglioIsCoding May 8, 2024
ea217f9
Merge branch 'cryptomator:develop' into feature/thumbnail-playground
taglioIsCoding May 8, 2024
a487f6d
Manage rename and move files in cache
taglioIsCoding May 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.cryptomator.data.cloud.crypto

import org.cryptomator.domain.Cloud
import org.cryptomator.domain.CloudFile
import java.io.File
import java.util.Date

class CryptoFile(
Expand All @@ -12,6 +13,8 @@ class CryptoFile(
val cloudFile: CloudFile
) : CloudFile, CryptoNode {

var thumbnail : File? = null

override val cloud: Cloud?
get() = parent.cloud

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
package org.cryptomator.data.cloud.crypto

import android.content.Context
import android.graphics.Bitmap
import android.graphics.BitmapFactory
import android.media.ThumbnailUtils
import com.google.common.util.concurrent.ThreadFactoryBuilder
import com.tomclaw.cache.DiskLruCache
import org.cryptomator.cryptolib.api.Cryptor
import org.cryptomator.cryptolib.common.DecryptingReadableByteChannel
import org.cryptomator.cryptolib.common.EncryptingWritableByteChannel
Expand All @@ -9,6 +14,7 @@ import org.cryptomator.domain.Cloud
import org.cryptomator.domain.CloudFile
import org.cryptomator.domain.CloudFolder
import org.cryptomator.domain.CloudNode
import org.cryptomator.domain.CloudType
import org.cryptomator.domain.exception.BackendException
import org.cryptomator.domain.exception.CloudNodeAlreadyExistsException
import org.cryptomator.domain.exception.EmptyDirFileException
Expand All @@ -24,18 +30,31 @@ import org.cryptomator.domain.usecases.cloud.DownloadState
import org.cryptomator.domain.usecases.cloud.FileBasedDataSource.Companion.from
import org.cryptomator.domain.usecases.cloud.Progress
import org.cryptomator.domain.usecases.cloud.UploadState
import org.cryptomator.util.SharedPreferencesHandler
import org.cryptomator.util.ThumbnailsOption
import org.cryptomator.util.file.LruFileCacheUtil
import org.cryptomator.util.file.MimeType
import org.cryptomator.util.file.MimeTypeMap
import org.cryptomator.util.file.MimeTypes
import java.io.ByteArrayOutputStream
import java.io.Closeable
import java.io.File
import java.io.FileInputStream
import java.io.FileOutputStream
import java.io.IOException
import java.io.OutputStream
import java.io.PipedInputStream
import java.io.PipedOutputStream
import java.nio.ByteBuffer
import java.nio.channels.Channels
import java.util.LinkedList
import java.util.Queue
import java.util.UUID
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
import java.util.concurrent.Future
import java.util.function.Supplier
import timber.log.Timber


abstract class CryptoImplDecorator(
Expand All @@ -50,6 +69,58 @@ abstract class CryptoImplDecorator(
@Volatile
private var root: RootCryptoFolder? = null

private val sharedPreferencesHandler = SharedPreferencesHandler(context)

private var diskLruCache: MutableMap<LruFileCacheUtil.Cache, DiskLruCache?> = mutableMapOf()

private val mimeTypes = MimeTypes(MimeTypeMap())

private val thumbnailExecutorService: ExecutorService by lazy {
val threadFactory = ThreadFactoryBuilder().setNameFormat("thumbnail-generation-thread-%d").build()
Executors.newFixedThreadPool(3, threadFactory)
}

protected fun getLruCacheFor(type: CloudType): DiskLruCache? {
return getOrCreateLruCache(getCacheTypeFromCloudType(type), sharedPreferencesHandler.lruCacheSize())
}

private fun getOrCreateLruCache(key: LruFileCacheUtil.Cache, cacheSize: Int): DiskLruCache? {
return diskLruCache.computeIfAbsent(key) {
Comment on lines +87 to +88
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private fun getOrCreateLruCache(key: LruFileCacheUtil.Cache, cacheSize: Int): DiskLruCache? {
return diskLruCache.computeIfAbsent(key) {
private fun getOrCreateLruCache(cache: LruFileCacheUtil.Cache, cacheSize: Int): DiskLruCache? {
return diskLruCache.computeIfAbsent(cache) {

val where = LruFileCacheUtil(context).resolve(it)
try {
DiskLruCache.create(where, cacheSize.toLong())
} catch (e: IOException) {
Timber.tag("CryptoImplDecorator").e(e, "Failed to setup LRU cache for $where.name")
Comment on lines +89 to +93
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val where = LruFileCacheUtil(context).resolve(it)
try {
DiskLruCache.create(where, cacheSize.toLong())
} catch (e: IOException) {
Timber.tag("CryptoImplDecorator").e(e, "Failed to setup LRU cache for $where.name")
val cacheFile = LruFileCacheUtil(context).resolve(it)
try {
DiskLruCache.create(cacheFile, cacheSize.toLong())
} catch (e: IOException) {
Timber.tag("CryptoImplDecorator").e(e, "Failed to setup LRU cache for $cacheFile.name")

null
}
}
}
protected fun renameFileInCache(source: CryptoFile, target: CryptoFile){
Copy link
Member

Choose a reason for hiding this comment

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

Add a newline before this method and a blank before the {

val oldCacheKey = generateCacheKey(source.cloudFile)
val newCacheKey = generateCacheKey(target.cloudFile)
source.cloudFile.cloud?.type()?.let { cloudType ->
getLruCacheFor(cloudType)?.let { diskCache ->
if (diskCache[oldCacheKey] != null) {
target.thumbnail = diskCache.put(newCacheKey, diskCache[oldCacheKey])
diskCache.delete(oldCacheKey)
}
}
}
}

private fun getCacheTypeFromCloudType(type: CloudType): LruFileCacheUtil.Cache {
return when (type) {
CloudType.DROPBOX -> LruFileCacheUtil.Cache.DROPBOX
CloudType.GOOGLE_DRIVE -> LruFileCacheUtil.Cache.GOOGLE_DRIVE
CloudType.ONEDRIVE -> LruFileCacheUtil.Cache.ONEDRIVE
CloudType.PCLOUD -> LruFileCacheUtil.Cache.PCLOUD
CloudType.WEBDAV -> LruFileCacheUtil.Cache.WEBDAV
CloudType.S3 -> LruFileCacheUtil.Cache.S3
CloudType.LOCAL -> LruFileCacheUtil.Cache.LOCAL
else -> throw IllegalStateException()
}
}

@Throws(BackendException::class)
abstract fun folder(cryptoParent: CryptoFolder, cleartextName: String): CryptoFolder

Expand Down Expand Up @@ -309,8 +380,21 @@ abstract class CryptoImplDecorator(
@Throws(BackendException::class)
fun read(cryptoFile: CryptoFile, data: OutputStream, progressAware: ProgressAware<DownloadState>) {
val ciphertextFile = cryptoFile.cloudFile

val diskCache = cryptoFile.cloudFile.cloud?.type()?.let { getLruCacheFor(it) }
val cacheKey = generateCacheKey(ciphertextFile)
val genThumbnail = isGenerateThumbnailsEnabled(diskCache, cryptoFile.name)

val thumbnailWriter = PipedOutputStream()
val thumbnailReader = PipedInputStream(thumbnailWriter)

try {
val encryptedTmpFile = readToTmpFile(cryptoFile, ciphertextFile, progressAware)

if (genThumbnail) {
startThumbnailGeneratorThread(diskCache, cacheKey, thumbnailReader)
}

progressAware.onProgress(Progress.started(DownloadState.decryption(cryptoFile)))
try {
Channels.newChannel(FileInputStream(encryptedTmpFile)).use { readableByteChannel ->
Expand All @@ -322,7 +406,12 @@ abstract class CryptoImplDecorator(
while (decryptingReadableByteChannel.read(buff).also { read = it } > 0) {
buff.flip()
data.write(buff.array(), 0, buff.remaining())
if (genThumbnail) {
thumbnailWriter.write(buff.array(), 0, buff.remaining())
}

decrypted += read.toLong()

progressAware
.onProgress(
Progress.progress(DownloadState.decryption(cryptoFile)) //
Expand All @@ -332,16 +421,88 @@ abstract class CryptoImplDecorator(
)
}
}
thumbnailWriter.flush()
closeQuietly(thumbnailWriter)
}
} finally {
encryptedTmpFile.delete()
progressAware.onProgress(Progress.completed(DownloadState.decryption(cryptoFile)))
}

closeQuietly(thumbnailReader)
} catch (e: IOException) {
throw FatalBackendException(e)
}
}

private fun closeQuietly(closeable: Closeable) {
try {
closeable.close();
} catch (e: IOException) {
// ignore
}
}

private fun startThumbnailGeneratorThread(diskCache: DiskLruCache?, cacheKey: String, thumbnailReader: PipedInputStream): Future<*> {
return thumbnailExecutorService.submit {
try {
val options = BitmapFactory.Options()
val thumbnailBitmap: Bitmap?
options.inSampleSize = 4 // pixel number reduced by a factor of 1/16

val bitmap = BitmapFactory.decodeStream(thumbnailReader, null, options)
val thumbnailWidth = 100
val thumbnailHeight = 100
thumbnailBitmap = ThumbnailUtils.extractThumbnail(bitmap, thumbnailWidth, thumbnailHeight)

if (thumbnailBitmap != null) {
storeThumbnail(diskCache, cacheKey, thumbnailBitmap)
}

closeQuietly(thumbnailReader)
} catch (e: Exception) {
Timber.e("Bitmap generation crashed")
}
}
}

protected fun generateCacheKey(cloudFile: CloudFile): String {
return buildString {
if (cloudFile.cloud?.id() != null)
this.append(cloudFile.cloud!!.id())
else
this.append("c") // "common"
this.append("-")
this.append(cloudFile.path.hashCode())
}
Comment on lines +470 to +477
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return buildString {
if (cloudFile.cloud?.id() != null)
this.append(cloudFile.cloud!!.id())
else
this.append("c") // "common"
this.append("-")
this.append(cloudFile.path.hashCode())
}
return String.format("%s-%d", cloudFile.cloud?.id() ?: "common", cloudFile.path.hashCode())

}

private fun isGenerateThumbnailsEnabled(cache: DiskLruCache?, fileName: String): Boolean {
return sharedPreferencesHandler.useLruCache() &&
Copy link
Member

Choose a reason for hiding this comment

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

From a user perspective, it is currently not obvious that you need to enable caching for thumbnail generation to work. We may need to show a dialogue when changing the thumbnail generation from NONE to something else, and also for when caching gets disabled.

Even better, we could also decouple it here completely from the response cache so that we do not depend on this response caching feature, right?

sharedPreferencesHandler.generateThumbnails() != ThumbnailsOption.NEVER &&
cache != null &&
isImageMediaType(fileName)
}

private fun storeThumbnail(cache: DiskLruCache?, cacheKey: String, thumbnailBitmap: Bitmap) {
val thumbnailFile: File = File.createTempFile(UUID.randomUUID().toString(), ".thumbnail", internalCache)
thumbnailBitmap.compress(Bitmap.CompressFormat.JPEG, 100, thumbnailFile.outputStream())

try {
cache?.let {
LruFileCacheUtil.storeToLruCache(it, cacheKey, thumbnailFile)
} ?: Timber.tag("CryptoImplDecorator").e("Failed to store item in LRU cache")
} catch (e: IOException) {
Timber.tag("CryptoImplDecorator").e(e, "Failed to write the thumbnail in DiskLruCache")
}

thumbnailFile.delete()
}

protected fun isImageMediaType(filename: String): Boolean {
return (mimeTypes.fromFilename(filename) ?: MimeType.WILDCARD_MIME_TYPE).mediatype == "image"
}

@Throws(BackendException::class, IOException::class)
private fun readToTmpFile(cryptoFile: CryptoFile, file: CloudFile, progressAware: ProgressAware<DownloadState>): File {
val encryptedTmpFile = File.createTempFile(UUID.randomUUID().toString(), ".crypto", internalCache)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ open class CryptoImplVaultFormat7 : CryptoImplDecorator {
val shortFileName = BaseEncoding.base64Url().encode(hash) + LONG_NODE_FILE_EXT
var dirFolder = cloudContentRepository.folder(getOrCreateCachingAwareDirIdInfo(cryptoParent).cloudFolder, shortFileName)

// if folder already exists in case of renaming
if (!cloudContentRepository.exists(dirFolder)) {
dirFolder = cloudContentRepository.create(dirFolder)
}
Expand Down Expand Up @@ -166,6 +165,18 @@ open class CryptoImplVaultFormat7 : CryptoImplDecorator {
}
}.map { node ->
ciphertextToCleartextNode(cryptoFolder, dirId, node)
}.onEach { cryptoNode ->
if (cryptoNode is CryptoFile && isImageMediaType(cryptoNode.name)) {
val cacheKey = generateCacheKey(cryptoNode.cloudFile)
cryptoNode.cloudFile.cloud?.type()?.let { cloudType ->
getLruCacheFor(cloudType)?.let { diskCache ->
val cacheFile = diskCache[cacheKey]
if (cacheFile != null) {
cryptoNode.thumbnail = cacheFile
}
}
}
}
Comment on lines +168 to +179
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit concerned about performance here, did you check how long it takes to open a folder containing 2000 images

  1. with generated thumbnail
  2. without generated thumbnail

}.toList().filterNotNull()
}

Expand Down Expand Up @@ -380,6 +391,7 @@ open class CryptoImplVaultFormat7 : CryptoImplDecorator {

@Throws(BackendException::class)
override fun move(source: CryptoFile, target: CryptoFile): CryptoFile {
renameFileInCache(source, target)
return if (source.cloudFile.parent.name.endsWith(LONG_NODE_FILE_EXT)) {
val targetDirFolder = cloudContentRepository.folder(target.cloudFile.parent, target.cloudFile.name)
val cryptoFile: CryptoFile = if (target.cloudFile.name.endsWith(LONG_NODE_FILE_EXT)) {
Expand Down Expand Up @@ -449,6 +461,15 @@ open class CryptoImplVaultFormat7 : CryptoImplDecorator {
} else {
cloudContentRepository.delete(node.cloudFile)
}

val cacheKey = generateCacheKey(node.cloudFile)
node.cloudFile.cloud?.type()?.let { cloudType ->
getLruCacheFor(cloudType)?.let { diskCache ->
if (diskCache[cacheKey] != null) {
diskCache.delete(cacheKey)
}
}
}
}
}

Expand Down Expand Up @@ -493,7 +514,7 @@ open class CryptoImplVaultFormat7 : CryptoImplDecorator {
cryptoFile, //
cloudContentRepository.write( //
targetFile, //
data.decorate(from(encryptedTmpFile)),
data.decorate(from(encryptedTmpFile)), //
UploadFileReplacingProgressAware(cryptoFile, progressAware), //
replace, //
encryptedTmpFile.length()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,18 @@ internal class CryptoImplVaultFormatPre7(
.filterIsInstance<CloudFile>()
.map { node ->
ciphertextToCleartextNode(cryptoFolder, dirId, node)
}.onEach { cryptoNode ->
if (cryptoNode is CryptoFile && isImageMediaType(cryptoNode.name)) {
val cacheKey = generateCacheKey(cryptoNode.cloudFile)
cryptoNode.cloudFile.cloud?.type()?.let { cloudType ->
getLruCacheFor(cloudType)?.let { diskCache ->
val cacheFile = diskCache[cacheKey]
if (cacheFile != null) {
cryptoNode.thumbnail = cacheFile
}
}
}
}
Comment on lines +131 to +142
Copy link
Member

Choose a reason for hiding this comment

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

The same as in the other list implementation, did you check the performance impact?

}
.toList()
.filterNotNull()
Expand Down Expand Up @@ -228,6 +240,7 @@ internal class CryptoImplVaultFormatPre7(
@Throws(BackendException::class)
override fun move(source: CryptoFile, target: CryptoFile): CryptoFile {
assertCryptoFileAlreadyExists(target)
renameFileInCache(source, target)
return file(target, cloudContentRepository.move(source.cloudFile, target.cloudFile), source.size)
}

Expand All @@ -248,6 +261,15 @@ internal class CryptoImplVaultFormatPre7(
evictFromCache(node)
} else if (node is CryptoFile) {
cloudContentRepository.delete(node.cloudFile)

val cacheKey = generateCacheKey(node.cloudFile)
node.cloudFile.cloud?.type()?.let { cloudType ->
getLruCacheFor(cloudType)?.let { diskCache ->
if (diskCache[cacheKey] != null) {
diskCache.delete(cacheKey)
}
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
package org.cryptomator.presentation.model

import org.cryptomator.data.cloud.crypto.CryptoFile
import org.cryptomator.domain.CloudFile
import org.cryptomator.domain.usecases.ResultRenamed
import org.cryptomator.presentation.util.FileIcon
import java.io.File
import java.util.Date

class CloudFileModel(cloudFile: CloudFile, val icon: FileIcon) : CloudNodeModel<CloudFile>(cloudFile) {

val modified: Date? = cloudFile.modified
val size: Long? = cloudFile.size
var thumbnail : File? = if (cloudFile is CryptoFile) cloudFile.thumbnail else null

constructor(cloudFileRenamed: ResultRenamed<CloudFile>, icon: FileIcon) : this(cloudFileRenamed.value(), icon) {
oldName = cloudFileRenamed.oldName
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.cryptomator.presentation.model

import android.graphics.Bitmap
Copy link
Member

Choose a reason for hiding this comment

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

Unused import

import org.cryptomator.domain.CloudNode
import java.io.Serializable

Expand All @@ -8,6 +9,7 @@ abstract class CloudNodeModel<T : CloudNode> internal constructor(private val cl
var oldName: String? = null
var progress: ProgressModel? = null
var isSelected = false

Copy link
Member

Choose a reason for hiding this comment

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

Remove newline

val name: String
get() = cloudNode.name
val simpleName: String
Expand Down
Loading