lmi-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi-commits] [lmi] master 6bb44d6: Add pdf_writer_wx::save() to explici


From: Greg Chicares
Subject: [lmi-commits] [lmi] master 6bb44d6: Add pdf_writer_wx::save() to explicitly save PDF documents
Date: Sun, 1 Apr 2018 12:37:07 -0400 (EDT)

branch: master
commit 6bb44d6eb39cbde24ab2b4b93a17256de0a25a4e
Author: Vadim Zeitlin <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Add pdf_writer_wx::save() to explicitly save PDF documents
    
    Instead of always calling wxPdfDC::EndDoc() in pdf_writer_wx dtor, only
    do it from the newly added save() method. This ensures that the PDF file
    is not saved at all if an error occurs while generating it, instead of
    creating an invalid (incomplete, and possibly empty) file.
    
    Add various safety run-time checks to ensure that save() is called once
    and once only and that no other methods of pdf_writer_wx are called
    after it.
    
    Update all code using pdf_writer_wx to call save(). For pdf_illustration
    class this involved replacing "writer_" member variable with a "writer"
    local one and, as consequence, passing the output file name parameter to
    render_all() method instead of ctor -- which is more logical in any case
    as only render_all() really creates the output and, in principle, could
    be called multiple times with different file names on the same object.
---
 group_quote_pdf_gen_wx.cpp  |  2 ++
 ledger_pdf_generator_wx.cpp | 61 ++++++++++++++++++---------------------------
 pdf_writer_wx.cpp           | 56 +++++++++++++++++++++++++++++++++++++++--
 pdf_writer_wx.hpp           | 14 ++++++++++-
 4 files changed, 93 insertions(+), 40 deletions(-)

diff --git a/group_quote_pdf_gen_wx.cpp b/group_quote_pdf_gen_wx.cpp
index 7816beb..8162182 100644
--- a/group_quote_pdf_gen_wx.cpp
+++ b/group_quote_pdf_gen_wx.cpp
@@ -794,6 +794,8 @@ void group_quote_pdf_generator_wx::save(std::string const& 
output_filename)
 
     LMI_ASSERT(current_page == total_pages);
     output_page_number_and_version(pdf_writer, total_pages, current_page);
+
+    std::move(pdf_writer).save();
 }
 
 int group_quote_pdf_generator_wx::compute_pages_for_table_rows
diff --git a/ledger_pdf_generator_wx.cpp b/ledger_pdf_generator_wx.cpp
index 20a4825..21aa230 100644
--- a/ledger_pdf_generator_wx.cpp
+++ b/ledger_pdf_generator_wx.cpp
@@ -727,11 +727,8 @@ class page
 class pdf_illustration : protected html_interpolator
 {
   public:
-    pdf_illustration(Ledger const& ledger
-                    ,fs::path const& output
-                    )
+    explicit pdf_illustration(Ledger const& ledger)
         :html_interpolator(ledger.make_evaluator())
-        ,writer_(output.string(), wxPORTRAIT, &html_font_sizes)
         ,ledger_(ledger)
     {
         init_variables();
@@ -752,15 +749,17 @@ class pdf_illustration : protected html_interpolator
         pages_.emplace_back(std::move(page));
     }
 
-    // Render all pages.
-    void render_all()
+    // Render all pages to the specified PDF file.
+    void render_all(fs::path const& output)
     {
+        pdf_writer_wx writer(output.string(), wxPORTRAIT, &html_font_sizes);
+
         html_cell_for_pdf_output::pdf_context_setter
-            set_pdf_context(ledger_, writer_, *this);
+            set_pdf_context(ledger_, writer, *this);
 
         for(auto const& page : pages_)
             {
-            page->pre_render(ledger_, writer_, *this);
+            page->pre_render(ledger_, writer, *this);
             }
 
         bool first = true;
@@ -776,11 +775,13 @@ class pdf_illustration : protected html_interpolator
                 // Do start a new physical page before rendering all the
                 // subsequent pages (notice that a page is also free to call
                 // StartPage() from its render()).
-                writer_.dc().StartPage();
+                writer.dc().StartPage();
                 }
 
-            page->render(ledger_, writer_, *this);
+            page->render(ledger_, writer, *this);
             }
+
+        std::move(writer).save();
     }
 
     // Methods to be implemented by the derived classes to indicate which
@@ -936,9 +937,6 @@ class pdf_illustration : protected html_interpolator
     // simpler to replicate the existing illustrations.
     static std::array<int, 7> const html_font_sizes;
 
-    // Writer object used for the page metrics and higher level functions.
-    pdf_writer_wx writer_;
-
     // Source of the data.
     Ledger const& ledger_;
 
@@ -1986,10 +1984,8 @@ class ill_reg_supplemental_report : public 
standard_supplemental_report
 class pdf_illustration_regular : public pdf_illustration
 {
   public:
-    pdf_illustration_regular(Ledger const& ledger
-                            ,fs::path const& output
-                            )
-        :pdf_illustration(ledger, output)
+    explicit pdf_illustration_regular(Ledger const& ledger)
+        :pdf_illustration(ledger)
     {
         auto const& invar = ledger.GetLedgerInvariant();
         auto const& policy_name = invar.PolicyLegalName;
@@ -2489,11 +2485,8 @@ class nasd_assumption_detail : public 
page_with_tabular_report
 class pdf_illustration_nasd : public pdf_illustration
 {
   public:
-    pdf_illustration_nasd
-        (Ledger const& ledger
-        ,fs::path const& output
-        )
-        :pdf_illustration(ledger, output)
+    explicit pdf_illustration_nasd(Ledger const& ledger)
+        :pdf_illustration(ledger)
     {
         auto const& invar = ledger.GetLedgerInvariant();
 
@@ -2592,11 +2585,8 @@ class reg_d_group_basic : public 
page_with_basic_tabular_report
 class pdf_illustration_reg_d_group : public pdf_illustration
 {
   public:
-    pdf_illustration_reg_d_group
-        (Ledger const& ledger
-        ,fs::path const& output
-        )
-        :pdf_illustration(ledger, output)
+    explicit pdf_illustration_reg_d_group(Ledger const& ledger)
+        :pdf_illustration(ledger)
     {
         // Define variables specific to this illustration.
         auto const& invar = ledger.GetLedgerInvariant();
@@ -2871,11 +2861,8 @@ class reg_d_individual_curr : public 
page_with_tabular_report
 class pdf_illustration_reg_d_individual : public pdf_illustration
 {
   public:
-    pdf_illustration_reg_d_individual
-        (Ledger const& ledger
-        ,fs::path const& output
-        )
-        :pdf_illustration(ledger, output)
+    explicit pdf_illustration_reg_d_individual(Ledger const& ledger)
+        :pdf_illustration(ledger)
     {
         auto const& invar = ledger.GetLedgerInvariant();
 
@@ -2942,16 +2929,16 @@ void ledger_pdf_generator_wx::write
     switch(z)
         {
         case mce_ill_reg:
-            pdf_ill = std::make_unique<pdf_illustration_regular>(ledger, 
output);
+            pdf_ill = std::make_unique<pdf_illustration_regular>(ledger);
             break;
         case mce_nasd:
-            pdf_ill = std::make_unique<pdf_illustration_nasd>(ledger, output);
+            pdf_ill = std::make_unique<pdf_illustration_nasd>(ledger);
             break;
         case mce_group_private_placement:
-            pdf_ill = std::make_unique<pdf_illustration_reg_d_group>(ledger, 
output);
+            pdf_ill = std::make_unique<pdf_illustration_reg_d_group>(ledger);
             break;
         case mce_individual_private_placement:
-            pdf_ill = 
std::make_unique<pdf_illustration_reg_d_individual>(ledger, output);
+            pdf_ill = 
std::make_unique<pdf_illustration_reg_d_individual>(ledger);
             break;
         case mce_prospectus_obsolete:                 // fall through
         case mce_offshore_private_placement_obsolete: // fall through
@@ -2960,7 +2947,7 @@ void ledger_pdf_generator_wx::write
             alarum() << "Unsupported ledger type '" << z << "'." << LMI_FLUSH;
         }
 
-    pdf_ill->render_all();
+    pdf_ill->render_all(output);
 }
 
 volatile bool ensure_setup = ledger_pdf_generator::set_creator
diff --git a/pdf_writer_wx.cpp b/pdf_writer_wx.cpp
index 349d3f3..1bc3a2c 100644
--- a/pdf_writer_wx.cpp
+++ b/pdf_writer_wx.cpp
@@ -23,6 +23,8 @@
 
 #include "pdf_writer_wx.hpp"
 
+#include "alert.hpp"                    // safely_show_message()
+#include "assert_lmi.hpp"
 #include "contains.hpp"
 #include "global_settings.hpp"
 #include "html.hpp"
@@ -30,7 +32,9 @@
 #include <wx/filesys.h>
 #include <wx/html/htmlcell.h>
 
+#include <exception>                    // uncaught_exceptions()
 #include <limits>
+#include <sstream>
 
 namespace
 {
@@ -121,6 +125,18 @@ pdf_writer_wx::pdf_writer_wx
     html_parser_.SetFS(html_vfs_.get());
 }
 
+wxDC& pdf_writer_wx::dc()
+{
+    LMI_ASSERT_WITH_MSG
+        (!was_saved_
+        ,"Can't use device context of the PDF file \""
+            << print_data_.GetFilename().ToStdString(wxConvUTF8)
+            << "\" which was already saved"
+        );
+
+    return pdf_dc_;
+}
+
 /// Output an image at the given scale into the PDF.
 ///
 /// The scale specifies how many times the image should be shrunk:
@@ -138,6 +154,13 @@ void pdf_writer_wx::output_image
     ,oenum_render_or_only_measure output_mode
     )
 {
+    LMI_ASSERT_WITH_MSG
+        (!was_saved_
+        ,"Can't add an image to the PDF file \""
+            << print_data_.GetFilename().ToStdString(wxConvUTF8)
+            << "\" which was already saved"
+        );
+
     int const y = wxRound(image.GetHeight() / scale);
 
     switch(output_mode)
@@ -176,6 +199,13 @@ int pdf_writer_wx::output_html
     ,oenum_render_or_only_measure output_mode
     )
 {
+    LMI_ASSERT_WITH_MSG
+        (!was_saved_
+        ,"Can't output HTML to the PDF file \""
+            << print_data_.GetFilename().ToStdString(wxConvUTF8)
+            << "\" which was already saved"
+        );
+
     // We don't really want to change the font, but to preserve the current DC
     // font which is changed by rendering the HTML contents.
     wxDCFontChanger preserve_font(pdf_dc_, wxFont());
@@ -240,8 +270,30 @@ int pdf_writer_wx::get_page_bottom() const
     return total_page_size_.y - vert_margin;
 }
 
-pdf_writer_wx::~pdf_writer_wx()
+void pdf_writer_wx::save() &&
 {
-    // This will finally generate the PDF file.
     pdf_dc_.EndDoc();
+
+    was_saved_ = true;
+}
+
+pdf_writer_wx::~pdf_writer_wx()
+{
+    // We keep things simple and just check whether any exceptions are
+    // currently in progress instead of storing the number of exceptions being
+    // handled in ctor and checking if this number is greater here, because it
+    // seems highly unlikely that a pdf_writer_wx object would ever be created
+    // in a dtor of some other object, which is the only situation in which the
+    // two versions would behave differently -- and even if it did happen, it
+    // would just result in incorrectly skipping the check, i.e. not critical.
+    if(!std::uncaught_exceptions() && !was_saved_)
+        {
+        std::ostringstream oss;
+        oss
+            << "Please report this: PDF file \""
+            << print_data_.GetFilename().ToStdString(wxConvUTF8)
+            << "\" was not saved."
+            ;
+        safely_show_message(oss.str());
+        }
 }
diff --git a/pdf_writer_wx.hpp b/pdf_writer_wx.hpp
index c5bcc11..c9be498 100644
--- a/pdf_writer_wx.hpp
+++ b/pdf_writer_wx.hpp
@@ -52,8 +52,17 @@ class pdf_writer_wx
     pdf_writer_wx(pdf_writer_wx const&) = delete;
     pdf_writer_wx& operator=(pdf_writer_wx const&) = delete;
 
+    // Dtor checks if save() had been called, so don't forget to do it.
     ~pdf_writer_wx();
 
+    // Save the PDF to the output file name specified in the ctor.
+    //
+    // This object becomes unusable after saving, i.e. no other methods can be
+    // called on it. To help with preventing using any of them accidentally,
+    // this method is rvalue-reference-qualified, meaning that calling
+    // std::move() is required to call it.
+    void save() &&;
+
     // High level functions which should be preferably used if possible.
     int output_html
         (int                          x
@@ -73,7 +82,7 @@ class pdf_writer_wx
         );
 
     // Accessors allowing to use lower level wxDC API directly.
-    wxDC& dc() { return pdf_dc_; }
+    wxDC& dc();
 
     // Page metrics: the page width and height are the size of the page region
     // reserved for the normal contents, excluding horizontal and vertical
@@ -95,6 +104,9 @@ class pdf_writer_wx
     wxHtmlWinParser html_parser_;
 
     wxSize const total_page_size_;
+
+    // Set to true after save() was called.
+    bool was_saved_{false};
 };
 
 #endif // pdf_writer_wx_hpp



reply via email to

[Prev in Thread] Current Thread [Next in Thread]