Merge lp:~michihenning/storage-framework/marshal-metadata into lp:storage-framework/devel

Proposed by Michi Henning
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
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Approve
James Henstridge Approve
Review via email: mp+302360@code.launchpad.net

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.

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:86
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/53/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/321/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/327
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/254
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/254
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/254
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/183/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/183/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/53/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
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::storage::qt::client::internal::remote_client::validate()? I didn't notice any tests checking that it is correctly interpreting the timestamps and other metadata.

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.

review: Needs Fixing
91. By Michi Henning

Merged devel.

Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:90
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/66/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/353
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/359
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/280
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/280
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/280
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/210/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/210
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/210/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/66/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

Looks good!

review: Approve
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:91
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/67/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/354
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/360
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/281
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/281
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/281
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/211/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/211
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/211/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/67/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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 }

Subscribers

People subscribed via source and target branches

to all changes: