Merge pull request #113878 from m4gr3d/fix_obb_dir_access

Fix storage scope for the obb directory

platform/android/java/app/src/androidTestInstrumented/java/com/godot/game/GodotAppTest.kt

@@ -60,6 +60,11 @@ class GodotAppTest {
 		private val TEST_COMMAND_LINE_PARAMS = arrayOf("This is a test")
 	}
 
+	private fun getTestPlugin(): GodotAppInstrumentedTestPlugin? {
+		return GodotPluginRegistry.getPluginRegistry()
+			.getPlugin("GodotAppInstrumentedTestPlugin") as GodotAppInstrumentedTestPlugin?
+	}
+
 	/**
 	 * Runs the JavaClassWrapper tests via the GodotAppInstrumentedTestPlugin.
 	 */
@@ -67,8 +72,7 @@ class GodotAppTest {
 	fun runJavaClassWrapperTests() {
 		ActivityScenario.launch(GodotApp::class.java).use { scenario ->
 			scenario.onActivity { activity ->
-				val testPlugin = GodotPluginRegistry.getPluginRegistry()
+				val testPlugin = getTestPlugin()
-					.getPlugin("GodotAppInstrumentedTestPlugin") as GodotAppInstrumentedTestPlugin?
 				assertNotNull(testPlugin)
 
 				Log.d(TAG, "Waiting for the Godot main loop to start...")
@@ -84,6 +88,28 @@ class GodotAppTest {
 		}
 	}
 
+	/**
+	 * Runs file access related tests.
+	 */
+	@Test
+	fun runFileAccessTests() {
+		ActivityScenario.launch(GodotApp::class.java).use { scenario ->
+			scenario.onActivity { activity ->
+				val testPlugin = getTestPlugin()
+				assertNotNull(testPlugin)
+
+				Log.d(TAG, "Waiting for the Godot main loop to start...")
+				testPlugin.waitForGodotMainLoopStarted()
+
+				Log.d(TAG, "Running FileAccess tests...")
+				val result = testPlugin.runFileAccessTests()
+				assertNotNull(result)
+				result.exceptionOrNull()?.let { throw it }
+				assertTrue(result.isSuccess)
+			}
+		}
+	}
+
 	/**
 	 * Test implicit launch of the Godot app, and validates this resolves to the `GodotAppLauncher` activity alias.
 	 */

platform/android/java/app/src/instrumented/assets/.godot/global_script_class_cache.cfg

@@ -8,6 +8,14 @@ list=[{
 "path": "res://test/base_test.gd"
 }, {
 "base": &"BaseTest",
+"class": &"FileAccessTests",
+"icon": "",
+"is_abstract": false,
+"is_tool": false,
+"language": &"GDScript",
+"path": "res://test/file_access/file_access_tests.gd"
+}, {
+"base": &"BaseTest",
 "class": &"JavaClassWrapperTests",
 "icon": "",
 "is_abstract": false,

platform/android/java/app/src/instrumented/assets/main.gd

@@ -16,6 +16,8 @@ func _launch_tests(test_label: String) -> void:
 	match test_label:
 		"javaclasswrapper_tests":
 			test_instance = JavaClassWrapperTests.new()
+		"file_access_tests":
+			test_instance = FileAccessTests.new()
 
 	if test_instance:
 		test_instance.__reset_tests()

platform/android/java/app/src/instrumented/assets/test/file_access/file_access_tests.gd

@@ -0,0 +1,73 @@
+class_name FileAccessTests
+extends BaseTest
+
+const FILE_CONTENT = "This is a test for reading / writing to the "
+
+func run_tests():
+	print("FileAccess tests starting...")
+	__exec_test(test_obb_dir_access)
+	__exec_test(test_internal_app_dir_access)
+	__exec_test(test_internal_cache_dir_access)
+	__exec_test(test_external_app_dir_access)
+	__exec_test(test_downloads_dir_access)
+	__exec_test(test_documents_dir_access)
+
+func _test_dir_access(dir_path: String, data_file_content: String) -> void:
+	print("Testing access to " + dir_path)
+	var data_file_path = dir_path.path_join("data.dat")
+
+	var data_file = FileAccess.open(data_file_path, FileAccess.WRITE)
+	assert_true(data_file != null)
+	assert_true(data_file.store_string(data_file_content))
+	data_file.close()
+
+	data_file = FileAccess.open(data_file_path, FileAccess.READ)
+	assert_true(data_file != null)
+	var file_content = data_file.get_as_text()
+	assert_equal(file_content, data_file_content)
+	data_file.close()
+
+	var deletion_result = DirAccess.remove_absolute(data_file_path)
+	assert_equal(deletion_result, OK)
+
+func test_obb_dir_access() -> void:
+	var android_runtime = Engine.get_singleton("AndroidRuntime")
+	assert_true(android_runtime != null)
+
+	var app_context = android_runtime.getApplicationContext()
+	var obb_dir: String = app_context.getObbDir().getCanonicalPath()
+	_test_dir_access(obb_dir, FILE_CONTENT + "obb dir.")
+
+func test_internal_app_dir_access() -> void:
+	var android_runtime = Engine.get_singleton("AndroidRuntime")
+	assert_true(android_runtime != null)
+
+	var app_context = android_runtime.getApplicationContext()
+	var internal_app_dir: String = app_context.getFilesDir().getCanonicalPath()
+	_test_dir_access(internal_app_dir, FILE_CONTENT + "internal app dir.")
+
+func test_internal_cache_dir_access() -> void:
+	var android_runtime = Engine.get_singleton("AndroidRuntime")
+	assert_true(android_runtime != null)
+
+	var app_context = android_runtime.getApplicationContext()
+	var internal_cache_dir: String = app_context.getCacheDir().getCanonicalPath()
+	_test_dir_access(internal_cache_dir, FILE_CONTENT + "internal cache dir.")
+
+func test_external_app_dir_access() -> void:
+	var android_runtime = Engine.get_singleton("AndroidRuntime")
+	assert_true(android_runtime != null)
+
+	var app_context = android_runtime.getApplicationContext()
+	var external_app_dir: String = app_context.getExternalFilesDir("").getCanonicalPath()
+	_test_dir_access(external_app_dir, FILE_CONTENT + "external app dir.")
+
+func test_downloads_dir_access() -> void:
+	var EnvironmentClass = JavaClassWrapper.wrap("android.os.Environment")
+	var downloads_dir = EnvironmentClass.getExternalStoragePublicDirectory(EnvironmentClass.DIRECTORY_DOWNLOADS).getCanonicalPath()
+	_test_dir_access(downloads_dir, FILE_CONTENT + "downloads dir.")
+
+func test_documents_dir_access() -> void:
+	var EnvironmentClass = JavaClassWrapper.wrap("android.os.Environment")
+	var documents_dir = EnvironmentClass.getExternalStoragePublicDirectory(EnvironmentClass.DIRECTORY_DOCUMENTS).getCanonicalPath()
+	_test_dir_access(documents_dir, FILE_CONTENT + "documents dir.")

platform/android/java/app/src/instrumented/assets/test/file_access/file_access_tests.gd.uid

@@ -0,0 +1 @@
+uid://b1o8wj1s4vghn

platform/android/java/app/src/instrumented/java/com/godot/game/test/GodotAppInstrumentedTestPlugin.kt

@@ -49,6 +49,7 @@ class GodotAppInstrumentedTestPlugin(godot: Godot) : GodotPlugin(godot) {
 		private const val MAIN_LOOP_STARTED_LATCH_KEY = "main_loop_started_latch"
 
 		private const val JAVACLASSWRAPPER_TESTS = "javaclasswrapper_tests"
+		private const val FILE_ACCESS_TESTS = "file_access_tests"
 
 		private val LAUNCH_TESTS_SIGNAL = SignalInfo("launch_tests", String::class.java)
 
@@ -94,6 +95,13 @@ class GodotAppInstrumentedTestPlugin(godot: Godot) : GodotPlugin(godot) {
 		return launchTests(JAVACLASSWRAPPER_TESTS)
 	}
 
+	/**
+	 * Launches the FileAccess tests, and wait until the tests are complete before returning.
+	 */
+	internal fun runFileAccessTests(): Result<Any>? {
+		return launchTests(FILE_ACCESS_TESTS)
+	}
+
 	private fun launchTests(testLabel: String): Result<Any>? {
 		val latch = latches.getOrPut(testLabel) { CountDownLatch(1) }
 		emitSignal(LAUNCH_TESTS_SIGNAL.name, testLabel)

platform/android/java/lib/src/main/java/org/godotengine/godot/io/StorageScope.kt

@@ -77,6 +77,7 @@ internal enum class StorageScope {
 		private val internalAppDir: String? = context.filesDir.canonicalPath
 		private val internalCacheDir: String? = context.cacheDir.canonicalPath
 		private val externalAppDir: String? = context.getExternalFilesDir(null)?.canonicalPath
+		private val obbDir: String? = context.obbDir.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
@@ -140,6 +141,10 @@ internal enum class StorageScope {
 				return APP
 			}
 
+			if (obbDir != null && canonicalPathFile.startsWith(obbDir)) {
+				return APP
+			}
+
 			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

platform/android/java_class_wrapper.cpp

@@ -1657,7 +1657,7 @@ Ref<JavaClass> JavaClassWrapper::_wrap(const String &p_class, bool p_allow_non_p
 		String str_field = jstring_to_string(name, env);
 		env->DeleteLocalRef(name);
 		int mods = env->CallIntMethod(obj, Field_getModifiers);
-		if ((mods & 0x8) && (mods & 0x10) && (mods & 0x1)) { //static final public!
+		if ((mods & 0x8) && (mods & 0x1)) { //static public!
 
 			jobject objc = env->CallObjectMethod(obj, Field_get, nullptr);
 			if (objc) {