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)
+OPDSDumper::OPDSDumper(Library* library, NameMapper* nameMapper)
-  : library(library)
+  : 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

@@ -19,7 +19,7 @@ protected:
 
 protected:
   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 {
@@ -774,4 +774,67 @@ TEST_F(LibraryServerTest, catalog_search_excludes_hidden_tags)
 #undef EXPECT_ZERO_RESULTS
 }
 
+
+// Same as CHARLES_RAY_CATALOG_ENTRY but with link using the uuid (charlesray).
+#define NO_MAPPER_CHARLES_RAY_CATALOG_ENTRY \
+    "  <entry>\n"                                                       \
+    "    <id>urn:uuid:charlesray</id>\n"                                \
+    "    <title>Charles, Ray</title>\n"                                 \
+    "    <updated>YYYY-MM-DDThh:mm:ssZ</updated>\n"                     \
+    "    <summary>Wikipedia articles about Ray Charles</summary>\n"     \
+    "    <language>fra</language>\n"                                    \
+    "    <name>wikipedia_fr_ray_charles</name>\n"                       \
+    "    <flavour></flavour>\n"                                         \
+    "    <category>jazz</category>\n"                                   \
+    "    <tags>unittest;wikipedia;_category:jazz;_pictures:no;_videos:no;_details:no;_ftindex:yes</tags>\n" \
+    "    <articleCount>284</articleCount>\n"                            \
+    "    <mediaCount>2</mediaCount>\n"                                  \
+    "    <link type=\"text/html\" href=\"/ROOT/content/charlesray\" />\n"               \
+    "    <author>\n"                                                    \
+    "      <name>Wikipedia</name>\n"                                    \
+    "    </author>\n"                                                   \
+    "    <publisher>\n"                                                 \
+    "      <name>Kiwix</name>\n"                                        \
+    "    </publisher>\n"                                                \
+    "    <dc:issued>2020-03-31T00:00:00Z</dc:issued>\n"                 \
+    "    <link rel=\"http://opds-spec.org/acquisition/open-access\" type=\"application/x-zim\" href=\"https://github.com/kiwix/libkiwix/raw/master/test/data/zimfile%26other.zim\" length=\"569344\" />\n" \
+    "  </entry>\n"
+
+class NoMapperLibraryServerTest : public ::testing::Test
+{
+protected:
+  std::unique_ptr<ZimFileServer>   zfs1_;
+
+  const int PORT = 8002;
+
+protected:
+  void SetUp() override {
+    zfs1_.reset(new ZimFileServer(PORT, ZimFileServer::NO_NAME_MAPPER, "./test/library.xml"));
+  }
+
+  void TearDown() override {
+    zfs1_.reset();
+  }
+};
+
+TEST_F(NoMapperLibraryServerTest, returned_catalog_use_uuid_in_link)
+{
+  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"
+  );
+}
+
+
+
 #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) << "url: /ROOT/content/6f1d19d0-633f-087b-fb55-7ac324ff9baf/A/index";
+  EXPECT_EQ(404, zfs1_->GET("/ROOT/content/zimfile/A/index")->status) << "url: /ROOT/content/zimfile/A/index";
+}
+
 TEST_F(ServerTest, CompressibleContentIsCompressedIfAcceptable)
 {
   for ( const Resource& res : resources200Compressible ) {

test/server_testing_tools.h

@@ -61,6 +61,7 @@ public: // types
     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 @@ public: // types
   };
 
 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 @@ private:
 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);