Expose alternate publication identifiers via metadata.altIdentifiers#837
Expose alternate publication identifiers via metadata.altIdentifiers#837raphi011 wants to merge 2 commits into
Conversation
|
In general, we don't want specialized properties when a generic one could fulfill the same goal and provide extensibility at the same time. The shared models for Swift/Kotlin/TS are based on RWPM where we included the concept of alt identifiers: https://readium.org/webpub-manifest/contexts/default/#identifier Therefore, additional identifiers (that do not identify themselves as the unique identifier of the publication) should be parsed and exposed in |
edcebe1 to
c131a92
Compare
Add the RWPM altIdentifier model — already shipped in the go and ts
toolkits — to Metadata and Contributor, surfacing identifiers other than
the primary one.
A new Shared AltIdentifier type ({value, scheme?}) parses from a bare URI
string or an object and collapses back to a string when no scheme is set.
For EPUB, Metadata.altIdentifiers is populated from every dc:identifier
other than the package's unique-identifier, mirroring go-toolkit: values
are returned as declared (only trimmed), in document order, with no scheme
synthesis.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c131a92 to
7c833b8
Compare
Makes sense 👍 I've updated the PR to match how the go implementation does it. Let me know what you think! |
mickael-menu
left a comment
There was a problem hiding this comment.
Thank you @raphi011!
Currently we're focusing on migrating the toolkit to Swift 6 with strict concurrency support in the swift6 branch. We accept bug fixes but new features are frozen until this is done (we're almost there). I'm putting your PR as a draft for now.
Overall your PR looks great and should be ready to merge without much changes. I'm wondering if we could potentially add a helper to access the values of known schemes directly, and where would that live (an ISBN could be in identifier too).
As a side note, would you mind opening an issue or discussion on the repo before you start working on a new feature or API change? I would like to make sure the proposal aligns with our vision and existing APIs. It also helps ensure that no one else has already started working on the same thing.
Thank you for your understanding 🙏
| /// The identifier value. | ||
| public var value: String | ||
|
|
||
| /// URI of the identifier's scheme, when known (e.g. `urn:isbn`). |
There was a problem hiding this comment.
This scheme is only for identifiers that cannot be represented as a URI. For example, the scheme https://www.example.com/ has the identifier abc123. An ISBN would be represented as urn:isbn:9789510184356 without a scheme.
There was a problem hiding this comment.
Pull request overview
Adds support for RWPM altIdentifier across the Swift toolkit by introducing a shared AltIdentifier model, wiring it into Metadata and Contributor JSON round-trips, and extracting alternate identifiers from EPUB OPF (dc:identifier) during parsing.
Changes:
- Introduces
AltIdentifier(value + optional scheme) with compact JSON serialization (string when scheme is absent). - Adds
altIdentifierstoMetadataandContributor, including JSON parsing/serialization under thealtIdentifierkey. - Updates EPUB metadata parsing and fixtures/tests to surface all non-unique
dc:identifiervalues asMetadata.altIdentifiers.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/StreamerTests/Parser/EPUB/EPUBMetadataParserTests.swift | Adds coverage for EPUB alt identifier extraction, order, and verbatim preservation. |
| Tests/StreamerTests/Fixtures/OPF/alt-identifier-urn.opf | New OPF fixture exercising urn: alternate identifier handling. |
| Tests/StreamerTests/Fixtures/OPF/alt-identifier-multiple.opf | New OPF fixture covering multiple alternate identifiers and document order. |
| Tests/StreamerTests/Fixtures/OPF/alt-identifier-epub2.opf | New OPF2 fixture ensuring verbatim identifier value handling (e.g. hyphens). |
| Tests/SharedTests/Publication/MetadataTests.swift | Extends JSON round-trip tests for Metadata.altIdentifiers. |
| Tests/SharedTests/Publication/ContributorTests.swift | Extends JSON round-trip tests for Contributor.altIdentifiers. |
| Tests/SharedTests/Publication/AltIdentifierTests.swift | New unit tests for string/object parsing and compact serialization. |
| Sources/Streamer/Parser/EPUB/EPUBMetadataParser.swift | Populates Metadata.altIdentifiers from OPF dc:identifier entries excluding the unique identifier. |
| Sources/Shared/Publication/Metadata.swift | Adds stored property + JSON key mapping for altIdentifiers. |
| Sources/Shared/Publication/Contributor.swift | Adds stored property + JSON key mapping for altIdentifiers. |
| Sources/Shared/Publication/AltIdentifier.swift | New shared model type implementing RWPM parsing/serialization rules. |
| CHANGELOG.md | Documents the new altIdentifiers API and EPUB parsing behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private lazy var altIdentifiers: [AltIdentifier] = { | ||
| let uniqueIdentifierID = document.firstChild(xpath: "/opf:package")?.attr("unique-identifier") | ||
| return metas["identifier", in: .dcterms] | ||
| .filter { $0.id != uniqueIdentifierID } |
Summary
Following the review feedback on the original
metadata.isbnsproposal: rather than a specialized accessor, this exposes all alternate publication identifiers through the generic RWPMaltIdentifierconcept — already implemented in go-toolkit and ts-toolkit, but missing from Swift.The EPUB parser splits the publication's
dc:identifiers: the one matching the package@unique-identifierstays the primarymetadata.identifier, and every other becomes an entry in the newmetadata.altIdentifiers. An ISBN — usually a secondarydc:identifier— is therefore surfaced as one alternate identifier among others (DOI, ISSN, a second UUID, …).altIdentifieris also added toContributor(parsing/serialization only), matching go-toolkit and ts-toolkit, both of which expose it onContributoras well.API
New
Sharedtype, mirroring go/ts:AltIdentifierparses from RWPM as either a bare URI string or a{ value, scheme }object (per altIdentifier.schema.json) and serializes back to a bare string when no scheme is set.Metadata.altIdentifiersround-trips under thealtIdentifierJSON key.Parsing
Mirrors go-toolkit's EPUB behavior: identifier values are returned as declared (only trimmed), in document order. No normalization and no scheme synthesis — the
opf:schemeattribute / ONIXidentifier-typerefinements are not interpreted, matching go-toolkit'sparseDcElement.Testing
AltIdentifierTests: string/object parsing, requiredvalue, and string-collapse serialization.MetadataTests/ContributorTests: full JSON round-trips extended with a mixed bare-string +{ value, scheme }array.EPUBMetadataParserTests+ OPF fixtures: alt extraction, verbatim values (hyphens preserved,opf:schemenot mapped to a scheme),urn:isbn, multiple alts in document order, and the no-alt case.make formatreports no changes.Notes
altIdentifieras an array. (ts-toolkit'sMetadatacurrently models it as a single value, which diverges from both the schema and its ownContributor.altIdentifiers.)Contributor.altIdentifiersis parsing/serialization only — the EPUB parser doesn't populate it, consistent with it not parsingContributor.identifierfrom EPUB today either.altIdentifieryet.schemeis left empty when parsing EPUB — an ISBN declared viaopf:scheme="ISBN"comes through as a bare value rather than aurn:isbnscheme. This matches every toolkit: go-toolkit keeps identifier values verbatim with no scheme (itsparseDcElementignoresopf:scheme), ts-toolkit has theAltIdentifiermodel but no EPUB parser, and kotlin-toolkit hasn't implementedaltIdentifieryet. So no Readium toolkit currently derives a scheme from EPUB markup; deriving well-known schemes (ISBN/DOI/ISSN) here is feasible but would make Swift the first to do so — happy to add it if you'd like that.