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

Proposed by Michal Hruby
Status: Merged
Approved by: Michal Hruby
Approved revision: 281
Merged at revision: 276
Proposed branch: lp:~mhr3/libunity/scope-result-in-python
Merge into: lp:libunity
Diff against target: 318 lines (+163/-41)
11 files modified
bindings/python/Unity.py (+50/-37)
configure.ac (+1/-1)
debian/changelog (+6/-0)
debian/libunity9.symbols (+1/-0)
extras/Makefile.am (+2/-2)
src/Makefile.am (+1/-0)
src/unity-scope-interface.vala (+31/-0)
test/python/Makefile.am (+1/-1)
test/python/container-ownership.py (+12/-0)
test/python/scope-result.py (+28/-0)
test/vala/test-scope-base.vala (+30/-0)
To merge this branch: bzr merge lp:~mhr3/libunity/scope-result-in-python
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Paweł Stołowski (community) Approve
Review via email: mp+180158@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote :

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
Revision history for this message
Michal Hruby (mhr3) wrote :

> 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.

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Ok then!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
281. By Michal Hruby

Don't require overrides when running python tests

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
50 if len(args) > 0:50 if len(args) > 0:
51 Unity.ResultSet.add_result(self, *args)51 Unity.ResultSet.add_result(self, *args)
52 elif len(kwargs) > 0:52 elif len(kwargs) > 0:
53 uri = None53 result = kwargs_to_result_variant(**kwargs)
54 icon = None
55 category = 0
56 result_type = 0
57 mimetype = None
58 title = None
59 comment = None
60 dnd_uri = None
61 metadata = {}
62
63 for col_name, value in kwargs.items():
64 if col_name == "uri": uri = value
65 elif col_name == "icon": icon = value
66 elif col_name == "category": category = value
67 elif col_name == "result_type": result_type = value
68 elif col_name == "mimetype": mimetype = value
69 elif col_name == "title": title = value
70 elif col_name == "comment": comment = value
71 elif col_name == "dnd_uri": dnd_uri = value
72 else:
73 if isinstance(value, GLib.Variant):
74 metadata[col_name] = value
75 elif isinstance(value, str):
76 metadata[col_name] = GLib.Variant("s", value)
77 elif isinstance(value, int):
78 metadata[col_name] = GLib.Variant("i", value)
79 elif sys.version_info < (3, 0, 0):
80 # unicode is not defined in py3
81 if isinstance(value, unicode):
82 metadata[col_name] = GLib.Variant("s", value)
83
84
85 result = GLib.Variant("(ssuussssa{sv})", (uri, icon, category,
86 result_type, mimetype,
87 title, comment, dnd_uri,
88 metadata))
89
90 Unity.ResultSet.add_result_from_variant(self, result)54 Unity.ResultSet.add_result_from_variant(self, result)
9155
9256
57def kwargs_to_result_variant(**kwargs):
58 uri = None
59 icon = ""
60 category = 0
61 result_type = 0
62 mimetype = None
63 title = None
64 comment = ""
65 dnd_uri = None
66 metadata = {}
67
68 for col_name, value in kwargs.items():
69 if col_name == "uri": uri = value
70 elif col_name == "icon": icon = value
71 elif col_name == "category": category = value
72 elif col_name == "result_type": result_type = value
73 elif col_name == "mimetype": mimetype = value
74 elif col_name == "title": title = value
75 elif col_name == "comment": comment = value
76 elif col_name == "dnd_uri": dnd_uri = value
77 else:
78 if isinstance(value, GLib.Variant):
79 metadata[col_name] = value
80 elif isinstance(value, str):
81 metadata[col_name] = GLib.Variant("s", value)
82 elif isinstance(value, int):
83 metadata[col_name] = GLib.Variant("i", value)
84 elif sys.version_info < (3, 0, 0):
85 # unicode is not defined in py3
86 if isinstance(value, unicode):
87 metadata[col_name] = GLib.Variant("s", value)
88
89
90 result = GLib.Variant("(ssuussssa{sv})", (uri, icon, category,
91 result_type, mimetype,
92 title, comment, dnd_uri,
93 metadata))
94 return result
95
96
97def scope_result_create(*args, **kwargs):
98 if len(kwargs) > 0:
99 result = kwargs_to_result_variant(**kwargs)
100 return Unity.ScopeResult.create_from_variant(result)
101 return original_scope_result_create(*args)
102
103
93ScopeSearchBase = override(ScopeSearchBase)104ScopeSearchBase = override(ScopeSearchBase)
94__all__.append('ScopeSearchBase')105__all__.append('ScopeSearchBase')
95ResultPreviewer = override(ResultPreviewer)106ResultPreviewer = override(ResultPreviewer)
96__all__.append('ResultPreviewer')107__all__.append('ResultPreviewer')
97ResultSet = override(ResultSet)108ResultSet = override(ResultSet)
98__all__.append('ResultSet')109__all__.append('ResultSet')
110original_scope_result_create = Unity.ScopeResult.create
111Unity.ScopeResult.create = scope_result_create
99112
=== 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 @@
1# When releasing also remember to update the soname as instructed below1# When releasing also remember to update the soname as instructed below
2AC_INIT(libunity, 7.0.10)2AC_INIT(libunity, 7.0.11)
33
4AM_INIT_AUTOMAKE(AC_PACKAGE_NAME, AC_PACKAGE_VERSION)4AM_INIT_AUTOMAKE(AC_PACKAGE_NAME, AC_PACKAGE_VERSION)
5AM_CONFIG_HEADER(config.h)5AM_CONFIG_HEADER(config.h)
66
=== 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 @@
1libunity (7.0.11-0ubuntu1) UNRELEASED; urgency=low
2
3 * Added new ScopeResult.create_from_variant
4
5 -- Michal Hruby <michal.hruby@canonical.com> Wed, 14 Aug 2013 17:04:10 +0200
6
1libunity (7.0.10+13.10.20130809.1-0ubuntu1) saucy; urgency=low7libunity (7.0.10+13.10.20130809.1-0ubuntu1) saucy; urgency=low
28
3 [ Michal Hruby ]9 [ Michal Hruby ]
410
=== 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 @@
596 unity_scope_loader_new@Base 7.0.7+13.10.20130703596 unity_scope_loader_new@Base 7.0.7+13.10.20130703
597 unity_scope_result_copy@Base 7.0.0daily13.05.31ubuntu.unity.next597 unity_scope_result_copy@Base 7.0.0daily13.05.31ubuntu.unity.next
598 unity_scope_result_create@Base 7.0.0daily13.05.31ubuntu.unity.next598 unity_scope_result_create@Base 7.0.0daily13.05.31ubuntu.unity.next
599 unity_scope_result_create_from_variant@Base 0replaceme
599 unity_scope_result_destroy@Base 7.0.0daily13.05.31ubuntu.unity.next600 unity_scope_result_destroy@Base 7.0.0daily13.05.31ubuntu.unity.next
600 unity_scope_result_dup@Base 7.0.0daily13.05.31ubuntu.unity.next601 unity_scope_result_dup@Base 7.0.0daily13.05.31ubuntu.unity.next
601 unity_scope_result_free@Base 7.0.0daily13.05.31ubuntu.unity.next602 unity_scope_result_free@Base 7.0.0daily13.05.31ubuntu.unity.next
602603
=== 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 @@
8282
83libunity_extras_la_vala.stamp: $(libunity_extras_la_VALASOURCES)83libunity_extras_la_vala.stamp: $(libunity_extras_la_VALASOURCES)
84 $(AM_V_GEN) $(VALAC) $(libunity_extras_la_VALAFLAGS) $^84 $(AM_V_GEN) $(VALAC) $(libunity_extras_la_VALAFLAGS) $^
85 @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@.gir85 @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
86 @sed -i -e 's/"Extras/"/;s/"extras_/"/' UnityExtras-@GIR_VERSION@.gir86 @sed -i -e 's/"Extras/"/g;s/"extras_/"/' UnityExtras-@GIR_VERSION@.gir
87 @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@.gir87 @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
88 @touch $@88 @touch $@
8989
9090
=== 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 @@
149 @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@.gir149 @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
150 @sed -i -e 's/emit_preview_ready/preview_ready/g' Unity-@GIR_VERSION@.gir150 @sed -i -e 's/emit_preview_ready/preview_ready/g' Unity-@GIR_VERSION@.gir
151 @sed -i -e 's/<parameter name="async_callback" transfer-ownership="none" /<parameter name="async_callback" transfer-ownership="none" scope="async" /g' Unity-@GIR_VERSION@.gir151 @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
152 @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
152 @touch $@153 @touch $@
153154
154BUILT_SOURCES += libunity_la_vala.stamp155BUILT_SOURCES += libunity_la_vala.stamp
155156
=== 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 @@
154154
155 return result;155 return result;
156 }156 }
157
158 /**
159 * Create a new ScopeResult from a tuple variant
160 *
161 * This method will create a new heap-allocated ScopeResult.
162 * It is primarily meant for low-level language bindings.
163 */
164 public static ScopeResult? create_from_variant (Variant variant)
165 {
166 const string EXPECTED_SIG = "(ssuussssa{sv})";
167 if (variant.get_type_string () != EXPECTED_SIG)
168 {
169 warning ("Incorrect variant signature, expected \"%s\", got \"%s\"",
170 EXPECTED_SIG, variant.get_type_string ());
171 return null;
172 }
173 Variant dict;
174 // careful here, using dark corners of vala compiler to not copy everything
175 ScopeResult real_result = ScopeResult ();
176 unowned ScopeResult result = real_result;
177
178 variant.get ("(&s&suu&s&s&s&s@a{sv})",
179 out result.uri, out result.icon_hint, out result.category,
180 out result.result_type, out result.mimetype, out result.title,
181 out result.comment, out result.dnd_uri, out dict);
182
183 HashTable<string, Variant> metadata = (HashTable<string, Variant>) dict;
184 result.metadata = metadata;
185
186 return result;
187 }
157}188}
158189
159public abstract class Cancellable: Object190public abstract class Cancellable: Object
160191
=== 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 @@
1include $(top_srcdir)/Makefile.decl1include $(top_srcdir)/Makefile.decl
22
3TESTS = bug-1062331.py extras.py3TESTS = bug-1062331.py extras.py container-ownership.py scope-result.py
4TEST_EXTENSIONS = .py4TEST_EXTENSIONS = .py
55
6# gtester doesn't care about our TESTS_ENVIRONMENT, so can't use it6# gtester doesn't care about our TESTS_ENVIRONMENT, so can't use it
77
=== 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 @@
1#!/usr/bin/python
2from gi.repository import Unity, Gio
3
4category_set = Unity.CategorySet.new()
5cat = Unity.Category.new("example", "Example", Gio.ThemedIcon.new("test"), Unity.CategoryRenderer.DEFAULT)
6category_set.add(cat)
7cat = Unity.Category.new("another", "Another", Gio.ThemedIcon.new("test"), Unity.CategoryRenderer.GRID)
8category_set.add(cat)
9cat_list = category_set.get_categories()
10#if the binding is broken there'll be double free
11del cat_list
12del category_set
013
=== 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 @@
1#!/usr/bin/python
2from gi.repository import Unity, GLib
3
4class TestResultSet(Unity.ResultSet):
5 def __init__(self):
6 Unity.ResultSet.__init__(self)
7 self.results = []
8
9 def do_add_result(self, result):
10 assert(result.uri == "file:///foo")
11 assert(result.title == "Title")
12 assert(len(result.metadata) > 0)
13 assert("whatever" in result.metadata)
14 # bug in pygi? copy() shouldn't be needed
15 self.results.append(result.copy())
16
17rs = TestResultSet()
18# overrides are not installed when running tests, so don't use add_result
19variant = GLib.Variant('(ssuussssa{sv})', ("file:///foo", "file:///", 0, 0,
20 "text/plain", "Title", "",
21 "file:///foo", {'whatever': GLib.Variant("s", "foo")}))
22rs.add_result_from_variant(variant)
23
24saved_result = rs.results[0]
25assert(saved_result.uri == "file:///foo")
26assert(saved_result.title == "Title")
27assert(len(saved_result.metadata) > 0)
28assert("whatever" in saved_result.metadata)
029
=== 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 @@
32 Fixture.create<AbstractScopeTester> (AbstractScopeTester.test_add_variant_result));32 Fixture.create<AbstractScopeTester> (AbstractScopeTester.test_add_variant_result));
33 GLib.Test.add_data_func ("/Unit/Cancellable/GetGCancellable",33 GLib.Test.add_data_func ("/Unit/Cancellable/GetGCancellable",
34 Fixture.create<CancellableTester> (CancellableTester.test_get_gcancellable));34 Fixture.create<CancellableTester> (CancellableTester.test_get_gcancellable));
35 GLib.Test.add_data_func ("/Unit/ScopeResult/CreateFromVariant",
36 Fixture.create<ScopeResultTester> (ScopeResultTester.test_create_from_variant));
35 }37 }
3638
37 class AbstractScopeTester: Object, Fixture39 class AbstractScopeTester: Object, Fixture
@@ -205,5 +207,33 @@
205 assert (cancellable.get_gcancellable () is GLib.Cancellable);207 assert (cancellable.get_gcancellable () is GLib.Cancellable);
206 }208 }
207 }209 }
210
211 class ScopeResultTester: Object, Fixture
212 {
213 public void test_create_from_variant ()
214 {
215 var metadata_ht = new HashTable<string, Variant> (str_hash, str_equal);
216 metadata_ht["foo"] = new Variant.int32 (32);
217 metadata_ht["bar"] = new Variant.string ("qoo");
218 Variant metadata = metadata_ht;
219 var variant = new Variant ("(ssuussss@a{sv})",
220 "test://", "unknown",
221 0, Unity.ResultType.PERSONAL,
222 "text/html", "Test", "comment",
223 "test://", metadata);
224
225 var scope_result = Unity.ScopeResult.create_from_variant (variant);
226 assert (scope_result.uri == "test://");
227 assert (scope_result.icon_hint == "unknown");
228 assert (scope_result.category == 0);
229 assert (scope_result.result_type == Unity.ResultType.PERSONAL);
230 assert (scope_result.mimetype == "text/html");
231 assert (scope_result.title == "Test");
232 assert (scope_result.comment == "comment");
233 assert (scope_result.dnd_uri == scope_result.uri);
234 assert (scope_result.metadata.size () == 2);
235 assert (scope_result.metadata["bar"].get_string () == "qoo");
236 }
237 }
208 }238 }
209}239}

Subscribers

People subscribed via source and target branches