-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[FIX] Image autoupload uploads images twice under certain conditions #4571
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
Changes from all commits
a55e45a
db2d919
bb7b0dd
c6b2ea3
5383f85
428adca
d4f60b9
d5cd00d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
Bugfix: Changes in the automatic uploads algorithm to prevent duplications | ||
|
||
The timestamp for automatic uploads is now updated at the beginning of the upload process instead of at the end. Additionally, the filter | ||
used in AutomaticUploadsWorker to select the files to upload has been modified in order to reduce time and charge when evaluating all files. | ||
|
||
https://github.com/owncloud/android/issues/3983 | ||
https://github.com/owncloud/android/pull/4571 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,9 @@ | |
* | ||
* @author Abel García de Prada | ||
* @author Juan Carlos Garrote Gascón | ||
* @author Jorge Aguado Recio | ||
* | ||
* Copyright (C) 2022 ownCloud GmbH. | ||
* Copyright (C) 2025 ownCloud GmbH. | ||
* <p> | ||
* This program is free software: you can redistribute it and/or modify | ||
* it under the terms of the GNU General Public License version 2, | ||
|
@@ -122,9 +123,7 @@ class AutomaticUploadsWorker( | |
WorkManager.getInstance(appContext).cancelUniqueWork(AUTOMATIC_UPLOADS_WORKER) | ||
} | ||
|
||
private fun syncFolder(folderBackUpConfiguration: FolderBackUpConfiguration?) { | ||
if (folderBackUpConfiguration == null) return | ||
|
||
private fun syncFolder(folderBackUpConfiguration: FolderBackUpConfiguration) { | ||
val syncType = when { | ||
folderBackUpConfiguration.isPictureUploads -> SyncType.PICTURE_UPLOADS | ||
folderBackUpConfiguration.isVideoUploads -> SyncType.VIDEO_UPLOADS | ||
|
@@ -133,6 +132,7 @@ class AutomaticUploadsWorker( | |
} | ||
|
||
val currentTimestamp = System.currentTimeMillis() | ||
updateTimestamp(folderBackUpConfiguration, syncType, currentTimestamp) | ||
|
||
val localPicturesDocumentFiles: List<DocumentFile> = getFilesReadyToUpload( | ||
syncType = syncType, | ||
|
@@ -166,7 +166,6 @@ class AutomaticUploadsWorker( | |
chargingOnly = folderBackUpConfiguration.chargingOnly | ||
) | ||
} | ||
updateTimestamp(folderBackUpConfiguration, syncType, currentTimestamp) | ||
} | ||
|
||
private fun showNotification( | ||
|
@@ -248,10 +247,14 @@ class AutomaticUploadsWorker( | |
val arrayOfLocalFiles = documentTree?.listFiles() ?: arrayOf() | ||
|
||
val filteredList: List<DocumentFile> = arrayOfLocalFiles | ||
.asSequence() | ||
.filter { | ||
it.lastModified() in lastSyncTimestamp..<currentTimestamp && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if it would make more sense to move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's right, but there are a lot of use cases... 🤔 As you said, you can have a folder in which the number of videos and photos is only a 1% and, with this filter, you will have 99% unnecessary calls. Also you can have a folder with a high number of photos and videos (99%) so, if we change the order there will be a lot of unnecessary calls anyway. In any case, @jesmrec is doing some tests so we will see what happens... But we will take into account your comments if something goes wrong. Thanks a lot for your opinion! 😄 |
||
MimetypeIconUtil.getBestMimeTypeByFilename(it.name).startsWith(syncType.prefixForType) && | ||
!it.name.orEmpty().startsWith(".") | ||
} | ||
.sortedBy { it.lastModified() } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for sorting? Assuming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if it works as you comment here but it could be... 🤔 The reason for sorting is: upload the files in the same order that were taken, so we can have an "history" in the uploads section... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah okay, if one sees that as important then ordering makes sense. |
||
.filter { it.lastModified() >= lastSyncTimestamp } | ||
.filter { it.lastModified() < currentTimestamp } | ||
.filter { MimetypeIconUtil.getBestMimeTypeByFilename(it.name).startsWith(syncType.prefixForType) } | ||
.toList() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not knowing kotlin at all I am curious: What is the usecase for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The sequence is used to load and evaluate the files individually. The list loads all the files together and then the necessary checks are performed. The final |
||
|
||
Timber.i("Last sync ${syncType.name}: ${Date(lastSyncTimestamp)}") | ||
Timber.i("CurrentTimestamp ${Date(currentTimestamp)}") | ||
|
Uh oh!
There was an error while loading. Please reload this page.