Introduce a pagination object to render search result.

src/search_renderer.cpp

@@ -86,8 +86,69 @@ void SearchRenderer::setSearchProtocolPrefix(const std::string& prefix)
   this->searchProtocolPrefix = prefix;
 }
 
+kainjow::mustache::data buildPagination(
+  unsigned int pageLength,
+  unsigned int resultsCount,
+  unsigned int resultsStart
+)
+{
+  assert(pageLength!=0);
+  kainjow::mustache::data pagination;
+  kainjow::mustache::data pages{kainjow::mustache::data::type::list};
+
+  if (resultsCount == 0) {
+    // Easy case
+    pagination.set("itemsPerPage", to_string(pageLength));
+    pagination.set("hasPages", false);
+    pagination.set("pages", pages);
+    return pagination;
+  }
+
+  // First we want to display pages starting at a multiple of `pageLength`
+  // so, let's calculate the start index of the current page.
+  auto currentPage = resultsStart/pageLength;
+  auto lastPage = ((resultsCount-1)/pageLength);
+  auto lastPageStart = lastPage*pageLength;
+  auto nbPages = lastPage + 1;
+
+  auto firstPageGenerated = currentPage > 4 ? currentPage-4 : 0;
+  auto lastPageGenerated = min(currentPage+4, lastPage);
+
+  if (nbPages != 1) {
+    if (firstPageGenerated!=0) {
+      kainjow::mustache::data page;
+      page.set("label", "◀");
+      page.set("start", to_string(0));
+      page.set("current", false);
+      pages.push_back(page);
+    }
+
+    for (auto i=firstPageGenerated; i<=lastPageGenerated; i++) {
+      kainjow::mustache::data page;
+      page.set("label", to_string(i+1));
+      page.set("start", to_string(i*pageLength));
+      page.set("current", bool(i == currentPage));
+      pages.push_back(page);
+    }
+
+    if (lastPageGenerated!=lastPage) {
+      kainjow::mustache::data page;
+      page.set("label", "▶");
+      page.set("start", to_string(lastPageStart));
+      page.set("current", false);
+      pages.push_back(page);
+    }
+  }
+
+  pagination.set("itemsPerPage", to_string(pageLength));
+  pagination.set("hasPages", firstPageGenerated < lastPageGenerated);
+  pagination.set("pages", pages);
+  return pagination;
+}
+
 std::string SearchRenderer::getHtml()
 {
+  // Build the results list
   kainjow::mustache::data results{kainjow::mustache::data::type::list};
 
   for (auto it = m_srs.begin(); it != m_srs.end(); it++) {
@@ -110,57 +171,31 @@ std::string SearchRenderer::getHtml()
     results.push_back(result);
   }
 
-  // pages
-  kainjow::mustache::data pages{kainjow::mustache::data::type::list};
 
-  auto resultEnd = 0U;
+  // pagination
-  auto currentPage = 0U;
+  auto pagination = buildPagination(
-  auto pageStart = 0U;
+    pageLength,
-  auto pageEnd = 0U;
+    estimatedResultCount,
-  auto lastPageStart = 0U;
+    resultStart
-  if (pageLength) {
+  );
-    currentPage = resultStart/pageLength;
-    pageStart = currentPage > 4 ? currentPage-4 : 0;
-    pageEnd = currentPage + 5;
-    if (pageEnd > estimatedResultCount / pageLength) {
-      pageEnd = (estimatedResultCount + pageLength - 1) / pageLength;
-    }
-    if (estimatedResultCount > pageLength) {
-      lastPageStart = ((estimatedResultCount-1)/pageLength)*pageLength;
-    }
-  }
 
-  resultEnd = resultStart+pageLength; //setting result end
+  auto resultEnd = min(resultStart+pageLength, estimatedResultCount);
-
-  for (unsigned int i = pageStart; i < pageEnd; i++) {
-    kainjow::mustache::data page;
-    page.set("label", to_string(i + 1));
-    page.set("start", to_string(i * pageLength));
-
-    if (i == currentPage) {
-      page.set("selected", true);
-    }
-    pages.push_back(page);
-  }
 
   std::string template_str = RESOURCE::templates::search_result_html;
   kainjow::mustache::mustache tmpl(template_str);
 
   kainjow::mustache::data allData;
   allData.set("results", results);
-  allData.set("pages", pages);
   allData.set("hasResults", estimatedResultCount != 0);
-  allData.set("hasPages", pageStart + 1 < pageEnd);
   allData.set("count", kiwix::beautifyInteger(estimatedResultCount));
   allData.set("searchPattern", kiwix::encodeDiples(this->searchPattern));
   allData.set("searchPatternEncoded", urlEncode(this->searchPattern));
   allData.set("resultStart", to_string(resultStart + 1));
-  allData.set("resultEnd", to_string(min(resultEnd, estimatedResultCount)));
+  allData.set("resultEnd", to_string(resultEnd));
-  allData.set("pageLength", to_string(pageLength));
-  allData.set("resultLastPageStart", to_string(lastPageStart));
   allData.set("protocolPrefix", this->protocolPrefix);
   allData.set("searchProtocolPrefix", this->searchProtocolPrefix);
   allData.set("contentId", this->searchContent);
+  allData.set("pagination", pagination);
 
   std::stringstream ss;
   tmpl.render(allData, [&ss](const std::string& str) { ss << str; });

static/templates/search_result.html

@@ -143,34 +143,18 @@
     </div>
 
     <div class="footer">
-      {{#hasPages}}
+      {{#pagination.hasPages}}
         <ul>
-          {{#resultLastPageStart}}
+          {{#pagination.pages}}
             <li>
-              <a {{! let the format of this tag be identical to the case below }}
+              <a {{#current}}class="selected"{{/current}}
-                 href="{{searchProtocolPrefix}}pattern={{searchPatternEncoded}}{{#contentId}}&content={{.}}{{/contentId}}&start=0&pageLength={{pageLength}}">
+                 href="{{searchProtocolPrefix}}pattern={{searchPatternEncoded}}{{#contentId}}&content={{.}}{{/contentId}}&start={{start}}&pageLength={{pagination.itemsPerPage}}">
-                ◀
-              </a>
-            </li>
-          {{/resultLastPageStart}}
-          {{#pages}}
-            <li>
-              <a {{#selected}}class="selected"{{/selected}}
-                 href="{{searchProtocolPrefix}}pattern={{searchPatternEncoded}}{{#contentId}}&content={{.}}{{/contentId}}&start={{start}}&pageLength={{pageLength}}">
                 {{label}}
               </a>
             </li>
-          {{/pages}}
+          {{/pagination.pages}}
-          {{#resultLastPageStart}}
-            <li>
-              <a {{! let the format of this tag be identical to the case above }}
-                 href="{{searchProtocolPrefix}}pattern={{searchPatternEncoded}}{{#contentId}}&content={{.}}{{/contentId}}&start={{resultLastPageStart}}&pageLength={{pageLength}}">
-                ▶
-              </a>
-            </li>
-          {{/resultLastPageStart}}
         </ul>
-      {{/hasPages}}
+      {{/pagination.hasPages}}
     </div>
   </body>
 </html>

test/meson.build

@@ -9,7 +9,8 @@ tests = [
     'book',
     'manager',
     'name_mapper',
-    'opds_catalog'
+    'opds_catalog',
+    'server_helper'
 ]
 
 if build_machine.system() != 'windows'
@@ -59,6 +60,7 @@ if gtest_dep.found() and not meson.is_cross_build()
         # XXX: '#include <regex>' includes the regex unit test binary
         test_exe = executable(test_name, [test_name+'.cpp'],
                               implicit_include_directories: false,
+                              include_directories : inc,
                               link_with : kiwixlib,
                               link_args: extra_link_args,
                               dependencies : all_deps + [gtest_dep],

test/server.cpp

@@ -1849,7 +1849,6 @@ R"SEARCHRESULT(
       },
 
       /* pagination */ {
-        { "◀", 0,  false },
         { "1", 0,  true  },
         { "2", 5,  false },
         { "3", 10, false },
@@ -1874,7 +1873,6 @@ R"SEARCHRESULT(
       },
 
       /* pagination */ {
-        { "◀", 0,  false },
         { "1", 0,  false },
         { "2", 5,  true  },
         { "3", 10, false },
@@ -1900,7 +1898,6 @@ R"SEARCHRESULT(
       },
 
       /* pagination */ {
-        { "◀", 0,  false },
         { "1", 0,  false },
         { "2", 5,  false },
         { "3", 10, true  },
@@ -1927,7 +1924,6 @@ R"SEARCHRESULT(
       },
 
       /* pagination */ {
-        { "◀", 0,  false },
         { "1", 0,  false },
         { "2", 5,  false },
         { "3", 10, false },
@@ -1955,7 +1951,6 @@ R"SEARCHRESULT(
       },
 
       /* pagination */ {
-        { "◀", 0,  false },
         { "1", 0,  false },
         { "2", 5,  false },
         { "3", 10, false },
@@ -1965,7 +1960,6 @@ R"SEARCHRESULT(
         { "7", 30, false },
         { "8", 35, false },
         { "9", 40, false },
-        { "▶", 40, false },
       }
     },
 
@@ -1993,7 +1987,6 @@ R"SEARCHRESULT(
         { "7", 30, false },
         { "8", 35, false },
         { "9", 40, false },
-        { "▶", 40, false },
       }
     },
 
@@ -2020,7 +2013,6 @@ R"SEARCHRESULT(
         { "7", 30, true  },
         { "8", 35, false },
         { "9", 40, false },
-        { "▶", 40, false },
       }
     },
 
@@ -2046,7 +2038,6 @@ R"SEARCHRESULT(
         { "7", 30, false },
         { "8", 35, true  },
         { "9", 40, false },
-        { "▶", 40, false },
       }
     },
 
@@ -2070,7 +2061,6 @@ R"SEARCHRESULT(
         { "7", 30, false },
         { "8", 35, false },
         { "9", 40, true  },
-        { "▶", 40, false },
       }
     },
 
@@ -2117,7 +2107,6 @@ R"SEARCHRESULT(
         { "7", 30, false },
         { "8", 35, false },
         { "9", 40, false  },
-        { "▶", 40, false },
       }
     },
   };

test/server_helper.cpp

@@ -0,0 +1,215 @@
+
+#include <mustache.hpp>
+#include "gtest/gtest.h"
+#include "../src/tools/stringTools.h"
+
+namespace {
+  struct ExpectedPage {
+    ExpectedPage(const std::string& label, unsigned int start, bool current)
+     : label(label),
+       start(start),
+       current(current)
+    {}
+
+    testing::AssertionResult isEqual(const kainjow::mustache::data& data) {
+      if (!data.is_object()
+         || data.get("label") == nullptr
+         || data.get("start") == nullptr
+         || data.get("current") == nullptr) {
+        return testing::AssertionFailure() << "data is not a valid object";
+      }
+
+      if (data.get("label")->string_value() != label) {
+        return testing::AssertionFailure() << data.get("label")->string_value()
+                                           << " is not equal to "
+                                           << label;
+      }
+
+      if (data.get("start")->string_value() != kiwix::to_string(start)) {
+        return testing::AssertionFailure() << data.get("start")->string_value()
+                                           << " is not equal to "
+                                           << kiwix::to_string(start);
+      }
+
+      if (current && !data.get("current")->is_true()) {
+        return testing::AssertionFailure() << "data is not true";
+      }
+      if (!current && !data.get("current")->is_false()) {
+        return testing::AssertionFailure() << "data is not false";
+      }
+      return testing::AssertionSuccess();
+    }
+
+    std::string label;
+    unsigned int start;
+    bool current;
+  };
+
+  class ExpectedPages : public std::vector<ExpectedPage>
+  {
+    public:
+      ExpectedPages(std::vector<ExpectedPage>&& v)
+        : std::vector<ExpectedPage>(v)
+      {}
+
+      testing::AssertionResult isEqual(const kainjow::mustache::data& data) {
+        if (empty()) {
+          if (data.is_empty_list()) {
+            return testing::AssertionSuccess();
+          } else {
+            return testing::AssertionFailure() << "data is not an empty list.";
+          }
+        }
+
+        if (! data.is_non_empty_list()) {
+          return testing::AssertionFailure() << "data is not a non empty list.";
+        }
+        auto& data_pages = data.list_value();
+        if (size() != data_pages.size()) {
+          return testing::AssertionFailure() << "data (size " << data_pages.size() << ")"
+                                             << " and expected (size " << size() << ")"
+                                             << " must have the same size";
+        }
+        auto it1 = begin();
+        auto it2 = data_pages.begin();
+        while (it1!=end()) {
+          auto result = it1->isEqual(*it2);
+          if(!result) {
+            return result;
+          }
+          it1++; it2++;
+        }
+        return testing::AssertionSuccess();
+      }
+  };
+}
+
+namespace kiwix {
+kainjow::mustache::data buildPagination(
+  unsigned int pageLength,
+  unsigned int resultsCount,
+  unsigned int resultsStart
+);
+}
+
+TEST(SearchRenderer, buildPagination) {
+  {
+    auto pagination = kiwix::buildPagination(10, 0, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_false());
+    ASSERT_TRUE(ExpectedPages({}).isEqual(*pagination.get("pages")));
+  }
+  {
+    auto pagination = kiwix::buildPagination(10, 1, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_false());
+    ASSERT_TRUE(ExpectedPages({}).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 10, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_false());
+    ASSERT_TRUE(ExpectedPages({}).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 11, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"1", 0, true},
+      {"2", 10, false},
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 20, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"1", 0, true},
+      {"2", 10, false},
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 21, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"1", 0, true},
+      {"2", 10, false},
+      {"3", 20, false}
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 21, 11);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"1", 0, false},
+      {"2", 10, true},
+      {"3", 20, false}
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 21, 21);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"1", 0, false},
+      {"2", 10, false},
+      {"3", 20, true}
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 200, 0);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"1", 0, true},
+      {"2", 10, false},
+      {"3", 20, false},
+      {"4", 30, false},
+      {"5", 40, false},
+      {"▶", 190, false}
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 200, 105);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"◀", 0, false},
+      {"7", 60, false},
+      {"8", 70, false},
+      {"9", 80, false},
+      {"10", 90, false},
+      {"11", 100, true},
+      {"12", 110, false},
+      {"13", 120, false},
+      {"14", 130, false},
+      {"15", 140, false},
+      {"▶", 190, false}
+    }).isEqual(*pagination.get("pages")));
+  }
+
+  {
+    auto pagination = kiwix::buildPagination(10, 200, 199);
+    ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10");
+    ASSERT_TRUE(pagination.get("hasPages")->is_true());
+    ASSERT_TRUE(ExpectedPages({
+      {"◀", 0, false},
+      {"16", 150, false},
+      {"17", 160, false},
+      {"18", 170, false},
+      {"19", 180, false},
+      {"20", 190, true}
+    }).isEqual(*pagination.get("pages")));
+  }
+}