* fix(scanner): stop classifying "other" content folders as extras Regression from #322 (trailers and extras for movies and series), which introduced the extrasDirKinds map. The extras directory classifier mapped the generic labels "other" and "others" to ExtraKindOther. These are not part of the Jellyfin/Plex extras folder convention the map claims to mirror, and they collide with real content-scope folder names. A library organized as "movies/other/<Title (year) {ids}>/<file>" tripped the depth-2 ancestor lookup in classifyExtraPath: every title two levels under the scope folder "other" was classified as an "other"-kind extra. Such files are partitioned out of primary root/group inference and matching, then deferred in processExtraFiles because their parent cannot resolve (they are the primary titles, not children of one). The result on one deployment was ~10k movies under a folder named "other" funneled through the slow extras path every scan (parent-unresolved deferrals at ~9.5/s), stalling the scan and freezing that scope for new/changed primary content. Remove "other"/"others" from extrasDirKinds. The ExtraKindOther kind stays reachable through genuine convention labels (extra/extras/interviews/ scenes/shorts). Add regression coverage asserting titles under a scope folder named other/others stay primary. * perf(scanner): rewrite identity-only changes without re-probing A pure identity/grouping change on an already-probed file — a root_assignment_changed or group_assignment_changed reason with nothing else — used to fall into the full update branch, which unconditionally ran ffprobe (probeFile) and then upserted every column, including probe columns, from the freshly built row. When a group-key or root scheme changes across the library (see #319), this reprobed nearly every file on the next scan: an incremental scan that normally takes ~1h ran 7h+ as a full-library ffprobe storm, even though the media bytes were untouched. Add a metadata-only update path in processFile: when identityOnlyUpdateReasons reports every reason is a root/group reassignment, rewrite just the derived identity columns via the new FileRepository.UpdateIdentity and skip ffprobe, OSHash, and marker fetch entirely. UpdateIdentity issues a targeted UPDATE of the root/group/identity and edition/presentation columns only, mirroring Upsert's column handling, and leaves probe data, file bytes/mtime/hash, subtitles, chapters, markers, and content/episode/extra linkage intact. The stored group key converges to the recomputed value on the next scan, so the file takes the unchanged fast-path thereafter — without a probe storm. The shared identity-column population is extracted into populateScanIdentity so the full path and the metadata-only path stay in lockstep. Verification: unit test for the identityOnlyUpdateReasons classifier; a DB-backed test (skipped without SILO_TEST_DATABASE_URL) asserting UpdateIdentity rewrites grouping while preserving probe/linkage columns; the UPDATE statement was also exercised against the live schema inside a rolled-back transaction. * fix(scanner): harden identity fast path and extras scope classification Review follow-ups for the two scan-regression fixes on this branch, addressing both Codex review comments on PR #341 plus adversarial-review findings. Identity fast path (processFile/UpdateIdentity): - Gate the metadata-only path on existing.ExtraID == "": a row still linked as an extra reaching processFile is being reclassified as primary, and only the full upsert clears extra linkage; UpdateIdentity would have frozen it out of matching forever (match backlog filters extra_id IS NULL). - Gate on existing.FileHash != "": the full path backfills the OSHash and fetches hash-keyed S3 intro/credits markers, which no later scan reason would repair; hash-less legacy rows now take the full path once instead of silently losing that repair channel. file_hash is added to the scan-state row shape to support the gate. - Clear match_suppressed_at like every other scan write, so files with fresh identity re-enter the match backlog (suppression is documented as lasting "until retried or seen by a new scan"). - Write media_folder_id, mirroring Upsert's ON CONFLICT reassignment. - Return ErrFileNotFound when the row vanished mid-scan (concurrent delete) and fall through to the full upsert path instead of surfacing a per-file scan error. - Return only the row id instead of RETURNING all ~75 columns: the fast path fires once per file during library-wide grouping migrations, and dragging the track/chapter JSONB payloads along for a million rows dominated the cost of the path built to be cheap. - Extract identityColumnDefaults shared by Upsert and UpdateIdentity so the defaulting rules cannot drift, and drop the no-op editionConfidence indirection copied between them. - Use populateScanIdentity in the new-file insert path too; it still carried a verbatim copy of the extracted block (with a provably dead existingByPath lookup). Extras classification: - Restore "other" to extrasDirKinds: it is part of both the documented Jellyfin and Plex extras-folder conventions (the removed-label fix overshot and broke "movies/<Title>/Other/<file>" libraries, ingesting their extras as bogus primary titles). "others" stays removed - it is in neither convention. - Replace label removal with the structural guard the PR had deferred: classifyExtraPath now rejects a supplemental-named directory sitting at library-scope depth (the dir, any supplemental ancestor, or the first non-supplemental ancestor is a configured library root). This fixes the original "movies/other/<Title>" defer-storm generically, covering every convention label (shorts, scenes, extras, ...) used as a content-scope folder. - Scope extras parent binding by folder.Paths instead of the walk roots, so a subtree scan targeting a single movie folder still binds that movie's own extras instead of deferring them. Tests: eligibility-gate unit tests, scope-guard classifier cases (convention Other/ inside a title binds; scope-level other/shorts stay primary), and the DB-backed UpdateIdentity test now also covers folder moves, suppression clearing, and ErrFileNotFound. Full scanner suite ran green against a migrated scratch PostgreSQL 17 container. * refactor(scanner): simplify extras scope guard to title-folder rule Replace the ancestor-walking supplementalDirAtScopeDepth loop with the plain rule it was approximating: a convention-named directory counts as an extras dir only when it sits inside a title folder — it must not be a configured library root or directly under one. Same outcome for the layouts that matter (movies/other/<Title> stays primary, <Title>/Other classifies), less machinery. * test(scanner): assert all rewritten identity columns in UpdateIdentity test * fix(scanner): make extras scope classification structure-aware The title-folder rule from 53632022 anchored on library roots, so it missed both directions: chained convention dirs at the root ("movies/extras/behind the scenes/clip.mkv") classified as extras with an unresolvable parent (deferred forever), and category folders nested below the root ("movies/4K/other/<Title>/") still misclassified their titles. Replace the root-distance heuristic with the structural property that actually distinguishes the two cases: a convention-named directory only counts as an extras dir when its owner (first non-supplemental ancestor) is a title folder — a directory that holds media of its own. The new extrasClassifier derives that from the scan's walked path list (no extra I/O): movie folders must hold a file directly beside the extras dir; series folders may hold episodes one level down in season folders (media hiding inside a folder's own extras dirs doesn't count). Library roots never qualify. Watch-event scans, which have no walked list, probe ownership with bounded os.ReadDir instead. This handles title folders at any depth below the root and keeps scope/category folders primary at any depth, with two known edges: a title folder holding only extras (its media file missing) stays primary until the file appears, and a mixed dir holding both loose media and a category folder degrades to deferral, never wrong linkage. resolveExtraParent's inline supplemental-chain walk is extracted into the shared firstNonSupplementalAncestor.
268 lines
8.6 KiB
Go
268 lines
8.6 KiB
Go
package scanner
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"sort"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/Silo-Server/silo-server/internal/models"
|
|
)
|
|
|
|
func TestCollectLogicalFilePaths_PreservesLogicalSymlinkRootPaths(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
base := t.TempDir()
|
|
physicalRoot := filepath.Join(base, "real")
|
|
logicalRoot := filepath.Join(base, "library")
|
|
seasonDir := filepath.Join(physicalRoot, "Season 1")
|
|
|
|
if err := os.MkdirAll(seasonDir, 0o755); err != nil {
|
|
t.Fatalf("mkdir season dir: %v", err)
|
|
}
|
|
filePath := filepath.Join(seasonDir, "Episode 01.mkv")
|
|
if err := os.WriteFile(filePath, []byte("test"), 0o644); err != nil {
|
|
t.Fatalf("write media file: %v", err)
|
|
}
|
|
if err := os.Symlink(physicalRoot, logicalRoot); err != nil {
|
|
t.Skipf("symlinks not supported on this platform: %v", err)
|
|
}
|
|
|
|
files, err := collectLogicalFilePaths(context.Background(), []string{logicalRoot}, "series")
|
|
if err != nil {
|
|
t.Fatalf("collect logical paths: %v", err)
|
|
}
|
|
|
|
want := filepath.Join(logicalRoot, "Season 1", "Episode 01.mkv")
|
|
if len(files) != 1 {
|
|
t.Fatalf("files len = %d, want 1 (%v)", len(files), files)
|
|
}
|
|
if files[0] != want {
|
|
t.Fatalf("files[0] = %q, want %q", files[0], want)
|
|
}
|
|
}
|
|
|
|
func TestCollectLogicalFilePaths_DedupesSharedPhysicalDirsAndCycles(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
base := t.TempDir()
|
|
physicalRoot := filepath.Join(base, "real")
|
|
aliasRoot := filepath.Join(base, "alias")
|
|
loopPath := filepath.Join(physicalRoot, "loop")
|
|
|
|
if err := os.MkdirAll(physicalRoot, 0o755); err != nil {
|
|
t.Fatalf("mkdir root: %v", err)
|
|
}
|
|
filePath := filepath.Join(physicalRoot, "Movie.mkv")
|
|
if err := os.WriteFile(filePath, []byte("test"), 0o644); err != nil {
|
|
t.Fatalf("write media file: %v", err)
|
|
}
|
|
if err := os.Symlink(physicalRoot, aliasRoot); err != nil {
|
|
t.Skipf("symlinks not supported on this platform: %v", err)
|
|
}
|
|
if err := os.Symlink(physicalRoot, loopPath); err != nil {
|
|
t.Skipf("symlinks not supported on this platform: %v", err)
|
|
}
|
|
|
|
files, err := collectLogicalFilePaths(context.Background(), []string{physicalRoot, aliasRoot}, "movie")
|
|
if err != nil {
|
|
t.Fatalf("collect logical paths: %v", err)
|
|
}
|
|
|
|
sort.Strings(files)
|
|
want := []string{filepath.Join(physicalRoot, "Movie.mkv")}
|
|
if len(files) != len(want) {
|
|
t.Fatalf("files len = %d, want %d (%v)", len(files), len(want), files)
|
|
}
|
|
for i := range want {
|
|
if files[i] != want[i] {
|
|
t.Fatalf("files[%d] = %q, want %q", i, files[i], want[i])
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestShouldSkipStableConfirmedFile_DoesNotSkipAssignmentChanges(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
modifiedAt := time.Now().UTC().Truncate(time.Microsecond)
|
|
existing := &models.MediaFile{
|
|
ContentID: "matched-content",
|
|
FileSize: 1_000,
|
|
FileModifiedAt: &modifiedAt,
|
|
}
|
|
|
|
if shouldSkipStableConfirmedFile(existing, "matched", 1_000, modifiedAt, []string{"root_assignment_changed"}, false) {
|
|
t.Fatal("expected assignment changes to bypass stable-file skip")
|
|
}
|
|
if !shouldSkipStableConfirmedFile(existing, "matched", 1_000, modifiedAt, nil, false) {
|
|
t.Fatal("expected unchanged matched file to use stable-file skip")
|
|
}
|
|
}
|
|
|
|
func TestScanStateUpdateReasons_DetectsMissingExternalSubtitle(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
modifiedAt := time.Now().UTC().Truncate(time.Microsecond)
|
|
existing := &scanStateFile{
|
|
ContentID: "matched-content",
|
|
FileSize: 1_000,
|
|
FileModifiedAt: &modifiedAt,
|
|
ExternalSubtitlePaths: []string{filepath.Join(t.TempDir(), "Movie.en.srt")},
|
|
}
|
|
|
|
reasons := scanStateUpdateReasons(existing, 1_000, modifiedAt, nil, false, fileRootAssignment{}, fileGroupAssignment{}, "movies", false)
|
|
if !testStringSliceContains(reasons, "external_subtitle_missing") {
|
|
t.Fatalf("expected external_subtitle_missing reason, got %#v", reasons)
|
|
}
|
|
if shouldSkipStableConfirmedScanState(existing, "matched", 1_000, modifiedAt, reasons, false) {
|
|
t.Fatal("expected missing external subtitle to bypass stable scan-state skip")
|
|
}
|
|
}
|
|
|
|
func TestScanStateUpdateReasons_DetectsExternalSubtitleInventoryChange(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
dir := t.TempDir()
|
|
modifiedAt := time.Now().UTC().Truncate(time.Microsecond)
|
|
existing := &scanStateFile{
|
|
ContentID: "matched-content",
|
|
FileSize: 1_000,
|
|
FileModifiedAt: &modifiedAt,
|
|
ExternalSubtitlePaths: []string{filepath.Join(dir, "Movie.en.srt")},
|
|
}
|
|
|
|
reasons := scanStateUpdateReasons(
|
|
existing,
|
|
1_000,
|
|
modifiedAt,
|
|
[]string{filepath.Join(dir, "Movie.en.srt"), filepath.Join(dir, "Movie.fr.srt")},
|
|
true,
|
|
fileRootAssignment{},
|
|
fileGroupAssignment{},
|
|
"movies",
|
|
false,
|
|
)
|
|
if !testStringSliceContains(reasons, "external_subtitle_changed") {
|
|
t.Fatalf("expected external_subtitle_changed reason, got %#v", reasons)
|
|
}
|
|
if shouldSkipStableConfirmedScanState(existing, "matched", 1_000, modifiedAt, reasons, false) {
|
|
t.Fatal("expected external subtitle change to bypass stable scan-state skip")
|
|
}
|
|
}
|
|
|
|
func TestIdentityOnlyUpdateReasons(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
cases := []struct {
|
|
name string
|
|
reasons []string
|
|
want bool
|
|
}{
|
|
{"empty", nil, false},
|
|
{"group only", []string{"group_assignment_changed"}, true},
|
|
{"root only", []string{"root_assignment_changed"}, true},
|
|
{"group and root", []string{"group_assignment_changed", "root_assignment_changed"}, true},
|
|
{"group plus mtime needs reprobe", []string{"group_assignment_changed", "mtime_changed"}, false},
|
|
{"probe repair needs reprobe", []string{"probe_repair"}, false},
|
|
{"size change needs reprobe", []string{"size_changed"}, false},
|
|
{"was missing needs reprobe", []string{"was_missing"}, false},
|
|
{"subtitle change is not identity-only", []string{"external_subtitle_changed"}, false},
|
|
{"group plus subtitle needs full path", []string{"group_assignment_changed", "external_subtitle_changed"}, false},
|
|
}
|
|
for _, tc := range cases {
|
|
if got := identityOnlyUpdateReasons(tc.reasons); got != tc.want {
|
|
t.Errorf("identityOnlyUpdateReasons(%#v) = %v, want %v", tc.reasons, got, tc.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestIdentityOnlyFastPathEligible(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
identityReasons := []string{"group_assignment_changed"}
|
|
cases := []struct {
|
|
name string
|
|
existing scanStateFile
|
|
reasons []string
|
|
want bool
|
|
}{
|
|
{"probed primary row", scanStateFile{FileHash: "abc"}, identityReasons, true},
|
|
{"non-identity reasons need full path", scanStateFile{FileHash: "abc"}, []string{"size_changed"}, false},
|
|
// A row still linked as an extra is being reclassified as primary;
|
|
// only the full upsert clears extra_id so matching can pick it up.
|
|
{"former extra needs full path", scanStateFile{ExtraID: "extra-1", FileHash: "abc"}, identityReasons, false},
|
|
// A hash-less row needs the full path once to backfill OSHash and the
|
|
// hash-keyed S3 markers.
|
|
{"missing hash needs full path", scanStateFile{}, identityReasons, false},
|
|
}
|
|
for _, tc := range cases {
|
|
if got := identityOnlyFastPathEligible(&tc.existing, tc.reasons); got != tc.want {
|
|
t.Errorf("%s: identityOnlyFastPathEligible = %v, want %v", tc.name, got, tc.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func testStringSliceContains(values []string, target string) bool {
|
|
for _, value := range values {
|
|
if value == target {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
func TestWalkModeForEbookLibraryTypes(t *testing.T) {
|
|
for _, libraryType := range []string{"ebook", "ebooks", " EBOOKS "} {
|
|
if got := walkModeFor(libraryType); got != walkModeEbook {
|
|
t.Fatalf("walkModeFor(%q) = %v, want %v", libraryType, got, walkModeEbook)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestWalkModeEbookAcceptsEbookExtensionsOnly(t *testing.T) {
|
|
for _, ext := range []string{".epub", ".pdf", ".mobi", ".azw", ".azw3", ".fb2", ".fbz", ".cbz", ".cbr"} {
|
|
if !walkModeEbook.acceptsExt(ext) {
|
|
t.Fatalf("walkModeEbook should accept %s", ext)
|
|
}
|
|
}
|
|
for _, ext := range []string{".txt", ".md", ".mp4", ".mp3", ".mkv"} {
|
|
if walkModeEbook.acceptsExt(ext) {
|
|
t.Fatalf("walkModeEbook should reject %s", ext)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestScanFolderEbookLibraryRoutesToEbookScanner(t *testing.T) {
|
|
scanner := &Scanner{}
|
|
result, err := scanner.ScanFolder(context.Background(), &models.MediaFolder{
|
|
ID: 0,
|
|
Type: " ebooks ",
|
|
Paths: []string{t.TempDir()},
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("ScanFolder ebook error = %v, want nil", err)
|
|
}
|
|
if result == nil {
|
|
t.Fatal("ScanFolder ebook result = nil, want empty result")
|
|
}
|
|
}
|
|
|
|
func TestScanSubtreeEbookLibraryRoutesToEbookScanner(t *testing.T) {
|
|
subtree := t.TempDir()
|
|
|
|
scanner := &Scanner{}
|
|
result, err := scanner.ScanSubtree(context.Background(), &models.MediaFolder{
|
|
ID: 0,
|
|
Type: "ebook",
|
|
Paths: []string{subtree},
|
|
}, subtree)
|
|
if err != nil {
|
|
t.Fatalf("ScanSubtree ebook error = %v, want nil", err)
|
|
}
|
|
if result == nil {
|
|
t.Fatal("ScanSubtree ebook result = nil, want empty result")
|
|
}
|
|
}
|