Merge lp:~mandel/ubuntu-download-manager/count-ref-cache-management into lp:ubuntu-download-manager

[Manuel de la Peña]

Keep track of the replies so that when the daemon is stoppable we clear the cache.

Keep track of the replies so that when the daemon is stoppable we clear the cache.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:143
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/91/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/91/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/82/console

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

review: Needs Fixing (continuous-integration)

[lp:~mandel/ubuntu-download-manager/count-ref-cache-management]

144. By Manuel de la Peña on 2013-09-27

Merged allow-empty-hash into count-ref-cache-management.

145. By Manuel de la Peña on 2013-09-27

Deal with ssl errors correctly.

[Barry Warsaw (barry)]

Ran the s-i full test suite against this branch and everything passes!

[lp:~mandel/ubuntu-download-manager/count-ref-cache-management]

146. By Manuel de la Peña on 2013-09-27

Remove not needed explicit.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Barry Warsaw (barry)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Alejandro J. Cura (alecu)]

All of the testing code enabled by _stoppable looks like it should belong in FakeRequestFactory, in the testing classes.
Is there a reason why this has to be in the code used for production?

review: Needs Information

[Manuel de la Peña (mandel)]

> All of the testing code enabled by _stoppable looks like it should belong in
> FakeRequestFactory, in the testing classes.
> Is there a reason why this has to be in the code used for production?

This is only used in production if the daemon is started with the -stoppable option that is only used in integration testes where a real daemon is used to perform downloads from a local server. The reason to add this is explained in the linked bug.

[Alejandro J. Cura (alecu)]

+1

review: Approve

=== modified file 'libubuntudownloadmanager/download_manager.cpp'
--- libubuntudownloadmanager/download_manager.cpp	2013-09-25 16:07:02 +0000
+++ libubuntudownloadmanager/download_manager.cpp	2013-09-27 22:32:13 +0000
@@ -47,7 +47,7 @@
         _networkInfo = QSharedPointer<SystemNetworkInfo>(
             new SystemNetworkInfo());
         QSharedPointer<RequestFactory> nam = QSharedPointer<RequestFactory>(
-            new RequestFactory());
+            new RequestFactory(_stoppable));
         _processFactory = QSharedPointer<ProcessFactory>(new ProcessFactory());
         _downloadFactory = QSharedPointer<DownloadFactory>(
             new DownloadFactory(_apparmor, _networkInfo, nam,

=== modified file 'libubuntudownloadmanager/libubuntudownloadmanager.pro'
--- libubuntudownloadmanager/libubuntudownloadmanager.pro	2013-09-09 09:19:11 +0000
+++ libubuntudownloadmanager/libubuntudownloadmanager.pro	2013-09-27 22:32:13 +0000
@@ -10,6 +10,7 @@
 
 SOURCES += \
     application.cpp \
+    apparmor.cpp \
     dbus_connection.cpp \
     download.cpp \
     download_adaptor.cpp \
@@ -35,13 +36,14 @@
     file_manager.cpp \
     download_struct.cpp \
     downloads_db.cpp \
-    dbus_proxy.cpp \
-    apparmor.cpp
+    dbus_proxy.cpp
 
 HEADERS +=\
     app-downloader-lib_global.h \
+    apparmor.h \
     application.h \
     dbus_connection.h \
+    dbus_proxy.h \
     download.h \
     download_adaptor.h \
     download_daemon.h \
@@ -66,9 +68,7 @@
     download_factory.h \
     file_manager.h \
     download_struct.h \
-    downloads_db.h \
-    dbus_proxy.h \
-    apparmor.h
+    downloads_db.h
 
 OTHER_FILES += \
     generate_adaptors.sh \

=== modified file 'libubuntudownloadmanager/request_factory.cpp'
--- libubuntudownloadmanager/request_factory.cpp	2013-09-23 11:17:17 +0000
+++ libubuntudownloadmanager/request_factory.cpp	2013-09-27 22:32:13 +0000
@@ -16,6 +16,7 @@
  * Boston, MA 02110-1301, USA.
  */
 
+#include <QDebug>
 #include <QNetworkAccessManager>
 #include <QSslError>
 #include "./request_factory.h"
@@ -28,18 +29,35 @@
     Q_DECLARE_PUBLIC(RequestFactory)
 
  public:
-    explicit RequestFactoryPrivate(RequestFactory* parent)
-        : q_ptr(parent) {
+    RequestFactoryPrivate(bool stoppable, RequestFactory* parent)
+        : _stoppable(stoppable),
+        q_ptr(parent) {
         _nam = new QNetworkAccessManager();
     }
 
     NetworkReply* get(const QNetworkRequest& request) {
+        Q_Q(RequestFactory);
+
         QNetworkReply* qreply = _nam->get(request);
         NetworkReply* reply = new NetworkReply(qreply);
 
         if (_certs.count() > 0) {
             reply->setAcceptedCertificates(_certs);
         }
+
+        if (_stoppable) {
+            // if the daemon was started as stoppable it means that
+            // we are in testing mode and we do not want to keep
+            // the connections for too long
+            _replies.append(reply);
+
+            q->connect(reply, SIGNAL(error(QNetworkReply::NetworkError)),
+                q, SLOT(onError(QNetworkReply::NetworkError)));
+            q->connect(reply, SIGNAL(finished()),
+                q, SLOT(onFinished()));
+            q->connect(reply, SIGNAL(sslErrors(const QList<QSslError>&)),
+                q, SLOT(onSslErrors(const QList<QSslError>&)));
+        }
         return reply;
     }
 
@@ -51,7 +69,57 @@
         _certs = certs;
     }
 
+    void removeNetworkReply(NetworkReply* reply) {
+        if (_replies.contains(reply)) {
+            _replies.removeAll(reply);
+            // stoppable is not really needed but is better check
+            if (_stoppable && _replies.count() == 0) {
+                qDebug() << "Clearing the connections cache.";
+                _nam->clearAccessCache();
+            }
+        }
+    }
+
+    void onError(QNetworkReply::NetworkError) {
+        Q_Q(RequestFactory);
+        NetworkReply* sender = qobject_cast<NetworkReply*>(q->sender());
+        removeNetworkReply(sender);
+    }
+
+    void onFinished() {
+        Q_Q(RequestFactory);
+        NetworkReply* sender = qobject_cast<NetworkReply*>(q->sender());
+        removeNetworkReply(sender);
+    }
+
+    void onSslErrors(const QList<QSslError>& errors) {
+        Q_Q(RequestFactory);
+        NetworkReply* sender = qobject_cast<NetworkReply*>(q->sender());
+        // only remove the connection and clear the cache if we cannot
+        // ignore the ssl errors!
+
+        foreach(QSslError error, errors) {
+            QSslError::SslError type = error.error();
+            if (type != QSslError::NoError &&
+                type != QSslError::SelfSignedCertificate) {
+                // we only support self signed certificates all errors
+                // will not be ignored
+                qDebug() << "SSL error type not ignored clearing cache";
+                removeNetworkReply(sender);
+            } else if (type == QSslError::SelfSignedCertificate) {
+                // just ignore those errors of the added errors
+                if (!_certs.contains(error.certificate())) {
+                    qDebug() << "SSL certificate not ignored clearing cache";
+                    removeNetworkReply(sender);
+                }
+            }
+       }
+
+    }
+
  private:
+    bool _stoppable = false;
+    QList<NetworkReply*> _replies;
     QList<QSslCertificate> _certs;
     QNetworkAccessManager* _nam;
     RequestFactory* q_ptr;
@@ -61,9 +129,9 @@
  * PUBLIC IMPLEMENTATION
  */
 
-RequestFactory::RequestFactory(QObject *parent)
+RequestFactory::RequestFactory(bool stoppable, QObject *parent)
     : QObject(parent),
-      d_ptr(new RequestFactoryPrivate(this)) {
+      d_ptr(new RequestFactoryPrivate(stoppable, this)) {
 }
 
 NetworkReply*
@@ -83,3 +151,5 @@
     Q_D(RequestFactory);
     d->setAcceptedCertificates(certs);
 }
+
+#include "moc_request_factory.cpp"

=== modified file 'libubuntudownloadmanager/request_factory.h'
--- libubuntudownloadmanager/request_factory.h	2013-09-17 10:37:08 +0000
+++ libubuntudownloadmanager/request_factory.h	2013-09-27 22:32:13 +0000
@@ -31,7 +31,7 @@
     Q_DECLARE_PRIVATE(RequestFactory)
 
  public:
-    explicit RequestFactory(QObject *parent = 0);
+    RequestFactory(bool stoppable = false, QObject *parent = 0);
 
     virtual NetworkReply* get(const QNetworkRequest& request);
 
@@ -40,6 +40,11 @@
     virtual void setAcceptedCertificates(const QList<QSslCertificate>& certs);
 
  private:
+    Q_PRIVATE_SLOT(d_func(), void onError(QNetworkReply::NetworkError))
+    Q_PRIVATE_SLOT(d_func(), void onFinished())
+    Q_PRIVATE_SLOT(d_func(), void onSslErrors(const QList<QSslError>&))
+
+ private:
     // use pimpl so that we can mantains ABI compatibility
     RequestFactoryPrivate* d_ptr;
 };