From 967a57f2aa091ee7939a5ba7be825da30b7e117b Mon Sep 17 00:00:00 2001 From: Kevin Li Date: Wed, 24 Jun 2020 18:24:17 +0800 Subject: [PATCH] Not change file source status while changing cache path. (#429) * Not change file source status while changing cache path. * Fix test cases error. * Change default state of FileSource to active. --- MapboxGLAndroidSDK/src/cpp/file_source.cpp | 23 ++++++------------- .../mapbox/mapboxsdk/storage/FileSource.java | 21 +++++++++-------- .../testapp/storage/FileSourceMapTest.kt | 7 +++--- .../storage/FileSourceStandaloneTest.kt | 22 +++++++----------- 4 files changed, 30 insertions(+), 43 deletions(-) diff --git a/MapboxGLAndroidSDK/src/cpp/file_source.cpp b/MapboxGLAndroidSDK/src/cpp/file_source.cpp index f95066546..8af8d6228 100644 --- a/MapboxGLAndroidSDK/src/cpp/file_source.cpp +++ b/MapboxGLAndroidSDK/src/cpp/file_source.cpp @@ -19,7 +19,8 @@ namespace android { // FileSource // -FileSource::FileSource(jni::JNIEnv& _env, const jni::String& accessToken, const jni::String& _cachePath) { +FileSource::FileSource(jni::JNIEnv& _env, const jni::String& accessToken, const jni::String& _cachePath) +:activationCounter( optional(1)) { std::string path = jni::Make(_env, _cachePath); mapbox::sqlite::setTempPath(path); @@ -140,11 +141,6 @@ void FileSource::resume(jni::JNIEnv&) { return; } - if (!activationCounter) { - activationCounter = optional(1) ; - return; - } - activationCounter.value()++; if (activationCounter == 1) { resourceLoader->resume(); @@ -156,19 +152,14 @@ void FileSource::pause(jni::JNIEnv&) { return; } - if (activationCounter) { - activationCounter.value()--; - if (activationCounter == 0) { - resourceLoader->pause(); - } + activationCounter.value()--; + if (activationCounter == 0) { + resourceLoader->pause(); } } -jni::jboolean FileSource::isResumed(jni::JNIEnv&) { - if (activationCounter) { - return (jboolean) (activationCounter > 0); - } - return (jboolean) false; +jni::jboolean FileSource::isResumed(jni::JNIEnv &) { + return (jboolean) (activationCounter > 0); } FileSource* FileSource::getNativePeer(jni::JNIEnv& env, const jni::Object& jFileSource) { diff --git a/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java b/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java index 4d3fc3056..363e38157 100644 --- a/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java +++ b/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/storage/FileSource.java @@ -293,12 +293,7 @@ public class FileSource { 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(getResourcesCachePath(applicationContext))) { + if (path.equals(getResourcesCachePath(applicationContext))) { // no need to change the path callback.onSuccess(path); } else { @@ -326,10 +321,17 @@ public class FileSource { private static void internalSetResourcesCachePath(@NonNull Context context, @NonNull String path, @NonNull final ResourcesCachePathChangeCallback callback) { final FileSource fileSource = getInstance(context); + final boolean active = fileSource.isActivated(); + if (!active) { + fileSource.activate(); + } + fileSource.setResourceCachePath(path, new ResourcesCachePathChangeCallback() { @Override public void onSuccess(@NonNull String path) { - fileSource.deactivate(); + if (!active) { + fileSource.deactivate(); + } resourcesCachePathLoaderLock.lock(); resourcesCachePath = path; resourcesCachePathLoaderLock.unlock(); @@ -338,11 +340,12 @@ public class FileSource { @Override public void onError(@NonNull String message) { - fileSource.deactivate(); + if (!active) { + fileSource.deactivate(); + } callback.onError(message); } }); - fileSource.activate(); } private static boolean isPathWritable(String path) { diff --git a/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt b/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt index b1bae5479..fe4a49b48 100644 --- a/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt +++ b/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceMapTest.kt @@ -39,13 +39,12 @@ open class FileSourceMapTest : AppCenter() { rule.activity.runOnUiThread { FileSource.setResourcesCachePath(fileSourceTestUtils.testPath, object : FileSource.ResourcesCachePathChangeCallback { override fun onSuccess(path: String) { - Assert.fail("Requested resources change while the map is running should fail") + latch.countDown() + Assert.assertEquals(fileSourceTestUtils.testPath, path) } 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() + Assert.fail("Resources path can be changed while the map is running") } }) } diff --git a/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceStandaloneTest.kt b/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceStandaloneTest.kt index a1cbb73ba..0b3d40ae6 100644 --- a/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceStandaloneTest.kt +++ b/MapboxGLAndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/storage/FileSourceStandaloneTest.kt @@ -35,29 +35,31 @@ class FileSourceStandaloneTest : AppCenter() { @Test @UiThreadTest fun testDefault() { - Assert.assertFalse("FileSource should not be active", fileSource.isActivated) + Assert.assertTrue("FileSource should be active", fileSource.isActivated) } @Test @UiThreadTest fun testActivateDeactivate() { - Assert.assertFalse("1) FileSource should not be active", fileSource.isActivated) - fileSource.activate() - Assert.assertTrue("2) FileSource should be active", fileSource.isActivated) + Assert.assertTrue("1) FileSource should be active", fileSource.isActivated) fileSource.deactivate() - Assert.assertFalse("3) FileSource should not be active", fileSource.isActivated) + Assert.assertFalse("2) FileSource should not be active", fileSource.isActivated) + fileSource.activate() + Assert.assertTrue("3) FileSource should be active", fileSource.isActivated) } @Test fun pathChangeTest() { - Assert.assertFalse("FileSource should not be active", fileSource.isActivated) + Assert.assertTrue("FileSource should be active", fileSource.isActivated) Assert.assertEquals(fileSourceTestUtils.originalPath, FileSource.getResourcesCachePath(rule.activity)) fileSourceTestUtils.changePath(fileSourceTestUtils.testPath) Assert.assertEquals(fileSourceTestUtils.testPath, FileSource.getResourcesCachePath(rule.activity)) + Assert.assertTrue("FileSource should be active", fileSource.isActivated) fileSourceTestUtils.changePath(fileSourceTestUtils.originalPath) Assert.assertEquals(fileSourceTestUtils.originalPath, FileSource.getResourcesCachePath(rule.activity)) + Assert.assertTrue("FileSource should be active", fileSource.isActivated) } @Test @@ -93,18 +95,10 @@ class FileSourceStandaloneTest : AppCenter() { } if (!secondLatch.await(5, TimeUnit.SECONDS)) { - rule.runOnUiThread { - // if we fail to call a callback, the file source is not going to be deactivated - fileSource.deactivate() - } Assert.fail("Second attempt should fail.") } if (!firstLatch.await(5, TimeUnit.SECONDS)) { - rule.runOnUiThread { - // if we fail to call a callback, the file source is not going to be deactivated - fileSource.deactivate() - } Assert.fail("First attempt should succeed.") } }