Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decouple storage from activity lifecycle #62

Merged
merged 3 commits into from
Jun 28, 2016
Merged

decouple storage from activity lifecycle #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4980956
to allow background data processing in particular push notification p…
dryganets Jun 21, 2016
eea03f6
native plugin is decoupled from activity lifecycle as well
dryganets Jun 21, 2016
1b4227c
backward incompability iliminated, Activity method is marked as depre…
dryganets Jun 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 13 additions & 60 deletions src/android-native/src/main/java/io/liteglue/SQLitePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

package io.liteglue;

import android.app.Activity;
import android.app.Application;
import android.content.Context;
import android.os.Bundle;
import android.util.Log;

Expand Down Expand Up @@ -37,7 +36,7 @@
import java.io.OutputStream;
import java.io.IOException;

public class SQLitePlugin extends ReactContextBaseJavaModule implements Application.ActivityLifecycleCallbacks {
public class SQLitePlugin extends ReactContextBaseJavaModule {

/**
* Multiple database runner map (static).
Expand All @@ -52,14 +51,12 @@ public class SQLitePlugin extends ReactContextBaseJavaModule implements Applicat
static SQLiteConnector connector = new SQLiteConnector();

static String LOG_TAG = SQLitePlugin.class.getSimpleName();

protected Activity activity = null;
protected ExecutorService threadPool;
private Context context;

public SQLitePlugin(ReactApplicationContext reactContext, Activity activity) {
public SQLitePlugin(ReactApplicationContext reactContext) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned, this create unnecessary backward incompatibility. Instead of removing original constructor, I think it will be more appropriate to add a new constructor taking only reactContext. This way both modes of creation are supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no incompatibility as everyone would use Package, there is no need to create Module directly, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, since we have these two constructors at the Package level it should be all right.

super(reactContext);
this.activity = activity;
this.activity.getApplication().registerActivityLifecycleCallbacks(this);
this.context = reactContext.getApplicationContext();
this.threadPool = Executors.newCachedThreadPool();
}

Expand Down Expand Up @@ -147,54 +144,10 @@ protected ExecutorService getThreadPool(){
return this.threadPool;
}

protected Activity getActivity(){
return this.activity;
}

@Override
public void onActivityCreated(Activity activity, Bundle savedInstanceState) {

}

@Override
public void onActivityStarted(Activity activity) {

}

@Override
public void onActivityResumed(Activity activity) {

protected Context getContext(){
return this.context;
}

@Override
public void onActivityPaused(Activity activity) {

}

@Override
public void onActivityStopped(Activity activity) {

}

@Override
public void onActivitySaveInstanceState(Activity activity, Bundle outState) {

}

/**
* If activity matche linked Activity of this plugin, all open databases are closed.
*
* @param activity
*/
@Override
public void onActivityDestroyed(Activity activity) {
Activity myActivity = this.getActivity();
if (activity == myActivity){
myActivity.getApplication().unregisterActivityLifecycleCallbacks(this);
Log.d(LOG_TAG, "linked activity destroyed - closing all open databases");
this.closeAllOpenDatabases();
}
}
/**
* Executes the request and returns PluginResult.
*
Expand Down Expand Up @@ -362,14 +315,14 @@ private SQLiteAndroidDatabase openDatabase(String dbname, String assetFilePath,
if (assetFilePath != null && assetFilePath.length() > 0) {
if (assetFilePath.compareTo("1") == 0) {
assetFilePath = "www/" + dbname;
in = this.getActivity().getAssets().open(assetFilePath);
in = this.getContext().getAssets().open(assetFilePath);
Log.v("info", "Located pre-populated DB asset in app bundle www subdirectory: " + assetFilePath);
} else if (assetFilePath.charAt(0) == '~') {
assetFilePath = assetFilePath.startsWith("~/") ? assetFilePath.substring(2) : assetFilePath.substring(1);
in = this.getActivity().getAssets().open(assetFilePath);
in = this.getContext().getAssets().open(assetFilePath);
Log.v("info", "Located pre-populated DB asset in app bundle subdirectory: " + assetFilePath);
} else {
File filesDir = this.getActivity().getFilesDir();
File filesDir = this.getContext().getFilesDir();
assetFilePath = assetFilePath.startsWith("/") ? assetFilePath.substring(1) : assetFilePath;
File assetFile = new File(filesDir, assetFilePath);
in = new FileInputStream(assetFile);
Expand All @@ -383,7 +336,7 @@ private SQLiteAndroidDatabase openDatabase(String dbname, String assetFilePath,

if (dbfile == null) {
openFlags = SQLiteOpenFlags.CREATE | SQLiteOpenFlags.READWRITE;
dbfile = this.getActivity().getDatabasePath(dbname);
dbfile = this.getContext().getDatabasePath(dbname);

if (!dbfile.exists() && in != null) {
Log.v("info", "Copying pre-populated db asset to destination");
Expand Down Expand Up @@ -543,10 +496,10 @@ private void deleteDatabase(String dbname, CallbackContext cbc) {
* @return true if successful or false if an exception was encountered
*/
private boolean deleteDatabaseNow(String dbname) {
File dbfile = this.getActivity().getDatabasePath(dbname);
File dbfile = this.getContext().getDatabasePath(dbname);

try {
return this.getActivity().deleteDatabase(dbfile.getAbsolutePath());
return this.getContext().deleteDatabase(dbfile.getAbsolutePath());
} catch (Exception e) {
Log.e(LOG_TAG, "couldn't delete database", e);
return false;
Expand Down
17 changes: 10 additions & 7 deletions src/android-native/src/main/java/io/liteglue/SQLitePluginPackage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/
package io.liteglue;


import android.app.Activity;

import com.facebook.react.ReactPackage;
Expand All @@ -19,19 +18,23 @@
import java.util.List;

public class SQLitePluginPackage implements ReactPackage {
private Activity activity = null;

public SQLitePluginPackage(Activity activity){
this.activity = activity;
/**
* @deprecated Please use version without activity parameter
* activity parameter is ignored
*/
public SQLitePluginPackage(Activity activity) {
this();
}

public SQLitePluginPackage(){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same backward incompatibility issue. Keep both constructors.

}

@Override
public List<NativeModule> createNativeModules(
ReactApplicationContext reactContext) {
List<NativeModule> modules = new ArrayList<>();

modules.add(new SQLitePlugin(reactContext, activity));

modules.add(new SQLitePlugin(reactContext));
return modules;
}

Expand Down
76 changes: 14 additions & 62 deletions src/android/src/main/java/org/pgsqlite/SQLitePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
package org.pgsqlite;

import android.annotation.SuppressLint;
import android.app.Activity;
import android.app.Application;
import android.database.Cursor;
import android.database.CursorWindow;
import android.database.sqlite.SQLiteCursor;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteException;
import android.database.sqlite.SQLiteStatement;
import android.content.Context;
import android.os.Bundle;
import android.util.Base64;
import android.util.Log;
Expand Down Expand Up @@ -48,7 +47,7 @@
import java.io.IOException;


public class SQLitePlugin extends ReactContextBaseJavaModule implements Application.ActivityLifecycleCallbacks {
public class SQLitePlugin extends ReactContextBaseJavaModule {

private static final String PLUGIN_NAME = "SQLite";

Expand Down Expand Up @@ -76,62 +75,16 @@ public class SQLitePlugin extends ReactContextBaseJavaModule implements Applicat
/**
* Linked activity
*/
protected Activity activity = null;
protected Context context = null;

/**
* Thread pool for database operations
*/
protected ExecutorService threadPool;

@Override
public void onActivityCreated(Activity activity, Bundle savedInstanceState) {

}

@Override
public void onActivityStarted(Activity activity) {

}

@Override
public void onActivityResumed(Activity activity) {

}

@Override
public void onActivityPaused(Activity activity) {

}

@Override
public void onActivityStopped(Activity activity) {

}

@Override
public void onActivitySaveInstanceState(Activity activity, Bundle outState) {

}

/**
* If activity matche linked Activity of this plugin, all open databases are closed.
*
* @param activity
*/
@Override
public void onActivityDestroyed(Activity activity) {
Activity myActivity = this.getActivity();
if (activity == myActivity){
myActivity.getApplication().unregisterActivityLifecycleCallbacks(this);
Log.d(LOG_TAG, "linked activity destroyed - closing all open databases");
this.closeAllOpenDatabases();
}
}

public SQLitePlugin(ReactApplicationContext reactContext, Activity activity) {
public SQLitePlugin(ReactApplicationContext reactContext) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backward compatibility issue.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the SQLitePluginPackage class from pgsqlite is not included in this PR? Should be change in similar fashion as the liteglue class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no compatibility issue as Package is used by external users.
They are not going to instantiate your module manually, as there is a package to do so, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, looks good.

super(reactContext);
this.activity = activity;
this.activity.getApplication().registerActivityLifecycleCallbacks(this);
this.context = reactContext.getApplicationContext();
this.threadPool = Executors.newCachedThreadPool();
}

Expand Down Expand Up @@ -228,8 +181,8 @@ protected ExecutorService getThreadPool(){
*
* @return linked activity
*/
protected Activity getActivity(){
return this.activity;
protected Context getContext(){
return this.context;
}

/**
Expand Down Expand Up @@ -416,14 +369,14 @@ private SQLiteDatabase openDatabase(String dbname, String assetFilePath, int ope
if (assetFilePath != null && assetFilePath.length() > 0) {
if (assetFilePath.compareTo("1") == 0) {
assetFilePath = "www/" + dbname;
in = this.getActivity().getAssets().open(assetFilePath);
in = this.getContext().getAssets().open(assetFilePath);
Log.v("info", "Located pre-populated DB asset in app bundle www subdirectory: " + assetFilePath);
} else if (assetFilePath.charAt(0) == '~') {
assetFilePath = assetFilePath.startsWith("~/") ? assetFilePath.substring(2) : assetFilePath.substring(1);
in = this.getActivity().getAssets().open(assetFilePath);
in = this.getContext().getAssets().open(assetFilePath);
Log.v("info", "Located pre-populated DB asset in app bundle subdirectory: " + assetFilePath);
} else {
File filesDir = this.getActivity().getFilesDir();
File filesDir = this.getContext().getFilesDir();
assetFilePath = assetFilePath.startsWith("/") ? assetFilePath.substring(1) : assetFilePath;
File assetFile = new File(filesDir, assetFilePath);
in = new FileInputStream(assetFile);
Expand All @@ -437,7 +390,7 @@ private SQLiteDatabase openDatabase(String dbname, String assetFilePath, int ope

if (dbfile == null) {
openFlags = SQLiteDatabase.OPEN_READWRITE | SQLiteDatabase.CREATE_IF_NECESSARY;
dbfile = this.getActivity().getDatabasePath(dbname);
dbfile = this.getContext().getDatabasePath(dbname);

if (!dbfile.exists() && in != null) {
Log.v("info", "Copying pre-populated db asset to destination");
Expand Down Expand Up @@ -479,8 +432,7 @@ private SQLiteDatabase openDatabase(String dbname, String assetFilePath, int ope
* @param dbfile The File of the destination db
* @param assetFileInputStream input file stream for pre-populated db asset
*/
private void createFromAssets(String dbName, File dbfile, InputStream assetFileInputStream)
{
private void createFromAssets(String dbName, File dbfile, InputStream assetFileInputStream) {
OutputStream out = null;

try {
Expand Down Expand Up @@ -587,7 +539,7 @@ private void deleteDatabase(String dbname, CallbackContext cbc) {
*/
@SuppressLint("NewApi")
private boolean deleteDatabaseNow(String dbname) {
File dbfile = this.getActivity().getDatabasePath(dbname);
File dbfile = this.getContext().getDatabasePath(dbname);
if (android.os.Build.VERSION.SDK_INT >= 11) {
// Use try & catch just in case android.os.Build.VERSION.SDK_INT >= 16 was lying:
try {
Expand All @@ -604,7 +556,7 @@ private boolean deleteDatabaseNow(String dbname) {

private boolean deleteDatabasePreHoneycomb(File dbfile) {
try {
return this.getActivity().deleteDatabase(dbfile.getAbsolutePath());
return this.getContext().deleteDatabase(dbfile.getAbsolutePath());
} catch (Exception e) {
Log.e(SQLitePlugin.class.getSimpleName(), "couldn't delete database", e);
return false;
Expand Down
14 changes: 9 additions & 5 deletions src/android/src/main/java/org/pgsqlite/SQLitePluginPackage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
*/
package org.pgsqlite;



import android.app.Activity;

import com.facebook.react.ReactPackage;
Expand All @@ -21,18 +19,24 @@
import java.util.List;

public class SQLitePluginPackage implements ReactPackage {
private Activity activity = null;

/**
* @deprecated, use method without activity
* activity parameter is ignored
*/
public SQLitePluginPackage(Activity activity){
this.activity = activity;
this();
}

public SQLitePluginPackage() {
}

@Override
public List<NativeModule> createNativeModules(
ReactApplicationContext reactContext) {
List<NativeModule> modules = new ArrayList<>();

modules.add(new SQLitePlugin(reactContext, activity));
modules.add(new SQLitePlugin(reactContext));

return modules;
}
Expand Down