ubuntu-touch-coreapps-reviewers team mailing list archive

[Michael Zanetti <michael.zanetti@xxxxxxxxxxxxx>]

Michael Zanetti has proposed merging lp:~mzanetti/reminders-app/jobqueue-tweaks into lp:reminders-app with lp:~mzanetti/reminders-app/update-thrift as a prerequisite.

Commit message:
Tweak the jobqueue

better prioritze requests that are triggered by a user interaction.


Requested reviews:
  Ubuntu Reminders app developers (reminders-app-dev)

For more details, see:
https://code.launchpad.net/~mzanetti/reminders-app/jobqueue-tweaks/+merge/251188
-- 
Your team Ubuntu Reminders app developers is requested to review the proposed merge of lp:~mzanetti/reminders-app/jobqueue-tweaks into lp:reminders-app.

=== modified file 'src/libqtevernote/evernoteconnection.cpp'
--- src/libqtevernote/evernoteconnection.cpp	2015-02-26 22:49:24 +0000
+++ src/libqtevernote/evernoteconnection.cpp	2015-02-26 22:49:24 +0000
@@ -351,10 +351,10 @@
     return nullptr;
 }
 
-void EvernoteConnection::enqueue(EvernoteJob *job, JobPriority priority)
+void EvernoteConnection::enqueue(EvernoteJob *job)
 {
     if (!isConnected()) {
-        qWarning() << "Not connected to evernote. Can't enqueue job.";
+        qWarning() << "[JobQueue] Not connected to evernote. Can't enqueue job.";
         job->emitJobDone(ErrorCodeConnectionLost, gettext("Disconnected from Evernote."));
         job->deleteLater();
         return;
@@ -364,15 +364,20 @@
         job->attachToDuplicate(duplicate);
         connect(duplicate, &EvernoteJob::finished, job, &EvernoteJob::deleteLater);
         // reprioritze the repeated request
-        if (priority == JobPriorityHigh) {
+        qDebug() << "[JobQueue] Duplicate job already queued:" << job->toString();
+        if (job->jobPriority() == EvernoteJob::JobPriorityHigh) {
+            qDebug() << "[JobQueue] Reprioritising duplicate job:" << job->toString();
+            duplicate->setJobPriority(job->jobPriority());
             m_jobQueue.prepend(m_jobQueue.takeAt(m_jobQueue.indexOf(duplicate)));
         }
     } else {
         connect(job, &EvernoteJob::finished, job, &EvernoteJob::deleteLater);
         connect(job, &EvernoteJob::finished, this, &EvernoteConnection::startNextJob);
-        if (priority == JobPriorityHigh) {
+        if (job->jobPriority() == EvernoteJob::JobPriorityHigh) {
+            qDebug() << "[JobQueue] Prepending high priority job request:" << job->toString();
             m_jobQueue.prepend(job);
         } else {
+            qDebug() << "[JobQueue] Appending low priority job request:" << job->toString();
             m_jobQueue.append(job);
         }
         startJobQueue();
@@ -405,6 +410,7 @@
     }
 
     m_currentJob = m_jobQueue.takeFirst();
+    qDebug() << "[JobQueue] Starting job:" << m_currentJob->toString();
     m_currentJob->start();
 }
 

=== modified file 'src/libqtevernote/evernoteconnection.h'
--- src/libqtevernote/evernoteconnection.h	2014-12-08 10:25:48 +0000
+++ src/libqtevernote/evernoteconnection.h	2015-02-26 22:49:24 +0000
@@ -64,11 +64,6 @@
         ErrorCodeQutaExceeded
     };
 
-    enum JobPriority {
-        JobPriorityHigh,
-        JobPriorityLow
-    };
-
     static EvernoteConnection* instance();
     ~EvernoteConnection();
 
@@ -78,7 +73,7 @@
     QString token() const;
     void setToken(const QString &token);
 
-    void enqueue(EvernoteJob *job, JobPriority priority = JobPriorityHigh);
+    void enqueue(EvernoteJob *job);
 
     bool isConnected() const;
 

=== modified file 'src/libqtevernote/jobs/evernotejob.cpp'
--- src/libqtevernote/jobs/evernotejob.cpp	2014-12-16 21:01:28 +0000
+++ src/libqtevernote/jobs/evernotejob.cpp	2015-02-26 22:49:24 +0000
@@ -39,9 +39,10 @@
 using namespace apache::thrift::protocol;
 using namespace apache::thrift::transport;
 
-EvernoteJob::EvernoteJob(QObject *parent) :
+EvernoteJob::EvernoteJob(QObject *parent, JobPriority jobPriority) :
     QThread(parent),
-    m_token(EvernoteConnection::instance()->token())
+    m_token(EvernoteConnection::instance()->token()),
+    m_jobPriority(jobPriority)
 {
 }
 
@@ -49,6 +50,16 @@
 {
 }
 
+EvernoteJob::JobPriority EvernoteJob::jobPriority() const
+{
+    return m_jobPriority;
+}
+
+void EvernoteJob::setJobPriority(EvernoteJob::JobPriority priority)
+{
+    m_jobPriority = priority;
+}
+
 void EvernoteJob::run()
 {
     if (!EvernoteConnection::instance()->isConnected()) {
@@ -183,6 +194,11 @@
     } while (retry);
 }
 
+QString EvernoteJob::toString() const
+{
+    return metaObject()->className();
+}
+
 QString EvernoteJob::token()
 {
     return m_token;

=== modified file 'src/libqtevernote/jobs/evernotejob.h'
--- src/libqtevernote/jobs/evernotejob.h	2014-12-08 10:25:48 +0000
+++ src/libqtevernote/jobs/evernotejob.h	2015-02-26 22:49:24 +0000
@@ -44,15 +44,25 @@
 {
     Q_OBJECT
 public:
-    explicit EvernoteJob(QObject *parent = 0);
+    enum JobPriority {
+        JobPriorityHigh,
+        JobPriorityLow
+    };
+
+    explicit EvernoteJob(QObject *parent = 0, JobPriority jobPriority = JobPriorityHigh);
     virtual ~EvernoteJob();
 
+    JobPriority jobPriority() const;
+    void setJobPriority(JobPriority priority = JobPriorityHigh);
+
     void run() final;
 
     virtual bool operator==(const EvernoteJob *other) const = 0;
 
     virtual void attachToDuplicate(const EvernoteJob *other) = 0;
 
+    virtual QString toString() const;
+
 signals:
     void connectionLost(const QString &errorMessage);
 
@@ -65,6 +75,7 @@
 
 private:
     QString m_token;
+    JobPriority m_jobPriority;
 
     friend class EvernoteConnection;
 };

=== modified file 'src/libqtevernote/jobs/fetchnotejob.cpp'
--- src/libqtevernote/jobs/fetchnotejob.cpp	2014-10-24 18:45:35 +0000
+++ src/libqtevernote/jobs/fetchnotejob.cpp	2015-02-26 22:49:24 +0000
@@ -43,6 +43,14 @@
     connect(otherJob, &FetchNoteJob::resultReady, this, &FetchNoteJob::resultReady);
 }
 
+QString FetchNoteJob::toString() const
+{
+    return QString("%1, NoteGuid: %2, What: %3")
+            .arg(metaObject()->className())
+            .arg(m_guid)
+            .arg(m_what == LoadContent ? "Content" : "Resources");
+}
+
 void FetchNoteJob::startJob()
 {
     client()->getNote(m_result, token().toStdString(), m_guid.toStdString(), m_what == LoadContent, m_what == LoadResources, false, false);

=== modified file 'src/libqtevernote/jobs/fetchnotejob.h'
--- src/libqtevernote/jobs/fetchnotejob.h	2014-10-24 18:45:35 +0000
+++ src/libqtevernote/jobs/fetchnotejob.h	2015-02-26 22:49:24 +0000
@@ -36,6 +36,7 @@
 
     virtual bool operator==(const EvernoteJob *other) const override;
     virtual void attachToDuplicate(const EvernoteJob *other) override;
+    virtual QString toString() const override;
 
 signals:
     void resultReady(EvernoteConnection::ErrorCode error, const QString &errorMessage, const evernote::edam::Note &note, LoadWhat what);

=== modified file 'src/libqtevernote/jobs/fetchnotesjob.cpp'
--- src/libqtevernote/jobs/fetchnotesjob.cpp	2014-11-07 19:37:09 +0000
+++ src/libqtevernote/jobs/fetchnotesjob.cpp	2015-02-26 22:49:24 +0000
@@ -52,6 +52,16 @@
     connect(otherJob, &FetchNotesJob::jobDone, this, &FetchNotesJob::jobDone);
 }
 
+QString FetchNotesJob::toString() const
+{
+    return QString("%1, NotebookFilter: %2, SearchWords: %3, StartIndex: %4, ChunkSize: %5")
+            .arg(metaObject()->className())
+            .arg(m_filterNotebookGuid)
+            .arg(m_searchWords)
+            .arg(m_startIndex)
+            .arg(m_chunkSize);
+}
+
 void FetchNotesJob::startJob()
 {
     int32_t start = m_startIndex;

=== modified file 'src/libqtevernote/jobs/fetchnotesjob.h'
--- src/libqtevernote/jobs/fetchnotesjob.h	2014-11-06 15:57:43 +0000
+++ src/libqtevernote/jobs/fetchnotesjob.h	2015-02-26 22:49:24 +0000
@@ -34,6 +34,7 @@
 
     virtual bool operator==(const EvernoteJob *other) const override;
     virtual void attachToDuplicate(const EvernoteJob *other) override;
+    virtual QString toString() const override;
 
 signals:
     void jobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage, const evernote::edam::NotesMetadataList &results, const QString &filterNotebookGuid);

=== modified file 'src/libqtevernote/jobs/notesstorejob.cpp'
--- src/libqtevernote/jobs/notesstorejob.cpp	2014-04-30 21:53:07 +0000
+++ src/libqtevernote/jobs/notesstorejob.cpp	2015-02-26 22:49:24 +0000
@@ -29,15 +29,17 @@
 
 void NotesStoreJob::resetConnection()
 {
+    try {
+        EvernoteConnection::instance()->m_notesStoreHttpClient->readEnd();
+    } catch(...) {}
+    try {
+        EvernoteConnection::instance()->m_notesStoreHttpClient->flush();
+    } catch(...) {}
     if (EvernoteConnection::instance()->m_notesStoreHttpClient->isOpen()) {
-        EvernoteConnection::instance()->m_notesStoreHttpClient->close();
+        try {
+            EvernoteConnection::instance()->m_notesStoreHttpClient->close();
+        } catch(...) {}
     }
-    try {
-        EvernoteConnection::instance()->m_notesStoreHttpClient->readEnd();
-    } catch(...) {}
-    try {
-        EvernoteConnection::instance()->m_notesStoreHttpClient->flush();
-    } catch(...) {}
     EvernoteConnection::instance()->m_notesStoreHttpClient->open();
 }
 

=== modified file 'src/libqtevernote/note.cpp'
--- src/libqtevernote/note.cpp	2015-02-24 22:21:04 +0000
+++ src/libqtevernote/note.cpp	2015-02-26 22:49:24 +0000
@@ -287,7 +287,7 @@
 QString Note::tagline() const
 {
     if (m_tagline.isEmpty()) {
-        load();
+        load(false);
     }
     return m_tagline;
 }
@@ -638,12 +638,12 @@
     }
 }
 
-void Note::load() const
+void Note::load(bool priorityHigh) const
 {
     if (!m_loaded && isCached()) {
         loadFromCacheFile();
-    } else if (!m_loaded && !m_loading) {
-        NotesStore::instance()->refreshNoteContent(m_guid);
+    } else if (!m_loaded && (priorityHigh || !m_loading)) {
+        NotesStore::instance()->refreshNoteContent(m_guid, FetchNoteJob::LoadContent, priorityHigh ? EvernoteJob::JobPriorityHigh : EvernoteJob::JobPriorityLow);
     }
 }
 

=== modified file 'src/libqtevernote/note.h'
--- src/libqtevernote/note.h	2015-02-16 19:28:02 +0000
+++ src/libqtevernote/note.h	2015-02-26 22:49:24 +0000
@@ -201,7 +201,7 @@
     void setConflicting(bool conflicting);
 
     // const because we want to load on demand in getters. Keep this private!
-    void load() const;
+    void load(bool priorityHigh = true) const;
     void loadFromCacheFile() const;
 
 private:

=== modified file 'src/libqtevernote/notesstore.cpp'
--- src/libqtevernote/notesstore.cpp	2015-02-24 18:55:55 +0000
+++ src/libqtevernote/notesstore.cpp	2015-02-26 22:49:24 +0000
@@ -604,7 +604,7 @@
             if (note->updateSequenceNumber() < result.updateSequenceNum) {
                 qDebug() << "refreshing note from network. suequence number changed: " << note->updateSequenceNumber() << "->" << result.updateSequenceNum;
                 changedRoles = updateFromEDAM(result, note);
-                refreshNoteContent(note->guid(), FetchNoteJob::LoadContent, EvernoteConnection::JobPriorityLow);
+                refreshNoteContent(note->guid(), FetchNoteJob::LoadContent, EvernoteJob::JobPriorityLow);
                 syncToCacheFile(note);
             }
         } else {
@@ -705,7 +705,7 @@
     }
 }
 
-void NotesStore::refreshNoteContent(const QString &guid, FetchNoteJob::LoadWhat what, EvernoteConnection::JobPriority priority)
+void NotesStore::refreshNoteContent(const QString &guid, FetchNoteJob::LoadWhat what, EvernoteJob::JobPriority priority)
 {
     qDebug() << "fetching note content from network for note" << guid << (what == FetchNoteJob::LoadContent ? "content" : "image");
     Note *note = m_notesHash.value(guid);
@@ -715,8 +715,9 @@
     }
     if (EvernoteConnection::instance()->isConnected()) {
         FetchNoteJob *job = new FetchNoteJob(guid, what, this);
+        job->setJobPriority(priority);
         connect(job, &FetchNoteJob::resultReady, this, &NotesStore::fetchNoteJobDone);
-        EvernoteConnection::instance()->enqueue(job, priority);
+        EvernoteConnection::instance()->enqueue(job);
 
         note->setLoading(true);
         int idx = m_notes.indexOf(note);
@@ -726,6 +727,7 @@
 
 void NotesStore::fetchNoteJobDone(EvernoteConnection::ErrorCode errorCode, const QString &errorMessage, const evernote::edam::Note &result, FetchNoteJob::LoadWhat what)
 {
+    FetchNoteJob *job = static_cast<FetchNoteJob*>(sender());
     Note *note = m_notesHash.value(QString::fromStdString(result.guid));
     if (!note) {
         qWarning() << "can't find note for this update... ignoring...";
@@ -833,7 +835,7 @@
 
     if (refreshWithResourceData) {
         qDebug() << "refreshWithResourceData";
-        refreshNoteContent(note->guid(), FetchNoteJob::LoadResources);
+        refreshNoteContent(note->guid(), FetchNoteJob::LoadResources, job->jobPriority());
     } else {
         syncToCacheFile(note); // Syncs into the list cache
         note->syncToCacheFile(); // Syncs note's content into notes cache

=== modified file 'src/libqtevernote/notesstore.h'
--- src/libqtevernote/notesstore.h	2015-02-24 18:55:55 +0000
+++ src/libqtevernote/notesstore.h	2015-02-26 22:49:24 +0000
@@ -142,7 +142,7 @@
     void refreshNotes(const QString &filterNotebookGuid = QString(), int startIndex = 0);
 
     // Defaulting to High priority to provide fast feedback to the ui. Use low priority if you call this to prefetch things in the background
-    void refreshNoteContent(const QString &guid, FetchNoteJob::LoadWhat what = FetchNoteJob::LoadContent, EvernoteConnection::JobPriority priority = EvernoteConnection::JobPriorityHigh);
+    void refreshNoteContent(const QString &guid, FetchNoteJob::LoadWhat what = FetchNoteJob::LoadContent, EvernoteJob::JobPriority priority = EvernoteJob::JobPriorityHigh);
     void refreshNotebooks();
     void refreshTags();