smallsolar
/
libkiwix
mirror of https://github.com/kiwix/libkiwix.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Do not store raw pointer to Library. 

While it was "ok" to store raw pointer as, in our use case, the library
always live longer than object using it; it is not safe to store
raw pointer on object than may be deleted.

All classes storing a library now store a shared_ptr.
Functionr only using the library (as `HumanReadableNameMapper`) continue
to use a (const) reference.
This commit is contained in:
Matthieu Gautier 2022-10-06 12:00:27 +02:00
parent 5896691b31
commit 0bd5a5ec3b
13 changed files with 119 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/manager.h
View File

@ -37,10 +37,10 @@ namespace kiwix
class LibraryManipulator class LibraryManipulator
{ {
public: // functions public: // functions
explicit LibraryManipulator(Library* library); explicit LibraryManipulator(std::shared_ptr<Library> library);
virtual ~LibraryManipulator(); virtual ~LibraryManipulator();
Library& getLibrary() const { return library; } Library& getLibrary() const { return *library.get(); }
bool addBookToLibrary(const Book& book); bool addBookToLibrary(const Book& book);
void addBookmarkToLibrary(const Bookmark& bookmark); void addBookmarkToLibrary(const Bookmark& bookmark);
@ -52,7 +52,7 @@ class LibraryManipulator
virtual void booksWereRemovedFromLibrary(); virtual void booksWereRemovedFromLibrary();
private: // data private: // data
kiwix::Library& library; std::shared_ptr<kiwix::Library> library;
}; };
/** /**
@ -65,7 +65,7 @@ class Manager
public: // functions public: // functions
explicit Manager(LibraryManipulator* manipulator); explicit Manager(LibraryManipulator* manipulator);
explicit Manager(Library* library); explicit Manager(std::shared_ptr<Library> library);
/** /**
* Read a `library.xml` and add book in the file to the library. * Read a `library.xml` and add book in the file to the library.

6
include/name_mapper.h
View File

@ -50,7 +50,7 @@ class HumanReadableNameMapper : public NameMapper {
std::map<std::string, std::string> m_nameToId; std::map<std::string, std::string> m_nameToId;
public: public:
HumanReadableNameMapper(kiwix::Library& library, bool withAlias); HumanReadableNameMapper(const kiwix::Library& library, bool withAlias);
virtual ~HumanReadableNameMapper() = default; virtual ~HumanReadableNameMapper() = default;
virtual std::string getNameForId(const std::string& id) const; virtual std::string getNameForId(const std::string& id) const;
virtual std::string getIdForName(const std::string& name) const; virtual std::string getIdForName(const std::string& name) const;
@ -59,7 +59,7 @@ class HumanReadableNameMapper : public NameMapper {
class UpdatableNameMapper : public NameMapper { class UpdatableNameMapper : public NameMapper {
typedef std::shared_ptr<NameMapper> NameMapperHandle; typedef std::shared_ptr<NameMapper> NameMapperHandle;
public: public:
UpdatableNameMapper(Library& library, bool withAlias); UpdatableNameMapper(std::shared_ptr<Library> library, bool withAlias);
virtual std::string getNameForId(const std::string& id) const; virtual std::string getNameForId(const std::string& id) const;
virtual std::string getIdForName(const std::string& name) const; virtual std::string getIdForName(const std::string& name) const;
@ -71,7 +71,7 @@ class UpdatableNameMapper : public NameMapper {
private: private:
mutable std::mutex mutex; mutable std::mutex mutex;
Library& library; std::shared_ptr<Library> library;
NameMapperHandle nameMapper; NameMapperHandle nameMapper;
const bool withAlias; const bool withAlias;
}; };

4
include/opds_dumper.h
View File

@ -42,7 +42,7 @@ class OPDSDumper
{ {
public: public:
OPDSDumper() = default; OPDSDumper() = default;
OPDSDumper(Library* library, NameMapper* NameMapper); OPDSDumper(std::shared_ptr<Library> library, NameMapper* NameMapper);
~OPDSDumper(); ~OPDSDumper();
/** /**
@ -110,7 +110,7 @@ class OPDSDumper
void setOpenSearchInfo(int totalResult, int startIndex, int count); void setOpenSearchInfo(int totalResult, int startIndex, int count);
protected: protected:
kiwix::Library* library; std::shared_ptr<kiwix::Library> library;
kiwix::NameMapper* nameMapper; kiwix::NameMapper* nameMapper;
std::string libraryId; std::string libraryId;
std::string rootLocation; std::string rootLocation;

4
include/search_renderer.h
View File

@ -58,7 +58,7 @@ class SearchRenderer
* @param start The start offset used for the srs. * @param start The start offset used for the srs.
* @param estimatedResultCount The estimatedResultCount of the whole search * @param estimatedResultCount The estimatedResultCount of the whole search
*/ */
SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, Library* library, SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, std::shared_ptr<Library> library,
unsigned int start, unsigned int estimatedResultCount); unsigned int start, unsigned int estimatedResultCount);
~SearchRenderer(); ~SearchRenderer();
@ -107,7 +107,7 @@ class SearchRenderer
std::string beautifyInteger(const unsigned int number); std::string beautifyInteger(const unsigned int number);
zim::SearchResultSet m_srs; zim::SearchResultSet m_srs;
NameMapper* mp_nameMapper; NameMapper* mp_nameMapper;
Library* mp_library; std::shared_ptr<Library> mp_library;
std::string searchBookQuery; std::string searchBookQuery;
std::string searchPattern; std::string searchPattern;
std::string protocolPrefix; std::string protocolPrefix;

4
include/server.h
View File

@ -34,7 +34,7 @@ namespace kiwix
public: public:
class Configuration { class Configuration {
public: public:
explicit Configuration(Library* library, NameMapper* nameMapper=nullptr) explicit Configuration(std::shared_ptr<Library> library, NameMapper* nameMapper=nullptr)
: mp_library(library), : mp_library(library),
mp_nameMapper(nameMapper) mp_nameMapper(nameMapper)
{} {}
@ -86,7 +86,7 @@ namespace kiwix
return *this; return *this;
} }
Library* mp_library; std::shared_ptr<Library> mp_library;
NameMapper* mp_nameMapper; NameMapper* mp_nameMapper;
std::string m_root = ""; std::string m_root = "";
std::string m_addr = ""; std::string m_addr = "";

12
src/manager.cpp
View File

@ -41,8 +41,8 @@ struct NoDelete
// LibraryManipulator // LibraryManipulator
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
LibraryManipulator::LibraryManipulator(Library* library) LibraryManipulator::LibraryManipulator(std::shared_ptr<Library> library)
: library(*library) : library(library)
{} {}
LibraryManipulator::~LibraryManipulator() LibraryManipulator::~LibraryManipulator()
@ -50,7 +50,7 @@ LibraryManipulator::~LibraryManipulator()
bool LibraryManipulator::addBookToLibrary(const Book& book) bool LibraryManipulator::addBookToLibrary(const Book& book)
{ {
const auto ret = library.addBook(book); const auto ret = library->addBook(book);
if ( ret ) { if ( ret ) {
bookWasAddedToLibrary(book); bookWasAddedToLibrary(book);
} }
@ -59,13 +59,13 @@ bool LibraryManipulator::addBookToLibrary(const Book& book)
void LibraryManipulator::addBookmarkToLibrary(const Bookmark& bookmark) void LibraryManipulator::addBookmarkToLibrary(const Bookmark& bookmark)
{ {
library.addBookmark(bookmark); library->addBookmark(bookmark);
bookmarkWasAddedToLibrary(bookmark); bookmarkWasAddedToLibrary(bookmark);
} }
uint32_t LibraryManipulator::removeBooksNotUpdatedSince(Library::Revision rev) uint32_t LibraryManipulator::removeBooksNotUpdatedSince(Library::Revision rev)
{ {
const auto n = library.removeBooksNotUpdatedSince(rev); const auto n = library->removeBooksNotUpdatedSince(rev);
if ( n != 0 ) { if ( n != 0 ) {
booksWereRemovedFromLibrary(); booksWereRemovedFromLibrary();
} }
@ -95,7 +95,7 @@ Manager::Manager(LibraryManipulator* manipulator):
{ {
} }
Manager::Manager(Library* library) : Manager::Manager(std::shared_ptr<Library> library) :
writableLibraryPath(""), writableLibraryPath(""),
manipulator(new LibraryManipulator(library)) manipulator(new LibraryManipulator(library))
{ {

6
src/name_mapper.cpp
View File

@ -24,7 +24,7 @@
namespace kiwix { namespace kiwix {
HumanReadableNameMapper::HumanReadableNameMapper(kiwix::Library& library, bool withAlias) { HumanReadableNameMapper::HumanReadableNameMapper(const kiwix::Library& library, bool withAlias) {
for (auto& bookId: library.filter(kiwix::Filter().local(true).valid(true))) { for (auto& bookId: library.filter(kiwix::Filter().local(true).valid(true))) {
auto& currentBook = library.getBookById(bookId); auto& currentBook = library.getBookById(bookId);
auto bookName = currentBook.getHumanReadableIdFromPath(); auto bookName = currentBook.getHumanReadableIdFromPath();
@ -63,7 +63,7 @@ std::string HumanReadableNameMapper::getIdForName(const std::string& name) const
// UpdatableNameMapper // UpdatableNameMapper
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
UpdatableNameMapper::UpdatableNameMapper(Library& lib, bool withAlias) UpdatableNameMapper::UpdatableNameMapper(std::shared_ptr<Library> lib, bool withAlias)
: library(lib) : library(lib)
, withAlias(withAlias) , withAlias(withAlias)
{ {
@ -72,7 +72,7 @@ UpdatableNameMapper::UpdatableNameMapper(Library& lib, bool withAlias)
void UpdatableNameMapper::update() void UpdatableNameMapper::update()
{ {
const auto newNameMapper = new HumanReadableNameMapper(library, withAlias); const auto newNameMapper = new HumanReadableNameMapper(*library, withAlias);
std::lock_guard<std::mutex> lock(mutex); std::lock_guard<std::mutex> lock(mutex);
nameMapper.reset(newNameMapper); nameMapper.reset(newNameMapper);
} }

10
src/opds_dumper.cpp
View File

@ -30,7 +30,7 @@ namespace kiwix
{ {
/* Constructor */ /* Constructor */
OPDSDumper::OPDSDumper(Library* library, NameMapper* nameMapper) OPDSDumper::OPDSDumper(std::shared_ptr<Library> library, NameMapper* nameMapper)
: library(library), : library(library),
nameMapper(nameMapper) nameMapper(nameMapper)
{ {
@ -113,12 +113,12 @@ std::string partialEntryXML(const Book& book, const std::string& rootLocation)
return render_template(xmlTemplate, data); return render_template(xmlTemplate, data);
} }
BooksData getBooksData(const Library* library, const NameMapper& nameMapper, const std::vector<std::string>& bookIds, const std::string& rootLocation, bool partial) BooksData getBooksData(const Library& library, const NameMapper& nameMapper, const std::vector<std::string>& bookIds, const std::string& rootLocation, bool partial)
{ {
BooksData booksData; BooksData booksData;
for ( const auto& bookId : bookIds ) { for ( const auto& bookId : bookIds ) {
try { try {
const Book book = library->getBookByIdThreadSafe(bookId); const Book book = library.getBookByIdThreadSafe(bookId);
const std::string bookName = nameMapper.getNameForId(bookId); const std::string bookName = nameMapper.getNameForId(bookId);
const auto entryXML = partial const auto entryXML = partial
? partialEntryXML(book, rootLocation) ? partialEntryXML(book, rootLocation)
@ -190,7 +190,7 @@ std::string getLanguageSelfName(const std::string& lang) {
string OPDSDumper::dumpOPDSFeed(const std::vector<std::string>& bookIds, const std::string& query) const string OPDSDumper::dumpOPDSFeed(const std::vector<std::string>& bookIds, const std::string& query) const
{ {
const auto booksData = getBooksData(library, *nameMapper, bookIds, rootLocation, false); const auto booksData = getBooksData(*library, *nameMapper, bookIds, rootLocation, false);
const kainjow::mustache::object template_data{ const kainjow::mustache::object template_data{
{"date", gen_date_str()}, {"date", gen_date_str()},
{"root", rootLocation}, {"root", rootLocation},
@ -208,7 +208,7 @@ string OPDSDumper::dumpOPDSFeed(const std::vector<std::string>& bookIds, const s
string OPDSDumper::dumpOPDSFeedV2(const std::vector<std::string>& bookIds, const std::string& query, bool partial) const string OPDSDumper::dumpOPDSFeedV2(const std::vector<std::string>& bookIds, const std::string& query, bool partial) const
{ {
const auto endpointRoot = rootLocation + "/catalog/v2"; const auto endpointRoot = rootLocation + "/catalog/v2";
const auto booksData = getBooksData(library, *nameMapper, bookIds, rootLocation, partial); const auto booksData = getBooksData(*library, *nameMapper, bookIds, rootLocation, partial);
const char* const endpoint = partial ? "/partial_entries" : "/entries"; const char* const endpoint = partial ? "/partial_entries" : "/entries";
const kainjow::mustache::object template_data{ const kainjow::mustache::object template_data{

2
src/search_renderer.cpp
View File

@ -41,7 +41,7 @@ SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper,
: SearchRenderer(srs, mapper, nullptr, start, estimatedResultCount) : SearchRenderer(srs, mapper, nullptr, start, estimatedResultCount)
{} {}
SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, Library* library, SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, std::shared_ptr<Library> library,
unsigned int start, unsigned int estimatedResultCount) unsigned int start, unsigned int estimatedResultCount)
: m_srs(srs), : m_srs(srs),
mp_nameMapper(mapper), mp_nameMapper(mapper),

102
test/library.cpp
View File

@ -236,14 +236,14 @@ namespace
TEST(LibraryOpdsImportTest, allInOne) TEST(LibraryOpdsImportTest, allInOne)
{ {
kiwix::Library lib; auto lib = std::make_shared<kiwix::Library>();
kiwix::Manager manager(&lib); kiwix::Manager manager(lib);
manager.readOpds(sampleOpdsStream, "library-opds-import.unittests.dev"); 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.getTitle(), "Encyclopédie de la Tunisie");
EXPECT_EQ(book1.getName(), "wikipedia_fr_tunisie_novid_2018-10"); EXPECT_EQ(book1.getName(), "wikipedia_fr_tunisie_novid_2018-10");
@ -268,7 +268,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.getTitle(), "TED talks - Business");
EXPECT_EQ(book2.getName(), ""); EXPECT_EQ(book2.getName(), "");
EXPECT_EQ(book2.getFlavour(), ""); EXPECT_EQ(book2.getFlavour(), "");
@ -297,8 +297,12 @@ class LibraryTest : public ::testing::Test {
typedef kiwix::Library::BookIdCollection BookIdCollection; typedef kiwix::Library::BookIdCollection BookIdCollection;
typedef std::vector<std::string> TitleCollection; typedef std::vector<std::string> TitleCollection;
explicit LibraryTest()
: lib(std::make_shared<kiwix::Library>())
{}
void SetUp() override { void SetUp() override {
kiwix::Manager manager(&lib); kiwix::Manager manager(lib);
manager.readOpds(sampleOpdsStream, "foo.urlHost"); manager.readOpds(sampleOpdsStream, "foo.urlHost");
manager.readXml(sampleLibraryXML, false, "./test/library.xml", true); manager.readXml(sampleLibraryXML, false, "./test/library.xml", true);
} }
@ -312,25 +316,25 @@ class LibraryTest : public ::testing::Test {
TitleCollection ids2Titles(const BookIdCollection& ids) { TitleCollection ids2Titles(const BookIdCollection& ids) {
TitleCollection titles; TitleCollection titles;
for ( const auto& bookId : ids ) { 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()); std::sort(titles.begin(), titles.end());
return titles; return titles;
} }
kiwix::Library lib; std::shared_ptr<kiwix::Library> lib;
}; };
TEST_F(LibraryTest, getBookMarksTest) TEST_F(LibraryTest, getBookMarksTest)
{ {
auto bookId1 = lib.getBooksIds()[0]; auto bookId1 = lib->getBooksIds()[0];
auto bookId2 = lib.getBooksIds()[1]; auto bookId2 = lib->getBooksIds()[1];
lib.addBookmark(createBookmark(bookId1)); lib->addBookmark(createBookmark(bookId1));
lib.addBookmark(createBookmark("invalid-bookmark-id")); lib->addBookmark(createBookmark("invalid-bookmark-id"));
lib.addBookmark(createBookmark(bookId2)); lib->addBookmark(createBookmark(bookId2));
auto onlyValidBookmarks = lib.getBookmarks(); auto onlyValidBookmarks = lib->getBookmarks();
auto allBookmarks = lib.getBookmarks(false); auto allBookmarks = lib->getBookmarks(false);
EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1); EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1);
EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2); EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
@ -342,11 +346,11 @@ TEST_F(LibraryTest, getBookMarksTest)
TEST_F(LibraryTest, sanityCheck) TEST_F(LibraryTest, sanityCheck)
{ {
EXPECT_EQ(lib.getBookCount(true, true), 12U); EXPECT_EQ(lib->getBookCount(true, true), 12U);
EXPECT_EQ(lib.getBooksLanguages(), EXPECT_EQ(lib->getBooksLanguages(),
std::vector<std::string>({"deu", "eng", "fra"}) std::vector<std::string>({"deu", "eng", "fra"})
); );
EXPECT_EQ(lib.getBooksCreators(), std::vector<std::string>({ EXPECT_EQ(lib->getBooksCreators(), std::vector<std::string>({
"Islam Stack Exchange", "Islam Stack Exchange",
"Movies & TV Stack Exchange", "Movies & TV Stack Exchange",
"Mythology & Folklore Stack Exchange", "Mythology & Folklore Stack Exchange",
@ -357,7 +361,7 @@ TEST_F(LibraryTest, sanityCheck)
"Wikipedia", "Wikipedia",
"Wikiquote" "Wikiquote"
})); }));
EXPECT_EQ(lib.getBooksPublishers(), std::vector<std::string>({ EXPECT_EQ(lib->getBooksPublishers(), std::vector<std::string>({
"", "",
"Kiwix", "Kiwix",
"Kiwix & Some Enthusiasts", "Kiwix & Some Enthusiasts",
@ -367,22 +371,22 @@ TEST_F(LibraryTest, sanityCheck)
TEST_F(LibraryTest, categoryHandling) TEST_F(LibraryTest, categoryHandling)
{ {
EXPECT_EQ("", lib.getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd").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_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_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("1123e574-6eef-6d54-28fc-13e4caeae474").getCategory());
EXPECT_EQ("category_element_overrides_tags", lib.getBookById("14829621-c490-c376-0792-9de558b57efa").getCategory()); EXPECT_EQ("category_element_overrides_tags", lib->getBookById("14829621-c490-c376-0792-9de558b57efa").getCategory());
} }
TEST_F(LibraryTest, emptyFilter) TEST_F(LibraryTest, emptyFilter)
{ {
const auto bookIds = lib.filter(kiwix::Filter()); const auto bookIds = lib->filter(kiwix::Filter());
EXPECT_EQ(bookIds, lib.getBooksIds()); EXPECT_EQ(bookIds, lib->getBooksIds());
} }
#define EXPECT_FILTER_RESULTS(f, ...) \ #define EXPECT_FILTER_RESULTS(f, ...) \
EXPECT_EQ( \ EXPECT_EQ( \
ids2Titles(lib.filter(f)), \ ids2Titles(lib->filter(f)), \
TitleCollection({ __VA_ARGS__ }) \ TitleCollection({ __VA_ARGS__ }) \
) )
@ -732,33 +736,33 @@ TEST_F(LibraryTest, filterByMultipleCriteria)
TEST_F(LibraryTest, getBookByPath) TEST_F(LibraryTest, getBookByPath)
{ {
kiwix::Book book = lib.getBookById(lib.getBooksIds()[0]); kiwix::Book book = lib->getBookById(lib->getBooksIds()[0]);
#ifdef _WIN32 #ifdef _WIN32
auto path = "C:\\some\\abs\\path.zim"; auto path = "C:\\some\\abs\\path.zim";
#else #else
auto path = "/some/abs/path.zim"; auto path = "/some/abs/path.zim";
#endif #endif
book.setPath(path); book.setPath(path);
lib.addBook(book); lib->addBook(book);
EXPECT_EQ(lib.getBookByPath(path).getId(), book.getId()); EXPECT_EQ(lib->getBookByPath(path).getId(), book.getId());
EXPECT_THROW(lib.getBookByPath("non/existant/path.zim"), std::out_of_range); EXPECT_THROW(lib->getBookByPath("non/existant/path.zim"), std::out_of_range);
} }
TEST_F(LibraryTest, removeBookByIdRemovesTheBook) TEST_F(LibraryTest, removeBookByIdRemovesTheBook)
{ {
const auto initialBookCount = lib.getBookCount(true, true); const auto initialBookCount = lib->getBookCount(true, true);
ASSERT_GT(initialBookCount, 0U); ASSERT_GT(initialBookCount, 0U);
EXPECT_NO_THROW(lib.getBookById("raycharles")); EXPECT_NO_THROW(lib->getBookById("raycharles"));
lib.removeBookById("raycharles"); lib->removeBookById("raycharles");
EXPECT_EQ(initialBookCount - 1, lib.getBookCount(true, true)); EXPECT_EQ(initialBookCount - 1, lib->getBookCount(true, true));
EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range); EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range);
}; };
TEST_F(LibraryTest, removeBookByIdDropsTheReader) TEST_F(LibraryTest, removeBookByIdDropsTheReader)
{ {
EXPECT_NE(nullptr, lib.getArchiveById("raycharles")); EXPECT_NE(nullptr, lib->getArchiveById("raycharles"));
lib.removeBookById("raycharles"); lib->removeBookById("raycharles");
EXPECT_THROW(lib.getArchiveById("raycharles"), std::out_of_range); EXPECT_THROW(lib->getArchiveById("raycharles"), std::out_of_range);
}; };
TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB) TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB)
@ -766,17 +770,17 @@ TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB)
kiwix::Filter f; kiwix::Filter f;
f.local(true).valid(true).query(R"(title:"ray charles")", false); f.local(true).valid(true).query(R"(title:"ray charles")", false);
EXPECT_NO_THROW(lib.getBookById("raycharles")); EXPECT_NO_THROW(lib->getBookById("raycharles"));
EXPECT_EQ(1U, lib.filter(f).size()); EXPECT_EQ(1U, lib->filter(f).size());
lib.removeBookById("raycharles"); lib->removeBookById("raycharles");
EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range); EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range);
EXPECT_EQ(0U, lib.filter(f).size()); EXPECT_EQ(0U, lib->filter(f).size());
// make sure that Library::filter() doesn't add an empty book with // make sure that Library::filter() doesn't add an empty book with
// an id surviving in the search DB // 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) TEST_F(LibraryTest, removeBooksNotUpdatedSince)
@ -796,12 +800,12 @@ TEST_F(LibraryTest, removeBooksNotUpdatedSince)
"Wikiquote" "Wikiquote"
); );
const uint64_t rev = lib.getRevision(); const uint64_t rev = lib->getRevision();
for ( const auto& id : lib.filter(kiwix::Filter().query("exchange")) ) { for ( const auto& id : lib->filter(kiwix::Filter().query("exchange")) ) {
lib.addBook(lib.getBookByIdThreadSafe(id)); lib->addBook(lib->getBookByIdThreadSafe(id));
} }
EXPECT_EQ(9u, lib.removeBooksNotUpdatedSince(rev)); EXPECT_EQ(9u, lib->removeBooksNotUpdatedSince(rev));
EXPECT_FILTER_RESULTS(kiwix::Filter(), EXPECT_FILTER_RESULTS(kiwix::Filter(),
"Islam Stack Exchange", "Islam Stack Exchange",

26
test/manager.cpp
View File

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

27
test/name_mapper.cpp
View File

@ -18,18 +18,23 @@ const char libraryXML[] = R"(
)"; )";
class NameMapperTest : public ::testing::Test { class NameMapperTest : public ::testing::Test {
public:
explicit NameMapperTest() :
lib(std::make_shared<kiwix::Library>())
{}
protected: protected:
void SetUp() override { void SetUp() override {
kiwix::Manager manager(&lib); kiwix::Manager manager(lib);
manager.readXml(libraryXML, false, "./library.xml", true); manager.readXml(libraryXML, false, "./library.xml", true);
for ( const std::string& id : lib.getBooksIds() ) { for ( const std::string& id : lib->getBooksIds() ) {
kiwix::Book bookCopy = lib.getBookById(id); kiwix::Book bookCopy = lib->getBookById(id);
bookCopy.setPathValid(true); bookCopy.setPathValid(true);
lib.addBook(bookCopy); lib->addBook(bookCopy);
} }
} }
kiwix::Library lib; std::shared_ptr<kiwix::Library> lib;
}; };
class CapturedStderr class CapturedStderr
@ -73,13 +78,13 @@ void checkUnaliasedEntriesInNameMapper(const kiwix::NameMapper& nm)
TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases) TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases)
{ {
CapturedStderr stderror; CapturedStderr stderror;
kiwix::HumanReadableNameMapper nm(lib, false); kiwix::HumanReadableNameMapper nm(*lib, false);
EXPECT_EQ("", std::string(stderror)); EXPECT_EQ("", std::string(stderror));
checkUnaliasedEntriesInNameMapper(nm); checkUnaliasedEntriesInNameMapper(nm);
EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range); 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("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_2021-10"));
EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range); EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range);
@ -88,7 +93,7 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases)
TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases) TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
{ {
CapturedStderr stderror; CapturedStderr stderror;
kiwix::HumanReadableNameMapper nm(lib, true); kiwix::HumanReadableNameMapper nm(*lib, true);
EXPECT_EQ( EXPECT_EQ(
"Path collision: /data/zero_four_2021-10.zim and" "Path collision: /data/zero_four_2021-10.zim and"
" /data/zero_four_2021-11.zim can't share the same URL path 'zero_four'." " /data/zero_four_2021-11.zim can't share the same URL path 'zero_four'."
@ -99,7 +104,7 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
checkUnaliasedEntriesInNameMapper(nm); checkUnaliasedEntriesInNameMapper(nm);
EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four")); 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("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_2021-10"));
EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four")); EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four"));
@ -114,7 +119,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases)
checkUnaliasedEntriesInNameMapper(nm); checkUnaliasedEntriesInNameMapper(nm);
EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range); EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range);
lib.removeBookById("04-2021-10"); lib->removeBookById("04-2021-10");
nm.update(); nm.update();
EXPECT_THROW(nm.getNameForId("04-2021-10"), std::out_of_range); EXPECT_THROW(nm.getNameForId("04-2021-10"), std::out_of_range);
EXPECT_THROW(nm.getIdForName("zero_four_2021-10"), std::out_of_range); EXPECT_THROW(nm.getIdForName("zero_four_2021-10"), std::out_of_range);
@ -137,7 +142,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithAliases)
{ {
CapturedStderr nmUpdateStderror; CapturedStderr nmUpdateStderror;
lib.removeBookById("04-2021-10"); lib->removeBookById("04-2021-10");
nm.update(); nm.update();
EXPECT_EQ("", std::string(nmUpdateStderror)); EXPECT_EQ("", std::string(nmUpdateStderror));
} }

16
test/server_testing_tools.h
View File

@ -90,7 +90,7 @@ private:
void run(int serverPort, std::string indexTemplateString = ""); void run(int serverPort, std::string indexTemplateString = "");
private: // data private: // data
kiwix::Library library; std::shared_ptr<kiwix::Library> library;
kiwix::Manager manager; kiwix::Manager manager;
std::unique_ptr<kiwix::NameMapper> nameMapper; std::unique_ptr<kiwix::NameMapper> nameMapper;
std::unique_ptr<kiwix::Server> server; std::unique_ptr<kiwix::Server> server;
@ -99,8 +99,9 @@ private: // data
}; };
ZimFileServer::ZimFileServer(int serverPort, Options _options, std::string libraryFilePath) ZimFileServer::ZimFileServer(int serverPort, Options _options, std::string libraryFilePath)
: manager(&this->library) : library(new kiwix::Library()),
, options(_options) manager(library),
options(_options)
{ {
if ( kiwix::isRelativePath(libraryFilePath) ) if ( kiwix::isRelativePath(libraryFilePath) )
libraryFilePath = kiwix::computeAbsolutePath(kiwix::getCurrentDirectory(), libraryFilePath); libraryFilePath = kiwix::computeAbsolutePath(kiwix::getCurrentDirectory(), libraryFilePath);
@ -112,8 +113,9 @@ ZimFileServer::ZimFileServer(int serverPort,
Options _options, Options _options,
const FilePathCollection& zimpaths, const FilePathCollection& zimpaths,
std::string indexTemplateString) std::string indexTemplateString)
: manager(&this->library) : library(new kiwix::Library()),
, options(_options) manager(library),
options(_options)
{ {
for ( const auto& zimpath : zimpaths ) { for ( const auto& zimpath : zimpaths ) {
if (!manager.addBookFromPath(zimpath, zimpath, "", false)) if (!manager.addBookFromPath(zimpath, zimpath, "", false))
@ -128,9 +130,9 @@ void ZimFileServer::run(int serverPort, std::string indexTemplateString)
if (options & NO_NAME_MAPPER) { if (options & NO_NAME_MAPPER) {
nameMapper.reset(new kiwix::IdNameMapper()); nameMapper.reset(new kiwix::IdNameMapper());
} else { } else {
nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); nameMapper.reset(new kiwix::HumanReadableNameMapper(*library, false));
} }
kiwix::Server::Configuration configuration(&library, nameMapper.get()); kiwix::Server::Configuration configuration(library, nameMapper.get());
configuration.setRoot("ROOT") configuration.setRoot("ROOT")
.setAddress(address) .setAddress(address)
.setPort(serverPort) .setPort(serverPort)