From bf7b51168db76e46e0dafc0f7d72f719fe7a1703 Mon Sep 17 00:00:00 2001 From: Junghyun Yeon Date: Mon, 5 Apr 2021 16:13:36 +0900 Subject: [PATCH] Refactor query construction logic of query handler Query requests could be handled on sqlite side by using sqlite3_prepare_v2() and sqlite3_bind_text(). Change-Id: I6bc6906a6dc42d00ec21724fd16a88cae28b9e2b Signed-off-by: Junghyun Yeon --- src/common/database/query_handler.cc | 240 ++++++++++++---------------------- src/common/database/query_handler.hh | 3 - src/common/parcel/query_parcelable.cc | 17 +-- src/common/parcel/query_parcelable.hh | 16 ++- src/pkginfo_internal.c | 53 ++++++-- src/pkgmgrinfo_internal.h | 9 +- 6 files changed, 145 insertions(+), 193 deletions(-) diff --git a/src/common/database/query_handler.cc b/src/common/database/query_handler.cc index 91e1a83..9f14ac2 100644 --- a/src/common/database/query_handler.cc +++ b/src/common/database/query_handler.cc @@ -27,116 +27,116 @@ namespace { constexpr const char query_appinfo_get_localed_label[] = "SELECT COALESCE((SELECT app_label FROM package_app_localized_info " - "WHERE app_id=%Q AND app_locale=%Q)," + "WHERE app_id=? AND app_locale=?)," "(SELECT app_label FROM package_app_localized_info WHERE " - "app_id=%Q AND app_locale='No Locale'))"; + "app_id=? AND app_locale='No Locale'))"; constexpr const char query_appinfo_get_datacontrol_info[] = "SELECT app_id, access FROM " "package_app_data_control WHERE " - "providerid=%Q AND type=%Q"; + "providerid=? AND type=?"; constexpr const char query_appinfo_get_datacontrol_appid[] = "SELECT app_id FROM package_app_data_control " - "WHERE providerid=%Q"; + "WHERE providerid=?"; constexpr const char query_appinfo_get_datacontrol_trusted_info[] = "SELECT app_id, trusted FROM package_app_data_control " - "WHERE providerid=%Q AND type=%Q"; + "WHERE providerid=? AND type=?"; constexpr const char query_appinfo_get_datacontrol_privileges[] = "SELECT privilege FROM package_app_data_control_privilege " - "WHERE providerid=%Q AND type=%Q"; + "WHERE providerid=? AND type=?"; constexpr const char query_appinfo_get_appcontrol_privileges[] = "SELECT app_control, privilege FROM package_app_app_control_privilege " - "WHERE app_id=%Q"; + "WHERE app_id=?"; constexpr const char query_plugininfo_get_appids[] = "SELECT appid FROM " - "package_plugin_info WHERE pkgid=%Q AND " - "plugin_type=%Q AND plugin_name=%Q"; + "package_plugin_info WHERE pkgid=? AND " + "plugin_type=? AND plugin_name=?"; constexpr const char query_get_pkg_updateinfo_1[] = "SELECT package, update_version, update_type " "FROM package_update_info"; constexpr const char query_get_pkg_updateinfo_2[] = "SELECT package, update_version, update_type " - "FROM package_update_info WHERE package=%Q"; + "FROM package_update_info WHERE package=?"; constexpr const char query_pkginfo_set_usr_installed_storage_1[] = - "UPDATE package_info SET installed_storage=%Q, external_path=%Q " - "WHERE package=%Q"; + "UPDATE package_info SET installed_storage=?, external_path=? " + "WHERE package=?"; constexpr const char query_pkginfo_set_usr_installed_storage_2[] = - "UPDATE package_app_info SET app_installed_storage=%Q, " - "app_external_path=%Q WHERE package=%Q"; + "UPDATE package_app_info SET app_installed_storage=?, " + "app_external_path=? WHERE package=?"; constexpr const char query_certinfo_compare_pkg_certinfo[] = "SELECT package, " "COALESCE(author_signer_cert, -1) FROM package_cert_info WHERE " - "package IN (%Q, %Q)"; + "package IN (?, ?)"; constexpr const char query_certinfo_compare_app_certinfo[] = "SELECT app_id, package FROM " - "package_app_info WHERE app_id IN (%Q, %Q)"; + "package_app_info WHERE app_id IN (?, ?)"; constexpr const char query_pkginfo_delete_certinfo[] = "UPDATE package_cert_info SET " - "package_count = package_count - 1 WHERE package=%Q"; + "package_count = package_count - 1 WHERE package=?"; // For pkgmgr_parser constexpr const char query_insert_package_plugin_execution_info[] = "INSERT INTO package_plugin_info " "(pkgid, appid, plugin_type, plugin_name) " - "VALUES (%Q, %Q, %Q, %Q)"; + "VALUES (?, ?, ?, ?)"; constexpr const char query_delete_package_plugin_execution_info[] = - "DELETE FROM package_plugin_info WHERE pkgid=%Q"; + "DELETE FROM package_plugin_info WHERE pkgid=?"; constexpr const char query_update_global_app_disable[] = "INSERT OR REPLACE INTO package_app_info_for_uid (" " app_id, uid, is_disabled, is_splash_screen_enabled) " - "VALUES (%Q, %Q, %Q," + "VALUES (?, ?, ?," " (SELECT app_splash_screen_display FROM package_app_info" - " WHERE app_id=%Q))"; + " WHERE app_id=?))"; constexpr const char query_update_app_disable_info[] = - "UPDATE package_app_info SET app_disable=%Q " - "WHERE app_id=%Q"; + "UPDATE package_app_info SET app_disable=? " + "WHERE app_id=?"; constexpr const char query_update_pkg_disable_info[] = - "UPDATE package_info SET package_disable=%Q " - "WHERE package=%Q"; + "UPDATE package_info SET package_disable=? " + "WHERE package=?"; constexpr const char query_update_global_app_splash_screen_display_info[] = "INSERT OR REPLACE INTO package_app_info_for_uid(" " app_id, uid, is_splash_screen_enabled) " - "VALUES (%Q, %Q, %Q)"; + "VALUES (?, ?, ?)"; constexpr const char query_update_app_splash_screen_display_info[] = - "UPDATE package_app_info SET app_splash_screen_display=%Q " - "WHERE app_id=%Q"; + "UPDATE package_app_info SET app_splash_screen_display=? " + "WHERE app_id=?"; constexpr const char query_update_app_label_info[] = - "UPDATE package_app_localized_info SET app_label=%Q " - "WHERE app_id=%Q AND app_label IS NOT NULL"; + "UPDATE package_app_localized_info SET app_label=? " + "WHERE app_id=? AND app_label IS NOT NULL"; constexpr const char query_update_app_icon_info[] = - "UPDATE package_app_localized_info SET app_icon=%Q " - "WHERE app_id=%Q AND app_icon IS NOT NULL"; + "UPDATE package_app_localized_info SET app_icon=? " + "WHERE app_id=? AND app_icon IS NOT NULL"; constexpr const char query_update_tep_info[] = - "UPDATE package_info SET package_tep_name=%Q " - "WHERE package=%Q"; + "UPDATE package_info SET package_tep_name=? " + "WHERE package=?"; constexpr const char query_register_pkg_update_info[] = "UPDATE package_update_info " - "SET update_version=%Q, update_type=%Q " - "WHERE package=%Q"; + "SET update_version=?, update_type=? " + "WHERE package=?"; constexpr const char query_unregister_pkg_update_info[] = "UPDATE package_update_info " - "SET update_type='none' WHERE package=%Q"; + "SET update_type='none' WHERE package=?"; constexpr const char query_unregister_all_pkg_update_info[] = "UPDATE package_update_info " @@ -175,99 +175,24 @@ class QueryMaker { query_unregister_all_pkg_update_info, }; - std::vector arg_cnt_ = { - 3, // query_appinfo_get_localed_label - 2, // query_appinfo_get_datacontrol_info - 1, // query_appinfo_get_datacontrol_appid - 2, // query_appinfo_get_datacontrol_trusted_info - 2, // query_appinfo_get_datacontrol_privileges - 1, // query_appinfo_get_appcontrol_privileges - 3, // query_plugininfo_get_appids - 0, // query_get_pkg_updateinfo_1 - 1, // query_get_pkg_updateinfo_2 - 3, // query_pkginfo_set_usr_installed_storage_1 - 3, // query_pkginfo_set_usr_installed_storage_2 - 2, // query_certinfo_compare_pkg_certinfo - 2, // query_certinfo_compare_app_certinfo - 1, // query_pkginfo_delete_certinfo - - 4, // query_insert_package_plugin_execution_info, - 1, // query_delete_package_plugin_execution_info, - 4, // query_update_global_app_disable, - 2, // query_update_app_disable_info, - 2, // query_update_pkg_disable_info, - 3, // query_update_global_app_splash_screen_display_info, - 2, // query_update_app_splash_screen_display_info, - 2, // query_update_app_label_info, - 2, // query_update_app_icon_info, - 2, // query_update_tep_info, - 3, // query_register_pkg_update_info, - 1, // query_unregister_pkg_update_info, - 0, // query_unregister_all_pkg_update_info, - }; - - std::string Make(int index, const std::vector& args) { - if (index < 0 || index >= QUERY_INDEX_MAX) - return ""; - const char* raw = query_raw_[index]; - char* query = nullptr; - - if (arg_cnt_[index] != args.size()) - return ""; - - switch (arg_cnt_[index]) { - case 0: - query = sqlite3_mprintf(raw); - break; - - case 1: - query = sqlite3_mprintf(raw, args[0].c_str()); - break; - - case 2: - query = sqlite3_mprintf(raw, args[0].c_str(), args[1].c_str()); - break; - - case 3: - query = sqlite3_mprintf(raw, args[0].c_str(), args[1].c_str(), - args[2].c_str()); - break; - - case 4: - query = sqlite3_mprintf(raw, args[0].c_str(), args[1].c_str(), - args[2].c_str(), args[3].c_str()); - break; - - case 5: - query = sqlite3_mprintf(raw, args[0].c_str(), args[1].c_str(), - args[2].c_str(), args[3].c_str(), args[4].c_str()); - break; - - case 6: - query = sqlite3_mprintf(raw, args[0].c_str(), args[1].c_str(), - args[2].c_str(), args[3].c_str(), args[4].c_str(), - args[5].c_str()); - break; - - case 7: - query = sqlite3_mprintf(raw, args[0].c_str(), args[1].c_str(), - args[2].c_str(), args[3].c_str(), args[4].c_str(), - args[5].c_str(), args[6].c_str()); - break; - } - - if (query == nullptr) - return ""; - - std::string ret = query; - sqlite3_free(query); - - return ret; + const char* GetQuery(int index) { + return query_raw_[index]; } }; QueryMaker __query_maker; +void __free_argument(gpointer data, gpointer user_data) { + query_args* args = (query_args*)data; + g_list_free(args->argument); + free(args); +} + +void __free_query_list(GList* queries, GList* args_list) { + g_list_free(queries); + g_list_foreach(args_list, __free_argument, NULL); +} + } // namespace namespace pkgmgr_common { @@ -291,18 +216,6 @@ std::vector> QueryHandler::GetResult() { return std::move(result_); } -int QueryHandler::MakeQuery() { - query_.clear(); - for (auto& i : query_args_) { - std::string q = __query_maker.Make(i.first, i.second); - if (q.empty()) - return -1; - query_.push_back(std::move(q)); - } - - return 0; -} - int QueryHandler::Execute() { std::shared_lock s(lock_); if (!Connect()) { @@ -310,25 +223,44 @@ int QueryHandler::Execute() { return PMINFO_R_ERROR; } - if (MakeQuery() != 0) - return PMINFO_R_ERROR; + GList* queries = nullptr; + GList* args_list = nullptr; + for (auto& i : query_args_) { + const char* query = __query_maker.GetQuery(i.first); + if (query == nullptr) { + _LOGE("Failed to get query"); + __free_query_list(queries, args_list); + return PMINFO_R_ERROR; + } - if (query_.size() == 0) { - _LOGE("Empty query"); - return PMINFO_R_ERROR; + queries = g_list_append(queries, (gpointer)query); + query_args* arg = (query_args*)calloc(1, sizeof(query_args)); + if (arg == nullptr) { + _LOGE("Out of memory"); + __free_query_list(queries, args_list); + return PMINFO_R_ERROR; + } + arg->len = i.second.size(); + for (auto& argument : i.second) + arg->argument = g_list_append(arg->argument, (gpointer)argument.c_str()); + + args_list = g_list_append(args_list, arg); } - int ret = PMINFO_R_ERROR; std::vector> conn_list = GetConnection(); + int ret; if (GetOpType() == OPERATION_TYPE_READ) { for (auto& conn : conn_list) { GList* list = nullptr; int row = 0; int col = 0; - ret = get_query_result(conn.first, query_[0].c_str(), &list, &row, &col); + query_args* params = (query_args*)args_list->data; + ret = get_query_result(conn.first, (const char *)queries->data, + params->argument, &list, &row, &col); if (ret == PMINFO_R_ERROR) { _LOGE("Failed to execute query"); + __free_query_list(queries, args_list); return ret; } @@ -341,32 +273,22 @@ int QueryHandler::Execute() { } result_.emplace_back(std::move(vt)); } - - g_list_free_full(list, free); } + __free_query_list(queries, args_list); + return ret; } else { - const char **queries = (const char **)calloc(query_.size(), sizeof(char *)); - if (queries == nullptr) { - _LOGE("Out of memory"); - return PMINFO_R_ERROR; - } - - int i = 0; - for (const auto& query : query_) - queries[i++] = query.c_str(); - for (auto& conn : conn_list) { - ret = execute_write_queries(conn.first, queries, query_.size()); + ret = execute_write_queries(conn.first, queries, args_list); if (ret != PMINFO_R_OK) { _LOGE("Failed to execute"); break; } } - free(queries); + __free_query_list(queries, args_list); return ret; - } + } return ret; } diff --git a/src/common/database/query_handler.hh b/src/common/database/query_handler.hh index 6b0febc..3cf692b 100644 --- a/src/common/database/query_handler.hh +++ b/src/common/database/query_handler.hh @@ -47,9 +47,6 @@ class EXPORT_API QueryHandler : public AbstractDBHandler{ std::vector> GetResult(); private: - int MakeQuery(); - - private: uid_t uid_; std::vector query_; std::vector>> query_args_; diff --git a/src/common/parcel/query_parcelable.cc b/src/common/parcel/query_parcelable.cc index c4f8d4a..4d47811 100644 --- a/src/common/parcel/query_parcelable.cc +++ b/src/common/parcel/query_parcelable.cc @@ -30,18 +30,16 @@ QueryParcelable::QueryParcelable() db_type_(AbstractDBHandler::DBType::DB_TYPE_NONE), op_type_(AbstractDBHandler::OperationType::OPERATION_TYPE_NONE) {} - QueryParcelable::QueryParcelable(uid_t uid, - std::pair> query_args, + QueryArgs query_args, AbstractDBHandler::DBType db_type, AbstractDBHandler::OperationType op_type) : AbstractParcelable(0, ParcelableType::Query), - query_args_( - std::vector>>{query_args}), + query_args_(std::vector{query_args}), db_type_(db_type), op_type_(op_type) {} QueryParcelable::QueryParcelable(uid_t uid, - std::vector>> query_args, + std::vector query_args, AbstractDBHandler::DBType db_type, AbstractDBHandler::OperationType op_type) : AbstractParcelable(uid, ParcelableType::Query), @@ -74,11 +72,10 @@ void QueryParcelable::ReadFromParcel(tizen_base::Parcel* parcel) { ReadInt(parcel, &index); ReadInt(parcel, &arg_cnt); - for (int j = 0; j < arg_cnt; ++j) { + for (int j = 0; j < arg_cnt; ++j) args.push_back(parcel->ReadString()); - } - query_args_.push_back( - std::pair>(index, std::move(args))); + + query_args_.push_back(QueryArgs(index, std::move(args))); } ReadInt(parcel, &db_type); db_type_ = static_cast(db_type); @@ -86,7 +83,7 @@ void QueryParcelable::ReadFromParcel(tizen_base::Parcel* parcel) { op_type_ = static_cast(op_type); } -const std::vector>>& QueryParcelable::GetQueryArgs() { +const std::vector& QueryParcelable::GetQueryArgs() { return query_args_; } diff --git a/src/common/parcel/query_parcelable.hh b/src/common/parcel/query_parcelable.hh index bb328bb..7ef9e58 100644 --- a/src/common/parcel/query_parcelable.hh +++ b/src/common/parcel/query_parcelable.hh @@ -17,14 +17,18 @@ namespace parcel { #define EXPORT_API __attribute__((visibility("default"))) #endif +using QueryArgs = std::pair>; + class EXPORT_API QueryParcelable : public AbstractParcelable { public: QueryParcelable(); - QueryParcelable(uid_t uid, std::pair> query_args, - AbstractDBHandler::DBType db_type, AbstractDBHandler::OperationType op_type); - QueryParcelable(uid_t uid, std::vector>> query_args, - AbstractDBHandler::DBType db_type, AbstractDBHandler::OperationType op_type); - const std::vector>>& GetQueryArgs(); + QueryParcelable(uid_t uid, QueryArgs query_args, + AbstractDBHandler::DBType db_type, + AbstractDBHandler::OperationType op_type); + QueryParcelable(uid_t uid, std::vector query_args, + AbstractDBHandler::DBType db_type, + AbstractDBHandler::OperationType op_type); + const std::vector& GetQueryArgs(); AbstractDBHandler::DBType GetDBType(); AbstractDBHandler::OperationType GetOpType(); @@ -32,7 +36,7 @@ class EXPORT_API QueryParcelable : public AbstractParcelable { void ReadFromParcel(tizen_base::Parcel* parcel) override; private: - std::vector>> query_args_; + std::vector query_args_; AbstractDBHandler::DBType db_type_; AbstractDBHandler::OperationType op_type_; }; diff --git a/src/pkginfo_internal.c b/src/pkginfo_internal.c index d35ea10..4668b4e 100644 --- a/src/pkginfo_internal.c +++ b/src/pkginfo_internal.c @@ -23,6 +23,8 @@ #include "pkgmgrinfo_debug.h" #include "pkgmgr-info.h" +#include "pkgmgrinfo_internal.h" + #define __BEGIN_TRANSACTION(db) \ do { \ if (sqlite3_exec(db, "BEGIN DEFERRED", NULL, NULL, NULL) != \ @@ -647,7 +649,7 @@ API int pkginfo_internal_filter_get_list( return ret; } -API int get_query_result(sqlite3 *db, const char *query, +API int get_query_result(sqlite3 *db, const char *query, GList *param, GList **list, int *row, int *col) { int ret = 0; @@ -663,6 +665,14 @@ API int get_query_result(sqlite3 *db, const char *query, return PMINFO_R_ERROR; } + if (g_list_length(param) != 0) { + ret = __bind_params(stmt, param); + if (ret != PMINFO_R_OK) { + LOGE("failed to bind parameters: %s", sqlite3_errmsg(db)); + return ret; + } + } + col_cnt = sqlite3_column_count(stmt); while (sqlite3_step(stmt) == SQLITE_ROW) { @@ -775,7 +785,7 @@ API int pkginfo_internal_filter_get_depends_on(sqlite3 *db, const char *pkgid, return PMINFO_R_OK; } -static int __execute_query(sqlite3 *db, const char *query) +static int __execute_query(sqlite3 *db, const char *query, GList *param) { int ret = 0; sqlite3_stmt *stmt = NULL; @@ -786,31 +796,48 @@ static int __execute_query(sqlite3 *db, const char *query) return -1; } + if (g_list_length(param) != 0) { + ret = __bind_params(stmt, param); + if (ret != PMINFO_R_OK) { + LOGE("failed to bind parameters: %s", sqlite3_errmsg(db)); + return ret; + } + } + ret = sqlite3_step(stmt); - if (ret != SQLITE_DONE) { - LOGE("step failed: %s", sqlite3_errmsg(db)); + if (ret != SQLITE_DONE && ret != SQLITE_OK) { + LOGE("step failed:%d %s", ret, sqlite3_errmsg(db)); sqlite3_finalize(stmt); return -1; } sqlite3_finalize(stmt); - return 0; + return PMINFO_R_OK; } -API int execute_write_queries(sqlite3 *db, const char **queries, int len) +API int execute_write_queries(sqlite3 *db, GList *queries, GList *params_list) { int i; - + int j; + GList *list = NULL; + query_args *tmp_ptr = NULL; __BEGIN_TRANSACTION(db); - for (i = 0; i < len; ++i) - __DO_TRANSACTION(db, __execute_query(db, queries[i])); - __END_TRANSACTION(db); + for (i = 0; i < g_list_length(queries); ++i) { + tmp_ptr = (query_args *)g_list_nth_data(params_list, i); + if (tmp_ptr == NULL) { + _LOGE("Failed to get parameter list"); + sqlite3_exec(db, "ROLLBACK", NULL, NULL, NULL); + return PMINFO_R_ERROR; + } - // Is db handel freed by AbstractDBHandler? - // sqlite3_close_v2(db); + __DO_TRANSACTION(db, + __execute_query(db, + g_list_nth_data(queries, i), list)); + } + __END_TRANSACTION(db); - return 0; + return PMINFO_R_OK; } static int __check_dpi(const char *dpi_char, int dpi_int) diff --git a/src/pkgmgrinfo_internal.h b/src/pkgmgrinfo_internal.h index b212137..b00aa93 100644 --- a/src/pkgmgrinfo_internal.h +++ b/src/pkgmgrinfo_internal.h @@ -31,14 +31,19 @@ extern "C" { #endif +typedef struct { + int len; + GList *argument; +} query_args; + int pkginfo_internal_filter_get_list(sqlite3 *db, pkgmgrinfo_pkginfo_filter_h filter, uid_t uid, const char *locale, GHashTable *list); int appinfo_internal_filter_get_list(sqlite3 *db, pkgmgrinfo_appinfo_filter_h filter, uid_t db_uid, uid_t uid, const char *locale, GHashTable *list); int certinfo_internal_get(sqlite3 *db, const char *pkgid, uid_t uid, pkgmgrinfo_certinfo_h certinfo); int certinfo_internal_set(sqlite3 *db, const char *pkgid, pkgmgrinfo_instcertinfo_h handle, uid_t uid); int certinfo_internal_delete(sqlite3 *db, const char *pkgid); -int get_query_result(sqlite3 *db, const char *query, GList **list, int *row, int *col); +int get_query_result(sqlite3 *db, const char *query, GList *param, GList **list, int *row, int *col); +int execute_write_queries(sqlite3 *db, GList *queries, GList *params_list); int pkginfo_internal_filter_get_depends_on(sqlite3 *db, const char *pkgid, GList **list); -int execute_write_queries(sqlite3 *db, const char **queries, int len); int pkgmgr_parser_insert_pkg_info(sqlite3 *db, manifest_x *mfx, uid_t uid); int pkgmgr_parser_update_pkg_info(sqlite3 *db, manifest_x *mfx, uid_t uid); -- 2.7.4