From 85993ebc3e8287042b9e62e16280964cbb87f386 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 20:26:15 -0400 Subject: [PATCH] feat(persist): persist BankBook under `banks` key, retire legacy bank_index, route capture to active bank (B2) --- src/bank_book.cpp | 20 +++++++++ src/bank_book.h | 16 +++++++ src/main.cpp | 20 +++++---- src/persist.cpp | 92 ++++++++++++++++++++++++++-------------- src/persist.h | 57 ++++++++++++++++++------- tests/test_bank_book.cpp | 68 +++++++++++++++++++++++++++++ 6 files changed, 218 insertions(+), 55 deletions(-) diff --git a/src/bank_book.cpp b/src/bank_book.cpp index 9d6df3c..bec9a03 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -668,4 +668,24 @@ std::optional BankBook::deserialize(const std::string& json) { return book; } +BankBook BankBook::loadFromPersisted(const std::string& banksJson, + const std::string& legacyJson) { + // Precedence 1: the authoritative `banks` blob. A present-but-malformed blob is + // an error, not an absence — degrade to an empty book rather than falling through + // to a stale legacy key (which would resurrect superseded single-bank state). + if (!banksJson.empty()) { + auto book = deserialize(banksJson); + return book ? std::move(*book) : BankBook{}; + } + // Precedence 2: no `banks` yet, but a legacy `bank_index` — one-way pool migration + // (deserialize's parse-time legacy path promotes it into the pool). A malformed + // legacy blob likewise degrades to empty. + if (!legacyJson.empty()) { + auto book = deserialize(legacyJson); + return book ? std::move(*book) : BankBook{}; + } + // Precedence 3: a brand-new / never-captured project — a fresh empty book. + return BankBook{}; +} + } // namespace reasampler diff --git a/src/bank_book.h b/src/bank_book.h index e4f1ed7..7aeece9 100644 --- a/src/bank_book.h +++ b/src/bank_book.h @@ -190,6 +190,22 @@ public: // shape going forward; the legacy key is retired by the B2 shell). static std::optional deserialize(const std::string& json); + // Resolve a BankBook from the two persisted ext-state values a project may carry: + // the authoritative `banks` blob and the retired-but-possibly-present legacy + // `bank_index` blob. The persist shell (B2) hands both raw strings straight here so + // the load-source decision stays REAPER-free and unit-tested. Precedence: + // 1. non-empty `banksJson` present -> deserialize it (authoritative). If it is + // MALFORMED, do NOT silently fall back to the legacy blob — a corrupt `banks` + // blob is an error, not an absence; return an empty book so a stale legacy key + // can never resurrect a superseded single-bank state over a broken book. + // 2. else non-empty `legacyJson` -> deserialize it (one-way pool migration). + // 3. else (both absent/empty) -> a fresh empty book (pool only). + // Never returns nullopt: an unloadable input degrades to the empty book (matching + // the shell's existing "malformed -> ignore, start empty" behaviour), so the caller + // has one branchless install path. + static BankBook loadFromPersisted(const std::string& banksJson, + const std::string& legacyJson); + private: std::vector banks_; // ordinal order; banks_[0] is always the pool std::string activeBankId_; // always names a live bank; defaults to pool diff --git a/src/main.cpp b/src/main.cpp index 200a11a..176d7e0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -111,9 +111,10 @@ static int g_cmdCancelRealtime = 0; // The persistence session (M4): owns the in-memory BankIndex and bridges it to // project ext state. A timer tick drives g_session.poll() to detect project -// load / Save-As; capture adds Samples to g_session.bank(); after a capture we -// serialize the bank back into the active project's ext state so it travels with -// the .rpp. Replaces the M3 session-only g_bank. +// load / Save-As; capture adds Samples to g_session.bank() — which (B2) resolves to +// the ACTIVE bank's index inside the session's BankBook; after a capture we serialize +// the book back into the active project's ext state (the `banks` key) so it travels +// with the .rpp. Replaces the M3 session-only g_bank. static reasampler::ReaSamplerSession g_session; // --- M8 in-flight realtime capture (async, timer-driven) -------------------- @@ -133,8 +134,9 @@ static reasampler::RealtimeCaptureHandle g_rtCapture; static ReaProject* g_rtCaptureProject = nullptr; // Commit a finished realtime capture (a Done tick/abort with an Ok result): add the -// Sample to the bank, persist + MarkProjectDirty, log. Shared by the tick-completion -// path and the abort paths. On a non-Ok result, logs the failure only. +// Sample to the ACTIVE bank (g_session.bank() resolves to book.activeIndex() — B2), +// persist + MarkProjectDirty, log. Shared by the tick-completion path and the abort +// paths. On a non-Ok result, logs the failure only. static void CommitRealtimeResult(const reasampler::CaptureResult& res) { if (res.status != reasampler::CaptureStatus::Ok) @@ -547,10 +549,12 @@ static void RunCapture(const reasampler::CaptureActionDef& def) return; } + // Add to the ACTIVE bank: g_session.bank() resolves to book.activeIndex() (B2). reasampler::AddResult added = g_session.bank().add(res.sample); - // Persist the updated bank into the active project's ext state so the capture - // survives Save / close+reopen (M4) and travels with the .rpp. saveToActiveProject - // also calls MarkProjectDirty. Non-destructive: writes only our own ext-state key. + // Persist the updated book into the active project's ext state (the `banks` key) + // so the capture survives Save / close+reopen (M4) and travels with the .rpp. + // saveToActiveProject also clears the retired legacy key and calls MarkProjectDirty. + // Non-destructive: writes only our own ext-state keys. g_session.saveToActiveProject(); std::string log = "ReaSampler: " + res.message + "\n"; diff --git a/src/persist.cpp b/src/persist.cpp index e4c316e..4bb88b8 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -5,11 +5,15 @@ // WITHOUT REAPERAPI_IMPLEMENT — main.cpp is the one TU that defines the API // pointers; here they are extern (CLAUDE.md §contract). // -// Storage: SetProjExtState / GetProjExtState, namespace "reasampler", key -// "bank_index". Ext state is stored INSIDE the .rpp, so the index travels with -// the project automatically (CONTEXT.md §Persistence & paths). The only thing -// that does NOT travel for free is the physical bank folder; on Save-As to a new -// directory we relocate it so the index's relative paths still resolve. +// Storage: SetProjExtState / GetProjExtState, namespace "reasampler". Phase B: the +// whole BankBook (pool as bank-zero + named banks) is written under key "banks" +// (authoritative); the legacy single-bank key "bank_index" is RETIRED — cleared on +// save (SetProjExtState with "" deletes it) and read only once, to migrate a pre- +// multi-bank project's index into the pool. Ext state is stored INSIDE the .rpp, so +// the banks travel with the project automatically (CONTEXT.md §Persistence & paths). +// The only thing that does NOT travel for free is the physical bank folder; on +// Save-As to a new directory we relocate it so the indices' relative paths still +// resolve. // // PROJECT-LOAD / SAVE-AS DETECTION (chosen mechanism): // Driven by REAPER's "timer" register (main.cpp). Each poll() reads the active @@ -174,12 +178,22 @@ void ReaSamplerSession::saveToActiveProject() { if (!proj) return; // no active project — nothing to persist if (rppPath.empty()) return; // unsaved project — no .rpp to store into - const std::string json = bank_.serialize(); + // Phase B: the whole book (pool as bank-zero + named banks) is authoritative and + // rides in the `banks` key. + const std::string banksJson = book_.serialize(); SetProjExtState(static_cast(proj), kProjExtNamespace, - kProjExtIndexKey, json.c_str()); + kProjExtBanksKey, banksJson.c_str()); - // Additive: the Design-View model rides alongside the bank in its own key. - // Independent write — does not disturb the bank_index above. + // Retire the legacy single-bank `bank_index` key: SetProjExtState with an empty + // value DELETES the key (SDK header ~6288: val NULL or "" deletes the data). This + // realizes retirement concretely — after any save, a formerly-legacy project + // carries `banks` and NO `bank_index`, and going forward the legacy key is never + // written. Cheap and idempotent when the key is already absent. + SetProjExtState(static_cast(proj), kProjExtNamespace, + kProjExtIndexKey, ""); + + // Additive: the Design-View model rides alongside the banks in its own key. + // Independent write — does not disturb the `banks` blob above. const std::string viewJson = view_.serialize(); SetProjExtState(static_cast(proj), kProjExtNamespace, kProjExtViewKey, viewJson.c_str()); @@ -226,32 +240,46 @@ void ReaSamplerSession::loadFromProject(void* proj, const std::string& projectDi view_ = loadViewModel(static_cast(proj)); if (!proj) { - bank_ = BankIndex{}; + book_ = BankBook{}; return; } - const std::string json = - getProjExtStateString(static_cast(proj), kProjExtNamespace, - kProjExtIndexKey); - if (json.empty()) { - // No stored index (new or never-captured project) — start empty. - bank_ = BankIndex{}; - return; - } - std::optional loaded = BankIndex::deserialize(json); - if (!loaded) { - ShowConsoleMsg("ReaSampler: stored bank index is malformed — ignoring.\n"); - bank_ = BankIndex{}; - return; - } - bank_ = std::move(*loaded); - // Project-relative resolution is a READ-time concern: the index stores only - // relative paths (invariant), and consumers (M5 panel, M6 insert) resolve - // each entry against the CURRENT project dir via resolveBankFile(projectDir, - // relativePath). We do NOT rewrite the stored paths to absolute here — that - // would break the relative-only invariant and the travel-with-.rpp property. - // projectDir is threaded through for those consumers; nothing to do at load - // time beyond replacing the in-memory bank. + // Read both possible sources: the authoritative `banks` blob and the retired-but- + // possibly-still-present legacy `bank_index`. The precedence + migration decision + // (`banks` wins; else the legacy index migrates into the pool; else an empty book) + // is pure logic; it is inlined here rather than via BankBook::loadFromPersisted only + // so a malformed `banks` blob can be warned on the console (single parse) — a corrupt + // blob must read as "ignored", not silent loss, mirroring the prior malformed-index + // warning. A malformed `banks` degrades to an empty book and does NOT fall back to + // the stale legacy key (which would resurrect superseded single-bank state). + const std::string banksJson = + getProjExtStateString(static_cast(proj), kProjExtNamespace, + kProjExtBanksKey); + if (!banksJson.empty()) { + std::optional loaded = BankBook::deserialize(banksJson); + if (!loaded) { + ShowConsoleMsg("ReaSampler: stored banks are malformed — ignoring.\n"); + book_ = BankBook{}; + } else { + book_ = std::move(*loaded); + } + } else { + // No `banks` yet — fall back to the legacy `bank_index`, migrated into the pool + // by BankBook's parse-time promotion. loadFromPersisted covers the legacy-or- + // empty tail; passing "" for banksJson takes exactly that branch. + const std::string legacyJson = + getProjExtStateString(static_cast(proj), kProjExtNamespace, + kProjExtIndexKey); + book_ = BankBook::loadFromPersisted(std::string{}, legacyJson); + } + + // Project-relative resolution is a READ-time concern: every BankIndex in the book + // stores only relative paths (invariant, enforced per-bank at add()), and consumers + // (M5 panel, M6 insert) resolve each entry against the CURRENT project dir via + // resolveBankFile(projectDir, relativePath). We do NOT rewrite stored paths to + // absolute here — that would break the relative-only invariant and travel-with-.rpp. + // projectDir is threaded through for those consumers; nothing to do at load time + // beyond replacing the in-memory book. (void)projectDir; } diff --git a/src/persist.h b/src/persist.h index f00b271..8db7d1a 100644 --- a/src/persist.h +++ b/src/persist.h @@ -19,6 +19,7 @@ #include +#include "bank_book.h" #include "bank_model.h" #include "view_mode_model.h" @@ -28,10 +29,21 @@ namespace reasampler { // shipped: changing it orphans every already-saved project's index. inline constexpr const char* kProjExtNamespace = "reasampler"; -// The ext-state key the index JSON is stored under (one key holds the whole -// serialized BankIndex). FOREVER-STABLE for the same reason. +// The RETIRED legacy ext-state key: pre-multi-bank projects stored the whole +// serialized BankIndex here (single bank). Phase B2 no longer WRITES it — on save +// the key is cleared (SetProjExtState with "" deletes it) and the book is written +// under kProjExtBanksKey instead. It is still READ once, on load of a legacy +// project, to migrate its single index into the pool (BankBook's parse-time +// promotion). FOREVER-STABLE as a read key for that migration path. inline constexpr const char* kProjExtIndexKey = "bank_index"; +// The multi-bank ext-state key (Phase B): one key holds the whole serialized +// BankBook — the pool folded in as bank-zero plus every named bank, each with its +// own BankIndex, ordinals, and the active-bank id. AUTHORITATIVE going forward; +// supersedes kProjExtIndexKey. FOREVER-STABLE once shipped: changing it orphans +// every already-saved project's banks. +inline constexpr const char* kProjExtBanksKey = "banks"; + // The ext-state key the Design-View ViewModeModel JSON is stored under (one key // holds the whole serialized model: modes + membership + show-both + snapshots + // active mode). Distinct from kProjExtIndexKey — one namespace, two keys. @@ -45,8 +57,9 @@ inline constexpr const char* kProjExtViewKey = "view_state"; // it strands the identity of every already-saved project. See persist.cpp. inline constexpr const char* kProjExtGuidKey = "project_guid"; -// Owns the session's BankIndex and drives persistence against the active REAPER -// project. One instance lives for the extension's lifetime (main.cpp). It tracks +// Owns the session's BankBook (Phase B: pool + named banks) and drives persistence +// against the active REAPER project. One instance lives for the extension's +// lifetime (main.cpp). It tracks // the project identity it last saw so the timer tick can detect a project load // (a different project became active) and a Save-As (SAME project, path changed): // @@ -62,16 +75,28 @@ inline constexpr const char* kProjExtGuidKey = "project_guid"; // W12 defect that stopped the bank reloading); the pointer catches forks (Save-As // copies our GUID onto a distinct object — the W10 defect that clobbered a bank). // -// The bank itself is exposed for the capture/action layer to mutate; persist +// The book itself is exposed for the capture/action layer to mutate; persist // only reads it on save and replaces it on load. class ReaSamplerSession { public: ReaSamplerSession() = default; - // The in-memory bank. The action/capture layer adds captures here; persist - // serializes it on save and replaces it on project load. - BankIndex& bank() { return bank_; } - const BankIndex& bank() const { return bank_; } + // The multi-bank book (Phase B): the pool + named banks, each wrapping a + // BankIndex, plus the active-bank id. The action layer (B3) creates / renames / + // reorders / deletes banks and moves samples here; the panel (B4) reads it; + // persist serializes it under the `banks` key on save and replaces it on load. + BankBook& book() { return book_; } + const BankBook& book() const { return book_; } + + // The capture add-target: the ACTIVE bank's BankIndex (defaults to the pool). + // The capture path adds a captured Sample through this seam, so a capture lands + // in whichever bank is active — the single behavioural change B2 wires in over + // M7/M8 (the capture backends are untouched; only the target index moved). The + // panel/insert readers that displayed the single index continue to read it here + // unchanged; today it resolves to the pool (default active), matching prior + // single-bank behaviour, until B3/B4 let the user switch the active bank. + BankIndex& bank() { return book_.activeIndex(); } + const BankIndex& bank() const { return book_.activeIndex(); } // The in-memory Design-View model. The view/action layer mutates it (tag, // toggle, snapshot); persist serializes it on save and replaces it on project @@ -80,8 +105,9 @@ public: ViewModeModel& view() { return view_; } const ViewModeModel& view() const { return view_; } - // Serialize the current bank to the active project's ext state (namespace - // "reasampler"). Non-destructive beyond writing our own ext-state key. Safe + // Serialize the current book (under the `banks` key) and view model to the active + // project's ext state (namespace "reasampler"), and clear the retired legacy + // `bank_index` key. Non-destructive beyond writing our own ext-state keys. Safe // to call when there is no active/saved project (it no-ops). void saveToActiveProject(); @@ -103,7 +129,7 @@ public: bool consumeLoadSignal(); private: - BankIndex bank_; + BankBook book_; // The Design-View model. Default-constructed = Arrange + Design seeded, active // = Arrange; loadFromProject leaves this default when a project has no stored @@ -124,9 +150,10 @@ private: bool primed_ = false; // false until the first poll() observes state bool loadPending_ = false; // raised by loadFromProject; drained by consumeLoadSignal - // Load the index from the given project's ext state and resolve bank paths - // against projectDir. Replaces the in-memory bank. projectDir empty -> clears - // the bank (unsaved project has no resolvable bank). + // Load the book from the given project's ext state (the `banks` key, else the + // legacy `bank_index` key migrated into the pool) and resolve bank paths against + // projectDir at read time. Replaces the in-memory book. projectDir empty -> the + // book is reset to empty (unsaved project has no resolvable banks). void loadFromProject(void* proj, const std::string& projectDir); }; diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index d0e7f3e..f254da5 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -341,6 +341,70 @@ static void testMalformedJson() { CHECK(!BankBook::deserialize(dup).has_value()); } +// --- B2: persist-load source-precedence decision (pure) -------------------- +// loadFromPersisted picks the load source the persist shell will feed it from the +// two ext-state values a project may carry: the authoritative `banks` blob and the +// retired legacy `bank_index` blob. Precedence: banks > legacy > empty; a malformed +// banks blob degrades to empty WITHOUT falling back to the stale legacy key. + +static void testLoadPrefersBanksBlob() { + // A full book + a stale legacy index both present: `banks` wins, legacy ignored. + BankBook src; + CHECK(src.createBank("drums", "Drums")); + CHECK(src.setActiveBank("drums")); + CHECK(src.bank("drums")->index.add(sampleWith("new1")) == AddResult::Added); + const std::string banksJson = src.serialize(); + + BankIndex stale; + CHECK(stale.add(sampleWith("stale-old")) == AddResult::Added); + const std::string legacyJson = stale.serialize(); + + BankBook loaded = BankBook::loadFromPersisted(banksJson, legacyJson); + // The book equals the source book — the legacy key had NO effect. + CHECK(loaded == src); + CHECK(loaded.activeBankId() == "drums"); + CHECK(loaded.bank("drums") != nullptr); + CHECK(loaded.bank("drums")->index.query("id-new1") != nullptr); + // The stale legacy sample must NOT have leaked into the pool. + CHECK(loaded.pool().index.query("id-stale-old") == nullptr); +} + +static void testLoadMigratesLegacyWhenNoBanks() { + // No `banks` key, a legacy `bank_index` present: migrate into the pool, zero named. + BankIndex legacy; + CHECK(legacy.add(sampleWith("l1")) == AddResult::Added); + CHECK(legacy.add(sampleWith("l2")) == AddResult::Added); + const std::string legacyJson = legacy.serialize(); + + BankBook loaded = BankBook::loadFromPersisted(std::string{}, legacyJson); + CHECK(loaded.size() == 1); // pool only + CHECK(loaded.pool().id == kPoolBankId); + CHECK(loaded.activeBankId() == std::string(kPoolBankId)); + CHECK(loaded.pool().index.size() == 2); + CHECK(loaded.pool().index == legacy); // lossless +} + +static void testLoadEmptyWhenNeither() { + // Both absent: a fresh empty book (pool only, empty index, active = pool). + BankBook loaded = BankBook::loadFromPersisted(std::string{}, std::string{}); + CHECK(loaded == BankBook{}); + CHECK(loaded.size() == 1); + CHECK(loaded.pool().index.empty()); + CHECK(loaded.activeBankId() == std::string(kPoolBankId)); +} + +static void testLoadMalformedBanksDegradesWithoutLegacyFallback() { + // A present-but-malformed `banks` blob must degrade to an empty book and must NOT + // resurrect the stale legacy key (that would revive superseded single-bank state). + BankIndex stale; + CHECK(stale.add(sampleWith("stale")) == AddResult::Added); + const std::string legacyJson = stale.serialize(); + + BankBook loaded = BankBook::loadFromPersisted("{\"banks\":[", legacyJson); + CHECK(loaded == BankBook{}); // empty, NOT the legacy + CHECK(loaded.pool().index.query("id-stale") == nullptr); // legacy did not leak +} + static void testActiveBankResolveAfterCorruptPersistedId() { // A book blob whose activeBank names no bank resolves to the pool (defensive). const char* json = @@ -366,6 +430,10 @@ int main() { testJsonEmptyBookRoundTrip(); testLegacyMigration(); testMalformedJson(); + testLoadPrefersBanksBlob(); + testLoadMigratesLegacyWhenNoBanks(); + testLoadEmptyWhenNeither(); + testLoadMalformedBanksDegradesWithoutLegacyFallback(); testActiveBankResolveAfterCorruptPersistedId(); if (g_fail == 0) std::printf("All tests passed.\n");