Skip to content

Commit 1bbd4da

Browse files
committed
Avoid SMB rename checks on the UI thread
The rename completion path in MainActivityHelper re-checked success by calling newFile.exists(context) on the UI thread because SAF-backed renames can report false negatives. For SMB targets that fallback routes into HybridFile.exists() and performs synchronous network I/O on the main thread. Limit the fallback probe to SAF-backed targets and cover it with a focused regression test. Constraint: Preserve the post-rename verification workaround for SAF-backed rename targets Rejected: Change HybridFile.exists() semantics for all SMB callers | too broad and would affect background operations that legitimately rely on it Confidence: high Scope-risk: narrow Directive: Keep rename completion fallbacks off the UI thread for remote backends; only use exists(context) as a SAF-specific verification path Tested: ./gradlew :app:testFdroidDebugUnitTest --tests com.amaze.filemanager.utils.MainActivityHelperTest -x kaptFdroidDebugUnitTestKotlin Tested: ./gradlew :app:compileFdroidDebugJavaWithJavac Tested: ./gradlew :app:compileFdroidDebugUnitTestJavaWithJavac -x kaptFdroidDebugUnitTestKotlin Not-tested: Full :app:testFdroidDebugUnitTest (blocked by existing kaptFdroidDebugUnitTestKotlin failure in unrelated tests referencing missing ShadowMultiDex)
1 parent e1e5619 commit 1bbd4da

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

app/src/main/java/com/amaze/filemanager/utils/MainActivityHelper.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,8 @@ public void done(final HybridFile hFile, final boolean b) {
388388
* DocumentFile.renameTo() may return false even when rename is successful. Hence we need an extra check
389389
* instead of merely looking at the return value
390390
*/
391-
if (b || newFile.exists(context)) {
391+
if (b
392+
|| (shouldVerifyRenameWithExists(newFile) && newFile.exists(context))) {
392393
Intent intent = new Intent(MainActivity.KEY_INTENT_LOAD_LIST);
393394

394395
intent.putExtra(
@@ -435,6 +436,10 @@ public void invalidName(final HybridFile file) {
435436
});
436437
}
437438

439+
static boolean shouldVerifyRenameWithExists(HybridFile file) {
440+
return file.isDocumentFile() || file.isOtgFile();
441+
}
442+
438443
public @FolderState int checkFolder(final @NonNull File folder, Context context) {
439444
return checkFolder(folder.getAbsolutePath(), OpenMode.FILE, context);
440445
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package com.amaze.filemanager.utils;
2+
3+
import static org.junit.Assert.assertFalse;
4+
import static org.junit.Assert.assertTrue;
5+
6+
import com.amaze.filemanager.fileoperations.filesystem.OpenMode;
7+
import com.amaze.filemanager.filesystem.HybridFile;
8+
9+
import org.junit.Test;
10+
11+
public class MainActivityHelperTest {
12+
@Test
13+
public void postRenameExistenceCheck_isLimitedToSafBackedTargets() {
14+
assertTrue(
15+
MainActivityHelper.shouldVerifyRenameWithExists(
16+
new HybridFile(OpenMode.DOCUMENT_FILE, "content://example/tree/primary:test")));
17+
assertTrue(
18+
MainActivityHelper.shouldVerifyRenameWithExists(
19+
new HybridFile(OpenMode.OTG, "otg:/example")));
20+
21+
assertFalse(
22+
MainActivityHelper.shouldVerifyRenameWithExists(
23+
new HybridFile(OpenMode.SMB, "smb://server/share/file")));
24+
assertFalse(
25+
MainActivityHelper.shouldVerifyRenameWithExists(
26+
new HybridFile(OpenMode.FILE, "/storage/emulated/0/file")));
27+
}
28+
}

0 commit comments

Comments
 (0)