Skip to content

Commit 7aaf1ad

Browse files
committed
perf: implement streaming XML parsing for large folder operations
- Add StreamingReadFolderRemoteOperation for memory-efficient folder reading using SAX parser - Add PropFindSaxHandler for streaming XML parsing of PROPFIND responses - Prevent OutOfMemoryError for folders with thousands of files by avoiding full XML loading - Integrate streaming parser into RefreshFolderOperation with fallback to legacy method - Add batch processing for large folders (500 files per batch) in FileDataStorageManager - Improve logging levels and add thread name tracking for better debugging
1 parent 142578f commit 7aaf1ad

7 files changed

Lines changed: 269 additions & 64 deletions

File tree

app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ class AutoUploadWorker(
502502
val dateTime = formatter.parse(exifDate, pos)
503503
if (dateTime != null) {
504504
lastModificationTime = dateTime.time
505-
Log_OC.w(TAG, "calculateLastModificationTime calculatedTime is: $lastModificationTime")
505+
Log_OC.i(TAG, "calculateLastModificationTime calculatedTime is: $lastModificationTime")
506506
} else {
507507
Log_OC.w(TAG, "calculateLastModificationTime dateTime is empty")
508508
}

app/src/main/java/com/nextcloud/client/jobs/utils/UploadErrorNotificationManager.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ object UploadErrorNotificationManager {
172172
result.code == ResultCode.USER_CANCELLED ||
173173
operation.isMissingPermissionThrown
174174
) {
175-
Log_OC.w(TAG, "operation is successful, cancelled or lack of storage permission")
175+
Log_OC.i(TAG, "operation is successful, cancelled or lack of storage permission")
176176
return false
177177
}
178178

app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java

Lines changed: 116 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ public class FileDataStorageManager {
9898
private static final String FAILED_TO_INSERT_MSG = "Fail to insert insert file to database ";
9999
private static final String SENDING_TO_FILECONTENTPROVIDER_MSG = "Sending %d operations to FileContentProvider";
100100
private static final String EXCEPTION_MSG = "Exception in batch of operations ";
101+
private static final int BATCH_SIZE = 500; // Maximum number of operations per batch to avoid memory issues
101102

102103
public static final int ROOT_PARENT_ID = 0;
103104
private static final String JSON_NULL_STRING = "null";
@@ -670,13 +671,59 @@ public void saveNewFile(OCFile newFile) {
670671
* @param filesToRemove
671672
*/
672673
public void saveFolder(OCFile folder, List<OCFile> updatedFiles, Collection<OCFile> filesToRemove) {
674+
String threadName = Thread.currentThread().getName();
673675
Log_OC.d(TAG, "Saving folder " + folder.getRemotePath() + " with " + updatedFiles.size()
674-
+ " children and " + filesToRemove.size() + " files to remove");
676+
+ " children and " + filesToRemove.size() + " files to remove [Thread: " + threadName + "]");
677+
678+
// Process files in batches to avoid memory issues with large folders
679+
int totalFiles = updatedFiles.size();
680+
if (totalFiles > BATCH_SIZE) {
681+
Log_OC.d(TAG, "Large folder detected (" + totalFiles + " files). Processing in batches of " + BATCH_SIZE + " [Thread: " + threadName + "]");
682+
683+
// Process files in batches
684+
for (int i = 0; i < totalFiles; i += BATCH_SIZE) {
685+
int endIndex = Math.min(i + BATCH_SIZE, totalFiles);
686+
List<OCFile> batchFiles = updatedFiles.subList(i, endIndex);
687+
saveFolderBatch(folder, batchFiles, i);
688+
}
689+
} else {
690+
// Small folder - process normally
691+
saveFolderBatch(folder, updatedFiles, 0);
692+
}
675693

676-
ArrayList<ContentProviderOperation> operations = new ArrayList<>(updatedFiles.size());
694+
// Process deletions separately
695+
if (!filesToRemove.isEmpty()) {
696+
processFileRemovals(folder, filesToRemove);
697+
}
698+
699+
// Update folder metadata (always last)
700+
updateFolderMetadata(folder);
701+
}
702+
703+
/**
704+
* Saves a batch of files to the database without updating folder metadata.
705+
* This is used when processing large folders in batches to avoid memory issues.
706+
*
707+
* @param folder The parent folder
708+
* @param batchFiles The files to save in this batch
709+
* @param startIndex The starting index in the original list (for logging)
710+
*/
711+
public void saveFolderBatchOnly(OCFile folder, List<OCFile> batchFiles, int startIndex) {
712+
saveFolderBatch(folder, batchFiles, startIndex);
713+
}
714+
715+
/**
716+
* Saves a batch of files to the database.
717+
*
718+
* @param folder The parent folder
719+
* @param batchFiles The files to save in this batch
720+
* @param startIndex The starting index in the original list (for logging)
721+
*/
722+
private void saveFolderBatch(OCFile folder, List<OCFile> batchFiles, int startIndex) {
723+
ArrayList<ContentProviderOperation> operations = new ArrayList<>(batchFiles.size());
677724

678725
// prepare operations to insert or update files to save in the given folder
679-
for (OCFile ocFile : updatedFiles) {
726+
for (OCFile ocFile : batchFiles) {
680727
ContentValues contentValues = createContentValuesForFile(ocFile);
681728
contentValues.put(ProviderTableMeta.FILE_PARENT, folder.getFileId());
682729

@@ -700,10 +747,51 @@ public void saveFolder(OCFile folder, List<OCFile> updatedFiles, Collection<OCFi
700747
}
701748
}
702749

703-
// prepare operations to remove files in the given folder
750+
// apply operations in batch
751+
ContentProviderResult[] results = null;
752+
Log_OC.d(TAG, String.format(Locale.ENGLISH, SENDING_TO_FILECONTENTPROVIDER_MSG + " (batch starting at index %d)",
753+
operations.size(), startIndex));
754+
755+
try {
756+
if (getContentResolver() != null) {
757+
results = getContentResolver().applyBatch(MainApp.getAuthority(), operations);
758+
} else {
759+
results = getContentProviderClient().applyBatch(operations);
760+
}
761+
} catch (OperationApplicationException | RemoteException e) {
762+
Log_OC.e(TAG, EXCEPTION_MSG + e.getMessage(), e);
763+
return;
764+
}
765+
766+
// update new id in file objects for insertions
767+
if (results != null) {
768+
Iterator<OCFile> fileIterator = batchFiles.iterator();
769+
for (ContentProviderResult result : results) {
770+
OCFile ocFile = fileIterator.hasNext() ? fileIterator.next() : null;
771+
if (result.uri != null && ocFile != null) {
772+
try {
773+
long newId = Long.parseLong(result.uri.getPathSegments().get(1));
774+
ocFile.setFileId(newId);
775+
} catch (NumberFormatException | IndexOutOfBoundsException e) {
776+
Log_OC.e(TAG, "Failed to parse file ID from URI: " + result.uri, e);
777+
}
778+
}
779+
}
780+
}
781+
}
782+
783+
/**
784+
* Processes file removals for a folder.
785+
*
786+
* @param folder The parent folder
787+
* @param filesToRemove The files to remove
788+
*/
789+
public void processFileRemovals(OCFile folder, Collection<OCFile> filesToRemove) {
790+
ArrayList<ContentProviderOperation> operations = new ArrayList<>();
704791
String where = ProviderTableMeta.FILE_ACCOUNT_OWNER + AND + ProviderTableMeta.FILE_PATH + " = ?";
705792
String[] whereArgs = new String[2];
706793
whereArgs[0] = user.getAccountName();
794+
707795
for (OCFile ocFile : filesToRemove) {
708796
if (ocFile.getParentId() == folder.getFileId()) {
709797
whereArgs[1] = ocFile.getRemotePath();
@@ -731,49 +819,43 @@ public void saveFolder(OCFile folder, List<OCFile> updatedFiles, Collection<OCFi
731819
}
732820
}
733821

734-
// update metadata of folder
735-
ContentValues contentValues = createContentValuesForFolder(folder);
822+
if (!operations.isEmpty()) {
823+
Log_OC.d(TAG, String.format(Locale.ENGLISH, SENDING_TO_FILECONTENTPROVIDER_MSG + " (deletions)", operations.size()));
824+
try {
825+
if (getContentResolver() != null) {
826+
getContentResolver().applyBatch(MainApp.getAuthority(), operations);
827+
} else {
828+
getContentProviderClient().applyBatch(operations);
829+
}
830+
} catch (OperationApplicationException | RemoteException e) {
831+
Log_OC.e(TAG, EXCEPTION_MSG + e.getMessage(), e);
832+
}
833+
}
834+
}
736835

836+
/**
837+
* Updates the metadata of a folder.
838+
*
839+
* @param folder The folder to update
840+
*/
841+
public void updateFolderMetadata(OCFile folder) {
842+
ContentValues contentValues = createContentValuesForFolder(folder);
843+
ArrayList<ContentProviderOperation> operations = new ArrayList<>(1);
844+
737845
operations.add(ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI)
738846
.withValues(contentValues)
739847
.withSelection(ProviderTableMeta._ID + " = ?", new String[]{String.valueOf(folder.getFileId())})
740848
.build());
741849

742-
// apply operations in batch
743-
ContentProviderResult[] results = null;
744-
Log_OC.d(TAG, String.format(Locale.ENGLISH, SENDING_TO_FILECONTENTPROVIDER_MSG, operations.size()));
745-
746850
try {
747851
if (getContentResolver() != null) {
748-
results = getContentResolver().applyBatch(MainApp.getAuthority(), operations);
749-
852+
getContentResolver().applyBatch(MainApp.getAuthority(), operations);
750853
} else {
751-
results = getContentProviderClient().applyBatch(operations);
854+
getContentProviderClient().applyBatch(operations);
752855
}
753-
754856
} catch (OperationApplicationException | RemoteException e) {
755857
Log_OC.e(TAG, EXCEPTION_MSG + e.getMessage(), e);
756858
}
757-
758-
// update new id in file objects for insertions
759-
if (results != null) {
760-
long newId;
761-
Iterator<OCFile> fileIterator = updatedFiles.iterator();
762-
OCFile ocFile;
763-
for (ContentProviderResult result : results) {
764-
if (fileIterator.hasNext()) {
765-
ocFile = fileIterator.next();
766-
} else {
767-
ocFile = null;
768-
}
769-
if (result.uri != null) {
770-
newId = Long.parseLong(result.uri.getPathSegments().get(1));
771-
if (ocFile != null) {
772-
ocFile.setFileId(newId);
773-
}
774-
}
775-
}
776-
}
777859
}
778860

779861
/**

app/src/main/java/com/owncloud/android/datamodel/OCFile.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,18 @@ public long getLocalId() {
630630
if (localId > 0) {
631631
return localId;
632632
} else if (remoteId != null && remoteId.length() > 8) {
633-
return Long.parseLong(remoteId.substring(0, 8).replaceAll("^0*", ""));
633+
try {
634+
// Only try to parse as number if it looks like a number (starts with digit)
635+
String potentialNumber = remoteId.substring(0, 8).replaceAll("^0*", "");
636+
if (potentialNumber.matches("\\d+")) {
637+
return Long.parseLong(potentialNumber);
638+
} else {
639+
return -1;
640+
}
641+
} catch (NumberFormatException e) {
642+
Log_OC.w("OCFile", "Failed to parse localId from remoteId: '" + remoteId + "', returning -1");
643+
return -1;
644+
}
634645
} else {
635646
return -1;
636647
}

app/src/main/java/com/owncloud/android/datamodel/ThumbnailsCacheManager.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
import java.io.FileNotFoundException;
6969
import java.io.InputStream;
7070
import java.lang.ref.WeakReference;
71+
import java.net.URLEncoder;
7172
import java.util.List;
7273
import java.util.Objects;
7374
import java.util.concurrent.ExecutorService;
@@ -644,9 +645,24 @@ private Bitmap doThumbnailFromOCFileInBackground() {
644645
try {
645646
String uri;
646647
if (file instanceof OCFile) {
647-
uri = mClient.getBaseUri() + "/index.php/core/preview?fileId="
648-
+ file.getLocalId()
649-
+ "&x=" + pxW + "&y=" + pxH + "&a=1&mode=cover&forceIcon=0";
648+
long localId = file.getLocalId();
649+
if (localId > 0) {
650+
// Use fileId if available
651+
uri = mClient.getBaseUri() + "/index.php/core/preview?fileId="
652+
+ localId
653+
+ "&x=" + pxW + "&y=" + pxH + "&a=1&mode=cover&forceIcon=0";
654+
} else {
655+
// Try different API endpoints for Nextcloud 31
656+
String filePath = ((OCFile) file).getRemotePath();
657+
// Try Nextcloud 31 files API thumbnail endpoint
658+
String cleanPath = filePath.startsWith("/") ? filePath.substring(1) : filePath;
659+
try {
660+
cleanPath = URLEncoder.encode(cleanPath, "UTF-8");
661+
} catch (Exception e) {
662+
// Ignore encoding errors
663+
}
664+
uri = mClient.getBaseUri() + "/index.php/apps/files/api/v1/thumbnail/" + pxW + "/" + pxH + "/" + cleanPath;
665+
}
650666
} else {
651667
uri = mClient.getBaseUri() + "/index.php/apps/files_trashbin/preview?fileId="
652668
+ file.getLocalId() + "&x=" + pxW + "&y=" + pxH;

0 commit comments

Comments
 (0)