Merge pull request #201 from NetsBlox/project-cleanup-s3

Use project actions to cleanup empty transient projects

crates/cloud/src/auth/mod.rs

@@ -5,6 +5,7 @@ pub(crate) mod libraries;
 pub(crate) mod network;
 pub(crate) mod oauth;
 pub(crate) mod projects;
+pub(crate) mod system;
 pub(crate) mod users;
 
 pub(crate) use crate::auth::collaboration::*;
@@ -14,6 +15,7 @@ pub(crate) use crate::auth::libraries::*;
 pub(crate) use crate::auth::network::*;
 pub(crate) use crate::auth::oauth::*;
 pub(crate) use crate::auth::projects::*;
+pub(crate) use crate::auth::system::*;
 pub(crate) use crate::auth::users::*;
 
 use crate::app_data::AppData;

crates/cloud/src/auth/projects.rs

@@ -7,7 +7,7 @@ use crate::app_data::AppData;
 use crate::errors::UserError;
 use crate::utils;
 
-use super::is_moderator;
+use super::{is_moderator, ManageSystem};
 
 /// Permissions to view a specific project
 pub(crate) struct ViewProject {
@@ -28,43 +28,20 @@ pub(crate) struct EditProject {
 }
 
 pub(crate) struct DeleteProject {
-    pub(crate) metadata: ProjectMetadata,
+    pub(crate) id: api::ProjectId,
     _private: (),
 }
 
-/// Permissions to approve projects that trigger manual approval
-pub(crate) struct ModerateProjects {
-    _private: (),
-}
-
-#[cfg(test)]
-impl ViewProject {
-    pub(crate) fn test(metadata: ProjectMetadata) -> Self {
-        Self {
-            metadata,
-            _private: (),
-        }
-    }
-}
-
-#[cfg(test)]
 impl DeleteProject {
-    pub(crate) fn test(metadata: ProjectMetadata) -> Self {
-        Self {
-            metadata,
-            _private: (),
-        }
+    /// Get project deletion permissions from system management permissions.
+    pub(crate) fn from_manage_system(_witness: &ManageSystem, id: api::ProjectId) -> Self {
+        Self { id, _private: () }
     }
 }
 
-#[cfg(test)]
-impl EditProject {
-    pub(crate) fn test(metadata: ProjectMetadata) -> Self {
-        Self {
-            metadata,
-            _private: (),
-        }
-    }
+/// Permissions to approve projects that require manual approval
+pub(crate) struct ModerateProjects {
+    _private: (),
 }
 
 pub(crate) async fn try_view_project(
@@ -146,7 +123,7 @@ pub(crate) async fn try_delete_project(
     super::try_edit_user(app, req, client_id.as_ref(), &metadata.owner)
         .await
         .map(|_eu| DeleteProject {
-            metadata,
+            id: metadata.id,
             _private: (),
         })
 }
@@ -212,3 +189,68 @@ fn flatten<T>(nested: Option<Option<T>>) -> Option<T> {
         None => None,
     }
 }
+
+#[cfg(test)]
+mod test_utils {
+    use super::*;
+
+    impl ViewProject {
+        pub(crate) fn test(metadata: ProjectMetadata) -> Self {
+            Self {
+                metadata,
+                _private: (),
+            }
+        }
+    }
+
+    impl DeleteProject {
+        pub(crate) fn test(metadata: ProjectMetadata) -> Self {
+            Self {
+                id: metadata.id,
+                _private: (),
+            }
+        }
+    }
+
+    impl EditProject {
+        pub(crate) fn test(metadata: ProjectMetadata) -> Self {
+            Self {
+                metadata,
+                _private: (),
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    #[actix_web::test]
+    #[ignore]
+    async fn test_try_view_project_owner() {
+        todo!();
+    }
+
+    #[actix_web::test]
+    #[ignore]
+    async fn test_try_view_project_invited() {
+        todo!();
+    }
+
+    #[actix_web::test]
+    #[ignore]
+    async fn test_try_view_project_group_owner() {
+        todo!();
+    }
+
+    #[actix_web::test]
+    #[ignore]
+    async fn test_try_view_project_admin() {
+        todo!();
+    }
+
+    #[actix_web::test]
+    #[ignore]
+    async fn test_try_view_project_403() {
+        todo!();
+    }
+}

crates/cloud/src/auth/system.rs

@@ -0,0 +1,15 @@
+use crate::network::topology::network;
+
+/// This is used to get "system" permissions used when the network topology needs
+/// to perform actions. It is not intended to be used directly but, rather, to be
+/// able to be converted to other concrete authorizations
+
+pub(crate) struct ManageSystem {
+    _private: (),
+}
+
+/// Get permissions to manage the system. Only the network topology can obtain these
+/// permissions.
+pub(crate) fn try_manage_system(_network: &network::Topology) -> ManageSystem {
+    ManageSystem { _private: () }
+}

crates/cloud/src/network/topology/mod.rs

@@ -1,6 +1,6 @@
 mod address;
 mod client;
-mod network;
+pub(crate) mod network;
 
 use crate::app_data::AppData;
 use crate::common::api::{ClientId, ExternalClient, ProjectId, RoleData, RoomState};

crates/cloud/src/network/topology/network.rs

@@ -1,3 +1,4 @@
+use crate::auth;
 use crate::common::api;
 use crate::common::api::{
     AppId, BrowserClientState, ClientState, ExternalClient, OccupantState, RoleId, RoleState,
@@ -136,7 +137,7 @@ impl ProjectNetwork {
     }
 }
 
-pub struct Topology {
+pub(crate) struct Topology {
     app_data: Option<AppData>,
 
     clients: HashMap<ClientId, Client>,
@@ -614,7 +615,7 @@ impl Topology {
                 .await
                 .map_err(InternalError::DatabaseConnectionError)?
                 .map(|md| match md.save_state {
-                    SaveState::Created => unreachable!(),
+                    SaveState::Created => unreachable!(), // Cannot reach here since this is triggered when the last user leaves
                     SaveState::Transient => ProjectCleanup::Immediately,
                     SaveState::Broken => ProjectCleanup::Delayed,
                     SaveState::Saved => ProjectCleanup::None,
@@ -623,17 +624,26 @@ impl Topology {
 
             match cleanup {
                 ProjectCleanup::Immediately => {
-                    app.project_metadata
-                        .delete_one(query, None)
-                        .await
-                        .map_err(InternalError::DatabaseConnectionError)?;
+                    let actions = app.as_project_actions();
+                    let system_auth = auth::try_manage_system(self);
+                    let dp =
+                        auth::DeleteProject::from_manage_system(&system_auth, project_id.clone());
+
+                    if let Err(err) = actions.delete_project(&dp).await {
+                        log::error!("Unable to delete project {}: {}", project_id, &err);
+                    }
                 }
                 ProjectCleanup::Delayed => {
                     let ten_minutes = Duration::new(10 * 60, 0);
                     let delete_at = SystemTime::now() + ten_minutes;
                     let update = doc! {"$set": {
                         "deleteAt": DateTime::from_system_time(delete_at)}
                     };
+
+                    // FIXME: this should call delete_project since it:
+                    //   - can leave data on s3 if deleted by MongoDB
+                    //   - won't invalidate the cache
+                    // We need to remove the index from app data
                     app.project_metadata
                         .update_one(query, update, None)
                         .await

crates/cloud/src/projects/actions.rs

@@ -615,7 +615,7 @@ impl<'a> ProjectActions<'a> {
         &self,
         dp: &auth::projects::DeleteProject,
     ) -> Result<api::ProjectMetadata, UserError> {
-        let query = doc! {"id": &dp.metadata.id};
+        let query = doc! {"id": &dp.id};
         let metadata = self
             .project_metadata
             .find_one_and_delete(query, None)