From 59e5c7ff4e003af27b7eee3ce9359c4e622c67db Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 7 Nov 2021 11:04:37 +0400 Subject: [PATCH 01/20] Enhanced Book.updateFromXMLTest with favicon --- test/book.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/book.cpp b/test/book.cpp index 22eca5428..1eb23d24d 100644 --- a/test/book.cpp +++ b/test/book.cpp @@ -68,6 +68,9 @@ TEST(BookTest, updateFromXMLTest) articleCount="123456" mediaCount="234567" size="345678" + favicon="ZmFrZS1ib29rLWZhdmljb24tZGF0YQ==" + faviconMimeType="text/plain" + faviconUrl="http://who.org/zara.fav" > )"); @@ -85,6 +88,9 @@ TEST(BookTest, updateFromXMLTest) EXPECT_EQ(book.getArticleCount(), 123456U); EXPECT_EQ(book.getMediaCount(), 234567U); EXPECT_EQ(book.getSize(), 345678U*1024U); + EXPECT_EQ(book.getFavicon(), "fake-book-favicon-data"); + EXPECT_EQ(book.getFaviconMimeType(), "text/plain"); + EXPECT_EQ(book.getFaviconUrl(), "http://who.org/zara.fav"); } TEST(BookTest, updateFromXMLCategoryHandlingTest) From 4f65811011469465ddb1b0809e971d8fb3997b02 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 7 Nov 2021 11:08:12 +0400 Subject: [PATCH 02/20] Moved Book.updateTest below Book.updateFromXMLTest Book.updateTest is going to be modified so that it relies on functionality tested by Book.updateFromXMLTest. Hence the order of the tests better reflect that dependency. --- test/book.cpp | 72 +++++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/test/book.cpp b/test/book.cpp index 1eb23d24d..a4f4e8784 100644 --- a/test/book.cpp +++ b/test/book.cpp @@ -2,42 +2,6 @@ #include "../include/book.h" #include -TEST(BookTest, updateTest) -{ - kiwix::Book book; - - book.setId("xyz"); - book.setReadOnly(false); - book.setPath("/home/user/Downloads/skin-of-color-society_en_all_2019-11.zim"); - book.setPathValid(true); - book.setUrl("book-url"); - book.setTags("youtube;_videos:yes;_ftindex:yes;_ftindex:yes;_pictures:yes;_details:yes"); - book.setName("skin-of-color-society_en_all"); - book.setFavicon("book-favicon"); - book.setFaviconMimeType("book-favicon-mimetype"); - - kiwix::Book newBook; - - newBook.setReadOnly(true); - EXPECT_FALSE(newBook.update(book)); - - newBook.setReadOnly(false); - EXPECT_FALSE(newBook.update(book)); - - newBook.setId("xyz"); - EXPECT_TRUE(newBook.update(book)); - - EXPECT_EQ(newBook.readOnly(), book.readOnly()); - EXPECT_EQ(newBook.getPath(), book.getPath()); - EXPECT_EQ(newBook.isPathValid(), book.isPathValid()); - EXPECT_EQ(newBook.getUrl(), book.getUrl()); - EXPECT_EQ(newBook.getTags(), book.getTags()); - EXPECT_EQ(newBook.getCategory(), book.getCategory()); - EXPECT_EQ(newBook.getName(), book.getName()); - EXPECT_EQ(newBook.getFavicon(), book.getFavicon()); - EXPECT_EQ(newBook.getFaviconMimeType(), book.getFaviconMimeType()); -} - namespace { @@ -172,3 +136,39 @@ TEST(BookTest, updateCopiesCategory) newBook.update(book); EXPECT_EQ(newBook.getCategory(), "ted"); } + +TEST(BookTest, updateTest) +{ + kiwix::Book book; + + book.setId("xyz"); + book.setReadOnly(false); + book.setPath("/home/user/Downloads/skin-of-color-society_en_all_2019-11.zim"); + book.setPathValid(true); + book.setUrl("book-url"); + book.setTags("youtube;_videos:yes;_ftindex:yes;_ftindex:yes;_pictures:yes;_details:yes"); + book.setName("skin-of-color-society_en_all"); + book.setFavicon("book-favicon"); + book.setFaviconMimeType("book-favicon-mimetype"); + + kiwix::Book newBook; + + newBook.setReadOnly(true); + EXPECT_FALSE(newBook.update(book)); + + newBook.setReadOnly(false); + EXPECT_FALSE(newBook.update(book)); + + newBook.setId("xyz"); + EXPECT_TRUE(newBook.update(book)); + + EXPECT_EQ(newBook.readOnly(), book.readOnly()); + EXPECT_EQ(newBook.getPath(), book.getPath()); + EXPECT_EQ(newBook.isPathValid(), book.isPathValid()); + EXPECT_EQ(newBook.getUrl(), book.getUrl()); + EXPECT_EQ(newBook.getTags(), book.getTags()); + EXPECT_EQ(newBook.getCategory(), book.getCategory()); + EXPECT_EQ(newBook.getName(), book.getName()); + EXPECT_EQ(newBook.getFavicon(), book.getFavicon()); + EXPECT_EQ(newBook.getFaviconMimeType(), book.getFaviconMimeType()); +} From abfd9d88d89be6f667920d11d74a53e15fb60129 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 7 Nov 2021 11:17:26 +0400 Subject: [PATCH 03/20] Book.updateTest creates the source book from XML ... thus eliminating the need for the Book::setFavicon*() methods. --- test/book.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/book.cpp b/test/book.cpp index a4f4e8784..f477b6765 100644 --- a/test/book.cpp +++ b/test/book.cpp @@ -139,17 +139,23 @@ TEST(BookTest, updateCopiesCategory) TEST(BookTest, updateTest) { - kiwix::Book book; + const XMLDoc xml(R"( + + + )"); + + kiwix::Book book; + book.updateFromXml(xml.child("book"), "/data/zim"); - book.setId("xyz"); book.setReadOnly(false); - book.setPath("/home/user/Downloads/skin-of-color-society_en_all_2019-11.zim"); book.setPathValid(true); - book.setUrl("book-url"); - book.setTags("youtube;_videos:yes;_ftindex:yes;_ftindex:yes;_pictures:yes;_details:yes"); - book.setName("skin-of-color-society_en_all"); - book.setFavicon("book-favicon"); - book.setFaviconMimeType("book-favicon-mimetype"); kiwix::Book newBook; From 3e5372ef2976af523f3481ff279d28cc1e147366 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 13:25:34 +0400 Subject: [PATCH 04/20] RIP Book::{setFavicon(),setFaviconMimeType()} --- include/book.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/book.h b/include/book.h index a9db6df2c..6629c6850 100644 --- a/include/book.h +++ b/include/book.h @@ -96,8 +96,6 @@ class Book void setArticleCount(uint64_t articleCount) { m_articleCount = articleCount; } void setMediaCount(uint64_t mediaCount) { m_mediaCount = mediaCount; } void setSize(uint64_t size) { m_size = size; } - void setFavicon(const std::string& favicon) { m_favicon = favicon; } - void setFaviconMimeType(const std::string& faviconMimeType) { m_faviconMimeType = faviconMimeType; } void setDownloadId(const std::string& downloadId) { m_downloadId = downloadId; } private: From 811b73a4f175c5b57c4206c8f6b9e62e669c148e Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 13:27:27 +0400 Subject: [PATCH 05/20] Moved 2 small method definitions to cpp --- include/book.h | 4 ++-- src/book.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/include/book.h b/include/book.h index 6629c6850..6378fac73 100644 --- a/include/book.h +++ b/include/book.h @@ -74,8 +74,8 @@ class Book const uint64_t& getMediaCount() const { return m_mediaCount; } const uint64_t& getSize() const { return m_size; } const std::string& getFavicon() const; - const std::string& getFaviconUrl() const { return m_faviconUrl; } - const std::string& getFaviconMimeType() const { return m_faviconMimeType; } + const std::string& getFaviconUrl() const; + const std::string& getFaviconMimeType() const; const std::string& getDownloadId() const { return m_downloadId; } void setReadOnly(bool readOnly) { m_readOnly = readOnly; } diff --git a/src/book.cpp b/src/book.cpp index 020fa7f4c..01e742ca9 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -226,6 +226,16 @@ const std::string& Book::getFavicon() const { return m_favicon; } +const std::string& Book::getFaviconUrl() const +{ + return m_faviconUrl; +} + +const std::string& Book::getFaviconMimeType() const +{ + return m_faviconMimeType; +} + std::string Book::getTagStr(const std::string& tagName) const { return getTagValueFromTagList(convertTags(m_tags), tagName); } From ec5a4239241c852c814116d49985dbe966fcf993 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 16:38:21 +0400 Subject: [PATCH 06/20] Enter Book::Illustration `Book::m_favicon` and its 2 friends are replaced with a single `Book::m_illustration` data member. --- include/book.h | 15 +++++++++++---- src/book.cpp | 28 +++++++++++++--------------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/include/book.h b/include/book.h index 6378fac73..e18a8142b 100644 --- a/include/book.h +++ b/include/book.h @@ -41,7 +41,15 @@ class Reader; */ class Book { - public: + public: // types + struct Illustration + { + std::string mimeType; + std::string url; + mutable std::string data; + }; + + public: // functions Book(); ~Book(); @@ -76,6 +84,7 @@ class Book const std::string& getFavicon() const; const std::string& getFaviconUrl() const; const std::string& getFaviconMimeType() const; + const std::string& getDownloadId() const { return m_downloadId; } void setReadOnly(bool readOnly) { m_readOnly = readOnly; } @@ -122,9 +131,7 @@ class Book uint64_t m_mediaCount = 0; bool m_readOnly = false; uint64_t m_size = 0; - mutable std::string m_favicon; - std::string m_faviconUrl; - std::string m_faviconMimeType; + Illustration m_illustration; }; } diff --git a/src/book.cpp b/src/book.cpp index 01e742ca9..0ac5b6a8b 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -72,9 +72,7 @@ bool Book::update(const kiwix::Book& other) m_articleCount = other.m_articleCount; m_mediaCount = other.m_mediaCount; m_size = other.m_size; - m_favicon = other.m_favicon; - m_faviconMimeType = other.m_faviconMimeType; - m_faviconUrl = other.m_faviconUrl; + m_illustration = other.m_illustration; m_downloadId = other.m_downloadId; @@ -104,7 +102,7 @@ void Book::update(const zim::Archive& archive) { m_mediaCount = getArchiveMediaCount(archive); m_size = static_cast(getArchiveFileSize(archive)) << 10; - getArchiveFavicon(archive, 48, m_favicon, m_faviconMimeType); + getArchiveFavicon(archive, 48, m_illustration.data, m_illustration.mimeType); } #define ATTR(name) node.attribute(name).value() @@ -131,9 +129,9 @@ 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; - m_favicon = base64_decode(ATTR("favicon")); - m_faviconMimeType = ATTR("faviconMimeType"); - m_faviconUrl = ATTR("faviconUrl"); + m_illustration.data = base64_decode(ATTR("favicon")); + m_illustration.mimeType = ATTR("faviconMimeType"); + m_illustration.url = ATTR("faviconUrl"); try { m_downloadId = ATTR("downloadId"); } catch(...) {} @@ -181,8 +179,8 @@ void Book::updateFromOpds(const pugi::xml_node& node, const std::string& urlHost m_size = strtoull(linkNode.attribute("length").value(), 0, 0); } if (rel == "http://opds-spec.org/image/thumbnail") { - m_faviconUrl = urlHost + linkNode.attribute("href").value(); - m_faviconMimeType = linkNode.attribute("type").value(); + m_illustration.url = urlHost + linkNode.attribute("href").value(); + m_illustration.mimeType = linkNode.attribute("type").value(); } } @@ -216,24 +214,24 @@ void Book::setPath(const std::string& path) } const std::string& Book::getFavicon() const { - if (m_favicon.empty() && !m_faviconUrl.empty()) { + if (m_illustration.data.empty() && !m_illustration.url.empty()) { try { - m_favicon = download(m_faviconUrl); + m_illustration.data = download(m_illustration.url); } catch(...) { - std::cerr << "Cannot download favicon from " << m_faviconUrl; + std::cerr << "Cannot download favicon from " << m_illustration.url; } } - return m_favicon; + return m_illustration.data; } const std::string& Book::getFaviconUrl() const { - return m_faviconUrl; + return m_illustration.url; } const std::string& Book::getFaviconMimeType() const { - return m_faviconMimeType; + return m_illustration.mimeType; } std::string Book::getTagStr(const std::string& tagName) const { From 7d8a83cc97694be59baea728cbe8d7730e3c0f35 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 16:46:46 +0400 Subject: [PATCH 07/20] Encapsulated access to Book::m_illustration --- include/book.h | 6 ++++-- src/book.cpp | 38 ++++++++++++++++++++++++++------------ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/include/book.h b/include/book.h index e18a8142b..b5ff34052 100644 --- a/include/book.h +++ b/include/book.h @@ -107,10 +107,12 @@ class Book void setSize(uint64_t size) { m_size = size; } void setDownloadId(const std::string& downloadId) { m_downloadId = downloadId; } - private: + private: // functions std::string getCategoryFromTags() const; + const Illustration& getDefaultIllustration() const; + Illustration& getMutableDefaultIllustration(); - protected: + protected: // data std::string m_id; std::string m_downloadId; std::string m_path; diff --git a/src/book.cpp b/src/book.cpp index 0ac5b6a8b..ed219ce79 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -102,7 +102,8 @@ void Book::update(const zim::Archive& archive) { m_mediaCount = getArchiveMediaCount(archive); m_size = static_cast(getArchiveFileSize(archive)) << 10; - getArchiveFavicon(archive, 48, m_illustration.data, m_illustration.mimeType); + Illustration& favicon = getMutableDefaultIllustration(); + getArchiveFavicon(archive, 48, favicon.data, favicon.mimeType); } #define ATTR(name) node.attribute(name).value() @@ -129,9 +130,10 @@ 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; - m_illustration.data = base64_decode(ATTR("favicon")); - m_illustration.mimeType = ATTR("faviconMimeType"); - m_illustration.url = ATTR("faviconUrl"); + Illustration& favicon = getMutableDefaultIllustration(); + favicon.data = base64_decode(ATTR("favicon")); + favicon.mimeType = ATTR("faviconMimeType"); + favicon.url = ATTR("faviconUrl"); try { m_downloadId = ATTR("downloadId"); } catch(...) {} @@ -179,8 +181,9 @@ void Book::updateFromOpds(const pugi::xml_node& node, const std::string& urlHost m_size = strtoull(linkNode.attribute("length").value(), 0, 0); } if (rel == "http://opds-spec.org/image/thumbnail") { - m_illustration.url = urlHost + linkNode.attribute("href").value(); - m_illustration.mimeType = linkNode.attribute("type").value(); + Illustration& favicon = getMutableDefaultIllustration(); + favicon.url = urlHost + linkNode.attribute("href").value(); + favicon.mimeType = linkNode.attribute("type").value(); } } @@ -213,25 +216,36 @@ void Book::setPath(const std::string& path) : path; } +const Book::Illustration& Book::getDefaultIllustration() const +{ + return m_illustration; +} + +Book::Illustration& Book::getMutableDefaultIllustration() +{ + return m_illustration; +} + const std::string& Book::getFavicon() const { - if (m_illustration.data.empty() && !m_illustration.url.empty()) { + const Illustration& favicon = getDefaultIllustration(); + if (favicon.data.empty() && !favicon.url.empty()) { try { - m_illustration.data = download(m_illustration.url); + favicon.data = download(favicon.url); } catch(...) { - std::cerr << "Cannot download favicon from " << m_illustration.url; + std::cerr << "Cannot download favicon from " << favicon.url; } } - return m_illustration.data; + return favicon.data; } const std::string& Book::getFaviconUrl() const { - return m_illustration.url; + return getDefaultIllustration().url; } const std::string& Book::getFaviconMimeType() const { - return m_illustration.mimeType; + return getDefaultIllustration().mimeType; } std::string Book::getTagStr(const std::string& tagName) const { From 9f0db6b7fa9d250f8f7d086122d33c243130d1d6 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 17:18:45 +0400 Subject: [PATCH 08/20] Book::Illustration::getData() --- include/book.h | 8 +++++++- src/book.cpp | 16 ++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/book.h b/include/book.h index b5ff34052..d53de75d4 100644 --- a/include/book.h +++ b/include/book.h @@ -42,10 +42,16 @@ class Reader; class Book { public: // types - struct Illustration + class Illustration { + friend class Book; + public: std::string mimeType; std::string url; + + const std::string& getData() const; + + private: mutable std::string data; }; diff --git a/src/book.cpp b/src/book.cpp index ed219ce79..8c992deac 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -226,16 +226,20 @@ Book::Illustration& Book::getMutableDefaultIllustration() return m_illustration; } -const std::string& Book::getFavicon() const { - const Illustration& favicon = getDefaultIllustration(); - if (favicon.data.empty() && !favicon.url.empty()) { +const std::string& Book::Illustration::getData() const +{ + if (data.empty() && !url.empty()) { try { - favicon.data = download(favicon.url); + data = download(url); } catch(...) { - std::cerr << "Cannot download favicon from " << favicon.url; + std::cerr << "Cannot download favicon from " << url; } } - return favicon.data; + return data; +} + +const std::string& Book::getFavicon() const { + return getDefaultIllustration().getData(); } const std::string& Book::getFaviconUrl() const From c129952605514957074e5d94a5058d035570582c Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 16:50:38 +0400 Subject: [PATCH 09/20] Added a couple of notes on data consistency --- src/book.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/book.cpp b/src/book.cpp index 8c992deac..eccae35fc 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -104,6 +104,7 @@ void Book::update(const zim::Archive& archive) { Illustration& favicon = getMutableDefaultIllustration(); getArchiveFavicon(archive, 48, favicon.data, favicon.mimeType); + // XXX: isn't favicon.url neglected here? } #define ATTR(name) node.attribute(name).value() @@ -182,6 +183,7 @@ void Book::updateFromOpds(const pugi::xml_node& node, const std::string& urlHost } if (rel == "http://opds-spec.org/image/thumbnail") { Illustration& favicon = getMutableDefaultIllustration(); + // XXX: shouldn't favicon.data be cleared()? favicon.url = urlHost + linkNode.attribute("href").value(); favicon.mimeType = linkNode.attribute("type").value(); } From 5263f6880c4a7218c3d38ce9895c659f3dffd56e Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 16:59:06 +0400 Subject: [PATCH 10/20] Internally Book supports multiple illustrations --- include/book.h | 4 +++- src/book.cpp | 14 +++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/book.h b/include/book.h index d53de75d4..0f21bc481 100644 --- a/include/book.h +++ b/include/book.h @@ -21,6 +21,8 @@ #define KIWIX_BOOK_H #include +#include +#include namespace pugi { class xml_node; @@ -139,7 +141,7 @@ class Book uint64_t m_mediaCount = 0; bool m_readOnly = false; uint64_t m_size = 0; - Illustration m_illustration; + std::vector> m_illustrations; }; } diff --git a/src/book.cpp b/src/book.cpp index eccae35fc..faee20dfe 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -40,7 +40,10 @@ Book::Book() : m_pathValid(false), m_readOnly(false) { + const auto illustration = std::make_shared(); + m_illustrations.assign(1, illustration); } + /* Destructor */ Book::~Book() { @@ -72,7 +75,10 @@ bool Book::update(const kiwix::Book& other) m_articleCount = other.m_articleCount; m_mediaCount = other.m_mediaCount; m_size = other.m_size; - m_illustration = other.m_illustration; + m_illustrations.clear(); + for ( const auto& ill : other.m_illustrations ) { + m_illustrations.push_back(std::make_shared(*ill)); + } m_downloadId = other.m_downloadId; @@ -220,12 +226,14 @@ void Book::setPath(const std::string& path) const Book::Illustration& Book::getDefaultIllustration() const { - return m_illustration; + assert(m_illustrations.size() == 1); + return *m_illustrations.front(); } Book::Illustration& Book::getMutableDefaultIllustration() { - return m_illustration; + const Book* const const_this = this; + return const_cast(const_this->getDefaultIllustration()); } const std::string& Book::Illustration::getData() const From f4bc3c8ced2a4b8954e2b8cb2b334d514c19ec9f Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 17:02:42 +0400 Subject: [PATCH 11/20] Book::Illustration got dimensions --- include/book.h | 2 ++ src/book.cpp | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/book.h b/include/book.h index 0f21bc481..04d71b1a3 100644 --- a/include/book.h +++ b/include/book.h @@ -48,6 +48,8 @@ class Book { friend class Book; public: + uint16_t width = 48; + uint16_t height = 48; std::string mimeType; std::string url; diff --git a/src/book.cpp b/src/book.cpp index faee20dfe..50a144bb6 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -226,8 +226,12 @@ void Book::setPath(const std::string& path) const Book::Illustration& Book::getDefaultIllustration() const { - assert(m_illustrations.size() == 1); - return *m_illustrations.front(); + for ( const auto& ilPtr : m_illustrations ) { + if (ilPtr->width == 48 && ilPtr->height == 48) { + return *ilPtr; + } + } + throw std::runtime_error("No default illustration"); } Book::Illustration& Book::getMutableDefaultIllustration() From 537ba7e6b99559f83d974f26dadf74447aacf553 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sun, 31 Oct 2021 17:28:55 +0400 Subject: [PATCH 12/20] Book::update() reads illustrations from ZIM file --- src/book.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/book.cpp b/src/book.cpp index 50a144bb6..2a764c994 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -108,9 +108,16 @@ void Book::update(const zim::Archive& archive) { m_mediaCount = getArchiveMediaCount(archive); m_size = static_cast(getArchiveFileSize(archive)) << 10; - Illustration& favicon = getMutableDefaultIllustration(); - getArchiveFavicon(archive, 48, favicon.data, favicon.mimeType); - // XXX: isn't favicon.url neglected here? + m_illustrations.clear(); + for ( const auto illustrationSize : archive.getIllustrationSizes() ) { + const auto illustration = std::make_shared(); + const zim::Item illustrationItem = archive.getIllustrationItem(illustrationSize); + illustration->width = illustration->height = illustrationSize; + illustration->mimeType = illustrationItem.getMimetype(); + illustration->data = illustrationItem.getData(); + // NOTE: illustration->url is left uninitialized + m_illustrations.push_back(illustration); + } } #define ATTR(name) node.attribute(name).value() From e52a4a646bbf7367bf3b0ffd0c07d1f584a81054 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 13:48:48 +0400 Subject: [PATCH 13/20] Book::updateFromXml() resets Book::m_illustrations --- src/book.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/book.cpp b/src/book.cpp index 2a764c994..69d70a2a9 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -144,10 +144,11 @@ 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; - Illustration& favicon = getMutableDefaultIllustration(); - favicon.data = base64_decode(ATTR("favicon")); - favicon.mimeType = ATTR("faviconMimeType"); - favicon.url = ATTR("faviconUrl"); + 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); try { m_downloadId = ATTR("downloadId"); } catch(...) {} From bd29c4c7ef7aa6bdfd0e84ce70a3db4bfd18b05a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 13:50:43 +0400 Subject: [PATCH 14/20] Book::updateFromOpds() resets Book::m_illustrations --- src/book.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/book.cpp b/src/book.cpp index 69d70a2a9..7d36b782a 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -196,10 +196,11 @@ void Book::updateFromOpds(const pugi::xml_node& node, const std::string& urlHost m_size = strtoull(linkNode.attribute("length").value(), 0, 0); } if (rel == "http://opds-spec.org/image/thumbnail") { - Illustration& favicon = getMutableDefaultIllustration(); - // XXX: shouldn't favicon.data be cleared()? - favicon.url = urlHost + linkNode.attribute("href").value(); - favicon.mimeType = linkNode.attribute("type").value(); + const auto favicon = std::make_shared(); + favicon->data.clear(); + favicon->url = urlHost + linkNode.attribute("href").value(); + favicon->mimeType = linkNode.attribute("type").value(); + m_illustrations.assign(1, favicon); } } From c8da5eea2ba9e463303e9c315ffafa3c8df2917a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 13:54:55 +0400 Subject: [PATCH 15/20] Dropped Book::getMutableDefaultIllustration() Now a Book is created without a default illustration. --- include/book.h | 1 - src/book.cpp | 8 -------- 2 files changed, 9 deletions(-) diff --git a/include/book.h b/include/book.h index 04d71b1a3..c395ffce2 100644 --- a/include/book.h +++ b/include/book.h @@ -120,7 +120,6 @@ class Book private: // functions std::string getCategoryFromTags() const; const Illustration& getDefaultIllustration() const; - Illustration& getMutableDefaultIllustration(); protected: // data std::string m_id; diff --git a/src/book.cpp b/src/book.cpp index 7d36b782a..3074a843d 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -40,8 +40,6 @@ Book::Book() : m_pathValid(false), m_readOnly(false) { - const auto illustration = std::make_shared(); - m_illustrations.assign(1, illustration); } /* Destructor */ @@ -243,12 +241,6 @@ const Book::Illustration& Book::getDefaultIllustration() const throw std::runtime_error("No default illustration"); } -Book::Illustration& Book::getMutableDefaultIllustration() -{ - const Book* const const_this = this; - return const_cast(const_this->getDefaultIllustration()); -} - const std::string& Book::Illustration::getData() const { if (data.empty() && !url.empty()) { From 8a6adddc161e133fdf1f02f63accf96bcdb7485a Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 13:57:34 +0400 Subject: [PATCH 16/20] Non-throwing Book::getDefaultIllustration() --- include/book.h | 4 ++++ src/book.cpp | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/include/book.h b/include/book.h index c395ffce2..4a8c8a7d2 100644 --- a/include/book.h +++ b/include/book.h @@ -143,6 +143,10 @@ class Book bool m_readOnly = false; uint64_t m_size = 0; std::vector> m_illustrations; + + // Used as the return value of getDefaultIllustration() when no default + // illustration is found in the book + static const Illustration missingDefaultIllustration; }; } diff --git a/src/book.cpp b/src/book.cpp index 3074a843d..35370bfff 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -231,6 +231,8 @@ void Book::setPath(const std::string& path) : path; } +const Book::Illustration Book::missingDefaultIllustration; + const Book::Illustration& Book::getDefaultIllustration() const { for ( const auto& ilPtr : m_illustrations ) { @@ -238,7 +240,7 @@ const Book::Illustration& Book::getDefaultIllustration() const return *ilPtr; } } - throw std::runtime_error("No default illustration"); + return missingDefaultIllustration; } const std::string& Book::Illustration::getData() const From 9f428845078bcff045c9f8ccfa98f6d470992e5b Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 14:23:59 +0400 Subject: [PATCH 17/20] Book's illustrations are now immutable --- include/book.h | 2 +- src/book.cpp | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/include/book.h b/include/book.h index 4a8c8a7d2..7dc1ed8c5 100644 --- a/include/book.h +++ b/include/book.h @@ -142,7 +142,7 @@ class Book uint64_t m_mediaCount = 0; bool m_readOnly = false; uint64_t m_size = 0; - std::vector> m_illustrations; + std::vector> m_illustrations; // Used as the return value of getDefaultIllustration() when no default // illustration is found in the book diff --git a/src/book.cpp b/src/book.cpp index 35370bfff..73063a141 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -73,11 +73,7 @@ bool Book::update(const kiwix::Book& other) m_articleCount = other.m_articleCount; m_mediaCount = other.m_mediaCount; m_size = other.m_size; - m_illustrations.clear(); - for ( const auto& ill : other.m_illustrations ) { - m_illustrations.push_back(std::make_shared(*ill)); - } - + m_illustrations = other.m_illustrations; m_downloadId = other.m_downloadId; return true; From e2544799a140d5d23bbf42eb4f15b5efb1eacda3 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Tue, 9 Nov 2021 18:40:28 +0400 Subject: [PATCH 18/20] Shorter Book::update() --- src/book.cpp | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/book.cpp b/src/book.cpp index 73063a141..de82c6fbc 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -55,27 +55,7 @@ bool Book::update(const kiwix::Book& other) if (m_id != other.m_id) return false; - m_readOnly = other.m_readOnly; - m_path = other.m_path; - m_pathValid = other.m_pathValid; - m_title = other.m_title; - m_description = other.m_description; - m_language = other.m_language; - m_creator = other.m_creator; - m_publisher = other.m_publisher; - m_date = other.m_date; - m_url = other.m_url; - m_name = other.m_name; - m_flavour = other.m_flavour; - m_tags = other.m_tags; - m_category = other.m_category; - m_origId = other.m_origId; - m_articleCount = other.m_articleCount; - m_mediaCount = other.m_mediaCount; - m_size = other.m_size; - m_illustrations = other.m_illustrations; - m_downloadId = other.m_downloadId; - + *this = other; return true; } From eb6a0d6456cf8ff2bd2235ab50b779f42cb7e3fb Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 18 Nov 2021 14:25:03 +0400 Subject: [PATCH 19/20] Enter Book::getIllustrations() --- include/book.h | 6 +++++- src/book.cpp | 5 +++++ src/opds_dumper.cpp | 6 +++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/include/book.h b/include/book.h index 7dc1ed8c5..5d049ad6c 100644 --- a/include/book.h +++ b/include/book.h @@ -59,6 +59,8 @@ class Book mutable std::string data; }; + typedef std::vector> Illustrations; + public: // functions Book(); ~Book(); @@ -95,6 +97,8 @@ class Book const std::string& getFaviconUrl() const; const std::string& getFaviconMimeType() const; + Illustrations getIllustrations() const; + const std::string& getDownloadId() const { return m_downloadId; } void setReadOnly(bool readOnly) { m_readOnly = readOnly; } @@ -142,7 +146,7 @@ class Book uint64_t m_mediaCount = 0; bool m_readOnly = false; uint64_t m_size = 0; - std::vector> m_illustrations; + Illustrations m_illustrations; // Used as the return value of getDefaultIllustration() when no default // illustration is found in the book diff --git a/src/book.cpp b/src/book.cpp index de82c6fbc..bf7c3e18f 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -47,6 +47,11 @@ Book::~Book() { } +Book::Illustrations Book::getIllustrations() const +{ + return m_illustrations; +} + bool Book::update(const kiwix::Book& other) { if (m_readOnly) diff --git a/src/opds_dumper.cpp b/src/opds_dumper.cpp index 6897b3142..7d463f714 100644 --- a/src/opds_dumper.cpp +++ b/src/opds_dumper.cpp @@ -58,10 +58,10 @@ IllustrationInfo getBookIllustrationInfo(const Book& book) { kainjow::mustache::list illustrations; if ( book.isPathValid() ) { - for ( auto illustration_size : zim::Archive(book.getPath()).getIllustrationSizes() ) { + for ( const auto& illustration : book.getIllustrations() ) { illustrations.push_back(kainjow::mustache::object{ - {"icon_width", to_string(illustration_size)}, - {"icon_height", to_string(illustration_size)}, + {"icon_width", to_string(illustration->width)}, + {"icon_height", to_string(illustration->height)}, {"icon_scale", "1"}, }); } From 4a01081e83b1092a426c2ded19b02f56db46effd Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 19 Nov 2021 14:58:00 +0400 Subject: [PATCH 20/20] Thread-safe Book::Illustration::getData() --- include/book.h | 2 ++ src/book.cpp | 11 +++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/include/book.h b/include/book.h index 5d049ad6c..cdcbf51fe 100644 --- a/include/book.h +++ b/include/book.h @@ -23,6 +23,7 @@ #include #include #include +#include namespace pugi { class xml_node; @@ -57,6 +58,7 @@ class Book private: mutable std::string data; + mutable std::mutex mutex; }; typedef std::vector> Illustrations; diff --git a/src/book.cpp b/src/book.cpp index bf7c3e18f..63ebbec19 100644 --- a/src/book.cpp +++ b/src/book.cpp @@ -227,10 +227,13 @@ const Book::Illustration& Book::getDefaultIllustration() const const std::string& Book::Illustration::getData() const { if (data.empty() && !url.empty()) { - try { - data = download(url); - } catch(...) { - std::cerr << "Cannot download favicon from " << url; + const std::lock_guard l(mutex); + if ( data.empty() ) { + try { + data = download(url); + } catch(...) { + std::cerr << "Cannot download favicon from " << url; + } } } return data;