From 3a8f175fa89c78799015627f6206e8d6b087db41 Mon Sep 17 00:00:00 2001 From: cuihaikuo Date: Wed, 4 Apr 2018 16:12:12 +0800 Subject: [PATCH] Use transaction when creating a new group. De-duplicate in getting groups with ancestors. --- net/common/ccnet-db.c | 2 +- net/common/rpc-service.c | 5 +- net/common/rpc-service.h | 2 +- net/server/group-mgr.c | 175 +++++++++++++++++++++++++++------------ 4 files changed, 125 insertions(+), 59 deletions(-) diff --git a/net/common/ccnet-db.c b/net/common/ccnet-db.c index e7dccd3..80ef07f 100644 --- a/net/common/ccnet-db.c +++ b/net/common/ccnet-db.c @@ -834,7 +834,7 @@ ccnet_db_rollback (CcnetDBTrans *trans) DBConnection *conn = trans->conn; GError *error = NULL; - if (!db_connection_commit (conn, &error)) { + if (!db_connection_rollback(conn, &error)) { ccnet_warning ("Rollback failed: %s.\n", error->message); g_clear_error (&error); return -1; diff --git a/net/common/rpc-service.c b/net/common/rpc-service.c index 284dd6a..92f6473 100644 --- a/net/common/rpc-service.c +++ b/net/common/rpc-service.c @@ -1145,7 +1145,7 @@ ccnet_rpc_quit_group (int group_id, const char *user_name, GError **error) } GList * -ccnet_rpc_get_groups (const char *username, gboolean return_ancestors, GError **error) +ccnet_rpc_get_groups (const char *username, int return_ancestors, GError **error) { CcnetGroupManager *group_mgr = ((CcnetServerSession *)session)->group_mgr; @@ -1157,7 +1157,8 @@ ccnet_rpc_get_groups (const char *username, gboolean return_ancestors, GError ** return NULL; } - ret = ccnet_group_manager_get_groups_by_user (group_mgr, username, return_ancestors, error); + ret = ccnet_group_manager_get_groups_by_user (group_mgr, username, + return_ancestors ? TRUE : FALSE, error); return ret; } diff --git a/net/common/rpc-service.h b/net/common/rpc-service.h index 3969127..9c2ba24 100644 --- a/net/common/rpc-service.h +++ b/net/common/rpc-service.h @@ -213,7 +213,7 @@ int ccnet_rpc_quit_group (int group_id, const char *user_name, GError **error); GList * -ccnet_rpc_get_groups (const char *username, gboolean return_ancestors, GError **error); +ccnet_rpc_get_groups (const char *username, int return_ancestors, GError **error); GList * ccnet_rpc_get_all_groups (int start, int limit, const char *source, GError **error); diff --git a/net/server/group-mgr.c b/net/server/group-mgr.c index ba8ee5e..bbc3f10 100644 --- a/net/server/group-mgr.c +++ b/net/server/group-mgr.c @@ -135,7 +135,7 @@ static int check_db_table (CcnetGroupManager *manager, CcnetDB *db) "CREATE TABLE IF NOT EXISTS `%s` (`group_id` INTEGER" " PRIMARY KEY AUTOINCREMENT, `group_name` VARCHAR(255)," " `creator_name` VARCHAR(255), `timestamp` BIGINT," - " `type` VARCHAR(32))", table_name); + " `type` VARCHAR(32), `parent_group_id` INTEGER)", table_name); if (ccnet_db_query (db, group_sql->str) < 0) { g_string_free (group_sql, TRUE); return -1; @@ -170,7 +170,7 @@ static int check_db_table (CcnetGroupManager *manager, CcnetDB *db) "CREATE TABLE IF NOT EXISTS \"%s\" (group_id SERIAL" " PRIMARY KEY, group_name VARCHAR(255)," " creator_name VARCHAR(255), timestamp BIGINT," - " type VARCHAR(32))", table_name); + " type VARCHAR(32), parent_group_id INTEGER)", table_name); if (ccnet_db_query (db, group_sql->str) < 0) { g_string_free (group_sql, TRUE); return -1; @@ -203,6 +203,26 @@ static int check_db_table (CcnetGroupManager *manager, CcnetDB *db) return 0; } +static gboolean +get_group_id_cb (CcnetDBRow *row, void *data) +{ + int *id = data; + int group_id = ccnet_db_row_get_column_int(row, 0); + *id = group_id; + + return FALSE; +} + +static gboolean +get_group_path_cb (CcnetDBRow *row, void *data) +{ + char **path = (char **)data; + const char *group_path = ccnet_db_row_get_column_text (row, 0); + *path = g_strdup (group_path); + + return FALSE; +} + static int create_group_common (CcnetGroupManager *mgr, const char *group_name, @@ -215,6 +235,7 @@ create_group_common (CcnetGroupManager *mgr, GString *sql = g_string_new (""); const char *table_name = mgr->priv->table_name; int group_id = -1; + CcnetDBTrans *trans = ccnet_db_begin_transaction (db); char *user_name_l = g_ascii_strdown (user_name, -1); @@ -227,12 +248,10 @@ create_group_common (CcnetGroupManager *mgr, "INSERT INTO `%s`(group_name, " "creator_name, timestamp, parent_group_id) VALUES(?, ?, ?, ?)", table_name); - if (ccnet_db_statement_query (db, sql->str, 4, - "string", group_name, "string", user_name_l, - "int64", now, "int", parent_group_id) < 0) { - g_set_error (error, CCNET_DOMAIN, 0, "Failed to create group"); - goto out; - } + if (ccnet_db_trans_query (trans, sql->str, 4, + "string", group_name, "string", user_name_l, + "int64", now, "int", parent_group_id) < 0) + goto error; if (ccnet_db_type(db) == CCNET_DB_TYPE_PGSQL) g_string_printf (sql, @@ -245,49 +264,48 @@ create_group_common (CcnetGroupManager *mgr, "group_name = ? AND creator_name = ? " "AND timestamp = ?", table_name); - group_id = ccnet_db_statement_get_int (db, sql->str, 3, - "string", group_name, "string", user_name_l, - "int64", now); - if (group_id < 0) { - g_set_error (error, CCNET_DOMAIN, 0, "Failed to create group"); - goto out; - } + ccnet_db_trans_foreach_selected_row (trans, sql->str, get_group_id_cb, + &group_id, 3, "string", group_name, + "string", user_name_l, "int64", now); + + if (group_id < 0) + goto error; g_string_printf (sql, "INSERT INTO GroupUser VALUES (?, ?, ?)"); - if (ccnet_db_statement_query (db, sql->str, 3, - "int", group_id, "string", user_name_l, - "int", 1) < 0) + if (ccnet_db_trans_query (trans, sql->str, 3, + "int", group_id, "string", user_name_l, + "int", 1) < 0) goto error; if (parent_group_id == -1) { g_string_printf (sql, "INSERT INTO GroupStructure VALUES (?,'%d')", group_id); - if (ccnet_db_statement_query (db, sql->str, 1, "int", group_id) < 0) + if (ccnet_db_trans_query (trans, sql->str, 1, "int", group_id) < 0) goto error; } else if (parent_group_id > 0) { g_string_printf (sql, "SELECT path FROM GroupStructure WHERE group_id=?"); - char *path = ccnet_db_statement_get_string (db, sql->str, 1, "int", parent_group_id); + char *path = NULL; + ccnet_db_trans_foreach_selected_row (trans, sql->str, get_group_path_cb, + &path, 1, "int", parent_group_id); if (!path) goto error; g_string_printf (sql, "INSERT INTO GroupStructure VALUES (?, '%s, %d')", path, group_id); - if (ccnet_db_statement_query (db, sql->str, 1, "int", group_id) < 0) { + if (ccnet_db_trans_query (trans, sql->str, 1, "int", group_id) < 0) { g_free (path); goto error; } g_free (path); } -out: + ccnet_db_commit (trans); + ccnet_db_trans_close (trans); g_string_free (sql, TRUE); g_free (user_name_l); return group_id; error: - if (ccnet_db_type(db) == CCNET_DB_TYPE_PGSQL) - g_string_printf (sql, "DELETE FROM \"%s\" WHERE group_id=?", table_name); - else - g_string_printf (sql, "DELETE FROM `%s` WHERE group_id=?", table_name); - ccnet_db_statement_query (db, sql->str, 1, "int", group_id); + ccnet_db_rollback (trans); + ccnet_db_trans_close (trans); g_set_error (error, CCNET_DOMAIN, 0, "Failed to create group"); g_string_free (sql, TRUE); g_free (user_name_l); @@ -663,6 +681,34 @@ ccnet_group_manager_get_ancestor_groups (CcnetGroupManager *mgr, int group_id) return ret; } +static gint +group_comp_func (gconstpointer a, gconstpointer b) +{ + CcnetGroup *g1 = (CcnetGroup *)a; + CcnetGroup *g2 = (CcnetGroup *)b; + int id_1 = 0, id_2 = 0; + g_object_get (g1, "id", &id_1, NULL); + g_object_get (g2, "id", &id_2, NULL); + + if (id_1 == id_2) + return 0; + return id_1 > id_2 ? -1 : 1; +} + +gboolean +get_group_paths_cb (CcnetDBRow *row, void *data) +{ + GString *paths = data; + const char *path = ccnet_db_row_get_column_text (row, 0); + + if (g_strcmp0 (paths->str, "") == 0) + g_string_append_printf (paths, "%s", path); + else + g_string_append_printf (paths, ", %s", path); + + return TRUE; +} + GList * ccnet_group_manager_get_groups_by_user (CcnetGroupManager *mgr, const char *user_name, @@ -670,7 +716,7 @@ ccnet_group_manager_get_groups_by_user (CcnetGroupManager *mgr, GError **error) { CcnetDB *db = mgr->priv->db; - GList *groups = NULL, *ancestor_list = NULL, *ret = NULL; + GList *groups = NULL, *ret = NULL; GList *ptr; GString *sql = g_string_new (""); const char *table_name = mgr->priv->table_name; @@ -696,42 +742,61 @@ ccnet_group_manager_get_groups_by_user (CcnetGroupManager *mgr, g_string_free (sql, TRUE); return NULL; } - g_string_free (sql, TRUE); - /* tmp_ptr, tmp_group and tmp_group_id are used to check if a group already exists in the list, - * The list @groups sorts by group_id in descending order so that we can store each group chain from bottom to top - * and avoid to store repeatly. - */ - GList *tmp_ptr; - CcnetGroup *tmp_group; - int tmp_group_id; - gboolean exists = FALSE; + if (!return_ancestors) { + g_string_free (sql, TRUE); + return groups; + } + + /* Get ancestor groups in descending order by group_id.*/ + GString *paths = g_string_new (""); + g_string_printf (sql, ""); for (ptr = groups; ptr; ptr = ptr->next) { group = ptr->data; g_object_get (group, "parent_group_id", &parent_group_id, NULL); - if (parent_group_id > 0) { - g_object_get (group, "id", &group_id, NULL); - for (tmp_ptr = ret; tmp_ptr; tmp_ptr = tmp_ptr->next) { - tmp_group = tmp_ptr->data; - g_object_get (tmp_group, "id", &tmp_group_id, NULL); - if (group_id == tmp_group_id) { - exists = TRUE; - break; - } - } - if (exists) { - exists = FALSE; - continue; - } - ancestor_list = ccnet_group_manager_get_ancestor_groups (mgr, group_id); - if (ancestor_list) - ret = g_list_concat(ret, ancestor_list); + g_object_get (group, "id", &group_id, NULL); + if (parent_group_id != 0) { + if (g_strcmp0(sql->str, "") == 0) + g_string_append_printf (sql, "SELECT path FROM GroupStructure WHERE group_id IN (%d", group_id); + else + g_string_append_printf (sql, ", %d", group_id); } else { g_object_ref (group); - ret = g_list_append (ret, group); + ret = g_list_insert_sorted (ret, group, group_comp_func); } } + if (g_strcmp0(sql->str, "") != 0) { + g_string_append_printf (sql, ")"); + if (ccnet_db_statement_foreach_row (db, + sql->str, + get_group_paths_cb, + paths, 0) < 0) { + g_list_free_full (ret, g_object_unref); + goto out; + } + if (g_strcmp0(paths->str, "") == 0) { + ccnet_warning ("Failed to get groups path for user %s\n", user_name); + g_list_free_full (ret, g_object_unref); + goto out; + } + + g_string_printf (sql, "SELECT g.group_id, group_name, creator_name, timestamp, parent_group_id FROM " + "`%s` g WHERE g.group_id IN (%s) ORDER BY g.group_id DESC", + table_name, paths->str); + if (ccnet_db_statement_foreach_row (db, + sql->str, + get_user_groups_cb, + &ret, 0) < 0) { + g_list_free_full (ret, g_object_unref); + goto out; + } + } + ret = g_list_sort (ret, group_comp_func); + +out: + g_string_free (sql, TRUE); g_list_free_full (groups, g_object_unref); + g_string_free (paths, TRUE); return ret; }