From 7b221a2ca1983898f14c93bbe91dc8fbd4d6300d Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Mon, 7 Sep 2026 16:46:35 -0700 Subject: [PATCH] Add read-only endpoints for the two legacy /wxycdb cross-reference views (BS#2386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /library/crossreferences/artists` and `GET /library/crossreferences/releases` serve `artist_crossreference` and `artist_library_crossreference` — the data behind `/wxycdb`'s `xrefsToLibraryCodes.jsp` and `xrefsToLibraryReleases.jsp`, the last two `mainmenu.jsp` links with no successor in this service. dj-site#1163 needs both screens; dj-site#1211 recorded the read path as an open question, and the answer is that the tables survive in the schema and are populated. Read only, by decision rather than omission. WXYC/wiki#89's D5 freezes the artist-code set at the tubafrenzy cutover and drops the release set outright, so a create/update/delete sibling would unfreeze what that decision froze. `jobs/library-etl` keeps its own ETL write path; that is not an API surface. `catalog: ['write']` on both READS. `mainmenu.jsp:32-38` wraps these two links in ``, unlike Missing Releases and the rotation links beside them; `catalog:write` selects musicDirector + stationManager, the same pair that flag names, without minting a statement key. `/library/bmi-performance-list`, `/library/artists/search`, and `/library/artists/peek-code` set the precedent for an admin-gated read at that tier. Paginated in the `{ results, total, page, totalPages }` shape `GET /library/query` uses, but with its own bounds (default 200, max 500) rather than the shared 50/100. The collections are frozen at 119 and 35 rows, so a 100-row ceiling would make the artist set permanently un-fetchable in one request for a cap that never binds; the cap still exists because the freeze is a decision and not something the query can enforce while library-etl is still upserting. Both queries project call-number PARTS rather than a composed string, matching every other catalog projection here. `target_code_artist_number` is a correlated subquery, not a join: `artist_genre_key` is unique on `(artist_id, genre_id)`, so joining would fan one cross-reference row out into several for a multi-genre artist. It collapses to the lowest `genre_id` so the code this view shows matches the artist's own card. The release listing LEFT joins `genre_artist_crossreference` where `getReleasesForArtist` INNER joins it — there the join is a scoping predicate, here it supplies one display column, and a frozen legacy row must not silently vanish from the only listing that reproduces it. Divergence from the JSPs: both render a "Time Last Modified" column and neither table has a timestamp. The field is omitted; no migration invents one. --- apps/backend/app.yaml | 200 ++++++++++++++ .../backend/controllers/library.controller.ts | 95 +++++++ apps/backend/routes/library.route.ts | 33 +++ apps/backend/services/library.service.ts | 217 +++++++++++++++ .../library.crossReferences.test.ts | 253 ++++++++++++++++++ ...-crossreferences-permissions.route.test.ts | 197 ++++++++++++++ 6 files changed, 995 insertions(+) create mode 100644 tests/unit/controllers/library.crossReferences.test.ts create mode 100644 tests/unit/routes/library-crossreferences-permissions.route.test.ts diff --git a/apps/backend/app.yaml b/apps/backend/app.yaml index 2ef7e35db..c932a55a4 100644 --- a/apps/backend/app.yaml +++ b/apps/backend/app.yaml @@ -2615,6 +2615,206 @@ paths: '404': description: Artist not found + /library/crossreferences/artists: + get: + summary: List the legacy artist-to-artist cross-references + description: > + Successor to `/wxycdb`'s `xrefsToLibraryCodes.jsp` — the whole + `artist_crossreference` collection, the "see also" pointers a librarian + filed between two artist cards. READ ONLY, deliberately: WXYC/wiki#89's + decision D5 freezes this set at the tubafrenzy cutover ("post-cutover + artists don't gain aliases; unfreezing is a future ticket"), so there is + no create, update, or delete sibling and adding one would unfreeze it. + + + Enforced by `requirePermissions({ catalog: ['write'] })` — musicDirector + or above. A READ at the write tier, matching + `/library/bmi-performance-list`: `mainmenu.jsp` wraps this link in + ``, unlike the Missing Releases + and rotation links beside it, and `catalog:write` is the grant that + selects the same musicDirector + stationManager pair that flag names. + + + `target_code_artist_number` is genre-scoped and collapses to the lowest + `genre_id`, matching `GET /library/artists/{id}`; it is null for an + artist with no `genre_artist_crossreference` row. Only the TARGET + carries a call number, matching the JSP's columns. Ordered by the + cross-referencing artist's name, then both FK columns — the table has no + primary key, so the FK pair is what makes the order total and the pages + stable. + + + Known divergence from the JSP: it renders a "Time Last Modified" column + and NEITHER cross-reference table has a timestamp column. The field is + omitted here rather than invented. + security: + - BearerAuth: ['catalog-write'] + parameters: + - in: query + name: page + required: false + description: Zero-based page index. + schema: + type: integer + minimum: 0 + default: 0 + - in: query + name: limit + required: false + description: > + Defaults above the whole frozen collection (119 rows at most) so a + caller can fetch it in one request. + schema: + type: integer + minimum: 1 + maximum: 500 + default: 200 + responses: + '200': + description: > + One page of artist cross-references. An empty collection is a 200 + with `total: 0` — the JSP's "There are no Library Code + Cross-References" state — not a 404. + content: + application/json: + schema: + type: object + properties: + results: + type: array + items: + type: object + properties: + source_artist_id: + type: integer + source_artist_name: + type: string + target_artist_id: + type: integer + target_artist_name: + type: string + target_code_letters: + type: string + target_code_artist_number: + type: integer + nullable: true + comment: + type: string + nullable: true + total: + type: integer + page: + type: integer + totalPages: + type: integer + '400': + description: Invalid page or limit + + /library/crossreferences/releases: + get: + summary: List the legacy artist-to-release cross-references + description: > + Successor to `/wxycdb`'s `xrefsToLibraryReleases.jsp` — the whole + `artist_library_crossreference` collection, the pointers a librarian + filed from an artist card to a specific release. READ ONLY for the same + reason as its artist sibling; WXYC/wiki#89 D5 drops this set rather than + freezing it, which is a still stronger argument against a write path. + + + Enforced by `requirePermissions({ catalog: ['write'] })` — musicDirector + or above, same `hasAdminAccess()` reasoning as the sibling above. + + + Each row carries TWO artists: `artist_id`/`artist_name` is the + cross-REFERENCING artist, `album_artist_name` is whoever the + cross-referenced release is filed under. They are usually different — + that difference is the association the row records. The call number + belongs to the release and ships as PARTS (`code_letters` + + `code_artist_number` + `code_number` + `code_volume_letters`), matching + `GET /library/artists/{id}/releases`, not as a composed string. + `code_artist_number` is genre-scoped (hence the per-row `genre_id`) and + is null when the release's artist has no `genre_artist_crossreference` + row for that genre — LEFT joined so a frozen legacy row can never + silently vanish from a listing nothing else reproduces. + + + Known divergence from the JSP: no "Time Last Modified" — the table has + no timestamp column. + security: + - BearerAuth: ['catalog-write'] + parameters: + - in: query + name: page + required: false + description: Zero-based page index. + schema: + type: integer + minimum: 0 + default: 0 + - in: query + name: limit + required: false + schema: + type: integer + minimum: 1 + maximum: 500 + default: 200 + responses: + '200': + description: > + One page of release cross-references. An empty collection is a 200 + with `total: 0` — the JSP's "There are no Library Release + Cross-References" state — not a 404. + content: + application/json: + schema: + type: object + properties: + results: + type: array + items: + type: object + properties: + artist_id: + type: integer + artist_name: + type: string + library_id: + type: integer + album_title: + type: string + album_artist_name: + type: string + nullable: true + alternate_artist_name: + type: string + nullable: true + format_name: + type: string + genre_id: + type: integer + code_letters: + type: string + code_artist_number: + type: integer + nullable: true + code_number: + type: integer + code_volume_letters: + type: string + nullable: true + comment: + type: string + nullable: true + total: + type: integer + page: + type: integer + totalPages: + type: integer + '400': + description: Invalid page or limit + /library/formats: get: summary: Get format list diff --git a/apps/backend/controllers/library.controller.ts b/apps/backend/controllers/library.controller.ts index b01e97d34..4da7be88b 100644 --- a/apps/backend/controllers/library.controller.ts +++ b/apps/backend/controllers/library.controller.ts @@ -906,6 +906,101 @@ export const getArtistReleases: RequestHandler< res.status(200).json({ artist_id: artistId, releases, total, page, totalPages: Math.ceil(total / limit) }); }; +/** + * Page bounds for the two cross-reference collections. + * + * NOT `DEFAULT_LIMIT`/`MAX_LIMIT` (50/100), which the catalog-search endpoints + * share. Those cap an open-ended catalog; these cap two frozen legacy tables + * that hold 78 and 22 rows on prod (WXYC/wiki#89's 2026-08-11 measurement), + * with `artist_crossreference` rising to at most 119 once + * `scripts/audit/bs_2117_crossref_backfill.sql` has loaded the resolvable + * pairs. A 100-row ceiling would make the artist collection permanently + * un-fetchable in one request for the sake of a cap that never binds, so the + * default is set above the whole frozen set and the maximum a few multiples + * beyond it. The cap still exists rather than the endpoint serving the table + * whole: `jobs/library-etl` keeps upserting into both on a 30-minute cron + * until the tubafrenzy cutover, and the freeze is a decision, not something + * the query can enforce. + */ +const CROSSREFERENCE_DEFAULT_LIMIT = 200; +const CROSSREFERENCE_MAX_LIMIT = 500; + +type CrossReferenceQueryParams = { page?: string; limit?: string }; + +/** + * Parse `?page=`/`?limit=` for the two cross-reference listings. + * + * Same rules as `getArtistReleases` and `searchLibraryQueryEndpoint` — a + * repeated key is a 400 rather than a silent coercion, because Express's + * `simple` query parser yields `string[]` and `parseInt(['1','2'])` + * stringifies to `'1,2'` and returns `1` (#1553). Factored out here because + * two handlers need it identically; the existing callers keep their inline + * copies, which validate against different bounds. + */ +const parseCrossReferencePage = (query: CrossReferenceQueryParams): { page: number; limit: number } => { + if (query.page !== undefined && typeof query.page !== 'string') { + throw new WxycError('page must be a single string value', 400); + } + const page = parseInt(query.page ?? '0'); + if (isNaN(page) || page < 0) { + throw new WxycError('page must be a non-negative integer', 400); + } + + if (query.limit !== undefined && typeof query.limit !== 'string') { + throw new WxycError('limit must be a single string value', 400); + } + const limit = parseInt(query.limit ?? String(CROSSREFERENCE_DEFAULT_LIMIT)); + if (isNaN(limit) || limit < 1) { + throw new WxycError('limit must be a positive integer', 400); + } + if (limit > CROSSREFERENCE_MAX_LIMIT) { + throw new WxycError(`limit must not exceed ${CROSSREFERENCE_MAX_LIMIT}`, 400); + } + + return { page, limit }; +}; + +/** + * GET /library/crossreferences/artists — the whole `artist_crossreference` + * collection, successor to `/wxycdb`'s `xrefsToLibraryCodes.jsp`. + * + * Read-only by decision, not by omission: WXYC/wiki#89 D5 freezes this set at + * the tubafrenzy cutover, so there is no POST/PATCH/DELETE sibling and adding + * one would unfreeze it. An empty collection is a 200 with `total: 0`, which + * is what the JSP's "There are no Library Code Cross-References" state + * renders from — not a 404, since the collection exists and is empty. + */ +export const listArtistCrossReferences: RequestHandler = async ( + req, + res +) => { + const { page, limit } = parseCrossReferencePage(req.query); + const [results, total] = await Promise.all([ + libraryService.getArtistCrossReferences(page, limit), + libraryService.countArtistCrossReferences(), + ]); + res.status(200).json({ results, total, page, totalPages: Math.ceil(total / limit) }); +}; + +/** + * GET /library/crossreferences/releases — the whole + * `artist_library_crossreference` collection, successor to `/wxycdb`'s + * `xrefsToLibraryReleases.jsp`. Read-only for the same reason as its sibling + * above; D5 drops this set rather than freezing it, which is a still stronger + * argument against a write path. + */ +export const listReleaseCrossReferences: RequestHandler = async ( + req, + res +) => { + const { page, limit } = parseCrossReferencePage(req.query); + const [results, total] = await Promise.all([ + libraryService.getReleaseCrossReferences(page, limit), + libraryService.countReleaseCrossReferences(), + ]); + res.status(200).json({ results, total, page, totalPages: Math.ceil(total / limit) }); +}; + /** * Validate one optional free-text body field: must be a string, must not be * blank after trimming, must fit the column. Returns the trimmed value. diff --git a/apps/backend/routes/library.route.ts b/apps/backend/routes/library.route.ts index 6386fba47..bc95fb116 100644 --- a/apps/backend/routes/library.route.ts +++ b/apps/backend/routes/library.route.ts @@ -89,6 +89,39 @@ library_route.get( libraryController.exportBmiPerformanceList ); +// The two legacy cross-reference listings — successors to `/wxycdb`'s +// `xrefsToLibraryCodes.jsp` and `xrefsToLibraryReleases.jsp`. Read-only: both +// sets are frozen (artist codes) or dropped (releases) by WXYC/wiki#89's +// decision D5, so no write sibling belongs on this router. +// +// `catalog: ['write']` on a READ, deliberately, and for the same reason +// `/bmi-performance-list` above carries it: `mainmenu.jsp:32-38` wraps both of +// these links in ``, unlike Missing +// Releases and the rotation links right below them, which sit outside it. That +// flag is tubafrenzy's librarian/MD tier, and `catalog: ['write']` is the +// grant that selects musicDirector + stationManager here — the same pair — +// without minting a statement key. `catalog: ['read']` would hand every DJ and +// `member` a screen the legacy system gated; a new `catalog: ['admin']`-style +// key would have to be decided for all four roles (auth.roles.ts) to express a +// tier that already exists. `/artists/search` and `/artists/peek-code` set the +// same precedent for an admin-gated read. +// +// Two literal segments, so neither collides with the templated +// `/:id/compilation-tracks` GET further down (different second segment) — but +// they are registered here, ahead of every templated route on this router, +// rather than relying on that. +library_route.get( + '/crossreferences/artists', + requirePermissions({ catalog: ['write'] }), + libraryController.listArtistCrossReferences +); + +library_route.get( + '/crossreferences/releases', + requirePermissions({ catalog: ['write'] }), + libraryController.listReleaseCrossReferences +); + library_route.post('/', requirePermissions({ catalog: ['write'] }), libraryController.addAlbum); library_route.get('/rotation', requirePermissions({ catalog: ['read'] }), libraryController.getRotation); diff --git a/apps/backend/services/library.service.ts b/apps/backend/services/library.service.ts index 5f7d97872..5e255165b 100644 --- a/apps/backend/services/library.service.ts +++ b/apps/backend/services/library.service.ts @@ -1,4 +1,5 @@ import { and, asc, desc, eq, inArray, isNull, ne, notInArray, or, sql, SQL, type Column } from 'drizzle-orm'; +import { alias } from 'drizzle-orm/pg-core'; import { LRUCache } from 'lru-cache'; import * as Sentry from '@sentry/node'; import type { ReconciledIdentity, TrackMatchHint } from '@wxyc/shared/dtos'; @@ -15,6 +16,7 @@ import { RotationRelease, album_plays, album_popularity, + artist_crossreference, artist_library_crossreference, artists, bins, @@ -2565,6 +2567,221 @@ export const countReleasesForArtist = async (artist_id: number): Promise return Number(response[0]?.count ?? 0); }; +/** + * The two legacy cross-reference collections, read-only. + * + * `/wxycdb`'s admin menu carries two whole-collection views with no successor + * anywhere in this service — `xrefsToLibraryCodes.jsp` over + * `artist_crossreference` (artist -> artist, tubafrenzy's + * `LIBRARY_CODE_CROSS_REFERENCE`) and `xrefsToLibraryReleases.jsp` over + * `artist_library_crossreference` (artist -> release, tubafrenzy's + * `RELEASE_CROSS_REFERENCE`). Both are READ paths only, and deliberately so: + * WXYC/wiki#89 decision D5 freezes the artist-code cross-references at the + * tubafrenzy cutover ("post-cutover artists don't gain aliases; unfreezing is + * a future ticket") and drops the release cross-references outright. Adding a + * write path here would unfreeze a set that decision deliberately froze. + * + * Both collections are small and bounded by that freeze — the 2026-08-11 + * prod measurement on WXYC/wiki#89 has 78 rows in `artist_crossreference` + * (rising to at most 119, the source table's full size, once + * `scripts/audit/bs_2117_crossref_backfill.sql` has loaded its 110 resolvable + * pairs) and 22 in `artist_library_crossreference`. They are nonetheless + * paginated rather than served whole: `jobs/library-etl` still upserts into + * both on a 30-minute cron until the cutover, so the ceiling is a decision + * rather than a constraint the query can rely on, and the page/limit envelope + * costs a caller that wants everything exactly one parameter. + */ +const sourceArtist = alias(artists, 'source_artist'); +const targetArtist = alias(artists, 'target_artist'); + +export type ArtistCrossReferenceRow = { + source_artist_id: number; + source_artist_name: string; + target_artist_id: number; + target_artist_name: string; + target_code_letters: string; + target_code_artist_number: number | null; + comment: string | null; +}; + +/** + * Join chain for `xrefsToLibraryCodes.jsp`'s four columns. Shared verbatim by + * the page query and its `total` so the two cannot disagree about scope, the + * same arrangement `artistReleasesQuery` uses. + * + * Both FKs are `NOT NULL` and `ON DELETE CASCADE` to `artists`, so neither + * INNER JOIN can drop a row that the base table holds. + * + * `target_code_artist_number` is a correlated subquery, not a join. + * `artist_genre_key` is unique on `(artist_id, genre_id)` rather than on + * `artist_id`, so a legacy artist filed under several genres owns several + * `artist_genre_code`s — joining would fan one cross-reference row out into + * several. It picks the lowest `genre_id`, matching `getArtistCardById`'s + * collapse, so the code this view shows for an artist is the code that + * artist's own card shows. Nullable: `genre_artist_crossreference` has no row + * for an artist that was never filed under a genre. + * + * Only the TARGET carries a call number, matching the JSP: its + * "Cross-Referencing Artist" column renders a bare presentation name and its + * "Cross-Referenced Library Code" column renders code + name. + */ +const artistCrossReferencesQuery = () => + db + .select({ + source_artist_id: artist_crossreference.source_artist_id, + source_artist_name: sourceArtist.artist_name, + target_artist_id: artist_crossreference.target_artist_id, + target_artist_name: targetArtist.artist_name, + target_code_letters: targetArtist.code_letters, + target_code_artist_number: sql`( + SELECT gac.artist_genre_code + FROM ${genre_artist_crossreference} AS gac + WHERE gac.artist_id = ${targetArtist.id} + ORDER BY gac.genre_id ASC + LIMIT 1 + )`, + comment: artist_crossreference.comment, + }) + .from(artist_crossreference) + .innerJoin(sourceArtist, eq(sourceArtist.id, artist_crossreference.source_artist_id)) + .innerJoin(targetArtist, eq(targetArtist.id, artist_crossreference.target_artist_id)); + +/** + * One page of `artist_crossreference`, alphabetical by the cross-referencing + * artist. + * + * The table has no primary key and no timestamp — its only unique constraint + * is `artist_crossref_source_target` on the FK pair — so the sort carries both + * FK columns after the name to reach a total order. Without that, two artists + * sharing a `artist_name` (the legacy catalog has several) would order + * arbitrarily and rows could repeat or vanish across page boundaries. The name + * itself sorts under the database's default collation, which is what a + * librarian reading an alphabetical list expects; determinism comes from the + * id tiebreak, not from the collation. + */ +export const getArtistCrossReferences = async (page: number, limit: number): Promise => { + return artistCrossReferencesQuery() + .orderBy( + asc(sourceArtist.artist_name), + asc(artist_crossreference.source_artist_id), + asc(artist_crossreference.target_artist_id) + ) + .limit(limit) + .offset(page * limit); +}; + +/** Total row count for `getArtistCrossReferences`' page envelope (same join scope). */ +export const countArtistCrossReferences = async (): Promise => { + const response = await db + .select({ count: sql`count(*)::int` }) + .from(artistCrossReferencesQuery().as('artist_cross_references')); + + return Number(response[0]?.count ?? 0); +}; + +const referencingArtist = alias(artists, 'referencing_artist'); +const releaseArtist = alias(artists, 'release_artist'); + +export type ReleaseCrossReferenceRow = { + artist_id: number; + artist_name: string; + library_id: number; + album_title: string; + album_artist_name: string | null; + alternate_artist_name: string | null; + format_name: string; + genre_id: number; + code_letters: string; + code_artist_number: number | null; + code_number: number; + code_volume_letters: string | null; + comment: string | null; +}; + +/** + * Join chain for `xrefsToLibraryReleases.jsp`'s five columns, shared by the + * page query and its `total`. + * + * TWO artist rows per cross-reference, aliased apart because they are usually + * different artists: `referencing_artist` is the artist the cross-reference + * hangs off (`artist_library_crossreference.artist_id`), `release_artist` is + * whoever the cross-referenced release is filed under (`library.artist_id`). + * That difference is the whole point of the table — "Barry Black" pointing at + * an Eric Bachmann release — so collapsing them would lose the association + * the row records. The call number belongs to the RELEASE, hence + * `release_artist.code_letters`. + * + * `genre_artist_crossreference` is LEFT joined, unlike `artistReleasesQuery`'s + * INNER join of the same pair. There the join is a scoping predicate over one + * artist's shelf; here it supplies one display column, and this endpoint's job + * is to show a frozen legacy set in full. An artist missing its genre + * crossreference row would silently drop its cross-reference from a list + * nothing else in the system can reproduce. The key is still the + * `(artist_id, genre_id)` PAIR, because `artist_genre_code` is genre-scoped, + * so the join cannot fan out. + * + * `album_artist_name` COALESCEs `library.artist_name` over the joined artist, + * matching `getCatalogExportRows` — the denormalized column is the ETL's own + * value for the release and wins where it is present. + */ +const releaseCrossReferencesQuery = () => + db + .select({ + artist_id: artist_library_crossreference.artist_id, + artist_name: referencingArtist.artist_name, + library_id: artist_library_crossreference.library_id, + album_title: library.album_title, + album_artist_name: sql`COALESCE(${library.artist_name}, ${releaseArtist.artist_name})`, + alternate_artist_name: library.alternate_artist_name, + format_name: format.format_name, + genre_id: library.genre_id, + code_letters: releaseArtist.code_letters, + code_artist_number: genre_artist_crossreference.artist_genre_code, + code_number: library.code_number, + code_volume_letters: library.code_volume_letters, + comment: artist_library_crossreference.comment, + }) + .from(artist_library_crossreference) + .innerJoin(referencingArtist, eq(referencingArtist.id, artist_library_crossreference.artist_id)) + .innerJoin(library, eq(library.id, artist_library_crossreference.library_id)) + .innerJoin(releaseArtist, eq(releaseArtist.id, library.artist_id)) + .innerJoin(format, eq(format.id, library.format_id)) + .leftJoin( + genre_artist_crossreference, + and( + eq(genre_artist_crossreference.artist_id, library.artist_id), + eq(genre_artist_crossreference.genre_id, library.genre_id) + ) + ); + +/** + * One page of `artist_library_crossreference`, alphabetical by the + * cross-referencing artist. + * + * Same no-primary-key situation as `getArtistCrossReferences`: the unique + * constraint is `library_id_artist_id` over the FK pair, so both FK columns + * follow the name to make the order total and the pages stable. + */ +export const getReleaseCrossReferences = async (page: number, limit: number): Promise => { + return releaseCrossReferencesQuery() + .orderBy( + asc(referencingArtist.artist_name), + asc(artist_library_crossreference.artist_id), + asc(artist_library_crossreference.library_id) + ) + .limit(limit) + .offset(page * limit); +}; + +/** Total row count for `getReleaseCrossReferences`' page envelope (same join scope). */ +export const countReleaseCrossReferences = async (): Promise => { + const response = await db + .select({ count: sql`count(*)::int` }) + .from(releaseCrossReferencesQuery().as('release_cross_references')); + + return Number(response[0]?.count ?? 0); +}; + export const generateAlbumCodeNumber = async (artist_id: number): Promise => { const response = await db .select({ code_number: library.code_number }) diff --git a/tests/unit/controllers/library.crossReferences.test.ts b/tests/unit/controllers/library.crossReferences.test.ts new file mode 100644 index 000000000..f4acde4d4 --- /dev/null +++ b/tests/unit/controllers/library.crossReferences.test.ts @@ -0,0 +1,253 @@ +import { Request, Response, NextFunction } from 'express'; + +jest.mock('../../../apps/backend/services/library.service'); + +import * as libraryService from '../../../apps/backend/services/library.service'; +import { + listArtistCrossReferences, + listReleaseCrossReferences, +} from '../../../apps/backend/controllers/library.controller'; + +function mockReqResNext(overrides: Partial = {}) { + const req = { params: {}, query: {}, body: {}, auth: { id: 'test-user-id' }, ...overrides } as unknown as Request; + const statusMock = jest.fn().mockReturnThis(); + const jsonMock = jest.fn().mockReturnThis(); + const res = { status: statusMock, json: jsonMock } as unknown as Response; + const next = jest.fn() as unknown as NextFunction; + return { req, res, next, statusMock, jsonMock }; +} + +// Real rows from the tubafrenzy set these endpoints exist to preserve — the +// "Barry Black is filed w/ Eric Bachmann" pointer and the Don Caballero / +// Thee Speaking Canaries pair both appear in +// scripts/audit/bs_2117_crossref_backfill.sql's enumerated source data. +const ARTIST_CROSSREFERENCES: libraryService.ArtistCrossReferenceRow[] = [ + { + source_artist_id: 4102, + source_artist_name: 'Barry Black', + target_artist_id: 991, + target_artist_name: 'Eric Bachmann', + target_code_letters: 'BA', + target_code_artist_number: 42, + comment: 'Barry Black is filed w/ Eric Bachmann', + }, + { + source_artist_id: 1200, + source_artist_name: 'Don Caballero', + target_artist_id: 1855, + target_artist_name: 'Thee Speaking Canaries', + target_code_letters: 'SP', + target_code_artist_number: 72, + comment: null, + }, +]; + +const RELEASE_CROSSREFERENCES: libraryService.ReleaseCrossReferenceRow[] = [ + { + artist_id: 4102, + artist_name: 'Barry Black', + library_id: 20114, + album_title: 'To The Races', + album_artist_name: 'Eric Bachmann', + alternate_artist_name: null, + format_name: 'CD', + genre_id: 11, + code_letters: 'BA', + code_artist_number: 42, + code_number: 7, + code_volume_letters: null, + comment: 'see also Barry Black', + }, +]; + +const mockedService = libraryService as jest.Mocked; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +/** + * The two frozen `/wxycdb` cross-reference listings (`xrefsToLibraryCodes.jsp` + * and `xrefsToLibraryReleases.jsp`). Read-only by decision — WXYC/wiki#89 D5 + * freezes the artist set and drops the release set — so these cover the read + * contract only: the page shape, the empty-collection state each JSP renders + * as "There are no ... Cross-References", and the page/limit bounds. + */ +describe('GET /library/crossreferences/artists', () => { + it('answers the page envelope with one row per cross-reference', async () => { + mockedService.getArtistCrossReferences.mockResolvedValue(ARTIST_CROSSREFERENCES); + mockedService.countArtistCrossReferences.mockResolvedValue(2); + + const { req, res, next, statusMock, jsonMock } = mockReqResNext(); + await listArtistCrossReferences(req, res, next); + + expect(statusMock).toHaveBeenCalledWith(200); + expect(jsonMock).toHaveBeenCalledWith({ + results: ARTIST_CROSSREFERENCES, + total: 2, + page: 0, + totalPages: 1, + }); + }); + + it('projects both endpoints of the pair, with the call number on the target only', async () => { + mockedService.getArtistCrossReferences.mockResolvedValue(ARTIST_CROSSREFERENCES); + mockedService.countArtistCrossReferences.mockResolvedValue(2); + + const { req, res, next, jsonMock } = mockReqResNext(); + await listArtistCrossReferences(req, res, next); + + // The JSP renders a bare presentation name for the cross-REFERENCING + // artist and code + name for the cross-REFERENCED one; a row that carried + // a code for both, or for neither, would not reproduce that table. + const [row] = jsonMock.mock.calls[0][0].results; + expect(Object.keys(row).sort()).toEqual( + [ + 'comment', + 'source_artist_id', + 'source_artist_name', + 'target_artist_id', + 'target_artist_name', + 'target_code_artist_number', + 'target_code_letters', + ].sort() + ); + }); + + it('answers 200 with an empty page when there are no cross-references', async () => { + mockedService.getArtistCrossReferences.mockResolvedValue([]); + mockedService.countArtistCrossReferences.mockResolvedValue(0); + + const { req, res, next, statusMock, jsonMock } = mockReqResNext(); + await listArtistCrossReferences(req, res, next); + + // A 200 with total 0, NOT a 404: the collection exists and is empty, which + // is what the JSP's "There are no Library Code Cross-References" row is. + expect(statusMock).toHaveBeenCalledWith(200); + expect(jsonMock).toHaveBeenCalledWith({ results: [], total: 0, page: 0, totalPages: 0 }); + }); + + it('defaults to a limit that covers the whole frozen collection in one request', async () => { + mockedService.getArtistCrossReferences.mockResolvedValue(ARTIST_CROSSREFERENCES); + mockedService.countArtistCrossReferences.mockResolvedValue(2); + + const { req, res, next } = mockReqResNext(); + await listArtistCrossReferences(req, res, next); + + // 119 is the full size of tubafrenzy's LIBRARY_CODE_CROSS_REFERENCE, the + // ceiling D5's freeze pins this table to. A default at or below 100 would + // make the collection permanently un-fetchable in one call. + const [, limit] = mockedService.getArtistCrossReferences.mock.calls[0]; + expect(limit).toBeGreaterThanOrEqual(119); + }); + + it('passes page and limit through to the service', async () => { + mockedService.getArtistCrossReferences.mockResolvedValue([]); + mockedService.countArtistCrossReferences.mockResolvedValue(240); + + const { req, res, next, jsonMock } = mockReqResNext({ query: { page: '2', limit: '100' } } as Partial); + await listArtistCrossReferences(req, res, next); + + expect(mockedService.getArtistCrossReferences).toHaveBeenCalledWith(2, 100); + expect(jsonMock).toHaveBeenCalledWith({ results: [], total: 240, page: 2, totalPages: 3 }); + }); + + it.each([ + ['a negative page', { page: '-1' }], + ['a non-numeric page', { page: 'first' }], + ['a repeated page key', { page: ['0', '1'] }], + ['a zero limit', { limit: '0' }], + ['a non-numeric limit', { limit: 'all' }], + ['a limit over the maximum', { limit: '501' }], + ['a repeated limit key', { limit: ['10', '20'] }], + ])('rejects %s with a 400 before reading anything', async (_label, query) => { + const { req, res, next } = mockReqResNext({ query } as Partial); + await expect(listArtistCrossReferences(req, res, next)).rejects.toMatchObject({ statusCode: 400 }); + expect(mockedService.getArtistCrossReferences).not.toHaveBeenCalled(); + expect(mockedService.countArtistCrossReferences).not.toHaveBeenCalled(); + }); +}); + +describe('GET /library/crossreferences/releases', () => { + it('answers the page envelope with one row per cross-reference', async () => { + mockedService.getReleaseCrossReferences.mockResolvedValue(RELEASE_CROSSREFERENCES); + mockedService.countReleaseCrossReferences.mockResolvedValue(1); + + const { req, res, next, statusMock, jsonMock } = mockReqResNext(); + await listReleaseCrossReferences(req, res, next); + + expect(statusMock).toHaveBeenCalledWith(200); + expect(jsonMock).toHaveBeenCalledWith({ + results: RELEASE_CROSSREFERENCES, + total: 1, + page: 0, + totalPages: 1, + }); + }); + + it('carries the referencing artist and the release separately', async () => { + mockedService.getReleaseCrossReferences.mockResolvedValue(RELEASE_CROSSREFERENCES); + mockedService.countReleaseCrossReferences.mockResolvedValue(1); + + const { req, res, next, jsonMock } = mockReqResNext(); + await listReleaseCrossReferences(req, res, next); + + // The referencing artist is usually NOT the release's own artist -- that + // difference is the association the row records -- so the two must not + // collapse into one name field. + const [row] = jsonMock.mock.calls[0][0].results; + expect(row.artist_name).toBe('Barry Black'); + expect(row.album_artist_name).toBe('Eric Bachmann'); + // Call-number PARTS, not a composed "BA 42/7" string, matching every other + // catalog projection in this service. + expect(Object.keys(row).sort()).toEqual( + [ + 'album_artist_name', + 'album_title', + 'alternate_artist_name', + 'artist_id', + 'artist_name', + 'code_artist_number', + 'code_letters', + 'code_number', + 'code_volume_letters', + 'comment', + 'format_name', + 'genre_id', + 'library_id', + ].sort() + ); + }); + + it('answers 200 with an empty page when there are no cross-references', async () => { + mockedService.getReleaseCrossReferences.mockResolvedValue([]); + mockedService.countReleaseCrossReferences.mockResolvedValue(0); + + const { req, res, next, statusMock, jsonMock } = mockReqResNext(); + await listReleaseCrossReferences(req, res, next); + + expect(statusMock).toHaveBeenCalledWith(200); + expect(jsonMock).toHaveBeenCalledWith({ results: [], total: 0, page: 0, totalPages: 0 }); + }); + + it('passes page and limit through to the service', async () => { + mockedService.getReleaseCrossReferences.mockResolvedValue([]); + mockedService.countReleaseCrossReferences.mockResolvedValue(35); + + const { req, res, next, jsonMock } = mockReqResNext({ query: { page: '1', limit: '20' } } as Partial); + await listReleaseCrossReferences(req, res, next); + + expect(mockedService.getReleaseCrossReferences).toHaveBeenCalledWith(1, 20); + expect(jsonMock).toHaveBeenCalledWith({ results: [], total: 35, page: 1, totalPages: 2 }); + }); + + it.each([ + ['a negative page', { page: '-1' }], + ['a limit over the maximum', { limit: '501' }], + ])('rejects %s with a 400 before reading anything', async (_label, query) => { + const { req, res, next } = mockReqResNext({ query } as Partial); + await expect(listReleaseCrossReferences(req, res, next)).rejects.toMatchObject({ statusCode: 400 }); + expect(mockedService.getReleaseCrossReferences).not.toHaveBeenCalled(); + expect(mockedService.countReleaseCrossReferences).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/routes/library-crossreferences-permissions.route.test.ts b/tests/unit/routes/library-crossreferences-permissions.route.test.ts new file mode 100644 index 000000000..aabb58207 --- /dev/null +++ b/tests/unit/routes/library-crossreferences-permissions.route.test.ts @@ -0,0 +1,197 @@ +// Set required env vars before module load (ts-jest transforms imports to +// requires, so these execute before the auth middleware module's top-level +// code runs). Mirrors tests/unit/routes/library-artist-card-permissions.route.test.ts. +process.env.BETTER_AUTH_JWKS_URL = 'https://test.example.com/.well-known/jwks.json'; +process.env.BETTER_AUTH_ISSUER = 'https://test.example.com'; +process.env.BETTER_AUTH_AUDIENCE = 'https://test.example.com'; +delete process.env.AUTH_BYPASS; + +// Mock jose so we can hand back an arbitrary role in the verified JWT payload +// without a real JWKS endpoint. requirePermissions's non-bypass branch is +// what actually enforces role/permission checks. +jest.mock('jose', () => ({ + createRemoteJWKSet: jest.fn(() => jest.fn()), + jwtVerify: jest.fn(), + decodeJwt: jest.fn(), +})); + +// jest.unit.config.ts's moduleNameMapper sends `@wxyc/authentication` to +// tests/mocks/authentication.mock.ts (a stub that ignores the role/permission +// argument entirely). library.route.ts imports requirePermissions from that +// package specifier, so this route-wiring test needs the REAL implementation +// wired back in to actually exercise the catalog gates. +jest.mock('@wxyc/authentication', () => jest.requireActual('../../../shared/authentication/src/auth.middleware')); + +import { jest as jestGlobals } from '@jest/globals'; +import { jwtVerify } from 'jose'; +import express from 'express'; +import request from 'supertest'; +import type { ArtistCrossReferenceRow, ReleaseCrossReferenceRow } from '../../../apps/backend/services/library.service'; + +const mockedJwtVerify = jwtVerify as jest.MockedFunction; + +function mockRole(role: string) { + mockedJwtVerify.mockResolvedValue({ + payload: { sub: 'test-user-id', email: 'test@wxyc.org', role }, + protectedHeader: { alg: 'RS256' }, + key: {} as any, + }); +} + +const mockGetArtistCrossReferences = jestGlobals.fn<() => Promise>(); +const mockCountArtistCrossReferences = jestGlobals.fn<() => Promise>(); +const mockGetReleaseCrossReferences = jestGlobals.fn<() => Promise>(); +const mockCountReleaseCrossReferences = jestGlobals.fn<() => Promise>(); + +// Collaborator mocks below mirror the artist-card route-permission test -- +// only enough is stubbed here to let library.route's import chain resolve +// without touching a real DB, LML, or lru-cache. +jest.mock('../../../apps/backend/services/library.service', () => ({ + markAlbumMissing: jest.fn(), + markAlbumFound: jest.fn(), + getAlbumFromDB: jest.fn(), + getCatalogLastModifiedAt: jest.fn(), + serializeLibraryArtistViewEntry: (row: unknown) => row, + serializeArtist: (row: unknown) => row, + fuzzySearchLibrary: jest.fn(), + enrichWithArtwork: jest.fn(), + getFormatsFromDB: jest.fn(), + getRotationFromDB: jest.fn(), + addToRotation: jest.fn(), + killRotationInDB: jest.fn(), + insertAlbum: jest.fn(), + updateArtworkUrl: jest.fn(), + updateOnStreaming: jest.fn(), + updateCanonicalEntity: jest.fn(), + mapLookupToCanonicalEntity: jest.fn(), + artistIdFromName: jest.fn(), + getArtistNameById: jest.fn(), + insertArtist: jest.fn(), + insertArtistGenreCrossreference: jest.fn(), + getArtistByCode: jest.fn(), + getArtistById: jest.fn(), + generateAlbumCodeNumber: jest.fn(), + generateArtistNumber: jest.fn(), + getGenresFromDB: jest.fn(), + insertGenre: jest.fn(), + insertFormat: jest.fn(), + getFormatById: jest.fn(), + isISODate: jest.fn(), + resolveRotationPickerSource: jest.fn(), + getRotationTracksFromRelease: jest.fn(), + getLibraryRowById: jest.fn(), + updateAlbumInDB: jest.fn(), + artistExistsInGenre: jest.fn(), + albumCodeNumberTaken: jest.fn(), + recheckDiscogsAvailability: jest.fn(), + getArtistCardById: jest.fn(), + updateArtistInDB: jest.fn(), + getReleasesForArtist: jest.fn(), + countReleasesForArtist: jest.fn(), + getArtistCrossReferences: mockGetArtistCrossReferences, + countArtistCrossReferences: mockCountArtistCrossReferences, + getReleaseCrossReferences: mockGetReleaseCrossReferences, + countReleaseCrossReferences: mockCountReleaseCrossReferences, +})); + +jest.mock('../../../apps/backend/services/labels.service', () => ({ + createLabel: jest.fn(), + getLabelById: jest.fn(), +})); + +jest.mock('../../../apps/backend/services/library-search.service', () => ({ + parseEnumQueryList: () => undefined, + parseRotationBinsQueryList: () => undefined, + searchLibrary: jest.fn(), +})); + +jest.mock('@wxyc/lml-client', () => ({ + checkStreamingAvailability: jest.fn(), + lookupMetadata: jest.fn(), + isLmlConfigured: () => true, + envInt: (_name: string, fallback: number) => fallback, +})); + +jest.mock('../../../apps/backend/services/lml/lookup-coordinator', () => ({ + lmlLookupCoordinator: { lookup: jest.fn() }, +})); + +jest.mock('../../../apps/backend/controllers/requestLine.controller', () => ({ + searchLibraryEndpoint: (_req: unknown, res: { status: (n: number) => { json: (b: unknown) => void } }) => + res.status(200).json([]), +})); + +import { library_route } from '../../../apps/backend/routes/library.route'; + +const app = express(); +app.use(express.json()); +app.use('/library', library_route); + +/** + * The two legacy cross-reference listings are `catalog:['write']` READS. + * + * `mainmenu.jsp:32-38` wraps both links in `` -- unlike Missing Releases and the + * rotation links immediately below them, which sit outside it. `catalog: + * ['write']` is the grant that selects musicDirector + stationManager, the + * same pair that flag names. Without this test, relaxing the gate to + * `catalog:['read']` and handing every DJ and `member` a screen the legacy + * system gated would leave the rest of the suite green -- the controller + * tests drive the handlers directly. + */ +describe('Legacy cross-reference routes -- permission tier', () => { + beforeEach(() => { + mockGetArtistCrossReferences.mockReset().mockResolvedValue([]); + mockCountArtistCrossReferences.mockReset().mockResolvedValue(0); + mockGetReleaseCrossReferences.mockReset().mockResolvedValue([]); + mockCountReleaseCrossReferences.mockReset().mockResolvedValue(0); + }); + + describe.each([ + ['/library/crossreferences/artists', () => mockGetArtistCrossReferences], + ['/library/crossreferences/releases', () => mockGetReleaseCrossReferences], + ])('GET %s (catalog:write)', (path, reader) => { + test.each(['stationManager', 'musicDirector'])('a %s-role token is authorized', async (role) => { + mockRole(role); + const res = await request(app).get(path).set('Authorization', 'Bearer test-token'); + expect(res.status).toBe(200); + expect(reader()).toHaveBeenCalled(); + }); + + test.each(['dj', 'member'])('a %s-role token (catalog:read only) is rejected', async (role) => { + mockRole(role); + const res = await request(app).get(path).set('Authorization', 'Bearer test-token'); + expect(res.status).toBe(403); + expect(reader()).not.toHaveBeenCalled(); + }); + + test('a request with no Authorization header is rejected', async () => { + const res = await request(app).get(path); + expect(res.status).toBe(401); + expect(reader()).not.toHaveBeenCalled(); + }); + + // The empty collection is the JSP's "There are no ... Cross-References" + // state, and it is a 200 rather than a 404 -- asserted here alongside the + // gate so a permission change cannot be mistaken for an empty result. + test('an empty collection answers 200 with an empty page', async () => { + mockRole('musicDirector'); + const res = await request(app).get(path).set('Authorization', 'Bearer test-token'); + expect(res.status).toBe(200); + expect(res.body).toEqual({ results: [], total: 0, page: 0, totalPages: 0 }); + }); + }); + + // Registration order: `/crossreferences/*` must not fall through to a + // templated route. `GET /:id/compilation-tracks` is the only other + // two-segment GET on this router; it carries `catalog:['read']`, so a + // shadowed route would show up as a `dj` token succeeding above rather than + // as a 404. This pins the literal directly. + test('the two literals are distinct routes, not one templated handler', async () => { + mockRole('musicDirector'); + await request(app).get('/library/crossreferences/artists').set('Authorization', 'Bearer test-token'); + await request(app).get('/library/crossreferences/releases').set('Authorization', 'Bearer test-token'); + expect(mockGetArtistCrossReferences).toHaveBeenCalledTimes(1); + expect(mockGetReleaseCrossReferences).toHaveBeenCalledTimes(1); + }); +});