Merge lp:~aacid/geonames/more_liberal_city_search into lp:geonames

[Albert Astals Cid]

Be less strict on city search

Include matches that ignore words at the beggining (i.e. searching for Hague will return "The Hague" as result)

Was wondering if we wanted a new flag to GeonamesQueryFlags or making it by default less strict is ok.

[Sebastien Bacher (seb128)]

Thanks but as discussed on IRC that contredicts what is describe on bug #1454186 / on the design wiki, the matching should be done on full word or start of words not in subtrings in the middle of a word

review: Needs Fixing

[lp:~aacid/geonames/more_liberal_city_search]

27. By Albert Astals Cid on 2017-02-01

Be less strict on city search

Include matches that ignore words at the beggining (i.e. searching for Hague will return "The Hague" as result)

[Albert Astals Cid (aacid)]

> Thanks but as discussed on IRC that contredicts what is describe on bug
> #1454186 / on the design wiki, the matching should be done on full word or
> start of words not in subtrings in the middle of a word

Ok, changed to only "ignore" words at the beginning.

[Sebastien Bacher (seb128)]

Thanks, looks fine and works as it should

review: Approve

=== modified file 'src/geonames-query.c'
--- src/geonames-query.c	2016-04-20 15:49:48 +0000
+++ src/geonames-query.c	2017-02-01 15:41:11 +0000
@@ -82,22 +82,33 @@
 
 static gdouble
 match_query (gchar       **query_tokens,
-             const gchar  *potential_hit)
+             const gchar  *potential_hit,
+             gboolean     *all_prefix_match)
 {
   g_auto(GStrv) tokens = NULL;
   gint i;
   gdouble weight = 0.0;
 
+  *all_prefix_match = TRUE;
+
   tokens = g_str_tokenize_and_fold (potential_hit, NULL, NULL);
   for (i = 0; query_tokens[i]; i++)
     {
-      if (tokens[i] == 0)
-        return 0.0;
-
-      if (!str_prefix_matches (tokens[i], query_tokens[i]))
-        return 0.0;
-
-      weight += (gdouble) strlen (query_tokens[i]) / strlen (tokens[i]);
+      if (tokens[i] == 0) {
+        *all_prefix_match = FALSE;
+        return 0.0;
+      }
+
+      if (str_prefix_matches (tokens[i], query_tokens[i]))
+        {
+          weight += (gdouble) strlen (query_tokens[i]) / strlen (tokens[i]);
+        }
+      else
+        {
+          const gdouble aux = match_query(query_tokens, &potential_hit[strlen (tokens[i])], all_prefix_match);
+          *all_prefix_match = FALSE;
+          return aux;
+        }
     }
 
   return weight / i;
@@ -120,9 +131,12 @@
                   gdouble best_weight)
 {
   gdouble weight;
+  gboolean all_prefix_match;
 
-  weight = match_query (query_tokens, name);
+  weight = match_query (query_tokens, name, &all_prefix_match);
   weight *= (gdouble) CLAMP (population, 1, 1000000) / 1000000;
+  if (all_prefix_match)
+    weight += 1;
 
   return MAX (weight, best_weight);
 }

=== modified file 'tests/test-geonames.c'
--- tests/test-geonames.c	2016-04-04 18:21:31 +0000
+++ tests/test-geonames.c	2017-02-01 15:41:11 +0000
@@ -30,6 +30,39 @@
 }
 
 static void
+assert_contains (const gchar *query,
+                 const gchar *expected_city,
+                 const gchar *expected_country_code,
+                 gdouble expected_latitude,
+                 gdouble expected_longitude,
+                 guint expected_population)
+{
+  g_autofree gint *indices;
+  guint i, len;
+  g_autoptr(GeonamesCity) city = NULL;
+
+  indices = geonames_query_cities_sync (query, GEONAMES_QUERY_DEFAULT, &len, NULL, NULL);
+  g_assert (indices);
+  g_assert_cmpint (indices[len], ==, -1);
+
+  g_assert_cmpint (len, >, 0);
+
+  for (i = 0; i < len; ++i) {
+    city = geonames_get_city (indices[i]);
+
+    if (g_strcmp0(geonames_city_get_name (city), expected_city) == 0) {
+      g_assert_cmpstr (geonames_city_get_country_code (city), ==, expected_country_code);
+      g_assert_cmpfloat (geonames_city_get_latitude (city), ==, expected_latitude);
+      g_assert_cmpfloat (geonames_city_get_longitude (city), ==, expected_longitude);
+      g_assert_cmpint (geonames_city_get_population (city), ==, expected_population);
+      break;
+    }
+  }
+  g_assert (i < len);
+}
+
+
+static void
 assert_first_stats (const gchar *query,
                     const gchar *expected_city,
                     const gchar *expected_country_code,
@@ -115,6 +148,13 @@
 }
 
 static void
+test_cities_without_some_words (void)
+{
+    assert_contains ("hospitalet", "L'Hospitalet de Llobregat", "ES", 41.35967, 2.10028, 257038);
+    assert_contains ("hague", "The Hague", "NL", 52.07667, 4.29861, 474292);
+}
+
+static void
 test_edge_cases (void)
 {
   gint *indices;
@@ -142,6 +182,7 @@
   g_test_add_func ("/common-cities", test_common_cities);
   g_test_add_func ("/translations", test_translations);
   g_test_add_func ("/edge-cases", test_edge_cases);
+  g_test_add_func ("/cities-without-some-words", test_cities_without_some_words);
 
   return g_test_run ();
 }