Merge lp:~stolowski/unity-scopes-api/user-agent into lp:unity-scopes-api/devel

Proposed by Paweł Stołowski
Status: Superseded
Proposed branch: lp:~stolowski/unity-scopes-api/user-agent
Merge into: lp:unity-scopes-api/devel
Diff against target: 396 lines (+114/-18)
12 files modified
STRUCTS (+19/-0)
debian/libunity-scopes3.symbols (+1/-1)
include/unity/scopes/internal/smartscopes/HttpClientInterface.h (+6/-1)
include/unity/scopes/internal/smartscopes/HttpClientQt.h (+4/-2)
include/unity/scopes/internal/smartscopes/HttpClientQtThread.h (+4/-1)
include/unity/scopes/internal/smartscopes/SmartScopesClient.h (+3/-1)
src/scopes/internal/smartscopes/HttpClientQt.cpp (+7/-5)
src/scopes/internal/smartscopes/HttpClientQtThread.cpp (+6/-1)
src/scopes/internal/smartscopes/SmartScope.cpp (+16/-3)
src/scopes/internal/smartscopes/SmartScopesClient.cpp (+18/-2)
test/gtest/scopes/internal/smartscopes/SmartScopesClient/FakeSss.py (+3/-0)
test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp (+27/-1)
To merge this branch: bzr merge lp:~stolowski/unity-scopes-api/user-agent
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michi Henning (community) Needs Fixing
Pete Woods (community) Approve
Review via email: mp+235765@code.launchpad.net

This proposal has been superseded by a proposal from 2014-09-25.

Commit message

Pass User-Agent http header to SSS if recieved with SearchMetadata or PreviewMetadata.

Description of the change

Pass User-Agent http header to SSS if recieved with SearchMetadata or PreviewMetadata.

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
Pete Woods (pete-woods) wrote :

LGTM

review: Approve
Revision history for this message
Pete Woods (pete-woods) wrote :

Apart from the symbols file stuff. This could be another example of the compiler producing amd64-only symbols.

i.e. (c++|arch=amd64) should be used as the symbol tag

Revision history for this message
Michi Henning (michihenning) wrote :

BIG problem here: there is a redundant semicolon ;-)

247 - }
248 + };

I think the user-agent attribute should be documented in the STRUCTS file?

Other than that, this looks good!

review: Needs Fixing
259. By Paweł Stołowski

Removed semicolon.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
260. By Paweł Stołowski

Merged lp:~michihenning/unity-scopes-api/fix-amd64-symbol

261. By Paweł Stołowski

Updated STRUCTS.

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

> BIG problem here: there is a redundant semicolon ;-)
>
> 247 - }
> 248 + };
>
> I think the user-agent attribute should be documented in the STRUCTS file?
>

Right, I noticed we were missing descriptions of the 3 *metadata structures that. I fixed that, however this particular value as well as e.g. session-id are transported using general hints container, and we don't really care what's there in the API, so these are not documented.

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

I've merged Michi's symbols fix into this branch and am trying to resubmit it with new prerequisite branch, but LP gives me OOPses all the time. Setting to WIP till it succeeds.

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

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'STRUCTS'
2--- STRUCTS 2014-06-26 05:59:38 +0000
3+++ STRUCTS 2014-09-25 08:34:38 +0000
4@@ -154,3 +154,22 @@
5 ============================================
6 'status' : int (corresponds to ActivationResponse::Status enum)
7 'scope_data' : variant
8+
9+QueryMetadata (returned by serialize())
10+=======================================
11+ 'type' : string (metadata type)
12+ 'locale' : string
13+ 'form_factor' : string
14+ 'hints' : dict
15+ 'internet_connectivity' : bool (optional)
16+
17+SearchMetadata (returned by serialize())
18+========================================
19+ all QueryMetadata attributes
20+ 'cardinality' : int
21+ 'location' : dict
22+
23+ActionMetadata (returned by serialize())
24+========================================
25+ all QueryMetadata attributes
26+ 'scope_data' : variant
27
28=== modified file 'debian/libunity-scopes3.symbols'
29--- debian/libunity-scopes3.symbols 2014-09-23 08:33:51 +0000
30+++ debian/libunity-scopes3.symbols 2014-09-25 08:34:38 +0000
31@@ -377,7 +377,7 @@
32 (c++)"unity::scopes::internal::RegistryObject::remove_desktop_file(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.6.6+14.10.20140916
33 (c++)"unity::scopes::internal::RegistryObject::remove_local_scope(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.4.0+14.04.20140312.1
34 (c++)"unity::scopes::internal::RegistryObject::ScopeExecData::~ScopeExecData()@Base" 0.4.2+14.04.20140404.2
35- (c++)"unity::scopes::internal::RegistryObject::ScopeExecData::ScopeExecData(unity::scopes::internal::RegistryObject::ScopeExecData const&)@Base" 0.6.6+14.10.20140916
36+ (c++|arch=amd64)"unity::scopes::internal::RegistryObject::ScopeExecData::ScopeExecData(unity::scopes::internal::RegistryObject::ScopeExecData const&)@Base" 0.6.6+14.10.20140916
37 (c++)"unity::scopes::internal::RegistryObject::ScopeProcess::clear_handle_unlocked()@Base" 0.4.2+14.04.20140404.2
38 (c++)"unity::scopes::internal::RegistryObject::ScopeProcess::exec(core::posix::ChildProcess::DeathObserver&, std::shared_ptr<unity::scopes::internal::Executor>)@Base" 0.4.3+14.10.20140428
39 (c++)"unity::scopes::internal::RegistryObject::ScopeProcess::expand_custom_exec()@Base" 0.5.0+14.10.20140619
40
41=== modified file 'include/unity/scopes/internal/smartscopes/HttpClientInterface.h'
42--- include/unity/scopes/internal/smartscopes/HttpClientInterface.h 2014-08-28 15:27:19 +0000
43+++ include/unity/scopes/internal/smartscopes/HttpClientInterface.h 2014-09-25 08:34:38 +0000
44@@ -25,6 +25,8 @@
45 #include <future>
46 #include <string>
47 #include <functional>
48+#include <list>
49+#include <tuple>
50
51 namespace unity
52 {
53@@ -40,6 +42,8 @@
54
55 class HttpResponseHandle;
56
57+typedef std::list<std::pair<std::string, std::string>> HttpHeaders;
58+
59 class HttpClientInterface : public std::enable_shared_from_this<HttpClientInterface>
60 {
61 public:
62@@ -50,7 +54,8 @@
63 virtual ~HttpClientInterface() = default;
64
65 virtual std::shared_ptr<HttpResponseHandle> get(std::string const& request_url,
66- std::function<void(std::string const&)> const& lineData = [](std::string const&) {}) = 0;
67+ std::function<void(std::string const&)> const& lineData = [](std::string const&) {},
68+ HttpHeaders const& headers = HttpHeaders()) = 0;
69
70 virtual std::string to_percent_encoding(std::string const& string) = 0;
71
72
73=== modified file 'include/unity/scopes/internal/smartscopes/HttpClientQt.h'
74--- include/unity/scopes/internal/smartscopes/HttpClientQt.h 2014-09-02 11:58:04 +0000
75+++ include/unity/scopes/internal/smartscopes/HttpClientQt.h 2014-09-25 08:34:38 +0000
76@@ -44,7 +44,8 @@
77 explicit HttpClientQt(unsigned int no_reply_timeout);
78 ~HttpClientQt();
79
80- HttpResponseHandle::SPtr get(std::string const& request_url, std::function<void(std::string const&)> const& lineData = [](std::string const&) {}) override;
81+ HttpResponseHandle::SPtr get(std::string const& request_url, std::function<void(std::string const&)> const& lineData = [](std::string const&) {},
82+ HttpHeaders const& headers = HttpHeaders()) override;
83
84 std::string to_percent_encoding(std::string const& string) override;
85
86@@ -55,7 +56,8 @@
87 class HttpSession
88 {
89 public:
90- HttpSession(std::string const& request_url, unsigned int timeout, std::function<void(std::string const&)> const& lineData);
91+ HttpSession(std::string const& request_url, unsigned int timeout, std::function<void(std::string const&)> const& lineData, HttpHeaders const& headers =
92+ HttpHeaders());
93 ~HttpSession();
94
95 std::future<void> get_future();
96
97=== modified file 'include/unity/scopes/internal/smartscopes/HttpClientQtThread.h'
98--- include/unity/scopes/internal/smartscopes/HttpClientQtThread.h 2014-08-26 12:17:18 +0000
99+++ include/unity/scopes/internal/smartscopes/HttpClientQtThread.h 2014-09-25 08:34:38 +0000
100@@ -36,6 +36,7 @@
101 #include <QUrl>
102 #endif
103
104+#include <unity/scopes/internal/smartscopes/HttpClientInterface.h>
105 #include <unity/util/NonCopyable.h>
106
107 #include <mutex>
108@@ -62,7 +63,8 @@
109 public:
110 NONCOPYABLE(HttpClientQtThread);
111
112- HttpClientQtThread(const QUrl& url, unsigned int timeout, std::function<void(std::string const&)> const& lineData);
113+ HttpClientQtThread(const QUrl& url, unsigned int timeout, std::function<void(std::string const&)> const& lineData, const HttpHeaders& headers =
114+ HttpHeaders());
115 ~HttpClientQtThread();
116
117 bool get_reply(std::string& reply);
118@@ -82,6 +84,7 @@
119 private:
120 QUrl url_;
121 const std::function<void(std::string const&)> lineDataCallback_;
122+ HttpHeaders headers_;
123 unsigned int timeout_;
124 std::mutex reply_mutex_;
125
126
127=== modified file 'include/unity/scopes/internal/smartscopes/SmartScopesClient.h'
128--- include/unity/scopes/internal/smartscopes/SmartScopesClient.h 2014-09-03 14:34:14 +0000
129+++ include/unity/scopes/internal/smartscopes/SmartScopesClient.h 2014-09-25 08:34:38 +0000
130@@ -174,6 +174,7 @@
131 VariantMap const& filter_state = VariantMap(),
132 std::string const& locale = "",
133 std::string const& country = "",
134+ std::string const& user_agent_hdr = "",
135 const unsigned int limit = 0);
136
137 PreviewHandle::UPtr preview(PreviewReplyHandler const& handler,
138@@ -184,7 +185,8 @@
139 const unsigned int widgets_api_version,
140 VariantMap const& settings = VariantMap(),
141 std::string const& locale = "",
142- std::string const& country = "");
143+ std::string const& country = "",
144+ std::string const& user_agent_hdr = "");
145
146 private:
147 friend class SearchHandle;
148
149=== modified file 'src/scopes/internal/smartscopes/HttpClientQt.cpp'
150--- src/scopes/internal/smartscopes/HttpClientQt.cpp 2014-09-02 11:58:04 +0000
151+++ src/scopes/internal/smartscopes/HttpClientQt.cpp 2014-09-25 08:34:38 +0000
152@@ -48,12 +48,13 @@
153 {
154 }
155
156-HttpResponseHandle::SPtr HttpClientQt::get(std::string const& request_url, std::function<void(std::string const&)> const& lineData)
157+HttpResponseHandle::SPtr HttpClientQt::get(std::string const& request_url, std::function<void(std::string const&)> const& lineData,
158+ HttpHeaders const& headers)
159 {
160 std::lock_guard<std::mutex> lock(sessions_mutex_);
161
162 // start new session
163- auto session = std::make_shared<HttpSession>(request_url, no_reply_timeout_, lineData);
164+ auto session = std::make_shared<HttpSession>(request_url, no_reply_timeout_, lineData, headers);
165 sessions_[session_index_] = session;
166
167 return std::make_shared<HttpResponseHandle>(shared_from_this(), session_index_++, session->get_future());
168@@ -79,17 +80,18 @@
169
170 //-- HttpClientQt::HttpSession
171
172-HttpClientQt::HttpSession::HttpSession(std::string const& request_url, uint timeout, std::function<void(std::string const&)> const& lineData)
173+HttpClientQt::HttpSession::HttpSession(std::string const& request_url, uint timeout, std::function<void(std::string const&)> const& lineData,
174+ HttpHeaders const& headers)
175 : qt_thread_(nullptr)
176 {
177 get_thread_ =
178- std::thread([this, request_url, timeout, lineData]()
179+ std::thread([this, request_url, headers, timeout, lineData]()
180 {
181 QUrl url(request_url.c_str());
182
183 {
184 std::lock_guard<std::mutex> lock(qt_thread_mutex_);
185- qt_thread_ = std::unique_ptr<HttpClientQtThread>(new HttpClientQtThread(url, timeout, lineData));
186+ qt_thread_ = std::unique_ptr<HttpClientQtThread>(new HttpClientQtThread(url, timeout, lineData, headers));
187 }
188
189 QEventLoop loop;
190
191=== modified file 'src/scopes/internal/smartscopes/HttpClientQtThread.cpp'
192--- src/scopes/internal/smartscopes/HttpClientQtThread.cpp 2014-08-28 15:51:48 +0000
193+++ src/scopes/internal/smartscopes/HttpClientQtThread.cpp 2014-09-25 08:34:38 +0000
194@@ -38,10 +38,11 @@
195 namespace smartscopes
196 {
197
198-HttpClientQtThread::HttpClientQtThread(const QUrl& url, uint timeout, std::function<void(std::string const&)> const& lineData)
199+HttpClientQtThread::HttpClientQtThread(const QUrl& url, uint timeout, std::function<void(std::string const&)> const& lineData, HttpHeaders const& headers)
200 : QThread()
201 , url_(url)
202 , lineDataCallback_(lineData)
203+ , headers_(headers)
204 , timeout_(timeout)
205 , success_(false)
206 {
207@@ -67,6 +68,10 @@
208 QNetworkAccessManager* manager = new QNetworkAccessManager();
209
210 QNetworkRequest request(url_);
211+ for (auto const& hdr: headers_)
212+ {
213+ request.setRawHeader(QString::fromStdString(hdr.first).toUtf8(), QString::fromStdString(hdr.second).toUtf8());
214+ }
215
216 QNetworkReply* reply = manager->get(request);
217 reply->setReadBufferSize(0); // unlimited buffer
218
219=== modified file 'src/scopes/internal/smartscopes/SmartScope.cpp'
220--- src/scopes/internal/smartscopes/SmartScope.cpp 2014-09-03 14:34:14 +0000
221+++ src/scopes/internal/smartscopes/SmartScope.cpp 2014-09-25 08:34:38 +0000
222@@ -163,6 +163,7 @@
223 ///! TODO: country (+location data)
224 int query_id = 0;
225 std::string session_id;
226+ std::string agent;
227 auto const metadata = search_metadata();
228 if (metadata.contains_hint("session-id") && metadata["session-id"].which() == Variant::String)
229 {
230@@ -181,8 +182,13 @@
231 {
232 std::cout << "SmartScope: missing or invalid query id for \"" << scope_id_ << "\": \"" << query_.query_string() << "\"" << std::endl;
233 }
234+ if (metadata.contains_hint("user-agent") && metadata["user-agent"].which() == Variant::String)
235+ {
236+ agent = metadata["user-agent"].get_string();
237+ }
238
239- search_handle_ = ss_client_->search(handler, base_url_, query_.query_string(), query_.department_id(), session_id, query_id, hints_.form_factor(), settings(), query_.filter_state().serialize(), hints_.locale(), "", hints_.cardinality());
240+ search_handle_ = ss_client_->search(handler, base_url_, query_.query_string(), query_.department_id(), session_id, query_id, hints_.form_factor(),
241+ settings(), query_.filter_state().serialize(), hints_.locale(), "", agent, hints_.cardinality());
242 search_handle_->wait();
243
244 std::cout << "SmartScope: query for \"" << scope_id_ << "\": \"" << query_.query_string() << "\" complete" << std::endl;
245@@ -250,8 +256,9 @@
246 }
247 };
248
249- ///! TODO: widgets_api_version, country (+location data)
250+ ///! TODO: country (+location data)
251 std::string session_id;
252+ std::string agent;
253 auto const metadata = action_metadata();
254 if (metadata.contains_hint("session-id") && metadata["session-id"].which() == Variant::String)
255 {
256@@ -263,7 +270,13 @@
257 std::cout << "SmartScope: missing or invalid session id for \"" << scope_id_ << "\" preview: \"" << result().uri() << "\"" << std::endl;
258 }
259
260- preview_handle_ = ss_client_->preview(handler, base_url_, result_["result_json"].get_string(), session_id, hints_.form_factor(), 0, settings(), hints_.locale(), "");
261+ if (metadata.contains_hint("user-agent") && metadata["user-agent"].which() == Variant::String)
262+ {
263+ agent = metadata["user-agent"].get_string();
264+ }
265+
266+ preview_handle_ = ss_client_->preview(handler, base_url_, result_["result_json"].get_string(), session_id, hints_.form_factor(), 0, settings(),
267+ hints_.locale(), "", agent);
268
269 preview_handle_->wait();
270
271
272=== modified file 'src/scopes/internal/smartscopes/SmartScopesClient.cpp'
273--- src/scopes/internal/smartscopes/SmartScopesClient.cpp 2014-09-03 14:34:14 +0000
274+++ src/scopes/internal/smartscopes/SmartScopesClient.cpp 2014-09-25 08:34:38 +0000
275@@ -335,6 +335,7 @@
276 VariantMap const& filter_state,
277 std::string const& locale,
278 std::string const& country,
279+ std::string const& user_agent_hdr,
280 uint limit)
281 {
282 std::ostringstream search_uri;
283@@ -381,6 +382,14 @@
284 uint search_id = ++query_counter_;
285
286 std::cout << "SmartScopesClient.search(): GET " << search_uri.str() << std::endl;
287+
288+ HttpHeaders headers;
289+ if (!user_agent_hdr.empty())
290+ {
291+ std::cout << "User agent: " << user_agent_hdr;
292+ headers.push_back(std::make_pair("User-Agent", user_agent_hdr));
293+ }
294+
295 query_results_[search_id] = http_client_->get(search_uri.str(), [this, handler](std::string const& lineData) {
296 try
297 {
298@@ -390,7 +399,7 @@
299 {
300 std::cerr << "Failed to parse: " << e.what() << std::endl;
301 }
302- });
303+ }, headers);
304
305 return SearchHandle::UPtr(new SearchHandle(search_id, shared_from_this()));
306 }
307@@ -403,6 +412,7 @@
308 const uint widgets_api_version,
309 VariantMap const& settings,
310 std::string const& locale,
311+ std::string const& user_agent_hdr,
312 std::string const& country)
313 {
314 std::ostringstream preview_uri;
315@@ -415,6 +425,12 @@
316 preview_uri << "&platform=" << platform;
317 preview_uri << "&widgets_api_version=" << std::to_string(widgets_api_version);
318
319+ HttpHeaders headers;
320+ if (!user_agent_hdr.empty())
321+ {
322+ headers.push_back(std::make_pair("User-Agent", user_agent_hdr));
323+ }
324+
325 // optional parameters
326
327 if (!settings.empty())
328@@ -447,7 +463,7 @@
329 {
330 std::cerr << "Failed to parse: " << e.what() << std::endl;
331 }
332- });
333+ }, headers);
334
335 return PreviewHandle::UPtr(new PreviewHandle(preview_id, shared_from_this()));
336 }
337
338=== modified file 'test/gtest/scopes/internal/smartscopes/SmartScopesClient/FakeSss.py'
339--- test/gtest/scopes/internal/smartscopes/SmartScopesClient/FakeSss.py 2014-08-05 15:08:30 +0000
340+++ test/gtest/scopes/internal/smartscopes/SmartScopesClient/FakeSss.py 2014-09-25 08:34:38 +0000
341@@ -32,6 +32,9 @@
342 if environ['PATH_INFO'] == '/remote-scopes' and (environ['QUERY_STRING'] == '' or environ['QUERY_STRING'] == 'locale=test_TEST'):
343 return [remote_scopes_response]
344
345+ if environ['PATH_INFO'] == '/demo/search' and environ['QUERY_STRING'].find('test_user_agent_header') >= 0:
346+ return [search_response + '\r\n{"result": {"cat_id": "cat1", "art": "https://dash.ubuntu.com/imgs/cat.png", "uri": "URI", "title": "' + environ['HTTP_USER_AGENT'] + '"}}']
347+
348 if environ['PATH_INFO'] == '/demo/search' and environ['QUERY_STRING'] != '':
349 return [search_response]
350
351
352=== modified file 'test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp'
353--- test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp 2014-08-28 15:51:48 +0000
354+++ test/gtest/scopes/internal/smartscopes/SmartScopesClient/SmartScopesClient_test.cpp 2014-09-25 08:34:38 +0000
355@@ -140,7 +140,7 @@
356 dept = deptinfo;
357 };
358
359- auto search_handle = ssc_->search(handler, sss_url_ + "/demo", "stuff", "", "session_id", 0, "platform");
360+ auto search_handle = ssc_->search(handler, sss_url_ + "/demo", "stuff", "", "session_id", 0, "platform", VariantMap(), VariantMap(), "en_US", "", "ThisIsUserAgentHeader");
361 search_handle->wait();
362
363 ASSERT_EQ(3u, results.size());
364@@ -221,6 +221,32 @@
365 EXPECT_EQ(active_option->id(), "salesrank");
366 }
367
368+TEST_F(SmartScopesClientTest, userAgentHeader)
369+{
370+ std::vector<SearchResult> results;
371+
372+ SearchReplyHandler handler;
373+ handler.filters_handler = [](Filters const &) {
374+ };
375+ handler.filter_state_handler = [](FilterState const&) {
376+ };
377+ handler.category_handler = [](std::shared_ptr<SearchCategory> const&) {
378+ };
379+ handler.result_handler = [&results](SearchResult const& result) {
380+ results.push_back(result);
381+ };
382+ handler.departments_handler = [](std::shared_ptr<DepartmentInfo> const&) {
383+ };
384+
385+ auto search_handle = ssc_->search(handler, sss_url_ + "/demo", "test_user_agent_header", "", "session_id", 0, "platform", VariantMap(), VariantMap(), "en_US", "", "ThisIsUserAgentHeader");
386+ search_handle->wait();
387+
388+ ASSERT_EQ(4u, results.size());
389+
390+ // user agent string is expected in the result title
391+ EXPECT_EQ("ThisIsUserAgentHeader", results[3].other_params["title"]->as_string());
392+}
393+
394 TEST_F(SmartScopesClientTest, preview)
395 {
396 PreviewReplyHandler handler;

Subscribers

People subscribed via source and target branches

to all changes: