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

@@ -951,7 +951,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 @@ class LibraryServerTest : public ::testing::Test
 
 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,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);