fixup! Do not store raw pointer to Library.

This commit is contained in:
Matthieu Gautier 2022-10-12 16:53:27 +02:00
parent 0bd5a5ec3b
commit 4842fa0355
5 changed files with 108 additions and 85 deletions

View File

@ -20,6 +20,7 @@
#include "gtest/gtest.h"
#include <string>
#include "testing_tools.h"
const char * sampleOpdsStream = R"(
<feed xmlns="http://www.w3.org/2005/Atom"
@ -236,14 +237,14 @@ namespace
TEST(LibraryOpdsImportTest, allInOne)
{
auto lib = std::make_shared<kiwix::Library>();
kiwix::Manager manager(lib);
kiwix::Library lib;
kiwix::Manager manager{NotOwned<kiwix::Library>(lib)};
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");
@ -268,7 +269,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(), "");
@ -298,11 +299,10 @@ class LibraryTest : public ::testing::Test {
typedef std::vector<std::string> TitleCollection;
explicit LibraryTest()
: lib(std::make_shared<kiwix::Library>())
{}
void SetUp() override {
kiwix::Manager manager(lib);
kiwix::Manager manager{NotOwned<kiwix::Library>(lib)};
manager.readOpds(sampleOpdsStream, "foo.urlHost");
manager.readXml(sampleLibraryXML, false, "./test/library.xml", true);
}
@ -316,25 +316,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;
}
std::shared_ptr<kiwix::Library> lib;
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 +346,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"})
);
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 +361,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 +371,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 +736,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 +770,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,12 +800,12 @@ 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_EQ(9u, lib->removeBooksNotUpdatedSince(rev));
EXPECT_EQ(9u, lib.removeBooksNotUpdatedSince(rev));
EXPECT_FILTER_RESULTS(kiwix::Filter(),
"Islam Stack Exchange",

View File

@ -6,20 +6,22 @@
#include <iostream>
#include <fstream>
#include "testing_tools.h"
TEST(ManagerTest, addBookFromPathAndGetIdTest)
{
auto lib = std::make_shared<kiwix::Library>();
kiwix::Manager manager = kiwix::Manager(lib);
kiwix::Library lib;
kiwix::Manager manager = kiwix::Manager(NotOwned<kiwix::Library>(lib));
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 +50,11 @@ const char sampleLibraryXML[] = R"(
TEST(ManagerTest, readXml)
{
auto lib = std::make_shared<kiwix::Library>();
kiwix::Manager manager = kiwix::Manager(lib);
kiwix::Library lib;
kiwix::Manager manager = kiwix::Manager(NotOwned<kiwix::Library>(lib));
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 +72,24 @@ TEST(ManagerTest, readXml)
TEST(Manager, reload)
{
auto lib = std::make_shared<kiwix::Library>();
kiwix::Manager manager(lib);
kiwix::Library lib;
kiwix::Manager manager = kiwix::Manager(NotOwned<kiwix::Library>(lib));
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

@ -4,9 +4,12 @@
#include "../include/manager.h"
#include "gtest/gtest.h"
#include "testing_tools.h"
namespace
{
const char libraryXML[] = R"(
<library version="1.0">
<book id="01" path="/data/zero_one.zim"> </book>
@ -19,22 +22,21 @@ const char libraryXML[] = R"(
class NameMapperTest : public ::testing::Test {
public:
explicit NameMapperTest() :
lib(std::make_shared<kiwix::Library>())
explicit NameMapperTest()
{}
protected:
void SetUp() override {
kiwix::Manager manager(lib);
kiwix::Manager manager{NotOwned<kiwix::Library>(lib)};
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);
}
}
std::shared_ptr<kiwix::Library> lib;
kiwix::Library lib;
};
class CapturedStderr
@ -78,13 +80,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);
@ -93,7 +95,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'."
@ -104,7 +106,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"));
@ -113,13 +115,13 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases)
{
CapturedStderr stderror;
kiwix::UpdatableNameMapper nm(lib, false);
kiwix::UpdatableNameMapper nm(NotOwned<kiwix::Library>(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);
@ -129,7 +131,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases)
TEST_F(NameMapperTest, UpdatableNameMapperWithAliases)
{
CapturedStderr stderror;
kiwix::UpdatableNameMapper nm(lib, true);
kiwix::UpdatableNameMapper nm(NotOwned<kiwix::Library>(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'."
@ -142,7 +144,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

@ -6,6 +6,8 @@
#include <zim/entry.h>
#include <zim/item.h>
#include "testing_tools.h"
// Output generated via mustache templates sometimes contains end-of-line
// whitespace. This complicates representing the expected output of a unit-test
// as C++ raw strings in editors that are configured to delete EOL whitespace.
@ -90,7 +92,7 @@ private:
void run(int serverPort, std::string indexTemplateString = "");
private: // data
std::shared_ptr<kiwix::Library> library;
kiwix::Library library;
kiwix::Manager manager;
std::unique_ptr<kiwix::NameMapper> nameMapper;
std::unique_ptr<kiwix::Server> server;
@ -99,8 +101,7 @@ private: // data
};
ZimFileServer::ZimFileServer(int serverPort, Options _options, std::string libraryFilePath)
: library(new kiwix::Library()),
manager(library),
: manager(NotOwned<kiwix::Library>(library)),
options(_options)
{
if ( kiwix::isRelativePath(libraryFilePath) )
@ -113,8 +114,7 @@ ZimFileServer::ZimFileServer(int serverPort,
Options _options,
const FilePathCollection& zimpaths,
std::string indexTemplateString)
: library(new kiwix::Library()),
manager(library),
: manager(NotOwned<kiwix::Library>(library)),
options(_options)
{
for ( const auto& zimpath : zimpaths ) {
@ -130,9 +130,9 @@ void ZimFileServer::run(int serverPort, std::string indexTemplateString)
if (options & NO_NAME_MAPPER) {
nameMapper.reset(new kiwix::IdNameMapper());
} 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(NotOwned<kiwix::Library>(library), nameMapper.get());
configuration.setRoot("ROOT")
.setAddress(address)
.setPort(serverPort)

19
test/testing_tools.h Normal file
View File

@ -0,0 +1,19 @@
#ifndef TESTING_TOOLS
#define TESTING_TOOLS
#include <memory>
struct NoDelete {
template<class T> void operator()(T*) {}
};
template<class T>
class NotOwned : public std::shared_ptr<T> {
public:
NotOwned(T& object) :
std::shared_ptr<T>(&object, NoDelete()) {};
};
#endif // TESTING_TOOLS