From e80fad86e95af978ada2a6cc5c4b209a1f65f7c0 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Fri, 29 Apr 2022 20:09:10 -0400 Subject: [PATCH] Add new QPDFObjectHandle methods for more fluent programming --- ChangeLog | 13 +++++++ TODO | 7 ---- include/qpdf/QPDFObjectHandle.hh | 64 +++++++++++++++++++++++++------- libqpdf/QPDFObjectHandle.cc | 59 ++++++++++++++++++++++++++--- manual/release-notes.rst | 9 +++++ qpdf/qtest/qpdf.test | 11 ++++-- qpdf/qtest/qpdf/test88.out | 2 + qpdf/test_driver.cc | 59 ++++++++++++++++++++++++++++- 8 files changed, 193 insertions(+), 31 deletions(-) create mode 100644 qpdf/qtest/qpdf/test88.out diff --git a/ChangeLog b/ChangeLog index 5615db2a..2f06936f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2022-04-29 Jay Berkenbilt + + * 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 * Bug fix: "removeAttachment" in the job JSON now takes an array diff --git a/TODO b/TODO index ffdb8f4b..eeabea11 100644 --- a/TODO +++ b/TODO @@ -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). diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 3beab3f5..77bef52b 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -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 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 - void insertItem(int at, QPDFObjectHandle const& item); + QPDFObjectHandle& insertItem(int at, QPDFObjectHandle const& item); + // Like insertItem but return the item that was inserted. QPDF_DLL - void appendItem(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 + 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 - void eraseItem(int at); + QPDFObjectHandle& eraseItem(int at); + // Erase and item and return the item that was removed. + QPDF_DLL + 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 - void replaceKey(std::string const& key, QPDFObjectHandle const& value); - // Remove key, doing nothing if key does not exist + QPDFObjectHandle& + replaceKey(std::string const& key, QPDFObjectHandle const& value); + // Replace value of key and return the value. QPDF_DLL - void removeKey(std::string const& key); + 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 + 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 diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 297221df..68849f46 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -959,7 +959,7 @@ QPDFObjectHandle::setArrayFromVector(std::vector const& items) } } -void +QPDFObjectHandle& QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const& item) { if (isArray()) { @@ -968,9 +968,17 @@ QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const& 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& 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& 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 diff --git a/manual/release-notes.rst b/manual/release-notes.rst index d6ef1c9b..b5e4e150 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -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 diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index 8878aa31..4dcf7384 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -1440,13 +1440,16 @@ foreach (my $i = 1; $i <= 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(); diff --git a/qpdf/qtest/qpdf/test88.out b/qpdf/qtest/qpdf/test88.out new file mode 100644 index 00000000..327212b6 --- /dev/null +++ b/qpdf/qtest/qpdf/test88.out @@ -0,0 +1,2 @@ +WARNING: test array: ignoring attempt to erase out of bounds array item +test 88 done diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index ead0b484..1268adb9 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -3178,6 +3178,63 @@ test_87(QPDF& 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()) {