From 9e2cbfd3a51bef0f615679d36b70c2424ca58c34 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:04:31 +0000 Subject: [PATCH] fix(metadata): preserve epub creators with multiple roles Read every EPUB 3 role refinement so authors and narrators survive regardless of role order, including creators who hold both roles. Preserve EPUB 2 role precedence and contributor handling, and add regression coverage for ordering, normalization, and deduplication. Fixes #1344 --- .../modules/metadata/lib/opf-parser.test.ts | 144 ++++++++++++++++++ server/src/modules/metadata/lib/opf-parser.ts | 25 ++- 2 files changed, 161 insertions(+), 8 deletions(-) diff --git a/server/src/modules/metadata/lib/opf-parser.test.ts b/server/src/modules/metadata/lib/opf-parser.test.ts index 0edb55eed..f13bb113a 100644 --- a/server/src/modules/metadata/lib/opf-parser.test.ts +++ b/server/src/modules/metadata/lib/opf-parser.test.ts @@ -167,6 +167,97 @@ describe('parseOpf', () => { expect(r.authors[0].sortName).toBe('Pratchett, Terry'); }); + it.each( + [ + ['aut', 'ann', 'dto', 'ill', 'wpr'], + ['ann', 'aut', 'dto', 'ill', 'wpr'], + ['ann', 'dto', 'aut', 'ill', 'wpr'], + ['ann', 'dto', 'ill', 'aut', 'wpr'], + ['ann', 'dto', 'ill', 'wpr', 'aut'], + ].map((roles) => ({ roles })), + )('includes a multi-role author with roles $roles', ({ roles }) => { + const xml = epub3Opf(` + Flatland + Edwin A. Abbott + Abbott, Edwin A. + ${roles.map((role) => `${role}`).join('\n')} + `); + + const r = parseOpf(xml); + expect(r.title).toBe('Flatland'); + expect(r.authors).toEqual([{ name: 'Edwin A. Abbott', sortName: 'Abbott, Edwin A.' }]); + expect(r.narrators).toEqual([]); + }); + + it.each([' AUT ', 'http://id.loc.gov/vocabulary/relators/aut', 'marc:aut'])('normalizes a later author role %s', (role) => { + const xml = epub3Opf(` + Author Name + ann + ${role} + `); + + expect(parseOpf(xml).authors).toEqual([{ name: 'Author Name', sortName: null }]); + }); + + it.each([{ roles: ['ann', 'ill'] }, { roles: ['', 'edt'] }, { roles: ['edt', ''] }])( + 'excludes a creator with no author role among $roles', + ({ roles }) => { + const xml = epub3Opf(` + Editor Name + ${roles.map((role) => `${role}`).join('\n')} + `); + + expect(parseOpf(xml).authors).toEqual([]); + expect(parseOpf(xml).narrators).toEqual([]); + }, + ); + + it('keeps a creator whose role refinements are all empty as an author', () => { + const xml = epub3Opf(` + Author Name + + + `); + + expect(parseOpf(xml).authors).toEqual([{ name: 'Author Name', sortName: null }]); + }); + + it.each([ + { attribute: 'edt', refinements: ['ann', 'aut'], isAuthor: true }, + { attribute: 'aut', refinements: ['ann', 'ill'], isAuthor: false }, + { attribute: 'edt', refinements: [], isAuthor: false }, + { attribute: 'aut', refinements: [], isAuthor: true }, + { attribute: 'edt', refinements: [''], isAuthor: true }, + ])('preserves role precedence for attribute $attribute and refinements $refinements', ({ attribute, refinements, isAuthor }) => { + const xml = epub3Opf(` + Author Name + ${refinements.map((role) => `${role}`).join('\n')} + `); + + expect(parseOpf(xml).authors).toEqual(isAuthor ? [{ name: 'Author Name', sortName: 'Name, Author' }] : []); + }); + + it('keeps roles scoped to their person and emits each creator once in document order', () => { + const xml = epub3Opf(` + Editor Name + First Author + Second Author + edt + ill + ann + aut + aut + aut + Author, First + Unused Sort Name + `); + + expect(parseOpf(xml).authors).toEqual([ + { name: 'First Author', sortName: 'Author, First' }, + { name: 'Second Author', sortName: null }, + ]); + }); + it('accepts EPUB3 role values in full relator URI form', () => { const xml = epub3Opf(` Ursula Le Guin @@ -229,6 +320,59 @@ describe('parseOpf', () => { expect(r.authors).toEqual([{ name: 'Neil Gaiman', sortName: null }]); }); + it.each(['creator', 'contributor'])('recognizes a later narrator role on a %s', (element) => { + const xml = epub3Opf(` + Narrator Name + ann + http://id.loc.gov/vocabulary/relators/nrt + `); + + const r = parseOpf(xml); + expect(r.narrators).toEqual(['Narrator Name']); + expect(r.authors).toEqual([]); + }); + + it.each([{ roles: ['aut', 'nrt'] }, { roles: ['nrt', 'aut'] }])('records a self-narrating creator in both roles for $roles', ({ roles }) => { + const xml = epub3Opf(` + Author Narrator + Narrator, Author + ${roles.map((role) => `${role}`).join('\n')} + `); + + const r = parseOpf(xml); + expect(r.authors).toEqual([{ name: 'Author Narrator', sortName: 'Narrator, Author' }]); + expect(r.narrators).toEqual(['Author Narrator']); + }); + + it('does not skip authorship when a multi-role creator repeats a previously collected narrator', () => { + const xml = epub3Opf(` + Author Narrator + author narrator + nrt + nrt + aut + narrator + `); + + const r = parseOpf(xml); + expect(r.authors).toEqual([{ name: 'author narrator', sortName: null }]); + expect(r.narrators).toEqual(['Author Narrator']); + }); + + it.each([{ roles: [] }, { roles: [''] }, { roles: ['aut'] }, { roles: ['aut', 'nrt'] }])( + 'does not promote a contributor with roles $roles to an author', + ({ roles }) => { + const xml = epub3Opf(` + Contributor Name + ${roles.map((role) => `${role}`).join('\n')} + `); + + const r = parseOpf(xml); + expect(r.authors).toEqual([]); + expect(r.narrators).toEqual(roles.includes('nrt') ? ['Contributor Name'] : []); + }, + ); + it('accepts a narrator role in full relator URI form', () => { const xml = epub3Opf(` Rosamund Pike diff --git a/server/src/modules/metadata/lib/opf-parser.ts b/server/src/modules/metadata/lib/opf-parser.ts index e4c81f103..469c8c7b1 100644 --- a/server/src/modules/metadata/lib/opf-parser.ts +++ b/server/src/modules/metadata/lib/opf-parser.ts @@ -279,6 +279,15 @@ function getRefineValue(refines: Map, id: string, property: s return null; } +function getRefineValues(refines: Map, id: string, property: string): string[] { + const values: string[] = []; + for (const node of refines.get(id) ?? []) { + const m = node as Record; + if (m['@_property'] === property) values.push(getText(m)); + } + return values; +} + export function parseOpf(xml: string): ParsedOpf { const root = parser.parse(xml) as Record; const pkg = (root['package'] ?? root['opf:package'] ?? {}) as Record; @@ -350,21 +359,21 @@ export function parseOpf(xml: string): ParsedOpf { const id = mo['@_id'] as string | undefined; // EPUB 3: role via refines - const role3 = id ? getRefineValue(refineMap, id, 'role') : null; + const roles3 = id ? getRefineValues(refineMap, id, 'role') : []; // EPUB 2: opf:role attribute const role2 = (mo['@_opf:role'] ?? mo['@_role']) as string | undefined; - const role = normalizeCreatorRole(role3 ?? role2 ?? defaults.role); + const roles = (roles3.length > 0 ? roles3 : [role2 ?? defaults.role]).map(normalizeCreatorRole).filter(Boolean); - if (NARRATOR_ROLES.has(role)) { + if (roles.some((role) => NARRATOR_ROLES.has(role))) { const key = name.toLowerCase(); - if (seenNarrators.has(key)) return; - seenNarrators.add(key); - narrators.push(name); - return; + if (!seenNarrators.has(key)) { + seenNarrators.add(key); + narrators.push(name); + } } if (!defaults.allowAuthor) return; - if (role !== 'aut' && role !== '') return; // skip editors, illustrators, etc. + if (roles.length > 0 && !roles.includes('aut')) return; // skip editors, illustrators, etc. // EPUB 3: file-as via refines; EPUB 2: opf:file-as attribute const sortName = (id ? getRefineValue(refineMap, id, 'file-as') : null) ?? ((mo['@_opf:file-as'] ?? mo['@_file-as'] ?? null) as string | null);