Skip to content

Fix resource leaks and improve file upload collision handling #16235

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.

Sign up for GitHub

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

Open
dmiales wants to merge 1 commit into
base: master
Choose a base branch
from
+79 −36
Open

Fix resource leaks and improve file upload collision handling #16235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ class AutoUploadWorker(
val dateTime = formatter.parse(exifDate, pos)
if (dateTime != null) {
lastModificationTime = dateTime.time
Log_OC.w(TAG, "calculateLastModificationTime calculatedTime is: $lastModificationTime")
Log_OC.i(TAG, "calculateLastModificationTime calculatedTime is: $lastModificationTime")
} else {
Log_OC.w(TAG, "calculateLastModificationTime dateTime is empty")
}
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/nextcloud/client/jobs/utils/UploadErrorNotificationManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ object UploadErrorNotificationManager {
result.code == ResultCode.USER_CANCELLED ||
operation.isMissingPermissionThrown
) {
Log_OC.w(TAG, "operation is successful, cancelled or lack of storage permission")
Log_OC.i(TAG, "operation is successful, cancelled or lack of storage permission")
return false
}

Expand Down
42 changes: 28 additions & 14 deletions app/src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,21 @@ private void updateUploadStatus(long id, UploadStatus status, UploadResult resul
null
);

if (c != null) {
if (c.getCount() != SINGLE_RESULT) {
Log_OC.e(TAG, c.getCount() + " items for id=" + id
+ " available in UploadDb. Expected 1. Failed to update upload db.");
try {
if (c != null) {
if (c.getCount() != SINGLE_RESULT) {
Log_OC.e(TAG, c.getCount() + " items for id=" + id
+ " available in UploadDb. Expected 1. Failed to update upload db.");
} else {
updateUploadInternal(c, status, result, remotePath, localPath);
}
} else {
updateUploadInternal(c, status, result, remotePath, localPath);
Log_OC.e(TAG, "Cursor is null");
}
} finally {
if (c != null) {
c.close();
}
c.close();
} else {
Log_OC.e(TAG, "Cursor is null");
}

}
Expand Down Expand Up @@ -302,9 +307,15 @@ OCUpload getUploadById(long id) {
new String[]{Long.toString(id)},
"_id ASC");

if (cursor != null) {
if (cursor.moveToFirst()) {
result = createOCUploadFromCursor(cursor);
try {
if (cursor != null) {
if (cursor.moveToFirst()) {
result = createOCUploadFromCursor(cursor);
}
}
} finally {
if (cursor != null) {
cursor.close();
}
}
Log_OC.d(TAG, "Retrieve job " + result + " for id " + id);
Expand Down Expand Up @@ -414,8 +425,8 @@ private List<OCUpload> getUploadPage(long limit, final long afterId, final boole
pageSelectionArgs,
sortOrder);

if (c != null) {
if (c.moveToFirst()) {
try {
if (c != null && c.moveToFirst()) {
do {
OCUpload upload = createOCUploadFromCursor(c);
if (upload == null) {
Expand All @@ -425,7 +436,10 @@ private List<OCUpload> getUploadPage(long limit, final long afterId, final boole
}
} while (c.moveToNext() && !c.isAfterLast());
}
c.close();
} finally {
if (c != null) {
c.close();
}
}
return uploads;
}
Expand Down
28 changes: 26 additions & 2 deletions app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ public UploadFileOperation(UploadsStorageManager uploadsStorageManager,
this.user = user;
mUpload = upload;
if (file == null) {
Log_OC.w(TAG, "UploadFileOperation file is null, obtaining from upload");
Log_OC.i(TAG, "UploadFileOperation file is null, obtaining from upload");
mFile = obtainNewOCFileToUpload(
upload.getRemotePath(),
upload.getLocalPath(),
Expand Down Expand Up @@ -1232,7 +1232,31 @@ private RemoteOperationResult checkNameCollision(OCFile parentFile,
switch (mNameCollisionPolicy) {
case SKIP:
Log_OC.d(TAG, "user choose to skip upload if same file exists");
return new RemoteOperationResult<>(ResultCode.OK);
// For encrypted files, we can't easily compare content, so skip based on name only
// For non-encrypted files, check if it's actually the same file by content
if (!encrypted && mContext != null && user != null && mOriginalStoragePath != null) {
File localFile = new File(mOriginalStoragePath);
if (localFile.exists()) {
boolean isSameFile = FileUploadHelper.Companion.instance().isSameFileOnRemote(
user, localFile, mRemotePath, mContext);
if (isSameFile) {
Log_OC.d(TAG, "File is the same on remote, skipping upload");
return new RemoteOperationResult<>(ResultCode.OK);
} else {
Log_OC.d(TAG, "File with same name exists but content is different, reporting conflict");
// File exists but is different, return conflict so system can handle it
return new RemoteOperationResult(ResultCode.SYNC_CONFLICT);
}
} else {
Log_OC.w(TAG, "Local file does not exist, cannot compare content");
// If local file doesn't exist, skip based on name only (old behavior)
return new RemoteOperationResult<>(ResultCode.OK);
}
} else {
// For encrypted files or when context/user/file path is unavailable,
// skip based on name only (preserve old behavior)
return new RemoteOperationResult<>(ResultCode.OK);
}
case RENAME:
mRemotePath = getNewAvailableRemotePath(client, mRemotePath, fileNames, encrypted);
mWasRenamed = true;
Expand Down
41 changes: 23 additions & 18 deletions app/src/main/java/com/owncloud/android/ui/fragment/FileDetailSharingFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,30 +624,35 @@ private void handleContactResult(@NonNull Uri contactUri) {

Cursor cursor = fileActivity.getContentResolver().query(contactUri, projection, null, null, null);

if (cursor != null) {
if (cursor.moveToFirst()) {
// The contact has only one email address, use it.
int columnIndex = cursor.getColumnIndex(ContactsContract.CommonDataKinds.Email.ADDRESS);
if (columnIndex != -1) {
// Use the email address as needed.
// email variable contains the selected contact's email address.
String email = cursor.getString(columnIndex);
binding.searchView.post(() -> {
binding.searchView.setQuery(email, false);
binding.searchView.requestFocus();
});
try {
if (cursor != null) {
if (cursor.moveToFirst()) {
// The contact has only one email address, use it.
int columnIndex = cursor.getColumnIndex(ContactsContract.CommonDataKinds.Email.ADDRESS);
if (columnIndex != -1) {
// Use the email address as needed.
// email variable contains the selected contact's email address.
String email = cursor.getString(columnIndex);
binding.searchView.post(() -> {
binding.searchView.setQuery(email, false);
binding.searchView.requestFocus();
});
} else {
DisplayUtils.showSnackMessage(binding.getRoot(), R.string.email_pick_failed);
Log_OC.e(FileDetailSharingFragment.class.getSimpleName(), "Failed to pick email address.");
}
} else {
DisplayUtils.showSnackMessage(binding.getRoot(), R.string.email_pick_failed);
Log_OC.e(FileDetailSharingFragment.class.getSimpleName(), "Failed to pick email address.");
Log_OC.e(FileDetailSharingFragment.class.getSimpleName(), "Failed to pick email address as no Email found.");
}
} else {
DisplayUtils.showSnackMessage(binding.getRoot(), R.string.email_pick_failed);
Log_OC.e(FileDetailSharingFragment.class.getSimpleName(), "Failed to pick email address as no Email found.");
Log_OC.e(FileDetailSharingFragment.class.getSimpleName(), "Failed to pick email address as Cursor is null.");
}
} finally {
if (cursor != null) {
cursor.close();
}
cursor.close();
} else {
DisplayUtils.showSnackMessage(binding.getRoot(), R.string.email_pick_failed);
Log_OC.e(FileDetailSharingFragment.class.getSimpleName(), "Failed to pick email address as Cursor is null.");
}
}

Expand Down
Loading