Refactor QPDF_Array

Move all array-specific methods to new class qpdf::Array.

Refactor QPDF_Array
Move all array-specific methods to new class qpdf::Array.
m-holger
1 parent a03c6863
Showing 7 changed files with 203 additions and 108 deletions
include/qpdf/ObjectHandle.hh
include/qpdf/QPDFObjectHandle.hh
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDF_Array.cc
libqpdf/qpdf/QPDFObjectHandle_private.hh
libqpdf/qpdf/QPDF_Array.hh
libtests/sparse_array.cc
@@ -26,6 +26,7 @@
 #include <qpdf/Types.h>
 #include <cstdint>
+#include <memory>
 class QPDF_Dictionary;
 class QPDFObject;
@@ -33,6 +34,7 @@ class QPDFObjectHandle;
 namespace qpdf
 {
+    class Array;
     class Dictionary;
     class BaseDictionary;
@@ -1355,6 +1355,7 @@ class QPDFObjectHandle final: public qpdf::BaseHandle
     void writeJSON(int json_version, JSON::Writer& p, bool dereference_indirect = false) const;
+    inline qpdf::Array as_array(qpdf::typed options = qpdf::typed::any) const;
     inline qpdf::Dictionary as_dictionary(qpdf::typed options = qpdf::typed::any) const;
   private:
@@ -777,22 +777,20 @@ QPDFObjectHandle::aitems()
 int
 QPDFObjectHandle::getArrayNItems() const
 {
-    if (auto array = asArray()) {
-        return array->size();
-    } else {
-        typeWarning("array", "treating as empty");
-        QTC::TC("qpdf", "QPDFObjectHandle array treating as empty");
-        return 0;
+    if (auto array = as_array(strict)) {
+        return array.size();
     }
+    typeWarning("array", "treating as empty");
+    QTC::TC("qpdf", "QPDFObjectHandle array treating as empty");
+    return 0;
 }
 QPDFObjectHandle
 QPDFObjectHandle::getArrayItem(int n) const
 {
-    if (auto array = asArray()) {
-        auto result = array->at(n);
-        if (result.first) {
-            return result.second;
+    if (auto array = as_array(strict)) {
+        if (auto const [success, oh] = array.at(n); success) {
+            return oh;
         } else {
             objectWarning("returning null for out of bounds array access");
             QTC::TC("qpdf", "QPDFObjectHandle array bounds");
@@ -808,13 +806,13 @@ QPDFObjectHandle::getArrayItem(int n) const
 bool
 QPDFObjectHandle::isRectangle() const
 {
-    if (auto array = asArray()) {
+    if (auto array = as_array(strict)) {
         for (int i = 0; i < 4; ++i) {
-            if (auto item = array->at(i).second; !item.isNumber()) {
+            if (auto item = array.at(i).second; !item.isNumber()) {
                 return false;
             }
         }
-        return array->size() == 4;
+        return array.size() == 4;
     }
     return false;
 }
@@ -822,13 +820,13 @@ QPDFObjectHandle::isRectangle() const
 bool
 QPDFObjectHandle::isMatrix() const
 {
-    if (auto array = asArray()) {
+    if (auto array = as_array(strict)) {
         for (int i = 0; i < 6; ++i) {
-            if (auto item = array->at(i).second; !item.isNumber()) {
+            if (auto item = array.at(i).second; !item.isNumber()) {
                 return false;
             }
         }
-        return array->size() == 6;
+        return array.size() == 6;
     }
     return false;
 }
@@ -836,13 +834,13 @@ QPDFObjectHandle::isMatrix() const
 QPDFObjectHandle::Rectangle
 QPDFObjectHandle::getArrayAsRectangle() const
 {
-    if (auto array = asArray()) {
-        if (array->size() != 4) {
+    if (auto array = as_array(strict)) {
+        if (array.size() != 4) {
             return {};
         }
         double items[4];
         for (int i = 0; i < 4; ++i) {
-            if (auto item = array->at(i).second; !item.getValueAsNumber(items[i])) {
+            if (auto item = array.at(i).second; !item.getValueAsNumber(items[i])) {
                 return {};
             }
         }
@@ -858,13 +856,13 @@ QPDFObjectHandle::getArrayAsRectangle() const
 QPDFObjectHandle::Matrix
 QPDFObjectHandle::getArrayAsMatrix() const
 {
-    if (auto array = asArray()) {
-        if (array->size() != 6) {
+    if (auto array = as_array(strict)) {
+        if (array.size() != 6) {
             return {};
         }
         double items[6];
         for (int i = 0; i < 6; ++i) {
-            if (auto item = array->at(i).second; !item.getValueAsNumber(items[i])) {
+            if (auto item = array.at(i).second; !item.getValueAsNumber(items[i])) {
                 return {};
             }
         }
@@ -876,13 +874,11 @@ QPDFObjectHandle::getArrayAsMatrix() const
 std::vector<QPDFObjectHandle>
 QPDFObjectHandle::getArrayAsVector() const
 {
-    auto array = asArray();
-    if (array) {
-        return array->getAsVector();
-    } else {
-        typeWarning("array", "treating as empty");
-        QTC::TC("qpdf", "QPDFObjectHandle array treating as empty vector");
+    if (auto array = as_array(strict)) {
+        return array.getAsVector();
     }
+    typeWarning("array", "treating as empty");
+    QTC::TC("qpdf", "QPDFObjectHandle array treating as empty vector");
     return {};
 }
@@ -891,8 +887,8 @@ QPDFObjectHandle::getArrayAsVector() const
 void
 QPDFObjectHandle::setArrayItem(int n, QPDFObjectHandle const& item)
 {
-    if (auto array = asArray()) {
-        if (!array->setAt(n, item)) {
+    if (auto array = as_array(strict)) {
+        if (!array.setAt(n, item)) {
             objectWarning("ignoring attempt to set out of bounds array item");
             QTC::TC("qpdf", "QPDFObjectHandle set array bounds");
         }
@@ -904,8 +900,8 @@ QPDFObjectHandle::setArrayItem(int n, QPDFObjectHandle const&amp; item)
 void
 QPDFObjectHandle::setArrayFromVector(std::vector<QPDFObjectHandle> const& items)
 {
-    if (auto array = asArray()) {
-        array->setFromVector(items);
+    if (auto array = as_array(strict)) {
+        array.setFromVector(items);
     } else {
         typeWarning("array", "ignoring attempt to replace items");
         QTC::TC("qpdf", "QPDFObjectHandle array ignoring replace items");
@@ -915,8 +911,8 @@ QPDFObjectHandle::setArrayFromVector(std::vector&lt;QPDFObjectHandle&gt; const&amp; items)
 void
 QPDFObjectHandle::insertItem(int at, QPDFObjectHandle const& item)
 {
-    if (auto array = asArray()) {
-        if (!array->insert(at, item)) {
+    if (auto array = as_array(strict)) {
+        if (!array.insert(at, item)) {
             objectWarning("ignoring attempt to insert out of bounds array item");
             QTC::TC("qpdf", "QPDFObjectHandle insert array bounds");
         }
@@ -936,8 +932,8 @@ QPDFObjectHandle::insertItemAndGetNew(int at, QPDFObjectHandle const&amp; item)
 void
 QPDFObjectHandle::appendItem(QPDFObjectHandle const& item)
 {
-    if (auto array = asArray()) {
-        array->push_back(item);
+    if (auto array = as_array(strict)) {
+        array.push_back(item);
     } else {
         typeWarning("array", "ignoring attempt to append item");
         QTC::TC("qpdf", "QPDFObjectHandle array ignoring append item");
@@ -954,8 +950,8 @@ QPDFObjectHandle::appendItemAndGetNew(QPDFObjectHandle const&amp; item)
 void
 QPDFObjectHandle::eraseItem(int at)
 {
-    if (auto array = asArray()) {
-        if (!array->erase(at)) {
+    if (auto array = as_array(strict)) {
+        if (!array.erase(at)) {
             objectWarning("ignoring attempt to erase out of bounds array item");
             QTC::TC("qpdf", "QPDFObjectHandle erase array bounds");
         }
@@ -968,8 +964,8 @@ QPDFObjectHandle::eraseItem(int at)
 QPDFObjectHandle
 QPDFObjectHandle::eraseItemAndGetOld(int at)
 {
-    auto array = asArray();
-    auto result = (array && at < array->size() && at >= 0) ? array->at(at).second : newNull();
+    auto array = as_array(strict);
+    auto result = (array && at < array.size() && at >= 0) ? array.at(at).second : newNull();
     eraseItem(at);
     return result;
 }
@@ -1344,12 +1340,12 @@ QPDFObjectHandle::arrayOrStreamToStreamArray(
 {
     all_description = description;
     std::vector<QPDFObjectHandle> result;
-    if (auto array = asArray()) {
-        int n_items = array->size();
+    if (auto array = as_array(strict)) {
+        int n_items = array.size();
         for (int i = 0; i < n_items; ++i) {
-            QPDFObjectHandle item = array->at(i).second;
+            QPDFObjectHandle item = array.at(i).second;
             if (item.isStream()) {
-                result.push_back(item);
+                result.emplace_back(item);
             } else {
                 QTC::TC("qpdf", "QPDFObjectHandle non-stream in stream array");
                 warn(
@@ -1363,7 +1359,7 @@ QPDFObjectHandle::arrayOrStreamToStreamArray(
             }
         }
     } else if (isStream()) {
-        result.push_back(*this);
+        result.emplace_back(*this);
     } else if (!isNull()) {
         warn(
             getOwningQPDF(),
@@ -1993,10 +1989,10 @@ QPDFObjectHandle::makeDirect(QPDFObjGen::set&amp; visited, bool stop_at_streams)
         this->obj = obj->copy(true);
     } else if (isArray()) {
         std::vector<QPDFObjectHandle> items;
-        auto array = asArray();
-        int n = array->size();
+        auto array = as_array(strict);
+        int n = array.size();
         for (int i = 0; i < n; ++i) {
-            items.push_back(array->at(i).second);
+            items.emplace_back(array.at(i).second);
             items.back().makeDirect(visited, stop_at_streams);
         }
         this->obj = QPDF_Array::create(items);
 #include <qpdf/QPDF_Array.hh>
 #include <qpdf/JSON_writer.hh>
-#include <qpdf/QPDFObjectHandle.hh>
+#include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QPDFObject_private.hh>
 #include <qpdf/QTC.hh>
+using namespace qpdf;
+
 static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull();
 inline void
 QPDF_Array::checkOwnership(QPDFObjectHandle const& item) const
 {
-    if (auto obj = item.getObjectPtr()) {
-        if (qpdf) {
-            if (auto item_qpdf = obj->getQPDF()) {
-                if (qpdf != item_qpdf) {
+    // This is only called from QPDF_Array::setFromVector, which in turn is only called from create.
+    // At his point qpdf is a nullptr and therefore the ownership check reduces to an uninitialized
+    // check
+    if (!item.getObjectPtr()) {
+        throw std::logic_error("Attempting to add an uninitialized object to a QPDF_Array.");
+    }
+}
+
+inline void
+Array::checkOwnership(QPDFObjectHandle const& item) const
+{
+    if (auto o = item.getObjectPtr()) {
+        if (auto pdf = obj->getQPDF()) {
+            if (auto item_qpdf = o->getQPDF()) {
+                if (pdf != item_qpdf) {
                     throw std::logic_error(
                         "Attempting to add an object from a different QPDF. Use "
                         "QPDF::copyForeignObject to add objects from another file.");
@@ -184,47 +197,75 @@ QPDF_Array::writeJSON(int json_version, JSON::Writer&amp; p)
     p.writeEnd(']');
 }
+QPDF_Array*
+Array::array() const
+{
+    if (obj) {
+        if (auto a = obj->as<QPDF_Array>()) {
+            return a;
+        }
+    }
+    throw std::runtime_error("Expected an array but found a non-array object");
+    return nullptr; // unreachable
+}
+
+QPDFObjectHandle
+Array::null() const
+{
+    return null_oh;
+}
+
+int
+Array::size() const
+{
+    auto a = array();
+    return a->sp ? a->sp->size : int(a->elements.size());
+}
+
 std::pair<bool, QPDFObjectHandle>
-QPDF_Array::at(int n) const noexcept
+Array::at(int n) const
 {
+    auto a = array();
     if (n < 0 || n >= size()) {
         return {false, {}};
-    } else if (sp) {
-        auto const& iter = sp->elements.find(n);
-        return {true, iter == sp->elements.end() ? null_oh : (*iter).second};
-    } else {
-        return {true, elements[size_t(n)]};
     }
+    if (!a->sp) {
+        return {true, a->elements[size_t(n)]};
+    }
+    auto const& iter = a->sp->elements.find(n);
+    return {true, iter == a->sp->elements.end() ? null() : iter->second};
 }
 std::vector<QPDFObjectHandle>
-QPDF_Array::getAsVector() const
+Array::getAsVector() const
 {
-    if (sp) {
+    auto a = array();
+    if (a->sp) {
         std::vector<QPDFObjectHandle> v;
         v.reserve(size_t(size()));
-        for (auto const& item: sp->elements) {
+        for (auto const& item: a->sp->elements) {
             v.resize(size_t(item.first), null_oh);
             v.emplace_back(item.second);
         }
         v.resize(size_t(size()), null_oh);
         return v;
     } else {
-        return {elements.cbegin(), elements.cend()};
+        return {a->elements.cbegin(), a->elements.cend()};
     }
 }
 bool
-QPDF_Array::setAt(int at, QPDFObjectHandle const& oh)
+Array::setAt(int at, QPDFObjectHandle const& oh)
 {
     if (at < 0 || at >= size()) {
         return false;
     }
+    auto a = array();
     checkOwnership(oh);
-    if (sp) {
-        sp->elements[at] = oh.getObj();
+    if (a->sp) {
+        a->sp->elements[at] = oh.getObj();
     } else {
-        elements[size_t(at)] = oh.getObj();
+        a->elements[size_t(at)] = oh.getObj();
     }
     return true;
 }
@@ -240,9 +281,22 @@ QPDF_Array::setFromVector(std::vector&lt;QPDFObjectHandle&gt; const&amp; v)
     }
 }
+void
+Array::setFromVector(std::vector<QPDFObjectHandle> const& v)
+{
+    auto a = array();
+    a->elements.resize(0);
+    a->elements.reserve(v.size());
+    for (auto const& item: v) {
+        checkOwnership(item);
+        a->elements.push_back(item.getObj());
+    }
+}
+
 bool
-QPDF_Array::insert(int at, QPDFObjectHandle const& item)
+Array::insert(int at, QPDFObjectHandle const& item)
 {
+    auto a = array();
     int sz = size();
     if (at < 0 || at > sz) {
         // As special case, also allow insert beyond the end
@@ -251,61 +305,63 @@ QPDF_Array::insert(int at, QPDFObjectHandle const&amp; item)
         push_back(item);
     } else {
         checkOwnership(item);
-        if (sp) {
-            auto iter = sp->elements.crbegin();
-            while (iter != sp->elements.crend()) {
+        if (a->sp) {
+            auto iter = a->sp->elements.crbegin();
+            while (iter != a->sp->elements.crend()) {
                 auto key = (iter++)->first;
                 if (key >= at) {
-                    auto nh = sp->elements.extract(key);
+                    auto nh = a->sp->elements.extract(key);
                     ++nh.key();
-                    sp->elements.insert(std::move(nh));
+                    a->sp->elements.insert(std::move(nh));
                 } else {
                     break;
                 }
             }
-            sp->elements[at] = item.getObj();
-            ++sp->size;
+            a->sp->elements[at] = item.getObj();
+            ++a->sp->size;
         } else {
-            elements.insert(elements.cbegin() + at, item.getObj());
+            a->elements.insert(a->elements.cbegin() + at, item.getObj());
         }
     }
     return true;
 }
 void
-QPDF_Array::push_back(QPDFObjectHandle const& item)
+Array::push_back(QPDFObjectHandle const& item)
 {
+    auto a = array();
     checkOwnership(item);
-    if (sp) {
-        sp->elements[(sp->size)++] = item.getObj();
+    if (a->sp) {
+        a->sp->elements[(a->sp->size)++] = item.getObj();
     } else {
-        elements.push_back(item.getObj());
+        a->elements.push_back(item.getObj());
     }
 }
 bool
-QPDF_Array::erase(int at)
+Array::erase(int at)
 {
+    auto a = array();
     if (at < 0 || at >= size()) {
         return false;
     }
-    if (sp) {
-        auto end = sp->elements.end();
-        if (auto iter = sp->elements.lower_bound(at); iter != end) {
+    if (a->sp) {
+        auto end = a->sp->elements.end();
+        if (auto iter = a->sp->elements.lower_bound(at); iter != end) {
             if (iter->first == at) {
                 iter++;
-                sp->elements.erase(at);
+                a->sp->elements.erase(at);
             }
             while (iter != end) {
-                auto nh = sp->elements.extract(iter++);
+                auto nh = a->sp->elements.extract(iter++);
                 --nh.key();
-                sp->elements.insert(std::move(nh));
+                a->sp->elements.insert(std::move(nh));
             }
         }
-        --(sp->size);
+        --(a->sp->size);
     } else {
-        elements.erase(elements.cbegin() + at);
+        a->elements.erase(a->elements.cbegin() + at);
     }
     return true;
 }
@@ -4,10 +4,40 @@
 #include <qpdf/QPDFObjectHandle.hh>
 #include <qpdf/QPDFObject_private.hh>
+#include <qpdf/QPDF_Array.hh>
 #include <qpdf/QPDF_Dictionary.hh>
 namespace qpdf
 {
+    class Array final: public BaseHandle
+    {
+      public:
+        explicit Array(std::shared_ptr<QPDFObject> const& obj) :
+            BaseHandle(obj)
+        {
+        }
+
+        explicit Array(std::shared_ptr<QPDFObject>&& obj) :
+            BaseHandle(std::move(obj))
+        {
+        }
+
+        int size() const;
+        std::pair<bool, QPDFObjectHandle> at(int n) const;
+        bool setAt(int at, QPDFObjectHandle const& oh);
+        bool insert(int at, QPDFObjectHandle const& item);
+        void push_back(QPDFObjectHandle const& item);
+        bool erase(int at);
+
+        std::vector<QPDFObjectHandle> getAsVector() const;
+        void setFromVector(std::vector<QPDFObjectHandle> const& items);
+
+      private:
+        QPDF_Array* array() const;
+        void checkOwnership(QPDFObjectHandle const& item) const;
+        QPDFObjectHandle null() const;
+    };
+
     // BaseDictionary is only used as a base class. It does not contain any methods exposed in the
     // public API.
     class BaseDictionary: public BaseHandle
@@ -58,6 +88,19 @@ namespace qpdf
 } // namespace qpdf
+inline qpdf::Array
+QPDFObjectHandle::as_array(qpdf::typed options) const
+{
+    if (options & qpdf::error) {
+        assertType("array", false);
+    }
+    if (options & qpdf::any_flag || type_code() == ::ot_array ||
+        (options & qpdf::optional && type_code() == ::ot_null)) {
+        return qpdf::Array(obj);
+    }
+    return qpdf::Array({});
+}
+
 inline qpdf::Dictionary
 QPDFObjectHandle::as_dictionary(qpdf::typed options) const
 {
 #ifndef QPDF_ARRAY_HH
 #define QPDF_ARRAY_HH
+#include <qpdf/QPDFObjectHandle.hh>
+#include <qpdf/QPDFObject_private.hh>
 #include <qpdf/QPDFValue.hh>
 #include <map>
@@ -25,25 +27,20 @@ class QPDF_Array: public QPDFValue
     void writeJSON(int json_version, JSON::Writer& p) override;
     void disconnect() override;
-    int
-    size() const noexcept
-    {
-        return sp ? sp->size : int(elements.size());
-    }
-    std::pair<bool, QPDFObjectHandle> at(int n) const noexcept;
-    bool setAt(int n, QPDFObjectHandle const& oh);
-    std::vector<QPDFObjectHandle> getAsVector() const;
-    void setFromVector(std::vector<QPDFObjectHandle> const& items);
-    bool insert(int at, QPDFObjectHandle const& item);
-    void push_back(QPDFObjectHandle const& item);
-    bool erase(int at);
-
   private:
+    friend class qpdf::Array;
     QPDF_Array();
     QPDF_Array(QPDF_Array const&);
     QPDF_Array(std::vector<QPDFObjectHandle> const& items);
     QPDF_Array(std::vector<std::shared_ptr<QPDFObject>>&& items, bool sparse);
+    int
+    size() const
+    {
+        return sp ? sp->size : int(elements.size());
+    }
+    void setFromVector(std::vector<QPDFObjectHandle> const& items);
+
     void checkOwnership(QPDFObjectHandle const& item) const;
     std::unique_ptr<Sparse> sp;
 #include <qpdf/assert_test.h>
 #include <qpdf/QPDF.hh>
-#include <qpdf/QPDFObjectHandle.hh>
+#include <qpdf/QPDFObjectHandle_private.hh>
 #include <qpdf/QPDFObject_private.hh>
 #include <qpdf/QPDF_Array.hh>
@@ -11,7 +11,7 @@ int
 main()
 {
     auto obj = QPDF_Array::create({}, true);
-    QPDF_Array& a = *obj->as<QPDF_Array>();
+    auto a = qpdf::Array(obj);
     assert(a.size() == 0);
@@ -89,15 +89,15 @@ main()
     pdf.emptyPDF();
     obj = QPDF_Array::create({10, "null"_qpdf.getObj()}, true);
-    QPDF_Array& b = *obj->as<QPDF_Array>();
+    auto b = qpdf::Array(obj);
     b.setAt(5, pdf.newIndirectNull());
     b.setAt(7, "[0 1 2 3]"_qpdf);
     assert(b.at(3).second.isNull());
     assert(b.at(8).second.isNull());
     assert(b.at(5).second.isIndirect());
-    assert(b.unparse() == "[ null null null null null 3 0 R null [ 0 1 2 3 ] null null ]");
-    auto c = b.copy(true);
-    auto d = b.copy(false);
+    assert(obj->unparse() == "[ null null null null null 3 0 R null [ 0 1 2 3 ] null null ]");
+    auto c = obj->copy(true);
+    auto d = obj->copy(false);
     b.at(7).second.setArrayItem(2, "42"_qpdf);
     assert(c->unparse() == "[ null null null null null 3 0 R null [ 0 1 42 3 ] null null ]");
     assert(d->unparse() == "[ null null null null null 3 0 R null [ 0 1 2 3 ] null null ]");