Storage Framework

Merge lp:~michihenning/storage-framework/bug1644577 into lp:storage-framework/devel

  1. bug1644577
  2. Merge into devel
Proposed by Michi Henning
Status: Merged
Approved by: Michi Henning
Approved revision: 97
Merged at revision: 96
Proposed branch: lp:~michihenning/storage-framework/bug1644577
Merge into: lp:storage-framework/devel
Diff against target: 537 lines (+161/-118)
6 files modified
src/qt/internal/AccountImpl.cpp (+2/-1)
src/qt/internal/ItemImpl.cpp (+10/-8)
src/qt/internal/ItemListJobImpl.cpp (+4/-6)
src/qt/internal/MultiItemListJobImpl.cpp (+1/-4)
src/qt/internal/validate.cpp (+75/-63)
tests/remote-client/remote-client_test.cpp (+69/-36)
To merge this branch: bzr merge lp:~michihenning/storage-framework/bug1644577
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Approve
James Henstridge Approve
Review via email: mp+311788@code.launchpad.net

Commit message

Fix for bug 1644577, fail list job if metadata for any item is incorrect.
Always emit itemsReady(), even if list is empty.
validate() failures now log the error.
Lots of improvements to log messages to provide more detail.

Description of the change

Fix for bug 1644577, fail list job if metadata for any item is incorrect.
Always emit itemsReady(), even if list is empty.
validate() failures now log the error.
Lots of improvements to log messages to provide more detail.

To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

I've left a few inline comments.

Also, do we want to make the "unconditionally emit itemsReady" change to MultiItemListJob too?

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

FAILED: Continuous integration, rev:96
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/220/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1159/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1166
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/953
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/953/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/953/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/953
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/953/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/953/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/953
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/953/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/953/console

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

review: Needs Fixing (continuous-integration)
lp:~michihenning/storage-framework/bug1644577 updated
97. By Michi Henning

Review comments from James.

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

Good catch, thanks!

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 :

FAILED: Continuous integration, rev:97
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/221/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/1164/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1171
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/957
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/957/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/957/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/957
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/957/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/957/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/957
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/957/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/957/console

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

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:97
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/223/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1174
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1181
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/967
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/967/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/967
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/967/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/967
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/967/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/967
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/967/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/967
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/967/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/967
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/967/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/223/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 'src/qt/internal/AccountImpl.cpp'
2--- src/qt/internal/AccountImpl.cpp 2016-11-03 06:41:00 +0000
3+++ src/qt/internal/AccountImpl.cpp 2016-11-25 04:22:53 +0000
4@@ -101,7 +101,8 @@
5 {
6 if (md.type != ItemType::root)
7 {
8- QString msg = method + ": provider returned non-root item type: " + QString::number(int(md.type));
9+ QString msg = method + ": provider returned non-root item type: " + QString::number(int(md.type)) +
10+ " (id = " + md.item_id + ")";
11 qCritical().noquote() << msg;
12 throw StorageErrorImpl::local_comms_error(msg);
13 }
14
15=== modified file 'src/qt/internal/ItemImpl.cpp'
16--- src/qt/internal/ItemImpl.cpp 2016-11-03 03:41:24 +0000
17+++ src/qt/internal/ItemImpl.cpp 2016-11-25 04:22:53 +0000
18@@ -153,7 +153,7 @@
19 {
20 if (md.type == ItemType::file)
21 {
22- QString msg = method + ": provider returned a file as a parent";
23+ QString msg = method + ": provider returned a file as a parent (id = " + md.item_id + ")";
24 qCritical().noquote() << msg;
25 throw StorageErrorImpl::local_comms_error(msg);
26 }
27@@ -178,7 +178,8 @@
28 ||
29 (md_.type != ItemType::file && md.type == ItemType::file))
30 {
31- QString msg = method + ": source and target item type differ";
32+ QString msg = method + "provider error: source and target item type differ (source id = " +
33+ md_.item_id + ", target id = " + md.item_id + ")";
34 qCritical().noquote() << msg;
35 throw StorageErrorImpl::local_comms_error(msg);
36 }
37@@ -203,7 +204,7 @@
38 {
39 if (md.type == ItemType::root)
40 {
41- QString msg = method + ": impossible root item returned by provider";
42+ QString msg = method + ": impossible root item returned by provider (id = " + md.item_id + ")";
43 qCritical().noquote() << msg;
44 throw StorageErrorImpl::local_comms_error(msg);
45 }
46@@ -211,7 +212,8 @@
47 ||
48 (md_.type != ItemType::file && md.type == ItemType::file))
49 {
50- QString msg = method + ": provider error: source and target item type differ";
51+ QString msg = method + ": provider error: source and target item type differ (source id = " +
52+ md_.item_id + ", target id = " + md.item_id + ")";
53 qCritical().noquote() << msg;
54 throw StorageErrorImpl::local_comms_error(msg);
55 }
56@@ -266,7 +268,7 @@
57 {
58 if (md.type != storage::ItemType::file)
59 {
60- QString msg = method + ": impossible folder item returned by provider";
61+ QString msg = method + ": impossible folder item returned by provider (id = " + md.item_id + ")";
62 qCritical().noquote() << msg;
63 throw StorageErrorImpl::local_comms_error(msg);
64 }
65@@ -318,7 +320,7 @@
66 {
67 if (md.type == storage::ItemType::root)
68 {
69- QString msg = method + ": impossible root item returned by provider";
70+ QString msg = method + ": impossible root item returned by provider (id = " + md.item_id + ")";
71 qCritical().noquote() << msg;
72 throw StorageErrorImpl::local_comms_error(msg);
73 }
74@@ -379,7 +381,7 @@
75 {
76 return;
77 }
78- QString msg = method + ": impossible file item returned by provider";
79+ QString msg = method + ": impossible file item returned by provider (id = " + md.item_id + ")";
80 qCritical().noquote() << msg;
81 throw StorageErrorImpl::local_comms_error(msg);
82 };
83@@ -423,7 +425,7 @@
84 {
85 if (md.type != storage::ItemType::file)
86 {
87- QString msg = method + ": impossible folder item returned by provider";
88+ QString msg = method + ": impossible folder item returned by provider (id = " + md.item_id + ")";
89 qCritical().noquote() << msg;
90 throw StorageErrorImpl::local_comms_error(msg);
91 }
92
93=== modified file 'src/qt/internal/ItemListJobImpl.cpp'
94--- src/qt/internal/ItemListJobImpl.cpp 2016-10-12 07:23:15 +0000
95+++ src/qt/internal/ItemListJobImpl.cpp 2016-11-25 04:22:53 +0000
96@@ -62,16 +62,14 @@
97 auto item = ItemImpl::make_item(method_, md, account_impl_);
98 items.append(item);
99 }
100- catch (StorageError const&)
101+ catch (StorageError const& e)
102 {
103 // Bad metadata received from provider, validate_() or make_item() have logged it.
104+ error_ = e;
105 }
106 }
107- status_ = ItemListJob::Status::Finished;
108- if (!items.isEmpty())
109- {
110- Q_EMIT public_instance_->itemsReady(items);
111- }
112+ status_ = error_.type() == StorageError::NoError ? ItemListJob::Finished : ItemListJob::Error;
113+ Q_EMIT public_instance_->itemsReady(items);
114 Q_EMIT public_instance_->statusChanged(status_);
115 };
116
117
118=== modified file 'src/qt/internal/MultiItemListJobImpl.cpp'
119--- src/qt/internal/MultiItemListJobImpl.cpp 2016-10-12 07:23:15 +0000
120+++ src/qt/internal/MultiItemListJobImpl.cpp 2016-11-25 04:22:53 +0000
121@@ -86,10 +86,7 @@
122 {
123 status_ = ItemListJob::Status::Finished;
124 }
125- if (!items.isEmpty())
126- {
127- Q_EMIT public_instance_->itemsReady(items);
128- }
129+ Q_EMIT public_instance_->itemsReady(items);
130 if (token.isEmpty())
131 {
132 Q_EMIT public_instance_->statusChanged(status_);
133
134=== modified file 'src/qt/internal/validate.cpp'
135--- src/qt/internal/validate.cpp 2016-11-02 06:06:32 +0000
136+++ src/qt/internal/validate.cpp 2016-11-25 04:22:53 +0000
137@@ -110,69 +110,81 @@
138 {
139 using namespace unity::storage::metadata;
140
141- QString prefix = method + ": received invalid metadata from provider: ";
142-
143- // Basic sanity checks for mandatory fields.
144- if (md.item_id.isEmpty())
145- {
146- throw StorageErrorImpl::local_comms_error(prefix + "item_id cannot be empty");
147- }
148- if (md.type != ItemType::root)
149- {
150- if (md.parent_ids.isEmpty())
151- {
152- throw StorageErrorImpl::local_comms_error(prefix + "file or folder must have at least one parent ID");
153- }
154- for (int i = 0; i < md.parent_ids.size(); ++i)
155- {
156- if (md.parent_ids.at(i).isEmpty())
157- {
158- throw StorageErrorImpl::local_comms_error(prefix + "parent_id of file or folder cannot be empty");
159- }
160- }
161- }
162- if (md.type == ItemType::root && !md.parent_ids.isEmpty())
163- {
164- throw StorageErrorImpl::local_comms_error(prefix + "parent_ids of root must be empty");
165- }
166- if (md.type != ItemType::root) // Dropbox does not support metadata for roots.
167- {
168- if (md.name.isEmpty())
169- {
170- throw StorageErrorImpl::local_comms_error(prefix + "name cannot be empty");
171- }
172- }
173- if (md.type == ItemType::file && md.etag.isEmpty()) // WebDav doesn't do etag for folders.
174- {
175- throw StorageErrorImpl::local_comms_error(prefix + "etag of a file cannot be empty");
176- }
177-
178- // Sanity check metadata to make sure only known metadata keys appear.
179- QMapIterator<QString, QVariant> actual(md.metadata);
180- while (actual.hasNext())
181- {
182- actual.next();
183- auto known = known_metadata.find(actual.key().toStdString());
184- if (known == known_metadata.end())
185- {
186- qWarning().noquote().nospace() << prefix << "unknown metadata key: \"" << actual.key() << "\"";
187- }
188- else
189- {
190- validate_type_and_value(prefix, actual, known);
191- }
192- }
193-
194- // Sanity check metadata to make sure that mandatory fields are present.
195- if (md.type == ItemType::file)
196- {
197- if (!md.metadata.contains(metadata::SIZE_IN_BYTES) ||
198- !md.metadata.contains(metadata::LAST_MODIFIED_TIME))
199- {
200- QString msg = prefix + "missing key \"" + metadata::SIZE_IN_BYTES + "\" "
201- "in metadata for \"" + md.item_id + "\"";
202- throw StorageErrorImpl::local_comms_error(msg);
203- }
204+ QString prefix = method + ": received invalid metadata from provider";
205+ if (!md.item_id.isEmpty())
206+ {
207+ prefix += " (id = " + md.item_id + ")";
208+ }
209+ prefix += ": ";
210+
211+ try
212+ {
213+ // Basic sanity checks for mandatory fields.
214+ if (md.item_id.isEmpty())
215+ {
216+ throw StorageErrorImpl::local_comms_error(prefix + "item_id cannot be empty");
217+ }
218+ if (md.type != ItemType::root)
219+ {
220+ if (md.parent_ids.isEmpty())
221+ {
222+ throw StorageErrorImpl::local_comms_error(prefix + "file or folder must have at least one parent ID");
223+ }
224+ for (int i = 0; i < md.parent_ids.size(); ++i)
225+ {
226+ if (md.parent_ids.at(i).isEmpty())
227+ {
228+ throw StorageErrorImpl::local_comms_error(prefix + "parent_id of file or folder cannot be empty");
229+ }
230+ }
231+ }
232+ if (md.type == ItemType::root && !md.parent_ids.isEmpty())
233+ {
234+ throw StorageErrorImpl::local_comms_error(prefix + "parent_ids of root must be empty");
235+ }
236+ if (md.type != ItemType::root) // Dropbox does not support metadata for roots.
237+ {
238+ if (md.name.isEmpty())
239+ {
240+ throw StorageErrorImpl::local_comms_error(prefix + "name cannot be empty");
241+ }
242+ }
243+ if (md.type == ItemType::file && md.etag.isEmpty()) // WebDav doesn't do etag for folders.
244+ {
245+ throw StorageErrorImpl::local_comms_error(prefix + "etag of a file cannot be empty");
246+ }
247+
248+ // Sanity check metadata to make sure only known metadata keys appear.
249+ QMapIterator<QString, QVariant> actual(md.metadata);
250+ while (actual.hasNext())
251+ {
252+ actual.next();
253+ auto known = known_metadata.find(actual.key().toStdString());
254+ if (known == known_metadata.end())
255+ {
256+ qWarning().noquote().nospace() << prefix << "unknown metadata key: \"" << actual.key() << "\"";
257+ }
258+ else
259+ {
260+ validate_type_and_value(prefix, actual, known);
261+ }
262+ }
263+
264+ // Sanity check metadata to make sure that mandatory fields are present.
265+ if (md.type == ItemType::file)
266+ {
267+ if (!md.metadata.contains(metadata::SIZE_IN_BYTES) ||
268+ !md.metadata.contains(metadata::LAST_MODIFIED_TIME))
269+ {
270+ QString msg = prefix + "missing key \"" + metadata::SIZE_IN_BYTES + "\" in metadata";
271+ throw StorageErrorImpl::local_comms_error(msg);
272+ }
273+ }
274+ }
275+ catch (StorageError const& e)
276+ {
277+ qCritical().noquote() << e.errorString();
278+ throw;
279 }
280 }
281
282
283=== modified file 'tests/remote-client/remote-client_test.cpp'
284--- tests/remote-client/remote-client_test.cpp 2016-11-04 07:33:07 +0000
285+++ tests/remote-client/remote-client_test.cpp 2016-11-25 04:22:53 +0000
286@@ -504,11 +504,23 @@
287 QSignalSpy ready_spy(j.get(), &ItemListJob::itemsReady);
288 QSignalSpy status_spy(j.get(), &ItemListJob::statusChanged);
289 status_spy.wait(SIGNAL_WAIT_TIME);
290- auto arg = status_spy.takeFirst();
291-
292- // Bad metadata is ignored, so status is finished, and itemsReady was never called.
293- EXPECT_EQ(ItemListJob::Status::Finished, qvariant_cast<ItemListJob::Status>(arg.at(0)));
294- EXPECT_EQ(0, status_spy.count());
295+ {
296+ auto arg = status_spy.takeFirst();
297+ EXPECT_EQ(ItemListJob::Status::Error, qvariant_cast<ItemListJob::Status>(arg.at(0)));
298+ }
299+
300+ if (ready_spy.count() != 1)
301+ {
302+ ready_spy.wait(SIGNAL_WAIT_TIME);
303+ }
304+ auto arg = ready_spy.takeFirst();
305+ auto items = qvariant_cast<QList<Item>>(arg.at(0));
306+ EXPECT_EQ(0, items.size());
307+
308+ EXPECT_EQ(ItemListJob::Status::Error, j->status());
309+ EXPECT_EQ(StorageError::Type::LocalCommsError, j->error().type());
310+ EXPECT_EQ("LocalCommsError: Account::roots(): provider returned non-root item type: 0 (id = root_id)",
311+ j->error().errorString());
312 }
313
314 TEST_F(GetTest, basic)
315@@ -679,8 +691,9 @@
316 spy.wait(SIGNAL_WAIT_TIME);
317
318 EXPECT_EQ(ItemJob::Status::Error, j->status());
319- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
320- "file or folder must have at least one parent ID", j->error().errorString());
321+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
322+ "file or folder must have at least one parent ID"
323+ , j->error().errorString());
324 }
325
326 TEST_F(MetadataTest, empty_parent)
327@@ -693,8 +706,9 @@
328 spy.wait(SIGNAL_WAIT_TIME);
329
330 EXPECT_EQ(ItemJob::Status::Error, j->status());
331- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
332- "parent_id of file or folder cannot be empty", j->error().errorString());
333+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
334+ "parent_id of file or folder cannot be empty",
335+ j->error().errorString());
336 }
337
338 TEST_F(MetadataTest, root_with_parent)
339@@ -707,8 +721,9 @@
340 spy.wait(SIGNAL_WAIT_TIME);
341
342 EXPECT_EQ(ItemJob::Status::Error, j->status());
343- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
344- "parent_ids of root must be empty", j->error().errorString());
345+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = root_id): "
346+ "parent_ids of root must be empty",
347+ j->error().errorString());
348 }
349
350 TEST_F(MetadataTest, empty_name)
351@@ -721,8 +736,9 @@
352 spy.wait(SIGNAL_WAIT_TIME);
353
354 EXPECT_EQ(ItemJob::Status::Error, j->status());
355- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
356- "name cannot be empty", j->error().errorString());
357+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
358+ "name cannot be empty",
359+ j->error().errorString());
360 }
361
362 TEST_F(MetadataTest, empty_etag)
363@@ -735,8 +751,9 @@
364 spy.wait(SIGNAL_WAIT_TIME);
365
366 EXPECT_EQ(ItemJob::Status::Error, j->status());
367- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
368- "etag of a file cannot be empty", j->error().errorString());
369+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
370+ "etag of a file cannot be empty",
371+ j->error().errorString());
372 }
373
374 TEST_F(MetadataTest, unknown_key)
375@@ -762,8 +779,9 @@
376 spy.wait(SIGNAL_WAIT_TIME);
377
378 EXPECT_EQ(ItemJob::Status::Error, j->status());
379- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
380- "missing key \"size_in_bytes\" in metadata for \"child_id\"", j->error().errorString());
381+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
382+ "missing key \"size_in_bytes\" in metadata",
383+ j->error().errorString());
384 }
385
386 TEST_F(MetadataTest, wrong_type_for_time)
387@@ -776,8 +794,9 @@
388 spy.wait(SIGNAL_WAIT_TIME);
389
390 EXPECT_EQ(ItemJob::Status::Error, j->status());
391- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: last_modified_time: "
392- "expected value of type QString, but received value of type qlonglong", j->error().errorString());
393+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
394+ "last_modified_time: expected value of type QString, but received value of type qlonglong",
395+ j->error().errorString());
396 }
397
398 TEST_F(MetadataTest, bad_parse_for_time)
399@@ -790,8 +809,9 @@
400 spy.wait(SIGNAL_WAIT_TIME);
401
402 EXPECT_EQ(ItemJob::Status::Error, j->status());
403- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: last_modified_time: "
404- "value \"xyz\" does not parse as ISO-8601 date", j->error().errorString());
405+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
406+ "last_modified_time: value \"xyz\" does not parse as ISO-8601 date",
407+ j->error().errorString());
408 }
409
410 TEST_F(MetadataTest, missing_timezone)
411@@ -804,8 +824,9 @@
412 spy.wait(SIGNAL_WAIT_TIME);
413
414 EXPECT_EQ(ItemJob::Status::Error, j->status());
415- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: last_modified_time: "
416- "value \"2007-04-05T14:30\" lacks a time zone specification", j->error().errorString());
417+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
418+ "last_modified_time: value \"2007-04-05T14:30\" lacks a time zone specification",
419+ j->error().errorString());
420 }
421
422 TEST_F(MetadataTest, wrong_type_for_size)
423@@ -818,8 +839,9 @@
424 spy.wait(SIGNAL_WAIT_TIME);
425
426 EXPECT_EQ(ItemJob::Status::Error, j->status());
427- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: size_in_bytes: "
428- "expected value of type qlonglong, but received value of type QString", j->error().errorString());
429+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
430+ "size_in_bytes: expected value of type qlonglong, but received value of type QString",
431+ j->error().errorString());
432 }
433
434 TEST_F(MetadataTest, negative_size)
435@@ -832,8 +854,9 @@
436 spy.wait(SIGNAL_WAIT_TIME);
437
438 EXPECT_EQ(ItemJob::Status::Error, j->status());
439- EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: size_in_bytes: "
440- "expected value >= 0, but received -1", j->error().errorString());
441+ EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider (id = child_id): "
442+ "size_in_bytes: expected value >= 0, but received -1",
443+ j->error().errorString());
444 }
445
446 TEST_F(DeleteTest, basic)
447@@ -1485,7 +1508,7 @@
448 auto arg = spy.takeFirst();
449 EXPECT_EQ(ItemListJob::Status::Error, qvariant_cast<ItemListJob::Status>(arg.at(0)));
450
451- EXPECT_EQ("Item::parents(): provider returned a file as a parent", j->error().message());
452+ EXPECT_EQ("Item::parents(): provider returned a file as a parent (id = root_id)", j->error().message());
453 }
454 }
455
456@@ -1751,7 +1774,9 @@
457 EXPECT_FALSE(j->isValid());
458 EXPECT_EQ(ItemJob::Status::Error, j->status());
459 EXPECT_EQ(StorageError::Type::LocalCommsError, j->error().type());
460- EXPECT_EQ("LocalCommsError: Item::copy(): source and target item type differ", j->error().errorString());
461+ EXPECT_EQ("LocalCommsError: Item::copy()provider error: source and target item type differ "
462+ "(source id = child_id, target id = new_item_id)",
463+ j->error().errorString());
464 }
465
466 TEST_F(MoveTest, basic)
467@@ -1863,7 +1888,8 @@
468 EXPECT_EQ(ItemJob::Status::Error, j->status());
469 EXPECT_EQ(StorageError::Type::LocalCommsError, j->error().type());
470 EXPECT_EQ("LocalCommsError", j->error().name());
471- EXPECT_EQ("LocalCommsError: Item::move(): impossible root item returned by provider", j->error().errorString());
472+ EXPECT_EQ("LocalCommsError: Item::move(): impossible root item returned by provider (id = root_id)",
473+ j->error().errorString());
474 }
475
476 TEST_F(MoveTest, type_mismatch)
477@@ -1899,7 +1925,8 @@
478 EXPECT_FALSE(j->isValid());
479 EXPECT_EQ(ItemJob::Status::Error, j->status());
480 EXPECT_EQ(StorageError::Type::LocalCommsError, j->error().type());
481- EXPECT_EQ("LocalCommsError: Item::move(): provider error: source and target item type differ",
482+ EXPECT_EQ("LocalCommsError: Item::move(): provider error: source and target item type differ "
483+ "(source id = child_id, target id = child_id)",
484 j->error().errorString());
485 }
486
487@@ -2094,7 +2121,7 @@
488 auto status = qvariant_cast<ItemJob::Status>(arg.at(0));
489 EXPECT_EQ(ItemJob::Status::Error, status);
490
491- EXPECT_EQ("LocalCommsError: Item::createFolder(): impossible file item returned by provider",
492+ EXPECT_EQ("LocalCommsError: Item::createFolder(): impossible file item returned by provider (id = new_folder_id)",
493 j->error().errorString());
494 }
495
496@@ -2157,7 +2184,10 @@
497 QSignalSpy ready_spy(j.get(), &ItemListJob::itemsReady);
498 QSignalSpy status_spy(j.get(), &ItemListJob::statusChanged);
499 status_spy.wait(SIGNAL_WAIT_TIME);
500- ASSERT_EQ(0, ready_spy.count());
501+ if (ready_spy.count() == 0)
502+ {
503+ ASSERT_TRUE(ready_spy.wait(SIGNAL_WAIT_TIME));
504+ }
505
506 EXPECT_EQ(ItemListJob::Status::Finished, j->status());
507
508@@ -2303,7 +2333,8 @@
509 auto arg = spy.takeFirst();
510 EXPECT_EQ(ItemListJob::Status::Error, qvariant_cast<ItemListJob::Status>(arg.at(0)));
511
512- EXPECT_EQ("LocalCommsError: Item::list(): impossible root item returned by provider", j->error().errorString());
513+ EXPECT_EQ("LocalCommsError: Item::list(): impossible root item returned by provider (id = child_id)",
514+ j->error().errorString());
515 }
516
517 TEST_F(ListTest, runtime_destroyed_while_item_list_job_running)
518@@ -3370,7 +3401,8 @@
519
520 EXPECT_EQ(Uploader::Status::Error, uploader->status());
521 EXPECT_EQ(StorageError::Type::LocalCommsError, uploader->error().type());
522- EXPECT_EQ("Item::createUploader(): impossible folder item returned by provider", uploader->error().message());
523+ EXPECT_EQ("Item::createUploader(): impossible folder item returned by provider (id = some_id)",
524+ uploader->error().message());
525 }
526
527 TEST_F(UploadTest, cancel_success)
528@@ -3702,7 +3734,8 @@
529 EXPECT_EQ(Uploader::Status::Error, qvariant_cast<Uploader::Status>(arg.at(0)));
530
531 EXPECT_EQ(StorageError::Type::LocalCommsError, uploader->error().type());
532- EXPECT_EQ("Item::createFile(): impossible folder item returned by provider", uploader->error().message());
533+ EXPECT_EQ("Item::createFile(): impossible folder item returned by provider (id = some_id)",
534+ uploader->error().message());
535 }
536
537 TEST_F(CreateFileTest, exists)

Subscribers

People subscribed via source and target branches

to all changes: