Merge lp:~jpakkane/unity-lens-applications/columbustestintegration into lp:unity-lens-applications

[Jussi Pakkanen]

Add libcolumbus fuzzy matching.

Add libcolumbus fuzzy matching.

[Paweł Stołowski (stolowski)]

325 + //cout << "Columbus matched " << results.size() << " documents." << endl;

Please remove.

357 + bool has_category = strstr(search_string, "category:") != NULL;
358 + const char *tmp = strstr(search_string, "AND");
359 + if(has_category || !tmp) {

Could you rename tmp? Shouldn't it actually say "tmp", not "!tmp" (we want complex queries to be handled by xapian search and only simple queries to be routed to columbus, right?). I don't quite understand what case we are trying to cover here and why - can you add a comment to this code?

Could you also add a manual tests to tests/manual/? Ideally it should fail completly if exectued on old app lens (without columbus); the test should cover cases with and without category filters.

Please check with didrocks if debian/changelog needs updating after chaning control.

review: Needs Fixing

[Jussi Pakkanen (jpakkane)]

The query string is of the type "xapian keyword stuff AND query text". If the query text is empty, the AND part is missing and strstr returns NULL. This is a Xapian keyword only search and thus we pass it to Xapian.

I renamed the variable so it is now clearer. Will contact didrocks about the changelog soon.

[Jussi Pakkanen (jpakkane)]

All issues have been fixed. The correct way to populate the changelog is to use bugs and link them. This has now been done.

However libcolumbus is currently in universe so let's not merge this until it gets to main.

[Michael Terry (mterry)]

libcolumbus's MIR is approved (bug 1122269). It's just waiting for something in main to depend on it before it gets promoted. If you want to be the first, go ahead and land this.

[Paweł Stołowski (stolowski)]

Looks good.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-lens-applications-mbs-autolanding/19/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-lens-applications-mbs-autolanding/./build=panda-pbuilder,distribution=raring,flavor=armhf/19/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-lens-applications-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=amd64/19/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-lens-applications-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=i386/19/console

review: Needs Fixing (continuous-integration)

[lp:~jpakkane/unity-lens-applications/columbustestintegration]

334. By Jussi Pakkanen on 2013-03-01

Bump required libcolumbus version.

335. By Jussi Pakkanen on 2013-03-01

Add libcolumbus flags to test subdirectory too.

=== modified file 'configure.ac'
--- configure.ac	2013-02-15 10:11:07 +0000
+++ configure.ac	2013-03-01 15:43:20 +0000
@@ -14,7 +14,7 @@
 #############################################
 # Init the other things we depend on
 #############################################
-AM_MAINTAINER_MODE
+AM_MAINTAINER_MODE([enable])
 AM_PROG_VALAC([0.18.0])
 AS_IF([test -z "$VALAC"], [AC_MSG_ERROR(["No valac compiler found."])])
 AC_PROG_CC
@@ -75,6 +75,7 @@
                   gee-1.0
                   dee-1.0 >= 0.5.16
                   zeitgeist-1.0 >= 0.3.8
+                  libcolumbus0 >= 0.4.0
                   unity >= 6.7.0
                   libgnome-menu-3.0 >= 3.6.0)
 

=== modified file 'debian/control'
--- debian/control	2012-11-08 04:49:06 +0000
+++ debian/control	2013-03-01 15:43:20 +0000
@@ -16,6 +16,7 @@
                libxapian-dev,
                libdb-dev,
                libapt-pkg-dev,
+               libcolumbus0-dev,
 Standards-Version: 3.9.3
 Homepage: https://launchpad.net/unity-lens-applications
 # If you aren't a member of ~unity-team but need to upload packaging changes,

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2013-02-20 15:31:37 +0000
+++ src/Makefile.am	2013-03-01 15:43:20 +0000
@@ -1,7 +1,7 @@
 NULL =
 BUILT_SOURCES =
 CLEANFILES =
-EXTRA_DIST =
+EXTRA_DIST =	
 
 DATADIR = $(datadir)
 
@@ -75,7 +75,7 @@
   $(NULL)
 
 unity-package-search.o : $(srcdir)/unity-package-search.cc $(srcdir)/unity-package-search.h
-	$(AM_V_GEN)$(CXX) $(COVERAGE_CFLAGS) -g $(unity_package_search_libs) -DGMENU_I_KNOW_THIS_IS_UNSTABLE `pkg-config --cflags --libs glib-2.0 libgnome-menu-3.0 unity gee-1.0` -c $(srcdir)/unity-package-search.cc
+	$(AM_V_GEN)$(CXX) --std=c++11 $(COVERAGE_CFLAGS) -g $(unity_package_search_libs) -DGMENU_I_KNOW_THIS_IS_UNSTABLE `pkg-config --cflags --libs glib-2.0 libgnome-menu-3.0 unity gee-1.0 libcolumbus0` -c $(srcdir)/unity-package-search.cc
 
 unity-ratings-db.o : $(srcdir)/unity-ratings-db.c $(srcdir)/unity-ratings-db.h
 	$(AM_V_CC)$(CC) -g $(COVERAGE_CFLAGS) $(unity_ratings_db_libs) `pkg-config --cflags --libs glib-2.0` -c $(srcdir)/unity-ratings-db.c

=== modified file 'src/unity-package-search.cc'
--- src/unity-package-search.cc	2012-11-05 19:18:41 +0000
+++ src/unity-package-search.cc	2013-03-01 15:43:20 +0000
@@ -25,14 +25,15 @@
  * Xapian index of the menu contents.
  */
 
-using namespace std;
-
 #include <xapian.h>
 #include <iostream>
 #include <gmenu-tree.h>
 #include <glib.h>
 #include <unity.h>
 #include <string.h>
+#include <vector>
+
+using namespace std;
 
 #define SOFTWARE_CENTER_INDEX "/var/cache/software-center/xapian"
 #define QUERY_PARSER_EXACTSEARCH_FLAGS Xapian::QueryParser::FLAG_BOOLEAN|Xapian::QueryParser::FLAG_PHRASE|Xapian::QueryParser::FLAG_LOVEHATE
@@ -55,14 +56,11 @@
 #define XAPIAN_VALUE_SCREENSHOT_URLS 185
 #define XAPIAN_VALUE_DESCRIPTION 188
 #define XAPIAN_VALUE_VERSION_INFO 198
+#define XAPIAN_VALUE_EXENAME 294
 #define XAPIAN_VALUE_CURRENCY 201
 
 #include "unity-package-search.h"
-
-extern "C"
-{
-  extern gchar* unity_applications_lens_utils_preprocess_string (const gchar* input);
-}
+#include "columbus.hh"
 
 struct _UnityPackageSearcher
 {
@@ -71,8 +69,58 @@
   Xapian::Enquire     *enquire;
   Xapian::QueryParser *query_parser;
   GRand               *random;
+  Columbus::Matcher   *matcher;
+  vector<string>      col_mapping;
+  bool                db_merged;
 };
 
+static void buildMatcher(UnityPackageSearcher *searcher) {
+    Columbus::Matcher *m  = searcher->matcher;
+    Xapian::Database *db = searcher->db;
+    Columbus::Corpus c;
+    Columbus::Word appnameField("appname");
+    Columbus::Word summaryField("summary");
+    Columbus::Word pkgnameField("pkgname");
+    Columbus::Word exenameField("exename");
+
+    for(Xapian::PostingIterator post = db->postlist_begin(""); post != db->postlist_end(""); post++) {
+        Xapian::Document xdoc = db->get_document(*post);
+        DocumentID id;
+        if(searcher->db_merged) {
+            searcher->col_mapping.push_back(xdoc.get_value(XAPIAN_VALUE_APPNAME));
+            id = searcher->col_mapping.size()-1;
+        } else {
+            id = xdoc.get_docid();
+        }
+        Columbus::Document cdoc(id);
+        std::string val;
+
+        val = xdoc.get_value(XAPIAN_VALUE_APPNAME);
+        if(!val.empty())
+            cdoc.addText(appnameField, val.c_str());
+        val = xdoc.get_value(XAPIAN_VALUE_SUMMARY);
+        if(!val.empty())
+            cdoc.addText(summaryField, val.c_str());
+        val = xdoc.get_value(XAPIAN_VALUE_PKGNAME);
+        if(!val.empty())
+            cdoc.addText(pkgnameField, val.c_str());
+        val = xdoc.get_value(XAPIAN_VALUE_EXENAME);
+        if(!val.empty())
+            cdoc.addText(exenameField, val.c_str());
+        c.addDocument(cdoc);
+    }
+    m->index(c);
+    m->getErrorValues().addStandardErrors();
+    m->getErrorValues().setSubstringMode();
+    m->getIndexWeights().setWeight(summaryField, 0.5);
+}
+
+
+extern "C"
+{
+  extern gchar* unity_applications_lens_utils_preprocess_string (const gchar* input);
+}
+
 /* A Xapian::Sorter that respects the collation rules for the current locale */
 class LocaleKeyMaker : public Xapian::KeyMaker
 {
@@ -264,6 +312,7 @@
         dum2 = strstr (dum1, " "); // const
         dum2 == NULL ? : *dum2 = '\0'; // const
         dum2 = g_path_get_basename (dum1); // alloc
+        doc.add_value(XAPIAN_VALUE_EXENAME, dum2);
         indexer->index_text (dum2, 2);
         g_free (dum1);
         dum1 = g_strdelimit (dum2, "-", '_'); // in place switch, dum1 own mem
@@ -299,7 +348,7 @@
   UnityPackageSearcher *searcher;
   Xapian::WritableDatabase *db;
 
-  searcher = g_new0 (UnityPackageSearcher, 1);
+  searcher = new UnityPackageSearcher;
   db = new Xapian::WritableDatabase ();
   searcher->db = db;
   searcher->db->add_database (Xapian::InMemory::open ());
@@ -307,11 +356,15 @@
   init_searcher (searcher);
 
   /* Index the menu recursively */
+  searcher->db_merged = false;
   Xapian::TermGenerator *indexer = new Xapian::TermGenerator ();
   index_menu_item (db, indexer, gmenu_tree_get_root_directory (menu));
   delete indexer;
   db->flush ();
 
+  searcher->matcher = new Columbus::Matcher();
+  buildMatcher(searcher);
+
   return searcher;
 }
 
@@ -323,7 +376,7 @@
   UnityPackageSearcher *searcher;
   gchar *agent = NULL;
 
-  searcher = g_new0 (UnityPackageSearcher, 1);
+  searcher = new UnityPackageSearcher;
 
   // Xapian initialization
   try
@@ -351,6 +404,9 @@
   }
 
   init_searcher (searcher);
+  searcher->db_merged = true;
+  searcher->matcher = new Columbus::Matcher();
+  buildMatcher(searcher);
 
   return searcher;
 }
@@ -364,8 +420,9 @@
   delete searcher->sorter;
   delete searcher->enquire;
   delete searcher->query_parser;
+  delete searcher->matcher;
   g_rand_free (searcher->random);
-  g_free (searcher);
+  delete searcher;
 }
 
 static UnityPackageInfo*
@@ -425,20 +482,37 @@
   g_slice_free (UnityPackageInfo, pkginfo);
 }
 
-UnityPackageSearchResult*
-unity_package_searcher_search (UnityPackageSearcher  *searcher,
-                               const gchar           *search_string,
-                               guint                  max_hits,
-                               UnityPackageSearchType search_type,
-                               UnityPackageSort       sort)
+Xapian::Document get_doc_from_col_match(UnityPackageSearcher *searcher, DocumentID id) {
+    if(searcher->db_merged) {
+        string name = searcher->col_mapping[id];
+        string query = "AA\"";
+        query += name;
+        query += "\"";
+        Xapian::QueryParser p;
+        Xapian::Query q;
+        Xapian::Enquire e(*searcher->db);
+        Xapian::MSet matches;
+        p.set_database(*searcher->db);
+        q = p.parse_query(query);
+        e.set_query(q);
+        matches = e.get_mset(0, 1);
+        return matches.begin().get_document();
+    } else {
+        return searcher->db->get_document(id);
+    }
+}
+
+static UnityPackageSearchResult*
+xapian_search (UnityPackageSearcher  *searcher,
+               const gchar           *search_string,
+               guint                  max_hits,
+               UnityPackageSearchType search_type,
+               UnityPackageSort       sort)
 {
   UnityPackageSearchResult* result;
-
-  g_return_val_if_fail (searcher != NULL, NULL);
-  g_return_val_if_fail (search_string != NULL, NULL);
-
   string _search_string (search_string);
   Xapian::Query query;
+
   try
   {
     switch (search_type)
@@ -455,12 +529,11 @@
         break;
     }
   }
-  catch (Xapian::Error e)
+  catch (Xapian::Error &e)
   {
     g_warning ("Error parsing query '%s': %s", search_string, e.get_msg().c_str());
     return g_slice_new0 (UnityPackageSearchResult);
   }
-  //cout << "Performing query `" << query.get_description() << "'" << endl;
 
   switch (sort)
   {
@@ -488,24 +561,26 @@
     // Retrieve the results, note that we build the result->results
     // list in reverse order and then reverse it before we return it
     result->num_hits = matches.get_matches_estimated ();
+
     for (Xapian::MSetIterator i = matches.begin(); i != matches.end(); ++i)
-    {
-      try
-      {
-        Xapian::Document doc = i.get_document();
-        UnityPackageInfo *pkginfo = _pkginfo_from_document (doc);
-        pkginfo->relevancy = i.get_percent ();
-        result->results = g_slist_prepend (result->results, pkginfo);
-      }
-      catch (Xapian::Error e)
-      {
-        g_warning ("Unable to read document from result set: %s", e.get_msg().c_str());
-      }
-    }
+      {
+        try
+          {
+            Xapian::Document doc = i.get_document();
+            UnityPackageInfo *pkginfo = _pkginfo_from_document (doc);
+            pkginfo->relevancy = i.get_percent();
+            result->results = g_slist_prepend (result->results, pkginfo);
+          }
+        catch (Xapian::Error &e)
+          {
+            g_warning ("Unable to read document from result set: %s",
+                       e.get_msg().c_str());
+          }
+      }
 
     result->results = g_slist_reverse (result->results);
   }
-  catch(const Xapian::Error e)
+  catch(const Xapian::Error &e)
   {
     g_warning ("Error running query '%s': %s", search_string, e.get_msg().c_str());
   }
@@ -513,6 +588,55 @@
   return result;
 }
 
+static UnityPackageSearchResult*
+libcolumbus_search(UnityPackageSearcher *searcher, const char *str) {
+    Columbus::MatchResults results;
+    UnityPackageSearchResult* result;
+
+    result = g_slice_new0 (UnityPackageSearchResult);
+    searcher->matcher->match(str, results);
+    for (size_t i = 0; i < results.size(); ++i)
+      {
+        try
+          {
+            Xapian::Document doc = get_doc_from_col_match(searcher, results.getDocumentID(i));
+            UnityPackageInfo *pkginfo = _pkginfo_from_document (doc);
+            pkginfo->relevancy = results.getRelevancy(i);
+            result->results = g_slist_prepend (result->results, pkginfo);
+          }
+        catch (Xapian::Error &e)
+          {
+            g_warning ("Unable to read document from result set: %s",
+                       e.get_msg().c_str());
+          }
+      }
+
+    result->results = g_slist_reverse (result->results);
+    return result;
+}
+
+UnityPackageSearchResult*
+unity_package_searcher_search (UnityPackageSearcher  *searcher,
+                               const gchar           *search_string,
+                               guint                  max_hits,
+                               UnityPackageSearchType search_type,
+                               UnityPackageSort       sort)
+{
+
+  g_return_val_if_fail (searcher != NULL, NULL);
+  g_return_val_if_fail (search_string != NULL, NULL);
+
+  bool has_category = strstr(search_string, "category:") != NULL;
+  const char *col_query_str = strstr(search_string, "AND");
+  if(has_category || !col_query_str) {
+      return xapian_search(searcher, search_string, max_hits, search_type, sort);
+  } else {
+      col_query_str += 3;
+      return libcolumbus_search(searcher, col_query_str);
+  }
+}
+
+
 /**
  * Get applications matching given xapian filter query and additionally filter results out
  * using AppFilterCallback, until n_apps matching apps are found.
@@ -618,7 +742,7 @@
           n_unique++;
         }
       }
-      catch (Xapian::Error e)
+      catch (Xapian::Error &e)
       {
         g_warning ("Error getting random apps: %s", e.get_msg().c_str());
         continue;
@@ -653,7 +777,7 @@
         }
       }
     }
-    catch (Xapian::Error e)
+    catch (Xapian::Error &e)
     {
       g_debug ("Error getting random apps for query '%s': %s", filter_query, e.get_msg().c_str());
       return g_slice_new0 (UnityPackageSearchResult);

=== added file 'tests/manual/fuzzy-search-test.txt'
--- tests/manual/fuzzy-search-test.txt	1970-01-01 00:00:00 +0000
+++ tests/manual/fuzzy-search-test.txt	2013-03-01 15:43:20 +0000
@@ -0,0 +1,42 @@
+Fuzzy searching in the Dash
+---------------------------
+
+Tests that fuzzy matching works in the Dash.
+
+Actions:
+
+Tap super.
+Enter an application name that is slightly misspelled. Check that the Dash.
+returns valid replies.
+
+Examples:
+
+  fimp -> Gimp
+  fierfox -> Firefox
+  sttings -> settings
+
+Fuzzy searching in the app lens
+-------------------------------
+
+Tests that fuzzy matching works in the app lens.
+
+Actions:
+
+Tap super.
+Select app lens.
+Do the same searches as in the previous test. Check that they return the
+same results.
+
+
+Searching with filters
+----------------------
+
+When filters are enabled, the search falls back to non-fuzzy mode.
+
+Actions:
+
+Tap super.
+Select app lens.
+Enable filters, activate "graphics".
+Type "fimp" in search field, Gimp should not be show in the results.
+Type "gimp" in search field, Gimp should be shown in the results.

=== modified file 'tests/unit/Makefile.am'
--- tests/unit/Makefile.am	2013-02-20 16:09:23 +0000
+++ tests/unit/Makefile.am	2013-03-01 15:43:20 +0000
@@ -106,7 +106,7 @@
 nodist_test_software_center_details_SOURCES = $(test_software_center_details_VALASOURCES:.vala=.c)
 
 unity-package-search.o : $(top_srcdir)/src/unity-package-search.cc $(top_srcdir)/src/unity-package-search.h
-	$(AM_V_GEN)$(CXX) -g $(COVERAGE_CFLAGS) $(UNITY_PACKAGE_SEARCH_LIBS) -DGMENU_I_KNOW_THIS_IS_UNSTABLE `pkg-config --cflags --libs glib-2.0 libgnome-menu-3.0 unity gee-1.0` -c $(top_srcdir)/src/unity-package-search.cc
+	$(AM_V_GEN)$(CXX) --std=c++11 -g $(COVERAGE_CFLAGS) $(UNITY_PACKAGE_SEARCH_LIBS) -DGMENU_I_KNOW_THIS_IS_UNSTABLE `pkg-config --cflags --libs glib-2.0 libgnome-menu-3.0 unity gee-1.0 libcolumbus0` -c $(top_srcdir)/src/unity-package-search.cc
 unity-ratings-db.o : $(top_srcdir)/src/unity-ratings-db.c $(top_srcdir)/src/unity-ratings-db.h
 	$(AM_V_CC)$(CC) -g $(COVERAGE_CFLAGS) $(unity_ratings_db_libs) `pkg-config --cflags --libs glib-2.0` -c $(top_srcdir)/src/unity-ratings-db.c