From 136d2a5bbf44eb83e9c54c72a8f04b072fb31317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Paczos?= Date: Thu, 9 May 2019 15:06:43 +0200 Subject: [android] keep strong callback reference and use application context during cache path change (cherry picked from commit 0f599ce8048b3428ebff8f59c32f4908d44fabc9) --- .../com/mapbox/mapboxsdk/storage/FileSource.java | 83 +++++++++++----------- .../mapboxsdk/testapp/storage/FileSourceMapTest.kt | 6 +- .../testapp/storage/FileSourceTestUtils.kt | 5 +- .../offline/ChangeResourcesCachePathActivity.kt | 28 +++++++- 4 files changed, 71 insertions(+), 51 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java index cc4988b549..1301c103f4 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java @@ -20,7 +20,6 @@ import com.mapbox.mapboxsdk.utils.FileUtils; import com.mapbox.mapboxsdk.utils.ThreadUtils; import java.io.File; -import java.lang.ref.WeakReference; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -69,14 +68,14 @@ public class FileSource { * * @param path the path of the current resources cache database */ - void onSuccess(String path); + void onSuccess(@NonNull String path); /** * Receives an error message if setting the path was not successful * * @param message the error message */ - void onError(String message); + void onError(@NonNull String message); } @@ -265,63 +264,63 @@ public class FileSource { /** * Changes the path of the resources cache database. * Note that the external storage setting needs to be activated in the manifest. + *

+ * The callback reference is strongly kept throughout the process, + * so it needs to be wrapped in a weak reference or released on the client side if necessary. * * @param context the context of the path * @param path the new database path * @param callback the callback to obtain the result + * @deprecated Use {@link #setResourcesCachePath(String, ResourcesCachePathChangeCallback)} */ - public static void setResourcesCachePath(@NonNull Context context, + @Deprecated + public static void setResourcesCachePath(@NonNull final Context context, @NonNull final String path, - @NonNull ResourcesCachePathChangeCallback callback) { - final String fileSourceActivatedMessage = "Cannot set path, file source is activated." - + " Make sure that the map or a resources download is not running."; - if (getInstance(context).isActivated()) { + @NonNull final ResourcesCachePathChangeCallback callback) { + setResourcesCachePath(path, callback); + } + + /** + * Changes the path of the resources cache database. + * Note that the external storage setting needs to be activated in the manifest. + *

+ * The callback reference is strongly kept throughout the process, + * so it needs to be wrapped in a weak reference or released on the client side if necessary. + * + * @param path the new database path + * @param callback the callback to obtain the result + */ + public static void setResourcesCachePath(@NonNull final String path, + @NonNull final ResourcesCachePathChangeCallback callback) { + final Context applicationContext = Mapbox.getApplicationContext(); + final FileSource fileSource = FileSource.getInstance(applicationContext); + + if (fileSource.isActivated()) { + String fileSourceActivatedMessage = "Cannot set path, file source is activated." + + " Make sure that the map or a resources download is not running."; Logger.w(TAG, fileSourceActivatedMessage); callback.onError(fileSourceActivatedMessage); - } else if (path.equals(resourcesCachePath)) { + } else if (path.equals(getResourcesCachePath(applicationContext))) { // no need to change the path callback.onSuccess(path); } else { - final WeakReference contextWeakReference = new WeakReference<>(context); - final WeakReference callbackWeakReference = new WeakReference<>(callback); new FileUtils.CheckFileWritePermissionTask(new FileUtils.OnCheckFileWritePermissionListener() { @Override public void onWritePermissionGranted() { - final Context context = contextWeakReference.get(); - final ResourcesCachePathChangeCallback callback = callbackWeakReference.get(); - - if (callback == null) { - Logger.w(TAG, "Lost callback reference."); - return; - } else if (context == null) { - String lostContextMessage = "Lost context reference."; - Logger.w(TAG, lostContextMessage); - callback.onError(lostContextMessage); - return; - } - - // verify fileSource's activation again after the async task returns - if (getInstance(context).isActivated()) { - Logger.w(TAG, fileSourceActivatedMessage); - callback.onError(fileSourceActivatedMessage); - } else { - final SharedPreferences.Editor editor = - context.getSharedPreferences(MapboxConstants.MAPBOX_SHARED_PREFERENCES, Context.MODE_PRIVATE).edit(); - editor.putString(MAPBOX_SHARED_PREFERENCE_RESOURCES_CACHE_PATH, path); - editor.apply(); - setResourcesCachePath(context, path); - callback.onSuccess(path); - } + final SharedPreferences.Editor editor = + applicationContext.getSharedPreferences(MapboxConstants.MAPBOX_SHARED_PREFERENCES, + Context.MODE_PRIVATE).edit(); + editor.putString(MAPBOX_SHARED_PREFERENCE_RESOURCES_CACHE_PATH, path); + editor.apply(); + setResourcesCachePath(applicationContext, path); + callback.onSuccess(path); } @Override public void onError() { - final ResourcesCachePathChangeCallback callback = callbackWeakReference.get(); - if (callback != null) { - String message = "Path is not writable: " + path; - Logger.e(TAG, message); - callback.onError(message); - } + String message = "Path is not writable: " + path; + Logger.e(TAG, message); + callback.onError(message); } }).execute(new File(path)); } diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt index 0d4823a93d..5e3489d755 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt @@ -36,12 +36,12 @@ open class FileSourceMapTest { fun changeResourcesPathWhileMapVisible() { val latch = CountDownLatch(1) rule.activity.runOnUiThread { - FileSource.setResourcesCachePath(rule.activity, fileSourceTestUtils.testPath, object : FileSource.ResourcesCachePathChangeCallback { - override fun onSuccess(path: String?) { + FileSource.setResourcesCachePath(fileSourceTestUtils.testPath, object : FileSource.ResourcesCachePathChangeCallback { + override fun onSuccess(path: String) { Assert.fail("Requested resources change while the map is running should fail") } - override fun onError(message: String?) { + override fun onError(message: String) { Assert.assertEquals("Cannot set path, file source is activated." + " Make sure that the map or a resources download is not running.", message) latch.countDown() diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceTestUtils.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceTestUtils.kt index 040b288aa2..c69321581a 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceTestUtils.kt +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceTestUtils.kt @@ -33,14 +33,13 @@ class FileSourceTestUtils(private val activity: Activity) { val latch = CountDownLatch(1) activity.runOnUiThread { FileSource.setResourcesCachePath( - activity, path, object : FileSource.ResourcesCachePathChangeCallback { - override fun onSuccess(path: String?) { + override fun onSuccess(path: String) { latch.countDown() } - override fun onError(message: String?) { + override fun onError(message: String) { Assert.fail("Resource path change failed - path: $path, message: $message") } }) diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/ChangeResourcesCachePathActivity.kt b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/ChangeResourcesCachePathActivity.kt index 75ea9f6970..74b5bde17d 100644 --- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/ChangeResourcesCachePathActivity.kt +++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/ChangeResourcesCachePathActivity.kt @@ -32,6 +32,8 @@ class ChangeResourcesCachePathActivity : AppCompatActivity(), private lateinit var offlineManager: OfflineManager + private val callback = PathChangeCallback(this) + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.activity_change_resources_cache_path) @@ -50,18 +52,23 @@ class ChangeResourcesCachePathActivity : AppCompatActivity(), Toast.makeText(this, "Current path: $path", Toast.LENGTH_LONG).show() } + override fun onDestroy() { + super.onDestroy() + callback.onActivityDestroy() + } + override fun onItemClick(parent: AdapterView<*>?, view: View?, position: Int, id: Long) { listView.onItemClickListener = null val path: String = adapter.getItem(position) as String - FileSource.setResourcesCachePath(this, path, this) + FileSource.setResourcesCachePath(path, callback) } - override fun onError(message: String?) { + override fun onError(message: String) { listView.onItemClickListener = this Toast.makeText(this, "Error: $message", Toast.LENGTH_LONG).show() } - override fun onSuccess(path: String?) { + override fun onSuccess(path: String) { listView.onItemClickListener = this Toast.makeText(this, "New path: $path", Toast.LENGTH_LONG).show() @@ -118,6 +125,21 @@ class ChangeResourcesCachePathActivity : AppCompatActivity(), return paths } + private class PathChangeCallback(private var activity: ChangeResourcesCachePathActivity?) : FileSource.ResourcesCachePathChangeCallback { + + override fun onSuccess(path: String) { + activity?.onSuccess(path) + } + + override fun onError(message: String) { + activity?.onError(message) + } + + fun onActivityDestroy() { + activity = null + } + } + class PathAdapter(private val context: Context, private val paths: List) : BaseAdapter() { override fun getItem(position: Int): Any { -- cgit v1.2.1