Skip to content

Commit

Permalink
CR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Kai Guo committed Apr 9, 2020
1 parent 060cf02 commit bc585f3
Show file tree
Hide file tree
Showing 6 changed files with 796 additions and 295 deletions.
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@
"flow-bin": "0.92.0",
"jest": "24.8.0",
"metro-react-native-babel-preset": "0.54.1",
"react": "16.9.0",
"react-dom": "16.9.0",
"react-native": "0.61.5",
"react": "16.6.3",
"react-dom": "16.6.3",
"react-native": "0.59.10",
"react-native-web": "~0.12.0",
"react-native-macos": "0.60.0-microsoft.50",
"react-test-renderer": "16.8.3",
Expand Down
47 changes: 40 additions & 7 deletions windows/ReactNativeAsyncStorage/DBStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

namespace winrt {
using namespace Microsoft::ReactNative;
using namespace Windows::ApplicationModel::Core;
using namespace Windows::Foundation;
using namespace Windows::Storage;
} // namespace winrt
Expand Down Expand Up @@ -194,13 +195,45 @@ namespace {
return CheckSQLiteResult(db, callback, sqlite3_bind_text(stmt.get(), index, str.c_str(), -1, SQLITE_TRANSIENT));
}

struct slim_shared_lock_guard
{
explicit slim_shared_lock_guard(winrt::slim_mutex& m) noexcept :
m_mutex(m)
{
m_mutex.lock_shared();
}

~slim_shared_lock_guard() noexcept
{
m_mutex.unlock_shared();
}

private:
winrt::slim_mutex& m_mutex;
};

} // namespace

const winrt::hstring DBStorage::s_dbPathProperty = L"React-Native-Community-Async-Storage-Database-Path";

DBStorage::DBStorage() {
auto const localAppDataParh = winrt::ApplicationData::Current().LocalFolder().Path();
std::wstring wPath(localAppDataParh.data());
wPath += L"\\AsyncStorage.db";
auto const path = ConvertWstrToStr(wPath);
std::string path;
if (auto pathInspectable = winrt::CoreApplication::Properties().TryLookup(s_dbPathProperty)) {
auto pathHstring = winrt::unbox_value<winrt::hstring>(pathInspectable);
path = ConvertWstrToStr(std::wstring(pathHstring.c_str()));
}
else {
try {
auto const localAppDataPath = winrt::ApplicationData::Current().LocalFolder().Path();
std::wstring wPath(localAppDataPath.data());
wPath += L"\\AsyncStorage.db";
path = ConvertWstrToStr(wPath);
}
catch (winrt::hresult_error const&) {
throw std::runtime_error("Please specify 'React-Native-Community-Async-Storage-Database-Path' in CoreApplication::Properties");
}
}

if (sqlite3_open_v2(
path.c_str(),
&m_db,
Expand Down Expand Up @@ -230,14 +263,14 @@ DBStorage::DBStorage() {
}

DBStorage::~DBStorage() {
decltype(m_action) action;
decltype(m_tasks) tasks;
{
// If there is an in-progress async task, cancel it and wait on the
// condition_variable for the async task to acknowledge cancellation by
// nulling out m_action. Once m_action is null, it is safe to proceed
// wth closing the DB connection
winrt::slim_lock_guard guard{ m_lock };
m_tasks.clear();
slim_shared_lock_guard guard{ m_lock };
swap(tasks, m_tasks);
if (m_action) {
m_action.Cancel();
m_cv.wait(m_lock, [this]() { return m_action == nullptr; });
Expand Down
1 change: 1 addition & 0 deletions windows/ReactNativeAsyncStorage/DBStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class DBStorage {
winrt::slim_condition_variable m_cv;
winrt::Windows::Foundation::IAsyncAction m_action{ nullptr };
std::vector<DBTask> m_tasks;
const static winrt::hstring s_dbPathProperty;

std::string ConvertWstrToStr(const std::wstring& wstr);
};
14 changes: 7 additions & 7 deletions windows/ReactNativeAsyncStorage/RNCAsyncStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace ReactNativeAsyncStorage
REACT_METHOD(multiGet);
void multiGet(std::vector<JSValue>& keys, std::function<void(JSValueArray const& errors, JSValueArray const& results)>&& callback) noexcept {
dbStorage.AddTask(DBStorage::DBTask::Type::multiGet, std::move(keys),
[callback](std::vector<JSValue> const& callbackParams) {
[callback{ std::move(callback) }](std::vector<JSValue> const& callbackParams) {
if (callbackParams.size() > 0) {
auto& errors = callbackParams[0].AsArray();
if (callbackParams.size() > 1) {
Expand All @@ -33,7 +33,7 @@ namespace ReactNativeAsyncStorage
REACT_METHOD(multiSet);
void multiSet(std::vector<JSValue>& pairs, std::function<void(JSValueArray const&)>&& callback) noexcept {
dbStorage.AddTask(DBStorage::DBTask::Type::multiSet, std::move(pairs),
[callback](std::vector<JSValue> const& callbackParams) {
[callback{ std::move(callback) }](std::vector<JSValue> const& callbackParams) {
if (callbackParams.size() > 0) {
auto& errors = callbackParams[0].AsArray();
callback(errors);
Expand All @@ -50,7 +50,7 @@ namespace ReactNativeAsyncStorage
newValues.push_back(pair.AsArray()[1].AsString());
}

multiGet(std::move(keys), [newValues, callback, this](JSValueArray const& errors, JSValueArray const& results) {
multiGet(std::move(keys), [newValues{ std::move(newValues) }, callback{ std::move(callback) }, this](JSValueArray const& errors, JSValueArray const& results) {
if (errors.size() > 0) {
callback(errors);
return;
Expand Down Expand Up @@ -88,7 +88,7 @@ namespace ReactNativeAsyncStorage
}
}

multiSet(std::move(mergedResults), [callback](JSValueArray const& errors) {
multiSet(std::move(mergedResults), [callback{ std::move(callback) }](JSValueArray const& errors) {
callback(errors);
});
});
Expand All @@ -97,7 +97,7 @@ namespace ReactNativeAsyncStorage
REACT_METHOD(multiRemove);
void multiRemove(std::vector<JSValue>& keys, std::function<void(JSValueArray const&)>&& callback) noexcept {
dbStorage.AddTask(DBStorage::DBTask::Type::multiRemove, std::move(keys),
[callback](std::vector<JSValue> const& callbackParams) {
[callback{ std::move(callback) }](std::vector<JSValue> const& callbackParams) {
if (callbackParams.size() > 0) {
auto& errors = callbackParams[0].AsArray();
callback(errors);
Expand All @@ -108,7 +108,7 @@ namespace ReactNativeAsyncStorage
REACT_METHOD(getAllKeys);
void getAllKeys(std::function<void(JSValueArray const& errors, JSValueArray const& keys)>&& callback) noexcept {
dbStorage.AddTask(DBStorage::DBTask::Type::getAllKeys,
[callback](std::vector<JSValue> const& callbackParams) {
[callback{ std::move(callback) }](std::vector<JSValue> const& callbackParams) {
if (callbackParams.size() > 0) {
auto& errors = callbackParams[0].AsArray();
if (callbackParams.size() > 1) {
Expand All @@ -124,7 +124,7 @@ namespace ReactNativeAsyncStorage
REACT_METHOD(clear);
void clear(std::function<void(JSValueArray const&)>&& callback) noexcept {
dbStorage.AddTask(DBStorage::DBTask::Type::clear,
[callback](std::vector<JSValue> const& callbackParams) {
[callback{ std::move(callback) }](std::vector<JSValue> const& callbackParams) {
if (callbackParams.size() > 0) {
auto& errors = callbackParams[0].AsArray();
callback(errors);
Expand Down
1 change: 1 addition & 0 deletions windows/ReactNativeAsyncStorage/pch.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <unknwn.h>
#include <functional>
#include <winrt/Windows.ApplicationModel.Core.h>
#include <winrt/Windows.Data.Json.h>
#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>
Expand Down
Loading

0 comments on commit bc585f3

Please sign in to comment.