Merge lp:~michael-sheldon/ubuntu-download-manager/fix-1411866 into lp:ubuntu-download-manager

Proposed by Michael Sheldon
Status: Merged
Approved by: Bill Filler
Approved revision: 375
Merged at revision: 372
Proposed branch: lp:~michael-sheldon/ubuntu-download-manager/fix-1411866
Merge into: lp:ubuntu-download-manager
Prerequisite: lp:~michael-sheldon/ubuntu-download-manager/translation-support
Diff against target: 120 lines (+34/-8)
3 files modified
po/ubuntu-download-manager.pot (+14/-1)
src/downloads/priv/ubuntu/downloads/file_download.cpp (+19/-7)
src/downloads/priv/ubuntu/downloads/file_download.h (+1/-0)
To merge this branch: bzr merge lp:~michael-sheldon/ubuntu-download-manager/fix-1411866
Reviewer Review Type Date Requested Status
system-apps-ci-bot continuous-integration Approve
Ubuntu Phablet Team Pending
Review via email: mp+305104@code.launchpad.net

Commit message

Fix queue blocking and error reporting when file path is unwritable

Description of the change

Fix queue blocking and error reporting when file path is unwritable

To post a comment you must log in.
Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

FAILED: Continuous integration, rev:374
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-download-manager-ci/14/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/system-apps/job/build/1436/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1436
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1287/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1287/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1287/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1287/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1287/console
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1287/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1287/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1287/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1287
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1287/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-download-manager-ci/14/rebuild

review: Needs Fixing (continuous-integration)
375. By Michael Sheldon

Don't send start/queued signal when we're about to go to an error state

Revision history for this message
system-apps-ci-bot (system-apps-ci-bot) wrote :

PASSED: Continuous integration, rev:375
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-download-manager-ci/19/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build/1461
    SUCCESS: https://jenkins.canonical.com/system-apps/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=default/343
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-0-fetch/1461
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=amd64,release=yakkety/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=armhf,release=yakkety/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=vivid+overlay/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=xenial+overlay/1311/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1311
        deb: https://jenkins.canonical.com/system-apps/job/build-2-binpkg/arch=i386,release=yakkety/1311/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-download-manager-ci/19/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 'po/ubuntu-download-manager.pot'
2--- po/ubuntu-download-manager.pot 2016-09-09 11:06:56 +0000
3+++ po/ubuntu-download-manager.pot 2016-09-09 11:06:56 +0000
4@@ -8,7 +8,7 @@
5 msgstr ""
6 "Project-Id-Version: ubuntu-download-manager\n"
7 "Report-Msgid-Bugs-To: \n"
8-"POT-Creation-Date: 2016-08-31 12:57+0100\n"
9+"POT-Creation-Date: 2016-09-07 14:08+0100\n"
10 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
11 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
12 "Language-Team: LANGUAGE <LL@li.org>\n"
13@@ -23,35 +23,48 @@
14
15 #: src/downloads/priv/ubuntu/downloads/file_download.cpp:210
16 #: src/downloads/priv/ubuntu/downloads/group_download.cpp:123
17+#: src/downloads/priv/ubuntu/downloads/file_download.cpp:211
18+#: src/downloads/priv/ubuntu/downloads/group_download.cpp:124
19 #, qt-format
20 msgid "Invalid hash algorithm: '%1'"
21 msgstr ""
22
23 #: src/downloads/priv/ubuntu/downloads/file_download.cpp:863
24+#: src/downloads/priv/ubuntu/downloads/file_download.cpp:872
25 #, qt-format
26 msgid "Invalid URL: '%1'"
27 msgstr ""
28
29 #: src/downloads/priv/ubuntu/downloads/file_download.cpp:873
30+#: src/downloads/priv/ubuntu/downloads/file_download.cpp:882
31 #, qt-format
32 msgid "Downloads that are set to be deflated cannot have a hash: '%1'"
33 msgstr ""
34
35 #: src/downloads/priv/ubuntu/downloads/file_download.cpp:964
36+#: src/downloads/priv/ubuntu/downloads/file_download.cpp:973
37 #, qt-format
38 msgid "File already exists at: '%2'"
39 msgstr ""
40
41 #: src/downloads/priv/ubuntu/downloads/group_download.cpp:139
42+#: src/downloads/priv/ubuntu/downloads/group_download.cpp:140
43 msgid "Duplicated local path passed: "
44 msgstr ""
45
46 #: src/uploads/priv/ubuntu/uploads/file_upload.cpp:160
47+#: src/uploads/priv/ubuntu/uploads/file_upload.cpp:161
48 #, qt-format
49 msgid "Path is not absolute: '%1'"
50 msgstr ""
51
52 #: src/uploads/priv/ubuntu/uploads/file_upload.cpp:166
53+#: src/uploads/priv/ubuntu/uploads/file_upload.cpp:167
54 #, qt-format
55 msgid "Path does not exist: '%1'"
56 msgstr ""
57+
58+#: src/downloads/priv/ubuntu/downloads/file_download.cpp:344
59+#, qt-format
60+msgid "Destination file path is not writable: '%2'"
61+msgstr ""
62
63=== modified file 'src/downloads/priv/ubuntu/downloads/file_download.cpp'
64--- src/downloads/priv/ubuntu/downloads/file_download.cpp 2016-09-09 11:06:56 +0000
65+++ src/downloads/priv/ubuntu/downloads/file_download.cpp 2016-09-09 11:06:56 +0000
66@@ -337,7 +337,14 @@
67 bool canWrite = _currentData->open(QIODevice::ReadWrite | QFile::Append);
68
69 if (!canWrite) {
70- emit started(false);
71+ DOWN_LOG(ERROR) << "Destination file path is not writable: " << _filePath;
72+ setIsValid(false);
73+ if (calledFromDBus()) {
74+ sendErrorReply(QDBusError::AccessDenied, QString(_("Destination file path is not writable: '%2'")).arg(
75+ _filePath));
76+ errorCleanup();
77+ setState(Transfer::ERROR);
78+ }
79 return;
80 }
81
82@@ -1206,15 +1213,20 @@
83 return request;
84 }
85
86+void
87+FileDownload::errorCleanup() {
88+ disconnectFromReplySignals();
89+ _reply->deleteLater();
90+ _reply = nullptr;
91+ cleanUpCurrentData();
92+ // let other downloads use the same file name
93+ unlockFilePath();
94+}
95+
96 void
97 FileDownload::emitError(const QString& error) {
98 TRACE << error;
99- disconnectFromReplySignals();
100- _reply->deleteLater();
101- _reply = nullptr;
102- cleanUpCurrentData();
103- // let other downloads use the same file name
104- unlockFilePath();
105+ errorCleanup();
106 Download::emitError(error);
107 }
108
109
110=== modified file 'src/downloads/priv/ubuntu/downloads/file_download.h'
111--- src/downloads/priv/ubuntu/downloads/file_download.h 2015-11-13 14:40:50 +0000
112+++ src/downloads/priv/ubuntu/downloads/file_download.h 2016-09-09 11:06:56 +0000
113@@ -126,6 +126,7 @@
114 void unlockFilePath();
115 void updateFileNamePerContentDisposition();
116 void writeDataUri();
117+ void errorCleanup();
118
119 // slots used to react to signals
120 void onDownloadProgress(qint64 currentProgress, qint64);

Subscribers

People subscribed via source and target branches