Add test for user cleanup and fix user cleanup function

src/core.rs

@@ -1512,7 +1512,7 @@ pub fn admin_do_cleanup<T: MedalConnection>(conn: &T, session_token: &str, csrf_
 
     let mut data = json_val::Map::new();
     if let Ok((n_users, n_groups, n_teachers, n_other)) = result {
-        let infodata = format!(",n_users:{},n_groups:{},n_teachers:{},n_other:{}",
+        let infodata = format!(",\"n_users\":{},\"n_groups\":{},\"n_teachers\":{},\"n_other\":{}",
                            n_users, n_groups, n_teachers, n_other);
         data.insert("data".to_string(), to_json(&infodata));
         Ok(("delete_ok".to_string(), data))

src/db_conn.base.rs

@@ -274,7 +274,7 @@ impl MedalConnection for Connection {
                           sex = $11,
                           is_admin = $12,
                           is_teacher = $13,
-                          account_created = $14,
+                          managed_by = $14,
                           email = $15,
                           email_unconfirmed = $16
                       WHERE id = $17",
@@ -291,7 +291,7 @@ impl MedalConnection for Connection {
                        &session.sex,
                        &session.is_admin,
                        &session.is_teacher,
-                       &session.account_created,
+                       &session.managed_by,
                        &session.email,
                        &session.email_unconfirmed,
                        &session.id])
@@ -310,6 +310,12 @@ impl MedalConnection for Connection {
 
         SessionUser::minimal(id, session_token.to_owned(), csrf_token)
     }
+    fn session_set_activity_dates(&self, session_id: i32, account_created: Option<time::Timespec>, last_login: Option<time::Timespec>, last_activity: Option<time::Timespec>) {
+        let query = "UPDATE session
+                     SET account_created = $2, last_login = $3, last_activity = $4
+                     WHERE id = $1";
+        self.execute(query, &[&session_id, &account_created, &last_login, &last_activity]).unwrap();
+    }
     fn get_session_or_new(&self, key: &str) -> Result<SessionUser, ()> {
         fn disable_old_session_and_create_new(conn: &Connection, key: &str) -> Result<SessionUser, ()> {
             let query = "UPDATE session
@@ -1458,15 +1464,17 @@ impl MedalConnection for Connection {
 
     // TODO, should those unwraps be handled?
     fn remove_old_users_and_groups(&self, maxstudentage: time::Timespec, maxteacherage: Option<time::Timespec>, maxage: Option<time::Timespec>) -> Result<(i32, i32, i32, i32),()> {
+        // Get list of all groups where students will be removed
         let query = "SELECT managed_by
                      FROM session
-                     WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL
+                     WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL AND managed_by IS NOT NULL
                      AND ((last_login < $1 AND last_activity < $1)
                           OR (last_login < $1 AND last_activity IS NULL)
                           OR (last_login IS NULL AND last_activity < $1)
                           OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
         let mut groups: Vec<i32> = self.query_map_many(query, &[&maxstudentage], |row| row.get(0)).unwrap();
 
+        // Remove students
         let query = "DELETE
                      FROM session
                      WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL
@@ -1476,13 +1484,17 @@ impl MedalConnection for Connection {
                           OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
         self.execute(query, &[&maxstudentage]).unwrap();
 
+        // Bookkeeping
         let n_users = groups.len() as i32;
         let mut n_groups = 0;
         let mut n_teachers = 0;
         let mut n_other = 0;
+
+        // Get list of groups, where users have been removed from
         groups.sort();
         groups.dedup();
 
+        // Delete all groups that became empty by removing students
         let query = "SELECT count(*)
                      FROM session
                      WHERE managed_by = $1;";
@@ -1499,6 +1511,7 @@ impl MedalConnection for Connection {
             }
         }
 
+        // Remove teachers
         let query = "SELECT id
                      FROM session
                      WHERE is_teacher = $1
@@ -1509,6 +1522,7 @@ impl MedalConnection for Connection {
         if let Some(maxteacherage) = maxteacherage {
             let teachers: Vec<i32> = self.query_map_many(query, &[&true, &maxteacherage], |row| row.get(1)).unwrap();
 
+            // Only remove if no groups are remaining
             let query = "SELECT count(*)
                          FROM usergroup
                          WHERE admin = $1;";
@@ -1526,21 +1540,22 @@ impl MedalConnection for Connection {
             }
         }
 
+        // Remove other users
         if let Some(maxage) = maxage {
             let query = "SELECT count(*)
                          FROM session
-                         AND ((last_login < $1 AND last_activity < $1)
-                              OR (last_login < $1 AND last_activity IS NULL)
-                              OR (last_login IS NULL AND last_activity < $1)
-                              OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
-            n_other = self.query_map_one(query, &[&maxage], |row| row.get(1)).unwrap().unwrap();
+                         WHERE ((last_login < $1 AND last_activity < $1)
+                                OR (last_login < $1 AND last_activity IS NULL)
+                                OR (last_login IS NULL AND last_activity < $1)
+                                OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
+            n_other = self.query_map_one(query, &[&maxage], |row| row.get(0)).unwrap().unwrap();
 
             let query = "DELETE
                          FROM session
-                         AND ((last_login < $1 AND last_activity < $1)
-                              OR (last_login < $1 AND last_activity IS NULL)
-                              OR (last_login IS NULL AND last_activity < $1)
-                              OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
+                         WHERE ((last_login < $1 AND last_activity < $1)
+                                OR (last_login < $1 AND last_activity IS NULL)
+                                OR (last_login IS NULL AND last_activity < $1)
+                                OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
             self.execute(query, &[&maxage]).unwrap();
         }
 

src/db_conn.rs

@@ -44,6 +44,8 @@ pub trait MedalConnection {
     ///
     /// Returns the `SessionUser` of the session.
     fn new_session(&self, key: &str) -> SessionUser;
+    /// Set activity date (for testing purposes)
+    fn session_set_activity_dates(&self, session_id: i32, account_created: Option<time::Timespec>, last_login: Option<time::Timespec>, last_activity: Option<time::Timespec>);
     /// Saves the session data of `session` in the database.
     fn save_session(&self, session: SessionUser);
     /// Combination of [`get_session`](#tymethod.get_session) and  [`new_session`](#tymethod.new_session).

src/db_conn_postgres.rs

@@ -393,7 +393,7 @@ impl MedalConnection for Connection {
                           sex = $11,
                           is_admin = $12,
                           is_teacher = $13,
-                          account_created = $14,
+                          managed_by = $14,
                           email = $15,
                           email_unconfirmed = $16
                       WHERE id = $17",
@@ -410,7 +410,7 @@ impl MedalConnection for Connection {
                        &session.sex,
                        &session.is_admin,
                        &session.is_teacher,
-                       &session.account_created,
+                       &session.managed_by,
                        &session.email,
                        &session.email_unconfirmed,
                        &session.id])
@@ -429,6 +429,12 @@ impl MedalConnection for Connection {
 
         SessionUser::minimal(id, session_token.to_owned(), csrf_token)
     }
+    fn session_set_activity_dates(&self, session_id: i32, account_created: Option<time::Timespec>, last_login: Option<time::Timespec>, last_activity: Option<time::Timespec>) {
+        let query = "UPDATE session
+                     SET account_created = $2, last_login = $3, last_activity = $4
+                     WHERE id = $1";
+        self.execute(query, &[&session_id, &account_created, &last_login, &last_activity]).unwrap();
+    }
     fn get_session_or_new(&self, key: &str) -> Result<SessionUser, ()> {
         fn disable_old_session_and_create_new(conn: &Connection, key: &str) -> Result<SessionUser, ()> {
             let query = "UPDATE session
@@ -1577,15 +1583,23 @@ impl MedalConnection for Connection {
 
     // TODO, should those unwraps be handled?
     fn remove_old_users_and_groups(&self, maxstudentage: time::Timespec, maxteacherage: Option<time::Timespec>, maxage: Option<time::Timespec>) -> Result<(i32, i32, i32, i32),()> {
+        let query = "SELECT id
+                     FROM session
+                      WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL AND managed_by IS NOT NULL AND
+  ((last_login < $1 AND last_activity < $1))";
+        self.query_map_many(query, &[&maxstudentage], |row| println!("===== ===== blub: {}", row.get::<_, i32>(0))).unwrap();
+
+        // Get list of all groups where students will be removed
         let query = "SELECT managed_by
                      FROM session
-                     WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL
+                     WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL AND managed_by IS NOT NULL
                      AND ((last_login < $1 AND last_activity < $1)
                           OR (last_login < $1 AND last_activity IS NULL)
                           OR (last_login IS NULL AND last_activity < $1)
                           OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
         let mut groups: Vec<i32> = self.query_map_many(query, &[&maxstudentage], |row| row.get(0)).unwrap();
 
+        // Remove students
         let query = "DELETE
                      FROM session
                      WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL
@@ -1595,13 +1609,18 @@ impl MedalConnection for Connection {
                           OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
         self.execute(query, &[&maxstudentage]).unwrap();
 
+        // Bookkeeping
         let n_users = groups.len() as i32;
+        println!("====     {}", n_users);
         let mut n_groups = 0;
         let mut n_teachers = 0;
         let mut n_other = 0;
+
+        // Get list of groups, where users have been removed from
         groups.sort();
         groups.dedup();
 
+        // Delete all groups that became empty by removing students
         let query = "SELECT count(*)
                      FROM session
                      WHERE managed_by = $1;";
@@ -1618,6 +1637,7 @@ impl MedalConnection for Connection {
             }
         }
 
+        // Remove teachers
         let query = "SELECT id
                      FROM session
                      WHERE is_teacher = $1
@@ -1628,6 +1648,7 @@ impl MedalConnection for Connection {
         if let Some(maxteacherage) = maxteacherage {
             let teachers: Vec<i32> = self.query_map_many(query, &[&true, &maxteacherage], |row| row.get(1)).unwrap();
 
+            // Only remove if no groups are remaining
             let query = "SELECT count(*)
                          FROM usergroup
                          WHERE admin = $1;";
@@ -1645,21 +1666,22 @@ impl MedalConnection for Connection {
             }
         }
 
+        // Remove other users
         if let Some(maxage) = maxage {
             let query = "SELECT count(*)
                          FROM session
-                         AND ((last_login < $1 AND last_activity < $1)
-                              OR (last_login < $1 AND last_activity IS NULL)
-                              OR (last_login IS NULL AND last_activity < $1)
-                              OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
-            n_other = self.query_map_one(query, &[&maxage], |row| row.get(1)).unwrap().unwrap();
+                         WHERE ((last_login < $1 AND last_activity < $1)
+                                OR (last_login < $1 AND last_activity IS NULL)
+                                OR (last_login IS NULL AND last_activity < $1)
+                                OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
+            n_other = self.query_map_one(query, &[&maxage], |row| row.get(0)).unwrap().unwrap();
 
             let query = "DELETE
                          FROM session
-                         AND ((last_login < $1 AND last_activity < $1)
-                              OR (last_login < $1 AND last_activity IS NULL)
-                              OR (last_login IS NULL AND last_activity < $1)
-                              OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
+                         WHERE ((last_login < $1 AND last_activity < $1)
+                                OR (last_login < $1 AND last_activity IS NULL)
+                                OR (last_login IS NULL AND last_activity < $1)
+                                OR (last_login IS NULL AND last_activity IS NULL AND account_created < $1))";
             self.execute(query, &[&maxage]).unwrap();
         }
 

src/db_conn_sqlite_new.rs

@@ -393,7 +393,7 @@ impl MedalConnection for Connection {
                           sex = ?11,
                           is_admin = ?12,
                           is_teacher = ?13,
-                          account_created = ?14,
+                          managed_by = ?14,
                           email = ?15,
                           email_unconfirmed = ?16
                       WHERE id = ?17",
@@ -410,7 +410,7 @@ impl MedalConnection for Connection {
                        &session.sex,
                        &session.is_admin,
                        &session.is_teacher,
-                       &session.account_created,
+                       &session.managed_by,
                        &session.email,
                        &session.email_unconfirmed,
                        &session.id])
@@ -429,6 +429,12 @@ impl MedalConnection for Connection {
 
         SessionUser::minimal(id, session_token.to_owned(), csrf_token)
     }
+    fn session_set_activity_dates(&self, session_id: i32, account_created: Option<time::Timespec>, last_login: Option<time::Timespec>, last_activity: Option<time::Timespec>) {
+        let query = "UPDATE session
+                     SET account_created = ?2, last_login = ?3, last_activity = ?4
+                     WHERE id = ?1";
+        self.execute(query, &[&session_id, &account_created, &last_login, &last_activity]).unwrap();
+    }
     fn get_session_or_new(&self, key: &str) -> Result<SessionUser, ()> {
         fn disable_old_session_and_create_new(conn: &Connection, key: &str) -> Result<SessionUser, ()> {
             let query = "UPDATE session
@@ -1577,15 +1583,23 @@ impl MedalConnection for Connection {
 
     // TODO, should those unwraps be handled?
     fn remove_old_users_and_groups(&self, maxstudentage: time::Timespec, maxteacherage: Option<time::Timespec>, maxage: Option<time::Timespec>) -> Result<(i32, i32, i32, i32),()> {
+        let query = "SELECT id
+                     FROM session
+                      WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL AND managed_by IS NOT NULL AND
+  ((last_login < ?1 AND last_activity < ?1))";
+        self.query_map_many(query, &[&maxstudentage], |row| println!("===== ===== blub: {}", row.get::<_, i32>(0))).unwrap();
+
+        // Get list of all groups where students will be removed
         let query = "SELECT managed_by
                      FROM session
-                     WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL
+                     WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL AND managed_by IS NOT NULL
                      AND ((last_login < ?1 AND last_activity < ?1)
                           OR (last_login < ?1 AND last_activity IS NULL)
                           OR (last_login IS NULL AND last_activity < ?1)
                           OR (last_login IS NULL AND last_activity IS NULL AND account_created < ?1))";
         let mut groups: Vec<i32> = self.query_map_many(query, &[&maxstudentage], |row| row.get(0)).unwrap();
 
+        // Remove students
         let query = "DELETE
                      FROM session
                      WHERE username IS NULL AND password IS NULL AND oauth_foreign_id IS NULL AND oauth_provider IS NULL
@@ -1595,13 +1609,18 @@ impl MedalConnection for Connection {
                           OR (last_login IS NULL AND last_activity IS NULL AND account_created < ?1))";
         self.execute(query, &[&maxstudentage]).unwrap();
 
+        // Bookkeeping
         let n_users = groups.len() as i32;
+        println!("====     {}", n_users);
         let mut n_groups = 0;
         let mut n_teachers = 0;
         let mut n_other = 0;
+
+        // Get list of groups, where users have been removed from
         groups.sort();
         groups.dedup();
 
+        // Delete all groups that became empty by removing students
         let query = "SELECT count(*)
                      FROM session
                      WHERE managed_by = ?1;";
@@ -1618,6 +1637,7 @@ impl MedalConnection for Connection {
             }
         }
 
+        // Remove teachers
         let query = "SELECT id
                      FROM session
                      WHERE is_teacher = ?1
@@ -1628,6 +1648,7 @@ impl MedalConnection for Connection {
         if let Some(maxteacherage) = maxteacherage {
             let teachers: Vec<i32> = self.query_map_many(query, &[&true, &maxteacherage], |row| row.get(1)).unwrap();
 
+            // Only remove if no groups are remaining
             let query = "SELECT count(*)
                          FROM usergroup
                          WHERE admin = ?1;";
@@ -1645,21 +1666,22 @@ impl MedalConnection for Connection {
             }
         }
 
+        // Remove other users
         if let Some(maxage) = maxage {
             let query = "SELECT count(*)
                          FROM session
-                         AND ((last_login < ?1 AND last_activity < ?1)
-                              OR (last_login < ?1 AND last_activity IS NULL)
-                              OR (last_login IS NULL AND last_activity < ?1)
-                              OR (last_login IS NULL AND last_activity IS NULL AND account_created < ?1))";
-            n_other = self.query_map_one(query, &[&maxage], |row| row.get(1)).unwrap().unwrap();
+                         WHERE ((last_login < ?1 AND last_activity < ?1)
+                                OR (last_login < ?1 AND last_activity IS NULL)
+                                OR (last_login IS NULL AND last_activity < ?1)
+                                OR (last_login IS NULL AND last_activity IS NULL AND account_created < ?1))";
+            n_other = self.query_map_one(query, &[&maxage], |row| row.get(0)).unwrap().unwrap();
 
             let query = "DELETE
                          FROM session
-                         AND ((last_login < ?1 AND last_activity < ?1)
-                              OR (last_login < ?1 AND last_activity IS NULL)
-                              OR (last_login IS NULL AND last_activity < ?1)
-                              OR (last_login IS NULL AND last_activity IS NULL AND account_created < ?1))";
+                         WHERE ((last_login < ?1 AND last_activity < ?1)
+                                OR (last_login < ?1 AND last_activity IS NULL)
+                                OR (last_login IS NULL AND last_activity < ?1)
+                                OR (last_login IS NULL AND last_activity IS NULL AND account_created < ?1))";
             self.execute(query, &[&maxage]).unwrap();
         }
 

src/main.rs

@@ -981,24 +981,29 @@ mod tests {
     }
 
     #[test]
-    fn check_cleanup() { // STUB
+    fn check_cleanup() {
         start_server_and_fn(8091, |conn| {
+            let ago170days = Some(time::get_time() - time::Duration::days(170));
+            let ago190days = Some(time::get_time() - time::Duration::days(190));
+
             let mut test_user = conn.new_session("");
             test_user.username = Some("testusr".to_string());
             test_user.set_password(&"testpw").expect("Set Password did not work correctly.");
-            test_user.last_login = Some(time::get_time() - time::Duration::days(190));
+            conn.session_set_activity_dates(test_user.id, ago190days, ago190days, ago190days);
             conn.save_session(test_user);
 
             let mut test_user = conn.new_session("");
-            test_user.logincode = Some("teststdold".to_string());
+            test_user.lastname = Some("teststdold".to_string());
             test_user.logincode = Some("logincode1".to_string());
-            test_user.last_login = Some(time::get_time() - time::Duration::days(190));
+            test_user.managed_by = Some(1); // Fake id, should this group really exist?
+            conn.session_set_activity_dates(test_user.id, ago190days, ago190days, ago190days);
             conn.save_session(test_user);
 
             let mut test_user = conn.new_session("");
-            test_user.logincode = Some("teststdnew".to_string());
+            test_user.lastname = Some("teststdnew".to_string());
             test_user.logincode = Some("logincode2".to_string());
-            test_user.last_login = Some(time::get_time() - time::Duration::days(170));
+            test_user.managed_by = Some(1);
+            conn.session_set_activity_dates(test_user.id, ago190days, ago170days, ago190days);
             conn.save_session(test_user);
 
             addsimpleuser(conn, "testadm".to_string(), "testpw1".to_string(), false, true);
@@ -1007,10 +1012,18 @@ mod tests {
                                                    .redirect(reqwest::RedirectPolicy::none())
                                                    .build()
                                                    .unwrap();
-
+            // Login as Admin