Skip to content

fix(opds): align mapping with OPDS 2.0 WebPub properties, support OPD…#125

Open
zeedif wants to merge 5 commits intojohnfactotum:mainfrom
zeedif:fix/opds-2.0-compliance-and-pse
Open

fix(opds): align mapping with OPDS 2.0 WebPub properties, support OPD…#125
zeedif wants to merge 5 commits intojohnfactotum:mainfrom
zeedif:fix/opds-2.0-compliance-and-pse

Conversation

@zeedif
Copy link
Copy Markdown
Contributor

@zeedif zeedif commented Apr 9, 2026

This PR aims to refine the parser so that OPDS 1.x attributes perfectly align with the target OPDS 2.0 (Readium WebPub Manifest) layout without bleeding deprecated metadata onto the root object, while fixing deep DOM evaluation bugs and bringing support for OPDS-PSE (Page Streaming) and Paging extensions.

Improvements & Bug Fixes

  • Aligning OPDS 1.x mapping exclusively with OPDS 2.0 properties:

  • Prices & Indirect Acquisitions Hierarchy Bug Fix:

  • OPDS Page Streaming Extension (OPDS-PSE) Support:

    • Added REL.STREAM (http://vaemendis.net/opds-pse/stream) to properly recognize stream endpoints as OPDS publications rather than navigational entries.
    • Safe-maps namespaced extensions (pse:count, pse:lastRead, and pse:lastReadDate) into the properties object to match the WebPub Manifest specification for custom properties.
  • RFC 5005 (Paging and Archiving):

    • Adds evaluation for the fh:complete and fh:archive elements using the http://purl.org/syndication/history/1.0 namespace exposing them transparently at the root level of the returned feed wrapper.

@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch from 5eca82c to 518a173 Compare April 9, 2026 20:43
@johnfactotum
Copy link
Copy Markdown
Owner

Thanks. Currently the PR is rather large, with many unrelated changes. Can you perhaps make each change a separate PR?

In particular, a lot of the changes seem to be merely avoiding having properties with undefined values on the resulting objects. Is there a reason for this? It doesn't seem to be related to the main changes that you're describing.

@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch from 518a173 to 0f4822c Compare April 10, 2026 15:58
@zeedif
Copy link
Copy Markdown
Contributor Author

zeedif commented Apr 10, 2026

The reason for removing null and omitting empty properties is strict compliance with the OPDS 2.0 Specification. The spec defines a Blank Value as null, "", [], or {}. Furthermore, Section 5.2 Metadata explicitly states: "A metadata object in OPDS 2.0 should not contain any properties with a blank value."

I viewed the initial mapping fixes and the blank value omissions as a single logical unit, as both serve the exact same goal: mapping OPDS 1.x feeds into a perfectly spec-compliant OPDS 2.0 / WebPub layout.

However, the middle commit was definitely a tangent. I have removed the 2nd commit (Legacy/Stanza support) from this branch right now and move it to its own isolated PR.

Regarding the 1st and 3rd commits: since they both work together to achieve strict OPDS 2.0 compliance, does it make sense to keep them together here, or would you still prefer I split the "Blank Values" refactor into a third PR?

@johnfactotum
Copy link
Copy Markdown
Owner

OK. I think I got a bit confused, as the objects returned by getOpenSearch/getSearch are not OPDS documents at all. Personally I think empty values are OK as it's not really intended to be used to publish OPDS, but merely to convert them for use in reading systems ("SHOULD NOT [...] mean[s] that there may exist valid reasons in particular circumstances when the particular behavior is acceptable or even useful"). I also find it cleaner to just clean up the object afterwards, like what I did in epub.js with tidy(), though I guess that's a matter of taste depending on one's specific needs.

@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch from 0f4822c to 6240826 Compare April 10, 2026 18:12
Comment thread opds.js Fixed
Comment thread opds.js Fixed
@zeedif
Copy link
Copy Markdown
Contributor Author

zeedif commented Apr 10, 2026

My initial reasoning for omitting empty properties was to strictly adhere to the OPDS 2.0 specification regarding "Blank Values," aiming for a minimal JSON output. However, as you pointed out, since this library acts as a client consumer and not a publisher, the benefits of such strictness are minimal and unnecessary.

I've refactored the code to align with your suggested approach. The defensive checks have been removed in favor of a more direct, declarative style. I only retained the change from null to undefined and the conditional extraction logic for link attributes based on their role. Thank you for the feedback.

@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch from 6240826 to b459222 Compare April 10, 2026 18:24
@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch 2 times, most recently from ce1c8eb to 86568fd Compare April 10, 2026 23:48
Comment thread opds.js Fixed
@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch from 86568fd to 4fcabc9 Compare April 10, 2026 23:52
@zeedif zeedif force-pushed the fix/opds-2.0-compliance-and-pse branch from 4fcabc9 to c9ee8d7 Compare April 10, 2026 23:52
@zeedif
Copy link
Copy Markdown
Contributor Author

zeedif commented Apr 12, 2026

I've mapped DOM null returns to undefined. This makes it much simpler when working with TypeScript, as the object will only expect undefined rather than having to handle both types. I also generalized the numberOfItems fallback because, according to OPDS 1.2 spec regarding the thr:count attribute (extracted from RFC4685), I realized that the thr:count property is not exclusively tied to links pointing to facet feeds. Because of this, the count attribute should apply as a fallback for numberOfItems on all non-stream links. Lastly, I removed the redundant check on properties (if (Object.keys(obj.properties).length === 0) delete obj.properties) because it was an unnecessary cleanup step.

I consider it ready for review and acceptance, if there are no other details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants