Ported: Address remaining scoped storage regressions

- Accelerate common path used to check the storage scope for a given path
- Update the logic for the `get_as_text()` method - previous logic loads the content of a text file one byte at a time
- m4gr3d
fc2c202312

core/bind/core_bind.cpp

@@ -2090,16 +2090,10 @@ PoolVector<uint8_t> _File::get_buffer(int64_t p_length) const {
 String _File::get_as_text() const {
 	ERR_FAIL_COND_V_MSG(!f, String(), "File must be opened before use.");
 
-	String text;
 	uint64_t original_pos = f->get_position();
 	f->seek(0);
 
-	String l = get_line();
-	while (!eof_reached()) {
-		text += l + "\n";
-		l = get_line();
-	}
-	text += l;
+	String text = f->get_as_utf8_string();
 
 	f->seek(original_pos);
 

platform/android/file_access_filesystem_jandroid.cpp

@@ -166,6 +166,51 @@ uint8_t FileAccessFilesystemJAndroid::get_8() const {
 	return byte;
 }
 
+String FileAccessFilesystemJAndroid::get_line() const {
+	ERR_FAIL_COND_V_MSG(!is_open(), String(), "File must be opened before use.");
+
+	const size_t buffer_size_limit = 2048;
+	const uint64_t file_size = get_len();
+	const uint64_t start_position = get_position();
+
+	String result;
+	LocalVector<uint8_t> line_buffer;
+	size_t current_buffer_size = 0;
+	uint64_t line_buffer_position = 0;
+
+	while (true) {
+		size_t line_buffer_size = MIN(buffer_size_limit, file_size - get_position());
+		if (line_buffer_size <= 0) {
+			break;
+		}
+
+		current_buffer_size += line_buffer_size;
+		line_buffer.resize(current_buffer_size);
+
+		uint64_t bytes_read = get_buffer(&line_buffer[line_buffer_position], current_buffer_size - line_buffer_position);
+		if (bytes_read <= 0) {
+			break;
+		}
+
+		for (; bytes_read > 0; line_buffer_position++, bytes_read--) {
+			uint8_t elem = line_buffer[line_buffer_position];
+			if (elem == '\n' || elem == '\0') {
+				// Found the end of the line
+				const_cast<FileAccessFilesystemJAndroid *>(this)->seek(start_position + line_buffer_position + 1);
+				if (result.parse_utf8((const char *)line_buffer.ptr(), line_buffer_position)) {
+					return String();
+				}
+				return result;
+			}
+		}
+	}
+
+	if (result.parse_utf8((const char *)line_buffer.ptr(), line_buffer_position)) {
+		return String();
+	}
+	return result;
+}
+
 uint64_t FileAccessFilesystemJAndroid::get_buffer(uint8_t *p_dst, uint64_t p_length) const {
 	if (_file_read) {
 		ERR_FAIL_COND_V_MSG(!is_open(), 0, "File must be opened before use.");

platform/android/file_access_filesystem_jandroid.h

@@ -73,6 +73,7 @@ public:
 	virtual bool eof_reached() const override; ///< reading passed EOF
 
 	virtual uint8_t get_8() const override; ///< get a byte
+	virtual String get_line() const override; ///< get a line
 	virtual uint64_t get_buffer(uint8_t *p_dst, uint64_t p_length) const override;
 
 	virtual Error get_error() const override; ///< get last error

platform/android/java/lib/src/net/relintai/pandemonium/pandemonium/io/StorageScope.kt

@@ -54,11 +54,19 @@ internal enum class StorageScope {
 	 */
 	UNKNOWN;
 
-	companion object {
+  class Identifier(context: Context) {
+
+		private val internalAppDir: String? = context.filesDir.canonicalPath
+		private val internalCacheDir: String? = context.cacheDir.canonicalPath
+		private val externalAppDir: String? = context.getExternalFilesDir(null)?.canonicalPath
+		private val sharedDir : String? = Environment.getExternalStorageDirectory().canonicalPath
+		private val downloadsSharedDir: String? = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).canonicalPath
+		private val documentsSharedDir: String? = Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOCUMENTS).canonicalPath
+
 		/**
 		 * Determines which [StorageScope] the given path falls under.
 		 */
-		fun getStorageScope(context: Context, path: String?): StorageScope {
+    fun identifyStorageScope(path: String?): StorageScope {
 			if (path == null) {
 				return UNKNOWN
 			}
@@ -70,23 +78,19 @@ internal enum class StorageScope {
 
 			val canonicalPathFile = pathFile.canonicalPath
 
-			val internalAppDir = context.filesDir.canonicalPath ?: return UNKNOWN
-			if (canonicalPathFile.startsWith(internalAppDir)) {
+      if (internalAppDir != null && canonicalPathFile.startsWith(internalAppDir)) {
 				return APP
 			}
 
-			val internalCacheDir = context.cacheDir.canonicalPath ?: return UNKNOWN
-			if (canonicalPathFile.startsWith(internalCacheDir)) {
+      if (internalCacheDir != null && canonicalPathFile.startsWith(internalCacheDir)) {
 				return APP
 			}
 
-			val externalAppDir = context.getExternalFilesDir(null)?.canonicalPath ?: return UNKNOWN
-			if (canonicalPathFile.startsWith(externalAppDir)) {
+      if (externalAppDir != null && canonicalPathFile.startsWith(externalAppDir)) {
 				return APP
 			}
 
-			val sharedDir =	Environment.getExternalStorageDirectory().canonicalPath ?: return UNKNOWN
-			if (canonicalPathFile.startsWith(sharedDir)) {
+      if (sharedDir != null && canonicalPathFile.startsWith(sharedDir)) {
 				if (Build.VERSION.SDK_INT < Build.VERSION_CODES.R) {
 					// Before R, apps had access to shared storage so long as they have the right
 					// permissions (and flag on Q).
@@ -95,13 +99,8 @@ internal enum class StorageScope {
 
 				// Post R, access is limited based on the target destination
 				// 'Downloads' and 'Documents' are still accessible
-				val downloadsSharedDir =
-					Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS).canonicalPath
-						?: return SHARED
-				val documentsSharedDir =
-					Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOCUMENTS).canonicalPath
-						?: return SHARED
-				if (canonicalPathFile.startsWith(downloadsSharedDir) || canonicalPathFile.startsWith(documentsSharedDir)) {
+        if ((downloadsSharedDir != null && canonicalPathFile.startsWith(downloadsSharedDir))
+					|| (documentsSharedDir != null && canonicalPathFile.startsWith(documentsSharedDir))) {
 					return APP
 				}
 

platform/android/java/lib/src/net/relintai/pandemonium/pandemonium/io/directory/FilesystemDirectoryAccess.kt

@@ -54,6 +54,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
 
 	private data class DirData(val dirFile: File, val files: Array<File>, var current: Int = 0)
 
+  private val storageScopeIdentifier = StorageScope.Identifier(context)
 	private val storageManager = context.getSystemService(Context.STORAGE_SERVICE) as StorageManager
 	private var lastDirId = STARTING_DIR_ID
 	private val dirs = SparseArray<DirData>()
@@ -62,7 +63,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
 		// Directory access is available for shared storage on Android 11+
 		// On Android 10, access is also available as long as the `requestLegacyExternalStorage`
 		// tag is available.
-		return StorageScope.getStorageScope(context, path) != StorageScope.UNKNOWN
+    return storageScopeIdentifier.identifyStorageScope(path) != StorageScope.UNKNOWN
 	}
 
 	override fun hasDirId(dirId: Int) = dirs.indexOfKey(dirId) >= 0
@@ -102,7 +103,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
 		}
 	}
 
-	override fun fileExists(path: String) = FileAccessHandler.fileExists(context, path)
+  override fun fileExists(path: String) = FileAccessHandler.fileExists(context, storageScopeIdentifier, path)
 
 	override fun dirNext(dirId: Int): String {
 		val dirData = dirs[dirId]
@@ -199,7 +200,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
 			if (fromFile.isDirectory) {
 				fromFile.renameTo(File(to))
 			} else {
-				FileAccessHandler.renameFile(context, from, to)
+        FileAccessHandler.renameFile(context, storageScopeIdentifier, from, to)
 			}
 		} catch (e: SecurityException) {
 			false
@@ -218,7 +219,7 @@ internal class FilesystemDirectoryAccess(private val context: Context):
 				if (deleteFile.isDirectory) {
 					deleteFile.delete()
 				} else {
-					FileAccessHandler.removeFile(context, filename)
+          FileAccessHandler.removeFile(context, storageScopeIdentifier, filename)
 				}
 			} else {
 				true

platform/android/java/lib/src/net/relintai/pandemonium/pandemonium/io/file/DataAccess.kt

@@ -161,8 +161,9 @@ internal abstract class DataAccess(private val filePath: String) {
 	fun read(buffer: ByteBuffer): Int {
 		return try {
 			val readBytes = fileChannel.read(buffer)
+      endOfFile = readBytes == -1
+					|| (fileChannel.position() >= fileChannel.size() && fileChannel.size() > 0)
 			if (readBytes == -1) {
-				endOfFile = true
 				0
 			} else {
 				readBytes

platform/android/java/lib/src/net/relintai/pandemonium/pandemonium/io/file/FileAccessHandler.kt

@@ -49,8 +49,8 @@ class FileAccessHandler(val context: Context) {
 		private const val INVALID_FILE_ID = 0
 		private const val STARTING_FILE_ID = 1
 
-		fun fileExists(context: Context, path: String?): Boolean {
-			val storageScope = StorageScope.getStorageScope(context, path)
+    internal fun fileExists(context: Context, storageScopeIdentifier: StorageScope.Identifier, path: String?): Boolean {
+			val storageScope = storageScopeIdentifier.identifyStorageScope(path)
 			if (storageScope == StorageScope.UNKNOWN) {
 				return false
 			}
@@ -62,8 +62,8 @@ class FileAccessHandler(val context: Context) {
 			}
 		}
 
-		fun removeFile(context: Context, path: String?): Boolean {
-			val storageScope = StorageScope.getStorageScope(context, path)
+    internal fun removeFile(context: Context, storageScopeIdentifier: StorageScope.Identifier, path: String?): Boolean {
+			val storageScope = storageScopeIdentifier.identifyStorageScope(path)
 			if (storageScope == StorageScope.UNKNOWN) {
 				return false
 			}
@@ -75,8 +75,8 @@ class FileAccessHandler(val context: Context) {
 			}
 		}
 
-		fun renameFile(context: Context, from: String?, to: String?): Boolean {
-			val storageScope = StorageScope.getStorageScope(context, from)
+    internal fun renameFile(context: Context, storageScopeIdentifier: StorageScope.Identifier, from: String?, to: String?): Boolean {
+			val storageScope = storageScopeIdentifier.identifyStorageScope(from)
 			if (storageScope == StorageScope.UNKNOWN) {
 				return false
 			}
@@ -89,13 +89,14 @@ class FileAccessHandler(val context: Context) {
 		}
 	}
 
+  private val storageScopeIdentifier = StorageScope.Identifier(context)
 	private val files = SparseArray<DataAccess>()
 	private var lastFileId = STARTING_FILE_ID
 
 	private fun hasFileId(fileId: Int) = files.indexOfKey(fileId) >= 0
 
 	fun fileOpen(path: String?, modeFlags: Int): Int {
-		val storageScope = StorageScope.getStorageScope(context, path)
+    val storageScope = storageScopeIdentifier.identifyStorageScope(path)
 		if (storageScope == StorageScope.UNKNOWN) {
 			return INVALID_FILE_ID
 		}
@@ -162,10 +163,10 @@ class FileAccessHandler(val context: Context) {
 		files[fileId].flush()
 	}
 
-	fun fileExists(path: String?) = Companion.fileExists(context, path)
+  fun fileExists(path: String?) = Companion.fileExists(context, storageScopeIdentifier, path)
 
 	fun fileLastModified(filepath: String?): Long {
-		val storageScope = StorageScope.getStorageScope(context, filepath)
+    val storageScope = storageScopeIdentifier.identifyStorageScope(filepath)
 		if (storageScope == StorageScope.UNKNOWN) {
 			return 0L
 		}