From b438a99999bee748c4179ba738ca38b7495faf4d Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Sat, 18 Jul 2026 20:46:27 +0200 Subject: [PATCH] fix: better database rebuild --- CLEANUP.md | 276 +++++++++++++++++++ crates/bds-core/src/engine/media.rs | 97 ++++--- crates/bds-core/src/engine/meta.rs | 20 ++ crates/bds-core/src/engine/metadata_diff.rs | 65 ++++- crates/bds-core/src/engine/rebuild.rs | 280 +++++++++++++++++++- specs/metadata_diff.allium | 20 +- 6 files changed, 694 insertions(+), 64 deletions(-) create mode 100644 CLEANUP.md diff --git a/CLEANUP.md b/CLEANUP.md new file mode 100644 index 0000000..e86d5b7 --- /dev/null +++ b/CLEANUP.md @@ -0,0 +1,276 @@ +# Cleanup Backlog + +Findings from a repo-wide audit (2026-07-18) for over-engineering, dead code, and +idiomatic-Rust issues beyond what clippy/rustfmt catch. Ordered by priority — +the first item is a real bug, the rest are cleanup. Each item is self-contained. + +Ground rules (from AGENTS.md): red/green TDD, run build + tests after each item, +remove unused code instead of keeping it, verify behaviour against the allium +specs in `specs/` and the Elixir baseline in `../bDS2` when in doubt. + +## 1. BUG: temp-file collision in atomic_write + +`crates/bds-core/src/util/atomic_write.rs` uses `path.with_extension("tmp")`, +which *replaces* the last extension. Sibling files that differ only in their +final extension map to the same temp path: + +- `esmeralda.en.md` → `esmeralda.en.tmp` +- `esmeralda.en.meta` → `esmeralda.en.tmp` + +Concurrent writes (e.g. publishing writes post file + sidecars in parallel) can +interleave on the shared temp file and corrupt or cross-write content. It also +clobbers any real file named `*.tmp`. + +**Fix:** append a unique suffix to the full filename instead of replacing the +extension, e.g. `format!("{}.{}.tmp", file_name, std::process::id())` or a +counter/uuid. Add a regression test writing `a.en.md` and `a.en.meta` +concurrently (or at least assert the two temp paths differ). + +## 2. Parallel record-struct layer in the DB glue (~450 lines) + +`crates/bds-core/src/db/from_row.rs` defines a `*Record` twin struct for all 12 +tables plus hand-written `From`/`TryFrom` conversions in both directions. Most +fields are copied 1:1; only a handful need conversion (status/kind enums, +`i32` bools, JSON string-lists). + +**Fix:** implement diesel `ToSql`/`FromSql` (or `serialize`/`deserialize` +wrappers) for `PostStatus`, `TemplateKind`, `TemplateStatus`, `ScriptKind`, +`ScriptStatus`, `NotificationEntity`, `NotificationAction`, plus a newtype for +JSON-serialized `Vec` (tags/categories) and bool-as-i32, then derive +`Queryable`/`Selectable`/`Insertable`/`AsChangeset` on the domain models in +`crates/bds-core/src/model/` directly. Delete the record structs and the +`convert`/`convert_all` helpers. Keep behaviour identical: tags serialize as +JSON arrays, unknown enum strings must still error, `treat_none_as_null` must +be preserved. + +## 3. Split the god file: bds-ui/src/app.rs (9,090 lines) + +`update()` alone spans ~2,250 lines (line 982–3238). The fix pattern already +exists in the same file: `handle_tags_msg` and `handle_settings_msg` delegate a +message sub-enum to a dedicated handler. + +**Fix:** apply the same pattern to the remaining domains — post editor, +media editor, template/script editors, preview/embedded webview, AI actions, +translation flows, sidebar/filtering. Move each handler group into its own +module (e.g. `crates/bds-ui/src/app/posts.rs`), keeping `Message` routing thin. +Pure refactor: no behaviour change, existing tests must stay green. + +## 4. Dead task machinery in engine/task.rs (~130 lines) + +Zero production call sites in the workspace for: + +- `ProgressThrottle` (whole struct + its 2 tests) — its logic is already + duplicated inside `TaskManager::report_progress` +- `TaskProgress` struct +- `try_start` +- `submit_grouped` and the `group_id`/`group_name` fields it fills (carried + through `TaskEntry` and `TaskSnapshot`, never read by the UI) +- `label()`, `message()` accessors +- `next_queued` (test-only) + +**Fix:** delete them and their tests. + +**Related wiring gap (do NOT delete, investigate):** `evict_expired()` and +`is_cancelled()` are also never called from production code. That means +finished tasks are never evicted at runtime and cooperative cancellation never +actually interrupts running work. Either wire them up in `bds-ui/src/app.rs` +(evict on the task-snapshot refresh tick; check `is_cancelled` inside +long-running engine loops) or confirm against the allium spec what the intended +behaviour is. + +## 5. Dead public functions (~250 lines) + +Zero call sites anywhere in the workspace (verified by sweep): + +- `crates/bds-core/src/engine/validate_translations.rs`: + `validate_translations` — only `validate_translations_with_progress` is used +- `crates/bds-core/src/engine/meta.rs`: `update_blog_languages`, + `update_project_metadata` +- `crates/bds-core/src/db/queries/generated_file_hash.rs`: + `delete_generated_file_hash`, `list_generated_file_hashes_by_project` +- `crates/bds-core/src/db/queries/post.rs`: `list_posts_by_project_limited` +- `crates/bds-core/src/db/queries/media.rs`: `list_media_by_project_limited` +- `crates/bds-core/src/db/queries/post_link.rs`: `list_post_backlinks`, + `list_post_outlinks` +- `crates/bds-core/src/db/from_row.rs`: `notification_entity_to_str`, + `notification_action_to_str` + +**Fix:** delete, along with any tests that exist only to exercise them. + +**Special case:** `crates/bds-core/src/db/fts.rs::search_media` has only +test callers. AGENTS.md requires functionality to be tied to UI — check the +allium spec: if media search is specced, wire it into the UI; if not, delete. + +## 6. Delete unused EngineContext + +`crates/bds-core/src/engine/context.rs` — a "shared context passed to engine +operations" that zero engine operations take (every engine fn takes +`conn`/`project_id`/`data_dir` individually). Its only test asserts that struct +fields hold assigned values. + +**Fix:** delete the file, its `pub use` in `crates/bds-core/src/engine/mod.rs`. + +## 7. Derive EngineError with thiserror + +`crates/bds-core/src/engine/error.rs` hand-writes `Display`, `Error::source`, +and four `From` impls (~85 lines). `DatabaseError` in +`crates/bds-core/src/db/connection.rs` already uses +`#[derive(thiserror::Error)]` — the crate is inconsistent. + +**Fix:** convert `EngineError` to a thiserror derive (`#[error(...)]`, +`#[from]`, `#[source]`). Keep the `From` mapping and the +reqwest/serde_json/serde_yaml → `Parse` conversions (thiserror `#[from]` can't +map two sources into one variant with `.to_string()`, so those three stay as +small manual impls or become dedicated variants). + +## 8. Single source of truth for status/kind enum strings + +`PostStatus` ↔ string mapping exists three times: + +1. serde `#[serde(rename_all = "lowercase")]` on the enum + (`crates/bds-core/src/model/post.rs`) +2. `post_status_to_str` / `post_status` in `crates/bds-core/src/db/from_row.rs` +3. the `serde_json::to_string(&status).trim_matches('"')` hack in + `crates/bds-core/src/util/frontmatter.rs` (`PostFrontmatter::from_post`, + `TranslationFrontmatter::from_translation`) + +**Fix:** give each enum (`PostStatus`, `TemplateKind`, `TemplateStatus`, +`ScriptKind`, `ScriptStatus`) an `as_str()` + `FromStr` impl next to its +definition, use it from the DB layer and frontmatter, delete the duplicates and +the serde_json hack. (Combines well with item 2.) + +## 9. Derive frontmatter deserialization (~120 lines) + +`crates/bds-core/src/util/frontmatter.rs`: the four `from_yaml` impls each +hand-roll near-identical `get_str`/`get_string_list` closures over +`serde_yaml::Value`. + +**Fix:** replace with `#[derive(Deserialize)]` + +`#[serde(rename_all = "camelCase")]` structs and one shared +`deserialize_with` helper for ISO-8601 → unix-ms timestamps. Preserve current +lenient behaviour (numbers/bools coerced to strings where fields expect +strings; missing optional fields default). + +**Do NOT touch the serialization side** (`to_yaml`): it is deliberately +hand-rolled for byte-identical golden output against the gray-matter format — +the `golden_output_*` tests assert exact equality with fixture files. + +## 10. Minor shrinks + +- `crates/bds-core/src/util/checksum.rs`: replace the inline `hex` module with + `bytes.iter().map(|b| format!("{b:02x}")).collect::()`. (Correctly + avoids adding a `hex` crate dep — keep it that way.) +- `crates/bds-core/src/i18n/mod.rs`: `RENDER_KEYS` hardcodes 34 keys that must + be kept in sync with the `.ftl` catalogs by hand. Enumerate the parsed + `FluentResource` entries of the English render catalog instead, so new keys + can't be silently missed. +- `crates/bds-core/src/i18n/mod.rs`: `locale_index` can be `locale as usize` + with explicit discriminants on `UiLocale`. + +--- + +# Test Suite Cleanup + +Findings from a full test-suite review (2026-07-18). The suite is largely +healthy: engine/query tests run against real in-memory SQLite, AI tests run the +real HTTP client against a local TCP test server, golden-output tests pin +byte-compatibility with the bDS2 baseline fixtures. No mock-object testing +anti-pattern exists. The problems are hot-air tests (assertions the compiler +already guarantees or that test a literal against itself) and duplication. + +## T1. Delete bds-ui/tests/app_smoke.rs entirely + +Every test in the file is compile-time noise: + +- `message_variants_constructable`, `new_message_variants_constructable` + (~80 lines): construct `Message` enum variants and discard them — the + compiler already guarantees this +- `message_clone_works`: tests `#[derive(Clone)]` +- `bds_app_type_is_public`: an inner function that is never called; purely a + type assertion with no runtime assertion + +The file's own header admits `BdsApp::new()` cannot be tested here. The real +smoke coverage lives in `app.rs`'s inline tests (which drive `update()` against +a real in-memory DB via `new_for_tests`). Delete the file. + +## T2. Hot air in bds-ui/tests/m2_validation.rs + +- `toast_level_variants`, `toast_preserves_message`: assert that a constructor + stores its arguments. Delete. +- `toast_ids_are_monotonically_increasing`, `fresh_toast_is_not_expired`: + duplicate the inline tests in `crates/bds-ui/src/state/toast.rs` + (`toast_ids_are_unique`, `fresh_toast_not_expired`). Keep one location + (suggest inline), delete the other. +- `menu_actions_that_need_project` / `menu_actions_that_need_tab` / + `menu_actions_gated_by_offline`: their own comment admits they don't test the + gating logic — they only re-verify i18n keys (already covered for ALL actions + in `menu_routing.rs::all_menu_actions_translate_in_all_locales`) and then + `assert_eq!(array.len(), N)` on the literal array declared five lines above. + Delete. **Real gap:** the actual enable/disable rules in + `BdsApp::sync_menu_state` (app.rs) are untested — write a test that drives + the real method (or extract its decision logic into a testable function). +- `accelerator_actions_match_spec`: asserts a locally declared array has 15 + elements and that i18n keys are non-empty; never touches the real + accelerator registration in `platform/menu.rs`. Delete, or rewrite to + inspect the actual menu construction. + +## T3. Hot air in bds-core/tests/spec_claims.rs + +- `remaining_value_specs_match` contains + `assert_eq!(["en", "de", "fr", "it", "es"].len(), 5)` — a literal tested + against itself. Drop that line (the serde asserts around it are fine). +- `slug_frozen_after_publish_semantics`: the name promises the slug-freeze + invariant; the body only asserts `published_at` is Some/None after insert + (already covered by roundtrip tests elsewhere). Either write the real test — + attempt a slug change on a published post through the engine API and assert + it's rejected — or delete. + +## T4. Vacuous inline tests in bds-core/src/i18n/mod.rs + +- `ui_locale_is_independent_type`: asserts `"de" != "fr"`. Delete. +- `translate_falls_back_to_english`: identical call and assertion as + `translate_menu_labels`, and it does NOT test fallback — the key it uses has + a German translation. Replace with a key that exists only in en.ftl (that + actually exercises `add_resource_overriding` fallback), or delete. +- `detect_os_locale_does_not_panic`: marginal smoke; keep or drop, zero cost. + +## T5. Tests that only exist to cover dead code + +Deleted together with their subjects (see items 4–6 above): + +- `engine/task.rs`: `progress_throttle_initial_reports`, + `progress_throttle_suppresses_rapid` (test the dead `ProgressThrottle`) +- `engine/context.rs`: `context_holds_references` (asserts struct fields hold + assigned values) +- `engine/error.rs`: `display_variants`, `from_io_error` become + derive-testing once EngineError moves to thiserror; delete with item 7 + +## T6. Change-detector test in bds-ui/tests/menu_routing.rs + +`menu_action_count_matches_spec` asserts `MenuAction::ALL.len() == 28`. It must +be hand-bumped on every menu change and only ever catches forgetting to update +itself; the useful properties (every action has a key, keys unique, all locales +translate) are covered by the other three tests in the file. Delete. + +## Explicitly fine (do not "clean up") + +- `tests/orm_boundary.rs`, `tests/i18n_completeness.rs`, + `tests/packaging_assets.rs`: lints-as-tests. They don't test runtime + behaviour but mechanically enforce AGENTS.md rules (no raw SQL outside the + backend boundary, no untranslated keys, packaging config integrity). Keep. +- `fixture_readability.rs::fixture_database_files_are_not_modified_by_tests`: + meta-test guarding fixture integrity for the whole compat suite. Keep. +- `m1_validation.rs`: the per-field diff-detection matrix looks repetitive but + each test pins one metadata-diff field required by the spec. Keep. +- AI engine tests spawn a real local TCP server — fake at the network + boundary, real production client code exercised. This is the right pattern. +- `util/timestamp.rs::now_is_recent` looks trivial but guards + milliseconds-vs-seconds confusion. Keep. + +--- + +## Explicitly out of scope + +- `crates/bds-cli` being a stub: intentional, part of upcoming extensions. +- The hand-rolled YAML *serialization* in frontmatter.rs and sidecar.rs: + required for byte-compatibility with the baseline output. diff --git a/crates/bds-core/src/engine/media.rs b/crates/bds-core/src/engine/media.rs index 1190c3c..2691e2a 100644 --- a/crates/bds-core/src/engine/media.rs +++ b/crates/bds-core/src/engine/media.rs @@ -8,9 +8,10 @@ use walkdir::WalkDir; use crate::db::fts; use crate::db::queries::media as qm; use crate::db::queries::media_translation as qmt; +use crate::db::queries::post as qp; use crate::db::queries::post_media as qpm; use crate::engine::{EngineError, EngineResult}; -use crate::model::{Media, MediaTranslation}; +use crate::model::{Media, MediaTranslation, PostMedia}; use crate::util::sidecar::{ MediaSidecar, MediaTranslationSidecar, read_sidecar, read_translation_sidecar, }; @@ -641,31 +642,6 @@ pub(crate) fn rebuild_canonical_media( .unwrap_or(&sidecar_rel) .to_string(); - let abs_file = data_dir.join(&file_path); - - // Get file size (if file exists) - let file_size = if abs_file.exists() { - fs::metadata(&abs_file)?.len() as i64 - } else { - sc.size - }; - - // Get checksum (if file exists) — streamed to avoid loading large files into memory - let checksum = if abs_file.exists() { - Some(crate::util::file_hash(&abs_file)?) - } else { - None - }; - - // Get dimensions (if file exists and is an image) - let (width, height) = if abs_file.exists() { - image_dimensions(&abs_file) - .map(|(w, h)| (Some(w as i32), Some(h as i32))) - .unwrap_or((sc.width, sc.height)) - } else { - (sc.width, sc.height) - }; - // Derive filename from file_path let filename = Path::new(&file_path) .file_name() @@ -673,7 +649,7 @@ pub(crate) fn rebuild_canonical_media( .unwrap_or("") .to_string(); - let now = now_unix_ms(); + let linked_post_ids = sc.linked_post_ids.clone(); let existing = qm::get_media_by_id(conn, &sc.id); let created = match existing { @@ -681,9 +657,9 @@ pub(crate) fn rebuild_canonical_media( // Update existing media media.original_name = sc.original_name; media.mime_type = sc.mime_type; - media.size = file_size; - media.width = width; - media.height = height; + media.size = sc.size; + media.width = sc.width; + media.height = sc.height; media.title = sc.title; media.alt = sc.alt; media.caption = sc.caption; @@ -691,9 +667,9 @@ pub(crate) fn rebuild_canonical_media( media.language = sc.language; media.file_path = file_path; media.sidecar_path = sidecar_rel; - media.checksum = checksum; + media.checksum = None; media.tags = sc.tags; - media.updated_at = now; + media.updated_at = sc.updated_at; qm::update_media(conn, &media)?; false } @@ -704,9 +680,9 @@ pub(crate) fn rebuild_canonical_media( filename, original_name: sc.original_name, mime_type: sc.mime_type, - size: file_size, - width, - height, + size: sc.size, + width: sc.width, + height: sc.height, title: sc.title, alt: sc.alt, caption: sc.caption, @@ -714,20 +690,35 @@ pub(crate) fn rebuild_canonical_media( language: sc.language, file_path, sidecar_path: sidecar_rel, - checksum, + checksum: None, tags: sc.tags, created_at: sc.created_at, - updated_at: now, + updated_at: sc.updated_at, }; qm::insert_media(conn, &media)?; true } }; - // Regenerate thumbnails if the binary file exists - if abs_file.exists() { - let thumbnails_dir = data_dir.join("thumbnails"); - let _ = generate_all_thumbnails(&abs_file, &thumbnails_dir, &sc.id); + let existing_post_ids: std::collections::HashSet<_> = + qpm::list_post_media_by_media(conn, &sc.id)? + .into_iter() + .map(|link| link.post_id) + .collect(); + for post_id in linked_post_ids { + if !existing_post_ids.contains(&post_id) && qp::get_post_by_id(conn, &post_id).is_ok() { + qpm::link_media( + conn, + &PostMedia { + id: Uuid::new_v4().to_string(), + project_id: project_id.to_string(), + post_id, + media_id: sc.id.clone(), + sort_order: 0, + created_at: now_unix_ms(), + }, + )?; + } } Ok(created) @@ -1227,6 +1218,30 @@ alt: \"Ein Bild\" assert_eq!(report2.translations_updated, 1); } + #[test] + fn rebuild_media_trusts_sidecars_without_generating_thumbnails() { + let (db, dir) = setup(); + let media_subdir = dir.path().join("media/2024/01"); + fs::create_dir_all(&media_subdir).unwrap(); + DynamicImage::new_rgb8(100, 80) + .save(media_subdir.join("sidecar-only.png")) + .unwrap(); + fs::write( + media_subdir.join("sidecar-only.png.meta"), + "---\nid: sidecar-only\noriginalName: \"photo.png\"\nmimeType: image/png\nsize: 123\nwidth: 640\nheight: 480\ncreatedAt: 2024-01-15T12:00:00.000Z\nupdatedAt: 2024-01-15T12:00:00.000Z\ntags: []\n---", + ) + .unwrap(); + + rebuild_media_from_filesystem(db.conn(), dir.path(), "p1").unwrap(); + + let media = qm::get_media_by_id(db.conn(), "sidecar-only").unwrap(); + assert_eq!( + (media.size, media.width, media.height), + (123, Some(640), Some(480)) + ); + assert!(!dir.path().join("thumbnails").exists()); + } + #[test] fn is_translation_sidecar_detection() { assert!(is_translation_sidecar("photo.jpg.de.meta")); diff --git a/crates/bds-core/src/engine/meta.rs b/crates/bds-core/src/engine/meta.rs index 007cae7..bff29c7 100644 --- a/crates/bds-core/src/engine/meta.rs +++ b/crates/bds-core/src/engine/meta.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::fs; use std::path::Path; +use crate::db::DbConnection as Connection; use crate::engine::EngineResult; use crate::model::PublishingPreferences; use crate::model::metadata::{CategorySettings, ProjectMetadata, TagEntry}; @@ -25,6 +26,25 @@ pub fn write_project_json(data_dir: &Path, meta: &ProjectMetadata) -> EngineResu Ok(()) } +/// Copy the project fields persisted in both representations from project.json +/// into the machine-local project row. +pub fn sync_project_from_file( + conn: &Connection, + data_dir: &Path, + project_id: &str, +) -> EngineResult<()> { + let metadata = read_project_json(data_dir)?; + metadata + .validate() + .map_err(crate::engine::EngineError::Validation)?; + let mut project = crate::db::queries::project::get_project_by_id(conn, project_id)?; + project.name = metadata.name; + project.description = metadata.description; + project.updated_at = crate::util::now_unix_ms(); + crate::db::queries::project::update_project(conn, &project)?; + Ok(()) +} + // ── categories.json ───────────────────────────────────────────────── /// Read meta/categories.json as a sorted array of strings. diff --git a/crates/bds-core/src/engine/metadata_diff.rs b/crates/bds-core/src/engine/metadata_diff.rs index 636486d..2c737f8 100644 --- a/crates/bds-core/src/engine/metadata_diff.rs +++ b/crates/bds-core/src/engine/metadata_diff.rs @@ -255,13 +255,7 @@ fn sync_project_from_file( data_dir: &Path, project_id: &str, ) -> EngineResult<()> { - let metadata = crate::engine::meta::read_project_json(data_dir)?; - let mut project = qproject::get_project_by_id(conn, project_id)?; - project.name = metadata.name; - project.description = metadata.description; - project.updated_at = crate::util::now_unix_ms(); - qproject::update_project(conn, &project)?; - Ok(()) + crate::engine::meta::sync_project_from_file(conn, data_dir, project_id) } fn rewrite_project_from_database( @@ -865,7 +859,7 @@ fn detect_orphan_files( "posts" => ext == "md", "media" => ext == "meta", "templates" => ext == "liquid", - "scripts" => ext == "lua" || ext == "py", + "scripts" => ext == "lua", _ => false, }; @@ -967,8 +961,8 @@ mod tests { use crate::db::queries::template::insert_template; use crate::engine::post::{create_post, publish_post}; use crate::model::{ - Media, Post, PostStatus, Script, ScriptKind, ScriptStatus, Template, TemplateKind, - TemplateStatus, + Media, Post, PostStatus, ProjectMetadata, Script, ScriptKind, ScriptStatus, Template, + TemplateKind, TemplateStatus, }; use crate::util::frontmatter::{ ScriptFrontmatter, TemplateFrontmatter, write_script_file, write_template_file, @@ -1067,6 +1061,57 @@ mod tests { assert_eq!(title_diffs[0].file_value, "Tampered Title"); } + #[test] + fn detects_missing_project_description() { + let (db, dir) = setup(); + crate::engine::meta::write_project_json( + dir.path(), + &ProjectMetadata { + name: "Project p1".into(), + description: None, + public_url: None, + main_language: None, + default_author: None, + max_posts_per_page: 50, + image_import_concurrency: 4, + blogmark_category: None, + pico_theme: None, + semantic_similarity_enabled: false, + blog_languages: vec![], + }, + ) + .unwrap(); + + let report = compute_metadata_diff(db.conn(), dir.path(), "p1").unwrap(); + let project = report + .diffs + .iter() + .find(|item| item.entity_type == "project") + .unwrap(); + assert!(project.fields.iter().any(|field| { + field.field_name == "description" + && field.db_value == "A test project" + && field.file_value.is_empty() + })); + } + + #[test] + fn ignores_python_scripts_during_orphan_scan() { + let (db, dir) = setup(); + let scripts = dir.path().join("scripts"); + fs::create_dir_all(&scripts).unwrap(); + fs::write(scripts.join("legacy.py"), "print('legacy')").unwrap(); + + let report = compute_metadata_diff(db.conn(), dir.path(), "p1").unwrap(); + + assert!( + report + .orphans + .iter() + .all(|orphan| !orphan.file_path.ends_with(".py")) + ); + } + #[test] fn repairs_one_post_diff_from_file_to_database() { let (db, dir) = setup(); diff --git a/crates/bds-core/src/engine/rebuild.rs b/crates/bds-core/src/engine/rebuild.rs index 53e49bb..acdbf5c 100644 --- a/crates/bds-core/src/engine/rebuild.rs +++ b/crates/bds-core/src/engine/rebuild.rs @@ -2,6 +2,7 @@ use std::path::Path; use std::sync::Arc; use crate::db::DbConnection as Connection; +use diesel::prelude::*; use crate::db::fts; use crate::engine::EngineResult; @@ -33,8 +34,8 @@ pub type ProgressFn = Arc; /// Orchestrate a full rebuild from filesystem into the database. /// -/// Ensures FTS tables exist, then rebuilds posts, media, templates, and scripts -/// from their respective filesystem directories. Returns an aggregated report. +/// Replaces the project-scoped database state with project metadata, content, +/// and relationships reconstructed from the filesystem. pub fn rebuild_from_filesystem( conn: &Connection, data_dir: &Path, @@ -49,6 +50,25 @@ pub fn rebuild_from_filesystem_with_progress( data_dir: &Path, project_id: &str, on_progress: Option, +) -> EngineResult { + conn.begin_savepoint()?; + match rebuild_from_filesystem_inner(conn, data_dir, project_id, on_progress) { + Ok(report) => { + conn.release_savepoint()?; + Ok(report) + } + Err(error) => { + let _ = conn.rollback_savepoint(); + Err(error) + } + } +} + +fn rebuild_from_filesystem_inner( + conn: &Connection, + data_dir: &Path, + project_id: &str, + on_progress: Option, ) -> EngineResult { let mut report = FullRebuildReport::default(); let progress = |pct: f32, msg: &str| { @@ -59,9 +79,11 @@ pub fn rebuild_from_filesystem_with_progress( // Phase weights: posts 0.0..0.35, media 0.35..0.70, templates 0.70..0.85, scripts 0.85..1.0 - // 1. Ensure FTS tables exist - progress(0.0, "Ensuring FTS tables..."); + // 1. Load portable project metadata and clear all reconstructible rows. + progress(0.0, "Loading project metadata..."); fts::ensure_fts_tables(conn)?; + crate::engine::meta::sync_project_from_file(conn, data_dir, project_id)?; + clear_project_rows(conn, project_id)?; // 2. Rebuild posts (0.00 .. 0.35) progress(0.01, "Scanning posts..."); @@ -135,15 +157,93 @@ pub fn rebuild_from_filesystem_with_progress( report.scripts_updated = script_report.updated; report.errors.extend(script_report.errors); - // 6. Import tags from tags.json and sync from posts (0.95 .. 1.0) + // 6. Restore relationships and tags (0.95 .. 1.0) progress(0.95, "Importing tags..."); - let _ = super::tag::import_tags_from_file(conn, data_dir, project_id); - let _ = super::tag::sync_tags_from_posts(conn, project_id); + super::tag::import_tags_from_file(conn, data_dir, project_id)?; + super::tag::sync_tags_from_posts(conn, project_id)?; + post::rebuild_all_links(conn, data_dir, project_id)?; + + if !report.errors.is_empty() { + return Err(crate::engine::EngineError::Validation(format!( + "rebuild failed: {}", + report.errors.join("; ") + ))); + } progress(1.0, "Rebuild complete"); Ok(report) } +fn clear_project_rows(conn: &Connection, project_id: &str) -> EngineResult<()> { + use crate::db::schema::{ + dismissed_duplicate_pairs, embedding_keys, generated_file_hashes, import_definitions, + media, media_translations, post_links, post_media, post_translations, posts, scripts, tags, + templates, + }; + + let post_ids = crate::db::queries::post::list_posts_by_project(conn, project_id)? + .into_iter() + .map(|post| post.id) + .collect::>(); + let media_ids = crate::db::queries::media::list_media_by_project(conn, project_id)? + .into_iter() + .map(|media| media.id) + .collect::>(); + for id in &post_ids { + fts::remove_post_from_index(conn, id)?; + } + for id in &media_ids { + fts::remove_media_from_index(conn, id)?; + } + + conn.with(|connection| { + diesel::delete( + post_links::table.filter( + post_links::source_post_id + .eq_any(&post_ids) + .or(post_links::target_post_id.eq_any(&post_ids)), + ), + ) + .execute(connection)?; + diesel::delete(post_media::table.filter(post_media::project_id.eq(project_id))) + .execute(connection)?; + diesel::delete( + post_translations::table.filter(post_translations::project_id.eq(project_id)), + ) + .execute(connection)?; + diesel::delete( + media_translations::table.filter(media_translations::project_id.eq(project_id)), + ) + .execute(connection)?; + diesel::delete( + generated_file_hashes::table.filter(generated_file_hashes::project_id.eq(project_id)), + ) + .execute(connection)?; + diesel::delete(embedding_keys::table.filter(embedding_keys::project_id.eq(project_id))) + .execute(connection)?; + diesel::delete( + dismissed_duplicate_pairs::table + .filter(dismissed_duplicate_pairs::project_id.eq(project_id)), + ) + .execute(connection)?; + diesel::delete( + import_definitions::table.filter(import_definitions::project_id.eq(project_id)), + ) + .execute(connection)?; + diesel::delete(tags::table.filter(tags::project_id.eq(project_id))).execute(connection)?; + diesel::delete(scripts::table.filter(scripts::project_id.eq(project_id))) + .execute(connection)?; + diesel::delete(templates::table.filter(templates::project_id.eq(project_id))) + .execute(connection)?; + diesel::delete(media::table.filter(media::project_id.eq(project_id))) + .execute(connection)?; + diesel::delete(posts::table.filter(posts::project_id.eq(project_id))) + .execute(connection)?; + Ok(()) + })?; + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -152,7 +252,12 @@ mod tests { use crate::db::queries::post as qp; use crate::db::queries::project::{insert_project, make_test_project}; use crate::db::queries::script as qs; + use crate::db::queries::tag as qtag; use crate::db::queries::template as qtpl; + use crate::model::metadata::ProjectMetadata; + use crate::model::{ + Script, ScriptKind, ScriptStatus, Tag, Template, TemplateKind, TemplateStatus, + }; use std::fs; use tempfile::TempDir; @@ -162,6 +267,23 @@ mod tests { fts::ensure_fts_tables(db.conn()).unwrap(); insert_project(db.conn(), &make_test_project("p1", "blog")).unwrap(); let dir = TempDir::new().unwrap(); + crate::engine::meta::write_project_json( + dir.path(), + &ProjectMetadata { + name: "Project p1".into(), + description: Some("A test project".into()), + public_url: None, + main_language: None, + default_author: None, + max_posts_per_page: 50, + image_import_concurrency: 4, + blogmark_category: None, + pico_theme: None, + semantic_similarity_enabled: false, + blog_languages: vec![], + }, + ) + .unwrap(); (db, dir) } @@ -253,6 +375,96 @@ tags: [] fn rebuild_all_entity_types() { let (db, dir) = setup(); + crate::engine::meta::write_project_json( + dir.path(), + &ProjectMetadata { + name: "Rebuilt Project".into(), + description: None, + public_url: None, + main_language: Some("en".into()), + default_author: None, + max_posts_per_page: 50, + image_import_concurrency: 4, + blogmark_category: None, + pico_theme: None, + semantic_similarity_enabled: false, + blog_languages: vec!["en".into()], + }, + ) + .unwrap(); + + qs::insert_script( + db.conn(), + &Script { + id: "stale-script".into(), + project_id: "p1".into(), + slug: "stale-script".into(), + title: "Stale Script".into(), + kind: ScriptKind::Utility, + entrypoint: "main".into(), + enabled: true, + version: 1, + file_path: "scripts/stale.lua".into(), + status: ScriptStatus::Published, + content: None, + created_at: 1, + updated_at: 1, + }, + ) + .unwrap(); + qtpl::insert_template( + db.conn(), + &Template { + id: "stale-template".into(), + project_id: "p1".into(), + slug: "stale-template".into(), + title: "Stale Template".into(), + kind: TemplateKind::Post, + enabled: true, + version: 1, + file_path: "templates/stale.liquid".into(), + status: TemplateStatus::Published, + content: None, + created_at: 1, + updated_at: 1, + }, + ) + .unwrap(); + qp::insert_post( + db.conn(), + &qp::make_test_post("stale-post", "p1", "stale-post"), + ) + .unwrap(); + qm::insert_media(db.conn(), &qm::make_test_media("stale-media", "p1")).unwrap(); + qtag::insert_tag( + db.conn(), + &Tag { + id: "stale-tag".into(), + project_id: "p1".into(), + name: "stale-tag".into(), + color: None, + post_template_slug: None, + created_at: 1, + updated_at: 1, + }, + ) + .unwrap(); + db.conn() + .with(|connection| { + use crate::db::schema::import_definitions; + diesel::insert_into(import_definitions::table) + .values(( + import_definitions::id.eq("stale-import"), + import_definitions::project_id.eq("p1"), + import_definitions::name.eq("Stale Import"), + import_definitions::created_at.eq(1_i64), + import_definitions::updated_at.eq(1_i64), + )) + .execute(connection)?; + Ok(()) + }) + .unwrap(); + // Post let posts_dir = dir.path().join("posts").join("2024").join("01"); fs::create_dir_all(&posts_dir).unwrap(); @@ -288,6 +500,7 @@ height: 600 createdAt: 2024-01-15T12:00:00.000Z updatedAt: 2024-01-15T12:00:00.000Z tags: [] +linkedPostIds: [\"test-post-1\"] --- "; fs::write(media_dir.join("test-media-1.jpg.meta"), sidecar_content).unwrap(); @@ -328,6 +541,7 @@ updatedAt: \"2024-01-15T12:00:00.000Z\" function render() end "; fs::write(scripts_dir.join("test-script.lua"), script_content).unwrap(); + fs::write(scripts_dir.join("legacy.py"), "print('ignore me')").unwrap(); let report = rebuild_from_filesystem(db.conn(), dir.path(), "p1").unwrap(); @@ -349,6 +563,36 @@ function render() end let script = qs::get_script_by_id(db.conn(), "test-script-1").unwrap(); assert_eq!(script.title, "Test Script"); + assert!(qs::get_script_by_id(db.conn(), "stale-script").is_err()); + assert!(qtpl::get_template_by_id(db.conn(), "stale-template").is_err()); + assert!(qp::get_post_by_id(db.conn(), "stale-post").is_err()); + assert!(qm::get_media_by_id(db.conn(), "stale-media").is_err()); + assert!(qtag::get_tag_by_id(db.conn(), "stale-tag").is_err()); + let import_count = db + .conn() + .with(|connection| { + use crate::db::schema::import_definitions::dsl::*; + import_definitions + .filter(project_id.eq("p1")) + .count() + .get_result::(connection) + }) + .unwrap(); + assert_eq!(import_count, 0); + assert_eq!( + qs::list_scripts_by_project(db.conn(), "p1").unwrap().len(), + 1 + ); + + let project = crate::db::queries::project::get_project_by_id(db.conn(), "p1").unwrap(); + assert_eq!(project.name, "Rebuilt Project"); + assert!(project.description.is_none()); + + let links = + crate::db::queries::post_media::list_post_media_by_media(db.conn(), "test-media-1") + .unwrap(); + assert_eq!(links.len(), 1); + assert_eq!(links[0].post_id, "test-post-1"); } #[test] @@ -398,12 +642,24 @@ updatedAt: \"2024-01-15T12:00:00.000Z\" assert_eq!(r1.templates_updated, 0); assert!(r1.errors.is_empty(), "errors: {:?}", r1.errors); - // Second rebuild - should update, not create + // A full rebuild clears the project rows before importing them again. let r2 = rebuild_from_filesystem(db.conn(), dir.path(), "p1").unwrap(); - assert_eq!(r2.posts_created, 0); - assert_eq!(r2.posts_updated, 1); - assert_eq!(r2.templates_created, 0); - assert_eq!(r2.templates_updated, 1); + assert_eq!(r2.posts_created, 1); + assert_eq!(r2.posts_updated, 0); + assert_eq!(r2.templates_created, 1); + assert_eq!(r2.templates_updated, 0); assert!(r2.errors.is_empty(), "errors: {:?}", r2.errors); } + + #[test] + fn failed_rebuild_rolls_back_truncation() { + let (db, dir) = setup(); + qp::insert_post(db.conn(), &qp::make_test_post("keep-me", "p1", "keep-me")).unwrap(); + let posts_dir = dir.path().join("posts"); + fs::create_dir_all(&posts_dir).unwrap(); + fs::write(posts_dir.join("broken.md"), "not frontmatter").unwrap(); + + assert!(rebuild_from_filesystem(db.conn(), dir.path(), "p1").is_err()); + assert!(qp::get_post_by_id(db.conn(), "keep-me").is_ok()); + } } diff --git a/specs/metadata_diff.allium b/specs/metadata_diff.allium index 080c71a..ab75b25 100644 --- a/specs/metadata_diff.allium +++ b/specs/metadata_diff.allium @@ -13,6 +13,7 @@ surface MetadataMaintenanceSurface { provides: MetadataDiffRequested(project) + RebuildDatabaseRequested(project) RebuildFromFilesystemRequested(project, entity_type) RepairMetadataDiffItemRequested(project, direction, item) } @@ -39,7 +40,8 @@ rule RunMetadataDiff { when: MetadataDiffRequested(project) -- Runs as background task via TaskManager -- Compares DB records against filesystem files for: - -- posts, translations, media, scripts, templates + -- project metadata (including a missing description), posts, + -- translations, media, Lua scripts, templates -- Detected fields: tags, categories, title, excerpt, author, -- language, status, templateSlug, dates for post in project.posts: @@ -74,6 +76,22 @@ rule RunMetadataDiff { -- content to detect vectors that need recomputation. } +rule RebuildDatabase { + when: RebuildDatabaseRequested(project) + -- The filesystem is the complete source of truth for portable project data. + -- Existing project-scoped content and relationship rows are removed before + -- project.json, posts, translations, media sidecars and links, tags, + -- templates, and Lua scripts are imported. Python files in scripts/ are ignored. + ensures: ProjectMetadataImported(project, "meta/project.json") + ensures: post/RebuildPostsFromFiles(project) + ensures: media/RebuildMediaFromFiles(project) + ensures: script/RebuildScriptsFromFiles(project) + ensures: template/RebuildTemplatesFromFiles(project) + @guidance + -- Rebuilding media trusts persisted sidecar size, dimensions, and metadata. + -- Thumbnail regeneration remains a separate maintenance operation. +} + rule RepairMetadataDiffItem { when: RepairMetadataDiffItemRequested(project, direction, item) -- Resolves a single diff in one direction.