From ca6dea8cbc600851a8a30cb1f624f2b502e8314e Mon Sep 17 00:00:00 2001
From: Thomas Fussell <thomas.fussell@gmail.com>
Date: Sun, 6 Mar 2016 10:39:50 +0800
Subject: [PATCH] update workbook tests

---
 include/xlnt/utils/exceptions.hpp       |   1 +
 include/xlnt/utils/key_error.hpp        |  41 ++++++
 include/xlnt/workbook/workbook.hpp      |   3 +
 include/xlnt/worksheet/worksheet.hpp    |   2 +
 source/detail/workbook_impl.hpp         |   3 +
 source/utils/exceptions.cpp             |  10 ++
 source/workbook/tests/test_workbook.hpp | 176 +++++++++++-------------
 source/workbook/workbook.cpp            |  38 +++--
 source/worksheet/worksheet.cpp          |  48 ++++++-
 9 files changed, 213 insertions(+), 109 deletions(-)
 create mode 100644 include/xlnt/utils/key_error.hpp

diff --git a/include/xlnt/utils/exceptions.hpp b/include/xlnt/utils/exceptions.hpp
index 25303da9..bff33038 100644
--- a/include/xlnt/utils/exceptions.hpp
+++ b/include/xlnt/utils/exceptions.hpp
@@ -31,6 +31,7 @@
 #include <xlnt/utils/data_type_exception.hpp>
 #include <xlnt/utils/illegal_character_error.hpp>
 #include <xlnt/utils/invalid_file_exception.hpp>
+#include <xlnt/utils/key_error.hpp>
 #include <xlnt/utils/missing_number_format.hpp>
 #include <xlnt/utils/named_range_exception.hpp>
 #include <xlnt/utils/read_only_workbook_exception.hpp>
diff --git a/include/xlnt/utils/key_error.hpp b/include/xlnt/utils/key_error.hpp
new file mode 100644
index 00000000..a757a5a2
--- /dev/null
+++ b/include/xlnt/utils/key_error.hpp
@@ -0,0 +1,41 @@
+// Copyright (c) 2014-2016 Thomas Fussell
+// Copyright (c) 2010-2015 openpyxl
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, WRISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE
+//
+// @license: http://www.opensource.org/licenses/mit-license.php
+// @author: see AUTHORS file
+#pragma once
+
+#include <stdexcept>
+
+#include <xlnt/xlnt_config.hpp>
+
+namespace xlnt {
+
+/// <summary>
+/// key_error
+/// </summary>
+class XLNT_CLASS key_error : public std::runtime_error
+{
+public:
+    key_error();
+};
+
+} // namespace xlnt
diff --git a/include/xlnt/workbook/workbook.hpp b/include/xlnt/workbook/workbook.hpp
index 9b5fde92..9ba429ca 100644
--- a/include/xlnt/workbook/workbook.hpp
+++ b/include/xlnt/workbook/workbook.hpp
@@ -92,6 +92,9 @@ public:
 
     bool get_data_only() const;
     void set_data_only(bool data_only);
+    
+    bool get_read_only() const;
+    void set_read_only(bool read_only);
 
     // create
     worksheet create_sheet();
diff --git a/include/xlnt/worksheet/worksheet.hpp b/include/xlnt/worksheet/worksheet.hpp
index 2024cdf4..3761831a 100644
--- a/include/xlnt/worksheet/worksheet.hpp
+++ b/include/xlnt/worksheet/worksheet.hpp
@@ -175,6 +175,8 @@ public:
     const range operator[](const std::string &range_string) const;
     range operator()(const cell_reference &top_left, const cell_reference &bottom_right);
     const range operator()(const cell_reference &top_left, const cell_reference &bottom_right) const;
+    
+    bool compare(const worksheet &other, bool reference) const;
 
     // page
     page_setup &get_page_setup();
diff --git a/source/detail/workbook_impl.hpp b/source/detail/workbook_impl.hpp
index af14665c..5f732f79 100644
--- a/source/detail/workbook_impl.hpp
+++ b/source/detail/workbook_impl.hpp
@@ -42,6 +42,7 @@ struct workbook_impl
           properties_(other.properties_),
           guess_types_(other.guess_types_),
           data_only_(other.data_only_),
+          read_only_(other.read_only_),
           styles_(other.styles_),
           colors_(other.colors_),
           borders_(other.borders_),
@@ -69,6 +70,7 @@ struct workbook_impl
         properties_ = other.properties_;
         guess_types_ = other.guess_types_;
         data_only_ = other.data_only_;
+        read_only_ = other.read_only_;
         styles_ = other.styles_;
         borders_ = other.borders_;
         fills_ = other.fills_;
@@ -91,6 +93,7 @@ struct workbook_impl
 
     bool guess_types_;
     bool data_only_;
+    bool read_only_;
 
     std::vector<style> styles_;
 
diff --git a/source/utils/exceptions.cpp b/source/utils/exceptions.cpp
index 0b2452ef..4f802e2c 100644
--- a/source/utils/exceptions.cpp
+++ b/source/utils/exceptions.cpp
@@ -86,4 +86,14 @@ value_error::value_error()
 {
 }
 
+key_error::key_error()
+    : std::runtime_error("key error")
+{
+}
+
+read_only_workbook_exception::read_only_workbook_exception()
+    : std::runtime_error("workbook is read-only")
+{
+}
+
 } // namespace xlnt
diff --git a/source/workbook/tests/test_workbook.hpp b/source/workbook/tests/test_workbook.hpp
index 6cb91c03..878475c4 100644
--- a/source/workbook/tests/test_workbook.hpp
+++ b/source/workbook/tests/test_workbook.hpp
@@ -7,40 +7,63 @@
 #include <xlnt/xlnt.hpp>
 #include "helpers/temporary_file.hpp"
 
+//checked with 52f25d6
+
 class test_workbook : public CxxTest::TestSuite
 {
 public:
     void test_get_active_sheet()
     {
         xlnt::workbook wb;
-        auto active_sheet = wb.get_active_sheet();
-        TS_ASSERT_EQUALS(active_sheet, wb[0]);
+        TS_ASSERT_EQUALS(wb.get_active_sheet(), wb[0]);
     }
 
     void test_create_sheet()
     {
         xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet(0);
-        TS_ASSERT_EQUALS(new_sheet, wb[0]);
+        auto new_sheet = wb.create_sheet();
+        auto last = std::distance(wb.begin(), wb.end()) - 1;
+        TS_ASSERT_EQUALS(new_sheet, wb[last]);
     }
 
     void test_create_sheet_with_name()
     {
         xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet(0, "LikeThisName");
-        TS_ASSERT_EQUALS(new_sheet, wb[0]);
+        auto new_sheet = wb.create_sheet("LikeThisName");
+        auto last = std::distance(wb.begin(), wb.end()) - 1;
+        TS_ASSERT_EQUALS(new_sheet, wb[last]);
     }
+    
+    void test_add_correct_sheet()
+    {
+        xlnt::workbook wb;
+        auto new_sheet = wb.create_sheet();
+        wb.add_sheet(new_sheet);
+        TS_ASSERT(new_sheet.compare(wb[2], false));
+    }
+    
+    // void test_add_sheetname() {} unnecessary
 
+    void test_add_sheet_from_other_workbook()
+    {
+        xlnt::workbook wb1, wb2;
+        auto new_sheet = wb1.get_active_sheet();
+        TS_ASSERT_THROWS(wb2.add_sheet(new_sheet), xlnt::value_error);
+    }
+    
+    void test_create_sheet_readonly()
+    {
+        xlnt::workbook wb;
+        wb.set_read_only(true);
+        TS_ASSERT_THROWS(wb.create_sheet(), xlnt::read_only_workbook_exception);
+    }
+    
     void test_remove_sheet()
     {
         xlnt::workbook wb;
         auto new_sheet = wb.create_sheet(0);
-        auto sheet_name = new_sheet.get_title();
         wb.remove_sheet(new_sheet);
-        for(auto worksheet : wb)
-        {
-            TS_ASSERT_DIFFERS(sheet_name, worksheet.get_title());
-        }
+        TS_ASSERT(std::find(wb.begin(), wb.end(), new_sheet) == wb.end());
     }
 
     void test_get_sheet_by_name()
@@ -53,19 +76,33 @@ public:
         TS_ASSERT_EQUALS(new_sheet, found_sheet);
     }
     
-    void test_getitem()
+    void test_index_operator() // test_getitem
     {
         xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet();
-        std::string title = "my sheet";
-        new_sheet.set_title(title);
-        auto found_sheet = wb.get_sheet_by_name(title);
-        TS_ASSERT_EQUALS(new_sheet, found_sheet);
-        
-        TS_ASSERT_EQUALS(wb.get_sheet_by_name("NotThere"), nullptr);
+        TS_ASSERT_THROWS_NOTHING(wb["Sheet"]);
+        TS_ASSERT_THROWS(wb["NotThere"], xlnt::key_error);
     }
-
-    void test_get_index2()
+    
+    // void test_delitem() {} doesn't make sense in c++
+    
+    void test_contains()
+    {
+        xlnt::workbook wb;
+        TS_ASSERT(wb.contains("Sheet"));
+        TS_ASSERT(!wb.contains("NotThere"));
+    }
+    
+    void test_iter()
+    {
+        xlnt::workbook wb;
+        
+        for(auto ws : wb)
+        {
+            TS_ASSERT_EQUALS(ws.get_title(), "Sheet");
+        }
+    }
+    
+    void test_get_index()
     {
         xlnt::workbook wb;
         auto new_sheet = wb.create_sheet(0);
@@ -78,14 +115,15 @@ public:
         xlnt::workbook wb;
         wb.clear();
         
-        std::vector<std::string> names = {"NewSheet", "NewSheet1", "NewSheet2", "NewSheet3", "NewSheet4", "NewSheet5"};
+        std::vector<std::string> names = {"Sheet", "Sheet1", "Sheet2", "Sheet3", "Sheet4", "Sheet5"};
         
         for(std::size_t count = 0; count < names.size(); count++)
         {
-            wb.create_sheet(names[count]);
+            wb.create_sheet();
         }
         
         auto actual_names = wb.get_sheet_names();
+        
         std::sort(actual_names.begin(), actual_names.end());
         std::sort(names.begin(), names.end());
         
@@ -94,46 +132,7 @@ public:
             TS_ASSERT_EQUALS(actual_names[count], names[count]);
         }
     }
-
-    void test_get_active_sheet2()
-    {
-        xlnt::workbook wb;
-        auto active_sheet = wb.get_active_sheet();
-        TS_ASSERT_EQUALS(active_sheet, wb[0]);
-    }
-
-    void test_create_sheet2()
-    {
-        xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet(0);
-        TS_ASSERT_EQUALS(new_sheet, wb[0]);
-    }
-
-    void test_create_sheet_with_name2()
-    {
-        xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet(0, "LikeThisName");
-        TS_ASSERT_EQUALS(new_sheet, wb[0]);
-    }
-
-    void test_get_sheet_by_name2()
-    {
-        xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet();
-        std::string title = "my sheet";
-        new_sheet.set_title(title);
-        auto found_sheet = wb.get_sheet_by_name(title);
-        TS_ASSERT_EQUALS(new_sheet, found_sheet);
-    }
-
-    void test_get_index()
-    {
-        xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet(0);
-        auto sheet_index = wb.get_index(new_sheet);
-        TS_ASSERT_EQUALS(sheet_index, 0);
-    }
-
+    
     void test_add_named_range()
     {
         xlnt::workbook wb;
@@ -142,8 +141,8 @@ public:
         TS_ASSERT(new_sheet.has_named_range("test_nr"));
         TS_ASSERT(wb.has_named_range("test_nr"));
     }
-
-    void test_get_named_range2()
+    
+    void test_get_named_range()
     {
         xlnt::workbook wb;
         auto new_sheet = wb.create_sheet();
@@ -163,54 +162,39 @@ public:
         TS_ASSERT(!wb.has_named_range("test_nr"));
     }
 
-    void test_add_local_named_range()
-    {
-        TemporaryFile temp_file;
-        xlnt::workbook wb;
-        auto new_sheet = wb.create_sheet();
-        wb.create_named_range("test_nr", new_sheet, "A1");
-        auto dest_filename = temp_file.GetFilename();
-        wb.save(dest_filename);
-    }
-
     void test_write_regular_date()
     {
-        /*make_tmpdir();
-        auto today = datetime.datetime(2010, 1, 18, 14, 15, 20, 1600);
-
-        xlnt::workbook wb;
+        const xlnt::datetime today(2010, 1, 18, 14, 15, 20, 1600);
+        
+        xlnt::workbook book;
         auto sheet = book.get_active_sheet();
-        sheet.cell("A1") = today;
-        dest_filename = osp.join(TMPDIR, "date_read_write_issue.xlsx");
-        book.save(dest_filename);
+        sheet.get_cell("A1").set_value(today);
+        TemporaryFile temp_file;
+        book.save(temp_file.GetFilename());
 
-        test_book = load_workbook(dest_filename);
-        test_sheet = test_book.get_active_sheet();
+        xlnt::workbook test_book;
+        test_book.load(temp_file.GetFilename());
+        auto test_sheet = test_book.get_active_sheet();
 
-        TS_ASSERT_EQUALS(test_sheet.cell("A1"), today);
-        clean_tmpdir();*/
+        TS_ASSERT_EQUALS(test_sheet.get_cell("A1").get_value<xlnt::datetime>(), today);
     }
 
     void test_write_regular_float()
     {
-        TemporaryFile temp_file;
         long double float_value = 1.0L / 3.0L;
+        
         xlnt::workbook book;
         auto sheet = book.get_active_sheet();
         sheet.get_cell("A1").set_value(float_value);
-        auto dest_filename = temp_file.GetFilename();
-        book.save(dest_filename);
+        TemporaryFile temp_file;
+        book.save(temp_file.GetFilename());
 
         xlnt::workbook test_book;
-        test_book.load(dest_filename);
+        test_book.load(temp_file.GetFilename());
         auto test_sheet = test_book.get_active_sheet();
 
         TS_ASSERT_EQUALS(test_sheet.get_cell("A1").get_value<long double>(), float_value);
     }
-
-    void test_read_empty_shared_strings()
-    {
-	xlnt::workbook test_ss;
-	TS_ASSERT_THROWS_NOTHING(test_ss.load(PathHelper::GetDataDirectory("/genuine/number_empty_shared_strings.xlsx")));
-    }
+    
+    // void test_add_invalid_worksheet_class_instance() {} not needed in c++
 };
diff --git a/source/workbook/workbook.cpp b/source/workbook/workbook.cpp
index 4378f8ac..b333e56b 100644
--- a/source/workbook/workbook.cpp
+++ b/source/workbook/workbook.cpp
@@ -59,7 +59,7 @@ namespace xlnt {
 namespace detail {
 
 workbook_impl::workbook_impl()
-    : active_sheet_index_(0), guess_types_(false), data_only_(false), next_custom_format_id_(164)
+    : active_sheet_index_(0), guess_types_(false), data_only_(false), read_only_(false), next_custom_format_id_(164)
 {
 }
 
@@ -135,8 +135,10 @@ bool workbook::has_named_range(const std::string &name) const
 
 worksheet workbook::create_sheet()
 {
-    std::string title = "Sheet1";
-    int index = 1;
+    if(get_read_only()) throw xlnt::read_only_workbook_exception();
+    
+    std::string title = "Sheet";
+    int index = 0;
 
     while (get_sheet_by_name(title) != nullptr)
     {
@@ -157,14 +159,7 @@ worksheet workbook::create_sheet()
 
 void workbook::add_sheet(xlnt::worksheet worksheet)
 {
-    for (auto ws : *this)
-    {
-        if (worksheet == ws)
-        {
-            throw std::runtime_error("worksheet already in workbook");
-        }
-    }
-
+    if(worksheet.d_->parent_ != this) throw xlnt::value_error();
     d_->worksheets_.emplace_back(*worksheet.d_);
 }
 
@@ -443,6 +438,7 @@ std::vector<std::string> workbook::get_sheet_names() const
 
 worksheet workbook::operator[](const std::string &name)
 {
+    if(!contains(name)) throw xlnt::key_error();
     return get_sheet_by_name(name);
 }
 
@@ -548,6 +544,16 @@ void workbook::set_data_only(bool data_only)
     d_->data_only_ = data_only;
 }
 
+bool workbook::get_read_only() const
+{
+    return d_->read_only_;
+}
+
+void workbook::set_read_only(bool read_only)
+{
+    d_->read_only_ = read_only;
+}
+
 void workbook::add_border(const xlnt::border &border_)
 {
     d_->borders_.push_back(border_);
@@ -955,4 +961,14 @@ void workbook::add_shared_string(const std::string &shared)
     d_->shared_strings_.push_back(shared);
 }
 
+bool workbook::contains(const std::string &sheet_title) const
+{
+    for(auto ws : *this)
+    {
+        if(ws.get_title() == sheet_title) return true;
+    }
+    
+    return false;
+}
+
 } // namespace xlnt
diff --git a/source/worksheet/worksheet.cpp b/source/worksheet/worksheet.cpp
index 2eb9b216..7b9da4e5 100644
--- a/source/worksheet/worksheet.cpp
+++ b/source/worksheet/worksheet.cpp
@@ -586,12 +586,56 @@ xlnt::range worksheet::columns() const
 
 bool worksheet::operator==(const worksheet &other) const
 {
-    return d_ == other.d_;
+    return compare(other, true);
+}
+
+bool worksheet::compare(const worksheet &other, bool reference) const
+{
+    if(reference)
+    {
+        return d_ == other.d_;
+    }
+    
+    if(d_->parent_ != other.d_->parent_) return false;
+    
+    for(auto &row : d_->cell_map_)
+    {
+        if(other.d_->cell_map_.find(row.first) == other.d_->cell_map_.end())
+        {
+            return false;
+        }
+        
+        for(auto &cell : row.second)
+        {
+            if(other.d_->cell_map_[row.first].find(cell.first) == other.d_->cell_map_[row.first].end())
+            {
+                return false;
+            }
+            
+            if(!(cell.second.self() == other.d_->cell_map_[row.first][cell.first].self()))
+            {
+                return false;
+            }
+        }
+    }
+    
+    // todo: missing some comparisons
+    
+    if(d_->auto_filter_ == other.d_->auto_filter_
+        && d_->comment_count_ == other.d_->comment_count_
+        && d_->freeze_panes_ == other.d_->freeze_panes_
+        && d_->merged_cells_ == other.d_->merged_cells_
+        && d_->relationships_ == other.d_->relationships_)
+    {
+        return true;
+    }
+    
+    return false;
 }
 
 bool worksheet::operator!=(const worksheet &other) const
 {
-    return d_ != other.d_;
+    return !(*this == other);
 }
 
 bool worksheet::operator==(std::nullptr_t) const