diff --git a/server/src/nest/assistant/assistant-tool-offer.ts b/server/src/nest/assistant/assistant-tool-offer.ts index 76c3f645c..98ab26557 100644 --- a/server/src/nest/assistant/assistant-tool-offer.ts +++ b/server/src/nest/assistant/assistant-tool-offer.ts @@ -178,13 +178,20 @@ export function isOfferedInJourney(name: string, className: string | undefined): return isStudioController(className) || (className !== undefined && JOURNEY_READ_CONTROLLERS.has(className)); } +/** + * The controllers whose reads work only for someone who may change the book: + * a design draft is its editor's, so a viewer's design_book, revise_book_design + * and get_book_design could only refuse. + */ +const EDITOR_READ_CONTROLLERS: ReadonlySet = new Set(['JourneyBookDesignMcp']); + /** * Whether a Studio conversation keeps an attached tool once its schema and * annotations are known. Only tools that work on the conversation's journey * (they take the `journeyId` the server pins) stay, so the conversation never - * reaches another journey; of those every read, and the book's writes only - * for someone who may change the book. The journey's own writes (entries, - * photos, its trips) stay with the journey page. + * reaches another journey; of those every read but the design draft's, which, + * like the book's writes, go only to someone who may change the book. The + * journey's own writes (entries, photos, its trips) stay with the journey page. */ export function keptInJourney( className: string | undefined, @@ -193,7 +200,7 @@ export function keptInJourney( canWrite: boolean, ): boolean { if (!pinned) return false; - if (kind === 'read') return true; + if (kind === 'read') return canWrite || className === undefined || !EDITOR_READ_CONTROLLERS.has(className); return canWrite && isStudioController(className); } diff --git a/server/src/nest/journey/journey-book-design.service.ts b/server/src/nest/journey/journey-book-design.service.ts index faec906f0..1c236bcb7 100644 --- a/server/src/nest/journey/journey-book-design.service.ts +++ b/server/src/nest/journey/journey-book-design.service.ts @@ -204,6 +204,8 @@ export class JourneyBookDesignService { * is and is named under `skipped`; nothing applied is a 409. */ async apply(journeyId: number, userId: number, args: ApplyBookDesignRequest): Promise { + // The rights come before the draft: a viewer or an outsider holds none, and is told 403 or 404, not 410. + await this.requireEditor(journeyId, userId); const draft = await this.draftOf(userId, journeyId, args.designId); const { record, result, revisionId } = await this.books.editBook( journeyId, diff --git a/server/src/nest/photos/photo-colour-signals.ts b/server/src/nest/photos/photo-colour-signals.ts index f849cc6f1..a5d4420b0 100644 --- a/server/src/nest/photos/photo-colour-signals.ts +++ b/server/src/nest/photos/photo-colour-signals.ts @@ -50,7 +50,7 @@ function samples(data: Uint8Array, total: number): Rgb[] { const out: Rgb[] = []; for (let i = 0; i < total; i += stride) { const o = i * 4; - out.push([data[o] ?? 0, data[o + 1] ?? 0, data[o + 2] ?? 0]); + out.push([data[o], data[o + 1], data[o + 2]]); } return out; } @@ -98,21 +98,20 @@ export function paletteOf(data: Uint8Array, width: number, height: number): stri for (const colour of pixels) { const at = nearest(centres, colour); const sum = sums[at]; - if (!sum) continue; sum[0] += colour[0]; sum[1] += colour[1]; sum[2] += colour[2]; - members[at] = (members[at] ?? 0) + 1; + members[at] += 1; } // The last round only counts: the centres stay where the counting found them. if (round === ROUNDS) break; sums.forEach((sum, i) => { - const n = members[i] ?? 0; + const n = members[i]; if (n > 0) centres[i] = [sum[0] / n, sum[1] / n, sum[2] / n]; }); } return centres - .map((centre, i) => ({ centre, share: (members[i] ?? 0) / pixels.length, i })) + .map((centre, i) => ({ centre, share: members[i] / pixels.length, i })) .filter((c) => c.share >= MIN_SHARE) .sort((a, b) => b.share - a.share || a.i - b.i) .map((c) => `#${hex2(c.centre[0])}${hex2(c.centre[1])}${hex2(c.centre[2])}:${c.share.toFixed(2)}`); @@ -127,9 +126,9 @@ export function edgeSignals(data: Uint8Array, width: number, height: number): Om const grey = new Float32Array(width * height); for (let i = 0; i < grey.length; i++) { const o = i * 4; - grey[i] = 0.299 * (data[o] ?? 0) + 0.587 * (data[o + 1] ?? 0) + 0.114 * (data[o + 2] ?? 0); + grey[i] = 0.299 * data[o] + 0.587 * data[o + 1] + 0.114 * data[o + 2]; } - const at = (k: number) => grey[k] ?? 0; + const at = (k: number) => grey[k]; const sums = new Array(GRID * GRID).fill(0); const counts = new Array(GRID * GRID).fill(0); let energy = 0; @@ -141,8 +140,8 @@ export function edgeSignals(data: Uint8Array, width: number, height: number): Om const i = row * width + col; const lap = Math.abs(at(i - 1) + at(i + 1) + at(i - width) + at(i + width) - 4 * at(i)); const cell = cellRow * GRID + Math.min(GRID - 1, Math.floor((col * GRID) / width)); - sums[cell] = (sums[cell] ?? 0) + lap; - counts[cell] = (counts[cell] ?? 0) + 1; + sums[cell] += lap; + counts[cell] += 1; energy += lap; sx += lap * ((col + 0.5) / width); sy += lap * ((row + 0.5) / height); @@ -150,7 +149,7 @@ export function edgeSignals(data: Uint8Array, width: number, height: number): Om } const busy = sums .map((sum, cell) => { - const n = counts[cell] ?? 0; + const n = counts[cell]; const level = n > 0 ? Math.min(15, Math.round(sum / n / BUSY_STEP)) : 0; return level.toString(16); }) diff --git a/server/src/nest/photos/photo-signals-runner.ts b/server/src/nest/photos/photo-signals-runner.ts index de924259f..a5a69787f 100644 --- a/server/src/nest/photos/photo-signals-runner.ts +++ b/server/src/nest/photos/photo-signals-runner.ts @@ -105,7 +105,7 @@ function greyStats(data: Uint8Array, width: number, height: number) { let clipped = 0; for (let i = 0; i < total; i++) { const o = i * 4; - const y = 0.299 * (data[o] ?? 0) + 0.587 * (data[o + 1] ?? 0) + 0.114 * (data[o + 2] ?? 0); + const y = 0.299 * data[o] + 0.587 * data[o + 1] + 0.114 * data[o + 2]; grey[i] = y; sum += y; if (y <= CLIP_LOW || y >= CLIP_HIGH) clipped++; @@ -114,7 +114,7 @@ function greyStats(data: Uint8Array, width: number, height: number) { let n = 0; let mean = 0; let m2 = 0; - const at = (k: number) => grey[k] ?? 0; + const at = (k: number) => grey[k]; for (let row = 1; row < height - 1; row++) { for (let col = 1; col < width - 1; col++) { const i = row * width + col; diff --git a/server/tests/unit/nest/assistant/assistant-journey-session.test.ts b/server/tests/unit/nest/assistant/assistant-journey-session.test.ts index e97075285..fcf224770 100644 --- a/server/tests/unit/nest/assistant/assistant-journey-session.test.ts +++ b/server/tests/unit/nest/assistant/assistant-journey-session.test.ts @@ -72,7 +72,7 @@ describe('a Studio tool session', () => { } }); - it('ASTJSES-002: a viewer is offered the reads and not one write', async () => { + it('ASTJSES-002: a viewer is offered the reads but the design draft tools, and not one write', async () => { const { journeyId } = await ownerWithJourneys(); const { user: viewer } = await makeUser(orm); await addJourneyContributor(orm, journeyId, viewer.id, 'viewer'); @@ -81,6 +81,11 @@ describe('a Studio tool session', () => { const names = session.specs().map((spec) => spec.name); expect(names).toEqual(expect.arrayContaining(['get_book', 'describe_book_page', 'list_journey_entries'])); for (const name of [...BOOK_WRITES, 'set_book_cover']) expect(names, name).not.toContain(name); + const listed = registry.list().map((entry) => entry.name); + for (const name of ['design_book', 'get_book_design', 'revise_book_design']) { + expect(listed, name).toContain(name); + expect(names, name).not.toContain(name); + } for (const name of names) expect(session.kindOf(name), name).toBe('read'); } finally { await session.close(); diff --git a/server/tests/unit/nest/journey/book-design-apply.test.ts b/server/tests/unit/nest/journey/book-design-apply.test.ts index 5207aafe8..8ae500c5c 100644 --- a/server/tests/unit/nest/journey/book-design-apply.test.ts +++ b/server/tests/unit/nest/journey/book-design-apply.test.ts @@ -6,14 +6,19 @@ * untouched; a page whose photos left the gallery stays out; the draft's * format and title come along unless someone set others since, and a page * size moved since takes nothing. A journey that had no book gets the chosen - * pages as a new book (a cover photo gone since leaves an empty frame), one - * that lost its book since, or got one since, gets nothing. + * pages as a new book (a cover photo gone since leaves an empty frame, a map + * stop's picture gone since leaves a dot), one that lost its book since, or + * got one since, gets nothing. A plan row the draft holds no page for writes + * nothing, and a book at the page limit takes no new page. */ import { applyDesignDraft, type DraftToApply } from '../../../../src/nest/journey/book-design-apply'; import { blankBook, + bookMapElementSchema, bookPageSetupForPreset, bookPhotoElementSchema, + bookShapeElementSchema, + MAX_SPREADS, type BookDesignPlanRow, type BookDocument, type BookSpread, @@ -243,4 +248,75 @@ describe('applyDesignDraft', () => { }; expect(applyDesignDraft(reordered, draft, new Set([4])).applied).toEqual(['a']); }); + + it('JBAPPLY-011: a new book empties only the gone pictures on its cover, a map stop among them', () => { + const point = (photoId: number | null) => ({ lat: 38.7, lng: -9.1, label: '', photoId }); + const map = bookMapElementSchema.parse({ + id: 'm-c', + kind: 'map', + frame: { x: 0, y: 0, w: 40, h: 40 }, + points: [point(5), point(6), point(null)], + }); + const empty = bookPhotoElementSchema.parse({ id: 'p-c-e', kind: 'photo', frame: { x: 0, y: 0, w: 20, h: 20 } }); + const shape = bookShapeElementSchema.parse({ id: 's-c', kind: 'shape', frame: { x: 0, y: 0, w: 10, h: 10 } }); + const framed = page('c', 'cover', [6]); + const cover = { ...framed, elements: [...framed.elements, empty, map, shape] }; + const doc = book([cover, ...pages('f:first', 'l:last', 'k:back')]); + const plan = doc.spreads.map((s) => row(s.id, 'insert')); + const draft: DraftToApply = { base: null, doc, plan, selection: [] }; + + const out = applyDesignDraft(null, draft, new Set([6])); + + const [frame, blank, stops, rest] = out.doc.spreads[0]?.elements ?? []; + expect(frame).toMatchObject({ kind: 'photo', photoId: 6 }); + expect(blank).toMatchObject({ kind: 'photo', photoId: null }); + expect(stops?.kind === 'map' ? stops.points.map((p) => p.photoId) : 'not a map').toEqual([null, 6, null]); + expect(rest).toEqual(shape); + expect(out.applied).toEqual(['c', 'f', 'l', 'k']); + }); + + it('JBAPPLY-012: a new page with only a dropped page and a page the book lacks before it follows the cover', () => { + const base = book(pages('c:cover', 'a', 'k:back')); + const draft: DraftToApply = { + base, + doc: book(pages('c:cover', 'm', 'n', 'k:back')), + plan: [row('x', 'drop'), row('m', 'insert'), row('n', 'insert')], + selection: ['n'], + }; + + const out = applyDesignDraft(base, draft, new Set()); + + expect(out.doc.spreads.map((s) => s.id)).toEqual(['c', 'n', 'a', 'k']); + expect(out.applied).toEqual(['n']); + expect(out.skipped).toEqual([ + { spreadId: 'x', reason: 'excluded' }, + { spreadId: 'm', reason: 'excluded' }, + ]); + }); + + it('JBAPPLY-013: a plan row whose page the draft does not hold changes nothing and is not named', () => { + const base = stored(); + const draft: DraftToApply = { + base, + doc: book(pages('c:cover')), + plan: [row('a', 'replace'), row('n', 'insert')], + selection: ['a', 'n'], + }; + + const out = applyDesignDraft(base, draft, new Set()); + + expect([out.applied, out.skipped]).toEqual([[], []]); + expect(out.doc.spreads).toEqual(base.spreads); + }); + + it('JBAPPLY-014: a book at the page limit takes no new page, the page named as changed since', () => { + const inner = Array.from({ length: MAX_SPREADS - 1 }, (_, i) => page(`i${i}`, 'inner')); + const full = book([page('c', 'cover'), ...inner]); + const draft: DraftToApply = { base: full, doc: book(pages('n')), plan: [row('n', 'insert')], selection: ['n'] }; + + const out = applyDesignDraft(full, draft, new Set()); + + expect(out.skipped).toEqual([{ spreadId: 'n', reason: 'changed_since' }]); + expect(out.doc.spreads).toHaveLength(MAX_SPREADS); + }); }); diff --git a/server/tests/unit/nest/journey/journey-book-design.mcp.test.ts b/server/tests/unit/nest/journey/journey-book-design.mcp.test.ts index 7f638d06f..c956c7b9c 100644 --- a/server/tests/unit/nest/journey/journey-book-design.mcp.test.ts +++ b/server/tests/unit/nest/journey/journey-book-design.mcp.test.ts @@ -7,9 +7,10 @@ * journey group, the design, its read and its revision as reads, the apply, * the undo targets and the book-level changes as writes; the tools answer * what the services answer, so an editor designs, revises and applies, a - * viewer is refused and an outsider told the journey is not there; the apply - * tells every member once, the asking one included; describe_book_page reads - * a draft page by its designId. + * viewer is refused every change and an outsider told the journey is not + * there by every tool, before a draft is looked for (403 or 404, not 410); + * the apply tells every member once, the asking one included; + * describe_book_page reads a draft page by its designId. */ import { JourneyEntryPhotos } from '../../../../src/db/entities/JourneyEntryPhotos.entity'; import { JourneyPhotos } from '../../../../src/db/entities/JourneyPhotos.entity'; @@ -194,4 +195,37 @@ describe('the design tools', () => { const restored = bookWriteResultSchema.parse(answered(await books.restoreBookLayout(back, ctx(owner.id)))); expect(restored.version).toBe(removed.version + 1); }); + + it('JBDMCP-005: the other tools refuse an outsider, and a viewer every change, before a draft is read', async () => { + const { journey, viewer, stranger } = await crew(); + const draft = { journeyId: journey.id, designId: 'd-none' }; + const revise = { ...draft, spreadId: 'sp-none', ops: [{ op: 'background' as const, color: '#f4efe6' }] }; + const revision = { journeyId: journey.id, revisionId: 1, expectedVersion: 1 }; + const page = { journeyId: journey.id, spreadId: 'sp-none' }; + const removal = { journeyId: journey.id, layoutId: 'l-none', expectedVersion: 1 }; + const putBack = { journeyId: journey.id, layout: '{}', expectedVersion: 1 }; + const format = { journeyId: journey.id, preset: 'a4-portrait' as const }; + const gone = 'Journey not found'; + + await expect(designs.getBookDesign(draft, ctx(stranger.id))).rejects.toThrow(gone); + await expect(designs.reviseBookDesign(revise, ctx(stranger.id))).rejects.toThrow(gone); + await expect(designs.applyBookDesign(draft, ctx(stranger.id))).rejects.toThrow(gone); + await expect(designs.restoreBookRevision(revision, ctx(stranger.id))).rejects.toThrow(gone); + await expect(books.duplicateBookPage(page, ctx(stranger.id))).rejects.toThrow(gone); + await expect(books.saveBookLayout(page, ctx(stranger.id))).rejects.toThrow(gone); + await expect(books.removeBookLayout(removal, ctx(stranger.id))).rejects.toThrow(gone); + await expect(books.restoreBookLayout(putBack, ctx(stranger.id))).rejects.toThrow(gone); + await expect(books.setBookSetup(format, ctx(stranger.id))).rejects.toThrow(gone); + + // A viewer may read the book, so a draft it never made is gone; every change is refused. + await expect(designs.getBookDesign(draft, ctx(viewer.id))).rejects.toThrow('This design is no longer kept'); + await expect(designs.reviseBookDesign(revise, ctx(viewer.id))).rejects.toThrow('Not allowed'); + await expect(designs.applyBookDesign(draft, ctx(viewer.id))).rejects.toThrow('Not allowed'); + await expect(designs.restoreBookRevision(revision, ctx(viewer.id))).rejects.toThrow('Not allowed'); + await expect(books.duplicateBookPage(page, ctx(viewer.id))).rejects.toThrow('Not allowed'); + await expect(books.saveBookLayout(page, ctx(viewer.id))).rejects.toThrow('Not allowed'); + await expect(books.removeBookLayout(removal, ctx(viewer.id))).rejects.toThrow('Not allowed'); + await expect(books.restoreBookLayout(putBack, ctx(viewer.id))).rejects.toThrow('Not allowed'); + expect(t.realtime.broadcastToUserMock).not.toHaveBeenCalled(); + }); }); diff --git a/server/tests/unit/nest/journey/journey-book-design.service.test.ts b/server/tests/unit/nest/journey/journey-book-design.service.test.ts index d6e09342b..be98bf8bf 100644 --- a/server/tests/unit/nest/journey/journey-book-design.service.test.ts +++ b/server/tests/unit/nest/journey/journey-book-design.service.test.ts @@ -3,18 +3,19 @@ * the shared engine. Taken from JourneyBookDesignModule over a snapshot test * db, with the realtime fake and the photo bytes stubbed (nothing to measure). * - * What it pins: a design is a draft, nothing written or sent, made by the - * owner and editors only (a viewer 403, an outsider 404) and read back by its - * maker only (410 for anyone else and once it is gone); a revision of a draft - * page is checked like an edit and changes only the draft, a kept page it - * revises is proposed; the selection names only pages the plan changes; the - * apply writes the chosen pages in one version with one announcement, keeps - * the book as it was as a revision in the same transaction, names the pages - * left out and the ones changed since, refuses when nothing is left (409, - * nothing written) and at a stale version, and forgets the draft; a journey - * without a book gets a new one and no revision; restoring the revision puts - * the book back only at the version the apply left, and never writes a revision that - * does not read back; five revisions are kept. + * What it pins: a design is a draft, nothing written or sent, made and + * applied by the owner and editors only (a viewer 403, an outsider 404, before + * any draft is looked for) and read back by its maker only (410 for anyone + * else and once it is gone); a revision of a draft page is checked like an + * edit and changes only the draft, a kept page it revises is proposed; the + * selection names only pages the plan changes; the apply writes the chosen + * pages in one version with one announcement, keeps the book as it was as a + * revision in the same transaction, names the pages left out and the ones + * changed since, refuses when nothing is left (409, nothing written) and at a + * stale version, and forgets the draft; a journey without a book gets a new + * one and no revision; restoring the revision puts the book back only at the + * version the apply left, and never writes a revision that does not read + * back; five revisions are kept. */ import { JourneyBookRevisions } from '../../../../src/db/entities/JourneyBookRevisions.entity'; import { JourneyEntryPhotos } from '../../../../src/db/entities/JourneyEntryPhotos.entity'; @@ -144,7 +145,7 @@ describe('a design draft', () => { expect(await designs.describeDraft(journey.id, editor.id, answer.design_id)).toEqual(answer); }); - it("JBDSN-002: only the owner and editors design; a draft is its maker's alone and gone for anyone else", async () => { + it("JBDSN-002: only the owner and editors design or apply; a draft is its maker's alone, gone for others", async () => { const { journey, owner, viewer, stranger } = await crew(); expect(await refusal(() => designs.design(journey.id, viewer.id))).toEqual({ status: 403, code: 'book_read_only' }); expect(await refusal(() => designs.design(journey.id, stranger.id))).toMatchObject({ status: 404 }); @@ -158,6 +159,13 @@ describe('a design draft', () => { expect(await refusal(() => designs.describeDraft(journey.id, owner.id, 'd-none'))).toEqual(gone); expect(await refusal(() => designs.draftView(journey.id, stranger.id, id))).toMatchObject({ status: 404 }); expect(await refusal(() => designs.select(journey.id, viewer.id, id, []))).toMatchObject({ status: 403 }); + // Taking the owner's draft is a question of rights first, never answered as a design gone. + const taking = { designId: id }; + expect(await refusal(() => designs.apply(journey.id, viewer.id, taking))).toEqual({ + status: 403, + code: 'book_read_only', + }); + expect(await refusal(() => designs.apply(journey.id, stranger.id, taking))).toMatchObject({ status: 404 }); }); it('JBDSN-003: a revision changes a page of the draft like an edit would, nothing of the book; a kept page revised is proposed', async () => { diff --git a/server/tests/unit/nest/photos/photo-colour-signals.test.ts b/server/tests/unit/nest/photos/photo-colour-signals.test.ts index c01f3d486..07b068c2d 100644 --- a/server/tests/unit/nest/photos/photo-colour-signals.test.ts +++ b/server/tests/unit/nest/photos/photo-colour-signals.test.ts @@ -2,8 +2,10 @@ * The colour signals of a photo (PHSIG3-*), on pixels drawn here: a picture * half flat and half textured has its busy cells and its salient point on * the textured side, a two-colour picture has both colours in its palette - * with their shares, a flat one has one colour, no salient point and a calm - * grid; the measurement of a whole photo carries them, at version 3. + * with their shares (two equal ones always in the same order), a flat one has + * one colour, no salient point and a calm grid, the smallest picture is busy + * only around its one inner pixel; the measurement of a whole photo carries + * them, at version 3. */ import { colourSignals, edgeSignals, paletteOf } from '../../../../src/nest/photos/photo-colour-signals'; import { measurePhoto, PHOTO_SIGNALS_ALGO } from '../../../../src/nest/photos/photo-signals-runner'; @@ -75,4 +77,18 @@ describe('colour signals', () => { expect(measured?.busy).toMatch(/^[0-9a-f]{64}$/); expect(measured?.salX).toBeGreaterThan(0.6); }); + + it('PHSIG3-005: on the smallest picture only the cell holding the one inner pixel can be busy', () => { + const dot = edgeSignals( + pixels(3, 3, (x, y) => (x === 1 && y === 1 ? [255, 255, 255] : [0, 0, 0])), + 3, + 3, + ); + expect(dot).toEqual({ busy: `${'0'.repeat(18)}f${'0'.repeat(45)}`, salX: 0.5, salY: 0.5 }); + }); + + it('PHSIG3-006: two colours of equal share come out in one order every time, the lower histogram bin first', () => { + const halves = pixels(100, 100, (x) => (x < 50 ? [200, 30, 30] : [20, 40, 220])); + expect(paletteOf(halves, 100, 100)).toEqual(['#1428dc:0.50', '#c81e1e:0.50']); + }); }); diff --git a/server/tests/unit/nest/photos/photo-signals.service.test.ts b/server/tests/unit/nest/photos/photo-signals.service.test.ts index feaae9c8f..54ad63b7f 100644 --- a/server/tests/unit/nest/photos/photo-signals.service.test.ts +++ b/server/tests/unit/nest/photos/photo-signals.service.test.ts @@ -311,6 +311,8 @@ describe('PhotoSignalsService, the line', () => { algo: PHOTO_SIGNALS_ALGO, palette: 'not json', busy: 'abc', + sal_x: 0.25, + sal_y: 0.75, }); const book = await main.signals.bookSignals([measured, older, broken, measured]); @@ -321,7 +323,8 @@ describe('PhotoSignalsService, the line', () => { expect(own?.salient).toBeUndefined(); // An older measurement still knows the size, never the colours. expect(book.get(older)).toEqual({ width: 800, height: null, luma: 128 }); - expect(book.get(broken)).toEqual({ width: null, height: null }); + // A broken palette or grid reads as none; the salient point beside it still counts. + expect(book.get(broken)).toEqual({ width: null, height: null, salient: { x: 0.25, y: 0.75 } }); expect(await main.signals.bookSignals([])).toEqual(new Map()); }); });