From 95164c3d4378285e84c88c9c0edda6166cc2972c Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 13 Mar 2026 17:14:20 -0300 Subject: [PATCH 1/3] Remove StrictLink to match OPDS 2.0 spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OPDS 2.0 spec (via Readium WebPub Manifest link schema) only requires href on links — rel and type are optional. StrictLink forced both to be present, causing parsing failures on spec-compliant feeds. The parent Link class already has these as optional, and collection-level validators enforce what the spec actually requires. --- src/palace/manager/feed/serializer/opds2.py | 28 ++++--- .../integration/license/opds/odl/extractor.py | 2 +- src/palace/manager/opds/odl/odl.py | 6 +- src/palace/manager/opds/opds2.py | 32 +++----- .../license/opds/odl/test_extractor.py | 8 +- .../integration/license/opds/test_requests.py | 6 +- tests/manager/opds/test_opds2.py | 75 ++++++++++++++----- 7 files changed, 90 insertions(+), 67 deletions(-) diff --git a/src/palace/manager/feed/serializer/opds2.py b/src/palace/manager/feed/serializer/opds2.py index f85764e0ed..bb3b4fb65d 100644 --- a/src/palace/manager/feed/serializer/opds2.py +++ b/src/palace/manager/feed/serializer/opds2.py @@ -221,10 +221,8 @@ def _serialize_publication_metadata( def _serialize_image_links(self, links: Iterable[Link]) -> list[opds2.Link]: return [self._serialize_link(link) for link in links] - def _serialize_publication_links( - self, data: WorkEntryData - ) -> list[opds2.StrictLink]: - links: list[opds2.StrictLink] = [] + def _serialize_publication_links(self, data: WorkEntryData) -> list[opds2.Link]: + links: list[opds2.Link] = [] for link in data.other_links: if link.rel is None: self.log.warning(f"Skipping OPDS2 link without rel: {link.href}") @@ -234,7 +232,7 @@ def _serialize_publication_links( self.log.error(f"Skipping OPDS2 link without type: {link.href}") continue links.append( - self._strict_link( + self._link( href=link.href, rel=link.rel, type=resolved_type, @@ -257,11 +255,11 @@ def _serialize_link(self, link: Link) -> opds2.Link: title=link.title, ) - def _serialize_acquisition_link(self, link: Acquisition) -> opds2.StrictLink | None: + def _serialize_acquisition_link(self, link: Acquisition) -> opds2.Link | None: link_type = self._acquisition_link_type(link) if link_type is None: return None - return self._strict_link( + return self._link( href=link.href, rel=link.rel or opds2.AcquisitionLinkRelations.acquisition, type=link_type, @@ -363,8 +361,8 @@ def entry_content_type(cls) -> str: def to_string(cls, data: dict[str, Any]) -> str: return json.dumps(data, indent=2) - def _serialize_feed_links(self, feed: FeedData) -> list[opds2.StrictLink]: - links: list[opds2.StrictLink] = [] + def _serialize_feed_links(self, feed: FeedData) -> list[opds2.Link]: + links: list[opds2.Link] = [] for link in feed.links: strict = self._serialize_feed_link(link) if strict is not None: @@ -372,12 +370,12 @@ def _serialize_feed_links(self, feed: FeedData) -> list[opds2.StrictLink]: return links - def _serialize_feed_link(self, link: Link) -> opds2.StrictLink | None: + def _serialize_feed_link(self, link: Link) -> opds2.Link | None: if link.rel is None: self.log.warning(f"Skipping OPDS2 feed link without rel: {link.href}") return None resolved_type = self._resolve_type(link.type) - return self._strict_link( + return self._link( href=link.href, rel=link.rel, type=resolved_type or self.content_type(), @@ -470,7 +468,7 @@ def _serialize_entry_groups( opds2.PublicationsGroup( metadata=opds2.FeedMetadata(title=group.title), links=[ - opds2.StrictLink( + opds2.Link( href=group.href, rel="self", type=self.content_type(), @@ -539,7 +537,7 @@ def _link_properties( palace_default=palace_default, ) - def _strict_link( + def _link( self, *, href: str, @@ -548,8 +546,8 @@ def _strict_link( title: str | None = None, properties: opds2.LinkProperties, templated: bool = False, - ) -> opds2.StrictLink: - return opds2.StrictLink( + ) -> opds2.Link: + return opds2.Link( href=href, rel=rel, type=type, diff --git a/src/palace/manager/integration/license/opds/odl/extractor.py b/src/palace/manager/integration/license/opds/odl/extractor.py index 0a9c380f4e..e0ac6f12c8 100644 --- a/src/palace/manager/integration/license/opds/odl/extractor.py +++ b/src/palace/manager/integration/license/opds/odl/extractor.py @@ -652,7 +652,7 @@ def _extract_bibliographic_links( def _extract_opds2_formats( self, - links: Sequence[opds2.StrictLink], + links: Sequence[opds2.Link], rights_uri: str, ) -> list[FormatData]: """Find circulation formats in non open-access acquisition links. diff --git a/src/palace/manager/opds/odl/odl.py b/src/palace/manager/opds/odl/odl.py index c4f168c65b..babcae137b 100644 --- a/src/palace/manager/opds/odl/odl.py +++ b/src/palace/manager/opds/odl/odl.py @@ -44,13 +44,13 @@ class License(BaseOpdsModel): """ metadata: LicenseMetadata - links: Annotated[CompactCollection[opds2.StrictLink], Field(min_length=1)] + links: Annotated[CompactCollection[opds2.Link], Field(min_length=1)] @field_validator("links") @classmethod def validate_links( - cls, value: CompactCollection[opds2.StrictLink] - ) -> CompactCollection[opds2.StrictLink]: + cls, value: CompactCollection[opds2.Link] + ) -> CompactCollection[opds2.Link]: """ Must have a self link and at least one acquisition link. diff --git a/src/palace/manager/opds/opds2.py b/src/palace/manager/opds/opds2.py index 73a46d1de5..7cb64d19cb 100644 --- a/src/palace/manager/opds/opds2.py +++ b/src/palace/manager/opds/opds2.py @@ -28,7 +28,7 @@ CompactCollection, validate_unique_links, ) -from palace.manager.opds.util import StrOrTuple, drop_if_falsy, obj_or_tuple_to_tuple +from palace.manager.opds.util import drop_if_falsy, obj_or_tuple_to_tuple from palace.manager.util.datetime_helpers import utc_now @@ -198,20 +198,6 @@ class Link(rwpm.Link): properties: LinkProperties = Field(default_factory=LinkProperties) -class StrictLink(Link): - """ - OPDS2 link with strict validation. - - These links require that the rel and type fields are present. - """ - - rel: StrOrTuple[str] - type: str - - alternate: CompactCollection[StrictLink] = Field(default_factory=CompactCollection) - children: CompactCollection[StrictLink] = Field(default_factory=CompactCollection) - - class TitleLink(Link): """ OPDS2 link with title. @@ -299,7 +285,7 @@ def content_type(cls) -> str: metadata: PublicationMetadata images: CompactCollection[Link] - links: CompactCollection[StrictLink] = Field(default_factory=CompactCollection) + links: CompactCollection[Link] = Field(default_factory=CompactCollection) _validate_images = field_validator("images")(validate_images) @@ -312,13 +298,13 @@ class Publication(BasePublication): https://github.com/opds-community/drafts/blob/main/schema/publication.schema.json """ - links: Annotated[CompactCollection[StrictLink], Field(min_length=1)] + links: Annotated[CompactCollection[Link], Field(min_length=1)] @field_validator("links") @classmethod def validate_acquisition_link( - cls, value: CompactCollection[StrictLink] - ) -> CompactCollection[StrictLink]: + cls, value: CompactCollection[Link] + ) -> CompactCollection[Link]: """ Must have at least one acquisition link. @@ -364,7 +350,7 @@ class PublicationsGroup(BaseOpdsModel): """ metadata: FeedMetadata - links: CompactCollection[StrictLink] = Field(default_factory=CompactCollection) + links: CompactCollection[Link] = Field(default_factory=CompactCollection) publications: Annotated[list[Publication], Field(min_length=1)] _validate_unique_links = field_validator("links")(validate_unique_links) @@ -379,7 +365,7 @@ class NavigationGroup(BaseOpdsModel): """ metadata: FeedMetadata - links: CompactCollection[StrictLink] = Field(default_factory=CompactCollection) + links: CompactCollection[Link] = Field(default_factory=CompactCollection) navigation: CompactCollection[TitleLink] = Field(..., min_length=1) _validate_unique_links = field_validator("links")(validate_unique_links) @@ -399,7 +385,7 @@ def content_type(cls) -> str: return "application/opds+json" metadata: FeedMetadata - links: CompactCollection[StrictLink] + links: CompactCollection[Link] navigation: CompactCollection[TitleLink] = Field(default_factory=CompactCollection) publications: list[Publication] = Field(default_factory=list) facets: list[Facet] = Field(default_factory=list) @@ -458,7 +444,7 @@ def content_type(cls) -> str: return "application/opds+json" metadata: FeedMetadata - links: CompactCollection[StrictLink] + links: CompactCollection[Link] publications: list[T] _validate_links = field_validator("links")(validate_self_link) diff --git a/tests/manager/integration/license/opds/odl/test_extractor.py b/tests/manager/integration/license/opds/odl/test_extractor.py index 6b65f45be0..78785096b9 100644 --- a/tests/manager/integration/license/opds/odl/test_extractor.py +++ b/tests/manager/integration/license/opds/odl/test_extractor.py @@ -14,7 +14,7 @@ from palace.manager.opds.odl.odl import License, LicenseMetadata, Publication from palace.manager.opds.odl.protection import Protection from palace.manager.opds.odl.terms import Terms -from palace.manager.opds.opds2 import PublicationFeedNoValidation, StrictLink +from palace.manager.opds.opds2 import Link, PublicationFeedNoValidation from palace.manager.opds.schema_org import Audience from palace.manager.sqlalchemy.constants import EditionConstants, MediaTypes from palace.manager.sqlalchemy.model.contributor import Contributor @@ -30,15 +30,15 @@ def __init__(self) -> None: self.license_identifier: str = "test-license-123" self.publication_identifier: str = "urn:isbn:9780306406157" - def license_links(self) -> list[StrictLink]: + def license_links(self) -> list[Link]: """Create standard license links for testing.""" return [ - StrictLink( + Link( rel=rwpm.LinkRelations.self, type=LicenseInfo.content_type(), href="http://example.org/license", ), - StrictLink( + Link( rel=opds2.AcquisitionLinkRelations.borrow, type=LoanStatus.content_type(), href="http://example.org/borrow", diff --git a/tests/manager/integration/license/opds/test_requests.py b/tests/manager/integration/license/opds/test_requests.py index fe71d5dc86..32aa33416f 100644 --- a/tests/manager/integration/license/opds/test_requests.py +++ b/tests/manager/integration/license/opds/test_requests.py @@ -24,8 +24,8 @@ from palace.manager.opds.authentication import AuthenticationDocument from palace.manager.opds.opds2 import ( FeedMetadata, + Link, PublicationFeedNoValidation, - StrictLink, ) from tests.fixtures.http import MockHttpClientFixture from tests.mocks.mock import MockRequestsResponse @@ -122,12 +122,12 @@ def opds2_feed_with_auth_link(self) -> str: metadata=FeedMetadata(title="Test Feed"), publications=[], links=[ - StrictLink( + Link( rel="http://opds-spec.org/auth/document", href=self.auth_doc_url, type=AuthenticationDocument.content_type(), ), - StrictLink( + Link( rel="self", href=self.auth_doc_url, type=PublicationFeedNoValidation.content_type(), diff --git a/tests/manager/opds/test_opds2.py b/tests/manager/opds/test_opds2.py index 36bdc19bcf..0ad7db6bdc 100644 --- a/tests/manager/opds/test_opds2.py +++ b/tests/manager/opds/test_opds2.py @@ -17,7 +17,6 @@ PublicationFeedNoValidation, PublicationMetadata, PublicationsGroup, - StrictLink, TitleLink, ) from palace.manager.util.datetime_helpers import utc_now @@ -317,7 +316,7 @@ def _minimal_publication(cls) -> Publication: ), images=[Link(href="http://img", type="image/png")], links=[ - StrictLink( + Link( href="http://acq", rel="http://opds-spec.org/acquisition/open-access", type="application/epub+zip", @@ -334,10 +333,8 @@ def _minimal_group(cls) -> PublicationsGroup: ) @classmethod - def _self_link(cls) -> list[StrictLink]: - return [ - StrictLink(href="http://feed", rel="self", type="application/opds+json") - ] + def _self_link(cls) -> list[Link]: + return [Link(href="http://feed", rel="self", type="application/opds+json")] def test_links_validates_self_link(self): """Feed.links must contain a self link.""" @@ -458,7 +455,7 @@ def _minimal_publication(cls) -> Publication: ), images=[Link(href="http://img", type="image/png")], links=[ - StrictLink( + Link( href="http://acq", rel="http://opds-spec.org/acquisition/open-access", type="application/epub+zip", @@ -467,7 +464,7 @@ def _minimal_publication(cls) -> Publication: ) def test_rejects_duplicate_links(self) -> None: - link = StrictLink(href="http://a", rel="other", type="text/html") + link = Link(href="http://a", rel="other", type="text/html") with pytest.raises(ValidationError, match="Duplicate link"): PublicationsGroup( metadata=FeedMetadata(title="Group"), @@ -479,8 +476,8 @@ def test_accepts_distinct_links(self) -> None: group = PublicationsGroup( metadata=FeedMetadata(title="Group"), links=[ - StrictLink(href="http://a", rel="other", type="text/html"), - StrictLink(href="http://b", rel="other", type="text/html"), + Link(href="http://a", rel="other", type="text/html"), + Link(href="http://b", rel="other", type="text/html"), ], publications=[self._minimal_publication()], ) @@ -491,7 +488,7 @@ class TestNavigationGroup: """Test the NavigationGroup model.""" def test_links_rejects_duplicate(self) -> None: - link = StrictLink(href="http://a", rel="other", type="text/html") + link = Link(href="http://a", rel="other", type="text/html") with pytest.raises(ValidationError, match="Duplicate link"): NavigationGroup( metadata=FeedMetadata(title="Nav"), @@ -511,8 +508,8 @@ def test_accepts_distinct(self) -> None: group = NavigationGroup( metadata=FeedMetadata(title="Nav"), links=[ - StrictLink(href="http://a", rel="other", type="text/html"), - StrictLink(href="http://b", rel="other", type="text/html"), + Link(href="http://a", rel="other", type="text/html"), + Link(href="http://b", rel="other", type="text/html"), ], navigation=[ TitleLink(href="http://n1", title="N1", type="text/html"), @@ -527,13 +524,13 @@ class TestBasePublicationFeed: """Test the BasePublicationFeed model (via PublicationFeed).""" @classmethod - def _self_link(cls) -> list[StrictLink]: + def _self_link(cls) -> list[Link]: return [ - StrictLink(href="http://feed", rel="self", type="application/opds+json") + Link(href="http://feed", rel="self", type="application/opds+json") ] def test_rejects_duplicate_links(self) -> None: - other_link = StrictLink(href="http://other", rel="other", type="text/html") + other_link = Link(href="http://other", rel="other", type="text/html") with pytest.raises(ValidationError, match="Duplicate link"): PublicationFeed( metadata=FeedMetadata(title="Feed"), @@ -546,9 +543,51 @@ def test_accepts_distinct_links(self) -> None: metadata=FeedMetadata(title="Feed"), links=self._self_link() + [ - StrictLink(href="http://a", rel="other", type="text/html"), - StrictLink(href="http://b", rel="other", type="text/html"), + Link(href="http://a", rel="other", type="text/html"), + Link(href="http://b", rel="other", type="text/html"), ], publications=[], ) assert len(feed.links) == 3 + + +class TestLinkRelaxedValidation: + """Links without rel or type are accepted, matching the OPDS 2.0 spec.""" + + def test_publication_link_without_rel_or_type(self) -> None: + """A publication can contain a link with only href.""" + pub = Publication( + metadata=PublicationMetadata( + title="Test", + identifier="urn:uuid:00000000-0000-0000-0000-000000000001", + type="http://schema.org/Book", + ), + images=[Link(href="http://img", type="image/png")], + links=[ + Link( + href="http://acq", + rel="http://opds-spec.org/acquisition/open-access", + type="application/epub+zip", + ), + Link(href="http://example.com/extra"), + ], + ) + assert len(pub.links) == 2 + + def test_feed_link_without_type(self) -> None: + """A feed can contain links that only have href and rel.""" + feed = Feed( + metadata=FeedMetadata(title="Feed"), + links=[Link(href="http://feed", rel="self")], + publications=[], + ) + assert len(feed.links) == 1 + + def test_publication_feed_link_without_type(self) -> None: + """A publication feed can contain links that only have href and rel.""" + feed = PublicationFeed( + metadata=FeedMetadata(title="Feed"), + links=[Link(href="http://feed", rel="self")], + publications=[], + ) + assert len(feed.links) == 1 From ebbe4c19ccd597f80d8e4c0d9447f860915e3938 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 13 Mar 2026 19:50:16 -0300 Subject: [PATCH 2/3] Fix remaining StrictLink references missed during merge conflict resolution --- tests/manager/opds/test_opds2.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/manager/opds/test_opds2.py b/tests/manager/opds/test_opds2.py index 0ad7db6bdc..ede5abbbd1 100644 --- a/tests/manager/opds/test_opds2.py +++ b/tests/manager/opds/test_opds2.py @@ -342,7 +342,7 @@ def test_links_validates_self_link(self): Feed( metadata=FeedMetadata(title="Feed"), links=[ - StrictLink( + Link( href="http://feed", rel="other", type="application/opds+json", @@ -353,7 +353,7 @@ def test_links_validates_self_link(self): def test_links_validates_unique(self): """Feed.links must not contain duplicate links.""" - other_link = StrictLink(href="http://other", rel="other", type="text/html") + other_link = Link(href="http://other", rel="other", type="text/html") with pytest.raises(ValidationError, match="Duplicate link"): Feed( metadata=FeedMetadata(title="Feed"), @@ -525,9 +525,7 @@ class TestBasePublicationFeed: @classmethod def _self_link(cls) -> list[Link]: - return [ - Link(href="http://feed", rel="self", type="application/opds+json") - ] + return [Link(href="http://feed", rel="self", type="application/opds+json")] def test_rejects_duplicate_links(self) -> None: other_link = Link(href="http://other", rel="other", type="text/html") From 8488bc8a2bfa82ca985a48366c9815311bda3c14 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 13 Mar 2026 19:54:32 -0300 Subject: [PATCH 3/3] Move relaxed validation tests into model test classes Rename TestLinkRelaxedValidation by moving its tests into the appropriate model-based test classes (TestPublication, TestFeed, TestBasePublicationFeed) to follow the project's naming convention. --- tests/manager/opds/test_opds2.py | 66 ++++++++++++++++---------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/tests/manager/opds/test_opds2.py b/tests/manager/opds/test_opds2.py index ede5abbbd1..349f9dd2b5 100644 --- a/tests/manager/opds/test_opds2.py +++ b/tests/manager/opds/test_opds2.py @@ -305,6 +305,28 @@ def test_model_serializer_populated_accessibility_serialized(self) -> None: assert data["accessibility"]["summary"] == "Fully accessible." +class TestPublication: + def test_link_without_rel_or_type(self) -> None: + """A publication can contain a link with only href.""" + pub = Publication( + metadata=PublicationMetadata( + title="Test", + identifier="urn:uuid:00000000-0000-0000-0000-000000000001", + type="http://schema.org/Book", + ), + images=[Link(href="http://img", type="image/png")], + links=[ + Link( + href="http://acq", + rel="http://opds-spec.org/acquisition/open-access", + type="application/epub+zip", + ), + Link(href="http://example.com/extra"), + ], + ) + assert len(pub.links) == 2 + + class TestFeed: @classmethod def _minimal_publication(cls) -> Publication: @@ -361,6 +383,15 @@ def test_links_validates_unique(self): publications=[self._minimal_publication()], ) + def test_link_without_type(self) -> None: + """A feed can contain links that only have href and rel.""" + feed = Feed( + metadata=FeedMetadata(title="Feed"), + links=[Link(href="http://feed", rel="self")], + publications=[], + ) + assert len(feed.links) == 1 + def test_serialization_only_truthy_collections_are_kept(self): """When only groups is truthy, publications and navigation are dropped.""" feed = Feed( @@ -548,40 +579,7 @@ def test_accepts_distinct_links(self) -> None: ) assert len(feed.links) == 3 - -class TestLinkRelaxedValidation: - """Links without rel or type are accepted, matching the OPDS 2.0 spec.""" - - def test_publication_link_without_rel_or_type(self) -> None: - """A publication can contain a link with only href.""" - pub = Publication( - metadata=PublicationMetadata( - title="Test", - identifier="urn:uuid:00000000-0000-0000-0000-000000000001", - type="http://schema.org/Book", - ), - images=[Link(href="http://img", type="image/png")], - links=[ - Link( - href="http://acq", - rel="http://opds-spec.org/acquisition/open-access", - type="application/epub+zip", - ), - Link(href="http://example.com/extra"), - ], - ) - assert len(pub.links) == 2 - - def test_feed_link_without_type(self) -> None: - """A feed can contain links that only have href and rel.""" - feed = Feed( - metadata=FeedMetadata(title="Feed"), - links=[Link(href="http://feed", rel="self")], - publications=[], - ) - assert len(feed.links) == 1 - - def test_publication_feed_link_without_type(self) -> None: + def test_link_without_type(self) -> None: """A publication feed can contain links that only have href and rel.""" feed = PublicationFeed( metadata=FeedMetadata(title="Feed"),