Merge lp:~mandel/ubuntu-download-manager/throw-error-exists into lp:ubuntu-download-manager

[Manuel de la Peña]

Deal with the presence of the download file in a nicer way.

Deal with the presence of the download file in a nicer way. This branch takes care of two possible cases:

1. Local file and not confined app => returns a dbus error.
2. All other cases (either confined or not but with the local path) => appends uuid at the end of the file.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:133
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/78/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/78
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/69

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-download-manager-ci/78/rebuild

review: Approve (continuous-integration)

[lp:~mandel/ubuntu-download-manager/throw-error-exists]

134. By Manuel de la Peña on 2013-09-25

Merged use-standard-dirs into throw-error-exists.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:134
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/79/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/79
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/70

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-download-manager-ci/79/rebuild

review: Approve (continuous-integration)

[lp:~mandel/ubuntu-download-manager/throw-error-exists]

135. By Manuel de la Peña on 2013-09-25

Merged use-standard-dirs into throw-error-exists.

136. By Manuel de la Peña on 2013-09-30

Merged with trunk.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:136
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/97/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/97
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/88

Click here to trigger a rebuild:
http://s-jenkins:8080/job/ubuntu-download-manager-ci/97/rebuild

review: Approve (continuous-integration)

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

=== modified file 'libubuntudownloadmanager/group_download.cpp'
--- libubuntudownloadmanager/group_download.cpp	2013-09-27 21:56:44 +0000
+++ libubuntudownloadmanager/group_download.cpp	2013-09-30 11:43:31 +0000
@@ -78,15 +78,6 @@
             QUrl url(download.getUrl());
             QString hash = download.getHash();
 
-            // before we create the download ensure that the url is valid
-            // if not we set the error and stop the loop
-            if (!url.isValid()) {
-                q->setIsValid(false);
-                q->setLastError(QString("Invalid URL: '%1'").arg(
-                    url.toString()));
-                break;
-            }
-
             SingleDownload* singleDownload;
             QVariantMap downloadMetadata = QVariantMap(metadata);
             downloadMetadata[LOCAL_PATH_KEY] = download.getLocalFile();
@@ -112,6 +103,14 @@
                         headers));
             }
 
+            // check that the download is valid, if it is not set to invalid
+            // and use the last error from the download
+            if (!singleDownload->isValid()) {
+                q->setIsValid(false);
+                q->setLastError(singleDownload->lastError());
+                break;
+            }
+
             singleDownload->allowGSMDownload(isGSMDownloadAllowed);
             _downloads.append(singleDownload);
             _downloadsProgress[singleDownload->url()] =

=== modified file 'libubuntudownloadmanager/single_download.cpp'
--- libubuntudownloadmanager/single_download.cpp	2013-09-25 09:12:24 +0000
+++ libubuntudownloadmanager/single_download.cpp	2013-09-30 11:43:31 +0000
@@ -28,6 +28,7 @@
 #include "./single_download.h"
 #include "./network_reply.h"
 
+
 #define DATA_FILE_NAME "data.download"
 #define METADATA_FILE_NAME "metadata"
 #define METADATA_COMMAND_KEY "post-download-command"
@@ -417,8 +418,23 @@
         if (!q->isConfined() && metadata.contains(LOCAL_PATH_KEY)) {
             qDebug() << "App is not confined and provided a local path";
             finalPath = metadata[LOCAL_PATH_KEY].toString();
+
+            // in this case and because the app is not confined we are
+            // going to check if the file exists, if it does we will
+            // raise an error
+            if (QFile::exists(finalPath)) {
+                q->setIsValid(false);
+                q->setLastError(QString("File already exists at: '%1'").arg(
+                    finalPath));
+            }
         } else {
             finalPath = q->rootPath() + QDir::separator() + basename;
+
+            // check if the file exists, if it does lets append the uuid to it
+            if (QFile::exists(finalPath)) {
+                finalPath += q->downloadId().toString().replace(
+                    QRegExp("[-{}]"), "");
+            }
         }
 
         return finalPath;

=== modified file 'ubuntu-download-manager-tests/test_download.cpp'
--- ubuntu-download-manager-tests/test_download.cpp	2013-09-27 23:43:33 +0000
+++ ubuntu-download-manager-tests/test_download.cpp	2013-09-30 11:43:31 +0000
@@ -54,7 +54,6 @@
         delete _reqFactory;
     if (_processFactory)
         delete _processFactory;
-
 }
 
 void
@@ -1750,3 +1749,65 @@
         QSharedPointer<ProcessFactory>(_processFactory));
     QVERIFY(download->isValid());
 }
+
+void
+TestDownload::testInvalidFilePresent() {
+    // create a file so that we get an error
+    QString filePath = testDirectory() + QDir::separator() + "test_file.jpg";
+    QFile* file = new QFile(filePath);
+    file->open(QIODevice::ReadWrite | QFile::Append);
+    file->write("data data data!");
+    file->close();
+
+    QVariantMap metadata;
+    metadata["local-path"] = filePath;
+
+    SingleDownload* download = new SingleDownload(_id, _path, false,
+        _rootPath, _url, metadata, _headers,
+        QSharedPointer<SystemNetworkInfo>(_networkInfo),
+        QSharedPointer<RequestFactory>(_reqFactory),
+        QSharedPointer<ProcessFactory>(_processFactory));
+    QVERIFY(!download->isValid());
+}
+
+void
+TestDownload::testValidFileNotPresent() {
+    QString filePath = testDirectory() + QDir::separator() + "test_file.jpg";
+
+    QVariantMap metadata;
+    metadata["local-path"] = filePath;
+
+    SingleDownload* download = new SingleDownload(_id, _path, false,
+        _rootPath, _url, metadata, _headers,
+        QSharedPointer<SystemNetworkInfo>(_networkInfo),
+        QSharedPointer<RequestFactory>(_reqFactory),
+        QSharedPointer<ProcessFactory>(_processFactory));
+    QVERIFY(download->isValid());
+}
+
+void
+TestDownload::testDownloadPresent() {
+    // create a download and get the filename to use, then write it
+    // and create the same download and assert that the filename is diff
+
+    SingleDownload* download = new SingleDownload(_id, _path, true,
+        _rootPath, _url, _metadata, _headers,
+        QSharedPointer<SystemNetworkInfo>(_networkInfo),
+        QSharedPointer<RequestFactory>(_reqFactory),
+        QSharedPointer<ProcessFactory>(_processFactory));
+
+    QString filePath = download->filePath();
+
+    QFile* file = new QFile(filePath);
+    file->open(QIODevice::ReadWrite | QFile::Append);
+    file->write("data data data!");
+    file->close();
+
+    download = new SingleDownload(_id, _path, true,
+        _rootPath, _url, _metadata, _headers,
+        QSharedPointer<SystemNetworkInfo>(_networkInfo),
+        QSharedPointer<RequestFactory>(_reqFactory),
+        QSharedPointer<ProcessFactory>(_processFactory));
+
+    QVERIFY(filePath != download->filePath());
+}

=== modified file 'ubuntu-download-manager-tests/test_download.h'
--- ubuntu-download-manager-tests/test_download.h	2013-09-25 10:30:57 +0000
+++ ubuntu-download-manager-tests/test_download.h	2013-09-30 11:43:31 +0000
@@ -130,6 +130,11 @@
     void testInvalidHashAlgorithm();
     void testValidHashAlgorithm_data();
     void testValidHashAlgorithm();
+    void testInvalidFilePresent();
+    void testValidFileNotPresent();
+
+    // filename tests
+    void testDownloadPresent();
 
  private:
     QUuid _id;

=== modified file 'ubuntu-download-manager-tests/test_group_download.cpp'
--- ubuntu-download-manager-tests/test_group_download.cpp	2013-09-27 23:43:33 +0000
+++ ubuntu-download-manager-tests/test_group_download.cpp	2013-09-30 11:43:31 +0000
@@ -856,3 +856,49 @@
 
     QVERIFY(group->isValid());
 }
+
+void
+TestGroupDownload::testInvalidFilePresent() {
+    QString filePath = testDirectory() + QDir::separator() + "test_file.jpg";
+    QFile* file = new QFile(filePath);
+    file->open(QIODevice::ReadWrite | QFile::Append);
+    file->write("data data data!");
+    file->close();
+
+    QList<GroupDownloadStruct> downloadsStruct;
+    downloadsStruct.append(GroupDownloadStruct("http://one.ubuntu.com",
+        "local_file", ""));
+    downloadsStruct.append(GroupDownloadStruct("http://ubuntu.com",
+        filePath, ""));
+    downloadsStruct.append(GroupDownloadStruct("http://reddit.com",
+        "other_reddit_local_file", ""));
+
+    GroupDownload* group = new GroupDownload(_id, _path, false, _rootPath,
+        downloadsStruct, "md5", _isGSMDownloadAllowed, _metadata, _headers,
+        QSharedPointer<SystemNetworkInfo>(_networkInfo),
+        QSharedPointer<DownloadFactory>(_downloadFactory),
+        QSharedPointer<FileManager>(_fileManager));
+
+    QVERIFY(!group->isValid());
+}
+
+void
+TestGroupDownload::testValidFileNotPresent() {
+    QString filePath = testDirectory() + QDir::separator() + "test_file.jpg";
+
+    QList<GroupDownloadStruct> downloadsStruct;
+    downloadsStruct.append(GroupDownloadStruct("http://one.ubuntu.com",
+        "local_file", ""));
+    downloadsStruct.append(GroupDownloadStruct("http://ubuntu.com",
+        filePath, ""));
+    downloadsStruct.append(GroupDownloadStruct("http://reddit.com",
+        "other_reddit_local_file", ""));
+
+    GroupDownload* group = new GroupDownload(_id, _path, false, _rootPath,
+        downloadsStruct, "md5", _isGSMDownloadAllowed, _metadata, _headers,
+        QSharedPointer<SystemNetworkInfo>(_networkInfo),
+        QSharedPointer<DownloadFactory>(_downloadFactory),
+        QSharedPointer<FileManager>(_fileManager));
+
+    QVERIFY(group->isValid());
+}

=== modified file 'ubuntu-download-manager-tests/test_group_download.h'
--- ubuntu-download-manager-tests/test_group_download.h	2013-09-25 10:30:57 +0000
+++ ubuntu-download-manager-tests/test_group_download.h	2013-09-30 11:43:31 +0000
@@ -75,6 +75,8 @@
     void testInvalidHashAlgorithm();
     void testValidHashAlgorithm_data();
     void testValidHashAlgorithm();
+    void testInvalidFilePresent();
+    void testValidFileNotPresent();
 
  private:
     QUuid _id;