From b4ddc698538cfa0bbb8191fb006d2c4e1c7f52a9 Mon Sep 17 00:00:00 2001 From: lianhuix Date: Fri, 5 May 2023 17:22:12 +0800 Subject: [PATCH 1/4] fix code reviews Signed-off-by: lianhuix --- .../data_share/gaussdb_rd_simple/BUILD.gn | 2 +- .../src/common/include/os_api.h | 5 +- .../src/common/src/collection_option.cpp | 19 +++--- .../src/common/src/db_config.cpp | 37 +++++++----- .../src/common/src/json_common.cpp | 2 +- .../src/executor/document/document_check.cpp | 21 ++----- .../include/document_store_manager.h | 3 +- .../src/interface/src/document_store.cpp | 58 +++++++++++++------ .../interface/src/document_store_manager.cpp | 36 ++++-------- .../src/interface/src/result_set.cpp | 8 +-- .../oh_adapter/include/kv_store_executor.h | 4 ++ .../src/sqlite_store_executor_impl.cpp | 35 ++++++++--- .../src/sqlite_store_executor_impl.h | 4 ++ .../src/oh_adapter/src/sqlite_utils.cpp | 2 +- .../gaussdb_rd_simple/test/unittest/BUILD.gn | 2 +- .../unittest/api/documentdb_find_test.cpp | 28 ++++----- 16 files changed, 154 insertions(+), 112 deletions(-) diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/BUILD.gn b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/BUILD.gn index 53e16705..6eab0b5d 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/BUILD.gn +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/BUILD.gn @@ -1,4 +1,4 @@ -# Copyright (c) 2021 Huawei Device Co., Ltd. +# Copyright (c) 2023 Huawei Device Co., Ltd. # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/os_api.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/os_api.h index 6bfbf3bb..f2c34f16 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/os_api.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/os_api.h @@ -12,10 +12,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include - #ifndef OS_API_H #define OS_API_H + +#include + namespace DocumentDB { namespace OSAPI { bool CheckPermission(const std::string &filePath); diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/collection_option.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/collection_option.cpp index c15c3895..eba7ec4c 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/collection_option.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/collection_option.cpp @@ -16,6 +16,7 @@ #include "collection_option.h" #include +#include #include #include "doc_errno.h" @@ -24,26 +25,23 @@ namespace DocumentDB { namespace { -const std::string OPT_MAX_DOC = "maxdoc"; -const std::vector DB_CONFIG = { - OPT_MAX_DOC, -}; +constexpr const char *OPT_MAX_DOC = "maxdoc"; -bool CheckConfigSupport(const JsonObject &config, int &errCode) +int CheckConfigValid(const JsonObject &config) { JsonObject child = config.GetChild(); while (!child.IsNull()) { std::string fieldName = child.GetItemFiled(); - if (std::find(DB_CONFIG.begin(), DB_CONFIG.end(), fieldName) == DB_CONFIG.end()) { + if (strcmp(OPT_MAX_DOC, fieldName.c_str()) != 0) { GLOGE("Invalid collection config."); - errCode = -E_INVALID_CONFIG_VALUE; - return false; + return -E_INVALID_CONFIG_VALUE; } child = child.GetNext(); } - return true; + return E_OK; } } // namespace + CollectionOption CollectionOption::ReadOption(const std::string &optStr, int &errCode) { if (optStr.empty()) { @@ -61,7 +59,8 @@ CollectionOption CollectionOption::ReadOption(const std::string &optStr, int &er return {}; } - if (!CheckConfigSupport(collOpt, errCode)) { + errCode = CheckConfigValid(collOpt); + if (errCode != E_OK) { GLOGE("Check collection option, not support config item. %d", errCode); return {}; } diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp index 9af67794..953ef32d 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp @@ -16,6 +16,7 @@ #include "db_config.h" #include +#include #include #include "doc_errno.h" @@ -32,14 +33,15 @@ const int MAX_CONNECTION_NUM = 1024; const int MIN_BUFFER_POOL_SIZE = 1024; const int MAX_BUFFER_POOL_SIZE = 4 * 1024 * 1024; -const std::string DB_CONFIG_PAGESIZE = "pagesize"; -const std::string DB_CONFIG_REDO_FLUSH_BY_TRX = "redoflushbytrx"; -const std::string DB_CONFIG_REDO_PUB_BUFF_SIZE = "redopubbufsize"; -const std::string DB_CONFIG_MAX_CONN_NUM = "maxconnnum"; -const std::string DB_CONFIG_BUFFER_POOL_SIZE = "bufferpoolsize"; -const std::string DB_CONFIG_CRC_CHECK_ENABLE = "crccheckenable"; +constexpr const char *DB_CONFIG_PAGESIZE = "pagesize"; +constexpr const char *DB_CONFIG_REDO_FLUSH_BY_TRX = "redoflushbytrx"; +constexpr const char *DB_CONFIG_REDO_PUB_BUFF_SIZE = "redopubbufsize"; +constexpr const char *DB_CONFIG_MAX_CONN_NUM = "maxconnnum"; +constexpr const char *DB_CONFIG_BUFFER_POOL_SIZE = "bufferpoolsize"; +constexpr const char *DB_CONFIG_CRC_CHECK_ENABLE = "crccheckenable"; -const std::vector DB_CONFIG = { DB_CONFIG_PAGESIZE, DB_CONFIG_REDO_FLUSH_BY_TRX, +const int DB_CONFIG_SIZE = 6; // db config size +const char *DB_CONFIG[DB_CONFIG_SIZE] = { DB_CONFIG_PAGESIZE, DB_CONFIG_REDO_FLUSH_BY_TRX, DB_CONFIG_REDO_PUB_BUFF_SIZE, DB_CONFIG_MAX_CONN_NUM, DB_CONFIG_BUFFER_POOL_SIZE, DB_CONFIG_CRC_CHECK_ENABLE }; bool CheckPageSizeConfig(const JsonObject &config, int32_t &pageSize, int &errCode) @@ -188,19 +190,27 @@ bool CheckCrcCheckEnableConfig(const JsonObject &config, uint32_t &crcCheckEnabl return true; } -bool CheckConfigSupport(const JsonObject &config, int &errCode) +int CheckConfigValid(const JsonObject &config) { JsonObject child = config.GetChild(); while (!child.IsNull()) { std::string fieldName = child.GetItemFiled(); - if (std::find(DB_CONFIG.begin(), DB_CONFIG.end(), fieldName) == DB_CONFIG.end()) { + bool isSupport = false; + for (int i = 0; i < DB_CONFIG_SIZE; i++) { + if (strcmp(DB_CONFIG[i], fieldName.c_str()) == 0) { + isSupport = true; + break; + } + } + + if (!isSupport) { GLOGE("Invalid db config."); - errCode = -E_INVALID_CONFIG_VALUE; - return false; + return -E_INVALID_CONFIG_VALUE; } + child = child.GetNext(); } - return true; + return E_OK; } } // namespace @@ -227,7 +237,8 @@ DBConfig DBConfig::ReadConfig(const std::string &confStr, int &errCode) return {}; } - if (!CheckConfigSupport(dbConfig, errCode)) { + errCode = CheckConfigValid(dbConfig); + if (errCode != E_OK) { GLOGE("Check DB config, not support config item. %d", errCode); return {}; } diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/json_common.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/json_common.cpp index c213f858..a2aa4b8d 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/json_common.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/json_common.cpp @@ -526,7 +526,7 @@ int JsonCommon::Append(const JsonObject &src, const JsonObject &add, bool isRepl if (!isCollapse) { bool ret = JsonValueReplace(src, fatherPath, father, item, externErrCode); if (!ret) { - GLOGE("replace faild"); + GLOGE("replace failed"); return false; } isAddedFlag = true; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/executor/document/document_check.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/executor/document/document_check.cpp index 285036fc..50f43ff8 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/executor/document/document_check.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/executor/document/document_check.cpp @@ -26,8 +26,8 @@ namespace { constexpr const char *KEY_ID = "_id"; constexpr const char *COLLECTION_PREFIX_GRD = "GRD_"; constexpr const char *COLLECTION_PREFIX_GM_SYS = "GM_SYS_"; -const int MAX_COLLECTION_NAME = 511; -const int MAX_ID_LENS = 899; +const int MAX_COLLECTION_NAME = 512; +const int MAX_ID_LENS = 900; const int JSON_DEEP_MAX = 4; bool CheckCollectionNamePrefix(const std::string &name, const std::string &prefix) @@ -36,16 +36,7 @@ bool CheckCollectionNamePrefix(const std::string &name, const std::string &prefi return false; } - auto itPrefix = prefix.begin(); - auto itName = name.begin(); - while (itPrefix != prefix.end()) { - if (std::tolower(*itPrefix) != std::tolower(*itName)) { - return false; - } - itPrefix++; - itName++; - } - return true; + return (strncasecmp(name.c_str(), prefix.c_str(), prefix.length()) == 0); } } // namespace @@ -55,7 +46,7 @@ bool CheckCommon::CheckCollectionName(const std::string &collectionName, std::st errCode = -E_INVALID_ARGS; return false; } - if (collectionName.length() > MAX_COLLECTION_NAME) { + if (collectionName.length() + 1 > MAX_COLLECTION_NAME) { // with '\0' errCode = -E_OVER_LIMIT; return false; } @@ -195,7 +186,7 @@ int CheckCommon::CheckIdFormat(JsonObject &filterJson) if (idValue.GetValueType() != ValueObject::ValueType::VALUE_STRING) { return -E_INVALID_ARGS; } - if (idValue.GetStringValue().length() > MAX_ID_LENS) { + if (idValue.GetStringValue().length() + 1 > MAX_ID_LENS) { // with '\0 return -E_OVER_LIMIT; } return E_OK; @@ -211,7 +202,7 @@ int CheckCommon::CheckIdFormat(JsonObject &filterJson, bool &isIdExisit) if (idValue.GetValueType() != ValueObject::ValueType::VALUE_STRING) { return -E_INVALID_ARGS; } - if (idValue.GetStringValue().length() > MAX_ID_LENS) { + if (idValue.GetStringValue().length() + 1 > MAX_ID_LENS) { // with '\0' return -E_OVER_LIMIT; } return E_OK; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h index 193118f4..fb394ac3 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h @@ -29,8 +29,7 @@ public: static int CloseDocumentStore(DocumentStore *store, unsigned int flags); private: - static bool CheckDBPath(const std::string &path, std::string &canonicalPath, std::string &dbName, int &errCode); - static bool CheckDBConfig(const std::string &config, int &errCode); + static int CheckDBPath(const std::string &path, std::string &canonicalPath, std::string &dbName); }; } // namespace DocumentDB #endif // DOCUMENT_STORE_MANAGER_H \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp index 210316d8..a3bc283e 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp @@ -59,13 +59,19 @@ int DocumentStore::CreateCollection(const std::string &name, const std::string & } std::lock_guard lock(dbMutex_); + errCode = executor_->StartTransaction(); + if (errCode != E_OK) { + return errCode; + } + + std::string oriOptStr; bool ignoreExists = (flags != CHK_EXIST_COLLECTION); errCode = executor_->CreateCollection(lowerCaseName, ignoreExists); if (errCode != E_OK) { GLOGE("Create collection failed. %d", errCode); - return errCode; + goto END; } - std::string oriOptStr; + errCode = executor_->GetCollectionOption(lowerCaseName, oriOptStr); if (errCode == -E_NOT_FOUND) { executor_->SetCollectionOption(lowerCaseName, collOption.ToString()); @@ -74,10 +80,16 @@ int DocumentStore::CreateCollection(const std::string &name, const std::string & CollectionOption oriOption = CollectionOption::ReadOption(oriOptStr, errCode); if (collOption != oriOption) { GLOGE("Create collection failed, option changed."); - return -E_INVALID_CONFIG_VALUE; + errCode = -E_INVALID_CONFIG_VALUE; } } +END: + if (errCode == E_OK) { + executor_->Commit(); + } else { + executor_->Rollback(); + } return errCode; } @@ -97,19 +109,31 @@ int DocumentStore::DropCollection(const std::string &name, int flags) bool ignoreNonExists = (flags != CHK_NON_EXIST_COLLECTION); std::lock_guard lock(dbMutex_); + errCode = executor_->StartTransaction(); + if (errCode != E_OK) { + return errCode; + } + errCode = executor_->DropCollection(lowerCaseName, ignoreNonExists); if (errCode != E_OK) { GLOGE("Drop collection failed. %d", errCode); - return errCode; + goto END; } errCode = executor_->CleanCollectionOption(lowerCaseName); if (errCode != E_OK && errCode != -E_NO_DATA) { GLOGE("Clean collection option failed. %d", errCode); - return errCode; + } else { + errCode = E_OK; } - return E_OK; +END: + if (errCode == E_OK) { + executor_->Commit(); + } else { + executor_->Rollback(); + } + return errCode; } int DocumentStore::UpdateDocument(const std::string &collection, const std::string &filter, const std::string &update, @@ -123,14 +147,14 @@ int DocumentStore::UpdateDocument(const std::string &collection, const std::stri } JsonObject updateObj = JsonObject::Parse(update, errCode, true); if (errCode != E_OK) { - GLOGE("update Parsed faild"); + GLOGE("update Parsed failed"); return errCode; } std::vector> allPath; if (update != "{}") { allPath = JsonCommon::ParsePath(updateObj, errCode); if (errCode != E_OK) { - GLOGE("updateObj ParsePath faild"); + GLOGE("updateObj ParsePath failed"); return errCode; } if (!CheckCommon::CheckUpdata(updateObj, allPath)) { @@ -144,13 +168,13 @@ int DocumentStore::UpdateDocument(const std::string &collection, const std::stri } JsonObject filterObj = JsonObject::Parse(filter, errCode, caseSensitive); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } std::vector> filterAllPath; filterAllPath = JsonCommon::ParsePath(filterObj, errCode); if (errCode != E_OK) { - GLOGE("filter ParsePath faild"); + GLOGE("filter ParsePath failed"); return errCode; } bool isOnlyId = true; @@ -204,7 +228,7 @@ int DocumentStore::UpsertDocument(const std::string &collection, const std::stri } JsonObject documentObj = JsonObject::Parse(document, errCode, true); if (errCode != E_OK) { - GLOGE("document Parsed faild"); + GLOGE("document Parsed failed"); return errCode; } std::vector> allPath; @@ -224,7 +248,7 @@ int DocumentStore::UpsertDocument(const std::string &collection, const std::stri } JsonObject filterObj = JsonObject::Parse(filter, errCode, caseSensitive); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } std::vector> filterAllPath; @@ -292,7 +316,7 @@ int DocumentStore::InsertDocument(const std::string &collection, const std::stri } JsonObject documentObj = JsonObject::Parse(document, errCode, caseSensitive); if (errCode != E_OK) { - GLOGE("Document Parsed faild"); + GLOGE("Document Parsed failed"); return errCode; } errCode = CheckCommon::CheckDocument(documentObj); @@ -334,7 +358,7 @@ int DocumentStore::DeleteDocument(const std::string &collection, const std::stri } JsonObject filterObj = JsonObject::Parse(filter, errCode, caseSensitive); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } std::vector> filterAllPath; @@ -390,7 +414,7 @@ int DocumentStore::FindDocument(const std::string &collection, const std::string } JsonObject filterObj = JsonObject::Parse(filter, errCode, caseSensitive); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } std::vector> filterAllPath; @@ -409,7 +433,7 @@ int DocumentStore::FindDocument(const std::string &collection, const std::string } JsonObject projectionObj = JsonObject::Parse(projection, errCode, caseSensitive); if (errCode != E_OK) { - GLOGE("projection Parsed faild"); + GLOGE("projection Parsed failed"); return errCode; } bool viewType = false; @@ -424,7 +448,7 @@ int DocumentStore::FindDocument(const std::string &collection, const std::string return -E_INVALID_ARGS; } if (GetViewType(projectionObj, viewType) != E_OK) { - GLOGE("GetViewType faild"); + GLOGE("GetViewType failed"); return -E_INVALID_ARGS; } } diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp index 40c6c541..4a36e6ab 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp @@ -50,8 +50,8 @@ int DocumentStoreManager::GetDocumentStore(const std::string &path, const std::s { std::string canonicalPath; std::string dbName; - int errCode = E_OK; - if (!CheckDBPath(path, canonicalPath, dbName, errCode)) { + int errCode = CheckDBPath(path, canonicalPath, dbName); + if (errCode != E_OK) { GLOGE("Check document db file path failed."); return errCode; } @@ -81,12 +81,10 @@ int DocumentStoreManager::GetDocumentStore(const std::string &path, const std::s store = new (std::nothrow) DocumentStore(executor); if (store == nullptr) { + delete executor; GLOGE("Memory allocation failed!"); return -E_FAILED_MEMORY_ALLOCATE; } - if (store == nullptr) { - return -E_OUT_OF_MEMORY; - } return errCode; } @@ -102,42 +100,32 @@ int DocumentStoreManager::CloseDocumentStore(DocumentStore *store, unsigned int return E_OK; } -bool DocumentStoreManager::CheckDBPath(const std::string &path, std::string &canonicalPath, std::string &dbName, - int &errCode) +int DocumentStoreManager::CheckDBPath(const std::string &path, std::string &canonicalPath, std::string &dbName) { if (path.empty()) { GLOGE("Invalid path empty"); - errCode = -E_INVALID_ARGS; - return false; + return -E_INVALID_ARGS; } if (path.back() == '/') { GLOGE("Invalid path end with slash"); - errCode = -E_INVALID_ARGS; - return false; + return -E_INVALID_ARGS; } std::string dirPath; OSAPI::SplitFilePath(path, dirPath, dbName); - int innerErrCode = OSAPI::GetRealPath(dirPath, canonicalPath); - if (innerErrCode != E_OK) { + int errCode = OSAPI::GetRealPath(dirPath, canonicalPath); + if (errCode != E_OK) { GLOGE("Get real path failed. %d", errCode); - errCode = -E_FILE_OPERATION; - return false; + return -E_FILE_OPERATION; } if (!OSAPI::CheckPermission(canonicalPath)) { - GLOGE("Check path permission failed. %d", errCode); - errCode = -E_FILE_OPERATION; - return false; + GLOGE("Check path permission failed."); + return -E_FILE_OPERATION; } - return true; -} - -bool DocumentStoreManager::CheckDBConfig(const std::string &config, int &errCode) -{ - return true; + return E_OK; } } // namespace DocumentDB \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/result_set.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/result_set.cpp index 72d5e23b..bcd511a8 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/result_set.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/result_set.cpp @@ -61,7 +61,7 @@ int ResultSet::GetNext() int errCode = 0; JsonObject filterObj = JsonObject::Parse(filter_, errCode, true); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } auto filterObjChild = filterObj.GetChild(); @@ -90,7 +90,7 @@ int ResultSet::GetNext() std::vector> values; JsonObject filterObj = JsonObject::Parse(filter_, errCode, true); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } errCode = coll.GetFilededDocument(filterObj, values); @@ -109,7 +109,7 @@ int ResultSet::GetNext() std::vector> values; JsonObject filterObj = JsonObject::Parse(filter_, errCode, true); if (errCode != E_OK) { - GLOGE("filter Parsed faild"); + GLOGE("filter Parsed failed"); return errCode; } errCode = coll.GetFilededDocument(filterObj, values); @@ -187,7 +187,7 @@ int ResultSet::CutJsonBranch(std::string &jsonData) int errCode; JsonObject cjsonObj = JsonObject::Parse(jsonData, errCode, true); if (errCode != E_OK) { - GLOGE("jsonData Parsed faild"); + GLOGE("jsonData Parsed failed"); return errCode; } std::vector> allCutPath; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h index 1051abe2..661923bc 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h @@ -25,6 +25,10 @@ class KvStoreExecutor { public: virtual ~KvStoreExecutor() = default; + virtual int StartTransaction() = 0; + virtual int Commit() = 0; + virtual int Rollback() = 0; + virtual int PutData(const std::string &collName, const Key &key, const Value &value) = 0; virtual int GetData(const std::string &collName, const Key &key, Value &value) const = 0; virtual int GetFilededData(const std::string &collName, const JsonObject &filterObj, diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp index 149c921f..bf6ffd09 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp @@ -40,6 +40,12 @@ int SqliteStoreExecutor::CreateDatabase(const std::string &path, const DBConfig goto END; } + errCode = SQLiteUtils::ExecSql(db, "PRAGMA journal_mode=WAL;"); + if (errCode != E_OK) { + GLOGE("Set db journal_mode failed. %d", errCode); + goto END; + } + errCode = SQLiteUtils::ExecSql(db, "CREATE TABLE IF NOT EXISTS grd_meta (key BLOB PRIMARY KEY, value BLOB);"); if (errCode != E_OK) { GLOGE("Create meta table failed. %d", errCode); @@ -80,6 +86,21 @@ int SqliteStoreExecutor::SetDBConfig(const std::string &config) return PutData("grd_meta", dbConfigKey, dbConfigVal); } +int SqliteStoreExecutor::StartTransaction() +{ + return SQLiteUtils::BeginTransaction(dbHandle_, TransactType::IMMEDIATE); +} + +int SqliteStoreExecutor::Commit() +{ + return SQLiteUtils::CommitTransaction(dbHandle_); +} + +int SqliteStoreExecutor:: Rollback() +{ + return SQLiteUtils::RollbackTransaction(dbHandle_); +} + int SqliteStoreExecutor::PutData(const std::string &collName, const Key &key, const Value &value) { if (dbHandle_ == nullptr) { @@ -94,7 +115,7 @@ int SqliteStoreExecutor::PutData(const std::string &collName, const Key &key, co return E_OK; }, nullptr); - if (errCode != SQLITE_OK) { + if (errCode != E_OK) { GLOGE("[sqlite executor] Put data failed. err=%d", errCode); if (errCode == -E_ERROR) { GLOGE("Cant find the collection"); @@ -124,7 +145,7 @@ int SqliteStoreExecutor::GetData(const std::string &collName, const Key &key, Va innerErrorCode = E_OK; return E_OK; }); - if (errCode != SQLITE_OK) { + if (errCode != E_OK) { GLOGE("[sqlite executor] Get data failed. err=%d", errCode); return errCode; } @@ -156,7 +177,7 @@ int SqliteStoreExecutor::GetFilededData(const std::string &collName, const JsonO int externErrCode; JsonObject srcObj = JsonObject::Parse(valueStr, externErrCode, true); if (externErrCode != E_OK) { - GLOGE("srcObj Parsed faild"); + GLOGE("srcObj Parsed failed"); return externErrCode; } if (JsonCommon::IsJsonNodeMatch(srcObj, filterObj, externErrCode)) { @@ -166,7 +187,7 @@ int SqliteStoreExecutor::GetFilededData(const std::string &collName, const JsonO innerErrorCode = E_OK; return E_OK; }); - if (errCode != SQLITE_OK) { + if (errCode != E_OK) { GLOGE("[sqlite executor] Get data failed. err=%d", errCode); return errCode; } @@ -195,7 +216,7 @@ int SqliteStoreExecutor::DelData(const std::string &collName, const Key &key) return E_OK; }, nullptr); - if (errCode != SQLITE_OK) { + if (errCode != E_OK) { GLOGE("[sqlite executor] Delete data failed. err=%d", errCode); if (errCode == -E_ERROR) { GLOGE("Cant find the collection"); @@ -225,7 +246,7 @@ int SqliteStoreExecutor::CreateCollection(const std::string &name, bool ignoreEx std::string sql = "CREATE TABLE IF NOT EXISTS '" + collName + "' (key BLOB PRIMARY KEY, value BLOB);"; int errCode = SQLiteUtils::ExecSql(dbHandle_, sql); - if (errCode != SQLITE_OK) { + if (errCode != E_OK) { GLOGE("[sqlite executor] Create collectoin failed. err=%d", errCode); return errCode; } @@ -253,7 +274,7 @@ int SqliteStoreExecutor::DropCollection(const std::string &name, bool ignoreNonE std::string sql = "DROP TABLE IF EXISTS '" + collName + "';"; int errCode = SQLiteUtils::ExecSql(dbHandle_, sql); - if (errCode != SQLITE_OK) { + if (errCode != E_OK) { GLOGE("[sqlite executor] Drop collectoin failed. err=%d", errCode); return errCode; } diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h index dfef5998..a3e78e26 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h @@ -32,6 +32,10 @@ public: int GetDBConfig(std::string &config); int SetDBConfig(const std::string &config); + int StartTransaction() override; + int Commit() override; + int Rollback() override; + int PutData(const std::string &collName, const Key &key, const Value &value) override; int GetData(const std::string &collName, const Key &key, Value &value) const override; int GetFilededData(const std::string &collName, const JsonObject &filterObj, diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_utils.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_utils.cpp index b8a2b552..76f2f887 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_utils.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_utils.cpp @@ -98,7 +98,7 @@ int SQLiteUtils::GetStatement(sqlite3 *db, const std::string &sql, sqlite3_stmt if (errCode != SQLITE_OK) { GLOGE("Prepare SQLite statement failed:%d", errCode); (void)SQLiteUtils::ResetStatement(statement, true); - return errCode; + return MapSqliteError(errCode); } if (statement == nullptr) { diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/BUILD.gn b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/BUILD.gn index 51d0a307..7345c5fd 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/BUILD.gn +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/BUILD.gn @@ -1,4 +1,4 @@ -# Copyright (c) 2021 Huawei Device Co., Ltd. +# Copyright (c) 2023 Huawei Device Co., Ltd. # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_find_test.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_find_test.cpp index 3691ff16..ab3cb15d 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_find_test.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_find_test.cpp @@ -169,7 +169,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest002, TestSize.Level1) { /** * @tc.steps: step1. Create filter with multiple and _id. and get the record according to filter condition. - * @tc.expected: step1. Faild to get the record, the result is GRD_INVALID_ARGS, GRD_GetValue return GRD_NOT_AVAILABLE and GRD_Next return GRD_NO_DATA. + * @tc.expected: step1. Failed to get the record, the result is GRD_INVALID_ARGS, GRD_GetValue return GRD_NOT_AVAILABLE and GRD_Next return GRD_NO_DATA. */ const char *filter = "{\"_id\" : \"6\", \"name\":\"doc6\"}"; GRD_ResultSet *resultSet = nullptr; @@ -197,7 +197,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) { /** * @tc.steps: step1. Create filter without _id and get the record according to filter condition. - * @tc.expected: step1. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step1. Failed to get the record, the result is GRD_INVALID_ARGS, */ const char *filter = "{\"name\":\"doc6\"}"; GRD_ResultSet *resultSet = nullptr; @@ -226,7 +226,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) // { // /** // * @tc.steps: step1. Create filter with _id and number -// * @tc.expected: step1. Faild to get the record, the result is GRD_INVALID_ARGS, +// * @tc.expected: step1. Failed to get the record, the result is GRD_INVALID_ARGS, // */ // GRD_ResultSet *resultSet1 = nullptr; // const char *filter1 = "{\"_id\" : \"1\", \"info\" : 1}"; @@ -236,7 +236,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) // /** // * @tc.steps: step2. Create filter with two _id -// * @tc.expected: step2. Faild to get the record, the result is GRD_INVALID_ARGS, +// * @tc.expected: step2. Failed to get the record, the result is GRD_INVALID_ARGS, // */ // GRD_ResultSet *resultSet2 = nullptr; // const char *filter2 = "{\"_id\" : \"1\", \"_id\" : \"2\"}"; @@ -246,7 +246,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) // /** // * @tc.steps: step3. Create filter with array and _id -// * @tc.expected: step3. Faild to get the record, the result is GRD_INVALID_ARGS, +// * @tc.expected: step3. Failed to get the record, the result is GRD_INVALID_ARGS, // */ // GRD_ResultSet *resultSet3 = nullptr; // const char *filter3 = "{\"_id\" : \"1\", \"info\" : [\"2\", 1]}"; @@ -255,7 +255,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) // /** // * @tc.steps: step4. Create filter with object and _id -// * @tc.expected: step4. Faild to get the record, the result is GRD_INVALID_ARGS, +// * @tc.expected: step4. Failed to get the record, the result is GRD_INVALID_ARGS, // */ // GRD_ResultSet *resultSet4 = nullptr; // const char *filter4 = "{\"_id\" : \"1\", \"info\" : {\"info_val\" : \"1\"}}"; @@ -264,7 +264,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) // /** // * @tc.steps: step5. Create filter with bool and _id -// * @tc.expected: step5. Faild to get the record, the result is GRD_INVALID_ARGS, +// * @tc.expected: step5. Failed to get the record, the result is GRD_INVALID_ARGS, // */ // GRD_ResultSet *resultSet5 = nullptr; // const char *filter5 = "{\"_id\" : \"1\", \"info\" : true}"; @@ -273,7 +273,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest004, TestSize.Level1) // /** // * @tc.steps: step6. Create filter with null and _id -// * @tc.expected: step6. Faild to get the record, the result is GRD_INVALID_ARGS, +// * @tc.expected: step6. Failed to get the record, the result is GRD_INVALID_ARGS, // */ // GRD_ResultSet *resultSet6 = nullptr; // const char *filter6 = "{\"_id\" : \"1\", \"info\" : null}"; @@ -292,7 +292,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest006, TestSize.Level1) { /** * @tc.steps: step1. Create filter with _id which value is string - * @tc.expected: step1. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step1. Failed to get the record, the result is GRD_INVALID_ARGS, */ GRD_ResultSet *resultSet1 = nullptr; const char *filter1 = "{\"_id\" : \"valstring\"}"; @@ -302,7 +302,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest006, TestSize.Level1) /** * @tc.steps: step2. Create filter with _id which value is number - * @tc.expected: step2. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step2. Failed to get the record, the result is GRD_INVALID_ARGS, */ GRD_ResultSet *resultSet2 = nullptr; const char *filter2 = "{\"_id\" : 1}"; @@ -311,7 +311,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest006, TestSize.Level1) /** * @tc.steps: step3. Create filter with _id which value is array - * @tc.expected: step3. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step3. Failed to get the record, the result is GRD_INVALID_ARGS, */ GRD_ResultSet *resultSet3 = nullptr; const char *filter3 = "{\"_id\" : [\"2\", 1]}"; @@ -320,7 +320,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest006, TestSize.Level1) /** * @tc.steps: step4. Create filter with _id which value is object - * @tc.expected: step4. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step4. Failed to get the record, the result is GRD_INVALID_ARGS, */ GRD_ResultSet *resultSet4 = nullptr; const char *filter4 = "{\"_id\" : {\"info_val\" : \"1\"}}"; @@ -329,7 +329,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest006, TestSize.Level1) /** * @tc.steps: step5. Create filter with _id which value is bool - * @tc.expected: step5. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step5. Failed to get the record, the result is GRD_INVALID_ARGS, */ GRD_ResultSet *resultSet5 = nullptr; const char *filter5 = "{\"_id\" : true}"; @@ -338,7 +338,7 @@ HWTEST_F(DocumentFindApiTest, DocumentFindApiTest006, TestSize.Level1) /** * @tc.steps: step6. Create filter with _id which value is null - * @tc.expected: step6. Faild to get the record, the result is GRD_INVALID_ARGS, + * @tc.expected: step6. Failed to get the record, the result is GRD_INVALID_ARGS, */ GRD_ResultSet *resultSet6 = nullptr; const char *filter6 = "{\"_id\" : null}"; -- Gitee From 9ac49f2195150cddc085bf9e215ce6e092429a6c Mon Sep 17 00:00:00 2001 From: lianhuix Date: Sat, 6 May 2023 16:23:31 +0800 Subject: [PATCH 2/4] Fix create collection check Signed-off-by: lianhuix --- .../src/interface/src/document_store.cpp | 14 +------- .../oh_adapter/include/kv_store_executor.h | 2 +- .../src/sqlite_store_executor_impl.cpp | 34 +++++++++++-------- .../src/sqlite_store_executor_impl.h | 2 +- .../api/documentdb_collection_test.cpp | 6 ++-- 5 files changed, 25 insertions(+), 33 deletions(-) diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp index a3bc283e..6be0d590 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp @@ -66,24 +66,12 @@ int DocumentStore::CreateCollection(const std::string &name, const std::string & std::string oriOptStr; bool ignoreExists = (flags != CHK_EXIST_COLLECTION); - errCode = executor_->CreateCollection(lowerCaseName, ignoreExists); + errCode = executor_->CreateCollection(lowerCaseName, oriOptStr, ignoreExists); if (errCode != E_OK) { GLOGE("Create collection failed. %d", errCode); goto END; } - errCode = executor_->GetCollectionOption(lowerCaseName, oriOptStr); - if (errCode == -E_NOT_FOUND) { - executor_->SetCollectionOption(lowerCaseName, collOption.ToString()); - errCode = E_OK; - } else { - CollectionOption oriOption = CollectionOption::ReadOption(oriOptStr, errCode); - if (collOption != oriOption) { - GLOGE("Create collection failed, option changed."); - errCode = -E_INVALID_CONFIG_VALUE; - } - } - END: if (errCode == E_OK) { executor_->Commit(); diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h index 661923bc..f608d3e6 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_executor.h @@ -35,7 +35,7 @@ public: std::vector> &values) const = 0; virtual int DelData(const std::string &collName, const Key &key) = 0; - virtual int CreateCollection(const std::string &name, bool ignoreExists) = 0; + virtual int CreateCollection(const std::string &name, const std::string &option, bool ignoreExists) = 0; virtual int DropCollection(const std::string &name, bool ignoreNonExists) = 0; virtual bool IsCollectionExists(const std::string &name, int &errCode) = 0; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp index bf6ffd09..a296965a 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.cpp @@ -226,30 +226,34 @@ int SqliteStoreExecutor::DelData(const std::string &collName, const Key &key) return errCode; } -int SqliteStoreExecutor::CreateCollection(const std::string &name, bool ignoreExists) +int SqliteStoreExecutor::CreateCollection(const std::string &name, const std::string &option, bool ignoreExists) { if (dbHandle_ == nullptr) { return -E_ERROR; } std::string collName = COLL_PREFIX + name; - if (!ignoreExists) { - int errCode = E_OK; - bool isExists = IsCollectionExists(collName, errCode); - if (errCode != E_OK) { - return errCode; - } - if (isExists) { - GLOGE("[sqlite executor] Create collectoin failed, collection already exists."); - return -E_COLLECTION_CONFLICT; - } + int errCode = E_OK; + bool isExists = IsCollectionExists(collName, errCode); + if (errCode != E_OK) { + return errCode; + } + + if (isExists) { + GLOGW("[sqlite executor] Create collection failed, collection already exists."); + return ignoreExists ? E_OK : -E_COLLECTION_CONFLICT; } std::string sql = "CREATE TABLE IF NOT EXISTS '" + collName + "' (key BLOB PRIMARY KEY, value BLOB);"; - int errCode = SQLiteUtils::ExecSql(dbHandle_, sql); + errCode = SQLiteUtils::ExecSql(dbHandle_, sql); if (errCode != E_OK) { - GLOGE("[sqlite executor] Create collectoin failed. err=%d", errCode); + GLOGE("[sqlite executor] Create collection failed. err=%d", errCode); return errCode; } + + errCode = SetCollectionOption(name, option); + if (errCode != E_OK) { + GLOGE("[sqlite executor] Set collection option failed. err=%d", errCode); + } return E_OK; } @@ -267,7 +271,7 @@ int SqliteStoreExecutor::DropCollection(const std::string &name, bool ignoreNonE return errCode; } if (!isExists) { - GLOGE("[sqlite executor] Drop collectoin failed, collection not exists."); + GLOGE("[sqlite executor] Drop collection failed, collection not exists."); return -E_INVALID_ARGS; } } @@ -275,7 +279,7 @@ int SqliteStoreExecutor::DropCollection(const std::string &name, bool ignoreNonE std::string sql = "DROP TABLE IF EXISTS '" + collName + "';"; int errCode = SQLiteUtils::ExecSql(dbHandle_, sql); if (errCode != E_OK) { - GLOGE("[sqlite executor] Drop collectoin failed. err=%d", errCode); + GLOGE("[sqlite executor] Drop collection failed. err=%d", errCode); return errCode; } return E_OK; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h index a3e78e26..bae50f8a 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/sqlite_store_executor_impl.h @@ -42,7 +42,7 @@ public: std::vector> &values) const override; int DelData(const std::string &collName, const Key &key) override; - int CreateCollection(const std::string &name, bool ignoreExists) override; + int CreateCollection(const std::string &name, const std::string &option, bool ignoreExists) override; int DropCollection(const std::string &name, bool ignoreNonExists) override; bool IsCollectionExists(const std::string &name, int &errCode) override; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_collection_test.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_collection_test.cpp index df17a4c9..f63fed02 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_collection_test.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_collection_test.cpp @@ -177,14 +177,14 @@ HWTEST_F(DocumentDBCollectionTest, CollectionTest005, TestSize.Level0) HWTEST_F(DocumentDBCollectionTest, CollectionTest006, TestSize.Level0) { EXPECT_EQ(GRD_CreateCollection(g_db, "student", R""({"maxDoc":1024})"", 0), GRD_OK); - - EXPECT_EQ(GRD_CreateCollection(g_db, "student", R""({"maxDoc":2048})"", 0), GRD_INVALID_ARGS); + EXPECT_EQ(GRD_CreateCollection(g_db, "student", R""({"maxDoc":2048})"", 0), GRD_OK); + EXPECT_EQ(GRD_CreateCollection(g_db, "student", R""({"maxDoc":2048})"", CHK_EXIST_COLLECTION), GRD_DATA_CONFLICT); EXPECT_EQ(GRD_DropCollection(g_db, "student", 0), GRD_OK); EXPECT_EQ(GRD_DropCollection(g_db, "student", 0), GRD_OK); EXPECT_EQ(GRD_DropCollection(g_db, "student", CHK_NON_EXIST_COLLECTION), GRD_INVALID_ARGS); - // Create collection with different option returnh OK after drop collection + // Create collection with different option return OK after drop collection EXPECT_EQ(GRD_CreateCollection(g_db, "student", R""({"maxDoc":2048})"", 0), GRD_OK); } -- Gitee From 455e843a72658140b667be487a11768a17dd40ec Mon Sep 17 00:00:00 2001 From: lianhuix Date: Sat, 6 May 2023 18:28:31 +0800 Subject: [PATCH 3/4] Fix open db with count Signed-off-by: lianhuix --- .../src/common/include/db_config.h | 2 ++ .../src/common/src/db_config.cpp | 12 ++++--- .../src/interface/include/document_store.h | 5 +++ .../include/document_store_manager.h | 5 +++ .../src/interface/src/document_store.cpp | 12 +++++++ .../interface/src/document_store_manager.cpp | 22 ++++++++++++- .../src/oh_adapter/include/kv_store_manager.h | 3 +- .../src/oh_adapter/src/kv_store_manager.cpp | 5 +-- .../test/unittest/api/documentdb_api_test.cpp | 31 ++++++++++++++++--- 9 files changed, 84 insertions(+), 13 deletions(-) diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/db_config.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/db_config.h index ec7d8f44..8e926bc8 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/db_config.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/include/db_config.h @@ -31,6 +31,8 @@ public: bool operator==(const DBConfig &targetConfig) const; bool operator!=(const DBConfig &targetConfig) const; + bool CheckPersistenceEqual(const DBConfig &targetConfig) const; + private: DBConfig() = default; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp index 953ef32d..e660ae3c 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/common/src/db_config.cpp @@ -291,14 +291,18 @@ int32_t DBConfig::GetPageSize() const bool DBConfig::operator==(const DBConfig &targetConfig) const { - return configStr_ == targetConfig.configStr_ && pageSize_ == targetConfig.pageSize_ && - redoFlushByTrx_ == targetConfig.redoFlushByTrx_ && redoPubBufSize_ == targetConfig.redoPubBufSize_ && - maxConnNum_ == targetConfig.maxConnNum_ && bufferPoolSize_ == targetConfig.bufferPoolSize_ && - crcCheckEnable_ == targetConfig.crcCheckEnable_; + return pageSize_ == targetConfig.pageSize_ && redoFlushByTrx_ == targetConfig.redoFlushByTrx_ && + redoPubBufSize_ == targetConfig.redoPubBufSize_ && maxConnNum_ == targetConfig.maxConnNum_ && + bufferPoolSize_ == targetConfig.bufferPoolSize_ && crcCheckEnable_ == targetConfig.crcCheckEnable_; } bool DBConfig::operator!=(const DBConfig &targetConfig) const { return !(*this == targetConfig); } + +bool DBConfig::CheckPersistenceEqual(const DBConfig &targetConfig) const +{ + return pageSize_ == targetConfig.pageSize_ && crcCheckEnable_ == targetConfig.crcCheckEnable_; +} } // namespace DocumentDB \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h index 5f20864b..ec22c97b 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h @@ -44,11 +44,16 @@ public: bool IsCollectionOpening(const std::string collection); int EraseCollection(const std::string collectionName); + void OnClose(const std::function ¬ifier); + + void Close(); + private: int GetViewType(JsonObject &jsonObj, bool &viewType); std::mutex dbMutex_; KvStoreExecutor *executor_ = nullptr; std::map collections_; + std::function closeNotifier_; }; } // namespace DocumentDB #endif // DOCUMENT_STORE_H \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h index fb394ac3..076c2791 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store_manager.h @@ -16,6 +16,8 @@ #ifndef DOCUMENT_STORE_MANAGER_H #define DOCUMENT_STORE_MANAGER_H +#include +#include #include #include "document_store.h" @@ -30,6 +32,9 @@ public: private: static int CheckDBPath(const std::string &path, std::string &canonicalPath, std::string &dbName); + + static std::mutex openCloseMutex_; + static std::map dbConnCount_; }; } // namespace DocumentDB #endif // DOCUMENT_STORE_MANAGER_H \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp index 6be0d590..56c60a1a 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp @@ -527,4 +527,16 @@ int DocumentStore::GetViewType(JsonObject &jsonObj, bool &viewType) } return E_OK; } + +void DocumentStore::OnClose(const std::function ¬ifier) +{ + closeNotifier_ = notifier; +} + +void DocumentStore::Close() +{ + if (closeNotifier_) { + closeNotifier_(); + } +} } // namespace DocumentDB \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp index 4a36e6ab..90a2e549 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp @@ -45,6 +45,9 @@ bool CheckDBCreate(unsigned int flags, const std::string &path) } } // namespace +std::mutex DocumentStoreManager::openCloseMutex_; +std::map DocumentStoreManager::dbConnCount_; + int DocumentStoreManager::GetDocumentStore(const std::string &path, const std::string &config, unsigned int flags, DocumentStore *&store) { @@ -72,8 +75,14 @@ int DocumentStoreManager::GetDocumentStore(const std::string &path, const std::s return -E_INVALID_ARGS; } + std::lock_guard lock(openCloseMutex_); + + std::string dbRealPath = canonicalPath + "/" + dbName; + auto it = dbConnCount_.find(dbRealPath); + bool isFirstOpen = (it == dbConnCount_.end() || it->second == 0); + KvStoreExecutor *executor = nullptr; - errCode = KvStoreManager::GetKvStore(canonicalPath + "/" + dbName, dbConfig, executor); + errCode = KvStoreManager::GetKvStore(dbRealPath, dbConfig, isFirstOpen, executor); if (errCode != E_OK) { GLOGE("Open document store failed. %d", errCode); return errCode; @@ -86,6 +95,15 @@ int DocumentStoreManager::GetDocumentStore(const std::string &path, const std::s return -E_FAILED_MEMORY_ALLOCATE; } + store->OnClose([dbRealPath]() { + dbConnCount_[dbRealPath]--; + }); + + if (isFirstOpen) { + dbConnCount_[dbRealPath] = 1; + } else { + dbConnCount_[dbRealPath]++; + } return errCode; } @@ -96,6 +114,8 @@ int DocumentStoreManager::CloseDocumentStore(DocumentStore *store, unsigned int return -E_INVALID_ARGS; } + std::lock_guard lock(openCloseMutex_); + store->Close(); delete store; return E_OK; } diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_manager.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_manager.h index 2a84cf89..fc71bbf7 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_manager.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/include/kv_store_manager.h @@ -24,7 +24,8 @@ namespace DocumentDB { class KvStoreManager { public: - static int GetKvStore(const std::string &path, const DBConfig &config, KvStoreExecutor *&executor); + static int GetKvStore(const std::string &path, const DBConfig &config, bool isFirstOpen, + KvStoreExecutor *&executor); }; } // namespace DocumentDB #endif // KV_STORE_MANAGER_H \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/kv_store_manager.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/kv_store_manager.cpp index 40fe20e5..32e89330 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/kv_store_manager.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/oh_adapter/src/kv_store_manager.cpp @@ -21,7 +21,8 @@ #include "sqlite_utils.h" namespace DocumentDB { -int KvStoreManager::GetKvStore(const std::string &path, const DBConfig &config, KvStoreExecutor *&executor) +int KvStoreManager::GetKvStore(const std::string &path, const DBConfig &config, bool isFirstOpen, + KvStoreExecutor *&executor) { if (executor != nullptr) { return -E_INVALID_ARGS; @@ -52,7 +53,7 @@ int KvStoreManager::GetKvStore(const std::string &path, const DBConfig &config, GLOGE("Read db config failed. %d", errCode); goto END; } - if (config != oriDbConfig) { + if ((isFirstOpen ? (!config.CheckPersistenceEqual(oriDbConfig)) : (config != oriDbConfig))) { errCode = -E_INVALID_CONFIG_VALUE; GLOGE("Get kv store failed, db config changed. %d", errCode); goto END; diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_api_test.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_api_test.cpp index a943af5a..70c159de 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_api_test.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/test/unittest/api/documentdb_api_test.cpp @@ -103,6 +103,27 @@ HWTEST_F(DocumentDBApiTest, OpenDBTest002, TestSize.Level0) EXPECT_EQ(status, GRD_OK); } +HWTEST_F(DocumentDBApiTest, OpenDBTest003, TestSize.Level0) +{ + std::string path = "./document.db"; + GRD_DB *db = nullptr; + int status = GRD_DBOpen(path.c_str(), nullptr, GRD_DB_OPEN_CREATE, &db); + EXPECT_EQ(status, GRD_OK); + EXPECT_NE(db, nullptr); + GLOGD("Open DB test 003: status: %d", status); + + GRD_DB *db1 = nullptr; + status = GRD_DBOpen(path.c_str(), nullptr, GRD_DB_OPEN_CREATE, &db1); + EXPECT_EQ(status, GRD_OK); + EXPECT_NE(db1, nullptr); + + status = GRD_DBClose(db, GRD_DB_CLOSE); + EXPECT_EQ(status, GRD_OK); + + status = GRD_DBClose(db1, GRD_DB_CLOSE); + EXPECT_EQ(status, GRD_OK); +} + /** * @tc.name: OpenDBPathTest001 * @tc.desc: Test open document db with NULL path @@ -253,14 +274,14 @@ HWTEST_F(DocumentDBApiTest, OpenDBConfigMaxConnNumTest003, TestSize.Level1) int status = GRD_DBOpen(path.c_str(), config.c_str(), GRD_DB_OPEN_CREATE, &db); EXPECT_EQ(status, GRD_OK); - status = GRD_DBClose(db, GRD_DB_CLOSE); - EXPECT_EQ(status, GRD_OK); - db = nullptr; - + GRD_DB *db1 = nullptr; config = R""({"maxConnNum":17})""; - status = GRD_DBOpen(path.c_str(), config.c_str(), GRD_DB_OPEN_CREATE, &db); + status = GRD_DBOpen(path.c_str(), config.c_str(), GRD_DB_OPEN_CREATE, &db1); EXPECT_EQ(status, GRD_INVALID_ARGS); + status = GRD_DBClose(db, GRD_DB_CLOSE); + EXPECT_EQ(status, GRD_OK); + db = nullptr; DocumentDBTestUtils::RemoveTestDbFiles(path); } -- Gitee From 445ac3c09002dbf87b70abd9a76c8b6b6647d781 Mon Sep 17 00:00:00 2001 From: lianhuix Date: Mon, 8 May 2023 11:22:03 +0800 Subject: [PATCH 4/4] Fix db close with resultSet Signed-off-by: lianhuix --- .../src/interface/include/document_store.h | 2 +- .../src/interface/src/document_store.cpp | 13 ++++++++++++- .../src/interface/src/document_store_manager.cpp | 7 ++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h index ec22c97b..2c54187b 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/include/document_store.h @@ -46,7 +46,7 @@ public: void OnClose(const std::function ¬ifier); - void Close(); + int Close(int flags); private: int GetViewType(JsonObject &jsonObj, bool &viewType); diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp index 56c60a1a..72b72961 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store.cpp @@ -462,14 +462,17 @@ int DocumentStore::FindDocument(const std::string &collection, const std::string bool DocumentStore::IsCollectionOpening(const std::string collection) { + std::lock_guard lock(dbMutex_); if (collections_.find(collection) != collections_.end()) { GLOGE("DB is resource busy"); return true; } return false; } + int DocumentStore::EraseCollection(const std::string collectionName) { + std::lock_guard lock(dbMutex_); if (collections_.find(collectionName) != collections_.end()) { collections_.erase(collectionName); return E_OK; @@ -477,6 +480,7 @@ int DocumentStore::EraseCollection(const std::string collectionName) GLOGE("erase collection failed"); return E_INVALID_ARGS; } + int DocumentStore::GetViewType(JsonObject &jsonObj, bool &viewType) { auto leafValue = JsonCommon::GetLeafValue(jsonObj); @@ -533,10 +537,17 @@ void DocumentStore::OnClose(const std::function ¬ifier) closeNotifier_ = notifier; } -void DocumentStore::Close() +int DocumentStore::Close(int flags) { + std::lock_guard lock(dbMutex_); + if (flags == GRD_DB_CLOSE && !collections_.empty()) { + GLOGE("Close store failed with result set not closed."); + return -E_RESOURCE_BUSY; + } + if (closeNotifier_) { closeNotifier_(); } + return E_OK; } } // namespace DocumentDB \ No newline at end of file diff --git a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp index 90a2e549..b52e9993 100644 --- a/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp +++ b/services/distributeddataservice/service/data_share/gaussdb_rd_simple/src/interface/src/document_store_manager.cpp @@ -115,7 +115,12 @@ int DocumentStoreManager::CloseDocumentStore(DocumentStore *store, unsigned int } std::lock_guard lock(openCloseMutex_); - store->Close(); + int errCode = store->Close(flags); + if (errCode != E_OK) { + GLOGE("Close document store failed. %d", errCode); + return errCode; + } + delete store; return E_OK; } -- Gitee