From ca965d448f6e0272fd64be792f9e3f9152cfb26f Mon Sep 17 00:00:00 2001
From: Veloman Yunkan <veloman.yunkan@gmail.com>
Date: Fri, 11 Mar 2022 22:31:33 +0400
Subject: [PATCH] Got rid of 2 parameters in Response::build_404()

Instead of passing the `bookName` and `bookTitle` parameters to
`Response::build_404()`, `withTaskbarInfo()` is applied to its result
when needed. Note, that in `InternalServer::handle_raw()`
`withTaskbarInfo()` was not utilized since the results of the `/raw`
endpoint are not supposed to be decorated with a taskbar.
---
 src/server/internalServer.cpp            | 33 ++++++++++++++----------
 src/server/internalServer_catalog_v2.cpp |  8 +++---
 src/server/response.cpp                  |  3 +--
 src/server/response.h                    |  2 +-
 test/server.cpp                          |  4 ---
 5 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp
index 569fe2c13..d89d77213 100644
--- a/src/server/internalServer.cpp
+++ b/src/server/internalServer.cpp
@@ -279,7 +279,7 @@ std::unique_ptr<Response> InternalServer::handle_request(const RequestContext& r
 {
   try {
     if (! request.is_valid_url())
-      return Response::build_404(*this, request.get_full_url(), "", "");
+      return Response::build_404(*this, request.get_full_url());
 
     const ETag etag = get_matching_if_none_match_etag(request);
     if ( etag )
@@ -406,7 +406,8 @@ std::unique_ptr<Response> InternalServer::handle_suggest(const RequestContext& r
 
   if (archive == nullptr) {
     const std::string error_details = "No such book: " + bookName;
-    return Response::build_404(*this, "", bookName, "", error_details);
+    auto response = Response::build_404(*this, "", error_details);
+    return withTaskbarInfo(bookName, nullptr, std::move(response));
   }
 
   const auto queryString = request.get_optional_param("term", std::string());
@@ -476,7 +477,7 @@ std::unique_ptr<Response> InternalServer::handle_skin(const RequestContext& requ
     response->set_cacheable();
     return std::move(response);
   } catch (const ResourceNotFound& e) {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 }
 
@@ -615,7 +616,8 @@ std::unique_ptr<Response> InternalServer::handle_random(const RequestContext& re
 
   if (archive == nullptr) {
     const std::string error_details = "No such book: " + bookName;
-    return Response::build_404(*this, "", bookName, "", error_details);
+    auto response = Response::build_404(*this, "", error_details);
+    return withTaskbarInfo(bookName, nullptr, std::move(response));
   }
 
   try {
@@ -623,7 +625,8 @@ std::unique_ptr<Response> InternalServer::handle_random(const RequestContext& re
     return build_redirect(bookName, getFinalItem(*archive, entry));
   } catch(zim::EntryNotFound& e) {
     const std::string error_details = "Oops! Failed to pick a random article :(";
-    return Response::build_404(*this, "", bookName, getArchiveTitle(*archive), error_details);
+    auto response = Response::build_404(*this, "", error_details);
+    return withTaskbarInfo(bookName, archive.get(), std::move(response));
   }
 }
 
@@ -635,7 +638,7 @@ std::unique_ptr<Response> InternalServer::handle_captured_external(const Request
   } catch (const std::out_of_range& e) {}
 
   if (source.empty())
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
 
   auto data = get_default_data();
   data.set("source", source);
@@ -654,7 +657,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog(const RequestContext& r
     host = request.get_header("Host");
     url  = request.get_url_part(1);
   } catch (const std::out_of_range&) {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 
   if (url == "v2") {
@@ -662,7 +665,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog(const RequestContext& r
   }
 
   if (url != "searchdescription.xml" && url != "root.xml" && url != "search") {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 
   if (url == "searchdescription.xml") {
@@ -799,7 +802,8 @@ std::unique_ptr<Response> InternalServer::handle_content(const RequestContext& r
     std::string searchURL = m_root + "/search?pattern=" + kiwix::urlEncode(pattern, true); // Make a full search on the entire library.
     const std::string details = searchSuggestionHTML(searchURL, kiwix::urlDecode(pattern));
 
-    return Response::build_404(*this, request.get_full_url(), bookName, "", details);
+    auto response = Response::build_404(*this, request.get_full_url(), details);
+    return withTaskbarInfo(bookName, nullptr, std::move(response));
   }
 
   auto urlStr = request.get_url().substr(bookName.size()+1);
@@ -832,7 +836,8 @@ std::unique_ptr<Response> InternalServer::handle_content(const RequestContext& r
     std::string searchURL = m_root + "/search?content=" + bookName + "&pattern=" + kiwix::urlEncode(pattern, true); // Make a search on this specific book only.
     const std::string details = searchSuggestionHTML(searchURL, kiwix::urlDecode(pattern));
 
-    return Response::build_404(*this, request.get_full_url(), bookName, getArchiveTitle(*archive), details);
+    auto response = Response::build_404(*this, request.get_full_url(), details);
+    return withTaskbarInfo(bookName, archive.get(), std::move(response));
   }
 }
 
@@ -849,12 +854,12 @@ std::unique_ptr<Response> InternalServer::handle_raw(const RequestContext& reque
      bookName = request.get_url_part(1);
      kind = request.get_url_part(2);
   } catch (const std::out_of_range& e) {
-     return Response::build_404(*this, request.get_full_url(), bookName, "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 
   if (kind != "meta" && kind!= "content") {
     const std::string error_details = kind + " is not a valid request for raw content.";
-    return Response::build_404(*this, request.get_full_url(), bookName, "", error_details);
+    return Response::build_404(*this, request.get_full_url(), error_details);
   }
 
   std::shared_ptr<zim::Archive> archive;
@@ -865,7 +870,7 @@ std::unique_ptr<Response> InternalServer::handle_raw(const RequestContext& reque
 
   if (archive == nullptr) {
     const std::string error_details = "No such book: " + bookName;
-    return Response::build_404(*this, request.get_full_url(), bookName, "", error_details);
+    return Response::build_404(*this, request.get_full_url(), error_details);
   }
 
   // Remove the beggining of the path:
@@ -890,7 +895,7 @@ std::unique_ptr<Response> InternalServer::handle_raw(const RequestContext& reque
       printf("Failed to find %s\n", itemPath.c_str());
     }
     const std::string error_details = "Cannot find " + kind + " entry " + itemPath;
-    return Response::build_404(*this, request.get_full_url(), bookName, getArchiveTitle(*archive), error_details);
+    return Response::build_404(*this, request.get_full_url(), error_details);
   }
 }
 
diff --git a/src/server/internalServer_catalog_v2.cpp b/src/server/internalServer_catalog_v2.cpp
index 3a578cb02..cd678ba55 100644
--- a/src/server/internalServer_catalog_v2.cpp
+++ b/src/server/internalServer_catalog_v2.cpp
@@ -43,7 +43,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2(const RequestContext
   try {
     url  = request.get_url_part(2);
   } catch (const std::out_of_range&) {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 
   if (url == "root.xml") {
@@ -69,7 +69,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2(const RequestContext
   } else if (url == "illustration") {
     return handle_catalog_v2_illustration(request);
   } else {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 }
 
@@ -110,7 +110,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_complete_entry(const
   try {
     mp_library->getBookById(entryId);
   } catch (const std::out_of_range&) {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 
   OPDSDumper opdsDumper(mp_library);
@@ -158,7 +158,7 @@ std::unique_ptr<Response> InternalServer::handle_catalog_v2_illustration(const R
     auto illustration = book.getIllustration(size);
     return ContentResponse::build(*this, illustration->getData(), illustration->mimeType);
   } catch(...) {
-    return Response::build_404(*this, request.get_full_url(), "", "");
+    return Response::build_404(*this, request.get_full_url());
   }
 }
 
diff --git a/src/server/response.cpp b/src/server/response.cpp
index 94f5df0b5..16c614b58 100644
--- a/src/server/response.cpp
+++ b/src/server/response.cpp
@@ -84,7 +84,7 @@ std::unique_ptr<Response> Response::build_304(const InternalServer& server, cons
   return response;
 }
 
-std::unique_ptr<ContentResponse> Response::build_404(const InternalServer& server, const std::string& url, const std::string& bookName, const std::string& bookTitle, const std::string& details)
+std::unique_ptr<ContentResponse> Response::build_404(const InternalServer& server, const std::string& url, const std::string& details)
 {
   MustacheData results;
   if ( !url.empty() ) {
@@ -94,7 +94,6 @@ std::unique_ptr<ContentResponse> Response::build_404(const InternalServer& serve
 
   auto response = ContentResponse::build(server, RESOURCE::templates::_404_html, results, "text/html");
   response->set_code(MHD_HTTP_NOT_FOUND);
-  response->set_taskbar(bookName, bookTitle);
 
   return response;
 }
diff --git a/src/server/response.h b/src/server/response.h
index e464ca54c..37bfc6929 100644
--- a/src/server/response.h
+++ b/src/server/response.h
@@ -51,7 +51,7 @@ class Response {
 
     static std::unique_ptr<Response> build(const InternalServer& server);
     static std::unique_ptr<Response> build_304(const InternalServer& server, const ETag& etag);
-    static std::unique_ptr<ContentResponse> build_404(const InternalServer& server, const std::string& url, const std::string& bookName, const std::string& bookTitle, const std::string& details="");
+    static std::unique_ptr<ContentResponse> build_404(const InternalServer& server, const std::string& url, const std::string& details="");
     static std::unique_ptr<Response> build_416(const InternalServer& server, size_t resourceLength);
     static std::unique_ptr<Response> build_500(const InternalServer& server, const std::string& msg);
     static std::unique_ptr<Response> build_redirect(const InternalServer& server, const std::string& redirectUrl);
diff --git a/test/server.cpp b/test/server.cpp
index 4eb22e063..658211216 100644
--- a/test/server.cpp
+++ b/test/server.cpp
@@ -638,8 +638,6 @@ TEST_F(ServerTest, 404WithBodyTesting)
 )"  },
 
     { /* url */ "/ROOT/raw/zimfile/meta/invalid-metadata",
-      book_name=="zimfile" &&
-      book_title=="Ray Charles" &&
       expected_body==R"(
     <h1>Not Found</h1>
     <p>
@@ -651,8 +649,6 @@ TEST_F(ServerTest, 404WithBodyTesting)
 )"  },
 
     { /* url */ "/ROOT/raw/zimfile/content/invalid-article",
-      book_name=="zimfile" &&
-      book_title=="Ray Charles" &&
       expected_body==R"(
     <h1>Not Found</h1>
     <p>