clean up test xml_helper, rename zip to zstream

source/detail/vector_streambuf.hpp

@@ -251,6 +251,36 @@ private:
     std::size_t position_;
 };
 
+/// <summary>
+/// Helper function to read all data from in_stream and store them in a vector.
+/// </summary>
+XLNT_API inline std::vector<std::uint8_t> to_vector(std::istream &in_stream)
+{
+    std::vector<std::uint8_t> bytes;
+    vector_ostreambuf buffer(bytes);
+    std::ostream out_stream(&buffer);
+    out_stream << in_stream.rdbuf();
+    return bytes;
+}
+
+/// <summary>
+/// Helper function to write all data from bytes into out_stream.
+/// </summary>
+XLNT_API inline void to_stream(const std::vector<std::uint8_t> &bytes, std::ostream &out_stream)
+{
+    vector_istreambuf buffer(bytes);
+    out_stream << &buffer;
+}
+
+/// <summary>
+/// Shortcut function to stream a vector of bytes into a std::ostream.
+/// </summary>
+XLNT_API inline std::ostream &operator<<(std::ostream &out_stream, const std::vector<std::uint8_t> &bytes)
+{
+    to_stream(bytes, out_stream);
+    return out_stream;
+}
+
 #pragma clang diagnostic pop
 
 } // namespace detail

source/detail/xlsx_consumer.cpp

@@ -35,7 +35,7 @@
 #include <detail/vector_streambuf.hpp>
 #include <detail/workbook_impl.hpp>
 #include <detail/xlsx_consumer.hpp>
-#include <detail/zip.hpp>
+#include <detail/zstream.hpp>
 
 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();
 }
 

source/detail/xlsx_consumer.hpp

@@ -31,7 +31,7 @@
 #include <vector>
 
 #include <detail/include_libstudxml.hpp>
-#include <detail/zip.hpp>
+#include <detail/zstream.hpp>
 
 namespace xlnt {
 
@@ -45,7 +45,7 @@ class worksheet;
 
 namespace detail {
 
-class zip_file_reader;
+class izstream;
 
 /// <summary>
 /// Handles writing a workbook into an XLSX file.
@@ -338,7 +338,7 @@ private:
 	/// <summary>
 	/// The ZIP file containing the files that make up the OOXML package.
 	/// </summary>
-	std::unique_ptr<zip_file_reader> archive_;
+	std::unique_ptr<izstream> archive_;
 
 	/// <summary>
 	/// Map of sheet titles to relationship IDs.

source/detail/xlsx_producer.cpp

@@ -37,7 +37,7 @@
 #include <detail/vector_streambuf.hpp>
 #include <detail/workbook_impl.hpp>
 #include <detail/xlsx_producer.hpp>
-#include <detail/zip.hpp>
+#include <detail/zstream.hpp>
 
 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();
 }

source/detail/xlsx_producer.hpp

@@ -44,7 +44,7 @@ class worksheet;
 
 namespace detail {
 
-class zip_file_writer;
+class ozstream;
 
 /// <summary>
 /// Handles writing a workbook into an XLSX file.
@@ -137,7 +137,7 @@ private:
 	/// </summary>
 	const workbook &source_;
     
-	zip_file_writer *archive_;
+	ozstream *archive_;
     std::unique_ptr<xml::serializer> current_part_serializer_;
     std::unique_ptr<std::streambuf> current_part_streambuf_;
     std::ostream current_part_stream_;

source/detail/zstream.cpp

@@ -49,7 +49,8 @@ extern "C" {
 }
 
 #include <xlnt/utils/exceptions.hpp>
-#include <detail/zip.hpp>
+#include <detail/vector_streambuf.hpp>
+#include <detail/zstream.hpp>
 
 namespace {
 
@@ -68,9 +69,9 @@ void write_int(std::ostream &stream, T value)
     stream.write(reinterpret_cast<char *>(&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<std::uint32_t>(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<char, buffer_size> in;
     std::array<char, buffer_size> 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<char, buffer_size> in;
     std::array<char, buffer_size> 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<std::uint16_t>(0)); // zip comment
 }
 
-std::unique_ptr<std::streambuf> zip_file_writer::open(const path &filename)
+std::unique_ptr<std::streambuf> ozstream::open(const path &filename)
 {
-    zip_file_header header;
+    zheader header;
     header.filename = filename.string();
     file_headers_.push_back(header);
     return std::make_unique<zip_streambuf_compress>(&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<std::streambuf> zip_file_reader::open(const path &filename)
+std::unique_ptr<std::streambuf> izstream::open(const path &filename) const
 {
     if (!has_file(filename))
     {
@@ -599,16 +600,25 @@ std::unique_ptr<std::streambuf> zip_file_reader::open(const path &filename)
     return std::make_unique<zip_streambuf_decompress>(source_stream_, header);
 }
 
-std::vector<path> 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<path> izstream::files() const
 {
     std::vector<path> filenames;
     std::transform(file_headers_.begin(), file_headers_.end(), std::back_inserter(filenames),
-        [](const std::pair<std::string, zip_file_header> &h) { return path(h.first); });
+        [](const std::pair<std::string, zheader> &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;
 }

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.
 /// </summary>
-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.
 /// </summary>
-class zip_file_writer
+class ozstream
 {
 public:
     /// <summary>
     /// Construct a new zip_file_writer which writes a ZIP archive to the given stream.
     /// </summary>
-    zip_file_writer(std::ostream &stream);
+    ozstream(std::ostream &stream);
 
     /// <summary>
     /// Destructor.
     /// </summary>
-    virtual ~zip_file_writer();
+    virtual ~ozstream();
 
     /// <summary>
     /// Returns a pointer to a streambuf which compresses the data it receives.
@@ -88,7 +88,7 @@ public:
     std::unique_ptr<std::streambuf> open(const path &file);
 
 private:
-    std::vector<zip_file_header> file_headers_;
+    std::vector<zheader> 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.
 /// </summary>
-class zip_file_reader
+class izstream
 {
 public:
     /// <summary>
     /// Construct a new zip_file_reader which reads a ZIP archive from the given stream.
     /// </summary>
-    zip_file_reader(std::istream &stream);
+    izstream(std::istream &stream);
 
     /// <summary>
     /// Destructor.
     /// </summary>
-    virtual ~zip_file_reader();
+    virtual ~izstream();
 
     /// <summary>
     ///
     /// </summary>
-    std::unique_ptr<std::streambuf> open(const path &file);
+    std::unique_ptr<std::streambuf> open(const path &file) const;
+
+    /// <summary>
+    ///
+    /// </summary>
+    std::string read(const path &file) const;
 
     /// <summary>
     ///
@@ -133,7 +138,7 @@ private:
     /// <summary>
     ///
     /// </summary>
-    std::unordered_map<std::string, zip_file_header> file_headers_;
+    std::unordered_map<std::string, zheader> file_headers_;
 
     /// <summary>
     ///

tests/helpers/xml_helper.hpp

@@ -4,37 +4,13 @@
 
 #include <detail/include_libstudxml.hpp>
 #include <detail/vector_streambuf.hpp>
-#include <detail/zip.hpp>
+#include <detail/zstream.hpp>
-#include <helpers/path_helper.hpp>
 #include <xlnt/packaging/manifest.hpp>
+#include <xlnt/workbook/workbook.hpp>
 
 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 is_xml ? compare_xml_exact(left, right) : left == right;
-        {
-            return compare_xml_exact(left, right);
-        }
-        
-        return 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,
+	static bool xlsx_archives_match(const std::vector<std::uint8_t> &left,
-		xlnt::workbook &wb, const xlnt::path &part, const std::string &content_type)
+        const std::vector<std::uint8_t> &right)
-	{
-		std::vector<std::uint8_t> 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<std::uint8_t> 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<char>(stream)), (std::istreambuf_iterator<char>()));
-		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<std::uint8_t> 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<std::uint8_t> &left, const std::vector<std::uint8_t> &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);
+            auto left_content_type = left_member.string() == "[Content_Types].xml"
-            std::istream left_member_stream(left_member_streambuf.get());
+                ? "[Content_Types].xml" : left_manifest.content_type(left_member);
-            std::vector<std::uint8_t> left_contents_raw;
+            auto right_content_type = left_member.string() == "[Content_Types].xml"
-            xlnt::detail::vector_ostreambuf left_contents_buffer(left_contents_raw);
+                ? "[Content_Types].xml" : right_manifest.content_type(left_member);
-            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 right_member_streambuf = left_archive.open(left_member);
-            std::istream right_member_stream(right_member_streambuf.get());
-            std::vector<std::uint8_t> 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;
 			}
 		}