Add new QPDFObjectHandle methods for more fluent programming

Jay Berkenbilt
1 parent ff73d71e
Showing 8 changed files with 193 additions and 31 deletions
ChangeLog
TODO
include/qpdf/QPDFObjectHandle.hh
libqpdf/QPDFObjectHandle.cc
manual/release-notes.rst
qpdf/qtest/qpdf.test
qpdf/qtest/qpdf/test88.out
qpdf/test_driver.cc
+2022-04-29  Jay Berkenbilt  <ejb@ql.org>
+
+        * QPDFObjectHandle: for the methods insertItem, appendItem,
+        eraseItem, replaceKey, and removeKey, have the methods return a
+        reference to the original object, making a fluent interface to
+        initializing or modifying QPDFObjectHandle possible. Also, for
+        each one, add a corresponding "AndGet" method (insertItemAndGet,
+        appendItemAndGet, eraseItemAndGet, replaceKeyAndGet, and
+        removeKeyAndGet) that returns the newly inserted, replaced, or
+        removed item. This makes it possible to create a new object, add
+        it to an array or dictionary, and get a handle to it all in one
+        line.
+
 2022-04-24  Jay Berkenbilt  <ejb@ql.org>
         * Bug fix: "removeAttachment" in the job JSON now takes an array
@@ -474,13 +474,6 @@ This is a list of changes to make next time there is an ABI change.
 Comments appear in the code prefixed by "ABI". Always Search for ABI
 in source and header files to find items not listed here.
-* Having QPDFObjectHandle setters return Class& to allow for
-  use of fluent interfaces. This includes array and dictionary
-  mutators.
-  newDictionary().replaceKey("/X", "1"_qpdf).replaceKey("/Y", "(asdf)"_qpdf);
-* Add replaceKeyAndGet, appendItemAndGet, setArrayItemAndGet,
-  insertItemAndGet that return the new item so you can say
-  auto oh = dict.replaceKeyAndGet("/Key", QPDFObjectHandle::newSomething());
 * Add getKeyOrInsert("/X", oh) that returns the existing value or adds
   oh as the new value and returns it.
 * Add default values to the getters, like getIntValue(default_value).
@@ -990,7 +990,20 @@ class QPDFObjectHandle
     QPDF_DLL
     QPDFObjectHandle copyStream();
-    // Mutator methods.  Use with caution.
+    // Mutator methods.
+
+    // Since qpdf 11: when a mutator object returns QPDFObjectHandle&,
+    // it is a reference to the object itself. This makes it possible
+    // to use a fluent style. For example:
+    //
+    //   array.appendItem(i1).appendItem(i2);
+    //
+    // would append i1 and then i2 to the array. There are also items
+    // that end with AndGet and return a QPDFObjectHandle. These
+    // return the newly added object. For example:
+    //
+    //   auto new_dict = dict.replaceKeyAndGet(
+    //       "/New", QPDFObjectHandle::newDictionary());
     // Recursively copy this object, making it direct. An exception is
     // thrown if a loop is detected. With allow_streams true, keep
@@ -1010,31 +1023,54 @@ class QPDFObjectHandle
     QPDF_DLL
     void setArrayFromVector(std::vector<QPDFObjectHandle> const& items);
     // Insert an item before the item at the given position ("at") so
-    // that it has that position after insertion.  If "at" is equal to
-    // the size of the array, insert the item at the end.
+    // that it has that position after insertion. If "at" is equal to
+    // the size of the array, insert the item at the end. Return a
+    // reference to the array (not the new item).
+    QPDF_DLL
+    QPDFObjectHandle& insertItem(int at, QPDFObjectHandle const& item);
+    // Like insertItem but return the item that was inserted.
     QPDF_DLL
-    void insertItem(int at, QPDFObjectHandle const& item);
+    QPDFObjectHandle insertItemAndGet(int at, QPDFObjectHandle const& item);
+    // Append an item, and return a reference to the original array
+    // (not the new item).
     QPDF_DLL
-    void appendItem(QPDFObjectHandle const& item);
+    QPDFObjectHandle& appendItem(QPDFObjectHandle const& item);
+    // Append an item, and return the newly added item.
+    QPDF_DLL
+    QPDFObjectHandle appendItemAndGet(QPDFObjectHandle const& item);
     // Remove the item at that position, reducing the size of the
-    // array by one.
+    // array by one. Return a reference the original array (not the
+    // item that was removed).
+    QPDF_DLL
+    QPDFObjectHandle& eraseItem(int at);
+    // Erase and item and return the item that was removed.
     QPDF_DLL
-    void eraseItem(int at);
+    QPDFObjectHandle eraseItemAndGet(int at);
     // Mutator methods for dictionary objects
     // Replace value of key, adding it if it does not exist. If value
-    // is null, remove the key.
+    // is null, remove the key. Return a reference to the original
+    // dictionary (not the new item).
+    QPDF_DLL
+    QPDFObjectHandle&
+    replaceKey(std::string const& key, QPDFObjectHandle const& value);
+    // Replace value of key and return the value.
     QPDF_DLL
-    void replaceKey(std::string const& key, QPDFObjectHandle const& value);
-    // Remove key, doing nothing if key does not exist
+    QPDFObjectHandle
+    replaceKeyAndGet(std::string const& key, QPDFObjectHandle const& value);
+    // Remove key, doing nothing if key does not exist. Return the
+    // original dictionary (not the removed item).
     QPDF_DLL
-    void removeKey(std::string const& key);
+    QPDFObjectHandle& removeKey(std::string const& key);
+    // Remove key and return the old value. If the old value didn't
+    // exist, return a null object.
+    QPDF_DLL
+    QPDFObjectHandle removeKeyAndGet(std::string const& key);
     // ABI: Remove in qpdf 12
-    [[deprecated("use replaceKey -- it does the same thing")]]
-    QPDF_DLL
-    void replaceOrRemoveKey(std::string const& key, QPDFObjectHandle const&);
+    [[deprecated("use replaceKey -- it does the same thing")]] QPDF_DLL void
+    replaceOrRemoveKey(std::string const& key, QPDFObjectHandle const&);
     // Methods for stream objects
     QPDF_DLL
@@ -959,7 +959,7 @@ QPDFObjectHandle::setArrayFromVector(std::vector&lt;QPDFObjectHandle&gt; const&amp; items)
     }
 }
-void
+QPDFObjectHandle&
 QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const& item)
 {
     if (isArray()) {
@@ -968,9 +968,17 @@ QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const&amp; item)
         typeWarning("array", "ignoring attempt to insert item");
         QTC::TC("qpdf", "QPDFObjectHandle array ignoring insert item");
     }
+    return *this;
 }
-void
+QPDFObjectHandle
+QPDFObjectHandle::insertItemAndGet(int at, QPDFObjectHandle const& item)
+{
+    insertItem(at, item);
+    return item;
+}
+
+QPDFObjectHandle&
 QPDFObjectHandle::appendItem(QPDFObjectHandle const& item)
 {
     if (isArray()) {
@@ -980,9 +988,17 @@ QPDFObjectHandle::appendItem(QPDFObjectHandle const&amp; item)
         typeWarning("array", "ignoring attempt to append item");
         QTC::TC("qpdf", "QPDFObjectHandle array ignoring append item");
     }
+    return *this;
 }
-void
+QPDFObjectHandle
+QPDFObjectHandle::appendItemAndGet(QPDFObjectHandle const& item)
+{
+    appendItem(item);
+    return item;
+}
+
+QPDFObjectHandle&
 QPDFObjectHandle::eraseItem(int at)
 {
     if (isArray() && (at < getArrayNItems()) && (at >= 0)) {
@@ -996,6 +1012,18 @@ QPDFObjectHandle::eraseItem(int at)
             QTC::TC("qpdf", "QPDFObjectHandle array ignoring erase item");
         }
     }
+    return *this;
+}
+
+QPDFObjectHandle
+QPDFObjectHandle::eraseItemAndGet(int at)
+{
+    auto result = QPDFObjectHandle::newNull();
+    if (isArray() && (at < getArrayNItems()) && (at >= 0)) {
+        result = getArrayItem(at);
+    }
+    eraseItem(at);
+    return result;
 }
 // Dictionary accessors
@@ -1267,7 +1295,7 @@ QPDFObjectHandle::getOwningQPDF()
 // Dictionary mutators
-void
+QPDFObjectHandle&
 QPDFObjectHandle::replaceKey(
     std::string const& key, QPDFObjectHandle const& value)
 {
@@ -1278,9 +1306,18 @@ QPDFObjectHandle::replaceKey(
         typeWarning("dictionary", "ignoring key replacement request");
         QTC::TC("qpdf", "QPDFObjectHandle dictionary ignoring replaceKey");
     }
+    return *this;
 }
-void
+QPDFObjectHandle
+QPDFObjectHandle::replaceKeyAndGet(
+    std::string const& key, QPDFObjectHandle const& value)
+{
+    replaceKey(key, value);
+    return value;
+}
+
+QPDFObjectHandle&
 QPDFObjectHandle::removeKey(std::string const& key)
 {
     if (isDictionary()) {
@@ -1289,6 +1326,18 @@ QPDFObjectHandle::removeKey(std::string const&amp; key)
         typeWarning("dictionary", "ignoring key removal request");
         QTC::TC("qpdf", "QPDFObjectHandle dictionary ignoring removeKey");
     }
+    return *this;
+}
+
+QPDFObjectHandle
+QPDFObjectHandle::removeKeyAndGet(std::string const& key)
+{
+    auto result = QPDFObjectHandle::newNull();
+    if (isDictionary()) {
+        result = getKey(key);
+    }
+    removeKey(key);
+    return result;
 }
 void
@@ -73,6 +73,15 @@ For a detailed list of changes, please see the file
       ``QPDFNumberTreeObjectHelper`` constructors that don't take a
       ``QPDF&`` argument.
+  - Library Enhancements
+
+    - Support for more fluent programming with ``QPDFObjectHandle``.
+      The methods ``insertItem``, ``appendItem``, ``eraseItem``,
+      ``replaceKey``, and ``removeKey`` all return a reference to the
+      object being modified. New methods ``insertItemAndGet``,
+      ``appendItemAndGet``, ``eraseItemAndGet``, ``replaceKeyAndGet``,
+      and ``removeKeyAndGet`` return the newly added or removed object.
+
   - Other changes
     - A new chapter on contributing to qpdf has been added to the
@@ -1440,13 +1440,16 @@ foreach (my $i = 1; $i &lt;= 3; ++$i)
 show_ntests();
 # ----------
-$td->notify("--- Dictionary keys ---");
-$n_tests += 1;
+$td->notify("--- Miscellaneous QPDFObjectHandle API ---");
+$n_tests += 2;
 $td->runtest("dictionary keys",
              {$td->COMMAND => "test_driver 87 - -"},
-             {$td->STRING => "test 87 done\n",
-                  $td->EXIT_STATUS => 0},
+             {$td->STRING => "test 87 done\n", $td->EXIT_STATUS => 0},
+             $td->NORMALIZE_NEWLINES);
+$td->runtest("fluent interfaces",
+             {$td->COMMAND => "test_driver 88 minimal.pdf -"},
+             {$td->FILE => "test88.out", $td->EXIT_STATUS => 0},
              $td->NORMALIZE_NEWLINES);
 show_ntests();
+WARNING: test array: ignoring attempt to erase out of bounds array item
+test 88 done
@@ -3178,6 +3178,63 @@ test_87(QPDF&amp; pdf, char const* arg2)
     assert(dict.getJSON().unparse() == "{\n  \"/A\": 2\n}");
 }
+static void
+test_88(QPDF& pdf, char const* arg2)
+{
+    // Exercise fluent QPDFObjectHandle mutators and similar methods
+    // added for qpdf 11.
+    auto dict = QPDFObjectHandle::newDictionary()
+                    .replaceKey("/One", QPDFObjectHandle::newInteger(1))
+                    .replaceKey("/Two", QPDFObjectHandle::newInteger(2));
+    dict.replaceKeyAndGet("/Three", QPDFObjectHandle::newArray())
+        .appendItem("(a)"_qpdf)
+        .appendItem("(b)"_qpdf)
+        .appendItemAndGet(QPDFObjectHandle::newDictionary())
+        .replaceKey("/Z", "/Y"_qpdf)
+        .replaceKey("/X", "/W"_qpdf);
+    assert(dict.unparse() == R"(
+      <<
+        /One 1
+        /Two 2
+        /Three [ (a) (b) << /Z /Y /X /W >> ]
+      >>
+    )"_qpdf.unparse());
+    auto arr = dict.getKey("/Three")
+                   .insertItem(0, QPDFObjectHandle::newString("0"))
+                   .insertItem(0, QPDFObjectHandle::newString("00"));
+    assert(
+        arr.unparse() ==
+        "[ (00) (0) (a) (b) << /Z /Y /X /W >> ]"_qpdf.unparse());
+    auto new_dict = arr.insertItemAndGet(1, "<< /P /Q /R /S >>"_qpdf);
+    arr.eraseItem(2).eraseItem(0);
+    assert(
+        arr.unparse() ==
+        "[ << /P /Q /R /S >> (a) (b) << /Z /Y /X /W >> ]"_qpdf.unparse());
+
+    // new_dict shares internals with the one in the array. It has
+    // always been this way, and there is code that relies on this
+    // behavior. Maybe it would be different if I could start over
+    // again...
+    new_dict.removeKey("/R").replaceKey("/T", "/U"_qpdf);
+    assert(
+        arr.unparse() ==
+        "[ << /P /Q /T /U >> (a) (b) << /Z /Y /X /W >> ]"_qpdf.unparse());
+    auto s = arr.eraseItemAndGet(1);
+    assert(s.unparse() == "(a)");
+    assert(
+        arr.unparse() ==
+        "[ << /P /Q /T /U >> (b) << /Z /Y /X /W >> ]"_qpdf.unparse());
+
+    assert(new_dict.removeKeyAndGet("/M").isNull());
+    assert(new_dict.removeKeyAndGet("/P").unparse() == "/Q");
+    assert(new_dict.unparse() == "<< /T /U >>"_qpdf.unparse());
+
+    // Test errors
+    auto arr2 = pdf.getRoot().replaceKeyAndGet("/QTest", "[1 2]"_qpdf);
+    arr2.setObjectDescription(&pdf, "test array");
+    assert(arr2.eraseItemAndGet(50).isNull());
+}
+
 void
 runtest(int n, char const* filename1, char const* arg2)
 {
@@ -3280,7 +3337,7 @@ runtest(int n, char const* filename1, char const* arg2)
         {76, test_76}, {77, test_77}, {78, test_78}, {79, test_79},
         {80, test_80}, {81, test_81}, {82, test_82}, {83, test_83},
         {84, test_84}, {85, test_85}, {86, test_86}, {87, test_87},
-    };
+        {88, test_88}};
     auto fn = test_functions.find(n);
     if (fn == test_functions.end()) {