Commit fcc7f702305a8d099f3c0b565be9e912491674f7
1 parent
64649e31
Refactor: replace dictionary access via operator[] with `get` method for consistency and clarity.
Showing
6 changed files
with
85 additions
and
22 deletions
libqpdf/QPDFAnnotationObjectHelper.cc
| ... | ... | @@ -33,14 +33,14 @@ QPDFAnnotationObjectHelper::getAppearanceDictionary() |
| 33 | 33 | std::string |
| 34 | 34 | QPDFAnnotationObjectHelper::getAppearanceState() |
| 35 | 35 | { |
| 36 | - Name AS = (*this)["/AS"]; | |
| 36 | + Name AS = get("/AS"); | |
| 37 | 37 | return AS ? AS.value() : ""; |
| 38 | 38 | } |
| 39 | 39 | |
| 40 | 40 | int |
| 41 | 41 | QPDFAnnotationObjectHelper::getFlags() |
| 42 | 42 | { |
| 43 | - Integer flags_obj = (*this)["/F"]; | |
| 43 | + Integer flags_obj = get("/F"); | |
| 44 | 44 | return flags_obj ? flags_obj : 0; |
| 45 | 45 | } |
| 46 | 46 | ... | ... |
libqpdf/QPDFFileSpecObjectHelper.cc
| ... | ... | @@ -42,7 +42,7 @@ std::string |
| 42 | 42 | QPDFFileSpecObjectHelper::getFilename() |
| 43 | 43 | { |
| 44 | 44 | for (auto const& i: name_keys) { |
| 45 | - if (String k = oh()[i]) { | |
| 45 | + if (String k = get(i)) { | |
| 46 | 46 | return k.utf8_value(); |
| 47 | 47 | } |
| 48 | 48 | } |
| ... | ... | @@ -54,7 +54,7 @@ QPDFFileSpecObjectHelper::getFilenames() |
| 54 | 54 | { |
| 55 | 55 | std::map<std::string, std::string> result; |
| 56 | 56 | for (auto const& i: name_keys) { |
| 57 | - if (String k = oh()[i]) { | |
| 57 | + if (String k = get(i)) { | |
| 58 | 58 | result[i] = k.utf8_value(); |
| 59 | 59 | } |
| 60 | 60 | } |
| ... | ... | @@ -64,7 +64,7 @@ QPDFFileSpecObjectHelper::getFilenames() |
| 64 | 64 | QPDFObjectHandle |
| 65 | 65 | QPDFFileSpecObjectHelper::getEmbeddedFileStream(std::string const& key) |
| 66 | 66 | { |
| 67 | - if (Dictionary EF = oh()["/EF"]) { | |
| 67 | + if (Dictionary EF = get("/EF")) { | |
| 68 | 68 | if (!key.empty() && EF.contains(key)) { |
| 69 | 69 | if (auto result = EF[key]) { |
| 70 | 70 | return result; | ... | ... |
libqpdf/QPDFOutlineObjectHelper.cc
| ... | ... | @@ -53,9 +53,9 @@ QPDFOutlineObjectHelper::getKids() |
| 53 | 53 | QPDFObjectHandle |
| 54 | 54 | QPDFOutlineObjectHelper::getDest() |
| 55 | 55 | { |
| 56 | - auto dest = (*this)["/Dest"]; | |
| 56 | + auto dest = get("/Dest"); | |
| 57 | 57 | if (dest.null()) { |
| 58 | - auto const& A = (*this)["/A"]; | |
| 58 | + auto const& A = get("/A"); | |
| 59 | 59 | if (Name(A["/S"]) == "/GoTo") { |
| 60 | 60 | dest = A["/D"]; |
| 61 | 61 | } | ... | ... |
libqpdf/QPDF_Dictionary.cc
| ... | ... | @@ -191,7 +191,7 @@ QPDFObjectHandle::hasKey(std::string const& key) const |
| 191 | 191 | QPDFObjectHandle |
| 192 | 192 | QPDFObjectHandle::getKey(std::string const& key) const |
| 193 | 193 | { |
| 194 | - if (auto result = (*this)[key]) { | |
| 194 | + if (auto result = get(key)) { | |
| 195 | 195 | return result; |
| 196 | 196 | } |
| 197 | 197 | if (isDictionary()) { |
| ... | ... | @@ -269,7 +269,7 @@ QPDFObjectHandle::removeKey(std::string const& key) |
| 269 | 269 | QPDFObjectHandle |
| 270 | 270 | QPDFObjectHandle::removeKeyAndGetOld(std::string const& key) |
| 271 | 271 | { |
| 272 | - auto result = (*this)[key]; | |
| 272 | + auto result = get(key); | |
| 273 | 273 | erase(key); |
| 274 | 274 | return result ? result : newNull(); |
| 275 | 275 | } | ... | ... |
libtests/objects.cc
| ... | ... | @@ -54,35 +54,30 @@ test_0(QPDF& pdf, char const* arg2) |
| 54 | 54 | assert_compare_numbers(INT_MAX, t.getKey("/Q1").getIntValueAsInt()); |
| 55 | 55 | try { |
| 56 | 56 | assert_compare_numbers(0u, QPDFObjectHandle::newNull().getUIntValueAsUInt()); |
| 57 | - std::cerr << "convert null to uint did not throw\n"; | |
| 58 | 57 | } catch (QPDFExc const&) { |
| 59 | 58 | std::cerr << "caught expected type error\n"; |
| 60 | 59 | } |
| 61 | 60 | assert_compare_numbers(std::numeric_limits<int8_t>::max(), Integer(q1).value<int8_t>()); |
| 62 | 61 | assert_compare_numbers(std::numeric_limits<int8_t>::min(), Integer(-q1).value<int8_t>()); |
| 63 | 62 | try { |
| 64 | - int8_t q1_8 = Integer(q1); | |
| 65 | - std::cerr << "q1_8: " << std::to_string(q1_8) << '\n'; | |
| 63 | + [[maybe_unused]] int8_t q1_8 = Integer(q1); | |
| 66 | 64 | } catch (std::overflow_error const&) { |
| 67 | 65 | std::cerr << "caught expected int8_t overflow error\n"; |
| 68 | 66 | } |
| 69 | 67 | try { |
| 70 | - int8_t q1_8 = Integer(-q1); | |
| 71 | - std::cerr << "q1_8: " << std::to_string(q1_8) << '\n'; | |
| 68 | + [[maybe_unused]] int8_t q1_8 = Integer(-q1); | |
| 72 | 69 | } catch (std::underflow_error const&) { |
| 73 | 70 | std::cerr << "caught expected int8_t underflow error\n"; |
| 74 | 71 | } |
| 75 | 72 | assert_compare_numbers(std::numeric_limits<uint8_t>::max(), Integer(q1).value<uint8_t>()); |
| 76 | 73 | assert_compare_numbers(0, Integer(-q1).value<uint8_t>()); |
| 77 | 74 | try { |
| 78 | - uint8_t q1_u8 = Integer(q1); | |
| 79 | - std::cerr << "q1_u8: " << std::to_string(q1_u8) << '\n'; | |
| 75 | + [[maybe_unused]] uint8_t q1_u8 = Integer(q1); | |
| 80 | 76 | } catch (std::overflow_error const&) { |
| 81 | 77 | std::cerr << "caught expected uint8_t overflow error\n"; |
| 82 | 78 | } |
| 83 | 79 | try { |
| 84 | - uint8_t q1_u8 = Integer(-q1); | |
| 85 | - std::cerr << "q1_u8: " << std::to_string(q1_u8) << '\n'; | |
| 80 | + [[maybe_unused]] uint8_t q1_u8 = Integer(-q1); | |
| 86 | 81 | } catch (std::underflow_error const&) { |
| 87 | 82 | std::cerr << "caught expected uint8_t underflow error\n"; |
| 88 | 83 | } |
| ... | ... | @@ -95,6 +90,70 @@ test_0(QPDF& pdf, char const* arg2) |
| 95 | 90 | assert_compare_numbers(UINT_MAX, t.getKey("/Q3").getUIntValueAsUInt()); |
| 96 | 91 | } |
| 97 | 92 | |
| 93 | + | |
| 94 | +static void | |
| 95 | +test_1(QPDF& pdf, char const* arg2) | |
| 96 | +{ | |
| 97 | + // Test new dictionary methods. | |
| 98 | + using namespace qpdf; | |
| 99 | + auto d = Dictionary({{"/A", {}}, {"/B", Null()}, {"/C", Dictionary::empty()}}); | |
| 100 | + | |
| 101 | + // contains | |
| 102 | + assert(!d.contains("/A")); | |
| 103 | + assert(!d.contains("/B")); | |
| 104 | + assert(d.contains("/C")); | |
| 105 | + | |
| 106 | + auto i = Integer(42); | |
| 107 | + assert(!i.contains("/A")); | |
| 108 | + | |
| 109 | + // at | |
| 110 | + assert(!d.at("/A")); | |
| 111 | + assert(d.at("/B")); | |
| 112 | + assert(d.at("/B").null()); | |
| 113 | + assert(d.at("/C")); | |
| 114 | + assert(!d.at("/C").null()); | |
| 115 | + d.at("/C") = Integer(42); | |
| 116 | + assert(d.at("/C") == 42); | |
| 117 | + assert(!d.at("/D")); | |
| 118 | + assert(d.at("/D").null()); | |
| 119 | + assert(QPDFObjectHandle(d).getDictAsMap().contains("/D")); | |
| 120 | + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); | |
| 121 | + | |
| 122 | + bool thrown = false; | |
| 123 | + try { | |
| 124 | + i.at("/A"); | |
| 125 | + } catch (std::runtime_error const&) { | |
| 126 | + thrown = true; | |
| 127 | + } | |
| 128 | + assert(thrown); | |
| 129 | + | |
| 130 | + // find | |
| 131 | + assert(!d.find("/A")); | |
| 132 | + assert(d.find("/B")); | |
| 133 | + assert(d.find("/B").null()); | |
| 134 | + assert(d.find("/C")); | |
| 135 | + assert(Integer(d.find("/C")) == 42); | |
| 136 | + d.find("/C") = Name("/DontPanic"); | |
| 137 | + assert(Name(d.find("/C")) == "/DontPanic"); | |
| 138 | + assert(!d.find("/E")); | |
| 139 | + assert(!QPDFObjectHandle(d).getDictAsMap().contains("/E")); | |
| 140 | + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); | |
| 141 | + | |
| 142 | + // replace | |
| 143 | + assert(!i.replace("/A", Name("/DontPanic"))); | |
| 144 | + Dictionary di = i.oh(); | |
| 145 | + thrown = false; | |
| 146 | + try { | |
| 147 | + di.replace("/A", Name("/DontPanic")); | |
| 148 | + } catch (std::runtime_error const&) { | |
| 149 | + thrown = true; | |
| 150 | + } | |
| 151 | + assert(thrown); | |
| 152 | + d.replace("/C", Integer(42)); | |
| 153 | + assert(Integer(d["/C"]) == 42); | |
| 154 | + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); | |
| 155 | +} | |
| 156 | + | |
| 98 | 157 | void |
| 99 | 158 | runtest(int n, char const* filename1, char const* arg2) |
| 100 | 159 | { |
| ... | ... | @@ -102,7 +161,7 @@ runtest(int n, char const* filename1, char const* arg2) |
| 102 | 161 | // the test suite to see how the test is invoked to find the file |
| 103 | 162 | // that the test is supposed to operate on. |
| 104 | 163 | |
| 105 | - std::set<int> ignore_filename = {}; | |
| 164 | + std::set<int> ignore_filename = {1,}; | |
| 106 | 165 | |
| 107 | 166 | QPDF pdf; |
| 108 | 167 | std::shared_ptr<char> file_buf; |
| ... | ... | @@ -116,7 +175,7 @@ runtest(int n, char const* filename1, char const* arg2) |
| 116 | 175 | } |
| 117 | 176 | |
| 118 | 177 | std::map<int, void (*)(QPDF&, char const*)> test_functions = { |
| 119 | - {0, test_0}, | |
| 178 | + {0, test_0}, {1, test_1}, | |
| 120 | 179 | }; |
| 121 | 180 | |
| 122 | 181 | auto fn = test_functions.find(n); | ... | ... |
libtests/qtest/objects.test
| ... | ... | @@ -11,12 +11,16 @@ require TestDriver; |
| 11 | 11 | |
| 12 | 12 | my $td = new TestDriver('objects'); |
| 13 | 13 | |
| 14 | -my $n_tests = 1; | |
| 15 | - | |
| 14 | +my $n_tests = 2; | |
| 16 | 15 | |
| 17 | 16 | $td->runtest("integer type checks", |
| 18 | 17 | {$td->COMMAND => "objects 0 minimal.pdf"}, |
| 19 | 18 | {$td->FILE => "test0.out", $td->EXIT_STATUS => 0}, |
| 20 | 19 | $td->NORMALIZE_NEWLINES); |
| 21 | 20 | |
| 21 | +$td->runtest("dictionary checks", | |
| 22 | + {$td->COMMAND => "objects 1 -"}, | |
| 23 | + {$td->STRING => => "test 1 done\n", $td->EXIT_STATUS => 0}, | |
| 24 | + $td->NORMALIZE_NEWLINES); | |
| 25 | + | |
| 22 | 26 | $td->report($n_tests); | ... | ... |