Merge lp:~mhr3/libunity/scope-result-in-python into lp:libunity

[Michal Hruby]

Implement new ScopeResult.create_from_variant, which makes ScopeResult instantiatable from python.

Add a few regression tests and implement new ScopeResult.create_from_variant, which makes ScopeResult instantiatable from python.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:279
http://jenkins.qa.ubuntu.com/job/libunity-ci/92/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-amd64-ci/77/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-armhf-ci/77/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-i386-ci/77/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/libunity-ci/92/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:280
http://jenkins.qa.ubuntu.com/job/libunity-ci/93/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-amd64-ci/78/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-armhf-ci/78/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-i386-ci/78/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/libunity-ci/93/rebuild

review: Needs Fixing (continuous-integration)

[Paweł Stołowski (stolowski)]

193 + // careful here, using dark corners of vala compiler to not copy everything
194 + ScopeResult real_result = ScopeResult ();
195 + unowned ScopeResult result = real_result;
...
205 + return result;

This looks suspicious... since real_result is freed on return, are you sure we're actually returning reference of a valid object? I've never been to dark corners of valac ;)

review: Needs Information

[Michal Hruby (mhr3)]

> 193 + // careful here, using dark corners of vala compiler to not copy
> everything
> 194 + ScopeResult real_result = ScopeResult ();
> 195 + unowned ScopeResult result = real_result;
> ...
> 205 + return result;
>
> This looks suspicious... since real_result is freed on return, are you sure
> we're actually returning reference of a valid object? I've never been to dark
> corners of valac ;)

Yes, the result is copied before returning (as with any other variable when you assign unowned var to owned one). The comment refers to the real_result vs result, if the variant.get() call used "out real_result.xyz", vala generates some crazy scary code with ~10 copies of the ScopeResult.

[Paweł Stołowski (stolowski)]

Ok then!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/libunity-autolanding/59/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/generic-cleanup-mbs/1436/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-amd64-autolanding/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-armhf-autolanding/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-i386-autolanding/54/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:280
http://jenkins.qa.ubuntu.com/job/libunity-ci/96/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-amd64-ci/81/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-armhf-ci/81/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/libunity-saucy-i386-ci/81/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/libunity-ci/96/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mhr3/libunity/scope-result-in-python]

281. By Michal Hruby on 2013-08-15

Don't require overrides when running python tests

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:281
http://jenkins.qa.ubuntu.com/job/libunity-ci/97/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libunity-saucy-amd64-ci/82
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libunity-saucy-armhf-ci/82
    SUCCESS: http://jenkins.qa.ubuntu.com/job/libunity-saucy-i386-ci/82

Click here to trigger a rebuild:
http://s-jenkins:8080/job/libunity-ci/97/rebuild

review: Approve (continuous-integration)

=== modified file 'bindings/python/Unity.py'
--- bindings/python/Unity.py	2013-04-17 10:41:45 +0000
+++ bindings/python/Unity.py	2013-08-15 07:56:46 +0000
@@ -50,49 +50,62 @@
         if len(args) > 0:
             Unity.ResultSet.add_result(self, *args)
         elif len(kwargs) > 0:
-            uri = None
-            icon = None
-            category = 0
-            result_type = 0
-            mimetype = None
-            title = None
-            comment = None
-            dnd_uri = None
-            metadata = {}
-
-            for col_name, value in kwargs.items():
-                if col_name == "uri": uri = value
-                elif col_name == "icon": icon = value
-                elif col_name == "category": category = value
-                elif col_name == "result_type": result_type = value
-                elif col_name == "mimetype": mimetype = value
-                elif col_name == "title": title = value
-                elif col_name == "comment": comment = value
-                elif col_name == "dnd_uri": dnd_uri = value
-                else:
-                    if isinstance(value, GLib.Variant):
-                        metadata[col_name] = value
-                    elif isinstance(value, str):
-                        metadata[col_name] = GLib.Variant("s", value)
-                    elif isinstance(value, int):
-                        metadata[col_name] = GLib.Variant("i", value)
-                    elif sys.version_info < (3, 0, 0):
-                        # unicode is not defined in py3
-                        if isinstance(value, unicode):
-                            metadata[col_name] = GLib.Variant("s", value)
-
-
-            result = GLib.Variant("(ssuussssa{sv})", (uri, icon, category,
-                                                      result_type, mimetype,
-                                                      title, comment, dnd_uri,
-                                                      metadata))
-
+            result = kwargs_to_result_variant(**kwargs)
             Unity.ResultSet.add_result_from_variant(self, result)
 
 
+def kwargs_to_result_variant(**kwargs):
+    uri = None
+    icon = ""
+    category = 0
+    result_type = 0
+    mimetype = None
+    title = None
+    comment = ""
+    dnd_uri = None
+    metadata = {}
+
+    for col_name, value in kwargs.items():
+        if col_name == "uri": uri = value
+        elif col_name == "icon": icon = value
+        elif col_name == "category": category = value
+        elif col_name == "result_type": result_type = value
+        elif col_name == "mimetype": mimetype = value
+        elif col_name == "title": title = value
+        elif col_name == "comment": comment = value
+        elif col_name == "dnd_uri": dnd_uri = value
+        else:
+            if isinstance(value, GLib.Variant):
+                metadata[col_name] = value
+            elif isinstance(value, str):
+                metadata[col_name] = GLib.Variant("s", value)
+            elif isinstance(value, int):
+                metadata[col_name] = GLib.Variant("i", value)
+            elif sys.version_info < (3, 0, 0):
+                # unicode is not defined in py3
+                if isinstance(value, unicode):
+                    metadata[col_name] = GLib.Variant("s", value)
+
+
+    result = GLib.Variant("(ssuussssa{sv})", (uri, icon, category,
+                                              result_type, mimetype,
+                                              title, comment, dnd_uri,
+                                              metadata))
+    return result
+
+
+def scope_result_create(*args, **kwargs):
+    if len(kwargs) > 0:
+        result = kwargs_to_result_variant(**kwargs)
+        return Unity.ScopeResult.create_from_variant(result)
+    return original_scope_result_create(*args)
+
+
 ScopeSearchBase = override(ScopeSearchBase)
 __all__.append('ScopeSearchBase')
 ResultPreviewer = override(ResultPreviewer)
 __all__.append('ResultPreviewer')
 ResultSet = override(ResultSet)
 __all__.append('ResultSet')
+original_scope_result_create = Unity.ScopeResult.create
+Unity.ScopeResult.create = scope_result_create

=== modified file 'configure.ac'
--- configure.ac	2013-08-01 10:22:39 +0000
+++ configure.ac	2013-08-15 07:56:46 +0000
@@ -1,5 +1,5 @@
 # When releasing also remember to update the soname as instructed below
-AC_INIT(libunity, 7.0.10)
+AC_INIT(libunity, 7.0.11)
 
 AM_INIT_AUTOMAKE(AC_PACKAGE_NAME, AC_PACKAGE_VERSION)
 AM_CONFIG_HEADER(config.h)

=== modified file 'debian/changelog'
--- debian/changelog	2013-08-09 11:50:54 +0000
+++ debian/changelog	2013-08-15 07:56:46 +0000
@@ -1,3 +1,9 @@
+libunity (7.0.11-0ubuntu1) UNRELEASED; urgency=low
+
+  * Added new ScopeResult.create_from_variant
+
+ -- Michal Hruby <[e-mail]>  Wed, 14 Aug 2013 17:04:10 +0200
+
 libunity (7.0.10+13.10.20130809.1-0ubuntu1) saucy; urgency=low
 
   [ Michal Hruby ]

=== modified file 'debian/libunity9.symbols'
--- debian/libunity9.symbols	2013-08-06 07:39:12 +0000
+++ debian/libunity9.symbols	2013-08-15 07:56:46 +0000
@@ -596,6 +596,7 @@
  unity_scope_loader_new@Base 7.0.7+13.10.20130703
  unity_scope_result_copy@Base 7.0.0daily13.05.31ubuntu.unity.next
  unity_scope_result_create@Base 7.0.0daily13.05.31ubuntu.unity.next
+ unity_scope_result_create_from_variant@Base 0replaceme
  unity_scope_result_destroy@Base 7.0.0daily13.05.31ubuntu.unity.next
  unity_scope_result_dup@Base 7.0.0daily13.05.31ubuntu.unity.next
  unity_scope_result_free@Base 7.0.0daily13.05.31ubuntu.unity.next

=== modified file 'extras/Makefile.am'
--- extras/Makefile.am	2013-02-27 18:19:03 +0000
+++ extras/Makefile.am	2013-08-15 07:56:46 +0000
@@ -82,8 +82,8 @@
 
 libunity_extras_la_vala.stamp: $(libunity_extras_la_VALASOURCES)
 	$(AM_V_GEN) $(VALAC) $(libunity_extras_la_VALAFLAGS) $^
-	@sed -i -e 's/<namespace name="UnityExtras" version="@GIR_VERSION@" c:prefix="Unity">/<namespace name="UnityExtras" version="@GIR_VERSION@" c:prefix="Unity" shared-library="libunity-extras.so.@LIBUNITY_LT_CURRENT@">/g' UnityExtras-@GIR_VERSION@.gir
-	@sed -i -e 's/"Extras/"/;s/"extras_/"/' UnityExtras-@GIR_VERSION@.gir
+	@sed -i -e 's/<namespace name="UnityExtras" version="@GIR_VERSION@" c:prefix="Unity">/<namespace name="UnityExtras" version="@GIR_VERSION@" c:prefix="UnityExtras" shared-library="libunity-extras.so.@LIBUNITY_LT_CURRENT@">/g' UnityExtras-@GIR_VERSION@.gir
+	@sed -i -e 's/"Extras/"/g;s/"extras_/"/' UnityExtras-@GIR_VERSION@.gir
 	@sed -i -e 's/<parameter name="scope_creation_cb" transfer-ownership="none"/<parameter name="scope_creation_cb" transfer-ownership="none" scope="call"/' UnityExtras-@GIR_VERSION@.gir
 	@touch $@
 

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2013-07-23 15:56:18 +0000
+++ src/Makefile.am	2013-08-15 07:56:46 +0000
@@ -149,6 +149,7 @@
 	@sed -i -e 's/<namespace name="Unity" version="@GIR_VERSION@" c:prefix="Unity">/<namespace name="Unity" version="@GIR_VERSION@" c:prefix="Unity" shared-library="libunity.so.@LIBUNITY_LT_CURRENT@">/g' Unity-@GIR_VERSION@.gir
 	@sed -i -e 's/emit_preview_ready/preview_ready/g' Unity-@GIR_VERSION@.gir
 	@sed -i -e 's/<parameter name="async_callback" transfer-ownership="none" /<parameter name="async_callback" transfer-ownership="none" scope="async" /g' Unity-@GIR_VERSION@.gir
+	@sed -i -e 's/<record name="ScopeResult">/<record name="ScopeResult" c:type="UnityScopeResult" glib:type-name="UnityScopeResult" glib:get-type="unity_scope_result_get_type">/;s/<record name="SearchContext">/<record name="SearchContext" c:type="UnitySearchContext" glib:type-name="UnitySearchContext" glib:get-type="unity_search_context_get_type">/' Unity-@GIR_VERSION@.gir
 	@touch $@
 
 BUILT_SOURCES += libunity_la_vala.stamp

=== modified file 'src/unity-scope-interface.vala'
--- src/unity-scope-interface.vala	2013-07-22 14:02:05 +0000
+++ src/unity-scope-interface.vala	2013-08-15 07:56:46 +0000
@@ -154,6 +154,37 @@
 
     return result;
   }
+
+  /**
+   * Create a new ScopeResult from a tuple variant
+   *
+   * This method will create a new heap-allocated ScopeResult.
+   * It is primarily meant for low-level language bindings.
+   */
+  public static ScopeResult? create_from_variant (Variant variant)
+  {
+    const string EXPECTED_SIG = "(ssuussssa{sv})";
+    if (variant.get_type_string () != EXPECTED_SIG)
+    {
+      warning ("Incorrect variant signature, expected \"%s\", got \"%s\"",
+               EXPECTED_SIG, variant.get_type_string ());
+      return null;
+    }
+    Variant dict;
+    // careful here, using dark corners of vala compiler to not copy everything
+    ScopeResult real_result = ScopeResult ();
+    unowned ScopeResult result = real_result;
+
+    variant.get ("(&s&suu&s&s&s&s@a{sv})",
+                 out result.uri, out result.icon_hint, out result.category,
+                 out result.result_type, out result.mimetype, out result.title,
+                 out result.comment, out result.dnd_uri, out dict);
+
+    HashTable<string, Variant> metadata = (HashTable<string, Variant>) dict;
+    result.metadata = metadata;
+
+    return result;
+  }
 }
 
 public abstract class Cancellable: Object

=== modified file 'test/python/Makefile.am'
--- test/python/Makefile.am	2012-10-26 14:42:43 +0000
+++ test/python/Makefile.am	2013-08-15 07:56:46 +0000
@@ -1,6 +1,6 @@
 include $(top_srcdir)/Makefile.decl
 
-TESTS = bug-1062331.py extras.py
+TESTS = bug-1062331.py extras.py container-ownership.py scope-result.py
 TEST_EXTENSIONS = .py
 
 # gtester doesn't care about our TESTS_ENVIRONMENT, so can't use it

=== added file 'test/python/container-ownership.py'
--- test/python/container-ownership.py	1970-01-01 00:00:00 +0000
+++ test/python/container-ownership.py	2013-08-15 07:56:46 +0000
@@ -0,0 +1,12 @@
+#!/usr/bin/python
+from gi.repository import Unity, Gio
+
+category_set = Unity.CategorySet.new()
+cat = Unity.Category.new("example", "Example", Gio.ThemedIcon.new("test"), Unity.CategoryRenderer.DEFAULT)
+category_set.add(cat)
+cat = Unity.Category.new("another", "Another", Gio.ThemedIcon.new("test"), Unity.CategoryRenderer.GRID)
+category_set.add(cat)
+cat_list = category_set.get_categories()
+#if the binding is broken there'll be double free
+del cat_list
+del category_set

=== added file 'test/python/scope-result.py'
--- test/python/scope-result.py	1970-01-01 00:00:00 +0000
+++ test/python/scope-result.py	2013-08-15 07:56:46 +0000
@@ -0,0 +1,28 @@
+#!/usr/bin/python
+from gi.repository import Unity, GLib
+
+class TestResultSet(Unity.ResultSet):
+    def __init__(self):
+        Unity.ResultSet.__init__(self)
+        self.results = []
+
+    def do_add_result(self, result):
+        assert(result.uri == "file:///foo")
+        assert(result.title == "Title")
+        assert(len(result.metadata) > 0)
+        assert("whatever" in result.metadata)
+        # bug in pygi? copy() shouldn't be needed
+        self.results.append(result.copy())
+
+rs = TestResultSet()
+# overrides are not installed when running tests, so don't use add_result
+variant = GLib.Variant('(ssuussssa{sv})', ("file:///foo", "file:///", 0, 0,
+                                           "text/plain", "Title", "",
+                                           "file:///foo", {'whatever': GLib.Variant("s", "foo")}))
+rs.add_result_from_variant(variant)
+
+saved_result = rs.results[0]
+assert(saved_result.uri == "file:///foo")
+assert(saved_result.title == "Title")
+assert(len(saved_result.metadata) > 0)
+assert("whatever" in saved_result.metadata)

=== modified file 'test/vala/test-scope-base.vala'
--- test/vala/test-scope-base.vala	2013-07-23 13:23:06 +0000
+++ test/vala/test-scope-base.vala	2013-08-15 07:56:46 +0000
@@ -32,6 +32,8 @@
         Fixture.create<AbstractScopeTester> (AbstractScopeTester.test_add_variant_result));
       GLib.Test.add_data_func ("/Unit/Cancellable/GetGCancellable",
         Fixture.create<CancellableTester> (CancellableTester.test_get_gcancellable));
+      GLib.Test.add_data_func ("/Unit/ScopeResult/CreateFromVariant",
+        Fixture.create<ScopeResultTester> (ScopeResultTester.test_create_from_variant));
     }
 
     class AbstractScopeTester: Object, Fixture
@@ -205,5 +207,33 @@
         assert (cancellable.get_gcancellable () is GLib.Cancellable);
       }
     }
+    
+    class ScopeResultTester: Object, Fixture
+    {
+      public void test_create_from_variant ()
+      {
+        var metadata_ht = new HashTable<string, Variant> (str_hash, str_equal);
+        metadata_ht["foo"] = new Variant.int32 (32);
+        metadata_ht["bar"] = new Variant.string ("qoo");
+        Variant metadata = metadata_ht;
+        var variant = new Variant ("(ssuussss@a{sv})",
+                                   "test://", "unknown",
+                                   0, Unity.ResultType.PERSONAL,
+                                   "text/html", "Test", "comment",
+                                   "test://", metadata);
+
+        var scope_result = Unity.ScopeResult.create_from_variant (variant);
+        assert (scope_result.uri == "test://");
+        assert (scope_result.icon_hint == "unknown");
+        assert (scope_result.category == 0);
+        assert (scope_result.result_type == Unity.ResultType.PERSONAL);
+        assert (scope_result.mimetype == "text/html");
+        assert (scope_result.title == "Test");
+        assert (scope_result.comment == "comment");
+        assert (scope_result.dnd_uri == scope_result.uri);
+        assert (scope_result.metadata.size () == 2);
+        assert (scope_result.metadata["bar"].get_string () == "qoo");
+      }
+    }
   }
 }