Merge lp:~stolowski/unity-scope-home/canned-queries-session-id into lp:unity-scope-home

[Paweł Stołowski]

Reuse existing session_id for canned queries.

Added new CANNED_QUERY state to SearchQueryChange / SearchQueryState, so that we're able to reuse existing session_id for canned queries.
When a scopes-query:// item is activated, the activation handler sets a flag indicating that upcoming search query for current channel will be a canned query. Search request handler will then receive state == CANNED_QUERY when it calls query_state.search_query_changed.

[lp:~stolowski/unity-scope-home/canned-queries-session-id]

166. By Paweł Stołowski on 2013-09-19

Remove canned_query flag on remove().

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:166
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/86/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/50
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/50
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/50

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/86/rebuild

review: Approve (continuous-integration)

[Michal Hruby (mhr3)]

What if the scope gets a query "foo", then a canned query is activated, but meanwhile the user changes something and instead of "keyword:foo" the scope gets "bar" as next query?

review: Needs Information

[lp:~stolowski/unity-scope-home/canned-queries-session-id]

167. By Paweł Stołowski on 2013-09-19

Keep canned query string in the query state object rather than just bool, and only consider a query to be canned query if the search string matches current search request.

[Paweł Stołowski (stolowski)]

> What if the scope gets a query "foo", then a canned query is activated, but
> meanwhile the user changes something and instead of "keyword:foo" the scope
> gets "bar" as next query?

Good point, though rather unlikely. Anyway, update the code to deal with this.

[Michal Hruby (mhr3)]

Looks good now :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:167
http://jenkins.qa.ubuntu.com/job/unity-scope-home-ci/89/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-amd64-ci/53
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-armhf-ci/53
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-home-saucy-i386-ci/53/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-scope-home-ci/89/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/scope.vala'
--- src/scope.vala	2013-09-04 10:28:06 +0000
+++ src/scope.vala	2013-09-19 13:59:12 +0000
@@ -539,7 +539,10 @@
         warning ("Invalid UTF8 received from the server");
       }
 
-      return new ActivationResponse.with_search (escaped_query != null ? escaped_query : query, null, null);
+      if (escaped_query != null)
+          query = escaped_query;
+      query_state.set_canned_query (activation.channel_id, query);
+      return new ActivationResponse.with_search (query, null, null);
     }
 
     if (!scope_mgr.remote_content_search || sss_client == null ||

=== modified file 'src/search-query-state.vala'
--- src/search-query-state.vala	2013-03-21 21:40:00 +0000
+++ src/search-query-state.vala	2013-09-19 13:59:12 +0000
@@ -23,6 +23,7 @@
   {
     NOT_CHANGED,
     NEW_QUERY,
+    CANNED_QUERY,
     APPENDS_TO_PREVIOUS_QUERY,
     REMOVES_FROM_PREVIOUS_QUERY
   }
@@ -30,10 +31,12 @@
   public class SearchQueryState
   {
     private HashTable<string, string> previous_query = new HashTable<string, string> (str_hash, str_equal);
+    private HashTable<string, string> canned_query = new HashTable<string, string> (str_hash, str_equal);
     
     public void remove (string home_channel_id)
     {
       previous_query.remove (home_channel_id);
+      canned_query.remove (home_channel_id);
     }
 
     public bool has_channel (string home_channel_id)
@@ -41,34 +44,47 @@
       return previous_query.contains (home_channel_id);
     }
 
+    public void set_canned_query (string channel_id, string search_string)
+    {
+      canned_query[channel_id] = search_string;
+    }
+
     /**
      * Compare search string with previous string for given home channel. Store new search string.
      */
     public SearchQueryChange search_query_changed (string channel_id, string search_string)
     {
       var query = search_string.strip ();
-
       var changed = SearchQueryChange.NEW_QUERY;
-      if (previous_query.contains (channel_id))
-      {
-        var prev = previous_query[channel_id];
-        if (query == "" && prev != "") // there was a query previously, but user removed all characters in new one
-        {
-          changed = SearchQueryChange.NEW_QUERY;
-        }
-        else
-        {
-          if (prev == query)
-            changed = SearchQueryChange.NOT_CHANGED;
-          else if (prev == "")
+
+      if (canned_query.contains(channel_id) && canned_query[channel_id] == query)
+      {
+        changed = SearchQueryChange.CANNED_QUERY;
+      }
+      else
+      {
+        if (previous_query.contains (channel_id))
+        {
+          var prev = previous_query[channel_id];
+          if (query == "" && prev != "") // there was a query previously, but user removed all characters in new one
+          {
             changed = SearchQueryChange.NEW_QUERY;
-          else if (query.has_prefix (prev))
-            changed = SearchQueryChange.APPENDS_TO_PREVIOUS_QUERY;
-          else if (prev.has_prefix (query))
-            changed = SearchQueryChange.REMOVES_FROM_PREVIOUS_QUERY;
+          }
+          else
+          {
+            if (prev == query)
+              changed = SearchQueryChange.NOT_CHANGED;
+            else if (prev == "")
+              changed = SearchQueryChange.NEW_QUERY;
+            else if (query.has_prefix (prev))
+              changed = SearchQueryChange.APPENDS_TO_PREVIOUS_QUERY;
+            else if (prev.has_prefix (query))
+              changed = SearchQueryChange.REMOVES_FROM_PREVIOUS_QUERY;
+          }
         }
       }
-
+      
+      canned_query.remove (channel_id);
       previous_query[channel_id] = query;
 
       debug ("search_query_changed for channel %s: %s", channel_id, changed.to_string ());

=== modified file 'tests/unit/test-home-scope.vala'
--- tests/unit/test-home-scope.vala	2013-09-04 10:28:06 +0000
+++ tests/unit/test-home-scope.vala	2013-09-19 13:59:12 +0000
@@ -230,6 +230,18 @@
       assert (search_state.search_query_changed ("channel1", "te") == SearchQueryChange.REMOVES_FROM_PREVIOUS_QUERY);
       assert (search_state.search_query_changed ("channel1", "") == SearchQueryChange.NEW_QUERY);
       assert (search_state.has_channel ("channel1") == true);
+
+      // simulate canned queries
+      assert (search_state.search_query_changed ("channel1", "ab") == SearchQueryChange.NEW_QUERY);
+      search_state.set_canned_query("channel1", "foo:lo");
+      assert (search_state.search_query_changed ("channel1", "foo:lo") == SearchQueryChange.CANNED_QUERY);
+      assert (search_state.search_query_changed ("channel1", "foo:lo") == SearchQueryChange.NOT_CHANGED);
+
+      // simulate canned queries, user types a query immediately after activation
+      assert (search_state.search_query_changed ("channel1", "me") == SearchQueryChange.NEW_QUERY);
+      search_state.set_canned_query("channel1", "foo:metallica");
+      assert (search_state.search_query_changed ("channel1", "iron maiden") == SearchQueryChange.NEW_QUERY);
+      assert (search_state.search_query_changed ("channel1", "foo:metallica") == SearchQueryChange.NEW_QUERY); //it's no longer considered canned query
     }
     
     internal void test_category_manager_binary_search ()