From fc1f5992b9e3a454a092bb8c3635c4e6e64f610d Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Sun, 28 Nov 2021 10:27:58 -0800 Subject: [PATCH 1/6] Fix windows module asynchronous code --- example/windows/ReactTestApp.sln | 23 +- .../ReactNativeAsyncStorage.vcxproj | 4 +- windows/code/DBStorage.cpp | 666 ++++++++++-------- windows/code/DBStorage.h | 169 +++-- windows/code/RNCAsyncStorage.h | 219 ++---- windows/code/ReactPackageProvider.cpp | 2 +- windows/code/ReactPackageProvider.h | 2 +- windows/code/pch.cpp | 2 +- windows/code/pch.h | 2 +- 9 files changed, 581 insertions(+), 508 deletions(-) diff --git a/example/windows/ReactTestApp.sln b/example/windows/ReactTestApp.sln index 5f1937cb..f6408ae0 100644 --- a/example/windows/ReactTestApp.sln +++ b/example/windows/ReactTestApp.sln @@ -35,22 +35,21 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Mso", "..\..\node_modules\r EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Include", "..\..\node_modules\react-native-windows\include\Include.vcxitems", "{EF074BA1-2D54-4D49-A28E-5E040B47CD2E}" EndProject - Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ReactNativeAsyncStorage", "..\..\windows\ReactNativeAsyncStorage\ReactNativeAsyncStorage.vcxproj", "{4855D892-E16C-404D-8286-0089E0F7F9C4}" EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution - ..\node_modules\react-native-windows\JSI\Shared\JSI.Shared.vcxitems*{0cc28589-39e4-4288-b162-97b959f8b843}*SharedItemsImports = 9 - ..\node_modules\react-native-windows\Shared\Shared.vcxitems*{2049dbe9-8d13-42c9-ae4b-413ae38fffd0}*SharedItemsImports = 9 - ..\node_modules\react-native-windows\Mso\Mso.vcxitems*{84e05bfa-cbaf-4f0d-bfb6-4ce85742a57e}*SharedItemsImports = 9 - ..\node_modules\react-native-windows\JSI\Shared\JSI.Shared.vcxitems*{a62d504a-16b8-41d2-9f19-e2e86019e5e4}*SharedItemsImports = 4 - ..\node_modules\react-native-windows\Chakra\Chakra.vcxitems*{c38970c0-5fbf-4d69-90d8-cbac225ae895}*SharedItemsImports = 9 - ..\node_modules\react-native-windows\Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{da8b35b3-da00-4b02-bde6-6a397b3fd46b}*SharedItemsImports = 9 - ..\node_modules\react-native-windows\include\Include.vcxitems*{ef074ba1-2d54-4d49-a28e-5e040b47cd2e}*SharedItemsImports = 9 - ..\node_modules\react-native-windows\Chakra\Chakra.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 - ..\node_modules\react-native-windows\Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 - ..\node_modules\react-native-windows\Mso\Mso.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 - ..\node_modules\react-native-windows\Shared\Shared.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 + ..\..\node_modules\react-native-windows\JSI\Shared\JSI.Shared.vcxitems*{0cc28589-39e4-4288-b162-97b959f8b843}*SharedItemsImports = 9 + ..\..\node_modules\react-native-windows\Shared\Shared.vcxitems*{2049dbe9-8d13-42c9-ae4b-413ae38fffd0}*SharedItemsImports = 9 + ..\..\node_modules\react-native-windows\Mso\Mso.vcxitems*{84e05bfa-cbaf-4f0d-bfb6-4ce85742a57e}*SharedItemsImports = 9 + ..\..\node_modules\react-native-windows\JSI\Shared\JSI.Shared.vcxitems*{a62d504a-16b8-41d2-9f19-e2e86019e5e4}*SharedItemsImports = 4 + ..\..\node_modules\react-native-windows\Chakra\Chakra.vcxitems*{c38970c0-5fbf-4d69-90d8-cbac225ae895}*SharedItemsImports = 9 + ..\..\node_modules\react-native-windows\Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{da8b35b3-da00-4b02-bde6-6a397b3fd46b}*SharedItemsImports = 9 + ..\..\node_modules\react-native-windows\include\Include.vcxitems*{ef074ba1-2d54-4d49-a28e-5e040b47cd2e}*SharedItemsImports = 9 + ..\..\node_modules\react-native-windows\Chakra\Chakra.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 + ..\..\node_modules\react-native-windows\Microsoft.ReactNative.Cxx\Microsoft.ReactNative.Cxx.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 + ..\..\node_modules\react-native-windows\Mso\Mso.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 + ..\..\node_modules\react-native-windows\Shared\Shared.vcxitems*{f7d32bd0-2749-483e-9a0d-1635ef7e3136}*SharedItemsImports = 4 EndGlobalSection GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|ARM = Debug|ARM diff --git a/windows/ReactNativeAsyncStorage/ReactNativeAsyncStorage.vcxproj b/windows/ReactNativeAsyncStorage/ReactNativeAsyncStorage.vcxproj index f76e5083..ead5e6c1 100644 --- a/windows/ReactNativeAsyncStorage/ReactNativeAsyncStorage.vcxproj +++ b/windows/ReactNativeAsyncStorage/ReactNativeAsyncStorage.vcxproj @@ -161,7 +161,7 @@ - + This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. @@ -169,4 +169,4 @@ - + \ No newline at end of file diff --git a/windows/code/DBStorage.cpp b/windows/code/DBStorage.cpp index 809f5d93..5b30a352 100644 --- a/windows/code/DBStorage.cpp +++ b/windows/code/DBStorage.cpp @@ -1,180 +1,170 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" #include "DBStorage.h" +#include + namespace winrt { using namespace Microsoft::ReactNative; using namespace Windows::ApplicationModel::Core; + using namespace Windows::Data::Json; using namespace Windows::Foundation; using namespace Windows::Storage; } // namespace winrt +// All functions below return std::nullopt on error. +#define CHECK(expr) \ + if (!(expr)) { \ + return std::nullopt; \ + } + +// Convenience macro to call CheckSQLiteResult. +#define CHECK_SQL_OK(expr) CHECK(CheckSQLiteResult(db, m_errorHandler, (expr))) + namespace { + // To implement safe operator& for unique_ptr. + template + struct UniquePtrSetter { + UniquePtrSetter(std::unique_ptr &ptr) noexcept : m_ptr(ptr) + { + } - void InvokeError(const DBStorage::Callback &callback, const char *message) - { - auto writer = winrt::MakeJSValueTreeWriter(); - writer.WriteObjectBegin(); - winrt::WriteProperty(writer, L"message", message); - writer.WriteObjectEnd(); - callback(winrt::JSValueArray{winrt::TakeJSValue(writer)}); - } + ~UniquePtrSetter() + { + m_ptr = {m_rawPtr, m_ptr.get_deleter()}; + } - using ExecCallback = int(SQLITE_CALLBACK *)(void *, int, char **, char **); + operator T **() noexcept + { + return &m_rawPtr; + } - // Execute the provided SQLite statement (and optional execCallback & user data - // in pv). On error, throw a runtime_error with the SQLite error message - void Exec(sqlite3 *db, - const char *statement, - ExecCallback execCallback = nullptr, - void *pv = nullptr) + private: + T *m_rawPtr{}; + std::unique_ptr &m_ptr; + }; + + template + UniquePtrSetter operator&(std::unique_ptr &ptr) noexcept { - char *errMsg = nullptr; - int rc = sqlite3_exec(db, statement, execCallback, pv, &errMsg); - if (errMsg) { - std::runtime_error exception(errMsg); - sqlite3_free(errMsg); - sqlite3_close(db); - throw exception; - } - if (rc != SQLITE_OK) { - std::runtime_error exception(sqlite3_errmsg(db)); - sqlite3_free(errMsg); - sqlite3_close(db); - throw exception; - } + return UniquePtrSetter(ptr); } + using ExecCallback = int(SQLITE_CALLBACK *)(void *callbackData, + int columnCount, + char **columnTexts, + char **columnNames); + // Execute the provided SQLite statement (and optional execCallback & user data - // in pv). On error, reports it to the callback and returns false. - bool Exec(sqlite3 *db, - const DBStorage::Callback &callback, - const char *statement, - ExecCallback execCallback = nullptr, - void *pv = nullptr) + // in callbackData). On error, report it to the errorHandler and return std::nullopt. + std::optional Exec(sqlite3 *db, + DBStorage::ErrorHandler &errorHandler, + const char *statement, + ExecCallback execCallback = nullptr, + void *callbackData = nullptr) noexcept { - char *errMsg = nullptr; - int rc = sqlite3_exec(db, statement, execCallback, pv, &errMsg); + auto errMsg = std::unique_ptr{nullptr, &sqlite3_free}; + int rc = sqlite3_exec(db, statement, execCallback, callbackData, &errMsg); if (errMsg) { - InvokeError(callback, errMsg); - sqlite3_free(errMsg); - return false; + return errorHandler.AddError(errMsg.get()); } if (rc != SQLITE_OK) { - InvokeError(callback, sqlite3_errmsg(db)); - return false; + return errorHandler.AddError(sqlite3_errmsg(db)); } return true; } - // Convenience wrapper for using Exec with lambda expressions - template - bool Exec(sqlite3 *db, const DBStorage::Callback &callback, const char *statement, Fn &fn) + // Convenience wrapper for using Exec with lambda expressions. + template + std::optional + Exec(sqlite3 *db, DBStorage::ErrorHandler &errorHandler, const char *statement, Fn &fn) noexcept { return Exec( db, - callback, + errorHandler, statement, - [](void *pv, int i, char **x, char **y) { return (*static_cast(pv))(i, x, y); }, + [](void *callbackData, int columnCount, char **columnTexts, char **columnNames) { + return (*static_cast(callbackData))(columnCount, columnTexts, columnNames); + }, &fn); } - // Checks that the args parameter is an array, that args.size() is less than - // SQLITE_LIMIT_VARIABLE_NUMBER, and that every member of args is a string. - // Invokes callback to report an error and returns false. - bool CheckArgs(sqlite3 *db, - const std::vector &args, - const DBStorage::Callback &callback) + // Check that the args collection size is less than SQLITE_LIMIT_VARIABLE_NUMBER, and that + // every member of args is not an empty string. + // On error, report it to the errorHandler and return std::nullopt. + std::optional CheckArgs(sqlite3 *db, + DBStorage::ErrorHandler &errorHandler, + const std::vector &args) noexcept { int varLimit = sqlite3_limit(db, SQLITE_LIMIT_VARIABLE_NUMBER, -1); auto argCount = args.size(); - if (argCount > INT_MAX || static_cast(argCount) > varLimit) { + if (argCount > static_cast(std::numeric_limits::max()) || + static_cast(argCount) > varLimit) { char errorMsg[60]; - sprintf_s(errorMsg, "Too many keys. Maximum supported keys :%d", varLimit); - InvokeError(callback, errorMsg); - return false; + sprintf_s(errorMsg, "Too many keys. Maximum supported keys :%d.", varLimit); + return errorHandler.AddError(errorMsg); } for (int i = 0; i < static_cast(argCount); i++) { - if (!args[i].TryGetString()) { - InvokeError(callback, "Invalid key type. Expected a string"); + if (args[i].empty()) { + return errorHandler.AddError("The key must be a non-empty string."); } } return true; } // RAII object to manage SQLite transaction. On destruction, if - // Commit() has not been called, rolls back the transactions - // The provided sqlite connection handle & Callback must outlive - // the Sqlite3Transaction object - class Sqlite3Transaction - { - sqlite3 *m_db{nullptr}; - const DBStorage::Callback *m_callback{nullptr}; - - public: - Sqlite3Transaction() = default; - Sqlite3Transaction(sqlite3 *db, const DBStorage::Callback &callback) - : m_db(db), m_callback(&callback) + // Commit() has not been called, rolls back the transactions. + // The provided SQLite connection handle & errorHandler must outlive + // the Sqlite3Transaction object. + struct Sqlite3Transaction { + Sqlite3Transaction(sqlite3 *db, DBStorage::ErrorHandler &errorHandler) noexcept + : m_db(db), m_errorHandler(errorHandler) { - if (!Exec(m_db, *m_callback, u8"BEGIN TRANSACTION")) { + if (!Exec(m_db, m_errorHandler, "BEGIN TRANSACTION")) { m_db = nullptr; - m_callback = nullptr; } } + Sqlite3Transaction(const Sqlite3Transaction &) = delete; - Sqlite3Transaction(Sqlite3Transaction &&other) - : m_db(other.m_db), m_callback(other.m_callback) - { - other.m_db = nullptr; - other.m_callback = nullptr; - } Sqlite3Transaction &operator=(const Sqlite3Transaction &) = delete; - Sqlite3Transaction &operator=(Sqlite3Transaction &&rhs) + + ~Sqlite3Transaction() { - if (this != &rhs) { - Commit(); - std::swap(m_db, rhs.m_db); - std::swap(m_callback, rhs.m_callback); - } + Rollback(); } - explicit operator bool() const + explicit operator bool() const noexcept { return m_db != nullptr; } - void Rollback() + std::optional Commit() noexcept { if (m_db) { - Exec(m_db, *m_callback, u8"ROLLBACK"); - m_db = nullptr; - m_callback = nullptr; + return Exec(std::exchange(m_db, nullptr), m_errorHandler, "COMMIT"); } + return std::nullopt; } - bool Commit() + std::optional Rollback() noexcept { - if (!m_db) { - return false; + if (m_db) { + return Exec(std::exchange(m_db, nullptr), m_errorHandler, "ROLLBACK"); } - auto result = Exec(m_db, *m_callback, u8"COMMIT"); - m_db = nullptr; - m_callback = nullptr; - return result; + return std::nullopt; } - ~Sqlite3Transaction() - { - Rollback(); - } + private: + sqlite3 *m_db{}; + DBStorage::ErrorHandler &m_errorHandler; }; - // Appends argcount variables to prefix in a comma-separated list. - std::string MakeSQLiteParameterizedStatement(const char *prefix, int argCount) + // Append argCount variables to prefix in a comma-separated list. + std::string MakeSQLiteParameterizedStatement(const char *prefix, int argCount) noexcept { assert(argCount != 0); std::string result(prefix); @@ -187,160 +177,158 @@ namespace return result; } - // Checks if sqliteResult is SQLITE_OK. If not, reports the error via - // callback & returns false. - bool CheckSQLiteResult(sqlite3 *db, const DBStorage::Callback &callback, int sqliteResult) + // Check if sqliteResult is SQLITE_OK. + // If not, report the error to the errorHandler and return std::nullopt. + std::optional CheckSQLiteResult(sqlite3 *db, + DBStorage::ErrorHandler &errorHandler, + int sqliteResult) noexcept { if (sqliteResult == SQLITE_OK) { return true; } else { - InvokeError(callback, sqlite3_errmsg(db)); - return false; + return errorHandler.AddError(sqlite3_errmsg(db)); } } - using Statement = std::unique_ptr; + using StatementPtr = std::unique_ptr; - // Creates a prepared SQLite statement. On error, returns nullptr - Statement PrepareStatement(sqlite3 *db, DBStorage::Callback &callback, const char *stmt) + // A convenience wrapper for sqlite3_prepare_v2 function. + int PrepareStatement(sqlite3 *db, + const std::string &statementText, + sqlite3_stmt **statement) noexcept { - sqlite3_stmt *pStmt{nullptr}; - if (!CheckSQLiteResult(db, callback, sqlite3_prepare_v2(db, stmt, -1, &pStmt, nullptr))) { - return {nullptr, sqlite3_finalize}; - } - return {pStmt, &sqlite3_finalize}; + return sqlite3_prepare_v2(db, statementText.c_str(), -1, statement, nullptr); } - // Binds the index-th variable in this prepared statement to str. - bool BindString(sqlite3 *db, - const DBStorage::Callback &callback, - const Statement &stmt, - int index, - const std::string &str) + // A convenience wrapper for sqlite3_bind_text function. + int BindString(StatementPtr &statement, int index, const std::string &str) noexcept { - return CheckSQLiteResult( - db, callback, sqlite3_bind_text(stmt.get(), index, str.c_str(), -1, SQLITE_TRANSIENT)); + return sqlite3_bind_text(statement.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(); + // Merge source into destination. + // It only merges objects - all other types are just clobbered (including arrays). + void MergeJsonObjects(winrt::Windows::Data::Json::JsonObject const &destination, + winrt::Windows::Data::Json::JsonObject const &source) noexcept + { + for (auto keyValue : source) { + auto key = keyValue.Key(); + auto sourceValue = keyValue.Value(); + if (destination.HasKey(key)) { + auto destinationValue = destination.GetNamedValue(key); + if (destinationValue.ValueType() == + winrt::Windows::Data::Json::JsonValueType::Object && + sourceValue.ValueType() == winrt::Windows::Data::Json::JsonValueType::Object) { + MergeJsonObjects(destinationValue.GetObject(), sourceValue.GetObject()); + continue; + } + } + destination.SetNamedValue(key, sourceValue); } - - private: - winrt::slim_mutex &m_mutex; - }; - + } } // namespace -DBStorage::DBStorage() +// Initialize storage. On error, report it to the errorHandler and return std::nullopt. +std::optional +DBStorage::InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept { + winrt::slim_lock_guard guard{m_lock}; + if (m_db) { + return m_db.get(); + } + std::string path; - if (auto pathInspectable = winrt::CoreApplication::Properties().TryLookup(s_dbPathProperty)) { - auto pathHstring = winrt::unbox_value(pathInspectable); - path = ConvertWstrToStr(std::wstring(pathHstring.c_str())); - } else { - try { + try { + if (auto pathInspectable = + winrt::CoreApplication::Properties().TryLookup(s_dbPathProperty)) { + path = winrt::to_string(winrt::unbox_value(pathInspectable)); + } else { 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"); + path = winrt::to_string(localAppDataPath) + "\\AsyncStorage.db"; } + } catch (const winrt::hresult_error &error) { + errorHandler.AddError(winrt::to_string(error.message())); + return errorHandler.AddError( + "Please specify 'React-Native-Community-Async-Storage-Database-Path' in " + "CoreApplication::Properties"); } + auto db = DatabasePtr{nullptr, &sqlite3_close}; if (sqlite3_open_v2(path.c_str(), - &m_db, + &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, nullptr) != SQLITE_OK) { - auto exception = std::runtime_error(sqlite3_errmsg(m_db)); - sqlite3_close(m_db); - throw exception; + if (db) { + return errorHandler.AddError(sqlite3_errmsg(db.get())); + } else { + return errorHandler.AddError("Storage database cannot be opened."); + } } int userVersion = 0; auto getUserVersionCallback = - [](void *pv, int cCol, char **rgszColText, char ** /*rgszColName*/) { - if (cCol < 1) { + [](void *callbackData, int colomnCount, char **columnTexts, char ** /*columnNames*/) { + if (colomnCount < 1) { return 1; } - *static_cast(pv) = atoi(rgszColText[0]); + *static_cast(callbackData) = atoi(columnTexts[0]); return SQLITE_OK; }; - Exec(m_db, u8"PRAGMA user_version", getUserVersionCallback, &userVersion); + CHECK( + Exec(db.get(), errorHandler, "PRAGMA user_version", getUserVersionCallback, &userVersion)); if (userVersion == 0) { - Exec(m_db, - u8"CREATE TABLE IF NOT EXISTS AsyncLocalStorage(key TEXT PRIMARY KEY, value TEXT NOT " - u8"NULL); PRAGMA user_version=1"); + CHECK(Exec(db.get(), + errorHandler, + "CREATE TABLE IF NOT EXISTS AsyncLocalStorage(key TEXT PRIMARY KEY, value TEXT " + "NOT NULL); PRAGMA user_version=1")); } + + m_db = std::move(db); + return m_db.get(); } DBStorage::~DBStorage() { 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 - slim_shared_lock_guard guard{m_lock}; + // 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 with closing the DB connection. The DB connection is closed + // by the m_db destructor. + winrt::slim_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; }); } } - sqlite3_close(m_db); -} - -std::string DBStorage::ConvertWstrToStr(const std::wstring &wstr) -{ - if (wstr.empty()) { - return std::string(); - } - - int size = WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), NULL, 0, NULL, NULL); - std::string str(size, 0); - WideCharToMultiByte(CP_UTF8, 0, &wstr[0], (int)wstr.size(), &str[0], size, NULL, NULL); - return str; } -// Under the lock, add a task to m_tasks and, if no async task is in progress, -// schedule it -void DBStorage::AddTask(DBStorage::DBTask::Type type, - std::vector &&args, - DBStorage::Callback &&jsCallback) +// Under the lock, add a task to m_tasks and, if no async task is in progress schedule it. +void DBStorage::AddTask(ErrorHandler &errorHandler, + std::function &&onRun) noexcept { winrt::slim_lock_guard guard(m_lock); - m_tasks.emplace_back(type, std::move(args), std::move(jsCallback)); + m_tasks.push_back(std::make_unique(errorHandler, std::move(onRun))); if (!m_action) { m_action = RunTasks(); } } -// On a background thread, while the async task has not been cancelled and +// On a background thread, while the async task has not been canceled and // there are more tasks to do, run the tasks. When there are either no more // tasks or cancellation has been requested, set m_action to null to report // that and complete the coroutine. N.B., it is important that detecting that // m_tasks is empty and acknowledging completion is done atomically; otherwise // there would be a race between the background task detecting m_tasks.empty() // and AddTask checking the coroutine is running. -winrt::Windows::Foundation::IAsyncAction DBStorage::RunTasks() +winrt::Windows::Foundation::IAsyncAction DBStorage::RunTasks() noexcept { auto cancellationToken = co_await winrt::get_cancellation_token(); co_await winrt::resume_background(); - while (!cancellationToken()) { + for (;;) { decltype(m_tasks) tasks; sqlite3 *db{nullptr}; { @@ -351,174 +339,236 @@ winrt::Windows::Foundation::IAsyncAction DBStorage::RunTasks() co_return; } std::swap(tasks, m_tasks); - db = m_db; + db = m_db.get(); } for (auto &task : tasks) { - task.Run(db); - if (cancellationToken()) - break; + if (!cancellationToken()) { + task->Run(*this, db); + } else { + task->Cancel(); + } } } - winrt::slim_lock_guard guard(m_lock); - m_action = nullptr; - m_cv.notify_all(); } -void DBStorage::DBTask::Run(sqlite3 *db) +// Add new Error to the error list. +// Return std::nullopt for convenience to other methods that use std::nullopt to indicate error +// result. +std::nullopt_t DBStorage::ErrorHandler::AddError(std::string &&message) noexcept { - switch (m_type) { - case Type::multiGet: - multiGet(db); - break; - case Type::multiSet: - multiSet(db); - break; - case Type::multiRemove: - multiRemove(db); - break; - case Type::clear: - clear(db); - break; - case Type::getAllKeys: - getAllKeys(db); - break; - } + m_errors.push_back(Error{std::move(message)}); + return std::nullopt; } -void DBStorage::DBTask::multiGet(sqlite3 *db) +const std::vector &DBStorage::ErrorHandler::GetErrors() const noexcept { - if (!CheckArgs(db, m_args, m_callback)) { - return; + return m_errors; +} + +DBStorage::DBTask::DBTask(DBStorage::ErrorHandler &errorHandler, + std::function &&onRun) noexcept + : m_errorHandler(errorHandler), m_onRun(std::move(onRun)) +{ +} + +void DBStorage::DBTask::Run(DBStorage &storage, sqlite3 *db) noexcept +{ + if (!db) { + // We initialize DB handler on demand to report errors in the task context. + if (auto res = storage.InitializeStorage(m_errorHandler)) { + db = *res; + } + } + if (db) { + m_onRun(*this, db); } +} + +void DBStorage::DBTask::Cancel() noexcept +{ + m_errorHandler.AddError("Task is canceled."); +} + +std::optional> +DBStorage::DBTask::MultiGet(sqlite3 *db, const std::vector &keys) noexcept +{ + CHECK(m_errorHandler.GetErrors().empty()); + CHECK(CheckArgs(db, m_errorHandler, keys)); - auto argCount = static_cast(m_args.size()); + auto argCount = static_cast(keys.size()); auto sql = MakeSQLiteParameterizedStatement( - u8"SELECT key, value FROM AsyncLocalStorage WHERE key IN ", argCount); - auto pStmt = PrepareStatement(db, m_callback, sql.data()); - if (!pStmt) { - return; - } + "SELECT key, value FROM AsyncLocalStorage WHERE key IN ", argCount); + auto statement = StatementPtr{nullptr, &sqlite3_finalize}; + CHECK_SQL_OK(PrepareStatement(db, sql, &statement)); for (int i = 0; i < argCount; i++) { - if (!BindString(db, m_callback, pStmt, i + 1, m_args[i].AsString())) - return; + CHECK_SQL_OK(BindString(statement, i + 1, keys[i])); } - auto result = winrt::JSValueArray{}; - for (auto stepResult = sqlite3_step(pStmt.get()); stepResult != SQLITE_DONE; - stepResult = sqlite3_step(pStmt.get())) { + std::vector result; + for (;;) { + auto stepResult = sqlite3_step(statement.get()); + if (stepResult == SQLITE_DONE) { + break; + } if (stepResult != SQLITE_ROW) { - InvokeError(m_callback, sqlite3_errmsg(db)); - return; + return m_errorHandler.AddError(sqlite3_errmsg(db)); } - auto key = reinterpret_cast(sqlite3_column_text(pStmt.get(), 0)); + auto key = reinterpret_cast(sqlite3_column_text(statement.get(), 0)); if (!key) { - InvokeError(m_callback, sqlite3_errmsg(db)); - return; + return m_errorHandler.AddError(sqlite3_errmsg(db)); } - auto value = reinterpret_cast(sqlite3_column_text(pStmt.get(), 1)); + auto value = reinterpret_cast(sqlite3_column_text(statement.get(), 1)); if (!value) { - InvokeError(m_callback, sqlite3_errmsg(db)); - return; + return m_errorHandler.AddError(sqlite3_errmsg(db)); } - result.push_back(winrt::JSValueArray({key, value})); + result.push_back(KeyValue{key, value}); } - auto writer = winrt::MakeJSValueTreeWriter(); - result.WriteTo(writer); - std::vector callbackParams; - callbackParams.push_back(winrt::JSValueArray()); - callbackParams.push_back(winrt::TakeJSValue(writer)); - m_callback(callbackParams); + return result; } -void DBStorage::DBTask::multiSet(sqlite3 *db) +std::optional DBStorage::DBTask::MultiSet(sqlite3 *db, + const std::vector &keyValues) noexcept { - Sqlite3Transaction transaction(db, m_callback); - if (!transaction) { - return; - } - auto pStmt = - PrepareStatement(db, m_callback, u8"INSERT OR REPLACE INTO AsyncLocalStorage VALUES(?, ?)"); - if (!pStmt) { - return; + CHECK(m_errorHandler.GetErrors().empty()); + if (keyValues.empty()) { + return true; // nothing to do } - for (auto &&arg : m_args) { - if (!BindString(db, m_callback, pStmt, 1, arg[0].AsString()) || - !BindString(db, m_callback, pStmt, 2, arg[1].AsString())) { - return; - } - auto rc = sqlite3_step(pStmt.get()); - if (rc != SQLITE_DONE && !CheckSQLiteResult(db, m_callback, rc)) { - return; - } - if (!CheckSQLiteResult(db, m_callback, sqlite3_reset(pStmt.get()))) { - return; - } - } - if (!transaction.Commit()) { - return; + + Sqlite3Transaction transaction(db, m_errorHandler); + CHECK(transaction); + auto statement = StatementPtr{nullptr, &sqlite3_finalize}; + CHECK_SQL_OK( + PrepareStatement(db, "INSERT OR REPLACE INTO AsyncLocalStorage VALUES(?, ?)", &statement)); + for (const auto &keyValue : keyValues) { + CHECK_SQL_OK(BindString(statement, 1, keyValue.Key)); + CHECK_SQL_OK(BindString(statement, 2, keyValue.Value)); + auto rc = sqlite3_step(statement.get()); + CHECK(rc == SQLITE_DONE || CheckSQLiteResult(db, m_errorHandler, rc)); + CHECK_SQL_OK(sqlite3_reset(statement.get())); } - std::vector callbackParams; - callbackParams.push_back(winrt::JSValueArray()); - m_callback(callbackParams); + CHECK(transaction.Commit()); + return true; } -void DBStorage::DBTask::multiRemove(sqlite3 *db) +std::optional DBStorage::DBTask::MultiMerge(sqlite3 *db, + const std::vector &keyValues) noexcept { - if (!CheckArgs(db, m_args, m_callback)) { - return; + CHECK(m_errorHandler.GetErrors().empty()); + + std::vector keys; + std::unordered_map newValues; + keys.reserve(keyValues.size()); + for (const auto &keyValue : keyValues) { + keys.push_back(keyValue.Key); + newValues.try_emplace(keyValue.Key, keyValue.Value); } - auto argCount = static_cast(m_args.size()); - auto sql = - MakeSQLiteParameterizedStatement(u8"DELETE FROM AsyncLocalStorage WHERE key IN ", argCount); - auto pStmt = PrepareStatement(db, m_callback, sql.data()); - if (!pStmt) { - return; + auto oldValues = MultiGet(db, keys); + CHECK(oldValues); + + std::vector mergedResults; + for (size_t i = 0; i < oldValues->size(); i++) { + auto &key = oldValues->at(i).Key; + auto &oldValue = oldValues->at(i).Value; + auto &newValue = newValues[key]; + + winrt::JsonObject oldJson; + winrt::JsonObject newJson; + if (winrt::JsonObject::TryParse(winrt::to_hstring(oldValue), oldJson) && + winrt::JsonObject::TryParse(winrt::to_hstring(newValue), newJson)) { + MergeJsonObjects(oldJson, newJson); + mergedResults.push_back(KeyValue{key, winrt::to_string(oldJson.ToString())}); + } else { + return m_errorHandler.AddError("Values must be valid JSON object strings"); + } } + + return MultiSet(db, mergedResults); +} + +std::optional DBStorage::DBTask::MultiRemove(sqlite3 *db, + const std::vector &keys) noexcept +{ + CHECK(m_errorHandler.GetErrors().empty()); + CHECK(CheckArgs(db, m_errorHandler, keys)); + auto argCount = static_cast(keys.size()); + auto sql = + MakeSQLiteParameterizedStatement("DELETE FROM AsyncLocalStorage WHERE key IN ", argCount); + auto statement = StatementPtr{nullptr, &sqlite3_finalize}; + CHECK_SQL_OK(PrepareStatement(db, sql, &statement)); for (int i = 0; i < argCount; i++) { - if (!BindString(db, m_callback, pStmt, i + 1, m_args[i].AsString())) { - return; - } + CHECK(BindString(statement, i + 1, keys[i])); } - for (auto stepResult = sqlite3_step(pStmt.get()); stepResult != SQLITE_DONE; - stepResult = sqlite3_step(pStmt.get())) { + for (;;) { + auto stepResult = sqlite3_step(statement.get()); + if (stepResult == SQLITE_DONE) { + break; + } if (stepResult != SQLITE_ROW) { - InvokeError(m_callback, sqlite3_errmsg(db)); - return; + return m_errorHandler.AddError(sqlite3_errmsg(db)); } } - std::vector callbackParams; - callbackParams.push_back(winrt::JSValueArray()); - m_callback(callbackParams); + return true; } -void DBStorage::DBTask::getAllKeys(sqlite3 *db) +std::optional> DBStorage::DBTask::GetAllKeys(sqlite3 *db) noexcept { - winrt::JSValueArray result; - auto getAllKeysCallback = [&](int cCol, char **rgszColText, char **) { - if (cCol >= 1) { - result.push_back(rgszColText[0]); + CHECK(m_errorHandler.GetErrors().empty()); + std::vector result; + auto getAllKeysCallback = [&](int columnCount, char **columnTexts, char **) { + if (columnCount > 0) { + result.emplace_back(columnTexts[0]); } return SQLITE_OK; }; - if (Exec(db, m_callback, u8"SELECT key FROM AsyncLocalStorage", getAllKeysCallback)) { - auto writer = winrt::MakeJSValueTreeWriter(); - result.WriteTo(writer); - std::vector callbackParams; - callbackParams.push_back(nullptr); - callbackParams.push_back(winrt::TakeJSValue(writer)); - m_callback(callbackParams); - } + CHECK(Exec(db, m_errorHandler, "SELECT key FROM AsyncLocalStorage", getAllKeysCallback)); + return result; } -void DBStorage::DBTask::clear(sqlite3 *db) +std::optional DBStorage::DBTask::RemoveAll(sqlite3 *db) noexcept { - if (Exec(db, m_callback, u8"DELETE FROM AsyncLocalStorage")) { - std::vector callbackParams; - callbackParams.push_back(nullptr); - m_callback(callbackParams); + CHECK(m_errorHandler.GetErrors().empty()); + CHECK(Exec(db, m_errorHandler, "DELETE FROM AsyncLocalStorage")); + return true; +} + +// Read KeyValue from a JSON array. +void ReadValue(const winrt::IJSValueReader &reader, + /*out*/ DBStorage::KeyValue &value) noexcept +{ + if (reader.ValueType() == winrt::JSValueType::Array) { + int index = 0; + while (reader.GetNextArrayItem()) { + if (index == 0) { + ReadValue(reader, value.Key); + } else if (index == 1) { + ReadValue(reader, value.Value); + } else { + // Read the array till the end to keep reader in a good state. + winrt::SkipValue(reader); + } + ++index; + } } } + +// Write KeyValue to a JSON array. +void WriteValue(const winrt::Microsoft::ReactNative::IJSValueWriter &writer, + const DBStorage::KeyValue &value) noexcept +{ + writer.WriteArrayBegin(); + WriteValue(writer, value.Key); + WriteValue(writer, value.Value); + writer.WriteArrayEnd(); +} + +// Write Error object to JSON. +void WriteValue(const winrt::IJSValueWriter &writer, const DBStorage::Error &value) noexcept +{ + writer.WriteObjectBegin(); + winrt::WriteProperty(writer, L"message", value.Message); + writer.WriteObjectEnd(); +} diff --git a/windows/code/DBStorage.h b/windows/code/DBStorage.h index 7d402e07..f5f845e9 100644 --- a/windows/code/DBStorage.h +++ b/windows/code/DBStorage.h @@ -1,71 +1,160 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once +#include + #include #include "NativeModules.h" -class DBStorage -{ -public: - typedef std::function const &)> - Callback; +struct DBStorage { + // To pass KeyValue pairs in the native module API. + // It has custom ReadValue and WriteValue to read/write to/from JSON. + struct KeyValue { + std::string Key; + std::string Value; + }; - class DBTask - { - public: - enum class Type { multiGet, multiSet, multiRemove, clear, getAllKeys }; + // An Error object for the native module API. + // It has a custom WriteValue to write to JSON. + struct Error { + std::string Message; + }; + + // An error list shared between Promise and DBTask. + struct ErrorHandler { + std::nullopt_t AddError(std::string &&message) noexcept; + const std::vector &GetErrors() const noexcept; + + private: + std::vector m_errors; + }; + + // Ensure that only one result onResolve or onReject callback is called once. + template + struct Promise { + + Promise(TOnResolve &&onResolve, TOnReject &&onReject) noexcept + : m_onResolve(std::move(onResolve)), m_onReject(std::move(onReject)) + { + } + + ~Promise() + { + Reject(); + } + + Promise(const Promise &other) = delete; + Promise &operator=(const Promise &other) = delete; + + ErrorHandler &GetErrorHandler() noexcept + { + return m_errorHandler; + } + + template + void Resolve(const TValue &value) noexcept + { + Complete([&] { m_onResolve(value); }); + } + + void Reject() noexcept + { + Complete([&] { + // Ensure that we have at least one error on rejection. + if (m_errorHandler.GetErrors().empty()) { + m_errorHandler.AddError("Promise is rejected."); + } + m_onReject(m_errorHandler.GetErrors()); + }); + } - DBTask(Type type, - std::vector &&args, - Callback &&callback) - : m_type{type}, m_args{std::move(args)}, m_callback{std::move(callback)} + template + void ResolveOrReject(const std::optional &value) noexcept { + if (value) { + Resolve(*value); + } else { + Reject(); + } } + private: + template + void Complete(Fn &&fn) + { + if (m_isCompleted.test_and_set() == false) { + fn(); + } + } + + private: + ErrorHandler m_errorHandler; + std::atomic_flag m_isCompleted{false}; + TOnResolve m_onResolve; + TOnReject m_onReject; + }; + + // An asynchronous task that run in a background thread. + struct DBTask { + DBTask(ErrorHandler &errorHandler, + std::function &&onRun) noexcept; + + DBTask() = default; DBTask(const DBTask &) = delete; - DBTask(DBTask &&) = default; DBTask &operator=(const DBTask &) = delete; - DBTask &operator=(DBTask &&) = default; - void Run(sqlite3 *db); + + void Run(DBStorage &storage, sqlite3 *db) noexcept; + void Cancel() noexcept; + + std::optional> + MultiGet(sqlite3 *db, const std::vector &keys) noexcept; + std::optional MultiSet(sqlite3 *db, const std::vector &keyValues) noexcept; + std::optional MultiMerge(sqlite3 *db, + const std::vector &keyValues) noexcept; + std::optional MultiRemove(sqlite3 *db, const std::vector &keys) noexcept; + std::optional> GetAllKeys(sqlite3 *db) noexcept; + std::optional RemoveAll(sqlite3 *db) noexcept; private: - Type m_type; - std::vector m_args; - Callback m_callback; - - void multiGet(sqlite3 *db); - void multiSet(sqlite3 *db); - void multiRemove(sqlite3 *db); - void clear(sqlite3 *db); - void getAllKeys(sqlite3 *db); + std::function m_onRun; + ErrorHandler &m_errorHandler; }; - DBStorage(); - ~DBStorage(); + using DatabasePtr = std::unique_ptr; - void AddTask(DBTask::Type type, - std::vector &&args, - Callback &&jsCallback); + std::optional InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept; + ~DBStorage(); - void AddTask(DBTask::Type type, Callback &&jsCallback) + template + static auto CreatePromise(TOnResolve &&onResolve, TOnReject &&onReject) noexcept { - AddTask(type, - std::move(std::vector()), - std::move(jsCallback)); + using PromiseType = Promise, std::decay_t>; + return std::make_shared(std::forward(onResolve), + std::forward(onReject)); } - winrt::Windows::Foundation::IAsyncAction RunTasks(); + void AddTask(ErrorHandler &errorHandler, + std::function &&onRun) noexcept; + + winrt::Windows::Foundation::IAsyncAction RunTasks() noexcept; private: static constexpr auto s_dbPathProperty = L"React-Native-Community-Async-Storage-Database-Path"; - sqlite3 *m_db; + DatabasePtr m_db{nullptr, &sqlite3_close}; winrt::slim_mutex m_lock; winrt::slim_condition_variable m_cv; winrt::Windows::Foundation::IAsyncAction m_action{nullptr}; - std::vector m_tasks; - - std::string ConvertWstrToStr(const std::wstring &wstr); + std::vector> m_tasks; }; + +void ReadValue(const winrt::Microsoft::ReactNative::IJSValueReader &reader, + /*out*/ DBStorage::KeyValue &value) noexcept; + +void WriteValue(const winrt::Microsoft::ReactNative::IJSValueWriter &writer, + const DBStorage::KeyValue &value) noexcept; + +void WriteValue(const winrt::Microsoft::ReactNative::IJSValueWriter &writer, + const DBStorage::Error &value) noexcept; diff --git a/windows/code/RNCAsyncStorage.h b/windows/code/RNCAsyncStorage.h index d784c23e..4aa9eb6d 100644 --- a/windows/code/RNCAsyncStorage.h +++ b/windows/code/RNCAsyncStorage.h @@ -1,175 +1,110 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once -#include "pch.h" - #include "DBStorage.h" #include "NativeModules.h" namespace winrt::ReactNativeAsyncStorage::implementation { - REACT_MODULE(RNCAsyncStorage); + REACT_MODULE(RNCAsyncStorage) struct RNCAsyncStorage { - DBStorage dbStorage; - REACT_METHOD(multiGet); - void multiGet(std::vector keys, - std::function - &&callback) noexcept + REACT_METHOD(multiGet) + void multiGet( + std::vector &&keys, + std::function &errors, + const std::vector &result)> &&callback) noexcept { - dbStorage.AddTask( - DBStorage::DBTask::Type::multiGet, - std::move(keys), - [callback{std::move(callback)}](std::vector const &callbackParams) { - if (callbackParams.size() > 0) { - auto &errors = callbackParams[0].AsArray(); - if (callbackParams.size() > 1) { - callback(errors, callbackParams[1].AsArray()); - } else { - callback(errors, {}); - } - } + auto promise = DBStorage::CreatePromise( + [callback](const std::vector &result) { + callback({}, result); + }, + [callback](const std::vector &errors) { callback(errors, {}); }); + m_dbStorage.AddTask( + promise->GetErrorHandler(), + [promise, keys = std::move(keys)](DBStorage::DBTask &task, sqlite3 *db) noexcept { + promise->ResolveOrReject(task.MultiGet(db, keys)); }); } - REACT_METHOD(multiSet); - void multiSet(std::vector pairs, - std::function &&callback) noexcept + REACT_METHOD(multiSet) + void multiSet( + std::vector &&keyValues, + std::function &errors)> &&callback) noexcept { - dbStorage.AddTask( - DBStorage::DBTask::Type::multiSet, - std::move(pairs), - [callback{std::move(callback)}](std::vector const &callbackParams) { - if (callbackParams.size() > 0) { - auto &errors = callbackParams[0].AsArray(); - callback(errors); - } - }); + auto promise = DBStorage::CreatePromise( + [callback](bool /*value*/) { callback({}); }, + [callback](const std::vector &errors) { callback(errors); }); + m_dbStorage.AddTask(promise->GetErrorHandler(), + [promise, keyValues = std::move(keyValues)](DBStorage::DBTask &task, + sqlite3 *db) noexcept { + promise->ResolveOrReject(task.MultiSet(db, keyValues)); + }); } - REACT_METHOD(multiMerge); - void multiMerge(std::vector pairs, - std::function &&callback) noexcept + REACT_METHOD(multiMerge) + void multiMerge( + std::vector &&keyValues, + std::function &errors)> &&callback) noexcept { - std::vector keys; - std::vector newValues; - for (const auto &pair : pairs) { - keys.push_back(pair.AsArray()[0].AsString()); - newValues.push_back(pair.AsArray()[1].AsString()); - } - - 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; - } - - std::vector mergedResults; - - for (int i = 0; i < results.size(); i++) { - auto &oldPair = results[i].AsArray(); - auto &key = oldPair[0]; - auto oldValue = oldPair[1].AsString(); - auto &newValue = newValues[i]; - - winrt::Windows::Data::Json::JsonObject oldJson; - winrt::Windows::Data::Json::JsonObject newJson; - if (winrt::Windows::Data::Json::JsonObject::TryParse( - winrt::to_hstring(oldValue), oldJson) && - winrt::Windows::Data::Json::JsonObject::TryParse( - winrt::to_hstring(newValue), newJson)) { - MergeJsonObjects(oldJson, newJson); - - JSValue value; - auto writer = MakeJSValueTreeWriter(); - writer.WriteArrayBegin(); - WriteValue(writer, key); - WriteValue(writer, oldJson.ToString()); - writer.WriteArrayEnd(); - mergedResults.push_back(TakeJSValue(writer)); - } else { - auto writer = MakeJSValueTreeWriter(); - writer.WriteObjectBegin(); - WriteProperty( - writer, L"message", L"Values must be valid Json strings"); - writer.WriteObjectEnd(); - callback(JSValueArray{TakeJSValue(writer)}); - return; - } - } - - multiSet(std::move(mergedResults), - [callback{std::move(callback)}](JSValueArray const &errors) { - callback(errors); - }); - }); + auto promise = DBStorage::CreatePromise( + [callback](bool /*value*/) { callback({}); }, + [callback](const std::vector &errors) { callback(errors); }); + m_dbStorage.AddTask(promise->GetErrorHandler(), + [promise, keyValues = std::move(keyValues)](DBStorage::DBTask &task, + sqlite3 *db) noexcept { + promise->ResolveOrReject(task.MultiMerge(db, keyValues)); + }); } - REACT_METHOD(multiRemove); - void multiRemove(std::vector keys, - std::function &&callback) noexcept + REACT_METHOD(multiRemove) + void multiRemove( + std::vector &&keys, + std::function &errors)> &&callback) noexcept { - dbStorage.AddTask( - DBStorage::DBTask::Type::multiRemove, - std::move(keys), - [callback{std::move(callback)}](std::vector const &callbackParams) { - if (callbackParams.size() > 0) { - auto &errors = callbackParams[0].AsArray(); - callback(errors); - } + auto promise = DBStorage::CreatePromise( + [callback](bool /*value*/) { callback({}); }, + [callback](const std::vector &errors) { callback(errors); }); + m_dbStorage.AddTask( + promise->GetErrorHandler(), + [promise, keys = std::move(keys)](DBStorage::DBTask &task, sqlite3 *db) noexcept { + promise->ResolveOrReject(task.MultiRemove(db, keys)); }); } - REACT_METHOD(getAllKeys); - void getAllKeys( - std::function &&callback) noexcept + REACT_METHOD(getAllKeys) + void + getAllKeys(std::function &error, + const std::vector &keys)> &&callback) noexcept { - dbStorage.AddTask( - DBStorage::DBTask::Type::getAllKeys, - [callback{std::move(callback)}](std::vector const &callbackParams) { - if (callbackParams.size() > 0) { - auto &error = callbackParams[0]; - if (callbackParams.size() > 1) { - callback(error, callbackParams[1].AsArray()); - } else { - callback(error, {}); - } - } + auto promise = DBStorage::CreatePromise( + [callback](const std::vector &keys) { callback(std::nullopt, keys); }, + [callback](const std::vector &errors) { + callback(errors.at(0), {}); }); + m_dbStorage.AddTask(promise->GetErrorHandler(), + [promise](DBStorage::DBTask &task, sqlite3 *db) noexcept { + promise->ResolveOrReject(task.GetAllKeys(db)); + }); } - REACT_METHOD(clear); - void clear(std::function &&callback) noexcept + REACT_METHOD(clear) + void + clear(std::function &error)> &&callback) noexcept { - dbStorage.AddTask( - DBStorage::DBTask::Type::clear, - [callback{std::move(callback)}](std::vector const &callbackParams) { - if (callbackParams.size() > 0) { - auto &error = callbackParams[0]; - callback(error); - } - }); + auto promise = + DBStorage::CreatePromise([callback](bool /*value*/) { callback(std::nullopt); }, + [callback](const std::vector &errors) { + callback(errors.at(0)); + }); + m_dbStorage.AddTask(promise->GetErrorHandler(), + [promise](DBStorage::DBTask &task, sqlite3 *db) noexcept { + promise->ResolveOrReject(task.RemoveAll(db)); + }); } - // Merge newJson into oldJson - void MergeJsonObjects(winrt::Windows::Data::Json::JsonObject const &oldJson, - winrt::Windows::Data::Json::JsonObject const &newJson) - { - for (auto pair : newJson) { - auto key = pair.Key(); - auto newValue = pair.Value(); - if (newValue.ValueType() == winrt::Windows::Data::Json::JsonValueType::Object && - oldJson.HasKey(key)) { - auto oldValue = oldJson.GetNamedObject(key); - MergeJsonObjects(oldValue, newValue.GetObject()); - oldJson.SetNamedValue(key, oldValue); - } else { - oldJson.SetNamedValue(key, newValue); - } - } - } + private: + DBStorage m_dbStorage; }; } // namespace winrt::ReactNativeAsyncStorage::implementation diff --git a/windows/code/ReactPackageProvider.cpp b/windows/code/ReactPackageProvider.cpp index ae1f5a82..438c7b76 100644 --- a/windows/code/ReactPackageProvider.cpp +++ b/windows/code/ReactPackageProvider.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" diff --git a/windows/code/ReactPackageProvider.h b/windows/code/ReactPackageProvider.h index b7dbc017..1f609686 100644 --- a/windows/code/ReactPackageProvider.h +++ b/windows/code/ReactPackageProvider.h @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once diff --git a/windows/code/pch.cpp b/windows/code/pch.cpp index aef4c60c..e4b1bd69 100644 --- a/windows/code/pch.cpp +++ b/windows/code/pch.cpp @@ -1,3 +1,3 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" diff --git a/windows/code/pch.h b/windows/code/pch.h index 1bf564d0..be475e74 100644 --- a/windows/code/pch.h +++ b/windows/code/pch.h @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once From 55726e1c4f83b5d5167d8631ce73ea1fd7b2f0b4 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 29 Nov 2021 15:18:22 -0800 Subject: [PATCH 2/6] Update windows/code/DBStorage.cpp Shorten type names suggested by @tido64 in PR review Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com> --- windows/code/DBStorage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/windows/code/DBStorage.cpp b/windows/code/DBStorage.cpp index 5b30a352..58d2c21e 100644 --- a/windows/code/DBStorage.cpp +++ b/windows/code/DBStorage.cpp @@ -208,8 +208,8 @@ namespace // Merge source into destination. // It only merges objects - all other types are just clobbered (including arrays). - void MergeJsonObjects(winrt::Windows::Data::Json::JsonObject const &destination, - winrt::Windows::Data::Json::JsonObject const &source) noexcept + void MergeJsonObjects(winrt::JsonObject const &destination, + winrt::JsonObject const &source) noexcept { for (auto keyValue : source) { auto key = keyValue.Key(); From 945444f097f42a159e359d01aa891aec749f1937 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 29 Nov 2021 15:40:51 -0800 Subject: [PATCH 3/6] Remove redundant winrt::Windows::Data::Json long namespace --- windows/code/DBStorage.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/windows/code/DBStorage.cpp b/windows/code/DBStorage.cpp index 58d2c21e..d35b27ee 100644 --- a/windows/code/DBStorage.cpp +++ b/windows/code/DBStorage.cpp @@ -216,9 +216,8 @@ namespace auto sourceValue = keyValue.Value(); if (destination.HasKey(key)) { auto destinationValue = destination.GetNamedValue(key); - if (destinationValue.ValueType() == - winrt::Windows::Data::Json::JsonValueType::Object && - sourceValue.ValueType() == winrt::Windows::Data::Json::JsonValueType::Object) { + if (destinationValue.ValueType() == winrt::JsonValueType::Object && + sourceValue.ValueType() == winrt::JsonValueType::Object) { MergeJsonObjects(destinationValue.GetObject(), sourceValue.GetObject()); continue; } From 7b5f2ed13943662c1c28f3b3e2c22a06eb19fb82 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 29 Nov 2021 15:59:21 -0800 Subject: [PATCH 4/6] Fix C++20 compilation issues --- windows/code/DBStorage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/windows/code/DBStorage.h b/windows/code/DBStorage.h index f5f845e9..625adf25 100644 --- a/windows/code/DBStorage.h +++ b/windows/code/DBStorage.h @@ -91,7 +91,7 @@ struct DBStorage { private: ErrorHandler m_errorHandler; - std::atomic_flag m_isCompleted{false}; + std::atomic_flag m_isCompleted = ATOMIC_FLAG_INIT; TOnResolve m_onResolve; TOnReject m_onReject; }; From 078b16e9e86ad68333ca1fbf0f9666488832f949 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 29 Nov 2021 18:22:59 -0800 Subject: [PATCH 5/6] Combine all error messages when we return a single error --- windows/code/DBStorage.cpp | 133 +++++++++++++++++++-------------- windows/code/DBStorage.h | 28 +++---- windows/code/RNCAsyncStorage.h | 48 +++++++----- 3 files changed, 121 insertions(+), 88 deletions(-) diff --git a/windows/code/DBStorage.cpp b/windows/code/DBStorage.cpp index d35b27ee..cf415e43 100644 --- a/windows/code/DBStorage.cpp +++ b/windows/code/DBStorage.cpp @@ -22,7 +22,7 @@ namespace winrt } // Convenience macro to call CheckSQLiteResult. -#define CHECK_SQL_OK(expr) CHECK(CheckSQLiteResult(db, m_errorHandler, (expr))) +#define CHECK_SQL_OK(expr) CHECK(CheckSQLiteResult(db, m_errorManager, (expr))) namespace { @@ -60,9 +60,9 @@ namespace char **columnNames); // Execute the provided SQLite statement (and optional execCallback & user data - // in callbackData). On error, report it to the errorHandler and return std::nullopt. + // in callbackData). On error, report it to the errorManager and return std::nullopt. std::optional Exec(sqlite3 *db, - DBStorage::ErrorHandler &errorHandler, + DBStorage::ErrorManager &errorManager, const char *statement, ExecCallback execCallback = nullptr, void *callbackData = nullptr) noexcept @@ -70,10 +70,10 @@ namespace auto errMsg = std::unique_ptr{nullptr, &sqlite3_free}; int rc = sqlite3_exec(db, statement, execCallback, callbackData, &errMsg); if (errMsg) { - return errorHandler.AddError(errMsg.get()); + return errorManager.AddError(errMsg.get()); } if (rc != SQLITE_OK) { - return errorHandler.AddError(sqlite3_errmsg(db)); + return errorManager.AddError(sqlite3_errmsg(db)); } return true; } @@ -81,11 +81,11 @@ namespace // Convenience wrapper for using Exec with lambda expressions. template std::optional - Exec(sqlite3 *db, DBStorage::ErrorHandler &errorHandler, const char *statement, Fn &fn) noexcept + Exec(sqlite3 *db, DBStorage::ErrorManager &errorManager, const char *statement, Fn &fn) noexcept { return Exec( db, - errorHandler, + errorManager, statement, [](void *callbackData, int columnCount, char **columnTexts, char **columnNames) { return (*static_cast(callbackData))(columnCount, columnTexts, columnNames); @@ -95,9 +95,9 @@ namespace // Check that the args collection size is less than SQLITE_LIMIT_VARIABLE_NUMBER, and that // every member of args is not an empty string. - // On error, report it to the errorHandler and return std::nullopt. + // On error, report it to the errorManager and return std::nullopt. std::optional CheckArgs(sqlite3 *db, - DBStorage::ErrorHandler &errorHandler, + DBStorage::ErrorManager &errorManager, const std::vector &args) noexcept { int varLimit = sqlite3_limit(db, SQLITE_LIMIT_VARIABLE_NUMBER, -1); @@ -106,11 +106,11 @@ namespace static_cast(argCount) > varLimit) { char errorMsg[60]; sprintf_s(errorMsg, "Too many keys. Maximum supported keys :%d.", varLimit); - return errorHandler.AddError(errorMsg); + return errorManager.AddError(errorMsg); } for (int i = 0; i < static_cast(argCount); i++) { if (args[i].empty()) { - return errorHandler.AddError("The key must be a non-empty string."); + return errorManager.AddError("The key must be a non-empty string."); } } return true; @@ -118,13 +118,13 @@ namespace // RAII object to manage SQLite transaction. On destruction, if // Commit() has not been called, rolls back the transactions. - // The provided SQLite connection handle & errorHandler must outlive + // The provided SQLite connection handle & errorManager must outlive // the Sqlite3Transaction object. struct Sqlite3Transaction { - Sqlite3Transaction(sqlite3 *db, DBStorage::ErrorHandler &errorHandler) noexcept - : m_db(db), m_errorHandler(errorHandler) + Sqlite3Transaction(sqlite3 *db, DBStorage::ErrorManager &errorManager) noexcept + : m_db(db), m_errorManager(errorManager) { - if (!Exec(m_db, m_errorHandler, "BEGIN TRANSACTION")) { + if (!Exec(m_db, m_errorManager, "BEGIN TRANSACTION")) { m_db = nullptr; } } @@ -145,7 +145,7 @@ namespace std::optional Commit() noexcept { if (m_db) { - return Exec(std::exchange(m_db, nullptr), m_errorHandler, "COMMIT"); + return Exec(std::exchange(m_db, nullptr), m_errorManager, "COMMIT"); } return std::nullopt; } @@ -153,14 +153,14 @@ namespace std::optional Rollback() noexcept { if (m_db) { - return Exec(std::exchange(m_db, nullptr), m_errorHandler, "ROLLBACK"); + return Exec(std::exchange(m_db, nullptr), m_errorManager, "ROLLBACK"); } return std::nullopt; } private: sqlite3 *m_db{}; - DBStorage::ErrorHandler &m_errorHandler; + DBStorage::ErrorManager &m_errorManager; }; // Append argCount variables to prefix in a comma-separated list. @@ -178,15 +178,15 @@ namespace } // Check if sqliteResult is SQLITE_OK. - // If not, report the error to the errorHandler and return std::nullopt. + // If not, report the error to the errorManager and return std::nullopt. std::optional CheckSQLiteResult(sqlite3 *db, - DBStorage::ErrorHandler &errorHandler, + DBStorage::ErrorManager &errorManager, int sqliteResult) noexcept { if (sqliteResult == SQLITE_OK) { return true; } else { - return errorHandler.AddError(sqlite3_errmsg(db)); + return errorManager.AddError(sqlite3_errmsg(db)); } } @@ -227,9 +227,9 @@ namespace } } // namespace -// Initialize storage. On error, report it to the errorHandler and return std::nullopt. +// Initialize storage. On error, report it to the errorManager and return std::nullopt. std::optional -DBStorage::InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept +DBStorage::InitializeStorage(DBStorage::ErrorManager &errorManager) noexcept { winrt::slim_lock_guard guard{m_lock}; if (m_db) { @@ -246,8 +246,8 @@ DBStorage::InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept path = winrt::to_string(localAppDataPath) + "\\AsyncStorage.db"; } } catch (const winrt::hresult_error &error) { - errorHandler.AddError(winrt::to_string(error.message())); - return errorHandler.AddError( + errorManager.AddError(winrt::to_string(error.message())); + return errorManager.AddError( "Please specify 'React-Native-Community-Async-Storage-Database-Path' in " "CoreApplication::Properties"); } @@ -258,9 +258,9 @@ DBStorage::InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, nullptr) != SQLITE_OK) { if (db) { - return errorHandler.AddError(sqlite3_errmsg(db.get())); + return errorManager.AddError(sqlite3_errmsg(db.get())); } else { - return errorHandler.AddError("Storage database cannot be opened."); + return errorManager.AddError("Storage database cannot be opened."); } } @@ -275,11 +275,11 @@ DBStorage::InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept }; CHECK( - Exec(db.get(), errorHandler, "PRAGMA user_version", getUserVersionCallback, &userVersion)); + Exec(db.get(), errorManager, "PRAGMA user_version", getUserVersionCallback, &userVersion)); if (userVersion == 0) { CHECK(Exec(db.get(), - errorHandler, + errorManager, "CREATE TABLE IF NOT EXISTS AsyncLocalStorage(key TEXT PRIMARY KEY, value TEXT " "NOT NULL); PRAGMA user_version=1")); } @@ -306,11 +306,11 @@ DBStorage::~DBStorage() } // Under the lock, add a task to m_tasks and, if no async task is in progress schedule it. -void DBStorage::AddTask(ErrorHandler &errorHandler, +void DBStorage::AddTask(ErrorManager &errorManager, std::function &&onRun) noexcept { winrt::slim_lock_guard guard(m_lock); - m_tasks.push_back(std::make_unique(errorHandler, std::move(onRun))); + m_tasks.push_back(std::make_unique(errorManager, std::move(onRun))); if (!m_action) { m_action = RunTasks(); } @@ -354,20 +354,43 @@ winrt::Windows::Foundation::IAsyncAction DBStorage::RunTasks() noexcept // Add new Error to the error list. // Return std::nullopt for convenience to other methods that use std::nullopt to indicate error // result. -std::nullopt_t DBStorage::ErrorHandler::AddError(std::string &&message) noexcept +std::nullopt_t DBStorage::ErrorManager::AddError(std::string &&message) noexcept { m_errors.push_back(Error{std::move(message)}); return std::nullopt; } -const std::vector &DBStorage::ErrorHandler::GetErrors() const noexcept +bool DBStorage::ErrorManager::HasErrors() const noexcept { - return m_errors; + return !m_errors.empty(); } -DBStorage::DBTask::DBTask(DBStorage::ErrorHandler &errorHandler, +const std::vector &DBStorage::ErrorManager::GetErrorList() const noexcept +{ + if (HasErrors()) { + return m_errors; + } + static std::vector s_unknownError{Error{"Unknown error."}}; + return s_unknownError; +} + +DBStorage::Error DBStorage::ErrorManager::GetCombinedError() const noexcept +{ + auto &errors = GetErrorList(); + if (errors.size() == 1) { + return errors[0]; + } + + std::string combinedMessage; + for (const auto &error : errors) { + combinedMessage += error.Message + '\n'; + } + return Error{std::move(combinedMessage)}; +} + +DBStorage::DBTask::DBTask(DBStorage::ErrorManager &errorManager, std::function &&onRun) noexcept - : m_errorHandler(errorHandler), m_onRun(std::move(onRun)) + : m_errorManager(errorManager), m_onRun(std::move(onRun)) { } @@ -375,7 +398,7 @@ void DBStorage::DBTask::Run(DBStorage &storage, sqlite3 *db) noexcept { if (!db) { // We initialize DB handler on demand to report errors in the task context. - if (auto res = storage.InitializeStorage(m_errorHandler)) { + if (auto res = storage.InitializeStorage(m_errorManager)) { db = *res; } } @@ -386,14 +409,14 @@ void DBStorage::DBTask::Run(DBStorage &storage, sqlite3 *db) noexcept void DBStorage::DBTask::Cancel() noexcept { - m_errorHandler.AddError("Task is canceled."); + m_errorManager.AddError("Task is canceled."); } std::optional> DBStorage::DBTask::MultiGet(sqlite3 *db, const std::vector &keys) noexcept { - CHECK(m_errorHandler.GetErrors().empty()); - CHECK(CheckArgs(db, m_errorHandler, keys)); + CHECK(!m_errorManager.HasErrors()); + CHECK(CheckArgs(db, m_errorManager, keys)); auto argCount = static_cast(keys.size()); auto sql = MakeSQLiteParameterizedStatement( @@ -411,16 +434,16 @@ DBStorage::DBTask::MultiGet(sqlite3 *db, const std::vector &keys) n break; } if (stepResult != SQLITE_ROW) { - return m_errorHandler.AddError(sqlite3_errmsg(db)); + return m_errorManager.AddError(sqlite3_errmsg(db)); } auto key = reinterpret_cast(sqlite3_column_text(statement.get(), 0)); if (!key) { - return m_errorHandler.AddError(sqlite3_errmsg(db)); + return m_errorManager.AddError(sqlite3_errmsg(db)); } auto value = reinterpret_cast(sqlite3_column_text(statement.get(), 1)); if (!value) { - return m_errorHandler.AddError(sqlite3_errmsg(db)); + return m_errorManager.AddError(sqlite3_errmsg(db)); } result.push_back(KeyValue{key, value}); } @@ -430,12 +453,12 @@ DBStorage::DBTask::MultiGet(sqlite3 *db, const std::vector &keys) n std::optional DBStorage::DBTask::MultiSet(sqlite3 *db, const std::vector &keyValues) noexcept { - CHECK(m_errorHandler.GetErrors().empty()); + CHECK(!m_errorManager.HasErrors()); if (keyValues.empty()) { return true; // nothing to do } - Sqlite3Transaction transaction(db, m_errorHandler); + Sqlite3Transaction transaction(db, m_errorManager); CHECK(transaction); auto statement = StatementPtr{nullptr, &sqlite3_finalize}; CHECK_SQL_OK( @@ -444,7 +467,7 @@ std::optional DBStorage::DBTask::MultiSet(sqlite3 *db, CHECK_SQL_OK(BindString(statement, 1, keyValue.Key)); CHECK_SQL_OK(BindString(statement, 2, keyValue.Value)); auto rc = sqlite3_step(statement.get()); - CHECK(rc == SQLITE_DONE || CheckSQLiteResult(db, m_errorHandler, rc)); + CHECK(rc == SQLITE_DONE || CheckSQLiteResult(db, m_errorManager, rc)); CHECK_SQL_OK(sqlite3_reset(statement.get())); } CHECK(transaction.Commit()); @@ -454,7 +477,7 @@ std::optional DBStorage::DBTask::MultiSet(sqlite3 *db, std::optional DBStorage::DBTask::MultiMerge(sqlite3 *db, const std::vector &keyValues) noexcept { - CHECK(m_errorHandler.GetErrors().empty()); + CHECK(!m_errorManager.HasErrors()); std::vector keys; std::unordered_map newValues; @@ -480,7 +503,7 @@ std::optional DBStorage::DBTask::MultiMerge(sqlite3 *db, MergeJsonObjects(oldJson, newJson); mergedResults.push_back(KeyValue{key, winrt::to_string(oldJson.ToString())}); } else { - return m_errorHandler.AddError("Values must be valid JSON object strings"); + return m_errorManager.AddError("Values must be valid JSON object strings"); } } @@ -490,8 +513,8 @@ std::optional DBStorage::DBTask::MultiMerge(sqlite3 *db, std::optional DBStorage::DBTask::MultiRemove(sqlite3 *db, const std::vector &keys) noexcept { - CHECK(m_errorHandler.GetErrors().empty()); - CHECK(CheckArgs(db, m_errorHandler, keys)); + CHECK(!m_errorManager.HasErrors()); + CHECK(CheckArgs(db, m_errorManager, keys)); auto argCount = static_cast(keys.size()); auto sql = MakeSQLiteParameterizedStatement("DELETE FROM AsyncLocalStorage WHERE key IN ", argCount); @@ -506,7 +529,7 @@ std::optional DBStorage::DBTask::MultiRemove(sqlite3 *db, break; } if (stepResult != SQLITE_ROW) { - return m_errorHandler.AddError(sqlite3_errmsg(db)); + return m_errorManager.AddError(sqlite3_errmsg(db)); } } return true; @@ -514,7 +537,7 @@ std::optional DBStorage::DBTask::MultiRemove(sqlite3 *db, std::optional> DBStorage::DBTask::GetAllKeys(sqlite3 *db) noexcept { - CHECK(m_errorHandler.GetErrors().empty()); + CHECK(!m_errorManager.HasErrors()); std::vector result; auto getAllKeysCallback = [&](int columnCount, char **columnTexts, char **) { if (columnCount > 0) { @@ -523,14 +546,14 @@ std::optional> DBStorage::DBTask::GetAllKeys(sqlite3 *d return SQLITE_OK; }; - CHECK(Exec(db, m_errorHandler, "SELECT key FROM AsyncLocalStorage", getAllKeysCallback)); + CHECK(Exec(db, m_errorManager, "SELECT key FROM AsyncLocalStorage", getAllKeysCallback)); return result; } std::optional DBStorage::DBTask::RemoveAll(sqlite3 *db) noexcept { - CHECK(m_errorHandler.GetErrors().empty()); - CHECK(Exec(db, m_errorHandler, "DELETE FROM AsyncLocalStorage")); + CHECK(!m_errorManager.HasErrors()); + CHECK(Exec(db, m_errorManager, "DELETE FROM AsyncLocalStorage")); return true; } diff --git a/windows/code/DBStorage.h b/windows/code/DBStorage.h index 625adf25..63f38806 100644 --- a/windows/code/DBStorage.h +++ b/windows/code/DBStorage.h @@ -23,9 +23,11 @@ struct DBStorage { }; // An error list shared between Promise and DBTask. - struct ErrorHandler { + struct ErrorManager { std::nullopt_t AddError(std::string &&message) noexcept; - const std::vector &GetErrors() const noexcept; + bool HasErrors() const noexcept; + const std::vector &GetErrorList() const noexcept; + Error GetCombinedError() const noexcept; private: std::vector m_errors; @@ -48,9 +50,9 @@ struct DBStorage { Promise(const Promise &other) = delete; Promise &operator=(const Promise &other) = delete; - ErrorHandler &GetErrorHandler() noexcept + ErrorManager &GetErrorManager() noexcept { - return m_errorHandler; + return m_errorManager; } template @@ -63,10 +65,10 @@ struct DBStorage { { Complete([&] { // Ensure that we have at least one error on rejection. - if (m_errorHandler.GetErrors().empty()) { - m_errorHandler.AddError("Promise is rejected."); + if (!m_errorManager.HasErrors()) { + m_errorManager.AddError("Promise is rejected."); } - m_onReject(m_errorHandler.GetErrors()); + m_onReject(m_errorManager); }); } @@ -90,7 +92,7 @@ struct DBStorage { } private: - ErrorHandler m_errorHandler; + ErrorManager m_errorManager; std::atomic_flag m_isCompleted = ATOMIC_FLAG_INIT; TOnResolve m_onResolve; TOnReject m_onReject; @@ -98,7 +100,7 @@ struct DBStorage { // An asynchronous task that run in a background thread. struct DBTask { - DBTask(ErrorHandler &errorHandler, + DBTask(ErrorManager &errorManager, std::function &&onRun) noexcept; DBTask() = default; @@ -119,12 +121,12 @@ struct DBStorage { private: std::function m_onRun; - ErrorHandler &m_errorHandler; + ErrorManager &m_errorManager; }; using DatabasePtr = std::unique_ptr; - std::optional InitializeStorage(DBStorage::ErrorHandler &errorHandler) noexcept; + std::optional InitializeStorage(ErrorManager &errorManager) noexcept; ~DBStorage(); template @@ -135,8 +137,8 @@ struct DBStorage { std::forward(onReject)); } - void AddTask(ErrorHandler &errorHandler, - std::function &&onRun) noexcept; + void AddTask(ErrorManager &errorManager, + std::function &&onRun) noexcept; winrt::Windows::Foundation::IAsyncAction RunTasks() noexcept; diff --git a/windows/code/RNCAsyncStorage.h b/windows/code/RNCAsyncStorage.h index 4aa9eb6d..64e3c8bf 100644 --- a/windows/code/RNCAsyncStorage.h +++ b/windows/code/RNCAsyncStorage.h @@ -20,9 +20,11 @@ namespace winrt::ReactNativeAsyncStorage::implementation [callback](const std::vector &result) { callback({}, result); }, - [callback](const std::vector &errors) { callback(errors, {}); }); + [callback](const DBStorage::ErrorManager &errorManager) { + callback(errorManager.GetErrorList(), {}); + }); m_dbStorage.AddTask( - promise->GetErrorHandler(), + promise->GetErrorManager(), [promise, keys = std::move(keys)](DBStorage::DBTask &task, sqlite3 *db) noexcept { promise->ResolveOrReject(task.MultiGet(db, keys)); }); @@ -33,10 +35,12 @@ namespace winrt::ReactNativeAsyncStorage::implementation std::vector &&keyValues, std::function &errors)> &&callback) noexcept { - auto promise = DBStorage::CreatePromise( - [callback](bool /*value*/) { callback({}); }, - [callback](const std::vector &errors) { callback(errors); }); - m_dbStorage.AddTask(promise->GetErrorHandler(), + auto promise = + DBStorage::CreatePromise([callback](bool /*value*/) { callback({}); }, + [callback](const DBStorage::ErrorManager &errorManager) { + callback(errorManager.GetErrorList()); + }); + m_dbStorage.AddTask(promise->GetErrorManager(), [promise, keyValues = std::move(keyValues)](DBStorage::DBTask &task, sqlite3 *db) noexcept { promise->ResolveOrReject(task.MultiSet(db, keyValues)); @@ -48,10 +52,12 @@ namespace winrt::ReactNativeAsyncStorage::implementation std::vector &&keyValues, std::function &errors)> &&callback) noexcept { - auto promise = DBStorage::CreatePromise( - [callback](bool /*value*/) { callback({}); }, - [callback](const std::vector &errors) { callback(errors); }); - m_dbStorage.AddTask(promise->GetErrorHandler(), + auto promise = + DBStorage::CreatePromise([callback](bool /*value*/) { callback({}); }, + [callback](const DBStorage::ErrorManager &errorManager) { + callback(errorManager.GetErrorList()); + }); + m_dbStorage.AddTask(promise->GetErrorManager(), [promise, keyValues = std::move(keyValues)](DBStorage::DBTask &task, sqlite3 *db) noexcept { promise->ResolveOrReject(task.MultiMerge(db, keyValues)); @@ -63,11 +69,13 @@ namespace winrt::ReactNativeAsyncStorage::implementation std::vector &&keys, std::function &errors)> &&callback) noexcept { - auto promise = DBStorage::CreatePromise( - [callback](bool /*value*/) { callback({}); }, - [callback](const std::vector &errors) { callback(errors); }); + auto promise = + DBStorage::CreatePromise([callback](bool /*value*/) { callback({}); }, + [callback](const DBStorage::ErrorManager &errorManager) { + callback(errorManager.GetErrorList()); + }); m_dbStorage.AddTask( - promise->GetErrorHandler(), + promise->GetErrorManager(), [promise, keys = std::move(keys)](DBStorage::DBTask &task, sqlite3 *db) noexcept { promise->ResolveOrReject(task.MultiRemove(db, keys)); }); @@ -80,10 +88,10 @@ namespace winrt::ReactNativeAsyncStorage::implementation { auto promise = DBStorage::CreatePromise( [callback](const std::vector &keys) { callback(std::nullopt, keys); }, - [callback](const std::vector &errors) { - callback(errors.at(0), {}); + [callback](const DBStorage::ErrorManager &errorManager) { + callback(errorManager.GetCombinedError(), {}); }); - m_dbStorage.AddTask(promise->GetErrorHandler(), + m_dbStorage.AddTask(promise->GetErrorManager(), [promise](DBStorage::DBTask &task, sqlite3 *db) noexcept { promise->ResolveOrReject(task.GetAllKeys(db)); }); @@ -95,10 +103,10 @@ namespace winrt::ReactNativeAsyncStorage::implementation { auto promise = DBStorage::CreatePromise([callback](bool /*value*/) { callback(std::nullopt); }, - [callback](const std::vector &errors) { - callback(errors.at(0)); + [callback](const DBStorage::ErrorManager &errorManager) { + callback(errorManager.GetCombinedError()); }); - m_dbStorage.AddTask(promise->GetErrorHandler(), + m_dbStorage.AddTask(promise->GetErrorManager(), [promise](DBStorage::DBTask &task, sqlite3 *db) noexcept { promise->ResolveOrReject(task.RemoveAll(db)); }); From d79a0dd9f1a0e2c5a52cc851fb241e3a71e87050 Mon Sep 17 00:00:00 2001 From: Vladimir Morozov Date: Mon, 29 Nov 2021 18:39:32 -0800 Subject: [PATCH 6/6] Fix ReadValue for KeyValue per PR feedback --- windows/code/DBStorage.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/windows/code/DBStorage.cpp b/windows/code/DBStorage.cpp index cf415e43..fa34cfd2 100644 --- a/windows/code/DBStorage.cpp +++ b/windows/code/DBStorage.cpp @@ -574,6 +574,9 @@ void ReadValue(const winrt::IJSValueReader &reader, } ++index; } + } else { + // To keep reader in a good state. + winrt::SkipValue(reader); } }