From 0ae8e20342b0a9cbcfa383cc472a3d4c89c949ce Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Wed, 25 Jul 2018 16:42:20 +0800 Subject: [PATCH 1/6] make calls to AsyncStorageModule serial --- .../modules/storage/AsyncStorageModule.java | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java index c9fc96e575f169..9c74269dc2fd79 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java @@ -1,4 +1,4 @@ -/** +/* * Copyright (c) 2015-present, Facebook, Inc. * * This source code is licensed under the MIT license found in the @@ -8,6 +8,8 @@ package com.facebook.react.modules.storage; import java.util.HashSet; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; import android.database.Cursor; import android.database.sqlite.SQLiteStatement; @@ -15,7 +17,6 @@ import com.facebook.common.logging.FLog; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; -import com.facebook.react.bridge.GuardedAsyncTask; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; @@ -42,6 +43,7 @@ public final class AsyncStorageModule private ReactDatabaseSupplier mReactDatabaseSupplier; private boolean mShuttingDown = false; + private Executor executor = Executors.newSingleThreadExecutor(); public AsyncStorageModule(ReactApplicationContext reactContext) { super(reactContext); @@ -83,9 +85,9 @@ public void multiGet(final ReadableArray keys, final Callback callback) { return; } - new GuardedAsyncTask(getReactApplicationContext()) { + execute(new Runnable() { @Override - protected void doInBackgroundGuarded(Void... params) { + public void run() { if (!ensureDatabase()) { callback.invoke(AsyncStorageErrorUtil.getDBError(null), null); return; @@ -141,7 +143,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(null, data); } - }.execute(); + }); } /** @@ -156,9 +158,9 @@ public void multiSet(final ReadableArray keyValueArray, final Callback callback) return; } - new GuardedAsyncTask(getReactApplicationContext()) { + execute(new Runnable() { @Override - protected void doInBackgroundGuarded(Void... params) { + public void run() { if (!ensureDatabase()) { callback.invoke(AsyncStorageErrorUtil.getDBError(null)); return; @@ -208,7 +210,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(); } } - }.execute(); + }); } /** @@ -221,9 +223,9 @@ public void multiRemove(final ReadableArray keys, final Callback callback) { return; } - new GuardedAsyncTask(getReactApplicationContext()) { + execute(new Runnable() { @Override - protected void doInBackgroundGuarded(Void... params) { + public void run() { if (!ensureDatabase()) { callback.invoke(AsyncStorageErrorUtil.getDBError(null)); return; @@ -259,7 +261,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(); } } - }.execute(); + }); } /** @@ -268,9 +270,9 @@ protected void doInBackgroundGuarded(Void... params) { */ @ReactMethod public void multiMerge(final ReadableArray keyValueArray, final Callback callback) { - new GuardedAsyncTask(getReactApplicationContext()) { + execute(new Runnable() { @Override - protected void doInBackgroundGuarded(Void... params) { + public void run() { if (!ensureDatabase()) { callback.invoke(AsyncStorageErrorUtil.getDBError(null)); return; @@ -322,7 +324,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(); } } - }.execute(); + }); } /** @@ -330,9 +332,9 @@ protected void doInBackgroundGuarded(Void... params) { */ @ReactMethod public void clear(final Callback callback) { - new GuardedAsyncTask(getReactApplicationContext()) { + execute(new Runnable() { @Override - protected void doInBackgroundGuarded(Void... params) { + public void run() { if (!mReactDatabaseSupplier.ensureDatabase()) { callback.invoke(AsyncStorageErrorUtil.getDBError(null)); return; @@ -345,7 +347,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(AsyncStorageErrorUtil.getError(null, e.getMessage())); } } - }.execute(); + }); } /** @@ -353,9 +355,9 @@ protected void doInBackgroundGuarded(Void... params) { */ @ReactMethod public void getAllKeys(final Callback callback) { - new GuardedAsyncTask(getReactApplicationContext()) { + execute(new Runnable() { @Override - protected void doInBackgroundGuarded(Void... params) { + public void run() { if (!ensureDatabase()) { callback.invoke(AsyncStorageErrorUtil.getDBError(null), null); return; @@ -379,7 +381,7 @@ protected void doInBackgroundGuarded(Void... params) { } callback.invoke(null, data); } - }.execute(); + }); } /** @@ -388,4 +390,8 @@ protected void doInBackgroundGuarded(Void... params) { private boolean ensureDatabase() { return !mShuttingDown && mReactDatabaseSupplier.ensureDatabase(); } + + private void execute(Runnable r) { + executor.execute(r); + } } From 0227e955cc2eb79aa9e8d6fc93244d43ede81f29 Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Mon, 30 Jul 2018 08:46:45 +0800 Subject: [PATCH 2/6] shutdown executor on onCatalystInstanceDestroy, handle RuntimeException --- .../react/modules/storage/AsyncStorageModule.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java index 9c74269dc2fd79..6aff698720c955 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java @@ -8,7 +8,7 @@ package com.facebook.react.modules.storage; import java.util.HashSet; -import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import android.database.Cursor; @@ -43,7 +43,7 @@ public final class AsyncStorageModule private ReactDatabaseSupplier mReactDatabaseSupplier; private boolean mShuttingDown = false; - private Executor executor = Executors.newSingleThreadExecutor(); + private ExecutorService executor = Executors.newSingleThreadExecutor(); public AsyncStorageModule(ReactApplicationContext reactContext) { super(reactContext); @@ -63,6 +63,7 @@ public void initialize() { @Override public void onCatalystInstanceDestroy() { + executor.shutdown(); mShuttingDown = true; } @@ -392,6 +393,10 @@ private boolean ensureDatabase() { } private void execute(Runnable r) { - executor.execute(r); + try { + executor.execute(r); + } catch (RuntimeException e) { + getReactApplicationContext().handleException(e); + } } } From 038432322d7d48636ea40242e35a8b4feb7e32c5 Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Mon, 30 Jul 2018 19:45:57 +0800 Subject: [PATCH 3/6] AsyncStorageModuleTest works --- .../react/modules/storage/AsyncStorageModule.java | 14 +++++++++++--- .../modules/storage/AsyncStorageModuleTest.java | 13 ++----------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java index 6aff698720c955..2cae7c7dcae056 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java @@ -15,6 +15,7 @@ import android.database.sqlite.SQLiteStatement; import com.facebook.common.logging.FLog; +import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.ReactApplicationContext; @@ -43,11 +44,17 @@ public final class AsyncStorageModule private ReactDatabaseSupplier mReactDatabaseSupplier; private boolean mShuttingDown = false; - private ExecutorService executor = Executors.newSingleThreadExecutor(); + private ExecutorService mExecutor; public AsyncStorageModule(ReactApplicationContext reactContext) { super(reactContext); mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext); + mExecutor = Executors.newSingleThreadExecutor(); + } + + public AsyncStorageModule(ReactApplicationContext reactContext, ExecutorService executor) { + this(reactContext); + mExecutor = executor; } @Override @@ -63,7 +70,7 @@ public void initialize() { @Override public void onCatalystInstanceDestroy() { - executor.shutdown(); + mExecutor.shutdown(); mShuttingDown = true; } @@ -393,8 +400,9 @@ private boolean ensureDatabase() { } private void execute(Runnable r) { + Assertions.assertNotNull(mExecutor); try { - executor.execute(r); + mExecutor.execute(r); } catch (RuntimeException e) { getReactApplicationContext().handleException(e); } diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java index 472f9678f79008..1b01d4c0b5a316 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java @@ -11,18 +11,11 @@ import java.util.HashMap; import java.util.Map; -import android.app.Activity; -import android.content.Context; -import android.content.ContextWrapper; - import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; -import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactTestHelper; import com.facebook.react.bridge.JavaOnlyArray; import com.facebook.react.bridge.JavaOnlyMap; -import com.facebook.react.modules.storage.AsyncStorageModule; -import com.facebook.react.modules.storage.ReactDatabaseSupplier; import org.json.JSONArray; import org.json.JSONObject; @@ -40,13 +33,11 @@ import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.modules.junit4.rule.PowerMockRule; import org.robolectric.RuntimeEnvironment; -import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; -import org.robolectric.annotation.Config; +import org.robolectric.util.concurrent.RoboExecutorService; import static org.mockito.Mockito.mock; import static org.fest.assertions.api.Assertions.assertThat; - /** * Tests for {@link AsyncStorageModule}. */ @@ -81,7 +72,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { }); // don't use Robolectric before initializing mocks - mStorage = new AsyncStorageModule(ReactTestHelper.createCatalystContextForTest()); + mStorage = new AsyncStorageModule(ReactTestHelper.createCatalystContextForTest(), new RoboExecutorService()); mEmptyArray = new JavaOnlyArray(); } From b41c95ab6eb7e24ca93a4e4efeee85ff732ec661 Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Thu, 23 Aug 2018 07:52:35 +0800 Subject: [PATCH 4/6] change AsyncStorageModule constructor --- .../react/modules/storage/AsyncStorageModule.java | 8 +++----- .../react/modules/storage/AsyncStorageModuleTest.java | 2 -- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java index 4a40d248159911..761f281e88af38 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java @@ -15,7 +15,6 @@ import android.database.sqlite.SQLiteStatement; import com.facebook.common.logging.FLog; -import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.GuardedAsyncTask; @@ -48,13 +47,12 @@ public final class AsyncStorageModule private ExecutorService mExecutor; public AsyncStorageModule(ReactApplicationContext reactContext) { - super(reactContext); - mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext); mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext); - mExecutor = Executors.newSingleThreadExecutor(); + this(reactContext, Executors.newSingleThreadExecutor()); } public AsyncStorageModule(ReactApplicationContext reactContext, ExecutorService executor) { - this(reactContext); + super(reactContext); + mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext); mExecutor = executor; } diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java index 14b7f2896820b0..18072f7d57ccc0 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java @@ -25,11 +25,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import org.mockito.verification.VerificationMode; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.core.classloader.annotations.PowerMockIgnore; From 2c6b4978a45ac761e810416a38ea051b20ff19b0 Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Mon, 17 Sep 2018 20:52:37 +0800 Subject: [PATCH 5/6] AsyncStorageModule.java: rename variable mExecutor -> executor --- .../modules/storage/AsyncStorageModule.java | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java index 37756833434d56..7d4aea2af7ca6c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java @@ -1,10 +1,5 @@ -<<<<<<< HEAD -/* - * Copyright (c) 2015-present, Facebook, Inc. -======= /** * Copyright (c) Facebook, Inc. and its affiliates. ->>>>>>> master * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. @@ -49,7 +44,7 @@ public final class AsyncStorageModule private ReactDatabaseSupplier mReactDatabaseSupplier; private boolean mShuttingDown = false; - private ExecutorService mExecutor; + private ExecutorService executor; public AsyncStorageModule(ReactApplicationContext reactContext) { this(reactContext, Executors.newSingleThreadExecutor()); @@ -58,7 +53,7 @@ public AsyncStorageModule(ReactApplicationContext reactContext) { public AsyncStorageModule(ReactApplicationContext reactContext, ExecutorService executor) { super(reactContext); mReactDatabaseSupplier = ReactDatabaseSupplier.getInstance(reactContext); - mExecutor = executor; + this.executor = executor; } @Override @@ -74,7 +69,7 @@ public void initialize() { @Override public void onCatalystInstanceDestroy() { - mExecutor.shutdown(); + executor.shutdown(); mShuttingDown = true; } @@ -155,7 +150,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(null, data); } - }.executeOnExecutor(mExecutor); + }.executeOnExecutor(executor); } /** @@ -222,7 +217,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(); } } - }.executeOnExecutor(mExecutor); + }.executeOnExecutor(executor); } /** @@ -273,7 +268,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(); } } - }.executeOnExecutor(mExecutor); + }.executeOnExecutor(executor); } /** @@ -336,7 +331,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(); } } - }.executeOnExecutor(mExecutor); + }.executeOnExecutor(executor); } /** @@ -359,7 +354,7 @@ protected void doInBackgroundGuarded(Void... params) { callback.invoke(AsyncStorageErrorUtil.getError(null, e.getMessage())); } } - }.executeOnExecutor(mExecutor); + }.executeOnExecutor(executor); } /** @@ -393,7 +388,7 @@ protected void doInBackgroundGuarded(Void... params) { } callback.invoke(null, data); } - }.executeOnExecutor(mExecutor); + }.executeOnExecutor(executor); } /** From cde3ac55389dd773a6604049c3a7f3821f6b0a6f Mon Sep 17 00:00:00 2001 From: Dulmandakh Date: Mon, 17 Sep 2018 20:55:38 +0800 Subject: [PATCH 6/6] AsyncStorageModuleTest.java: fix indent --- .../react/modules/storage/AsyncStorageModuleTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java index 4db94a6d512293..8064722a6d137b 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/storage/AsyncStorageModuleTest.java @@ -74,7 +74,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { }); // don't use Robolectric before initializing mocks - mStorage = new AsyncStorageModule(ReactTestHelper.createCatalystContextForTest(), new RoboExecutorService()); + mStorage = new AsyncStorageModule( + ReactTestHelper.createCatalystContextForTest(), + new RoboExecutorService() + ); mEmptyArray = new JavaOnlyArray(); }