Fix issues with the worksheet "title" setter.

-- Resolves two test failures caused by a crash when setting the title to the existing value removing the sheet from the 'sheet_title_rel_id_map_'
-- added empty title check
This commit is contained in:
Crzyrndm 2018-06-16 15:45:43 +12:00
parent e350a7734d
commit 04c0b4a1ca
2 changed files with 54 additions and 18 deletions

View File

@ -242,25 +242,31 @@ std::string worksheet::title() const
void worksheet::title(const std::string &title)
{
if (title.length() > 31)
// do no work if we don't need to
if (d_->title_ == title)
{
return;
}
// excel limits worksheet titles to 31 characters
if (title.empty() || title.length() > 31)
{
throw invalid_sheet_title(title);
}
// invalid characters in a worksheet name
if (title.find_first_of("*:/\\?[]") != std::string::npos)
{
throw invalid_sheet_title(title);
}
auto same_title = std::find_if(workbook().begin(), workbook().end(),
[&](worksheet ws) { return ws.title() == title; });
if (same_title != workbook().end() && *same_title != *this)
// try and insert the new name into the worksheets map
// if the insert fails, we have a duplicate sheet name
auto insert_result = workbook().d_->sheet_title_rel_id_map_.insert(
std::make_pair(title, workbook().d_->sheet_title_rel_id_map_[d_->title_]));
if (!insert_result.second) // insert failed, duplication detected
{
throw invalid_sheet_title(title);
}
workbook().d_->sheet_title_rel_id_map_[title] = workbook().d_->sheet_title_rel_id_map_[d_->title_];
// if the insert succeeded (i.e. wasn't a duplicate sheet name)
// update the worksheet title and remove the old relation
workbook().d_->sheet_title_rel_id_map_.erase(d_->title_);
d_->title_ = title;

View File

@ -25,10 +25,10 @@
#include <iostream>
#include <helpers/test_suite.hpp>
#include <xlnt/workbook/workbook.hpp>
#include <xlnt/worksheet/header_footer.hpp>
#include <xlnt/worksheet/worksheet.hpp>
#include <helpers/test_suite.hpp>
class worksheet_test_suite : public test_suite
{
@ -101,6 +101,7 @@ public:
register_test(test_view_properties_serialization);
register_test(test_clear_cell);
register_test(test_clear_row);
register_test(test_set_title);
}
void test_new_worksheet()
@ -376,7 +377,6 @@ public:
xlnt_assert(!merged.contains("O1"));
}
void test_merged_cell_ranges()
{
xlnt::workbook wb;
@ -391,7 +391,7 @@ public:
ws.cell("A1").value(1);
ws.cell("D4").value(16);
ws.merge_cells("A1:D4");
std::vector<xlnt::range_reference> expected = { xlnt::range_reference("A1:D4") };
std::vector<xlnt::range_reference> expected = {xlnt::range_reference("A1:D4")};
xlnt_assert_equals(ws.merged_ranges(), expected);
xlnt_assert(!ws.cell("D4").has_value());
}
@ -825,7 +825,7 @@ public:
xlnt::header_footer hf;
using hf_loc = xlnt::header_footer::location;
for (auto location : { hf_loc::left, hf_loc::center, hf_loc::right })
for (auto location : {hf_loc::left, hf_loc::center, hf_loc::right})
{
xlnt_assert(!hf.has_header(location));
xlnt_assert(!hf.has_odd_even_header(location));
@ -850,7 +850,7 @@ public:
xlnt::header_footer hf;
using hf_loc = xlnt::header_footer::location;
for (auto location : { hf_loc::left, hf_loc::center, hf_loc::right })
for (auto location : {hf_loc::left, hf_loc::center, hf_loc::right})
{
xlnt_assert(!hf.has_footer(location));
xlnt_assert(!hf.has_odd_even_footer(location));
@ -1220,4 +1220,34 @@ public:
xlnt_assert_equals(ws2.calculate_dimension().height(), height - 1);
xlnt_assert(!ws2.has_cell(xlnt::cell_reference(1, last_row)));
}
void test_set_title()
{
xlnt::workbook wb;
auto ws1 = wb.active_sheet();
// empty titles are invalid
xlnt_assert_throws(ws1.title(""), xlnt::invalid_sheet_title);
// titles longer than 31 chars are invalid
std::string test_long_title(32, 'a');
xlnt_assert(test_long_title.size() > 31);
xlnt_assert_throws(ws1.title(test_long_title), xlnt::invalid_sheet_title);
// titles containing any of the following characters are invalid
std::string invalid_chars = "*:/\\?[]";
for (char &c : invalid_chars)
{
std::string invalid_char = std::string("Sheet") + c;
xlnt_assert_throws(ws1.title(invalid_char), xlnt::invalid_sheet_title);
}
// duplicate names are invalid
auto ws2 = wb.create_sheet();
xlnt_assert_throws(ws2.title(ws1.title()), xlnt::invalid_sheet_title);
xlnt_assert_throws(ws1.title(ws2.title()), xlnt::invalid_sheet_title);
// naming as self is valid and is ignored
auto ws1_title = ws1.title();
auto ws2_title = ws2.title();
xlnt_assert_throws_nothing(ws1.title(ws1.title()));
xlnt_assert_throws_nothing(ws2.title(ws2.title()));
xlnt_assert(ws1_title == ws1.title());
xlnt_assert(ws2_title == ws2.title());
}
};