diff --git a/include/opds_dumper.h b/include/opds_dumper.h index c824493de..cb6566f4f 100644 --- a/include/opds_dumper.h +++ b/include/opds_dumper.h @@ -27,6 +27,7 @@ #include #include "library.h" +#include "name_mapper.h" using namespace std; @@ -41,7 +42,7 @@ class OPDSDumper { public: OPDSDumper() = default; - OPDSDumper(Library* library); + OPDSDumper(Library* library, NameMapper* NameMapper); ~OPDSDumper(); /** @@ -110,6 +111,7 @@ class OPDSDumper protected: kiwix::Library* library; + kiwix::NameMapper* nameMapper; std::string libraryId; std::string rootLocation; int m_totalResults; diff --git a/src/opds_dumper.cpp b/src/opds_dumper.cpp index 7436ebc29..fc1f01c1b 100644 --- a/src/opds_dumper.cpp +++ b/src/opds_dumper.cpp @@ -30,8 +30,9 @@ namespace kiwix { /* Constructor */ -OPDSDumper::OPDSDumper(Library* library) - : library(library) +OPDSDumper::OPDSDumper(Library* library, NameMapper* nameMapper) + : library(library), + nameMapper(nameMapper) { } /* Destructor */ @@ -71,7 +72,7 @@ IllustrationInfo getBookIllustrationInfo(const Book& book) return illustrations; } -std::string fullEntryXML(const Book& book, const std::string& rootLocation) +std::string fullEntryXML(const Book& book, const std::string& rootLocation, const std::string& contentId) { const auto bookDate = book.getDate() + "T00:00:00Z"; const kainjow::mustache::object data{ @@ -81,7 +82,7 @@ std::string fullEntryXML(const Book& book, const std::string& rootLocation) {"title", book.getTitle()}, {"description", book.getDescription()}, {"language", book.getLanguage()}, - {"content_id", urlEncode(book.getHumanReadableIdFromPath(), true)}, + {"content_id", urlEncode(contentId, true)}, {"updated", bookDate}, // XXX: this should be the entry update datetime {"book_date", bookDate}, {"category", book.getCategory()}, @@ -112,15 +113,16 @@ std::string partialEntryXML(const Book& book, const std::string& rootLocation) return render_template(xmlTemplate, data); } -BooksData getBooksData(const Library* library, const std::vector& bookIds, const std::string& rootLocation, bool partial) +BooksData getBooksData(const Library* library, const NameMapper* nameMapper, const std::vector& bookIds, const std::string& rootLocation, bool partial) { BooksData booksData; for ( const auto& bookId : bookIds ) { try { const Book book = library->getBookByIdThreadSafe(bookId); + const std::string contentId = nameMapper->getNameForId(bookId); const auto entryXML = partial ? partialEntryXML(book, rootLocation) - : fullEntryXML(book, rootLocation); + : fullEntryXML(book, rootLocation, contentId); booksData.push_back(kainjow::mustache::object{ {"entry", entryXML} }); } catch ( const std::out_of_range& ) { // the book was removed from the library since its id was obtained @@ -188,7 +190,7 @@ std::string getLanguageSelfName(const std::string& lang) { string OPDSDumper::dumpOPDSFeed(const std::vector& bookIds, const std::string& query) const { - const auto booksData = getBooksData(library, bookIds, rootLocation, false); + const auto booksData = getBooksData(library, nameMapper, bookIds, rootLocation, false); const kainjow::mustache::object template_data{ {"date", gen_date_str()}, {"root", rootLocation}, @@ -206,7 +208,7 @@ string OPDSDumper::dumpOPDSFeed(const std::vector& bookIds, const s string OPDSDumper::dumpOPDSFeedV2(const std::vector& bookIds, const std::string& query, bool partial) const { const auto endpointRoot = rootLocation + "/catalog/v2"; - const auto booksData = getBooksData(library, bookIds, rootLocation, partial); + const auto booksData = getBooksData(library, nameMapper, bookIds, rootLocation, partial); const char* const endpoint = partial ? "/partial_entries" : "/entries"; const kainjow::mustache::object template_data{ @@ -228,9 +230,10 @@ string OPDSDumper::dumpOPDSFeedV2(const std::vector& bookIds, const std::string OPDSDumper::dumpOPDSCompleteEntry(const std::string& bookId) const { const auto book = library->getBookById(bookId); + const std::string contentId = nameMapper->getNameForId(bookId); return XML_HEADER + "\n" - + fullEntryXML(book, rootLocation); + + fullEntryXML(book, rootLocation, contentId); } std::string OPDSDumper::categoriesOPDSFeed() const diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index eeadd15d2..dd65d7e17 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -992,7 +992,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } zim::Uuid uuid; - kiwix::OPDSDumper opdsDumper(mp_library); + kiwix::OPDSDumper opdsDumper(mp_library, mp_nameMapper); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); std::vector bookIdsToDump; diff --git a/src/server/internalServer_catalog_v2.cpp b/src/server/internalServer_catalog_v2.cpp index 802d81cda..c0cb69ece 100644 --- a/src/server/internalServer_catalog_v2.cpp +++ b/src/server/internalServer_catalog_v2.cpp @@ -96,7 +96,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_root(const RequestCo std::unique_ptr InternalServer::handle_catalog_v2_entries(const RequestContext& request, bool partial) { - OPDSDumper opdsDumper(mp_library); + OPDSDumper opdsDumper(mp_library, mp_nameMapper); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); const auto bookIds = search_catalog(request, opdsDumper); @@ -117,7 +117,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const + urlNotFoundMsg; } - OPDSDumper opdsDumper(mp_library); + OPDSDumper opdsDumper(mp_library, mp_nameMapper); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); const auto opdsFeed = opdsDumper.dumpOPDSCompleteEntry(entryId); @@ -130,7 +130,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const std::unique_ptr InternalServer::handle_catalog_v2_categories(const RequestContext& request) { - OPDSDumper opdsDumper(mp_library); + OPDSDumper opdsDumper(mp_library, mp_nameMapper); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( @@ -142,7 +142,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_categories(const Req std::unique_ptr InternalServer::handle_catalog_v2_languages(const RequestContext& request) { - OPDSDumper opdsDumper(mp_library); + OPDSDumper opdsDumper(mp_library, mp_nameMapper); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( diff --git a/test/library_server.cpp b/test/library_server.cpp index bed7806bc..fda5916b4 100644 --- a/test/library_server.cpp +++ b/test/library_server.cpp @@ -18,8 +18,13 @@ class LibraryServerTest : public ::testing::Test const int PORT = 8002; protected: + void resetServer(ZimFileServer::Options options) { + zfs1_.reset(); + zfs1_.reset(new ZimFileServer(PORT, options, "./test/library.xml")); + } + void SetUp() override { - zfs1_.reset(new ZimFileServer(PORT, "./test/library.xml")); + zfs1_.reset(new ZimFileServer(PORT, ZimFileServer::DEFAULT_OPTIONS, "./test/library.xml")); } void TearDown() override { @@ -95,7 +100,7 @@ std::string maskVariableOPDSFeedData(std::string s) " \n" -#define CHARLES_RAY_CATALOG_ENTRY CATALOG_ENTRY( \ +#define _CHARLES_RAY_CATALOG_ENTRY(CONTENT_NAME) CATALOG_ENTRY( \ "charlesray", \ "Charles, Ray", \ "Wikipedia articles about Ray Charles", \ @@ -104,12 +109,15 @@ std::string maskVariableOPDSFeedData(std::string s) "jazz",\ "unittest;wikipedia;_category:jazz;_pictures:no;_videos:no;_details:no;_ftindex:yes",\ "", \ - "zimfile%26other", \ + CONTENT_NAME, \ "zimfile%26other", \ "569344" \ ) -#define RAY_CHARLES_CATALOG_ENTRY CATALOG_ENTRY(\ +#define CHARLES_RAY_CATALOG_ENTRY _CHARLES_RAY_CATALOG_ENTRY("zimfile%26other") +#define CHARLES_RAY_CATALOG_ENTRY_NO_MAPPER _CHARLES_RAY_CATALOG_ENTRY("charlesray") + +#define _RAY_CHARLES_CATALOG_ENTRY(CONTENT_NAME) CATALOG_ENTRY(\ "raycharles",\ "Ray Charles",\ "Wikipedia articles about Ray Charles",\ @@ -120,11 +128,14 @@ std::string maskVariableOPDSFeedData(std::string s) "\n ", \ - "zimfile", \ + CONTENT_NAME, \ "zimfile", \ "569344"\ ) +#define RAY_CHARLES_CATALOG_ENTRY _RAY_CHARLES_CATALOG_ENTRY("zimfile") +#define RAY_CHARLES_CATALOG_ENTRY_NO_MAPPER _RAY_CHARLES_CATALOG_ENTRY("raycharles") + #define UNCATEGORIZED_RAY_CHARLES_CATALOG_ENTRY CATALOG_ENTRY(\ "raycharles_uncategorized",\ "Ray (uncategorized) Charles",\ @@ -774,4 +785,40 @@ TEST_F(LibraryServerTest, catalog_search_excludes_hidden_tags) #undef EXPECT_ZERO_RESULTS } +TEST_F(LibraryServerTest, no_name_mapper_returned_catalog_use_uuid_in_link) +{ + resetServer(ZimFileServer::NO_NAME_MAPPER); + const auto r = zfs1_->GET("/ROOT/catalog/search?tag=_category:jazz"); + EXPECT_EQ(r->status, 200); + EXPECT_EQ(maskVariableOPDSFeedData(r->body), + OPDS_FEED_TAG + " 12345678-90ab-cdef-1234-567890abcdef\n" + " Filtered zims (tag=_category:jazz)\n" + " YYYY-MM-DDThh:mm:ssZ\n" + " 1\n" + " 0\n" + " 1\n" + CATALOG_LINK_TAGS + CHARLES_RAY_CATALOG_ENTRY_NO_MAPPER + "\n" + ); +} + + +TEST_F(LibraryServerTest, no_name_mapper_catalog_v2_individual_entry_access) +{ + resetServer(ZimFileServer::NO_NAME_MAPPER); + const auto r = zfs1_->GET("/ROOT/catalog/v2/entry/raycharles"); + EXPECT_EQ(r->status, 200); + EXPECT_EQ(maskVariableOPDSFeedData(r->body), + "\n" + RAY_CHARLES_CATALOG_ENTRY_NO_MAPPER + ); + + const auto r1 = zfs1_->GET("/ROOT/catalog/v2/entry/non-existent-entry"); + EXPECT_EQ(r1->status, 404); +} + + + #undef EXPECT_SEARCH_RESULTS diff --git a/test/server.cpp b/test/server.cpp index 0bb97f76a..c774d0f1a 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -209,6 +209,15 @@ TEST_F(ServerTest, 200) EXPECT_EQ(200, zfs1_->GET(res.url)->status) << "res.url: " << res.url; } +TEST_F(ServerTest, 200_IdNameMapper) +{ + EXPECT_EQ(404, zfs1_->GET("/ROOT/content/6f1d19d0-633f-087b-fb55-7ac324ff9baf/A/index")->status); + EXPECT_EQ(200, zfs1_->GET("/ROOT/content/zimfile/A/index")->status); + resetServer(ZimFileServer::NO_NAME_MAPPER); + EXPECT_EQ(200, zfs1_->GET("/ROOT/content/6f1d19d0-633f-087b-fb55-7ac324ff9baf/A/index")->status); + EXPECT_EQ(404, zfs1_->GET("/ROOT/content/zimfile/A/index")->status); +} + TEST_F(ServerTest, CompressibleContentIsCompressedIfAcceptable) { for ( const Resource& res : resources200Compressible ) { diff --git a/test/server_testing_tools.h b/test/server_testing_tools.h index ab0b4a511..7af5834e6 100644 --- a/test/server_testing_tools.h +++ b/test/server_testing_tools.h @@ -61,6 +61,7 @@ class ZimFileServer WITH_TASKBAR = 1 << 1, WITH_LIBRARY_BUTTON = 1 << 2, BLOCK_EXTERNAL_LINKS = 1 << 3, + NO_NAME_MAPPER = 1 << 4, WITH_TASKBAR_AND_LIBRARY_BUTTON = WITH_TASKBAR | WITH_LIBRARY_BUTTON, @@ -68,7 +69,7 @@ class ZimFileServer }; public: // functions - ZimFileServer(int serverPort, std::string libraryFilePath); + ZimFileServer(int serverPort, Options options, std::string libraryFilePath); ZimFileServer(int serverPort, Options options, const FilePathCollection& zimpaths, @@ -91,14 +92,15 @@ class ZimFileServer private: // data kiwix::Library library; kiwix::Manager manager; - std::unique_ptr nameMapper; + std::unique_ptr nameMapper; std::unique_ptr server; std::unique_ptr client; const Options options = DEFAULT_OPTIONS; }; -ZimFileServer::ZimFileServer(int serverPort, std::string libraryFilePath) +ZimFileServer::ZimFileServer(int serverPort, Options _options, std::string libraryFilePath) : manager(&this->library) +, options(_options) { if ( kiwix::isRelativePath(libraryFilePath) ) libraryFilePath = kiwix::computeAbsolutePath(kiwix::getCurrentDirectory(), libraryFilePath); @@ -123,7 +125,11 @@ ZimFileServer::ZimFileServer(int serverPort, void ZimFileServer::run(int serverPort, std::string indexTemplateString) { const std::string address = "127.0.0.1"; - nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); + if (options & NO_NAME_MAPPER) { + nameMapper.reset(new kiwix::IdNameMapper()); + } else { + nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); + } server.reset(new kiwix::Server(&library, nameMapper.get())); server->setRoot("ROOT"); server->setAddress(address);