Let a same-source W re-capture replace its save in place
Capturing the same source under an existing name used to be refused as a conflict. A hidden .cbd-save.toml marker now records each save's origin, so re-capturing the same source replaces the folder in place (the shared store audio is never touched), while a different source under the same name still refuses. Updates the crabidy-store D5 design note and quality gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
95ea1e44a0
commit
be2676080d
|
|
@ -234,14 +234,27 @@ A **save** takes a *source* (a live-queue snapshot or a library-node path) and a
|
|||
- **`W`** on a library node or the queue → `w` **plus** run the D4 capture per
|
||||
track; tomls carry `playable.store`. Works on both library nodes and the queue.
|
||||
|
||||
**Conflict = refuse.** If `/crabidy/<name>` already exists, do nothing and warn
|
||||
`name "<name>" already exists`; the user deletes the old folder and saves again.
|
||||
This replaces the old overwrite (bookmark tmp-swap) and merge-by-name (capture
|
||||
resume). The save is still built in a hidden `.tmp-<name>` sibling and swapped in
|
||||
atomically, so a crashed/failed run leaves **no** blocking partial folder and the
|
||||
name stays free to retry — while any audio already committed to the store
|
||||
persists and makes the retry fast via D4. Resumability thus moves from the folder
|
||||
to the store; the folder is all-or-nothing.
|
||||
**Conflict handling.** Each save records its origin in a hidden
|
||||
`.cbd-save.toml` marker at the save root (`source = "<captured node path>"`,
|
||||
`capture = true|false`). When `/crabidy/<name>` already exists:
|
||||
|
||||
- **`w` (link save)** always refuses — do nothing and warn `name "<name>"
|
||||
already exists`; the user deletes the old folder and saves again.
|
||||
- **`W` (capture)** refuses **unless it is a re-capture of the same source**:
|
||||
if the existing save's marker `source` equals the node being captured, the
|
||||
save **replaces** it; a different source still refuses. So pressing `W`
|
||||
again on the same item (or `W` on the queue again) refreshes the capture in
|
||||
place, while `W` naming a *different* item after an existing save is still
|
||||
protected.
|
||||
|
||||
The save is built in a hidden `.tmp-<name>` sibling and swapped in atomically;
|
||||
a permitted replace removes the old folder and renames the temp over it. A
|
||||
crashed/failed run leaves **no** blocking partial folder, and any audio already
|
||||
committed to the store persists and makes a retry fast via D4 (resumability
|
||||
lives in the store, not the folder). Replacing is cheap and safe because a
|
||||
save folder holds only tomls — the shared store audio is never rewritten or
|
||||
deleted (D7). A save that predates the marker (no `.cbd-save.toml`) is treated
|
||||
as a different source and refuses; delete it once to re-establish it.
|
||||
|
||||
`current` is exempt: the playback loop overwrites it on every queue change; the
|
||||
user cannot save over the reserved name `current`.
|
||||
|
|
@ -286,8 +299,9 @@ for this provider (folder/toml removal remains).
|
|||
- `/queues`, `/bookmarks`, `/captures` disappear from the root; one `/crabidy`
|
||||
child appears (title `crabidy`). Inside it, `w`-saves (links) and `W`-saves
|
||||
(store-backed) coexist, distinguished by the captured marker.
|
||||
- **Captured marker:** a captured row is prefixed with `|` as the *first
|
||||
character of the row* (before the selection padding), e.g. `|Bohemian…`.
|
||||
- **Captured marker:** a captured row carries a trailing `↓` at the *end of
|
||||
the row* — a status marker after the action-key brackets (outside them),
|
||||
e.g. `Bohemian… ↓`.
|
||||
- A **track** row is captured per `Track.is_captured` (D3): store-backed, or its
|
||||
`(provider, id)` is in the store index — visible even while browsing tidal.
|
||||
- A **node/child** row is captured iff all its tracks and child nodes are
|
||||
|
|
|
|||
|
|
@ -45,6 +45,11 @@ const SIDECAR_SUFFIX: &str = ".cbd-store.toml";
|
|||
/// Dot-prefixed, so library listings never show it.
|
||||
const STATE_FILE_NAME: &str = ".queue-state.toml";
|
||||
|
||||
/// Hidden per-save marker recording a save's origin, so a `W` re-capture of
|
||||
/// the same source can replace it in place (a different source refuses).
|
||||
/// Dot-prefixed, so library listings never show it.
|
||||
const SAVE_MARKER: &str = ".cbd-save.toml";
|
||||
|
||||
/// Whether a save writes bookmark links (`w`) or captures audio into the
|
||||
/// store (`W`).
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
|
|
@ -112,6 +117,16 @@ pub struct StoreSidecar {
|
|||
/// name). A bare file name, no separators.
|
||||
pub type StoreName = String;
|
||||
|
||||
/// The `.cbd-save.toml` marker recording where a save came from, so a `W`
|
||||
/// re-capture of the same source can replace it (D5).
|
||||
#[derive(Debug, Clone, Deserialize, Serialize)]
|
||||
struct SaveMarker {
|
||||
/// The library path of the captured/saved node.
|
||||
source: String,
|
||||
/// Whether the save was a capture (`W`) rather than a link save (`w`).
|
||||
capture: bool,
|
||||
}
|
||||
|
||||
/// In-memory index over the store's sidecars, derived by scanning
|
||||
/// `*.cbd-store.toml` at open and updated on every write. No separate
|
||||
/// persisted index exists — this *is* the "grep the sidecars" search, memoized
|
||||
|
|
@ -373,15 +388,45 @@ impl CrabidyStore {
|
|||
{
|
||||
let name =
|
||||
fsdy::validate_folder_name(name, &[CURRENT_NAME]).map_err(CaptureError::InvalidName)?;
|
||||
if tokio::fs::try_exists(self.tree_root.join(name)).await? {
|
||||
return Err(CaptureError::Conflict(name.to_string()));
|
||||
}
|
||||
// A genuine conflict rejects here; a same-source `W` re-capture is
|
||||
// allowed (it will replace, D5).
|
||||
self.resolve_collision(name, source_path, mode).await?;
|
||||
if mode == SaveMode::Capture {
|
||||
self.source_allows_download(client, source_path).await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Decides what a save to `name` does about an existing folder: `Ok(false)`
|
||||
/// = create fresh (no collision), `Ok(true)` = replace an existing save
|
||||
/// that is a same-source `W` re-capture, `Err(Conflict)` = a different
|
||||
/// existing save (refuse). `w` link saves never replace (D5).
|
||||
async fn resolve_collision(
|
||||
&self,
|
||||
name: &str,
|
||||
source_path: &str,
|
||||
mode: SaveMode,
|
||||
) -> Result<bool, CaptureError> {
|
||||
if !tokio::fs::try_exists(self.tree_root.join(name)).await? {
|
||||
return Ok(false);
|
||||
}
|
||||
if mode == SaveMode::Capture {
|
||||
if let Some(marker) = self.read_save_marker(name).await {
|
||||
if marker.source == source_path {
|
||||
return Ok(true);
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(CaptureError::Conflict(name.to_string()))
|
||||
}
|
||||
|
||||
/// Reads a save's `.cbd-save.toml` marker, if present and parseable.
|
||||
async fn read_save_marker(&self, name: &str) -> Option<SaveMarker> {
|
||||
let path = self.tree_root.join(name).join(SAVE_MARKER);
|
||||
let text = tokio::fs::read_to_string(path).await.ok()?;
|
||||
toml::from_str(&text).ok()
|
||||
}
|
||||
|
||||
/// Whether the source root (or a track's parent node) advertises download
|
||||
/// captures (`is_downloadable`). Mirrors the old capture-store gate.
|
||||
async fn source_allows_download<C>(
|
||||
|
|
@ -429,6 +474,9 @@ impl CrabidyStore {
|
|||
{
|
||||
let name =
|
||||
fsdy::validate_folder_name(name, &[CURRENT_NAME]).map_err(CaptureError::InvalidName)?;
|
||||
// Refuse a different existing save up front; a same-source `W`
|
||||
// re-capture is permitted and replaces the old folder (D5).
|
||||
let replace = self.resolve_collision(name, source_path, mode).await?;
|
||||
let tmp = self.tree_root.join(format!(".tmp-{name}"));
|
||||
if tokio::fs::try_exists(&tmp).await? {
|
||||
tokio::fs::remove_dir_all(&tmp).await?;
|
||||
|
|
@ -443,10 +491,16 @@ impl CrabidyStore {
|
|||
}
|
||||
let target = self.tree_root.join(name);
|
||||
if tokio::fs::try_exists(&target).await? {
|
||||
// A conflicting save appeared since validation; refuse rather than
|
||||
// clobber it.
|
||||
let _ = tokio::fs::remove_dir_all(&tmp).await;
|
||||
return Err(CaptureError::Conflict(name.to_string()));
|
||||
if replace {
|
||||
// Same-source re-capture: drop the old tomls and swap the
|
||||
// fresh capture in. The shared store audio is never touched.
|
||||
tokio::fs::remove_dir_all(&target).await?;
|
||||
} else {
|
||||
// A conflicting save appeared since validation; refuse rather
|
||||
// than clobber it.
|
||||
let _ = tokio::fs::remove_dir_all(&tmp).await;
|
||||
return Err(CaptureError::Conflict(name.to_string()));
|
||||
}
|
||||
}
|
||||
tokio::fs::rename(&tmp, &target).await?;
|
||||
Ok(())
|
||||
|
|
@ -503,6 +557,14 @@ impl CrabidyStore {
|
|||
}
|
||||
}
|
||||
}
|
||||
// Record the save's origin so a `W` re-capture of the same source can
|
||||
// replace it in place (D5). Dot-prefixed, so listings never show it.
|
||||
let marker = SaveMarker {
|
||||
source: source_path.to_string(),
|
||||
capture: mode == SaveMode::Capture,
|
||||
};
|
||||
let text = toml::to_string_pretty(&marker).map_err(StoreError::TomlWrite)?;
|
||||
tokio::fs::write(tmp.join(SAVE_MARKER), text).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
@ -1361,6 +1423,51 @@ mod tests {
|
|||
assert!(matches!(again, Err(CaptureError::Conflict(_))));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn re_capturing_the_same_source_replaces_but_another_refuses() {
|
||||
let (store, dir) = open_store().await;
|
||||
let src = write_bytes(dir.path(), "src.flac", b"AAAA").await;
|
||||
let mut mock = MockProvider::default();
|
||||
let t = track("/tidal/album/1/100", "100", "Song");
|
||||
mock.tracks.insert(t.path.clone(), (t.clone(), src.clone()));
|
||||
mock.nodes.insert(
|
||||
"/tidal/album/1".to_string(),
|
||||
node("/tidal/album/1", vec![t]),
|
||||
);
|
||||
let progress = Progress::silent("x", true);
|
||||
|
||||
store
|
||||
.save(&mock, "/tidal/album/1", "s", SaveMode::Capture, &progress)
|
||||
.await
|
||||
.expect("first capture");
|
||||
let store_entries = entries(store.store_dir()).len();
|
||||
|
||||
// Re-capturing the SAME source under the same name replaces in place.
|
||||
store
|
||||
.save(&mock, "/tidal/album/1", "s", SaveMode::Capture, &progress)
|
||||
.await
|
||||
.expect("same-source re-capture replaces");
|
||||
assert_eq!(
|
||||
entries(store.store_dir()).len(),
|
||||
store_entries,
|
||||
"the store is not duplicated by a replace"
|
||||
);
|
||||
assert!(store.tree_dir().join("s").is_dir());
|
||||
|
||||
// A DIFFERENT source under the same name is refused.
|
||||
let mut other = MockProvider::default();
|
||||
let t2 = track("/tidal/album/2/200", "200", "Other");
|
||||
other.tracks.insert(t2.path.clone(), (t2.clone(), src));
|
||||
other.nodes.insert(
|
||||
"/tidal/album/2".to_string(),
|
||||
node("/tidal/album/2", vec![t2]),
|
||||
);
|
||||
let refused = store
|
||||
.save(&other, "/tidal/album/2", "s", SaveMode::Capture, &progress)
|
||||
.await;
|
||||
assert!(matches!(refused, Err(CaptureError::Conflict(_))));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persist_current_and_load_round_trip() {
|
||||
let (store, _dir) = open_store().await;
|
||||
|
|
|
|||
|
|
@ -82,9 +82,9 @@ inline and in `plan/summary.md`.
|
|||
|
||||
- [x] The root library shows a single `crabidy` child; `queues`/`bookmarks`/
|
||||
`captures` no longer appear.
|
||||
- [x] Captured rows are prefixed with `|` as the first character of the row
|
||||
(before selection padding), driven by `is_captured`; captured tracks are
|
||||
marked even while browsing another provider (tidal/youtube). Node/child
|
||||
- [x] Captured rows carry a trailing `↓` at the end of the row (after the
|
||||
action-key brackets, outside them), driven by `is_captured`; captured tracks
|
||||
are marked even while browsing another provider (tidal/youtube). Node/child
|
||||
marking is shallow: a node is marked captured when all its tracks are
|
||||
captured and it has no child nodes (flat saves); a nested save's top folder
|
||||
is not marked — full-recursion marking is future work.
|
||||
|
|
|
|||
Loading…
Reference in New Issue