From 60fcdff48b5940da81c1014e015c9d8e4351d8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Duran?= Date: Mon, 13 Jul 2026 13:19:09 +0200 Subject: [PATCH] Index: Finalize incremental XMP sidecar handling #2260 --- internal/photoprism/README.md | 4 +- internal/photoprism/index.go | 176 +++++++----- internal/photoprism/index_mediafile.go | 9 +- internal/photoprism/index_sidecar.go | 112 +++++--- internal/photoprism/index_sidecar_test.go | 333 +++++++++++++++------- 5 files changed, 427 insertions(+), 207 deletions(-) diff --git a/internal/photoprism/README.md b/internal/photoprism/README.md index eea63f7d4..683a3c930 100644 --- a/internal/photoprism/README.md +++ b/internal/photoprism/README.md @@ -1,6 +1,6 @@ ## PhotoPrism — Core Package -**Last Updated:** May 21, 2026 +**Last Updated:** July 13, 2026 ### Overview @@ -58,5 +58,5 @@ - Exec calls to external tools are parameterized by config paths/binaries (`config.Config`). - Stacking rules honor document IDs, time/place proximity, and configuration (`StackUUID`, `StackMeta`). - Forced rescans (`IndexOptions.Rescan=true`) run folder album reconciliation at the end of indexing via `entity.ReconcileOriginalsFolderAlbums(...)`; normal incremental runs skip this pass. -- Updated XMP sidecars are re-read on normal incremental passes: `Index.xmpSidecarChanged` compares each sidecar's on-disk modification time against the recorded `files.mod_time` (its last-parsed value), so external edits (darktable, digiKam) merge with `SrcXmp` priority without a forced rescan, while manual edits (`SrcManual`) are preserved. A sidecar that fails to parse keeps its previous `mod_time` so a fixed file is retried on a later pass. +- Updated or newly added XMP sidecars next to originals are re-read on normal incremental passes. The filesystem walk compares each sidecar's modification time with `files.mod_time`, resolves its primary from the Files cache, and queues deduplicated primary jobs only after a successful walk. External XMP edits merge with `SrcXmp` priority, while `SrcManual` values are preserved. A sidecar that fails to parse records the error and advances its `mod_time`, so it is retried only after another edit instead of on every pass. Incremental sidecar deletion is not supported, and automatic removal of stale XMP-derived metadata is not guaranteed by a forced rescan: fields such as `UUID`, `CameraSerial`, and primary `InstanceID` do not retain enough source information for complete reconciliation. - Folder create/index conflict lookup uses unscoped folder reads in `internal/entity/folder.go` so soft-deleted rows are detectable for troubleshooting instead of causing repeated create/find mismatches. diff --git a/internal/photoprism/index.go b/internal/photoprism/index.go index eaa1c9f94..253a9a58c 100644 --- a/internal/photoprism/index.go +++ b/internal/photoprism/index.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" "runtime/debug" + "sort" "sync" "time" @@ -170,8 +171,78 @@ func (ind *Index) Start(o IndexOptions) (found fs.Done, updated int) { log.Infof(`index: ignored "%s"`, fs.RelName(fileName, originalsPath)) } + // enqueueRelated queues unprocessed related files as one indexing job. + enqueueRelated := func(mf *MediaFile) { + related, relErr := mf.RelatedFiles(ind.conf.Settings().StackSequences()) + + if relErr != nil { + log.Warnf("index: %s", relErr) + return + } + + // Main media file is required to proceed. + if related.Main == nil { + return + } + + var files MediaFiles + skip := false + + // Check related files. + for _, f := range related.Files { + if found[f.FileName()].Processed() { + // Ignore already processed files. + continue + } else { + fileSize, limitErr := f.ExceedsBytes(o.ByteLimit) + + switch { + case fileSize == 0 || ind.files.Indexed(f.RootRelName(), f.Root(), f.ModTime(), o.Rescan): + // Flag file as found but not processed. + found[f.FileName()] = fs.Found + continue + case limitErr == nil: + // Add to file list. + files = append(files, f) + case related.Main.FileName() != f.FileName(): + // Sidecar file is too large, ignore. + log.Infof("index: %s", limitErr) + default: + // Main file is too large, skip all. + log.Warnf("index: %s", limitErr) + skip = true + } + } + + found[f.FileName()] = fs.Processed + } + + found[mf.FileName()] = fs.Processed + + // Skip if main file is too large or there are no files left to index. + if skip || len(files) == 0 { + return + } + + updated += len(files) + related.Files = files + + jobs <- IndexJob{ + FileName: mf.FileName(), + Related: related, + IndexOpt: o, + Ind: ind, + } + } + + changedXmpPrimaries := make(map[string]struct{}) + err := godirwalk.Walk(optionsPath, &godirwalk.Options{ ErrorCallback: func(fileName string, err error) godirwalk.ErrorAction { + if errors.Is(err, status.ErrCanceled) || errors.Is(err, status.ErrInsufficientStorage) { + return godirwalk.Halt + } + return godirwalk.SkipNode }, Callback: func(fileName string, info *godirwalk.Dirent) error { @@ -219,6 +290,17 @@ func (ind *Index) Start(o IndexOptions) (found fs.Done, updated int) { found[fileName] = fs.Found + // Defer changed XMP sidecars until all main files have been visited. + if fs.FileType(fileName) == fs.SidecarXMP { + if !ind.files.Indexed(relName, entity.RootOriginals, fs.ModTime(fileName), o.Rescan) { + if primaryRel := ind.primaryForSidecar(relName); primaryRel != "" { + changedXmpPrimaries[primaryRel] = struct{}{} + } + } + + return nil + } + if !media.MainFile(fileName) { return nil } @@ -241,8 +323,8 @@ func (ind *Index) Start(o IndexOptions) (found fs.Done, updated int) { return nil } - // Skip already indexed, unless an XMP sidecar was updated since its last parse. - if ind.files.Indexed(relName, entity.RootOriginals, mf.modTime, o.Rescan) && !ind.xmpSidecarChanged(mf, o) { + // Skip already indexed? + if ind.files.Indexed(relName, entity.RootOriginals, mf.modTime, o.Rescan) { return nil } @@ -264,68 +346,7 @@ func (ind *Index) Start(o IndexOptions) (found fs.Done, updated int) { log.Warnf("index: %s", err) } - // Find related files to index. - related, err := mf.RelatedFiles(ind.conf.Settings().StackSequences()) - - if err != nil { - log.Warnf("index: %s", err) - return nil - } - - var files MediaFiles - - // Main media file is required to proceed. - if related.Main == nil { - return nil - } - - skip := false - - // Check related files. - for _, f := range related.Files { - if found[f.FileName()].Processed() { - // Ignore already processed files. - continue - } else { - fileSize, limitErr := f.ExceedsBytes(o.ByteLimit) - - switch { - case fileSize == 0 || ind.files.Indexed(f.RootRelName(), f.Root(), f.ModTime(), o.Rescan): - // Flag file as found but not processed. - found[f.FileName()] = fs.Found - continue - case limitErr == nil: - // Add to file list. - files = append(files, f) - case related.Main.FileName() != f.FileName(): - // Sidecar file is too large, ignore. - log.Infof("index: %s", limitErr) - default: - // Main file is too large, skip all. - log.Warnf("index: %s", limitErr) - skip = true - } - } - - found[f.FileName()] = fs.Processed - } - - found[fileName] = fs.Processed - - // Skip if main file is too large or there are no files left to index. - if skip || len(files) == 0 { - return nil - } - - updated += len(files) - related.Files = files - - jobs <- IndexJob{ - FileName: mf.FileName(), - Related: related, - IndexOpt: o, - Ind: ind, - } + enqueueRelated(mf) return nil }, @@ -333,6 +354,33 @@ func (ind *Index) Start(o IndexOptions) (found fs.Done, updated int) { FollowSymbolicLinks: true, }) + // Queue sidecar-triggered jobs only after a complete, successful walk. + if err == nil && !mutex.IndexWorker.Canceled() { + primaries := make([]string, 0, len(changedXmpPrimaries)) + + for primaryRel := range changedXmpPrimaries { + primaries = append(primaries, primaryRel) + } + + sort.Strings(primaries) + + for _, primaryRel := range primaries { + if mutex.IndexWorker.Canceled() { + break + } + + primaryAbs := filepath.Join(originalsPath, primaryRel) + + if found[primaryAbs].Processed() { + continue + } + + if mf, mediaErr := NewMediaFile(primaryAbs); mediaErr == nil { + enqueueRelated(mf) + } + } + } + close(jobs) wg.Wait() diff --git a/internal/photoprism/index_mediafile.go b/internal/photoprism/index_mediafile.go index 40bb1d714..895002ebb 100644 --- a/internal/photoprism/index_mediafile.go +++ b/internal/photoprism/index_mediafile.go @@ -78,7 +78,6 @@ func (ind *Index) UserMediaFile(m *MediaFile, o IndexOptions, originalName, phot fileRenamed := false fileExists := false fileStacked := false - xmpParseFailed := false photoExists := false @@ -559,7 +558,6 @@ func (ind *Index) UserMediaFile(m *MediaFile, o IndexOptions, originalName, phot } else { log.Warn(dataErr.Error()) file.FileError = clip.Bytes(dataErr.Error(), txt.ClipError) - xmpParseFailed = true } case m.IsRaw(), m.IsImage(): if data := m.MetaData(); data.Error == nil { @@ -929,12 +927,7 @@ func (ind *Index) UserMediaFile(m *MediaFile, o IndexOptions, originalName, phot file.FileType = m.FileType().String() file.FileMime = m.ContentType() file.SetOrientation(m.Orientation(), entity.SrcMeta) - - // Preserve the previously recorded mtime when an XMP sidecar fails to parse, so a fixed - // sidecar is re-read on a future pass instead of being permanently silenced by one error. - if !xmpParseFailed { - file.ModTime = modTime.UTC().Truncate(time.Second).Unix() - } + file.ModTime = modTime.UTC().Truncate(time.Second).Unix() // Detect ICC color profile for JPEGs if still unknown at this point. if file.FileColorProfile == "" && fs.ImageJpeg.Equal(file.FileType) { diff --git a/internal/photoprism/index_sidecar.go b/internal/photoprism/index_sidecar.go index 7d1047e78..b01485f6b 100644 --- a/internal/photoprism/index_sidecar.go +++ b/internal/photoprism/index_sidecar.go @@ -1,43 +1,91 @@ package photoprism import ( + "path" + "sort" + "strings" + + "github.com/photoprism/photoprism/internal/entity" "github.com/photoprism/photoprism/pkg/fs" + "github.com/photoprism/photoprism/pkg/media" ) -// xmpSidecarChanged reports whether an XMP sidecar of the given media file is new or has a -// different modification time than the value recorded at its last index pass. It lets the -// indexer re-read externally edited sidecars (darktable, digiKam, …) on a normal incremental -// pass, even when the primary media file itself is unchanged. -// -// The recorded "last parsed mtime" is the sidecar's own files.mod_time, loaded into the Files -// cache by query.IndexedFiles, so an unchanged sidecar reads as indexed and a new or touched -// one reads as changed. -func (ind *Index) xmpSidecarChanged(mf *MediaFile, o IndexOptions) bool { - if mf == nil { +// xmpPrimaryExts lists sorted main-media extensions for XMP primary lookup. +var xmpPrimaryExts = mainFileExts() + +// mainFileExts returns sorted lowercase and uppercase main-media extensions. +func mainFileExts() []string { + known := make(map[string]struct{}, len(fs.Extensions)*2) + + for ext, fileType := range fs.Extensions { + if media.Formats[fileType].IsMain() { + known[strings.ToLower(ext)] = struct{}{} + known[strings.ToUpper(ext)] = struct{}{} + } + } + + exts := make([]string, 0, len(known)) + + for ext := range known { + exts = append(exts, ext) + } + + sort.Strings(exts) + + return exts +} + +// primaryForSidecar resolves an XMP primary from the originals Files cache. +func (ind *Index) primaryForSidecar(relName string) string { + // Full-name convention: "tok/photo.jpg.xmp" -> "tok/photo.jpg". + if primary := fs.StripExt(relName); media.MainFile(primary) { + if ind.files.Exists(primary, entity.RootOriginals) && ind.sidecarPrimaryEnabled(primary) { + return primary + } + + return "" + } + + // Base-prefix convention: "tok/photo.xmp" -> "tok/photo.". + dir := path.Dir(relName) + base := fs.BasePrefix(relName, false) + bases := []string{base, strings.ToLower(base), strings.ToUpper(base)} + seen := make(map[string]struct{}, len(bases)) + + for _, ext := range xmpPrimaryExts { + for _, candidateBase := range bases { + if _, ok := seen[candidateBase+ext]; ok { + continue + } + + seen[candidateBase+ext] = struct{}{} + candidate := path.Join(dir, candidateBase+ext) + + if ind.files.Exists(candidate, entity.RootOriginals) && ind.sidecarPrimaryEnabled(candidate) { + return candidate + } + } + } + + return "" +} + +// sidecarPrimaryEnabled reports whether a cached primary is enabled by the current config. +func (ind *Index) sidecarPrimaryEnabled(relName string) bool { + fileType := fs.FileType(relName) + + if !media.Formats[fileType].IsMain() { return false } - stripSequence := ind.conf.Settings().StackSequences() - - // Resolve candidate sidecars next to the original, covering both the "IMG_1234.xmp" and - // "IMG_1234.jpg.xmp" naming conventions. Detection is scoped to the same location the indexer's - // RelatedFiles actually discovers and merges sidecars from; XMPs in the sidecar/hidden paths are - // never read during indexing, so probing them would falsely re-trigger on every pass. - xmpFiles := fs.SidecarXMP.FindAll(mf.FileName(), nil, ind.conf.OriginalsPath(), stripSequence) - - for _, fileName := range xmpFiles { - f, err := NewMediaFileSkipResolve(fileName, fileName) - - if err != nil { - continue - } - - // Indexed returns false when no mtime is recorded yet (new sidecar) or the on-disk - // mtime differs from the recorded value (sidecar was updated). - if !ind.files.Indexed(f.RootRelName(), f.Root(), f.ModTime(), o.Rescan) { - return true - } + switch { + case (fileType == fs.ImageRaw || fileType == fs.ImageDng) && ind.conf.DisableRaw(): + return false + case fileType == fs.ImageJpegXL && ind.conf.DisableJpegXL(): + return false + case media.Formats[fileType] == media.Vector && ind.conf.DisableVectors(): + return false + default: + return true } - - return false } diff --git a/internal/photoprism/index_sidecar_test.go b/internal/photoprism/index_sidecar_test.go index 3f38ec8ee..2ea9227d4 100644 --- a/internal/photoprism/index_sidecar_test.go +++ b/internal/photoprism/index_sidecar_test.go @@ -3,6 +3,7 @@ package photoprism import ( "os" "path/filepath" + "sort" "testing" "time" @@ -53,98 +54,190 @@ func writeSidecar(t *testing.T, path, content string, modUnix int64) { } } -func TestIndex_xmpSidecarChanged(t *testing.T) { - cfg := newIndexRelatedTestConfig(t, "index-sidecar-changed") +func TestMainFileExts(t *testing.T) { + exts := mainFileExts() + assert.True(t, sort.StringsAreSorted(exts)) + assert.Contains(t, exts, ".jpg") + assert.Contains(t, exts, ".JPG") + assert.NotContains(t, exts, ".xmp") + assert.NotContains(t, exts, ".XMP") +} + +func TestIndex_sidecarPrimaryEnabled(t *testing.T) { + cfg := newIndexRelatedTestConfig(t, "index-sidecar-primary-enabled") ind := NewIndex(cfg, NewConvert(cfg), NewFiles(), NewPhotos()) - opt := IndexOptionsNone(cfg) - // Copy a primary file into an isolated originals subfolder. - token := rnd.Base36(8) - testPath := filepath.Join(cfg.OriginalsPath(), token) - jpgPath := filepath.Join(testPath, "photo.jpg") - - src, err := NewMediaFile("testdata/2015-02-04.jpg") - if err != nil { - t.Fatal(err) - } - if err = src.Copy(jpgPath, false); err != nil { - t.Fatal(err) - } - - mf, err := NewMediaFile(jpgPath) - if err != nil { - t.Fatal(err) - } - - t.Run("NoSidecar", func(t *testing.T) { - assert.False(t, ind.xmpSidecarChanged(mf, opt)) + t.Run("Jpeg", func(t *testing.T) { + assert.True(t, ind.sidecarPrimaryEnabled("photo.jpg")) }) - - xmpPath := filepath.Join(testPath, "photo.xmp") - writeSidecar(t, xmpPath, xmpSidecarDoc("T", "C", "", ""), 1700000000) - - xf, err := NewMediaFileSkipResolve(xmpPath, xmpPath) - if err != nil { - t.Fatal(err) - } - - t.Run("NewSidecar", func(t *testing.T) { - // No mtime recorded yet ⇒ treated as needing a parse. - assert.True(t, ind.xmpSidecarChanged(mf, opt)) + t.Run("Unknown", func(t *testing.T) { + assert.False(t, ind.sidecarPrimaryEnabled("photo.unknown")) }) - t.Run("Unchanged", func(t *testing.T) { - // Record the sidecar's current mtime, then it must read as unchanged. - ind.files.Ignore(xf.RootRelName(), xf.Root(), xf.ModTime(), false) - assert.False(t, ind.xmpSidecarChanged(mf, opt)) + t.Run("DisabledRaw", func(t *testing.T) { + cfg.Options().DisableRaw = true + assert.False(t, ind.sidecarPrimaryEnabled("photo.nef")) }) - t.Run("Changed", func(t *testing.T) { - // Advance the sidecar's mtime past the recorded value. - stamp := time.Unix(1700000060, 0) - if err = os.Chtimes(xmpPath, stamp, stamp); err != nil { - t.Fatal(err) - } - assert.True(t, ind.xmpSidecarChanged(mf, opt)) + t.Run("DisabledJpegXL", func(t *testing.T) { + cfg.Options().DisableJpegXL = true + assert.False(t, ind.sidecarPrimaryEnabled("photo.jxl")) }) - t.Run("NilMediaFile", func(t *testing.T) { - assert.False(t, ind.xmpSidecarChanged(nil, opt)) + t.Run("DisabledVector", func(t *testing.T) { + cfg.Options().DisableVectors = true + assert.False(t, ind.sidecarPrimaryEnabled("photo.svg")) }) } -func TestIndex_xmpSidecarChanged_SidecarPathIgnored(t *testing.T) { - cfg := newIndexRelatedTestConfig(t, "index-sidecar-path") +func TestIndex_primaryForSidecar(t *testing.T) { + cfg := newIndexRelatedTestConfig(t, "index-primary-for-sidecar") ind := NewIndex(cfg, NewConvert(cfg), NewFiles(), NewPhotos()) - opt := IndexOptionsNone(cfg) + modTime := time.Unix(1700000000, 0) + // Seed the Files cache with indexed primaries directly (no filesystem access needed). + seed := func(relName, root string) { + ind.files.Ignore(relName, root, modTime, false) + } + seed("tok/photo.jpg", entity.RootOriginals) + seed("tok/pic.jpeg", entity.RootOriginals) + seed("root.JPG", entity.RootOriginals) + seed("clip.mp4", entity.RootOriginals) + seed("case/Photo.JPG", entity.RootOriginals) + seed("case/lower.JPG", entity.RootOriginals) + seed("case/UPPER.jpg", entity.RootOriginals) + seed("same/multi.jpg", entity.RootOriginals) + seed("same/multi.nef", entity.RootOriginals) + seed("fallback/orphan.nef", entity.RootOriginals) + seed("disabled/capture.nef", entity.RootOriginals) + seed("disabled/full.nef", entity.RootOriginals) + seed("disabled/full.jpg", entity.RootOriginals) + seed("disabled/wide.jxl", entity.RootOriginals) + seed("disabled/art.svg", entity.RootOriginals) + seed("side/only.jpg", entity.RootSidecar) + + t.Run("FullNameConvention", func(t *testing.T) { + // "tok/photo.jpg.xmp" -> "tok/photo.jpg" by stripping the trailing extension. + assert.Equal(t, "tok/photo.jpg", ind.primaryForSidecar("tok/photo.jpg.xmp")) + }) + t.Run("BasePrefixConvention", func(t *testing.T) { + // "tok/photo.xmp" -> "tok/photo.jpg" via the candidate-extension scan. + assert.Equal(t, "tok/photo.jpg", ind.primaryForSidecar("tok/photo.xmp")) + }) + t.Run("BasePrefixJpeg", func(t *testing.T) { + assert.Equal(t, "tok/pic.jpeg", ind.primaryForSidecar("tok/pic.xmp")) + }) + t.Run("RootLevelImage", func(t *testing.T) { + assert.Equal(t, "root.JPG", ind.primaryForSidecar("root.xmp")) + }) + t.Run("RootLevelVideo", func(t *testing.T) { + assert.Equal(t, "clip.mp4", ind.primaryForSidecar("clip.xmp")) + }) + t.Run("ExactBaseCase", func(t *testing.T) { + assert.Equal(t, "case/Photo.JPG", ind.primaryForSidecar("case/Photo.xmp")) + }) + t.Run("LowercaseBaseVariant", func(t *testing.T) { + assert.Equal(t, "case/lower.JPG", ind.primaryForSidecar("case/LOWER.xmp")) + }) + t.Run("UppercaseBaseVariant", func(t *testing.T) { + assert.Equal(t, "case/UPPER.jpg", ind.primaryForSidecar("case/upper.xmp")) + }) + t.Run("MultipleCandidates", func(t *testing.T) { + // The resolver only selects a deterministic group entry; RelatedFiles chooses the main. + assert.Equal(t, "same/multi.jpg", ind.primaryForSidecar("same/multi.xmp")) + }) + t.Run("OrphanSidecar", func(t *testing.T) { + assert.Equal(t, "", ind.primaryForSidecar("tok/missing.xmp")) + }) + t.Run("FullNameOrphanDoesNotFallback", func(t *testing.T) { + assert.Equal(t, "", ind.primaryForSidecar("fallback/orphan.jpg.xmp")) + }) + t.Run("SidecarRootIgnored", func(t *testing.T) { + // A primary indexed only under the sidecar root must not match originals-scoped detection. + assert.Equal(t, "", ind.primaryForSidecar("side/only.xmp")) + }) + t.Run("DisabledRaw", func(t *testing.T) { + cfg.Options().DisableRaw = true + assert.Equal(t, "", ind.primaryForSidecar("disabled/capture.xmp")) + assert.Equal(t, "", ind.primaryForSidecar("disabled/capture.nef.xmp")) + assert.Equal(t, "", ind.primaryForSidecar("disabled/full.nef.xmp")) + }) + t.Run("DisabledJpegXL", func(t *testing.T) { + cfg.Options().DisableJpegXL = true + assert.Equal(t, "", ind.primaryForSidecar("disabled/wide.xmp")) + assert.Equal(t, "", ind.primaryForSidecar("disabled/wide.jxl.xmp")) + }) + t.Run("DisabledVector", func(t *testing.T) { + cfg.Options().DisableVectors = true + assert.Equal(t, "", ind.primaryForSidecar("disabled/art.xmp")) + assert.Equal(t, "", ind.primaryForSidecar("disabled/art.svg.xmp")) + }) +} + +func TestIndex_Start_XmpSidecarAfterWalk(t *testing.T) { + // Register a test-only main extension that sorts after .xmp. + const testExt = ".zzz" + previousType, typeExisted := fs.Extensions[testExt] + previousPrimaryExts := xmpPrimaryExts + fs.Extensions[testExt] = fs.ImageJpeg + xmpPrimaryExts = mainFileExts() + t.Cleanup(func() { + if typeExisted { + fs.Extensions[testExt] = previousType + } else { + delete(fs.Extensions, testExt) + } + xmpPrimaryExts = previousPrimaryExts + }) + + cfg := newIndexRelatedTestConfig(t, "index-sidecar-after-walk") + prevConf := Config() + SetConfig(cfg) + defer SetConfig(prevConf) + + ind := NewIndex(cfg, NewConvert(cfg), NewFiles(), NewPhotos()) + opt := NewIndexOptions("/", false, false, false, false, false, cfg) token := rnd.Base36(8) - jpgPath := filepath.Join(cfg.OriginalsPath(), token, "photo.jpg") + testPath := filepath.Join(cfg.OriginalsPath(), token) + jpgPath := filepath.Join(testPath, "photo"+testExt) + xmpPath := filepath.Join(testPath, "photo.xmp") - src, err := NewMediaFile("testdata/2015-02-04.jpg") + src, err := NewMediaFile("testdata/apple-test-2.jpg") if err != nil { t.Fatal(err) } if err = src.Copy(jpgPath, false); err != nil { t.Fatal(err) } - mf, err := NewMediaFile(jpgPath) - if err != nil { + + writeSidecar(t, xmpPath, xmpSidecarDoc("Title One", "Caption One", "", ""), 1700000000) + stamp := time.Unix(1700000000, 0) + if err = os.Chtimes(jpgPath, stamp, stamp); err != nil { + t.Fatal(err) + } + if _, updated := ind.Start(opt); updated != 2 { + t.Fatalf("expected initial related job to process 2 files, got %d", updated) + } + + // photo.xmp sorts before photo.zzz, so the second pass exercises deferred sidecar handling. + writeSidecar(t, xmpPath, xmpSidecarDoc("Title Two", "Caption Two", "", ""), 1700000100) + stamp = time.Unix(1700000100, 0) + if err = os.Chtimes(jpgPath, stamp, stamp); err != nil { t.Fatal(err) } - // A new XMP in the dedicated sidecar path (not next to the original) must be ignored, since the - // indexer's RelatedFiles never reads it — otherwise detection would re-trigger on every pass. - sidecarXmp := filepath.Join(cfg.SidecarPath(), token, "photo.xmp") - if err = os.MkdirAll(filepath.Dir(sidecarXmp), fs.ModeDir); err != nil { - t.Fatal(err) + found, updated := ind.Start(opt) + assert.Equal(t, 2, updated) + assert.True(t, found[jpgPath].Processed()) + assert.True(t, found[xmpPath].Processed()) + + var file entity.File + if dbErr := entity.UnscopedDb().First(&file, "file_name = ?", filepath.Join(token, "photo"+testExt)).Error; dbErr != nil { + t.Fatal(dbErr) } - writeSidecar(t, sidecarXmp, xmpSidecarDoc("T", "C", "", ""), 1700000000) - t.Run("SidecarPathIgnored", func(t *testing.T) { - assert.False(t, ind.xmpSidecarChanged(mf, opt)) - }) - t.Run("OriginalsAdjacentDetected", func(t *testing.T) { - // Sanity check: an adjacent sidecar is still detected. - writeSidecar(t, filepath.Join(cfg.OriginalsPath(), token, "photo.xmp"), xmpSidecarDoc("T", "C", "", ""), 1700000000) - assert.True(t, ind.xmpSidecarChanged(mf, opt)) - }) + photo, dbErr := query.PhotoByUID(file.PhotoUID) + if dbErr != nil { + t.Fatal(dbErr) + } + assert.Equal(t, "Caption Two", photo.PhotoCaption) + assert.Equal(t, entity.SrcXmp, photo.CaptionSrc) } func TestIndex_Start_XmpSidecarReread(t *testing.T) { @@ -203,15 +296,19 @@ func TestIndex_Start_XmpSidecarReread(t *testing.T) { return f } - // Pass 1: initial index with a valid sidecar. The GPS coordinates differ from the JPEG's - // embedded EXIF GPS so the assertions prove the sidecar (src: xmp) wins over src: meta. - writeSidecar(t, xmpPath, xmpSidecarDoc("Title One", "Caption One", "35.6762", "139.6503"), 1700000000) - + // Pass 0: index the primary while no sidecar exists yet. if _, updated := ind.Start(opt); updated == 0 { t.Fatal("expected initial index to process at least one file") } - t.Run("InitialXmpApplied", func(t *testing.T) { + // Pass 1: a sidecar added next to the already-indexed, otherwise-unchanged primary must be + // detected and merged on a plain incremental run, without a forced rescan. The GPS coordinates + // differ from the JPEG's embedded EXIF GPS so the assertions prove the sidecar (src: xmp) wins + // over src: meta. + t.Run("NewSidecarDetectedAndApplied", func(t *testing.T) { + writeSidecar(t, xmpPath, xmpSidecarDoc("Title One", "Caption One", "35.6762", "139.6503"), 1700000000) + _, updated := ind.Start(opt) + assert.Greater(t, updated, 0) p := reloadPhoto() assert.Equal(t, "Caption One", p.PhotoCaption) assert.Equal(t, entity.SrcXmp, p.CaptionSrc) @@ -242,7 +339,7 @@ func TestIndex_Start_XmpSidecarReread(t *testing.T) { // Manual edits: set the title and caption from the UI (src: manual). if dbErr := entity.UnscopedDb().Model(&entity.Photo{}). Where("photo_uid = ?", reloadPhoto().PhotoUID). - Updates(map[string]interface{}{ + Updates(entity.Values{ "photo_title": "Manual Title", "title_src": entity.SrcManual, "photo_caption": "Manual Caption", @@ -267,47 +364,81 @@ func TestIndex_Start_XmpSidecarReread(t *testing.T) { assert.Equal(t, int64(1700000200), xmpRow().ModTime) }) - // Pass 5: a malformed sidecar must not change metadata or advance the recorded mtime. - t.Run("MalformedSidecarNotAdvanced", func(t *testing.T) { + // Pass 5: a malformed sidecar records the parse error and advances its mtime to the on-disk + // value, so it is retried only when edited again (not on every pass) and already-merged + // metadata is left untouched. + t.Run("MalformedSidecarRecordsError", func(t *testing.T) { writeSidecar(t, xmpPath, "this is not valid xml", 1700000300) - _, _ = ind.Start(opt) + _, updated := ind.Start(opt) + assert.Greater(t, updated, 0) p := reloadPhoto() assert.Equal(t, "Manual Caption", p.PhotoCaption) assert.InDelta(t, 48.858, p.PhotoLat, 0.01) row := xmpRow() - // mod_time stays at the last successful parse so a fixed file is retried. - assert.Equal(t, int64(1700000200), row.ModTime) + // mtime advances to the on-disk value and the parse error is recorded. + assert.Equal(t, int64(1700000300), row.ModTime) assert.NotEqual(t, "", row.FileError) }) - // Pass 6: a fixed sidecar carrying an unsupported tag (xmp:Rating) is retried, parses without - // error, advances the recorded mtime, and changes no PhotoPrism field (the rating is ignored). - t.Run("UnsupportedTagIgnoredAndRetried", func(t *testing.T) { - ratingDoc := ` - - - 4 - Caption Four - 48.8583701 - N - 2.2944813 - E - - -` - writeSidecar(t, xmpPath, ratingDoc, 1700000400) + // Pass 6: the unchanged malformed sidecar must not be retried on every incremental run. + t.Run("UnchangedMalformedSidecarSkipped", func(t *testing.T) { + _, updated := ind.Start(opt) + assert.Equal(t, 0, updated) + row := xmpRow() + assert.Equal(t, int64(1700000300), row.ModTime) + assert.NotEqual(t, "", row.FileError) + }) + + // Pass 7: changing the malformed file makes it eligible again and clears the stored error. + t.Run("FixedSidecarClearsError", func(t *testing.T) { + writeSidecar(t, xmpPath, xmpSidecarDoc("Title Four", "Caption Four", "51.5007", "0.1246"), 1700000400) _, updated := ind.Start(opt) assert.Greater(t, updated, 0) p := reloadPhoto() - // Manual title/caption stay; no field is set from the rating tag. + // Manual title and caption stay while the non-manual location is updated. assert.Equal(t, "Manual Title", p.PhotoTitle) assert.Equal(t, "Manual Caption", p.PhotoCaption) + assert.InDelta(t, 51.5007, p.PhotoLat, 0.01) row := xmpRow() - // Valid parse ⇒ error cleared and mtime advanced (the previously malformed file was retried). assert.Equal(t, "", row.FileError) assert.Equal(t, int64(1700000400), row.ModTime) }) } + +func TestIndex_Start_XmpSidecarScope(t *testing.T) { + cfg := newIndexRelatedTestConfig(t, "index-sidecar-scope") + prevConf := Config() + SetConfig(cfg) + defer SetConfig(prevConf) + + ind := NewIndex(cfg, NewConvert(cfg), NewFiles(), NewPhotos()) + opt := NewIndexOptions("/", false, false, false, false, false, cfg) + token := rnd.Base36(8) + testPath := filepath.Join(cfg.OriginalsPath(), token) + jpgPath := filepath.Join(testPath, "photo.jpg") + + src, err := NewMediaFile("testdata/apple-test-2.jpg") + if err != nil { + t.Fatal(err) + } + if err = src.Copy(jpgPath, false); err != nil { + t.Fatal(err) + } + if _, updated := ind.Start(opt); updated == 0 { + t.Fatal("expected initial index to process the primary") + } + + dedicatedPath := filepath.Join(cfg.SidecarPath(), token) + hiddenPath := filepath.Join(cfg.OriginalsPath(), fs.PPHiddenPathname, token) + if err = os.MkdirAll(dedicatedPath, fs.ModeDir); err != nil { + t.Fatal(err) + } + if err = os.MkdirAll(hiddenPath, fs.ModeDir); err != nil { + t.Fatal(err) + } + writeSidecar(t, filepath.Join(dedicatedPath, "photo.xmp"), xmpSidecarDoc("Dedicated", "Dedicated", "", ""), 1700000000) + writeSidecar(t, filepath.Join(hiddenPath, "photo.xmp"), xmpSidecarDoc("Hidden", "Hidden", "", ""), 1700000000) + + _, updated := ind.Start(opt) + assert.Equal(t, 0, updated) +}