Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[android] use strong references to the offline merge callbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
@LukasPaczos
LukasPaczos committed May 8, 2019
1 parent e69efb9 commit 9a6eb2f
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 72 deletions.
121 changes: 51 additions & 70 deletions ...android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

import android.annotation.SuppressLint;
import android.content.Context;
import android.os.AsyncTask;
import android.os.Handler;
import android.os.Looper;
import android.support.annotation.AnyThread;
import android.support.annotation.Keep;
import android.support.annotation.NonNull;
import android.support.annotation.UiThread;

import com.mapbox.mapboxsdk.LibraryLoader;
import com.mapbox.mapboxsdk.Mapbox;
Expand All @@ -21,7 +22,6 @@
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.lang.ref.WeakReference;
import java.nio.channels.FileChannel;

/**
Expand All @@ -32,6 +32,7 @@
*
* @see <a href="https://docs.mapbox.com/help/troubleshooting/mobile-offline/">Offline Maps Information/</a>
*/
@UiThread
public class OfflineManager {

private static final String TAG = "Mbgl - OfflineManager";
Expand Down Expand Up @@ -156,6 +157,7 @@ public static synchronized OfflineManager getInstance(@NonNull Context context)
return instance;
}

@AnyThread
private Handler getHandler() {
if (handler == null) {
handler = new Handler(Looper.getMainLooper());
Expand Down Expand Up @@ -205,6 +207,8 @@ public void run() {
* Merge offline regions from a secondary database into the main offline database.
* <p>
* When the merge is completed, or fails, the {@link MergeOfflineRegionsCallback} will be invoked on the main thread.
* The callback reference is <b>strongly kept</b> throughout the process,
* so it needs to be wrapped in a weak reference or released on the client side if necessary.
* <p>
* The secondary database may need to be upgraded to the latest schema.
* This is done in-place and requires write-access to the provided path.
Expand All @@ -225,73 +229,50 @@ public void run() {
*/
public void mergeOfflineRegions(@NonNull String path, @NonNull final MergeOfflineRegionsCallback callback) {
final File src = new File(path);
new FileUtils.CheckFileReadPermissionTask(new FileUtils.OnCheckFileReadPermissionListener() {
new Thread(new Runnable() {
@Override
public void onReadPermissionGranted() {
new FileUtils.CheckFileWritePermissionTask(new FileUtils.OnCheckFileWritePermissionListener() {
@Override
public void onWritePermissionGranted() {
// path writable, merge and update schema in place if necessary
mergeOfflineDatabaseFiles(src, callback, false);
}

@Override
public void onError() {
// path not writable, copy the the file to temp directory, then merge and update schema on a copy if
// necessary
File dst = new File(FileSource.getInternalCachePath(context), src.getName());
new CopyTempDatabaseFileTask(OfflineManager.this, callback).execute(src, dst);
public void run() {
String errorMessage = null;
if (src.canWrite()) {
getHandler().post(new Runnable() {
@Override
public void run() {
// path writable, merge and update schema in place if necessary
mergeOfflineDatabaseFiles(src, callback, false);
}
});
} else if (src.canRead()) {
// path not writable, copy the the file to temp directory
final File dst = new File(FileSource.getInternalCachePath(context), src.getName());
try {
copyTempDatabaseFile(src, dst);
getHandler().post(new Runnable() {
@Override
public void run() {
// merge and update schema using the copy
OfflineManager.this.mergeOfflineDatabaseFiles(dst, callback, true);
}
});
} catch (IOException ex) {
ex.printStackTrace();
errorMessage = ex.getMessage();
}
}).execute(src);
}

@Override
public void onError() {
// path not readable, abort
callback.onError("Secondary database needs to be located in a readable path.");
}
}).execute(src);
}

private static final class CopyTempDatabaseFileTask extends AsyncTask<Object, Void, Object> {
@NonNull
private final WeakReference<OfflineManager> offlineManagerWeakReference;
@NonNull
private final WeakReference<MergeOfflineRegionsCallback> callbackWeakReference;

CopyTempDatabaseFileTask(OfflineManager offlineManager, MergeOfflineRegionsCallback callback) {
this.offlineManagerWeakReference = new WeakReference<>(offlineManager);
this.callbackWeakReference = new WeakReference<>(callback);
}

@Override
protected Object doInBackground(Object... objects) {
File src = (File) objects[0];
File dst = (File) objects[1];

try {
copyTempDatabaseFile(src, dst);
return dst;
} catch (IOException ex) {
return ex.getMessage();
}
}
} else {
// path not readable, abort
errorMessage = "Secondary database needs to be located in a readable path.";
}

@Override
protected void onPostExecute(Object object) {
MergeOfflineRegionsCallback callback = callbackWeakReference.get();
if (callback != null) {
OfflineManager offlineManager = offlineManagerWeakReference.get();
if (object instanceof File && offlineManager != null) {
// successfully copied the file, perform merge
File dst = (File) object;
offlineManager.mergeOfflineDatabaseFiles(dst, callback, true);
} else if (object instanceof String) {
// error occurred
callback.onError((String) object);
if (errorMessage != null) {
final String finalErrorMessage = errorMessage;
getHandler().post(new Runnable() {
@Override
public void run() {
callback.onError(finalErrorMessage);
}
});
}
}
}
}).start();
}

private static void copyTempDatabaseFile(@NonNull File sourceFile, File destFile) throws IOException {
Expand Down Expand Up @@ -324,27 +305,27 @@ private void mergeOfflineDatabaseFiles(@NonNull final File file, @NonNull final
mergeOfflineRegions(fileSource, file.getAbsolutePath(), new MergeOfflineRegionsCallback() {
@Override
public void onMerge(final OfflineRegion[] offlineRegions) {
if (isTemporaryFile) {
file.delete();
}
getHandler().post(new Runnable() {
@Override
public void run() {
fileSource.deactivate();
if (isTemporaryFile) {
file.delete();
}
callback.onMerge(offlineRegions);
}
});
}

@Override
public void onError(final String error) {
if (isTemporaryFile) {
file.delete();
}
getHandler().post(new Runnable() {
@Override
public void run() {
fileSource.deactivate();
if (isTemporaryFile) {
file.delete();
}
callback.onError(error);
}
});
Expand Down
26 changes: 24 additions & 2 deletions ...rc/main/java/com/mapbox/mapboxsdk/testapp/activity/offline/MergeOfflineRegionsActivity.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class MergeOfflineRegionsActivity : AppCompatActivity() {
override fun onFileCopiedFromAssets() {
Toast.makeText(
this@MergeOfflineRegionsActivity,
String.format("OnFileCOpied."),
String.format("OnFileCopied."),
Toast.LENGTH_LONG).show()
mergeDb()
}
Expand Down Expand Up @@ -57,6 +57,27 @@ class MergeOfflineRegionsActivity : AppCompatActivity() {
}
}

/**
* Since we expect from the results of the offline merge callback to interact with the hosting activity,
* we need to ensure that we are not interacting with a destroyed activity.
*/
private class MergeCallback(private var activityCallback: OfflineManager.MergeOfflineRegionsCallback?) : OfflineManager.MergeOfflineRegionsCallback {

override fun onMerge(offlineRegions: Array<out OfflineRegion>?) {
activityCallback?.onMerge(offlineRegions)
}

override fun onError(error: String?) {
activityCallback?.onError(error)
}

fun onActivityDestroy() {
activityCallback = null
}
}

private val mergeCallback = MergeCallback(onRegionMergedListener)

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(R.layout.activity_merge_offline_regions)
Expand All @@ -81,7 +102,7 @@ class MergeOfflineRegionsActivity : AppCompatActivity() {

private fun mergeDb() {
OfflineManager.getInstance(this).mergeOfflineRegions(
FileSource.getResourcesCachePath(this) + "/" + TEST_DB_FILE_NAME, onRegionMergedListener
FileSource.getResourcesCachePath(this) + "/" + TEST_DB_FILE_NAME, mergeCallback
)
}

Expand Down Expand Up @@ -112,6 +133,7 @@ class MergeOfflineRegionsActivity : AppCompatActivity() {

override fun onDestroy() {
super.onDestroy()
mergeCallback.onActivityDestroy()
mapView.onDestroy()

// restoring connectivity state
Expand Down

0 comments on commit 9a6eb2f

Please sign in to comment.