Make the opds_dumper respect the provided nameMapper used in the server.

Fix #828

include/opds_dumper.h

@@ -27,6 +27,7 @@
 #include <pugixml.hpp>
 
 #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;

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& bookName)
 {
  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(bookName, 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<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;
  for ( const auto& bookId : bookIds ) {
  try {
  const Book book = library->getBookByIdThreadSafe(bookId);
+ const std::string bookName = nameMapper->getNameForId(bookId);
  const auto entryXML = partial
  ? partialEntryXML(book, rootLocation)
- : fullEntryXML(book, rootLocation);
+ : fullEntryXML(book, rootLocation, bookName);
  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<std::string>& 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<std::string>& bookIds, const s
 string OPDSDumper::dumpOPDSFeedV2(const std::vector<std::string>& 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<std::string>& bookIds, const
 std::string OPDSDumper::dumpOPDSCompleteEntry(const std::string& bookId) const
 {
  const auto book = library->getBookById(bookId);
+ const std::string bookName = nameMapper->getNameForId(bookId);
  return XML_HEADER
  + "\n"
- + fullEntryXML(book, rootLocation);
+ + fullEntryXML(book, rootLocation, bookName);
 }
 
 std::string OPDSDumper::categoriesOPDSFeed() const

src/server/internalServer.cpp

@@ -967,7 +967,7 @@ std::unique_ptr<Response> 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(m_library_id);
  std::vector<std::string> bookIdsToDump;

src/server/internalServer_catalog_v2.cpp

@@ -95,7 +95,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_root(const RequestCo
 
 std::unique_ptr<Response> 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(m_library_id);
  const auto bookIds = search_catalog(request, opdsDumper);
@@ -116,7 +116,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_complete_entry(const
  + urlNotFoundMsg;
  }
 
- OPDSDumper opdsDumper(mp_library);
+ OPDSDumper opdsDumper(mp_library, mp_nameMapper);
  opdsDumper.setRootLocation(m_root);
  opdsDumper.setLibraryId(m_library_id);
  const auto opdsFeed = opdsDumper.dumpOPDSCompleteEntry(entryId);
@@ -129,7 +129,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_complete_entry(const
 
 std::unique_ptr<Response> InternalServer::handle_catalog_v2_categories(const RequestContext& request)
 {
- OPDSDumper opdsDumper(mp_library);
+ OPDSDumper opdsDumper(mp_library, mp_nameMapper);
  opdsDumper.setRootLocation(m_root);
  opdsDumper.setLibraryId(m_library_id);
  return ContentResponse::build(
@@ -141,7 +141,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_categories(const Req
 
 std::unique_ptr<Response> InternalServer::handle_catalog_v2_languages(const RequestContext& request)
 {
- OPDSDumper opdsDumper(mp_library);
+ OPDSDumper opdsDumper(mp_library, mp_nameMapper);
  opdsDumper.setRootLocation(m_root);
  opdsDumper.setLibraryId(m_library_id);
  return ContentResponse::build(

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)
  " </entry>\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,14 @@ 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 _RAY_CHARLES_CATALOG_ENTRY(CONTENT_NAME) CATALOG_ENTRY(\
  "raycharles",\
  "Ray Charles",\
  "Wikipedia articles about Ray Charles",\
@@ -120,11 +127,13 @@ std::string maskVariableOPDSFeedData(std::string s)
  "<link rel=\"http://opds-spec.org/image/thumbnail\"\n" \
  " href=\"/ROOT/catalog/v2/illustration/raycharles/?size=48\"\n" \
  " type=\"image/png;width=48;height=48;scale=1\"/>\n ", \
- "zimfile", \
+ CONTENT_NAME, \
  "zimfile", \
  "569344"\
 )
 
+#define RAY_CHARLES_CATALOG_ENTRY _RAY_CHARLES_CATALOG_ENTRY("zimfile")
+
 #define UNCATEGORIZED_RAY_CHARLES_CATALOG_ENTRY CATALOG_ENTRY(\
  "raycharles_uncategorized",\
  "Ray (uncategorized) Charles",\
@@ -774,4 +783,42 @@ TEST_F(LibraryServerTest, catalog_search_excludes_hidden_tags)
 #undef EXPECT_ZERO_RESULTS
 }
 
+#define NO_MAPPER_CHARLES_RAY_CATALOG_ENTRY _CHARLES_RAY_CATALOG_ENTRY("charlesray")
+#define NO_MAPPER_RAY_CHARLES_CATALOG_ENTRY _RAY_CHARLES_CATALOG_ENTRY("raycharles")
+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
+ " <id>12345678-90ab-cdef-1234-567890abcdef</id>\n"
+ " <title>Filtered zims (tag=_category:jazz)</title>\n"
+ " <updated>YYYY-MM-DDThh:mm:ssZ</updated>\n"
+ " <totalResults>1</totalResults>\n"
+ " <startIndex>0</startIndex>\n"
+ " <itemsPerPage>1</itemsPerPage>\n"
+ CATALOG_LINK_TAGS
+ NO_MAPPER_CHARLES_RAY_CATALOG_ENTRY
+ "</feed>\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),
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+ NO_MAPPER_RAY_CHARLES_CATALOG_ENTRY
+ );
+
+ const auto r1 = zfs1_->GET("/ROOT/catalog/v2/entry/non-existent-entry");
+ EXPECT_EQ(r1->status, 404);
+}
+
+
+
 #undef EXPECT_SEARCH_RESULTS

test/server.cpp

@@ -131,6 +131,13 @@ TEST_F(ServerTest, 200)
  EXPECT_EQ(200, zfs1_->GET(res.url)->status) << "res.url: " << res.url;
 }
 
+TEST_F(ServerTest, 200_IdNameMapper)
+{
+ 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 ) {

test/server_testing_tools.h

@@ -61,14 +61,15 @@ 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,
 
  DEFAULT_OPTIONS = WITH_TASKBAR | WITH_LIBRARY_BUTTON
  };
 
 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<kiwix::HumanReadableNameMapper> nameMapper;
+ std::unique_ptr<kiwix::NameMapper> nameMapper;
  std::unique_ptr<kiwix::Server> server;
  std::unique_ptr<httplib::Client> 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);