Skip to content

Commit a036ec2

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 a036ec2

2 files changed

Lines changed: 36 additions & 1 deletion

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080

8181
import androidx.annotation.NonNull;
8282
import androidx.annotation.StringRes;
83+
import androidx.annotation.VisibleForTesting;
8384
import androidx.appcompat.widget.AppCompatEditText;
8485
import androidx.appcompat.widget.AppCompatImageView;
8586
import androidx.appcompat.widget.AppCompatTextView;
@@ -388,7 +389,8 @@ public void done(final HybridFile hFile, final boolean b) {
388389
* DocumentFile.renameTo() may return false even when rename is successful. Hence we need an extra check
389390
* instead of merely looking at the return value
390391
*/
391-
if (b || newFile.exists(context)) {
392+
if (b
393+
|| (shouldVerifyRenameWithExists(newFile) && newFile.exists(context))) {
392394
Intent intent = new Intent(MainActivity.KEY_INTENT_LOAD_LIST);
393395

394396
intent.putExtra(
@@ -435,6 +437,11 @@ public void invalidName(final HybridFile file) {
435437
});
436438
}
437439

440+
@VisibleForTesting
441+
static boolean shouldVerifyRenameWithExists(HybridFile file) {
442+
return file.isDocumentFile() || file.isOtgFile();
443+
}
444+
438445
public @FolderState int checkFolder(final @NonNull File folder, Context context) {
439446
return checkFolder(folder.getAbsolutePath(), OpenMode.FILE, context);
440447
}
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 testPostRenameExistenceCheckIsLimitedToSafBackedTargets() {
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)