diff --git a/apps/locallibrary/GLOSSARY.md b/apps/locallibrary/GLOSSARY.md index 80f986fa1..e708e0aaa 100644 --- a/apps/locallibrary/GLOSSARY.md +++ b/apps/locallibrary/GLOSSARY.md @@ -127,6 +127,21 @@ _Avoid_: stick, device, volume, USB A saved copy of a drive's library in the store. _Avoid_: backup, version, commit +**Backup**: +What a change to a drive keeps on that drive so `undo` can put it back: the databases and the files +the change replaces or deletes. +_Avoid_: snapshot + +**Demo sample**: +One of the tracks rekordbox ships in its collection and puts back with every export. `clean` and +`refresh` remove them. +_Avoid_: bundled track, sample track + +**Unbacked file**: +A file a change deleted and left out of its backup, because rekordbox's next export puts it back. +Only demo samples' files are. Undoing the change brings the track back without the file. +_Avoid_: skipped file, excluded file + **History**: A play history list a player recorded. _Avoid_: session, play log diff --git a/apps/locallibrary/changes/loc-176-demo-backups.md b/apps/locallibrary/changes/loc-176-demo-backups.md new file mode 100644 index 000000000..b771e8eec --- /dev/null +++ b/apps/locallibrary/changes/loc-176-demo-backups.md @@ -0,0 +1,5 @@ +### Fixed + +- `clean` and `refresh` no longer copy rekordbox's demo samples into a backup, so those backups + stop piling up, and a drive too full for that copy can be cleaned. `undo` brings the tracks back + without their files and says which, until the next export. diff --git a/apps/locallibrary/docs/adr/0003-demo-samples-stay-out-of-backups.md b/apps/locallibrary/docs/adr/0003-demo-samples-stay-out-of-backups.md new file mode 100644 index 000000000..df66ab699 --- /dev/null +++ b/apps/locallibrary/docs/adr/0003-demo-samples-stay-out-of-backups.md @@ -0,0 +1,31 @@ +# Demo samples stay out of backups, so undoing a clean can't restore them + +Every change to a drive in place is backed up with every file it deletes, which is what makes +`undo` whole. `clean` is the exception. rekordbox puts its demo samples back with every export and +`refresh` removes them after each one, so backing them up kept another copy of the same audio on +the drive each time, and a drive too full for that copy couldn't be cleaned at all. A clean's +backup holds only the databases, and undoing it brings the tracks and their playlist entries back +without their files. The next export brings the files back, or another `clean` removes the tracks. + +`delete_unbacked` is its own edit, so the journal carries it and `edits replay` behaves as `clean` +does. Only plugins locallibrary ships can ask for it (`library:delete-unbacked`), and `edit.plan` +refuses it, since it deletes what no backup can bring back and a capability means little if the +sidecar goes around it. A plan that deletes anything else as well backs up everything it deletes. +Undo says which tracks come back without audio instead of refusing, because the refusal's `--force` +also overrides "the drive changed since", which is the check worth keeping. + +A journal holding the edit is version 5, so builds from before it refuse that drive's journal. + +## Considered options + +- A flag on `delete_tracks`: older builds could still read the journal, but `keep_files` and the + flag together mean nothing, and every place that deletes tracks would have to say it doesn't + want it. +- Deciding per operation, in the plugin's plan or its manifest: the journal wouldn't carry it, so + `edits replay` after an export would pile up the same audio. +- Refusing to undo a clean unless forced: a clean is undone so rarely that the refusal would mostly + stop people stepping back to a change before it. +- Letting sidecar clients ask for it, as they're ours today: that stops being true once plugins or + third parties speak RPC. +- Removing the tracks from the database again after undoing: undo would no longer put back exactly + what the backup holds. diff --git a/apps/locallibrary/docs/commands.md b/apps/locallibrary/docs/commands.md index 7d9741e53..d7940ff3f 100644 --- a/apps/locallibrary/docs/commands.md +++ b/apps/locallibrary/docs/commands.md @@ -113,7 +113,7 @@ its drives. | `locallibrary drive space []` / `drive tidy ` / `drive recordings []` | What's using a drive and when it fills; remove OS clutter; copy off set recordings, see [Space](drives.md#space) | | `locallibrary drive speed [--measure]` | How fast a drive writes, which plans estimate their time from, see [Speed](drives.md#speed) | | `locallibrary drive firmware [file.UPD] [--remove]` | Stage a player or mixer firmware update, see [Setting up and handing over](drives.md#setting-up-and-handing-over) | -| `locallibrary drive backups []` / `drive restore \| ` | The backups taken before every change to a library made in place; put any of them back (`--backup ID`; `--dry-run` shows what would change; refuses a backup whose audio has since been removed, unless `--force`). `--playlist P` brings back one playlist or folder from a snapshot or backup, see [Bringing back a playlist](editing.md#bringing-back-a-playlist) | +| `locallibrary drive backups []` / `drive restore \| ` | The backups taken before every change to a library made in place; put any of them back (`--backup ID`; `--dry-run` shows what would change; refuses a backup whose audio has since been removed, unless `--force`; a `clean`'s backup doesn't hold the demo samples, so those tracks come back without audio, and it says so). `--playlist P` brings back one playlist or folder from a snapshot or backup, see [Bringing back a playlist](editing.md#bringing-back-a-playlist) | | `locallibrary provision ` | Set up a new drive in one step, see [Setting up and handing over](drives.md#setting-up-and-handing-over) | | `locallibrary doctor [] [--hash-audio] [--by-sound] [--audio] [--analyze] [--only SECTION,...]` | Health report: duplicates with a suggested keeper (`--by-sound`: also the same recording in another file or encoding), missing or cut-short files, orphaned files, tracks in no playlist, database drift, files over 4 GB, how players read the drive, what plugins' checks find (`--audio`: audio files cut short or that don't decode; `--analyze`: tempos and keys an analysis of the audio disagrees with, and tracks too loud, clipped or far from the library's level); `--fix-orphans` deletes orphans after a backup; `--fix-tempo` doubles or halves the beat grids of tempos that are half or double | | `locallibrary analysis bench [ \| ] [--keys PATH] [--tempos DIR] [--audio DIR]` | Score rekordbox's stored keys and tempos, and the analysis `doctor --analyze` runs, against the GiantSteps datasets (`--keys`, `--tempos` or both), see [Checking the analysis](drives.md#checking-the-analysis) | @@ -161,7 +161,7 @@ its drives. | `locallibrary merge ` | Add the source's missing tracks and playlists to the target library (`--history`: and its history playlists) | | `locallibrary pack --playlist P` | A drive for a set holding only some playlists, with `--transcode` as copies its players play, see [Drives for a set](editing.md#drives-for-a-set) | | `locallibrary convert --to onelibrary\|device-library` | Build OneLibrary from `export.pdb` or the reverse; one the drive has gains what it lacks (`--replace` rebuilds it), see [Convert](editing.md#convert) | -| `locallibrary clean ` | Remove rekordbox's bundled demo tracks (which pile up with every export) with their files, analysis and playlist entries: the demos plugin's, see [Clean](editing.md#clean) | +| `locallibrary clean ` | Remove rekordbox's bundled demo tracks (which pile up with every export) with their files, analysis and playlist entries: the demos plugin's, see [Clean](editing.md#clean). `undo` brings back the tracks, not their files, which the next export does | | `locallibrary settings [] ...` | The settings profile, or a drive's settings; `settings export [FILE]` and `settings import FILE` take everything set here (the player's preferences and plugins' settings included) to another computer, see [Settings](settings.md) | **Plays and players** diff --git a/apps/locallibrary/docs/diagrams/changing-drives.md b/apps/locallibrary/docs/diagrams/changing-drives.md index a9d356997..0ce00fd88 100644 --- a/apps/locallibrary/docs/diagrams/changing-drives.md +++ b/apps/locallibrary/docs/diagrams/changing-drives.md @@ -87,7 +87,7 @@ flowchart LR subgraph before["Before the swap"] direction TB lock["Lock the drive,
check its databases"] --> build["Build and read back
in memory"] - build --> backup["Back up what changes, but
for repairs, firmware, recordings"] + build --> backup["Back up what changes, but for repairs,
firmware, recordings, demo samples' files"] backup --> marker["Mark it as being written"] marker --> stage["Stage files and databases,
flush, set times"] end @@ -232,9 +232,9 @@ How `undo` picks a backup and puts it back. ```mermaid flowchart TD - pick["undo picks the last change's
backup the drive still holds"] --> changed{"Drive changed since,
or audio gone?"} + pick["undo picks the last change's
backup the drive still holds"] --> changed{"Drive changed since, or audio
gone that no backup left out?"} changed -->|"yes, no --force"| refused["Refused"] - changed -->|"no, or --force"| plan["Plan: files, playlists, history lists
dropped, journal entries taken off"] + changed -->|"no, or --force"| plan["Plan: files, playlists, history lists
dropped, journal entries taken off,
tracks coming back without audio"] plan --> redo["Once confirmed, back up what it
replaces: putting that back is the redo"] redo --> write["Put the backup back through
the one write routine"] write --> journal["Put the backup's journal back"] diff --git a/apps/locallibrary/docs/editing.md b/apps/locallibrary/docs/editing.md index 62bd9f3e0..e7571b342 100644 --- a/apps/locallibrary/docs/editing.md +++ b/apps/locallibrary/docs/editing.md @@ -447,7 +447,7 @@ rekordbox's collection ships with demo samples ("rekordbox - House 1", "Techno 1 and they end up on drives, often once per export (`House1.wav`, `House1-1.wav`, `House1-2.wav`...). `clean` is the demos plugin's (see [Plugins](plugins.md)): it deletes every track on its list, from `export.pdb` and OneLibrary's `exportLibrary.db` (one only OneLibrary lists too), as an edit -(`delete_tracks`) along with its playlist and history entries, audio, analysis files and artwork, +(`delete_unbacked`) along with its playlist and history entries, audio, analysis files and artwork, plus artists, albums, genres, labels, keys and artwork rows nothing else uses. Files another track (in either database) still points at are kept, and a demo track sharing its audio file with another track stays, as deleting the file's rows would delete that track too. The list lives in @@ -455,6 +455,13 @@ another track stays, as deleting the file's rows would delete that track too. Th and `doctor`. The next export brings them back unless they're removed from the rekordbox collection too; `refresh` removes them first. +A clean's backup holds only the databases: every export brings the samples back, so keeping a copy +after each `refresh` would only fill the drive, and a drive too full to copy them can still be +cleaned. `locallibrary undo` can't restore those files. It brings the tracks and their playlist +entries back without them, says which, and they stay that way until rekordbox's next export or +another `clean`. `drive backups` counts the files a backup left out. `edits replay` leaves them +out too, but a clean an earlier version recorded is replayed as it was made, files backed up. + ## Convert ```bash diff --git a/apps/locallibrary/docs/how-it-works.md b/apps/locallibrary/docs/how-it-works.md index 15ee82c22..a9fd113dd 100644 --- a/apps/locallibrary/docs/how-it-works.md +++ b/apps/locallibrary/docs/how-it-works.md @@ -56,7 +56,7 @@ plugins/coverart/ the coverart plugin: front covers and track details all, asked to), each album looked up on MusicBrainz and its cover fetched from the Cover Art Archive, offered as candidates locallibrary plans the edits of plugins/demos/ the demos plugin: rekordbox's demo tracks on a drive, found by its check and - removed by `clean` as one `delete_tracks` edit + removed by `clean` as one `delete_unbacked` edit plugins/interop/ the interop plugin: exporters for Traktor, Serato, Engine DJ and rekordbox XML, writing a drive's library with loclib-interop on this computer, and an importer and a library source reading Traktor's collection.nml back @@ -297,7 +297,9 @@ apps/locallibrary/docs/rpc/ the sidecar's contract: schema.json and locallibrary packs: `locallibrary undo` puts back the backup of the last change the drive still holds, and run again, the one before (`drive restore --backup ID` puts back any). Backups include every file a change overwrote or deleted (artwork images, analysis files, settings files, removed tracks' - files), with their modification times. Each notes what its change left, and putting it back is + files), with their modification times; a `clean`'s hold only the databases, as rekordbox's next + export puts the demo samples back, so undoing one brings those tracks back without their files + and says so. Each notes what its change left, and putting it back is refused, unless `--force`, when the drive changed since: a rekordbox export, a sync or push, or history players recorded would be lost too. Only the drive a change was made on is backed up: copying it onto a library's other drives isn't, and an undo is copied onto them like any change. diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index cc25ba91d..0a943e6da 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -31,7 +31,9 @@ are: Traktor](other-software.md#from-traktor). - **demos**: `locallibrary clean LIBRARY` removes rekordbox's demo tracks and their files (see [Editing](editing.md#clean)), and its check lists them in `check` and `doctor`, with `clean` as - the fix. `refresh` runs it first. + the fix. `refresh` runs it first. Its `delete_unbacked` edit leaves the files out of the backup, + which only plugins locallibrary ship can ask for (`library:delete-unbacked`), and `edit.plan` + refuses. - **coverart**: finds front covers on MusicBrainz and the Cover Art Archive for `locallibrary art fetch`, and track details on MusicBrainz for `locallibrary track fetch` (see [Providers](#providers)). diff --git a/apps/locallibrary/docs/rpc/locallibrary.d.ts b/apps/locallibrary/docs/rpc/locallibrary.d.ts index 45e6e77c5..617121147 100644 --- a/apps/locallibrary/docs/rpc/locallibrary.d.ts +++ b/apps/locallibrary/docs/rpc/locallibrary.d.ts @@ -629,6 +629,16 @@ export type Edit = tracks: TrackSelector; keep_files?: boolean; } + /** + * Delete tracks as `delete_tracks` does, leaving their files out of the backup: for tracks + * rekordbox's next export puts back, whose copies would only fill the drive. Undoing brings + * the tracks back without their files, until that export. Only a plugin with the + * `library:delete-unbacked` capability can ask for it: `edit.plan` refuses it. + */ + | { + op: "delete_unbacked"; + tracks: TrackSelector; + } /** * Replace one copy of a recording with another: playlist and history entries (and My Tags) * move to `keep`, then `duplicate` is deleted unless `keep_duplicate`. @@ -3588,12 +3598,20 @@ export type BackupInfo = { * medium copy rekordbox didn't leave): putting it back removes them. */ added?: string[]; + /** + * Files its change deleted that it doesn't hold, as rekordbox's next export puts them back + * (see [`crate::edit::Edit::DeleteUnbacked`]): putting it back leaves their tracks without + * them until then. + */ + unbacked?: string[]; }; export type RestorePreview = { backup: BackupInfo; /** From the drive as it is to the drive restored. */ diff: PlaylistDiff; + /** Tracks that come back without the audio their change left out of its backup. */ + unbacked_audio: string[]; /** * Tracks in the backup whose audio is neither on the drive nor in the backup: restoring * refuses them unless forced. @@ -3656,6 +3674,11 @@ export type UndoPlan = { history_lists: string[]; /** Recorded edits taken off the journal: `edits replay` won't make them again. */ journal: JournalEntry[]; + /** + * Tracks that come back without the audio their change left out of its backup, which + * rekordbox's next export puts back. + */ + unbacked_audio: string[]; /** * Tracks the backup lists whose audio is neither on the drive nor in the backup: putting it * back is refused unless forced. @@ -3735,6 +3758,12 @@ export type UndoableBackup = { * medium copy rekordbox didn't leave): putting it back removes them. */ added?: string[]; + /** + * Files its change deleted that it doesn't hold, as rekordbox's next export puts them back + * (see [`crate::edit::Edit::DeleteUnbacked`]): putting it back leaves their tracks without + * them until then. + */ + unbacked?: string[]; /** Recorded edits putting it back takes off the journal. */ journal: number; /** It's the one `undo` puts back. */ @@ -4347,6 +4376,8 @@ export type EditPlan = { history_removed: number; /** Files to delete (removed tracks' audio, analysis and artwork), as drive keys. */ files: string[]; + /** Those of `files` the backup leaves out (see [`Edit::DeleteUnbacked`]). */ + unbacked?: string[]; bytes: number; /** `export.pdb` and/or `exportLibrary.db`. */ databases: string[]; @@ -4708,6 +4739,16 @@ export type EditOutput = tracks: TrackSelector; keep_files: boolean; } + /** + * Delete tracks as `delete_tracks` does, leaving their files out of the backup: for tracks + * rekordbox's next export puts back, whose copies would only fill the drive. Undoing brings + * the tracks back without their files, until that export. Only a plugin with the + * `library:delete-unbacked` capability can ask for it: `edit.plan` refuses it. + */ + | { + op: "delete_unbacked"; + tracks: TrackSelector; + } /** * Replace one copy of a recording with another: playlist and history entries (and My Tags) * move to `keep`, then `duplicate` is deleted unless `keep_duplicate`. diff --git a/apps/locallibrary/docs/rpc/schema.json b/apps/locallibrary/docs/rpc/schema.json index 55d8e0a11..2a46c8626 100644 --- a/apps/locallibrary/docs/rpc/schema.json +++ b/apps/locallibrary/docs/rpc/schema.json @@ -7567,6 +7567,23 @@ ], "description": "Delete tracks with their playlist and history entries, and (unless `keep_files`) their\naudio, analysis and artwork files when no other track uses them. A track goes with every\nrow of its audio file, in each database; a path may name one only the drive's other\ndatabase lists (OneLibrary, drifted from `export.pdb`), which is deleted there." }, + { + "type": "object", + "properties": { + "op": { + "type": "string", + "const": "delete_unbacked" + }, + "tracks": { + "$ref": "#/$defs/TrackSelector" + } + }, + "required": [ + "op", + "tracks" + ], + "description": "Delete tracks as `delete_tracks` does, leaving their files out of the backup: for tracks\nrekordbox's next export puts back, whose copies would only fill the drive. Undoing brings\nthe tracks back without their files, until that export. Only a plugin with the\n`library:delete-unbacked` capability can ask for it: `edit.plan` refuses it." + }, { "type": "object", "properties": { @@ -15972,6 +15989,13 @@ "type": "string" }, "description": "Files its change was to write in place that weren't there when it was taken (an image's\nmedium copy rekordbox didn't leave): putting it back removes them." + }, + "unbacked": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Files its change deleted that it doesn't hold, as rekordbox's next export puts them back\n(see [`crate::edit::Edit::DeleteUnbacked`]): putting it back leaves their tracks without\nthem until then." } }, "required": [ @@ -15993,6 +16017,13 @@ "$ref": "#/$defs/PlaylistDiff", "description": "From the drive as it is to the drive restored." }, + "unbacked_audio": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Tracks that come back without the audio their change left out of its backup." + }, "missing_audio": { "type": "array", "items": { @@ -16004,6 +16035,7 @@ "required": [ "backup", "diff", + "unbacked_audio", "missing_audio" ] }, @@ -16202,6 +16234,13 @@ }, "description": "Recorded edits taken off the journal: `edits replay` won't make them again." }, + "unbacked_audio": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Tracks that come back without the audio their change left out of its backup, which\nrekordbox's next export puts back." + }, "missing_audio": { "type": "array", "items": { @@ -16218,6 +16257,7 @@ "diff", "history_lists", "journal", + "unbacked_audio", "missing_audio" ], "description": "What putting a backup back would change." @@ -16422,6 +16462,13 @@ }, "description": "Files its change was to write in place that weren't there when it was taken (an image's\nmedium copy rekordbox didn't leave): putting it back removes them." }, + "unbacked": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Files its change deleted that it doesn't hold, as rekordbox's next export puts them back\n(see [`crate::edit::Edit::DeleteUnbacked`]): putting it back leaves their tracks without\nthem until then." + }, "journal": { "type": "integer", "format": "uint", @@ -18619,6 +18666,13 @@ }, "description": "Files to delete (removed tracks' audio, analysis and artwork), as drive keys." }, + "unbacked": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Those of `files` the backup leaves out (see [`Edit::DeleteUnbacked`])." + }, "bytes": { "type": "integer", "format": "uint64", @@ -19810,6 +19864,23 @@ ], "description": "Delete tracks with their playlist and history entries, and (unless `keep_files`) their\naudio, analysis and artwork files when no other track uses them. A track goes with every\nrow of its audio file, in each database; a path may name one only the drive's other\ndatabase lists (OneLibrary, drifted from `export.pdb`), which is deleted there." }, + { + "type": "object", + "properties": { + "op": { + "type": "string", + "const": "delete_unbacked" + }, + "tracks": { + "$ref": "#/$defs/TrackSelector" + } + }, + "required": [ + "op", + "tracks" + ], + "description": "Delete tracks as `delete_tracks` does, leaving their files out of the backup: for tracks\nrekordbox's next export puts back, whose copies would only fill the drive. Undoing brings\nthe tracks back without their files, until that export. Only a plugin with the\n`library:delete-unbacked` capability can ask for it: `edit.plan` refuses it." + }, { "type": "object", "properties": { @@ -32148,6 +32219,7 @@ "collection:read", "library:plan", "library:delete", + "library:delete-unbacked", "tags:plan", "cues:plan", "artwork:plan", diff --git a/apps/locallibrary/docs/sidecar.md b/apps/locallibrary/docs/sidecar.md index 472b577ed..d43e16938 100644 --- a/apps/locallibrary/docs/sidecar.md +++ b/apps/locallibrary/docs/sidecar.md @@ -220,7 +220,7 @@ if (!preview.noop && (await confirmDialog(preview.summary, preview.warnings))) { `remove_tracks`, `move_tracks`, `set_tracks`, `set_entries`, `sort_playlist`, `dedupe`, `plan_set`, `edit_tracks`, `delete_tracks`, `replace_track`, `set_path`, `delete_history`, `my_tag`, `cues`, `set_artwork`; see `edit::Edit`), applied as one change with one backup and one - journal entry. `my_tag`, `cues` and `set_artwork` are made first, in their order, then the rest; + journal entry. `delete_unbacked` is refused here: it's only for plugins with its capability. `my_tag`, `cues` and `set_artwork` are made first, in their order, then the rest; the plan checks the tracks they name are on the drive, and applying stops (`replay_stopped`) at one that can't be made: what was made is recorded, and `undo` takes it back (having made nothing, the error says it changed nothing and no backup is left). `set_artwork`'s `image` is diff --git a/apps/locallibrary/src/drive_cmd.rs b/apps/locallibrary/src/drive_cmd.rs index c04b45db5..f2309a095 100644 --- a/apps/locallibrary/src/drive_cmd.rs +++ b/apps/locallibrary/src/drive_cmd.rs @@ -919,11 +919,16 @@ fn backups(drive: Option, out: Output) -> Result { Ok(outcome.exit()) } -/// What a backup holds, in a few words: `12 file(s), 4 audio` (audio may be a track's only copy). +/// What a backup holds, in a few words: `12 file(s), 4 audio` (audio may be a track's only copy), +/// and how many files its change deleted that it can't put back. pub fn backup_files(b: &locallibrary::backup::BackupInfo) -> String { - match b.audio { + let held = match b.audio { 0 => format!("{} file(s)", b.files.len()), n => format!("{} file(s), {n} audio", b.files.len()), + }; + match b.unbacked.len() { + 0 => held, + n => format!("{held}, {n} deleted not kept"), } } diff --git a/apps/locallibrary/src/plugins.rs b/apps/locallibrary/src/plugins.rs index 527ebca3d..d92e9a116 100644 --- a/apps/locallibrary/src/plugins.rs +++ b/apps/locallibrary/src/plugins.rs @@ -513,6 +513,9 @@ fn means(c: &Capability) -> String { Capability::AccountStatus => "see whether you're signed in to your Atmosphere account", Capability::CollectionRead => "read rekordbox's collection on this computer", Capability::LibraryPlan => "plan changes to playlists and tracks, which you confirm", + Capability::LibraryDeleteUnbacked => { + "remove tracks rekordbox's next export puts back, without backing up their files" + } Capability::LibraryDelete => "plan removing tracks, playlists and their entries, which you confirm", Capability::TagsPlan => "plan My Tag changes, which you confirm", Capability::CuesPlan => "plan cue changes, which you confirm", diff --git a/apps/locallibrary/src/restore_cmd.rs b/apps/locallibrary/src/restore_cmd.rs index 7504f4bfd..985272c87 100644 --- a/apps/locallibrary/src/restore_cmd.rs +++ b/apps/locallibrary/src/restore_cmd.rs @@ -128,6 +128,12 @@ fn render_undo(p: &api::UndoPlan) { if !p.history_lists.is_empty() { warn(format!("history lists players recorded since are dropped: {}", p.history_lists.join(", "))); } + if let Some(example) = p.unbacked_audio.first() { + warn(format!( + "{} track(s) come back without their audio until rekordbox's next export (e.g. {example})", + p.unbacked_audio.len() + )); + } for m in p.missing_audio.iter().take(5) { warn(format!("no audio on the drive for {m}")); } diff --git a/apps/locallibrary/tests/it/cli.rs b/apps/locallibrary/tests/it/cli.rs index 76830882f..1200001d4 100644 --- a/apps/locallibrary/tests/it/cli.rs +++ b/apps/locallibrary/tests/it/cli.rs @@ -340,21 +340,118 @@ fn sync_many_updates_every_older_drive() { } } -#[test] -fn clean_shows_a_plan_then_removes_demo_samples() { - let env = Env::new(); - let a = demo_drive(&env.root, "A"); +/// rekordbox exports its demo sample to the drive at `root`, with `audio`'s size: returns its file. +fn export_demo_sample(root: &Path, audio: u64) -> PathBuf { let file = "/Contents/rekordbox/GROOVE CIRCUIT FACTORY SAMPLES/House1.wav"; - let mut ed = rekordbox_pdb::PdbEditor::from_file(a.join("PIONEER/rekordbox/export.pdb")).unwrap(); + let mut ed = rekordbox_pdb::PdbEditor::from_file(root.join("PIONEER/rekordbox/export.pdb")).unwrap(); ed.add_track(&rekordbox_pdb::NewTrack { artist: Some("rekordbox".into()), ..rekordbox_pdb::NewTrack::new("House 1", file) }) .unwrap(); - ed.save(a.join("PIONEER/rekordbox/export.pdb")).unwrap(); - let audio = a.join(file.trim_start_matches('/')); - std::fs::create_dir_all(audio.parent().unwrap()).unwrap(); - std::fs::write(&audio, "demo sample").unwrap(); + ed.save(root.join("PIONEER/rekordbox/export.pdb")).unwrap(); + let sample = root.join(file.trim_start_matches('/')); + std::fs::create_dir_all(sample.parent().unwrap()).unwrap(); + std::fs::File::create(&sample).unwrap().set_len(audio).unwrap(); + sample +} + +/// A size the drive at `root` has no room to copy (a sparse file takes none of it). +#[cfg(unix)] +fn too_big_to_copy(root: &Path) -> u64 { + 2 * locallibrary::fsutil::available_space(root).unwrap() + (1 << 30) +} + +/// `--json` output of `args`, unable to write a file past about 100 MB: a copy of a sparse sample +/// fails there instead of filling the disk the tests run on. +#[cfg(unix)] +fn json_with_writes_capped(env: &Env, args: &[&str]) -> Value { + let inner = env.locallibrary(); + let mut capped = Command::new("sh"); + capped.args(["-c", "ulimit -f 200000 && exec \"$@\"", "sh"]).arg(inner.get_program()).arg("--json").args(args); + for (key, value) in inner.get_envs() { + match value { + Some(value) => capped.env(key, value), + None => capped.env_remove(key), + }; + } + let out = capped.output().unwrap(); + serde_json::from_slice(&out.stdout).unwrap_or_else(|e| panic!("{args:?}: {e}: {out:?}")) +} + +/// Each refresh after an export removes the demo sample, and however many there have been, no +/// backup on the drive holds its audio. +#[test] +fn a_refresh_leaves_no_demo_audio_in_a_backup() { + let env = Env::new(); + let a = demo_drive(&env.root, "A"); + for _ in 0..2 { + let audio = export_demo_sample(&a, 11); + let v = env.json(&["--yes", "refresh", path(&a), "--skip", "pull"]); + assert_eq!(v["applied"], true, "{v}"); + assert!(!audio.exists()); + } + let backups = env.json(&["drive", "backups", path(&a)]); + let backups = backups.as_array().unwrap(); + assert!(backups.len() >= 2, "{backups:?}"); + assert!(backups.iter().all(|b| b["audio"] == 0), "{backups:?}"); + let wavs = crate::support::Env::files_in(&a).into_iter().filter(|(p, _)| p.extension().is_some_and(|e| e == "wav")); + assert_eq!(wavs.count(), 0); +} + +/// A sample the drive has no room to copy stops neither `clean` nor `refresh`, nor does a clean +/// an earlier version recorded (which backed files up), left on the journal to replay. +#[cfg(unix)] +#[test] +fn clean_and_refresh_run_on_a_drive_too_full_to_copy_the_demo_samples() { + let env = Env::new(); + let a = demo_drive(&env.root, "A"); + let audio = export_demo_sample(&a, too_big_to_copy(&a)); + let v = json_with_writes_capped(&env, &["--yes", "clean", path(&a)]); + assert_eq!(v["applied"], true, "{v}"); + assert!(!audio.exists()); + + let journal = a.join(".locallibrary/edits.json"); + let recorded = std::fs::read_to_string(&journal).unwrap(); + std::fs::write(&journal, recorded.replace("delete_unbacked", "delete_tracks")).unwrap(); + let audio = export_demo_sample(&a, too_big_to_copy(&a)); + let v = json_with_writes_capped(&env, &["--yes", "refresh", path(&a), "--skip", "pull"]); + assert_eq!(v["applied"], true, "{v}"); + assert!(!audio.exists()); +} + +/// `clean` says up front what undoing it can't do, `drive backups` and `undo --list` say it of +/// the backup, and `undo` says which tracks come back without their audio. It's still undone. +#[test] +fn undoing_a_clean_says_what_it_cant_restore() { + let env = Env::new(); + let a = demo_drive(&env.root, "A"); + let audio = export_demo_sample(&a, 11); + // What it prints, on either stream. + let says = |args: &[&str]| { + let out = env.locallibrary().args(args).output().unwrap(); + assert!(out.status.success(), "{args:?}: {out:?}"); + String::from_utf8_lossy(&[out.stdout, out.stderr].concat()).into_owned() + }; + let plan = says(&["clean", path(&a), "--dry-run"]); + assert!(plan.contains("delete 1 track(s) (rekordbox's next export brings their files back, undo can't)"), "{plan}"); + says(&["--yes", "clean", path(&a)]); + + let listed = says(&["drive", "backups", path(&a)]); + assert!(listed.contains("file(s), 1 deleted not kept"), "{listed}"); + let undoing = says(&["undo", path(&a), "--dry-run"]); + let said = "1 track(s) come back without their audio until rekordbox's next export (e.g. rekordbox - House 1)"; + assert!(undoing.contains(said), "{undoing}"); + says(&["--yes", "undo", path(&a)]); + assert_eq!(env.json(&["info", path(&a)])["summary"]["tracks"], 3); + assert!(!audio.exists()); +} + +#[test] +fn clean_shows_a_plan_then_removes_demo_samples() { + let env = Env::new(); + let a = demo_drive(&env.root, "A"); + let audio = export_demo_sample(&a, 11); // The demos plugin's operation. let v = env.json(&["clean", path(&a)]); diff --git a/apps/locallibrary/tests/it/rpc.rs b/apps/locallibrary/tests/it/rpc.rs index 66e0dae12..a193413b1 100644 --- a/apps/locallibrary/tests/it/rpc.rs +++ b/apps/locallibrary/tests/it/rpc.rs @@ -807,6 +807,14 @@ fn errors_say_why_and_what_would_resolve_them() { assert!(data(&v)["fields"]["path"].as_str().is_some_and(|p| p.starts_with("edits[0]")), "{v}"); assert!(v["error"]["message"].as_str().unwrap().contains("edits[0]"), "{v}"); + // An edit no backup can undo is only for plugins allowed to ask for it, planned or applied. + let unbacked = json!({ "op": "delete_unbacked", "tracks": { "ids": [1] } }); + for method in ["edit.plan", "edit.apply"] { + let v = s.call(1, method, json!({ "drive": a, "edits": [unbacked] })); + let message = v["error"]["message"].as_str().unwrap_or_default(); + assert!(message.contains("only for plugins with the library:delete-unbacked capability"), "{method}: {v}"); + } + // A library the store doesn't have: start it, by sending the same request with one more // parameter. let v = s.call(2, "pull", json!({ "drive": a, "library": "Main" })); diff --git a/crates/locallibrary/src/api/diff.rs b/crates/locallibrary/src/api/diff.rs index 825664739..8234c55fb 100644 --- a/crates/locallibrary/src/api/diff.rs +++ b/crates/locallibrary/src/api/diff.rs @@ -1,6 +1,5 @@ //! Playlist diffs between drives and snapshots, and what a restore would change. -use rustc_hash::FxHashSet; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -9,7 +8,6 @@ use crate::backup::{self, BackupInfo}; use crate::device::Device; use crate::diff::{self, PlaylistDiff}; use crate::doctor::Databases; -use crate::paths; use crate::store::{Snapshot, Store}; use crate::{Error, Result}; @@ -103,6 +101,8 @@ pub struct RestorePreview { pub backup: BackupInfo, /// From the drive as it is to the drive restored. pub diff: PlaylistDiff, + /// Tracks that come back without the audio their change left out of its backup. + pub unbacked_audio: Vec, /// Tracks in the backup whose audio is neither on the drive nor in the backup: restoring /// refuses them unless forced. pub missing_audio: Vec, @@ -117,19 +117,10 @@ pub fn restore_preview(req: &RestoreBackup) -> Result { None => backups.into_iter().next().ok_or_else(|| Error::NotFound("no backups on this drive".into()))?, }; let backed_up = Device::open(backup::dir(&d, &info.id))?; - let restored = backed_up.library()?; - let saved: FxHashSet = info.files.iter().map(|f| f.to_lowercase()).collect(); - let missing_audio = restored - .tracks - .iter() - .filter(|t| { - let key = paths::key(&t.file_path); - !saved.contains(&key.to_lowercase()) && !d.checked_path(&key).is_ok_and(|p| p.exists()) - }) - .map(|t| t.display_name()) - .collect(); + let missing_audio = backup::missing_audio(&d, &info)?; + let unbacked_audio = backup::unbacked_audio(&d, &info)?; let diff = to_backup(&d, &backed_up, &format!("backup {}", info.id)); - Ok(RestorePreview { backup: info, diff, missing_audio }) + Ok(RestorePreview { backup: info, diff, unbacked_audio, missing_audio }) } /// How `d` would differ with the backup `backed_up` put back (a database that can't be read diff --git a/crates/locallibrary/src/api/editing.rs b/crates/locallibrary/src/api/editing.rs index 26fb8ca93..0554917e4 100644 --- a/crates/locallibrary/src/api/editing.rs +++ b/crates/locallibrary/src/api/editing.rs @@ -38,6 +38,10 @@ pub struct EditRequest { /// What edits would do to a drive. pub fn edit_plan(req: &EditRequest, ctx: &Ctx<'_>) -> Result> { + // No backup can bring its files back, so it comes only from a plugin that may ask for it. + if req.edits.iter().any(|e| matches!(e, Edit::DeleteUnbacked { .. })) { + return Err(Error::invalid("delete_unbacked is only for plugins with the library:delete-unbacked capability")); + } let d = resolve_drive(&req.drive)?; planned_edit("edit", edit::plan(&d, &req.edits, &req.options, ctx)?, Touched::library(&d, true), ctx) } diff --git a/crates/locallibrary/src/api/undo.rs b/crates/locallibrary/src/api/undo.rs index 7b37cb5e4..723cb75ba 100644 --- a/crates/locallibrary/src/api/undo.rs +++ b/crates/locallibrary/src/api/undo.rs @@ -65,6 +65,9 @@ pub struct UndoPlan { pub history_lists: Vec, /// Recorded edits taken off the journal: `edits replay` won't make them again. pub journal: Vec, + /// Tracks that come back without the audio their change left out of its backup, which + /// rekordbox's next export puts back. + pub unbacked_audio: Vec, /// Tracks the backup lists whose audio is neither on the drive nor in the backup: putting it /// back is refused unless forced. pub missing_audio: Vec, @@ -109,6 +112,7 @@ pub fn undo_plan(req: &UndoRequest) -> Result> { let info = backup::choose(&d, req.backup.as_deref(), req.force)?; let dir = backup::dir(&d, &info.id); let missing_audio = backup::missing_audio(&d, &info)?; + let unbacked_audio = backup::unbacked_audio(&d, &info)?; if !missing_audio.is_empty() && !req.force { return Err(Error::conflict( Reason::MissingAudio, @@ -176,6 +180,13 @@ pub fn undo_plan(req: &UndoRequest) -> Result> { history_lists.join(", ") )); } + if !unbacked_audio.is_empty() { + warnings.push(format!( + "{} track(s) come back without their audio until rekordbox's next export (e.g. {}): it isn't in any backup", + unbacked_audio.len(), + unbacked_audio[0] + )); + } if !missing_audio.is_empty() { warnings.push(format!( "{} track(s) in the backup have no audio on the drive (e.g. {})", @@ -193,6 +204,7 @@ pub fn undo_plan(req: &UndoRequest) -> Result> { diff, history_lists, journal, + unbacked_audio, missing_audio, }; Planned::new("undo", plan, req.force) diff --git a/crates/locallibrary/src/backup.rs b/crates/locallibrary/src/backup.rs index 47740522d..a14d15d18 100644 --- a/crates/locallibrary/src/backup.rs +++ b/crates/locallibrary/src/backup.rs @@ -4,8 +4,11 @@ //! existing file the change overwrites or deletes (artwork images, analysis files, settings //! files, removed tracks' files), so restoring one undoes the change (`locallibrary undo`): files //! that were added stay on the drive but are no longer referenced, save those it wrote in place -//! of files the drive didn't have (an image's medium copy), which go ([`BackupInfo::added`]). A -//! backup also carries the drive's journal of edits (see [`crate::edit::journal`]) and the player +//! of files the drive didn't have (an image's medium copy), which go ([`BackupInfo::added`]). The +//! exception is the files of tracks rekordbox's next export puts back, as `clean` removes: a +//! backup leaves them out, or every refresh would keep another copy of the demo samples, and +//! putting it back brings those tracks back without them ([`BackupInfo::unbacked`]). A backup +//! also carries the drive's journal of edits (see [`crate::edit::journal`]) and the player //! name and colour it remembers (`Carried`), and restoring one puts those back as they were: //! `edits replay` then makes again exactly the edits the restored library had, also after an undo //! is redone. Restoring a backup an earlier version took, which carries neither, takes the edits @@ -26,7 +29,7 @@ use indexmap::IndexMap; use std::collections::BTreeSet; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use std::path::{Path, PathBuf}; use chrono::{DateTime, Utc}; @@ -77,6 +80,11 @@ pub struct BackupInfo { /// medium copy rekordbox didn't leave): putting it back removes them. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub added: Vec, + /// Files its change deleted that it doesn't hold, as rekordbox's next export puts them back + /// (see [`crate::edit::Edit::DeleteUnbacked`]): putting it back leaves their tracks without + /// them until then. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub unbacked: Vec, } fn backups_dir(device: &Device) -> PathBuf { @@ -151,12 +159,19 @@ fn put_back_carried(device: &mut Device, info: &BackupInfo, shown: Option/`, with their modification times: a track put back must /// not look changed to the next sync. Those of `extra` not there are noted as added (see -/// [`BackupInfo::added`]). `undoes`: the backup the change puts back, if it's an undo. +/// [`BackupInfo::added`]). `unbacked`: files the change deletes that the backup leaves out (see +/// [`BackupInfo::unbacked`]). `undoes`: the backup the change puts back, if it's an undo. /// /// Every older backup is kept: a write that fails before it changes anything takes its backup /// away again and must leave the others as they were. [`seal`] it once the change is made, and /// [`prune`] then. -pub(crate) fn create(device: &Device, reason: &str, extra: &[String], undoes: Option<&str>) -> Result { +pub(crate) fn create( + device: &Device, + reason: &str, + extra: &[String], + unbacked: &[String], + undoes: Option<&str>, +) -> Result { let seq = list(device)?.iter().map(|b| b.seq).max().unwrap_or(0) + 1; let created = Utc::now(); let mut id = created.format("%Y%m%dT%H%M%S%.3fZ").to_string(); @@ -195,6 +210,7 @@ pub(crate) fn create(device: &Device, reason: &str, extra: &[String], undoes: Op hashes, after: None, added, + unbacked: unbacked.to_vec(), }; write_info(&dir, &info)?; Ok(info) @@ -476,17 +492,32 @@ pub fn restore_with(device: &mut Device, id: Option<&str>, force: bool, ctx: &Ct Ok(info) } -/// Tracks a backup's database lists whose audio is neither on the drive nor in the backup -/// (restoring refuses them unless forced), by name. -pub fn missing_audio(device: &Device, info: &BackupInfo) -> Result> { +/// Tracks a backup's database lists whose audio is neither on the drive nor in the backup, by +/// name: those a change on this drive left out of its backup (`left_out`), or the others. +fn audio_gone(device: &Device, info: &BackupInfo, left_out: bool) -> Result> { let old = Device::open(dir(device, &info.id))?.library()?; - let saved: rustc_hash::FxHashSet = info.files.iter().map(|f| f.to_lowercase()).collect(); + let saved: FxHashSet = info.files.iter().map(|f| f.to_lowercase()).collect(); + let backups = list(device)?; + let unbacked: FxHashSet = backups.iter().flat_map(|b| &b.unbacked).map(|f| f.to_lowercase()).collect(); Ok(old .tracks .iter() .map(|t| (t, paths::key(&t.file_path))) .filter(|(_, k)| !saved.contains(&k.to_lowercase())) + .filter(|(_, k)| unbacked.contains(&k.to_lowercase()) == left_out) .filter(|(_, k)| !device.checked_path(k).is_ok_and(|p| p.exists())) .map(|(t, _)| t.display_name()) .collect()) } + +/// Tracks a backup's database lists whose audio is gone, and no backup left it out on purpose +/// (restoring refuses them unless forced), by name. +pub fn missing_audio(device: &Device, info: &BackupInfo) -> Result> { + audio_gone(device, info, false) +} + +/// Tracks a backup's database lists that come back without the audio a change left out of its +/// backup (see [`BackupInfo::unbacked`]), by name. Restoring lets them through, saying so. +pub fn unbacked_audio(device: &Device, info: &BackupInfo) -> Result> { + audio_gone(device, info, true) +} diff --git a/crates/locallibrary/src/collection_export.rs b/crates/locallibrary/src/collection_export.rs index a037c8052..e7477e69b 100644 --- a/crates/locallibrary/src/collection_export.rs +++ b/crates/locallibrary/src/collection_export.rs @@ -834,7 +834,7 @@ pub fn apply( let _lock = crate::commit::lock(target, None)?; if target.has_export() { let reason = format!("before exporting {}", plan.what()); - report.backup = Some(backup::create(target, &reason, &plan.written.backed_up, None)?.id); + report.backup = Some(backup::create(target, &reason, &plan.written.backed_up, &[], None)?.id); } target.begin(LineageOp::Export, format!("export {}", plan.what()), report.backup.clone())?; let identity = crate::player::stash(target, true)?; diff --git a/crates/locallibrary/src/commit.rs b/crates/locallibrary/src/commit.rs index 5c999f7ea..01d3b511c 100644 --- a/crates/locallibrary/src/commit.rs +++ b/crates/locallibrary/src/commit.rs @@ -183,10 +183,10 @@ pub(crate) struct Options<'a> { pub op: LineageOp, /// For the pending marker an interrupted write leaves. pub description: String, - /// Back up first (the databases and every file overwritten or removed), giving this reason. - /// `None` when there's nothing to undo (a repair writes back good copies of the same state; - /// copies of firmware or recordings would only fill the drive) or the caller has made one (a - /// replay backs up once for all its steps). + /// Back up first (the databases and every file overwritten or removed but `unbacked`), giving + /// this reason. `None` when there's nothing to undo (a repair writes back good copies of the + /// same state; copies of firmware or recordings would only fill the drive) or the caller has + /// made one (a replay backs up once for all its steps). pub backup: Option, /// The backup this write puts back, when it's an undo (see [`backup::BackupInfo::undoes`]). pub undoes: Option, @@ -200,6 +200,9 @@ pub(crate) struct Options<'a> { /// Files it writes in place of ones the databases name (artwork replaced in place), for the /// backup: kept like those overwritten, or noted as added when they aren't there. pub in_place: Vec, + /// Of the files it removes, those the backup leaves out (see + /// [`backup::BackupInfo::unbacked`]). + pub unbacked: Vec, } impl Options<'_> { @@ -213,6 +216,7 @@ impl Options<'_> { source: None, journal: None, in_place: Vec::new(), + unbacked: Vec::new(), } } @@ -458,8 +462,8 @@ fn prepare( if let Some(reason) = &opts.backup { let mut extra: Vec = files.iter().zip(&targets).filter(|(_, p)| p.is_file()).map(|(f, _)| f.key.clone()).collect(); - extra.extend(deletes.iter().chain(&opts.in_place).cloned()); - let info = backup::create(device, reason, &extra, opts.undoes.as_deref())?; + extra.extend(deletes.iter().filter(|k| !opts.unbacked.contains(k)).chain(&opts.in_place).cloned()); + let info = backup::create(device, reason, &extra, &opts.unbacked, opts.undoes.as_deref())?; undo.backup = Some(backup::dir(device, &info.id)); done.backup = Some(info.id); } diff --git a/crates/locallibrary/src/edit.rs b/crates/locallibrary/src/edit.rs index 213406a93..e43c134db 100644 --- a/crates/locallibrary/src/edit.rs +++ b/crates/locallibrary/src/edit.rs @@ -169,6 +169,13 @@ pub enum Edit { #[serde(default)] keep_files: bool, }, + /// Delete tracks as `delete_tracks` does, leaving their files out of the backup: for tracks + /// rekordbox's next export puts back, whose copies would only fill the drive. Undoing brings + /// the tracks back without their files, until that export. Only a plugin with the + /// `library:delete-unbacked` capability can ask for it: `edit.plan` refuses it. + DeleteUnbacked { + tracks: TrackSelector, + }, /// Replace one copy of a recording with another: playlist and history entries (and My Tags) /// move to `keep`, then `duplicate` is deleted unless `keep_duplicate`. ReplaceTrack { @@ -1926,7 +1933,7 @@ impl<'a> Resolver<'a> { canonical, ) } - Edit::DeleteTracks { tracks, keep_files } => { + Edit::DeleteTracks { tracks, .. } | Edit::DeleteUnbacked { tracks } => { let (elsewhere, here): (Vec, Vec) = tracks.paths.iter().cloned().partition(|p| self.elsewhere.contains(&paths::file_key(p))); // Replaying, the tracks already gone are gone. @@ -1939,11 +1946,18 @@ impl<'a> Resolver<'a> { let desc = format!( "delete {} track(s){}", ids.len() + elsewhere.len(), - if *keep_files { " (their files stay on the drive)" } else { "" } + match edit { + Edit::DeleteTracks { keep_files: true, .. } => " (their files stay on the drive)", + Edit::DeleteUnbacked { .. } => " (rekordbox's next export brings their files back, undo can't)", + _ => "", + } ); - let canonical = Edit::DeleteTracks { - tracks: TrackSelector::paths(Self::device_paths(lib, &ids).into_iter().chain(elsewhere.clone())), - keep_files: *keep_files, + let deleted = TrackSelector::paths(Self::device_paths(lib, &ids).into_iter().chain(elsewhere.clone())); + let canonical = match edit { + Edit::DeleteTracks { keep_files, .. } => { + Edit::DeleteTracks { tracks: deleted, keep_files: *keep_files } + } + _ => Edit::DeleteUnbacked { tracks: deleted }, }; let elsewhere = elsewhere.iter().map(|p| paths::file_key(p)).collect(); (vec![Op::DeleteTracks { tracks: Self::keys(lib, &ids), elsewhere }], desc, canonical) @@ -2253,6 +2267,9 @@ pub struct EditPlan { pub history_removed: usize, /// Files to delete (removed tracks' audio, analysis and artwork), as drive keys. pub files: Vec, + /// Those of `files` the backup leaves out (see [`Edit::DeleteUnbacked`]). + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub unbacked: Vec, pub bytes: u64, /// `export.pdb` and/or `exportLibrary.db`. pub databases: Vec, @@ -2405,6 +2422,7 @@ impl Edit { | Edit::SetTracks { tracks, .. } | Edit::EditTracks { tracks, .. } | Edit::DeleteTracks { tracks, .. } + | Edit::DeleteUnbacked { tracks } | Edit::SetArtwork { tracks, .. } => vec![tracks], Edit::RemoveTracks { tracks, .. } => tracks.iter().collect(), Edit::PlanSet { start, .. } => start.iter().collect(), @@ -2500,6 +2518,8 @@ fn plan_with( let mut recorded = Vec::new(); // Tracks deleted by an edit that keeps their files (the others' files go). let mut kept: FxHashSet = FxHashSet::default(); + // Tracks deleted by an edit leaving their files out of the backup. + let mut unbacked: FxHashSet = FxHashSet::default(); let mut index = None; for edit in &rest { ctx.check()?; @@ -2508,10 +2528,15 @@ fn plan_with( match resolved { Ok((edit_ops, desc, canonical)) => { warnings.extend(notes); - if let Edit::DeleteTracks { keep_files: true, .. } = edit { + let noted = match edit { + Edit::DeleteTracks { keep_files: true, .. } => Some(&mut kept), + Edit::DeleteUnbacked { .. } => Some(&mut unbacked), + _ => None, + }; + if let Some(noted) = noted { for op in &edit_ops { if let Op::DeleteTracks { tracks, elsewhere } = op { - kept.extend(tracks.iter().chain(elsewhere).cloned()); + noted.extend(tracks.iter().chain(elsewhere).cloned()); } } } @@ -2631,6 +2656,11 @@ fn plan_with( )); } } + // All or none: edits deleting other tracks too keep the backup whole, as before. + let unbacked: Vec = match doomed.iter().all(|k| unbacked.contains(k) || kept.contains(k)) { + true => files.keys().cloned().collect(), + false => Vec::new(), + }; let playlist_ops = ops.iter().any(|op| !matches!(op, Op::SetFields { .. } | Op::SetPath { .. } | Op::DeleteHistory { .. })); if playlist_ops { @@ -2654,11 +2684,13 @@ fn plan_with( let size = |key| std::fs::metadata(device.path(key)).map(|m| m.len()).unwrap_or(0); let db_bytes: u64 = paths::DATABASE_FILES.iter().map(|k| round(size(k))).sum(); let bytes: u64 = files.values().sum(); - // Backup of the databases and deleted files, plus each new database staged next to the old, - // and each image artwork edits stage, with the two sizes made from it in each database (no - // bigger than it; a staged waveform, which no database holds, is counted as one too). + // Backup of the databases and the deleted files it holds, plus each new database staged next + // to the old, and each image artwork edits stage, with the two sizes made from it in each + // database (no bigger than it; a staged waveform, which no database holds, is counted as one + // too). let staged = images.values().map(|b| round(b.len() as u64)).sum::() * (1 + 2 * databases.len() as u64); - let needed_bytes = backup::needed_bytes(device, files.keys().map(String::as_str)) + db_bytes + staged; + let backed_up = files.keys().filter(|k| !unbacked.contains(*k)).map(String::as_str); + let needed_bytes = backup::needed_bytes(device, backed_up) + db_bytes + staged; let free_space = fsutil::available_space(device.root()); let fits = free_space.is_none_or(|f| needed_bytes <= f); if !fits { @@ -2680,6 +2712,7 @@ fn plan_with( tracks_removed, history_removed, files: files.keys().cloned().collect(), + unbacked, bytes, databases, needed_bytes, @@ -2934,11 +2967,12 @@ struct Steps { /// A plan with steps (see [`Steps`]): one backup first (the databases, the analysis files cue /// edits may rewrite, the images artwork edits may replace and the files removed tracks leave), -/// so `restore` undoes all of it; then each step, planned as it's made; then the others, planned -/// again. The backup notes what the plan left, all of it made or not, as each step is made. +/// so `restore` undoes all of it, save the files the plan leaves out; then each step, planned as +/// it's made; then the others, planned again. The backup notes what the plan left, all of it made +/// or not, as each step is made. fn replay(plan: &EditPlan, steps: &Steps, device: &mut Device, ctx: &Ctx<'_>) -> Result { let library = device.library()?; - let mut extra = plan.files.clone(); + let mut extra: Vec = plan.files.iter().filter(|f| !plan.unbacked.contains(f)).cloned().collect(); for step in &steps.steps { // The analysis files the step rewrites (a cue edit's, and a pointed track's), and the // images an artwork edit replaces in place. @@ -2958,7 +2992,7 @@ fn replay(plan: &EditPlan, steps: &Steps, device: &mut Device, ctx: &Ctx<'_>) -> extra.dedup(); // A plan of new edits (a replace carrying cues) is an edit like any other to undo. let label = if steps.opts.no_journal { "before replaying edits" } else { "before editing" }; - let backup = backup::create(device, label, &extra, None)?; + let backup = backup::create(device, label, &extra, &plan.unbacked, None)?; // Sealed as each step is made, from the start: a replay cut off part way can be undone // without forcing it, as what it left is known. if let Err(e) = backup::seal(device, &backup.id) { @@ -3061,7 +3095,7 @@ fn replay_steps( let done = steps.steps.len(); let mut again = crate::edit::plan(device, &steps.rest, &steps.opts, ctx) .map_err(|e| stopped(e, done, device.swaps() != start))?; - if again.files.iter().any(|f| !extra.contains(f) && !written.contains(f)) { + if again.files.iter().any(|f| !extra.contains(f) && !written.contains(f) && !again.unbacked.contains(f)) { let changed = Error::conflict(Reason::Changed, "the drive changed while replaying"); return Err(stopped(changed, done, device.swaps() != start)); } @@ -3208,6 +3242,7 @@ fn apply_locked( if backup { opts = opts.backup("before editing"); } + opts.unbacked = plan.unbacked.clone(); opts.journal = plan.journal.then_some((&plan.recorded, &plan.changes)); let done = crate::commit::commit(device, changes, opts, ctx)?; for w in &warnings { @@ -3439,9 +3474,10 @@ pub mod journal { /// and its entries can hold edits older builds don't know (`set_entries`, /// `delete_history_lists`, `my_tag`, `cues`); 3 adds `set_artwork`, and track details 2 would /// drop (`album_artist`, `bpm`, `isrc`, `release_date`); 4 adds `set_analysis` and cue edits - /// setting several (`set_many`). Each is written only when an entry uses what it adds. A - /// journal of a later version is refused rather than misread or overwritten. - pub const VERSION: u32 = 4; + /// setting several (`set_many`); 5 adds `delete_unbacked`. Each is written only when an entry + /// uses what it adds. A journal of a later version is refused rather than misread or + /// overwritten. + pub const VERSION: u32 = 5; #[derive(Serialize)] struct Versioned<'a> { @@ -3488,11 +3524,13 @@ pub mod journal { staged(entries).into_iter().try_for_each(|(kind, keep)| kind.prune(device, &keep)) } - /// The version `entries` need: 4 once one holds an analysis edit or a cue edit setting - /// several, 3 once one holds an artwork edit or a track detail 3 added, else 2, so a build - /// that reads an older one keeps editing a drive whose edits use nothing newer. + /// The version `entries` need: 5 once one leaves files out of a backup, 4 once one holds an + /// analysis edit or a cue edit setting several, 3 once one holds an artwork edit or a track + /// detail 3 added, else 2, so a build that reads an older one keeps editing a drive whose + /// edits use nothing newer. fn version(entries: &[JournalEntry]) -> u32 { let needs = |e: &Edit| match e { + Edit::DeleteUnbacked { .. } => 5, Edit::SetAnalysis { .. } | Edit::Cues { change: crate::cue_edit::CueChange::SetMany(_), .. } => 4, Edit::SetArtwork { .. } => 3, Edit::EditTracks { fields, .. } diff --git a/crates/locallibrary/src/pack.rs b/crates/locallibrary/src/pack.rs index 408488657..bb3f9e55a 100644 --- a/crates/locallibrary/src/pack.rs +++ b/crates/locallibrary/src/pack.rs @@ -462,7 +462,7 @@ pub fn apply( let _lock = crate::commit::lock(target, None)?; if target.has_export() { let reason = format!("before packing from {}", plan.source); - report.backup = Some(backup::create(target, &reason, &plan.written.backed_up, None)?.id); + report.backup = Some(backup::create(target, &reason, &plan.written.backed_up, &[], None)?.id); } target.begin(LineageOp::Pack, format!("pack from {}", plan.source), report.backup.clone())?; let identity = crate::player::stash(target, plan.databases.plus.is_some())?; diff --git a/crates/locallibrary/src/plugin/operations.rs b/crates/locallibrary/src/plugin/operations.rs index dbd57bd6b..90e3003ea 100644 --- a/crates/locallibrary/src/plugin/operations.rs +++ b/crates/locallibrary/src/plugin/operations.rs @@ -156,6 +156,7 @@ impl Slot { Edit::Cues { .. } => Capability::CuesPlan, Edit::SetArtwork { .. } => Capability::ArtworkPlan, Edit::SetAnalysis { .. } => Capability::AnalysisPlan, + Edit::DeleteUnbacked { .. } => Capability::LibraryDeleteUnbacked, _ => Capability::LibraryPlan, }; if !self.manifest.capabilities.contains(&needs) { diff --git a/crates/locallibrary/tests/it/demos.rs b/crates/locallibrary/tests/it/demos.rs index a89623c37..a376f0a9a 100644 --- a/crates/locallibrary/tests/it/demos.rs +++ b/crates/locallibrary/tests/it/demos.rs @@ -6,12 +6,14 @@ use crate::common; use std::fs; use std::path::Path; -use locallibrary::api::{self, LibraryCache, LibraryRequest}; +use locallibrary::api::{self, DriveRef, LibraryCache, LibraryRequest, UndoRequest}; use locallibrary::device::Device; use locallibrary::edit::{self, Edit, EditOptions, EditPlan}; use locallibrary::onelibrary::OneLibrary; use locallibrary::progress::Ctx; -use locallibrary::{backup, paths}; +use locallibrary::query::TrackSelector; +use locallibrary::tags::TagChange; +use locallibrary::{Error, backup, paths}; use loclib_plugin::{Incoming, OpPlan, PROTOCOL_VERSION, Peer, RpcError, method}; use rekordbox_pdb::{Database, NewTrack, PdbEditor}; use serde_json::{Value, json}; @@ -98,9 +100,9 @@ fn export(root: &Path, artist: &str, title: &str, album: Option<&str>, file: &st /// The demo tracks go from both databases with their entries, files and the album and genre /// only they used (entries renumbered without gaps); the DJ's track by rekordbox, its artist and -/// its files stay; undoing brings them all back. +/// its files stay; undoing brings the tracks back, without the files the backup left out. #[test] -fn demo_tracks_go_from_both_databases_and_come_back_on_undo() { +fn demo_tracks_go_from_both_databases_and_come_back_on_undo_without_their_files() { let dir = tempfile::tempdir().unwrap(); let root = common::bigger_drive(dir.path(), "Main"); let album = Some("GROOVE CIRCUIT FACTORY SAMPLES"); @@ -166,11 +168,186 @@ fn demo_tracks_go_from_both_databases_and_come_back_on_undo() { for lib in [common::library(&root), common::plus_library(&root)] { assert_eq!(lib.tracks.len(), 16); for t in &lib.tracks { - assert!(d.path(&paths::key(&t.file_path)).is_file(), "{} restored", t.file_path); + let sample = t.file_path.starts_with(SAMPLES); + assert_eq!(d.path(&paths::key(&t.file_path)).is_file(), !sample, "{}", t.file_path); } } } +/// A drive holding one demo sample, in the demo export's playlists' company. +fn drive_with_sample(dir: &Path) -> std::path::PathBuf { + let root = common::bigger_drive(dir, "Main"); + export(&root, "rekordbox", "House 1", None, &format!("{SAMPLES}/House1.wav"), 1); + root +} + +/// Every file below `dir`. +fn files_below(dir: &Path) -> Vec { + let mut found = Vec::new(); + let mut left = vec![dir.to_path_buf()]; + while let Some(d) = left.pop() { + for e in fs::read_dir(d).unwrap().map(Result::unwrap) { + match e.file_type().unwrap().is_dir() { + true => left.push(e.path()), + false => found.push(e.path().display().to_string()), + } + } + } + found +} + +/// A clean's backup holds the databases: it names the files it left out, and none of them is +/// anywhere in it. The plan says what undo can't do before anything is deleted. +#[test] +fn a_clean_backs_up_only_the_databases() { + let dir = tempfile::tempdir().unwrap(); + let root = drive_with_sample(dir.path()); + let (plan, report) = clean(&root); + assert_eq!(plan.files.len(), 3, "audio, .DAT and .EXT: {:?}", plan.files); + assert_eq!(plan.unbacked, plan.files); + assert_eq!(plan.changes, ["delete 1 track(s) (rekordbox's next export brings their files back, undo can't)"]); + + let d = Device::open_export(&root).unwrap(); + let b = backup::list(&d).unwrap().remove(0); + assert_eq!(Some(&b.id), report.backup.as_ref()); + assert_eq!(b.audio, 0); + assert!(b.files.iter().all(|f| paths::DATABASE_FILES.contains(&f.as_str())), "{:?}", b.files); + assert_eq!(b.unbacked, plan.files); + let held = files_below(&backup::dir(&d, &b.id)); + assert!(!held.iter().any(|f| f.contains("GROOVE") || f.contains("P0AA")), "{held:?}"); +} + +/// A sample too big for the drive to copy doesn't stop a clean: its plan counts only the +/// databases. The same tracks deleted the usual way are backed up, so that plan doesn't fit. +#[test] +fn a_clean_runs_on_a_drive_too_full_to_copy_the_samples() { + let dir = tempfile::tempdir().unwrap(); + let root = drive_with_sample(dir.path()); + let file = format!("{SAMPLES}/House1.wav"); + let sample = paths::to_fs(&root, &paths::key(&file)); + let free = locallibrary::fsutil::available_space(&root).unwrap(); + let too_big = 2 * free + (1 << 30); + // NTFS allocates a file grown with `set_len` unless it's marked sparse first. + #[cfg(windows)] + assert!(std::process::Command::new("fsutil").args(["sparse", "setflag"]).arg(&sample).status().unwrap().success()); + fs::File::options().write(true).open(&sample).unwrap().set_len(too_big).unwrap(); + + let plan = clean_plan(&root); + assert!(plan.fits && plan.needed_bytes < too_big, "needs {} of {too_big}", plan.needed_bytes); + + let mut d = Device::open_export(&root).unwrap(); + let plain = Edit::DeleteTracks { tracks: TrackSelector::paths([file]), keep_files: false }; + let usual = edit::plan(&d, &[plain], &EditOptions::default(), &Ctx::silent()).unwrap(); + assert!(!usual.fits && usual.needed_bytes > too_big, "needs {}", usual.needed_bytes); + let refused = edit::apply(&usual, &mut d, &Ctx::silent()).unwrap_err(); + assert!(matches!(refused, Error::NoSpace { .. }), "{refused}"); + + // Small again: a copy of the sparse sample, were one to creep back, would fill the real disk. + fs::File::options().write(true).open(&sample).unwrap().set_len(11).unwrap(); + edit::apply(&plan, &mut d, &Ctx::silent()).unwrap(); + assert!(!sample.exists()); +} + +/// Undoing a clean needs no force: the plan names the tracks that come back without their audio, +/// and stepping back past it to an older backup does too. Nothing brings the files back, and a +/// clean takes the tracks away again. +#[test] +fn undoing_a_clean_says_what_it_cant_restore() { + let dir = tempfile::tempdir().unwrap(); + let root = drive_with_sample(dir.path()); + common::apply(&root, &[Edit::CreatePlaylist { path: "Before".into(), folder: false }]); + let (plan, _) = clean(&root); + let name = "rekordbox - House 1"; + let undo = || UndoRequest { drive: Some(common::path(&root)), backup: None, force: false }; + + let planned = api::undo_plan(&undo()).unwrap(); + assert_eq!(planned.plan.unbacked_audio, [name]); + assert!(planned.plan.missing_audio.is_empty(), "{:?}", planned.plan.missing_audio); + let warned = format!( + "1 track(s) come back without their audio until rekordbox's next export (e.g. {name}): it isn't in any backup" + ); + assert_eq!(planned.warnings, [warned]); + let backup = Some(planned.plan.backup.id.clone()); + let preview = + api::restore_preview(&api::RestoreBackup { drive: common::path(&root), backup, force: false }).unwrap(); + assert_eq!((preview.unbacked_audio.as_slice(), preview.missing_audio.len()), ([name.to_string()].as_slice(), 0)); + planned.apply(None, &Ctx::silent()).unwrap(); + assert!(plan.files.iter().all(|f| !paths::to_fs(&root, f).exists()), "{:?}", plan.files); + + // The backup before the clean lists the track too. + let older = api::undo_plan(&undo()).unwrap(); + assert_eq!( + (older.plan.unbacked_audio.as_slice(), older.plan.missing_audio.len()), + ([name.to_string()].as_slice(), 0) + ); + older.apply(None, &Ctx::silent()).unwrap(); + assert!(common::library(&root).find_playlist("Before").is_none()); + + let again = clean_plan(&root); + assert_eq!((again.tracks_removed.len(), again.files.len()), (1, 0), "{:?}", again.files); + edit::apply(&again, &mut Device::open_export(&root).unwrap(), &Ctx::silent()).unwrap(); + assert!(!common::library(&root).tracks.iter().any(|t| t.file_path.starts_with(SAMPLES))); +} + +/// `edit.plan` and `edit.apply` take no `delete_unbacked`, whatever the drive: nothing changes. +#[test] +fn edit_plan_refuses_delete_unbacked() { + let dir = tempfile::tempdir().unwrap(); + let root = drive_with_sample(dir.path()); + let file = format!("{SAMPLES}/House1.wav"); + let tracks = TrackSelector::paths([file.clone()]); + let req = api::EditRequest { + drive: common::path(&root), + edits: vec![Edit::DeleteUnbacked { tracks }], + options: EditOptions::default(), + }; + let before = common::Before::take(&root); + let said = "delete_unbacked is only for plugins with the library:delete-unbacked capability"; + let planned = api::edit_plan(&req, &Ctx::silent()).map(drop).unwrap_err(); + assert!(planned.to_string().contains(said), "{planned}"); + let applied = api::edit_apply(&req, &Ctx::silent()).map(drop).unwrap_err(); + assert!(applied.to_string().contains(said), "{applied}"); + before.untouched(&root).unwrap(); + assert!(paths::to_fs(&root, &paths::key(&file)).exists()); +} + +/// A clean is recorded as it was made, so replaying it after an export leaves the audio out of +/// that backup too, also when a My Tag edit makes the plan one of steps. +#[test] +fn replaying_a_clean_leaves_the_audio_out_of_its_backup_too() { + let dir = tempfile::tempdir().unwrap(); + let root = drive_with_sample(dir.path()); + let file = format!("{SAMPLES}/House1.wav"); + let (_, report) = clean(&root); + let d = Device::open_export(&root).unwrap(); + let recorded = edit::journal::list(&d).unwrap(); + let one = recorded.iter().find(|e| e.backup == report.backup).unwrap(); + assert!(matches!(one.edits.as_slice(), [Edit::DeleteUnbacked { .. }]), "{:?}", one.edits); + let stored: Value = serde_json::from_slice(&fs::read(d.state_dir().join("edits.json")).unwrap()).unwrap(); + assert_eq!(stored["version"], 5); + + export(&root, "rekordbox", "House 1", None, &file, 1); + let replay = api::edits_replay_apply(&DriveRef { drive: common::path(&root) }, &Ctx::silent()).unwrap(); + assert!(replay.backup.is_some()); + let d = Device::open_export(&root).unwrap(); + let b = backup::list(&d).unwrap().remove(0); + assert_eq!((b.id.as_str(), b.audio, b.unbacked.len()), (replay.backup.as_deref().unwrap(), 0, 3)); + assert!(!paths::to_fs(&root, &paths::key(&file)).exists()); + + export(&root, "rekordbox", "House 1", None, &file, 1); + common::add_onelibrary(&root); + let keeper = d.library().unwrap().tracks[0].file_path.clone(); + let tag = Edit::MyTag(TagChange::Add { tag: "Genre/Deep".into(), tracks: vec![keeper] }); + let delete = Edit::DeleteUnbacked { tracks: TrackSelector::paths([file.clone()]) }; + let mut d = Device::open_export(&root).unwrap(); + let plan = edit::plan(&d, &[tag, delete], &EditOptions::default(), &Ctx::silent()).unwrap(); + assert_eq!(plan.unbacked.len(), 3, "{:?}", plan.unbacked); + let report = edit::apply(&plan, &mut d, &Ctx::silent()).unwrap(); + let b = backup::list(&d).unwrap().remove(0); + assert_eq!((Some(&b.id), b.audio, &b.unbacked), (report.backup.as_ref(), 0, &plan.unbacked)); + assert!(!paths::to_fs(&root, &paths::key(&file)).exists()); +} + /// A demo track only OneLibrary lists (rekordbox exported it there alone) goes from it, with its /// file; export.pdb is left as it was. #[test] @@ -188,7 +365,7 @@ fn a_demo_track_only_onelibrary_lists_goes_too() { let (plan, _) = clean(&root); assert_eq!(plan.tracks_removed.iter().map(|t| t.file_path.as_str()).collect::>(), [file.as_str()]); - assert_eq!(plan.changes, ["delete 1 track(s)"]); + assert_eq!(plan.changes, ["delete 1 track(s) (rekordbox's next export brings their files back, undo can't)"]); assert!(plan.files.iter().any(|f| *f == paths::key(&file)), "{:?}", plan.files); assert!(plan.onelibrary_changes); assert_eq!(common::plus_library(&root).tracks.len(), 12); diff --git a/crates/locallibrary/tests/it/plugins/hooks.rs b/crates/locallibrary/tests/it/plugins/hooks.rs index b926a84ad..d46af1ff0 100644 --- a/crates/locallibrary/tests/it/plugins/hooks.rs +++ b/crates/locallibrary/tests/it/plugins/hooks.rs @@ -52,6 +52,11 @@ fn an_operations_edits_need_their_capabilities() { // However it gives the image (`blobs.rs` has what a plugin with it may give). let e = given(art(json!({ "path": "/tmp/cover.jpg" }))).unwrap_err(); assert_eq!(e, "the Echo plugin failed: its edit 0 needs the artwork:plan capability, which it doesn't have"); + let unbacked = json!({ "op": "delete_unbacked", "tracks": { "ids": [1] } }); + assert_eq!( + given(unbacked).unwrap_err(), + "the Echo plugin failed: its edit 0 needs the library:delete-unbacked capability, which it doesn't have" + ); let e = given(json!({ "op": "explode" })).unwrap_err(); assert!(e.starts_with("the Echo plugin failed: its edit 0 isn't one: "), "{e}"); f.plan(json!({ "name": "Doomed" })).unwrap().apply(None, &Ctx::silent()).unwrap(); diff --git a/crates/locallibrary/tests/it/props_demos.rs b/crates/locallibrary/tests/it/props_demos.rs index aa22cb72d..345312e64 100644 --- a/crates/locallibrary/tests/it/props_demos.rs +++ b/crates/locallibrary/tests/it/props_demos.rs @@ -1,7 +1,8 @@ //! The demos plugin's `clean` on drawn drives, in one database or both: exactly the demo tracks //! go (however their names are spaced or cased, OneLibrary's own too), with their entries and //! their files; a demo track sharing its file with another track stays, and so does every other -//! track, entry and file; cleaning again does nothing, and undoing gives the drive back. +//! track, entry and file; cleaning again does nothing, and undoing gives the drive back but for +//! those files, which no backup holds. use crate::{common, demos, drawn}; @@ -10,9 +11,12 @@ use std::fs; use hegel::generators as gs; use hegel::{HealthCheck, TestCase}; +use locallibrary::api; use locallibrary::device::Device; +use locallibrary::edit::{Edit, EditOptions}; use locallibrary::progress::Ctx; -use locallibrary::{Library, backup, edit, paths, pdb}; +use locallibrary::query::TrackSelector; +use locallibrary::{Library, backup, edit, fsutil, paths, pdb}; use rekordbox_pdb::{NewTrack, PdbEditor}; /// Each playlist's entries as file keys, by path. @@ -159,10 +163,70 @@ fn cleaning_removes_exactly_the_demo_tracks(tc: TestCase) { assert_eq!(files(&one), files(&after), "OneLibrary loses the same tracks, and its own"); } - // Again: nothing to clean. Undone: the drive as it was. + // Again: nothing to clean. Undone: the drive as it was, but for the files no backup holds. assert!(demos::clean_plan(&root).is_noop()); if let Some(id) = report.backup { - backup::restore(&mut d, Some(&id), false).unwrap(); - assert!(common::changed_files(&files_before, &common::drive_files(&root)).is_empty()); + // Undoing says which tracks come back without their audio, and needs no force for them. + let backup = Some(id.clone()); + let drive = common::path(&root); + let undoing = api::UndoRequest { drive: Some(drive.clone()), backup: backup.clone(), force: false }; + let undo = api::undo_plan(&undoing).unwrap(); + let preview = api::restore_preview(&api::RestoreBackup { drive, backup, force: false }).unwrap(); + assert_eq!(undo.plan.unbacked_audio.len(), sample_ids.len()); + assert_eq!(undo.warnings.iter().any(|w| w.contains("without their audio")), !sample_ids.is_empty()); + assert_eq!(preview.unbacked_audio, undo.plan.unbacked_audio); + assert!(undo.plan.missing_audio.is_empty() && preview.missing_audio.is_empty()); + let info = backup::restore(&mut d, Some(&id), false).unwrap(); + assert_eq!(info.audio, 0); + assert_eq!(info.unbacked, plan.files); + let changed = common::changed_files(&files_before, &common::drive_files(&root)); + assert_eq!(changed.iter().map(|f| f.to_lowercase()).collect::>(), planned); + } +} + +/// Edits deleting tracks, some leaving their files out of the backup: the backup leaves those +/// files out only when nothing else is deleted with them, the space a plan asks for counts just +/// what it holds, and what it left out it says. +#[hegel::test(test_cases = 6, suppress_health_check = [HealthCheck::TooSlow])] +fn a_backup_leaves_out_only_what_nothing_else_deleted_needs(tc: TestCase) { + let dir = tempfile::tempdir().unwrap(); + let (root, lib) = drawn::drive(&tc, dir.path(), "A"); + let files: Vec = + lib.tracks.iter().map(|t| t.file_path.clone()).collect::>().into_iter().collect(); + let plain = tc.draw(gs::integers::().max_value(2)); + let unbacked = tc.draw(gs::integers::().min_value(1).max_value(3)); + let keep_files = tc.draw(gs::booleans()); + let (others, chosen) = files[..plain + unbacked].split_at(plain); + let delete = Edit::DeleteUnbacked { tracks: TrackSelector::paths(chosen.to_vec()) }; + // Only a plugin can ask for it: the sidecar's way in refuses it, whatever else comes with it. + let drive = common::path(&root); + let request = api::EditRequest { drive, edits: vec![delete.clone()], options: EditOptions::default() }; + assert!(api::edit_plan(&request, &Ctx::silent()).is_err()); + let other = Edit::DeleteTracks { tracks: TrackSelector::paths(others.to_vec()), keep_files }; + let edits = match (plain, tc.draw(gs::booleans())) { + (0, _) => vec![delete], + (_, true) => vec![delete, other], + (_, false) => vec![other, delete], + }; + + let mut d = Device::open_export(&root).unwrap(); + let plan = edit::plan(&d, &edits, &EditOptions::default(), &Ctx::silent()).unwrap(); + assert!(!plan.files.is_empty()); + let left_out = plain == 0 || keep_files; + assert_eq!(plan.unbacked, if left_out { plan.files.clone() } else { Vec::new() }); + + let cluster = fsutil::cluster_size(&root); + let size = |key: &str| fs::metadata(paths::to_fs(&root, key)).map_or(0, |m| m.len().div_ceil(cluster) * cluster); + let databases: u64 = paths::DATABASE_FILES.iter().map(|k| size(k)).sum(); + let backed_up: u64 = plan.files.iter().filter(|_| !left_out).map(|k| size(k)).sum(); + assert_eq!(plan.needed_bytes, backed_up + 2 * databases); + + let report = edit::apply(&plan, &mut d, &Ctx::silent()).unwrap(); + let b = backup::list(&d).unwrap().remove(0); + assert_eq!(Some(&b.id), report.backup.as_ref()); + assert_eq!(b.unbacked, plan.unbacked); + for f in &plan.files { + assert_eq!(b.files.contains(f), !left_out, "{f}"); + assert_eq!(paths::to_fs(&backup::dir(&d, &b.id), f).exists(), !left_out, "{f}"); } } diff --git a/crates/loclib-plugin/README.md b/crates/loclib-plugin/README.md index e6f7c658b..cf172c313 100644 --- a/crates/loclib-plugin/README.md +++ b/crates/loclib-plugin/README.md @@ -144,7 +144,8 @@ assert!(manifest.validate(false).is_err()); - `capabilities`: `library:read`, `analysis:read`, `artwork:read`, `drive:info`, `health:read`, `store:read`, `settings:read`, `plays:read`, `sets:read`, `account:status`, - `collection:read`, `library:plan`, `library:delete`, `tags:plan`, `cues:plan`, + `collection:read`, `library:plan`, `library:delete`, `library:delete-unbacked` (plugins + locallibrary ships only), `tags:plan`, `cues:plan`, `artwork:plan`, `analysis:plan`, `events:drives`, `events:jobs`, `events:plans`, `fs:read`, and `net:`, or `net:*.` for any host below the domain: what `net/fetch` may reach (shown to the user; a native plugin can't be held to it otherwise). Any other is refused, and each is granted once. diff --git a/crates/loclib-plugin/src/manifest.rs b/crates/loclib-plugin/src/manifest.rs index e32009959..6aa3e57e4 100644 --- a/crates/loclib-plugin/src/manifest.rs +++ b/crates/loclib-plugin/src/manifest.rs @@ -234,6 +234,10 @@ pub enum Capability { CollectionRead, LibraryPlan, LibraryDelete, + /// Delete tracks whose files the backup leaves out (`delete_unbacked`), for tracks rekordbox's + /// next export puts back: undoing can't bring those files back, so only plugins locallibrary + /// ships have it. + LibraryDeleteUnbacked, TagsPlan, CuesPlan, ArtworkPlan, @@ -257,7 +261,7 @@ pub enum Capability { impl Capability { /// Every capability but `net:`. - pub const NAMED: [Capability; 21] = [ + pub const NAMED: [Capability; 22] = [ Capability::LibraryRead, Capability::AnalysisRead, Capability::ArtworkRead, @@ -271,6 +275,7 @@ impl Capability { Capability::CollectionRead, Capability::LibraryPlan, Capability::LibraryDelete, + Capability::LibraryDeleteUnbacked, Capability::TagsPlan, Capability::CuesPlan, Capability::ArtworkPlan, @@ -297,6 +302,7 @@ impl Capability { Capability::CollectionRead => "collection:read", Capability::LibraryPlan => "library:plan", Capability::LibraryDelete => "library:delete", + Capability::LibraryDeleteUnbacked => "library:delete-unbacked", Capability::TagsPlan => "tags:plan", Capability::CuesPlan => "cues:plan", Capability::ArtworkPlan => "artwork:plan", @@ -592,6 +598,12 @@ impl Manifest { if let Some((i, c)) = self.capabilities.iter().enumerate().find(|(_, c)| !granted.insert(*c)) { return Err(invalid(format!("capabilities[{i}]"), format!("{c} is there twice"))); } + if !built_in && let Some(i) = self.capabilities.iter().position(|c| *c == Capability::LibraryDeleteUnbacked) { + return Err(invalid( + format!("capabilities[{i}]"), + "only plugins locallibrary ships delete without a backup", + )); + } let c = &self.contributes; let commands = c.commands.iter().enumerate().map(|(i, x)| ("contributes.commands", i, x.cli.as_deref())); let operations = c.operations.iter().enumerate().map(|(i, x)| ("contributes.operations", i, x.cli.as_deref())); diff --git a/crates/loclib-plugin/src/manifest/tests.rs b/crates/loclib-plugin/src/manifest/tests.rs index a3e18ea0d..0ded4e566 100644 --- a/crates/loclib-plugin/src/manifest/tests.rs +++ b/crates/loclib-plugin/src/manifest/tests.rs @@ -17,6 +17,11 @@ const HEAD: &str = "id = \"demo\"\nname = \"Demo\"\nversion = \"0.1.0\"\nprotoco const ECHO: &str = include_str!("../../examples/echo/locallibrary-plugin.toml"); +/// The capabilities a third-party plugin may have (`net:` aside). +fn third_party_capabilities() -> Vec { + Capability::NAMED.into_iter().filter(|c| *c != Capability::LibraryDeleteUnbacked).collect() +} + /// What's wrong with `HEAD` followed by `rest`, for a third-party plugin (`""`: nothing). fn problem(rest: &str) -> String { match Manifest::parse(&format!("{HEAD}{rest}")).and_then(|m| m.validate(false)) { @@ -52,6 +57,18 @@ fn a_manifest_reads_back_from_json_unchanged() { assert_eq!(back, m); } +#[hegel::test(test_cases = 8)] +fn only_built_in_plugins_delete_without_a_backup(tc: TestCase) { + let mut capabilities = tc.draw(gs::subsequences(third_party_capabilities())); + let at = tc.draw(gs::integers::().max_value(capabilities.len())); + capabilities.insert(at, Capability::LibraryDeleteUnbacked); + let names: Vec = capabilities.iter().map(|c| format!("{:?}", c.to_string())).collect(); + let m = Manifest::parse(&format!("{HEAD}capabilities = [{}]\n", names.join(", "))).unwrap(); + m.validate(true).unwrap(); + let e = m.validate(false).unwrap_err().to_string(); + assert_eq!(e, format!("capabilities[{at}]: only plugins locallibrary ships delete without a backup")); +} + #[test] fn only_built_in_plugins_add_cli_commands() { let m = Manifest::parse(&format!( @@ -1749,7 +1766,7 @@ fn manifest(tc: &TestCase, built_in: bool) -> Manifest { .collect(); let panels = drawn_names(tc, 3).into_iter().map(|name| Panel { name, title: title(tc), offered: Where::Track }); let themes = drawn_names(tc, 3).into_iter().map(|name| Theme { name, title: title(tc), tokens: tokens(tc) }); - let mut capabilities = tc.draw(gs::subsequences(Capability::NAMED.to_vec())); + let mut capabilities = tc.draw(gs::subsequences(third_party_capabilities())); // Granted reading the sets and mixes they write out. let reads_sets = exporters.iter().flat_map(Exporter::sources).any(|s| *s != ExportFrom::Drive); if reads_sets && !capabilities.contains(&Capability::SetsRead) { @@ -2962,6 +2979,7 @@ fn capabilities_read_back_from_their_names_and_nothing_else(tc: TestCase) { "collection:read", "library:plan", "library:delete", + "library:delete-unbacked", "tags:plan", "cues:plan", "artwork:plan", @@ -3183,7 +3201,7 @@ fn each_kind_of_account_type_mistake_is_refused_saying_where(tc: TestCase) { } "beside" => { match tc.draw(gs::sampled_from(vec!["capability", "accounts", "net", "contribution"])) { - "capability" => m.capabilities.push(tc.draw(gs::sampled_from(Capability::NAMED.to_vec()))), + "capability" => m.capabilities.push(tc.draw(gs::sampled_from(third_party_capabilities()))), "accounts" => m.accounts.push(account_need(&tc, tc.draw(gs::from_regex(DRAWN_NAME)))), "net" => m.net.push(NetLimit { host: "a.example".into(), per_second: 1.0 }), _ => add(&mut m.contributes, tc.draw(gs::sampled_from(CONTRIBUTIONS.to_vec())), 1), diff --git a/plugins/demos/locallibrary-plugin.toml b/plugins/demos/locallibrary-plugin.toml index 0150d984e..2f55b147c 100644 --- a/plugins/demos/locallibrary-plugin.toml +++ b/plugins/demos/locallibrary-plugin.toml @@ -3,7 +3,8 @@ name = "rekordbox's demo tracks" version = "0.1.0" protocol = "^0.2" # library:delete: removing the demo tracks deletes them and their files. -capabilities = ["library:read", "library:plan", "library:delete"] +# library:delete-unbacked: their files stay out of the backup, as every export puts them back. +capabilities = ["library:read", "library:plan", "library:delete", "library:delete-unbacked"] # In `locallibrary check` and doctor, fixed by `clean`. [[contributes.checks]] diff --git a/plugins/demos/src/lib.rs b/plugins/demos/src/lib.rs index ca7b730a6..6d9e77844 100644 --- a/plugins/demos/src/lib.rs +++ b/plugins/demos/src/lib.rs @@ -5,11 +5,12 @@ //! //! A track is one when its artist and title are an entry of [`DEMO_TRACKS`] (ignoring case and //! spacing), in either of the drive's databases (the library read `combined`). Removing them is -//! one `delete_tracks` edit naming their audio files, which locallibrary plans, confirms and +//! one `delete_unbacked` edit naming their audio files, which locallibrary plans, confirms and //! applies: every row of those files goes from both databases with its playlist and history //! entries, and so do the audio, analysis files and artwork no other track uses, and the artists, -//! albums, genres, labels and keys only they used. A demo track sharing its file with another -//! track stays, as deleting the file's rows would delete that track too. +//! albums, genres, labels and keys only they used. The backup holds only the databases, as the +//! next export puts the files back and each refresh removes them again. A demo track sharing its +//! file with another track stays, as deleting the file's rows would delete that track too. use std::io::{Read, Write}; @@ -90,7 +91,7 @@ fn plan_op(req: PlanOp, host: &Host) -> Result { }; let edits = match paths.is_empty() { true => Vec::new(), - false => vec![json!({ "op": "delete_tracks", "tracks": { "paths": paths } })], + false => vec![json!({ "op": "delete_unbacked", "tracks": { "paths": paths } })], }; Ok(OpPlan { summary, warnings, edits, inputs: Vec::new() }) } diff --git a/plugins/demos/src/tests.rs b/plugins/demos/src/tests.rs index 26157e68c..a51fb1153 100644 --- a/plugins/demos/src/tests.rs +++ b/plugins/demos/src/tests.rs @@ -92,7 +92,7 @@ impl Session { } fn delete(paths: &[&str]) -> Value { - json!({ "op": "delete_tracks", "tracks": { "paths": paths } }) + json!({ "op": "delete_unbacked", "tracks": { "paths": paths } }) } /// Two demo tracks (spelled as rekordbox does, and not), one of them twice on one file, one @@ -119,7 +119,12 @@ fn the_manifest_is_a_built_in_one_with_a_check_and_a_destructive_operation() { let manifest = Manifest::parse(MANIFEST).unwrap(); assert!(manifest.validate(true).is_ok()); assert!(manifest.validate(false).is_err(), "only a plugin locallibrary ships has a top-level CLI command"); - let capabilities = [Capability::LibraryRead, Capability::LibraryPlan, Capability::LibraryDelete]; + let capabilities = [ + Capability::LibraryRead, + Capability::LibraryPlan, + Capability::LibraryDelete, + Capability::LibraryDeleteUnbacked, + ]; assert_eq!(manifest.capabilities, capabilities); let check = &manifest.contributes.checks[0]; assert_eq!((check.name.as_str(), check.default_on, check.when.as_slice()), ("tracks", true, &[When::Check][..]));