From 8a3cdfd2af4a95d8daede45bcb36eecdcdc8f964 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Fri, 9 Sep 2022 15:40:02 -0400 Subject: [PATCH] Change QPDFObjectHandle == to isSameObjectAs Replace operator== and operator!=, which were testing for the same underlying object, with isSameObjectAs. This change was motivated by the fact that pikepdf internally had its own operator== method for QPDFObjectHandle that did structural comparison. I backed out qpdf's operator== as a courtesy to pikepdf (in my own testing) but also because I think people might naturally assume that operator== does a structural comparison, and isSameObjectAs is clearer in its intent. --- ChangeLog | 7 ++- include/qpdf/QPDFObjectHandle.hh | 13 +++--- libqpdf/QPDFObjectHandle.cc | 8 +--- manual/release-notes.rst | 12 +++--- qpdf/test_driver.cc | 74 ++++++++++++++++---------------- 5 files changed, 53 insertions(+), 61 deletions(-) diff --git a/ChangeLog b/ChangeLog index db589d34..2ffd53cb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2022-09-09 Jay Berkenbilt + * Add QPDFObjectHandle::isSameObjectAs to test whether two + QPDFObjectHandle objects point to the same underlying object. + * Expose ability to create custom loggers and to get and set the logger for QPDF and QPDFJob through the C API. @@ -29,10 +32,6 @@ * Add new methods getArtBox and getBleedBox to QPDFPageObjectHelper, completing the set of bounding box methods. - * Add == equality for QPDFObjectHandle. Two QPDFObjectHandle - objects are equal if they point to the same underlying object, - meaning changes to one will be reflected in the other. - * The --show-encryption option now works even if a correct password is not supplied. If you were using --show-encryption to test whether you have the right password, use --requires-password diff --git a/include/qpdf/QPDFObjectHandle.hh b/include/qpdf/QPDFObjectHandle.hh index 21f82e58..66fd4a5a 100644 --- a/include/qpdf/QPDFObjectHandle.hh +++ b/include/qpdf/QPDFObjectHandle.hh @@ -334,14 +334,13 @@ class QPDFObjectHandle QPDF_DLL inline bool isInitialized() const; - // Two QPDFObjectHandle objects are equal if they point to exactly - // the same underlying object, meaning that changes to one are - // reflected in the other, or "if you paint one, the other one - // changes color." + // This method returns true if the QPDFObjectHandle objects point + // to exactly the same underlying object, meaning that changes to + // one are reflected in the other, or "if you paint one, the other + // one changes color." This does not perform a structural + // comparison of the contents of the objects. QPDF_DLL - bool operator==(QPDFObjectHandle const&) const; - QPDF_DLL - bool operator!=(QPDFObjectHandle const&) const; + bool isSameObjectAs(QPDFObjectHandle const&) const; // Return type code and type name of underlying object. These are // useful for doing rapid type tests (like switch statements) or diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 6225dba9..c52ac2fc 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -237,17 +237,11 @@ LastChar::getLastChar() } bool -QPDFObjectHandle::operator==(QPDFObjectHandle const& rhs) const +QPDFObjectHandle::isSameObjectAs(QPDFObjectHandle const& rhs) const { return this->obj == rhs.obj; } -bool -QPDFObjectHandle::operator!=(QPDFObjectHandle const& rhs) const -{ - return this->obj != rhs.obj; -} - void QPDFObjectHandle::disconnect() { diff --git a/manual/release-notes.rst b/manual/release-notes.rst index bb9bdb7f..694b28e0 100644 --- a/manual/release-notes.rst +++ b/manual/release-notes.rst @@ -231,12 +231,12 @@ For a detailed list of changes, please see the file still valid, but it's also possible to have direct objects that don't have an owning ``QPDF``. - - It is now possible to test ``QPDFObjectHandle`` equality with - ``==`` and ``!=``. Two ``QPDFObjectHandle`` objects are equal if - they point to the same underlying object, meaning changes to one - will be reflected in the other. Note that this method does not - compare the contents of the objects, so two distinct but - structurally identical objects will not be considered equal. + - Add method ``QPDFObjectHandle::isSameObjectAs`` for testing + whether two ``QPDFObjectHandle`` objects point to the same + underlying object, meaning changes to one will be reflected in + the other. Note that this method does not compare the contents + of the objects, so two distinct but structurally identical + objects will not be considered the same object. - New factory method ``QPDF::create()`` returns a ``std::shared_ptr``. diff --git a/qpdf/test_driver.cc b/qpdf/test_driver.cc index 3e74c106..e174aa28 100644 --- a/qpdf/test_driver.cc +++ b/qpdf/test_driver.cc @@ -3340,22 +3340,22 @@ test_93(QPDF& pdf, char const* arg2) auto trailer = pdf.getTrailer(); auto root1 = trailer.getKey("/Root"); auto root2 = pdf.getRoot(); - assert(root1 == root2); + assert(root1.isSameObjectAs(root2)); auto oh1 = "<< /One /Two >>"_qpdf; auto oh2 = oh1; - assert(oh1 == oh2); + assert(oh1.isSameObjectAs(oh2)); auto oh3 = "<< /One /Two >>"_qpdf; - assert(oh1 != oh3); + assert(!oh1.isSameObjectAs(oh3)); oh2.replaceKey("/One", "/Three"_qpdf); - assert(oh1 == oh2); + assert(oh1.isSameObjectAs(oh2)); assert(oh2.unparse() == "<< /One /Three >>"); assert(!oh1.isIndirect()); auto oh4 = pdf.makeIndirectObject(oh1); - assert(oh1 == oh4); + assert(oh1.isSameObjectAs(oh4)); assert(oh1.isIndirect()); assert(oh4.isIndirect()); trailer.replaceKey("/Potato", oh1); - assert(trailer.getKey("/Potato") == oh2); + assert(trailer.getKey("/Potato").isSameObjectAs(oh2)); } static void @@ -3385,76 +3385,76 @@ test_94(QPDF& pdf, char const* arg2) assert(p1.getObjectHandle().getKey("/MediaBox").isNull()); // MediaBox not present, so get inherited one - assert(p1.getMediaBox(false) == root_media); + assert(p1.getMediaBox(false).isSameObjectAs(root_media)); // Other boxesBox not present, so fall back to MediaBox - assert(p1.getCropBox(false, false) == root_media); - assert(p1.getBleedBox(false, false) == root_media); - assert(p1.getTrimBox(false, false) == root_media); - assert(p1.getArtBox(false, false) == root_media); + assert(p1.getCropBox(false, false).isSameObjectAs(root_media)); + assert(p1.getBleedBox(false, false).isSameObjectAs(root_media)); + assert(p1.getTrimBox(false, false).isSameObjectAs(root_media)); + assert(p1.getArtBox(false, false).isSameObjectAs(root_media)); // Make copy of artbox auto p1_new_art = p1.getArtBox(false, true); assert(p1_new_art.unparse() == root_media_unparse); - assert(p1_new_art != root_media); + assert(!p1_new_art.isSameObjectAs(root_media)); // This also copied cropbox auto p1_new_crop = p1.getCropBox(false, false); - assert(p1_new_crop != root_media); - assert(p1_new_crop != p1_new_art); + assert(!p1_new_crop.isSameObjectAs(root_media)); + assert(!p1_new_crop.isSameObjectAs(p1_new_art)); assert(p1_new_crop.unparse() == root_media_unparse); // But it didn't copy Media - assert(p1.getMediaBox(false) == root_media); + assert(p1.getMediaBox(false).isSameObjectAs(root_media)); // Now fall back to new crop - assert(p1.getTrimBox(false, false) == p1_new_crop); + assert(p1.getTrimBox(false, false).isSameObjectAs(p1_new_crop)); // Request copy. The value returned has the same structure but is // a different object. auto p1_effective_media = p1.getMediaBox(true); assert(p1_effective_media.unparse() == root_media_unparse); - assert(p1_effective_media != root_media); + assert(!p1_effective_media.isSameObjectAs(root_media)); // copy_on_fallback didn't have to copy media to crop - assert(p2.getMediaBox(false) == root_media); + assert(p2.getMediaBox(false).isSameObjectAs(root_media)); auto p2_crop = p2.getCropBox(false, false); auto p2_new_trim = p2.getTrimBox(false, true); assert(p2_new_trim.unparse() == p2_crop.unparse()); - assert(p2_new_trim != p2_crop); - assert(p2.getMediaBox(false) == root_media); + assert(!p2_new_trim.isSameObjectAs(p2_crop)); + assert(p2.getMediaBox(false).isSameObjectAs(root_media)); // We didn't need to copy anything auto p3_media = p3.getMediaBox(false); auto p3_crop = p3.getCropBox(false, false); - assert(p3.getMediaBox(true) == p3_media); - assert(p3.getCropBox(true, true) == p3_crop); + assert(p3.getMediaBox(true).isSameObjectAs(p3_media)); + assert(p3.getCropBox(true, true).isSameObjectAs(p3_crop)); // We didn't have to copy for bleed but we did for art auto p4_orig_crop = p4.getObjectHandle().getKey("/CropBox"); auto p4_crop = p4.getCropBox(false, false); - assert(p4_orig_crop == p4_crop); + assert(p4_orig_crop.isSameObjectAs(p4_crop)); auto p4_bleed1 = p4.getBleedBox(false, false); auto p4_bleed2 = p4.getBleedBox(false, true); - assert(p4_bleed1 != p4_crop); - assert(p4_bleed1 == p4_bleed2); + assert(!p4_bleed1.isSameObjectAs(p4_crop)); + assert(p4_bleed1.isSameObjectAs(p4_bleed2)); auto p4_art1 = p4.getArtBox(false, false); - assert(p4_art1 == p4_crop); + assert(p4_art1.isSameObjectAs(p4_crop)); auto p4_art2 = p4.getArtBox(false, true); - assert(p4_art2 != p4_crop); + assert(!p4_art2.isSameObjectAs(p4_crop)); auto p4_new_crop = p4.getCropBox(true, false); - assert(p4_new_crop != p4_orig_crop); + assert(!p4_new_crop.isSameObjectAs(p4_orig_crop)); assert(p4_orig_crop.isIndirect()); assert(!p4_new_crop.isIndirect()); assert(p4_new_crop.unparse() == p4_orig_crop.unparseResolved()); // Exercise copying for inheritance and fallback - assert(p5.getMediaBox(false) == root_media); - assert(p5.getCropBox(false, false) == root_media); - assert(p5.getBleedBox(false, false) == root_media); + assert(p5.getMediaBox(false).isSameObjectAs(root_media)); + assert(p5.getCropBox(false, false).isSameObjectAs(root_media)); + assert(p5.getBleedBox(false, false).isSameObjectAs(root_media)); auto p5_new_bleed = p5.getBleedBox(true, true); auto p5_new_media = p5.getMediaBox(false); auto p5_new_crop = p5.getCropBox(false, false); - assert(p5_new_media != root_media); - assert(p5_new_crop != root_media); - assert(p5_new_crop != p5_new_media); - assert(p5_new_bleed != root_media); - assert(p5_new_bleed != p5_new_media); - assert(p5_new_bleed != p5_new_crop); + assert(!p5_new_media.isSameObjectAs(root_media)); + assert(!p5_new_crop.isSameObjectAs(root_media)); + assert(!p5_new_crop.isSameObjectAs(p5_new_media)); + assert(!p5_new_bleed.isSameObjectAs(root_media)); + assert(!p5_new_bleed.isSameObjectAs(p5_new_media)); + assert(!p5_new_bleed.isSameObjectAs(p5_new_crop)); assert(p5_new_media.unparse() == root_media_unparse); assert(p5_new_crop.unparse() == root_media_unparse); assert(p5_new_bleed.unparse() == root_media_unparse);