From cc1a5e15f65f55589ed55690476a5583ff6be50b Mon Sep 17 00:00:00 2001 From: Thomas Fussell Date: Wed, 4 Jan 2017 19:02:31 -0500 Subject: [PATCH] clean up test xml_helper, rename zip to zstream --- source/detail/vector_streambuf.hpp | 30 ++++++ source/detail/xlsx_consumer.cpp | 4 +- source/detail/xlsx_consumer.hpp | 6 +- source/detail/xlsx_producer.cpp | 4 +- source/detail/xlsx_producer.hpp | 4 +- source/detail/{zip.cpp => zstream.cpp} | 48 +++++---- source/detail/{zip.hpp => zstream.hpp} | 25 +++-- tests/helpers/xml_helper.hpp | 135 ++++--------------------- 8 files changed, 103 insertions(+), 153 deletions(-) rename source/detail/{zip.cpp => zstream.cpp} (93%) rename source/detail/{zip.hpp => zstream.hpp} (89%) diff --git a/source/detail/vector_streambuf.hpp b/source/detail/vector_streambuf.hpp index fb34d9d3..8411e09e 100644 --- a/source/detail/vector_streambuf.hpp +++ b/source/detail/vector_streambuf.hpp @@ -251,6 +251,36 @@ private: std::size_t position_; }; +/// +/// Helper function to read all data from in_stream and store them in a vector. +/// +XLNT_API inline std::vector to_vector(std::istream &in_stream) +{ + std::vector bytes; + vector_ostreambuf buffer(bytes); + std::ostream out_stream(&buffer); + out_stream << in_stream.rdbuf(); + return bytes; +} + +/// +/// Helper function to write all data from bytes into out_stream. +/// +XLNT_API inline void to_stream(const std::vector &bytes, std::ostream &out_stream) +{ + vector_istreambuf buffer(bytes); + out_stream << &buffer; +} + +/// +/// Shortcut function to stream a vector of bytes into a std::ostream. +/// +XLNT_API inline std::ostream &operator<<(std::ostream &out_stream, const std::vector &bytes) +{ + to_stream(bytes, out_stream); + return out_stream; +} + #pragma clang diagnostic pop } // namespace detail diff --git a/source/detail/xlsx_consumer.cpp b/source/detail/xlsx_consumer.cpp index 1d3cbb26..f39e8800 100644 --- a/source/detail/xlsx_consumer.cpp +++ b/source/detail/xlsx_consumer.cpp @@ -35,7 +35,7 @@ #include #include #include -#include +#include namespace std { @@ -530,7 +530,7 @@ xlsx_consumer::xlsx_consumer(workbook &target) void xlsx_consumer::read(std::istream &source) { - archive_.reset(new zip_file_reader(source)); + archive_.reset(new izstream(source)); populate_workbook(); } diff --git a/source/detail/xlsx_consumer.hpp b/source/detail/xlsx_consumer.hpp index bee9a709..86f1ff4a 100644 --- a/source/detail/xlsx_consumer.hpp +++ b/source/detail/xlsx_consumer.hpp @@ -31,7 +31,7 @@ #include #include -#include +#include namespace xlnt { @@ -45,7 +45,7 @@ class worksheet; namespace detail { -class zip_file_reader; +class izstream; /// /// Handles writing a workbook into an XLSX file. @@ -338,7 +338,7 @@ private: /// /// The ZIP file containing the files that make up the OOXML package. /// - std::unique_ptr archive_; + std::unique_ptr archive_; /// /// Map of sheet titles to relationship IDs. diff --git a/source/detail/xlsx_producer.cpp b/source/detail/xlsx_producer.cpp index 98781b93..a62564a9 100644 --- a/source/detail/xlsx_producer.cpp +++ b/source/detail/xlsx_producer.cpp @@ -37,7 +37,7 @@ #include #include #include -#include +#include using namespace std::string_literals; @@ -64,7 +64,7 @@ xlsx_producer::xlsx_producer(const workbook &target) void xlsx_producer::write(std::ostream &destination) { - zip_file_writer archive(destination); + ozstream archive(destination); archive_ = &archive; populate_archive(); } diff --git a/source/detail/xlsx_producer.hpp b/source/detail/xlsx_producer.hpp index dcd6a66e..5cca60e8 100644 --- a/source/detail/xlsx_producer.hpp +++ b/source/detail/xlsx_producer.hpp @@ -44,7 +44,7 @@ class worksheet; namespace detail { -class zip_file_writer; +class ozstream; /// /// Handles writing a workbook into an XLSX file. @@ -137,7 +137,7 @@ private: /// const workbook &source_; - zip_file_writer *archive_; + ozstream *archive_; std::unique_ptr current_part_serializer_; std::unique_ptr current_part_streambuf_; std::ostream current_part_stream_; diff --git a/source/detail/zip.cpp b/source/detail/zstream.cpp similarity index 93% rename from source/detail/zip.cpp rename to source/detail/zstream.cpp index cdfb9c9d..9032e70e 100644 --- a/source/detail/zip.cpp +++ b/source/detail/zstream.cpp @@ -49,7 +49,8 @@ extern "C" { } #include -#include +#include +#include namespace { @@ -68,9 +69,9 @@ void write_int(std::ostream &stream, T value) stream.write(reinterpret_cast(&value), sizeof(T)); } -xlnt::detail::zip_file_header read_header(std::istream &istream, const bool global) +xlnt::detail::zheader read_header(std::istream &istream, const bool global) { - xlnt::detail::zip_file_header header; + xlnt::detail::zheader header; auto sig = read_int(istream); @@ -128,7 +129,7 @@ xlnt::detail::zip_file_header read_header(std::istream &istream, const bool glob return header; } -void write_header(const xlnt::detail::zip_file_header &header, std::ostream &ostream, const bool global) +void write_header(const xlnt::detail::zheader &header, std::ostream &ostream, const bool global) { if (global) { @@ -180,7 +181,7 @@ class zip_streambuf_decompress : public std::streambuf z_stream strm; std::array in; std::array out; - zip_file_header header; + zheader header; std::size_t total_read; std::size_t total_uncompressed; bool valid; @@ -190,7 +191,7 @@ class zip_streambuf_decompress : public std::streambuf static const unsigned short UNCOMPRESSED = 0; public: - zip_streambuf_decompress(std::istream &stream, zip_file_header central_header) + zip_streambuf_decompress(std::istream &stream, zheader central_header) : istream(stream), header(central_header), total_read(0), total_uncompressed(0), valid(true) { strm.zalloc = Z_NULL; @@ -322,14 +323,14 @@ class zip_streambuf_compress : public std::streambuf std::array in; std::array out; - zip_file_header *header; + zheader *header; std::uint32_t uncompressed_size; std::uint32_t crc; bool valid; public: - zip_streambuf_compress(zip_file_header *central_header, std::ostream &stream) + zip_streambuf_compress(zheader *central_header, std::ostream &stream) : ostream(stream), header(central_header), valid(true) { strm.zalloc = Z_NULL; @@ -448,7 +449,7 @@ int zip_streambuf_compress::overflow(int c) return c; } -zip_file_writer::zip_file_writer(std::ostream &stream) +ozstream::ozstream(std::ostream &stream) : destination_stream_(stream) { if (!destination_stream_) @@ -457,7 +458,7 @@ zip_file_writer::zip_file_writer(std::ostream &stream) } } -zip_file_writer::~zip_file_writer() +ozstream::~ozstream() { // Write all file headers std::ios::streampos final_position = destination_stream_.tellp(); @@ -480,15 +481,15 @@ zip_file_writer::~zip_file_writer() write_int(destination_stream_, static_cast(0)); // zip comment } -std::unique_ptr zip_file_writer::open(const path &filename) +std::unique_ptr ozstream::open(const path &filename) { - zip_file_header header; + zheader header; header.filename = filename.string(); file_headers_.push_back(header); return std::make_unique(&file_headers_.back(), destination_stream_); } -zip_file_reader::zip_file_reader(std::istream &stream) +izstream::izstream(std::istream &stream) : source_stream_(stream) { if (!stream) @@ -499,11 +500,11 @@ zip_file_reader::zip_file_reader(std::istream &stream) read_central_header(); } -zip_file_reader::~zip_file_reader() +izstream::~izstream() { } -bool zip_file_reader::read_central_header() +bool izstream::read_central_header() { // Find the header // NOTE: this assumes the zip file header is the last thing written to file... @@ -587,7 +588,7 @@ bool zip_file_reader::read_central_header() return true; } -std::unique_ptr zip_file_reader::open(const path &filename) +std::unique_ptr izstream::open(const path &filename) const { if (!has_file(filename)) { @@ -599,16 +600,25 @@ std::unique_ptr zip_file_reader::open(const path &filename) return std::make_unique(source_stream_, header); } -std::vector zip_file_reader::files() const +std::string izstream::read(const path &filename) const +{ + auto buffer = open(filename); + std::istream stream(buffer.get()); + auto bytes = to_vector(stream); + + return std::string(bytes.begin(), bytes.end()); +} + +std::vector izstream::files() const { std::vector filenames; std::transform(file_headers_.begin(), file_headers_.end(), std::back_inserter(filenames), - [](const std::pair &h) { return path(h.first); }); + [](const std::pair &h) { return path(h.first); }); return filenames; } -bool zip_file_reader::has_file(const path &filename) const +bool izstream::has_file(const path &filename) const { return file_headers_.count(filename.string()) != 0; } diff --git a/source/detail/zip.hpp b/source/detail/zstream.hpp similarity index 89% rename from source/detail/zip.hpp rename to source/detail/zstream.hpp index bb9475b8..938b83b8 100644 --- a/source/detail/zip.hpp +++ b/source/detail/zstream.hpp @@ -49,7 +49,7 @@ namespace detail { /// A structure representing the header that occurs before each compressed file in a ZIP /// archive and again at the end of the file with more information. /// -struct zip_file_header +struct zheader { std::uint16_t version = 20; std::uint16_t flags = 0; @@ -69,18 +69,18 @@ struct zip_file_header /// Writes a series of uncompressed binary file data as ostreams into another ostream /// according to the ZIP format. /// -class zip_file_writer +class ozstream { public: /// /// Construct a new zip_file_writer which writes a ZIP archive to the given stream. /// - zip_file_writer(std::ostream &stream); + ozstream(std::ostream &stream); /// /// Destructor. /// - virtual ~zip_file_writer(); + virtual ~ozstream(); /// /// Returns a pointer to a streambuf which compresses the data it receives. @@ -88,7 +88,7 @@ public: std::unique_ptr open(const path &file); private: - std::vector file_headers_; + std::vector file_headers_; std::ostream &destination_stream_; }; @@ -96,23 +96,28 @@ private: /// Reads an archive containing a number of files from an istream and allows them /// to be decompressed into an istream. /// -class zip_file_reader +class izstream { public: /// /// Construct a new zip_file_reader which reads a ZIP archive from the given stream. /// - zip_file_reader(std::istream &stream); + izstream(std::istream &stream); /// /// Destructor. /// - virtual ~zip_file_reader(); + virtual ~izstream(); /// /// /// - std::unique_ptr open(const path &file); + std::unique_ptr open(const path &file) const; + + /// + /// + /// + std::string read(const path &file) const; /// /// @@ -133,7 +138,7 @@ private: /// /// /// - std::unordered_map file_headers_; + std::unordered_map file_headers_; /// /// diff --git a/tests/helpers/xml_helper.hpp b/tests/helpers/xml_helper.hpp index cd13d603..3b4d6e59 100644 --- a/tests/helpers/xml_helper.hpp +++ b/tests/helpers/xml_helper.hpp @@ -4,37 +4,13 @@ #include #include -#include -#include +#include #include +#include class xml_helper { public: - enum class difference_type - { - names_differ, - missing_attribute, - attribute_values_differ, - missing_text, - text_values_differ, - missing_child, - child_order_differs, - equivalent, - }; - - struct comparison_result - { - difference_type difference; - std::string value_left; - std::string value_right; - - operator bool() const - { - return difference == difference_type::equivalent; - } - }; - static bool compare_files(const std::string &left, const std::string &right, const std::string &content_type) { @@ -46,16 +22,12 @@ public: || content_type == "application/xml" || content_type == "[Content_Types].xml" || content_type == "application/vnd.openxmlformats-officedocument.vmlDrawing"; - - if (is_xml) - { - return compare_xml_exact(left, right); - } - - return left == right; + + return is_xml ? compare_xml_exact(left, right) : left == right; } - static bool compare_xml_exact(const std::string &left, const std::string &right, bool suppress_debug_info = false) + static bool compare_xml_exact(const std::string &left, + const std::string &right, bool suppress_debug_info = false) { xml::parser left_parser(left.data(), left.size(), "left"); xml::parser right_parser(right.data(), right.size(), "right"); @@ -175,66 +147,18 @@ public: return !difference; } - static bool string_matches_workbook_part(const std::string &expected, - xlnt::workbook &wb, const xlnt::path &part, const std::string &content_type) - { - std::vector bytes; - wb.save(bytes); - std::istringstream file_stream(std::string(bytes.begin(), bytes.end())); - xlnt::detail::zip_file_reader archive(file_stream); - - return string_matches_archive_member(expected, archive, part, content_type); - } - - static bool file_matches_workbook_part(const xlnt::path &expected, - xlnt::workbook &wb, const xlnt::path &part, const std::string &content_type) - { - std::vector bytes; - wb.save(bytes); - std::istringstream file_stream(std::string(bytes.begin(), bytes.end())); - xlnt::detail::zip_file_reader archive(file_stream); - - return file_matches_archive_member(expected, archive, part, content_type); - } - - static bool string_matches_archive_member(const std::string &expected, - xlnt::detail::zip_file_reader &archive, - const xlnt::path &member, - const std::string &content_type) - { - auto streambuf = archive.open(member); - std::istream stream(streambuf.get()); - std::string contents((std::istreambuf_iterator(stream)), (std::istreambuf_iterator())); - return compare_files(expected, contents, content_type); - } - - static bool file_matches_archive_member(const xlnt::path &file, - xlnt::detail::zip_file_reader &archive, - const xlnt::path &member, - const std::string &content_type) - { - if (!archive.has_file(member)) return false; - std::vector member_data; - xlnt::detail::vector_ostreambuf member_data_buffer(member_data); - std::ostream member_data_stream(&member_data_buffer); - auto member_streambuf = archive.open(member); - std::ostream member_stream(member_streambuf.get()); - member_data_stream << member_stream.rdbuf(); - std::string contents(member_data.begin(), member_data.end()); - return compare_files(file.read_contents(), contents, content_type); - } - - static bool xlsx_archives_match(const std::vector &left, const std::vector &right) + static bool xlsx_archives_match(const std::vector &left, + const std::vector &right) { xlnt::detail::vector_istreambuf left_buffer(left); std::istream left_stream(&left_buffer); - xlnt::detail::zip_file_reader left_archive(left_stream); + xlnt::detail::izstream left_archive(left_stream); const auto left_info = left_archive.files(); xlnt::detail::vector_istreambuf right_buffer(right); std::istream right_stream(&right_buffer); - xlnt::detail::zip_file_reader right_archive(right_stream); + xlnt::detail::izstream right_archive(right_stream); const auto right_info = right_archive.files(); @@ -274,37 +198,14 @@ public: { match = false; std::cout << "right is missing file: " << left_member.string() << std::endl; - continue; + break; } - auto left_member_streambuf = left_archive.open(left_member); - std::istream left_member_stream(left_member_streambuf.get()); - std::vector left_contents_raw; - xlnt::detail::vector_ostreambuf left_contents_buffer(left_contents_raw); - std::ostream left_contents_stream(&left_contents_buffer); - left_contents_stream << left_member_stream.rdbuf(); - std::string left_member_contents(left_contents_raw.begin(), left_contents_raw.end()); + auto left_content_type = left_member.string() == "[Content_Types].xml" + ? "[Content_Types].xml" : left_manifest.content_type(left_member); + auto right_content_type = left_member.string() == "[Content_Types].xml" + ? "[Content_Types].xml" : right_manifest.content_type(left_member); - auto right_member_streambuf = left_archive.open(left_member); - std::istream right_member_stream(right_member_streambuf.get()); - std::vector right_contents_raw; - xlnt::detail::vector_ostreambuf right_contents_buffer(right_contents_raw); - std::ostream right_contents_stream(&right_contents_buffer); - right_contents_stream << right_member_stream.rdbuf(); - std::string right_member_contents(right_contents_raw.begin(), right_contents_raw.end()); - - std::string left_content_type, right_content_type; - - if (left_member.string() != "[Content_Types].xml") - { - left_content_type = left_manifest.content_type(left_member); - right_content_type = right_manifest.content_type(left_member); - } - else - { - left_content_type = right_content_type = "[Content_Types].xml"; - } - if (left_content_type != right_content_type) { std::cout << "content types differ: " @@ -315,11 +216,15 @@ public: << right_content_type << std::endl; match = false; + break; } - else if (!compare_files(left_member_contents, right_member_contents, left_content_type)) + + if (!compare_files(left_archive.read(left_member), + left_archive.read(left_member), left_content_type)) { std::cout << left_member.string() << std::endl; match = false; + break; } }