refactor: Move content folder validation logic to database module

Author: angrynode

src/database/content_folder.rs

@@ -4,9 +4,12 @@ use sea_orm::entity::prelude::*;
 use sea_orm::*;
 use snafu::prelude::*;
 
-use crate::database::category::{self, CategoryError, CategoryOperator};
+use std::str::FromStr;
+
+use crate::database::category::{self, CategoryError};
 use crate::database::operation::{Operation, OperationId, OperationLog, OperationType, Table};
 use crate::database::operator::DatabaseOperator;
+use crate::extractors::normalized_path::{NormalizedPathAbsolute, NormalizedPathComponent};
 use crate::extractors::user::User;
 use crate::routes::content_folder::ContentFolderForm;
 use crate::state::AppState;
@@ -25,7 +28,9 @@ pub struct Model {
     pub id: i32,
     pub name: String,
     #[sea_orm(unique)]
-    pub path: String,
+    // TODO: maybe we'd like relative paths in fact? Why did
+    // we have a leading slash in the first place?
+    pub path: NormalizedPathAbsolute,
     pub category_id: i32,
     #[sea_orm(belongs_to, from = "category_id", to = "id")]
     pub category: HasOne<category::Entity>,
@@ -40,10 +45,12 @@ impl ActiveModelBehavior for ActiveModel {}
 #[derive(Debug, Snafu)]
 #[snafu(visibility(pub))]
 pub enum ContentFolderError {
-    #[snafu(display("There is already a content folder called `{name}`"))]
+    #[snafu(display("There is already a content folder called `{name}` in the current folder."))]
     NameTaken { name: String },
-    #[snafu(display("There is already a content folder in dir `{path}`"))]
-    PathTaken { path: String },
+    #[snafu(display("The folder name is invalid. It must not contain slashes."))]
+    NameInvalid,
+    #[snafu(display("The folder path must appear absolute"))]
+    PathInvalid,
     #[snafu(display("The Content Folder (Path: {path}) does not exist"))]
     NotFound { path: String },
     #[snafu(display("Database error"))]
@@ -52,6 +59,8 @@ pub enum ContentFolderError {
     Logger { source: LoggerError },
     #[snafu(display("Category operation failed"))]
     Category { source: CategoryError },
+    #[snafu(display("Failed to create the folder on disk"))]
+    IO { source: std::io::Error },
 }
 
 #[derive(Clone, Debug)]
@@ -90,14 +99,19 @@ impl ContentFolderOperator {
     ///
     /// Should not fail, unless SQLite was corrupted for some reason.
     pub async fn find_by_path(&self, path: String) -> Result<Model, ContentFolderError> {
+        let path = NormalizedPathAbsolute::from_str(&path)
+            .map_err(|_e| ContentFolderError::PathInvalid)?;
+
         let content_folder = Entity::find_by_path(path.clone())
             .one(&self.state.database)
             .await
             .context(DBSnafu)?;
 
         match content_folder {
             Some(category) => Ok(category),
-            None => Err(ContentFolderError::NotFound { path }),
+            None => Err(ContentFolderError::NotFound {
+                path: path.to_string(),
+            }),
         }
     }
 
@@ -122,35 +136,48 @@ impl ContentFolderOperator {
     ///
     /// Fails if:
     ///
-    /// - name or path is already taken (they should be unique in one folder)
+    /// - name is already taken (they should be unique in one folder)
     /// - path parent directory does not exist (to avoid completely wrong paths)
     pub async fn create(&self, f: &ContentFolderForm) -> Result<Model, ContentFolderError> {
-        // Check duplicates in same folder
-        let list = if let Some(parent_id) = f.parent_id {
-            self.list_child_folders(parent_id).await?
-        } else {
-            let category = CategoryOperator::new(self.state.clone(), None);
-            category
-                .list_folders(f.category_id)
-                .await
-                .context(CategorySnafu)?
-        };
+        let name = NormalizedPathComponent::from_str(&f.name)
+            .map_err(|_e| ContentFolderError::NameInvalid)?;
 
-        if list.iter().any(|x| x.name == f.name) {
-            return Err(ContentFolderError::NameTaken {
-                name: f.name.clone(),
-            });
-        }
+        let category = self
+            .db()
+            .category()
+            .find_by_id(f.category_id)
+            .await
+            .context(CategorySnafu)?;
 
-        if list.iter().any(|x| x.path == f.path) {
-            return Err(ContentFolderError::PathTaken {
-                path: f.path.clone(),
-            });
+        // Check duplicates in same category/folder
+        {
+            let siblings = if let Some(parent_id) = f.parent_id {
+                self.list_child_folders(parent_id).await?
+            } else {
+                self.db()
+                    .category()
+                    .list_folders(f.category_id)
+                    .await
+                    .context(CategorySnafu)?
+            };
+            if siblings.iter().any(|x| x.name == f.name) {
+                return Err(ContentFolderError::NameTaken {
+                    name: f.name.clone(),
+                });
+            }
         }
 
+        // This path is an absolute path, but relative to a category path
+        let inner_path = if let Some(parent_id) = f.parent_id {
+            let parent = self.find_by_id(parent_id).await?;
+            NormalizedPathAbsolute::from_str(&format!("{}/{}", parent.path, name,)).unwrap()
+        } else {
+            NormalizedPathAbsolute::from_str(&format!("/{}", name)).unwrap()
+        };
+
         let model = ActiveModel {
-            name: Set(f.name.clone()),
-            path: Set(f.path.clone()),
+            name: Set(name.to_string()),
+            path: Set(inner_path.clone()),
             category_id: Set(f.category_id),
             parent_id: Set(f.parent_id),
             ..Default::default()
@@ -159,6 +186,13 @@ impl ContentFolderOperator {
         .await
         .context(DBSnafu)?;
 
+        let real_path =
+            NormalizedPathAbsolute::from_str(&format!("{}{}", category.path, inner_path)).unwrap();
+
+        tokio::fs::create_dir_all(&real_path)
+            .await
+            .context(IOSnafu)?;
+
         // Should not fail
         let model = model.try_into_model().unwrap();
 

src/extractors/normalized_path/absolute.rs

@@ -9,7 +9,7 @@ use std::str::FromStr;
 use super::*;
 
 /// [NormalizedPath] with extra constraint that it's absolute.
-#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, Hash, DeriveValueType)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, DeriveValueType)]
 #[sea_orm(value_type = "String")]
 #[serde(into = "String", try_from = "String")]
 pub struct NormalizedPathAbsolute {

src/extractors/normalized_path/component.rs

@@ -9,7 +9,7 @@ use std::str::FromStr;
 use super::*;
 
 /// [NormalizedPath] with extra constraint that it contains no slashes.
-#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, Hash, DeriveValueType)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, DeriveValueType)]
 #[sea_orm(value_type = "String")]
 #[serde(into = "String", try_from = "String")]
 pub struct NormalizedPathComponent {

src/extractors/normalized_path/mod.rs

@@ -22,7 +22,7 @@ pub use relative::*;
 /// - disallows parent dir traversal (`..`)
 /// - has no trailing slash
 /// - may contain current dir `./` but these will be discarded
-#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, Hash, DeriveValueType)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, DeriveValueType)]
 #[sea_orm(value_type = "String")]
 #[serde(into = "String", try_from = "String")]
 pub struct NormalizedPath {

src/extractors/normalized_path/relative.rs

@@ -9,7 +9,7 @@ use std::str::FromStr;
 use super::*;
 
 /// [NormalizedPath] with extra constraint that it's relative.
-#[derive(Clone, Debug, PartialEq, Deserialize, Serialize, Hash, DeriveValueType)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Hash, DeriveValueType)]
 #[sea_orm(value_type = "String")]
 #[serde(into = "String", try_from = "String")]
 pub struct NormalizedPathRelative {

src/routes/content_folder.rs

@@ -3,7 +3,6 @@ use askama_web::WebTemplate;
 use axum::Form;
 use axum::response::Redirect;
 use axum_extra::extract::CookieJar;
-use camino::Utf8PathBuf;
 use serde::{Deserialize, Serialize};
 use snafu::prelude::*;
 
@@ -20,7 +19,6 @@ use crate::state::{AppStateContext, error::*};
 pub struct ContentFolderForm {
     pub name: String,
     pub parent_id: Option<i32>,
-    pub path: String,
     pub category_id: i32,
 }
 
@@ -75,56 +73,20 @@ pub async fn show(
     status.with_template(ContentFolderShowTemplate::new(context, folder))
 }
 
+// TODO: currently an error takes us back to /
 pub async fn create(
     context: AppStateContext,
     jar: CookieJar,
-    Form(mut form): Form<ContentFolderForm>,
+    Form(form): Form<ContentFolderForm>,
 ) -> Result<FlashRedirect, AppStateError> {
-    let categories = context.db.category();
-    let content_folders = context.db.content_folder();
-
-    // build path with Parent folder path (or category path if parent is None) + folder.name
-    let parent_path = if let Some(parent_id) = form.parent_id {
-        let parent_folder = content_folders
-            .find_by_id(parent_id)
-            .await
-            .context(ContentFolderSnafu)?;
-        Utf8PathBuf::from(parent_folder.path)
-    } else {
-        Utf8PathBuf::new()
-    };
-
-    // Get folder category
-    let category: category::Model = categories
-        .find_by_id(form.category_id)
-        .await
-        .context(CategorySnafu)?;
-
-    // If name contains "/" returns an error
-    if form.name.contains("/") {
-        let status = StatusCookie::error(
-            jar,
-            format!(
-                "Failed to create Folder, {} is not valid (it contains '/')",
-                form.name
-            ),
-        );
-
-        let uri = format!("/folders/{}{}", category.name, parent_path.into_string());
-        return Ok(status.redirect(&uri));
-    }
-
-    // build final path with parent_path and path of form
-    form.path = format!("{}/{}", parent_path, form.name);
-
-    let created = content_folders.create(&form).await;
-
-    match created {
+    match context.db.content_folder().create(&form).await {
         Ok(created) => {
-            tokio::fs::create_dir_all(format!("{}/{}", category.path, created.path.clone()))
+            let category = context
+                .db
+                .category()
+                .find_by_id(created.category_id)
                 .await
-                .context(IOSnafu)?;
-
+                .context(CategorySnafu)?;
             let status = StatusCookie::success(
                 jar,
                 format!(
@@ -137,6 +99,7 @@ pub async fn create(
             Ok(status.redirect(&uri))
         }
         // TODO: why don't we produce an error here?
+        // We should also redirect to the appropriate page
         Err(_error) => Ok((jar, Redirect::to("/"))),
     }
 }

templates/categories/show.html

@@ -15,14 +15,12 @@
       <input 
         type="text"
         class="form-control"
-        placeholder="Ex: Saison 1"
         name="name"
         id="name"
       />
     </div> 
 
     <input type="hidden" id="category_id" name="category_id" value="{{ category.id }}" />
-    <input type="hidden" id="path" name="path" value="fake_path" />
 
     <div class="text-center mt-4">
       <input type="Submit" class="btn btn-success" value="Create folder">

templates/content_folders/show.html

@@ -15,15 +15,13 @@
       <input 
         type="text"
         class="form-control"
-        placeholder="Ex: Saison 1"
         name="name"
         id="name"
       />
     </div> 
 
     <input type="hidden" id="category_id" name="category_id" value="{{ category.id }}" />
     <input type="hidden" id="parent_id" name="parent_id" value="{{ folder.id }}" />
-    <input type="hidden" id="path" name="path" value="fake_path" />
 
     <div class="text-center mt-4">
       <input type="Submit" class="btn btn-success" value="Create folder">