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..349f9dd2b5 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 @@ -306,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: @@ -317,7 +338,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 +355,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.""" @@ -345,7 +364,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", @@ -356,7 +375,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"), @@ -364,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( @@ -458,7 +486,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 +495,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 +507,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 +519,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 +539,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 +555,11 @@ class TestBasePublicationFeed: """Test the BasePublicationFeed model (via PublicationFeed).""" @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_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 +572,18 @@ 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 + + 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"), + links=[Link(href="http://feed", rel="self")], + publications=[], + ) + assert len(feed.links) == 1