Make the library creatable only within a shared_ptr.

This commit is contained in:
Matthieu Gautier 2023-09-18 17:16:12 +02:00
parent 49e99e7c22
commit c203e07ee9
5 changed files with 96 additions and 85 deletions

View File

@ -176,7 +176,7 @@ class ZimSearcher : public zim::Searcher
/**
* A Library store several books.
*/
class Library
class Library: public std::enable_shared_from_this<Library>
{
// all data fields must be added in LibraryBase
mutable std::mutex m_mutex;
@ -187,8 +187,13 @@ class Library
typedef std::map<std::string, int> AttributeCounts;
typedef std::set<std::string> BookIdSet;
public:
private:
Library();
public:
[[nodiscard]] static std::shared_ptr<Library> create() {
return std::shared_ptr<Library>(new Library());
}
~Library();
/**

View File

@ -238,14 +238,14 @@ typedef std::vector<std::string> Langs;
TEST(LibraryOpdsImportTest, allInOne)
{
kiwix::Library lib;
kiwix::Manager manager(&lib);
auto lib = kiwix::Library::create();
kiwix::Manager manager(lib.get());
manager.readOpds(sampleOpdsStream, "library-opds-import.unittests.dev");
EXPECT_EQ(10U, lib.getBookCount(true, true));
EXPECT_EQ(10U, lib->getBookCount(true, true));
{
const kiwix::Book& book1 = lib.getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd");
const kiwix::Book& book1 = lib->getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd");
EXPECT_EQ(book1.getTitle(), "Encyclopédie de la Tunisie");
EXPECT_EQ(book1.getName(), "wikipedia_fr_tunisie_novid_2018-10");
@ -271,7 +271,7 @@ TEST(LibraryOpdsImportTest, allInOne)
}
{
const kiwix::Book& book2 = lib.getBookById("0189d9be-2fd0-b4b6-7300-20fab0b5cdc8");
const kiwix::Book& book2 = lib->getBookById("0189d9be-2fd0-b4b6-7300-20fab0b5cdc8");
EXPECT_EQ(book2.getTitle(), "TED talks - Business");
EXPECT_EQ(book2.getName(), "");
EXPECT_EQ(book2.getFlavour(), "");
@ -301,8 +301,10 @@ class LibraryTest : public ::testing::Test {
typedef kiwix::Library::BookIdCollection BookIdCollection;
typedef std::vector<std::string> TitleCollection;
LibraryTest(): lib(kiwix::Library::create()) {}
void SetUp() override {
kiwix::Manager manager(&lib);
kiwix::Manager manager(lib.get());
manager.readOpds(sampleOpdsStream, "foo.urlHost");
manager.readXml(sampleLibraryXML, false, "./test/library.xml", true);
}
@ -316,25 +318,25 @@ class LibraryTest : public ::testing::Test {
TitleCollection ids2Titles(const BookIdCollection& ids) {
TitleCollection titles;
for ( const auto& bookId : ids ) {
titles.push_back(lib.getBookById(bookId).getTitle());
titles.push_back(lib->getBookById(bookId).getTitle());
}
std::sort(titles.begin(), titles.end());
return titles;
}
kiwix::Library lib;
std::shared_ptr<kiwix::Library> lib;
};
TEST_F(LibraryTest, getBookMarksTest)
{
auto bookId1 = lib.getBooksIds()[0];
auto bookId2 = lib.getBooksIds()[1];
auto bookId1 = lib->getBooksIds()[0];
auto bookId2 = lib->getBooksIds()[1];
lib.addBookmark(createBookmark(bookId1));
lib.addBookmark(createBookmark("invalid-bookmark-id"));
lib.addBookmark(createBookmark(bookId2));
auto onlyValidBookmarks = lib.getBookmarks();
auto allBookmarks = lib.getBookmarks(false);
lib->addBookmark(createBookmark(bookId1));
lib->addBookmark(createBookmark("invalid-bookmark-id"));
lib->addBookmark(createBookmark(bookId2));
auto onlyValidBookmarks = lib->getBookmarks();
auto allBookmarks = lib->getBookmarks(false);
EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1);
EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
@ -346,11 +348,11 @@ TEST_F(LibraryTest, getBookMarksTest)
TEST_F(LibraryTest, sanityCheck)
{
EXPECT_EQ(lib.getBookCount(true, true), 12U);
EXPECT_EQ(lib.getBooksLanguages(),
EXPECT_EQ(lib->getBookCount(true, true), 12U);
EXPECT_EQ(lib->getBooksLanguages(),
std::vector<std::string>({"deu", "eng", "fra", "ita", "spa"})
);
EXPECT_EQ(lib.getBooksCreators(), std::vector<std::string>({
EXPECT_EQ(lib->getBooksCreators(), std::vector<std::string>({
"Islam Stack Exchange",
"Movies & TV Stack Exchange",
"Mythology & Folklore Stack Exchange",
@ -361,7 +363,7 @@ TEST_F(LibraryTest, sanityCheck)
"Wikipedia",
"Wikiquote"
}));
EXPECT_EQ(lib.getBooksPublishers(), std::vector<std::string>({
EXPECT_EQ(lib->getBooksPublishers(), std::vector<std::string>({
"",
"Kiwix",
"Kiwix & Some Enthusiasts",
@ -371,22 +373,22 @@ TEST_F(LibraryTest, sanityCheck)
TEST_F(LibraryTest, categoryHandling)
{
EXPECT_EQ("", lib.getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd").getCategory());
EXPECT_EQ("category_defined_via_tags_only", lib.getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2").getCategory());
EXPECT_EQ("category_defined_via_category_element_only", lib.getBookById("0ea1cde6-441d-6c58-f2c7-21c2838e659f").getCategory());
EXPECT_EQ("category_element_overrides_tags", lib.getBookById("1123e574-6eef-6d54-28fc-13e4caeae474").getCategory());
EXPECT_EQ("category_element_overrides_tags", lib.getBookById("14829621-c490-c376-0792-9de558b57efa").getCategory());
EXPECT_EQ("", lib->getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd").getCategory());
EXPECT_EQ("category_defined_via_tags_only", lib->getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2").getCategory());
EXPECT_EQ("category_defined_via_category_element_only", lib->getBookById("0ea1cde6-441d-6c58-f2c7-21c2838e659f").getCategory());
EXPECT_EQ("category_element_overrides_tags", lib->getBookById("1123e574-6eef-6d54-28fc-13e4caeae474").getCategory());
EXPECT_EQ("category_element_overrides_tags", lib->getBookById("14829621-c490-c376-0792-9de558b57efa").getCategory());
}
TEST_F(LibraryTest, emptyFilter)
{
const auto bookIds = lib.filter(kiwix::Filter());
EXPECT_EQ(bookIds, lib.getBooksIds());
const auto bookIds = lib->filter(kiwix::Filter());
EXPECT_EQ(bookIds, lib->getBooksIds());
}
#define EXPECT_FILTER_RESULTS(f, ...) \
EXPECT_EQ( \
ids2Titles(lib.filter(f)), \
ids2Titles(lib->filter(f)), \
TitleCollection({ __VA_ARGS__ }) \
)
@ -736,33 +738,33 @@ TEST_F(LibraryTest, filterByMultipleCriteria)
TEST_F(LibraryTest, getBookByPath)
{
kiwix::Book book = lib.getBookById(lib.getBooksIds()[0]);
kiwix::Book book = lib->getBookById(lib->getBooksIds()[0]);
#ifdef _WIN32
auto path = "C:\\some\\abs\\path.zim";
#else
auto path = "/some/abs/path.zim";
#endif
book.setPath(path);
lib.addBook(book);
EXPECT_EQ(lib.getBookByPath(path).getId(), book.getId());
EXPECT_THROW(lib.getBookByPath("non/existant/path.zim"), std::out_of_range);
lib->addBook(book);
EXPECT_EQ(lib->getBookByPath(path).getId(), book.getId());
EXPECT_THROW(lib->getBookByPath("non/existant/path.zim"), std::out_of_range);
}
TEST_F(LibraryTest, removeBookByIdRemovesTheBook)
{
const auto initialBookCount = lib.getBookCount(true, true);
const auto initialBookCount = lib->getBookCount(true, true);
ASSERT_GT(initialBookCount, 0U);
EXPECT_NO_THROW(lib.getBookById("raycharles"));
lib.removeBookById("raycharles");
EXPECT_EQ(initialBookCount - 1, lib.getBookCount(true, true));
EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range);
EXPECT_NO_THROW(lib->getBookById("raycharles"));
lib->removeBookById("raycharles");
EXPECT_EQ(initialBookCount - 1, lib->getBookCount(true, true));
EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range);
};
TEST_F(LibraryTest, removeBookByIdDropsTheReader)
{
EXPECT_NE(nullptr, lib.getArchiveById("raycharles"));
lib.removeBookById("raycharles");
EXPECT_THROW(lib.getArchiveById("raycharles"), std::out_of_range);
EXPECT_NE(nullptr, lib->getArchiveById("raycharles"));
lib->removeBookById("raycharles");
EXPECT_THROW(lib->getArchiveById("raycharles"), std::out_of_range);
};
TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB)
@ -770,17 +772,17 @@ TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB)
kiwix::Filter f;
f.local(true).valid(true).query(R"(title:"ray charles")", false);
EXPECT_NO_THROW(lib.getBookById("raycharles"));
EXPECT_EQ(1U, lib.filter(f).size());
EXPECT_NO_THROW(lib->getBookById("raycharles"));
EXPECT_EQ(1U, lib->filter(f).size());
lib.removeBookById("raycharles");
lib->removeBookById("raycharles");
EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range);
EXPECT_EQ(0U, lib.filter(f).size());
EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range);
EXPECT_EQ(0U, lib->filter(f).size());
// make sure that Library::filter() doesn't add an empty book with
// an id surviving in the search DB
EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range);
EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range);
};
TEST_F(LibraryTest, removeBooksNotUpdatedSince)
@ -800,18 +802,18 @@ TEST_F(LibraryTest, removeBooksNotUpdatedSince)
"Wikiquote"
);
const uint64_t rev = lib.getRevision();
for ( const auto& id : lib.filter(kiwix::Filter().query("exchange")) ) {
lib.addBook(lib.getBookByIdThreadSafe(id));
const uint64_t rev = lib->getRevision();
for ( const auto& id : lib->filter(kiwix::Filter().query("exchange")) ) {
lib->addBook(lib->getBookByIdThreadSafe(id));
}
EXPECT_GT(lib.getRevision(), rev);
EXPECT_GT(lib->getRevision(), rev);
const uint64_t rev2 = lib.getRevision();
const uint64_t rev2 = lib->getRevision();
EXPECT_EQ(9u, lib.removeBooksNotUpdatedSince(rev));
EXPECT_EQ(9u, lib->removeBooksNotUpdatedSince(rev));
EXPECT_GT(lib.getRevision(), rev2);
EXPECT_GT(lib->getRevision(), rev2);
EXPECT_FILTER_RESULTS(kiwix::Filter(),
"Islam Stack Exchange",

View File

@ -8,18 +8,18 @@
TEST(ManagerTest, addBookFromPathAndGetIdTest)
{
kiwix::Library lib;
kiwix::Manager manager = kiwix::Manager(&lib);
auto lib = kiwix::Library::create();
kiwix::Manager manager = kiwix::Manager(lib.get());
auto bookId = manager.addBookFromPathAndGetId("./test/example.zim");
ASSERT_NE(bookId, "");
kiwix::Book book = lib.getBookById(bookId);
kiwix::Book book = lib->getBookById(bookId);
EXPECT_EQ(book.getPath(), kiwix::computeAbsolutePath("", "./test/example.zim"));
const std::string pathToSave = "./pathToSave";
const std::string url = "url";
bookId = manager.addBookFromPathAndGetId("./test/example.zim", pathToSave, url, true);
book = lib.getBookById(bookId);
book = lib->getBookById(bookId);
auto savedPath = kiwix::computeAbsolutePath(kiwix::removeLastPathElement(manager.writableLibraryPath), pathToSave);
EXPECT_EQ(book.getPath(), savedPath);
EXPECT_EQ(book.getUrl(), url);
@ -48,11 +48,11 @@ const char sampleLibraryXML[] = R"(
TEST(ManagerTest, readXml)
{
kiwix::Library lib;
kiwix::Manager manager = kiwix::Manager(&lib);
auto lib = kiwix::Library::create();
kiwix::Manager manager = kiwix::Manager(lib.get());
EXPECT_EQ(true, manager.readXml(sampleLibraryXML, true, "/data/lib.xml", true));
kiwix::Book book = lib.getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2");
kiwix::Book book = lib->getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2");
EXPECT_EQ("/data/zimfiles/unittest.zim", book.getPath());
EXPECT_EQ("https://example.com/zimfiles/unittest.zim", book.getUrl());
EXPECT_EQ("Unit Test", book.getTitle());
@ -70,24 +70,24 @@ TEST(ManagerTest, readXml)
TEST(Manager, reload)
{
kiwix::Library lib;
kiwix::Manager manager(&lib);
auto lib = kiwix::Library::create();
kiwix::Manager manager(lib.get());
manager.reload({ "./test/library.xml" });
EXPECT_EQ(lib.getBooksIds(), (kiwix::Library::BookIdCollection{
EXPECT_EQ(lib->getBooksIds(), (kiwix::Library::BookIdCollection{
"charlesray",
"raycharles",
"raycharles_uncategorized"
}));
lib.removeBookById("raycharles");
EXPECT_EQ(lib.getBooksIds(), (kiwix::Library::BookIdCollection{
lib->removeBookById("raycharles");
EXPECT_EQ(lib->getBooksIds(), (kiwix::Library::BookIdCollection{
"charlesray",
"raycharles_uncategorized"
}));
manager.reload({ "./test/library.xml" });
EXPECT_EQ(lib.getBooksIds(), kiwix::Library::BookIdCollection({
EXPECT_EQ(lib->getBooksIds(), kiwix::Library::BookIdCollection({
"charlesray",
"raycharles",
"raycharles_uncategorized"

View File

@ -18,18 +18,20 @@ const char libraryXML[] = R"(
)";
class NameMapperTest : public ::testing::Test {
public:
NameMapperTest(): lib(kiwix::Library::create()) {}
protected:
void SetUp() override {
kiwix::Manager manager(&lib);
kiwix::Manager manager(lib.get());
manager.readXml(libraryXML, false, "./library.xml", true);
for ( const std::string& id : lib.getBooksIds() ) {
kiwix::Book bookCopy = lib.getBookById(id);
for ( const std::string& id : lib->getBooksIds() ) {
kiwix::Book bookCopy = lib->getBookById(id);
bookCopy.setPathValid(true);
lib.addBook(bookCopy);
lib->addBook(bookCopy);
}
}
kiwix::Library lib;
std::shared_ptr<kiwix::Library> lib;
};
class CapturedStderr
@ -73,13 +75,13 @@ void checkUnaliasedEntriesInNameMapper(const kiwix::NameMapper& nm)
TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases)
{
CapturedStderr stderror;
kiwix::HumanReadableNameMapper nm(lib, false);
kiwix::HumanReadableNameMapper nm(*lib, false);
EXPECT_EQ("", std::string(stderror));
checkUnaliasedEntriesInNameMapper(nm);
EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range);
lib.removeBookById("04-2021-10");
lib->removeBookById("04-2021-10");
EXPECT_EQ("zero_four_2021-10", nm.getNameForId("04-2021-10"));
EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four_2021-10"));
EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range);
@ -88,7 +90,7 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases)
TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
{
CapturedStderr stderror;
kiwix::HumanReadableNameMapper nm(lib, true);
kiwix::HumanReadableNameMapper nm(*lib, true);
EXPECT_EQ(
"Path collision: /data/zero_four_2021-10.zim and"
" /data/zero_four_2021-11.zim can't share the same URL path 'zero_four'."
@ -99,7 +101,7 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
checkUnaliasedEntriesInNameMapper(nm);
EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four"));
lib.removeBookById("04-2021-10");
lib->removeBookById("04-2021-10");
EXPECT_EQ("zero_four_2021-10", nm.getNameForId("04-2021-10"));
EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four_2021-10"));
EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four"));
@ -108,13 +110,13 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases)
{
CapturedStderr stderror;
kiwix::UpdatableNameMapper nm(lib, false);
kiwix::UpdatableNameMapper nm(*lib, false);
EXPECT_EQ("", std::string(stderror));
checkUnaliasedEntriesInNameMapper(nm);
EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range);
lib.removeBookById("04-2021-10");
lib->removeBookById("04-2021-10");
nm.update();
EXPECT_THROW(nm.getNameForId("04-2021-10"), std::out_of_range);
EXPECT_THROW(nm.getIdForName("zero_four_2021-10"), std::out_of_range);
@ -124,7 +126,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases)
TEST_F(NameMapperTest, UpdatableNameMapperWithAliases)
{
CapturedStderr stderror;
kiwix::UpdatableNameMapper nm(lib, true);
kiwix::UpdatableNameMapper nm(*lib, true);
EXPECT_EQ(
"Path collision: /data/zero_four_2021-10.zim and"
" /data/zero_four_2021-11.zim can't share the same URL path 'zero_four'."
@ -137,7 +139,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithAliases)
{
CapturedStderr nmUpdateStderror;
lib.removeBookById("04-2021-10");
lib->removeBookById("04-2021-10");
nm.update();
EXPECT_EQ("", std::string(nmUpdateStderror));
}

View File

@ -98,7 +98,7 @@ private:
void run(int serverPort, std::string indexTemplateString = "");
private: // data
kiwix::Library library;
std::shared_ptr<kiwix::Library> library;
kiwix::Manager manager;
std::unique_ptr<kiwix::NameMapper> nameMapper;
std::unique_ptr<kiwix::Server> server;
@ -107,7 +107,8 @@ private: // data
};
ZimFileServer::ZimFileServer(int serverPort, Cfg _cfg, std::string libraryFilePath)
: manager(&this->library)
: library(kiwix::Library::create())
, manager(this->library.get())
, cfg(_cfg)
{
if ( kiwix::isRelativePath(libraryFilePath) )
@ -120,7 +121,8 @@ ZimFileServer::ZimFileServer(int serverPort,
Cfg _cfg,
const FilePathCollection& zimpaths,
std::string indexTemplateString)
: manager(&this->library)
: library(kiwix::Library::create())
, manager(this->library.get())
, cfg(_cfg)
{
for ( const auto& zimpath : zimpaths ) {
@ -136,9 +138,9 @@ void ZimFileServer::run(int serverPort, std::string indexTemplateString)
if (cfg.options & NO_NAME_MAPPER) {
nameMapper.reset(new kiwix::IdNameMapper());
} else {
nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false));
nameMapper.reset(new kiwix::HumanReadableNameMapper(*library, false));
}
server.reset(new kiwix::Server(&library, nameMapper.get()));
server.reset(new kiwix::Server(library.get(), nameMapper.get()));
server->setRoot(cfg.root);
server->setAddress(address);
server->setPort(serverPort);