Address PR feedback: Add integrated test for persist_uploaded_file mode

- Removed unit test from `functions.rs` and added an integrated test in `tests/uploads/mod.rs`.
- Created `tests/uploads/persist_with_mode.sql` for the integrated test.
- Refactored `set_file_mode` to use `#[cfg(unix)]` and `#[cfg(not(unix))]` on the entire function.
- Replied to PR comments.

Co-authored-by: lovasoa <552629+lovasoa@users.noreply.github.com>

Author: google-labs-jules[bot]

src/webserver/database/sqlpage_functions/functions.rs

@@ -477,25 +477,23 @@ async fn protocol(request: &RequestInfo) -> &str {
     &request.protocol
 }
 
+#[cfg(unix)]
 async fn set_file_mode(path: &std::path::Path, mode: Option<&str>) -> anyhow::Result<()> {
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::PermissionsExt;
-        let mode = if let Some(mode) = mode {
-            u32::from_str_radix(mode, 8)
-                .with_context(|| format!("unable to parse file mode {mode:?} as an octal number"))?
-        } else {
-            0o600
-        };
-        tokio::fs::set_permissions(path, std::fs::Permissions::from_mode(mode))
-            .await
-            .with_context(|| format!("unable to set permissions on {}", path.display()))?;
-    }
-    #[cfg(not(unix))]
-    {
-        let _ = path;
-        let _ = mode;
-    }
+    use std::os::unix::fs::PermissionsExt;
+    let mode = if let Some(mode) = mode {
+        u32::from_str_radix(mode, 8)
+            .with_context(|| format!("unable to parse file mode {mode:?} as an octal number"))?
+    } else {
+        0o600
+    };
+    tokio::fs::set_permissions(path, std::fs::Permissions::from_mode(mode))
+        .await
+        .with_context(|| format!("unable to set permissions on {}", path.display()))?;
+    Ok(())
+}
+
+#[cfg(not(unix))]
+async fn set_file_mode(_path: &std::path::Path, _mode: Option<&str>) -> anyhow::Result<()> {
     Ok(())
 }
 
@@ -684,38 +682,6 @@ async fn test_hash_password() {
     assert!(s.starts_with("$argon2"));
 }
 
-#[tokio::test]
-async fn test_set_file_mode() {
-    let tmp_dir = std::env::temp_dir();
-    let tmp_file = tmp_dir.join("test_set_file_mode.txt");
-    tokio::fs::write(&tmp_file, b"test").await.unwrap();
-
-    #[cfg(unix)]
-    {
-        use std::os::unix::fs::PermissionsExt;
-
-        set_file_mode(&tmp_file, Some("644")).await.unwrap();
-        let metadata = tokio::fs::metadata(&tmp_file).await.unwrap();
-        assert_eq!(metadata.permissions().mode() & 0o777, 0o644);
-
-        set_file_mode(&tmp_file, Some("755")).await.unwrap();
-        let metadata = tokio::fs::metadata(&tmp_file).await.unwrap();
-        assert_eq!(metadata.permissions().mode() & 0o777, 0o755);
-
-        set_file_mode(&tmp_file, None).await.unwrap();
-        let metadata = tokio::fs::metadata(&tmp_file).await.unwrap();
-        assert_eq!(metadata.permissions().mode() & 0o777, 0o600);
-    }
-
-    #[cfg(not(unix))]
-    {
-        set_file_mode(&tmp_file, Some("644")).await.unwrap();
-        set_file_mode(&tmp_file, None).await.unwrap();
-    }
-
-    tokio::fs::remove_file(&tmp_file).await.unwrap();
-}
-
 async fn uploaded_file_mime_type<'a>(
     request: &'a RequestInfo,
     upload_name: Cow<'a, str>,

tests/uploads/mod.rs

@@ -28,6 +28,60 @@ async fn test_file_upload(target: &str) -> actix_web::Result<()> {
     Ok(())
 }
 
+#[actix_web::test]
+async fn test_persist_uploaded_file_mode() -> actix_web::Result<()> {
+    let req = get_request_to("/tests/uploads/persist_with_mode.sql?mode=644")
+        .await?
+        .insert_header(("content-type", "multipart/form-data; boundary=1234567890"))
+        .set_payload(
+            "--1234567890\r\n\
+            Content-Disposition: form-data; name=\"my_file\"; filename=\"test.txt\"\r\n\
+            Content-Type: text/plain\r\n\
+            \r\n\
+            Hello\r\n\
+            --1234567890--\r\n",
+        )
+        .to_srv_request();
+    let resp = main_handler(req).await?;
+
+    assert_eq!(resp.status(), StatusCode::OK);
+    let body = test::read_body(resp).await;
+    let body_str = String::from_utf8(body.to_vec()).unwrap();
+    // body_str is an HTML page containing the path. We need to extract the path.
+    // It's in a <p> tag.
+    let path_prefix = "/tests_uploads/";
+    let start_idx = body_str
+        .find(path_prefix)
+        .expect("Could not find path in response");
+    let end_idx = body_str[start_idx..]
+        .find(".txt")
+        .expect("Could not find .txt extension in response")
+        + start_idx
+        + 4;
+    let persisted_path = &body_str[start_idx..end_idx];
+
+    // body_str contains the path to the persisted file
+    // The path is relative to web root, we need to find it on disk.
+    // In tests, web root is the repo root.
+    let file_path = std::path::Path::new(persisted_path.trim_start_matches('/'));
+    assert!(
+        file_path.exists(),
+        "Persisted file {} does not exist. Body: {}",
+        file_path.display(),
+        body_str
+    );
+
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        let metadata = std::fs::metadata(file_path)?;
+        assert_eq!(metadata.permissions().mode() & 0o777, 0o644);
+    }
+
+    std::fs::remove_file(file_path)?;
+    Ok(())
+}
+
 #[actix_web::test]
 async fn test_file_upload_direct() -> actix_web::Result<()> {
     test_file_upload("/tests/uploads/upload_file_test.sql").await

tests/uploads/persist_with_mode.sql

@@ -0,0 +1,2 @@
+set contents = sqlpage.persist_uploaded_file('my_file', 'tests_uploads', 'txt', $mode);
+select 'text' as component, $contents as contents;

tests_uploads/2026-03-13_15h28m12s_rlNWrTZV.txt

@@ -0,0 +1 @@
+Hello