ubuntu-touch-coreapps-reviewers team mailing list archive

[Michael Zanetti <michael.zanetti@xxxxxxxxxxxxx>]

Michael Zanetti has proposed merging lp:~mzanetti/reminders-app/fix-writeback-issue into lp:reminders-app.

Commit message:
some improvements in job handling:
  
* treat write operations with highest priority
* don't dedupe or reorder write operations
* don't update the local note after a write operation. that will break local changes while the write operation is running.

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

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

=== modified file 'src/app/qml/ui/NoteView.qml'
--- src/app/qml/ui/NoteView.qml	2015-06-21 16:56:53 +0000
+++ src/app/qml/ui/NoteView.qml	2015-07-05 17:04:45 +0000
@@ -81,7 +81,6 @@
 
         locationBarController {
             height: locationBar.height
-            mode: Oxide.LocationBarController.ModeAuto
         }
 
         property string html: root.note ? note.htmlContent : ""

=== modified file 'src/libqtevernote/evernoteconnection.cpp'
--- src/libqtevernote/evernoteconnection.cpp	2015-03-30 19:01:49 +0000
+++ src/libqtevernote/evernoteconnection.cpp	2015-07-05 17:04:45 +0000
@@ -446,6 +446,7 @@
         }
         return;
     }
+
     EvernoteJob *existingJob = findExistingDuplicate(job);
     if (existingJob) {
         qCDebug(dcJobQueue) << "Duplicate job already queued:" << job->toString();
@@ -491,6 +492,20 @@
     }
 }
 
+void EvernoteConnection::enqueueWrite(EvernoteJob *job)
+{
+    if (!isConnected()) {
+        qCWarning(dcJobQueue) << "Not connected to evernote. Can't enqueue job.";
+        job->emitJobDone(ErrorCodeConnectionLost, gettext("Disconnected from Evernote."));
+        job->deleteLater();
+        return;
+    }
+    connect(job, &EvernoteJob::jobFinished, job, &EvernoteJob::deleteLater);
+    connect(job, &EvernoteJob::jobFinished, this, &EvernoteConnection::startNextJob);
+    m_writeJobQueue.append(job);
+    startJobQueue();
+}
+
 bool EvernoteConnection::isConnected() const
 {
     return m_userstoreClient != nullptr &&
@@ -512,7 +527,9 @@
         return;
     }
 
-    if (!m_highPriorityJobQueue.isEmpty()) {
+    if (!m_writeJobQueue.isEmpty()) {
+        m_currentJob = m_writeJobQueue.takeFirst();
+    } else if (!m_highPriorityJobQueue.isEmpty()) {
         m_currentJob = m_highPriorityJobQueue.takeFirst();
     } else if (!m_mediumPriorityJobQueue.isEmpty()){
         m_currentJob = m_mediumPriorityJobQueue.takeFirst();

=== modified file 'src/libqtevernote/evernoteconnection.h'
--- src/libqtevernote/evernoteconnection.h	2015-03-08 21:29:28 +0000
+++ src/libqtevernote/evernoteconnection.h	2015-07-05 17:04:45 +0000
@@ -88,6 +88,9 @@
     //   multiple times.
     void enqueue(EvernoteJob *job);
 
+    // Use this to queue write calls. They won't be deduped and will be ordered
+    void enqueueWrite(EvernoteJob *job);
+
     bool isConnected() const;
 
     QString error() const;
@@ -134,6 +137,7 @@
     QList<EvernoteJob*> m_highPriorityJobQueue;
     QList<EvernoteJob*> m_mediumPriorityJobQueue;
     QList<EvernoteJob*> m_lowPriorityJobQueue;
+    QList<EvernoteJob*> m_writeJobQueue;
     EvernoteJob *m_currentJob;
 
     // Those need to be mutexed

=== modified file 'src/libqtevernote/jobs/savenotejob.cpp'
--- src/libqtevernote/jobs/savenotejob.cpp	2015-03-15 20:00:21 +0000
+++ src/libqtevernote/jobs/savenotejob.cpp	2015-07-05 17:04:45 +0000
@@ -37,7 +37,8 @@
     if (!otherJob) {
         return false;
     }
-    return this->m_note == otherJob->m_note;
+    return this->m_note == otherJob->m_note
+            && this->m_note->updateSequenceNumber() == otherJob->m_note->updateSequenceNumber();
 }
 
 void SaveNoteJob::attachToDuplicate(const EvernoteJob *other)
@@ -46,6 +47,14 @@
     connect(otherJob, &SaveNoteJob::jobDone, this, &SaveNoteJob::jobDone);
 }
 
+QString SaveNoteJob::toString() const
+{
+    return QString("%1, NoteGuid: %2, UpdateSeq: %3")
+            .arg(metaObject()->className())
+            .arg(m_note->guid())
+            .arg(m_note->updateSequenceNumber());
+}
+
 void SaveNoteJob::startJob()
 {
     evernote::edam::Note note;

=== modified file 'src/libqtevernote/jobs/savenotejob.h'
--- src/libqtevernote/jobs/savenotejob.h	2014-10-24 18:45:35 +0000
+++ src/libqtevernote/jobs/savenotejob.h	2015-07-05 17:04:45 +0000
@@ -34,6 +34,8 @@
     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::Note &note);
 

=== modified file 'src/libqtevernote/notesstore.cpp'
--- src/libqtevernote/notesstore.cpp	2015-06-21 16:28:58 +0000
+++ src/libqtevernote/notesstore.cpp	2015-07-05 17:04:45 +0000
@@ -878,13 +878,17 @@
         qCWarning(dcSync) << "can't find note for this update... ignoring...";
         return;
     }
+    if (note->updateSequenceNumber() > result.updateSequenceNum) {
+        qCWarning(dcSync) << "Local update sequence number higher than remote. Local:" << note->updateSequenceNumber() << "remote:" << result.updateSequenceNum;
+        return;
+    }
 
     QModelIndex noteIndex = index(m_notes.indexOf(note));
     QVector<int> roles;
 
     handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
-        qWarning(dcSync) << "Fetch note job failed:" << errorMessage;
+        qCWarning(dcSync) << "Fetch note job failed:" << errorMessage;
         note->setLoading(false);
         roles << RoleLoading;
         note->setSyncError(true);
@@ -1389,11 +1393,11 @@
             // This note hasn't been created on the server yet... try that first
             CreateNoteJob *job = new CreateNoteJob(note, this);
             connect(job, &CreateNoteJob::jobDone, this, &NotesStore::createNoteJobDone);
-            EvernoteConnection::instance()->enqueue(job);
+            EvernoteConnection::instance()->enqueueWrite(job);
         } else {
             SaveNoteJob *job = new SaveNoteJob(note, this);
             connect(job, &SaveNoteJob::jobDone, this, &NotesStore::saveNoteJobDone);
-            EvernoteConnection::instance()->enqueue(job);
+            EvernoteConnection::instance()->enqueueWrite(job);
         }
     }
 
@@ -1415,24 +1419,16 @@
 
     int idx = m_notes.indexOf(note);
     note->setLoading(false);
+    QModelIndex noteIndex = index(idx);
 
     handleUserError(errorCode);
     if (errorCode != EvernoteConnection::ErrorCodeNoError) {
         qCWarning(dcSync) << "Unhandled error saving note:" << errorCode << "Message:" << errorMessage;
         note->setSyncError(true);
-        emit dataChanged(index(idx), index(idx), QVector<int>() << RoleLoading << RoleSyncError);
+        emit dataChanged(noteIndex, noteIndex, QVector<int>() << RoleLoading << RoleSyncError);
         return;
     }
 
-    note->setUpdateSequenceNumber(result.updateSequenceNum);
-    note->setLastSyncedSequenceNumber(result.updateSequenceNum);
-    note->setTitle(QString::fromStdString(result.title));
-    note->setNotebookGuid(QString::fromStdString(result.notebookGuid));
-    note->setUpdated(QDateTime::fromMSecsSinceEpoch(result.updated));
-
-    syncToCacheFile(note);
-
-    QModelIndex noteIndex = index(m_notes.indexOf(note));
     emit dataChanged(noteIndex, noteIndex);
     emit noteChanged(note->guid(), note->notebookGuid());
 }