Refactor sort link serialization for V1 and V2 or the OPDS1 and OPDS2…

… feeds.

src/palace/manager/feed/serializer/opds.py

@@ -119,12 +119,9 @@ def serialize_feed(
             serialized.append(breadcrumbs)
 
         for link in feed.facet_links:
-            if is_sort_link(link):
-                serialized.append(self._serialize_sort_link(link))
-                # TODO once the clients are no longer relying on facet based sorting
-                # an "else" should be introduced here since we only need to provide one style of sorting links
             serialized.append(self._serialize_feed_entry("link", link))
-            # TODO end else here
+
+        self._add_sort_links(serialized, feed.sort_links)
 
         etree.indent(serialized)
         return self.to_string(serialized)
@@ -408,13 +405,8 @@ def to_string(cls, element: etree._Element) -> str:
     def content_type(self) -> str:
         return OPDSFeed.ACQUISITION_FEED_TYPE
 
-    def _serialize_sort_link(self, link: Link) -> etree._Element:
-        sort_link = Link(
-            href=link.href, title=link.title, rel=AtomFeed.PALACE_REL_NS + "sort"
-        )
-        if link.get("activeFacet", False):
-            sort_link.add_attributes(dict(activeSort="true"))
-        return self._serialize_feed_entry("link", sort_link)
+    def _add_sort_links(self, serialized, sort_links: list[Link] = None):
+        """Serialize and add sort links to the serialized representation"""
 
 
 class OPDS1Version2Serializer(OPDS1Version1Serializer):
@@ -423,3 +415,16 @@ class OPDS1Version2Serializer(OPDS1Version1Serializer):
 
     def __init__(self) -> None:
         pass
+
+    def _add_sort_links(self, serialized, sort_links: list[Link] = None):
+        if sort_links:
+            for link in sort_links:
+                serialized.append(self._serialize_sort_link(link))
+
+    def _serialize_sort_link(self, link: Link) -> etree._Element:
+        sort_link = Link(
+            href=link.href, title=link.title, rel=AtomFeed.PALACE_REL_NS + "sort"
+        )
+        if link.get("activeFacet", False):
+            sort_link.add_attributes(dict(activeSort="true"))
+        return self._serialize_feed_entry("link", sort_link)

src/palace/manager/feed/serializer/opds2.py

@@ -3,7 +3,6 @@
 from typing import Any
 
 from palace.manager.feed.serializer.base import SerializerInterface
-from palace.manager.feed.serializer.opds import is_sort_link
 from palace.manager.feed.types import (
     Acquisition,
     Author,
@@ -193,33 +192,20 @@ def _serialize_feed_links(self, feed: FeedData) -> dict[str, Any]:
         for link in feed.links:
             link_data["links"].append(self._serialize_link(link))
 
+        self._add_sort_links(link_data["links"], feed.sort_links)
+
         facet_links: dict[str, Any] = defaultdict(lambda: {"metadata": {}, "links": []})
         for link in feed.facet_links:
-            if is_sort_link(link):
-                # TODO: When we remove the facet-based sort links [PP-1814],
-                # this code path will be removed and we'll want to pull the sort
-                # link data from the feed.sort_links once that is in place.
-                link_data["links"].append(self._serialize_sort_link(link))
-            else:
-                group = getattr(link, "facetGroup", None)
-                if group:
-                    facet_links[group]["links"].append(self._serialize_link(link))
-                    facet_links[group]["metadata"]["title"] = group
+            group = getattr(link, "facetGroup", None)
+            if group:
+                facet_links[group]["links"].append(self._serialize_link(link))
+                facet_links[group]["metadata"]["title"] = group
+
         for _, facets in facet_links.items():
             link_data["facets"].append(facets)
 
         return link_data
 
-    def _serialize_sort_link(self, link: Link) -> dict[str, Any]:
-        sort_link: dict[str, Any] = {
-            "href": link.href,
-            "title": link.title,
-            "rel": PALACE_REL_SORT,
-        }
-        if link.get("activeFacet", False):
-            sort_link["properties"] = {PALACE_PROPERTIES_ACTIVE_SORT: "true"}
-        return sort_link
-
     def _serialize_contributor(self, author: Author) -> dict[str, Any]:
         result: dict[str, Any] = {"name": author.name}
         if author.sort_name:
@@ -238,9 +224,26 @@ def content_type(self) -> str:
     def to_string(cls, data: dict[str, Any]) -> str:
         return json.dumps(data, indent=2)
 
+    def _add_sort_links(self, serialized_links: list[Link], sort_links: list[Link]):
+        pass
+
 
 class OPDS2Version2Serializer(OPDS2Version1Serializer):
     CONTENT_TYPE = "application/opds+json"
 
     def __init__(self) -> None:
         pass
+
+    def _serialize_sort_link(self, link: Link) -> dict[str, Any]:
+        sort_link: dict[str, Any] = {
+            "href": link.href,
+            "title": link.title,
+            "rel": PALACE_REL_SORT,
+        }
+        if link.get("activeFacet", False):
+            sort_link["properties"] = {PALACE_PROPERTIES_ACTIVE_SORT: "true"}
+        return sort_link
+
+    def _add_sort_links(self, serialized_links: list[Link], sort_links: list[Link]):
+        for link in sort_links:
+            serialized_links.append(self._serialize_sort_link(link))

src/palace/manager/feed/types.py

@@ -235,6 +235,7 @@ class FeedData(BaseModel):
     links: list[Link] = field(default_factory=list)
     breadcrumbs: list[Link] = field(default_factory=list)
     facet_links: list[Link] = field(default_factory=list)
+    sort_links: list[Link] = field(default_factory=list)
     entries: list[WorkEntry] = field(default_factory=list)
     data_entries: list[DataEntry] = field(default_factory=list)
     metadata: FeedMetadata = field(default_factory=lambda: FeedMetadata())

tests/manager/feed/test_opds2_serializer.py

@@ -4,6 +4,7 @@
     PALACE_PROPERTIES_ACTIVE_SORT,
     PALACE_REL_SORT,
     OPDS2Version1Serializer,
+    OPDS2Version2Serializer,
 )
 from palace.manager.feed.types import (
     Acquisition,
@@ -232,12 +233,12 @@ def test_serialize_opds_message(self):
             OPDSMessage("URN", 200, "Description")
         ) == dict(urn="URN", description="Description")
 
-    def test_serialize_sort_links(self):
+    def test_serialize_sort_links_v2(self):
         feed_data = FeedData()
         link = Link(href="test", rel="test_rel", title="text1")
         link.add_attributes(dict(facetGroup="Sort by", activeFacet="true"))
-        feed_data.facet_links.append(link)
-        links = OPDS2Version1Serializer()._serialize_feed_links(feed=feed_data)
+        feed_data.sort_links.append(link)
+        links = OPDS2Version2Serializer()._serialize_feed_links(feed=feed_data)
 
         assert links == {
             "links": [
@@ -250,3 +251,28 @@ def test_serialize_sort_links(self):
             ],
             "facets": [],
         }
+
+    def test_serialize_sort_links_v1(self):
+        feed_data = FeedData()
+        link = Link(href="test", rel="test_rel", title="text1")
+        link.add_attributes(dict(facetGroup="Sort by", activeFacet="true"))
+        feed_data.facet_links.append(link)
+        links = OPDS2Version1Serializer()._serialize_feed_links(feed=feed_data)
+
+        assert links == {
+            "facets": [
+                {
+                    "metadata": {
+                        "title": "Sort by",
+                    },
+                    "links": [
+                        {
+                            "href": "test",
+                            "rel": "test_rel",
+                            "title": "text1",
+                        }
+                    ],
+                }
+            ],
+            "links": [],
+        }

tests/manager/feed/test_opds_serializer.py

@@ -3,7 +3,10 @@
 import pytz
 from lxml import etree
 
-from palace.manager.feed.serializer.opds import OPDS1Version1Serializer, is_sort_link
+from palace.manager.feed.serializer.opds import (
+    OPDS1Version1Serializer,
+    OPDS1Version2Serializer,
+)
 from palace.manager.feed.serializer.opds2 import PALACE_REL_SORT
 from palace.manager.feed.types import (
     Acquisition,
@@ -260,8 +263,7 @@ def test_serialize_opds_message(self):
     def test_serialize_sort_link(self):
         link = Link(href="test", rel="test_rel", title="text1")
         link.add_attributes(dict(facetGroup="Sort by", activeFacet="true"))
-        serializer = OPDS1Version1Serializer()
-        assert is_sort_link(link)
+        serializer = OPDS1Version2Serializer()
         sort_link = serializer._serialize_sort_link(link)
         assert sort_link.attrib["title"] == "text1"
         assert sort_link.attrib["href"] == "test"