Changes to comply with review comments

Author: larsga

pypdf/_writer.py

@@ -95,22 +95,21 @@
     DecodedStreamObject,
     Destination,
     DictionaryObject,
-    DirectRefLink,
     Fit,
     FloatObject,
     IndirectObject,
-    NamedRefLink,
     NameObject,
     NullObject,
     NumberObject,
     PdfObject,
     RectangleObject,
-    RefLink,
+    ReferenceLink,
     StreamObject,
     TextStringObject,
     TreeObject,
     ViewerPreferences,
     create_string_object,
+    extract_links,
     hex_to_rgb,
     is_null_or_none,
 )
@@ -212,9 +211,9 @@ def __init__(
         """The PDF file identifier,
         defined by the ID in the PDF file's trailer dictionary."""
 
-        self._unresolved_links: list[tuple[RefLink,RefLink]] = []
+        self._unresolved_links: list[tuple[ReferenceLink, ReferenceLink]] = []
         "Tracks links in pages added to the writer for resolving later."
-        self._merged_in_pages: Dict[Optional[IndirectObject],Optional[IndirectObject]] = {}
+        self._merged_in_pages: Dict[Optional[IndirectObject], Optional[IndirectObject]] = {}
         "Tracks pages added to the writer and what page they turned into."
 
         if self.incremental:
@@ -491,39 +490,6 @@ def _add_page(
         except Exception:
             pass
 
-        def _extract_links(new_page: PageObject, old_page: PageObject) -> List[Tuple[RefLink,RefLink]]:
-            new_links = [_build_link(link, new_page) for link in new_page.get("/Annots", [])]
-            old_links = [_build_link(link, old_page) for link in old_page.get("/Annots", [])]
-
-            return [(new_link, old_link) for (new_link, old_link)
-                    in zip(new_links, old_links)
-                    if new_link and old_link]
-
-        def _build_link(indir_obj: IndirectObject, page: PageObject) -> Optional[RefLink]:
-            src = cast(PdfReader, page.pdf)
-            link = cast(DictionaryObject, indir_obj.get_object())
-            if link.get("/Subtype") != "/Link":
-                return None
-
-            if "/A" in link:
-                action = cast(DictionaryObject, link["/A"])
-                if action.get("/S") != "/GoTo":
-                    return None
-
-                return _create_link(action["/D"], src)
-
-            if "/Dest" in link:
-                return _create_link(link["/Dest"], src)
-
-            return None # nothing we need to do
-
-        def _create_link(ref: PdfObject, src: PdfReader)-> Optional[RefLink]:
-            if isinstance(ref, TextStringObject):
-                return NamedRefLink(ref, src)
-            if isinstance(ref, ArrayObject):
-                return DirectRefLink(ref)
-            return None
-
         page = cast(
             "PageObject", page_org.clone(self, False, excluded_keys).get_object()
         )
@@ -554,7 +520,7 @@ def _create_link(ref: PdfObject, src: PdfReader)-> Optional[RefLink]:
             # pages may or may not already be added.  we store the
             # information we need, so that we can resolve the references
             # later.
-            self._unresolved_links.extend(_extract_links(page, page_org))
+            self._unresolved_links.extend(extract_links(page, page_org))
             self._merged_in_pages[page_org.indirect_reference] = page.indirect_reference
 
         return page

pypdf/generic/__init__.py

@@ -62,7 +62,7 @@
 )
 from ._files import EmbeddedFile
 from ._fit import Fit
-from ._link import DirectRefLink, NamedRefLink, RefLink
+from ._link import ReferenceLink, extract_links
 from ._outline import OutlineItem
 from ._rectangle import RectangleObject
 from ._utils import (
@@ -209,22 +209,20 @@ def link(
     "DecodedStreamObject",
     "Destination",
     "DictionaryObject",
-    "DirectRefLink",
     "EmbeddedFile",
     "EncodedStreamObject",
     "Field",
     "Fit",
     "FloatObject",
     "IndirectObject",
     "NameObject",
-    "NamedRefLink",
     "NullObject",
     "NumberObject",
     "OutlineFontFlag",
     "OutlineItem",
     "PdfObject",
     "RectangleObject",
-    "RefLink",
+    "ReferenceLink",
     "StreamObject",
     "TextStringObject",
     "TreeObject",
@@ -233,6 +231,7 @@ def link(
     "create_string_object",
     "decode_pdfdocencoding",
     "encode_pdfdocencoding",
+    "extract_links",
     "hex_to_rgb",
     "is_null_or_none",
     "read_hex_string_from_stream",

pypdf/generic/_link.py

@@ -25,50 +25,90 @@
 # POSSIBILITY OF SUCH DAMAGE.
 
 
-# This module contains classes used by _writer.py to track links in
-# pages being added to the writer until the links can be resolved.
+# This module contains code used by _writer.py to track links in pages
+# being added to the writer until the links can be resolved.
 
-from typing import TYPE_CHECKING, Union
+from typing import TYPE_CHECKING, List, Optional, Tuple, Union, cast
 
-from . import ArrayObject, IndirectObject, TextStringObject
+from . import ArrayObject, DictionaryObject, IndirectObject, PdfObject, TextStringObject
 
 if TYPE_CHECKING:
+    from .._page import PageObject
     from .._reader import PdfReader
     from .._writer import PdfWriter
 
 
-class NamedRefLink:
+class NamedReferenceLink:
     """Named reference link being preserved until we can resolve it correctly."""
 
-    def __init__(self, ref: TextStringObject, source_pdf: "PdfReader") -> None:
-        """ref: TextStringObject with named reference"""
-        self._ref = ref
+    def __init__(self, reference: TextStringObject, source_pdf: "PdfReader") -> None:
+        """reference: TextStringObject with named reference"""
+        self._reference = reference
         self._source_pdf = source_pdf
 
-    def find_referenced_page(self) -> Union[IndirectObject,None]:
-        dest = self._source_pdf.named_destinations.get(str(self._ref))
+    def find_referenced_page(self) -> Union[IndirectObject, None]:
+        dest = self._source_pdf.named_destinations.get(str(self._reference))
         return dest.page if dest else None
 
     def patch_reference(self, target_pdf: "PdfWriter", new_page: IndirectObject) -> None:
         """target_pdf: PdfWriter which the new link went into"""
         # point named destination in new PDF to the new page
-        if str(self._ref) not in target_pdf.named_destinations:
-            target_pdf.add_named_destination(str(self._ref), new_page.page_number)
+        if str(self._reference) not in target_pdf.named_destinations:
+            target_pdf.add_named_destination(str(self._reference), new_page.page_number)
 
 
-class DirectRefLink:
+class DirectReferenceLink:
     """Direct reference link being preserved until we can resolve it correctly."""
 
-    def __init__(self, ref: ArrayObject) -> None:
-        """ref: an ArrayObject whose first element is the Page indir obj"""
-        self._ref = ref
+    def __init__(self, reference: ArrayObject) -> None:
+        """reference: an ArrayObject whose first element is the Page indir obj"""
+        self._reference = reference
 
     def find_referenced_page(self) -> IndirectObject:
-        return self._ref[0]
+        return self._reference[0]
 
     def patch_reference(self, target_pdf: "PdfWriter", new_page: IndirectObject) -> None:
         """target_pdf: PdfWriter which the new link went into"""
-        self._ref[0] = new_page
+        self._reference[0] = new_page
 
 
-RefLink = Union[NamedRefLink,DirectRefLink]
+ReferenceLink = Union[NamedReferenceLink, DirectReferenceLink]
+
+
+def extract_links(new_page: "PageObject", old_page: "PageObject") -> List[Tuple[ReferenceLink, ReferenceLink]]:
+    """Extracts links from two pages on the assumption that the two pages are
+    the same. Produces one list of (new link, old link) tuples.
+    """
+    new_links = [_build_link(link, new_page) for link in new_page.get("/Annots", [])]
+    old_links = [_build_link(link, old_page) for link in old_page.get("/Annots", [])]
+
+    return [(new_link, old_link) for (new_link, old_link)
+            in zip(new_links, old_links)
+            if new_link and old_link]
+
+
+def _build_link(indir_obj: IndirectObject, page: "PageObject") -> Optional[ReferenceLink]:
+    src = cast("PdfReader", page.pdf)
+    link = cast(DictionaryObject, indir_obj.get_object())
+    if link.get("/Subtype") != "/Link":
+        return None
+
+    if "/A" in link:
+        action = cast(DictionaryObject, link["/A"])
+        if action.get("/S") != "/GoTo":
+            return None
+
+        return _create_link(action["/D"], src)
+
+    if "/Dest" in link:
+        return _create_link(link["/Dest"], src)
+
+    return None # nothing we need to do
+
+
+def _create_link(ref: PdfObject, src: "PdfReader")-> Optional[ReferenceLink]:
+    if isinstance(ref, TextStringObject):
+        return NamedReferenceLink(ref, src)
+    if isinstance(ref, ArrayObject):
+        return DirectReferenceLink(ref)
+    return None

tests/test_merger.py

@@ -501,7 +501,7 @@ def test_named_reference_preserved(pdf_file_path):
 
 
 @pytest.mark.enable_socket
-def test_named_ref_to_page_thats_gone(pdf_file_path):
+def test_named_ref_to_page_that_is_gone(pdf_file_path):
     source = PdfReader(BytesIO(get_data_from_url(name="named-reference.pdf")))
     buf = BytesIO()
     tmp = PdfWriter()