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

Fix #828
2022-10-05 15:41:04 +02:00 · 2022-10-05 15:41:04 +02:00 · e5b94fa1bb
parent b0d719431d
commit e5b94fa1bb
7 changed files with 91 additions and 24 deletions
--- a/include/opds_dumper.h
+++ b/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;
--- 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<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 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<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 contentId = nameMapper->getNameForId(bookId);
  return XML_HEADER
         + "\n"
-         + fullEntryXML(book, rootLocation);
+         + fullEntryXML(book, rootLocation, contentId);
 }

 std::string OPDSDumper::categoriesOPDSFeed() const
--- a/src/server/internalServer.cpp
+++ b/src/server/internalServer.cpp
@ -992,7 +992,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(getLibraryId());
  std::vector<std::string> bookIdsToDump;
--- a/src/server/internalServer_catalog_v2.cpp
+++ b/src/server/internalServer_catalog_v2.cpp
@ -96,7 +96,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(getLibraryId());
  const auto bookIds = search_catalog(request, opdsDumper);
@ -117,7 +117,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(getLibraryId());
  const auto opdsFeed = opdsDumper.dumpOPDSCompleteEntry(entryId);
@ -130,7 +130,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(getLibraryId());
  return ContentResponse::build(
@ -142,7 +142,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(getLibraryId());
  return ContentResponse::build(
--- a/test/library_server.cpp
+++ b/test/library_server.cpp
@ -18,8 +18,13 @@ protected:
  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,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)
  "<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 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
+    "  <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
+    CHARLES_RAY_CATALOG_ENTRY_NO_MAPPER
+    "</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"
+    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
--- 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 ) {
--- a/test/server_testing_tools.h
+++ b/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);