Merge lp:~mhr3/unity/home-lens-ordering into lp:unity
- home-lens-ordering
- Merge into trunk
Proposed by
Michal Hruby
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Paweł Stołowski | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 2649 | ||||
Proposed branch: | lp:~mhr3/unity/home-lens-ordering | ||||
Merge into: | lp:unity | ||||
Prerequisite: | lp:~mhr3/unity/home-lens-fixes | ||||
Diff against target: |
870 lines (+565/-20) 5 files modified
UnityCore/HomeLens.cpp (+268/-12) UnityCore/HomeLens.h (+1/-0) UnityCore/Lens.cpp (+35/-3) UnityCore/Lens.h (+1/-0) tests/test_home_lens.cpp (+260/-5) |
||||
To merge this branch: | bzr merge lp:~mhr3/unity/home-lens-ordering | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paweł Stołowski (community) | Approve | ||
Review via email: mp+122039@code.launchpad.net |
This proposal supersedes a proposal from 2012-08-30.
Commit message
Implement new ordering of categories for home lens
Description of the change
Implement new ordering of categories as requested by design (categories with the highest amount of personal content matches are on top, followed by categories without personal content [also sorted by number of matches]; plus apps are on top if they contain exact match or if they're binary name contains exact match).
To post a comment you must log in.
Revision history for this message
Paweł Stołowski (stolowski) wrote : | # |
Revision history for this message
Paweł Stołowski (stolowski) wrote : | # |
Looks good, tests pass. Minor comments:
498 + std::string key_name(key);
Can we get rid of an extra string copy here? I think it would be ok to use glib string comparison.
224 + std::string lens_id(
Make it const.
review:
Needs Fixing
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'UnityCore/HomeLens.cpp' |
2 | --- UnityCore/HomeLens.cpp 2012-08-29 09:46:21 +0000 |
3 | +++ UnityCore/HomeLens.cpp 2012-08-31 14:43:22 +0000 |
4 | @@ -15,14 +15,17 @@ |
5 | * along with this program. If not, see <http://www.gnu.org/licenses/>. |
6 | * |
7 | * Authored by: Mikkel Kamstrup Erlandsen <mikkel.kamstrup@canonical.com> |
8 | + * Michal Hruby <michal.hruby@canonical.com> |
9 | */ |
10 | |
11 | #include <glib.h> |
12 | +#include <dee-icu.h> |
13 | #include <string> |
14 | #include <stdexcept> |
15 | #include <map> |
16 | #include <set> |
17 | #include <utility> |
18 | +#include <algorithm> |
19 | |
20 | #include "GLibSignal.h" |
21 | #include "HomeLens.h" |
22 | @@ -44,6 +47,8 @@ |
23 | const gchar* const HOMELENS_PRIORITY = "unity-homelens-priority"; |
24 | const gchar* const HOMELENS_RESULTS_MODEL = "unity-homelens-results-model"; |
25 | |
26 | +const unsigned RESULTS_NAME_COLUMN = 4; |
27 | +const unsigned RESULTS_COMMENT_COLUMN = 5; |
28 | } |
29 | |
30 | /* |
31 | @@ -241,6 +246,9 @@ |
32 | void OnSourceRowRemoved(DeeModel* model, DeeModelIter* iter); |
33 | |
34 | std::vector<unsigned> GetDefaultOrder(); |
35 | + std::string GetLensIdForCategory(unsigned) const; |
36 | + std::map<unsigned, Lens::Ptr> const& GetCategoryToLensMap() const; |
37 | + MergeMode GetMergeMode() const { return merge_mode_; } |
38 | |
39 | protected: |
40 | void RemoveSource(glib::Object<DeeModel> const& old_source); |
41 | @@ -249,6 +257,7 @@ |
42 | HomeLens::CategoryRegistry* cat_registry_; |
43 | MergeMode merge_mode_; |
44 | std::multimap<unsigned, unsigned, std::greater<unsigned> > category_ordering_; |
45 | + std::map<unsigned, Lens::Ptr> category_to_owner_; |
46 | }; |
47 | |
48 | /* |
49 | @@ -274,12 +283,53 @@ |
50 | Impl(HomeLens* owner, MergeMode merge_mode); |
51 | ~Impl(); |
52 | |
53 | + struct CategorySorter |
54 | + { |
55 | + CategorySorter(std::map<unsigned, unsigned>& results_per_category, |
56 | + std::map<unsigned, Lens::Ptr> const& category_owner_map) |
57 | + : results_per_category_(results_per_category) |
58 | + , category_to_owner_(category_owner_map) |
59 | + {} |
60 | + |
61 | + bool operator() (unsigned cat_a, unsigned cat_b) |
62 | + { |
63 | + bool a_has_personal_content = false; |
64 | + bool b_has_personal_content = false; |
65 | + |
66 | + auto it = category_to_owner_.find(cat_a); |
67 | + if (it != category_to_owner_.end() && it->second) |
68 | + { |
69 | + a_has_personal_content = it->second->provides_personal_content(); |
70 | + } |
71 | + it = category_to_owner_.find(cat_b); |
72 | + if (it != category_to_owner_.end() && it->second) |
73 | + { |
74 | + b_has_personal_content = it->second->provides_personal_content(); |
75 | + } |
76 | + |
77 | + // prioritize categories that have private content |
78 | + if (a_has_personal_content != b_has_personal_content) |
79 | + { |
80 | + return a_has_personal_content ? true : false; |
81 | + } |
82 | + |
83 | + return results_per_category_[cat_a] > results_per_category_[cat_b]; |
84 | + } |
85 | + |
86 | + private: |
87 | + std::map<unsigned, unsigned>& results_per_category_; |
88 | + std::map<unsigned, Lens::Ptr> const& category_to_owner_; |
89 | + }; |
90 | + |
91 | void OnLensAdded(Lens::Ptr& lens); |
92 | gsize FindLensPriority (Lens::Ptr& lens); |
93 | void EnsureCategoryAnnotation(Lens::Ptr& lens, DeeModel* results, DeeModel* categories); |
94 | Lens::Ptr FindLensForUri(std::string const& uri); |
95 | std::vector<unsigned> GetCategoriesOrder(); |
96 | - void LensSearchFinished(Lens::Ptr& lens); |
97 | + void LensSearchFinished(Lens::Ptr const& lens); |
98 | + bool ResultsContainVisibleMatch(unsigned category); |
99 | + |
100 | + std::string const& last_search_string() const { return last_search_string_; } |
101 | |
102 | HomeLens* owner_; |
103 | Lenses::LensList lenses_; |
104 | @@ -288,7 +338,11 @@ |
105 | HomeLens::CategoryMerger categories_merger_; |
106 | HomeLens::FiltersMerger filters_merger_; |
107 | int running_searches_; |
108 | + bool apps_lens_contains_visible_match; |
109 | + std::string last_search_string_; |
110 | glib::Object<GSettings> settings_; |
111 | + std::vector<unsigned> cached_categories_order_; |
112 | + std::map<unsigned, glib::Object<DeeModel> > category_filter_models_; |
113 | }; |
114 | |
115 | /* |
116 | @@ -444,15 +498,19 @@ |
117 | target_tag = FindSourceToTargetTag(model); |
118 | unsigned source_cat_offset = dee_model_get_position(model, iter); |
119 | |
120 | + Lens::Ptr owner_lens; |
121 | + for (auto it = sources_by_owner_.begin(); it != sources_by_owner_.end(); ++it) |
122 | + { |
123 | + if (it->second == model) |
124 | + { |
125 | + owner_lens = it->first; |
126 | + break; |
127 | + } |
128 | + } |
129 | + |
130 | if (merge_mode_ == MergeMode::OWNER_LENS) |
131 | { |
132 | - for (auto it = sources_by_owner_.begin(); it != sources_by_owner_.end(); ++it) |
133 | - { |
134 | - if (it->second == model) |
135 | - { |
136 | - lens_name = it->first->name(); |
137 | - } |
138 | - } |
139 | + if (owner_lens) lens_name = owner_lens->name(); |
140 | display_name = lens_name.c_str(); |
141 | } |
142 | else |
143 | @@ -489,6 +547,9 @@ |
144 | cat_registry_->RegisterCategoryOffset(results_model, source_cat_offset, |
145 | display_name); |
146 | |
147 | + if (owner_lens) category_to_owner_[target_cat_index] = owner_lens; |
148 | + |
149 | + // ensure priorities are taken into account, so default order works |
150 | gsize lens_priority = GPOINTER_TO_SIZE(g_object_get_data( |
151 | G_OBJECT(model), HOMELENS_PRIORITY)); |
152 | unsigned lens_prio = static_cast<unsigned>(lens_priority); |
153 | @@ -661,6 +722,23 @@ |
154 | return result; |
155 | } |
156 | |
157 | +std::string HomeLens::CategoryMerger::GetLensIdForCategory(unsigned cat) const |
158 | +{ |
159 | + auto lens_it = category_to_owner_.find(cat); |
160 | + if (lens_it != category_to_owner_.end()) |
161 | + { |
162 | + if (lens_it->second) return lens_it->second->id(); |
163 | + } |
164 | + |
165 | + return ""; |
166 | +} |
167 | + |
168 | +std::map<unsigned, Lens::Ptr> const& |
169 | +HomeLens::CategoryMerger::GetCategoryToLensMap() const |
170 | +{ |
171 | + return category_to_owner_; |
172 | +} |
173 | + |
174 | HomeLens::Impl::Impl(HomeLens *owner, MergeMode merge_mode) |
175 | : owner_(owner) |
176 | , cat_registry_(owner) |
177 | @@ -668,6 +746,7 @@ |
178 | , categories_merger_(owner->categories()->model(), &cat_registry_, merge_mode) |
179 | , filters_merger_(owner->filters()->model()) |
180 | , running_searches_(0) |
181 | + , apps_lens_contains_visible_match(false) |
182 | , settings_(g_settings_new("com.canonical.Unity.Dash")) |
183 | { |
184 | DeeModel* results = owner->results()->model(); |
185 | @@ -782,7 +861,7 @@ |
186 | |
187 | /* When we dispatch a search we inc the search count and when we finish |
188 | * one we decrease it. When we reach 0 we'll emit search_finished. */ |
189 | - lens->global_search_finished.connect([&] (Hints const& hints) { |
190 | + lens->global_search_finished.connect([this, lens] (Hints const& hints) { |
191 | running_searches_--; |
192 | |
193 | LensSearchFinished(lens); |
194 | @@ -852,13 +931,175 @@ |
195 | } |
196 | } |
197 | |
198 | -void HomeLens::Impl::LensSearchFinished(Lens::Ptr& lens) |
199 | -{ |
200 | +void HomeLens::Impl::LensSearchFinished(Lens::Ptr const& lens) |
201 | +{ |
202 | + auto order_vector = categories_merger_.GetDefaultOrder(); |
203 | + |
204 | + // get number of results per category |
205 | + std::map<unsigned, unsigned> results_per_cat; |
206 | + for (unsigned i = 0; i < order_vector.size(); i++) |
207 | + { |
208 | + unsigned category = order_vector.at(i); |
209 | + auto model = owner_->GetFilterModelForCategory(category); |
210 | + results_per_cat[category] = model ? dee_model_get_n_rows(model) : 0; |
211 | + } |
212 | + |
213 | + CategorySorter sorter(results_per_cat, |
214 | + categories_merger_.GetCategoryToLensMap()); |
215 | + // stable sort based on number of results in each cat |
216 | + std::stable_sort(order_vector.begin(), order_vector.end(), sorter); |
217 | + |
218 | + // ensure shopping is second, need map[cat] = lens |
219 | + int shopping_index = -1; |
220 | + int apps_index = -1; |
221 | + for (unsigned i = 0; i < order_vector.size(); i++) |
222 | + { |
223 | + // get lens that owns this category |
224 | + std::string const& lens_id(categories_merger_.GetLensIdForCategory(order_vector.at(i))); |
225 | + if (lens_id == "shopping.lens") |
226 | + shopping_index = i; |
227 | + else if (lens_id == "applications.lens") |
228 | + apps_index = i; |
229 | + } |
230 | + |
231 | + if (lens->id() == "applications.lens") |
232 | + { |
233 | + // checking the results isn't extermely fast, so cache the result |
234 | + apps_lens_contains_visible_match = ResultsContainVisibleMatch(order_vector[apps_index]); |
235 | + } |
236 | + |
237 | + // if there are no results in the apps category, we can't reorder, |
238 | + // otherwise shopping won't end up being 2nd |
239 | + if (apps_lens_contains_visible_match && apps_index > 0 && |
240 | + results_per_cat[order_vector[apps_index]] > 0) |
241 | + { |
242 | + // we want apps first |
243 | + unsigned apps_cat_num = order_vector.at(apps_index); |
244 | + order_vector.erase(order_vector.begin() + apps_index); |
245 | + order_vector.insert(order_vector.begin(), apps_cat_num); |
246 | + |
247 | + // we might shift the shopping index |
248 | + if (shopping_index >= 0 && shopping_index < apps_index) shopping_index++; |
249 | + } |
250 | + |
251 | + if (shopping_index >= 0 && shopping_index != 2) |
252 | + { |
253 | + unsigned shopping_cat_num = order_vector.at(shopping_index); |
254 | + order_vector.erase(order_vector.begin() + shopping_index); |
255 | + order_vector.insert(order_vector.begin() + 2, shopping_cat_num); |
256 | + } |
257 | + |
258 | + if (cached_categories_order_ != order_vector) |
259 | + { |
260 | + cached_categories_order_ = order_vector; |
261 | + owner_->categories_reordered(); |
262 | + } |
263 | +} |
264 | + |
265 | +bool HomeLens::Impl::ResultsContainVisibleMatch(unsigned category) |
266 | +{ |
267 | + // this method searches for match of the search string in the display name |
268 | + // or comment fields |
269 | + auto filter_model = owner_->GetFilterModelForCategory(category); |
270 | + if (!filter_model) return false; |
271 | + if (last_search_string_.empty()) return true; |
272 | + |
273 | + int checked_results = 5; |
274 | + |
275 | + glib::Object<DeeModel> model(dee_sequence_model_new()); |
276 | + dee_model_set_schema(model, "s", "s", NULL); |
277 | + |
278 | + DeeModelIter* iter = dee_model_get_first_iter(filter_model); |
279 | + DeeModelIter* end_iter = dee_model_get_last_iter(filter_model); |
280 | + |
281 | + // add first few results to the temporary model |
282 | + while (iter != end_iter) |
283 | + { |
284 | + glib::Variant name(dee_model_get_value(filter_model, iter, RESULTS_NAME_COLUMN), |
285 | + glib::StealRef()); |
286 | + glib::Variant comment(dee_model_get_value(filter_model, iter, RESULTS_COMMENT_COLUMN), |
287 | + glib::StealRef()); |
288 | + GVariant* members[2] = { name, comment }; |
289 | + dee_model_append_row(model, members); |
290 | + |
291 | + iter = dee_model_next(filter_model, iter); |
292 | + if (--checked_results <= 0) break; |
293 | + } |
294 | + |
295 | + if (dee_model_get_n_rows(model) == 0) return false; |
296 | + |
297 | + // setup model reader, analyzer and instantiate an index |
298 | + DeeModelReader reader; |
299 | + dee_model_reader_new([] (DeeModel* m, DeeModelIter* iter, gpointer data) -> gchar* |
300 | + { |
301 | + return g_strdup_printf("%s\n%s", |
302 | + dee_model_get_string(m, iter, 0), |
303 | + dee_model_get_string(m, iter, 1)); |
304 | + }, NULL, NULL, &reader); |
305 | + glib::Object<DeeAnalyzer> analyzer(DEE_ANALYZER(dee_text_analyzer_new())); |
306 | + dee_analyzer_add_term_filter(analyzer, |
307 | + [] (DeeTermList* terms_in, DeeTermList* terms_out, gpointer data) -> void |
308 | + { |
309 | + auto filter = static_cast<DeeICUTermFilter*>(data); |
310 | + for (unsigned i = 0; i < dee_term_list_num_terms(terms_in); i++) |
311 | + { |
312 | + dee_term_list_add_term(terms_out, dee_icu_term_filter_apply(filter, dee_term_list_get_term(terms_in, i))); |
313 | + } |
314 | + }, |
315 | + dee_icu_term_filter_new_ascii_folder(), |
316 | + (GDestroyNotify)dee_icu_term_filter_destroy); |
317 | + // ready to instantiate the index |
318 | + glib::Object<DeeIndex> index(DEE_INDEX(dee_tree_index_new(model, analyzer, &reader))); |
319 | + |
320 | + // tokenize the search string, so this will work with multiple words |
321 | + glib::Object<DeeTermList> search_terms(DEE_TERM_LIST(g_object_new(DEE_TYPE_TERM_LIST, NULL))); |
322 | + dee_analyzer_tokenize(analyzer, last_search_string_.c_str(), search_terms); |
323 | + |
324 | + std::set<DeeModelIter*> iters; |
325 | + for (unsigned i = 0; i < dee_term_list_num_terms(search_terms); i++) |
326 | + { |
327 | + glib::Object<DeeResultSet> results(dee_index_lookup(index, dee_term_list_get_term(search_terms, i), DEE_TERM_MATCH_PREFIX)); |
328 | + if (i == 0) |
329 | + { |
330 | + while (dee_result_set_has_next(results)) |
331 | + { |
332 | + iters.insert(dee_result_set_next(results)); |
333 | + } |
334 | + } |
335 | + else |
336 | + { |
337 | + std::set<DeeModelIter*> iters2; |
338 | + while (dee_result_set_has_next(results)) |
339 | + { |
340 | + iters2.insert(dee_result_set_next(results)); |
341 | + } |
342 | + // intersect the sets, set iterators are stable, so we can do this |
343 | + auto it = iters.begin(); |
344 | + while (it != iters.end()) |
345 | + { |
346 | + if (iters2.find(*it) == iters2.end()) |
347 | + iters.erase(it++); |
348 | + else |
349 | + ++it; |
350 | + } |
351 | + // no need to check more terms if the base set is already empty |
352 | + if (iters.empty()) break; |
353 | + } |
354 | + } |
355 | + |
356 | + // there is a match if the iterator is isn't empty |
357 | + return !iters.empty(); |
358 | } |
359 | |
360 | std::vector<unsigned> HomeLens::Impl::GetCategoriesOrder() |
361 | { |
362 | - return categories_merger_.GetDefaultOrder(); |
363 | + auto default_order = categories_merger_.GetDefaultOrder(); |
364 | + if (!last_search_string_.empty() && |
365 | + cached_categories_order_.size() == default_order.size()) |
366 | + { |
367 | + return cached_categories_order_; |
368 | + } |
369 | + return default_order; |
370 | } |
371 | |
372 | HomeLens::HomeLens(std::string const& name, |
373 | @@ -871,6 +1112,9 @@ |
374 | , pimpl(new Impl(this, merge_mode)) |
375 | { |
376 | count.SetGetterFunction(sigc::mem_fun(&pimpl->lenses_, &Lenses::LensList::size)); |
377 | + last_search_string.SetGetterFunction(sigc::mem_fun(pimpl, &HomeLens::Impl::last_search_string)); |
378 | + last_global_search_string.SetGetterFunction(sigc::mem_fun(pimpl, &HomeLens::Impl::last_search_string)); |
379 | + |
380 | search_in_global = false; |
381 | } |
382 | |
383 | @@ -933,6 +1177,8 @@ |
384 | |
385 | /* Reset running search counter */ |
386 | pimpl->running_searches_ = 0; |
387 | + pimpl->last_search_string_ = search_string; |
388 | + pimpl->apps_lens_contains_visible_match = false; |
389 | |
390 | for (auto lens: pimpl->lenses_) |
391 | { |
392 | @@ -985,5 +1231,15 @@ |
393 | return pimpl->GetCategoriesOrder(); |
394 | } |
395 | |
396 | +glib::Object<DeeModel> HomeLens::GetFilterModelForCategory(unsigned category) |
397 | +{ |
398 | + auto it = pimpl->category_filter_models_.find(category); |
399 | + if (it != pimpl->category_filter_models_.end()) return it->second; |
400 | + |
401 | + auto model = Lens::GetFilterModelForCategory(category); |
402 | + pimpl->category_filter_models_[category] = model; |
403 | + return model; |
404 | +} |
405 | + |
406 | } |
407 | } |
408 | |
409 | === modified file 'UnityCore/HomeLens.h' |
410 | --- UnityCore/HomeLens.h 2012-08-16 14:41:23 +0000 |
411 | +++ UnityCore/HomeLens.h 2012-08-31 14:43:22 +0000 |
412 | @@ -75,6 +75,7 @@ |
413 | void Preview(std::string const& uri); |
414 | |
415 | std::vector<unsigned> GetCategoriesOrder(); |
416 | + glib::Object<DeeModel> GetFilterModelForCategory(unsigned category); |
417 | |
418 | private: |
419 | class Impl; |
420 | |
421 | === modified file 'UnityCore/Lens.cpp' |
422 | --- UnityCore/Lens.cpp 2012-08-28 14:34:19 +0000 |
423 | +++ UnityCore/Lens.cpp 2012-08-31 14:43:22 +0000 |
424 | @@ -79,7 +79,8 @@ |
425 | string const& results_model_name, |
426 | string const& global_results_model_name, |
427 | string const& categories_model_name, |
428 | - string const& filters_model_name); |
429 | + string const& filters_model_name, |
430 | + GVariantIter* hints_iter); |
431 | void OnViewTypeChanged(ViewType view_type); |
432 | |
433 | void GlobalSearch(std::string const& search_string); |
434 | @@ -112,6 +113,7 @@ |
435 | Categories::Ptr const& categories() const; |
436 | Filters::Ptr const& filters() const; |
437 | bool connected() const; |
438 | + bool provides_personal_content() const; |
439 | |
440 | string const& last_search_string() const { return last_search_string_; } |
441 | string const& last_global_search_string() const { return last_global_search_string_; } |
442 | @@ -133,6 +135,7 @@ |
443 | Categories::Ptr categories_; |
444 | Filters::Ptr filters_; |
445 | bool connected_; |
446 | + bool provides_personal_content_; |
447 | |
448 | string private_connection_name_; |
449 | string last_search_string_; |
450 | @@ -174,6 +177,7 @@ |
451 | , categories_(new Categories(model_type)) |
452 | , filters_(new Filters(model_type)) |
453 | , connected_(false) |
454 | + , provides_personal_content_(false) |
455 | , proxy_(NULL) |
456 | { |
457 | if (model_type == ModelType::REMOTE) |
458 | @@ -206,6 +210,7 @@ |
459 | owner_->categories.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::categories)); |
460 | owner_->filters.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::filters)); |
461 | owner_->connected.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::connected)); |
462 | + owner_->provides_personal_content.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::provides_personal_content)); |
463 | owner_->last_search_string.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::last_search_string)); |
464 | owner_->last_global_search_string.SetGetterFunction(sigc::mem_fun(this, &Lens::Impl::last_global_search_string)); |
465 | owner_->view_type.changed.connect(sigc::mem_fun(this, &Lens::Impl::OnViewTypeChanged)); |
466 | @@ -389,7 +394,8 @@ |
467 | results_model_swarm_name, |
468 | global_results_model_swarm_name, |
469 | categories_model_swarm_name, |
470 | - filters_model_swarm_name); |
471 | + filters_model_swarm_name, |
472 | + hints_iter); |
473 | |
474 | if (models_changed) owner_->models_changed.emit(); |
475 | } |
476 | @@ -414,7 +420,8 @@ |
477 | string const& results_model_name, |
478 | string const& global_results_model_name, |
479 | string const& categories_model_name, |
480 | - string const& filters_model_name) |
481 | + string const& filters_model_name, |
482 | + GVariantIter* hints_iter) |
483 | { |
484 | // Diff the properties received from those we have |
485 | if (search_hint_ != search_hint) |
486 | @@ -435,6 +442,26 @@ |
487 | owner_->visible.EmitChanged(visible_); |
488 | } |
489 | |
490 | + bool provides_personal_content = false; |
491 | + gchar* key; |
492 | + GVariant* value; |
493 | + |
494 | + // g_variant_iter_loop manages the memory automatically, as long |
495 | + // as the iteration is not stopped in the middle |
496 | + while (g_variant_iter_loop(hints_iter, "{sv}", &key, &value)) |
497 | + { |
498 | + if (g_strcmp0(key, "provides-personal-content") == 0) |
499 | + { |
500 | + provides_personal_content = g_variant_get_boolean(value) != FALSE; |
501 | + } |
502 | + } |
503 | + |
504 | + if (provides_personal_content_ != provides_personal_content) |
505 | + { |
506 | + provides_personal_content_ = provides_personal_content; |
507 | + owner_->provides_personal_content.EmitChanged(provides_personal_content_); |
508 | + } |
509 | + |
510 | if (private_connection_name_ != private_connection_name) |
511 | { |
512 | // FIXME: Update all the models as they are no longer valid when we use this |
513 | @@ -772,6 +799,11 @@ |
514 | return connected_; |
515 | } |
516 | |
517 | +bool Lens::Impl::provides_personal_content() const |
518 | +{ |
519 | + return provides_personal_content_; |
520 | +} |
521 | + |
522 | Lens::Lens(string const& id_, |
523 | string const& dbus_name_, |
524 | string const& dbus_path_, |
525 | |
526 | === modified file 'UnityCore/Lens.h' |
527 | --- UnityCore/Lens.h 2012-08-28 14:34:19 +0000 |
528 | +++ UnityCore/Lens.h 2012-08-31 14:43:22 +0000 |
529 | @@ -114,6 +114,7 @@ |
530 | nux::RWProperty<Categories::Ptr> categories; |
531 | nux::RWProperty<Filters::Ptr> filters; |
532 | nux::RWProperty<bool> connected; |
533 | + nux::RWProperty<bool> provides_personal_content; |
534 | nux::ROProperty<std::string> last_search_string; |
535 | nux::ROProperty<std::string> last_global_search_string; |
536 | |
537 | |
538 | === modified file 'tests/test_home_lens.cpp' |
539 | --- tests/test_home_lens.cpp 2012-08-16 14:41:23 +0000 |
540 | +++ tests/test_home_lens.cpp 2012-08-31 14:43:22 +0000 |
541 | @@ -18,6 +18,7 @@ |
542 | #include "test_utils.h" |
543 | |
544 | using namespace std; |
545 | +using namespace unity; |
546 | using namespace unity::dash; |
547 | |
548 | namespace |
549 | @@ -61,6 +62,7 @@ |
550 | : Lens(id, "", "", name, "lens-icon.png", |
551 | description, search_hint, true, "", |
552 | ModelType::LOCAL) |
553 | + , num_results_(-1) |
554 | { |
555 | search_in_global(true); |
556 | connected.SetGetterFunction(sigc::mem_fun(this, &StaticTestLens::force_connected)); |
557 | @@ -101,28 +103,37 @@ |
558 | |
559 | row_buf[1] = g_variant_new_string(""); |
560 | row_buf[3] = g_variant_new_string(""); |
561 | - row_buf[4] = g_variant_new_string(""); |
562 | row_buf[5] = g_variant_new_string(""); |
563 | row_buf[6] = g_variant_new_string(""); |
564 | row_buf[7] = NULL; |
565 | |
566 | unsigned int i; |
567 | - for (i = 0; i < search_string.size(); i++) |
568 | + unsigned int results_count = search_string.size(); |
569 | + if (num_results_ >= 0) results_count = static_cast<unsigned>(num_results_); |
570 | + for (i = 0; i < results_count; i++) |
571 | { |
572 | ostringstream uri; |
573 | - uri << "uri+" << search_string.at(i) << "+" << id(); |
574 | + char res_id(i >= search_string.size() ? '-' : search_string.at(i)); |
575 | + uri << "uri+" << res_id << "+" << id(); |
576 | row_buf[0] = g_variant_new_string(uri.str().c_str()); |
577 | row_buf[2] = g_variant_new_uint32(i % 3); |
578 | + glib::String name(g_strdup_printf("%s - %d", |
579 | + results_base_name_.empty() ? |
580 | + search_string.c_str() : results_base_name_.c_str(), |
581 | + i)); |
582 | + row_buf[4] = g_variant_new_string(name); |
583 | |
584 | dee_model_append_row(model, row_buf); |
585 | } |
586 | |
587 | g_free(row_buf); |
588 | + |
589 | + global_search_finished.emit(Hints()); |
590 | } |
591 | |
592 | void GlobalSearch(string const& search_string) |
593 | { |
594 | - /* Dispatch search async, because that's */ |
595 | + /* Dispatch search async, because that's what it'd normally do */ |
596 | LensSearchClosure* closure = g_new0(LensSearchClosure, 1); |
597 | closure->lens = this; |
598 | closure->search_string = g_strdup(search_string.c_str()); |
599 | @@ -144,6 +155,19 @@ |
600 | |
601 | } |
602 | |
603 | + void SetResultsBaseName(string const& name) |
604 | + { |
605 | + results_base_name_ = name; |
606 | + } |
607 | + |
608 | + void SetNumResults(int count) |
609 | + { |
610 | + num_results_ = count; |
611 | + } |
612 | + |
613 | +private: |
614 | + string results_base_name_; |
615 | + int num_results_; |
616 | }; |
617 | |
618 | static gboolean dispatch_global_search(gpointer userdata) |
619 | @@ -213,6 +237,25 @@ |
620 | Lens::Ptr lens_2_; |
621 | }; |
622 | |
623 | +class ThreeStaticTestLenses : public StaticTestLenses |
624 | +{ |
625 | +public: |
626 | + ThreeStaticTestLenses() |
627 | + : lens_1_(new StaticTestLens("first.lens", "First Lens", "The very first lens", "First search hint")) |
628 | + , lens_2_(new StaticTestLens("second.lens", "Second Lens", "The second lens", "Second search hint")) |
629 | + , lens_3_(new StaticTestLens("applications.lens", "Applications", "The applications lens", "Search applications")) |
630 | + { |
631 | + list_.push_back(lens_1_); |
632 | + list_.push_back(lens_2_); |
633 | + list_.push_back(lens_3_); |
634 | + } |
635 | + |
636 | +private: |
637 | + Lens::Ptr lens_1_; |
638 | + Lens::Ptr lens_2_; |
639 | + Lens::Ptr lens_3_; |
640 | +}; |
641 | + |
642 | TEST(TestHomeLens, TestConstruction) |
643 | { |
644 | HomeLens home_lens_("name", "description", "searchhint"); |
645 | @@ -354,7 +397,13 @@ |
646 | |
647 | home_lens_.Search("ape"); |
648 | |
649 | - Utils::WaitForTimeoutMSec(); |
650 | + bool finished = false; |
651 | + home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints) |
652 | + { |
653 | + finished = true; |
654 | + }); |
655 | + |
656 | + Utils::WaitUntil(finished); |
657 | |
658 | /* Validate counts */ |
659 | EXPECT_EQ(dee_model_get_n_rows(results), 6); // 3 hits from each lens |
660 | @@ -388,4 +437,210 @@ |
661 | EXPECT_EQ(cat_shared, dee_model_get_uint32(results, iter, CAT_COLUMN)); |
662 | } |
663 | |
664 | +TEST(TestHomeLens, TestOrderingAfterSearch) |
665 | +{ |
666 | + HomeLens home_lens_("name", "description", "searchhint", |
667 | + HomeLens::MergeMode::OWNER_LENS); |
668 | + ThreeStaticTestLenses lenses_; |
669 | + DeeModel* results = home_lens_.results()->model(); |
670 | + DeeModel* cats = home_lens_.categories()->model(); |
671 | + DeeModel* filters = home_lens_.filters()->model(); |
672 | + DeeModelIter* iter; |
673 | + unsigned int lens1_cat = 0; |
674 | + // the lens is added as third, so must have cat == 2 |
675 | + unsigned int apps_lens_cat = 2; |
676 | + const unsigned int URI_COLUMN = 0; |
677 | + const unsigned int CAT_COLUMN = 2; |
678 | + |
679 | + home_lens_.AddLenses(lenses_); |
680 | + |
681 | + bool order_changed = false; |
682 | + home_lens_.categories_reordered.connect([&order_changed] () |
683 | + { |
684 | + order_changed = true; |
685 | + }); |
686 | + |
687 | + home_lens_.Search("ape"); |
688 | + |
689 | + bool finished = false; |
690 | + home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints) |
691 | + { |
692 | + finished = true; |
693 | + }); |
694 | + Utils::WaitUntil(finished); |
695 | + |
696 | + /* Validate counts */ |
697 | + EXPECT_EQ(dee_model_get_n_rows(results), 9); // 3 hits from each lens |
698 | + EXPECT_EQ(dee_model_get_n_rows(cats), 3); // 3 cats since we are merging categories by lens |
699 | + EXPECT_EQ(dee_model_get_n_rows(filters), 0); // We ignore filters deliberately currently |
700 | + |
701 | + /* Validate the category order */ |
702 | + auto order = home_lens_.GetCategoriesOrder(); |
703 | + |
704 | + /* The home lens includes applications lens which contains exact match, must |
705 | + * be first category */ |
706 | + EXPECT_EQ(order.at(0), apps_lens_cat); |
707 | + |
708 | + /* Plus the categories reordered should have been emitted */ |
709 | + EXPECT_EQ(order_changed, true); |
710 | + |
711 | + /* The model will not be sorted acording to the categories though. */ |
712 | + iter = dee_model_get_iter_at_row(results, 0); |
713 | + EXPECT_EQ(string("uri+a+first.lens"), string(dee_model_get_string(results, iter, URI_COLUMN))); |
714 | + EXPECT_EQ( dee_model_get_uint32(results, iter, CAT_COLUMN), lens1_cat); |
715 | +} |
716 | + |
717 | +TEST(TestHomeLens, TestOrderingWithExactAppsMatch) |
718 | +{ |
719 | + HomeLens home_lens_("name", "description", "searchhint", |
720 | + HomeLens::MergeMode::OWNER_LENS); |
721 | + ThreeStaticTestLenses lenses_; |
722 | + // the lens is added as third, so must have cat == 2 |
723 | + unsigned int apps_lens_cat = 2; |
724 | + |
725 | + home_lens_.AddLenses(lenses_); |
726 | + Lens::Ptr apps_lens = lenses_.GetLens("applications.lens"); |
727 | + |
728 | + auto static_lens = dynamic_pointer_cast<StaticTestLens>(apps_lens); |
729 | + static_lens->SetNumResults(1); |
730 | + |
731 | + bool order_changed = false; |
732 | + home_lens_.categories_reordered.connect([&order_changed] () |
733 | + { |
734 | + order_changed = true; |
735 | + }); |
736 | + |
737 | + home_lens_.Search("ape"); |
738 | + |
739 | + bool finished = false; |
740 | + home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints) |
741 | + { |
742 | + finished = true; |
743 | + }); |
744 | + Utils::WaitUntil(finished); |
745 | + |
746 | + /* Validate counts */ |
747 | + EXPECT_EQ(home_lens_.results()->count(), 7); // 3+3+1 hits |
748 | + EXPECT_EQ(home_lens_.categories()->count(), 3); // 3 cats since we are merging categories by lens |
749 | + EXPECT_EQ(home_lens_.filters()->count(), 0); // We ignore filters deliberately currently |
750 | + |
751 | + /* Validate the category order */ |
752 | + auto order = home_lens_.GetCategoriesOrder(); |
753 | + |
754 | + /* The home lens includes applications lens and it contains exact match, |
755 | + * so must be the first category, even though there are fewer results than |
756 | + * in the other categories */ |
757 | + EXPECT_EQ(order.at(0), apps_lens_cat); |
758 | + |
759 | + /* Plus the categories reordered should have been emitted */ |
760 | + EXPECT_EQ(order_changed, true); |
761 | +} |
762 | + |
763 | +TEST(TestHomeLens, TestOrderingWithoutExactAppsMatch) |
764 | +{ |
765 | + HomeLens home_lens_("name", "description", "searchhint", |
766 | + HomeLens::MergeMode::OWNER_LENS); |
767 | + ThreeStaticTestLenses lenses_; |
768 | + // the lens is added as third, so must have cat == 2 |
769 | + unsigned int apps_lens_cat = 2; |
770 | + |
771 | + home_lens_.AddLenses(lenses_); |
772 | + Lens::Ptr apps_lens = lenses_.GetLens("applications.lens"); |
773 | + |
774 | + auto static_lens = dynamic_pointer_cast<StaticTestLens>(apps_lens); |
775 | + static_lens->SetResultsBaseName("noapes"); |
776 | + static_lens->SetNumResults(1); |
777 | + |
778 | + bool order_changed = false; |
779 | + home_lens_.categories_reordered.connect([&order_changed] () |
780 | + { |
781 | + order_changed = true; |
782 | + }); |
783 | + |
784 | + home_lens_.Search("ape"); |
785 | + |
786 | + bool finished = false; |
787 | + home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints) |
788 | + { |
789 | + finished = true; |
790 | + }); |
791 | + Utils::WaitUntil(finished); |
792 | + |
793 | + /* Validate counts */ |
794 | + EXPECT_EQ(home_lens_.results()->count(), 7); // 3+3+1 hits |
795 | + EXPECT_EQ(home_lens_.categories()->count(), 3); // 3 cats since we are merging categories by lens |
796 | + EXPECT_EQ(home_lens_.filters()->count(), 0); // We ignore filters deliberately currently |
797 | + |
798 | + /* Validate the category order */ |
799 | + auto order = home_lens_.GetCategoriesOrder(); |
800 | + |
801 | + /* The home lens includes applications lens but it doesn't contain exact |
802 | + * match, so can't be the first category */ |
803 | + EXPECT_NE(order.at(0), apps_lens_cat); |
804 | + |
805 | + /* Plus the categories reordered should have been emitted */ |
806 | + EXPECT_EQ(order_changed, true); |
807 | +} |
808 | + |
809 | +TEST(TestHomeLens, TestOrderingByNumResults) |
810 | +{ |
811 | + HomeLens home_lens_("name", "description", "searchhint", |
812 | + HomeLens::MergeMode::OWNER_LENS); |
813 | + ThreeStaticTestLenses lenses_; |
814 | + unsigned int lens1_cat = 0; |
815 | + unsigned int lens2_cat = 1; |
816 | + // the lens is added as third, so must have cat == 2 |
817 | + unsigned int apps_lens_cat = 2; |
818 | + |
819 | + home_lens_.AddLenses(lenses_); |
820 | + |
821 | + Lens::Ptr lens = lenses_.GetLensAtIndex(2); |
822 | + auto static_lens = dynamic_pointer_cast<StaticTestLens>(lens); |
823 | + static_lens->SetResultsBaseName("noapes"); // no exact match in apps lens |
824 | + static_lens->SetNumResults(2); |
825 | + |
826 | + static_lens = dynamic_pointer_cast<StaticTestLens>(lenses_.GetLensAtIndex(0)); |
827 | + static_lens->SetNumResults(1); |
828 | + static_lens = dynamic_pointer_cast<StaticTestLens>(lenses_.GetLensAtIndex(1)); |
829 | + static_lens->SetNumResults(3); |
830 | + |
831 | + bool order_changed = false; |
832 | + home_lens_.categories_reordered.connect([&order_changed] () |
833 | + { |
834 | + order_changed = true; |
835 | + }); |
836 | + |
837 | + home_lens_.Search("ape"); |
838 | + |
839 | + bool finished = false; |
840 | + home_lens_.search_finished.connect([&finished] (Lens::Hints const& hints) |
841 | + { |
842 | + finished = true; |
843 | + }); |
844 | + Utils::WaitUntil(finished); |
845 | + |
846 | + /* |
847 | + * lens1 -> 1 result |
848 | + * lens2 -> 3 results |
849 | + * lens3 -> 2 results (apps.lens) |
850 | + */ |
851 | + |
852 | + /* Validate counts */ |
853 | + EXPECT_EQ(home_lens_.results()->count(), 6); // 1+3+2 hits |
854 | + EXPECT_EQ(home_lens_.categories()->count(), 3); // 3 cats since we are merging categories by lens |
855 | + EXPECT_EQ(home_lens_.filters()->count(), 0); // We ignore filters deliberately currently |
856 | + |
857 | + /* Validate the category order */ |
858 | + auto order = home_lens_.GetCategoriesOrder(); |
859 | + |
860 | + /* The home lens includes applications lens but it doesn't contain exact |
861 | + * match, so can't be the first category */ |
862 | + EXPECT_EQ(order.at(0), lens2_cat); |
863 | + EXPECT_EQ(order.at(1), apps_lens_cat); |
864 | + EXPECT_EQ(order.at(2), lens1_cat); |
865 | + |
866 | + /* Plus the categories reordered should have been emitted */ |
867 | + EXPECT_EQ(order_changed, true); |
868 | +} |
869 | + |
870 | } |
The code looks good, looking forward for a test as agreed on IRC.