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

[Michal Hruby]

Make sure we don't prefix-compare collation keys when searching in the index

Makes sure we don't prefix-compare collation keys when searching in the index, instead the original terms have to be prefix-compared.

Tests are now using non-C locale to make sure this doesn't regress (is that a good idea though?)

[Mikkel Kamstrup Erlandsen (kamstrup)]

Great. Nice detective work there! :-)

I don't particularly mind setting the locale in the tests. If it ever becomes a problem we can set it to some hard coded value or something.

review: Approve

=== modified file 'src/dee-tree-index.c'
--- src/dee-tree-index.c	2012-02-28 09:59:38 +0000
+++ src/dee-tree-index.c	2012-03-16 11:57:22 +0000
@@ -119,10 +119,12 @@
 
 static GSequenceIter* find_term       (GSequence   *terms,
                                        const gchar *term,
+                                       const gchar *col_key,
                                        DeeAnalyzer *analyzer);
 
 static GSequenceIter* find_term_real (GSequence        *terms,
                                       const gchar      *term,
+                                      const gchar      *col_key,
                                       DeeAnalyzer      *analyzer,
                                       DeeTermMatchFlag  flags);
 /*
@@ -214,14 +216,16 @@
 /* Search priv->terms for a string from priv->term_list.
  * ! Doesn't work for strings not in priv->term_list ! */
 static GSequenceIter*
-find_term (GSequence *terms, const gchar *term, DeeAnalyzer *analyzer)
+find_term (GSequence *terms, const gchar *term, const gchar *col_key,
+           DeeAnalyzer *analyzer)
 {
-  return find_term_real (terms, term, analyzer, DEE_TERM_MATCH_EXACT);
+  return find_term_real (terms, term, col_key, analyzer, DEE_TERM_MATCH_EXACT);
 }
 
 static GSequenceIter*
 find_term_real (GSequence        *terms,
                 const gchar      *term,
+                const gchar      *col_key,
                 DeeAnalyzer      *analyzer,
                 DeeTermMatchFlag  flags)
 {
@@ -235,7 +239,7 @@
   if (begin == end)
     return NULL;
 
-  search_term.col_key = term;
+  search_term.col_key = col_key;
 
   if (flags & DEE_TERM_MATCH_EXACT)
     {
@@ -259,14 +263,14 @@
         {
           previous = g_sequence_iter_prev (previous);
           term_result = g_sequence_get (previous);
-          if (g_str_has_prefix (term_result->col_key, term)) iter = previous;
+          if (g_str_has_prefix (term_result->term, 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))
+          if (g_str_has_prefix (term_result->term, term))
             return iter;
         }
       else
@@ -421,14 +425,16 @@
   priv = DEE_TREE_INDEX (self)->priv;
   analyzer = dee_index_get_analyzer (self);
   col_key = dee_analyzer_collate_key (analyzer, term);
-  term_iter = find_term_real (priv->terms, col_key, analyzer, flags);
+  term_iter = find_term_real (priv->terms, term, col_key, analyzer, flags);
   g_free (col_key);
 
   if (term_iter == NULL ||
       term_iter == g_sequence_get_end_iter (priv->terms))
-    return dee_glist_result_set_new (NULL, /* The empty GList */
-                                      dee_index_get_model (self),
-                                      NULL);
+    {
+      return dee_glist_result_set_new (NULL, /* The empty GList */
+                                       dee_index_get_model (self),
+                                       NULL);
+    }
 
   if (flags & DEE_TERM_MATCH_EXACT)
     {
@@ -446,7 +452,8 @@
       end = g_sequence_get_end_iter (priv->terms);
       term_data = g_sequence_get (term_iter);
 
-      while (g_str_has_prefix (term_data->col_key, term))
+      /* We can't use collation keys for prefix matching */
+      while (g_str_has_prefix (term_data->term, term))
         {
           GList *rows = term_rows (term_data);
           iter = rows;
@@ -503,9 +510,11 @@
 {
   DeeTreeIndexPrivate *priv;
   DeeModel            *model;
+  DeeAnalyzer         *analyzer;
   DeeResultSet        *results;
   GSequenceIter       *iter, *end;
   Term                *term_data;
+  gchar               *col_key;
 
   g_return_if_fail (DEE_IS_TREE_INDEX (self));
   g_return_if_fail (func != NULL);
@@ -517,8 +526,10 @@
     iter = g_sequence_get_begin_iter (priv->terms);
   else
     {
-      iter = find_term (priv->terms, start_term, // FIXME: col key?
-                        dee_index_get_analyzer (self));
+      analyzer = dee_index_get_analyzer (self);
+      col_key = dee_analyzer_collate_key (analyzer, start_term);
+      iter = find_term (priv->terms, start_term, col_key, analyzer);
+      g_free (col_key);
       if (iter == NULL ||
           iter == g_sequence_get_end_iter (priv->terms))
         return;
@@ -568,13 +579,16 @@
   Term                *term_data;
   GSequenceIter       *term_iter;
   DeeAnalyzer         *analyzer;
+  gchar               *col_key;
 
   g_return_val_if_fail (DEE_IS_TREE_INDEX (self), 0);
   g_return_val_if_fail (term != NULL, 0);
 
   priv = DEE_TREE_INDEX (self)->priv;
   analyzer = dee_index_get_analyzer (self);
-  term_iter = find_term (priv->terms, term, analyzer); // FIXME: col key?
+  col_key = dee_analyzer_collate_key (analyzer, term);
+  term_iter = find_term (priv->terms, term, col_key, analyzer);
+  g_free (col_key);
 
   if (term_iter == NULL ||
       term_iter == g_sequence_get_end_iter (priv->terms))
@@ -641,7 +655,7 @@
       term = dee_term_list_get_term (priv->term_list, i);
 
       /* Update priv->terms */
-      term_iter = find_term (priv->terms, colkey, analyzer);
+      term_iter = find_term (priv->terms, term, colkey, analyzer);
 
       if (term_iter == NULL ||
           term_iter == g_sequence_get_end_iter (priv->terms))
@@ -698,7 +712,8 @@
       if (term_n_rows (term_data) == 0)
         {
           /* Removing the term from the sequence also frees it */
-          term_iter = find_term (priv->terms, term_data->col_key, analyzer);
+          term_iter = find_term (priv->terms, term_data->term,
+                                 term_data->col_key, analyzer);
           g_sequence_remove (term_iter);
         }
     }

=== modified file 'tests/test-dee.c'
--- tests/test-dee.c	2012-01-25 12:51:48 +0000
+++ tests/test-dee.c	2012-03-16 11:57:22 +0000
@@ -20,6 +20,7 @@
 #include "config.h"
 #include <glib.h>
 #include <glib-object.h>
+#include <locale.h>
 
 void test_model_column_create_suite (void);
 void test_model_complex_column_create_suite (void);
@@ -54,6 +55,7 @@
   g_thread_init (NULL);
 
   g_test_init (&argc, &argv, NULL);
+  setlocale (LC_ALL, "");
 
   test_model_column_create_suite ();
   test_model_complex_column_create_suite ();