Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e745acf88a | ||
|
|
6aeac42a46 | ||
|
|
68e7edb8c9 | ||
|
|
a3b5b43777 | ||
|
|
5fae61cdf1 |
@@ -0,0 +1,263 @@
|
||||
# 021 — Listening accounting: smart plays, skips, and a real history
|
||||
|
||||
**Issue:** none yet — open one before the first edit (tracker is the
|
||||
source of truth; `./scripts/issue.sh search "skip play count"` comes
|
||||
back empty as of this writing).
|
||||
|
||||
**Status:** plan — not started.
|
||||
|
||||
**Relates:** play-count rendering (`frontend/src/components/track-list/columns.ts`),
|
||||
smart playlists (`backend/smartplaylist/`), the event contract
|
||||
(`TrackPlayCountChanged`), and any future Wrapped / "minutes listened"
|
||||
surface.
|
||||
|
||||
---
|
||||
|
||||
## What exists now
|
||||
|
||||
Three facts, all load-bearing.
|
||||
|
||||
**A "play" is recorded only on a natural finish.** `recordPlay`
|
||||
(`backend/queue/playhistory.go:9`) is called from exactly one place —
|
||||
`OnPlaybackFinished` (`backend/queue/handlers.go:14`), and only when
|
||||
`srcErr == nil`. A track the user skips past at 90% is *not* a play;
|
||||
neither is one they pause at 60% and abandon. `play_count` /
|
||||
`last_played` on `audio_files` reflect "finished to the end," nothing
|
||||
more.
|
||||
|
||||
**There is no skip concept at all.** Skipping is indistinguishable
|
||||
from a natural finish, a pause, or a shutdown. Nothing records "the
|
||||
user rejected this track," so no downstream feature (smart playlists,
|
||||
shuffle, the revisit shelf, a future skip-rate heuristic) can ask
|
||||
about it.
|
||||
|
||||
**`play_history` is a write-only log.** It holds
|
||||
`(audio_file_id, played_at)` and nothing reads it — no sqlc query
|
||||
touches it, no `PlayHistory` read path exists. Its only recorded
|
||||
purpose is the timestamps a future "minutes listened over time"
|
||||
feature would need. It is classified `Authored, Cascade` in
|
||||
`backend/datamap/datamap.go:272` ("Listening history").
|
||||
|
||||
So the gaps are: (1) skips are invisible, and (2) "played" is
|
||||
under-counted — the opposite of the usual over-counting fear. The
|
||||
scrobble intuition (count a play once `min(50%, 4:00)` has been
|
||||
*heard*, independent of how it ends) is the fix for both.
|
||||
|
||||
---
|
||||
|
||||
## What we're building
|
||||
|
||||
A single classification of every track *exit*, plus one row per exit in
|
||||
a listening log, plus the existing denormalized `play_count` /
|
||||
`last_played` updated to match the new meaning. Three exit kinds:
|
||||
|
||||
| kind | condition |
|
||||
|---|---|
|
||||
| `complete` | reached natural end, **or** abandoned with `remaining <= tail` |
|
||||
| `play` | heard `>= playThreshold`, abandoned before the tail |
|
||||
| `skip` | user moved to a *different* track before `playThreshold` |
|
||||
|
||||
Not counted, not any kind: decode failure, pause/stop/shutdown before
|
||||
the threshold, and tracks shorter than `minTrackLength`.
|
||||
|
||||
### The thresholds — named judgements, one file
|
||||
|
||||
Follow the `PreviousRestartThreshold` precedent (`backend/queue/queue.go:28`,
|
||||
a bare `const` with a comment). A new `backend/queue/listen.go` (or a
|
||||
tiny `backend/listencount` package) declares:
|
||||
|
||||
```go
|
||||
const (
|
||||
// A track this short is deliberated jingle / interstitial and is
|
||||
// never counted, either way.
|
||||
minTrackLength = 30 * time.Second
|
||||
// The scrobble rule: half the track, or four minutes, whichever
|
||||
// comes first (Last.fm / ListenBrainz).
|
||||
playThresholdMax = 4 * time.Minute
|
||||
// "Finished enough": within 15s of the end, or the last 10%,
|
||||
// whichever is larger. A 10:00 ambient track gets a 60s fade
|
||||
// window; a 2:00 pop song gets 15s.
|
||||
tailWindowFloor = 15 * time.Second
|
||||
tailWindowFraction = 0.10
|
||||
)
|
||||
|
||||
func playThreshold(d time.Duration) time.Duration {
|
||||
return min(d/2, playThresholdMax)
|
||||
}
|
||||
func tailWindow(d time.Duration) time.Duration {
|
||||
return max(d/10, tailWindowFloor)
|
||||
}
|
||||
```
|
||||
|
||||
Classification is a pure function of `(reason, position, duration)` and
|
||||
*therefore unit-testable without a player*:
|
||||
|
||||
```go
|
||||
func classify(reason ExitReason, pos, dur time.Duration) Kind
|
||||
```
|
||||
|
||||
`ExitReason` is `finished | skipped | failed | abandoned`. `skipped`
|
||||
means the queue moved to a different track by user action (Next,
|
||||
Previous past the restart threshold, PlayIndex, queue replacement,
|
||||
select-from-a-list). `failed` is the decode-error path. `abandoned` is
|
||||
pause/stop/unload/shutdown — and in v1 is a no-op (see open question 3).
|
||||
|
||||
**"Heard" is approximated by the position at exit.** We read
|
||||
`player.CurrentPositionSeconds()` at the moment of the transition, not
|
||||
an accumulated listen-time ledger. A user who seeks to 80% and listens
|
||||
5 seconds reads as "heard 80%." That is deliberately accepted for v1:
|
||||
it is how most players actually behave, it is drastically simpler, and
|
||||
the failure mode ("counted a track you skimmed as played") is mild and
|
||||
exactly what the scrobble threshold already forgives. Written down
|
||||
because "position is not listen time" is the one assumption that will
|
||||
look like a bug if it is not.
|
||||
|
||||
**Fires once per listen.** Leaving a track already leaves it; the
|
||||
`chainID` guard in `player.onPlaybackFinished` (`backend/player/player.go:633`)
|
||||
already swallows a stale finish callback, and a transition advances
|
||||
`currentIndex` past the finished track. The classifier needs the same
|
||||
guard so a Next-then-stale-finish cannot produce two rows. Key it on the
|
||||
`(audioFileID, chainID)` the transition was about.
|
||||
|
||||
---
|
||||
|
||||
## Schema — resolved: fresh design, no migration
|
||||
|
||||
The A/B migration agonizing is moot. This app has two users and both
|
||||
are devs, and play counts are explicitly not worth preserving yet — so
|
||||
the schema is written as if listening accounting had been designed in
|
||||
from the start, and the existing two databases rebuild what they need
|
||||
(see below). There is no migration step and none is re-introduced.
|
||||
|
||||
**`play_history` is renamed to `listening_events`** and grows the three
|
||||
kinds, plus the raw position/duration the classification was made from:
|
||||
|
||||
```sql
|
||||
CREATE TABLE IF NOT EXISTS listening_events (
|
||||
id INTEGER PRIMARY KEY,
|
||||
audio_file_id INTEGER NOT NULL,
|
||||
kind TEXT NOT NULL DEFAULT 'complete'
|
||||
CHECK (kind IN ('complete','play','skip')),
|
||||
position_seconds INTEGER NOT NULL DEFAULT 0,
|
||||
duration_seconds INTEGER NOT NULL DEFAULT 0,
|
||||
occurred_at DATETIME NOT NULL DEFAULT (datetime('now')),
|
||||
FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE CASCADE
|
||||
);
|
||||
CREATE INDEX IF NOT EXISTS idx_listening_events_audio_file_id
|
||||
ON listening_events(audio_file_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_listening_events_occurred_at
|
||||
ON listening_events(occurred_at);
|
||||
```
|
||||
|
||||
`position_seconds`/`duration_seconds` are kept raw so a future re-tune
|
||||
of the threshold does not force the events to be re-recorded. `kind`
|
||||
stays the write-time classification; the raw reading is evidence, not
|
||||
a second copy of the rule.
|
||||
|
||||
**The counters are denormalized onto `audio_files`** — `skip_count` /
|
||||
`last_skipped` join the existing `play_count` / `last_played`, because
|
||||
that is where the hot read path already lives and a log join per track
|
||||
row is not acceptable. This does grow the MIXED-KIND wart (see the
|
||||
survey below for the structural answer), but it is the *continuation* of
|
||||
the existing design, not a new leak: play counts sat on `audio_files`
|
||||
from before this feature existed.
|
||||
|
||||
**What happens to the two real databases on next launch.**
|
||||
`listening_events` is a new table, created verbatim. `audio_files`
|
||||
gains two columns, which `retireStaleTables` treats as a stale Owned
|
||||
table and rebuilds by rescan — dropping `play_count` / `last_played` /
|
||||
`tag_status` with it, which is the accepted cost stated in the issue.
|
||||
`play_history` is gone from the schema and the datamap, so
|
||||
`obsoleteTables` drops it; its (natural-finish-only) timestamp rows go
|
||||
with it. Nothing here is wrong on a fresh install, and on the two dev
|
||||
machines the answer is the documented "delete and rescan."
|
||||
|
||||
---
|
||||
|
||||
## Wiring: where the classifier is called
|
||||
|
||||
The risk is not the classifier — it is that **every track-replacement
|
||||
path must classify the outgoing track**, and there are many: `Next`,
|
||||
`Previous` (past the 3s restart threshold), `PlayIndex`, `playFromStart`,
|
||||
`SetQueue` / clear-and-play, remove-current, and select-from-a-list.
|
||||
Miss one and that path silently never records a skip.
|
||||
|
||||
So the classification is centralized in one queue method —
|
||||
|
||||
```go
|
||||
// leaveCurrent(reason) classifies the track at currentIndex as it is
|
||||
// about to be replaced, and records exactly one listening event.
|
||||
// Must be called without q.mu held (it writes to SQLite).
|
||||
func (q *Queue) leaveCurrent(reason ExitReason)
|
||||
```
|
||||
|
||||
— which reads position/duration from the player, calls `classify`, and
|
||||
emits the play/skip row + `TrackPlayCountChanged` when `kind != skip`.
|
||||
`OnPlaybackFinished(nil)` routes through `leaveCurrent(finished)`, the
|
||||
navigation methods route through `leaveCurrent(skipped)` before they
|
||||
advance, and `recordPlay` becomes the "did a play happen" half of it.
|
||||
|
||||
Because "one path forgot to call it" is the failure mode, a **source
|
||||
sweep** pins it, on the pattern of `TestNoDirectRuntimeEmits`
|
||||
(`backend/events/noemit_test.go`) and `TestCatalogCoversSchema`: a test
|
||||
walks `backend/queue` for assignments to `currentIndex` (and the
|
||||
`SetQueue` / remove paths) and fails if a mutation site does not sit
|
||||
adjacent to a `leaveCurrent` call. The sweep is the enforcement; the
|
||||
central method is the convenience.
|
||||
|
||||
`recordPlay` keeps its existing contract *when a play happens* —
|
||||
`TrackPlayCountChanged` with `{audioFileId, filePath, playCount,
|
||||
lastPlayed}` — so the frontend patch path and
|
||||
`playhistory_test.go` keep passing. A skip emits no per-track event in
|
||||
v1 (open question 4).
|
||||
|
||||
---
|
||||
|
||||
## Phases
|
||||
|
||||
1. **The classifier.** `listen.go`: the constants, `playThreshold`,
|
||||
`tailWindow`, `classify`. Table-driven unit tests covering every
|
||||
cell of the tristate, the <30s exemption, the tail window on both a
|
||||
10:00 and a 2:00 track, and the clip at the 4:00 cap. No I/O.
|
||||
2. **Schema.** *Done in this session.* `listening_events` replaces
|
||||
`play_history`; `skip_count` / `last_skipped` added to
|
||||
`audio_files`; datamap entry and `TestAuthoredCascadesAreDeliberate`
|
||||
allow-list renamed; `recordPlay` writes `listening_events
|
||||
('complete')`. `make generate` run; database / datamap / queue
|
||||
tests green.
|
||||
3. **Wiring.** `leaveCurrent`, the navigation/finish/error call sites,
|
||||
the `fires once per listen` guard, and the source sweep. Extend
|
||||
`playhistory_test.go` for skip/complete classification through the
|
||||
queue rather than the pure function.
|
||||
4. **Smart-playlist field.** `skip_count` (and optionally
|
||||
`days_since_skipped`) in `smartplaylist.go` field/numeric maps and
|
||||
the editor's field list, via subquery. A frontend event for skip —
|
||||
if a UI wants a skip column — follows separately.
|
||||
|
||||
## Verification
|
||||
|
||||
- **Go:** the classifier is pure and exhaustively unit-tested; the
|
||||
queue wiring is tested in-process with `events.WithSink`
|
||||
(`backend/queue/emit_test.go` is the model), asserting a Next at 90%
|
||||
emits a *play*, a Next at 10% emits a *skip and no play*, a natural
|
||||
finish emits a *complete*.
|
||||
- **Database:** schema + datamap tests fail-loud on any new or
|
||||
reclassified table; `database_test.go`'s listening-events round-trip
|
||||
asserts the new table and the four denormalized counter columns.
|
||||
- **e2e:** `e2e/specs/play-count.spec.ts` already awaits
|
||||
`TrackPlayCountChanged`; add the skip case (advance early, assert no
|
||||
`TrackPlayCountChanged` and a `skip` row via the `__/test/sql`
|
||||
endpoint if convenient, or via the playlist effect).
|
||||
- No visual/component tier needed unless a skip column ships (phase 4).
|
||||
|
||||
## Open questions / decisions needed
|
||||
|
||||
1. **Migration mechanism.** Resolved — fresh design, no migration (see the schema section). Play counts are not worth preserving, both users are devs, and `audio_files` / `play_history` rebuild-or-drop on next launch.
|
||||
2. **"Position is not listen time."** Accept the approximation for v1,
|
||||
or track accumulated listen seconds (a real ledger on the player) now?
|
||||
3. **Abandon on shutdown.** A track paused at 70% and then app-killed:
|
||||
count a `play` (scrobble says heard) or leave it unrecorded? v1
|
||||
proposes *unrecorded* — same as today — to keep the write path off
|
||||
the shutdown critical path.
|
||||
4. **Skip event to the frontend.** Emit now (parallel to
|
||||
`TrackPlayCountChanged`) or only when a surface consumes it?
|
||||
@@ -775,6 +775,7 @@ func (yj *YellowJacketApp) startJanitor() {
|
||||
}
|
||||
|
||||
yj.janitor.Register(maintenance.ExpiredHTTPCacheJob(yj.database))
|
||||
yj.janitor.Register(maintenance.StaleArtistMetadataJob(yj.database))
|
||||
yj.janitor.Register(maintenance.OrphanedCoverFilesJob(
|
||||
yj.database, coversDir, library.CoverArtFileSet,
|
||||
))
|
||||
|
||||
@@ -662,19 +662,19 @@ func TestSmartPlaylistColumns(t *testing.T) {
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Migration 10 — play history tracking
|
||||
// Listening events tracking
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestPlayHistoryTable(t *testing.T) {
|
||||
func TestListeningEventsTable(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := NewTestDB(t)
|
||||
|
||||
// Verify play_history table exists.
|
||||
// Verify listening_events table exists.
|
||||
var tableCount int64
|
||||
|
||||
tblRows, err := db.QueryContext(
|
||||
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='play_history'",
|
||||
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='listening_events'",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("query sqlite_master: %v", err)
|
||||
@@ -695,12 +695,14 @@ func TestPlayHistoryTable(t *testing.T) {
|
||||
_ = tblRows.Close()
|
||||
|
||||
if tableCount != 1 {
|
||||
t.Errorf("play_history table count = %d, want 1", tableCount)
|
||||
t.Errorf("listening_events table count = %d, want 1", tableCount)
|
||||
}
|
||||
|
||||
// Verify audio_files has play_count and last_played columns.
|
||||
// Verify audio_files has the denormalized listening counters.
|
||||
hasPlayCount := false
|
||||
hasLastPlayed := false
|
||||
hasSkipCount := false
|
||||
hasLastSkipped := false
|
||||
|
||||
colRows, err := db.QueryContext("PRAGMA table_info(audio_files)")
|
||||
if err != nil {
|
||||
@@ -732,6 +734,14 @@ func TestPlayHistoryTable(t *testing.T) {
|
||||
if name == "last_played" {
|
||||
hasLastPlayed = true
|
||||
}
|
||||
|
||||
if name == "skip_count" {
|
||||
hasSkipCount = true
|
||||
}
|
||||
|
||||
if name == "last_skipped" {
|
||||
hasLastSkipped = true
|
||||
}
|
||||
}
|
||||
|
||||
_ = colRows.Close()
|
||||
@@ -744,6 +754,14 @@ func TestPlayHistoryTable(t *testing.T) {
|
||||
t.Error("audio_files missing last_played column")
|
||||
}
|
||||
|
||||
if !hasSkipCount {
|
||||
t.Error("audio_files missing skip_count column")
|
||||
}
|
||||
|
||||
if !hasLastSkipped {
|
||||
t.Error("audio_files missing last_skipped column")
|
||||
}
|
||||
|
||||
// Verify track_metadata VIEW includes play_count and last_played.
|
||||
viewCols := map[string]bool{}
|
||||
|
||||
@@ -783,7 +801,7 @@ func TestPlayHistoryTable(t *testing.T) {
|
||||
t.Error("track_metadata VIEW missing last_played column")
|
||||
}
|
||||
|
||||
// Round-trip: insert a play_history row and verify play_count update.
|
||||
// Round-trip: insert a listening_events row and verify play_count update.
|
||||
// First, set up test data. The test DB already has library id=0.
|
||||
InsertTestTrack(t, db, TestTrack{
|
||||
FilePath: "/test/play_history.mp3",
|
||||
@@ -821,12 +839,12 @@ func TestPlayHistoryTable(t *testing.T) {
|
||||
t.Errorf("initial play_count = %d, want 0", playCount)
|
||||
}
|
||||
|
||||
// Insert a play_history row and update play_count.
|
||||
// Insert a listening_events row (kind defaults to 'complete').
|
||||
_, err = db.ExecContext(
|
||||
"INSERT INTO play_history (audio_file_id) VALUES (1)",
|
||||
"INSERT INTO listening_events (audio_file_id, kind) VALUES (1, 'complete')",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("insert play_history: %v", err)
|
||||
t.Fatalf("insert listening_events: %v", err)
|
||||
}
|
||||
|
||||
_, err = db.ExecContext(
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
package database
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
)
|
||||
|
||||
// Preserving a playlist entry across the loss of its track is two
|
||||
// statements, not one, and the split is not tidiness -- it is what
|
||||
// makes the important half work in the situation that needs it most.
|
||||
//
|
||||
// `playlist_tracks.audio_file_id` is ON DELETE SET NULL, so an entry
|
||||
// outlives its file as an id-less row that says nothing about what the
|
||||
// user put in the playlist. The phantom_* columns carry the answer
|
||||
// across and ResolvePhantomTracksAfterScan re-links them afterwards --
|
||||
// but only if something fills them *before* the rows go.
|
||||
//
|
||||
// The two halves are not equally important and are not equally
|
||||
// available:
|
||||
//
|
||||
// - **phantom_file_path is the one that matters.**
|
||||
// ResolvePhantomTracksAfterScan matches it against
|
||||
// `audio_files.file_path`, so without it an entry can never be
|
||||
// re-linked and the playlist is empty for good. It comes straight
|
||||
// off `audio_files`, whose `file_path` is the table's natural key
|
||||
// and has been present in every shape it has ever had -- including
|
||||
// the pre-013 stub of `(id, file_path, recording_id)`.
|
||||
// - The rest is *display* for a phantom entry before a rescan
|
||||
// re-links it, and it comes from the `track_metadata` view, which
|
||||
// is the one definition of a track row and not worth restating.
|
||||
//
|
||||
// Reading the view is what cannot be relied on here, and that is the
|
||||
// whole reason for the split. This runs *before* applySchema, which is
|
||||
// precisely the moment the schema is inconsistent: the view is whatever
|
||||
// the last launch's schema declared, while `audio_files` is whatever
|
||||
// the launch before that left behind. A view over columns the table no
|
||||
// longer has is not merely empty -- `pragma_table_info` on it *errors*,
|
||||
// and so does selecting from it. `cmd/indexbuild`'s fixture is exactly
|
||||
// that shape and is what caught this.
|
||||
//
|
||||
// COALESCE keeps an existing phantom value in both halves: an entry
|
||||
// already phantom is one whose file went missing in an earlier pass,
|
||||
// and its recorded metadata is the only copy left. Overwriting that
|
||||
// from a NULL join erases the rows this exists to protect.
|
||||
const (
|
||||
preservePhantomPathSQL = `
|
||||
UPDATE playlist_tracks
|
||||
SET phantom_file_path = COALESCE(phantom_file_path, (
|
||||
SELECT af.file_path FROM audio_files af
|
||||
WHERE af.id = playlist_tracks.audio_file_id
|
||||
))
|
||||
WHERE audio_file_id IS NOT NULL
|
||||
`
|
||||
|
||||
preservePhantomDisplaySQL = `
|
||||
UPDATE playlist_tracks
|
||||
SET
|
||||
phantom_title = COALESCE(phantom_title, (
|
||||
SELECT tm.title FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_artist = COALESCE(phantom_artist, (
|
||||
SELECT tm.artist_name FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_album = COALESCE(phantom_album, (
|
||||
SELECT tm.album FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_duration_ms = COALESCE(phantom_duration_ms, (
|
||||
SELECT af.length_milliseconds FROM audio_files af
|
||||
WHERE af.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_genre = COALESCE(phantom_genre, (
|
||||
SELECT tm.genre FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_cover_art_path = COALESCE(phantom_cover_art_path, (
|
||||
SELECT tm.cover_art_path FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
))
|
||||
WHERE audio_file_id IS NOT NULL
|
||||
`
|
||||
)
|
||||
|
||||
// PreservePlaylistPhantoms records every linked playlist entry's track
|
||||
// metadata on the entry itself, so the entry survives the rows being
|
||||
// deleted underneath it.
|
||||
//
|
||||
// Every path that empties `audio_files` must call this first, inside
|
||||
// the same transaction as the delete. There are two such paths and
|
||||
// they had drifted: the full rescan in backend/library did this and the
|
||||
// stale-shape retire in this package did not, so the *documented*
|
||||
// repair ("delete and rescan") preserved playlists while the automatic
|
||||
// one that exists to spare the user that work silently emptied them.
|
||||
//
|
||||
// The display half is skipped, with a warning, when `track_metadata`
|
||||
// cannot answer -- see the note above. Skipping it costs a phantom
|
||||
// entry its title until a rescan re-links it; skipping the path half
|
||||
// would cost the entry outright, so that one is an error.
|
||||
func PreservePlaylistPhantoms(
|
||||
ctx context.Context, tx *sql.Tx, logger *slog.Logger,
|
||||
) error {
|
||||
if _, err := tx.ExecContext(ctx, preservePhantomPathSQL); err != nil {
|
||||
return fmt.Errorf(
|
||||
"could not preserve playlist track file paths: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
if _, err := tx.ExecContext(ctx, preservePhantomDisplaySQL); err != nil {
|
||||
// A failed statement does not roll back a SQLite transaction,
|
||||
// so the path half above stands and the entries remain
|
||||
// re-linkable.
|
||||
logger.Warn(
|
||||
"could not record display metadata for playlist entries; "+
|
||||
"they will be re-linked by the next scan but read as "+
|
||||
"unknown until then",
|
||||
"err", err,
|
||||
)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -68,8 +68,14 @@ CREATE TABLE IF NOT EXISTS audio_files (
|
||||
-- compared against the on-disk mtime during a scan to detect files
|
||||
-- another application retagged in place.
|
||||
modified_at INTEGER NOT NULL DEFAULT 0,
|
||||
-- Listening counts, denormalized from listening_events so the hot
|
||||
-- read path (track list sort, shelves, smart playlists) never joins
|
||||
-- a log table. Authored: a rescan cannot rebuild them. This is the
|
||||
-- "MIXED KIND" half of audio_files the datamap notes.
|
||||
play_count INTEGER NOT NULL DEFAULT 0,
|
||||
last_played DATETIME,
|
||||
skip_count INTEGER NOT NULL DEFAULT 0,
|
||||
last_skipped DATETIME,
|
||||
tag_status TEXT NOT NULL DEFAULT 'untagged'
|
||||
CHECK(tag_status IN (
|
||||
'untagged', 'auto_matched', 'user_confirmed', 'user_skipped_permanent'
|
||||
|
||||
@@ -45,8 +45,6 @@ CREATE INDEX IF NOT EXISTS idx_download_items_live
|
||||
CREATE INDEX IF NOT EXISTS idx_download_items_state
|
||||
ON download_items(state);
|
||||
|
||||
-- idx_download_items_download is deliberately NOT declared here: on an
|
||||
-- existing database this table already exists at schema-pass time with
|
||||
-- its old column still named request_id, so an inline CREATE INDEX on
|
||||
-- download_id would fail outright. See ensureDownloadIndexes in
|
||||
-- backend/database/download_rename_migration.go.
|
||||
-- ListDownloadItemsForDownload filters on the parent download.
|
||||
CREATE INDEX IF NOT EXISTS idx_download_items_download
|
||||
ON download_items(download_id);
|
||||
|
||||
@@ -66,14 +66,11 @@ CREATE TABLE IF NOT EXISTS download_requests (
|
||||
FOREIGN KEY(parent_id) REFERENCES download_requests(id) ON DELETE CASCADE
|
||||
);
|
||||
|
||||
-- idx_download_requests_{due,entity,parent} are deliberately NOT
|
||||
-- declared here. This table name is reused from the old one-shot
|
||||
-- attempt table (also called download_requests before the Want/Request
|
||||
-- rename), so on an existing database this CREATE TABLE is a no-op
|
||||
-- against a table that, at schema-pass time, is still shaped like the
|
||||
-- OLD attempts table and lacks these columns entirely — an inline
|
||||
-- CREATE INDEX here would fail outright rather than just no-op. See
|
||||
-- migrateDownloadRename/ensureDownloadIndexes in
|
||||
-- backend/database/download_rename_migration.go, which create these
|
||||
-- once the rename has actually happened (or immediately, on a fresh
|
||||
-- database where the columns exist from the start).
|
||||
CREATE INDEX IF NOT EXISTS idx_download_requests_due
|
||||
ON download_requests(state, next_try_at);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_download_requests_entity
|
||||
ON download_requests(entity, state);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_download_requests_parent
|
||||
ON download_requests(parent_id) WHERE parent_id IS NOT NULL;
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
-- One row per track *exit*, three ways a listen can end: it reached
|
||||
-- the end, it was heard enough to count and then skipped past, or it
|
||||
-- was abandoned for another track before anyone had really listened.
|
||||
--
|
||||
-- This is the source of truth for listening behaviour. The
|
||||
-- denormalized `play_count` / `last_played` / `skip_count` /
|
||||
-- `last_skipped` on audio_files are materialized from it, because the
|
||||
-- hot read path (track-list sort, the shelves, smart playlists) must
|
||||
-- not join a log that grows by one row per song forever.
|
||||
--
|
||||
-- `kind` is the classification, applied at write time:
|
||||
--
|
||||
-- complete the track reached its natural end, or was skipped in
|
||||
-- its tail window (the last few seconds of a long fade).
|
||||
-- play the scrobble threshold was heard — half the track or
|
||||
-- four minutes, whichever is less — and the user moved on
|
||||
-- before the end.
|
||||
-- skip the user moved to a different track before that.
|
||||
--
|
||||
-- `position_seconds` / `duration_seconds` are the raw reading the
|
||||
-- classification was made from, kept so a future re-tune of the
|
||||
-- threshold does not need the events re-recorded. 0/0 on a row means
|
||||
-- "not captured for this event" (e.g. a natural finish recorded before
|
||||
-- these columns existed), not "a zero-second track".
|
||||
CREATE TABLE IF NOT EXISTS listening_events (
|
||||
id INTEGER PRIMARY KEY,
|
||||
audio_file_id INTEGER NOT NULL,
|
||||
kind TEXT NOT NULL DEFAULT 'complete'
|
||||
CHECK (kind IN ('complete', 'play', 'skip')),
|
||||
position_seconds INTEGER NOT NULL DEFAULT 0,
|
||||
duration_seconds INTEGER NOT NULL DEFAULT 0,
|
||||
occurred_at DATETIME NOT NULL DEFAULT (datetime('now')),
|
||||
FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE CASCADE
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_listening_events_audio_file_id
|
||||
ON listening_events(audio_file_id);
|
||||
|
||||
-- "What did I listen to this month" walks this, rather than the
|
||||
-- per-track index above.
|
||||
CREATE INDEX IF NOT EXISTS idx_listening_events_occurred_at
|
||||
ON listening_events(occurred_at);
|
||||
@@ -1,9 +0,0 @@
|
||||
CREATE TABLE IF NOT EXISTS play_history (
|
||||
id INTEGER PRIMARY KEY,
|
||||
audio_file_id INTEGER NOT NULL,
|
||||
played_at DATETIME NOT NULL DEFAULT (datetime('now')),
|
||||
FOREIGN KEY(audio_file_id) REFERENCES audio_files(id) ON DELETE CASCADE
|
||||
);
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_play_history_audio_file_id
|
||||
ON play_history(audio_file_id);
|
||||
@@ -1,18 +1,15 @@
|
||||
CREATE TABLE IF NOT EXISTS queue (
|
||||
id INTEGER PRIMARY KEY CHECK(id = 1),
|
||||
source_playlist_id INTEGER,
|
||||
current_position INTEGER NOT NULL DEFAULT 0,
|
||||
shuffle_mode BOOLEAN NOT NULL DEFAULT false,
|
||||
repeat_mode TEXT NOT NULL DEFAULT 'off',
|
||||
shuffle_order TEXT,
|
||||
-- source_playlist_id above is unused dead weight (nothing has ever
|
||||
-- written it a nonzero value); source_type/source_id/source_label
|
||||
-- below are its generalized replacement, covering albums, playlists,
|
||||
-- smart playlists, genres and artists rather than playlists alone.
|
||||
-- What the queue was built from ("Playing from: X"): an album,
|
||||
-- playlist, smart playlist, genre or artist, identified by the id
|
||||
-- that source_type's namespace gives it.
|
||||
source_type TEXT NOT NULL DEFAULT '',
|
||||
source_id INTEGER NOT NULL DEFAULT 0,
|
||||
source_label TEXT NOT NULL DEFAULT '',
|
||||
FOREIGN KEY(source_playlist_id) REFERENCES playlists(id) ON DELETE SET NULL
|
||||
source_label TEXT NOT NULL DEFAULT ''
|
||||
);
|
||||
|
||||
-- Singleton row: there is exactly one playback queue.
|
||||
|
||||
@@ -26,17 +26,10 @@ CREATE TABLE IF NOT EXISTS tagging_items (
|
||||
-- complete rip of their own directory. parent_group_key is the
|
||||
-- original folder group they were split from.
|
||||
--
|
||||
-- These two columns are declared LAST, after created_at, even
|
||||
-- though that reads oddly next to the rest of the table: sql/
|
||||
-- migrations/0001 brings a pre-existing tagging_items up to date
|
||||
-- with `ALTER TABLE ADD COLUMN`, which SQLite always appends at
|
||||
-- the end of the column list. A fresh install (this file) and an
|
||||
-- upgraded database (this file + the migration) must end up with
|
||||
-- IDENTICAL column order, because sqlc-generated `SELECT *` scans
|
||||
-- (e.g. GetTaggingItem) bind columns positionally — see the
|
||||
-- schema/migration column-order test in database_test.go. Put
|
||||
-- new columns wherever reads best when adding a table for the
|
||||
-- first time; append-only from the second migration on.
|
||||
-- These columns are appended after created_at rather than grouped
|
||||
-- with the rest of the row: sqlc's `SELECT *` scans (GetTaggingItem)
|
||||
-- bind column order positionally, so new columns always go at the
|
||||
-- end.
|
||||
synthetic INTEGER NOT NULL DEFAULT 0,
|
||||
parent_group_key TEXT NOT NULL DEFAULT '',
|
||||
-- album_artist_conflict latches to 1 the first time two tracks
|
||||
@@ -58,10 +51,3 @@ CREATE INDEX IF NOT EXISTS idx_tagging_items_library_status
|
||||
|
||||
CREATE INDEX IF NOT EXISTS idx_tagging_items_status_pending
|
||||
ON tagging_items(library_id) WHERE status = 'pending';
|
||||
|
||||
-- idx_tagging_items_parent_group_key is NOT declared here on
|
||||
-- purpose: this file runs unconditionally, before migrations, even
|
||||
-- against a database that hasn't run 0001 yet — an index predicate
|
||||
-- referencing parent_group_key would fail on that table. It lives
|
||||
-- solely in sql/migrations/0001_tagging_items_synthetic.sql, which
|
||||
-- runs after the column exists either way (see database.go).
|
||||
|
||||
@@ -39,7 +39,7 @@ INSERT INTO audio_files (
|
||||
?, ?, ?, ?, ?, ?,
|
||||
?, ?, ?, ?, ?
|
||||
)
|
||||
RETURNING id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status
|
||||
RETURNING id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status
|
||||
`
|
||||
|
||||
type CreateAudioFileParams struct {
|
||||
@@ -135,6 +135,8 @@ func (q *Queries) CreateAudioFile(ctx context.Context, arg CreateAudioFileParams
|
||||
&i.ModifiedAt,
|
||||
&i.PlayCount,
|
||||
&i.LastPlayed,
|
||||
&i.SkipCount,
|
||||
&i.LastSkipped,
|
||||
&i.TagStatus,
|
||||
)
|
||||
return i, err
|
||||
@@ -192,7 +194,7 @@ func (q *Queries) GetAllAudioFilePaths(ctx context.Context) ([]GetAllAudioFilePa
|
||||
|
||||
const getAudioFile = `-- name: GetAudioFile :one
|
||||
|
||||
SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status FROM audio_files WHERE id = ? LIMIT 1
|
||||
SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status FROM audio_files WHERE id = ? LIMIT 1
|
||||
`
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
@@ -228,13 +230,15 @@ func (q *Queries) GetAudioFile(ctx context.Context, id int64) (AudioFile, error)
|
||||
&i.ModifiedAt,
|
||||
&i.PlayCount,
|
||||
&i.LastPlayed,
|
||||
&i.SkipCount,
|
||||
&i.LastSkipped,
|
||||
&i.TagStatus,
|
||||
)
|
||||
return i, err
|
||||
}
|
||||
|
||||
const getAudioFileByPath = `-- name: GetAudioFileByPath :one
|
||||
SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status FROM audio_files WHERE file_path = ? LIMIT 1
|
||||
SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status FROM audio_files WHERE file_path = ? LIMIT 1
|
||||
`
|
||||
|
||||
func (q *Queries) GetAudioFileByPath(ctx context.Context, filePath string) (AudioFile, error) {
|
||||
@@ -267,6 +271,8 @@ func (q *Queries) GetAudioFileByPath(ctx context.Context, filePath string) (Audi
|
||||
&i.ModifiedAt,
|
||||
&i.PlayCount,
|
||||
&i.LastPlayed,
|
||||
&i.SkipCount,
|
||||
&i.LastSkipped,
|
||||
&i.TagStatus,
|
||||
)
|
||||
return i, err
|
||||
@@ -334,7 +340,7 @@ func (q *Queries) GetAudioFilesByPaths(ctx context.Context, paths []string) ([]G
|
||||
}
|
||||
|
||||
const getAudioFilesInLibrary = `-- name: GetAudioFilesInLibrary :many
|
||||
SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, tag_status FROM audio_files WHERE library_id = ?
|
||||
SELECT id, file_path, library_id, file_type_id, length_milliseconds, sample_rate, bit_depth, channels, bitrate, file_size, title, artist_credit, artist_id, album_id, track_number, disc_number, total_tracks, year, composer, comment, recording_mbid, basename, group_key, modified_at, play_count, last_played, skip_count, last_skipped, tag_status FROM audio_files WHERE library_id = ?
|
||||
`
|
||||
|
||||
func (q *Queries) GetAudioFilesInLibrary(ctx context.Context, libraryID int64) ([]AudioFile, error) {
|
||||
@@ -373,6 +379,8 @@ func (q *Queries) GetAudioFilesInLibrary(ctx context.Context, libraryID int64) (
|
||||
&i.ModifiedAt,
|
||||
&i.PlayCount,
|
||||
&i.LastPlayed,
|
||||
&i.SkipCount,
|
||||
&i.LastSkipped,
|
||||
&i.TagStatus,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -94,6 +94,8 @@ type AudioFile struct {
|
||||
ModifiedAt int64
|
||||
PlayCount int64
|
||||
LastPlayed sql.NullTime
|
||||
SkipCount int64
|
||||
LastSkipped sql.NullTime
|
||||
TagStatus string
|
||||
}
|
||||
|
||||
@@ -261,6 +263,15 @@ type Library struct {
|
||||
AutotagWarningAcked int64
|
||||
}
|
||||
|
||||
type ListeningEvent struct {
|
||||
ID int64
|
||||
AudioFileID int64
|
||||
Kind string
|
||||
PositionSeconds int64
|
||||
DurationSeconds int64
|
||||
OccurredAt time.Time
|
||||
}
|
||||
|
||||
type Lyric struct {
|
||||
AudioFileID int64
|
||||
Text string
|
||||
@@ -273,12 +284,6 @@ type LyricsIndex struct {
|
||||
Lyrics string
|
||||
}
|
||||
|
||||
type PlayHistory struct {
|
||||
ID int64
|
||||
AudioFileID int64
|
||||
PlayedAt time.Time
|
||||
}
|
||||
|
||||
type PlayerState struct {
|
||||
ID int64
|
||||
Volume int64
|
||||
@@ -312,15 +317,14 @@ type PlaylistTrack struct {
|
||||
}
|
||||
|
||||
type Queue struct {
|
||||
ID int64
|
||||
SourcePlaylistID sql.NullInt64
|
||||
CurrentPosition int64
|
||||
ShuffleMode bool
|
||||
RepeatMode string
|
||||
ShuffleOrder sql.NullString
|
||||
SourceType string
|
||||
SourceID int64
|
||||
SourceLabel string
|
||||
ID int64
|
||||
CurrentPosition int64
|
||||
ShuffleMode bool
|
||||
RepeatMode string
|
||||
ShuffleOrder sql.NullString
|
||||
SourceType string
|
||||
SourceID int64
|
||||
SourceLabel string
|
||||
}
|
||||
|
||||
type QueueTrack struct {
|
||||
|
||||
@@ -245,6 +245,13 @@ func dropDeferred(
|
||||
ctx context.Context, db *sql.DB, logger *slog.Logger,
|
||||
drop map[string]string,
|
||||
) error {
|
||||
// Asked before the transaction opens, because the answer is about
|
||||
// which tables are live and that cannot change underneath us here.
|
||||
preserve, err := shouldPreservePhantoms(ctx, db, drop)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
tx, err := db.BeginTx(ctx, nil)
|
||||
if err != nil {
|
||||
return fmt.Errorf("could not begin the retire transaction: %w", err)
|
||||
@@ -256,6 +263,22 @@ func dropDeferred(
|
||||
return fmt.Errorf("could not defer foreign keys: %w", err)
|
||||
}
|
||||
|
||||
// Before any drop, so every entry still has a track to read. It is
|
||||
// in this transaction rather than beside it because the preservation
|
||||
// and the delete have to succeed or fail together: a commit that
|
||||
// dropped the files without the phantoms is the bug, and a commit
|
||||
// that wrote phantoms without dropping anything is a lie about rows
|
||||
// that are still there.
|
||||
if preserve {
|
||||
logger.Info(
|
||||
"preserving playlist entries across the retire of audio_files",
|
||||
)
|
||||
|
||||
if err := PreservePlaylistPhantoms(ctx, tx, logger); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// Sorted, so a failure is reproducible. Map order is random, and a
|
||||
// bug that depends on which table happens to go first reproduces on
|
||||
// one run in three and passes review on the other two -- which is
|
||||
@@ -283,6 +306,38 @@ func dropDeferred(
|
||||
return nil
|
||||
}
|
||||
|
||||
// shouldPreservePhantoms reports whether this retire is about to take
|
||||
// `audio_files` out from under the playlists.
|
||||
//
|
||||
// The `playlist_tracks` check is not defensive padding. This runs
|
||||
// *before* applySchema, which is the moment the schema is by definition
|
||||
// mid-repair, and the preservation reads a table it does not drop. A
|
||||
// database old enough not to have it would otherwise fail here, and
|
||||
// failing here means the app does not open at all -- while nothing is
|
||||
// lost by skipping, since an absent `playlist_tracks` holds no
|
||||
// playlists to save.
|
||||
//
|
||||
// It deliberately does *not* ask after `track_metadata`. Whether that
|
||||
// view can answer is PreservePlaylistPhantoms's own business, because a
|
||||
// view broken against an older `audio_files` is a state this function
|
||||
// cannot detect without hitting the same error it is trying to avoid:
|
||||
// pragma_table_info on such a view errors rather than reporting no
|
||||
// columns.
|
||||
func shouldPreservePhantoms(
|
||||
ctx context.Context, db *sql.DB, drop map[string]string,
|
||||
) (bool, error) {
|
||||
if _, going := drop["audio_files"]; !going {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
cols, err := liveColumns(ctx, db, "playlist_tracks")
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
return len(cols) > 0, nil
|
||||
}
|
||||
|
||||
// staleReason reports why a live table disagrees with its declaration,
|
||||
// or "" when it agrees. A column the live table does not have is the
|
||||
// additive case; a column whose declared type changed is the one an
|
||||
|
||||
@@ -497,3 +497,180 @@ func TestParseCreateTablesReadsTheRealSchema(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestRetiringAudioFilesKeepsPlaylistContents is the symptom this
|
||||
// repair exists for: a playlist survived the retire as a row count and
|
||||
// nothing else.
|
||||
//
|
||||
// TestRetiringOwnedTablesDoesNotDangle already asserts the entry does
|
||||
// not keep a stale id, which is the *dangerous* half. It is satisfied
|
||||
// just as well by an entry that says nothing at all, which is the
|
||||
// half that quietly emptied every playlist -- so this asserts what the
|
||||
// entry still knows, and specifically phantom_file_path, because that
|
||||
// is the column ResolvePhantomTracksAfterScan matches back against
|
||||
// audio_files.file_path.
|
||||
//
|
||||
// Note the seed drops `comment`, not `artist_credit`: the mutation has
|
||||
// to leave `track_metadata` standing, since a real launch reaches the
|
||||
// retire with the view the previous launch created. A test that drops
|
||||
// the view first is testing the skip path, not this one.
|
||||
func TestRetiringAudioFilesKeepsPlaylistContents(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
db := openRaw(t, t.TempDir())
|
||||
|
||||
if _, err := db.ExecContext(ctx, "PRAGMA foreign_keys = ON"); err != nil {
|
||||
t.Fatalf("pragma: %v", err)
|
||||
}
|
||||
|
||||
if err := applySchema(ctx, db); err != nil {
|
||||
t.Fatalf("applySchema: %v", err)
|
||||
}
|
||||
|
||||
if _, err := db.ExecContext(ctx, `
|
||||
INSERT INTO playlists (id, name) VALUES (1, 'keepme');
|
||||
INSERT INTO libraries (id, name, path) VALUES (0, 'test', '/music');
|
||||
INSERT INTO artists (id, name) VALUES (3, 'Aurora Fields');
|
||||
INSERT INTO cover_art (id, file_path, mime_type)
|
||||
VALUES (9, 'covers/7.jpg', 'image/jpeg');
|
||||
INSERT INTO genres (id, name) VALUES (5, 'Ambient');
|
||||
INSERT INTO albums (id, name, artist_id, cover_art_id)
|
||||
VALUES (4, 'Tideline', 3, 9);
|
||||
INSERT INTO audio_files
|
||||
(id, file_path, file_type_id, length_milliseconds,
|
||||
title, artist_credit, artist_id, album_id)
|
||||
VALUES (7, '/music/a.flac', 1, 1000,
|
||||
'Slack Water', 'Aurora Fields', 3, 4);
|
||||
INSERT INTO file_genres (audio_file_id, genre_id) VALUES (7, 5);
|
||||
INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
|
||||
VALUES (1, 7, 0);
|
||||
ALTER TABLE audio_files DROP COLUMN comment;
|
||||
`); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
if err := retireStaleTables(ctx, db, testLogger()); err != nil {
|
||||
t.Fatalf("retire: %v", err)
|
||||
}
|
||||
|
||||
if err := applySchema(ctx, db); err != nil {
|
||||
t.Fatalf("applySchema: %v", err)
|
||||
}
|
||||
|
||||
var (
|
||||
path, title, artist, album, genre, cover sql.NullString
|
||||
duration sql.NullInt64
|
||||
)
|
||||
|
||||
if err := db.QueryRowContext(ctx, `
|
||||
SELECT phantom_file_path, phantom_title, phantom_artist,
|
||||
phantom_album, phantom_duration_ms, phantom_genre,
|
||||
phantom_cover_art_path
|
||||
FROM playlist_tracks WHERE playlist_id = 1
|
||||
`).Scan(&path, &title, &artist, &album, &duration, &genre, &cover); err != nil {
|
||||
t.Fatalf("read the surviving entry: %v", err)
|
||||
}
|
||||
|
||||
// The one that matters: without it the entry can never be re-linked
|
||||
// by the rescan the retire itself provokes.
|
||||
if path.String != "/music/a.flac" {
|
||||
t.Fatalf(
|
||||
"phantom_file_path is %q, want %q -- the playlist entry "+
|
||||
"cannot be re-linked and the playlist is empty for good",
|
||||
path.String, "/music/a.flac",
|
||||
)
|
||||
}
|
||||
|
||||
if title.String != "Slack Water" {
|
||||
t.Errorf("phantom_title is %q, want %q", title.String, "Slack Water")
|
||||
}
|
||||
|
||||
if artist.String != "Aurora Fields" {
|
||||
t.Errorf("phantom_artist is %q, want %q", artist.String, "Aurora Fields")
|
||||
}
|
||||
|
||||
if album.String != "Tideline" {
|
||||
t.Errorf("phantom_album is %q, want %q", album.String, "Tideline")
|
||||
}
|
||||
|
||||
if duration.Int64 != 1000 {
|
||||
t.Errorf("phantom_duration_ms is %d, want 1000", duration.Int64)
|
||||
}
|
||||
|
||||
if genre.String != "Ambient" {
|
||||
t.Errorf("phantom_genre is %q, want %q", genre.String, "Ambient")
|
||||
}
|
||||
|
||||
if cover.String != "covers/7.jpg" {
|
||||
t.Errorf("phantom_cover_art_path is %q, want %q", cover.String, "covers/7.jpg")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRetiringAudioFilesKeepsPathsWhenTheViewCannotAnswer is the case
|
||||
// that broke cmd/indexbuild: this repair runs *before* applySchema, so
|
||||
// `track_metadata` is whatever the last launch declared while
|
||||
// `audio_files` is whatever the launch before that left behind, and a
|
||||
// view over columns the table no longer has does not read as empty --
|
||||
// it errors.
|
||||
//
|
||||
// The pre-013 stub shape below is the real one that fixture carries.
|
||||
// What must survive is phantom_file_path, because `file_path` is the
|
||||
// table's natural key and has been in every shape it ever had; the
|
||||
// display columns are allowed to be absent, and the open must not fail.
|
||||
func TestRetiringAudioFilesKeepsPathsWhenTheViewCannotAnswer(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
db := openRaw(t, t.TempDir())
|
||||
|
||||
if _, err := db.ExecContext(ctx, "PRAGMA foreign_keys = ON"); err != nil {
|
||||
t.Fatalf("pragma: %v", err)
|
||||
}
|
||||
|
||||
if err := applySchema(ctx, db); err != nil {
|
||||
t.Fatalf("applySchema: %v", err)
|
||||
}
|
||||
|
||||
// The rows go in *after* the reshape: dropping audio_files with
|
||||
// foreign keys on would fire the ON DELETE SET NULL and null the
|
||||
// entry this test is about, which would pass for the wrong reason.
|
||||
if _, err := db.ExecContext(ctx, `
|
||||
DROP TABLE audio_files;
|
||||
CREATE TABLE audio_files (
|
||||
id INTEGER PRIMARY KEY,
|
||||
file_path TEXT NOT NULL UNIQUE,
|
||||
recording_id INTEGER
|
||||
);
|
||||
INSERT INTO playlists (id, name) VALUES (1, 'keepme');
|
||||
INSERT INTO audio_files (id, file_path) VALUES (7, '/music/a.flac');
|
||||
INSERT INTO playlist_tracks (playlist_id, audio_file_id, position)
|
||||
VALUES (1, 7, 0);
|
||||
`); err != nil {
|
||||
t.Fatalf("seed: %v", err)
|
||||
}
|
||||
|
||||
// The symptom this guards: the repair must not turn a recoverable
|
||||
// database into one the app refuses to open.
|
||||
if err := retireStaleTables(ctx, db, testLogger()); err != nil {
|
||||
t.Fatalf(
|
||||
"the retire failed on a view it could not read, so the app "+
|
||||
"would not open at all: %v", err,
|
||||
)
|
||||
}
|
||||
|
||||
if err := applySchema(ctx, db); err != nil {
|
||||
t.Fatalf("applySchema: %v", err)
|
||||
}
|
||||
|
||||
var path sql.NullString
|
||||
if err := db.QueryRowContext(ctx,
|
||||
"SELECT phantom_file_path FROM playlist_tracks WHERE playlist_id = 1",
|
||||
).Scan(&path); err != nil {
|
||||
t.Fatalf("read the surviving entry: %v", err)
|
||||
}
|
||||
|
||||
if path.String != "/music/a.flac" {
|
||||
t.Fatalf(
|
||||
"phantom_file_path is %q, want %q -- the display half being "+
|
||||
"unavailable must not cost the entry its one re-link key",
|
||||
path.String, "/music/a.flac",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -269,10 +269,11 @@ var tables = []Table{
|
||||
"from owned files plus the LRCLIB backfill.",
|
||||
},
|
||||
{
|
||||
Name: "play_history", Kind: Authored, Lifetime: Cascade,
|
||||
Note: "Listening history. Authored, but intentionally cascades " +
|
||||
"with its track — history for a file no longer in the library " +
|
||||
"has nothing to point at.",
|
||||
Name: "listening_events", Kind: Authored, Lifetime: Cascade,
|
||||
Note: "Listening history, one row per track exit (complete, play " +
|
||||
"or skip). Authored, but intentionally cascades with its " +
|
||||
"track — history for a file no longer in the library has " +
|
||||
"nothing to point at.",
|
||||
},
|
||||
{
|
||||
Name: "player_state", Kind: Authored, Lifetime: Retained,
|
||||
|
||||
@@ -212,14 +212,14 @@ func TestLifetimesMatchSchema(t *testing.T) {
|
||||
|
||||
// Authored data is unrecoverable, so it must never be removed as a side
|
||||
// effect of deleting owned data. Cascade is allowed only where the
|
||||
// catalog explains why (play_history, queue_tracks); this test pins the
|
||||
// catalog explains why (listening_events, queue_tracks); this test pins the
|
||||
// set so a new cascade onto authored data is a deliberate decision.
|
||||
func TestAuthoredCascadesAreDeliberate(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
allowed := map[string]bool{
|
||||
"play_history": true,
|
||||
"queue_tracks": true,
|
||||
"listening_events": true,
|
||||
"queue_tracks": true,
|
||||
|
||||
// Download history is scoped to the library it imported into.
|
||||
// When that library is removed the files it acquired go with
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"time"
|
||||
|
||||
"yellowjacket/backend/coverart"
|
||||
"yellowjacket/backend/database"
|
||||
)
|
||||
|
||||
var errNoLibrariesConfigured = errors.New(
|
||||
@@ -137,34 +138,12 @@ func (l *Library) clearLibraryTables() error {
|
||||
// metadata for all linked tracks before audio_files are deleted.
|
||||
// ON DELETE SET NULL will null out audio_file_id, converting them
|
||||
// to phantoms that ResolvePhantomTracksAfterScan can re-link.
|
||||
if _, err := tx.ExecContext(l.ctx, `
|
||||
UPDATE playlist_tracks
|
||||
SET
|
||||
phantom_title = COALESCE(phantom_title, (
|
||||
SELECT tm.title FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_artist = COALESCE(phantom_artist, (
|
||||
SELECT tm.artist_name FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_album = COALESCE(phantom_album, (
|
||||
SELECT tm.album FROM track_metadata tm
|
||||
WHERE tm.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_duration_ms = COALESCE(phantom_duration_ms, (
|
||||
SELECT af.length_milliseconds FROM audio_files af
|
||||
WHERE af.id = playlist_tracks.audio_file_id
|
||||
)),
|
||||
phantom_file_path = COALESCE(phantom_file_path, (
|
||||
SELECT af.file_path FROM audio_files af
|
||||
WHERE af.id = playlist_tracks.audio_file_id
|
||||
))
|
||||
WHERE audio_file_id IS NOT NULL
|
||||
`); err != nil {
|
||||
return fmt.Errorf(
|
||||
"could not preserve playlist track metadata: %w", err,
|
||||
)
|
||||
//
|
||||
// Shared with the stale-shape retire in backend/database, which is
|
||||
// the other path that empties this table and which did not do this
|
||||
// (#183): the statement lives there so the two cannot drift again.
|
||||
if err := database.PreservePlaylistPhantoms(l.ctx, tx, l.logger); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Phase 2: the files. file_genres cascades with them.
|
||||
|
||||
@@ -666,3 +666,71 @@ func TestExpiredHTTPCacheJob_TrimsToBudget(t *testing.T) {
|
||||
t.Errorf("kept %q, want the longest-lived row", kept)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleArtistMetadataJob pins the sweep's two keep rules: an owned
|
||||
// artist's metadata survives, a browsed artist's survives while it still
|
||||
// holds cached artwork, and everything else goes (#248).
|
||||
func TestStaleArtistMetadataJob(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
|
||||
const (
|
||||
ownedMBID = "11111111-1111-1111-1111-111111111111"
|
||||
browsedMBID = "22222222-2222-2222-2222-222222222222"
|
||||
staleMBID = "33333333-3333-3333-3333-333333333333"
|
||||
)
|
||||
|
||||
// The owned artist is in the library - which means a *file* says
|
||||
// so. An artists row on its own is not ownership.
|
||||
database.InsertTestTrack(t, db, database.TestTrack{
|
||||
FilePath: "/music/owned.mp3",
|
||||
Artist: "Owned",
|
||||
ArtistMBID: ownedMBID,
|
||||
})
|
||||
|
||||
for _, mbid := range []string{ownedMBID, browsedMBID, staleMBID} {
|
||||
if _, err := db.ExecContext(
|
||||
`INSERT INTO artist_metadata (mbid, source, data, fetched_at)
|
||||
VALUES (?, 'wikidata-p18', x'00', CURRENT_TIMESTAMP)`,
|
||||
mbid,
|
||||
); err != nil {
|
||||
t.Fatalf("seed artist_metadata for %s: %v", mbid, err)
|
||||
}
|
||||
}
|
||||
|
||||
// The browsed artist holds cached artwork, so its metadata is still
|
||||
// referenced and must survive.
|
||||
if _, err := db.ExecContext(
|
||||
`INSERT INTO artist_images
|
||||
(artist_mbid, source, source_url, file_path)
|
||||
VALUES (?, 'test', 'http://x', '/art/primary.jpg')`,
|
||||
browsedMBID,
|
||||
); err != nil {
|
||||
t.Fatalf("seed artist_images: %v", err)
|
||||
}
|
||||
|
||||
if _, err := StaleArtistMetadataJob(db).Run(context.Background()); err != nil {
|
||||
t.Fatalf("run job: %v", err)
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
mbid string
|
||||
want int
|
||||
}{
|
||||
{ownedMBID, 1},
|
||||
{browsedMBID, 1},
|
||||
{staleMBID, 0},
|
||||
} {
|
||||
var n int
|
||||
if err := db.QueryRowWriter(
|
||||
"SELECT COUNT(*) FROM artist_metadata WHERE mbid = ?", tc.mbid,
|
||||
).Scan(&n); err != nil {
|
||||
t.Fatalf("count %s: %v", tc.mbid, err)
|
||||
}
|
||||
|
||||
if n != tc.want {
|
||||
t.Errorf("artist_metadata rows for %s = %d, want %d", tc.mbid, n, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -628,3 +628,37 @@ func dirSize(dir string) (bytes, files int64) {
|
||||
|
||||
return bytes, files
|
||||
}
|
||||
|
||||
// StaleArtistMetadataJob evicts long-lived artist metadata (bios, wiki
|
||||
// leads, relationships) for artists the user no longer has any reason
|
||||
// to keep around: not owned and holding no cached artwork.
|
||||
//
|
||||
// artist_metadata has no TTL by design — entity data changes rarely and
|
||||
// re-fetching spends someone else's rate limit — so without a sweep it
|
||||
// grows for the life of the install. This is the "swept when the
|
||||
// artist is no longer referenced" contract the datamap always declared
|
||||
// for it and nothing ever performed (#248).
|
||||
func StaleArtistMetadataJob(db *database.DB) Job {
|
||||
return Job{
|
||||
Name: "artist-metadata-sweep",
|
||||
MinInterval: dailyInterval,
|
||||
Run: func(_ context.Context) (Result, error) {
|
||||
res, err := db.ExecContext(
|
||||
`DELETE FROM artist_metadata
|
||||
WHERE mbid NOT IN (` + ownedArtistMBIDs + `)
|
||||
AND mbid NOT IN (
|
||||
SELECT artist_mbid FROM artist_images
|
||||
)`,
|
||||
)
|
||||
if err != nil {
|
||||
return Result{}, fmt.Errorf(
|
||||
"delete stale artist_metadata rows: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
rows, _ := res.RowsAffected()
|
||||
|
||||
return Result{RowsDeleted: rows}, nil
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@ import (
|
||||
"yellowjacket/backend/events"
|
||||
)
|
||||
|
||||
// recordPlay inserts a play_history row and updates the denormalized
|
||||
// recordPlay inserts a listening_events row and updates the denormalized
|
||||
// play_count / last_played columns on audio_files. Called from
|
||||
// OnPlaybackFinished for the track that just finished.
|
||||
//
|
||||
@@ -20,10 +20,12 @@ func (q *Queue) recordPlay(audioFileID int64) {
|
||||
|
||||
now := time.Now().UTC().Format(time.DateTime)
|
||||
|
||||
// Insert play_history row.
|
||||
// Insert the listening event. A natural finish is a 'complete' by
|
||||
// construction; position/duration are the classifier's to fill once
|
||||
// skips are recorded (see .planning/plans/active/021).
|
||||
_, err := q.db.ExecContext(
|
||||
`INSERT INTO play_history (audio_file_id, played_at)
|
||||
VALUES (?, ?)`,
|
||||
`INSERT INTO listening_events (audio_file_id, kind, occurred_at)
|
||||
VALUES (?, 'complete', ?)`,
|
||||
audioFileID, now,
|
||||
)
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user