Refactor `ObjCopier` into `Objects::Foreign::Copier`: improve encapsulation, str…

…eamline foreign object handling, and simplify method structure.

Refactor `ObjCopier` into `Objects::Foreign::Copier`: improve encapsulation, str…
…eamline foreign object handling, and simplify method structure.
m-holger
1 parent b20ff968
Showing 3 changed files with 69 additions and 62 deletions
include/qpdf/QPDF.hh
libqpdf/QPDF.cc
libqpdf/qpdf/QPDF_private.hh
@@ -742,7 +742,6 @@ class QPDF
     static std::string const qpdf_version;
     class ObjCache;
-    class ObjCopier;
     class EncryptionParameters;
     class ForeignStreamData;
     class CopiedStreamDataProvider;
@@ -167,8 +167,7 @@ QPDF::Members::Members(QPDF&amp; qpdf) :
     pages(doc.pages()),
     log(QPDFLogger::defaultLogger()),
     file(std::make_shared<InvalidInputSource>()),
-    encp(std::make_shared<EncryptionParameters>()),
-    obj_copier(qpdf)
+    encp(std::make_shared<EncryptionParameters>())
 {
 }
@@ -474,11 +473,11 @@ QPDF::getObjectByID(int objid, int generation)
 QPDFObjectHandle
 QPDF::copyForeignObject(QPDFObjectHandle foreign)
 {
-    return m->obj_copier.copied(foreign);
+    return m->objects.foreign().copied(foreign);
 }
-QPDF::ObjCopier::Copier&
-QPDF::ObjCopier::copier(QPDFObjectHandle const& foreign)
+Objects ::Foreign::Copier&
+Objects::Foreign::copier(QPDFObjectHandle const& foreign)
 {
     if (!foreign.isIndirect()) {
         throw std::logic_error("QPDF::copyForeign called with direct object handle");
@@ -491,7 +490,7 @@ QPDF::ObjCopier::copier(QPDFObjectHandle const&amp; foreign)
 }
 QPDFObjectHandle
-QPDF::ObjCopier::Copier::copied(QPDFObjectHandle const& foreign)
+Objects::Foreign::Copier::copied(QPDFObjectHandle const& foreign)
 {
     // Here's an explanation of what's going on here.
     //
@@ -501,7 +500,7 @@ QPDF::ObjCopier::Copier::copied(QPDFObjectHandle const&amp; foreign)
     // references to the corresponding object in the local file.
     //
     // To do this, we maintain mappings from foreign object IDs to local object IDs for each foreign
-    // QPDF that we are copying from. The mapping is stored in an ObjCopier, which contains a
+    // QPDF that we are copying from. The mapping is stored in an Foreign::Copier, which contains a
     // mapping from the foreign ObjGen to the local QPDFObjectHandle.
     //
     // To copy, we do a deep traversal of the foreign object with loop detection to discover all
@@ -561,7 +560,7 @@ QPDF::ObjCopier::Copier::copied(QPDFObjectHandle const&amp; foreign)
 }
 void
-QPDF::ObjCopier::Copier::reserve_objects(QPDFObjectHandle const& foreign, bool top)
+Objects::Foreign::Copier::reserve_objects(QPDFObjectHandle const& foreign, bool top)
 {
     auto foreign_tc = foreign.type_code();
     util::assertion(
@@ -609,7 +608,7 @@ QPDF::ObjCopier::Copier::reserve_objects(QPDFObjectHandle const&amp; foreign, bool t
 }
 QPDFObjectHandle
-QPDF::ObjCopier::Copier::replace_indirect_object(QPDFObjectHandle const& foreign, bool top)
+Objects::Foreign::Copier::replace_indirect_object(QPDFObjectHandle const& foreign, bool top)
 {
     auto foreign_tc = foreign.type_code();
@@ -44,55 +44,6 @@ class QPDF::ObjCache
     qpdf_offset_t end_after_space{0};
 };
-class QPDF::ObjCopier
-{
-    class Copier
-    {
-      public:
-        Copier(QPDF& qpdf) :
-            qpdf(qpdf)
-        {
-        }
-
-        QPDFObjectHandle copied(QPDFObjectHandle const& foreign);
-
-      private:
-        QPDFObjectHandle replace_indirect_object(QPDFObjectHandle const& foreign, bool top = false);
-        void reserve_objects(QPDFObjectHandle const& foreign, bool top = false);
-
-        QPDF& qpdf;
-        std::map<QPDFObjGen, QPDFObjectHandle> object_map;
-        std::vector<QPDFObjectHandle> to_copy;
-        QPDFObjGen::set visiting;
-    };
-
-  public:
-    ObjCopier(QPDF& qpdf) :
-        qpdf(qpdf)
-    {
-    }
-
-    ObjCopier() = delete;
-    ObjCopier(ObjCopier const&) = delete;
-    ObjCopier(ObjCopier&&) = delete;
-    ObjCopier& operator=(ObjCopier const&) = delete;
-    ObjCopier& operator=(ObjCopier&&) = delete;
-    ~ObjCopier() = default;
-
-    // Return a local handle to the foreign object. Copy the foreign object if necessary.
-    QPDFObjectHandle
-    copied(QPDFObjectHandle const& foreign)
-    {
-        return copier(foreign).copied(foreign);
-    }
-
-  private:
-    Copier& copier(QPDFObjectHandle const& foreign);
-
-    QPDF& qpdf;
-    std::map<unsigned long long, Copier> copiers;
-};
-
 class QPDF::EncryptionParameters
 {
     friend class QPDF;
@@ -619,6 +570,57 @@ class QPDF::Doc
     class Objects
     {
       public:
+        class Foreign
+        {
+            class Copier
+            {
+              public:
+                Copier(QPDF& qpdf) :
+                    qpdf(qpdf)
+                {
+                }
+
+                QPDFObjectHandle copied(QPDFObjectHandle const& foreign);
+
+              private:
+                QPDFObjectHandle
+                replace_indirect_object(QPDFObjectHandle const& foreign, bool top = false);
+                void reserve_objects(QPDFObjectHandle const& foreign, bool top = false);
+
+                QPDF& qpdf;
+                std::map<QPDFObjGen, QPDFObjectHandle> object_map;
+                std::vector<QPDFObjectHandle> to_copy;
+                QPDFObjGen::set visiting;
+            };
+
+          public:
+            Foreign(QPDF& qpdf) :
+                qpdf(qpdf)
+            {
+            }
+
+            Foreign() = delete;
+            Foreign(Foreign const&) = delete;
+            Foreign(Foreign&&) = delete;
+            Foreign& operator=(Foreign const&) = delete;
+            Foreign& operator=(Foreign&&) = delete;
+            ~Foreign() = default;
+
+            // Return a local handle to the foreign object. Copy the foreign object if necessary.
+            QPDFObjectHandle
+            copied(QPDFObjectHandle const& foreign)
+            {
+                return copier(foreign).copied(foreign);
+            }
+
+          private:
+            Copier& copier(QPDFObjectHandle const& foreign);
+
+            QPDF& qpdf;
+            std::map<unsigned long long, Copier> copiers;
+        }; // class QPDF::Doc::Objects::Foreign
+
+      public:
         Objects() = delete;
         Objects(Objects const&) = delete;
         Objects(Objects&&) = delete;
@@ -628,8 +630,15 @@ class QPDF::Doc
         Objects(QPDF& qpdf, QPDF::Members* m) :
             qpdf(qpdf),
-            m(m)
+            m(m),
+            foreign_(qpdf)
+        {
+        }
+
+        Foreign&
+        foreign()
         {
+            return foreign_;
         }
         void parse(char const* password);
@@ -704,9 +713,10 @@ class QPDF::Doc
         bool isUnresolved(QPDFObjGen og);
         void setLastObjectDescription(std::string const& description, QPDFObjGen og);
-      private:
         QPDF& qpdf;
         QPDF::Members* m;
+
+        Foreign foreign_;
     }; // class QPDF::Doc::Objects
     // This class is used to represent a PDF Pages tree.
@@ -888,7 +898,6 @@ class QPDF::Members
     bool ever_pushed_inherited_attributes_to_pages{false};
     bool ever_called_get_all_pages{false};
     std::vector<QPDFExc> warnings;
-    QPDF::ObjCopier obj_copier;
     std::shared_ptr<CopiedStreamDataProvider> copied_stream_data_provider;
     bool reconstructed_xref{false};
     bool in_read_xref_stream{false};