Merge lp:~michihenning/storage-framework/marshal-metadata into lp:storage-framework/devel
- marshal-metadata
- Merge into devel
Status: | Merged |
---|---|
Approved by: | James Henstridge |
Approved revision: | 91 |
Merged at revision: | 48 |
Proposed branch: | lp:~michihenning/storage-framework/marshal-metadata |
Merge into: | lp:storage-framework/devel |
Prerequisite: | lp:~michihenning/storage-framework/exception-marshaling |
Diff against target: |
885 lines (+367/-151) 16 files modified
demo/provider_test/CMakeLists.txt (+0/-1) demo/provider_test/provider-test.cpp (+21/-6) include/unity/storage/provider/ProviderBase.h (+6/-2) include/unity/storage/provider/metadata_keys.h (+19/-6) include/unity/storage/qt/client/internal/remote_client/validate.h (+51/-0) src/provider/internal/dbusmarshal.cpp (+19/-1) src/qt/client/internal/remote_client/CMakeLists.txt (+1/-0) src/qt/client/internal/remote_client/FileImpl.cpp (+3/-1) src/qt/client/internal/remote_client/FolderImpl.cpp (+28/-0) src/qt/client/internal/remote_client/ItemImpl.cpp (+22/-4) src/qt/client/internal/remote_client/RootImpl.cpp (+10/-0) src/qt/client/internal/remote_client/UploaderImpl.cpp (+10/-0) src/qt/client/internal/remote_client/validate.cpp (+155/-0) tests/CMakeLists.txt (+2/-2) tests/remote-client/CMakeLists.txt (+1/-1) tests/remote-client/remote-client_test.cpp (+19/-127) |
To merge this branch: | bzr merge lp:~michihenning/storage-framework/marshal-metadata |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
unity-api-1-bot | continuous-integration | Approve | |
James Henstridge | Approve | ||
Review via email:
|
Commit message
Marshal known metadata, with sanity checks on the client side.
Description of the change
Marshal known metadata, with sanity checks on the client side.
Still needs coverage, and native metadata is not yet dealt with, but the branch is large enough as is.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
unity-api-1-bot (unity-api-1-bot) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
James Henstridge (jamesh) wrote : | # |
Just a few small issues I noticed while reading through this:
1. we should be passing timestamps qualified with a time zone rather than be floating like this. So provider-test.cpp should have a "Z" or "+00:00" on the end of those time stamps. Hopefully Qt can tell the difference in the validation code.
2. how hard would it be to add some direct tests for unity::
We should also revisit the "metadata selection API" at some point that we discussed in a hangout, so clients can say what metadata they're interested in.
- 91. By Michi Henning
-
Merged devel.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
unity-api-1-bot (unity-api-1-bot) wrote : | # |
PASSED: Continuous integration, rev:90
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
unity-api-1-bot (unity-api-1-bot) wrote : | # |
PASSED: Continuous integration, rev:91
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'demo/provider_test/CMakeLists.txt' |
2 | --- demo/provider_test/CMakeLists.txt 2016-05-23 02:27:16 +0000 |
3 | +++ demo/provider_test/CMakeLists.txt 2016-08-11 04:54:23 +0000 |
4 | @@ -1,4 +1,3 @@ |
5 | - |
6 | add_definitions(-DBOOST_THREAD_VERSION=4) |
7 | |
8 | add_executable(provider-test provider-test.cpp) |
9 | |
10 | === modified file 'demo/provider_test/provider-test.cpp' |
11 | --- demo/provider_test/provider-test.cpp 2016-08-05 05:37:23 +0000 |
12 | +++ demo/provider_test/provider-test.cpp 2016-08-11 04:54:23 +0000 |
13 | @@ -18,6 +18,7 @@ |
14 | |
15 | #include <unity/storage/provider/DownloadJob.h> |
16 | #include <unity/storage/provider/Exceptions.h> |
17 | +#include <unity/storage/provider/metadata_keys.h> |
18 | #include <unity/storage/provider/ProviderBase.h> |
19 | #include <unity/storage/provider/Server.h> |
20 | #include <unity/storage/provider/TempfileUploadJob.h> |
21 | @@ -158,8 +159,12 @@ |
22 | { |
23 | return make_exceptional_future<tuple<ItemList,string>>(runtime_error("unknown page token")); |
24 | } |
25 | - ItemList children = { |
26 | - {"child_id", "root_id", "Child", "etag", ItemType::file, {}} |
27 | + ItemList children = |
28 | + { |
29 | + { |
30 | + "child_id", "root_id", "Child", "etag", ItemType::file, |
31 | + { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } |
32 | + } |
33 | }; |
34 | boost::promise<tuple<ItemList,string>> p; |
35 | p.set_value(make_tuple(children, string())); |
36 | @@ -174,8 +179,10 @@ |
37 | { |
38 | return make_exceptional_future<ItemList>(runtime_error("file not found")); |
39 | } |
40 | - ItemList children = { |
41 | - {"child_id", "root_id", "Child", "etag", ItemType::file, {}} |
42 | + ItemList children = |
43 | + { |
44 | + { "child_id", "root_id", "Child", "etag", ItemType::file, |
45 | + { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } } |
46 | }; |
47 | return make_ready_future<ItemList>(children); |
48 | } |
49 | @@ -191,7 +198,11 @@ |
50 | } |
51 | else if (item_id == "child_id") |
52 | { |
53 | - Item metadata{"child_id", "root_id", "Child", "etag", ItemType::file, {}}; |
54 | + Item metadata |
55 | + { |
56 | + "child_id", "root_id", "Child", "etag", ItemType::file, |
57 | + { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } |
58 | + }; |
59 | return make_ready_future<Item>(metadata); |
60 | } |
61 | else if (item_id == "child_folder_id") |
62 | @@ -291,7 +302,11 @@ |
63 | unlink(new_filename.c_str()); |
64 | link(old_filename.c_str(), new_filename.c_str()); |
65 | |
66 | - Item metadata{"some_id", "", "some_upload", "etag", ItemType::file, {}}; |
67 | + Item metadata |
68 | + { |
69 | + "some_id", "root_id", "some_upload", "etag", ItemType::file, |
70 | + { { SIZE_IN_BYTES, 10 }, { LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } } |
71 | + }; |
72 | return make_ready_future(metadata); |
73 | } |
74 | |
75 | |
76 | === modified file 'include/unity/storage/provider/ProviderBase.h' |
77 | --- include/unity/storage/provider/ProviderBase.h 2016-07-14 00:17:14 +0000 |
78 | +++ include/unity/storage/provider/ProviderBase.h 2016-08-11 04:54:23 +0000 |
79 | @@ -23,6 +23,7 @@ |
80 | #include <unity/storage/provider/Credentials.h> |
81 | |
82 | #include <boost/thread/future.hpp> |
83 | +#include <boost/variant.hpp> |
84 | |
85 | #include <sys/types.h> |
86 | #include <string> |
87 | @@ -48,6 +49,10 @@ |
88 | Credentials credentials; |
89 | }; |
90 | |
91 | +// Note: When growing the set of supported variant types, add new types |
92 | +// to the *end* of the list, and update the marshaling code in dbusmarshal.cpp. |
93 | +typedef boost::variant<std::string, int64_t> MetadataValue; |
94 | + |
95 | struct UNITY_STORAGE_EXPORT Item |
96 | { |
97 | std::string item_id; |
98 | @@ -55,8 +60,7 @@ |
99 | std::string name; |
100 | std::string etag; |
101 | unity::storage::ItemType type; |
102 | - // Should be map<string,variant> |
103 | - std::map<std::string,std::string> metadata; |
104 | + std::map<std::string, MetadataValue> metadata; |
105 | }; |
106 | |
107 | typedef std::vector<Item> ItemList; |
108 | |
109 | === renamed file 'include/unity/storage/internal/MetadataKeys.h' => 'include/unity/storage/provider/metadata_keys.h' |
110 | --- include/unity/storage/internal/MetadataKeys.h 2016-07-14 00:25:40 +0000 |
111 | +++ include/unity/storage/provider/metadata_keys.h 2016-08-11 04:54:23 +0000 |
112 | @@ -18,15 +18,28 @@ |
113 | |
114 | #pragma once |
115 | |
116 | +#include <unordered_map> |
117 | + |
118 | namespace unity |
119 | { |
120 | namespace storage |
121 | { |
122 | -namespace internal |
123 | -{ |
124 | - |
125 | -static char constexpr CREATION_TIME[] = "creation_time"; |
126 | - |
127 | -} // namespace internal |
128 | +namespace provider |
129 | +{ |
130 | + |
131 | +static char constexpr SIZE_IN_BYTES[] = "size_in_bytes"; // int64_t, >= 0 |
132 | +static char constexpr CREATION_TIME[] = "creation_time"; // String, ISO 8601 format |
133 | +static char constexpr LAST_MODIFIED_TIME[] = "last_modified_time"; // String, ISO 8601 format |
134 | + |
135 | +enum class MetadataType { int64, iso_8601_date_time }; |
136 | + |
137 | +static std::unordered_map<std::string, MetadataType> known_metadata = |
138 | +{ |
139 | + { SIZE_IN_BYTES, MetadataType::int64 }, |
140 | + { CREATION_TIME, MetadataType::iso_8601_date_time }, |
141 | + { LAST_MODIFIED_TIME, MetadataType::iso_8601_date_time } |
142 | +}; |
143 | + |
144 | +} // namespace provider |
145 | } // namespace storage |
146 | } // namespace unity |
147 | |
148 | === added file 'include/unity/storage/qt/client/internal/remote_client/validate.h' |
149 | --- include/unity/storage/qt/client/internal/remote_client/validate.h 1970-01-01 00:00:00 +0000 |
150 | +++ include/unity/storage/qt/client/internal/remote_client/validate.h 2016-08-11 04:54:23 +0000 |
151 | @@ -0,0 +1,51 @@ |
152 | +/* |
153 | + * Copyright (C) 2016 Canonical Ltd |
154 | + * |
155 | + * This program is free software: you can redistribute it and/or modify |
156 | + * it under the terms of the GNU Lesser General Public License version 3 as |
157 | + * published by the Free Software Foundation. |
158 | + * |
159 | + * This program is distributed in the hope that it will be useful, |
160 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
161 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
162 | + * GNU Lesser General Public License for more details. |
163 | + * |
164 | + * You should have received a copy of the GNU Lesser General Public License |
165 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
166 | + * |
167 | + * Authors: Michi Henning <michi.henning@canonical.com> |
168 | + */ |
169 | + |
170 | +#pragma once |
171 | + |
172 | +#include <QString> |
173 | + |
174 | +namespace unity |
175 | +{ |
176 | +namespace storage |
177 | +{ |
178 | + |
179 | +namespace internal |
180 | +{ |
181 | + |
182 | +class ItemMetadata; |
183 | + |
184 | +} // namespace internal |
185 | + |
186 | +namespace qt |
187 | +{ |
188 | +namespace client |
189 | +{ |
190 | +namespace internal |
191 | +{ |
192 | +namespace remote_client |
193 | +{ |
194 | + |
195 | +void validate(QString const& method, unity::storage::internal::ItemMetadata const& md); |
196 | + |
197 | +} // namespace remote_client |
198 | +} // namespace internal |
199 | +} // namespace client |
200 | +} // namespace qt |
201 | +} // namespace storage |
202 | +} // namespace unity |
203 | |
204 | === modified file 'src/provider/internal/dbusmarshal.cpp' |
205 | --- src/provider/internal/dbusmarshal.cpp 2016-07-21 12:48:40 +0000 |
206 | +++ src/provider/internal/dbusmarshal.cpp 2016-08-11 04:54:23 +0000 |
207 | @@ -31,6 +31,24 @@ |
208 | namespace provider |
209 | { |
210 | |
211 | +namespace |
212 | +{ |
213 | + |
214 | +QDBusVariant to_qdbus_variant(MetadataValue const& v) |
215 | +{ |
216 | + switch (v.which()) |
217 | + { |
218 | + case 0: |
219 | + return QDBusVariant(QString::fromStdString(boost::get<string>(v))); |
220 | + case 1: |
221 | + return QDBusVariant(qlonglong(boost::get<int64_t>(v))); |
222 | + default: |
223 | + abort(); // Impossible. // LCOV_EXCL_LINE |
224 | + } |
225 | +} |
226 | + |
227 | +} // namespace |
228 | + |
229 | QDBusArgument& operator<<(QDBusArgument& argument, Item const& item) |
230 | { |
231 | argument.beginStructure(); |
232 | @@ -43,7 +61,7 @@ |
233 | for (auto const& pair : item.metadata) |
234 | { |
235 | argument.beginMapEntry(); |
236 | - argument << QString::fromStdString(pair.first) << QDBusVariant(QString::fromStdString(pair.second)); |
237 | + argument << QString::fromStdString(pair.first) << to_qdbus_variant(pair.second); |
238 | argument.endMapEntry(); |
239 | } |
240 | argument.endMap(); |
241 | |
242 | === modified file 'src/qt/client/internal/remote_client/CMakeLists.txt' |
243 | --- src/qt/client/internal/remote_client/CMakeLists.txt 2016-07-13 01:38:00 +0000 |
244 | +++ src/qt/client/internal/remote_client/CMakeLists.txt 2016-08-11 04:54:23 +0000 |
245 | @@ -10,6 +10,7 @@ |
246 | ${CMAKE_CURRENT_SOURCE_DIR}/Runtime_create.cpp |
247 | ${CMAKE_CURRENT_SOURCE_DIR}/RuntimeImpl.cpp |
248 | ${CMAKE_CURRENT_SOURCE_DIR}/UploaderImpl.cpp |
249 | + ${CMAKE_CURRENT_SOURCE_DIR}/validate.cpp |
250 | ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/client/internal/remote_client/HandlerBase.h |
251 | ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/client/internal/remote_client/RuntimeImpl.h |
252 | PARENT_SCOPE) |
253 | |
254 | === modified file 'src/qt/client/internal/remote_client/FileImpl.cpp' |
255 | --- src/qt/client/internal/remote_client/FileImpl.cpp 2016-08-11 01:39:44 +0000 |
256 | +++ src/qt/client/internal/remote_client/FileImpl.cpp 2016-08-11 04:54:23 +0000 |
257 | @@ -19,10 +19,12 @@ |
258 | #include <unity/storage/qt/client/internal/remote_client/FileImpl.h> |
259 | |
260 | #include "ProviderInterface.h" |
261 | +#include <unity/storage/provider/metadata_keys.h> |
262 | #include <unity/storage/qt/client/File.h> |
263 | #include <unity/storage/qt/client/internal/remote_client/Handler.h> |
264 | #include <unity/storage/qt/client/internal/remote_client/DownloaderImpl.h> |
265 | #include <unity/storage/qt/client/internal/remote_client/UploaderImpl.h> |
266 | +#include <unity/storage/qt/client/internal/remote_client/validate.h> |
267 | |
268 | using namespace std; |
269 | |
270 | @@ -49,7 +51,7 @@ |
271 | int64_t FileImpl::size() const |
272 | { |
273 | throw_if_destroyed("File::size()"); |
274 | - return 0; // TODO |
275 | + return md_.metadata.value(provider::SIZE_IN_BYTES).toLongLong(); |
276 | } |
277 | |
278 | QFuture<shared_ptr<Uploader>> FileImpl::create_uploader(ConflictPolicy policy, int64_t size) |
279 | |
280 | === modified file 'src/qt/client/internal/remote_client/FolderImpl.cpp' |
281 | --- src/qt/client/internal/remote_client/FolderImpl.cpp 2016-08-03 06:29:48 +0000 |
282 | +++ src/qt/client/internal/remote_client/FolderImpl.cpp 2016-08-11 04:54:23 +0000 |
283 | @@ -23,6 +23,7 @@ |
284 | #include <unity/storage/qt/client/internal/remote_client/FileImpl.h> |
285 | #include <unity/storage/qt/client/internal/remote_client/Handler.h> |
286 | #include <unity/storage/qt/client/internal/remote_client/UploaderImpl.h> |
287 | +#include <unity/storage/qt/client/internal/remote_client/validate.h> |
288 | |
289 | #include <cassert> |
290 | |
291 | @@ -85,6 +86,15 @@ |
292 | auto metadata = reply.argumentAt<0>(); |
293 | for (auto const& md : metadata) |
294 | { |
295 | + try |
296 | + { |
297 | + validate("Folder::list()", md); |
298 | + } |
299 | + catch (StorageException const& e) |
300 | + { |
301 | + make_exceptional_future(qf, e); |
302 | + return; |
303 | + } |
304 | if (md.type == ItemType::root) |
305 | { |
306 | // TODO: log server error here |
307 | @@ -138,6 +148,15 @@ |
308 | auto metadata = reply.value(); |
309 | for (auto const& md : metadata) |
310 | { |
311 | + try |
312 | + { |
313 | + validate("Folder::lookup()", md); |
314 | + } |
315 | + catch (StorageException const& e) |
316 | + { |
317 | + make_exceptional_future(qf, e); |
318 | + return; |
319 | + } |
320 | if (md.type == ItemType::root) |
321 | { |
322 | // TODO: log server error here |
323 | @@ -182,6 +201,15 @@ |
324 | |
325 | shared_ptr<Item> item; |
326 | auto md = reply.value(); |
327 | + try |
328 | + { |
329 | + validate("Folder::create_folder()", md); |
330 | + } |
331 | + catch (StorageException const& e) |
332 | + { |
333 | + make_exceptional_future(qf, e); |
334 | + return; |
335 | + } |
336 | if (md.type != ItemType::folder) |
337 | { |
338 | // TODO: log server error here |
339 | |
340 | === modified file 'src/qt/client/internal/remote_client/ItemImpl.cpp' |
341 | --- src/qt/client/internal/remote_client/ItemImpl.cpp 2016-08-03 06:29:48 +0000 |
342 | +++ src/qt/client/internal/remote_client/ItemImpl.cpp 2016-08-11 04:54:23 +0000 |
343 | @@ -19,11 +19,13 @@ |
344 | #include <unity/storage/qt/client/internal/remote_client/ItemImpl.h> |
345 | |
346 | #include "ProviderInterface.h" |
347 | +#include <unity/storage/provider/metadata_keys.h> |
348 | #include <unity/storage/qt/client/Account.h> |
349 | #include <unity/storage/qt/client/internal/remote_client/AccountImpl.h> |
350 | #include <unity/storage/qt/client/internal/remote_client/FileImpl.h> |
351 | #include <unity/storage/qt/client/internal/remote_client/Handler.h> |
352 | #include <unity/storage/qt/client/internal/remote_client/RootImpl.h> |
353 | +#include <unity/storage/qt/client/internal/remote_client/validate.h> |
354 | |
355 | #include <cassert> |
356 | |
357 | @@ -70,8 +72,7 @@ |
358 | QDateTime ItemImpl::last_modified_time() const |
359 | { |
360 | throw_if_destroyed("Item::last_modified_time()"); |
361 | - // TODO: need to agree on metadata representation |
362 | - return QDateTime(); |
363 | + return QDateTime::fromString(md_.metadata.value(provider::LAST_MODIFIED_TIME).toString(), Qt::ISODate); |
364 | } |
365 | |
366 | QFuture<shared_ptr<Item>> ItemImpl::copy(shared_ptr<Folder> const& new_parent, QString const& new_name) |
367 | @@ -106,6 +107,15 @@ |
368 | } |
369 | |
370 | auto md = reply.value(); |
371 | + try |
372 | + { |
373 | + validate("Item::copy()", md); |
374 | + } |
375 | + catch (StorageException const& e) |
376 | + { |
377 | + make_exceptional_future(qf, e); |
378 | + return; |
379 | + } |
380 | if (md.type == ItemType::root) |
381 | { |
382 | // TODO: log server error here |
383 | @@ -158,6 +168,15 @@ |
384 | } |
385 | |
386 | auto md = reply.value(); |
387 | + try |
388 | + { |
389 | + validate("Item::move()", md); |
390 | + } |
391 | + catch (StorageException const& e) |
392 | + { |
393 | + make_exceptional_future(qf, e); |
394 | + return; |
395 | + } |
396 | if (md.type == ItemType::root) |
397 | { |
398 | // TODO: log server error here |
399 | @@ -220,8 +239,7 @@ |
400 | QDateTime ItemImpl::creation_time() const |
401 | { |
402 | throw_if_destroyed("Item::creation_time()"); |
403 | - // TODO: need to agree on metadata representation |
404 | - return QDateTime(); |
405 | + return QDateTime::fromString(md_.metadata.value(provider::CREATION_TIME).toString(), Qt::ISODate); |
406 | } |
407 | |
408 | MetadataMap ItemImpl::native_metadata() const |
409 | |
410 | === modified file 'src/qt/client/internal/remote_client/RootImpl.cpp' |
411 | --- src/qt/client/internal/remote_client/RootImpl.cpp 2016-08-05 05:37:23 +0000 |
412 | +++ src/qt/client/internal/remote_client/RootImpl.cpp 2016-08-11 04:54:23 +0000 |
413 | @@ -21,6 +21,7 @@ |
414 | #include "ProviderInterface.h" |
415 | #include <unity/storage/qt/client/internal/remote_client/FileImpl.h> |
416 | #include <unity/storage/qt/client/internal/remote_client/Handler.h> |
417 | +#include <unity/storage/qt/client/internal/remote_client/validate.h> |
418 | |
419 | #include <cassert> |
420 | |
421 | @@ -137,6 +138,15 @@ |
422 | } |
423 | |
424 | auto md = reply.value(); |
425 | + try |
426 | + { |
427 | + validate("Root::get()", md); |
428 | + } |
429 | + catch (StorageException const& e) |
430 | + { |
431 | + make_exceptional_future(qf, e); |
432 | + return; |
433 | + } |
434 | Item::SPtr item; |
435 | if (md.type == ItemType::root) |
436 | { |
437 | |
438 | === modified file 'src/qt/client/internal/remote_client/UploaderImpl.cpp' |
439 | --- src/qt/client/internal/remote_client/UploaderImpl.cpp 2016-08-03 04:13:28 +0000 |
440 | +++ src/qt/client/internal/remote_client/UploaderImpl.cpp 2016-08-11 04:54:23 +0000 |
441 | @@ -21,6 +21,7 @@ |
442 | #include "ProviderInterface.h" |
443 | #include <unity/storage/qt/client/internal/remote_client/FileImpl.h> |
444 | #include <unity/storage/qt/client/internal/remote_client/Handler.h> |
445 | +#include <unity/storage/qt/client/internal/remote_client/validate.h> |
446 | #include <unity/storage/qt/client/Uploader.h> |
447 | |
448 | #include <cassert> |
449 | @@ -79,6 +80,15 @@ |
450 | auto process_reply = [this](decltype(reply) const& reply, QFutureInterface<shared_ptr<File>>& qf) |
451 | { |
452 | auto md = reply.value(); |
453 | + try |
454 | + { |
455 | + validate("Uploader::finish_upload()", md); |
456 | + } |
457 | + catch (StorageException const& e) |
458 | + { |
459 | + make_exceptional_future(qf, e); |
460 | + return; |
461 | + } |
462 | if (md.type != ItemType::file) |
463 | { |
464 | // TODO: log server error here |
465 | |
466 | === added file 'src/qt/client/internal/remote_client/validate.cpp' |
467 | --- src/qt/client/internal/remote_client/validate.cpp 1970-01-01 00:00:00 +0000 |
468 | +++ src/qt/client/internal/remote_client/validate.cpp 2016-08-11 04:54:23 +0000 |
469 | @@ -0,0 +1,155 @@ |
470 | +/* |
471 | + * Copyright (C) 2016 Canonical Ltd |
472 | + * |
473 | + * This program is free software: you can redistribute it and/or modify |
474 | + * it under the terms of the GNU Lesser General Public License version 3 as |
475 | + * published by the Free Software Foundation. |
476 | + * |
477 | + * This program is distributed in the hope that it will be useful, |
478 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
479 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
480 | + * GNU Lesser General Public License for more details. |
481 | + * |
482 | + * You should have received a copy of the GNU Lesser General Public License |
483 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
484 | + * |
485 | + * Authors: Michi Henning <michi.henning@canonical.com> |
486 | + */ |
487 | + |
488 | +#include <unity/storage/internal/ItemMetadata.h> |
489 | + |
490 | +#include <unity/storage/provider/metadata_keys.h> |
491 | +#include <unity/storage/qt/client/Exceptions.h> |
492 | + |
493 | +#include <QDateTime> |
494 | +#include <QString> |
495 | + |
496 | +using namespace unity::storage::internal; |
497 | +using namespace std; |
498 | + |
499 | +namespace unity |
500 | +{ |
501 | +namespace storage |
502 | +{ |
503 | +namespace qt |
504 | +{ |
505 | +namespace client |
506 | +{ |
507 | +namespace internal |
508 | +{ |
509 | +namespace remote_client |
510 | +{ |
511 | + |
512 | +namespace |
513 | +{ |
514 | + |
515 | +// Check that actual type and value match the expect type and value for a particular metadata entry. |
516 | + |
517 | +void validate_type_and_value(QString const& prefix, |
518 | + QMapIterator<QString, QVariant> actual, |
519 | + unordered_map<string, provider::MetadataType>::const_iterator known) |
520 | +{ |
521 | + using namespace unity::storage::provider; |
522 | + |
523 | + switch (known->second) |
524 | + { |
525 | + case MetadataType::iso_8601_date_time: |
526 | + { |
527 | + if (actual.value().type() != QVariant::String) |
528 | + { |
529 | + throw LocalCommsException(prefix + actual.key() + ": expected value of type String, " |
530 | + " but received value of type " + actual.value().typeName()); |
531 | + } |
532 | + QDateTime dt = QDateTime::fromString(actual.value().toString(), Qt::ISODate); |
533 | + if (!dt.isValid()) |
534 | + { |
535 | + throw LocalCommsException(prefix + actual.key() + ": value \"" + actual.value().toString() |
536 | + + "\" does not parse as ISO-8601 date"); |
537 | + } |
538 | + auto timespec = dt.timeSpec(); |
539 | + if (timespec == Qt::LocalTime) |
540 | + { |
541 | + throw LocalCommsException(prefix + actual.key() + ": value \"" + actual.value().toString() |
542 | + + "\" lacks a time zone specification"); |
543 | + } |
544 | + break; |
545 | + } |
546 | + case MetadataType::int64: |
547 | + { |
548 | + if (actual.value().type() != QVariant::LongLong) |
549 | + { |
550 | + throw LocalCommsException(prefix + actual.key() + ": expected value of type LongLong, " |
551 | + " but received value of type " + actual.value().typeName()); |
552 | + } |
553 | + break; |
554 | + } |
555 | + default: |
556 | + { |
557 | + abort(); // Impossible. // LCOV_EXCL_LINE |
558 | + } |
559 | + } |
560 | +} |
561 | + |
562 | +} |
563 | + |
564 | +void validate(QString const& method, ItemMetadata const& md) |
565 | +{ |
566 | + using namespace unity::storage::provider; |
567 | + |
568 | + QString prefix = method + ": received invalid metadata from server: "; |
569 | + |
570 | + // Basic sanity checks for mandatory fields. |
571 | + if (md.item_id.isEmpty()) |
572 | + { |
573 | + throw LocalCommsException(prefix + "item_id cannot be empty"); |
574 | + } |
575 | + if (md.parent_id.isEmpty() && md.type != ItemType::root) |
576 | + { |
577 | + throw LocalCommsException(prefix + "parent_id of file or folder cannot be empty"); |
578 | + } |
579 | + if (!md.parent_id.isEmpty() && md.type == ItemType::root) |
580 | + { |
581 | + throw LocalCommsException(prefix + "metadata: parent_id of root cannot be non-empty"); |
582 | + } |
583 | + if (md.name.isEmpty()) |
584 | + { |
585 | + throw LocalCommsException(prefix + "name cannot be empty"); |
586 | + } |
587 | + if (md.etag.isEmpty()) |
588 | + { |
589 | + throw LocalCommsException(prefix + "etag cannot be empty"); |
590 | + } |
591 | + |
592 | + // Sanity check metadata to make sure only known metadata keys appear. |
593 | + QMapIterator<QString, QVariant> actual(md.metadata); |
594 | + while (actual.hasNext()) |
595 | + { |
596 | + actual.next(); |
597 | + auto known = known_metadata.find(actual.key().toStdString()); |
598 | + if (known == known_metadata.end()) |
599 | + { |
600 | + throw LocalCommsException(prefix + "unknown metadata key:" + actual.key()); |
601 | + } |
602 | + validate_type_and_value(prefix, actual, known); |
603 | + } |
604 | + |
605 | + // Sanity check metadata to make sure that mandatory fields are present. |
606 | + if (md.type == ItemType::file) |
607 | + { |
608 | + if (!md.metadata.contains(SIZE_IN_BYTES)) |
609 | + { |
610 | + throw LocalCommsException(prefix + "missing key " + SIZE_IN_BYTES + " in metadata for " + md.item_id); |
611 | + } |
612 | + if (!md.metadata.contains(LAST_MODIFIED_TIME)) |
613 | + { |
614 | + throw LocalCommsException(prefix + "missing key " + LAST_MODIFIED_TIME + " in metadata for " + md.item_id); |
615 | + } |
616 | + } |
617 | +} |
618 | + |
619 | +} // namespace remote_client |
620 | +} // namespace internal |
621 | +} // namespace client |
622 | +} // namespace qt |
623 | +} // namespace storage |
624 | +} // namespace unity |
625 | |
626 | === modified file 'tests/CMakeLists.txt' |
627 | --- tests/CMakeLists.txt 2016-08-04 10:12:01 +0000 |
628 | +++ tests/CMakeLists.txt 2016-08-11 04:54:23 +0000 |
629 | @@ -12,8 +12,8 @@ |
630 | add_subdirectory(utils) |
631 | |
632 | set(unit_test_dirs |
633 | - local-client # -TODO: Temporarily disabled due to a "unknown file: Failure" error in CI. |
634 | - remote-client # -TODO: Temporarily disabled due to a "Cannot find any online account" error in CI. |
635 | + local-client |
636 | + remote-client |
637 | provider-AccountData |
638 | provider-DBusPeerCache |
639 | provider-ProviderInterface |
640 | |
641 | === modified file 'tests/remote-client/CMakeLists.txt' |
642 | --- tests/remote-client/CMakeLists.txt 2016-07-21 04:00:29 +0000 |
643 | +++ tests/remote-client/CMakeLists.txt 2016-08-11 04:54:23 +0000 |
644 | @@ -14,6 +14,6 @@ |
645 | gtest |
646 | ) |
647 | add_test(remote-client remote-client_test) |
648 | -add_dependencies(remote-client_test qt-client-all-headers) |
649 | +add_dependencies(remote-client_test qt-client-all-headers provider-test) |
650 | |
651 | set(UNIT_TEST_TARGETS ${UNIT_TEST_TARGETS} PARENT_SCOPE) |
652 | |
653 | === modified file 'tests/remote-client/remote-client_test.cpp' |
654 | --- tests/remote-client/remote-client_test.cpp 2016-08-05 05:37:23 +0000 |
655 | +++ tests/remote-client/remote-client_test.cpp 2016-08-11 04:54:23 +0000 |
656 | @@ -107,13 +107,7 @@ |
657 | |
658 | Account::SPtr get_account(Runtime::SPtr const& runtime) |
659 | { |
660 | - auto accounts_fut = runtime->accounts(); |
661 | - QFutureWatcher<QVector<Account::SPtr>> w; |
662 | - QSignalSpy spy(&w, &decltype(w)::finished); |
663 | - w.setFuture(accounts_fut); |
664 | - assert(spy.wait(SIGNAL_WAIT_TIME)); |
665 | - |
666 | - auto accounts = accounts_fut.result(); |
667 | + auto accounts = call(runtime->accounts()); |
668 | if (accounts.size() == 0) |
669 | { |
670 | qCritical() << "Cannot find any online account"; |
671 | @@ -126,14 +120,7 @@ |
672 | Root::SPtr get_root(Runtime::SPtr const& runtime) |
673 | { |
674 | auto acc = get_account(runtime); |
675 | - auto roots_fut = acc->roots(); |
676 | - { |
677 | - QFutureWatcher<QVector<Root::SPtr>> w; |
678 | - QSignalSpy spy(&w, &decltype(w)::finished); |
679 | - w.setFuture(roots_fut); |
680 | - assert(spy.wait(SIGNAL_WAIT_TIME)); |
681 | - } |
682 | - auto roots = roots_fut.result(); |
683 | + auto roots = call(acc->roots()); |
684 | assert(roots.size() >= 1); |
685 | return roots[0]; |
686 | } |
687 | @@ -141,36 +128,18 @@ |
688 | Folder::SPtr get_parent(Item::SPtr const& item) |
689 | { |
690 | assert(item->type() != ItemType::root); |
691 | - auto parents_fut = item->parents(); |
692 | - { |
693 | - QFutureWatcher<QVector<Folder::SPtr>> w; |
694 | - QSignalSpy spy(&w, &decltype(w)::finished); |
695 | - w.setFuture(parents_fut); |
696 | - assert(spy.wait(SIGNAL_WAIT_TIME)); |
697 | - } |
698 | - auto parents = parents_fut.result(); |
699 | + auto parents = call(item->parents()); |
700 | assert(parents.size() >= 1); |
701 | return parents[0]; |
702 | } |
703 | |
704 | void clear_folder(Folder::SPtr const& folder) |
705 | { |
706 | - auto list_fut = folder->list(); |
707 | - { |
708 | - QFutureWatcher<QVector<Item::SPtr>> w; |
709 | - QSignalSpy spy(&w, &decltype(w)::finished); |
710 | - w.setFuture(list_fut); |
711 | - assert(spy.wait(SIGNAL_WAIT_TIME)); |
712 | - } |
713 | - auto items = list_fut.result(); |
714 | + auto items = call(folder->list()); |
715 | assert(items.size() != 0); // TODO: temporary hack for use with demo provider |
716 | for (auto i : items) |
717 | { |
718 | - auto delete_fut = i->delete_item(); |
719 | - QFutureWatcher<void> w; |
720 | - QSignalSpy spy(&w, &decltype(w)::finished); |
721 | - w.setFuture(delete_fut); |
722 | - assert(spy.wait(SIGNAL_WAIT_TIME)); |
723 | + wait(i->delete_item()); |
724 | } |
725 | } |
726 | |
727 | @@ -198,14 +167,7 @@ |
728 | |
729 | auto acc = get_account(runtime); |
730 | ASSERT_NE(nullptr, acc); |
731 | - auto roots_fut = acc->roots(); |
732 | - { |
733 | - QFutureWatcher<QVector<Root::SPtr>> w; |
734 | - QSignalSpy spy(&w, &decltype(w)::finished); |
735 | - w.setFuture(roots_fut); |
736 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
737 | - } |
738 | - auto roots = roots_fut.result(); |
739 | + auto roots = call(acc->roots()); |
740 | ASSERT_GE(roots.size(), 0); |
741 | EXPECT_EQ("root_id", roots[0]->native_identity()); |
742 | } |
743 | @@ -222,42 +184,21 @@ |
744 | EXPECT_EQ("Root", root->name()); |
745 | EXPECT_NE("", root->etag()); |
746 | |
747 | - auto parents = root->parents().result(); |
748 | + auto parents = call(root->parents()); |
749 | EXPECT_TRUE(parents.isEmpty()); |
750 | EXPECT_TRUE(root->parent_ids().isEmpty()); |
751 | |
752 | // get(<root-ID>) must return the root. |
753 | - auto get_fut = root->get(root->native_identity()); |
754 | - { |
755 | - QFutureWatcher<Item::SPtr> w; |
756 | - QSignalSpy spy(&w, &decltype(w)::finished); |
757 | - w.setFuture(get_fut); |
758 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
759 | - } |
760 | - auto item = get_fut.result(); |
761 | + auto item = call(root->get(root->native_identity())); |
762 | EXPECT_NE(nullptr, dynamic_pointer_cast<Root>(item)); |
763 | EXPECT_TRUE(root->equal_to(item)); |
764 | |
765 | // Free and used space can be anything, but must be > 0. |
766 | - auto free_space_fut = root->free_space_bytes(); |
767 | - { |
768 | - QFutureWatcher<int64_t> w; |
769 | - QSignalSpy spy(&w, &decltype(w)::finished); |
770 | - w.setFuture(free_space_fut); |
771 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
772 | - } |
773 | - auto free_space = free_space_fut.result(); |
774 | + auto free_space = call(root->free_space_bytes()); |
775 | cerr << "bytes free: " << free_space << endl; |
776 | EXPECT_GT(free_space, 0); |
777 | |
778 | - auto used_space_fut = root->used_space_bytes(); |
779 | - { |
780 | - QFutureWatcher<int64_t> w; |
781 | - QSignalSpy spy(&w, &decltype(w)::finished); |
782 | - w.setFuture(free_space_fut); |
783 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
784 | - } |
785 | - auto used_space = used_space_fut.result(); |
786 | + auto used_space = call(root->used_space_bytes()); |
787 | cerr << "bytes used: " << used_space << endl; |
788 | EXPECT_GT(used_space, 0); |
789 | } |
790 | @@ -269,48 +210,20 @@ |
791 | auto root = get_root(runtime); |
792 | clear_folder(root); |
793 | |
794 | - auto list_fut = root->list(); |
795 | - { |
796 | - QFutureWatcher<QVector<Item::SPtr>> w; |
797 | - QSignalSpy spy(&w, &decltype(w)::finished); |
798 | - w.setFuture(list_fut); |
799 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
800 | - } |
801 | - auto items = list_fut.result(); |
802 | + auto items = call(root->list()); |
803 | ASSERT_EQ(1, items.size()); |
804 | |
805 | // Create a file and check that it was created with correct type, name, and size 0. |
806 | - auto create_file_fut = root->create_file("file1", 0); |
807 | - { |
808 | - QFutureWatcher<Uploader::SPtr> w; |
809 | - QSignalSpy spy(&w, &decltype(w)::finished); |
810 | - w.setFuture(create_file_fut); |
811 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
812 | - } |
813 | - auto uploader = create_file_fut.result(); |
814 | - EXPECT_EQ(0, uploader->size()); |
815 | - auto finish_upload_fut = uploader->finish_upload(); |
816 | - { |
817 | - QFutureWatcher<File::SPtr> w; |
818 | - QSignalSpy spy(&w, &decltype(w)::finished); |
819 | - w.setFuture(finish_upload_fut); |
820 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
821 | - } |
822 | - auto file = finish_upload_fut.result(); |
823 | + auto uploader = call(root->create_file("file1", 10)); |
824 | + EXPECT_EQ(10, uploader->size()); |
825 | + auto file = call(uploader->finish_upload()); |
826 | EXPECT_EQ(ItemType::file, file->type()); |
827 | EXPECT_EQ("some_upload", file->name()); |
828 | - EXPECT_EQ(0, file->size()); |
829 | + EXPECT_EQ(10, file->size()); |
830 | EXPECT_EQ("some_id", file->native_identity()); |
831 | |
832 | // For coverage: getting a file must return the correct one. |
833 | - auto get_fut = root->get("child_id"); |
834 | - { |
835 | - QFutureWatcher<Item::SPtr> w; |
836 | - QSignalSpy spy(&w, &decltype(w)::finished); |
837 | - w.setFuture(get_fut); |
838 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
839 | - } |
840 | - file = dynamic_pointer_cast<File>(get_fut.result()); |
841 | + file = dynamic_pointer_cast<File>(call(root->get("child_id"))); |
842 | EXPECT_EQ("child_id", file->native_identity()); |
843 | EXPECT_EQ("Child", file->name()); |
844 | } |
845 | @@ -323,37 +236,16 @@ |
846 | clear_folder(root); |
847 | |
848 | // Get a file. |
849 | - auto lookup_fut = root->lookup("Child"); |
850 | - { |
851 | - QFutureWatcher<QVector<Item::SPtr>> w; |
852 | - QSignalSpy spy(&w, &decltype(w)::finished); |
853 | - w.setFuture(lookup_fut); |
854 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
855 | - } |
856 | - auto children = lookup_fut.result(); |
857 | + auto children = call(root->lookup("Child")); |
858 | ASSERT_EQ(1, children.size()); |
859 | auto file = dynamic_pointer_cast<File>(children[0]); |
860 | EXPECT_EQ("child_id", file->native_identity()); |
861 | EXPECT_EQ("Child", file->name()); |
862 | |
863 | - auto create_uploader_fut = file->create_uploader(ConflictPolicy::error_if_conflict, 0); |
864 | - { |
865 | - QFutureWatcher<Uploader::SPtr> w; |
866 | - QSignalSpy spy(&w, &decltype(w)::finished); |
867 | - w.setFuture(create_uploader_fut); |
868 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
869 | - } |
870 | - auto uploader = create_uploader_fut.result(); |
871 | + auto uploader = call(file->create_uploader(ConflictPolicy::error_if_conflict, 0)); |
872 | EXPECT_EQ(0, uploader->size()); |
873 | |
874 | - auto finish_upload_fut = uploader->finish_upload(); |
875 | - { |
876 | - QFutureWatcher<File::SPtr> w; |
877 | - QSignalSpy spy(&w, &decltype(w)::finished); |
878 | - w.setFuture(finish_upload_fut); |
879 | - ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME)); |
880 | - } |
881 | - auto uploaded_file = finish_upload_fut.result(); |
882 | + auto uploaded_file = call(uploader->finish_upload()); |
883 | EXPECT_EQ("some_id", uploaded_file->native_identity()); |
884 | EXPECT_EQ("some_upload", uploaded_file->name()); |
885 | } |
FAILED: Continuous integration, rev:86 /jenkins. canonical. com/unity- api-1/job/ lp-storage- framework- ci/53/ /jenkins. canonical. com/unity- api-1/job/ build/321/ console /jenkins. canonical. com/unity- api-1/job/ build-0- fetch/327 /jenkins. canonical. com/unity- api-1/job/ build-1- sourcepkg/ release= vivid+overlay/ 254 /jenkins. canonical. com/unity- api-1/job/ build-1- sourcepkg/ release= xenial+ overlay/ 254 /jenkins. canonical. com/unity- api-1/job/ build-1- sourcepkg/ release= yakkety/ 254 /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=amd64, release= yakkety/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=armhf, release= yakkety/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 183/console /jenkins. canonical. com/unity- api-1/job/ build-2- binpkg/ arch=i386, release= yakkety/ 183/console
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild: /jenkins. canonical. com/unity- api-1/job/ lp-storage- framework- ci/53/rebuild
https:/