From 84e9dd099ef3f2e1d9492997fedd3dbba5a751d2 Mon Sep 17 00:00:00 2001 From: Thomas Fussell Date: Thu, 29 Oct 2015 23:16:31 -0400 Subject: [PATCH] keep refactoring serialization code, updating tests --- include/xlnt/s11n/workbook_serializer.hpp | 10 +- source/s11n/workbook_serializer.cpp | 4 +- tests/helpers/helper.hpp | 96 +++++-------- tests/test_props.hpp | 36 +++-- tests/test_read.hpp | 164 +++++++++++++++++----- tests/test_style_writer.hpp | 4 +- tests/test_theme.hpp | 9 +- tests/test_worksheet.hpp | 2 +- 8 files changed, 212 insertions(+), 113 deletions(-) diff --git a/include/xlnt/s11n/workbook_serializer.hpp b/include/xlnt/s11n/workbook_serializer.hpp index 54311811..fb3d24e9 100644 --- a/include/xlnt/s11n/workbook_serializer.hpp +++ b/include/xlnt/s11n/workbook_serializer.hpp @@ -52,15 +52,15 @@ public: xml_document write_workbook() const; xml_document write_properties_app() const; xml_document write_properties_core() const; - -private: + using string_pair = std::pair; - std::vector read_sheets(zip_file &archive); - std::vector detect_worksheets(zip_file &archive); - + std::vector read_sheets(); + std::vector detect_worksheets(); + bool write_named_ranges(xml_node &named_ranges_node); +private: workbook &wb_; }; diff --git a/source/s11n/workbook_serializer.cpp b/source/s11n/workbook_serializer.cpp index e7ca1cfc..45347069 100644 --- a/source/s11n/workbook_serializer.cpp +++ b/source/s11n/workbook_serializer.cpp @@ -147,7 +147,7 @@ std::string workbook_serializer::determine_document_type(const manifest &manifes /// workbook has a list of titles and relIds but no paths /// workbook_rels has a list of relIds and paths but no titles /// -std::vector> workbook_serializer::detect_worksheets(zip_file &archive) +std::vector workbook_serializer::detect_worksheets() { static const std::string ValidWorksheet = "application/vnd.openxmlformats-officedocument.spreadsheetml.worksheet+xml"; @@ -164,7 +164,7 @@ std::vector> workbook_serializer::detect_wor auto &workbook_relationships = wb_.get_relationships(); std::vector> result; - for(const auto &ws : read_sheets(archive)) + for(const auto &ws : read_sheets()) { auto rel = *std::find_if(workbook_relationships.begin(), workbook_relationships.end(), [&](const relationship &r) { return r.get_id() == ws.first; }); auto target = rel.get_target_uri(); diff --git a/tests/helpers/helper.hpp b/tests/helpers/helper.hpp index d2f43f72..a03d72c2 100644 --- a/tests/helpers/helper.hpp +++ b/tests/helpers/helper.hpp @@ -1,7 +1,10 @@ #pragma once #include -#include + +#include +#include +#include #include "path_helper.hpp" @@ -28,39 +31,46 @@ public: operator bool() const { return difference == difference_type::equivalent; } }; + static comparison_result compare_xml(const std::string &expected, const xlnt::xml_document &observed) + { + std::ifstream f(expected); + std::ostringstream s; + f >> s.rdbuf(); + + auto expected_xml = xlnt::xml_serializer::deserialize(s.str()); + + return compare_xml(expected_xml.root(), observed.root()); + } + static comparison_result compare_xml(const std::string &left_contents, const std::string &right_contents) { - pugi::xml_document left_doc; - left_doc.load(left_contents.c_str()); - - pugi::xml_document right_doc; - right_doc.load(right_contents.c_str()); + auto left_doc = xlnt::xml_serializer::deserialize(left_contents); + auto right_doc = xlnt::xml_serializer::deserialize(right_contents); return compare_xml(left_doc.root(), right_doc.root()); } - static comparison_result compare_xml(const pugi::xml_node &left, const pugi::xml_node &right) + static comparison_result compare_xml(const xlnt::xml_node &left, const xlnt::xml_node &right) { - std::string left_temp = left.name(); - std::string right_temp = right.name(); + std::string left_temp = left.get_name(); + std::string right_temp = right.get_name(); if(left_temp != right_temp) { return {difference_type::names_differ, left_temp, right_temp}; } - for(auto left_attribute : left.attributes()) + for(auto &left_attribute : left.get_attributes()) { - left_temp = left_attribute.name(); - auto right_attribute = right.attribute(left_attribute.name()); + left_temp = left_attribute.second; - if(right_attribute == nullptr) + if(!right.has_attribute(left_attribute.first)) { return {difference_type::missing_attribute, left_temp, "((empty))"}; } - left_temp = left_attribute.value(); - right_temp = right_attribute.value(); + left_temp = left_attribute.second; + right_temp = right.get_attribute(left_attribute.first); if(left_temp != right_temp) { @@ -68,34 +78,35 @@ public: } } - if(left.text() != nullptr) + if(left.has_text()) { - left_temp = left.text().as_string(); + left_temp = left.get_text(); - if(right.text() == nullptr) + if(!right.has_text()) { return {difference_type::missing_text, left_temp, "((empty))"}; } - right_temp = right.text().as_string(); + right_temp = right.get_text(); if(left_temp != right_temp) { return {difference_type::text_values_differ, left_temp, right_temp}; } } - else if(right.text() != nullptr) + else if(right.has_text()) { - right_temp = right.text().as_string(); + right_temp = right.get_text(); return {difference_type::text_values_differ, "((empty))", right_temp}; } - auto right_child_iter = right.children().begin(); - for(auto left_child : left.children()) + auto right_child_iter = right.get_children().begin(); + + for(auto left_child : left.get_children()) { - left_temp = left_child.name(); + left_temp = left_child.get_name(); - if(right_child_iter == right.children().end()) + if(right_child_iter == right.get_children().end()) { return {difference_type::child_order_differs, left_temp, "((end))"}; } @@ -103,11 +114,6 @@ public: auto right_child = *right_child_iter; right_child_iter++; - if(left_child.type() == pugi::xml_node_type::node_cdata || left_child.type() == pugi::xml_node_type::node_pcdata) - { - continue; // ignore text children, these have already been compared - } - auto child_comparison_result = compare_xml(left_child, right_child); if(!child_comparison_result) @@ -116,38 +122,12 @@ public: } } - if(right_child_iter != right.children().end()) + if(right_child_iter != right.get_children().end()) { - right_temp = right_child_iter->name(); + right_temp = right_child_iter->get_name(); return {difference_type::child_order_differs, "((end))", right_temp}; } return {difference_type::equivalent, "", ""}; } - - static bool EqualsFileContent(const std::string &reference_file, const std::string &observed_content) - { - std::string expected_content; - - if(PathHelper::FileExists(reference_file)) - { - std::fstream file; - file.open(reference_file); - std::stringstream ss; - ss << file.rdbuf(); - expected_content = ss.str(); - } - else - { - throw std::runtime_error("file not found"); - } - - pugi::xml_document doc_observed; - doc_observed.load(observed_content.c_str()); - - pugi::xml_document doc_expected; - doc_expected.load(expected_content.c_str()); - - return compare_xml(doc_expected, doc_observed); - } }; diff --git a/tests/test_props.hpp b/tests/test_props.hpp index e747c124..b8754c2f 100644 --- a/tests/test_props.hpp +++ b/tests/test_props.hpp @@ -4,6 +4,7 @@ #include #include +#include #include "helpers/path_helper.hpp" #include "helpers/helper.hpp" @@ -17,7 +18,9 @@ public: auto content = archive.read("docProps/core.xml"); xlnt::workbook wb; xlnt::workbook_serializer serializer(wb); - auto prop = serializer.read_properties_core(content); + xlnt::xml_document xml; + serializer.read_properties_core(xml); + auto &prop = wb.get_properties(); TS_ASSERT_EQUALS(prop.creator, "*.*"); TS_ASSERT_EQUALS(prop.last_modified_by, "Charlie Clark"); TS_ASSERT_EQUALS(prop.created, xlnt::datetime(2010, 4, 9, 20, 43, 12)); @@ -32,7 +35,11 @@ public: const std::vector expected_titles = {"Sheet1 - Text", "Sheet2 - Numbers", "Sheet3 - Formulas", "Sheet4 - Dates"}; std::size_t i = 0; - for(auto sheet : xlnt::read_sheets(archive)) + + xlnt::workbook wb; + xlnt::workbook_serializer serializer(wb); + + for(auto sheet : serializer.read_sheets()) { TS_ASSERT_EQUALS(sheet.second, expected_titles.at(i++)); } @@ -42,7 +49,11 @@ public: { xlnt::zip_file archive(PathHelper::GetDataDirectory() + "/genuine/empty_libre.xlsx"); auto content = archive.read("docProps/core.xml"); - auto prop = xlnt::read_properties_core(content); + xlnt::workbook wb; + xlnt::workbook_serializer serializer(wb); + xlnt::xml_document xml; + serializer.read_properties_core(xml); + auto &prop = wb.get_properties(); TS_ASSERT_EQUALS(prop.excel_base_date, xlnt::calendar::windows_1900); } @@ -54,7 +65,11 @@ public: const std::vector expected_titles = {"Sheet1 - Text", "Sheet2 - Numbers", "Sheet3 - Formulas", "Sheet4 - Dates"}; std::size_t i = 0; - for(auto sheet : xlnt::read_sheets(archive)) + + xlnt::workbook wb; + xlnt::workbook_serializer serializer(wb); + + for(auto sheet : serializer.read_sheets()) { TS_ASSERT_EQUALS(sheet.second, expected_titles.at(i++)); } @@ -62,13 +77,15 @@ public: void test_write_properties_core() { - xlnt::document_properties prop; + xlnt::workbook wb; + xlnt::document_properties &prop = wb.get_properties(); prop.creator = "TEST_USER"; prop.last_modified_by = "SOMEBODY"; prop.created = xlnt::datetime(2010, 4, 1, 20, 30, 00); prop.modified = xlnt::datetime(2010, 4, 5, 14, 5, 30); - auto content = xlnt::write_properties_core(prop); - TS_ASSERT(Helper::EqualsFileContent(PathHelper::GetDataDirectory() + "/writer/expected/core.xml", content)); + xlnt::workbook_serializer serializer(wb); + xlnt::xml_document xml = serializer.write_properties_core(); + TS_ASSERT(Helper::compare_xml(PathHelper::GetDataDirectory() + "/writer/expected/core.xml", xml)); } void test_write_properties_app() @@ -76,7 +93,8 @@ public: xlnt::workbook wb; wb.create_sheet(); wb.create_sheet(); - auto content = xlnt::write_properties_app(wb); - TS_ASSERT(Helper::EqualsFileContent(PathHelper::GetDataDirectory() + "/writer/expected/app.xml", content)); + xlnt::workbook_serializer serializer(wb); + xlnt::xml_document xml = serializer.write_properties_app(); + TS_ASSERT(Helper::compare_xml(PathHelper::GetDataDirectory() + "/writer/expected/app.xml", xml)); } }; diff --git a/tests/test_read.hpp b/tests/test_read.hpp index 56a2152a..7ffbbec6 100644 --- a/tests/test_read.hpp +++ b/tests/test_read.hpp @@ -5,6 +5,12 @@ #include +#include +#include +#include +#include +#include + #include "helpers/path_helper.hpp" class test_read : public CxxTest::TestSuite @@ -13,8 +19,11 @@ public: xlnt::workbook standard_workbook() { + xlnt::workbook wb; auto path = PathHelper::GetDataDirectory("/genuine/empty.xlsx"); - return xlnt::excel_reader::load_workbook(path); + wb.load(path); + + return wb; } void test_read_standard_workbook() @@ -26,14 +35,20 @@ public: { auto path = PathHelper::GetDataDirectory("/genuine/empty.xlsx"); std::ifstream fo(path, std::ios::binary); - auto wb = xlnt::excel_reader::load_workbook(path); - TS_ASSERT_DIFFERS(standard_workbook(), nullptr); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_stream_workbook(fo); + + TS_ASSERT_DIFFERS(wb, nullptr); } void test_read_worksheet() { auto wb = standard_workbook(); auto sheet2 = wb.get_sheet_by_name("Sheet2 - Numbers"); + TS_ASSERT_DIFFERS(sheet2, nullptr); TS_ASSERT_EQUALS("This is cell G5", sheet2.get_cell("G5").get_value()); TS_ASSERT_EQUALS(18, sheet2.get_cell("D18").get_value()); @@ -44,32 +59,55 @@ public: void test_read_nostring_workbook() { auto path = PathHelper::GetDataDirectory("/genuine/empty-no-string.xlsx"); - auto wb = xlnt::excel_reader::load_workbook(path); - TS_ASSERT_DIFFERS(standard_workbook(), nullptr); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); + + TS_ASSERT_DIFFERS(wb, nullptr); } void test_read_empty_file() { auto path = PathHelper::GetDataDirectory("/reader/null_file.xlsx"); - TS_ASSERT_THROWS(xlnt::excel_reader::load_workbook(path), xlnt::invalid_file_exception); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + TS_ASSERT_THROWS(serializer.load_workbook(path), xlnt::invalid_file_exception); } void test_read_empty_archive() { auto path = PathHelper::GetDataDirectory("/reader/null_archive.xlsx"); - TS_ASSERT_THROWS(xlnt::excel_reader::load_workbook(path), xlnt::invalid_file_exception); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + TS_ASSERT_THROWS(serializer.load_workbook(path), xlnt::invalid_file_exception); } void test_read_workbook_with_no_properties() { auto path = PathHelper::GetDataDirectory("/genuine/empty_with_no_properties.xlsx"); - xlnt::excel_reader::load_workbook(path); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); } xlnt::workbook workbook_with_styles() { auto path = PathHelper::GetDataDirectory("/genuine/empty-with-styles.xlsx"); - return xlnt::excel_reader::load_workbook(path); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); + + return wb; } void test_read_workbook_with_styles_general() @@ -120,13 +158,25 @@ public: xlnt::workbook date_mac_1904() { auto path = PathHelper::GetDataDirectory("/reader/date_1904.xlsx"); - return xlnt::excel_reader::load_workbook(path); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); + + return wb; } xlnt::workbook date_std_1900() { auto path = PathHelper::GetDataDirectory("/reader/date_1900.xlsx"); - return xlnt::excel_reader::load_workbook(path); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); + + return wb; } void test_read_win_base_date() @@ -169,20 +219,25 @@ public: void test_repair_central_directory() { - std::string data_a = "foobarbaz" + xlnt::excel_reader::CentralDirectorySignature(); + std::string data_a = "foobarbaz" + xlnt::excel_serializer::central_directory_signature(); std::string data_b = "bazbarfoo12345678901234567890"; - auto f = xlnt::excel_reader::repair_central_directory(data_a + data_b); + auto f = xlnt::excel_serializer::repair_central_directory(data_a + data_b); TS_ASSERT_EQUALS(f, data_a + data_b.substr(0, 18)); - f = xlnt::excel_reader::repair_central_directory(data_b); + f = xlnt::excel_serializer::repair_central_directory(data_b); TS_ASSERT_EQUALS(f, data_b); } void test_read_no_theme() { auto path = PathHelper::GetDataDirectory("/genuine/libreoffice_nrt.xlsx"); - auto wb = xlnt::excel_reader::load_workbook(path); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); + TS_ASSERT_DIFFERS(wb, nullptr); } @@ -246,7 +301,13 @@ public: void test_data_only() { auto path = PathHelper::GetDataDirectory("/reader/formulae.xlsx"); - auto wb = xlnt::excel_reader::load_workbook(path, false, true); + + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path, false, true); + auto ws = wb.get_active_sheet(); TS_ASSERT(ws.get_formula_attributes().empty()); @@ -364,18 +425,27 @@ public: }; auto path = PathHelper::GetDataDirectory("/reader/contains_chartsheets.xlsx"); - xlnt::zip_file f(path); - auto result = xlnt::read_content_types(f); - if(result.size() != expected.size()) - { - TS_ASSERT_EQUALS(result.size(), expected.size()); - return; - } + xlnt::workbook wb; + xlnt::manifest_serializer serializer(wb.get_manifest()); + + xlnt::zip_file archive; + archive.load(path); + + serializer.read_manifest(xlnt::xml_serializer::deserialize(archive.read("[Content Types].xml"))); + + auto &result = wb.get_manifest().get_override_types(); + + if(result.size() != expected.size()) + { + TS_ASSERT_EQUALS(result.size(), expected.size()); + return; + } for(std::size_t i = 0; i < expected.size(); i++) { - TS_ASSERT_EQUALS(result[i], expected[i]); + TS_ASSERT_EQUALS(result[i].get_part_name(), expected[i].first); + TS_ASSERT_EQUALS(result[i].get_content_type(), expected[i].second); } } @@ -383,8 +453,11 @@ public: { { auto path = PathHelper::GetDataDirectory("/reader/bug137.xlsx"); - xlnt::zip_file f(path); - auto sheets = xlnt::read_sheets(f); + + xlnt::workbook wb; + xlnt::workbook_serializer serializer(wb); + + auto sheets = serializer.read_sheets(); std::vector> expected = {{"rId1", "Chart1"}, {"rId2", "Sheet1"}}; TS_ASSERT_EQUALS(sheets, expected); @@ -392,8 +465,11 @@ public: { auto path = PathHelper::GetDataDirectory("/reader/bug304.xlsx"); - xlnt::zip_file f(path); - auto sheets = xlnt::read_sheets(f); + + xlnt::workbook wb; + xlnt::workbook_serializer serializer(wb); + + auto sheets = serializer.read_sheets(); std::vector> expected = {{"rId1", "Sheet1"}, {"rId2", "Sheet2"}, {"rId3", "Sheet3"}}; TS_ASSERT_EQUALS(sheets, expected); @@ -410,7 +486,12 @@ public: { std::tie(guess, dtype) = expected; auto path = PathHelper::GetDataDirectory("/genuine/guess_types.xlsx"); - auto wb = xlnt::excel_reader::load_workbook(path, guess); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path, guess); + auto ws = wb.get_active_sheet(); TS_ASSERT(ws.get_cell("D2").get_data_type() == dtype); } @@ -419,7 +500,12 @@ public: void test_read_autofilter() { auto path = PathHelper::GetDataDirectory("/reader/bug275.xlsx"); - auto wb = xlnt::excel_reader::load_workbook(path); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + serializer.load_workbook(path); + auto ws = wb.get_active_sheet(); TS_ASSERT_EQUALS(ws.get_auto_filter().to_string(), "A1:B6"); } @@ -427,18 +513,30 @@ public: void test_bad_formats_xlsb() { auto path = PathHelper::GetDataDirectory("/genuine/a.xlsb"); - TS_ASSERT_THROWS(xlnt::excel_reader::load_workbook(path), xlnt::invalid_file_exception); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + TS_ASSERT_THROWS(serializer.load_workbook(path), xlnt::invalid_file_exception); } void test_bad_formats_xls() { auto path = PathHelper::GetDataDirectory("/genuine/a.xls"); - TS_ASSERT_THROWS(xlnt::excel_reader::load_workbook(path), xlnt::invalid_file_exception); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + TS_ASSERT_THROWS(serializer.load_workbook(path), xlnt::invalid_file_exception); } void test_bad_formats_no() { auto path = PathHelper::GetDataDirectory("/genuine/a.no-format"); - TS_ASSERT_THROWS(xlnt::excel_reader::load_workbook(path), xlnt::invalid_file_exception); + + xlnt::workbook wb; + xlnt::excel_serializer serializer(wb); + + TS_ASSERT_THROWS(serializer.load_workbook(path), xlnt::invalid_file_exception); } }; diff --git a/tests/test_style_writer.hpp b/tests/test_style_writer.hpp index 9423c0b3..c6e357b1 100644 --- a/tests/test_style_writer.hpp +++ b/tests/test_style_writer.hpp @@ -3,7 +3,7 @@ #include #include -#include +#include class test_style_writer : public CxxTest::TestSuite { @@ -12,7 +12,7 @@ public: { xlnt::workbook wb; wb.add_number_format(xlnt::number_format("YYYY")); - xlnt::style_writer writer(wb); + xlnt::style_serializer writer(wb); auto xml = writer.write_number_formats(); std::string expected = "" diff --git a/tests/test_theme.hpp b/tests/test_theme.hpp index ae7b8802..5d24791a 100644 --- a/tests/test_theme.hpp +++ b/tests/test_theme.hpp @@ -3,7 +3,8 @@ #include #include -#include +#include +#include #include "helpers/path_helper.hpp" #include "helpers/helper.hpp" @@ -13,7 +14,9 @@ class test_theme : public CxxTest::TestSuite public: void test_write_theme() { - auto content = xlnt::write_theme(); - TS_ASSERT(Helper::EqualsFileContent(PathHelper::GetDataDirectory() + "/writer/expected/theme1.xml", content)); + xlnt::theme_serializer serializer; + xlnt::workbook wb; + auto content = serializer.write_theme(wb.get_loaded_theme()); + TS_ASSERT(Helper::compare_xml(PathHelper::GetDataDirectory() + "/writer/expected/theme1.xml", content)); } }; diff --git a/tests/test_worksheet.hpp b/tests/test_worksheet.hpp index 4a8e1d55..9a35fc6e 100644 --- a/tests/test_worksheet.hpp +++ b/tests/test_worksheet.hpp @@ -3,7 +3,7 @@ #include #include -#include +#include class test_worksheet : public CxxTest::TestSuite {