Merge lp:~mzanetti/reminders-app/no-network into lp:reminders-app

[Michael Zanetti]

Make the app more robust against network outtakes.

This makes the app not crash when there's no network connection at startup.

Basically it moves the part that could fail from setupUserStore() and setupNotesStore() into a separate method connectToEvernote() which just does the open() on the network sockets (also checks the server's version number and prints a note for us that we should update it).

The jobs were already smart enough to reopen the connection on demand. I just added a small check for them to be opened before closing them to avoid the exception bailing out before reaching the open() call.

Now the app should recover automatically if the network fails for a while and comes back after.

Still TODO:
* Expose connection errors to the ui and show them to the user somehow.
* Use QNetworkConfigurationManager to detect when a connection comes up and refresh the notes store.

[lp:~mzanetti/reminders-app/no-network]

111. By Michael Zanetti on 2014-04-30

remove debug prints

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:111
http://91.189.93.70:8080/job/reminders-app-ci/277/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2327
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-saucy-amd64-ci/277
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-trusty-amd64-ci/277

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/277/rebuild

review: Approve (continuous-integration)

[lp:~mzanetti/reminders-app/no-network]

112. By Michael Zanetti on 2014-04-30

flush the pipes when reestablishing the connection

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:112
http://91.189.93.70:8080/job/reminders-app-ci/278/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2328
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-saucy-amd64-ci/278
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-trusty-amd64-ci/278

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/278/rebuild

review: Approve (continuous-integration)

[Riccardo Padovani (rpadovani)]

It's a good start, at least there are no crashes.

As said on IRC, if I disconnect before launch the app it doesn't riconnect, but as said on IRC you will investigate on this in the future and will do another branch

Good work!

review: Approve

=== modified file 'src/plugin/Evernote/evernoteconnection.cpp'
--- src/plugin/Evernote/evernoteconnection.cpp	2014-02-18 14:04:33 +0000
+++ src/plugin/Evernote/evernoteconnection.cpp	2014-04-30 20:46:23 +0000
@@ -61,81 +61,56 @@
     qRegisterMetaType<EvernoteConnection::ErrorCode>("EvernoteConnection::ErrorCode");
     setupUserStore();
     setupNotesStore();
-}
-
-bool EvernoteConnection::setupUserStore()
-{
-    bool versionOK = false;
-    try {
-        boost::shared_ptr<TSocket> socket;
-
-        if (m_useSSL) {
-            boost::shared_ptr<TSSLSocketFactory> sslSocketFactory(new TSSLSocketFactory());
-            socket = sslSocketFactory->createSocket(EVERNOTE_HOST.toStdString(), 443);
-            qDebug() << "created UserStore SSL socket";
-        } else {
-            // Create a non-secure socket
-            socket = boost::shared_ptr<TSocket> (new TSocket(EVERNOTE_HOST.toStdString(), 80));
-            qDebug() << "created insecure UserStore socket";
-        }
-
-        boost::shared_ptr<TBufferedTransport> bufferedTransport(new TBufferedTransport(socket));
-        m_userStoreHttpClient = boost::shared_ptr<THttpClient>(new THttpClient(bufferedTransport,
-                                                                            EVERNOTE_HOST.toStdString(),
-                                                                            EDAM_USER_STORE_PATH.toStdString()));
-        m_userStoreHttpClient->open();
-        boost::shared_ptr<TProtocol> iprot(new TBinaryProtocol(m_userStoreHttpClient));
-        m_userstoreClient = new evernote::edam::UserStoreClient(iprot);
-        evernote::edam::UserStoreConstants constants;
-        versionOK = m_userstoreClient->checkVersion(EDAM_CLIENT_NAME.toStdString(),
-                                                                      constants.EDAM_VERSION_MAJOR,
-                                                                      constants.EDAM_VERSION_MINOR);
-        qDebug() << "UserStoreClient created. Version check:" << versionOK;
-
-    } catch (const TTransportException & e) {
-        qWarning() << "Failed to create Transport for UserStore:" <<  e.what();
-    } catch (const TException & e) {
-        qWarning() << "Generic Thrift exception in UserStore setup:" << e.what();
-    }
-    return versionOK;
-}
-
-bool EvernoteConnection::setupNotesStore()
-{
-    try {
-        boost::shared_ptr<TSocket> socket;
-
-        if (m_useSSL) {
-            boost::shared_ptr<TSSLSocketFactory> sslSocketFactory(new TSSLSocketFactory());
-            socket = sslSocketFactory->createSocket(EVERNOTE_HOST.toStdString(), 443);
-            qDebug() << "created NotesStore SSL socket";
-        } else {
-            // Create a non-secure socket
-            socket = boost::shared_ptr<TSocket> (new TSocket(EVERNOTE_HOST.toStdString(), 80));
-            qDebug() << "created insecure NotesStore socket";
-        }
-
-        // setup UserStore
-        boost::shared_ptr<TBufferedTransport> bufferedTransport(new TBufferedTransport(socket));
-        m_notesStoreHttpClient = boost::shared_ptr<THttpClient>(new THttpClient(bufferedTransport,
-                                                                            EVERNOTE_HOST.toStdString(),
-                                                                            EDAM_NOTE_STORE_PATH.toStdString()));
-        m_notesStoreHttpClient->open();
-        boost::shared_ptr<TProtocol> iprot(new TBinaryProtocol(m_notesStoreHttpClient));
-
-        // setup notesstore
-        m_notesStoreClient = new evernote::edam::NoteStoreClient(iprot);
-
-        qDebug() << "NoteStore client created.";
-
-    } catch (const TTransportException & e) {
-        qWarning() << "Failed to create Transport for NotesStore:" <<  e.what();
-        return false;
-    } catch (const TException & e) {
-        qWarning() << "Generic Thrift exception in NotesStore setup:" << e.what();
-        return false;
-    }
-    return true;
+
+    connectToEvernote();
+}
+
+void EvernoteConnection::setupUserStore()
+{
+    boost::shared_ptr<TSocket> socket;
+
+    if (m_useSSL) {
+        boost::shared_ptr<TSSLSocketFactory> sslSocketFactory(new TSSLSocketFactory());
+        socket = sslSocketFactory->createSocket(EVERNOTE_HOST.toStdString(), 443);
+        qDebug() << "created UserStore SSL socket";
+    } else {
+        // Create a non-secure socket
+        socket = boost::shared_ptr<TSocket> (new TSocket(EVERNOTE_HOST.toStdString(), 80));
+        qDebug() << "created insecure UserStore socket";
+    }
+
+    // setup UserStore client
+    boost::shared_ptr<TBufferedTransport> bufferedTransport(new TBufferedTransport(socket));
+    m_userStoreHttpClient = boost::shared_ptr<THttpClient>(new THttpClient(bufferedTransport,
+                                                                        EVERNOTE_HOST.toStdString(),
+                                                                        EDAM_USER_STORE_PATH.toStdString()));
+
+    boost::shared_ptr<TProtocol> userstoreiprot(new TBinaryProtocol(m_userStoreHttpClient));
+    m_userstoreClient = new evernote::edam::UserStoreClient(userstoreiprot);
+}
+
+void EvernoteConnection::setupNotesStore()
+{
+    boost::shared_ptr<TSocket> socket;
+
+    if (m_useSSL) {
+        boost::shared_ptr<TSSLSocketFactory> sslSocketFactory(new TSSLSocketFactory());
+        socket = sslSocketFactory->createSocket(EVERNOTE_HOST.toStdString(), 443);
+        qDebug() << "created NotesStore SSL socket";
+    } else {
+        // Create a non-secure socket
+        socket = boost::shared_ptr<TSocket> (new TSocket(EVERNOTE_HOST.toStdString(), 80));
+        qDebug() << "created insecure NotesStore socket";
+    }
+
+    // setup NotesStore client
+    boost::shared_ptr<TBufferedTransport> bufferedTransport(new TBufferedTransport(socket));
+    m_notesStoreHttpClient = boost::shared_ptr<THttpClient>(new THttpClient(bufferedTransport,
+                                                                        EVERNOTE_HOST.toStdString(),
+                                                                        EDAM_NOTE_STORE_PATH.toStdString()));
+
+    boost::shared_ptr<TProtocol> notesstoreiprot(new TBinaryProtocol(m_notesStoreHttpClient));
+    m_notesStoreClient = new evernote::edam::NoteStoreClient(notesstoreiprot);
 }
 
 EvernoteConnection *EvernoteConnection::instance()
@@ -171,6 +146,42 @@
     }
 }
 
+void EvernoteConnection::connectToEvernote()
+{
+    if (m_userStoreHttpClient->isOpen() && m_notesStoreHttpClient->isOpen()) {
+        return;
+    }
+
+    try {
+        m_userStoreHttpClient->open();
+        qDebug() << "UserStoreClient socket opened.";
+
+        m_notesStoreHttpClient->open();
+        qDebug() << "NoteStoreClient socket opened.";
+
+    } catch (const TTransportException & e) {
+        qWarning() << "Failed to open connection:" <<  e.what();
+    } catch (const TException & e) {
+        qWarning() << "Generic Thrift exception when opening the connection:" << e.what();
+    }
+
+    try {
+        evernote::edam::UserStoreConstants constants;
+        bool versionOk = m_userstoreClient->checkVersion(EDAM_CLIENT_NAME.toStdString(),
+                                                                      constants.EDAM_VERSION_MAJOR,
+                                                                      constants.EDAM_VERSION_MINOR);
+
+        if (!versionOk) {
+            qWarning() << "Server version mismatch! This application should be updated!";
+        }
+
+    } catch (const TTransportException & e) {
+        qWarning() << "Failed to fetch server version:" <<  e.what();
+    } catch (const TException & e) {
+        qWarning() << "Generic Thrift exception when fetching server version:" << e.what();
+    }
+}
+
 void EvernoteConnection::enqueue(EvernoteJob *job)
 {
     connect(job, &EvernoteJob::finished, this, &EvernoteConnection::startNextJob);

=== modified file 'src/plugin/Evernote/evernoteconnection.h'
--- src/plugin/Evernote/evernoteconnection.h	2014-02-06 00:01:52 +0000
+++ src/plugin/Evernote/evernoteconnection.h	2014-04-30 20:46:23 +0000
@@ -72,6 +72,8 @@
     void tokenChanged();
 
 private slots:
+    void connectToEvernote();
+
     void startJobQueue();
     void startNextJob();
 
@@ -79,11 +81,10 @@
     explicit EvernoteConnection(QObject *parent = 0);
     static EvernoteConnection *s_instance;
 
-    bool setupUserStore();
-    bool setupNotesStore();
+    void setupUserStore();
+    void setupNotesStore();
 
     bool m_useSSL;
-
     QString m_token;
 
     // There must be only one job running at a time

=== modified file 'src/plugin/Evernote/jobs/notesstorejob.cpp'
--- src/plugin/Evernote/jobs/notesstorejob.cpp	2014-01-27 13:00:41 +0000
+++ src/plugin/Evernote/jobs/notesstorejob.cpp	2014-04-30 20:46:23 +0000
@@ -29,7 +29,12 @@
 
 void NotesStoreJob::resetConnection()
 {
-    EvernoteConnection::instance()->m_notesStoreHttpClient->close();
+    if (EvernoteConnection::instance()->m_notesStoreHttpClient->isOpen()) {
+        EvernoteConnection::instance()->m_notesStoreHttpClient->close();
+    }
+    try {
+        EvernoteConnection::instance()->m_notesStoreHttpClient->readEnd();
+    } catch(...) {}
     EvernoteConnection::instance()->m_notesStoreHttpClient->open();
 }
 

=== modified file 'src/plugin/Evernote/jobs/userstorejob.cpp'
--- src/plugin/Evernote/jobs/userstorejob.cpp	2014-01-27 13:00:41 +0000
+++ src/plugin/Evernote/jobs/userstorejob.cpp	2014-04-30 20:46:23 +0000
@@ -29,7 +29,12 @@
 
 void UserStoreJob::resetConnection()
 {
-    EvernoteConnection::instance()->m_userStoreHttpClient->close();
+    if (EvernoteConnection::instance()->m_userStoreHttpClient->isOpen()) {
+        EvernoteConnection::instance()->m_userStoreHttpClient->close();
+    }
+    try {
+        EvernoteConnection::instance()->m_userStoreHttpClient->readEnd();
+    } catch(...) {}
     EvernoteConnection::instance()->m_userStoreHttpClient->open();
 }