Add Post Mortem and fixes from the review

PostMortem.md

@@ -7,3 +7,16 @@ These issues were related to the deployment of the application.
 * unused capture in one of the lambdas. Again, `g++` was not reporting this warning, but reviewer's macos clang warned about it and with `-Werror` caused compilation to fail.
 
 Fixes to the above are included in the `appeals` tag.
+
+# Issues reported as part of the Review phase
+
+Three issues were reported. Quote:
+
+1. If the output directory does not exist, the CLI should create it.
+2. There's no way to provide a list of excluded tags as discussed in the forum.
+3. Your CLI does not handle invalid input files gracefully. It should catch the error and show a meaningful message. Right now, it throws something like the error below and crashes:
+`[2020-11-23 20:26:16.724] [error] Bad XML: <unspecified file>(1): expected <`
+
+For the first two, fixes are included in the `final` tag.
+
+As for the third one, this is the implemented graceful exit. Error notification is presented with the timestamp, `Bad XML` is clearly part of the message, and the exact failure reason reported by the used 3rd party XML parser is quoted verbatim. Arguably this one is unjust.

solution/app/src/filesystem.cpp

@@ -3,6 +3,7 @@
 
 #include <string>
 #include <fstream>
+#include <system_error>
 
 #if defined __cpp_lib_filesystem
 #include <filesystem>
@@ -39,12 +40,21 @@ Either<std::string, std::string> check_directory(std::string const & odir)
     auto const path = fs::path(odir);
     std::error_code ec;
 
-    if (not fs::is_directory(path, ec))
+    if (fs::exists(path, ec))
     {
-        return rv_type::leftOf(fmt::format("{}: {}", odir, ec.message()));
+        if (not fs::is_directory(path, ec))
+        {
+            // interestingly, ec is set to Success
+            return rv_type::leftOf(fmt::format("{}: Not a directory", odir));
+        }
     }
     else
     {
-        return rv_type::rightOf(odir);
+        if (not fs::create_directories(path, ec))
+        {
+            return rv_type::leftOf(fmt::format("Failed to create {}: {}", odir, ec.message()));
+        }
     }
+
+    return rv_type::rightOf(odir);
 }

solution/app/src/main.cpp

@@ -6,6 +6,8 @@
 
 #include <iostream>
 #include <string>
+#include <vector>
+#include <unordered_set>
 
 
 /*
@@ -50,10 +52,12 @@ int main(int argc, char **argv)
     bool verbose = false;
     bool debug = false;
     int json_indent = -1;
+    std::vector<std::string> to_ignore;
 
     auto cli = (
         clipp::required("--input", "-i").doc("Input XML file") & clipp::value("XML data file to read from", xml_ifname),
         clipp::required("--output-dir", "-o").doc("Destination folder where output files are to be created") & clipp::value("Destination folder", out_dir),
+        clipp::option("--ignore-tags", "-x").doc("Specify tags to ignore") & clipp::values("List of tags to ignore", to_ignore),
         clipp::option("--root-name").doc("Name for the root node to use, if enabled") & clipp::value("Name for the root node=" + root_name, root_name),
         (
             clipp::option("--skip-root").set(skip_root, true) |
@@ -86,10 +90,12 @@ int main(int argc, char **argv)
             {"json_indent", json_indent}
         };
 
+        std::unordered_set<std::string> const to_ignore_unique(to_ignore.cbegin(), to_ignore.cend());
+
         /*
          * Attempt to do the actual work
          */
-        auto rv = maybe_work(xml_ifname, out_dir, cli_params);
+        auto rv = maybe_work(xml_ifname, out_dir, to_ignore_unique, cli_params);
 
         spdlog::info("Done");
 

solution/app/src/parser.cpp

@@ -43,7 +43,10 @@ using boost::property_tree::ptree;
  * node will or will not be included in final result, and what the custom name
  * shall be. By default this root node is skipped.
  */
-tags_tree_t collect_tag_stats(ptree && pt, cli_params_t const & cli_params)
+tags_tree_t collect_tag_stats(
+    ptree && pt,
+    std::unordered_set<std::string> const & to_ignore,
+    cli_params_t const & cli_params)
 {
     tags_tree_t tags;
     std::queue<std::pair<std::string, ptree>> nodes;
@@ -86,7 +89,10 @@ tags_tree_t collect_tag_stats(ptree && pt, cli_params_t const & cli_params)
             }
 
             // update known children and their counts
-            unique_children.insert(child_name);
+            if (to_ignore.count(child_name) == 0)
+            {
+                unique_children.insert(child_name);
+            }
 
             nodes.emplace(child_name, child_subtree);
         }
@@ -103,12 +109,21 @@ tags_tree_t collect_tag_stats(ptree && pt, cli_params_t const & cli_params)
         tags.erase(params::root_name(cli_params));
     }
 
+    // drop tags to ignore
+    for (auto const & tag : to_ignore)
+    {
+        tags.erase(tag);
+    }
+
     return tags;
 }
 
 
 Either<std::string, tags_tree_t>
-parse_xml_file(std::ifstream & ifile, cli_params_t const & cli_params)
+parse_xml_file(
+    std::ifstream & ifile,
+    std::unordered_set<std::string> const & to_ignore,
+    cli_params_t const & cli_params)
 {
     using rv_type = Either<std::string, tags_tree_t>;
 
@@ -119,10 +134,10 @@ parse_xml_file(std::ifstream & ifile, cli_params_t const & cli_params)
         // call boost's XML parser
         read_xml(ifile, pt);
     }
-    catch(boost::property_tree::xml_parser::xml_parser_error & ex)
+    catch (boost::property_tree::xml_parser::xml_parser_error & ex)
     {
         return rv_type::leftOf(fmt::format("Bad XML: {}", ex.what()));
     }
 
-    return rv_type::rightOf(collect_tag_stats(std::move(pt), cli_params));
+    return rv_type::rightOf(collect_tag_stats(std::move(pt), to_ignore, cli_params));
 }

solution/app/src/parser.hpp

@@ -9,6 +9,7 @@
 #include <fstream>
 #include <map>
 #include <unordered_map>
+#include <unordered_set>
 
 
 /*
@@ -45,7 +46,10 @@ using tags_tree_t = std::unordered_map<std::string, tag_record_t>;
  */
 [[nodiscard]]
 neither::Either<std::string, tags_tree_t>
-parse_xml_file(std::ifstream & ifile, cli_params_t const & cli_params);
+parse_xml_file(
+    std::ifstream & ifile,
+    std::unordered_set<std::string> const & to_ignore,
+    cli_params_t const & cli_params);
 
 
 #endif /* APP_SRC_PARSER_HPP_ */

solution/app/src/work.cpp

@@ -8,6 +8,7 @@
 #include "spdlog/spdlog.h"
 
 #include <string>
+#include <unordered_set>
 
 
 /*
@@ -45,9 +46,10 @@
 int work(
     std::ifstream & ifile,
     std::string const & odir,
+    std::unordered_set<std::string> const & to_ignore,
     cli_params_t const & cli_params)
 {
-    int rv = parse_xml_file(ifile, cli_params)
+    int rv = parse_xml_file(ifile, to_ignore, cli_params)
         .rightFlatMap(
             [&cli_params](tags_tree_t && tags)
             {
@@ -70,21 +72,22 @@ int work(
 int maybe_work(
     std::string const & ifname,
     std::string const & odir,
+    std::unordered_set<std::string> const & to_ignore,
     cli_params_t const & cli_params)
 {
     int rv = open_file_r(ifname)
         .leftMap(error_printer)
         .rightMap(
-            [&odir, &cli_params](std::ifstream && ifile)
+            [&odir, &to_ignore, &cli_params](std::ifstream && ifile)
             {
                 int rv = check_directory(odir)
                     .leftMap(error_printer)
                     .rightMap(
-                        [&ifile, &cli_params](std::string const & odir)
+                        [&ifile, &to_ignore, &cli_params](std::string const & odir)
                         {
-                            work(ifile, odir, cli_params);
+                            int const rv = work(ifile, odir, to_ignore, cli_params);
 
-                            return 0;
+                            return rv;
                         }
                     ).join();
 

solution/app/src/work.hpp

@@ -6,6 +6,7 @@
 #include "cli_params.hpp"
 
 #include <string>
+#include <unordered_set>
 
 
 /*
@@ -19,6 +20,7 @@
 int maybe_work(
     std::string const & ifname,
     std::string const & odir,
+    std::unordered_set<std::string> const & to_ignore,
     cli_params_t const & cli_params);