Merge lp:~mhr3/dee/fix-prefix-search into lp:dee

[Michal Hruby]

Fixes a bug in prefix searching.

Added tests for the issue.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Tests pass and code looks swell

review: Approve

=== modified file 'src/dee-tree-index.c'
--- src/dee-tree-index.c	2011-11-29 12:17:25 +0000
+++ src/dee-tree-index.c	2012-02-15 22:09:17 +0000
@@ -226,7 +226,7 @@
                 DeeTermMatchFlag  flags)
 {
   Term           search_term, *term_result;
-  GSequenceIter *iter, *begin, *end;
+  GSequenceIter *found_iter, *iter, *previous, *begin, *end;
 
   begin = g_sequence_get_begin_iter (terms);
   end = g_sequence_get_end_iter (terms);
@@ -239,24 +239,38 @@
 
   if (flags & DEE_TERM_MATCH_EXACT)
     {
+      // FIXME: do we need to make sure this is the first iter?
       return g_sequence_lookup (terms, &search_term,
                                 (GCompareDataFunc) term_cmp, analyzer);
     }
   else if (flags & DEE_TERM_MATCH_PREFIX)
     {
-      iter = g_sequence_search (terms, &search_term,
-                                (GCompareDataFunc) term_cmp, analyzer);
-      term_result = g_sequence_get (iter);
-      if (g_str_has_prefix (term_result->col_key, term))
-        return iter;
-      else if (iter != begin)
-        {
-          /* We might be placed immediately after the iter we want */
-          iter = g_sequence_iter_prev (iter);
+      found_iter = g_sequence_search (terms, &search_term,
+                                      (GCompareDataFunc) term_cmp, analyzer);
+
+      /* What can happen now is:
+       * 1) found_iter has our prefix
+       * 2) found_iter as well as the found_iter->previous have our prefix
+       * 3) found_iter doesn't have it, but found_iter->previous does
+       * 4) there isn't any nearby iter that has the prefix */
+      previous = iter = found_iter;
+      /* We might be placed after the iter we want */
+      while (previous != begin)
+        {
+          previous = g_sequence_iter_prev (previous);
+          term_result = g_sequence_get (previous);
+          if (g_str_has_prefix (term_result->col_key, term)) iter = previous;
+          else break;
+        }
+      if (iter == found_iter)
+        {
+          /* We never checked this one */
           term_result = g_sequence_get (iter);
           if (g_str_has_prefix (term_result->col_key, term))
             return iter;
         }
+      else
+        return iter;
     }
   else
     {

=== modified file 'tests/test-index.c'
--- tests/test-index.c	2011-12-14 10:49:16 +0000
+++ tests/test-index.c	2012-02-15 22:09:17 +0000
@@ -369,6 +369,47 @@
 }
 
 static void
+test_prefix_2_rows (Fixture *fix, gconstpointer data)
+{
+  DeeModelIter *iter, *i0, *i1;
+  DeeResultSet *results;
+
+  g_assert (dee_index_get_supported_term_match_flags (fix->index) &
+            DEE_TERM_MATCH_PREFIX);
+
+  i0 = dee_model_append (fix->model, "Caravan scrap yard", 0);
+  i1 = dee_model_append (fix->model, "Scraper foo", 1);
+
+  /* NOTE: We can NOT infer anything about the ordering of the matching
+   *       iters inside a single term */
+  results = dee_index_lookup (fix->index, "scrap", DEE_TERM_MATCH_PREFIX);
+  g_assert_cmpint (dee_result_set_get_n_rows (results), ==, 2);
+  iter = dee_result_set_next (results);
+  g_assert (iter == i0 || iter == i1);
+  iter = dee_result_set_next (results);
+  g_assert (iter == i0 || iter == i1);
+  g_object_unref (results);
+
+  results = dee_index_lookup (fix->index, "scra", DEE_TERM_MATCH_PREFIX);
+  g_assert_cmpint (dee_result_set_get_n_rows (results), ==, 2);
+  iter = dee_result_set_next (results);
+  g_assert (iter == i0 || iter == i1);
+  iter = dee_result_set_next (results);
+  g_assert (iter == i0 || iter == i1);
+  g_object_unref (results);
+
+  results = dee_index_lookup (fix->index, "scraper", DEE_TERM_MATCH_PREFIX);
+  g_assert_cmpint (dee_result_set_get_n_rows (results), ==, 1);
+  iter = dee_result_set_next (results);
+  g_assert (iter == i1);
+  g_object_unref (results);
+
+  results = dee_index_lookup (fix->index, "scraperer", DEE_TERM_MATCH_PREFIX);
+  g_assert_cmpint (dee_result_set_get_n_rows (results), ==, 0);
+  g_object_unref (results);
+}
+
+static void
 test_prefix_3_rows (Fixture *fix, gconstpointer data)
 {
   DeeModelIter *iter, *i0, *i1;
@@ -393,6 +434,7 @@
 
   results = dee_index_lookup (fix->index, "hel", DEE_TERM_MATCH_PREFIX);
   g_assert_cmpint (dee_result_set_get_n_rows (results), ==, 2);
+  iter = dee_result_set_next (results);
   g_assert (iter == i0 || iter == i1);
   iter = dee_result_set_next (results);
   g_assert (iter == i0 || iter == i1);
@@ -470,6 +512,8 @@
               setup_text_tree, test_text_with_dupe_terms_per_row, teardown);
   g_test_add ("/Index/Tree/Prefix1Row", Fixture, 0,
               setup_text_tree, test_prefix_1_row, teardown);
+  g_test_add ("/Index/Tree/Prefix2Rows", Fixture, 0,
+              setup_text_tree, test_prefix_2_rows, teardown);
   g_test_add ("/Index/Tree/Prefix3Rows", Fixture, 0,
               setup_text_tree, test_prefix_3_rows, teardown);
   g_test_add ("/Index/Tree/Prefix4RowsAndClear", Fixture, 0,