diff --git a/docs/PLAN.md b/docs/PLAN.md index 0833a9a..b2ccbbe 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -2704,10 +2704,12 @@ on the import side, `bank_ops`, or `persist`. - Exporting an empty bank produces a valid, importable package with zero entries rather than refusing. An empty bank is a legitimate thing to carry. -**Open questions.** **[propose at review]** whether the export affordance is action-only, -panel-only, or both at ship. **[propose at review]** whether the default file name is derived -from the bank's display name (recommended, sanitized through -`capture_paths::sanitizeStem`) or from the project name. +**Open questions — both answered at implementation review, recorded at +`docs/product/bank-package.md` §"Implementation decisions — Ε-W2-T1".** Affordance: +**both** the action and the panel row (the action is the only spelling that can reach +the pool; the panel row is the direct gesture on a named bank). Default file name: +the bank's **display name**, sanitized through `capture_paths::sanitizeStem`, as +recommended. #### Ε-W2-T2 — `bank-import` diff --git a/docs/product/bank-package.md b/docs/product/bank-package.md index 92c9ee4..dd51dea 100644 --- a/docs/product/bank-package.md +++ b/docs/product/bank-package.md @@ -725,6 +725,30 @@ contemplates one untracked capture, an import strands hundreds). --- +## Implementation decisions — Ε-W2-T1 + +Not [Daniel]-class forks — both were `[propose at review]` calls in `docs/PLAN.md`'s +Ε-W2-T1 track, answered at implementation review rather than by Daniel, and recorded +here per this phase's own convention for keeping such answers where the design lives +rather than only in the track's own now-stale open-questions line. + +- **Affordance: both the bindable action and the panel row.** The action targets the + **active** bank and is the only spelling that can reach the **pool** (the panel's + `showTabMenu` returns early on `isPool()` — a named-bank-tab context menu has no tab + to right-click for the pool), while the exported unit's own definition above includes + the pool. The panel row is the direct gesture on a specific named bank. Neither + subsumes the other. +- **Default file name: the bank's display name**, sanitized through + `capture_paths::sanitizeStem`, seeded into `/.rsbank`. A + project-derived name was the rejected alternative: three banks exported from one + project must produce three distinguishable files, and a project-derived name + collides on the second export. Known wart, worth recording rather than hiding: + `sanitizeStem` collapses an all-non-ASCII display name to the literal `capture`, so + two such banks still collide — the existing rename verb is the recovery, same as the + import-side auto-suffix collisions above. + +--- + ## Non-goals and guardrails - **No auto-insertion of imported audio into the arrange.** Same rule as capture. diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 044ab51..fa863d4 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -46,13 +46,14 @@ add_library(reaper_reasampler MODULE ${REASAMPLER_SRC_DIR}/shell/actions/design_view_actions.cpp ${REASAMPLER_SRC_DIR}/shell/actions/bank_actions.cpp ${REASAMPLER_SRC_DIR}/shell/actions/prune_action.cpp + ${REASAMPLER_SRC_DIR}/shell/actions/package_export_action.cpp ${REASAMPLER_SRC_DIR}/shell/actions/ingest.cpp ${REASAMPLER_SRC_DIR}/shell/actions/arrange_drop_win.cpp ${REASAMPLER_SRC_DIR}/shell/actions/drag_out_win.cpp ${REASAMPLER_SRC_DIR}/shell/actions/instrument_drop_win.cpp ${REASAMPLER_SRC_DIR}/shell/persist/usage_scan.cpp ) -target_link_libraries(reaper_reasampler PRIVATE json wire file_bytes bank_model capture_paths capture_name peaks bank_grid mode_switch tab_strip view_mode_model view_tree guid_diff lane_keys solo_cache insert_plan render_settings render_window track_topology batch_capture tail_control capture_realtime bank_book wav_codec origin_ledger tracking_authority prune_reconcile prune_button app_version provenance drag_out instrument_drop theme component_geometry action_bar footer_bar overflow_menu mode_enable tooltip card_meta card_drag assignment_request bank_sync sample_usage bake_wire resample_name) +target_link_libraries(reaper_reasampler PRIVATE json wire file_bytes bank_model capture_paths capture_name peaks bank_grid mode_switch tab_strip view_mode_model view_tree guid_diff lane_keys solo_cache insert_plan render_settings render_window track_topology batch_capture tail_control capture_realtime bank_book wav_codec origin_ledger tracking_authority prune_reconcile prune_button app_version provenance drag_out instrument_drop theme component_geometry action_bar footer_bar overflow_menu mode_enable tooltip card_meta card_drag assignment_request bank_sync sample_usage bake_wire resample_name export_bank package_pickers) # NOT linked here, deliberately: sampler_core / pitch_shift / the filter. The instrument # renders its own bake in its own process, which is what keeps the extension's link graph # free of the voice engine — a link edge to it here means the design drifted. diff --git a/src/app/main.cpp b/src/app/main.cpp index 57696ae..b898cd6 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -26,6 +26,7 @@ #include "shell/actions/action_registry.h" // the registration table #include "shell/actions/bank_actions.h" // multi-bank action family #include "shell/actions/design_view_actions.h" // Design View action family +#include "shell/actions/package_export_action.h" // bank-package export action body #include "core/wire/bake_wire.h" // kBakeActionSuffix (the shared action id) #include "shell/capture/bake_land.h" // resample-bake landing action body #include "shell/capture/capture_batch.h" // batch + recapture action bodies @@ -91,6 +92,7 @@ static void RunCancelRealtime(int) { capture::RunCancelRealtime(g_session); } static void RunRecaptureFromSource(int) { capture::RunRecaptureFromSource(g_session); } static void RunRenderTrackInPlace(int) { capture::RunRenderTrackInPlace(g_session); } static void RunResampleBake(int) { capture::RunResampleBake(g_session); } +static void RunExportBankPackage(int) { reasampler::doBankPackageExport(g_session, g_session.book().activeBankId()); } static void RunShowVersion(int) { // On-demand only — no unconditional startup print (routine console chatter pops // the console window). @@ -147,6 +149,8 @@ static std::vector buildMainActionTable() { rows.push_back({reasampler::wire::kBakeActionSuffix, "land pending ReaSampler 9000 resample bake", &RunResampleBake}); + rows.push_back({"EXPORT_BANK_PACKAGE", "export active bank as package", + &RunExportBankPackage}); rows.push_back({"SHOW_VERSION", "show version", &RunShowVersion}); return rows; diff --git a/src/core/package/CLAUDE.md b/src/core/package/CLAUDE.md index 9114f83..fb9391a 100644 --- a/src/core/package/CLAUDE.md +++ b/src/core/package/CLAUDE.md @@ -6,8 +6,8 @@ The hand-rolled `RSBK` bank-package container, entirely pure (REAPER-free, unit-tested outside the DAW): the format contract and version ladder, the JSON manifest, and the framing/layout codec. No filesystem — the shell (`src/shell/package`) streams bytes against the layouts produced here. The -export/import *decisions* (`export_plan` / `import_plan`) are separate modules -landing after the format. +export/import *decisions* (`export_plan` / `import_plan`) are separate modules; +`export_plan` has landed, `import_plan` has not. ## Invariants @@ -73,6 +73,12 @@ landing after the format. here, computed where payloads are streamed (shell). The bank's `slot_map` rides along. Unknown keys skip at every level; duplicate entry names are rejected both ways. +- `export_plan` — the pure export decision over value inputs (the bank's members + plus the shell's per-file probe result): the verdict (`Ready` / `Incomplete` / + `Refused`), the transport name per shipping entry, and what is excluded and why + (missing / unreadable / an index record the format cannot represent). Owns the + name repair the codec's refusal backstops, and normalizes each shipping record's + `relativePath` to the bare package name — see the transport-name gotcha below. - `bank_package` — framing and arithmetic composing the two above: `encodePackage` (prefix bytes + layout + total size, stamping this build's ladder pair and `version::stampVersion()`), `decodePackage` (prefix + observed @@ -115,6 +121,22 @@ landing after the format. `minReaderVersion` bump, not additive: an old reader would otherwise compare a stored digest against bytes hashed the new way and silently misjudge corruption. +- **A written package carries no path in ANY field.** `isValidNestedSamplePath` + permits a relative `relativePath` because a *record* may hold one, but + `export_plan` writes each shipping entry's `relativePath` as its bare, sanitized + and disambiguated transport name (`export_plan.cpp`'s `e.fileName`), so an + emitted manifest has no separator anywhere and the entry name is the single + naming authority on both sides. The directory component it drops carries no + information — the bank subfolder is a fixed `capture_paths` constant + (`capture_paths.cpp`'s `deriveBankPaths`) the importer re-spells. **The + basename spelling is dropped too, not just the directory**: `e.fileName` is + `uniqueEntryName(sanitizeEntryName(...))`, not the source basename, so a + macOS-authored `Hit?.wav` survives only in `displayName` — the transport name + itself may differ. Accepted for the same reason the directory drop is: the + transport name exists to be a valid, collision-free package entry, not a + faithful copy of the source spelling, and `displayName` is the field that + carries the original for display. The nested-path rule stays as the decode-side + backstop for a package this build did not write. - **Obligation on the export track: sanitize, don't relay the refusal.** `serializeManifest` returns one indistinguishable `nullopt` for every rejection — an unrepresentable name, a case-folded collision, a traversing nested path, a diff --git a/src/core/package/CMakeLists.txt b/src/core/package/CMakeLists.txt index fc7fd2d..32ae08d 100644 --- a/src/core/package/CMakeLists.txt +++ b/src/core/package/CMakeLists.txt @@ -6,6 +6,11 @@ reasampler_pure_library(package_manifest LINK PUBLIC bank_model slot_map PRIVATE package_format json) reasampler_test(package_manifest LINK package_manifest) +reasampler_pure_library(export_plan + SOURCES export_plan.cpp + LINK PUBLIC package_manifest PRIVATE package_format) +reasampler_test(export_plan LINK export_plan package_format) + # bytes.h is header-only (see src/core/wire/CLAUDE.md) — no wire link edge needed. reasampler_pure_library(bank_package SOURCES bank_package.cpp diff --git a/src/core/package/export_plan.cpp b/src/core/package/export_plan.cpp new file mode 100644 index 0000000..4803cf0 --- /dev/null +++ b/src/core/package/export_plan.cpp @@ -0,0 +1,177 @@ +// export_plan.cpp — see export_plan.h for the contract. + +#include "core/package/export_plan.h" + +#include + +#include "core/package/package_format.h" + +namespace reasampler::package { + +namespace { + +// The bare file name a bank-relative path ends in. Separators are matched in both +// spellings: a persisted index may hold either on Windows. +std::string baseNameOf(const std::string& path) { + const std::size_t sep = path.find_last_of("/\\"); + return sep == std::string::npos ? path : path.substr(sep + 1); +} + +// Mirrors package_format.cpp's trailing-dot/space rule so a truncated stem never +// reintroduces the collision isValidEntryName exists to prevent. +std::string stripTrailingDotsAndSpaces(std::string s) { + while (!s.empty() && (s.back() == '.' || s.back() == ' ')) s.pop_back(); + return s; +} + +// Truncates to at most `max` bytes without splitting a UTF-8 sequence (the rule +// itself is package_format.cpp's isWellFormedUtf8). A sequence landing exactly on +// the cut is dropped whole, one character short of `max`, rather than checked for +// cleanliness — over-truncating by one character is cheap insurance against a +// subtly wrong boundary check. +std::string truncateUtf8(std::string s, std::size_t max) { + if (s.size() <= max) return s; + s.resize(max); + while (!s.empty() && (static_cast(s.back()) & 0xC0) == 0x80) s.pop_back(); + if (!s.empty() && static_cast(s.back()) >= 0xC0) s.pop_back(); + return s; +} + +// `name` with `suffix` inserted before its extension, trimmed so the result still +// fits the entry-name cap. `suffix.size() + ext.size()` can exceed the cap on its +// own (a long extension, a two-digit disambiguation suffix) — clamped rather than +// subtracted unchecked, which would underflow the size_t `room` below and turn +// truncateUtf8 into a silent no-op. An extension that alone leaves no room even +// after the whole stem is dropped is dropped too; uniqueEntryName's own floor +// covers what even that cannot fix. +std::string insertSuffix(const std::string& name, const std::string& suffix) { + const std::size_t dot = name.find_last_of('.'); + const bool hasExt = dot != std::string::npos && dot > 0; + std::string stem = hasExt ? name.substr(0, dot) : name; + std::string ext = hasExt ? name.substr(dot) : std::string(); + if (suffix.size() >= kMaxEntryNameBytes) return std::string(); + if (ext.size() > kMaxEntryNameBytes - suffix.size()) ext.clear(); + const std::size_t room = kMaxEntryNameBytes - suffix.size() - ext.size(); + stem = truncateUtf8(std::move(stem), room); + return stem + suffix + ext; +} + +bool nameTaken(const std::string& candidate, const std::vector& taken) { + for (const std::string& t : taken) + if (sameEntryName(candidate, t)) return true; + return false; +} + +// A transport name distinct from every name already claimed, under the format's own +// case-folding equivalence (package_format.h's sameEntryName). +std::string uniqueEntryName(const std::string& base, const std::vector& taken) { + if (!nameTaken(base, taken)) return base; + // Each iteration either returns a name both valid and distinct from `taken`, or + // advances to the next suffix; taken.size() + 2 attempts is enough by pigeonhole + // now that insertSuffix cannot underflow. The floor below is the residual case + // validity alone can still fail — an extension so long insertSuffix must drop it + // on every attempt tried here. + for (std::size_t n = 2; n <= taken.size() + 2; ++n) { + const std::string candidate = insertSuffix(base, "_" + std::to_string(n)); + if (!nameTaken(candidate, taken) && isValidEntryName(candidate)) return candidate; + } + // Floors like sanitizeEntryName's own "entry" floor: always valid, regardless of + // how base's own extension behaved. + return sanitizeEntryName("entry_" + std::to_string(taken.size() + 2)); +} + +// What BankModel::add and the manifest's nested-path rule together accept — the pair +// package_manifest::serializeManifest checks per entry. The codec's refusal is the +// backstop; classifying here is what lets the export name the offending entry. +bool recordRepresentable(const model::Sample& s) { + return !s.id.empty() && isValidNestedSamplePath(s.relativePath); +} + +ExcludedEntry excludedFrom(const model::Sample& s, ExclusionReason reason) { + ExcludedEntry e; + e.sampleId = s.id; + e.displayName = s.displayName; + e.relativePath = s.relativePath; + e.reason = reason; + return e; +} + +} // namespace + +std::string sanitizeEntryName(const std::string& rawFileName) { + std::string n = rawFileName; + for (char& c : n) { + const unsigned char u = static_cast(c); + if (u < 0x20 || u == 0x7F || u == '/' || u == '\\' || u == ':' || u == '*' || + u == '?' || u == '|' || u == '<' || u == '>' || u == '"') + c = '_'; + } + // One byte of headroom so the prefix repair below still fits the cap. + n = stripTrailingDotsAndSpaces(truncateUtf8(std::move(n), kMaxEntryNameBytes - 1)); + if (isValidEntryName(n)) return n; + + // One prefix answers every remaining reserved form at once: "." / "..", a DOS + // device name, and a name the strips emptied. + std::string prefixed = stripTrailingDotsAndSpaces("_" + n); + if (isValidEntryName(prefixed)) return prefixed; + + // Ill-formed UTF-8 is what is left, and isValidEntryName is the only authority on + // it here, so fold the whole non-ASCII range rather than re-deriving the scanner. + for (char& c : prefixed) + if (static_cast(c) >= 0x80) c = '_'; + prefixed = stripTrailingDotsAndSpaces(prefixed); + return isValidEntryName(prefixed) ? prefixed : std::string("entry"); +} + +ExportPlan planExport(const ExportInputs& in) { + ExportPlan plan; + plan.manifest.bankDisplayName = in.bankDisplayName; + + bool anyAbsent = false; + bool anyUnrepresentable = false; + std::vector takenNames; + std::vector shippedIds; + + for (const ExportCandidate& c : in.candidates) { + if (!recordRepresentable(c.sample)) { + plan.excluded.push_back( + excludedFrom(c.sample, ExclusionReason::RecordUnrepresentable)); + anyUnrepresentable = true; + continue; + } + if (c.fileState != SourceFileState::Present) { + plan.excluded.push_back(excludedFrom( + c.sample, c.fileState == SourceFileState::Unreadable + ? ExclusionReason::FileUnreadable + : ExclusionReason::FileMissing)); + anyAbsent = true; + continue; + } + + PackageEntry e; + e.sample = c.sample; + e.fileName = uniqueEntryName(sanitizeEntryName(baseNameOf(c.sample.relativePath)), + takenNames); + // The transport record names its payload by the package name and nothing + // else, so the manifest carries no path at all — the bank subfolder is a + // fixed constant the importer re-spells through capture_paths. + e.sample.relativePath = e.fileName; + + takenNames.push_back(e.fileName); + shippedIds.push_back(c.sample.id); + plan.sourceRelativePaths.push_back(c.sample.relativePath); + plan.manifest.entries.push_back(std::move(e)); + } + + // Display positions follow membership: an excluded entry's slot marker would name + // a sample the package does not carry. + plan.manifest.slots = in.slots; + plan.manifest.slots.reconcile(shippedIds); + + plan.verdict = anyUnrepresentable ? ExportVerdict::Refused + : anyAbsent ? ExportVerdict::Incomplete + : ExportVerdict::Ready; + return plan; +} + +} // namespace reasampler::package diff --git a/src/core/package/export_plan.h b/src/core/package/export_plan.h new file mode 100644 index 0000000..0d7505a --- /dev/null +++ b/src/core/package/export_plan.h @@ -0,0 +1,87 @@ +#pragma once +// export_plan — the pure export decision: which bank entries ship, what each one is +// named inside the package, what is absent, and therefore whether the export may +// proceed at all. Values in, verdict out — the shell probes the filesystem and hands +// the results here. Pure: no filesystem, no host types. + +#include +#include + +#include "core/model/bank_model.h" +#include "core/model/slot_map.h" +#include "core/package/package_manifest.h" + +namespace reasampler::package { + +// What the shell's filesystem probe found for one indexed entry. Missing and +// Unreadable stay distinct all the way to the refusal message: the file is gone vs. +// the file is there and will not open, which have opposite recoveries. +enum class SourceFileState { + Present, + Missing, + Unreadable, +}; + +struct ExportCandidate { + model::Sample sample; + SourceFileState fileState = SourceFileState::Missing; +}; + +// One bank as the planner sees it: the display name that rides in the manifest +// envelope, the members in bank insertion order, and the bank's display positions. +struct ExportInputs { + std::string bankDisplayName; + std::vector candidates; + model::SlotMap slots; +}; + +// Why an indexed entry cannot ship. +enum class ExclusionReason { + FileMissing, + FileUnreadable, + // The index record itself cannot be written: an empty id, or a relativePath the + // format's nested-path rule refuses. Not something a confirm can proceed past. + RecordUnrepresentable, +}; + +struct ExcludedEntry { + std::string sampleId; + std::string displayName; + std::string relativePath; + ExclusionReason reason = ExclusionReason::FileMissing; +}; + +enum class ExportVerdict { + Ready, // every candidate ships + Incomplete, // a file is absent or unreadable; the rest may ship behind an explicit confirm + Refused, // an index record the format cannot represent — no confirm path +}; + +struct ExportPlan { + ExportVerdict verdict = ExportVerdict::Ready; + + // Entries in bank order, each carrying its transport name and its record. The + // shell measures `byteLength`/`byteHash` from the payload, so they are 0/"" here; + // `exportTimestamp` is the shell's clock read and is 0 here too. + PackageManifest manifest; + + // Where each shipping entry's bytes are read from, parallel to + // `manifest.entries` — the record's own relativePath is normalized to the bare + // package name (see the transport-name note in this directory's CLAUDE.md), so + // the source spelling has to survive separately. + std::vector sourceRelativePaths; + + std::vector excluded; +}; + +ExportPlan planExport(const ExportInputs& in); + +// The smallest repair of one bare file name that satisfies isValidEntryName — +// separators, reserved characters and control bytes to '_', an over-long name +// truncated on a UTF-8 boundary, and an underscore prefix for the reserved forms +// ("." / ".." / a DOS device name). Never returns a name isValidEntryName refuses. +// Why sanitize rather than relay the codec's refusal: this directory's own +// CLAUDE.md, "Obligation on the export track." +std::string sanitizeEntryName(const std::string& rawFileName); + +} // namespace reasampler::package diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index befbaed..6b4b412 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -39,6 +39,7 @@ is owned by other directories and only skinned here. ## Modules - `shell/actions` (`action_registry` / `design_view_actions` / `bank_actions` / `prune_action`) — the bindable action families, all routed via the `command_id`/`gaccel`/`hookcommand` contract. `action_registry` owns the shared registration plumbing (interned channel-qualified id strings; register and mirror-unregister present the identical pointer) **and the Q-W6 registration TABLE**: `main.cpp`'s own family (capture scopes, panel toggle, insert, batch, realtime, recapture, version) is one `ActionTableRow` array — suffix, phrase, flat function-pointer handler — that registration, hookcommand dispatch, and the unload mirror-unregister all iterate, so adding an action touches the table only (OCP). Bank mutations flow through the promptless `shell/bank_ops` verbs (`bankOp*` + `persistBankOp`, taking `ReaSamplerSession&`), which the panel menus and `bank_actions` consume as thin UX skins. **Every bank index verb wraps its mutation in a batched REAPER undo point (`Undo_BeginBlock2`/`EndBlock2`, `UNDO_STATE_MISCCFG`) so one bank operation is one Ctrl-Z.** The prune action (`prune_action`, `BANK_PRUNE_FOLDER`) is **the ONLY file-deletion action in the system**; it opens no undo point (file deletion is not REAPER-undoable). `BANK_PRUNE_FOLDER` halts on `blockedByTracking` and prints each blocker that fired, with recovery instructions. +- `package_export_action` — the "export bank as package" skin: survey and report first, confirm what is absent (and, separately, a destination being replaced), pick a destination, write. Every prompt in the flow lives here so `shell/package/export_bank` stays promptless. Read-only against the project — it holds the session by `const&`, so no ext-state write, generation bump or undo point is reachable. Registration rides `main.cpp`'s action table (`EXPORT_BANK_PACKAGE`); the panel's tab menu is the second skin over the same body. - `drag_out_win` — OS drag-out shell: Windows OLE `DoDragDrop`/`CF_HDROP`, copy-only (`DROPEFFECT_MOVE` not offered); macOS/Linux via `SWELL_InitiateDragDropOfFileList`. - `instrument_drop_win` — instrument-drop shell: `probeDropTarget` resolves a screen point to a track + a `ReaperSurface` (via the pure `wire::classifyReaperSurface`, whose token rules `core/wire/CLAUDE.md` owns), and the drop half adds a ReaSampler 9000 instance and applies the dragged capture's state via a transient `.vstpreset` + `TrackFX_SetPreset` (the former `TrackFX_SetNamedConfigParm` "vst_chunk" write was silently unappliable for VST3). Exposes `loadInstrumentOntoTrack` (inner half, no own undo block) and `performInstrumentDrop` (wraps in its own undo block). **Never captures, never writes the bank, never inserts a timeline item.** - `arrange_drop_win` — the drag-out gesture's arrange outcome: `arrangeTimeAtScreenX` (pointer column → time via `GetSet_ArrangeView2`'s one-pixel-span reading — inferred, not SDK-documented) and `performArrangeDrop` (snap the drop time, then one `InsertMedia` per capture on the pointer's track — assumed, not confirmed, to land end-to-end via REAPER's own cursor advance — in ONE undo block, counting only InsertMedia's reported successes, with the caller's track selection and edit cursor restored). The one timeline-placing shell here, per the invariant above; it never captures and never writes the bank. diff --git a/src/shell/actions/package_export_action.cpp b/src/shell/actions/package_export_action.cpp new file mode 100644 index 0000000..fcaebe3 --- /dev/null +++ b/src/shell/actions/package_export_action.cpp @@ -0,0 +1,196 @@ +// package_export_action.cpp — see package_export_action.h for the contract this TU +// preserves. main.cpp owns the API pointers; this TU gets them extern. + +#include "shell/actions/package_export_action.h" + +#include +#include +#include +#include +#include + +#include "core/capture/capture_paths.h" // projectDirOfRpp, sanitizeStem +#include "shell/package/export_bank.h" +#include "shell/package/package_pickers.h" +#include "shell/persist/session.h" + +#define REAPERAPI_MINIMAL +#define REAPERAPI_WANT_EnumProjects +#define REAPERAPI_WANT_ShowConsoleMsg +#define REAPERAPI_WANT_ShowMessageBox +#include "reaper_plugin_functions.h" + +namespace reasampler { + +namespace { + +constexpr const char* kUnsavedProjectMsg = + "ReaSampler export: save the project first -- an unsaved project has no bank folder " + "to read from.\n"; + +std::string currentProjectDir() { + std::vector buf(4096, '\0'); + EnumProjects(-1, buf.data(), static_cast(buf.size())); + return capture::projectDirOfRpp(std::string(buf.data())); +} + +// 6 == YES; anything else cancels (SDK ~6544). +bool confirmed(const std::string& msg, const char* title) { + return ShowMessageBox(msg.c_str(), title, 4) == 6; +} + +std::string entryLine(const package::ExcludedEntry& e) { + const char* why = e.reason == package::ExclusionReason::FileMissing ? "missing" + : e.reason == package::ExclusionReason::FileUnreadable ? "unreadable" + : "unusable index record"; + return " " + (e.displayName.empty() ? e.sampleId : e.displayName) + " [" + why + + "] " + e.relativePath + "\n"; +} + +// `maxLines` == 0 lists everything (the console record); a positive cap keeps a +// confirm dialog readable on a bank with hundreds of absent files, prune's own +// truncate-the-confirm-not-the-report discipline. +std::string excludedManifest(const std::vector& excluded, + std::size_t maxLines) { + std::string msg; + std::size_t shown = 0; + for (const package::ExcludedEntry& e : excluded) { + if (maxLines != 0 && shown == maxLines) { + msg += " ... (" + std::to_string(excluded.size() - shown) + + " more, listed in the console)\n"; + break; + } + msg += entryLine(e); + ++shown; + } + return msg; +} + +void reportOutcome(const ExportOutcome& out, const std::string& destPath) { + switch (out.status) { + case ExportStatus::Written: + ShowConsoleMsg(("ReaSampler export: wrote " + std::to_string(out.entriesWritten) + + " entry/entries (" + std::to_string(out.bytesWritten) + + " bytes) to " + destPath + "\n") + .c_str()); + return; + case ExportStatus::SourceReadFailed: + ShowConsoleMsg(("ReaSampler export: ABORTED -- \"" + out.offendingName + + "\" could not be read. Nothing was written.\n") + .c_str()); + return; + case ExportStatus::SourceChanged: + ShowConsoleMsg(("ReaSampler export: ABORTED -- \"" + out.offendingName + + "\" changed on disk while the package was being written. " + "Nothing was written; run the export again.\n") + .c_str()); + return; + case ExportStatus::EncodeFailed: + ShowConsoleMsg("ReaSampler export: ABORTED -- this bank could not be encoded " + "as a package. Nothing was written.\n"); + return; + // Both refusals are re-derived from a FRESH plan, so reaching them after the + // survey means the bank changed under the export, not that the user declined. + case ExportStatus::RefusedIncomplete: + case ExportStatus::RefusedUnrepresentable: + ShowConsoleMsg("ReaSampler export: ABORTED -- the bank changed between the " + "report and the write. Nothing was written; run the export " + "again.\n"); + return; + case ExportStatus::RefusedDestinationExists: + ShowConsoleMsg("ReaSampler export: cancelled -- nothing was written.\n"); + return; + case ExportStatus::NoSuchBank: + ShowConsoleMsg("ReaSampler export: that bank no longer exists.\n"); + return; + case ExportStatus::NoProjectDir: + ShowConsoleMsg(kUnsavedProjectMsg); + return; + case ExportStatus::WriteFailed: + ShowConsoleMsg(("ReaSampler export: FAILED writing " + destPath + + ". No package was left behind; any file already at that path is " + "untouched.\n") + .c_str()); + return; + } +} + +} // namespace + +void doBankPackageExport(const ReaSamplerSession& session, const std::string& bankId) { + const std::string projectDir = currentProjectDir(); + if (projectDir.empty()) { + ShowConsoleMsg(kUnsavedProjectMsg); + return; + } + + // Report before acting, and before the picker opens: a refusal the user cannot + // act on should not cost them a trip through a save dialog first. + const ExportSurvey survey = surveyBankExport(session, projectDir, bankId); + if (!survey.bankFound) { + ShowConsoleMsg("ReaSampler export: no such bank.\n"); + return; + } + const std::string bankName = survey.plan.manifest.bankDisplayName; + + bool allowIncomplete = false; + if (survey.plan.verdict == package::ExportVerdict::Refused) { + ShowConsoleMsg(("ReaSampler export: ABORTED -- \"" + bankName + + "\" holds index record(s) a package cannot carry. Nothing was " + "written.\n" + + excludedManifest(survey.plan.excluded, 0)) + .c_str()); + return; + } + if (survey.plan.verdict == package::ExportVerdict::Incomplete) { + const std::string headline = + "ReaSampler export: \"" + bankName + "\" has " + + std::to_string(survey.plan.excluded.size()) + + " entry/entries whose file is missing or unreadable:\n"; + ShowConsoleMsg((headline + excludedManifest(survey.plan.excluded, 0)).c_str()); + if (!confirmed(headline + excludedManifest(survey.plan.excluded, 10) + + "\nExport the " + + std::to_string(survey.plan.manifest.entries.size()) + + " present entry/entries anyway?", + "ReaSampler: incomplete bank")) { + ShowConsoleMsg("ReaSampler export: cancelled -- nothing was written.\n"); + return; + } + allowIncomplete = true; + } + + // The bank's own name, not the project's: the artifact is a bank, and a user + // exporting three banks from one project needs three distinguishable files. + const std::string suggested = + projectDir + "/" + capture::sanitizeStem(bankName) + ".rsbank"; + std::string dest; + bool appended = false; + if (!pickPackageSavePath(suggested, dest, &appended)) return; // user cancelled the picker + + ExportRequest req; + req.projectDir = projectDir; + req.bankId = bankId; + req.destAbsPath = dest; + req.exportTimestamp = static_cast(std::time(nullptr)); + req.allowIncomplete = allowIncomplete; + // The dialog's own overwrite confirm covered exactly this path when the picker + // did not need to append `.rsbank` to reach it — asking again would be a second + // prompt for the same consent. An appended path is one the dialog never saw, so + // that case still falls through to exportBank's own refusal and the confirm below. + req.allowOverwrite = !appended; + + ExportOutcome out = exportBank(session, req); + if (out.status == ExportStatus::RefusedDestinationExists) { + if (!confirmed("A file already exists at:\n\n " + dest + + "\n\nReplace it with this bank package?", + "ReaSampler: replace package")) { + ShowConsoleMsg("ReaSampler export: cancelled -- nothing was written.\n"); + return; + } + req.allowOverwrite = true; + out = exportBank(session, req); + } + reportOutcome(out, dest); +} + +} // namespace reasampler diff --git a/src/shell/actions/package_export_action.h b/src/shell/actions/package_export_action.h new file mode 100644 index 0000000..695d5a3 --- /dev/null +++ b/src/shell/actions/package_export_action.h @@ -0,0 +1,18 @@ +#pragma once +// package_export_action — the "export bank as package" action body: survey and +// report first, confirm what is absent, pick a destination, write. Every prompt in +// the flow lives here; shell/package/export_bank stays promptless. Registration and +// dispatch for its FOREVER-STABLE id ride main.cpp's action table. + +#include + +namespace reasampler { + +class ReaSamplerSession; + +// Exports one bank (the pool included — it is structurally a bank) to a .rsbank the +// user picks. Read-only against the project: the session is const, so no ext-state +// write, generation bump or undo point is reachable from here. +void doBankPackageExport(const ReaSamplerSession& session, const std::string& bankId); + +} // namespace reasampler diff --git a/src/shell/package/CLAUDE.md b/src/shell/package/CLAUDE.md index b4db0c4..a24737f 100644 --- a/src/shell/package/CLAUDE.md +++ b/src/shell/package/CLAUDE.md @@ -5,12 +5,12 @@ The filesystem and dialog acts behind bank-package export/import: streaming package file I/O plus the file-status and exclusive-create acts (`package_io`), the UTF-8 path conversion every one of them goes through (`package_path`), the landed-file -journal and its rollback delete (`package_rollback`), and the two file pickers -(`package_pickers`). This seam is bytes-only — the package format (magic, manifest, -entry layout) is `core/package`'s business, and the export/import verbs that -orchestrate both do not live here yet. No REAPER project state is touched in this -directory: no ext-state read or write, no undo block, no generation bump — those -belong to the verbs. +journal and its rollback delete (`package_rollback`), the two file pickers +(`package_pickers`), and the promptless export verb (`export_bank`) — the import verb +does not live here yet. The package format itself (magic, manifest, entry layout) +stays `core/package`'s business. No REAPER project state is touched in this directory: +no ext-state read or write, no undo block, no generation bump — those belong to the +prompting skin (`shell/actions/package_export_action`), not this seam. ## Invariants @@ -47,14 +47,17 @@ belong to the verbs. writer could win the race against. Collision handling (auto-rename) remains the import plan's job upstream. The package writer itself DOES replace an existing destination — the export save dialog's own overwrite confirm is the consent — and - that asymmetry is deliberate. **Known gap, obligation on the export verb:** - `pickPackageSavePath`'s own `.rsbank` re-append (see its Gotcha below) can turn a - confirmed path `X` into a write target `X.rsbank` that the dialog never asked about. - The export verb MUST re-check `fileStatus()` on the path actually handed to - `PackageFileWriter` — after any extension append — and get its own consent if that - re-checked path is `Present`; the dialog's confirm only ever covered the pre-append - path. Not fixed at this seam: prompting is verb-level UX, and `pickPackageSavePath` - has no caller yet, so the gap is latent, not live. + that asymmetry is deliberate. **Closed, both halves.** `pickPackageSavePath`'s own + `.rsbank` re-append (see its Gotcha below) can turn a confirmed path `X` into a write + target `X.rsbank` that the dialog never asked about, so `exportBank` re-checks + `fileStatus()` on the path actually handed to `PackageFileWriter` — after any + extension append — and refuses `RefusedDestinationExists` until the caller sets + `allowOverwrite`. The caller does not always re-prompt to get there: + `pickPackageSavePath` reports whether it appended (`outAppended`), and + `package_export_action` pre-grants `allowOverwrite` whenever it did NOT — an + unappended path is exactly what the dialog's own confirm already covered, so asking + again would be a second prompt for the same consent. Only an appended path, one the + dialog never saw, still costs the verb's own confirm naming that exact path. - **The rollback delete is prune's ONE carve-out, and only HALF of it is structural.** The citation and the full discriminator live at `package_rollback.cpp`'s header. "Did this call create it" is structural: only exclusively-created paths are @@ -76,7 +79,8 @@ belong to the verbs. - `package_path` — header-only; the ONE UTF-8-narrow → `fs::path` conversion, so the encoding contract has a single enforcement point. - `package_io` — every filesystem act the verbs need: `PayloadBuffer` (move-only payload + the `alive()` seam counter), `PackageFileWriter` (append-only temp+atomic-rename writer), `PackageFileReader` (seek-and-read one range per call, range-checked against the real file size), `readFilePayload` (one source file as one entry's payload), `fileStatus` (Present/Absent/Unreadable — export's refusal message must distinguish the last two, and an empty payload cannot), `writeFileExclusive` (exclusive create + write, self-cleaning on a partial write), and `listFolderFileNames` (bare UTF-8 names, sorted, non-recursive, non-throwing). REAPER-free; tested without a DAW. - `package_rollback` — `LandedFileJournal`: `writeLandedFile` (exclusive-create land, path resolved absolute, recorded on success only), `markIndexCommitted` (disarms the journal), and `rollback` (deletes exactly the recorded set, hard unlink, tolerating a vanished file; refuses once disarmed). REAPER-free; tested without a DAW. -- `package_pickers` — `pickPackageForImport` and `pickPackageSavePath`, both `GetUserFileName`. Compile-only until the verbs land; nothing here can be exercised in a unit test. +- `package_pickers` — `pickPackageForImport` and `pickPackageSavePath`, both `GetUserFileName`; `pickPackageSavePath` also reports whether it appended `.rsbank` (`outAppended`), the signal `package_export_action` uses to skip a redundant overwrite confirm. `pickPackageForImport` stays compile-only until the import verb lands; neither picker can be exercised in a unit test. +- `export_bank` — the promptless export verb, in three composable public steps: `surveyBankExport` (the read-only plan, report-before-acting), `digestSources` (measures each entry's length + `hashBytes` digest, one payload at a time), and `writePackageFile` (prefix, then each payload re-read and re-verified against that digest before it is appended, then commit). `exportBank` composes the three and gates on the plan verdict, the incomplete confirm and the destination confirm. The session arrives **const**: `saveToActiveProject`, `bumpBankGeneration` and `writeAssignmentRequest` are the session's only non-const acts, so a const session cannot reach them and "an export writes no ext state, opens no undo point and never bumps the generation" holds by the type rather than by memory (`pruneReclaim`, the sole file-deletion path, is const too and sits outside this claim). Reads the session through inline accessors only, which is why its tests link and run without a DAW. ## Gotchas diff --git a/src/shell/package/CMakeLists.txt b/src/shell/package/CMakeLists.txt index aff4414..6f2f148 100644 --- a/src/shell/package/CMakeLists.txt +++ b/src/shell/package/CMakeLists.txt @@ -1,7 +1,8 @@ # The filesystem + dialog seam for bank packages. package_io / package_rollback are # REAPER-free (standard filesystem only), so the pure-library/test helpers fit and -# their tests run without a DAW. The export/import verbs that drive all three targets -# are not in this directory yet. +# their tests run without a DAW. export_bank, the promptless export verb, lives here +# too for the same reason (ReaSamplerSession's inline accessors keep it REAPER-free); +# the import verb does not live here yet. reasampler_pure_library(package_io SOURCES package_io.cpp) reasampler_test(package_io LINK package_io) @@ -9,6 +10,21 @@ reasampler_test(package_io LINK package_io) reasampler_pure_library(package_rollback SOURCES package_rollback.cpp LINK PUBLIC package_io) reasampler_test(package_rollback LINK package_rollback) +# export_bank reads the live session through ReaSamplerSession's INLINE accessors only, +# so it pulls in no REAPER-facing TU and its tests link (and run) without a DAW. +# bank_book / tail_control / origin_ledger / tracking_authority / prune_reconcile / +# app_version / view_mode_model are session.h's own transitive includes (BankBook::bank() +# in particular is out-of-line, in bank_book.cpp) — declared here, on the library that +# actually needs them, rather than left for every consumer to enumerate. +reasampler_pure_library(export_bank + SOURCES export_bank.cpp + LINK PUBLIC export_plan bank_package package_io + PRIVATE capture_paths wav_codec bank_book tail_control origin_ledger + tracking_authority prune_reconcile app_version view_mode_model) +reasampler_test(export_bank + LINK export_bank bank_book slot_map view_mode_model tail_control origin_ledger + tracking_authority prune_reconcile app_version capture_paths) + # The pickers call the REAPER API, so no test target can exercise them; declared as a # library so the TU stays compiled. reaper_plugin.h pulls SWELL in on non-Windows. add_library(package_pickers STATIC package_pickers.cpp) diff --git a/src/shell/package/export_bank.cpp b/src/shell/package/export_bank.cpp new file mode 100644 index 0000000..d2aee61 --- /dev/null +++ b/src/shell/package/export_bank.cpp @@ -0,0 +1,184 @@ +// export_bank.cpp — see export_bank.h for the contract. +// +// wav_codec is called for hashBytes ONLY. Payload bytes are copied and hashed, never +// rebuilt, trimmed, normalized or collapsed — the capture path's mono collapse must +// not reach an export. + +#include "shell/package/export_bank.h" + +#include +#include +#include + +#include "core/capture/capture_paths.h" // resolveBankFile — the index's relative -> absolute +#include "core/capture/wav_codec.h" // hashBytes +#include "core/model/bank_book.h" +#include "shell/package/package_io.h" +#include "shell/persist/session.h" // ReaSamplerSession — read through its inline book() only + +namespace reasampler { + +namespace { + +package::SourceFileState stateOf(const std::string& absPath) { + switch (fileStatus(absPath)) { + case FileStatus::Present: return package::SourceFileState::Present; + case FileStatus::Unreadable: return package::SourceFileState::Unreadable; + case FileStatus::Absent: break; + } + return package::SourceFileState::Missing; +} + +std::vector absoluteSources(const std::string& projectDir, + const std::vector& relativePaths) { + std::vector out; + out.reserve(relativePaths.size()); + for (const std::string& rel : relativePaths) + out.push_back(capture::resolveBankFile(projectDir, rel)); + return out; +} + +} // namespace + +ExportSurvey surveyBankExport(const ReaSamplerSession& session, + const std::string& projectDir, + const std::string& bankId) { + ExportSurvey survey; + const Bank* bank = session.book().bank(bankId); + if (!bank) return survey; + survey.bankFound = true; + + package::ExportInputs inputs; + inputs.bankDisplayName = bank->displayName; + inputs.slots = bank->slots; + for (const model::Sample& s : bank->index.all()) { + package::ExportCandidate c; + c.sample = s; + c.fileState = stateOf(capture::resolveBankFile(projectDir, s.relativePath)); + inputs.candidates.push_back(std::move(c)); + } + survey.plan = package::planExport(inputs); + return survey; +} + +bool digestSources(package::PackageManifest& manifest, + const std::vector& sourceAbsPaths, + std::string& outFailedName) { + outFailedName.clear(); + if (sourceAbsPaths.size() != manifest.entries.size()) return false; + for (std::size_t i = 0; i < manifest.entries.size(); ++i) { + const PayloadBuffer payload = readFilePayload(sourceAbsPaths[i]); + if (payload.empty()) { + outFailedName = manifest.entries[i].fileName; + return false; + } + manifest.entries[i].byteLength = payload.size(); + manifest.entries[i].byteHash = capture::hashBytes(payload.data(), payload.size()); + } + return true; +} + +ExportOutcome writePackageFile(const package::EncodedPackage& encoded, + const package::PackageManifest& manifest, + const std::vector& sourceAbsPaths, + const std::string& destAbsPath) { + ExportOutcome out; + if (sourceAbsPaths.size() != manifest.entries.size() || + encoded.layout.size() != manifest.entries.size()) { + out.status = ExportStatus::EncodeFailed; + return out; + } + + // Every early return below abandons the writer through its destructor, which + // removes the temp and leaves the destination untouched. + PackageFileWriter writer(destAbsPath); + if (!writer.ok() || !writer.appendRaw(encoded.prefix.data(), encoded.prefix.size())) { + out.status = ExportStatus::WriteFailed; + return out; + } + std::uint64_t written = encoded.prefix.size(); + + for (std::size_t i = 0; i < manifest.entries.size(); ++i) { + const package::PackageEntry& entry = manifest.entries[i]; + const PayloadBuffer payload = readFilePayload(sourceAbsPaths[i]); + if (payload.empty()) { + out.status = ExportStatus::SourceReadFailed; + out.offendingName = entry.fileName; + return out; + } + if (payload.size() != entry.byteLength || + capture::hashBytes(payload.data(), payload.size()) != entry.byteHash) { + out.status = ExportStatus::SourceChanged; + out.offendingName = entry.fileName; + return out; + } + if (!writer.appendPayload(payload)) { + out.status = ExportStatus::WriteFailed; + return out; + } + written += payload.size(); + } + + if (written != encoded.totalSize || !writer.commit()) { + out.status = ExportStatus::WriteFailed; + return out; + } + out.status = ExportStatus::Written; + out.entriesWritten = manifest.entries.size(); + out.bytesWritten = written; + return out; +} + +ExportOutcome exportBank(const ReaSamplerSession& session, const ExportRequest& req) { + ExportOutcome out; + if (req.projectDir.empty()) { + out.status = ExportStatus::NoProjectDir; + return out; + } + + const ExportSurvey survey = surveyBankExport(session, req.projectDir, req.bankId); + if (!survey.bankFound) { + out.status = ExportStatus::NoSuchBank; + return out; + } + out.bankDisplayName = survey.plan.manifest.bankDisplayName; + out.excluded = survey.plan.excluded; + + if (survey.plan.verdict == package::ExportVerdict::Refused) { + out.status = ExportStatus::RefusedUnrepresentable; + return out; + } + if (survey.plan.verdict == package::ExportVerdict::Incomplete && !req.allowIncomplete) { + out.status = ExportStatus::RefusedIncomplete; + return out; + } + // The save dialog's own overwrite confirm covered the path the USER chose, which + // is not necessarily the path handed here (the picker re-appends `.rsbank`), so + // consent for the real target is re-taken by the skin. + if (!req.allowOverwrite && fileStatus(req.destAbsPath) == FileStatus::Present) { + out.status = ExportStatus::RefusedDestinationExists; + return out; + } + + package::PackageManifest manifest = survey.plan.manifest; + manifest.exportTimestamp = req.exportTimestamp; + const std::vector sources = + absoluteSources(req.projectDir, survey.plan.sourceRelativePaths); + + if (!digestSources(manifest, sources, out.offendingName)) { + out.status = ExportStatus::SourceReadFailed; + return out; + } + const std::optional encoded = package::encodePackage(manifest); + if (!encoded) { + out.status = ExportStatus::EncodeFailed; + return out; + } + + ExportOutcome written = writePackageFile(*encoded, manifest, sources, req.destAbsPath); + written.bankDisplayName = out.bankDisplayName; + written.excluded = std::move(out.excluded); + return written; +} + +} // namespace reasampler diff --git a/src/shell/package/export_bank.h b/src/shell/package/export_bank.h new file mode 100644 index 0000000..d78ee3f --- /dev/null +++ b/src/shell/package/export_bank.h @@ -0,0 +1,95 @@ +// shell/package/export_bank — the promptless bank-export verb: survey, digest, +// stream, commit. No prompts and no message boxes (shell/actions/ +// package_export_action is the skin). The session arrives CONST, which is how "an +// export writes no ext state, opens no undo point and never bumps the bank +// generation" is enforced rather than remembered — those three acts +// (saveToActiveProject, bumpBankGeneration, writeAssignmentRequest) are exactly the +// session members that are non-const (session.h:113,108,147). Constness does not +// block every mutation, though: pruneReclaim (session.h:140-141) is const and is +// the system's sole file-deletion path — irrelevant to export, but not something a +// const session forbids in general. Blocking I/O: UI-thread actions only. + +#pragma once + +#include +#include +#include +#include + +#include "core/package/bank_package.h" +#include "core/package/export_plan.h" + +namespace reasampler { + +class ReaSamplerSession; + +struct ExportRequest { + std::string projectDir; // absolute; the root the index's relative paths hang off + std::string bankId; + std::string destAbsPath; // the .rsbank to write + std::int64_t exportTimestamp = 0; // manifest envelope; the caller's clock read + // Both default false and are set ONLY after the skin's explicit confirm: one + // lists what is absent, the other names the destination being replaced. + bool allowIncomplete = false; + bool allowOverwrite = false; +}; + +enum class ExportStatus { + Written, + NoSuchBank, + NoProjectDir, + RefusedIncomplete, + RefusedUnrepresentable, + RefusedDestinationExists, + SourceReadFailed, // a file the plan classified Present would not read, or is empty + SourceChanged, // a payload's bytes moved between the digest pass and the stream pass + EncodeFailed, + WriteFailed, +}; + +struct ExportOutcome { + ExportStatus status = ExportStatus::WriteFailed; + std::size_t entriesWritten = 0; + std::uint64_t bytesWritten = 0; + std::string bankDisplayName; + std::vector excluded; + std::string offendingName; // the entry a SourceReadFailed / SourceChanged names +}; + +struct ExportSurvey { + bool bankFound = false; + package::ExportPlan plan; +}; + +// Report-before-acting: the same plan exportBank recomputes, with nothing written. +// Read-only against both the project and the filesystem. +ExportSurvey surveyBankExport(const ReaSamplerSession& session, + const std::string& projectDir, + const std::string& bankId); + +// Fills each manifest entry's byteLength and byteHash from its source file — the +// digest pass, one payload in memory at a time. False with `outFailedName` set when a +// source will not read or is empty; a zero-length entry cannot round-trip the +// format's own seam, so it is a failure here rather than an entry. +bool digestSources(package::PackageManifest& manifest, + const std::vector& sourceAbsPaths, + std::string& outFailedName); + +// Streams one package to `destAbsPath`: the encoded prefix, then each payload re-read +// from `sourceAbsPaths` (parallel to `manifest.entries`) and re-checked against the +// length and digest recorded for it before it is appended — so the digest the +// manifest claims describes the bytes actually written, not the bytes a concurrent +// edit replaced. Any failure abandons the writer, leaving the destination absent or +// holding its prior contents. +// +// Public because that atomicity is this function's property: proving it needs a +// failure injected mid-stream, which is a call to this seam, not to exportBank. +ExportOutcome writePackageFile(const package::EncodedPackage& encoded, + const package::PackageManifest& manifest, + const std::vector& sourceAbsPaths, + const std::string& destAbsPath); + +// The verb: plan, gate on the verdict and the destination, digest, encode, stream. +ExportOutcome exportBank(const ReaSamplerSession& session, const ExportRequest& req); + +} // namespace reasampler diff --git a/src/shell/package/package_io.cpp b/src/shell/package/package_io.cpp index c268742..f057f03 100644 --- a/src/shell/package/package_io.cpp +++ b/src/shell/package/package_io.cpp @@ -29,6 +29,7 @@ namespace fs = std::filesystem; namespace { std::atomic g_alivePayloads{0}; +std::atomic g_peakAlivePayloads{0}; } // --------------------------------------------------------------------------- @@ -36,7 +37,13 @@ std::atomic g_alivePayloads{0}; PayloadBuffer::PayloadBuffer(std::vector bytes) : bytes_(std::move(bytes)), counted_(!bytes_.empty()) { - if (counted_) g_alivePayloads.fetch_add(1, std::memory_order_relaxed); + if (counted_) { + const int now = g_alivePayloads.fetch_add(1, std::memory_order_relaxed) + 1; + int peak = g_peakAlivePayloads.load(std::memory_order_relaxed); + while (now > peak && !g_peakAlivePayloads.compare_exchange_weak( + peak, now, std::memory_order_relaxed)) { + } + } } PayloadBuffer::~PayloadBuffer() { release(); } @@ -60,6 +67,7 @@ PayloadBuffer& PayloadBuffer::operator=(PayloadBuffer&& other) noexcept { } int PayloadBuffer::alive() { return g_alivePayloads.load(std::memory_order_relaxed); } +int PayloadBuffer::highWaterMark() { return g_peakAlivePayloads.load(std::memory_order_relaxed); } void PayloadBuffer::release() { if (counted_) g_alivePayloads.fetch_sub(1, std::memory_order_relaxed); diff --git a/src/shell/package/package_io.h b/src/shell/package/package_io.h index adf60ec..384a06b 100644 --- a/src/shell/package/package_io.h +++ b/src/shell/package/package_io.h @@ -33,6 +33,11 @@ public: // Buffers currently holding at least one byte, process-wide. static int alive(); + // The largest alive() has ever been, process-wide. A point-in-time alive() == 0 + // check after a call returns cannot fail on a whole-package-in-memory shape that + // allocated N buffers and freed them all one at a time — highWaterMark() can, + // since it is never reset. + static int highWaterMark(); private: void release(); diff --git a/src/shell/package/package_pickers.cpp b/src/shell/package/package_pickers.cpp index 16395a6..da04894 100644 --- a/src/shell/package/package_pickers.cpp +++ b/src/shell/package/package_pickers.cpp @@ -47,7 +47,8 @@ bool pickPackageForImport(std::string& outAbsPath) { return runPicker(1, "Import bank package", "", outAbsPath); } -bool pickPackageSavePath(const std::string& suggestedPath, std::string& outAbsPath) { +bool pickPackageSavePath(const std::string& suggestedPath, std::string& outAbsPath, + bool* outAppended) { // [verify — DAW] GetUserFileName takes no owner window, so the dialog's parenting // is REAPER's to do; the previous Win32 path passed GetMainHwnd() explicitly. if (!runPicker(0, "Export bank package", suggestedPath.c_str(), outAbsPath)) { @@ -56,7 +57,9 @@ bool pickPackageSavePath(const std::string& suggestedPath, std::string& outAbsPa // GetUserFileName has no lpstrDefExt equivalent (the old Win32 picker's // ofn.lpstrDefExt = L"rsbank"); whether mode 0 appends one itself from // kExtList is [verify — DAW], so append it ourselves whenever it's missing. - if (!hasCaseInsensitiveSuffix(outAbsPath, ".rsbank")) outAbsPath += ".rsbank"; + const bool appended = !hasCaseInsensitiveSuffix(outAbsPath, ".rsbank"); + if (appended) outAbsPath += ".rsbank"; + if (outAppended) *outAppended = appended; return true; } diff --git a/src/shell/package/package_pickers.h b/src/shell/package/package_pickers.h index 9f60512..136a6d9 100644 --- a/src/shell/package/package_pickers.h +++ b/src/shell/package/package_pickers.h @@ -18,7 +18,12 @@ bool pickPackageForImport(std::string& outAbsPath); // suggestedPath is a bare file name ("MyBank.rsbank") or a full path — a full one // also seeds the dialog's starting directory, which is how a caller keeps the picker // off REAPER's process working directory. True with outAbsPath set iff the user chose -// a destination; the dialog's own overwrite confirm has already run by then. -bool pickPackageSavePath(const std::string& suggestedPath, std::string& outAbsPath); +// a destination; the dialog's own overwrite confirm has already run by then, against +// the path the user actually chose — NOT necessarily outAbsPath, if the `.rsbank` +// re-append below fires. outAppended, when non-null, is set to whether it fired: the +// caller's signal that its own overwrite consent may not cover the returned path +// (see this directory's CLAUDE.md). +bool pickPackageSavePath(const std::string& suggestedPath, std::string& outAbsPath, + bool* outAppended = nullptr); } // namespace reasampler diff --git a/src/shell/panel/panel_bank_ops.cpp b/src/shell/panel/panel_bank_ops.cpp index 0ce2fbb..a45cb99 100644 --- a/src/shell/panel/panel_bank_ops.cpp +++ b/src/shell/panel/panel_bank_ops.cpp @@ -16,6 +16,7 @@ #include "shell/panel/panel_state.h" #include "shell/panel/panel_bank_ops.h" +#include "shell/actions/package_export_action.h" // doBankPackageExport — the export skin #include "shell/bank_ops/bank_ops.h" // bankOp* promptless verbs #include "shell/persist/session.h" // ReaSamplerSession — the live session the ops mutate @@ -241,6 +242,7 @@ enum : unsigned int { kMenuDelete, kMenuEvacuate, kMenuCreate, + kMenuExport, // export this bank as a .rsbank package kMenuRemove, // remove selected sample(s) from the source bank kMenuMoveBase = 1000, // move-to-bank: kMenuMoveBase + destination index kMenuCopyBase = 2000, // copy-to-bank: kMenuCopyBase + destination index @@ -249,7 +251,7 @@ enum : unsigned int { } // namespace // Shows the right-click context menu for a named-bank TAB: activate / rename / delete -// / evacuate that bank, plus a create entry. Drives the id-keyed ops. +// / evacuate / export that bank, plus a create entry. Drives the id-keyed ops. void showTabMenu(int screenX, int screenY, const std::string& bankId) { if (!book()) return; const Bank* bk = book()->bank(bankId); @@ -265,6 +267,7 @@ void showTabMenu(int screenX, int screenY, const std::string& bankId) { menuAppend(menu, kMenuRename, "Rename..."); menuAppend(menu, kMenuEvacuate, "Evacuate to pool", /*grayed=*/!nonEmpty); menuAppend(menu, kMenuDelete, "Delete..."); + menuAppend(menu, kMenuExport, "Export as package..."); menuSeparator(menu); menuAppend(menu, kMenuCreate, "New bank..."); @@ -277,6 +280,7 @@ void showTabMenu(int screenX, int screenY, const std::string& bankId) { case kMenuRename: doRenameBank(bankId); break; case kMenuEvacuate: doEvacuateBank(bankId); break; case kMenuDelete: doDeleteBank(bankId); break; + case kMenuExport: doBankPackageExport(*g_panel.session, bankId); break; case kMenuCreate: doCreateBank(); break; default: break; } diff --git a/tests/test_export_bank.cpp b/tests/test_export_bank.cpp new file mode 100644 index 0000000..33a6c1b --- /dev/null +++ b/tests/test_export_bank.cpp @@ -0,0 +1,471 @@ +// Standalone tests for shell/package/export_bank — no REAPER, no framework. The +// export reads the session through inline accessors only, so a real ReaSamplerSession +// and a real bank folder on disk are both constructible here. +// +// Mid-stream failure is INJECTED rather than simulated: the digest pass and the +// stream pass are separate public calls, so a source file removed or rewritten +// between them is exactly the concurrent-edit case the stream pass re-checks for. + +#include "../src/shell/package/export_bank.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../src/core/capture/wav_codec.h" +#include "../src/core/model/bank_book.h" +#include "../src/core/package/bank_package.h" +#include "../src/shell/package/package_io.h" +#include "../src/shell/package/package_path.h" +#include "../src/shell/persist/session.h" + +using namespace reasampler; +namespace fs = std::filesystem; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +// --- scratch filesystem ------------------------------------------------------- + +static std::string g_root; + +static std::string scratchRoot() { + if (g_root.empty()) { + std::error_code ec; + const fs::path p = fs::temp_directory_path(ec) / "reasampler_export_tests"; + fs::remove_all(p, ec); + fs::create_directories(p, ec); + g_root = p.generic_string(); + } + return g_root; +} + +static std::vector patternBytes(std::size_t n, std::uint8_t seed) { + std::vector v(n); + for (std::size_t i = 0; i < n; ++i) v[i] = static_cast(seed + i * 7u); + return v; +} + +static void writeFile(const std::string& path, const std::vector& bytes) { + std::ofstream f(utf8Path(path), std::ios::binary | std::ios::trunc); + f.write(reinterpret_cast(bytes.data()), + static_cast(bytes.size())); +} + +static std::vector readFile(const std::string& path) { + std::ifstream f(utf8Path(path), std::ios::binary); + return std::vector(std::istreambuf_iterator(f), + std::istreambuf_iterator()); +} + +static bool exists(const std::string& path) { return fs::exists(utf8Path(path)); } + +// --- fixture project ---------------------------------------------------------- + +// One scratch project directory with a bank folder, plus the session whose book +// names its contents. `fileNames` are written into the bank folder with distinct +// byte patterns; a name in `omit` gets an index entry but NO file on disk. +struct Fixture { + std::string projectDir; + ReaSamplerSession session; + std::string bankId = "bank-1"; + + explicit Fixture(const std::string& tag) { + projectDir = scratchRoot() + "/" + tag; + std::error_code ec; + fs::create_directories(utf8Path(projectDir + "/reasampler_bank"), ec); + session.book().createBank(bankId, "Drums " + tag); + } + + void addSample(const std::string& id, const std::string& fileName, + std::size_t bytes, std::uint8_t seed, bool writeToDisk = true, + const std::string& displayName = "") { + model::Sample s; + s.id = id; + s.displayName = displayName.empty() ? id : displayName; + s.relativePath = std::string("reasampler_bank/") + fileName; + s.sampleRate = 48000; + s.channelCount = 2; + s.contentHash = "hash-" + id; + CHECK(session.book().index(bankId)->add(s) == model::AddResult::Added); + session.book().reconcileSlots(); + if (writeToDisk) writeFile(absPathOf(fileName), patternBytes(bytes, seed)); + } + + std::string absPathOf(const std::string& fileName) const { + return projectDir + "/reasampler_bank/" + fileName; + } + std::string destPath() const { return projectDir + "/out.rsbank"; } + + ExportRequest request() const { + ExportRequest req; + req.projectDir = projectDir; + req.bankId = bankId; + req.destAbsPath = destPath(); + req.exportTimestamp = 1234567890; + return req; + } +}; + +// --- package readback --------------------------------------------------------- + +// Decodes an emitted package straight off disk, growing the prefix read the way the +// format's own requiredPrefixSize seam asks callers to. +static package::DecodedPackage decodeFromDisk(const std::string& path) { + PackageFileReader reader(path); + const std::uint64_t size = reader.fileSize(); + std::vector prefix; + for (int guard = 0; guard < 8; ++guard) { + const std::optional need = package::requiredPrefixSize(prefix); + if (!need) break; + if (*need <= prefix.size()) break; + PayloadBuffer buf = reader.readRange(0, *need); + if (buf.empty()) break; + prefix.assign(buf.data(), buf.data() + buf.size()); + } + return package::decodePackage(prefix, size); +} + +// --- tests -------------------------------------------------------------------- + +static void testHealthyExportCarriesEveryPayloadByteExact() { + Fixture fx("healthy"); + fx.addSample("s1", "kick.wav", 800, 1); + fx.addSample("s2", "snare.wav", 1300, 60); + fx.addSample("s3", "hat.wav", 97, 200); + + const ExportOutcome out = exportBank(fx.session, fx.request()); + CHECK(out.status == ExportStatus::Written); + CHECK(out.entriesWritten == 3); + CHECK(out.excluded.empty()); + CHECK(PayloadBuffer::alive() == 0); + // Point-in-time alive() == 0 alone cannot fail on a whole-package-in-memory + // shape (N buffers allocated and freed one at a time still ends at 0); the + // high-water mark can, across this three-entry export and everything the test + // binary ran before it — it must never exceed the "at most one payload" claim. + CHECK(PayloadBuffer::highWaterMark() == 1); + + const package::DecodedPackage decoded = decodeFromDisk(fx.destPath()); + CHECK(decoded.status == package::PackageReadability::Readable); + CHECK(decoded.manifest.entries.size() == 3); + CHECK(decoded.layout.size() == 3); + CHECK(decoded.manifest.bankDisplayName == "Drums healthy"); + CHECK(decoded.manifest.exportTimestamp == 1234567890); + + // PER ENTRY, not in aggregate: the digest the package records, the digest of the + // payload actually stored at that entry's span, and the digest of the source file + // on disk must all be the same string. + PackageFileReader reader(fx.destPath()); + const std::vector sourceNames = {"kick.wav", "snare.wav", "hat.wav"}; + CHECK(decoded.manifest.entries.size() == sourceNames.size()); + for (std::size_t i = 0; i < decoded.manifest.entries.size(); ++i) { + const package::PackageEntry& entry = decoded.manifest.entries[i]; + const PayloadBuffer stored = reader.readRange(decoded.layout[i].offset, + decoded.layout[i].length); + CHECK(!stored.empty()); + const std::vector source = readFile(fx.absPathOf(sourceNames[i])); + const std::string sourceDigest = capture::hashBytes(source.data(), source.size()); + CHECK(entry.byteLength == source.size()); + CHECK(entry.byteHash == sourceDigest); + CHECK(capture::hashBytes(stored.data(), stored.size()) == sourceDigest); + CHECK(stored.size() == source.size()); + CHECK(std::equal(source.begin(), source.end(), stored.data())); + } + CHECK(PayloadBuffer::alive() == 0); +} + +// Every occurrence of `"key":"value"` in `json`, value returned raw (escape-aware +// only enough to not stop early on an escaped quote — this file's own writer output +// never nests an unescaped quote, so that is sufficient here). +static std::vector jsonStringValuesForKey(const std::string& json, + const std::string& key) { + std::vector values; + const std::string marker = "\"" + key + "\":\""; + std::size_t pos = 0; + while ((pos = json.find(marker, pos)) != std::string::npos) { + std::size_t i = pos + marker.size(); + while (i < json.size() && json[i] != '"') { + if (json[i] == '\\') ++i; // skip the escaped char too + ++i; + } + values.push_back(json.substr(pos + marker.size(), i - (pos + marker.size()))); + pos = i; + } + return values; +} + +// True for a value opening with an ':' drive-relative prefix (":" alone is +// JSON's own key separator, so this is checked on isolated VALUES, never on raw text). +static bool looksLikeDriveForm(const std::string& value) { + return value.size() >= 2 && std::isalpha(static_cast(value[0])) && + value[1] == ':'; +} + +static void testEmittedManifestBytesCarryNoPath() { + Fixture fx("nopath"); + // The bank's own display name AND a sample's displayName each carry a literal + // '/' — free text, unlike the entry `name` / nested `relativePath` fields this + // test actually polices (docs/product/bank-package.md:282-289: the destination is + // derived from the entry name, never from free text). Present in the fixture so + // the scan below proves it is scoped correctly rather than merely holding by + // accident on names that happen not to collide with the rule. + CHECK(fx.session.book().renameBank(fx.bankId, "Drums/Bus")); + // Both source records carry a directory component; neither may reach the file. + fx.addSample("s1", "kick.wav", 200, 3, /*writeToDisk=*/true, "Kick / alt take"); + fx.addSample("s2", "snare take 2.wav", 200, 9); + CHECK(exportBank(fx.session, fx.request()).status == ExportStatus::Written); + + // Locate the MANIFEST REGION of the emitted file by walking the frozen header the + // way a reader does: magic | fv | minReader | len+semver | len+JSON. The binary + // length fields are deliberately excluded — a length whose byte happens to be + // 0x2F is not a separator. + const std::vector file = readFile(fx.destPath()); + CHECK(file.size() > 20); + auto le32 = [&](std::size_t at) { + return static_cast(file[at]) | + (static_cast(file[at + 1]) << 8) | + (static_cast(file[at + 2]) << 16) | + (static_cast(file[at + 3]) << 24); + }; + const std::uint32_t semverLen = le32(12); + const std::size_t manifestLenAt = 16 + semverLen; + const std::uint32_t manifestLen = le32(manifestLenAt); + const std::size_t manifestAt = manifestLenAt + 4; + CHECK(manifestAt + manifestLen <= file.size()); + const std::string manifest(reinterpret_cast(file.data() + manifestAt), + manifestLen); + CHECK(manifest.find("kick.wav") != std::string::npos); // the scan is looking at the manifest + + // The boundary, pinned rather than assumed: free text legitimately carries '/'. + CHECK(manifest.find("Drums/Bus") != std::string::npos); + CHECK(manifest.find("Kick / alt take") != std::string::npos); + + // The rule itself: scoped to the two fields the importer derives a destination + // from — the entry `name` and the nested Sample's own `relativePath` — never to + // `displayName` or the manifest's `bankDisplayName`. + const std::vector names = jsonStringValuesForKey(manifest, "name"); + const std::vector relPaths = jsonStringValuesForKey(manifest, "relativePath"); + CHECK(!names.empty()); + CHECK(!relPaths.empty()); + for (const std::string& v : names) { + CHECK(v.find('/') == std::string::npos); + CHECK(v.find('\\') == std::string::npos); + CHECK(v.find("..") == std::string::npos); + CHECK(!looksLikeDriveForm(v)); + } + for (const std::string& v : relPaths) { + CHECK(v.find('/') == std::string::npos); + CHECK(v.find('\\') == std::string::npos); + CHECK(v.find("..") == std::string::npos); + CHECK(!looksLikeDriveForm(v)); + } +} + +static void testFailureInjectedMidWriteLeavesNoPackageAndSparesThePriorFile() { + Fixture fx("midwrite"); + fx.addSample("s1", "kick.wav", 500, 1); + fx.addSample("s2", "snare.wav", 500, 2); + + const std::vector prior = patternBytes(64, 99); + writeFile(fx.destPath(), prior); + + ExportSurvey survey = surveyBankExport(fx.session, fx.projectDir, fx.bankId); + CHECK(survey.plan.verdict == package::ExportVerdict::Ready); + package::PackageManifest manifest = survey.plan.manifest; + const std::vector sources = {fx.absPathOf("kick.wav"), + fx.absPathOf("snare.wav")}; + std::string failed; + CHECK(digestSources(manifest, sources, failed)); + const std::optional encoded = package::encodePackage(manifest); + CHECK(encoded.has_value()); + + // Injection: the second payload vanishes after the framing that claims it was + // already encoded, so the failure lands with the prefix and one payload written. + std::error_code ec; + fs::remove(utf8Path(fx.absPathOf("snare.wav")), ec); + + const ExportOutcome out = + writePackageFile(*encoded, manifest, sources, fx.destPath()); + CHECK(out.status == ExportStatus::SourceReadFailed); + CHECK(out.offendingName == "snare.wav"); + CHECK(readFile(fx.destPath()) == prior); // the prior file is untouched + CHECK(!exists(fx.destPath() + ".rsbanktmp")); // and no debris is left behind + CHECK(PayloadBuffer::alive() == 0); +} + +static void testCommitFailureIsAGenuineMidWriteAbandon() { + // The mid-READ injection above (a source vanishing between digest and stream) is + // not what "mid-write" names in export_bank.cpp:115-118/122-125 — those guard a + // failure IN the write itself: appendPayload's stream going bad, or commit's + // rename failing. A directory squatting on the destination (test_package_io.cpp's + // own precedent for a real, not simulated, commit failure) makes every payload + // stream fine and only the final rename fail. + Fixture fx("commitfail"); + fx.addSample("s1", "kick.wav", 300, 1); + fx.addSample("s2", "snare.wav", 300, 2); + + std::error_code ec; + fs::create_directory(utf8Path(fx.destPath()), ec); + CHECK(!ec); + + ExportSurvey survey = surveyBankExport(fx.session, fx.projectDir, fx.bankId); + CHECK(survey.plan.verdict == package::ExportVerdict::Ready); + package::PackageManifest manifest = survey.plan.manifest; + const std::vector sources = {fx.absPathOf("kick.wav"), fx.absPathOf("snare.wav")}; + std::string failed; + CHECK(digestSources(manifest, sources, failed)); + const std::optional encoded = package::encodePackage(manifest); + CHECK(encoded.has_value()); + + const ExportOutcome out = writePackageFile(*encoded, manifest, sources, fx.destPath()); + CHECK(out.status == ExportStatus::WriteFailed); + CHECK(fs::is_directory(utf8Path(fx.destPath()))); // the squatting dir is untouched + CHECK(!exists(fx.destPath() + ".rsbanktmp")); // commit()'s own self-clean ran + CHECK(PayloadBuffer::alive() == 0); + + fs::remove(utf8Path(fx.destPath()), ec); +} + +static void testPayloadChangedBetweenDigestAndStreamAborts() { + Fixture fx("changed"); + fx.addSample("s1", "kick.wav", 500, 1); + CHECK(!exists(fx.destPath())); + + ExportSurvey survey = surveyBankExport(fx.session, fx.projectDir, fx.bankId); + package::PackageManifest manifest = survey.plan.manifest; + const std::vector sources = {fx.absPathOf("kick.wav")}; + std::string failed; + CHECK(digestSources(manifest, sources, failed)); + const std::optional encoded = package::encodePackage(manifest); + CHECK(encoded.has_value()); + + // Same length, different bytes — only the digest re-check can catch this. + writeFile(fx.absPathOf("kick.wav"), patternBytes(500, 77)); + + const ExportOutcome out = + writePackageFile(*encoded, manifest, sources, fx.destPath()); + CHECK(out.status == ExportStatus::SourceChanged); + CHECK(out.offendingName == "kick.wav"); + CHECK(!exists(fx.destPath())); +} + +static void testExportTouchesNoProjectState() { + Fixture fx("readonly"); + fx.addSample("s1", "kick.wav", 400, 5); + fx.addSample("s2", "snare.wav", 400, 6); + + // saveToActiveProject writes seven keys (shell/persist/ext_state_io.cpp): `banks`, + // the legacy-key clear, `view_state`, the tail setting, the tracking ledger, the + // version stamp, and the bank-generation counter. This asserts byte-identity of + // the three that have an in-memory string to diff (`banks`, `view_state`, the tail + // setting) plus bankGeneration() (the bank-generation-counter key IS its + // serialization). The legacy-key clear and the version stamp are session-external, + // nothing here to diff against. The tracking ledger has no public accessor to diff + // either, but needs none: exportBank/digestSources/writePackageFile all take the + // session by `const&`, and ReaSamplerSession::recordCreated — the ledger's one + // writer (session.h) — is non-const, so it is not reachable through this call at + // all; the compiler enforces "untouched" here rather than a runtime check proving it. + const std::string bankBookBefore = fx.session.book().serialize(); + const std::string viewBefore = fx.session.view().serialize(); + const std::string tailBefore = capture::serializeTailSetting(fx.session.tail()); + const std::int64_t generationBefore = fx.session.bankGeneration(); + + CHECK(exportBank(fx.session, fx.request()).status == ExportStatus::Written); + + CHECK(fx.session.book().serialize() == bankBookBefore); + CHECK(fx.session.view().serialize() == viewBefore); + CHECK(capture::serializeTailSetting(fx.session.tail()) == tailBefore); + CHECK(fx.session.bankGeneration() == generationBefore); +} + +static void testEmptyBankExportsAsAValidZeroEntryPackage() { + Fixture fx("empty"); + const ExportOutcome out = exportBank(fx.session, fx.request()); + CHECK(out.status == ExportStatus::Written); + CHECK(out.entriesWritten == 0); + + const package::DecodedPackage decoded = decodeFromDisk(fx.destPath()); + CHECK(decoded.status == package::PackageReadability::Readable); + CHECK(decoded.manifest.entries.empty()); + CHECK(decoded.layout.empty()); + CHECK(decoded.manifest.bankDisplayName == "Drums empty"); + // The size proof is decodePackage's, and it ran against the real on-disk size. + CHECK(decoded.prefixSize == readFile(fx.destPath()).size()); +} + +static void testIncompleteBankRefusesUntilConfirmed() { + Fixture fx("incomplete"); + fx.addSample("s1", "kick.wav", 300, 1); + fx.addSample("s2", "gone.wav", 300, 2, /*writeToDisk=*/false); + + ExportRequest req = fx.request(); + const ExportOutcome refused = exportBank(fx.session, req); + CHECK(refused.status == ExportStatus::RefusedIncomplete); + CHECK(refused.excluded.size() == 1); + CHECK(refused.excluded[0].sampleId == "s2"); + CHECK(refused.excluded[0].reason == package::ExclusionReason::FileMissing); + CHECK(!exists(fx.destPath())); + + req.allowIncomplete = true; + const ExportOutcome allowed = exportBank(fx.session, req); + CHECK(allowed.status == ExportStatus::Written); + CHECK(allowed.entriesWritten == 1); + CHECK(allowed.excluded.size() == 1); // the report survives into the summary + CHECK(decodeFromDisk(fx.destPath()).manifest.entries.size() == 1); +} + +static void testExistingDestinationRefusesUntilConfirmed() { + Fixture fx("overwrite"); + fx.addSample("s1", "kick.wav", 300, 1); + const std::vector prior = patternBytes(32, 11); + writeFile(fx.destPath(), prior); + + ExportRequest req = fx.request(); + const ExportOutcome refused = exportBank(fx.session, req); + CHECK(refused.status == ExportStatus::RefusedDestinationExists); + CHECK(readFile(fx.destPath()) == prior); + + req.allowOverwrite = true; + CHECK(exportBank(fx.session, req).status == ExportStatus::Written); + CHECK(readFile(fx.destPath()) != prior); +} + +static void testUnknownBankAndUnsavedProjectAreNamedSeparately() { + Fixture fx("guards"); + ExportRequest req = fx.request(); + req.bankId = "no-such-bank"; + CHECK(exportBank(fx.session, req).status == ExportStatus::NoSuchBank); + + ExportRequest unsaved = fx.request(); + unsaved.projectDir.clear(); + CHECK(exportBank(fx.session, unsaved).status == ExportStatus::NoProjectDir); + CHECK(!exists(fx.destPath())); +} + +int main() { + testHealthyExportCarriesEveryPayloadByteExact(); + testEmittedManifestBytesCarryNoPath(); + testFailureInjectedMidWriteLeavesNoPackageAndSparesThePriorFile(); + testCommitFailureIsAGenuineMidWriteAbandon(); + testPayloadChangedBetweenDigestAndStreamAborts(); + testExportTouchesNoProjectState(); + testEmptyBankExportsAsAValidZeroEntryPackage(); + testIncompleteBankRefusesUntilConfirmed(); + testExistingDestinationRefusesUntilConfirmed(); + testUnknownBankAndUnsavedProjectAreNamedSeparately(); + + if (g_fail == 0) { + std::printf("export_bank_tests: all passed\n"); + return 0; + } + std::printf("export_bank_tests: %d failure(s)\n", g_fail); + return 1; +} diff --git a/tests/test_export_plan.cpp b/tests/test_export_plan.cpp new file mode 100644 index 0000000..3ccd784 --- /dev/null +++ b/tests/test_export_plan.cpp @@ -0,0 +1,290 @@ +// Standalone tests for reasampler::package::export_plan — no REAPER, no filesystem, +// no test framework. The planner's totality claim is the point: every input class +// (missing / unreadable / unrepresentable / zero / one) classifies here, and the +// names it produces are asserted against the codec's OWN predicates rather than +// against a hand-copied rule. + +#include "../src/core/package/export_plan.h" + +#include +#include +#include +#include +#include + +#include "../src/core/package/package_format.h" +#include "../src/core/package/package_manifest.h" + +using namespace reasampler::package; +using namespace reasampler::model; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +// --- fixtures ---------------------------------------------------------------- + +static Sample sampleAt(const std::string& id, const std::string& relativePath) { + Sample s; + s.id = id; + s.displayName = id + " display"; + s.relativePath = relativePath; + s.sampleRate = 48000; + s.channelCount = 2; + s.contentHash = "0123456789abcdef"; + return s; +} + +static ExportCandidate present(const std::string& id, const std::string& rel) { + return ExportCandidate{sampleAt(id, rel), SourceFileState::Present}; +} + +static ExportCandidate withState(const std::string& id, const std::string& rel, + SourceFileState state) { + return ExportCandidate{sampleAt(id, rel), state}; +} + +static ExportInputs bankOf(std::vector candidates) { + ExportInputs in; + in.bankDisplayName = "Drums"; + in.candidates = std::move(candidates); + std::vector ids; + for (const ExportCandidate& c : in.candidates) ids.push_back(c.sample.id); + in.slots.resetDense(ids); + return in; +} + +static bool hasExclusion(const ExportPlan& p, const std::string& id, ExclusionReason why) { + for (const ExcludedEntry& e : p.excluded) + if (e.sampleId == id && e.reason == why) return true; + return false; +} + +// --- the four classification inputs the plan names --------------------------- + +static void testZeroSamplesIsAReadyEmptyPlan() { + const ExportPlan p = planExport(bankOf({})); + CHECK(p.verdict == ExportVerdict::Ready); + CHECK(p.manifest.entries.empty()); + CHECK(p.sourceRelativePaths.empty()); + CHECK(p.excluded.empty()); + CHECK(p.manifest.bankDisplayName == "Drums"); + CHECK(p.manifest.slots.empty()); +} + +static void testOneSamplePresentShips() { + const ExportPlan p = planExport(bankOf({present("s1", "reasampler_bank/kick.wav")})); + CHECK(p.verdict == ExportVerdict::Ready); + CHECK(p.manifest.entries.size() == 1); + CHECK(p.excluded.empty()); + CHECK(p.manifest.entries[0].fileName == "kick.wav"); + CHECK(p.manifest.entries[0].sample.id == "s1"); + // The source spelling survives only on the side channel; the transport record + // names the payload by its bare package name. + CHECK(p.sourceRelativePaths.size() == 1); + CHECK(p.sourceRelativePaths[0] == "reasampler_bank/kick.wav"); + CHECK(p.manifest.entries[0].sample.relativePath == "kick.wav"); +} + +static void testMissingFileIsIncompleteNotRefused() { + const ExportPlan p = planExport(bankOf({ + present("s1", "reasampler_bank/kick.wav"), + withState("s2", "reasampler_bank/gone.wav", SourceFileState::Missing), + })); + CHECK(p.verdict == ExportVerdict::Incomplete); + CHECK(p.manifest.entries.size() == 1); + CHECK(p.manifest.entries[0].sample.id == "s1"); + CHECK(p.excluded.size() == 1); + CHECK(hasExclusion(p, "s2", ExclusionReason::FileMissing)); + CHECK(p.excluded[0].relativePath == "reasampler_bank/gone.wav"); +} + +static void testUnreadableFileStaysDistinctFromMissing() { + const ExportPlan p = planExport(bankOf({ + withState("s1", "reasampler_bank/locked.wav", SourceFileState::Unreadable), + withState("s2", "reasampler_bank/gone.wav", SourceFileState::Missing), + })); + CHECK(p.verdict == ExportVerdict::Incomplete); + CHECK(p.manifest.entries.empty()); + CHECK(p.excluded.size() == 2); + CHECK(hasExclusion(p, "s1", ExclusionReason::FileUnreadable)); + CHECK(hasExclusion(p, "s2", ExclusionReason::FileMissing)); + CHECK(!hasExclusion(p, "s1", ExclusionReason::FileMissing)); +} + +static void testUnrepresentableRecordRefusesWholeExport() { + // A traversing nested path — the one thing an index record can carry that + // BankModel::add does not itself refuse. + const ExportPlan traversal = + planExport(bankOf({present("s1", "reasampler_bank/kick.wav"), + present("s2", "reasampler_bank/../evil.wav")})); + CHECK(traversal.verdict == ExportVerdict::Refused); + CHECK(hasExclusion(traversal, "s2", ExclusionReason::RecordUnrepresentable)); + CHECK(traversal.manifest.entries.size() == 1); // still reports what WOULD ship + + const ExportPlan emptyId = planExport(bankOf({present("", "reasampler_bank/kick.wav")})); + CHECK(emptyId.verdict == ExportVerdict::Refused); + + const ExportPlan absolute = + planExport(bankOf({present("s1", "C:/elsewhere/kick.wav")})); + CHECK(absolute.verdict == ExportVerdict::Refused); + CHECK(hasExclusion(absolute, "s1", ExclusionReason::RecordUnrepresentable)); + + // Refused outranks Incomplete: a corrupt record is not something the + // "export the present N" confirm can proceed past. + const ExportPlan both = planExport(bankOf({ + withState("s1", "reasampler_bank/gone.wav", SourceFileState::Missing), + present("s2", "reasampler_bank/../evil.wav"), + })); + CHECK(both.verdict == ExportVerdict::Refused); +} + +// --- transport names ---------------------------------------------------------- + +static void testHostileNamesAreRepairedNotRelayed() { + // Every one of these is a name the codec refuses and a filesystem somewhere + // produces honestly. + const std::vector hostile = { + "reasampler_bank/ki:ck?.wav", "reasampler_bank/a|bd\"e*f.wav", + "reasampler_bank/CON.wav", "reasampler_bank/nul", + "reasampler_bank/trailing .wav ", "reasampler_bank/dots...", + // A literal ".." COMPONENT is not a name to repair — it is a traversing + // record, and the Refused test above owns it. + "reasampler_bank/.....", "reasampler_bank/.", + std::string("reasampler_bank/bad\xC3.wav"), // truncated UTF-8 sequence + std::string("reasampler_bank/") + std::string(400, 'x') + ".wav", + }; + std::vector candidates; + for (std::size_t i = 0; i < hostile.size(); ++i) + candidates.push_back(present("s" + std::to_string(i), hostile[i])); + + const ExportPlan p = planExport(bankOf(candidates)); + CHECK(p.verdict == ExportVerdict::Ready); + CHECK(p.manifest.entries.size() == hostile.size()); + for (const PackageEntry& e : p.manifest.entries) { + CHECK(isValidEntryName(e.fileName)); + CHECK(isValidNestedSamplePath(e.sample.relativePath)); + } +} + +static void testUniqueNameSurvivesLongExtensionUnderflow() { + // insertSuffix computes room = kMaxEntryNameBytes - suffix.size() - ext.size() in + // size_t; an extension long enough that even a two-digit "_10" suffix pushes the + // sum past the cap must not wrap that subtraction. Ten same-named entries force + // the tenth collision into double digits against a 253-byte extension (253 + 3 = + // 256, one over kMaxEntryNameBytes). + const std::string hostileName = "a." + std::string(252, 'x'); // 254 bytes, otherwise valid + std::vector candidates; + for (int i = 0; i < 10; ++i) + candidates.push_back(present("s" + std::to_string(i), "reasampler_bank/" + hostileName)); + + const ExportPlan p = planExport(bankOf(candidates)); + CHECK(p.verdict == ExportVerdict::Ready); + CHECK(p.manifest.entries.size() == 10); + for (const PackageEntry& e : p.manifest.entries) CHECK(isValidEntryName(e.fileName)); + for (std::size_t i = 0; i < p.manifest.entries.size(); ++i) + for (std::size_t j = i + 1; j < p.manifest.entries.size(); ++j) + CHECK(!sameEntryName(p.manifest.entries[i].fileName, + p.manifest.entries[j].fileName)); +} + +static void testCaseFoldedCollisionsAreDisambiguated() { + const ExportPlan p = planExport(bankOf({ + present("s1", "reasampler_bank/Kick.wav"), + present("s2", "reasampler_bank/kick.wav"), + present("s3", "reasampler_bank/KICK.wav"), + })); + CHECK(p.verdict == ExportVerdict::Ready); + CHECK(p.manifest.entries.size() == 3); + for (std::size_t i = 0; i < p.manifest.entries.size(); ++i) + for (std::size_t j = i + 1; j < p.manifest.entries.size(); ++j) + CHECK(!sameEntryName(p.manifest.entries[i].fileName, + p.manifest.entries[j].fileName)); + CHECK(p.manifest.entries[0].fileName == "Kick.wav"); + // The suffix goes before the extension, so the payload keeps its type. + CHECK(p.manifest.entries[1].fileName == "kick_2.wav"); +} + +static void testSanitizeNeverReturnsANameTheCodecRefuses() { + const std::vector raws = { + "", ".", "..", "...", " ", "com1", "LPT9.WAV", "a/b", "a\\b", "C:evil", + std::string("\x01\x02\x03"), std::string(300, 'y'), + std::string("caf\xC3\xA9.wav"), // well-formed UTF-8 must survive intact + }; + for (const std::string& raw : raws) CHECK(isValidEntryName(sanitizeEntryName(raw))); + CHECK(sanitizeEntryName("caf\xC3\xA9.wav") == "caf\xC3\xA9.wav"); + CHECK(sanitizeEntryName("kick.wav") == "kick.wav"); +} + +// --- what the plan hands the codec ------------------------------------------- + +static void testPlannedManifestSatisfiesTheCodec() { + ExportPlan p = planExport(bankOf({ + present("s1", "reasampler_bank/Kick.wav"), + present("s2", "reasampler_bank/kick.wav"), + present("s3", "reasampler_bank/CON.wav"), + present("s4", std::string("reasampler_bank/caf\xC3\xA9 mix.wav")), + })); + // byteLength/byteHash are the shell's to measure; stand them in so the encode + // path under test is the naming, not the digest. + for (PackageEntry& e : p.manifest.entries) { + e.byteLength = 44; + e.byteHash = "aaaaaaaabbbbbbbb"; + } + const std::optional json = serializeManifest(p.manifest); + CHECK(json.has_value()); + if (json) { + const std::optional back = deserializeManifest(*json); + CHECK(back.has_value()); + if (back) CHECK(*back == p.manifest); + } +} + +static void testSlotsFollowMembership() { + ExportInputs in = bankOf({ + present("s1", "reasampler_bank/a.wav"), + withState("s2", "reasampler_bank/gone.wav", SourceFileState::Missing), + present("s3", "reasampler_bank/c.wav"), + }); + const ExportPlan p = planExport(in); + CHECK(p.manifest.slots.slotOf("s2") == -1); // an excluded id keeps no display position + CHECK(p.manifest.slots.slotOf("s1") >= 0); + CHECK(p.manifest.slots.slotOf("s3") >= 0); + CHECK(p.manifest.slots.size() == 2); +} + +static void testPlanIsDeterministic() { + const ExportInputs in = bankOf({ + present("s1", "reasampler_bank/Kick.wav"), + present("s2", "reasampler_bank/kick.wav"), + withState("s3", "reasampler_bank/gone.wav", SourceFileState::Missing), + }); + const ExportPlan a = planExport(in); + const ExportPlan b = planExport(in); + CHECK(a.verdict == b.verdict); + CHECK(a.manifest == b.manifest); + CHECK(a.sourceRelativePaths == b.sourceRelativePaths); + CHECK(a.excluded.size() == b.excluded.size()); +} + +int main() { + testZeroSamplesIsAReadyEmptyPlan(); + testOneSamplePresentShips(); + testMissingFileIsIncompleteNotRefused(); + testUnreadableFileStaysDistinctFromMissing(); + testUnrepresentableRecordRefusesWholeExport(); + testHostileNamesAreRepairedNotRelayed(); + testUniqueNameSurvivesLongExtensionUnderflow(); + testCaseFoldedCollisionsAreDisambiguated(); + testSanitizeNeverReturnsANameTheCodecRefuses(); + testPlannedManifestSatisfiesTheCodec(); + testSlotsFollowMembership(); + testPlanIsDeterministic(); + + if (g_fail == 0) { + std::printf("export_plan_tests: all passed\n"); + return 0; + } + std::printf("export_plan_tests: %d failure(s)\n", g_fail); + return 1; +} diff --git a/tests/test_package_io.cpp b/tests/test_package_io.cpp index d6237f6..eaddba9 100644 --- a/tests/test_package_io.cpp +++ b/tests/test_package_io.cpp @@ -72,7 +72,9 @@ static void testPayloadCounterTracksMovesNotCopies() { } static void testStreamingRoundTripHoldsOnePayload() { - const std::string dest = "pkg_io_scratch.rsbank"; + std::error_code destEc; + const std::string dest = + (fs::temp_directory_path(destEc) / "pkg_io_scratch.rsbank").generic_string(); const std::vector header = patternBytes(16, 0xA0); const std::vector> entries = { patternBytes(1000, 1), patternBytes(500, 2), patternBytes(1, 3)};