fix: sort numbered names by value in both folder trees

src/database.rs

@@ -372,7 +372,13 @@ impl Database {
             .foreign_keys(true)
             .journal_mode(SqliteJournalMode::Wal)
             .synchronous(SqliteSynchronous::Normal)
-            .busy_timeout(config.sqlite_busy_timeout);
+            .busy_timeout(config.sqlite_busy_timeout)
+            // Numbers in names by value, for listings paged in SQL; see
+            // `natural_order` for why no schema object may name it.
+            .collation(
+                crate::natural_order::COLLATION,
+                crate::natural_order::compare,
+            );
 
         let pool = SqlitePoolOptions::new()
             .max_connections(config.db_max_connections)

src/lib.rs

@@ -9,6 +9,7 @@ pub mod database;
 pub mod enrichment;
 pub mod lyrics;
 pub mod media;
+pub mod natural_order;
 pub mod oauth;
 pub mod outbound;
 pub mod pid;

src/natural_order.rs

@@ -0,0 +1,121 @@
+//! Natural order for names a person reads: runs of digits compare by value,
+//! so `2 Song.flac` comes before `10 Song.flac` and `100 Song.flac`.
+//!
+//! Registered on every pooled connection as the SQLite collation
+//! [`COLLATION`], so a listing paged in SQL is ordered in SQL. It is a
+//! collation of this binary only: it must never appear in a schema object —
+//! an index, a view, a `CHECK` — or the database would no longer open in a
+//! tool that lacks it. Queries name it; the schema never does.
+
+use std::cmp::Ordering;
+use std::iter::Peekable;
+use std::str::Chars;
+
+/// The name queries use: `ORDER BY x COLLATE natural_order`.
+/// Not `natural`, which SQLite reads as the keyword of `NATURAL JOIN`.
+pub const COLLATION: &str = "natural_order";
+
+/// Digits by value, everything else case-folded, character by character.
+///
+/// A total order, as SQLite requires of a collation: two names that compare
+/// equal by those rules — `07` and `7`, `Abc` and `abc` — are then ordered by
+/// their bytes, so the only names that compare equal are identical ones.
+pub fn compare(a: &str, b: &str) -> Ordering {
+    natural(a, b).then_with(|| a.cmp(b))
+}
+
+fn natural(a: &str, b: &str) -> Ordering {
+    let (mut left, mut right) = (a.chars().peekable(), b.chars().peekable());
+    loop {
+        match (left.peek().copied(), right.peek().copied()) {
+            (None, None) => return Ordering::Equal,
+            (None, Some(_)) => return Ordering::Less,
+            (Some(_), None) => return Ordering::Greater,
+            (Some(x), Some(y)) if x.is_ascii_digit() && y.is_ascii_digit() => {
+                let (m, n) = (digits(&mut left), digits(&mut right));
+                let (m, n) = (m.trim_start_matches('0'), n.trim_start_matches('0'));
+                // Without leading zeros, a longer run is a larger number; runs
+                // of one length compare as their digits do. No parsing, so a
+                // run longer than any integer type still has its place.
+                match m.len().cmp(&n.len()).then_with(|| m.cmp(n)) {
+                    Ordering::Equal => {}
+                    unequal => return unequal,
+                }
+            }
+            (Some(x), Some(y)) => {
+                left.next();
+                right.next();
+                match x.to_lowercase().cmp(y.to_lowercase()) {
+                    Ordering::Equal => {}
+                    unequal => return unequal,
+                }
+            }
+        }
+    }
+}
+
+fn digits(chars: &mut Peekable<Chars<'_>>) -> String {
+    let mut run = String::new();
+    while let Some(digit) = chars.next_if(char::is_ascii_digit) {
+        run.push(digit);
+    }
+    run
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn sorted(names: &[&str]) -> Vec<String> {
+        let mut names: Vec<String> = names.iter().map(|name| (*name).to_owned()).collect();
+        names.sort_by(|a, b| compare(a, b));
+        names
+    }
+
+    #[test]
+    fn numbers_sort_by_value() {
+        assert_eq!(
+            sorted(&[
+                "100 Snooze.m4a",
+                "2 Paradise.m4a",
+                "10 Ballad.m4a",
+                "1 Morning.mp3"
+            ]),
+            [
+                "1 Morning.mp3",
+                "2 Paradise.m4a",
+                "10 Ballad.m4a",
+                "100 Snooze.m4a"
+            ]
+        );
+        assert_eq!(
+            sorted(&["Disc 10", "Disc 9", "Disc 1"]),
+            ["Disc 1", "Disc 9", "Disc 10"]
+        );
+    }
+
+    #[test]
+    fn letters_fold_case_beyond_ascii() {
+        assert_eq!(
+            sorted(&["b", "Été", "A", "étoile"]),
+            ["A", "b", "étoile", "Été"]
+        );
+        assert_eq!(compare("abc", "ABD"), Ordering::Less);
+    }
+
+    #[test]
+    fn only_identical_names_compare_equal() {
+        assert_eq!(compare("7", "7"), Ordering::Equal);
+        assert_ne!(compare("07", "7"), Ordering::Equal);
+        assert_ne!(compare("Abc", "abc"), Ordering::Equal);
+        assert_eq!(compare("07", "7"), compare("07", "7"));
+        assert_eq!(compare("07", "7"), compare("7", "07").reverse());
+    }
+
+    #[test]
+    fn a_run_longer_than_any_integer_still_sorts() {
+        let huge = "9".repeat(40);
+        assert_eq!(compare("1", &huge), Ordering::Less);
+        assert_eq!(compare(&format!("{huge}0"), &huge), Ordering::Greater);
+    }
+}

src/services/library_folders.rs

@@ -46,7 +46,8 @@ pub struct FiledFolder {
 /// A library's whole filed tree, as one flat list a client draws from.
 #[derive(Debug, Clone, Serialize, ToSchema)]
 pub struct FiledTree {
-    /// Every folder, siblings by name, case-insensitively.
+    /// Every folder, siblings by name in natural order: numbers by value,
+    /// letters case-folded.
     pub folders: Vec<FiledFolder>,
     /// The available tracks filed nowhere: the root's **Unfiled** entry.
     pub unfiled_track_count: i64,
@@ -267,7 +268,7 @@ impl DomainServices {
                (SELECT COUNT(*) FROM track_placement p JOIN track t ON t.id=p.track_id \
                 WHERE p.folder_id=f.id AND t.is_available=1) AS track_count \
              FROM library_folder f WHERE f.library_id=? \
-             ORDER BY f.name_key, f.name, f.id",
+             ORDER BY f.name COLLATE natural_order, f.id",
         )
         .bind(library_id.to_string())
         .fetch_all(self.db.pool())

src/services/library_tree.rs

@@ -125,8 +125,9 @@ impl DomainServices {
     /// member of, and for a folder no available track runs through; the root
     /// of an empty library is an empty listing. `Invalid` for a malformed path.
     ///
-    /// Folders sort by name, case-insensitively. Tracks sort by file name,
-    /// except in the received folder, whose file names are hashes: there they
+    /// Folders and tracks sort by name in natural order — numbers by value,
+    /// letters case-folded (`crate::natural_order`) — so `2 …` comes before
+    /// `10 …`. Tracks sort by file name, except in the received folder, whose file names are hashes: there they
     /// sort by artist then title, a missing artist last.
     pub async fn disk_tree(
         &self,
@@ -160,7 +161,7 @@ impl DomainServices {
                     SUM(instr(rest, '/') > 0 \
                         AND instr(substr(rest, instr(rest, '/') + 1), '/') = 0) AS direct \
              FROM beneath GROUP BY name \
-             ORDER BY name IS NOT NULL, name COLLATE NOCASE, name",
+             ORDER BY name IS NOT NULL, name COLLATE natural_order",
         )
         .bind(tree.rest_from())
         .bind(user_id.to_string())
@@ -221,7 +222,7 @@ impl DomainServices {
                 concat!(
                     song_select!(),
                     directly_in!(),
-                    " ORDER BY t.relative_path COLLATE NOCASE, t.relative_path LIMIT ? OFFSET ?"
+                    " ORDER BY t.relative_path COLLATE natural_order LIMIT ? OFFSET ?"
                 )
             };
             tracks = sqlx::query(sql)

tests/native_api.rs

@@ -5115,3 +5115,125 @@ async fn filed_folders_sort_a_library_in_the_database_only() {
         StatusCode::CREATED
     );
 }
+
+/// Names with numbers in them sort by value, in SQL, so a page cut there cuts
+/// the order a person reads: `2 …` before `10 …` before `100 …`, `Disc 2`
+/// before `Disc 10`, in the disk tree and among filed folders alike.
+#[tokio::test]
+async fn numbered_names_sort_by_value_in_both_trees() {
+    let (_temp, config, state) = test_app().await;
+    let password = uuid::Uuid::new_v4().to_string();
+    let hash = security::hash_password(&password).unwrap();
+    let owner = state
+        .db
+        .create_account("natural-owner", &hash, AccountRole::User, now_ms())
+        .await
+        .unwrap();
+    let root = std::fs::canonicalize(&config.data_dir).unwrap();
+    let library_id = state
+        .db
+        .create_library(
+            owner,
+            "Natural",
+            &root,
+            LibraryVisibility::Private,
+            now_ms(),
+        )
+        .await
+        .unwrap();
+    let scan_id = state
+        .db
+        .create_scan_job(library_id, Some(owner), "manual")
+        .await
+        .unwrap();
+    let paths = [
+        "100 Snooze.m4a",
+        "2 Paradise.m4a",
+        "10 Ballad.m4a",
+        "1 Morning.mp3",
+        "Disc 10/a.flac",
+        "Disc 2/a.flac",
+        "disc 1/a.flac",
+    ];
+    state
+        .db
+        .start_scan_job(scan_id, paths.len() as i64, false)
+        .await
+        .unwrap();
+    for (index, path) in paths.iter().enumerate() {
+        let mut input = browse_input(40_000 + index, path, "Album", "Artist", None, None);
+        input.relative_path = (*path).to_owned();
+        state
+            .db
+            .apply_catalog_track(library_id, scan_id, &input, None, false)
+            .await
+            .unwrap();
+    }
+    state.db.finish_scan_job(scan_id, 0).await.unwrap();
+
+    let router = waveflow_server::app(&config, state.clone());
+    let token = login_token(&router, "natural-owner", &password).await;
+    let get = |uri: String| {
+        let router = router.clone();
+        let token = token.clone();
+        async move {
+            json_body(
+                router
+                    .oneshot(
+                        Request::get(uri)
+                            .header("authorization", format!("Bearer {token}"))
+                            .body(Body::empty())
+                            .unwrap(),
+                    )
+                    .await
+                    .unwrap(),
+            )
+            .await
+        }
+    };
+    let names = |listing: &serde_json::Value, key: &str| -> Vec<String> {
+        listing[key]
+            .as_array()
+            .unwrap()
+            .iter()
+            .map(|entry| entry["name"].as_str().unwrap().to_owned())
+            .collect()
+    };
+
+    let top = get(format!("/api/v2/libraries/{library_id}/tree")).await;
+    assert_eq!(names(&top, "folders"), ["disc 1", "Disc 2", "Disc 10"]);
+    assert_eq!(
+        names(&top, "tracks"),
+        [
+            "1 Morning.mp3",
+            "2 Paradise.m4a",
+            "10 Ballad.m4a",
+            "100 Snooze.m4a"
+        ]
+    );
+    // The page is cut in that order: past the three folders, the second and
+    // third tracks are 2 and 10, not 10 and 100.
+    let page = get(format!(
+        "/api/v2/libraries/{library_id}/tree?offset=4&limit=2"
+    ))
+    .await;
+    assert_eq!(names(&page, "tracks"), ["2 Paradise.m4a", "10 Ballad.m4a"]);
+
+    let folders_uri = format!("/api/v2/libraries/{library_id}/folders");
+    for name in ["Vol 10", "Vol 2", "vol 1"] {
+        let created = router
+            .clone()
+            .oneshot(
+                Request::post(folders_uri.clone())
+                    .header("authorization", format!("Bearer {token}"))
+                    .header("content-type", "application/json")
+                    .body(Body::from(serde_json::json!({ "name": name }).to_string()))
+                    .unwrap(),
+            )
+            .await
+            .unwrap();
+        assert_eq!(created.status(), StatusCode::CREATED);
+    }
+    let filed = get(folders_uri).await;
+    assert_eq!(names(&filed, "folders"), ["vol 1", "Vol 2", "Vol 10"]);
+}

webapp/src/api/folders.test.ts

@@ -15,11 +15,12 @@ const folder = (id: string, parent: string | null, name = id): FiledFolder => ({
 });
 
 describe("the filed tree, read on the client", () => {
+  // In the order the server answers: every folder by name, natural order.
   const tree = [
-    folder("rock", null, "Rock"),
+    folder("ambient", null, "ambient"),
     folder("indie", "rock", "Indie"),
     folder("lofi", "indie", "lo-fi"),
-    folder("ambient", null, "ambient"),
+    folder("rock", null, "Rock"),
   ];
 
   it("walks from the root down to a folder", () => {
@@ -37,7 +38,7 @@ describe("the filed tree, read on the client", () => {
     expect(folderOutline(looped)).toEqual([]);
   });
 
-  it("outlines siblings by name, case aside, each before its children", () => {
+  it("outlines siblings in the server's order, each before its children", () => {
     expect(
       folderOutline(tree).map(({ folder: entry, depth }) => [
         entry.name,
@@ -51,6 +52,21 @@ describe("the filed tree, read on the client", () => {
     ]);
   });
 
+  it("keeps the server's sibling order rather than sorting again", () => {
+    // `natural_order` puts these as listed; a locale sort would move "Ä 2"
+    // to the front, and the picker would disagree with the cards.
+    const answered = [
+      folder("a2", null, "a 2"),
+      folder("a10", null, "A 10"),
+      folder("umlaut", null, "Ä 2"),
+      folder("v2", null, "Vol 2"),
+      folder("v10", null, "Vol 10"),
+    ];
+    expect(
+      folderOutline(answered).map(({ folder: entry }) => entry.name),
+    ).toEqual(["a 2", "A 10", "Ä 2", "Vol 2", "Vol 10"]);
+  });
+
   it("knows what sits beneath a folder", () => {
     expect(isWithin(tree, "lofi", "rock")).toBe(true);
     expect(isWithin(tree, "rock", "rock")).toBe(true);

webapp/src/api/folders.ts

@@ -121,8 +121,14 @@ export function folderTrail(
 }
 
 /**
- * The tree flattened in display order — siblings by name, each followed by
- * its own sub-folders — with each folder's depth, the root's children at 0.
+ * The tree flattened in display order — siblings as the server ordered them,
+ * each followed by its own sub-folders — with each folder's depth, the
+ * root's children at 0.
+ *
+ * **No sort here.** The server orders folders by its `natural_order`
+ * collation, and the cards beside this list draw them in that order; a second
+ * sort written in JavaScript would be a second rule, and two rules drift — the
+ * picker and the cards would disagree on where "Ä 2" goes.
  */
 export function folderOutline(
   folders: FiledFolder[],
@@ -136,10 +142,7 @@ export function folderOutline(
   const outline: Array<{ folder: FiledFolder; depth: number }> = [];
   const seen = new Set<string>();
   const walk = (parent: string | null, depth: number) => {
-    const siblings = [...(children.get(parent) ?? [])].sort((a, b) =>
-      a.name.localeCompare(b.name, undefined, { sensitivity: "base" }),
-    );
-    for (const folder of siblings) {
+    for (const folder of children.get(parent) ?? []) {
       if (seen.has(folder.id)) continue;
       seen.add(folder.id);
       outline.push({ folder, depth });