From 22e5327dcf4dc6421c9bc690062747237d046084 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 8 Dec 2021 11:37:35 +0100 Subject: [PATCH 1/5] Do not create a dummy illustration if library.xml doesn't contain one. Fix #644 --- src/book.cpp | 13 ++++++++----- static/templates/catalog_v2_entry.xml | 6 ++---- test/data/library.xml | 2 ++ test/server.cpp | 6 ------ 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/book.cpp b/src/book.cpp index 13128565e..5e1952a43 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -123,11 +123,14 @@ void Book::updateFromXml(const pugi::xml_node& node, const std::string& baseDir) m_articleCount = strtoull(ATTR("articleCount"), 0, 0); m_mediaCount = strtoull(ATTR("mediaCount"), 0, 0); m_size = strtoull(ATTR("size"), 0, 0) << 10; - const auto favicon = std::make_shared(); - favicon->data = base64_decode(ATTR("favicon")); - favicon->mimeType = ATTR("faviconMimeType"); - favicon->url = ATTR("faviconUrl"); - m_illustrations.assign(1, favicon); + std::string favicon_mimetype = ATTR("faviconMimeType"); + if (! favicon_mimetype.empty()) { + const auto favicon = std::make_shared(); + favicon->data = base64_decode(ATTR("favicon")); + favicon->mimeType = favicon_mimetype; + favicon->url = ATTR("faviconUrl"); + m_illustrations.assign(1, favicon); + } try { m_downloadId = ATTR("downloadId"); } catch(...) {} diff --git a/static/templates/catalog_v2_entry.xml b/static/templates/catalog_v2_entry.xml index 33abd56d6..f86a477f0 100644 --- a/static/templates/catalog_v2_entry.xml +++ b/static/templates/catalog_v2_entry.xml @@ -15,12 +15,10 @@ {{tags}} {{article_count}} {{media_count}} - {{#icons}} - - {{/icons}} - + {{/icons}} {{author_name}} diff --git a/test/data/library.xml b/test/data/library.xml index 52cd9799b..579fb3848 100644 --- a/test/data/library.xml +++ b/test/data/library.xml @@ -14,6 +14,8 @@ articleCount="284" mediaCount="2" size="556" + faviconMimeType="image/png" + favicon="SOME DATA" > unittest;wikipedia;_category:jazz;_pictures:no;_videos:no;_details:no;_ftindex:yes\n" \ " 284\n" \ " 2\n" \ - " \n" \ " \n" \ " \n" \ " Wikipedia\n" \ @@ -711,9 +708,6 @@ std::string maskVariableOPDSFeedData(std::string s) " unittest;wikipedia;_pictures:no;_videos:no;_details:no\n" \ " 284\n" \ " 2\n" \ - " \n" \ " \n" \ " \n" \ " Wikipedia\n" \ From 66c40817eed316ea79a2f4f01d8be1e2160b6742 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 17 Nov 2021 15:13:21 +0100 Subject: [PATCH 2/5] =?UTF-8?q?Fix=20the=20OPDS=C2=A0stream=20to=20handle?= =?UTF-8?q?=20custom=20ROOT=20prefix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we render the entry's xml in a separated steps, we need to pass the rootLocation to all the internal rendering. Testing with and without root is not so easy. I've simply made all server tests using a ROOT prefix. We can assume that if the ROOT is present everywhere we need it, it will not when we don't need. (As long as we don't hardcode "ROOT" in the server.) --- src/opds_dumper.cpp | 13 +- static/templates/catalog_v2_entry.xml | 4 +- test/server.cpp | 245 +++++++++++++------------- 3 files changed, 132 insertions(+), 130 deletions(-) diff --git a/src/opds_dumper.cpp b/src/opds_dumper.cpp index 3570633c2..229bc92e2 100644 --- a/src/opds_dumper.cpp +++ b/src/opds_dumper.cpp @@ -95,23 +95,24 @@ kainjow::mustache::object getSingleBookData(const Book& book) }; } -std::string getSingleBookEntryXML(const Book& book, bool withXMLHeader, const std::string& endpointRoot, bool partial) +std::string getSingleBookEntryXML(const Book& book, bool withXMLHeader, const std::string& rootLocation, const std::string& endpointRoot, bool partial) { auto data = getSingleBookData(book); data["with_xml_header"] = MustacheData(withXMLHeader); data["dump_partial_entries"] = MustacheData(partial); data["endpoint_root"] = endpointRoot; + data["root"] = rootLocation; return render_template(RESOURCE::templates::catalog_v2_entry_xml, data); } -BooksData getBooksData(const Library* library, const std::vector& bookIds, const std::string& endpointRoot, bool partial) +BooksData getBooksData(const Library* library, const std::vector& bookIds, const std::string& rootLocation, const std::string& endpointRoot, bool partial) { BooksData booksData; for ( const auto& bookId : bookIds ) { try { const Book book = library->getBookByIdThreadSafe(bookId); booksData.push_back(kainjow::mustache::object{ - {"entry", getSingleBookEntryXML(book, false, endpointRoot, partial)} + {"entry", getSingleBookEntryXML(book, false, rootLocation, endpointRoot, partial)} }); } catch ( const std::out_of_range& ) { // the book was removed from the library since its id was obtained @@ -135,7 +136,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, "", false); + const auto booksData = getBooksData(library, bookIds, rootLocation, "", false); const kainjow::mustache::object template_data{ {"date", gen_date_str()}, {"root", rootLocation}, @@ -153,7 +154,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, endpointRoot, partial); + const auto booksData = getBooksData(library, bookIds, rootLocation, endpointRoot, partial); const char* const endpoint = partial ? "/partial_entries" : "/entries"; const kainjow::mustache::object template_data{ @@ -174,7 +175,7 @@ string OPDSDumper::dumpOPDSFeedV2(const std::vector& bookIds, const std::string OPDSDumper::dumpOPDSCompleteEntry(const std::string& bookId) const { - return getSingleBookEntryXML(library->getBookById(bookId), true, "", false); + return getSingleBookEntryXML(library->getBookById(bookId), true, rootLocation, "", false); } std::string OPDSDumper::categoriesOPDSFeed() const diff --git a/static/templates/catalog_v2_entry.xml b/static/templates/catalog_v2_entry.xml index f86a477f0..143872635 100644 --- a/static/templates/catalog_v2_entry.xml +++ b/static/templates/catalog_v2_entry.xml @@ -16,9 +16,9 @@ {{article_count}} {{media_count}} {{#icons}} - {{/icons}} + {{/icons}} {{author_name}} diff --git a/test/server.cpp b/test/server.cpp index ecdd212b1..5ae6296c9 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -84,7 +84,6 @@ ZimFileServer::ZimFileServer(int serverPort, std::string libraryFilePath) if ( kiwix::isRelativePath(libraryFilePath) ) libraryFilePath = kiwix::computeAbsolutePath(kiwix::getCurrentDirectory(), libraryFilePath); manager.readFile(libraryFilePath, true, true); - run(serverPort); } @@ -95,7 +94,6 @@ ZimFileServer::ZimFileServer(int serverPort, const FilePathCollection& zimpaths, if (!manager.addBookFromPath(zimpath, zimpath, "", false)) throw std::runtime_error("Unable to add the ZIM file '" + zimpath + "'"); } - run(serverPort, indexTemplateString); } @@ -104,6 +102,7 @@ void ZimFileServer::run(int serverPort, std::string indexTemplateString) const std::string address = "127.0.0.1"; nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); server.reset(new kiwix::Server(&library, nameMapper.get())); + server->setRoot("ROOT"); server->setAddress(address); server->setPort(serverPort); server->setNbThreads(2); @@ -162,52 +161,52 @@ std::ostream& operator<<(std::ostream& out, const Resource& r) typedef std::vector ResourceCollection; const ResourceCollection resources200Compressible{ - { WITH_ETAG, "/" }, + { WITH_ETAG, "/ROOT/" }, - { WITH_ETAG, "/skin/jquery-ui/jquery-ui.structure.min.css" }, - { WITH_ETAG, "/skin/jquery-ui/jquery-ui.min.js" }, - { WITH_ETAG, "/skin/jquery-ui/external/jquery/jquery.js" }, - { WITH_ETAG, "/skin/jquery-ui/jquery-ui.theme.min.css" }, - { WITH_ETAG, "/skin/jquery-ui/jquery-ui.min.css" }, - { WITH_ETAG, "/skin/taskbar.js" }, - { WITH_ETAG, "/skin/taskbar.css" }, - { WITH_ETAG, "/skin/block_external.js" }, + { WITH_ETAG, "/ROOT/skin/jquery-ui/jquery-ui.structure.min.css" }, + { WITH_ETAG, "/ROOT/skin/jquery-ui/jquery-ui.min.js" }, + { WITH_ETAG, "/ROOT/skin/jquery-ui/external/jquery/jquery.js" }, + { WITH_ETAG, "/ROOT/skin/jquery-ui/jquery-ui.theme.min.css" }, + { WITH_ETAG, "/ROOT/skin/jquery-ui/jquery-ui.min.css" }, + { WITH_ETAG, "/ROOT/skin/taskbar.js" }, + { WITH_ETAG, "/ROOT/skin/taskbar.css" }, + { WITH_ETAG, "/ROOT/skin/block_external.js" }, - { NO_ETAG, "/catalog/root.xml" }, - { NO_ETAG, "/catalog/searchdescription.xml" }, - { NO_ETAG, "/catalog/search" }, + { NO_ETAG, "/ROOT/catalog/root.xml" }, + { NO_ETAG, "/ROOT/catalog/searchdescription.xml" }, + { NO_ETAG, "/ROOT/catalog/search" }, - { NO_ETAG, "/search?content=zimfile&pattern=a" }, + { NO_ETAG, "/ROOT/search?content=zimfile&pattern=a" }, - { NO_ETAG, "/suggest?content=zimfile" }, - { NO_ETAG, "/suggest?content=zimfile&term=ray" }, + { NO_ETAG, "/ROOT/suggest?content=zimfile" }, + { NO_ETAG, "/ROOT/suggest?content=zimfile&term=ray" }, - { NO_ETAG, "/catch/external?source=www.example.com" }, + { NO_ETAG, "/ROOT/catch/external?source=www.example.com" }, - { WITH_ETAG, "/zimfile/A/index" }, - { WITH_ETAG, "/zimfile/A/Ray_Charles" }, + { WITH_ETAG, "/ROOT/zimfile/A/index" }, + { WITH_ETAG, "/ROOT/zimfile/A/Ray_Charles" }, }; const ResourceCollection resources200Uncompressible{ - { WITH_ETAG, "/skin/jquery-ui/images/animated-overlay.gif" }, - { WITH_ETAG, "/skin/caret.png" }, + { WITH_ETAG, "/ROOT/skin/jquery-ui/images/animated-overlay.gif" }, + { WITH_ETAG, "/ROOT/skin/caret.png" }, - { WITH_ETAG, "/meta?content=zimfile&name=title" }, - { WITH_ETAG, "/meta?content=zimfile&name=description" }, - { WITH_ETAG, "/meta?content=zimfile&name=language" }, - { WITH_ETAG, "/meta?content=zimfile&name=name" }, - { WITH_ETAG, "/meta?content=zimfile&name=tags" }, - { WITH_ETAG, "/meta?content=zimfile&name=date" }, - { WITH_ETAG, "/meta?content=zimfile&name=creator" }, - { WITH_ETAG, "/meta?content=zimfile&name=publisher" }, - { WITH_ETAG, "/meta?content=zimfile&name=favicon" }, - { WITH_ETAG, "/meta?content=zimfile&name=Illustration_48x48@1" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=title" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=description" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=language" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=name" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=tags" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=date" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=creator" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=publisher" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=favicon" }, + { WITH_ETAG, "/ROOT/meta?content=zimfile&name=Illustration_48x48@1" }, - { WITH_ETAG, "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg" }, + { WITH_ETAG, "/ROOT/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg" }, - { WITH_ETAG, "/corner_cases/A/empty.html" }, - { WITH_ETAG, "/corner_cases/-/empty.css" }, - { WITH_ETAG, "/corner_cases/-/empty.js" }, + { WITH_ETAG, "/ROOT/corner_cases/A/empty.html" }, + { WITH_ETAG, "/ROOT/corner_cases/-/empty.css" }, + { WITH_ETAG, "/ROOT/corner_cases/-/empty.js" }, }; ResourceCollection all200Resources() @@ -223,7 +222,7 @@ TEST(indexTemplateStringTest, emptyIndexTemplate) { }; ZimFileServer zfs(PORT, ZIMFILES, ""); - EXPECT_EQ(200, zfs.GET("/")->status); + EXPECT_EQ(200, zfs.GET("/ROOT/")->status); } TEST(indexTemplateStringTest, indexTemplateCheck) { @@ -239,9 +238,9 @@ TEST(indexTemplateStringTest, indexTemplateCheck) { ""); EXPECT_EQ("" "Welcome to kiwix library" - "" + "" "" - "", zfs.GET("/")->body); + "", zfs.GET("/ROOT/")->body); } TEST_F(ServerTest, 200) @@ -270,22 +269,24 @@ TEST_F(ServerTest, UncompressibleContentIsNotCompressed) } const char* urls404[] = { - "/non-existent-item", - "/skin/non-existent-skin-resource", - "/catalog", - "/catalog/non-existent-item", - "/catalogBLABLABLA/root.xml", - "/meta", - "/meta?content=zimfile", - "/meta?content=zimfile&name=non-existent-item", - "/meta?content=non-existent-book&name=title", - "/random", - "/random?content=non-existent-book", - "/search", - "/suggest", - "/suggest?content=non-existent-book&term=abcd", - "/catch/external", - "/zimfile/A/non-existent-article", + "/", + "/zimfile", + "/ROOT/non-existent-item", + "/ROOT/skin/non-existent-skin-resource", + "/ROOT/catalog", + "/ROOT/catalog/non-existent-item", + "/ROOT/catalogBLABLABLA/root.xml", + "/ROOT/meta", + "/ROOT/meta?content=zimfile", + "/ROOT/meta?content=zimfile&name=non-existent-item", + "/ROOT/meta?content=non-existent-book&name=title", + "/ROOT/random", + "/ROOT/random?content=non-existent-book", + "/ROOT/search", + "/ROOT/suggest", + "/ROOT/suggest?content=non-existent-book&term=abcd", + "/ROOT/catch/external", + "/ROOT/zimfile/A/non-existent-article", }; TEST_F(ServerTest, 404) @@ -296,7 +297,7 @@ TEST_F(ServerTest, 404) TEST_F(ServerTest, RandomPageRedirectsToAnExistingArticle) { - auto g = zfs1_->GET("/random?content=zimfile"); + auto g = zfs1_->GET("/ROOT/random?content=zimfile"); ASSERT_EQ(302, g->status); ASSERT_TRUE(g->has_header("Location")); ASSERT_TRUE(g->get_header_value("Location").find("/zimfile/A/") != std::string::npos); @@ -304,10 +305,10 @@ TEST_F(ServerTest, RandomPageRedirectsToAnExistingArticle) TEST_F(ServerTest, BookMainPageIsRedirectedToArticleIndex) { - auto g = zfs1_->GET("/zimfile"); + auto g = zfs1_->GET("/ROOT/zimfile"); ASSERT_EQ(302, g->status); ASSERT_TRUE(g->has_header("Location")); - ASSERT_EQ("/zimfile/A/index", g->get_header_value("Location")); + ASSERT_EQ("/ROOT/zimfile/A/index", g->get_header_value("Location")); } TEST_F(ServerTest, HeadMethodIsSupported) @@ -466,7 +467,7 @@ TEST_F(ServerTest, IfNoneMatchRequestsWithMismatchingETagResultIn200Responses) TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) { - const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + const char url[] = "/ROOT/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; const auto full = zfs1_->GET(url); EXPECT_FALSE(full->has_header("Content-Range")); EXPECT_EQ("bytes", full->get_header_value("Accept-Ranges")); @@ -516,7 +517,7 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly) TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses) { - const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + const char url[] = "/ROOT/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; const char* invalidRanges[] = { "0-10", "bytes=", "bytes=123", "bytes=-10-20", "bytes=10-20xxx", @@ -537,7 +538,7 @@ TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses) TEST_F(ServerTest, ValidByteRangeRequestsOfZeroSizedEntriesResultIn416Responses) { - const char url[] = "/corner_cases/-/empty.js"; + const char url[] = "/ROOT/corner_cases/-/empty.js"; const char* ranges[] = { "bytes=0-", @@ -556,7 +557,7 @@ TEST_F(ServerTest, ValidByteRangeRequestsOfZeroSizedEntriesResultIn416Responses) TEST_F(ServerTest, RangeHasPrecedenceOverCompression) { - const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + const char url[] = "/ROOT/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; const Headers onlyRange{ {"Range", "bytes=123-456"} }; Headers rangeAndCompression(onlyRange); @@ -571,7 +572,7 @@ TEST_F(ServerTest, RangeHasPrecedenceOverCompression) TEST_F(ServerTest, RangeHeaderIsCaseInsensitive) { - const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; + const char url[] = "/ROOT/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg"; const auto r0 = zfs1_->GET(url, { {"Range", "bytes=100-200"} } ); const char* header_variations[] = { "RANGE", "range", "rAnGe", "RaNgE" }; @@ -644,7 +645,7 @@ std::string maskVariableOPDSFeedData(std::string s) " \n" \ " \n" + " href=\"/ROOT/catalog/searchdescription.xml\" />\n" #define CHARLES_RAY_CATALOG_ENTRY \ " \n" \ @@ -659,7 +660,7 @@ std::string maskVariableOPDSFeedData(std::string s) " unittest;wikipedia;_category:jazz;_pictures:no;_videos:no;_details:no;_ftindex:yes\n" \ " 284\n" \ " 2\n" \ - " \n" \ + " \n" \ " \n" \ " Wikipedia\n" \ " \n" \ @@ -683,9 +684,9 @@ std::string maskVariableOPDSFeedData(std::string s) " 284\n" \ " 2\n" \ " \n" \ - " \n" \ + " \n" \ " \n" \ " Wikipedia\n" \ " \n" \ @@ -708,7 +709,7 @@ std::string maskVariableOPDSFeedData(std::string s) " unittest;wikipedia;_pictures:no;_videos:no;_details:no\n" \ " 284\n" \ " 2\n" \ - " \n" \ + " \n" \ " \n" \ " Wikipedia\n" \ " \n" \ @@ -720,7 +721,7 @@ std::string maskVariableOPDSFeedData(std::string s) TEST_F(LibraryServerTest, catalog_root_xml) { - const auto r = zfs1_->GET("/catalog/root.xml"); + const auto r = zfs1_->GET("/ROOT/catalog/root.xml"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -738,7 +739,7 @@ TEST_F(LibraryServerTest, catalog_root_xml) TEST_F(LibraryServerTest, catalog_searchdescription_xml) { - const auto r = zfs1_->GET("/catalog/searchdescription.xml"); + const auto r = zfs1_->GET("/ROOT/catalog/searchdescription.xml"); EXPECT_EQ(r->status, 200); EXPECT_EQ(r->body, "\n" @@ -749,14 +750,14 @@ TEST_F(LibraryServerTest, catalog_searchdescription_xml) " xmlns:atom=\"http://www.w3.org/2005/Atom\"\n" " xmlns:k=\"http://kiwix.org/opensearchextension/1.0\"\n" " indexOffset=\"0\"\n" - " template=\"/catalog/search?q={searchTerms?}&lang={language?}&name={k:name?}&tag={k:tag?}¬ag={k:notag?}&maxsize={k:maxsize?}&count={count?}&start={startIndex?}\"/>\n" + " template=\"/ROOT/catalog/search?q={searchTerms?}&lang={language?}&name={k:name?}&tag={k:tag?}¬ag={k:notag?}&maxsize={k:maxsize?}&count={count?}&start={startIndex?}\"/>\n" "\n" ); } TEST_F(LibraryServerTest, catalog_search_by_phrase) { - const auto r = zfs1_->GET("/catalog/search?q=\"ray%20charles\""); + const auto r = zfs1_->GET("/ROOT/catalog/search?q=\"ray%20charles\""); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -775,7 +776,7 @@ TEST_F(LibraryServerTest, catalog_search_by_phrase) TEST_F(LibraryServerTest, catalog_search_by_words) { - const auto r = zfs1_->GET("/catalog/search?q=ray%20charles"); + const auto r = zfs1_->GET("/ROOT/catalog/search?q=ray%20charles"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -796,7 +797,7 @@ TEST_F(LibraryServerTest, catalog_search_by_words) TEST_F(LibraryServerTest, catalog_prefix_search) { { - const auto r = zfs1_->GET("/catalog/search?q=description:ray%20description:charles"); + const auto r = zfs1_->GET("/ROOT/catalog/search?q=description:ray%20description:charles"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -813,7 +814,7 @@ TEST_F(LibraryServerTest, catalog_prefix_search) ); } { - const auto r = zfs1_->GET("/catalog/search?q=title:\"ray%20charles\""); + const auto r = zfs1_->GET("/ROOT/catalog/search?q=title:\"ray%20charles\""); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -832,7 +833,7 @@ TEST_F(LibraryServerTest, catalog_prefix_search) TEST_F(LibraryServerTest, catalog_search_with_word_exclusion) { - const auto r = zfs1_->GET("/catalog/search?q=ray%20-uncategorized"); + const auto r = zfs1_->GET("/ROOT/catalog/search?q=ray%20-uncategorized"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -851,7 +852,7 @@ TEST_F(LibraryServerTest, catalog_search_with_word_exclusion) TEST_F(LibraryServerTest, catalog_search_by_tag) { - const auto r = zfs1_->GET("/catalog/search?tag=_category:jazz"); + const auto r = zfs1_->GET("/ROOT/catalog/search?tag=_category:jazz"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -869,7 +870,7 @@ TEST_F(LibraryServerTest, catalog_search_by_tag) TEST_F(LibraryServerTest, catalog_search_by_category) { - const auto r = zfs1_->GET("/catalog/search?category=jazz"); + const auto r = zfs1_->GET("/ROOT/catalog/search?category=jazz"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -888,7 +889,7 @@ TEST_F(LibraryServerTest, catalog_search_by_category) TEST_F(LibraryServerTest, catalog_search_results_pagination) { { - const auto r = zfs1_->GET("/catalog/search?count=1"); + const auto r = zfs1_->GET("/ROOT/catalog/search?count=1"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -904,7 +905,7 @@ TEST_F(LibraryServerTest, catalog_search_results_pagination) ); } { - const auto r = zfs1_->GET("/catalog/search?start=1&count=1"); + const auto r = zfs1_->GET("/ROOT/catalog/search?start=1&count=1"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -920,7 +921,7 @@ TEST_F(LibraryServerTest, catalog_search_results_pagination) ); } { - const auto r = zfs1_->GET("/catalog/search?start=100&count=10"); + const auto r = zfs1_->GET("/ROOT/catalog/search?start=100&count=10"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), OPDS_FEED_TAG @@ -938,20 +939,20 @@ TEST_F(LibraryServerTest, catalog_search_results_pagination) TEST_F(LibraryServerTest, catalog_v2_root) { - const auto r = zfs1_->GET("/catalog/v2/root.xml"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/root.xml"); EXPECT_EQ(r->status, 200); const char expected_output[] = R"( 12345678-90ab-cdef-1234-567890abcdef OPDS Catalog Root YYYY-MM-DDThh:mm:ssZ @@ -959,7 +960,7 @@ TEST_F(LibraryServerTest, catalog_v2_root) All entries YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -968,7 +969,7 @@ TEST_F(LibraryServerTest, catalog_v2_root) All entries (partial) YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -977,7 +978,7 @@ TEST_F(LibraryServerTest, catalog_v2_root) List of categories YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -986,7 +987,7 @@ TEST_F(LibraryServerTest, catalog_v2_root) List of languages YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -999,7 +1000,7 @@ TEST_F(LibraryServerTest, catalog_v2_root) TEST_F(LibraryServerTest, catalog_v2_searchdescription_xml) { - const auto r = zfs1_->GET("/catalog/v2/searchdescription.xml"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/searchdescription.xml"); EXPECT_EQ(r->status, 200); EXPECT_EQ(r->body, "\n" @@ -1010,24 +1011,24 @@ TEST_F(LibraryServerTest, catalog_v2_searchdescription_xml) " xmlns:atom=\"http://www.w3.org/2005/Atom\"\n" " xmlns:k=\"http://kiwix.org/opensearchextension/1.0\"\n" " indexOffset=\"0\"\n" - " template=\"/catalog/v2/entries?q={searchTerms?}&lang={language?}&name={k:name?}&tag={k:tag?}&maxsize={k:maxsize?}&count={count?}&start={startIndex?}\"/>\n" + " template=\"/ROOT/catalog/v2/entries?q={searchTerms?}&lang={language?}&name={k:name?}&tag={k:tag?}&maxsize={k:maxsize?}&count={count?}&start={startIndex?}\"/>\n" "\n" ); } TEST_F(LibraryServerTest, catalog_v2_categories) { - const auto r = zfs1_->GET("/catalog/v2/categories"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/categories"); EXPECT_EQ(r->status, 200); const char expected_output[] = R"( 12345678-90ab-cdef-1234-567890abcdef List of categories YYYY-MM-DDThh:mm:ssZ @@ -1035,7 +1036,7 @@ TEST_F(LibraryServerTest, catalog_v2_categories) jazz YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -1044,7 +1045,7 @@ TEST_F(LibraryServerTest, catalog_v2_categories) wikipedia YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -1057,7 +1058,7 @@ TEST_F(LibraryServerTest, catalog_v2_categories) TEST_F(LibraryServerTest, catalog_v2_languages) { - const auto r = zfs1_->GET("/catalog/v2/languages"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/languages"); EXPECT_EQ(r->status, 200); const char expected_output[] = R"( 12345678-90ab-cdef-1234-567890abcdef List of languages YYYY-MM-DDThh:mm:ssZ @@ -1079,7 +1080,7 @@ TEST_F(LibraryServerTest, catalog_v2_languages) eng 1 YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -1089,7 +1090,7 @@ TEST_F(LibraryServerTest, catalog_v2_languages) fra 1 YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -1099,7 +1100,7 @@ TEST_F(LibraryServerTest, catalog_v2_languages) rus 1 YYYY-MM-DDThh:mm:ssZ 12345678-90ab-cdef-1234-567890abcdef @@ -1117,13 +1118,13 @@ TEST_F(LibraryServerTest, catalog_v2_languages) " 12345678-90ab-cdef-1234-567890abcdef\n" \ "\n" \ " \n" \ " \n" \ " \n" \ "\n" \ @@ -1135,7 +1136,7 @@ TEST_F(LibraryServerTest, catalog_v2_languages) TEST_F(LibraryServerTest, catalog_v2_entries) { - const auto r = zfs1_->GET("/catalog/v2/entries"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/entries"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), CATALOG_V2_ENTRIES_PREAMBLE("") @@ -1152,7 +1153,7 @@ TEST_F(LibraryServerTest, catalog_v2_entries) TEST_F(LibraryServerTest, catalog_v2_entries_filtered_by_range) { { - const auto r = zfs1_->GET("/catalog/v2/entries?start=1"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/entries?start=1"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), CATALOG_V2_ENTRIES_PREAMBLE("?start=1") @@ -1168,7 +1169,7 @@ TEST_F(LibraryServerTest, catalog_v2_entries_filtered_by_range) } { - const auto r = zfs1_->GET("/catalog/v2/entries?count=2"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/entries?count=2"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), CATALOG_V2_ENTRIES_PREAMBLE("?count=2") @@ -1184,7 +1185,7 @@ TEST_F(LibraryServerTest, catalog_v2_entries_filtered_by_range) } { - const auto r = zfs1_->GET("/catalog/v2/entries?start=1&count=1"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/entries?start=1&count=1"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), CATALOG_V2_ENTRIES_PREAMBLE("?count=1&start=1") @@ -1201,7 +1202,7 @@ TEST_F(LibraryServerTest, catalog_v2_entries_filtered_by_range) TEST_F(LibraryServerTest, catalog_v2_entries_filtered_by_search_terms) { - const auto r = zfs1_->GET("/catalog/v2/entries?q=\"ray%20charles\""); + const auto r = zfs1_->GET("/ROOT/catalog/v2/entries?q=\"ray%20charles\""); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), CATALOG_V2_ENTRIES_PREAMBLE("?q=%22ray%20charles%22") @@ -1227,7 +1228,7 @@ TEST_F(LibraryServerTest, suggestions_in_range) { int suggCount = 0; for (int i = 0; i < 10; i++) { - std::string url = "/suggest?content=zimfile&term=ray&start=" + std::to_string(i*5) + "&count=5"; + std::string url = "/ROOT/suggest?content=zimfile&term=ray&start=" + std::to_string(i*5) + "&count=5"; const auto r = zfs1_->GET(url.c_str()); std::string body = r->body; int currCount = std::count(body.begin(), body.end(), '{') - 1; @@ -1239,13 +1240,13 @@ TEST_F(LibraryServerTest, suggestions_in_range) // Attempt to get 10 suggestions in steps of 5 even though there are only 8 { - std::string url = "/suggest?content=zimfile&term=song+for+you&start=0&count=5"; + std::string url = "/ROOT/suggest?content=zimfile&term=song+for+you&start=0&count=5"; const auto r1 = zfs1_->GET(url.c_str()); std::string body = r1->body; int currCount = std::count(body.begin(), body.end(), '{') - 1; ASSERT_EQ(currCount, 5); - url = "/suggest?content=zimfile&term=song+for+you&start=5&count=5"; + url = "/ROOT/suggest?content=zimfile&term=song+for+you&start=5&count=5"; const auto r2 = zfs1_->GET(url.c_str()); body = r2->body; currCount = std::count(body.begin(), body.end(), '{') - 1; @@ -1254,7 +1255,7 @@ TEST_F(LibraryServerTest, suggestions_in_range) // Attempt to get 10 suggestions even though there is only 1 { - std::string url = "/suggest?content=zimfile&term=strong&start=0&count=5"; + std::string url = "/ROOT/suggest?content=zimfile&term=strong&start=0&count=5"; const auto r = zfs1_->GET(url.c_str()); std::string body = r->body; int currCount = std::count(body.begin(), body.end(), '{') - 1; @@ -1263,7 +1264,7 @@ TEST_F(LibraryServerTest, suggestions_in_range) // No Suggestion { - std::string url = "/suggest?content=zimfile&term=oops&start=0&count=5"; + std::string url = "/ROOT/suggest?content=zimfile&term=oops&start=0&count=5"; const auto r = zfs1_->GET(url.c_str()); std::string body = r->body; int currCount = std::count(body.begin(), body.end(), '{') - 1; @@ -1272,7 +1273,7 @@ TEST_F(LibraryServerTest, suggestions_in_range) // Out of bound value { - std::string url = "/suggest?content=zimfile&term=ray&start=-2&count=-1"; + std::string url = "/ROOT/suggest?content=zimfile&term=ray&start=-2&count=-1"; const auto r = zfs1_->GET(url.c_str()); std::string body = r->body; int currCount = std::count(body.begin(), body.end(), '{') - 1; @@ -1282,20 +1283,20 @@ TEST_F(LibraryServerTest, suggestions_in_range) TEST_F(LibraryServerTest, catalog_v2_individual_entry_access) { - const auto r = zfs1_->GET("/catalog/v2/entry/raycharles"); + 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 ); - const auto r1 = zfs1_->GET("/catalog/v2/entry/non-existent-entry"); + const auto r1 = zfs1_->GET("/ROOT/catalog/v2/entry/non-existent-entry"); EXPECT_EQ(r1->status, 404); } TEST_F(LibraryServerTest, catalog_v2_partial_entries) { - const auto r = zfs1_->GET("/catalog/v2/partial_entries"); + const auto r = zfs1_->GET("/ROOT/catalog/v2/partial_entries"); EXPECT_EQ(r->status, 200); EXPECT_EQ(maskVariableOPDSFeedData(r->body), CATALOG_V2_PARTIAL_ENTRIES_PREAMBLE("") @@ -1307,7 +1308,7 @@ TEST_F(LibraryServerTest, catalog_v2_partial_entries) " Charles, Ray\n" " YYYY-MM-DDThh:mm:ssZ\n" " \n" " \n" " \n" @@ -1315,7 +1316,7 @@ TEST_F(LibraryServerTest, catalog_v2_partial_entries) " Ray Charles\n" " YYYY-MM-DDThh:mm:ssZ\n" " \n" " \n" " \n" @@ -1323,7 +1324,7 @@ TEST_F(LibraryServerTest, catalog_v2_partial_entries) " Ray (uncategorized) Charles\n" " YYYY-MM-DDThh:mm:ssZ\n" " \n" " \n" "\n" From 9482bfb95bb6524ec780c6b9370626dad22acf20 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 17 Nov 2021 15:09:15 +0100 Subject: [PATCH 3/5] Add a method to get the a book illustration for a specific size. --- include/book.h | 1 + src/book.cpp | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/book.h b/include/book.h index cdcbf51fe..f0ac10aea 100644 --- a/include/book.h +++ b/include/book.h @@ -100,6 +100,7 @@ class Book const std::string& getFaviconMimeType() const; Illustrations getIllustrations() const; + std::shared_ptr getIllustration(unsigned int size) const; const std::string& getDownloadId() const { return m_downloadId; } diff --git a/src/book.cpp b/src/book.cpp index 5e1952a43..e04b0192e 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -217,14 +217,23 @@ void Book::setPath(const std::string& path) const Book::Illustration Book::missingDefaultIllustration; -const Book::Illustration& Book::getDefaultIllustration() const +std::shared_ptr Book::getIllustration(unsigned int size) const { for ( const auto& ilPtr : m_illustrations ) { - if (ilPtr->width == 48 && ilPtr->height == 48) { - return *ilPtr; + if (ilPtr->width == size && ilPtr->height == size) { + return ilPtr; } } - return missingDefaultIllustration; + throw std::runtime_error("Cannot find illustration"); +} + +const Book::Illustration& Book::getDefaultIllustration() const +{ + try { + return *getIllustration(48); + } catch (...) { + return missingDefaultIllustration; + } } const std::string& Book::Illustration::getData() const From e108fb0e47c983d318da7199a96631580d5bf0f6 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 17 Nov 2021 15:12:20 +0100 Subject: [PATCH 4/5] Add `/catalog/v2/illustration` endpoint --- src/server/internalServer.h | 1 + src/server/internalServer_catalog_v2.cpp | 16 ++++++++++++++++ test/server.cpp | 2 ++ 3 files changed, 19 insertions(+) diff --git a/src/server/internalServer.h b/src/server/internalServer.h index 3a7d98c26..7af45769e 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -82,6 +82,7 @@ class InternalServer { std::unique_ptr handle_catalog_v2_complete_entry(const RequestContext& request, const std::string& entryId); std::unique_ptr handle_catalog_v2_categories(const RequestContext& request); std::unique_ptr handle_catalog_v2_languages(const RequestContext& request); + std::unique_ptr handle_catalog_v2_illustration(const RequestContext& request); std::unique_ptr handle_meta(const RequestContext& request); std::unique_ptr handle_search(const RequestContext& request); std::unique_ptr handle_suggest(const RequestContext& request); diff --git a/src/server/internalServer_catalog_v2.cpp b/src/server/internalServer_catalog_v2.cpp index 475e16cb4..3a578cb02 100644 --- a/src/server/internalServer_catalog_v2.cpp +++ b/src/server/internalServer_catalog_v2.cpp @@ -66,6 +66,8 @@ std::unique_ptr InternalServer::handle_catalog_v2(const RequestContext return handle_catalog_v2_categories(request); } else if (url == "languages") { return handle_catalog_v2_languages(request); + } else if (url == "illustration") { + return handle_catalog_v2_illustration(request); } else { return Response::build_404(*this, request.get_full_url(), "", ""); } @@ -146,4 +148,18 @@ std::unique_ptr InternalServer::handle_catalog_v2_languages(const Requ ); } +std::unique_ptr InternalServer::handle_catalog_v2_illustration(const RequestContext& request) +{ + try { + const auto bookName = request.get_url_part(3); + const auto bookId = mp_nameMapper->getIdForName(bookName); + auto book = mp_library->getBookByIdThreadSafe(bookId); + auto size = request.get_argument("size"); + auto illustration = book.getIllustration(size); + return ContentResponse::build(*this, illustration->getData(), illustration->mimeType); + } catch(...) { + return Response::build_404(*this, request.get_full_url(), "", ""); + } +} + } // namespace kiwix diff --git a/test/server.cpp b/test/server.cpp index 5ae6296c9..08eba5043 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -201,6 +201,7 @@ const ResourceCollection resources200Uncompressible{ { WITH_ETAG, "/ROOT/meta?content=zimfile&name=publisher" }, { WITH_ETAG, "/ROOT/meta?content=zimfile&name=favicon" }, { WITH_ETAG, "/ROOT/meta?content=zimfile&name=Illustration_48x48@1" }, + { NO_ETAG, "/ROOT/catalog/v2/illustration/zimfile?size=48" }, { WITH_ETAG, "/ROOT/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg" }, @@ -276,6 +277,7 @@ const char* urls404[] = { "/ROOT/catalog", "/ROOT/catalog/non-existent-item", "/ROOT/catalogBLABLABLA/root.xml", + "/ROOT/catalog/v2/illustration/zimfile?size=96", "/ROOT/meta", "/ROOT/meta?content=zimfile", "/ROOT/meta?content=zimfile&name=non-existent-item", From 6f1799db9f9a97cf0d7b98f8080dd6a315e719f1 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 17 Nov 2021 15:12:53 +0100 Subject: [PATCH 5/5] Use the new endpoint in the OPDS stream. --- src/opds_dumper.cpp | 7 ++++--- static/templates/catalog_v2_entry.xml | 4 ++-- test/server.cpp | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/opds_dumper.cpp b/src/opds_dumper.cpp index 229bc92e2..32d8010ec 100644 --- a/src/opds_dumper.cpp +++ b/src/opds_dumper.cpp @@ -59,10 +59,11 @@ IllustrationInfo getBookIllustrationInfo(const Book& book) kainjow::mustache::list illustrations; if ( book.isPathValid() ) { for ( const auto& illustration : book.getIllustrations() ) { + // For now, we are handling only sizexsize@1 illustration. + // So we can simply pass one size to mustache. illustrations.push_back(kainjow::mustache::object{ - {"icon_width", to_string(illustration->width)}, - {"icon_height", to_string(illustration->height)}, - {"icon_scale", "1"}, + {"icon_size", to_string(illustration->width)}, + {"icon_mimetype", illustration->mimeType} }); } } diff --git a/static/templates/catalog_v2_entry.xml b/static/templates/catalog_v2_entry.xml index 143872635..9f7df876a 100644 --- a/static/templates/catalog_v2_entry.xml +++ b/static/templates/catalog_v2_entry.xml @@ -16,8 +16,8 @@ {{article_count}} {{media_count}} {{#icons}} + href="{{root}}/catalog/v2/illustration/{{{content_id}}}/?size={{icon_size}}" + type="{{icon_mimetype}};width={{icon_size}};height={{icon_size}};scale=1"/> {{/icons}} {{author_name}} diff --git a/test/server.cpp b/test/server.cpp index 08eba5043..68a7ac07e 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -686,7 +686,7 @@ std::string maskVariableOPDSFeedData(std::string s) " 284\n" \ " 2\n" \ " \n" \ " \n" \ " \n" \