Refactor: replace `BaseDictionary::replaceKey` method with `BaseHandle::replace`…

… for clarity and consistency across dictionary operations.

Refactor: replace `BaseDictionary::replaceKey` method with `BaseHandle::replace`…
… for clarity and consistency across dictionary operations.
m-holger
1 parent c290a82f
Showing 9 changed files with 40 additions and 30 deletions
include/qpdf/ObjectHandle.hh
libqpdf/NNTree.cc
libqpdf/QPDF.cc
libqpdf/QPDFAcroFormDocumentHelper.cc
libqpdf/QPDFEFStreamObjectHelper.cc
libqpdf/QPDFFormFieldObjectHelper.cc
libqpdf/QPDF_Dictionary.cc
libqpdf/qpdf/QPDFObjectHandle_private.hh
qpdf/qpdf.testcov
@@ -85,6 +85,7 @@ namespace qpdf
         bool contains(std::string const& key) const;
         size_t erase(std::string const& key);
+        bool replace(std::string const& key, QPDFObjectHandle value);
         QPDFObjectHandle const& operator[](std::string const& key) const;
         std::shared_ptr<QPDFObject> copy(bool shallow = false) const;
@@ -168,7 +168,7 @@ NNTreeIterator::resetLimits(Dictionary a_node, std::list&lt;PathElement&gt;::iterator 
                 }
             }
             if (a_node != path.begin()->node) {
-                a_node.replaceKey("/Limits", Array({first, last}));
+                a_node.replace("/Limits", Array({first, last}));
             }
         }
@@ -266,7 +266,7 @@ NNTreeIterator::split(Dictionary to_split, std::list&lt;PathElement&gt;::iterator pare
         new_kids.push_back(first_node);
         to_split.erase("/Limits"); // already shouldn't be there for root
         to_split.erase(impl.itemsKey());
-        to_split.replaceKey("/Kids", new_kids);
+        to_split.replace("/Kids", new_kids);
         if (is_leaf) {
             node = first_node;
         } else {
@@ -446,7 +446,7 @@ NNTreeIterator::remove()
         if (parent == path.end()) {
             // We erased the very last item. Convert the root to an empty items array.
             element->node.erase("/Kids");
-            element->node.replaceKey(impl.itemsKey(), Array::empty());
+            element->node.replace(impl.itemsKey(), Array::empty());
             path.clear();
             setItemNumber(impl.tree_root, -1);
             return;
@@ -673,8 +673,8 @@ NNTreeImpl::repair()
     for (auto const& [key, value]: items) {
         repl.insert(key, value);
     }
-    tree_root.replaceKey("/Kids", new_node["/Kids"]);
-    tree_root.replaceKey(itemsKey(), new_node[itemsKey()]);
+    tree_root.replace("/Kids", new_node["/Kids"]);
+    tree_root.replace(itemsKey(), new_node[itemsKey()]);
 }
 NNTreeImpl::iterator
@@ -572,7 +572,7 @@ Objects::Foreign::Copier::replace_indirect_object(QPDFObjectHandle const&amp; foreig
         auto result = Dictionary::empty();
         for (auto const& [key, value]: Dictionary(foreign)) {
             if (!value.null()) {
-                result.replaceKey(key, replace_indirect_object(value));
+                result.replace(key, replace_indirect_object(value));
             }
         }
         return result;
@@ -584,7 +584,7 @@ Objects::Foreign::Copier::replace_indirect_object(QPDFObjectHandle const&amp; foreig
         auto dict = result.getDict();
         for (auto const& [key, value]: stream.getDict()) {
             if (!value.null()) {
-                dict.replaceKey(key, replace_indirect_object(value));
+                dict.replace(key, replace_indirect_object(value));
             }
         }
         stream.copy_data_to(result);
@@ -658,7 +658,7 @@ QPDF::getRoot()
     // approach to more extensive checks and warning levels.
     if (m->cf.check_mode() && Name(Root["/Type"]) != "/Catalog") {
         warn(m->c.damagedPDF("", -1, "catalog /Type entry missing or invalid"));
-        Root.replaceKey("/Type", Name("/Catalog"));
+        Root.replace("/Type", Name("/Catalog"));
     }
     return Root.oh();
 }
@@ -799,7 +799,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations(
             }
             QPDFObjectHandle(dr).makeResourcesIndirect(qpdf);
             if (!dr.indirect()) {
-                acroform.replaceKey("/DR", qpdf.makeIndirectObject(dr));
+                acroform.replace("/DR", qpdf.makeIndirectObject(dr));
                 dr = acroform["/DR"];
             }
             // Merge the other document's /DR, creating a conflict map. mergeResources checks to
@@ -839,7 +839,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations(
                 if (parent.indirect()) {
                     auto parent_og = parent.id_gen();
                     if (orig_to_copy.contains(parent_og)) {
-                        obj.replaceKey("/Parent", orig_to_copy[parent_og]);
+                        obj.replace("/Parent", orig_to_copy[parent_og]);
                     } else {
                         parent.warn(
                             "while traversing field " + obj.id_gen().unparse(',') +
@@ -869,7 +869,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations(
                     // chrome, firefox, the mac Preview application, and several of the free
                     // readers on Linux all ignore /DR at the field level.
                     if (obj.contains("/DR")) {
-                        obj.replaceKey("/DR", dr);
+                        obj.replace("/DR", dr);
                     }
                     if (obj["/DA"].isString() && !dr_map.empty()) {
                         adjustDefaultAppearances(obj, dr_map);
@@ -988,7 +988,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations(
         Dictionary apdict = ah.getAppearanceDictionary();
         std::vector<QPDFObjectHandle> streams;
         auto replace_stream = [](auto& dict, auto& key, auto& old) {
-            dict.replaceKey(key, old.copyStream());
+            dict.replace(key, old.copyStream());
             return dict[key];
         };
@@ -1015,7 +1015,7 @@ QPDFAcroFormDocumentHelper::transformAnnotations(
             }
             apcm.concat(cm);
             if (omatrix || apcm != QPDFMatrix()) {
-                dict.replaceKey("/Matrix", QPDFObjectHandle::newFromMatrix(apcm));
+                dict.replace("/Matrix", QPDFObjectHandle::newFromMatrix(apcm));
             }
             Dictionary resources = dict["/Resources"];
             if (!dr_map.empty() && resources) {
@@ -31,7 +31,7 @@ void
 QPDFEFStreamObjectHelper::setParam(std::string const& pkey, QPDFObjectHandle const& pval)
 {
     if (Dictionary Params = oh().getDict()["/Params"]) {
-        Params.replaceKey(pkey, pval);
+        Params.replace(pkey, pval);
         return;
     }
     oh().getDict().replaceKey("/Params", Dictionary({{pkey, pval}}));
@@ -956,7 +956,7 @@ FormField::generateTextAppearance(QPDFAnnotationObjectHelper&amp; aoh)
             aoh.getObjectHandle().replaceKey("/AP", Dictionary::empty());
             AP = aoh.getAppearanceDictionary();
         }
-        AP.replaceKey("/N", AS);
+        AP.replace("/N", AS);
     }
     if (!AS.isStream()) {
         aoh.warn("unable to get normal appearance stream for update");
@@ -63,18 +63,29 @@ BaseHandle::erase(const std::string&amp; key)
     return 0;
 }
+bool
+BaseHandle::replace(std::string const& key, QPDFObjectHandle value)
+{
+    if (auto d = as<QPDF_Dictionary>()) {
+        if (value.null() && !value.indirect()) {
+            // The PDF spec doesn't distinguish between keys with null values and missing keys.
+            // Allow indirect nulls which are equivalent to a dangling reference, which is permitted
+            // by the spec.
+            d->items.erase(key);
+        } else {
+            // add or replace value
+            d->items[key] = value;
+        }
+        return true;
+    }
+    return false;
+}
+
 void
-BaseDictionary::replaceKey(std::string const& key, QPDFObjectHandle value)
-{
-    auto d = dict();
-    if (value.null() && !value.indirect()) {
-        // The PDF spec doesn't distinguish between keys with null values and missing keys.
-        // Allow indirect nulls which are equivalent to a dangling reference, which is
-        // permitted by the spec.
-        d->items.erase(key);
-    } else {
-        // add or replace value
-        d->items[key] = value;
+BaseDictionary::replace(std::string const& key, QPDFObjectHandle value)
+{
+    if (!BaseHandle::replace(key, value)) {
+        (void)dict();
     }
 }
@@ -166,11 +177,10 @@ QPDFObjectHandle::replaceKey(std::string const&amp; key, QPDFObjectHandle const&amp; val
 {
     if (auto dict = as_dictionary(strict)) {
         checkOwnership(value);
-        dict.replaceKey(key, value);
+        dict.replace(key, value);
         return;
     }
     typeWarning("dictionary", "ignoring key replacement request");
-    QTC::TC("qpdf", "QPDFObjectHandle dictionary ignoring replaceKey");
 }
 QPDFObjectHandle
@@ -127,7 +127,7 @@ namespace qpdf
         // The following methods are not part of the public API.
         std::set<std::string> getKeys();
         std::map<std::string, QPDFObjectHandle> const& getAsMap() const;
-        void replaceKey(std::string const& key, QPDFObjectHandle value);
+        void replace(std::string const& key, QPDFObjectHandle value);
         using iterator = std::map<std::string, QPDFObjectHandle>::iterator;
         using const_iterator = std::map<std::string, QPDFObjectHandle>::const_iterator;
@@ -171,7 +171,6 @@ QPDFObjectHandle array ignoring erase item 0
 QPDFObjectHandle dictionary false for hasKey 0
 QPDFObjectHandle dictionary empty set for getKeys 0
 QPDFObjectHandle dictionary empty map for asMap 0
-QPDFObjectHandle dictionary ignoring replaceKey 0
 QPDFObjectHandle numeric non-numeric 0
 QPDFObjectHandle erase array bounds 0
 qpdf-c called qpdf_check_pdf 0