Release 9.3.0

Add new thread safe suggestion API.

.github/workflows/ci.yml

@@ -53,30 +53,41 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        target:
+        name:
           - native_static
           - native_dyn
+          - native_dyn_bionic
           - android_arm
           - android_arm64
           - win32_static
           - win32_dyn
         include:
-          - target: native_static
+          - name: native_static
+            target: native_static
             image_variant: xenial
             lib_postfix: '/x86_64-linux-gnu'
-          - target: native_dyn
+          - name: native_dyn
+            target: native_dyn
             image_variant: xenial
             lib_postfix: '/x86_64-linux-gnu'
-          - target: android_arm
+          - name: native_dyn_bionic
+            target: native_dyn
+            image_variant: bionic
+            lib_postfix: '/x86_64-linux-gnu'
+          - name: android_arm
+            target: android_arm
             image_variant: xenial
             lib_postfix: '/x86_64-linux-gnu'
-          - target: android_arm64
+          - name: android_arm64
+            target: android_arm64
             image_variant: xenial
             lib_postfix: '/x86_64-linux-gnu'
-          - target: win32_static
+          - name: win32_static
+            target: win32_static
             image_variant: f31
             lib_postfix: '64'
-          - target: win32_dyn
+          - name: win32_dyn
+            target: win32_dyn
             image_variant: f31
             lib_postfix: '64'
     env:
@@ -147,6 +158,6 @@ jobs:
         curl https://codecov.io/bash -o codecov.sh
         bash codecov.sh -n "${OS_NAME}_${{matrix.target}}" -Z
         rm codecov.sh
-      if: startsWith(matrix.target, 'native_')
+      if: startsWith(matrix.target, 'native_') && matrix.image_variant == 'xenial'
       env:
         CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

ChangeLog

@@ -1,3 +1,17 @@
+kiwix-lib 9.3.0
+===============
+
+ * Add a thread safe method to search suggestions.
+   Old methods are now deprecated.
+
+kiwix-lib 9.2.3
+===============
+
+ * Add test on byte-range
+ * Fix compilation on bionic and windows.
+ * Allow building using debian packaged kainjow-mustache
+ * Pass `-latomic` compile option for architectures that need it
+
 kiwix-lib 9.2.2
 ===============
 

android-kiwix-lib-publisher/kiwixLibAndroid/build.gradle

@@ -26,7 +26,7 @@ task writePom {
     project {
         groupId 'org.kiwix.kiwixlib'
         artifactId 'kiwixlib'
-        version '9.2.2' + (System.env.KIWIXLIB_BUILDVERSION == null ? '' : '-'+System.env.KIWIXLIB_BUILDVERSION)
+        version '9.3.0' + (System.env.KIWIXLIB_BUILDVERSION == null ? '' : '-'+System.env.KIWIXLIB_BUILDVERSION)
         packaging 'aar'
         name 'kiwixlib'
         url 'https://github.com/kiwix/kiwix-lib'

include/reader.h

@@ -43,6 +43,8 @@ namespace kiwix
  * The Reader class is the class who allow to get an entry content from a zim
  * file.
  */
+
+using SuggestionsList_t = std::vector<std::vector<std::string>>;
 class Reader
 {
  public:
@@ -419,6 +421,10 @@ class Reader
    *
    * Suggestions are stored in an internal vector and can be retrieved using
    * `getNextSuggestion` method.
+   * This method is not thread safe and is deprecated. Use :
+   * bool searchSuggestions(const string& prefix,
+   *                        unsigned int suggestionsCount,
+   *                        SuggestionsList_t& results);
    *
    * @param prefix The prefix to search.
    * @param suggestionsCount How many suggestions to search for.
@@ -426,12 +432,49 @@ class Reader
    *              If false, add suggestions to the internal vector
    *              (until internal vector size is suggestionCount (or no more
    *               suggestion))
-   * @return True if some suggestions where added to the internal vector.
+   * @return True if some suggestions have been added to the internal vector.
    */
-  bool searchSuggestions(const string& prefix,
+  DEPRECATED bool searchSuggestions(const string& prefix,
                          unsigned int suggestionsCount,
                          const bool reset = true);
 
+  /**
+   * Search for entries with title starting with prefix (case sensitive).
+   *
+   * Suggestions are added to the `result` vector.
+   *
+   * @param prefix The prefix to search.
+   * @param suggestionsCount How many suggestions to search for.
+   * @param result The vector where to store the suggestions.
+   * @return True if some suggestions have been added to the vector.
+   */
+
+  bool searchSuggestions(const string& prefix,
+                         unsigned int suggestionsCount,
+                         SuggestionsList_t& resuls);
+
+  /**
+   * Search for entries for the given prefix.
+   *
+   * If the zim file has a internal fulltext index, the suggestions will be
+   * searched using it.
+   * Else the suggestions will be search using `searchSuggestions` while trying
+   * to be smart about case sensitivity (using `getTitleVariants`).
+   *
+   * In any case, suggestions are stored in an internal vector and can be
+   * retrieved using `getNextSuggestion` method.
+   * The internal vector will be reset.
+   * This method is not thread safe and is deprecated. Use :
+   * bool searchSuggestionsSmart(const string& prefix,
+   *                             unsigned int suggestionsCount,
+   *                             SuggestionsList_t& results);
+   *
+   * @param prefix The prefix to search for.
+   * @param suggestionsCount How many suggestions to search for.
+   */
+  DEPRECATED bool searchSuggestionsSmart(const string& prefix,
+                              unsigned int suggestionsCount);
+
   /**
    * Search for entries for the given prefix.
    *
@@ -446,9 +489,13 @@ class Reader
    *
    * @param prefix The prefix to search for.
    * @param suggestionsCount How many suggestions to search for.
+   * @param results The vector where to store the suggestions
+   * @return True if some suggestions have been added to the results.
    */
-  bool searchSuggestionsSmart(const string& prefix,
-                              unsigned int suggestionsCount);
+   bool searchSuggestionsSmart(const string& prefix,
+                              unsigned int suggestionsCount,
+                              SuggestionsList_t& results);
+
 
   /**
    * Check if the url exists in the zim file.
@@ -490,7 +537,7 @@ class Reader
    * @param[out] title the title of the suggestion.
    * @return True if title has been set.
    */
-  bool getNextSuggestion(string& title);
+  DEPRECATED bool getNextSuggestion(string& title);
 
   /**
    * Get the next suggestion title and url.
@@ -499,7 +546,7 @@ class Reader
    * @param[out] url the url of the suggestion.
    * @return True if title and url have been set.
    */
-  bool getNextSuggestion(string& title, string& url);
+  DEPRECATED bool getNextSuggestion(string& title, string& url);
 
   /**
    * Get if we can check zim file integrity (has a checksum).
@@ -559,8 +606,8 @@ class Reader
   zim::size_type nsICount;
   std::string zimFilePath;
 
-  std::vector<std::vector<std::string>> suggestions;
-  std::vector<std::vector<std::string>>::iterator suggestionsOffset;
+  SuggestionsList_t suggestions;
+  SuggestionsList_t::iterator suggestionsOffset;
 
  private:
   std::map<const std::string, unsigned int> parseCounterMetadata() const;

meson.build

@@ -1,5 +1,5 @@
 project('kiwix-lib', 'cpp',
-  version : '9.2.2', # Also change this in android-kiwix-lib-publisher/kiwixLibAndroid/build.gradle
+  version : '9.3.0', # Also change this in android-kiwix-lib-publisher/kiwixLibAndroid/build.gradle
   license : 'GPL',
   default_options : ['c_std=c11', 'cpp_std=c++11', 'werror=true'])
 
@@ -18,6 +18,11 @@ if 'java' in wrapper
   add_languages('java')
 endif
 
+# See https://github.com/kiwix/kiwix-lib/issues/371
+if target_machine.cpu_family() in ['arm', 'mips', 'm68k', 'ppc']
+  extra_libs += '-latomic'
+endif
+
 thread_dep = dependency('threads')
 libicu_dep = dependency('icu-i18n', static:static_deps)
 libzim_dep = dependency('libzim', version : '>=6.1.1', static:static_deps)
@@ -25,7 +30,11 @@ pugixml_dep = dependency('pugixml', static:static_deps)
 libcurl_dep = dependency('libcurl', static:static_deps)
 microhttpd_dep = dependency('libmicrohttpd', static:static_deps)
 
-if not compiler.has_header('mustache.hpp')
+if compiler.has_header('mustache.hpp')
+  extra_include = []
+elif compiler.has_header('mustache.hpp', args: '-I/usr/include/kainjow')
+  extra_include = ['/usr/include/kainjow']
+else
   error('Cannot found header mustache.hpp')
 endif
 
@@ -37,7 +46,7 @@ endif
 
 all_deps = [thread_dep, libicu_dep, libzim_dep, pugixml_dep, libcurl_dep, microhttpd_dep]
 
-inc = include_directories('include')
+inc = include_directories('include', extra_include)
 
 conf = configuration_data()
 conf.set('VERSION', '"@0@"'.format(meson.project_version()))

src/meson.build

@@ -53,6 +53,7 @@ kiwixlib = library('kiwix',
                    kiwix_sources,
                    include_directories : inc,
                    dependencies : all_deps,
+                   link_args: extra_libs,
                    version: meson.project_version(),
                    install: true,
                    install_dir: install_dir)

src/reader.cpp

@@ -709,12 +709,11 @@ bool Reader::hasFulltextIndex() const
 }
 
 /* Search titles by prefix */
+
 bool Reader::searchSuggestions(const string& prefix,
                                unsigned int suggestionsCount,
                                const bool reset)
 {
-  bool retVal = false;
-
   /* Reset the suggestions otherwise check if the suggestions number is less
    * than the suggestionsCount */
   if (reset) {
@@ -726,6 +725,21 @@ bool Reader::searchSuggestions(const string& prefix,
     }
   }
 
+  auto ret =  searchSuggestions(prefix, suggestionsCount, this->suggestions);
+
+  /* Set the cursor to the begining */
+  this->suggestionsOffset = this->suggestions.begin();
+
+  return ret;
+}
+
+
+bool Reader::searchSuggestions(const string& prefix,
+                               unsigned int suggestionsCount,
+                               SuggestionsList_t& results)
+{
+  bool retVal = false;
+
   /* Return if no prefix */
   if (prefix.size() == 0) {
     return false;
@@ -734,7 +748,7 @@ bool Reader::searchSuggestions(const string& prefix,
   for (auto articleItr = zimFileHandler->findByTitle('A', prefix);
        articleItr != zimFileHandler->end()
        && articleItr->getTitle().compare(0, prefix.size(), prefix) == 0
-       && this->suggestions.size() < suggestionsCount;
+       && results.size() < suggestionsCount;
        ++articleItr) {
     /* Extract the interesting part of article title & url */
     std::string normalizedArticleTitle
@@ -754,8 +768,8 @@ bool Reader::searchSuggestions(const string& prefix,
        title) */
     bool insert = true;
     std::vector<std::vector<std::string>>::iterator suggestionItr;
-    for (suggestionItr = this->suggestions.begin();
-         suggestionItr != this->suggestions.end();
+    for (suggestionItr = results.begin();
+         suggestionItr != results.end();
          suggestionItr++) {
       int result = normalizedArticleTitle.compare((*suggestionItr)[2]);
       if (result == 0 && articleFinalUrl.compare((*suggestionItr)[1]) == 0) {
@@ -772,16 +786,13 @@ bool Reader::searchSuggestions(const string& prefix,
       suggestion.push_back(articleItr->getTitle());
       suggestion.push_back(articleFinalUrl);
       suggestion.push_back(normalizedArticleTitle);
-      this->suggestions.insert(suggestionItr, suggestion);
+      results.insert(suggestionItr, suggestion);
     }
 
     /* Suggestions where found */
     retVal = true;
   }
 
-  /* Set the cursor to the begining */
-  this->suggestionsOffset = this->suggestions.begin();
-
   return retVal;
 }
 
@@ -796,15 +807,28 @@ std::vector<std::string> Reader::getTitleVariants(
   return variants;
 }
 
-/* Try also a few variations of the prefix to have better results */
+
 bool Reader::searchSuggestionsSmart(const string& prefix,
                                     unsigned int suggestionsCount)
+{
+  this->suggestions.clear();
+  this->suggestionsOffset = this->suggestions.begin();
+
+  auto ret = searchSuggestionsSmart(prefix, suggestionsCount, this->suggestions);
+
+  this->suggestionsOffset = this->suggestions.begin();
+
+  return ret;
+}
+
+/* Try also a few variations of the prefix to have better results */
+bool Reader::searchSuggestionsSmart(const string& prefix,
+                                    unsigned int suggestionsCount,
+                                    SuggestionsList_t& results)
 {
   std::vector<std::string> variants = this->getTitleVariants(prefix);
   bool retVal = false;
 
-  this->suggestions.clear();
-  this->suggestionsOffset = this->suggestions.begin();
   /* Try to search in the title using fulltext search database */
   const auto suggestionSearch
       = this->getZimFileHandler()->suggestions(prefix, 0, suggestionsCount);
@@ -820,15 +844,14 @@ bool Reader::searchSuggestionsSmart(const string& prefix,
       suggestion.push_back(current->getTitle());
       suggestion.push_back("/A/" + current->getUrl());
       suggestion.push_back(kiwix::normalize(current->getTitle()));
-      this->suggestions.push_back(suggestion);
+      results.push_back(suggestion);
     }
-    this->suggestionsOffset = this->suggestions.begin();
     retVal = true;
   } else {
     for (std::vector<std::string>::iterator variantsItr = variants.begin();
          variantsItr != variants.end();
          variantsItr++) {
-      retVal = this->searchSuggestions(*variantsItr, suggestionsCount, false)
+      retVal = this->searchSuggestions(*variantsItr, suggestionsCount, results)
                || retVal;
     }
   }

src/server.cpp

@@ -543,7 +543,6 @@ Response InternalServer::handle_suggest(const RequestContext& request)
   std::string mimeType;
   unsigned int maxSuggestionCount = 10;
   unsigned int suggestionCount = 0;
-  std::string suggestion;
 
   std::string bookName;
   std::string bookId;
@@ -567,11 +566,12 @@ Response InternalServer::handle_suggest(const RequestContext& request)
   bool first = true;
   if (reader != nullptr) {
     /* Get the suggestions */
-    reader->searchSuggestionsSmart(term, maxSuggestionCount);
-    while (reader->getNextSuggestion(suggestion)) {
+    SuggestionsList_t suggestions;
+    reader->searchSuggestionsSmart(term, maxSuggestionCount, suggestions);
+    for(auto& suggestion:suggestions) {
       MustacheData result;
-      result.set("label", suggestion);
-      result.set("value", suggestion);
+      result.set("label", suggestion[0]);
+      result.set("value", suggestion[0]);
       result.set("first", first);
       first = false;
       results.push_back(result);

src/server/byte_range.cpp

@@ -23,6 +23,7 @@
 #include "tools/stringTools.h"
 
 #include <cassert>
+#include <algorithm>
 
 namespace kiwix {
 

src/wrapper/java/kiwixreader.cpp

@@ -355,9 +355,12 @@ Java_org_kiwix_kiwixlib_JNIKiwixReader_searchSuggestions(JNIEnv* env,
   unsigned int cCount = jni2c(count, env);
 
   try {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
     if (READER->searchSuggestionsSmart(cPrefix, cCount)) {
       retVal = JNI_TRUE;
     }
+#pragma GCC diagnostic pop
   } catch (std::exception& e) {
     LOG("Unable to get search results for pattern: %s", cPrefix.c_str());
     LOG(e.what());
@@ -377,11 +380,14 @@ Java_org_kiwix_kiwixlib_JNIKiwixReader_getNextSuggestion(JNIEnv* env,
   std::string cUrl;
 
   try {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
     if (READER->getNextSuggestion(cTitle, cUrl)) {
       setStringObjValue(cTitle, titleObj, env);
       setStringObjValue(cUrl, urlObj, env);
       retVal = JNI_TRUE;
     }
+#pragma GCC diagnostic pop
   } catch (std::exception& e) {
     LOG("Unable to get next suggestion");
     LOG(e.what());

test/data/create_corner_cases_zim_file

@@ -0,0 +1,15 @@
+#!/usr/bin/env bash
+
+cd "$(dirname "$0")"
+rm -f corner_cases.zim
+zimwriterfs -w empty.html \
+            -f empty.png \
+            -l=en \
+            -t="ZIM corner cases" \
+            -d="" \
+            -c="" \
+            -p="" \
+            corner_cases \
+            corner_cases.zim \
+&& echo 'corner_cases.zim was successfully created' \
+|| echo '!!! Failed to create corner_cases.zim !!!' >&2

test/meson.build

@@ -29,6 +29,9 @@ if gtest_dep.found() and not meson.is_cross_build()
     configure_file(input : 'data/wikipedia_en_ray_charles_mini_2020-03.zim',
                    output : 'zimfile.zim',
                    copy: true )
+    configure_file(input : 'data/corner_cases.zim',
+                   output : 'corner_cases.zim',
+                   copy: true )
 
     foreach test_name : tests
         # XXX: implicit_include_directories must be set to false, otherwise

test/server.cpp

@@ -50,9 +50,10 @@ class ZimFileServer
 {
 public: // types
   typedef std::shared_ptr<httplib::Response>  Response;
+  typedef std::vector<std::string> FilePathCollection;
 
 public: // functions
-  ZimFileServer(int serverPort, std::string zimpath);
+  ZimFileServer(int serverPort, const FilePathCollection& zimpaths);
   ~ZimFileServer();
 
   Response GET(const char* path, const Headers& headers = Headers())
@@ -73,11 +74,13 @@ private: // data
   std::unique_ptr<httplib::Client> client;
 };
 
-ZimFileServer::ZimFileServer(int serverPort, std::string zimpath)
+ZimFileServer::ZimFileServer(int serverPort, const FilePathCollection& zimpaths)
 : manager(&this->library)
 {
-  if (!manager.addBookFromPath(zimpath, zimpath, "", false))
-    throw std::runtime_error("Unable to add the ZIM file '" + zimpath + "'");
+  for ( const auto zimpath : zimpaths ) {
+    if (!manager.addBookFromPath(zimpath, zimpath, "", false))
+      throw std::runtime_error("Unable to add the ZIM file '" + zimpath + "'");
+  }
 
   const std::string address = "127.0.0.1";
   nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false));
@@ -104,11 +107,14 @@ protected:
   std::unique_ptr<ZimFileServer>   zfs1_;
 
   const int PORT = 8001;
-  const std::string ZIMFILE = "./test/zimfile.zim";
+  const ZimFileServer::FilePathCollection ZIMFILES {
+    "./test/zimfile.zim",
+    "./test/corner_cases.zim"
+  };
 
 protected:
   void SetUp() override {
-    zfs1_.reset(new ZimFileServer(PORT, ZIMFILE));
+    zfs1_.reset(new ZimFileServer(PORT, ZIMFILES));
   }
 
   void TearDown() override {
@@ -174,6 +180,10 @@ const ResourceCollection resources200Uncompressible{
   { WITH_ETAG, "/meta?content=zimfile&name=favicon" },
 
   { WITH_ETAG, "/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" },
 };
 
 ResourceCollection all200Resources()
@@ -307,7 +317,7 @@ TEST_F(ServerTest, ETagIsTheSameAcrossHeadAndGet)
 
 TEST_F(ServerTest, DifferentServerInstancesProduceDifferentETags)
 {
-  ZimFileServer zfs2(PORT + 1, ZIMFILE);
+  ZimFileServer zfs2(PORT + 1, ZIMFILES);
   for ( const Resource& res : all200Resources() ) {
     if ( !res.etag_expected ) continue;
     const auto h1 = zfs1_->HEAD(res.url);
@@ -428,7 +438,7 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly)
     const auto p = zfs1_->GET(url, { {"Range", "bytes=0-10"} } );
     EXPECT_EQ(206, p->status);
     EXPECT_EQ("bytes 0-10/20077", p->get_header_value("Content-Range"));
-    EXPECT_EQ(11, p->body.size());
+    EXPECT_EQ(11U, p->body.size());
     EXPECT_EQ(full->body.substr(0, 11), p->body);
     EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges"));
   }
@@ -437,7 +447,7 @@ TEST_F(ServerTest, ValidSingleRangeByteRangeRequestsAreHandledProperly)
     const auto p = zfs1_->GET(url, { {"Range", "bytes=123-456"} } );
     EXPECT_EQ(206, p->status);
     EXPECT_EQ("bytes 123-456/20077", p->get_header_value("Content-Range"));
-    EXPECT_EQ(334, p->body.size());
+    EXPECT_EQ(334U, p->body.size());
     EXPECT_EQ(full->body.substr(123, 334), p->body);
     EXPECT_EQ("bytes", p->get_header_value("Accept-Ranges"));
   }
@@ -480,6 +490,25 @@ TEST_F(ServerTest, InvalidAndMultiRangeByteRangeRequestsResultIn416Responses)
   }
 }
 
+TEST_F(ServerTest, ValidByteRangeRequestsOfZeroSizedEntriesResultIn416Responses)
+{
+  const char url[] = "/corner_cases/-/empty.js";
+
+  const char* ranges[] = {
+    "bytes=0-",
+    "bytes=-100"
+  };
+
+  for( const char* range : ranges )
+  {
+    const TestContext ctx{ {"Range", range} };
+    const auto p = zfs1_->GET(url, { {"Range", range } } );
+    EXPECT_EQ(416, p->status) << ctx;
+    EXPECT_TRUE(p->body.empty()) << ctx;
+    EXPECT_EQ("bytes */0", p->get_header_value("Content-Range")) << ctx;
+  }
+}
+
 TEST_F(ServerTest, RangeHasPrecedenceOverCompression)
 {
   const char url[] = "/zimfile/I/m/Ray_Charles_classic_piano_pose.jpg";