Keeper

Merge lp:~xavi-garcia-mena/keeper/sf-error-handling-cleanup-checking-state into lp:keeper

  1. sf-error-handling-cleanup-checking-state
  2. Merge into trunk
Proposed by Xavi Garcia
Status: Merged
Approved by: Charles Kerr
Approved revision: 90
Merged at revision: 90
Proposed branch: lp:~xavi-garcia-mena/keeper/sf-error-handling-cleanup-checking-state
Merge into: lp:keeper
Diff against target: 189 lines (+26/-67)
6 files modified
src/service/keeper.cpp (+9/-1)
src/storage-framework/storage_framework_client.cpp (+3/-27)
src/storage-framework/storage_framework_client.h (+0/-4)
tests/integration/helpers/helpers-test-failure.cpp (+14/-0)
tests/integration/helpers/test-helpers-base.cpp (+0/-31)
tests/integration/helpers/test-helpers-base.h (+0/-4)
To merge this branch: bzr merge lp:~xavi-garcia-mena/keeper/sf-error-handling-cleanup-checking-state
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
Review via email: mp+302757@code.launchpad.net

Commit message

change storage-framework error handling.

Description of the change

change storage-framework error handling.

The first throw, when we ask for the socket is because we don't want to block the dbus call.
The second is so we can change the state of the task to failed, as the file could not be created in sf, although all data was successfully sent by the helper.

It also adds some state checks in the failure test.

To post a comment you must log in.
lp:~xavi-garcia-mena/keeper/sf-error-handling-cleanup-checking-state updated
90. By Xavi Garcia

Cleanup some unused methods

Revision history for this message
Charles Kerr (charlesk) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/service/keeper.cpp'
2--- src/service/keeper.cpp 2016-08-11 02:39:03 +0000
3+++ src/service/keeper.cpp 2016-08-12 04:14:53 +0000
4@@ -176,7 +176,15 @@
5 task_data_[current_task_].percent_done = 1;
6 set_current_task_action(QStringLiteral("complete"));
7 qDebug() << "Backup helper finished... closing the socket.";
8- storage_->finish(true);
9+ try
10+ {
11+ storage_->finish(true);
12+ }
13+ catch (std::exception & e)
14+ {
15+ qDebug() << "Failed finishing sf... seting the state to failed";
16+ set_current_task_action(QStringLiteral("failed"));
17+ }
18 break;
19 }
20 }
21
22=== modified file 'src/storage-framework/storage_framework_client.cpp'
23--- src/storage-framework/storage_framework_client.cpp 2016-08-11 01:36:33 +0000
24+++ src/storage-framework/storage_framework_client.cpp 2016-08-12 04:14:53 +0000
25@@ -41,7 +41,6 @@
26 {
27 QObject::connect(&uploader_ready_watcher_,&QFutureWatcher<std::shared_ptr<Uploader>>::finished, this, &StorageFrameworkClient::uploaderReady);
28 QObject::connect(&uploader_closed_watcher_,&QFutureWatcher<std::shared_ptr<unity::storage::qt::client::File>>::finished, this, &StorageFrameworkClient::onUploaderClosed);
29-
30 }
31
32
33@@ -68,17 +67,8 @@
34 catch (std::exception & e)
35 {
36 qDebug() << "ERROR: StorageFrameworkClient::getNewFileForBackup():" << e.what();
37- }
38-}
39-
40-int StorageFrameworkClient::getUploaderSocketDescriptor()
41-{
42- if (!uploader_)
43- {
44- return -1;
45- }
46- auto socket = uploader_->socket();
47- return int(socket->socketDescriptor());
48+ throw;
49+ }
50 }
51
52 void StorageFrameworkClient::finish(bool do_commit)
53@@ -98,6 +88,7 @@
54 catch (std::exception & e)
55 {
56 qDebug() << "ERROR: StorageFrameworkClient::closeUploader():" << e.what();
57+ throw;
58 }
59 }
60
61@@ -117,18 +108,3 @@
62
63 Q_EMIT (socketReady(uploader_->socket()));
64 }
65-
66-bool StorageFrameworkClient::deleteFile(std::shared_ptr<unity::storage::qt::client::File> const &file)
67-{
68- try
69- {
70- auto res = file->delete_item();
71- res.waitForFinished();
72- }
73- catch (std::exception & e)
74- {
75- qDebug() << "ERROR: StorageFrameworkClient::deleteFile():" << e.what();
76- return false;
77- }
78- return true;
79-}
80
81=== modified file 'src/storage-framework/storage_framework_client.h'
82--- src/storage-framework/storage_framework_client.h 2016-08-11 01:36:33 +0000
83+++ src/storage-framework/storage_framework_client.h 2016-08-12 04:14:53 +0000
84@@ -38,10 +38,6 @@
85
86 void finish(bool do_commit);
87
88- int getUploaderSocketDescriptor();
89-
90- bool deleteFile(std::shared_ptr<unity::storage::qt::client::File> const &file);
91-
92 Q_SIGNALS:
93 void socketReady(std::shared_ptr<QLocalSocket> const& sf_socket);
94 void finished();
95
96=== modified file 'tests/integration/helpers/helpers-test-failure.cpp'
97--- tests/integration/helpers/helpers-test-failure.cpp 2016-08-10 02:27:40 +0000
98+++ tests/integration/helpers/helpers-test-failure.cpp 2016-08-12 04:14:53 +0000
99@@ -74,6 +74,20 @@
100
101 // check that the content of the file is the expected
102 EXPECT_EQ(0, checkStorageFrameworkNbFiles());
103+
104+ // check that the state is failed
105+ QVariantDictMap state = user_iface->state();
106+
107+ // check that the state has the uuid
108+ QVariantDictMap::const_iterator iter = state.find(user_folder_uuid);
109+ EXPECT_TRUE(iter != state.end());
110+ auto state_values = state[user_folder_uuid];
111+
112+ EXPECT_EQ(state_values["action"].toString(), QStringLiteral("failed"));
113+ EXPECT_EQ(state_values["display-name"].toString(), QStringLiteral("Music"));
114+ // sent 1 byte more than the expected, so percentage has to be greater than 1.0
115+ EXPECT_TRUE(state_values["percent-done"].toFloat() > 1.0);
116+
117 // let's leave things clean
118 EXPECT_TRUE(removeHelperMarkBeforeStarting());
119 }
120
121=== modified file 'tests/integration/helpers/test-helpers-base.cpp'
122--- tests/integration/helpers/test-helpers-base.cpp 2016-08-11 06:55:59 +0000
123+++ tests/integration/helpers/test-helpers-base.cpp 2016-08-12 04:14:53 +0000
124@@ -298,17 +298,6 @@
125 return true;
126 }
127
128-bool TestHelpersBase::checkLastStorageFrameworkFile (QString const & sourceDir, bool compression)
129-{
130- QString lastFile = getLastStorageFrameworkFile();
131- if (lastFile.isEmpty())
132- {
133- qWarning() << "Last file from storage framework is empty";
134- return false;
135- }
136- return compareTarContent (lastFile, sourceDir, compression);
137-}
138-
139 bool TestHelpersBase::compareTarContent (QString const & tarPath, QString const & sourceDir, bool compression)
140 {
141 QTemporaryDir tempDir;
142@@ -404,26 +393,6 @@
143 : -1;
144 }
145
146-bool TestHelpersBase::checkStorageFrameworkContent(QString const & content)
147-{
148- QString lastFile = getLastStorageFrameworkFile();
149- if (lastFile.isEmpty())
150- {
151- qWarning() << "Last file from the storage framework was not found";
152- return false;
153- }
154- QFile storage_framework_file(lastFile);
155- if(!storage_framework_file.open(QFile::ReadOnly))
156- {
157- qWarning() << "ERROR: opening file:" << lastFile;
158- return false;
159- }
160-
161- QString file_content = storage_framework_file.readAll();
162-
163- return file_content == content;
164-}
165-
166 bool TestHelpersBase::removeHelperMarkBeforeStarting()
167 {
168 QFile helper_mark(SIMPLE_HELPER_MARK_FILE_PATH);
169
170=== modified file 'tests/integration/helpers/test-helpers-base.h'
171--- tests/integration/helpers/test-helpers-base.h 2016-08-11 06:55:59 +0000
172+++ tests/integration/helpers/test-helpers-base.h 2016-08-12 04:14:53 +0000
173@@ -102,16 +102,12 @@
174
175 bool checkStorageFrameworkFiles(QStringList const & sourceDirs, bool compression=false);
176
177- bool checkLastStorageFrameworkFile (QString const & sourceDir, bool compression=false);
178-
179 bool compareTarContent (QString const & tarPath, QString const & sourceDir, bool compression);
180
181 bool extractTarContents(QString const & tarPath, QString const & destination, bool compression=false);
182
183 QString getLastStorageFrameworkFile();
184
185- bool checkStorageFrameworkContent(QString const & content);
186-
187 bool removeHelperMarkBeforeStarting();
188
189 bool waitUntilHelperFinishes(QString const & app_id, int maxTimeout = 15000, int times = 1);

Subscribers

People subscribed via source and target branches

to all changes: