unity-scopes-api

Merge lp:unity-scopes-api/staging into lp:unity-scopes-api

  1. staging
  2. Merge into trunk
Proposed by Paweł Stołowski
Status: Merged
Approved by: Michi Henning
Approved revision: 285
Merged at revision: 328
Proposed branch: lp:unity-scopes-api/staging
Merge into: lp:unity-scopes-api
Diff against target: 123 lines (+26/-10)
5 files modified
debian/libunity-scopes3.symbols (+2/-2)
include/unity/scopes/internal/SearchQueryBaseImpl.h (+4/-2)
src/scopes/internal/ScopeImpl.cpp (+1/-0)
src/scopes/internal/ScopeObject.cpp (+2/-1)
src/scopes/internal/SearchQueryBaseImpl.cpp (+17/-5)
To merge this branch: bzr merge lp:unity-scopes-api/staging
Reviewer Review Type Date Requested Status
Unity Team Pending
Review via email: mp+260101@code.launchpad.net

Commit message

Fix for over-zealous loop detection. A loop is now detected only if
a query loops back with the exact same details, that is, query string,
department, filter state, user data, and metadata must all be the same. Fixes: https://bugs.launchpad.net/bugs/1457698 (cherry-picked from devel).

Description of the change

Fix for over-zealous loop detection. A loop is now detected only if
a query loops back with the exact same details, that is, query string,
department, filter state, user data, and metadata must all be the same. Fixes: https://bugs.launchpad.net/bugs/1457698. (cherry-picked from devel).

To post a comment you must log in.
lp:unity-scopes-api/staging updated
285. By Paweł Stołowski

Fix for over-zealous loop detection. A loop is now detected only if
a query loops back with the exact same details, that is, query string,
department, filter state, user data, and metadata must all be the same. Fixes: https://bugs.launchpad.net/bugs/1457698.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/libunity-scopes3.symbols'
2--- debian/libunity-scopes3.symbols 2015-04-23 16:02:37 +0000
3+++ debian/libunity-scopes3.symbols 2015-05-26 08:23:40 +0000
4@@ -516,12 +516,12 @@
5 (c++)"unity::scopes::internal::ScopeImpl::runtime() const@Base" 0.6.12+15.04.20150127.2
6 (c++)"unity::scopes::internal::ScopeImpl::~ScopeImpl()@Base" 0.4.0+14.04.20140312.1
7 (c++)"unity::scopes::internal::ScopeImpl::ScopeImpl(std::shared_ptr<unity::scopes::internal::MWScope> const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.6.11+15.04.20150119
8- (c++)"unity::scopes::internal::ScopeImpl::search(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unity::scopes::FilterState const&, std::unique_ptr<unity::scopes::Variant, std::default_delete<unity::scopes::Variant> >, unity::scopes::SearchMetadata const&, std::vector<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0.6.16+15.04.20150410.3
9+ (c++)"unity::scopes::internal::ScopeImpl::search(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unity::scopes::FilterState const&, std::unique_ptr<unity::scopes::Variant, std::default_delete<unity::scopes::Variant> >, unity::scopes::SearchMetadata const&, std::vector<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0replaceme
10 (c++)"unity::scopes::internal::ScopeImpl::search(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unity::scopes::FilterState const&, unity::scopes::SearchMetadata const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0.4.0+14.04.20140312.1
11 (c++)"unity::scopes::internal::ScopeImpl::search(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unity::scopes::FilterState const&, unity::scopes::Variant const&, unity::scopes::SearchMetadata const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0.6.16+15.04.20150410.3
12 (c++)"unity::scopes::internal::ScopeImpl::search(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unity::scopes::FilterState const&, unity::scopes::SearchMetadata const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0.4.0+14.04.20140312.1
13 (c++)"unity::scopes::internal::ScopeImpl::search(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unity::scopes::SearchMetadata const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0.4.0+14.04.20140312.1
14- (c++)"unity::scopes::internal::ScopeImpl::search(unity::scopes::CannedQuery const&, unity::scopes::SearchMetadata const&, std::vector<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0.6.12+15.04.20150127.2
15+ (c++)"unity::scopes::internal::ScopeImpl::search(unity::scopes::CannedQuery const&, unity::scopes::SearchMetadata const&, std::vector<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::tuple<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::shared_ptr<unity::scopes::SearchListenerBase> const&)@Base" 0replaceme
16 (c++)"unity::scopes::internal::ScopeImpl::set_child_scopes(std::vector<unity::scopes::ChildScope, std::allocator<unity::scopes::ChildScope> > const&)@Base" 0.6.15+15.04.20150407
17 (c++)"unity::scopes::internal::ScopeLoader::libpath() const@Base" 0.4.0+14.04.20140312.1
18 (c++)"unity::scopes::internal::ScopeLoader::load(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.6.0+14.10.20140804.1
19
20=== modified file 'include/unity/scopes/internal/SearchQueryBaseImpl.h'
21--- include/unity/scopes/internal/SearchQueryBaseImpl.h 2015-04-09 15:59:17 +0000
22+++ include/unity/scopes/internal/SearchQueryBaseImpl.h 2015-05-26 08:23:40 +0000
23@@ -48,7 +48,7 @@
24
25 void set_client_id(const std::string& id);
26
27- typedef std::tuple<std::string, std::string, std::string> HistoryData;
28+ typedef std::tuple<std::string, std::string, std::string, std::string> HistoryData;
29 typedef std::vector<HistoryData> History;
30
31 void set_history(History const& h);
32@@ -78,7 +78,9 @@
33 History history_;
34 std::vector<QueryCtrlProxy> subqueries_;
35
36- QueryCtrlProxy check_for_query_loop(ScopeProxy const& scope, SearchListenerBase::SPtr const& reply);
37+ QueryCtrlProxy check_for_query_loop(ScopeProxy const& scope,
38+ SearchListenerBase::SPtr const& reply,
39+ VariantMap const& details);
40 };
41
42 } // namespace internal
43
44=== modified file 'src/scopes/internal/ScopeImpl.cpp'
45--- src/scopes/internal/ScopeImpl.cpp 2015-04-09 15:59:17 +0000
46+++ src/scopes/internal/ScopeImpl.cpp 2015-05-26 08:23:40 +0000
47@@ -157,6 +157,7 @@
48 d["c"] = get<0>(tuple); // Client
49 d["a"] = get<1>(tuple); // Aggregator
50 d["r"] = get<2>(tuple); // Receiver
51+ d["d"] = get<3>(tuple); // Details (query string, department, filter state, user data, metadata)
52 hist.push_back(Variant(d));
53 }
54 context["history"] = Variant(hist);
55
56=== modified file 'src/scopes/internal/ScopeObject.cpp'
57--- src/scopes/internal/ScopeObject.cpp 2015-04-01 12:31:57 +0000
58+++ src/scopes/internal/ScopeObject.cpp 2015-05-26 08:23:40 +0000
59@@ -184,7 +184,8 @@
60 string client_id = t.get_dict()["c"].get_string();
61 string agg = t.get_dict()["a"].get_string();
62 string recv = t.get_dict()["r"].get_string();
63- SearchQueryBaseImpl::HistoryData hd = make_tuple(client_id, agg, recv);
64+ string details = t.get_dict()["d"].get_string();
65+ SearchQueryBaseImpl::HistoryData hd = make_tuple(client_id, agg, recv, details);
66 history.push_back(hd);
67 }
68 sqb->set_history(history);
69
70=== modified file 'src/scopes/internal/SearchQueryBaseImpl.cpp'
71--- src/scopes/internal/SearchQueryBaseImpl.cpp 2015-04-09 23:41:21 +0000
72+++ src/scopes/internal/SearchQueryBaseImpl.cpp 2015-05-26 08:23:40 +0000
73@@ -80,7 +80,13 @@
74 throw InvalidArgumentException("QueryBase::subsearch(): reply cannot be nullptr");
75 }
76
77- auto query_ctrl = check_for_query_loop(scope, reply);
78+ VariantMap details;
79+ details["query_string"] = query_string;
80+ details["department_id"] = department_id;
81+ details["filter_state"] = filter_state.serialize();
82+ details["user_data"] = user_data ? *user_data : Variant();
83+ details["metadata"] = metadata.serialize();
84+ auto query_ctrl = check_for_query_loop(scope, reply, move(details));
85 if (query_ctrl)
86 {
87 return query_ctrl; // Loop was detected, return dummy QueryCtrlProxy.
88@@ -179,17 +185,22 @@
89 // It is possible for an aggregator to send a query to itself, but only once.
90
91 QueryCtrlProxy SearchQueryBaseImpl::check_for_query_loop(ScopeProxy const& scope,
92- SearchListenerBase::SPtr const& reply)
93+ SearchListenerBase::SPtr const& reply,
94+ VariantMap const& details)
95 {
96 shared_ptr<QueryCtrlImpl> ctrl_proxy;
97
98 lock_guard<mutex> lock(mutex_);
99
100- HistoryData tuple = make_tuple(client_id_, canned_query_.scope_id(), scope->identity());
101+ HistoryData tuple = make_tuple(client_id_,
102+ canned_query_.scope_id(),
103+ scope->identity(),
104+ Variant(details).serialize_json());
105 auto it = find(history_.begin(), history_.end(), tuple);
106 if (it != history_.end())
107 {
108- // Query has been here before from the same client and with the same child scope as the target.
109+ // Query has been here before from the same client and with the same child scope as the target,
110+ // and with the same query details (query, department, filter state, user data, and metadata).
111 reply->finished(CompletionDetails(CompletionDetails::OK,
112 "empty result set due to aggregator loop or repeated query on aggregating scope "
113 + get<1>(tuple)));
114@@ -204,7 +215,8 @@
115 << "query loop for query \"" << canned_query_.query_string()
116 << "\", client: " << get<0>(tuple)
117 << ", aggregator: " << get<1>(tuple)
118- << ", receiver: " << get<2>(tuple) << endl;
119+ << ", receiver: " << get<2>(tuple)
120+ << ", details: " << get<3>(tuple) << endl;
121 }
122 }
123 else

Subscribers

People subscribed via source and target branches

to all changes: