diff --git a/frontend/src/component/lightbox.vue b/frontend/src/component/lightbox.vue index abc12af27..3878714ff 100644 --- a/frontend/src/component/lightbox.vue +++ b/frontend/src/component/lightbox.vue @@ -1706,20 +1706,23 @@ export default { // Ensure that content is focused. this.focusContent(); }, - // Fetches the full Photo model for the given UID using the LRU cache. - // Restricted roles (guest, visitor, contributor) skip the extra API - // call and let the sidebar work with the viewer data (Thumb model). + // Fetches the full Photo model for the given UID using the LRU + // cache, delegated to the Thumb model so the photo-fetch policy + // lives on the slide that owns it (Thumb.loadPhoto). Restricted + // roles (guest, visitor, contributor) skip the extra API call + // and let the sidebar work with the viewer data (Thumb model). fetchPhoto(uid) { if (!uid || this.$session.isSidebarRestricted()) { this.photo = new Photo(); return; } - Photo.findCached(uid) - .then((photo) => { + this.model + .loadPhoto() + .then((m) => { // Only apply if still showing this photo (prevents race on fast swiping). if (this.model && this.model.UID === uid) { - this.photo = photo; + this.photo = m; } }) .catch(() => {}); @@ -2600,16 +2603,17 @@ export default { return; } - this.model.Removed = true; - - $api - .delete(`albums/${this.collection.UID}/photos`, { data: { photos: [this.model.UID] } }) + this.model + .removeFromAlbum(this.collection.UID) .then(() => { - Photo.evictCache(this.model.UID); + // Album-remove publishes only albums.updated, not a photos + // event — manual eviction stays so the sidebar's cached + // Photo.Albums field doesn't surface stale membership. + // Optimistic Removed flip + rollback are handled inside + // Thumb.removeFromAlbum. + this.model.evictPhoto(); }) - .catch(() => { - this.model.Removed = false; - }); + .catch(() => {}); }, onArchive() { if (!this.canArchive) { @@ -2623,10 +2627,10 @@ export default { return; } - this.model.Archived = true; - - return $api.post("batch/photos/archive", { photos: [this.model.UID] }).then(() => { - Photo.evictCache(this.model.UID); + // Optimistic Archived flip + rollback live in Thumb.archive. + // Cache eviction is handled by the photos.archived WS + // subscriber in model/photo.js — no manual evictPhoto() here. + return this.model.archive().then(() => { this.$notify.success(this.$gettext("Archived")); }); }, @@ -2642,10 +2646,10 @@ export default { return; } - this.model.Archived = false; - - $api.post("batch/photos/restore", { photos: [this.model.UID] }).then(() => { - Photo.evictCache(this.model.UID); + // Optimistic Archived flip + rollback live in Thumb.restore. + // Cache eviction is handled by the photos.restored WS + // subscriber in model/photo.js — no manual evictPhoto() here. + this.model.restore().then(() => { this.$notify.success(this.$gettext("Restored")); }); }, diff --git a/frontend/src/model/photo.js b/frontend/src/model/photo.js index f46afbe0a..d5a0b14f5 100644 --- a/frontend/src/model/photo.js +++ b/frontend/src/model/photo.js @@ -1295,11 +1295,13 @@ export class Photo extends RestModel { return Photo._cache; } - // Removes a photo from the LRU cache. Mutating methods on this model no - // longer call this directly: the backend publishes "photos.updated" and - // "photos.deleted" via websocket and the cache is refreshed/evicted from - // there (see the module-level subscriptions below). This stays public as - // an escape hatch for code that mutates a photo outside of this model. + // Removes a photo from the LRU cache. Mutating methods on this model + // no longer call this directly: the backend publishes "photos.updated" + // / "photos.deleted" / "photos.archived" / "photos.restored" via + // websocket and the cache is evicted from there (see the module-level + // subscriptions below). This stays public as an escape hatch for flows + // that mutate a photo without firing one of those events — currently + // album-membership changes, which only emit "albums.updated". static evictCache(uid) { if (uid) { Photo._cache.evict(uid); @@ -1308,9 +1310,12 @@ export class Photo extends RestModel { // Drops every cached photo and any in-flight request. Called on session // reset so metadata fetched under one role cannot be served to another. - // Note: in-flight loader Promises are not aborted (see Open Question #1 - // in the cache proposal); a fetch resolving after clearCache() may still - // re-seed the cache under its own key. + // ModelCache.clear() bumps an internal session-epoch counter and any + // in-flight fetch whose epoch no longer matches REJECTS with + // ModelCacheStaleFetchError instead of resolving — so neither the + // cache nor a .then-chained UI assignment can leak role-A data into + // role B during the post-logout unmount window. See + // specs/frontend/model-lru-cache.md Decisions §5 for the design. static clearCache() { Photo._cache.clear(); } @@ -1354,41 +1359,42 @@ export class Photo extends RestModel { } } -// Evict cached entries when the backend reports a photo change. The -// websocket payload (see PublishPhotoEvent in internal/api/api_event.go) -// is a search.Photos result, NOT a full /photos/:uid entity — it flattens -// nested fields like Details into top-level columns (DetailsKeywords, -// DetailsSubject, ...) and would silently drop the nested Details object -// if written back via refreshIfPresent. Hydrating from that snapshot -// would leave Photo.Details === undefined, which collapses the sidebar's -// isEditable computed and disables editing on the next cache hit. +// Drops cached entries from the WS event payload. The backend uses +// two different shapes on the same channel family — handle both: // -// So the WS update channel is consumed as an EVICT signal: the next read -// goes back to find() and gets the field-complete entity. The local -// Photo instance held by lightbox.vue is unaffected by this eviction — -// it was already populated from the field-complete PUT response — so -// there is no visible flash on the currently-displayed slide. -$event.subscribe("photos.updated", (_ev, data) => { +// - "photos.updated" (PublishPhotoEvent in internal/api/api_event.go) +// emits a search.Photos result: an array of objects with .UID. +// - "photos.archived" / "photos.restored" / "photos.deleted" +// (EntitiesArchived / EntitiesRestored / EntitiesDeleted in +// internal/event/publish_entities.go) emit a []string of bare UIDs. +// +// A single helper covers both so we don't have to keep them in sync. +// The "photos.updated" payload is consumed as an EVICT signal (not a +// refresh) because search.Photos flattens nested fields like Details +// into top-level columns (DetailsKeywords, DetailsSubject, ...); +// hydrating from that snapshot would leave Photo.Details === undefined +// and collapse the sidebar's isEditable computed. Eviction sends the +// next read back to find() and the field-complete /photos/:uid endpoint. +function evictCachedFromEntities(data) { if (!data || !Array.isArray(data.entities)) { return; } data.entities.forEach((entity) => { - if (entity && entity.UID) { + if (typeof entity === "string" && entity) { + Photo._cache.evict(entity); + } else if (entity && typeof entity === "object" && entity.UID) { Photo._cache.evict(entity.UID); } }); -}); +} -// Drop cached entries for photos the backend reports as removed. -$event.subscribe("photos.deleted", (_ev, data) => { - if (!data || !Array.isArray(data.entities)) { - return; - } - data.entities.forEach((entity) => { - if (entity && entity.UID) { - Photo._cache.evict(entity.UID); - } - }); -}); +// Subscribe once per channel. Adding "archived" and "restored" here +// retires the per-mutation Photo.evictCache calls that lightbox.vue +// previously made after onArchive / onRestore — the WS round-trip +// covers it for every consumer in this tab. +$event.subscribe("photos.updated", (_ev, data) => evictCachedFromEntities(data)); +$event.subscribe("photos.deleted", (_ev, data) => evictCachedFromEntities(data)); +$event.subscribe("photos.archived", (_ev, data) => evictCachedFromEntities(data)); +$event.subscribe("photos.restored", (_ev, data) => evictCachedFromEntities(data)); export default Photo; diff --git a/frontend/src/model/thumb.js b/frontend/src/model/thumb.js index 82794850f..6fca62609 100644 --- a/frontend/src/model/thumb.js +++ b/frontend/src/model/thumb.js @@ -1,4 +1,5 @@ import Model from "model.js"; +import Photo from "model/photo"; import $api from "common/api"; import $util from "common/util"; import { $config } from "app/session.js"; @@ -8,6 +9,13 @@ const thumbs = window.__CONFIG__.thumbs; // Thumb represents a lightweight slide/photo preview record used by the lightbox. export class Thumb extends Model { + // Returns the default field shape for a Thumb. These fields are + // all reactive once the instance is wrapped by Vue's data() proxy + // and define the snapshot served when no server data is available. + // `Archived` and `Removed` are intentionally NOT declared here so + // the lightbox's tri-state visibility checks (e.g. the explicit + // `this.model?.Archived === false` at lightbox.vue:1437) can + // distinguish "never set" from "explicitly not archived". getDefaults() { return { UID: "", @@ -31,6 +39,9 @@ export class Thumb extends Model { }; } + // Returns the canonical identifier for this slide, preferring UID + // over numeric ID. Returns `false` when neither is set so callers + // can branch on truthiness. getId() { if (this.UID) { return this.UID; @@ -39,10 +50,17 @@ export class Thumb extends Model { return this.ID ? this.ID : false; } + // Convenience predicate around getId() — true when this Thumb + // represents a real photo (vs. a notFound() placeholder). hasId() { return !!this.getId(); } + // Toggles the favorite flag and posts/deletes the like to the + // backend. Flips `Favorite` synchronously first so the heart icon + // re-renders immediately; the API call is fire-and-forget. No + // rollback on failure — matches the existing optimistic-toggle + // pattern used elsewhere in the frontend. toggleLike() { this.Favorite = !this.Favorite; @@ -53,6 +71,88 @@ export class Thumb extends Model { } } + // Resolves to the full Photo entity for this slide, fetched via + // the shared LRU cache. Each call returns a fresh hydrated + // instance so consumers can mutate locally without aliasing the + // cached snapshot. Resolves to an empty Photo placeholder when + // this thumb has no UID. Lightbox uses this to load sidebar + // metadata for the current slide and to warm neighbours via + // Photo.prefetchAround. Rejections (e.g. ModelCacheStaleFetchError + // after a logout-clear) propagate to callers' .catch handlers. + loadPhoto() { + if (!this.UID) { + return Promise.resolve(new Photo()); + } + return Photo.findCached(this.UID); + } + + // Drops the cached Photo entity for this slide so the next + // loadPhoto() rehydrates from GET /photos/:uid. Used by flows + // that mutate the photo without firing a photos.* WS event the + // cache subscribes to (currently: album-membership changes, + // which only publish albums.updated). archive / restore / delete + // already auto-evict via the photo.js WS subscribers, so call + // sites for those don't need this. + evictPhoto() { + if (this.UID) { + Photo.evictCache(this.UID); + } + } + + // Moves this photo to the archive (soft delete) and flips the + // local Archived flag immediately so menu buttons re-render + // without waiting for the API round-trip. The pre-call value of + // Archived is captured in a closure and restored on rejection — + // a literal `false` rollback would be wrong if the field was + // undefined (defaults aren't declared) or already true (no-op + // re-archive). Resolves on success; the backend publishes + // photos.archived which the photo cache subscribes to for + // automatic eviction. Mirrors the batch-photos endpoint that + // view/cards.vue already targets via Photo.prototype.archive — + // kept on Thumb for the lightbox flow which holds a Thumb (not a + // Photo) for the current slide. + archive() { + const prev = this.Archived; + this.Archived = true; + return $api.post("batch/photos/archive", { photos: [this.UID] }).catch((err) => { + this.Archived = prev; + throw err; + }); + } + + // Restores this photo from the archive. Captures the pre-call + // Archived value and restores it on rejection (mirroring + // archive()) so a no-op restore on an already-restored photo + // doesn't leave Archived === true. Resolves on success; the + // backend publishes photos.restored for automatic cache eviction. + restore() { + const prev = this.Archived; + this.Archived = false; + return $api.post("batch/photos/restore", { photos: [this.UID] }).catch((err) => { + this.Archived = prev; + throw err; + }); + } + + // Removes this photo from the given album. Optimistic flip on + // Removed (drives menu visibility) with previous-value rollback + // on rejection. Backend publishes only albums.updated (not a + // photos event), so callers that mutate the sidebar's cached + // Photo.Albums list MUST also call evictPhoto() — see + // lightbox.vue onAlbumRemove for the pattern. + removeFromAlbum(albumUID) { + const prev = this.Removed; + this.Removed = true; + return $api.delete(`albums/${albumUID}/photos`, { data: { photos: [this.UID] } }).catch((err) => { + this.Removed = prev; + throw err; + }); + } + + // Formats Lat/Lng as an EXIF-style coordinate pair separated by + // an em-space (U+2003) — see the literal in the body. Returns a + // 0/0 placeholder when coordinates are missing so the sidebar + // EXIF row doesn't collapse. getLatLng() { if (!this.Lat || !this.Lng) { return `0°N\u20030°E`; @@ -61,6 +161,9 @@ export class Thumb extends Model { return `${this.Lat.toFixed(5)}°N\u2003${this.Lng.toFixed(5)}°E`; } + // Copies Lat/Lng to the system clipboard as `lat,lng` decimals so + // they paste cleanly into mapping tools. No-ops when coordinates + // are missing rather than copying a misleading "0,0". copyLatLng() { // Abort if latitude or longitude are not set. if (!this.Lat || !this.Lng) { @@ -71,6 +174,10 @@ export class Thumb extends Model { $util.copyText(`${this.Lat.toString()},${this.Lng.toString()}`); } + // Returns a rounded megapixel string (e.g. "12.0MP") for the + // type-info row. Returns the literal "0.0MP" — not 0 or empty — + // when dimensions are unknown; getTypeInfo() uses that sentinel + // to decide whether to skip the MP segment. getMegaPixels() { if (!this.Width || !this.Height) { return "0.0MP"; @@ -79,6 +186,9 @@ export class Thumb extends Model { return `${((this.Width * this.Height) / 1000000).toFixed(1)}MP`; } + // Returns the Material Design icon name for this slide's media + // type, used by the lightbox type chip. Falls back to a generic + // image icon for unknown types. getTypeIcon() { switch (this.Type) { case "raw": @@ -98,6 +208,12 @@ export class Thumb extends Model { } } + // Builds the EXIF-summary string shown next to the type chip + // (codec / megapixels / dimensions, joined by em-spaces). The + // segment order varies by media type so the most useful field + // leads — duration first for video, codec for raw, etc. Returns + // localized "Document" for documents and may return an empty + // string when nothing useful is known. getTypeInfo() { let info = []; const mp = this.getMegaPixels(); @@ -168,6 +284,12 @@ export class Thumb extends Model { return info.join("\u2003"); } + // Returns a placeholder Thumb-shaped object for slides that can't + // be rendered (missing hash, deleted file, etc.). Every Thumbs + // entry points at the static 404 image so the lightbox grid stays + // visually consistent without throwing on missing assets. Returns + // a plain object (not a Thumb instance) — callers that need an + // instance wrap the result themselves. static notFound() { const result = { UID: "", @@ -203,6 +325,9 @@ export class Thumb extends Model { return result; } + // Builds a Thumb array from a Photos search response. Each photo + // flows through fromPhoto(), which picks the best available file + // for thumbnail rendering (RAW/Live preferred over JPEG). static fromPhotos(photos) { let result = []; const n = photos.length; @@ -214,6 +339,11 @@ export class Thumb extends Model { return result; } + // Constructs a Thumb from a Photo entity, choosing the original + // file (RAW/Live preferred over JPEG via Photo.originalFile()) to + // source hash/dimensions/codec. Falls back to the Photo's own + // top-level fields when no Files are available. Returns a + // notFound() placeholder when neither hash nor files exist. static fromPhoto(photo) { if (!photo || (!photo.Hash && !photo.Files?.length)) { return this.notFound(); @@ -274,6 +404,11 @@ export class Thumb extends Model { return new this(result); } + // Constructs a Thumb from a specific File belonging to a Photo — + // used by the file-list view to surface each file as its own + // slide. The Photo provides title / location / EXIF metadata; the + // File provides hash / dimensions / codec. Returns notFound() + // when any required input or hash is missing. static fromFile(photo, file) { if (!photo || !file || !file.Hash) { return this.notFound(); @@ -314,10 +449,17 @@ export class Thumb extends Model { return new this(result); } + // Wraps an array of plain Thumb-shaped values as Thumb instances. + // Used by endpoints that already return Thumb-shaped JSON (e.g. + // /photos/view), bypassing the fromPhoto / fromFile mappers. static wrap(data) { return data.map((values) => new this(values)); } + // Like fromPhotos but expands each photo's Files[] into one Thumb + // per JPEG/PNG file — used by stack views where every variant + // should be its own slide. Skips photos without files and any + // file types other than jpg/png. static fromFiles(photos) { let result = []; @@ -352,6 +494,12 @@ export class Thumb extends Model { return result; } + // Scales a file's actual dimensions to fit within a (width, + // height) box, preserving aspect ratio. Returns the file's + // native size unchanged when the box is already big enough so we + // don't upscale low-resolution thumbnails. Mirrors + // Photo.calculateSize but operates on a File rather than a Photo + // so file/edit views can size individual files. static calculateSize(file, width, height) { if (width >= file.Width && height >= file.Height) { // Smaller @@ -374,6 +522,10 @@ export class Thumb extends Model { return { width: newW, height: newH }; } + // Builds the cached-thumbnail URL for a file at the given size + // (one of the keys from window.__CONFIG__.thumbs). Returns the + // static 404 image when the file has no hash so the lightbox grid + // still renders without throwing on broken assets. static thumbnailUrl(file, size) { if (!file.Hash) { return `${$config.staticUri}/img/404.jpg`; @@ -382,6 +534,9 @@ export class Thumb extends Model { return `${$config.contentUri}/t/${file.Hash}/${$config.previewToken}/${size}`; } + // Builds the original-file download URL for a file. Returns an + // empty string (not a "broken" URL) when the file has no hash so + // consumers can guard with truthiness. static downloadUrl(file) { if (!file || !file.Hash) { return ""; diff --git a/frontend/tests/vitest/component/lightbox.basic.test.js b/frontend/tests/vitest/component/lightbox.basic.test.js index 30fd22ec6..c8fd50de6 100644 --- a/frontend/tests/vitest/component/lightbox.basic.test.js +++ b/frontend/tests/vitest/component/lightbox.basic.test.js @@ -4,6 +4,8 @@ import * as contexts from "options/contexts"; import { nextTick } from "vue"; import PLightbox from "component/lightbox.vue"; import Photo from "model/photo"; +import Thumb from "model/thumb"; +import Album from "model/album"; import $util from "common/util"; import { buildNamespace } from "common/storage"; import clientConfig from "../config"; @@ -122,7 +124,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { const ctx = { ...wrapper.vm, photo: new Photo({ UID: "stale" }), - model: { UID: "ps6sg6be2lvl0yh7" }, + model: new Thumb({ UID: "ps6sg6be2lvl0yh7" }), $session: { isSidebarRestricted: () => true }, }; @@ -142,7 +144,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { const ctx = { ...wrapper.vm, photo: null, - model: { UID: "ps6sg6be2lvl0yh7" }, + model: new Thumb({ UID: "ps6sg6be2lvl0yh7" }), $session: { isSidebarRestricted: () => false }, }; @@ -233,7 +235,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { ...wrapper.vm, photo: placeholder, // Start with the user viewing slide N. - model: { UID: "uid-slide-n" }, + model: new Thumb({ UID: "uid-slide-n" }), $session: { isSidebarRestricted: () => false }, }; @@ -241,7 +243,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { wrapper.vm.$options.methods.fetchPhoto.call(ctx, "uid-slide-n"); // User swipes to slide N+1 BEFORE slide N's response arrives. - ctx.model = { UID: "uid-slide-n-plus-1" }; + ctx.model = new Thumb({ UID: "uid-slide-n-plus-1" }); // Slide N's response finally lands. resolveSlideN(); @@ -262,7 +264,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { const ctx = { ...wrapper.vm, photo: new Photo(), - model: { UID: "uid-slide-n" }, + model: new Thumb({ UID: "uid-slide-n" }), $session: { isSidebarRestricted: () => false }, }; @@ -283,7 +285,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { const ctx = { ...wrapper.vm, photo: placeholder, - model: { UID: "uid-slide-n" }, + model: new Thumb({ UID: "uid-slide-n" }), $session: { isSidebarRestricted: () => false }, }; @@ -319,7 +321,7 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { photo: placeholder, // model.UID intentionally STILL matches — to prove the rejection // (not the race-guard) is what protects this.photo here. - model: { UID: "uid-slide-n" }, + model: new Thumb({ UID: "uid-slide-n" }), $session: { isSidebarRestricted: () => false }, }; @@ -362,4 +364,209 @@ describe("PLightbox (low-mock, jsdom-friendly)", () => { spy.mockRestore(); }); }); + + // Wiring tests for the lightbox archive / restore / album-remove + // delegations. These pin the component-to-Thumb boundary so a + // future refactor that breaks the call edge (e.g., reverting back + // to inline $api.post or skipping the model methods) fails here + // instead of silently surviving the unit-test layer. + describe("onArchive wiring", () => { + it("calls this.model.archive() and notifies on success when canArchive is true", async () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-archive" }); + const archiveSpy = vi.spyOn(model, "archive").mockResolvedValue({ status: 200, data: {} }); + const ctx = { + ...wrapper.vm, + model, + canArchive: true, + pauseSlideshow: vi.fn(), + $notify: { ...wrapper.vm.$notify, success: vi.fn() }, + $gettext: VTUConfig.global.mocks.$gettext, + }; + + await wrapper.vm.$options.methods.onArchive.call(ctx); + + expect(ctx.pauseSlideshow).toHaveBeenCalledTimes(1); + expect(archiveSpy).toHaveBeenCalledTimes(1); + expect(ctx.$notify.success).toHaveBeenCalledTimes(1); + archiveSpy.mockRestore(); + }); + + it("does NOT call archive() when canArchive is false", () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-archive" }); + const archiveSpy = vi.spyOn(model, "archive"); + const ctx = { + ...wrapper.vm, + model, + canArchive: false, + pauseSlideshow: vi.fn(), + }; + + wrapper.vm.$options.methods.onArchive.call(ctx); + + expect(archiveSpy).not.toHaveBeenCalled(); + expect(ctx.pauseSlideshow).not.toHaveBeenCalled(); + archiveSpy.mockRestore(); + }); + + it("does NOT call archive() when model.UID is empty", () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "" }); + const archiveSpy = vi.spyOn(model, "archive"); + const ctx = { + ...wrapper.vm, + model, + canArchive: true, + pauseSlideshow: vi.fn(), + log: vi.fn(), + }; + + wrapper.vm.$options.methods.onArchive.call(ctx); + + expect(archiveSpy).not.toHaveBeenCalled(); + archiveSpy.mockRestore(); + }); + }); + + describe("onRestore wiring", () => { + it("calls this.model.restore() and notifies on success when canArchive is true", async () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-restore", Archived: true }); + const restoreSpy = vi.spyOn(model, "restore").mockResolvedValue({ status: 200, data: {} }); + const ctx = { + ...wrapper.vm, + model, + canArchive: true, + pauseSlideshow: vi.fn(), + $notify: { ...wrapper.vm.$notify, success: vi.fn() }, + $gettext: VTUConfig.global.mocks.$gettext, + }; + + wrapper.vm.$options.methods.onRestore.call(ctx); + // Drain the .then chain. + await Promise.resolve(); + await Promise.resolve(); + + expect(ctx.pauseSlideshow).toHaveBeenCalledTimes(1); + expect(restoreSpy).toHaveBeenCalledTimes(1); + expect(ctx.$notify.success).toHaveBeenCalledTimes(1); + restoreSpy.mockRestore(); + }); + + it("does NOT call restore() when canArchive is false", () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-restore", Archived: true }); + const restoreSpy = vi.spyOn(model, "restore"); + const ctx = { + ...wrapper.vm, + model, + canArchive: false, + pauseSlideshow: vi.fn(), + }; + + wrapper.vm.$options.methods.onRestore.call(ctx); + + expect(restoreSpy).not.toHaveBeenCalled(); + restoreSpy.mockRestore(); + }); + }); + + describe("onRemoveFromAlbum wiring", () => { + it("calls this.model.removeFromAlbum(collection.UID) then evicts the photo on success", async () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-remove" }); + const removeSpy = vi.spyOn(model, "removeFromAlbum").mockResolvedValue({ status: 200, data: {} }); + const evictSpy = vi.spyOn(model, "evictPhoto"); + const collection = new Album({ UID: "album-1" }); + const ctx = { + ...wrapper.vm, + model, + collection, + canManageAlbums: true, + pauseSlideshow: vi.fn(), + }; + + wrapper.vm.$options.methods.onRemoveFromAlbum.call(ctx); + // Drain the .then chain. + await Promise.resolve(); + await Promise.resolve(); + + expect(ctx.pauseSlideshow).toHaveBeenCalledTimes(1); + expect(removeSpy).toHaveBeenCalledWith("album-1"); + // Album-remove publishes only albums.updated, so the manual + // evictPhoto() in onRemoveFromAlbum.then is what drops the + // sidebar's stale Photo.Albums view. + expect(evictSpy).toHaveBeenCalledTimes(1); + removeSpy.mockRestore(); + evictSpy.mockRestore(); + }); + + it("does NOT call removeFromAlbum when canManageAlbums is false", () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-remove" }); + const removeSpy = vi.spyOn(model, "removeFromAlbum"); + const collection = new Album({ UID: "album-1" }); + const ctx = { + ...wrapper.vm, + model, + collection, + canManageAlbums: false, + pauseSlideshow: vi.fn(), + }; + + wrapper.vm.$options.methods.onRemoveFromAlbum.call(ctx); + + expect(removeSpy).not.toHaveBeenCalled(); + removeSpy.mockRestore(); + }); + + it("does NOT call removeFromAlbum when collection isn't an Album", () => { + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-remove" }); + const removeSpy = vi.spyOn(model, "removeFromAlbum"); + // A plain object (or a non-Album collection) must short-circuit. + const ctx = { + ...wrapper.vm, + model, + collection: { UID: "not-an-album-instance" }, + canManageAlbums: true, + pauseSlideshow: vi.fn(), + }; + + wrapper.vm.$options.methods.onRemoveFromAlbum.call(ctx); + + expect(removeSpy).not.toHaveBeenCalled(); + removeSpy.mockRestore(); + }); + + it("does NOT call evictPhoto when removeFromAlbum rejects", async () => { + // The optimistic Removed flip and its rollback live inside + // Thumb.removeFromAlbum (covered in thumb.test.js); here we + // pin that the lightbox does NOT evict the cache on failure + // — otherwise the sidebar would lose its (still-correct) + // Photo.Albums view after a no-op failed remove. + const wrapper = mountLightbox(); + const model = new Thumb({ UID: "uid-remove" }); + const removeSpy = vi.spyOn(model, "removeFromAlbum").mockRejectedValue(new Error("offline")); + const evictSpy = vi.spyOn(model, "evictPhoto"); + const collection = new Album({ UID: "album-1" }); + const ctx = { + ...wrapper.vm, + model, + collection, + canManageAlbums: true, + pauseSlideshow: vi.fn(), + }; + + wrapper.vm.$options.methods.onRemoveFromAlbum.call(ctx); + await Promise.resolve(); + await Promise.resolve(); + + expect(removeSpy).toHaveBeenCalledTimes(1); + expect(evictSpy).not.toHaveBeenCalled(); + removeSpy.mockRestore(); + evictSpy.mockRestore(); + }); + }); }); diff --git a/frontend/tests/vitest/model/photo.test.js b/frontend/tests/vitest/model/photo.test.js index 20c4758de..d51040301 100644 --- a/frontend/tests/vitest/model/photo.test.js +++ b/frontend/tests/vitest/model/photo.test.js @@ -1933,15 +1933,67 @@ describe("model/photo", () => { expect(Photo._cache.has("uid-ws-uncached")).toBe(false); }); - it("evicts cached entries when photos.deleted arrives", async () => { - seedCache("uid-ws-del", {}); + // The backend sends photos.deleted with a []string of bare UIDs + // (event.EntitiesDeleted("photos", deleted.UIDs()) in + // internal/api/batch_photos.go and internal/photoprism/cleanup.go), + // NOT objects with .UID — pin both shapes so nobody re-introduces + // the silent-no-op the subscriber had before consolidating the + // string/object handling. + it("evicts cached entries when photos.deleted arrives with bare-string UIDs", async () => { + seedCache("uid-ws-del-string", {}); $event.publish("photos.deleted", { - entities: [{ UID: "uid-ws-del" }], + entities: ["uid-ws-del-string"], }); await flushEvents(); - expect(Photo._cache.has("uid-ws-del")).toBe(false); + expect(Photo._cache.has("uid-ws-del-string")).toBe(false); + }); + + it("also tolerates the legacy object shape on photos.deleted", async () => { + seedCache("uid-ws-del-obj", {}); + + $event.publish("photos.deleted", { + entities: [{ UID: "uid-ws-del-obj" }], + }); + await flushEvents(); + + expect(Photo._cache.has("uid-ws-del-obj")).toBe(false); + }); + + it("evicts cached entries when photos.archived arrives (bare-string payload)", async () => { + // event.EntitiesArchived("photos", frm.Photos) in + // internal/api/batch_photos.go — frm.Photos is []string. + seedCache("uid-ws-arc", {}); + + $event.publish("photos.archived", { + entities: ["uid-ws-arc"], + }); + await flushEvents(); + + expect(Photo._cache.has("uid-ws-arc")).toBe(false); + }); + + it("evicts cached entries when photos.restored arrives (bare-string payload)", async () => { + // event.EntitiesRestored("photos", frm.Photos) — same shape. + seedCache("uid-ws-res", {}); + + $event.publish("photos.restored", { + entities: ["uid-ws-res"], + }); + await flushEvents(); + + expect(Photo._cache.has("uid-ws-res")).toBe(false); + }); + + it("ignores empty-string entries in archived/restored payloads", async () => { + seedCache("uid-keep-empty", {}); + + $event.publish("photos.archived", { entities: ["", "uid-keep-empty"] }); + await flushEvents(); + // The non-empty string evicts; the empty one is skipped (a + // malformed-payload guard, not a silent no-op for valid data). + expect(Photo._cache.has("uid-keep-empty")).toBe(false); }); // Regression for the edit-then-navigate-back scenario where the @@ -1985,15 +2037,17 @@ describe("model/photo", () => { expect(Photo._cache.has("uid-shape-bug")).toBe(false); }); - it("tolerates malformed payloads", async () => { + it("tolerates malformed payloads on every channel", async () => { seedCache("uid-keep", {}); - $event.publish("photos.updated", null); - $event.publish("photos.updated", {}); - $event.publish("photos.updated", { entities: "not-an-array" }); - $event.publish("photos.updated", { entities: [null, { Title: "no uid" }] }); - $event.publish("photos.deleted", null); - $event.publish("photos.deleted", { entities: [null] }); + // Each subscribed channel runs the same guard, so malformed + // payloads on any of them must leave the cache untouched. + ["photos.updated", "photos.deleted", "photos.archived", "photos.restored"].forEach((ev) => { + $event.publish(ev, null); + $event.publish(ev, {}); + $event.publish(ev, { entities: "not-an-array" }); + $event.publish(ev, { entities: [null, { Title: "no uid" }, 0, false, undefined] }); + }); await flushEvents(); expect(Photo._cache.has("uid-keep")).toBe(true); diff --git a/frontend/tests/vitest/model/thumb.test.js b/frontend/tests/vitest/model/thumb.test.js index 03d30e71a..f58448c14 100644 --- a/frontend/tests/vitest/model/thumb.test.js +++ b/frontend/tests/vitest/model/thumb.test.js @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import "../fixtures"; import Thumb from "model/thumb"; import Photo from "model/photo"; @@ -321,4 +321,170 @@ describe("model/thumb", () => { expect(result4.width).toBe(750); expect(result4.height).toBe(850); }); + + describe("loadPhoto", () => { + it("delegates to Photo.findCached for thumbs with a UID", async () => { + const result = new Photo({ UID: "abc123", Title: "Loaded" }); + const spy = vi.spyOn(Photo, "findCached").mockResolvedValue(result); + const thumb = new Thumb({ UID: "abc123" }); + const photo = await thumb.loadPhoto(); + expect(spy).toHaveBeenCalledWith("abc123"); + expect(photo).toBe(result); + spy.mockRestore(); + }); + + it("resolves to an empty Photo placeholder when the thumb has no UID", async () => { + const spy = vi.spyOn(Photo, "findCached"); + const thumb = new Thumb({ UID: "" }); + const photo = await thumb.loadPhoto(); + // Returns a Photo (not null/undefined) so consumers can read + // .X without nullable chains; UID stays empty as a "not loaded" + // signal. findCached must NOT be called for an empty UID. + expect(photo).toBeInstanceOf(Photo); + expect(photo.UID).toBe(""); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("propagates ModelCacheStaleFetchError so callers' .catch handlers fire", async () => { + const err = new Error("ModelCache: discarded stale fetch after clear()"); + err.name = "ModelCacheStaleFetchError"; + const spy = vi.spyOn(Photo, "findCached").mockRejectedValue(err); + const thumb = new Thumb({ UID: "abc123" }); + await expect(thumb.loadPhoto()).rejects.toBe(err); + spy.mockRestore(); + }); + }); + + describe("evictPhoto", () => { + it("calls Photo.evictCache with the thumb UID", () => { + const spy = vi.spyOn(Photo, "evictCache"); + const thumb = new Thumb({ UID: "abc123" }); + thumb.evictPhoto(); + expect(spy).toHaveBeenCalledWith("abc123"); + spy.mockRestore(); + }); + + it("is a no-op when the thumb has no UID", () => { + const spy = vi.spyOn(Photo, "evictCache"); + const thumb = new Thumb({ UID: "" }); + thumb.evictPhoto(); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + }); + + // The archive/restore/removeFromAlbum tests use $api directly (not + // axios-mock-adapter), spying on the verbs to assert the URL and + // payload shape. This avoids registering one-off Mock.onPost handlers + // in fixtures.js for endpoints that already have global mocks. + // archive/restore/removeFromAlbum spy on $api directly rather than + // registering one-off Mock.onPost handlers in fixtures.js for + // endpoints that already have global mocks. They also pin the + // optimistic-flip + rollback contract that drives lightbox menu + // visibility (this.model?.Archived / this.model?.Removed checks). + // archive/restore/removeFromAlbum spy on $api directly rather than + // registering one-off Mock.onPost handlers in fixtures.js for + // endpoints that already have global mocks. They also pin the + // optimistic-flip + restore-previous-value rollback contract that + // drives lightbox menu visibility (this.model?.Archived / + // this.model?.Removed checks). + describe("archive", () => { + it("flips Archived to true, posts to batch/photos/archive, and resolves on success", async () => { + const $api = (await import("common/api")).default; + const spy = vi.spyOn($api, "post").mockResolvedValue({ status: 200, data: { code: 200 } }); + const thumb = new Thumb({ UID: "abc123" }); + // Pre-condition: Archived is undefined (not in defaults — see + // the explicit-tri-state checks at lightbox.vue:1437). + expect(thumb.Archived).toBeUndefined(); + await thumb.archive(); + expect(spy).toHaveBeenCalledWith("batch/photos/archive", { photos: ["abc123"] }); + expect(thumb.Archived).toBe(true); + spy.mockRestore(); + }); + + it("restores the pre-call Archived value on rejection (was undefined)", async () => { + const $api = (await import("common/api")).default; + const err = new Error("offline"); + const spy = vi.spyOn($api, "post").mockRejectedValue(err); + const thumb = new Thumb({ UID: "abc123" }); + // Pre-state is undefined (default, since Archived isn't in + // getDefaults()). A literal `false` rollback would silently + // promote the field to a boolean — capturing prev preserves + // the tri-state semantics the menu logic depends on. + await expect(thumb.archive()).rejects.toBe(err); + expect(thumb.Archived).toBeUndefined(); + spy.mockRestore(); + }); + + it("preserves Archived on rejection when called on an already-archived thumb", async () => { + // No-op archive of an already-archived photo: backend may + // succeed or 4xx, but a literal `false` rollback would flip a + // truthful "archived" UI to "not archived" on failure. + const $api = (await import("common/api")).default; + const err = new Error("offline"); + const spy = vi.spyOn($api, "post").mockRejectedValue(err); + const thumb = new Thumb({ UID: "abc123", Archived: true }); + await expect(thumb.archive()).rejects.toBe(err); + expect(thumb.Archived).toBe(true); + spy.mockRestore(); + }); + }); + + describe("restore", () => { + it("flips Archived to false, posts to batch/photos/restore, and resolves on success", async () => { + const $api = (await import("common/api")).default; + const spy = vi.spyOn($api, "post").mockResolvedValue({ status: 200, data: { code: 200 } }); + const thumb = new Thumb({ UID: "abc123", Archived: true }); + await thumb.restore(); + expect(spy).toHaveBeenCalledWith("batch/photos/restore", { photos: ["abc123"] }); + expect(thumb.Archived).toBe(false); + spy.mockRestore(); + }); + + it("restores the pre-call Archived value on rejection (was true)", async () => { + const $api = (await import("common/api")).default; + const err = new Error("offline"); + const spy = vi.spyOn($api, "post").mockRejectedValue(err); + const thumb = new Thumb({ UID: "abc123", Archived: true }); + await expect(thumb.restore()).rejects.toBe(err); + expect(thumb.Archived).toBe(true); + spy.mockRestore(); + }); + + it("preserves Archived on rejection when called on an already-restored thumb", async () => { + // No-op restore of a non-archived photo: capturing prev + // ensures we don't silently flip undefined → true on failure. + const $api = (await import("common/api")).default; + const err = new Error("offline"); + const spy = vi.spyOn($api, "post").mockRejectedValue(err); + const thumb = new Thumb({ UID: "abc123" }); + await expect(thumb.restore()).rejects.toBe(err); + expect(thumb.Archived).toBeUndefined(); + spy.mockRestore(); + }); + }); + + describe("removeFromAlbum", () => { + it("flips Removed to true, DELETEs albums/:albumUID/photos, and resolves on success", async () => { + const $api = (await import("common/api")).default; + const spy = vi.spyOn($api, "delete").mockResolvedValue({ status: 200, data: { code: 200 } }); + const thumb = new Thumb({ UID: "abc123" }); + expect(thumb.Removed).toBeUndefined(); + await thumb.removeFromAlbum("album-1"); + expect(spy).toHaveBeenCalledWith("albums/album-1/photos", { data: { photos: ["abc123"] } }); + expect(thumb.Removed).toBe(true); + spy.mockRestore(); + }); + + it("restores the pre-call Removed value on rejection (was undefined)", async () => { + const $api = (await import("common/api")).default; + const err = new Error("offline"); + const spy = vi.spyOn($api, "delete").mockRejectedValue(err); + const thumb = new Thumb({ UID: "abc123" }); + await expect(thumb.removeFromAlbum("album-1")).rejects.toBe(err); + expect(thumb.Removed).toBeUndefined(); + spy.mockRestore(); + }); + }); });