From 7b040f4e6123061b02a0266b7d3fc64660173735 Mon Sep 17 00:00:00 2001 From: lianhuix Date: Sat, 5 Mar 2022 16:34:01 +0800 Subject: [PATCH 1/2] Fix code reviews Signed-off-by: lianhuix --- .../distributeddb/common/include/db_ability.h | 2 +- .../distributeddb/common/include/ischema.h | 8 ++++--- .../distributeddb/common/src/auto_launch.cpp | 2 +- .../distributeddb/common/src/db_ability.cpp | 2 +- .../relational/relational_store_instance.h | 2 +- .../relational/relational_sync_able_storage.h | 2 +- .../storage/src/data_transformer.cpp | 21 ++++++------------- .../storage/src/data_transformer.h | 8 +++---- .../storage/src/relational_store_instance.cpp | 5 +++-- .../src/relational_sync_able_storage.cpp | 2 +- .../sqlite_relational_store_connection.cpp | 7 ------- .../sqlite_relational_store_connection.h | 1 - ...single_ver_relational_storage_executor.cpp | 5 ++--- .../storage/src/sqlite/sqlite_utils.cpp | 8 +++---- 14 files changed, 30 insertions(+), 45 deletions(-) diff --git a/services/distributeddataservice/libs/distributeddb/common/include/db_ability.h b/services/distributeddataservice/libs/distributeddb/common/include/db_ability.h index 75466cfec..16c5abcb5 100644 --- a/services/distributeddataservice/libs/distributeddb/common/include/db_ability.h +++ b/services/distributeddataservice/libs/distributeddb/common/include/db_ability.h @@ -45,7 +45,7 @@ public: uint32_t GetAbilityBitsLen() const; - uint8_t GetAbilityItem(const AbilityItem abilityType) const; + uint8_t GetAbilityItem(const AbilityItem &abilityType) const; int SetAbilityItem(const AbilityItem &abilityType, uint8_t data); private: diff --git a/services/distributeddataservice/libs/distributeddb/common/include/ischema.h b/services/distributeddataservice/libs/distributeddb/common/include/ischema.h index dffc432ba..32f69fee4 100644 --- a/services/distributeddataservice/libs/distributeddb/common/include/ischema.h +++ b/services/distributeddataservice/libs/distributeddb/common/include/ischema.h @@ -13,11 +13,13 @@ * limitations under the License. */ -#ifndef SCHEMA_H -#define SCHEMA_H +#ifndef I_SCHEMA_H +#define I_SCHEMA_H #include +#include "db_types.h" + namespace DistributedDB { // SchemaType::NONE represent for KV database which do not have schema. Only invalid SchemaObject is NONE type. // Enum value must not be changed except SchemaType::UNRECOGNIZED. @@ -69,4 +71,4 @@ public: virtual std::string ToSchemaString() const = 0; }; } -#endif // SCHEMA_H \ No newline at end of file +#endif // I_SCHEMA_H \ No newline at end of file diff --git a/services/distributeddataservice/libs/distributeddb/common/src/auto_launch.cpp b/services/distributeddataservice/libs/distributeddb/common/src/auto_launch.cpp index 0c1277b48..7b4440fa0 100755 --- a/services/distributeddataservice/libs/distributeddb/common/src/auto_launch.cpp +++ b/services/distributeddataservice/libs/distributeddb/common/src/auto_launch.cpp @@ -522,7 +522,7 @@ void AutoLaunch::ConnectionLifeCycleCallbackTask(const std::string &identifier, autoLaunchItem = autoLaunchItemMap_[identifier][userId]; } LOGI("[AutoLaunch] ConnectionLifeCycleCallbackTask do CloseConnection"); - TryCloseConnection(autoLaunchItem); // do onthing if failed + TryCloseConnection(autoLaunchItem); // do nothing if failed LOGI("[AutoLaunch] ConnectionLifeCycleCallback do CloseConnection finished"); { std::lock_guard lock(dataLock_); diff --git a/services/distributeddataservice/libs/distributeddb/common/src/db_ability.cpp b/services/distributeddataservice/libs/distributeddb/common/src/db_ability.cpp index 673191fdc..87d0ca7eb 100644 --- a/services/distributeddataservice/libs/distributeddb/common/src/db_ability.cpp +++ b/services/distributeddataservice/libs/distributeddb/common/src/db_ability.cpp @@ -125,7 +125,7 @@ uint32_t DbAbility::GetAbilityBitsLen() const return dbAbility_.size(); } -uint8_t DbAbility::GetAbilityItem(const AbilityItem abilityType) const +uint8_t DbAbility::GetAbilityItem(const AbilityItem &abilityType) const { uint8_t data = 0; auto iter = dbAbilityItemSet_.find(abilityType); diff --git a/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_store_instance.h b/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_store_instance.h index 0bdc2aa93..5d9733a56 100644 --- a/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_store_instance.h +++ b/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_store_instance.h @@ -42,7 +42,7 @@ private: IRelationalStore *OpenDatabase(const RelationalDBProperties &properties, int &errCode); void RemoveKvDBFromCache(const RelationalDBProperties &properties); - void SaveKvDBToCache(IRelationalStore *store, const RelationalDBProperties &properties); + void SaveRelationalDBToCache(IRelationalStore *store, const RelationalDBProperties &properties); void EnterDBOpenCloseProcess(const std::string &identifier); void ExitDBOpenCloseProcess(const std::string &identifier); diff --git a/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_sync_able_storage.h b/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_sync_able_storage.h index 66fec6748..7cc831745 100644 --- a/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_sync_able_storage.h +++ b/services/distributeddataservice/libs/distributeddb/interfaces/src/relational/relational_sync_able_storage.h @@ -130,7 +130,7 @@ private: // data SQLiteSingleRelationalStorageEngine *storageEngine_ = nullptr; TimeStamp currentMaxTimeStamp_ = 0; - KvDBProperties properties; + KvDBProperties properties_; mutable std::mutex maxTimeStampMutex_; std::function onSchemaChanged_; diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.cpp b/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.cpp index 1804808b4..8e05736e9 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.cpp +++ b/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.cpp @@ -47,10 +47,10 @@ int DataTransformer::TransformDataItem(const std::vector &dataItems, return E_OK; } std::vector indexMapping; - ReduceMapping(remoteFieldInfo, localFieldInfo, indexMapping); + ReduceMapping(remoteFieldInfo, localFieldInfo); for (const DataItem &dataItem : dataItems) { OptRowDataWithLog dataWithLog; - int errCode = DeSerializeDataItem(dataItem, dataWithLog, remoteFieldInfo, indexMapping); + int errCode = DeSerializeDataItem(dataItem, dataWithLog, remoteFieldInfo); if (errCode != E_OK) { return errCode; } @@ -77,11 +77,10 @@ int DataTransformer::SerializeDataItem(const RowDataWithLog &data, } int DataTransformer::DeSerializeDataItem(const DataItem &dataItem, OptRowDataWithLog &data, - const std::vector &remoteFieldInfo, std::vector &indexMapping) + const std::vector &remoteFieldInfo) { - int errCode; if ((dataItem.flag & DataItem::DELETE_FLAG) == 0) { - errCode = DeSerializeValue(dataItem.value, data.optionalData, remoteFieldInfo, indexMapping); + int errCode = DeSerializeValue(dataItem.value, data.optionalData, remoteFieldInfo); if (errCode != E_OK) { return errCode; } @@ -126,20 +125,13 @@ uint32_t DataTransformer::CalDataValueLength(const DataValue &dataValue) } void DataTransformer::ReduceMapping(const std::vector &remoteFieldInfo, - const std::vector &localFieldInfo, std::vector &indexMapping) + const std::vector &localFieldInfo) { std::map fieldMap; for (int i = 0; i < static_cast(remoteFieldInfo.size()); ++i) { const auto &fieldInfo = remoteFieldInfo[i]; fieldMap[fieldInfo.GetFieldName()] = i; } - for (const auto &fieldInfo : localFieldInfo) { - if (fieldMap.find(fieldInfo.GetFieldName()) == fieldMap.end()) { - indexMapping.push_back(-E_NOT_FOUND); - continue; - } - indexMapping.push_back(fieldMap[fieldInfo.GetFieldName()]); - } } namespace { @@ -322,9 +314,8 @@ int DataTransformer::SerializeValue(Value &value, const RowData &rowData, const } int DataTransformer::DeSerializeValue(const Value &value, OptRowData &optionalData, - const std::vector &remoteFieldInfo, std::vector &indexMapping) + const std::vector &remoteFieldInfo) { - (void)indexMapping; Parcel parcel(const_cast(value.data()), value.size()); uint64_t fieldCount = 0; (void)parcel.ReadUInt64(fieldCount); diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.h b/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.h index 4d190e39d..92774ce5b 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.h +++ b/services/distributeddataservice/libs/distributeddb/storage/src/data_transformer.h @@ -27,7 +27,7 @@ using RowData = std::vector; using OptRowData = std::vector; struct LogInfo { - int dataKey = -1; + int64_t dataKey = -1; std::string device; std::string originDev; TimeStamp timestamp = 0; @@ -66,14 +66,14 @@ public: static int SerializeDataItem(const RowDataWithLog &data, const std::vector &fieldInfo, DataItem &dataItem); static int DeSerializeDataItem(const DataItem &dataItem, OptRowDataWithLog &data, - const std::vector &remoteFieldInfo, std::vector &indexMapping); + const std::vector &remoteFieldInfo); static void ReduceMapping(const std::vector &remoteFieldInfo, - const std::vector &localFieldInfo, std::vector &indexMapping); + const std::vector &localFieldInfo); private: static int SerializeValue(Value &value, const RowData &rowData, const std::vector &fieldInfoList); static int DeSerializeValue(const Value &value, OptRowData &optionalData, - const std::vector &remoteFieldInfo, std::vector &indexMapping); + const std::vector &remoteFieldInfo); static uint32_t CalDataValueLength(const DataValue &dataValue); }; diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/relational_store_instance.cpp b/services/distributeddataservice/libs/distributeddb/storage/src/relational_store_instance.cpp index de1dedeb3..cb44f6fbb 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/relational_store_instance.cpp +++ b/services/distributeddataservice/libs/distributeddb/storage/src/relational_store_instance.cpp @@ -103,7 +103,7 @@ void RelationalStoreInstance::RemoveKvDBFromCache(const RelationalDBProperties & dbs_.erase(identifier); } -void RelationalStoreInstance::SaveKvDBToCache(IRelationalStore *store, const RelationalDBProperties &properties) +void RelationalStoreInstance::SaveRelationalDBToCache(IRelationalStore *store, const RelationalDBProperties &properties) { if (store == nullptr) { return; @@ -122,6 +122,7 @@ IRelationalStore *RelationalStoreInstance::OpenDatabase(const RelationalDBProper { auto db = new (std::nothrow) SQLiteRelationalStore(); if (db == nullptr) { + errCode = -E_OUT_OF_MEMORY; LOGE("Failed to get relational store! err:%d", errCode); return nullptr; } @@ -138,7 +139,7 @@ IRelationalStore *RelationalStoreInstance::OpenDatabase(const RelationalDBProper return nullptr; } - SaveKvDBToCache(db, properties); + SaveRelationalDBToCache(db, properties); return db; } diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/relational_sync_able_storage.cpp b/services/distributeddataservice/libs/distributeddb/storage/src/relational_sync_able_storage.cpp index 02a32fab1..c62c09057 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/relational_sync_able_storage.cpp +++ b/services/distributeddataservice/libs/distributeddb/storage/src/relational_sync_able_storage.cpp @@ -206,7 +206,7 @@ int RelationalSyncAbleStorage::GetAllMetaKeys(std::vector &keys) const const KvDBProperties &RelationalSyncAbleStorage::GetDbProperties() const { - return properties; + return properties_; } static int GetKvEntriesByDataItems(std::vector &entries, std::vector &dataItems) diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.cpp b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.cpp index ff9965c39..be2cc3b9e 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.cpp +++ b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.cpp @@ -95,7 +95,6 @@ int SQLiteRelationalStoreConnection::StartTransaction() LOGD("[RelationalConnection] Start transaction finish."); writeHandle_ = handle; - transactingFlag_.store(true); return E_OK; } @@ -110,9 +109,6 @@ int SQLiteRelationalStoreConnection::Commit() int errCode = writeHandle_->Commit(); ReleaseExecutor(writeHandle_); - if (errCode == E_OK) { - transactingFlag_.store(false); - } LOGD("connection commit transaction!"); return errCode; } @@ -128,9 +124,6 @@ int SQLiteRelationalStoreConnection::RollBack() int errCode = writeHandle_->Rollback(); ReleaseExecutor(writeHandle_); - if (errCode == E_OK) { - transactingFlag_.store(false); - } LOGI("connection rollback transaction!"); return errCode; } diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.h b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.h index 6094af022..13b2dbd26 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.h +++ b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/relational/sqlite_relational_store_connection.h @@ -60,7 +60,6 @@ private: SQLiteSingleVerRelationalStorageExecutor *writeHandle_ = nullptr; mutable std::mutex transactionMutex_; // used for transaction - std::atomic transactingFlag_; }; } // namespace DistributedDB #endif diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_single_ver_relational_storage_executor.cpp b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_single_ver_relational_storage_executor.cpp index 4ac87ec80..76867db2f 100644 --- a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_single_ver_relational_storage_executor.cpp +++ b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_single_ver_relational_storage_executor.cpp @@ -431,7 +431,7 @@ static int BindDataValueByType(sqlite3_stmt *statement, const std::optional dev; int errCode = SQLiteUtils::GetColumnBlobValue(logStatement, 1, dev); // 1 means dev index @@ -748,9 +748,8 @@ int SQLiteSingleVerRelationalStorageExecutor::SaveSyncDataItem(const DataItem &d return DeleteSyncDataItem(dataItem, rmDataStmt); } - std::vector indexMapping; OptRowDataWithLog data; - int errCode = DataTransformer::DeSerializeDataItem(dataItem, data, fieldInfos, indexMapping); + int errCode = DataTransformer::DeSerializeDataItem(dataItem, data, fieldInfos); if (errCode != E_OK) { LOGE("[RelationalStorageExecutor] DeSerialize dataItem failed! errCode = [%d]", errCode); return errCode; diff --git a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_utils.cpp b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_utils.cpp index 5f0b8576a..368018710 100755 --- a/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_utils.cpp +++ b/services/distributeddataservice/libs/distributeddb/storage/src/sqlite/sqlite_utils.cpp @@ -594,7 +594,7 @@ int SQLiteUtils::CheckIntegrity(sqlite3 *db, const std::string &sql) namespace { // anonymous namespace for schema analysis int AnalysisSchemaSqlAndTrigger(sqlite3 *db, const std::string &tableName, TableInfo &table) { - std::string sql = "select type, name, tbl_name, rootpage, sql from sqlite_master where tbl_name = ?"; + std::string sql = "select type, sql from sqlite_master where tbl_name = ?"; sqlite3_stmt *statement = nullptr; int errCode = SQLiteUtils::GetStatement(db, sql, statement); if (errCode != E_OK) { @@ -620,7 +620,7 @@ int AnalysisSchemaSqlAndTrigger(sqlite3 *db, const std::string &tableName, Table (void) SQLiteUtils::GetColumnTextValue(statement, 0, type); if (type == "table") { std::string createTableSql; - (void) SQLiteUtils::GetColumnTextValue(statement, 4, createTableSql); // 4 means create table sql + (void) SQLiteUtils::GetColumnTextValue(statement, 1, createTableSql); // 1 means create table sql table.SetCreateTableSql(createTableSql); } } else { @@ -929,13 +929,13 @@ int SQLiteUtils::GetJournalMode(sqlite3 *db, std::string &mode) std::string sql = "PRAGMA journal_mode;"; sqlite3_stmt *statement = nullptr; - int errCode = sqlite3_prepare(db, sql.c_str(), -1, &statement, nullptr); + int errCode = SQLiteUtils::GetStatement(db, sql, statement); if (errCode != SQLITE_OK || statement == nullptr) { errCode = SQLiteUtils::MapSQLiteErrno(errCode); return errCode; } - if (sqlite3_step(statement) == SQLITE_ROW) { + if (SQLiteUtils::StepWithRetry(statement) == SQLiteUtils::MapSQLiteErrno(SQLITE_ROW)) { errCode = SQLiteUtils::GetColumnTextValue(statement, 0, mode); } else { LOGE("[SqlUtil][GetJournal] Get db journal_mode failed."); -- Gitee From 50fca74e987899874f36e1738c190ec89d3efe21 Mon Sep 17 00:00:00 2001 From: lianhuix Date: Tue, 8 Mar 2022 14:35:11 +0800 Subject: [PATCH 2/2] Fix ischema code check Signed-off-by: lianhuix --- .../distributeddb/common/include/ischema.h | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/services/distributeddataservice/libs/distributeddb/common/include/ischema.h b/services/distributeddataservice/libs/distributeddb/common/include/ischema.h index 32f69fee4..a864ff2bc 100644 --- a/services/distributeddataservice/libs/distributeddb/common/include/ischema.h +++ b/services/distributeddataservice/libs/distributeddb/common/include/ischema.h @@ -23,21 +23,6 @@ namespace DistributedDB { // SchemaType::NONE represent for KV database which do not have schema. Only invalid SchemaObject is NONE type. // Enum value must not be changed except SchemaType::UNRECOGNIZED. -enum class SchemaType : uint8_t { - NONE = 0, - JSON = 1, - FLATBUFFER = 2, - RELATIVE = 3, - UNRECOGNIZED = 4 -}; - -inline SchemaType ReadSchemaType(uint8_t inType) -{ - if (inType >= static_cast(SchemaType::UNRECOGNIZED)) { - return SchemaType::UNRECOGNIZED; - } - return static_cast(inType); -} struct SyncOpinion { bool permitSync = false; @@ -61,14 +46,34 @@ struct SchemaAttribute { std::string customFieldType {}; // Custom field type like BIGINT, DECIMAL, CHARACTER ... }; -class ISchema { +class Ischema { public: - ISchema() = default; - virtual ~ISchema() = default; + enum class SchemaType : uint8_t { + NONE = 0, + JSON = 1, + FLATBUFFER = 2, + RELATIVE = 3, + UNRECOGNIZED = 4 + }; + + Ischema() = default; + virtual ~Ischema() = default; virtual int ParseFromSchemaString(const std::string &inSchemaString) = 0; virtual bool IsSchemaValid() const = 0; virtual SchemaType GetSchemaType() const = 0; virtual std::string ToSchemaString() const = 0; }; + +using ISchema = Ischema; + +using SchemaType = ISchema::SchemaType; + +inline SchemaType ReadSchemaType(uint8_t inType) +{ + if (inType >= static_cast(SchemaType::UNRECOGNIZED)) { + return SchemaType::UNRECOGNIZED; + } + return static_cast(inType); +} } #endif // I_SCHEMA_H \ No newline at end of file -- Gitee