fix workbook::operator== always returning false, add tests for workbook::load

workbook::operator== was comparing the value of the raw pointer held by two std::unique_ptr's. By definition, this is always false in a well behaved program (if it's true, things go bang...). This then led to adding equality operators to nearly every other struct/class in xlnt to support workbook::operator==

workbook::load and the non-default ctors for loading data from a file are tested using the now functional equality operator

NOTE: a large number of copy ctors need updates/fixing. Many should be defaulted

include/xlnt/packaging/ext_list.hpp

@@ -46,7 +46,6 @@ class XLNT_API ext_list
 public:
     struct ext
     {
-    public:
         ext(xml::parser &parser, const std::string& ns);
         ext(const uri& ID, const std::string& serialised);
         void serialise(xml::serializer &serialiser, const std::string& ns);
@@ -66,7 +65,14 @@ public:
 
     const std::vector<ext> &extensions() const;
 
+    bool operator==(const ext_list& rhs) const;
 private:
     std::vector<ext> extensions_;
 };
+
+inline bool operator==(const ext_list::ext& lhs, const ext_list::ext& rhs)
+{
+    return lhs.extension_ID_ == rhs.extension_ID_
+        && lhs.serialised_value_ == rhs.serialised_value_;
+}
 } // namespace xlnt

include/xlnt/packaging/manifest.hpp

@@ -172,6 +172,8 @@ public:
     /// </summary>
     void unregister_override_type(const path &part);
 
+    bool operator==(const manifest &other) const;
+
 private:
     /// <summary>
     /// Returns the lowest rId for the given part that hasn't already been registered.

include/xlnt/styles/conditional_format.hpp

@@ -53,6 +53,11 @@ public:
 	static condition text_contains(const std::string &start);
 	static condition text_does_not_contain(const std::string &start);
 
+    bool operator==(const condition& rhs) const
+    {
+        return text_comparand_ == rhs.text_comparand_;
+    }
+
 private:
 	friend class detail::xlsx_producer;
 

include/xlnt/utils/optional.hpp

@@ -125,8 +125,14 @@ public:
     /// </summary>
     bool operator==(const optional<T> &other) const
     {
-        return has_value_ == other.has_value_
-            && (!has_value_ || value_ == other.value_);
+        if (has_value_ != other.has_value_) {
+            return false;
+        }
+        if (!has_value_)
+        {
+            return true;
+        }
+        return value_ == other.value_;
     }
 
 private:

include/xlnt/utils/variant.hpp

@@ -162,6 +162,8 @@ public:
     /// </summary>
     type value_type() const;
 
+    bool operator==(const variant &rhs) const;
+
 private:
     type type_;
     std::vector<variant> vector_value_;

include/xlnt/workbook/calculation_properties.hpp

@@ -46,4 +46,10 @@ public:
     bool concurrent_calc;
 };
 
+inline bool operator==(const calculation_properties &lhs, const calculation_properties &rhs)
+{
+    return lhs.calc_id == rhs.calc_id
+        && lhs.concurrent_calc == rhs.concurrent_calc;
+}
+
 } // namespace xlnt

include/xlnt/workbook/named_range.hpp

@@ -76,6 +76,8 @@ public:
     /// </summary>
     named_range &operator=(const named_range &other);
 
+    bool operator==(const named_range &rhs) const;
+
 private:
     /// <summary>
     /// The name of this named range.

include/xlnt/workbook/theme.hpp

@@ -33,6 +33,11 @@ namespace xlnt {
 /// </summary>
 class XLNT_API theme
 {
+public:
+    bool operator==(const theme&) const
+    {
+        return true;
+    }
 };
 
 } // namespace xlnt

include/xlnt/workbook/workbook_view.hpp

@@ -102,4 +102,17 @@ public:
     optional<int> y_window;
 };
 
+inline bool operator==(const workbook_view &lhs, const workbook_view &rhs)
+{
+    return lhs.active_tab == rhs.active_tab
+        && lhs.auto_filter_date_grouping == rhs.auto_filter_date_grouping
+        && lhs.first_sheet == rhs.first_sheet
+        && lhs.minimized == rhs.minimized
+        && lhs.show_horizontal_scroll == rhs.show_horizontal_scroll
+        && lhs.show_sheet_tabs == rhs.show_sheet_tabs
+        && lhs.show_vertical_scroll == rhs.show_vertical_scroll
+        && lhs.tab_ratio == rhs.tab_ratio
+        && lhs.visible == rhs.visible;
+}
+
 } // namespace xlnt

include/xlnt/worksheet/column_properties.hpp

@@ -63,4 +63,13 @@ public:
     bool hidden = false;
 };
 
+inline bool operator==(const column_properties &lhs, const column_properties &rhs)
+{
+    return lhs.width == rhs.width
+        && lhs.custom_width == rhs.custom_width
+        && lhs.style == rhs.style
+        && lhs.best_fit == rhs.best_fit
+        && lhs.hidden == rhs.hidden;
+}
+
 } // namespace xlnt

include/xlnt/worksheet/header_footer.hpp

@@ -312,6 +312,8 @@ public:
     /// </summary>
     rich_text even_footer(location where) const;
 
+    bool operator==(const header_footer &rhs) const;
+
 private:
     bool align_with_margins_ = false;
     bool different_odd_even_ = false;

include/xlnt/worksheet/page_margins.hpp

@@ -99,6 +99,8 @@ public:
     /// </summary>
     void footer(double footer);
 
+    bool operator==(const page_margins &rhs) const;
+
 private:
     /// <summary>
     /// The top margin

include/xlnt/worksheet/page_setup.hpp

@@ -172,6 +172,8 @@ public:
     /// </summary>
     xlnt::optional<std::size_t> vertical_dpi_;
 
+    bool operator==(const page_setup &rhs) const;
+
 private:
     /// <summary>
     /// The break

include/xlnt/worksheet/phonetic_pr.hpp

@@ -143,6 +143,8 @@ public:
     /// </summary>
     static align alignment_from_string(const std::string &str);
 
+    bool operator==(const phonetic_pr &rhs) const;
+
 private:
     /// <summary>
     /// zero based index into style sheet font record.

include/xlnt/worksheet/print_options.hpp

@@ -55,4 +55,13 @@ struct XLNT_API print_options
     /// </summary>
     optional<bool> vertical_centered;
 };
+
+inline bool operator==(const print_options& lhs, const print_options &rhs)
+{
+    return lhs.grid_lines_set == rhs.grid_lines_set
+        && lhs.horizontal_centered == rhs.horizontal_centered
+        && lhs.print_grid_lines == rhs.print_grid_lines
+        && lhs.print_headings == rhs.print_headings
+        && lhs.vertical_centered == rhs.vertical_centered;
+}
 } // namespace xlnt

include/xlnt/worksheet/row_properties.hpp

@@ -65,4 +65,14 @@ public:
     optional<std::size_t> style;
 };
 
+inline bool operator==(const row_properties &lhs, const row_properties &rhs)
+{
+    return lhs.height == rhs.height
+        && lhs.dy_descent == rhs.dy_descent
+        && lhs.custom_height == rhs.custom_height
+        && lhs.hidden == rhs.hidden
+        && lhs.custom_format == rhs.custom_format
+        && lhs.style == rhs.style;
+}
+
 } // namespace xlnt

include/xlnt/worksheet/sheet_format_properties.hpp

@@ -55,4 +55,12 @@ public:
     optional<double> dy_descent;
 };
 
+inline bool operator==(const sheet_format_properties &lhs, const sheet_format_properties &rhs)
+{
+    return lhs.base_col_width == rhs.base_col_width
+        && lhs.default_column_width == rhs.default_column_width
+        && lhs.default_row_height == rhs.default_row_height
+        && lhs.dy_descent == rhs.dy_descent;
+}
+
 } // namespace xlnt

include/xlnt/worksheet/sheet_pr.hpp

@@ -78,4 +78,17 @@ struct XLNT_API sheet_pr
     /// </summary>
     optional<bool> enable_format_condition_calculation;
 };
+
+inline bool operator==(const sheet_pr &lhs, const sheet_pr &rhs)
+{
+    return lhs.sync_horizontal == rhs.sync_horizontal
+        && lhs.sync_vertical == rhs.sync_vertical
+        && lhs.sync_ref == rhs.sync_ref
+        && lhs.transition_evaluation == rhs.transition_evaluation
+        && lhs.transition_entry == rhs.transition_entry
+        && lhs.published == rhs.published
+        && lhs.code_name == rhs.code_name
+        && lhs.filter_mode == rhs.filter_mode
+        && lhs.enable_format_condition_calculation == rhs.enable_format_condition_calculation;
+}
 } // namespace xlnt

source/detail/implementations/cell_impl.hpp

@@ -61,5 +61,21 @@ struct cell_impl
     optional<comment *> comment_;
 };
 
+inline bool operator==(const cell_impl &lhs, const cell_impl &rhs)
+{
+    // not comparing parent
+    return lhs.type_ == rhs.type_
+        && lhs.column_ == rhs.column_
+        && lhs.row_ == rhs.row_
+        && lhs.is_merged_ == rhs.is_merged_
+        && lhs.value_text_ == rhs.value_text_
+        && lhs.value_numeric_ == rhs.value_numeric_
+        && lhs.formula_ == rhs.formula_
+        && lhs.hyperlink_ == rhs.hyperlink_
+        // comparing pointers is unlikely to be what is wanted
+        /*&& lhs.format_ == rhs.format_
+        && lhs.comment_ == rhs.comment_*/;
+}
+
 } // namespace detail
 } // namespace xlnt

source/detail/implementations/conditional_format_impl.hpp

@@ -20,9 +20,22 @@ struct worksheet_impl;
 struct conditional_format_impl
 {
 	stylesheet *parent;
-	range_reference target_range;
     worksheet_impl *target_sheet;
 
+    bool operator==(const conditional_format_impl& rhs) const
+    {
+        // not comparing parent or target sheet
+        return target_range == rhs.target_range
+            && priority == rhs.priority
+            && differential_format_id == rhs.differential_format_id
+            && when == rhs.when
+            && border_id == rhs.border_id
+            && fill_id == rhs.fill_id
+            && font_id == rhs.font_id;
+    }
+
+	range_reference target_range;
+
 	std::size_t priority;
 	std::size_t differential_format_id;
 

source/detail/implementations/hyperlink_impl.hpp

@@ -37,5 +37,12 @@ struct hyperlink_impl
     std::string display;
 };
 
+inline bool operator==(const hyperlink_impl &lhs, const hyperlink_impl &rhs)
+{
+    return lhs.relationship == rhs.relationship
+        && lhs.tooltip == rhs.tooltip
+        && lhs.display == rhs.display;
+}
+
 } // namespace detail
 } // namespace xlnt

source/detail/implementations/style_impl.hpp

@@ -22,6 +22,30 @@ struct style_impl
 {
 	stylesheet *parent;
 
+    bool operator==(const style_impl& rhs) const
+    {
+        return name == rhs.name
+            && formatting_record_id == rhs.formatting_record_id
+            && custom_builtin == rhs.custom_builtin
+            && hidden_style == rhs.hidden_style
+            && builtin_id == rhs.builtin_id
+            && outline_style == rhs.outline_style
+            && alignment_id == rhs.alignment_id
+            && alignment_applied == rhs.alignment_applied
+            && border_id == rhs.border_id
+            && border_applied == rhs.border_applied
+            && fill_id == rhs.fill_id
+            && fill_applied == rhs.fill_applied
+            && font_id == rhs.font_id
+            && font_applied == rhs.font_applied
+            && number_format_id == rhs.number_format_id
+            && number_format_applied == number_format_applied
+            && protection_id == rhs.protection_id
+            && protection_applied == rhs.protection_applied
+            && pivot_button_ == rhs.pivot_button_
+            && quote_prefix_ == rhs.quote_prefix_;
+    }
+
 	std::string name;
 	std::size_t formatting_record_id;
 

source/detail/implementations/stylesheet.hpp

@@ -528,6 +528,25 @@ struct stylesheet
 
     workbook *parent;
 
+    bool operator==(const stylesheet& rhs) const
+    {
+        // no equality on parent as there is only 1 stylesheet per borkbook hence would always be false
+        return garbage_collection_enabled == rhs.garbage_collection_enabled
+            && known_fonts_enabled == rhs.known_fonts_enabled
+            && conditional_format_impls == rhs.conditional_format_impls
+            && format_impls == rhs.format_impls
+            && style_impls == rhs.style_impls
+            && style_names == rhs.style_names
+            && default_slicer_style == rhs.default_slicer_style
+            && alignments == rhs.alignments
+            && borders == rhs.borders
+            && fills == rhs.fills
+            && fonts == rhs.fonts
+            && number_formats == rhs.number_formats
+            && protections == rhs.protections
+            && colors == rhs.colors;
+    }
+    
     bool garbage_collection_enabled = true;
     bool known_fonts_enabled = false;
 

source/detail/implementations/workbook_impl.hpp

@@ -89,6 +89,30 @@ struct workbook_impl
         return *this;
     }
 
+    bool operator==(const workbook_impl &other)
+    {
+        return active_sheet_index_ == other.active_sheet_index_
+            && worksheets_ == other.worksheets_
+            && shared_strings_ == other.shared_strings_
+            && stylesheet_ == other.stylesheet_
+            && base_date_ == other.base_date_
+            && title_ == other.title_
+            && manifest_ == other.manifest_
+            && theme_ == other.theme_
+            && images_ == other.images_
+            && core_properties_ == other.core_properties_
+            && extended_properties_ == other.extended_properties_
+            && custom_properties_ == other.custom_properties_
+            && sheet_title_rel_id_map_ == other.sheet_title_rel_id_map_
+            && view_ == other.view_
+            && code_name_ == other.code_name_
+            && file_version_ == other.file_version_
+            && calculation_properties_ == other.calculation_properties_
+            && abs_path_ == other.abs_path_
+            && arch_id_flags_ == other.arch_id_flags_
+            && extensions_ == other.extensions_;
+    }
+
     optional<std::size_t> active_sheet_index_;
 
     std::list<worksheet_impl> worksheets_;
@@ -118,6 +142,14 @@ struct workbook_impl
 		std::size_t last_edited;
 		std::size_t lowest_edited;
 		std::size_t rup_build;
+
+        bool operator==(const file_version_t& rhs) const
+        {
+            return app_name == rhs.app_name
+                && last_edited == rhs.last_edited
+                && lowest_edited == rhs.lowest_edited
+                && rup_build == rhs.rup_build;
+        }
 	};
     
     optional<file_version_t> file_version_;

source/detail/implementations/worksheet_impl.hpp

@@ -99,6 +99,33 @@ struct worksheet_impl
 
     workbook *parent_;
 
+    bool operator==(const worksheet_impl& rhs) const
+    {
+        return id_ == rhs.id_
+            && title_ == rhs.title_
+            && format_properties_ == rhs.format_properties_
+            && column_properties_ == rhs.column_properties_
+            && row_properties_ == rhs.row_properties_
+            && cell_map_ == rhs.cell_map_
+            && page_setup_ == rhs.page_setup_
+            && auto_filter_ == rhs.auto_filter_
+            && page_margins_ == rhs.page_margins_
+            && merged_cells_ == rhs.merged_cells_
+            && named_ranges_ == rhs.named_ranges_
+            && phonetic_properties_ == rhs.phonetic_properties_
+            && header_footer_ == rhs.header_footer_
+            && print_title_cols_ == rhs.print_title_cols_
+            && print_title_rows_ == rhs.print_title_rows_
+            && print_area_ == rhs.print_area_
+            && views_ == rhs.views_
+            && column_breaks_ == rhs.column_breaks_
+            && row_breaks_ == rhs.row_breaks_
+            && comments_ == rhs.comments_
+            && print_options_ == rhs.print_options_
+            && sheet_properties_ == rhs.sheet_properties_
+            && extension_list_ == rhs.extension_list_;
+    }
+
     std::size_t id_;
     std::string title_;
 

source/packaging/ext_list.cpp

@@ -137,4 +137,9 @@ const std::vector<ext_list::ext> &ext_list::extensions() const
     return extensions_;
 }
 
+bool ext_list::operator==(const ext_list &rhs) const
+{
+    return std::equal(extensions_.begin(), extensions_.end(), rhs.extensions_.begin(), rhs.extensions_.end());
+}
+
 }

source/packaging/manifest.cpp

@@ -337,4 +337,11 @@ std::string manifest::override_type(const xlnt::path &part) const
     return override_content_types_.at(part);
 }
 
+bool manifest::operator==(const manifest &other) const
+{
+    return default_content_types_ == other.default_content_types_
+        && override_content_types_ == other.override_content_types_
+        && relationships_ == other.relationships_;
+}
+
 } // namespace xlnt

source/utils/variant.cpp

@@ -41,7 +41,7 @@ variant::variant(const char *value) : variant(std::string(value))
 {
 }
 
-variant::variant(int value)
+variant::variant(std::int32_t value)
     : type_(type::i4),
     i4_value_(value)
 {
@@ -125,6 +125,28 @@ variant::variant(const std::vector<variant> &value)
     }
 }
 
+bool variant::operator==(const variant &rhs) const
+{
+    if (type_ != rhs.type_)
+    {
+        return false;
+    }
+    switch (type_)
+    {
+    case type::vector:
+        return vector_value_ == rhs.vector_value_;
+    case type::i4:
+    case type::boolean:
+        return i4_value_ == rhs.i4_value_;
+    case type::date:
+    case type::lpstr:
+        return lpstr_value_ == rhs.lpstr_value_;
+    case type::null:
+        return true;
+    }
+    return false;
+}
+
 bool variant::is(type t) const
 {
     return type_ == t;

source/workbook/named_range.cpp

@@ -123,4 +123,10 @@ named_range &named_range::operator=(const named_range &other)
     return *this;
 }
 
+bool named_range::operator==(const named_range &rhs) const
+{
+    return name_ == rhs.name_
+        && targets_ == rhs.targets_;
+}
+
 } // namespace xlnt

source/workbook/workbook.cpp

@@ -1133,12 +1133,12 @@ void workbook::clear()
 
 bool workbook::operator==(const workbook &rhs) const
 {
-    return d_.get() == rhs.d_.get();
+    return *d_ == *rhs.d_;
 }
 
 bool workbook::operator!=(const workbook &rhs) const
 {
-    return d_.get() != rhs.d_.get();
+    return !operator==(rhs);
 }
 
 void workbook::swap(workbook &right)

source/worksheet/header_footer.cpp

@@ -295,4 +295,17 @@ rich_text header_footer::even_footer(location where) const
     return even_footers_.at(where);
 }
 
+bool header_footer::operator==(const header_footer &rhs) const
+{
+    return align_with_margins_ == rhs.align_with_margins_
+        && different_odd_even_ == rhs.different_odd_even_
+        && scale_with_doc_ == rhs.scale_with_doc_
+        && odd_headers_ == rhs.odd_headers_
+        && even_headers_ == rhs.even_headers_
+        && first_headers_ == rhs.first_headers_
+        && odd_footers_ == rhs.odd_footers_
+        && even_footers_ == rhs.even_footers_
+        && first_footers_ == rhs.first_footers_;
+}
+
 } // namespace xlnt

source/worksheet/page_margins.cpp

@@ -89,4 +89,13 @@ void page_margins::footer(double footer)
     footer_ = footer;
 }
 
+bool page_margins::operator==(const page_margins &rhs) const
+{
+    return top_ == rhs.top_
+        && left_ == rhs.left_
+        && right_ == rhs.right_
+        && header_ == rhs.header_
+        && footer_ == rhs.footer_;
+}
+
 } // namespace xlnt

source/worksheet/page_setup.cpp

@@ -106,4 +106,15 @@ double page_setup::scale() const
     return scale_;
 }
 
+bool page_setup::operator==(const page_setup &rhs) const
+{
+    return break_ == rhs.break_
+        && sheet_state_ == rhs.sheet_state_
+        && paper_size_ == rhs.paper_size_
+        && fit_to_page_ == rhs.fit_to_page_
+        && fit_to_height_ == rhs.fit_to_height_
+        && fit_to_width_ == rhs.fit_to_width_
+        && scale_ == rhs.scale_;
+}
+
 } // namespace xlnt

source/worksheet/phonetic_pr.cpp

@@ -140,4 +140,11 @@ phonetic_pr::align phonetic_pr::alignment_from_string(const std::string &str)
     return align::no_control;
 }
 
+bool phonetic_pr::operator==(const phonetic_pr &rhs) const
+{
+    return font_id_ == rhs.font_id_
+        && type_ == rhs.type_
+        && alignment_ == rhs.alignment_;
+}
+
 } // namespace xlnt

tests/workbook/workbook_test_suite.hpp

@@ -26,9 +26,10 @@
 #include <algorithm>
 #include <iostream>
 
+#include <xlnt/xlnt.hpp>
+#include <detail/serialization/open_stream.hpp>
 #include <helpers/temporary_file.hpp>
 #include <helpers/test_suite.hpp>
-#include <xlnt/xlnt.hpp>
 
 class workbook_test_suite : public test_suite
 {
@@ -57,6 +58,7 @@ public:
         register_test(test_clear);
         register_test(test_comparison);
         register_test(test_id_gen);
+        register_test(test_load_file);
     }
 
     void test_active_sheet()
@@ -339,4 +341,39 @@ public:
         wb.create_sheet();
         xlnt_assert_differs(wb[1].id(), wb[2].id());
     }
+
+    void test_load_file()
+    {
+        xlnt::path file = path_helper::test_file("2_minimal.xlsx");
+        xlnt::workbook wb_path(file);
+        // ctor from ifstream
+        std::ifstream file_reader(file.string(), std::ios::binary);
+        xlnt_assert_equals(wb_path, xlnt::workbook(file_reader));
+        // load with string
+        xlnt::workbook wb_load1;
+        xlnt_assert_differs(wb_path, wb_load1);
+        wb_load1.load(file.string());
+        xlnt_assert_equals(wb_path, wb_load1);
+        // load with wstring
+        xlnt::workbook wb_load2;
+        wb_load2.load(file.string());
+        xlnt_assert_equals(wb_path, wb_load2);
+        // load with path
+        xlnt::workbook wb_load3;
+        wb_load3.load(file);
+        xlnt_assert_equals(wb_path, wb_load3);
+        // load with istream
+        xlnt::workbook wb_load4;
+        std::ifstream file_reader2(file.string(), std::ios::binary);
+        wb_load4.load(file_reader2);
+        xlnt_assert_equals(wb_path, wb_load4);
+        // load with vector
+        std::ifstream file_reader3(file.string(), std::ios::binary);
+        file_reader3.unsetf(std::ios::skipws);
+        std::vector<uint8_t> data(std::istream_iterator<uint8_t>{file_reader3}, 
+                                  std::istream_iterator<uint8_t>());
+        xlnt::workbook wb_load5;
+        wb_load5.load(data);
+        xlnt_assert_equals(wb_path, wb_load5);
+    }
 };