Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
23f3d4b3b0 |
@@ -82,6 +82,100 @@ func TestPruneStaleLocalCrossReferences(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestPruneClearsInLibraryWithNoLocalID covers the fixed point: a row
|
||||||
|
// carrying in_library with a NULL local_*_id. The upsert's conflict
|
||||||
|
// clause is `in_library = MAX(in_library, excluded.in_library)`, so it
|
||||||
|
// can only ever raise the flag, and this pass used to be gated on the id
|
||||||
|
// being present — which meant nothing in the app could clear such a row,
|
||||||
|
// ever. It is asserted for all three entity types because the gate was
|
||||||
|
// written once and used three times, so a fix applied to one is a fix
|
||||||
|
// that looks complete.
|
||||||
|
//
|
||||||
|
// The rows are seeded with raw SQL rather than through seedIndexResult
|
||||||
|
// deliberately: upsertBatch writes a zero LocalArtistID as literal 0,
|
||||||
|
// not NULL, and 0 satisfies `IS NOT NULL` — so the old gate already
|
||||||
|
// caught that shape and a fixture built through the upsert cannot
|
||||||
|
// reproduce this at all. NULL is what the artifact importer and any
|
||||||
|
// older writer leave behind, the column being nullable with no default.
|
||||||
|
func TestPruneClearsInLibraryWithNoLocalID(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
db := database.NewTestDB(t)
|
||||||
|
si := NewSearchIndex(db, nil, nil, slog.Default())
|
||||||
|
|
||||||
|
// A genuinely owned artist, to prove the wider gate does not simply
|
||||||
|
// clear everything it now looks at.
|
||||||
|
database.InsertTestTrack(t, db, database.TestTrack{
|
||||||
|
FilePath: "/music/owned.mp3",
|
||||||
|
Artist: "Owned",
|
||||||
|
})
|
||||||
|
|
||||||
|
artist, err := db.Queries.GetArtistByName(t.Context(), "Owned")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("read seeded artist: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
seedIndexResult(t, db, SearchIndexResult{
|
||||||
|
EntityType: EntityArtist,
|
||||||
|
MBID: testMBID("owned"),
|
||||||
|
Title: "Owned",
|
||||||
|
ArtistName: "Owned",
|
||||||
|
ArtistMBID: testMBID("owned"),
|
||||||
|
InLibrary: true,
|
||||||
|
LocalArtistID: artist.ID,
|
||||||
|
})
|
||||||
|
|
||||||
|
orphans := []struct {
|
||||||
|
name string
|
||||||
|
entityType string
|
||||||
|
mbid string
|
||||||
|
}{
|
||||||
|
{"artist", EntityArtist, "orphan-artist"},
|
||||||
|
{"release group", EntityReleaseGroup, "orphan-release-group"},
|
||||||
|
{"recording", EntityRecording, "orphan-recording"},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, o := range orphans {
|
||||||
|
if _, err := db.ExecContext(
|
||||||
|
`INSERT INTO explore_index
|
||||||
|
(entity_type, mbid, title, artist_name, artist_mbid,
|
||||||
|
in_library,
|
||||||
|
local_artist_id, local_release_group_id, local_recording_id)
|
||||||
|
VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)`,
|
||||||
|
dbEntityType(o.entityType), dbMBID(testMBID(o.mbid)), o.name, o.name,
|
||||||
|
dbMBID(testMBID(o.mbid)),
|
||||||
|
nil, nil, nil,
|
||||||
|
); err != nil {
|
||||||
|
t.Fatalf("seed %s orphan: %v", o.name, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
si.pruneStaleLocalCrossReferences()
|
||||||
|
|
||||||
|
inLibrary := func(t *testing.T, mbid string) int {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var flag int
|
||||||
|
if err := db.QueryRowWriter(
|
||||||
|
"SELECT in_library FROM explore_index WHERE mbid = ?", dbMBID(mbid),
|
||||||
|
).Scan(&flag); err != nil {
|
||||||
|
t.Fatalf("read in_library for %q: %v", mbid, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return flag
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, o := range orphans {
|
||||||
|
if got := inLibrary(t, testMBID(o.mbid)); got != 0 {
|
||||||
|
t.Errorf("%s with a NULL local id: in_library = %d, want 0", o.name, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := inLibrary(t, testMBID("owned")); got != 1 {
|
||||||
|
t.Errorf("owned artist: in_library = %d, want 1 (it still has a file)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
|
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
|
||||||
// backfill queue prioritizes artists by how many tracks the user actually
|
// backfill queue prioritizes artists by how many tracks the user actually
|
||||||
// owns, not by how many duplicate-mbid artist rows happen to exist (the
|
// owns, not by how many duplicate-mbid artist rows happen to exist (the
|
||||||
|
|||||||
@@ -2562,6 +2562,19 @@ func (si *SearchIndex) PopulateLocalCrossReferences() {
|
|||||||
// The row itself is left in place (it may still be part of the shipped
|
// The row itself is left in place (it may still be part of the shipped
|
||||||
// catalog, just no longer owned) — only the "this is mine" bookkeeping
|
// catalog, just no longer owned) — only the "this is mine" bookkeeping
|
||||||
// is cleared.
|
// is cleared.
|
||||||
|
//
|
||||||
|
// It is gated on the flag *or* the id, not on the id alone. Gated on
|
||||||
|
// the id, `in_library = 1 AND local_*_id IS NULL` is a fixed point: the
|
||||||
|
// upsert can only ever raise the flag and this pass skipped such a row
|
||||||
|
// by construction, so nothing in the app could clear it — a row claiming
|
||||||
|
// to be owned, permanently, with no local row to check the claim
|
||||||
|
// against. Nothing in the tree writes that shape today
|
||||||
|
// (collectLibraryEntities sets both together), which is exactly why it
|
||||||
|
// is worth closing now: the exposure is a database written by an older
|
||||||
|
// version, and the next writer that sets the flag without an id, which
|
||||||
|
// nothing structurally prevents. A NULL id fails the existence test on
|
||||||
|
// its own, so the wider gate needs no second clause to say what "not
|
||||||
|
// owned" means.
|
||||||
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
||||||
type prune struct {
|
type prune struct {
|
||||||
entityType string
|
entityType string
|
||||||
@@ -2594,7 +2607,8 @@ func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
|||||||
result, err := si.db.ExecContext(
|
result, err := si.db.ExecContext(
|
||||||
`UPDATE explore_index
|
`UPDATE explore_index
|
||||||
SET in_library = 0, `+p.column+` = NULL
|
SET in_library = 0, `+p.column+` = NULL
|
||||||
WHERE entity_type = ? AND `+p.column+` IS NOT NULL
|
WHERE entity_type = ?
|
||||||
|
AND (`+p.column+` IS NOT NULL OR in_library = 1)
|
||||||
AND NOT EXISTS (`+p.exists+`)`,
|
AND NOT EXISTS (`+p.exists+`)`,
|
||||||
dbEntityType(p.entityType),
|
dbEntityType(p.entityType),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -97,55 +97,6 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
|
|||||||
fi
|
fi
|
||||||
rm -f "$PID_FILE"
|
rm -f "$PID_FILE"
|
||||||
|
|
||||||
# ── Refuse to inherit somebody else's port ───────────────────────────
|
|
||||||
# The PID check above only knows about *this* worktree: `make dev-stop`
|
|
||||||
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
|
|
||||||
# of this repo share the default port, so an app orphaned by a deleted
|
|
||||||
# worktree goes on listening with nothing left to stop it.
|
|
||||||
#
|
|
||||||
# Without this check the new app starts, fails to bind, exits — and every
|
|
||||||
# curl and playwright-cli call afterwards goes to the *other* process, so
|
|
||||||
# the harness reports facts about an app nobody asked for. That is not a
|
|
||||||
# quiet wrongness either: it presented as
|
|
||||||
# "no such table: libraries" against a freshly created YJ_HOME, which
|
|
||||||
# reads exactly like applySchema or staleshape.go having gone wrong and
|
|
||||||
# is a frightening place to start looking.
|
|
||||||
#
|
|
||||||
# The startup wait below cannot catch it, because the health check is
|
|
||||||
# satisfied by *any* app on the port — which is precisely the failure.
|
|
||||||
# So it is refused here, before anything is launched, rather than warned
|
|
||||||
# about. --port already exists for the legitimate second-app case.
|
|
||||||
port_holder() {
|
|
||||||
command -v ss >/dev/null || return 0
|
|
||||||
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
|
|
||||||
}
|
|
||||||
|
|
||||||
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
|
|
||||||
[ -n "$(port_holder)" ]; then
|
|
||||||
holder="$(port_holder)"
|
|
||||||
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
|
|
||||||
if [ -n "$holder" ]; then
|
|
||||||
# /proc/<pid>/cwd names the checkout it belongs to, and says
|
|
||||||
# "(deleted)" for the orphaned-worktree case that is the whole
|
|
||||||
# reason this is worth a check.
|
|
||||||
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
|
|
||||||
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
|
|
||||||
echo " pid $holder ($cmd)" >&2
|
|
||||||
echo " cwd $cwd" >&2
|
|
||||||
# The PID-file check above has already passed, so whatever this
|
|
||||||
# is, `make dev-stop` does not know about it — saying otherwise
|
|
||||||
# sends you to a command that will report success and change
|
|
||||||
# nothing. Never `pkill -f` here either: the pattern would
|
|
||||||
# match this script's own command line.
|
|
||||||
echo " 'make dev-stop' will not touch it (it is not in" >&2
|
|
||||||
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
|
|
||||||
else
|
|
||||||
echo " The holder could not be identified (no ss, or it belongs" >&2
|
|
||||||
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
|
|
||||||
fi
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
# ── Choose the YJ_HOME ───────────────────────────────────────────────
|
# ── Choose the YJ_HOME ───────────────────────────────────────────────
|
||||||
# A seed is a YJ_HOME that a previous run of the app produced, tarred
|
# A seed is a YJ_HOME that a previous run of the app produced, tarred
|
||||||
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
|
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
|
||||||
@@ -249,20 +200,6 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
|
|||||||
sleep 0.25
|
sleep 0.25
|
||||||
done
|
done
|
||||||
|
|
||||||
# The loop above exits on the first answer from the port, and "something
|
|
||||||
# answered" is not "the app we started answered". The pre-launch guard
|
|
||||||
# makes that unlikely rather than impossible — a race, or a listener
|
|
||||||
# started in between — and the check is one signal, so it is worth making
|
|
||||||
# here too. An empty log beside a dead pid is the "it exited immediately
|
|
||||||
# and nothing said so" case that the original report spent its time on.
|
|
||||||
if ! kill -0 "$APP_PID" 2>/dev/null; then
|
|
||||||
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
|
|
||||||
echo " $APP_PID) is gone — something else holds the port." >&2
|
|
||||||
tail -n 30 "$LOG_FILE" >&2
|
|
||||||
rm -f "$PID_FILE"
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
cat <<EOF
|
cat <<EOF
|
||||||
dev-headless: up
|
dev-headless: up
|
||||||
url http://localhost:$PORT
|
url http://localhost:$PORT
|
||||||
|
|||||||
Reference in New Issue
Block a user