From d246f5b436fb54103f930fe4d3931aa3d7a17991 Mon Sep 17 00:00:00 2001 From: OpenSauce Date: Mon, 29 Dec 2025 17:55:51 +0000 Subject: [PATCH] android: Fix rename + move behaviour for Google Play build (#1545) * android: Fix rename + move behaviour for Google Play build * android_storage: Handle rename/move with same source and destination --- externals/boost | 2 +- .../java/org/citra/citra_emu/NativeLibrary.kt | 13 ++ .../citra/citra_emu/utils/DocumentsTree.kt | 20 +++- .../org/citra/citra_emu/utils/FileUtil.kt | 15 +++ src/common/android_storage.cpp | 112 ++++++++++++++++-- src/common/android_storage.h | 7 +- src/common/file_util.cpp | 21 ++-- src/common/file_util.h | 8 +- 8 files changed, 170 insertions(+), 28 deletions(-) diff --git a/externals/boost b/externals/boost index 3c27c785a..2c82bd787 160000 --- a/externals/boost +++ b/externals/boost @@ -1 +1 @@ -Subproject commit 3c27c785ad0f8a742af02e620dc225673f3a12d8 +Subproject commit 2c82bd787302398bcae990e3c9ab2b451284f4ca diff --git a/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt b/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt index 7f44c3b0e..b64d63035 100644 --- a/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt +++ b/src/android/app/src/main/java/org/citra/citra_emu/NativeLibrary.kt @@ -733,6 +733,19 @@ object NativeLibrary { fun updateDocumentLocation(sourcePath: String, destinationPath: String): Boolean = CitraApplication.documentsTree.updateDocumentLocation(sourcePath, destinationPath) + @Keep + @JvmStatic + fun moveFile(filename: String, sourceDirPath: String, destinationDirPath: String): Boolean = + if (FileUtil.isNativePath(sourceDirPath)) { + try { + CitraApplication.documentsTree.moveFile(filename, sourceDirPath, destinationDirPath) + } catch (e: Exception) { + false + } + } else { + FileUtil.moveFile(filename, sourceDirPath, destinationDirPath) + } + @Keep @JvmStatic fun deleteDocument(path: String): Boolean = diff --git a/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.kt b/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.kt index 8bb46027e..d5c4f791d 100644 --- a/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.kt +++ b/src/android/app/src/main/java/org/citra/citra_emu/utils/DocumentsTree.kt @@ -193,7 +193,7 @@ class DocumentsTree { } @Synchronized - fun renameFile(filepath: String, destinationFilename: String?): Boolean { + fun renameFile(filepath: String, destinationFilename: String): Boolean { val node = resolvePath(filepath) ?: return false try { val filename = URLDecoder.decode(destinationFilename, FileUtil.DECODE_METHOD) @@ -205,6 +205,20 @@ class DocumentsTree { } } + @Synchronized + fun moveFile(filename: String, sourceDirPath: String, destDirPath: String): Boolean { + val sourceFileNode = resolvePath(sourceDirPath + "/" + filename) ?: return false + val sourceDirNode = resolvePath(sourceDirPath) ?: return false + val destDirNode = resolvePath(destDirPath) ?: return false + try { + val newUri = DocumentsContract.moveDocument(context.contentResolver, sourceFileNode.uri!!, sourceDirNode.uri!!, destDirNode.uri!!) + updateDocumentLocation("$sourceDirPath/$filename", "$destDirPath/$filename") + return true + } catch (e: Exception) { + error("[DocumentsTree]: Cannot move file, error: " + e.message) + } + } + @Synchronized fun deleteDocument(filepath: String): Boolean { val node = resolvePath(filepath) ?: return false @@ -223,15 +237,15 @@ class DocumentsTree { @Synchronized fun updateDocumentLocation(sourcePath: String, destinationPath: String): Boolean { - Log.error("Got paths: $sourcePath, $destinationPath") val sourceNode = resolvePath(sourcePath) val newName = Paths.get(destinationPath).fileName.toString() val parentPath = Paths.get(destinationPath).parent.toString() val newParent = resolvePath(parentPath) val newUri = (getUri(parentPath).toString() + "%2F$newName").toUri() // <- Is there a better way? - if (sourceNode == null || newParent == null) + if (sourceNode == null || newParent == null) { return false + } sourceNode.parent!!.removeChild(sourceNode) diff --git a/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.kt b/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.kt index e5bf4fea5..9d9063c59 100644 --- a/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.kt +++ b/src/android/app/src/main/java/org/citra/citra_emu/utils/FileUtil.kt @@ -11,6 +11,7 @@ import android.net.Uri import android.provider.DocumentsContract import android.system.Os import android.util.Pair +import androidx.core.net.toUri import androidx.documentfile.provider.DocumentFile import org.citra.citra_emu.CitraApplication import org.citra.citra_emu.model.CheapDocument @@ -434,6 +435,20 @@ object FileUtil { return false } + @JvmStatic + fun moveFile(filename: String, sourceDirUriString: String, destDirUriString: String): Boolean { + try { + val sourceFileUri = ("$sourceDirUriString%2F$filename").toUri() + val sourceDirUri = sourceDirUriString.toUri() + val destDirUri = destDirUriString.toUri() + DocumentsContract.moveDocument(context.contentResolver, sourceFileUri, sourceDirUri, destDirUri) + return true + } catch (e: Exception) { + Log.error("[FileUtil]: Cannot move file, error: " + e.message) + } + return false + } + @JvmStatic fun deleteDocument(path: String): Boolean { try { diff --git a/src/common/android_storage.cpp b/src/common/android_storage.cpp index 8ce8b0084..0dfd98c25 100644 --- a/src/common/android_storage.cpp +++ b/src/common/android_storage.cpp @@ -3,7 +3,11 @@ // Refer to the license.txt file included. #ifdef ANDROID +#include +#include #include "common/android_storage.h" +#include "common/file_util.h" +#include "common/logging/log.h" namespace AndroidStorage { JNIEnv* GetEnvForThread() { @@ -80,8 +84,9 @@ void CleanupJNI() { } bool CreateFile(const std::string& directory, const std::string& filename) { - if (create_file == nullptr) + if (create_file == nullptr) { return false; + } auto env = GetEnvForThread(); jstring j_directory = env->NewStringUTF(directory.c_str()); jstring j_filename = env->NewStringUTF(filename.c_str()); @@ -89,8 +94,9 @@ bool CreateFile(const std::string& directory, const std::string& filename) { } bool CreateDir(const std::string& directory, const std::string& filename) { - if (create_dir == nullptr) + if (create_dir == nullptr) { return false; + } auto env = GetEnvForThread(); jstring j_directory = env->NewStringUTF(directory.c_str()); jstring j_directory_name = env->NewStringUTF(filename.c_str()); @@ -98,8 +104,9 @@ bool CreateDir(const std::string& directory, const std::string& filename) { } int OpenContentUri(const std::string& filepath, AndroidOpenMode openmode) { - if (open_content_uri == nullptr) + if (open_content_uri == nullptr) { return -1; + } const char* mode = ""; switch (openmode) { @@ -135,8 +142,9 @@ int OpenContentUri(const std::string& filepath, AndroidOpenMode openmode) { std::vector GetFilesName(const std::string& filepath) { auto vector = std::vector(); - if (get_files_name == nullptr) + if (get_files_name == nullptr) { return vector; + } auto env = GetEnvForThread(); jstring j_filepath = env->NewStringUTF(filepath.c_str()); auto j_object = @@ -151,9 +159,10 @@ std::vector GetFilesName(const std::string& filepath) { } std::optional GetUserDirectory() { - if (get_user_directory == nullptr) + if (get_user_directory == nullptr) { throw std::runtime_error( "Unable to locate user directory: Function with ID 'get_user_directory' is missing"); + } auto env = GetEnvForThread(); auto j_user_directory = (jstring)(env->CallStaticObjectMethod(native_library, get_user_directory, nullptr)); @@ -165,9 +174,10 @@ std::optional GetUserDirectory() { } std::string GetBuildFlavor() { - if (get_build_flavor == nullptr) + if (get_build_flavor == nullptr) { throw std::runtime_error( "Unable get build flavor: Function with ID 'get_build_flavor' is missing"); + } auto env = GetEnvForThread(); const auto jflavor = (jstring)(env->CallStaticObjectMethod(native_library, get_build_flavor, nullptr)); @@ -176,8 +186,9 @@ std::string GetBuildFlavor() { bool CopyFile(const std::string& source, const std::string& destination_path, const std::string& destination_filename) { - if (copy_file == nullptr) + if (copy_file == nullptr) { return false; + } auto env = GetEnvForThread(); jstring j_source_path = env->NewStringUTF(source.c_str()); jstring j_destination_path = env->NewStringUTF(destination_path.c_str()); @@ -187,8 +198,14 @@ bool CopyFile(const std::string& source, const std::string& destination_path, } bool RenameFile(const std::string& source, const std::string& filename) { - if (rename_file == nullptr) + if (rename_file == nullptr) { return false; + } + if (std::string(FileUtil::GetFilename(source)) == + std::string(FileUtil::GetFilename(filename))) { + // TODO: Should this be treated as a success or failure? + return false; + } auto env = GetEnvForThread(); jstring j_source_path = env->NewStringUTF(source.c_str()); jstring j_destination_path = env->NewStringUTF(filename.c_str()); @@ -197,8 +214,9 @@ bool RenameFile(const std::string& source, const std::string& filename) { } bool UpdateDocumentLocation(const std::string& source_path, const std::string& destination_path) { - if (update_document_location == nullptr) + if (update_document_location == nullptr) { return false; + } auto env = GetEnvForThread(); jstring j_source_path = env->NewStringUTF(source_path.c_str()); jstring j_destination_path = env->NewStringUTF(destination_path.c_str()); @@ -206,6 +224,82 @@ bool UpdateDocumentLocation(const std::string& source_path, const std::string& d j_destination_path); } +bool MoveFile(const std::string& filename, const std::string& source_dir_path, + const std::string& destination_dir_path) { + if (move_file == nullptr) { + return false; + } + if (source_dir_path == destination_dir_path) { + // TODO: Should this be treated as a success or failure? + return false; + } + auto env = GetEnvForThread(); + jstring j_filename = env->NewStringUTF(filename.c_str()); + jstring j_source_dir_path = env->NewStringUTF(source_dir_path.c_str()); + jstring j_destination_dir_path = env->NewStringUTF(destination_dir_path.c_str()); + return env->CallStaticBooleanMethod(native_library, move_file, j_filename, j_source_dir_path, + j_destination_dir_path); +} + +bool MoveAndRenameFile(const std::string& src_full_path, const std::string& dest_full_path) { + if (src_full_path == dest_full_path) { + // TODO: Should this be treated as a success or failure? + return false; + } + const auto src_filename = std::string(FileUtil::GetFilename(src_full_path)); + const auto src_parent_path = std::string(FileUtil::GetParentPath(src_full_path)); + const auto dest_filename = std::string(FileUtil::GetFilename(dest_full_path)); + const auto dest_parent_path = std::string(FileUtil::GetParentPath(dest_full_path)); + bool result; + + const std::string tmp_path = "/tmp"; + AndroidStorage::CreateDir("/", "tmp"); + + // If a simultaneous move and rename are not necessary, use individual methods + // TODO: Uncomment this code for 2123.4 RC to allow stress testing of move + rename process in + // beta + /* + if (src_filename == dest_filename || src_parent_path == dest_parent_path) { + if (src_filename != dest_filename) { + return AndroidStorage::RenameFile(src_full_path, dest_filename); + } else if (src_parent_path != dest_parent_path) { + return AndroidStorage::MoveFile(src_filename, src_parent_path, dest_parent_path); + } + } + */ + + // Step 1: Create directory named after UUID inside /tmp to house the moved file. + // This prevents clashes if files with the same name are moved simultaneously. + const auto uuid = boost::uuids::to_string(boost::uuids::time_generator_v7()()); + const auto allocated_tmp_path = tmp_path + "/" + uuid; + AndroidStorage::CreateDir(tmp_path, uuid); + + // Step 2: Attempt to move to allocated temporary directory. + // If this step fails, skip everything except the cleanup. + result = AndroidStorage::MoveFile(src_filename, src_parent_path, allocated_tmp_path); + if (result == true) { + // Step 3: Rename to desired file name. + if (src_filename != dest_filename) { // TODO: Remove this if statement in 2123.4 RC, keeping + // the RenameFile call + AndroidStorage::RenameFile((allocated_tmp_path + "/" + src_filename), dest_filename); + } + + // Step 4: If a file with the desired name in the destination exists, remove it. + AndroidStorage::DeleteDocument(dest_full_path); + + // Step 5: Attempt to move file to desired location. + // If this step fails, move the file back to where it came from. + result = AndroidStorage::MoveFile(dest_filename, allocated_tmp_path, dest_parent_path); + if (result == false) { + AndroidStorage::MoveAndRenameFile((allocated_tmp_path + "/" + dest_filename), + src_full_path); + } + } + // Step 6: Clean up the allocated temp directory. + AndroidStorage::DeleteDocument(allocated_tmp_path); + return result; +} + #define FR(FunctionName, ReturnValue, JMethodID, Caller, JMethodName, Signature) \ F(FunctionName, ReturnValue, JMethodID, Caller) #define F(FunctionName, ReturnValue, JMethodID, Caller) \ diff --git a/src/common/android_storage.h b/src/common/android_storage.h index 1c23ee06c..a0e54f77c 100644 --- a/src/common/android_storage.h +++ b/src/common/android_storage.h @@ -31,7 +31,11 @@ (const std::string& source_path, const std::string& destination_path), \ update_document_location, "updateDocumentLocation", \ "(Ljava/lang/String;Ljava/lang/String;)Z") \ - V(GetBuildFlavor, std::string, (), get_build_flavor, "getBuildFlavor", "()Ljava/lang/String;") + V(GetBuildFlavor, std::string, (), get_build_flavor, "getBuildFlavor", "()Ljava/lang/String;") \ + V(MoveFile, bool, \ + (const std::string& filename, const std::string& source_dir_path, \ + const std::string& destination_dir_path), \ + move_file, "moveFile", "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Z") #define ANDROID_SINGLE_PATH_DETERMINE_FUNCTIONS(V) \ V(IsDirectory, bool, is_directory, CallStaticBooleanMethod, "isDirectory", \ "(Ljava/lang/String;)Z") \ @@ -51,6 +55,7 @@ ANDROID_STORAGE_FUNCTIONS(FS) #undef F #undef FS #undef FR +bool MoveAndRenameFile(const std::string& src_full_path, const std::string& dest_full_path); // Reference: // https://developer.android.com/reference/android/os/ParcelFileDescriptor#parseMode(java.lang.String) enum class AndroidOpenMode { diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 52418bd2a..2b8f75d58 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -304,30 +304,31 @@ bool DeleteDir(const std::string& filename) { return false; } -bool Rename(const std::string& srcFilename, const std::string& destFilename) { - LOG_TRACE(Common_Filesystem, "{} --> {}", srcFilename, destFilename); +bool Rename(const std::string& srcFullPath, const std::string& destFullPath) { + LOG_TRACE(Common_Filesystem, "{} --> {}", srcPath, destFullPath); #ifdef _WIN32 - if (_wrename(Common::UTF8ToUTF16W(srcFilename).c_str(), - Common::UTF8ToUTF16W(destFilename).c_str()) == 0) + if (_wrename(Common::UTF8ToUTF16W(srcFullPath).c_str(), + Common::UTF8ToUTF16W(destFullPath).c_str()) == 0) return true; #elif ANDROID + // srcFullPath and destFullPath are relative to the user directory if (AndroidStorage::GetBuildFlavor() == "googlePlay") { - if (AndroidStorage::RenameFile(srcFilename, std::string(GetFilename(destFilename)))) + if (AndroidStorage::MoveAndRenameFile(srcFullPath, destFullPath)) return true; } else { std::optional userDirLocation = AndroidStorage::GetUserDirectory(); - if (userDirLocation && rename((*userDirLocation + srcFilename).c_str(), - (*userDirLocation + destFilename).c_str()) == 0) { - AndroidStorage::UpdateDocumentLocation(srcFilename, destFilename); + if (userDirLocation && rename((*userDirLocation + srcFullPath).c_str(), + (*userDirLocation + destFullPath).c_str()) == 0) { + AndroidStorage::UpdateDocumentLocation(srcFullPath, destFullPath); // ^ TODO: This shouldn't fail, but what should we do if it somehow does? return true; } } #else - if (rename(srcFilename.c_str(), destFilename.c_str()) == 0) + if (rename(srcFullPath.c_str(), destFullPath.c_str()) == 0) return true; #endif - LOG_ERROR(Common_Filesystem, "failed {} --> {}: {}", srcFilename, destFilename, + LOG_ERROR(Common_Filesystem, "failed {} --> {}: {}", srcFullPath, destFullPath, GetLastErrorMsg()); return false; } diff --git a/src/common/file_util.h b/src/common/file_util.h index 57a1d67e1..1fa4d1f5d 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -136,13 +136,13 @@ bool Delete(const std::string& filename); // Deletes a directory filename, returns true on success bool DeleteDir(const std::string& filename); -// renames file srcFilename to destFilename, returns true on success -bool Rename(const std::string& srcFilename, const std::string& destFilename); +// Renames file srcFullPath to destFullPath, returns true on success +bool Rename(const std::string& srcFullPath, const std::string& destFullPath); -// copies file srcFilename to destFilename, returns true on success +// Copies file srcFilename to destFilename, returns true on success bool Copy(const std::string& srcFilename, const std::string& destFilename); -// creates an empty file filename, returns true on success +// Creates an empty file filename, returns true on success bool CreateEmptyFile(const std::string& filename); /**