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

Proposed by Michal Hruby on 2013-08-14
Status: Merged
Approved by: Michal Hruby on 2013-08-15
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 on 2013-08-15
Paweł Stołowski 2013-08-14 Approve on 2013-08-14
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.
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
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
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.

Paweł Stołowski (stolowski) wrote :

Ok then!

review: Approve
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
281. By Michal Hruby on 2013-08-15

Don't require overrides when running python tests

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
1=== modified file 'bindings/python/Unity.py'
2--- bindings/python/Unity.py 2013-04-17 10:41:45 +0000
3+++ bindings/python/Unity.py 2013-08-15 07:56:46 +0000
4@@ -50,49 +50,62 @@
5 if len(args) > 0:
6 Unity.ResultSet.add_result(self, *args)
7 elif len(kwargs) > 0:
8- uri = None
9- icon = None
10- category = 0
11- result_type = 0
12- mimetype = None
13- title = None
14- comment = None
15- dnd_uri = None
16- metadata = {}
17-
18- for col_name, value in kwargs.items():
19- if col_name == "uri": uri = value
20- elif col_name == "icon": icon = value
21- elif col_name == "category": category = value
22- elif col_name == "result_type": result_type = value
23- elif col_name == "mimetype": mimetype = value
24- elif col_name == "title": title = value
25- elif col_name == "comment": comment = value
26- elif col_name == "dnd_uri": dnd_uri = value
27- else:
28- if isinstance(value, GLib.Variant):
29- metadata[col_name] = value
30- elif isinstance(value, str):
31- metadata[col_name] = GLib.Variant("s", value)
32- elif isinstance(value, int):
33- metadata[col_name] = GLib.Variant("i", value)
34- elif sys.version_info < (3, 0, 0):
35- # unicode is not defined in py3
36- if isinstance(value, unicode):
37- metadata[col_name] = GLib.Variant("s", value)
38-
39-
40- result = GLib.Variant("(ssuussssa{sv})", (uri, icon, category,
41- result_type, mimetype,
42- title, comment, dnd_uri,
43- metadata))
44-
45+ result = kwargs_to_result_variant(**kwargs)
46 Unity.ResultSet.add_result_from_variant(self, result)
47
48
49+def kwargs_to_result_variant(**kwargs):
50+ uri = None
51+ icon = ""
52+ category = 0
53+ result_type = 0
54+ mimetype = None
55+ title = None
56+ comment = ""
57+ dnd_uri = None
58+ metadata = {}
59+
60+ for col_name, value in kwargs.items():
61+ if col_name == "uri": uri = value
62+ elif col_name == "icon": icon = value
63+ elif col_name == "category": category = value
64+ elif col_name == "result_type": result_type = value
65+ elif col_name == "mimetype": mimetype = value
66+ elif col_name == "title": title = value
67+ elif col_name == "comment": comment = value
68+ elif col_name == "dnd_uri": dnd_uri = value
69+ else:
70+ if isinstance(value, GLib.Variant):
71+ metadata[col_name] = value
72+ elif isinstance(value, str):
73+ metadata[col_name] = GLib.Variant("s", value)
74+ elif isinstance(value, int):
75+ metadata[col_name] = GLib.Variant("i", value)
76+ elif sys.version_info < (3, 0, 0):
77+ # unicode is not defined in py3
78+ if isinstance(value, unicode):
79+ metadata[col_name] = GLib.Variant("s", value)
80+
81+
82+ result = GLib.Variant("(ssuussssa{sv})", (uri, icon, category,
83+ result_type, mimetype,
84+ title, comment, dnd_uri,
85+ metadata))
86+ return result
87+
88+
89+def scope_result_create(*args, **kwargs):
90+ if len(kwargs) > 0:
91+ result = kwargs_to_result_variant(**kwargs)
92+ return Unity.ScopeResult.create_from_variant(result)
93+ return original_scope_result_create(*args)
94+
95+
96 ScopeSearchBase = override(ScopeSearchBase)
97 __all__.append('ScopeSearchBase')
98 ResultPreviewer = override(ResultPreviewer)
99 __all__.append('ResultPreviewer')
100 ResultSet = override(ResultSet)
101 __all__.append('ResultSet')
102+original_scope_result_create = Unity.ScopeResult.create
103+Unity.ScopeResult.create = scope_result_create
104
105=== modified file 'configure.ac'
106--- configure.ac 2013-08-01 10:22:39 +0000
107+++ configure.ac 2013-08-15 07:56:46 +0000
108@@ -1,5 +1,5 @@
109 # When releasing also remember to update the soname as instructed below
110-AC_INIT(libunity, 7.0.10)
111+AC_INIT(libunity, 7.0.11)
112
113 AM_INIT_AUTOMAKE(AC_PACKAGE_NAME, AC_PACKAGE_VERSION)
114 AM_CONFIG_HEADER(config.h)
115
116=== modified file 'debian/changelog'
117--- debian/changelog 2013-08-09 11:50:54 +0000
118+++ debian/changelog 2013-08-15 07:56:46 +0000
119@@ -1,3 +1,9 @@
120+libunity (7.0.11-0ubuntu1) UNRELEASED; urgency=low
121+
122+ * Added new ScopeResult.create_from_variant
123+
124+ -- Michal Hruby <michal.hruby@canonical.com> Wed, 14 Aug 2013 17:04:10 +0200
125+
126 libunity (7.0.10+13.10.20130809.1-0ubuntu1) saucy; urgency=low
127
128 [ Michal Hruby ]
129
130=== modified file 'debian/libunity9.symbols'
131--- debian/libunity9.symbols 2013-08-06 07:39:12 +0000
132+++ debian/libunity9.symbols 2013-08-15 07:56:46 +0000
133@@ -596,6 +596,7 @@
134 unity_scope_loader_new@Base 7.0.7+13.10.20130703
135 unity_scope_result_copy@Base 7.0.0daily13.05.31ubuntu.unity.next
136 unity_scope_result_create@Base 7.0.0daily13.05.31ubuntu.unity.next
137+ unity_scope_result_create_from_variant@Base 0replaceme
138 unity_scope_result_destroy@Base 7.0.0daily13.05.31ubuntu.unity.next
139 unity_scope_result_dup@Base 7.0.0daily13.05.31ubuntu.unity.next
140 unity_scope_result_free@Base 7.0.0daily13.05.31ubuntu.unity.next
141
142=== modified file 'extras/Makefile.am'
143--- extras/Makefile.am 2013-02-27 18:19:03 +0000
144+++ extras/Makefile.am 2013-08-15 07:56:46 +0000
145@@ -82,8 +82,8 @@
146
147 libunity_extras_la_vala.stamp: $(libunity_extras_la_VALASOURCES)
148 $(AM_V_GEN) $(VALAC) $(libunity_extras_la_VALAFLAGS) $^
149- @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
150- @sed -i -e 's/"Extras/"/;s/"extras_/"/' UnityExtras-@GIR_VERSION@.gir
151+ @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
152+ @sed -i -e 's/"Extras/"/g;s/"extras_/"/' UnityExtras-@GIR_VERSION@.gir
153 @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
154 @touch $@
155
156
157=== modified file 'src/Makefile.am'
158--- src/Makefile.am 2013-07-23 15:56:18 +0000
159+++ src/Makefile.am 2013-08-15 07:56:46 +0000
160@@ -149,6 +149,7 @@
161 @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
162 @sed -i -e 's/emit_preview_ready/preview_ready/g' Unity-@GIR_VERSION@.gir
163 @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
164+ @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
165 @touch $@
166
167 BUILT_SOURCES += libunity_la_vala.stamp
168
169=== modified file 'src/unity-scope-interface.vala'
170--- src/unity-scope-interface.vala 2013-07-22 14:02:05 +0000
171+++ src/unity-scope-interface.vala 2013-08-15 07:56:46 +0000
172@@ -154,6 +154,37 @@
173
174 return result;
175 }
176+
177+ /**
178+ * Create a new ScopeResult from a tuple variant
179+ *
180+ * This method will create a new heap-allocated ScopeResult.
181+ * It is primarily meant for low-level language bindings.
182+ */
183+ public static ScopeResult? create_from_variant (Variant variant)
184+ {
185+ const string EXPECTED_SIG = "(ssuussssa{sv})";
186+ if (variant.get_type_string () != EXPECTED_SIG)
187+ {
188+ warning ("Incorrect variant signature, expected \"%s\", got \"%s\"",
189+ EXPECTED_SIG, variant.get_type_string ());
190+ return null;
191+ }
192+ Variant dict;
193+ // careful here, using dark corners of vala compiler to not copy everything
194+ ScopeResult real_result = ScopeResult ();
195+ unowned ScopeResult result = real_result;
196+
197+ variant.get ("(&s&suu&s&s&s&s@a{sv})",
198+ out result.uri, out result.icon_hint, out result.category,
199+ out result.result_type, out result.mimetype, out result.title,
200+ out result.comment, out result.dnd_uri, out dict);
201+
202+ HashTable<string, Variant> metadata = (HashTable<string, Variant>) dict;
203+ result.metadata = metadata;
204+
205+ return result;
206+ }
207 }
208
209 public abstract class Cancellable: Object
210
211=== modified file 'test/python/Makefile.am'
212--- test/python/Makefile.am 2012-10-26 14:42:43 +0000
213+++ test/python/Makefile.am 2013-08-15 07:56:46 +0000
214@@ -1,6 +1,6 @@
215 include $(top_srcdir)/Makefile.decl
216
217-TESTS = bug-1062331.py extras.py
218+TESTS = bug-1062331.py extras.py container-ownership.py scope-result.py
219 TEST_EXTENSIONS = .py
220
221 # gtester doesn't care about our TESTS_ENVIRONMENT, so can't use it
222
223=== added file 'test/python/container-ownership.py'
224--- test/python/container-ownership.py 1970-01-01 00:00:00 +0000
225+++ test/python/container-ownership.py 2013-08-15 07:56:46 +0000
226@@ -0,0 +1,12 @@
227+#!/usr/bin/python
228+from gi.repository import Unity, Gio
229+
230+category_set = Unity.CategorySet.new()
231+cat = Unity.Category.new("example", "Example", Gio.ThemedIcon.new("test"), Unity.CategoryRenderer.DEFAULT)
232+category_set.add(cat)
233+cat = Unity.Category.new("another", "Another", Gio.ThemedIcon.new("test"), Unity.CategoryRenderer.GRID)
234+category_set.add(cat)
235+cat_list = category_set.get_categories()
236+#if the binding is broken there'll be double free
237+del cat_list
238+del category_set
239
240=== added file 'test/python/scope-result.py'
241--- test/python/scope-result.py 1970-01-01 00:00:00 +0000
242+++ test/python/scope-result.py 2013-08-15 07:56:46 +0000
243@@ -0,0 +1,28 @@
244+#!/usr/bin/python
245+from gi.repository import Unity, GLib
246+
247+class TestResultSet(Unity.ResultSet):
248+ def __init__(self):
249+ Unity.ResultSet.__init__(self)
250+ self.results = []
251+
252+ def do_add_result(self, result):
253+ assert(result.uri == "file:///foo")
254+ assert(result.title == "Title")
255+ assert(len(result.metadata) > 0)
256+ assert("whatever" in result.metadata)
257+ # bug in pygi? copy() shouldn't be needed
258+ self.results.append(result.copy())
259+
260+rs = TestResultSet()
261+# overrides are not installed when running tests, so don't use add_result
262+variant = GLib.Variant('(ssuussssa{sv})', ("file:///foo", "file:///", 0, 0,
263+ "text/plain", "Title", "",
264+ "file:///foo", {'whatever': GLib.Variant("s", "foo")}))
265+rs.add_result_from_variant(variant)
266+
267+saved_result = rs.results[0]
268+assert(saved_result.uri == "file:///foo")
269+assert(saved_result.title == "Title")
270+assert(len(saved_result.metadata) > 0)
271+assert("whatever" in saved_result.metadata)
272
273=== modified file 'test/vala/test-scope-base.vala'
274--- test/vala/test-scope-base.vala 2013-07-23 13:23:06 +0000
275+++ test/vala/test-scope-base.vala 2013-08-15 07:56:46 +0000
276@@ -32,6 +32,8 @@
277 Fixture.create<AbstractScopeTester> (AbstractScopeTester.test_add_variant_result));
278 GLib.Test.add_data_func ("/Unit/Cancellable/GetGCancellable",
279 Fixture.create<CancellableTester> (CancellableTester.test_get_gcancellable));
280+ GLib.Test.add_data_func ("/Unit/ScopeResult/CreateFromVariant",
281+ Fixture.create<ScopeResultTester> (ScopeResultTester.test_create_from_variant));
282 }
283
284 class AbstractScopeTester: Object, Fixture
285@@ -205,5 +207,33 @@
286 assert (cancellable.get_gcancellable () is GLib.Cancellable);
287 }
288 }
289+
290+ class ScopeResultTester: Object, Fixture
291+ {
292+ public void test_create_from_variant ()
293+ {
294+ var metadata_ht = new HashTable<string, Variant> (str_hash, str_equal);
295+ metadata_ht["foo"] = new Variant.int32 (32);
296+ metadata_ht["bar"] = new Variant.string ("qoo");
297+ Variant metadata = metadata_ht;
298+ var variant = new Variant ("(ssuussss@a{sv})",
299+ "test://", "unknown",
300+ 0, Unity.ResultType.PERSONAL,
301+ "text/html", "Test", "comment",
302+ "test://", metadata);
303+
304+ var scope_result = Unity.ScopeResult.create_from_variant (variant);
305+ assert (scope_result.uri == "test://");
306+ assert (scope_result.icon_hint == "unknown");
307+ assert (scope_result.category == 0);
308+ assert (scope_result.result_type == Unity.ResultType.PERSONAL);
309+ assert (scope_result.mimetype == "text/html");
310+ assert (scope_result.title == "Test");
311+ assert (scope_result.comment == "comment");
312+ assert (scope_result.dnd_uri == scope_result.uri);
313+ assert (scope_result.metadata.size () == 2);
314+ assert (scope_result.metadata["bar"].get_string () == "qoo");
315+ }
316+ }
317 }
318 }

Subscribers

People subscribed via source and target branches