[glom/glom-1-20] test_selfhosting_new_empty_then_users: Test user removal.

[Murray Cumming <murrayc src gnome org>]

commit 24daa1276cbd19f3894d0baa31e58db117955b3e
Author: Murray Cumming <murrayc murrayc com>
Date:   Thu Feb 2 10:44:17 2012 +0100

    test_selfhosting_new_empty_then_users: Test user removal.
    
    * glom/base_db.[h|cc]: Move remove_user() and
    remove_user_from_group() to
    * glom/libglom/db_utils.[h|cc].
    * glom/frame_glom.cc:
    * glom/mode_design/users/dialog_users_list.cc: Adapted.
    * tests/test_selfhosting_new_empty_then_users.cc: Test these
    functions.
    
    Conflicts:
    
    	glom/base_db.cc
    	glom/base_db.h
    	glom/libglom/db_utils.cc

 ChangeLog                                      |   12 ++++
 glom/base_db.cc                                |   81 +-----------------------
 glom/base_db.h                                 |   12 ----
 glom/frame_glom.cc                             |    6 +-
 glom/libglom/db_utils.cc                       |   32 +++++++++
 glom/libglom/db_utils.h                        |    6 ++
 glom/mode_design/users/dialog_users_list.cc    |    6 +-
 tests/test_selfhosting_new_empty_then_users.cc |   58 +++++++++++------
 8 files changed, 96 insertions(+), 117 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 8c1a298..4d0f3f5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2012-02-02  Murray Cumming  <murrayc murrayc com>
 
+	test_selfhosting_new_empty_then_users: Test user removal.
+
+	* glom/base_db.[h|cc]: Move remove_user() and 
+	remove_user_from_group() to
+	* glom/libglom/db_utils.[h|cc].
+	* glom/frame_glom.cc:
+	* glom/mode_design/users/dialog_users_list.cc: Adapted.
+	* tests/test_selfhosting_new_empty_then_users.cc: Test these
+	functions.
+
+2012-02-02  Murray Cumming  <murrayc murrayc com>
+
 	Really prevent changing to developer mode for non-developers.
 
 	* glom/frame_glom.cc: on_menu_developer_developer(): Actually use the
diff --git a/glom/base_db.cc b/glom/base_db.cc
index 211d67a..3f7369f 100644
--- a/glom/base_db.cc
+++ b/glom/base_db.cc
@@ -1733,85 +1733,6 @@ void Base_DB::set_found_set_where_clause_for_portal(FoundSet& found_set, const s
   }
 }
 
-bool Base_DB::add_user(const Glib::ustring& user, const Glib::ustring& password, const Glib::ustring& group)
-{
-  if(user.empty() || password.empty() || group.empty())
-    return false;
-
-  //Create the user:
-  //Note that ' around the user fails, so we use ".
-  Glib::ustring strQuery = "CREATE USER " + DbUtils::escape_sql_id(user) + " PASSWORD '" + password + "'" ; //TODO: Escape the password.
-  if(group == GLOM_STANDARD_GROUP_NAME_DEVELOPER)
-    strQuery += " SUPERUSER CREATEDB CREATEROLE"; //Because SUPERUSER is not "inherited" from groups to members.
-
-  bool test = DbUtils::query_execute_string(strQuery);
-  if(!test)
-  {
-    std::cerr << G_STRFUNC << ": CREATE USER failed." << std::endl;
-    return false;
-  }
-
-  //Add it to the group:
-  strQuery = DbUtils::build_query_add_user_to_group(group, user);
-  test = DbUtils::query_execute_string(strQuery);
-  if(!test)
-  {
-    std::cerr << G_STRFUNC << ": ALTER GROUP failed." << std::endl;
-    return false;
-  }
-
-  //Remove any user rights, so that all rights come from the user's presence in the group:
-  Document* document = get_document();
-  if(!document)
-    return true;
-
-  Document::type_listTableInfo table_list = document->get_tables();
-
-  for(Document::type_listTableInfo::const_iterator iter = table_list.begin(); iter != table_list.end(); ++iter)
-  {
-    const Glib::ustring table_name = (*iter)->get_name();
-    const Glib::ustring strQuery = "REVOKE ALL PRIVILEGES ON " + DbUtils::escape_sql_id(table_name) + " FROM " + DbUtils::escape_sql_id(user);
-    const bool test = DbUtils::query_execute_string(strQuery);
-    if(!test)
-      std::cerr << G_STRFUNC << ": REVOKE failed." << std::endl;
-  }
-
-  return true;
-}
-
-
-bool Base_DB::remove_user(const Glib::ustring& user)
-{
-  if(user.empty())
-    return false;
-
-  const Glib::ustring strQuery = "DROP USER " + DbUtils::escape_sql_id(user);
-  const bool test = DbUtils::query_execute_string(strQuery);
-  if(!test)
-  {
-    std::cerr << G_STRFUNC << ": DROP USER failed" << std::endl;
-    return false;
-  }
-
-  return true;
-}
-
-bool Base_DB::remove_user_from_group(const Glib::ustring& user, const Glib::ustring& group)
-{
-  if(user.empty() || group.empty())
-    return false;
-
-  const Glib::ustring strQuery = "ALTER GROUP " + DbUtils::escape_sql_id(group) + " DROP USER " + DbUtils::escape_sql_id(user);
-  const bool test = DbUtils::query_execute_string(strQuery);
-  if(!test)
-  {
-    std::cerr << G_STRFUNC << ": ALTER GROUP failed." << std::endl;
-    return false;
-  }
-
-  return true;
-}
-
 bool Base_DB::set_database_owner_user(const Glib::ustring& user)
 {
   if(user.empty())
@@ -1843,7 +1764,7 @@ bool Base_DB::disable_user(const Glib::ustring& user)
   for(type_vec_strings::const_iterator iter = vecGroups.begin(); iter != vecGroups.end(); ++iter)
   {
     const Glib::ustring group = *iter;
-    remove_user_from_group(user, group);
+    DbUtils::remove_user_from_group(user, group);
   }
 
   const Glib::ustring strQuery = "ALTER ROLE " + DbUtils::escape_sql_id(user) + " NOLOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE";
diff --git a/glom/base_db.h b/glom/base_db.h
index 5ccb1ad..676693c 100644
--- a/glom/base_db.h
+++ b/glom/base_db.h
@@ -319,18 +319,6 @@ protected:
   typedef std::vector<Glib::ustring> type_vec_strings;
   static type_vec_strings util_vecStrings_from_Fields(const type_vec_fields& fields);
 
-  /** Add a @a user to the database, with the specified @a password, in the specified @a group.
-   * @result true if the addition succeeded.
-   */
-  bool add_user(const Glib::ustring& user, const Glib::ustring& password, const Glib::ustring& group);
-
-  /** Remove the @a user from the database.
-   * @result true if the removal succeeded.
-   */
-  bool remove_user(const Glib::ustring& user);
-
-  bool remove_user_from_group(const Glib::ustring& user, const Glib::ustring& group);
-
   bool set_database_owner_user(const Glib::ustring& user);
 
   /** Revoke any login rights from the user and remove it from any groups.
diff --git a/glom/frame_glom.cc b/glom/frame_glom.cc
index 8f0b2ec..5ec0953 100644
--- a/glom/frame_glom.cc
+++ b/glom/frame_glom.cc
@@ -846,7 +846,7 @@ void Frame_Glom::on_menu_file_toggle_share(const Glib::RefPtr<Gtk::ToggleAction>
         const bool initial_password_provided = connection_request_initial_password(user, password);
         bool added = false;
         if(initial_password_provided)
-          added = add_user(user, password, GLOM_STANDARD_GROUP_NAME_DEVELOPER);
+          added = DbUtils::add_user(document, user, password, GLOM_STANDARD_GROUP_NAME_DEVELOPER);
 
         if(initial_password_provided && added)
         {
@@ -896,7 +896,7 @@ void Frame_Glom::on_menu_file_toggle_share(const Glib::RefPtr<Gtk::ToggleAction>
           const bool reowned = set_database_owner_user(connectionpool->get_user());
           bool removed = false;
           if(reowned)
-            removed = remove_user(default_user);
+            removed = DbUtils::remove_user(default_user);
 
           if(!removed)
           {
@@ -944,7 +944,7 @@ void Frame_Glom::on_menu_file_toggle_share(const Glib::RefPtr<Gtk::ToggleAction>
         Glib::ustring default_password;
         const Glib::ustring default_user = Privs::get_default_developer_user_name(default_password);
 
-        const bool added = add_user(default_user, default_password, GLOM_STANDARD_GROUP_NAME_DEVELOPER);
+        const bool added = DbUtils::add_user(document, default_user, default_password, GLOM_STANDARD_GROUP_NAME_DEVELOPER);
         if(!added)
         {
            shared = true;
diff --git a/glom/libglom/db_utils.cc b/glom/libglom/db_utils.cc
index 450581e..12f72c3 100644
--- a/glom/libglom/db_utils.cc
+++ b/glom/libglom/db_utils.cc
@@ -2061,6 +2061,38 @@ bool add_group(const Document* document, const Glib::ustring& group)
   return true;
 }
 
+bool remove_user(const Glib::ustring& user)
+{
+  if(user.empty())
+    return false;
+
+  const Glib::ustring strQuery = "DROP USER " + DbUtils::escape_sql_id(user);
+  const bool test = DbUtils::query_execute_string(strQuery);
+  if(!test)
+  {
+    std::cerr << G_STRFUNC << ": DROP USER failed" << std::endl;
+    return false;
+  }
+
+  return true;
+}
+
+bool remove_user_from_group(const Glib::ustring& user, const Glib::ustring& group)
+{
+  if(user.empty() || group.empty())
+    return false;
+
+  const Glib::ustring strQuery = "ALTER GROUP " + DbUtils::escape_sql_id(group) + " DROP USER " + DbUtils::escape_sql_id(user);
+  const bool test = DbUtils::query_execute_string(strQuery);
+  if(!test)
+  {
+    std::cerr << G_STRFUNC << ": ALTER GROUP failed." << std::endl;
+    return false;
+  }
+
+  return true;
+}
+
 void set_fake_connection()
 {
   //Allow a fake connection, so sqlbuilder_get_full_query() can work:
diff --git a/glom/libglom/db_utils.h b/glom/libglom/db_utils.h
index 3915d6a..763e0bd 100644
--- a/glom/libglom/db_utils.h
+++ b/glom/libglom/db_utils.h
@@ -168,11 +168,17 @@ Glib::ustring build_query_add_user_to_group(const Glib::ustring& group, const Gl
  */
 bool add_user(const Document* document, const Glib::ustring& user, const Glib::ustring& password, const Glib::ustring& group);
 
+/** Remove the @a user from the database.
+ * @result true if the removal succeeded.
+ */
+bool remove_user(const Glib::ustring& user);
+
 /** Add a @a group to the database.
  * @result true if the addition succeeded.
  */
 bool add_group(const Document* document, const Glib::ustring& group);
 
+bool remove_user_from_group(const Glib::ustring& user, const Glib::ustring& group);
 
 /** Get the value of the @a source_field from the @a relationship, using the @a key_value.
  */
diff --git a/glom/mode_design/users/dialog_users_list.cc b/glom/mode_design/users/dialog_users_list.cc
index 7269c78..e6a22e4 100644
--- a/glom/mode_design/users/dialog_users_list.cc
+++ b/glom/mode_design/users/dialog_users_list.cc
@@ -133,7 +133,7 @@ void Dialog_UsersList::on_button_user_remove()
         const Glib::ustring user = row[m_model_columns_users.m_col_name];
         if(!user.empty())
         {
-          remove_user_from_group(user, m_combo_group->get_active_text());
+          DbUtils::remove_user_from_group(user, m_combo_group->get_active_text());
 
           fill_list();
         }
@@ -169,7 +169,7 @@ void Dialog_UsersList::on_button_user_delete()
 
           if(response == Gtk::RESPONSE_OK)
           {
-            remove_user(user);
+            DbUtils::remove_user(user); //TODO: Warn about failure when this returns false?
             fill_list();
           }
         }
@@ -260,7 +260,7 @@ void Dialog_UsersList::on_button_user_new()
   if(response != Gtk::RESPONSE_OK)
     return;
 
-  add_user(user, password, m_combo_group->get_active_text() /* group */);
+  DbUtils::add_user(get_document(), user, password, m_combo_group->get_active_text() /* group */);
   fill_list();
 }
 
diff --git a/tests/test_selfhosting_new_empty_then_users.cc b/tests/test_selfhosting_new_empty_then_users.cc
index c55bae3..98b4ca4 100644
--- a/tests/test_selfhosting_new_empty_then_users.cc
+++ b/tests/test_selfhosting_new_empty_then_users.cc
@@ -85,6 +85,33 @@ static bool test_add_user(const Glom::Document& document, const Glib::ustring& u
 }
 
 
+static bool change_privileges(const Glib::ustring& group_name, const Glib::ustring& table_name, bool view, bool edit, bool create, bool del)
+{
+  //Change the privs and make sure that it worked:
+  Glom::Privileges privs_new;
+  privs_new.m_view = view;
+  privs_new.m_edit = edit;
+  privs_new.m_create = create;
+  privs_new.m_delete = del;
+  if(!Glom::Privs::set_table_privileges(group_name, table_name, privs_new, false))
+  {
+    std::cerr << "Privs::set_table_privileges() failed for group=" << group_name << ", table_name=" << table_name << std::endl;
+    return false;
+  }
+
+  const Glom::Privileges privs_changed = Glom::Privs::get_table_privileges(group_name, table_name);
+  if( (privs_changed.m_view != privs_new.m_view) ||
+    (privs_changed.m_edit != privs_new.m_edit) ||
+    (privs_changed.m_create != privs_new.m_create) ||
+    (privs_changed.m_delete != privs_new.m_delete) )
+  {
+    std::cerr << "Changing and re-reading privileges failed for group=" << group_name << ", table_name=" << table_name << std::endl;
+    return false;
+  }
+
+  return true;
+}
+
 static bool test(Glom::Document::HostingMode hosting_mode)
 {
   //Create and self-host the document:
@@ -202,27 +229,20 @@ static bool test(Glom::Document::HostingMode hosting_mode)
         }
         */
 
-        //Change the privs and make sure that it worked:
-        Glom::Privileges privs_new;
-        privs_new.m_view = true;
-        privs_new.m_edit = true;
-        privs_new.m_create = true;
-        privs_new.m_delete = false;
-        if(!Glom::Privs::set_table_privileges(group_name, table_name, privs_new, false))
-        {
-          std::cerr << "Privs::set_table_privileges() failed for group=" << group_name << ", table_name=" << table_name << std::endl;
+	if(!change_privileges(group_name, table_name, true, true, true, false))
           return false;
-        }
+      }
 
-        const Glom::Privileges privs_changed = Glom::Privs::get_table_privileges(group_name, table_name);
-        if( (privs_changed.m_view != privs_new.m_view) ||
-            (privs_changed.m_edit != privs_new.m_edit) ||
-            (privs_changed.m_create != privs_new.m_create) ||
-            (privs_changed.m_delete != privs_new.m_delete) )
-        {
-          std::cerr << "Changing and re-reading privileges failed for group=" << group_name << ", table_name=" << table_name << std::endl;
-          return false;
-        }
+      if(!Glom::DbUtils::remove_user_from_group(username, group_name))
+      {
+        std::cerr << "DbUtils::remove_user() failed for user=" << username << ", group=" << group_name << std::endl;
+        return false;
+      }
+
+      if(!Glom::DbUtils::remove_user(username))
+      {
+        std::cerr << "DbUtils::remove_user() failed for user=" << username << std::endl;
+        return false;
       }
 
       ++i;