Refactor `QPDFJob`: replace `PageSpec` with `Selection`, introduce `new_selectio…

…n` method, and update logic for handling page specifications.

Refactor `QPDFJob`: replace `PageSpec` with `Selection`, introduce `new_selectio…
…n` method, and update logic for handling page specifications.
m-holger
1 parent 3e39e0dc
Showing 5 changed files with 53 additions and 53 deletions
include/qpdf/QPDFJob.hh
libqpdf/QPDFJob.cc
libqpdf/QPDFJob_config.cc
libqpdf/qpdf/QPDFJob_private.hh
qpdf/qpdf.testcov
@@ -156,16 +156,6 @@ class QPDFJob
         bool replace{false};
     };
  
-    struct PageSpec
-    {
-        PageSpec(
-            std::string const& filename, std::string const& password, std::string const& range);
-
-        std::string filename;
-        std::string password;
-        std::string range;
-    };
-
   public:
     // CONFIGURATION
  
@@ -426,6 +416,7 @@ class QPDFJob
  
   private:
     struct PageNo;
+    struct Selection;
     struct RotationSpec;
     struct UnderOverlay;
     struct PageLabelSpec;
@@ -470,6 +461,8 @@ class QPDFJob
     void setQPDFOptions(QPDF& pdf);
     bool handlePageSpecs(QPDF& pdf);
     bool shouldRemoveUnreferencedResources(QPDF& pdf);
+    void new_selection(
+        std::string const& filename, std::string const& password, std::string const& range);
     void handleRotations(QPDF& pdf);
     void getUOPagenos(
         std::vector<UnderOverlay>& uo, std::vector<std::map<size_t, std::vector<int>>>& pagenos);
@@ -267,14 +267,6 @@ struct QPDFJob::PageNo
     int no{1};
 };
  
-QPDFJob::PageSpec::PageSpec(
-    std::string const& filename, std::string const& password, std::string const& range) :
-    filename(filename),
-    password(password.empty() ? "" : password),
-    range(range)
-{
-}
-
 QPDFPageData::QPDFPageData(std::string const& filename, QPDF* qpdf, std::string const& range) :
     filename(filename),
     qpdf(qpdf),
@@ -464,7 +456,7 @@ QPDFJob::createQPDF()
         pdf.updateFromJSON(m->update_from_json);
     }
  
-    if (!m->page_specs.empty()) {
+    if (!m->selections.empty()) {
         if (!handlePageSpecs(pdf)) {
             m->warnings = true;
         }
@@ -2374,6 +2366,13 @@ added_page(QPDF&amp; pdf, QPDFPageObjectHelper page)
     return added_page(pdf, page.getObjectHandle());
 }
  
+void
+QPDFJob::new_selection(
+    std::string const& filename, std::string const& password, std::string const& range)
+{
+    m->selections.emplace_back(filename, password, range);
+}
+
 // Handle all page specifications. Return true if it succeeded without warnings.
 bool
 QPDFJob::handlePageSpecs(QPDF& pdf)
@@ -2383,12 +2382,12 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
     // Parse all page specifications and translate them into lists of actual pages.
  
     // Handle "." as a shortcut for the input file
-    for (auto& page_spec: m->page_specs) {
-        if (page_spec.filename == ".") {
-            page_spec.filename = m->infilename;
+    for (auto& selection: m->selections) {
+        if (selection.filename == ".") {
+            selection.filename = m->infilename;
         }
-        if (page_spec.range.empty()) {
-            page_spec.range = "1-z";
+        if (selection.range.empty()) {
+            selection.range = "1-z";
         }
     }
  
@@ -2397,8 +2396,8 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
         // Rather than trying to code some portable heuristic based on OS limits, just hard-code
         // this at a given number and allow users to override.
         std::set<std::string> filenames;
-        for (auto& page_spec: m->page_specs) {
-            filenames.insert(page_spec.filename);
+        for (auto& selection: m->selections) {
+            filenames.insert(selection.filename);
         }
         m->keep_files_open = (filenames.size() <= m->keep_files_open_threshold);
         QTC::TC("qpdf", "QPDFJob automatically set keep files open", m->keep_files_open ? 0 : 1);
@@ -2414,8 +2413,8 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
     page_spec_qpdfs[m->infilename] = &pdf;
     std::vector<QPDFPageData> parsed_specs;
     std::map<unsigned long long, std::set<QPDFObjGen>> copied_pages;
-    for (auto& page_spec: m->page_specs) {
-        if (!page_spec_qpdfs.contains(page_spec.filename)) {
+    for (auto& selection: m->selections) {
+        if (!page_spec_qpdfs.contains(selection.filename)) {
             // Open the PDF file and store the QPDF object. Throw a std::shared_ptr to the qpdf into
             // a heap so that it survives through copying to the output but gets cleaned up
             // automatically at the end. Do not canonicalize the file name. Using two different
@@ -2423,41 +2422,41 @@ QPDFJob::handlePageSpecs(QPDF&amp; pdf)
             // you are using this an example of how to do this with the API, you can just create two
             // different QPDF objects to the same underlying file with the same path to achieve the
             // same effect.
-            auto password = page_spec.password;
+            auto password = selection.password;
             if (!m->encryption_file.empty() && password.empty() &&
-                page_spec.filename == m->encryption_file) {
+                selection.filename == m->encryption_file) {
                 QTC::TC("qpdf", "QPDFJob pages encryption password");
                 password = m->encryption_file_password;
             }
             doIfVerbose([&](Pipeline& v, std::string const& prefix) {
-                v << prefix << ": processing " << page_spec.filename << "\n";
+                v << prefix << ": processing " << selection.filename << "\n";
             });
             std::shared_ptr<InputSource> is;
             ClosedFileInputSource* cis = nullptr;
             if (!m->keep_files_open) {
                 QTC::TC("qpdf", "QPDFJob keep files open n");
-                cis = new ClosedFileInputSource(page_spec.filename.c_str());
+                cis = new ClosedFileInputSource(selection.filename.c_str());
                 is = std::shared_ptr<InputSource>(cis);
                 cis->stayOpen(true);
             } else {
                 QTC::TC("qpdf", "QPDFJob keep files open y");
-                FileInputSource* fis = new FileInputSource(page_spec.filename.c_str());
+                FileInputSource* fis = new FileInputSource(selection.filename.c_str());
                 is = std::shared_ptr<InputSource>(fis);
             }
             std::unique_ptr<QPDF> qpdf_sp;
             processInputSource(qpdf_sp, is, password.data(), true);
-            page_spec_qpdfs[page_spec.filename] = qpdf_sp.get();
+            page_spec_qpdfs[selection.filename] = qpdf_sp.get();
             page_heap.push_back(std::move(qpdf_sp));
             if (cis) {
                 cis->stayOpen(false);
-                page_spec_cfis[page_spec.filename] = cis;
+                page_spec_cfis[selection.filename] = cis;
             }
         }
  
         // Read original pages from the PDF, and parse the page range associated with this
         // occurrence of the file.
         parsed_specs.emplace_back(
-            page_spec.filename, page_spec_qpdfs[page_spec.filename], page_spec.range);
+            selection.filename, page_spec_qpdfs[selection.filename], selection.range);
     }
  
     std::map<unsigned long long, bool> remove_unreferenced;
@@ -978,7 +978,7 @@ QPDFJob::PagesConfig::PagesConfig(Config* c) :
 std::shared_ptr<QPDFJob::PagesConfig>
 QPDFJob::Config::pages()
 {
-    if (!o.m->page_specs.empty()) {
+    if (!o.m->selections.empty()) {
         usage("--pages may only be specified one time");
     }
     return std::shared_ptr<PagesConfig>(new PagesConfig(this));
@@ -987,7 +987,7 @@ QPDFJob::Config::pages()
 QPDFJob::Config*
 QPDFJob::PagesConfig::endPages()
 {
-    auto n_specs = config->o.m->page_specs.size();
+    auto n_specs = config->o.m->selections.size();
     if (n_specs == 0) {
         usage("--pages: no page specifications given");
     }
@@ -998,27 +998,25 @@ QPDFJob::PagesConfig*
 QPDFJob::PagesConfig::pageSpec(
     std::string const& filename, std::string const& range, char const* password)
 {
-    config->o.m->page_specs.emplace_back(filename, password, range);
+    config->o.new_selection(filename, {password ? password : ""}, range);
     return this;
 }
  
 QPDFJob::PagesConfig*
 QPDFJob::PagesConfig::file(std::string const& arg)
 {
-    config->o.m->page_specs.emplace_back(arg, "", "");
+    config->o.new_selection(arg, {}, {});
     return this;
 }
  
 QPDFJob::PagesConfig*
 QPDFJob::PagesConfig::range(std::string const& arg)
 {
-    if (config->o.m->page_specs.empty()) {
-        QTC::TC("qpdf", "QPDFJob misplaced page range");
+    if (config->o.m->selections.empty()) {
         usage("in --range must follow a file name");
     }
-    auto& last = config->o.m->page_specs.back();
+    auto& last = config->o.m->selections.back();
     if (!last.range.empty()) {
-        QTC::TC("qpdf", "QPDFJob duplicated range");
         usage("--range already specified for this file");
     }
     last.range = arg;
@@ -1028,13 +1026,11 @@ QPDFJob::PagesConfig::range(std::string const&amp; arg)
 QPDFJob::PagesConfig*
 QPDFJob::PagesConfig::password(std::string const& arg)
 {
-    if (config->o.m->page_specs.empty()) {
-        QTC::TC("qpdf", "QPDFJob misplaced pages password");
+    if (config->o.m->selections.empty()) {
         usage("in --pages, --password must follow a file name");
     }
-    auto& last = config->o.m->page_specs.back();
+    auto& last = config->o.m->selections.back();
     if (!last.password.empty()) {
-        QTC::TC("qpdf", "QPDFJob duplicated pages password");
         usage("--password already specified for this file");
     }
     last.password = arg;
@@ -6,6 +6,22 @@
 #include <qpdf/ClosedFileInputSource.hh>
 #include <qpdf/QPDFLogger.hh>
  
+// A selection of pages from a single input PDF to be included in the output. This corresponds to a
+// single clause in the --pages option.
+struct QPDFJob::Selection
+{
+    Selection(std::string const& filename, std::string const& password, std::string const& range) :
+        filename(filename),
+        password(password),
+        range(range)
+    {
+    }
+
+    std::string filename;
+    std::string password;
+    std::string range;
+};
+
 struct QPDFJob::RotationSpec
 {
     RotationSpec(int angle = 0, bool relative = false) :
@@ -195,7 +211,7 @@ class QPDFJob::Members
     std::vector<UnderOverlay> underlay;
     std::vector<UnderOverlay> overlay;
     UnderOverlay* under_overlay{nullptr};
-    std::vector<PageSpec> page_specs;
+    std::vector<Selection> selections;
     std::map<std::string, RotationSpec> rotations;
     bool require_outfile{true};
     bool replace_input{false};
@@ -491,8 +491,6 @@ qpdf-c called qpdf_oh_get_binary_string_value 0
 qpdf-c called qpdf_oh_get_binary_utf8_value 0
 qpdf-c called qpdf_oh_new_binary_string 0
 qpdf-c called qpdf_oh_new_binary_unicode_string 0
-QPDFJob duplicated pages password 0
-QPDFJob misplaced pages password 0
 QPDFJob check encrypted encrypted 0
 QPDFJob check encrypted not encrypted 0
 QPDFJob check password password incorrect 0
@@ -545,8 +543,6 @@ QPDFPageObjectHelper used fallback without copying 0
 QPDF skipping cache for known unchecked object 0
 QPDF fix dangling triggered xref reconstruction 0
 QPDF recover xref stream 0
-QPDFJob misplaced page range 0
-QPDFJob duplicated range 0
 QPDFJob json over/under no file 0
 QPDF_Array copy 1
 QPDF_json stream data not string 0