From 3f4b57635e38571d166840007982ed5b435d9001 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Tue, 7 Nov 2023 20:46:25 +0100 Subject: [PATCH] android: Use case insensitivity in DocumentsTree (#7115) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * android: Unify DocumentNode's `key` and `name` They're effectively the same data, just obtained in different ways. * android: Remove getFilenameWithExtensions method After the previous commit, there's only one remaining use of getFilenameWithExtensions. Let's get rid of that one in favor of DocumentFile.getName so we no longer need to do manual URI parsing. * android: Use case insensitivity in DocumentsTree External storage on Android is case insensitive. This is still the case when accessing it through SAF. (Of course, SAF makes no guarantees about whether the storage location picked by the user is backed by external storage or whether it's case insensitive, but I'm just going to ignore that for now because I am *so tired of SAF*) Because the underlying file system is case insensitive, Citra's caching layer that had to be implemented because SAF's performance is atrocious also needs to be case insensitive. Otherwise, we get a problem in the following scenario: 1. Citra wants to check if a particular folder exists in sdmc, and if not, create it. 2. The folder does exist, but it has a different capitalization than Citra expects, due to a mismatch between Citra's code and (typically) files dumped from a real 3DS using ThreeSD. 3. Citra tries to open the folder, but DocumentsTree fails to find it, because the case doesn't match. 4. Citra then tries to create the folder, but creating the folder fails, because the underlying filesystem considers the folder to exist. 5. The game fails to start. (Sorry, did I say creating the folder fails? Actually, a new folder does get created, with " (1)" appended to the end of the name. SAF makes no guarantees whatsoever about what happens in this situation – it's all determined by the storage provider!) This commit makes the caching layer case insensitive so that the described scenario will work better. --- .../citra/citra_emu/utils/DocumentsTree.java | 61 ++++++++++++++----- .../org/citra/citra_emu/utils/FileUtil.java | 12 +--- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.java b/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.java index 99d85aa44..7cf030748 100644 --- a/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.java +++ b/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.java @@ -4,6 +4,7 @@ import android.content.Context; import android.net.Uri; import android.provider.DocumentsContract; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.documentfile.provider.DocumentFile; @@ -14,6 +15,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.URLDecoder; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import java.util.StringTokenizer; @@ -48,12 +50,12 @@ public class DocumentsTree { Uri mUri = node.uri; try { String filename = URLDecoder.decode(name, FileUtil.DECODE_METHOD); - if (node.children.get(filename) != null) return true; + if (node.findChild(filename) != null) return true; DocumentFile createdFile = FileUtil.createFile(context, mUri.toString(), name); if (createdFile == null) return false; DocumentsNode document = new DocumentsNode(createdFile, false); document.parent = node; - node.children.put(document.key, document); + node.addChild(document); return true; } catch (Exception e) { Log.error("[DocumentsTree]: Cannot create file, error: " + e.getMessage()); @@ -69,12 +71,12 @@ public class DocumentsTree { Uri mUri = node.uri; try { String filename = URLDecoder.decode(name, FileUtil.DECODE_METHOD); - if (node.children.get(filename) != null) return true; + if (node.findChild(filename) != null) return true; DocumentFile createdDirectory = FileUtil.createDir(context, mUri.toString(), name); if (createdDirectory == null) return false; DocumentsNode document = new DocumentsNode(createdDirectory, true); document.parent = node; - node.children.put(document.key, document); + node.addChild(document); return true; } catch (Exception e) { Log.error("[DocumentsTree]: Cannot create file, error: " + e.getMessage()); @@ -105,7 +107,7 @@ public class DocumentsTree { } // If this directory have not been iterate struct it. if (!node.loaded) structTree(node); - return node.children.keySet().toArray(new String[0]); + return node.getChildNames(); } public long getFileSize(String filepath) { @@ -153,7 +155,7 @@ public class DocumentsTree { input.close(); output.flush(); output.close(); - destinationNode.children.put(document.key, document); + destinationNode.addChild(document); return true; } catch (Exception e) { Log.error("[DocumentsTree]: Cannot copy file, error: " + e.getMessage()); @@ -185,7 +187,7 @@ public class DocumentsTree { return false; } if (node.parent != null) { - node.parent.children.remove(node.key); + node.parent.removeChild(node); } return true; } catch (Exception e) { @@ -214,7 +216,7 @@ public class DocumentsTree { if (parent.isDirectory && !parent.loaded) { structTree(parent); } - return parent.children.get(filename); + return parent.findChild(filename); } /** @@ -227,15 +229,19 @@ public class DocumentsTree { for (CheapDocument document : documents) { DocumentsNode node = new DocumentsNode(document); node.parent = parent; - parent.children.put(node.key, node); + parent.addChild(node); } parent.loaded = true; } + @NonNull + private static String toLowerCase(@NonNull String str) { + return str.toLowerCase(Locale.ROOT); + } + private static class DocumentsNode { private DocumentsNode parent; private final Map children = new HashMap<>(); - private String key; private String name; private Uri uri; private boolean loaded = false; @@ -246,7 +252,6 @@ public class DocumentsTree { private DocumentsNode(CheapDocument document) { name = document.getFilename(); uri = document.getUri(); - key = FileUtil.getFilenameWithExtensions(uri); isDirectory = document.isDirectory(); loaded = !isDirectory; } @@ -254,18 +259,42 @@ public class DocumentsTree { private DocumentsNode(DocumentFile document, boolean isCreateDir) { name = document.getName(); uri = document.getUri(); - key = FileUtil.getFilenameWithExtensions(uri); isDirectory = isCreateDir; loaded = true; } - private void rename(String key) { + private void rename(String name) { if (parent == null) { return; } - parent.children.remove(this.key); - this.name = key; - parent.children.put(key, this); + parent.removeChild(this); + this.name = name; + parent.addChild(this); + } + + private void addChild(DocumentsNode node) { + children.put(toLowerCase(node.name), node); + } + + private void removeChild(DocumentsNode node) { + children.remove(toLowerCase(node.name)); + } + + @Nullable + private DocumentsNode findChild(String filename) { + return children.get(toLowerCase(filename)); + } + + @NonNull + private String[] getChildNames() { + String[] names = new String[children.size()]; + + int i = 0; + for (DocumentsNode child : children.values()) { + names[i++] = child.name; + } + + return names; } } } diff --git a/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.java b/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.java index 7de6f118f..6eb9de33e 100644 --- a/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.java +++ b/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.java @@ -338,13 +338,12 @@ public class FileUtil { for (Pair file : files) { DocumentFile to = file.second; Uri toUri = to.getUri(); - String filename = getFilenameWithExtensions(toUri); String toPath = toUri.getPath(); DocumentFile toParent = to.getParentFile(); if (toParent == null) continue; FileUtil.copyFile(context, file.first.getUri().toString(), - toParent.getUri().toString(), filename); + toParent.getUri().toString(), to.getName()); progress++; if (listener != null) { listener.onCopyProgress(toPath, progress, total); @@ -424,15 +423,6 @@ public class FileUtil { return false; } - public static String getFilenameWithExtensions(Uri uri) { - String path = uri.getPath(); - final int slashIndex = path.lastIndexOf('/'); - path = path.substring(slashIndex + 1); - // On Android versions below 10, it is possible to select the storage root, which might result in filenames with a colon. - final int colonIndex = path.indexOf(':'); - return path.substring(colonIndex + 1); - } - public static double getFreeSpace(Context context, Uri uri) { try { Uri docTreeUri = DocumentsContract.buildDocumentUriUsingTree(