ubuntu-touch-coreapps-reviewers team mailing list archive

[Riccardo Padovani <riccardo@xxxxxxxxxxxxx>]

Review: Needs Fixing

Tested, looks good to me, just 3 small inline comments

Diff comments:

> === modified file 'src/app/qml/reminders.qml'
> --- src/app/qml/reminders.qml	2015-03-06 12:16:37 +0000
> +++ src/app/qml/reminders.qml	2015-03-08 21:00:47 +0000
> @@ -111,6 +111,7 @@
>  
>      function displayNote(note) {
>          print("displayNote:", note.guid)
> +        note.load(true);
>          if (root.narrowMode) {
>              print("creating noteview");
>              var component = Qt.createComponent(Qt.resolvedUrl("ui/NotePage.qml"));
> @@ -127,6 +128,7 @@
>      }
>  
>      function switchToEditMode(note) {
> +        note.load(true)
>          if (root.narrowMode) {
>              if (pagestack.depth > 1) {
>                  pagestack.pop();
> 
> === modified file 'src/app/qml/ui/NotesPage.qml'
> --- src/app/qml/ui/NotesPage.qml	2015-03-01 22:32:41 +0000
> +++ src/app/qml/ui/NotesPage.qml	2015-03-08 21:00:47 +0000
> @@ -177,6 +177,10 @@
>              syncError: model.syncError
>              conflicting: model.conflicting
>  
> +            Component.onCompleted: {
> +                notes.note(model.guid).load(false);
> +            }
> +
>              onItemClicked: {
>                  if (!model.conflicting) {
>                      root.selectedNote = NotesStore.note(guid);
> 
> === modified file 'src/libqtevernote/evernoteconnection.cpp'
> --- src/libqtevernote/evernoteconnection.cpp	2015-03-06 00:47:45 +0000
> +++ src/libqtevernote/evernoteconnection.cpp	2015-03-08 21:00:47 +0000
> @@ -153,11 +153,23 @@
>          return;
>      }
>  
> -    foreach (EvernoteJob *job, m_jobQueue) {
> -        job->emitJobDone(EvernoteConnection::ErrorCodeConnectionLost, "Disconnected from Evernote");
> -        job->deleteLater();
> -    }
> -    m_jobQueue.clear();
> +    foreach (EvernoteJob *job, m_highPriorityJobQueue) {
> +        job->emitJobDone(EvernoteConnection::ErrorCodeConnectionLost, "Disconnected from Evernote");
> +        job->deleteLater();
> +    }
> +    m_highPriorityJobQueue.clear();
> +
> +    foreach (EvernoteJob *job, m_mediumPriorityJobQueue) {
> +        job->emitJobDone(EvernoteConnection::ErrorCodeConnectionLost, "Disconnected from Evernote");
> +        job->deleteLater();
> +    }
> +    m_mediumPriorityJobQueue.clear();
> +
> +    foreach (EvernoteJob *job, m_lowPriorityJobQueue) {
> +        job->emitJobDone(EvernoteConnection::ErrorCodeConnectionLost, "Disconnected from Evernote");
> +        job->deleteLater();
> +    }
> +    m_lowPriorityJobQueue.clear();
>  
>      m_errorMessage.clear();
>      emit errorChanged();
> @@ -340,11 +352,38 @@
>      return false;
>  }
>  
> -EvernoteJob* EvernoteConnection::findDuplicate(EvernoteJob *job)
> -{
> -    foreach (EvernoteJob *queuedJob, m_jobQueue) {
> -        // explicitly use custom operator==()
> -        if (job->operator ==(queuedJob)) {
> +void EvernoteConnection::attachDuplicate(EvernoteJob *original, EvernoteJob *duplicate)
> +{
> +    if (duplicate->originatingObject() && duplicate->originatingObject() != original->originatingObject()) {
> +        duplicate->attachToDuplicate(m_currentJob);
> +    }
> +    connect(original, &EvernoteJob::jobFinished, duplicate, &EvernoteJob::deleteLater);
> +}
> +
> +EvernoteJob* EvernoteConnection::findExistingDuplicate(EvernoteJob *job)
> +{
> +    qWarning(dcJobQueue) << "Length:"

Shouldn't be qCWarning?

> +                         << m_highPriorityJobQueue.count() + m_mediumPriorityJobQueue.count() + m_lowPriorityJobQueue.count()
> +                         << "(High:" << m_highPriorityJobQueue.count() << "Medium:" << m_mediumPriorityJobQueue.count() << "Low:" << m_lowPriorityJobQueue.count() << ")";
> +
> +    foreach (EvernoteJob *queuedJob, m_highPriorityJobQueue) {
> +        // explicitly use custom operator==()
> +        if (job->operator ==(queuedJob)) {
> +            qCDebug(dcJobQueue) << "Found duplicate in high priority queue";
> +            return queuedJob;
> +        }
> +    }
> +    foreach (EvernoteJob *queuedJob, m_mediumPriorityJobQueue) {
> +        // explicitly use custom operator==()
> +        if (job->operator ==(queuedJob)) {
> +            qCDebug(dcJobQueue) << "Found duplicate in medium priority queue";
> +            return queuedJob;
> +        }
> +    }
> +    foreach (EvernoteJob *queuedJob, m_lowPriorityJobQueue) {
> +        // explicitly use custom operator==()
> +        if (job->operator ==(queuedJob)) {
> +            qCDebug(dcJobQueue) << "Found duplicate in low priority queue";
>              return queuedJob;
>          }
>      }
> @@ -359,26 +398,56 @@
>          job->deleteLater();
>          return;
>      }
> -    EvernoteJob *duplicate = findDuplicate(job);
> -    if (duplicate) {
> -        job->attachToDuplicate(duplicate);
> -        connect(duplicate, &EvernoteJob::finished, job, &EvernoteJob::deleteLater);
> +    if (m_currentJob && m_currentJob->operator ==(job)) {
> +        qCDebug(dcJobQueue) << "Duplicate of new job request already running:" << job->toString();
> +        if (m_currentJob->isFinished()) {
> +            qCWarning(dcJobQueue) << "Job seems to be stuck in a loop. Deleting it:" << job->toString();
> +            job->deleteLater();
> +        } else {
> +            attachDuplicate(m_currentJob, job);
> +        }
> +        return;
> +    }
> +    EvernoteJob *existingJob = findExistingDuplicate(job);
> +    if (existingJob) {
> +        qCDebug(dcJobQueue) << "Duplicate job already queued:" << job->toString();
> +        attachDuplicate(existingJob, job);
>          // reprioritze the repeated request
> -        qCDebug(dcJobQueue) << "Duplicate job already queued:" << job->toString();
>          if (job->jobPriority() == EvernoteJob::JobPriorityHigh) {
> -            qCDebug(dcJobQueue) << "Reprioritising duplicate job:" << job->toString();
> -            duplicate->setJobPriority(job->jobPriority());
> -            m_jobQueue.prepend(m_jobQueue.takeAt(m_jobQueue.indexOf(duplicate)));
> +            qCDebug(dcJobQueue) << "Reprioritising duplicate job in high priority queue:" << job->toString();
> +            existingJob->setJobPriority(job->jobPriority());
> +            if (m_highPriorityJobQueue.contains(existingJob)) {
> +                m_highPriorityJobQueue.prepend(m_highPriorityJobQueue.takeAt(m_highPriorityJobQueue.indexOf(existingJob)));
> +            } else if (m_mediumPriorityJobQueue.contains(existingJob)){
> +                m_highPriorityJobQueue.prepend(m_mediumPriorityJobQueue.takeAt(m_mediumPriorityJobQueue.indexOf(existingJob)));
> +            } else {
> +                m_highPriorityJobQueue.prepend(m_lowPriorityJobQueue.takeAt(m_lowPriorityJobQueue.indexOf(existingJob)));
> +            }
> +        } else if (job->jobPriority() == EvernoteJob::JobPriorityMedium){
> +            if (m_mediumPriorityJobQueue.contains(existingJob)) {
> +                qCDebug(dcJobQueue) << "Reprioritising duplicate job in medium priority queue:" << job->toString();
> +                m_mediumPriorityJobQueue.prepend(m_mediumPriorityJobQueue.takeAt(m_mediumPriorityJobQueue.indexOf(existingJob)));
> +            } else if (m_lowPriorityJobQueue.contains(existingJob)) {
> +                m_mediumPriorityJobQueue.prepend(m_lowPriorityJobQueue.takeAt(m_lowPriorityJobQueue.indexOf(existingJob)));
> +            }
> +        } else if (job->jobPriority() == EvernoteJob::JobPriorityLow) {
> +            if (m_lowPriorityJobQueue.contains(existingJob)) {
> +                qCDebug(dcJobQueue) << "Reprioritising duplicate job in low priority queue:" << job->toString();
> +                m_lowPriorityJobQueue.prepend(m_lowPriorityJobQueue.takeAt(m_lowPriorityJobQueue.indexOf(existingJob)));
> +            }
>          }
>      } else {
> -        connect(job, &EvernoteJob::finished, job, &EvernoteJob::deleteLater);
> -        connect(job, &EvernoteJob::finished, this, &EvernoteConnection::startNextJob);
> +        connect(job, &EvernoteJob::jobFinished, job, &EvernoteJob::deleteLater);
> +        connect(job, &EvernoteJob::jobFinished, this, &EvernoteConnection::startNextJob);
>          if (job->jobPriority() == EvernoteJob::JobPriorityHigh) {
> -            qCDebug(dcJobQueue) << "Prepending high priority job request:" << job->toString();
> -            m_jobQueue.prepend(job);
> +            qCDebug(dcJobQueue) << "Adding high priority job request:" << job->toString();
> +            m_highPriorityJobQueue.prepend(job);
> +        } else if (job->jobPriority() == EvernoteJob::JobPriorityMedium){
> +            qCDebug(dcJobQueue) << "Adding medium priority job request:" << job->toString();
> +            m_mediumPriorityJobQueue.prepend(job);
>          } else {
> -            qCDebug(dcJobQueue) << "Appending low priority job request:" << job->toString();
> -            m_jobQueue.append(job);
> +            qCDebug(dcJobQueue) << "Adding low priority job request:" << job->toString();
> +            m_lowPriorityJobQueue.prepend(job);
>          }
>          startJobQueue();
>      }
> @@ -401,16 +470,24 @@
>  
>  void EvernoteConnection::startJobQueue()
>  {
> -    if (m_jobQueue.isEmpty()) {
> -        return;
> -    }
> -
>      if (m_currentJob) {
>          return;
>      }
>  
> -    m_currentJob = m_jobQueue.takeFirst();
> -    qCDebug(dcJobQueue) << "Starting job:" << m_currentJob->toString();
> +    if (!m_highPriorityJobQueue.isEmpty()) {
> +        m_currentJob = m_highPriorityJobQueue.takeFirst();
> +    } else if (!m_mediumPriorityJobQueue.isEmpty()){
> +        m_currentJob = m_mediumPriorityJobQueue.takeFirst();
> +    } else if (!m_lowPriorityJobQueue.isEmpty()){
> +        m_currentJob = m_lowPriorityJobQueue.takeFirst();
> +    }
> +
> +    if (!m_currentJob) {
> +        qCDebug(dcJobQueue) << "Queue empty. Nothing to do.";
> +        return;
> +    }
> +
> +    qCDebug(dcJobQueue) << QString("Starting job (Priority: %1):").arg(m_currentJob->jobPriority()) << m_currentJob->toString();
>      m_currentJob->start();
>  }
>  
> 
> === modified file 'src/libqtevernote/evernoteconnection.h'
> --- src/libqtevernote/evernoteconnection.h	2015-02-26 22:47:10 +0000
> +++ src/libqtevernote/evernoteconnection.h	2015-03-08 21:00:47 +0000
> @@ -73,6 +73,18 @@
>      QString token() const;
>      void setToken(const QString &token);
>  
> +    // This will add the job to the job queue. The job queue will take ownership of the object
> +    // and manage it's lifetime.
> +    // * If there is an identical job already existing in the queue, the duplicate will be
> +    //   attached to original job and not actually fetched a second time from the network in
> +    //   order to reduce network traffic.
> +    // * If the new job has a higher priority than the existing one, the existing one will
> +    //   reprioritized to the higher priorty.
> +    // * If the jobs have different originatingObjects, each job will emit the jobDone signal,
> +    //   if instead the originatingObject is the same in both jobs, only one of them will emit
> +    //   a jobDone signal. This is useful if you want to reschedule a job with higher priority
> +    //   without having to track previously queued jobs and avoid invoking the connected slot
> +    //   multiple times.
>      void enqueue(EvernoteJob *job);
>  
>      bool isConnected() const;
> @@ -104,7 +116,10 @@
>      bool connectUserStore();
>      bool connectNotesStore();
>  
> -    EvernoteJob* findDuplicate(EvernoteJob *job);
> +    EvernoteJob* findExistingDuplicate(EvernoteJob *job);
> +
> +    // "duplicate" will be attached to "original"
> +    void attachDuplicate(EvernoteJob *original, EvernoteJob *duplicate);
>  
>      bool m_useSSL;
>      bool m_isConnected;
> @@ -115,7 +130,9 @@
>  
>      // There must be only one job running at a time
>      // Do not start jobs other than with startJobQueue()
> -    QList<EvernoteJob*> m_jobQueue;
> +    QList<EvernoteJob*> m_highPriorityJobQueue;
> +    QList<EvernoteJob*> m_mediumPriorityJobQueue;
> +    QList<EvernoteJob*> m_lowPriorityJobQueue;
>      EvernoteJob *m_currentJob;
>  
>      // Those need to be mutexed
> 
> === modified file 'src/libqtevernote/jobs/evernotejob.cpp'
> --- src/libqtevernote/jobs/evernotejob.cpp	2015-03-06 00:42:42 +0000
> +++ src/libqtevernote/jobs/evernotejob.cpp	2015-03-08 21:00:47 +0000
> @@ -38,11 +38,13 @@
>  using namespace apache::thrift::protocol;
>  using namespace apache::thrift::transport;
>  
> -EvernoteJob::EvernoteJob(QObject *parent, JobPriority jobPriority) :
> -    QThread(parent),
> +EvernoteJob::EvernoteJob(QObject *originatingObject, JobPriority jobPriority) :
> +    QThread(nullptr),
>      m_token(EvernoteConnection::instance()->token()),
> -    m_jobPriority(jobPriority)
> +    m_jobPriority(jobPriority),
> +    m_originatingObject(originatingObject)
>  {
> +    connect(this, &QThread::finished, this, &EvernoteJob::jobFinished);
>  }
>  
>  EvernoteJob::~EvernoteJob()
> @@ -198,6 +200,11 @@
>      return metaObject()->className();
>  }
>  
> +QObject *EvernoteJob::originatingObject() const
> +{
> +    return m_originatingObject;
> +}
> +
>  QString EvernoteJob::token()
>  {
>      return m_token;
> 
> === modified file 'src/libqtevernote/jobs/evernotejob.h'
> --- src/libqtevernote/jobs/evernotejob.h	2015-02-26 22:47:10 +0000
> +++ src/libqtevernote/jobs/evernotejob.h	2015-03-08 21:00:47 +0000
> @@ -37,8 +37,7 @@
>   *   your job won't be executed but you should instead forward the other's job results.
>   *
>   * Jobs can be enqueue()d in NotesStore.
> - * They will destroy themselves when finished.
> - * The jobqueue will take care about starting them.
> + * The jobqueue will take care about starting them and deleting them.
>   */
>  class EvernoteJob : public QThread
>  {
> @@ -46,10 +45,11 @@
>  public:
>      enum JobPriority {
>          JobPriorityHigh,
> +        JobPriorityMedium,
>          JobPriorityLow
>      };
>  
> -    explicit EvernoteJob(QObject *parent = 0, JobPriority jobPriority = JobPriorityHigh);
> +    explicit EvernoteJob(QObject *originatingObject = 0, JobPriority jobPriority = JobPriorityHigh);
>      virtual ~EvernoteJob();
>  
>      JobPriority jobPriority() const;
> @@ -63,9 +63,13 @@
>  
>      virtual QString toString() const;
>  
> +    QObject* originatingObject() const;
> +
>  signals:
>      void connectionLost(const QString &errorMessage);
>  
> +    void jobFinished();
> +
>  protected:
>      virtual void resetConnection() = 0;
>      virtual void startJob() = 0;
> @@ -76,6 +80,7 @@
>  private:
>      QString m_token;
>      JobPriority m_jobPriority;
> +    QObject *m_originatingObject;
>  
>      friend class EvernoteConnection;
>  };
> 
> === modified file 'src/libqtevernote/jobs/fetchnotesjob.cpp'
> --- src/libqtevernote/jobs/fetchnotesjob.cpp	2015-03-06 00:47:45 +0000
> +++ src/libqtevernote/jobs/fetchnotesjob.cpp	2015-03-08 21:00:47 +0000
> @@ -41,7 +41,9 @@
>          return false;
>      }
>      return this->m_filterNotebookGuid == otherJob->m_filterNotebookGuid
> -            && this->m_searchWords == otherJob->m_searchWords;
> +            && this->m_searchWords == otherJob->m_searchWords
> +            && this->m_startIndex == otherJob->m_startIndex
> +            && this->m_chunkSize == otherJob->m_chunkSize;
>  }
>  
>  void FetchNotesJob::attachToDuplicate(const EvernoteJob *other)
> 
> === modified file 'src/libqtevernote/note.cpp'
> --- src/libqtevernote/note.cpp	2015-03-08 16:01:23 +0000
> +++ src/libqtevernote/note.cpp	2015-03-08 21:00:47 +0000
> @@ -38,7 +38,6 @@
>      m_isSearchResult(false),
>      m_updateSequenceNumber(updateSequenceNumber),
>      m_loading(false),
> -    m_loadingHighPriority(false),
>      m_loaded(false),
>      m_needsContentSync(false),
>      m_syncError(false),
> @@ -64,15 +63,9 @@
>  
>      infoFile.beginGroup("resources");
>      foreach (const QString &hash, infoFile.childGroups()) {
> -        if (Resource::isCached(hash)) {
> -            infoFile.beginGroup(hash);
> -            // Assuming the resource is already cached...
> -            addResource(QByteArray(), hash, infoFile.value("fileName").toString(), infoFile.value("type").toString());
> -            infoFile.endGroup();
> -        } else {
> -            // uh oh... have a resource description without file... reset sequence number to indicate we need a sync
> -            qCWarning(dcNotesStore) << "Have a resource description but no resource file for it";
> -        }
> +        infoFile.beginGroup(hash);
> +        addResource(hash, infoFile.value("fileName").toString(), infoFile.value("type").toString());
> +        infoFile.endGroup();
>      }
>      infoFile.endGroup();
>  
> @@ -243,7 +236,6 @@
>  
>  QString Note::enmlContent() const
>  {
> -    load();
>      return m_content.enml();
>  }
>  
> @@ -263,13 +255,11 @@
>  
>  QString Note::htmlContent() const
>  {
> -    load();
>      return m_content.toHtml(m_guid);
>  }
>  
>  QString Note::richTextContent() const
>  {
> -    load();
>      return m_content.toRichText(m_guid);
>  }
>  
> @@ -286,15 +276,11 @@
>  
>  QString Note::plaintextContent() const
>  {
> -    load();
>      return m_content.toPlaintext().trimmed();
>  }
>  
>  QString Note::tagline() const
>  {
> -    if (m_tagline.isEmpty()) {
> -        load(false);
> -    }
>      return m_tagline;
>  }
>  
> @@ -488,11 +474,12 @@
>  QStringList Note::resourceUrls() const
>  {
>      QList<QString> ret;
> -    foreach (const QString &hash, m_resources.keys()) {
> -        QUrl url("image://resource/" + m_resources.value(hash)->type());
> +    foreach (Resource *resource, m_resources) {
> +        QUrl url("image://resource/" + resource->type());
>          QUrlQuery arguments;
>          arguments.addQueryItem("noteGuid", m_guid);
> -        arguments.addQueryItem("hash", hash);
> +        arguments.addQueryItem("hash", resource->hash());
> +        arguments.addQueryItem("loaded", resource->isCached() ? "true" : "false");
>          url.setQuery(arguments);
>          ret << url.toString();
>      }
> @@ -505,25 +492,29 @@
>  }
>  
>  
> -Resource* Note::addResource(const QByteArray &data, const QString &hash, const QString &fileName, const QString &type)
> +Resource* Note::addResource(const QString &hash, const QString &fileName, const QString &type, const QByteArray &data)
>  {
> +    Resource *resource;
>      if (m_resources.contains(hash)) {
> -        return m_resources.value(hash);
> +        resource = m_resources.value(hash);
> +        if (!data.isEmpty()) {
> +            resource->setData(data);
> +        }
> +    } else {
> +        resource = new Resource(data, hash, fileName, type, this);
> +        m_resources.insert(hash, resource);
> +        QSettings infoFile(m_infoFile, QSettings::IniFormat);
> +        infoFile.beginGroup("resources");
> +        infoFile.beginGroup(hash);
> +        infoFile.setValue("fileName", fileName);
> +        infoFile.setValue("type", type);
> +        infoFile.endGroup();
> +        infoFile.endGroup();
>      }
>  
> -    Resource *resource = new Resource(data, hash, fileName, type, this);
> -    m_resources.insert(hash, resource);
>      emit resourcesChanged();
>      emit contentChanged();
>  
> -    QSettings infoFile(m_infoFile, QSettings::IniFormat);
> -    infoFile.beginGroup("resources");
> -    infoFile.beginGroup(hash);
> -    infoFile.setValue("fileName", fileName);
> -    infoFile.setValue("type", type);
> -    infoFile.endGroup();
> -    infoFile.endGroup();
> -
>      return resource;
>  }
>  
> @@ -593,7 +584,7 @@
>      note->setUpdateSequenceNumber(m_updateSequenceNumber);
>      note->setDeleted(m_deleted);
>      foreach (Resource *resource, m_resources) {
> -        note->addResource(resource->data(), resource->hash(), resource->fileName(), resource->type());
> +        note->addResource(resource->hash(), resource->fileName(), resource->type(), resource->data());
>      }
>      note->m_needsContentSync = m_needsContentSync;
>  
> @@ -625,12 +616,6 @@
>      if (m_loading != loading) {
>          m_loading = loading;
>          emit loadingChanged();
> -
> -        if (!m_loading) {
> -            m_loadingHighPriority = false;
> -        } else {
> -            m_loadingHighPriority = highPriority;

/home/rpadovani/Documents/ubuntu/touch/coreapps/reminders/two-job-queues/src/libqtevernote/note.cpp:614:42: warning: unused parameter ‘highPriority’ [-Wunused-parameter]

> -        }
>      }
>  }
>  
> @@ -670,13 +655,22 @@
>      }
>  }
>  
> -void Note::load(bool priorityHigh) const
> +void Note::load(bool priorityHigh)
>  {
>      if (!m_loaded && isCached()) {
>          loadFromCacheFile();
> -    } else if (!m_loaded) {
> -        if (!m_loading || (priorityHigh && !m_loadingHighPriority)) {
> -            NotesStore::instance()->refreshNoteContent(m_guid, FetchNoteJob::LoadContent, priorityHigh ? EvernoteJob::JobPriorityHigh : EvernoteJob::JobPriorityLow);
> +    }
> +
> +    if (!m_loaded) {
> +        NotesStore::instance()->refreshNoteContent(m_guid, FetchNoteJob::LoadContent, priorityHigh ? EvernoteJob::JobPriorityHigh : EvernoteJob::JobPriorityMedium);
> +        return;
> +    }
> +
> +    // Check if resources are loaded
> +    foreach (Resource *resource, m_resources) {
> +        if (!resource->isCached()) {
> +            NotesStore::instance()->refreshNoteContent(m_guid, FetchNoteJob::LoadResources, priorityHigh ? EvernoteJob::JobPriorityHigh : EvernoteJob::JobPriorityLow);
> +            break;
>          }
>      }
>  }
> 
> === modified file 'src/libqtevernote/note.h'
> --- src/libqtevernote/note.h	2015-03-08 16:01:23 +0000
> +++ src/libqtevernote/note.h	2015-03-08 21:00:47 +0000
> @@ -157,7 +157,7 @@
>      QStringList resourceUrls() const;
>      Q_INVOKABLE Resource* resource(const QString &hash);
>      QList<Resource*> resources() const;
> -    Resource *addResource(const QByteArray &data, const QString &hash, const QString &fileName, const QString &type);
> +
>  
>      Q_INVOKABLE void markTodo(const QString &todoId, bool checked);
>      Q_INVOKABLE void attachFile(int position, const QUrl &fileName);
> @@ -167,6 +167,8 @@
>      int renderWidth() const;
>      void setRenderWidth(int renderWidth);
>  
> +    Q_INVOKABLE void load(bool highPriority = false);
> +
>  public slots:
>      void save();
>      void remove();
> @@ -210,9 +212,10 @@
>      void setUpdateSequenceNumber(qint32 updateSequenceNumber);
>      void setLastSyncedSequenceNumber(qint32 lastSyncedSequenceNumber);
>      void setConflicting(bool conflicting);
> +    Resource *addResource(const QString &hash, const QString &fileName, const QString &type, const QByteArray &data = QByteArray());
> +    void addMissingResource();
> +    void setMissingResources(int missingResources);
>  
> -    // const because we want to load on demand in getters. Keep this private!
> -    void load(bool priorityHigh = true) const;
>      void loadFromCacheFile() const;
>  
>  private:
> @@ -236,7 +239,6 @@
>      QString m_infoFile;
>  
>      bool m_loading;
> -    bool m_loadingHighPriority;
>      mutable bool m_loaded;
>      bool m_synced;
>      bool m_needsContentSync;
> 
> === modified file 'src/libqtevernote/notesstore.cpp'
> --- src/libqtevernote/notesstore.cpp	2015-03-08 16:01:23 +0000
> +++ src/libqtevernote/notesstore.cpp	2015-03-08 21:00:47 +0000
> @@ -677,7 +677,7 @@
>              if (note->updateSequenceNumber() < result.updateSequenceNum) {
>                  qCDebug(dcSync) << "refreshing note from network. suequence number changed: " << note->updateSequenceNumber() << "->" << result.updateSequenceNum;
>                  changedRoles = updateFromEDAM(result, note);
> -                refreshNoteContent(note->guid(), FetchNoteJob::LoadContent, EvernoteJob::JobPriorityLow);
> +                refreshNoteContent(note->guid(), FetchNoteJob::LoadContent, EvernoteJob::JobPriorityMedium);
>                  syncToCacheFile(note);
>              }
>          } else {
> @@ -798,15 +798,18 @@
>          return;
>      }
>      if (EvernoteConnection::instance()->isConnected()) {
> -        qCDebug(dcNotesStore) << "Fetching note content from network for note" << guid << (what == FetchNoteJob::LoadContent ? "content" : "image");
> +        qCDebug(dcNotesStore) << "Fetching note content from network for note" << guid << (what == FetchNoteJob::LoadContent ? "Content" : "Resource") << "Priority:" << priority;
>          FetchNoteJob *job = new FetchNoteJob(guid, what, this);
>          job->setJobPriority(priority);
>          connect(job, &FetchNoteJob::resultReady, this, &NotesStore::fetchNoteJobDone);
>          EvernoteConnection::instance()->enqueue(job);
>  
> +        bool wasLoading = note->loading();
>          note->setLoading(true, priority == EvernoteJob::JobPriorityHigh);
> -        int idx = m_notes.indexOf(note);
> -        emit dataChanged(index(idx), index(idx), QVector<int>() << RoleLoading);
> +        if (!wasLoading) {
> +            int idx = m_notes.indexOf(note);
> +            emit dataChanged(index(idx), index(idx), QVector<int>() << RoleLoading);
> +        }
>      }
>  }
>  
> @@ -818,16 +821,13 @@
>          qCWarning(dcSync) << "can't find note for this update... ignoring...";
>          return;
>      }
> +
>      QModelIndex noteIndex = index(m_notes.indexOf(note));
>      QVector<int> roles;
>  
> -    note->setLoading(false);
> -    roles << RoleLoading;
> -
>      switch (errorCode) {
>      case EvernoteConnection::ErrorCodeNoError:
>          // All is well
> -        emit dataChanged(noteIndex, noteIndex, roles);
>          break;
>      case EvernoteConnection::ErrorCodeUserException:
>          qCWarning(dcSync) << "FetchNoteJobDone: EDAMUserException:" << errorMessage;
> @@ -877,15 +877,17 @@
>          QString mime = QString::fromStdString(resource.mime);
>  
>          if (what == FetchNoteJob::LoadResources) {
> -            qCDebug(dcSync) << "Resource fetched for note:" << note->guid() << "Filename:" << fileName << "Mimetype:" << mime << "Hash:" << hash;
> +            qCDebug(dcSync) << "Resource content fetched for note:" << note->guid() << "Filename:" << fileName << "Mimetype:" << mime << "Hash:" << hash;
>              QByteArray resourceData = QByteArray(resource.data.body.data(), resource.data.size);
> -            note->addResource(resourceData, hash, fileName, mime);
> -        } else if (Resource::isCached(hash)) {
> -            qCDebug(dcSync) << "Resource already cached for note:" << note->guid() << "Filename:" << fileName << "Mimetype:" << mime << "Hash:" << hash;
> -            note->addResource(QByteArray(), hash, fileName, mime);
> +            note->addResource(hash, fileName, mime, resourceData);
>          } else {
> -            qCDebug(dcSync) << "Resource not yet fetched for note:" << note->guid() << "Filename:" << fileName << "Mimetype:" << mime << "Hash:" << hash;
> -            refreshWithResourceData = true;
> +            qCDebug(dcSync) << "Adding resource info to note:" << note->guid() << "Filename:" << fileName << "Mimetype:" << mime << "Hash:" << hash;
> +            Resource *resource = note->addResource(hash, fileName, mime);
> +
> +            if (!resource->isCached()) {
> +                qCDebug(dcSync) << "Resource not yet fetched for note:" << note->guid() << "Filename:" << fileName << "Mimetype:" << mime << "Hash:" << hash;
> +                refreshWithResourceData = true;
> +            }
>          }
>          roles << RoleHtmlContent << RoleEnmlContent << RoleResourceUrls;
>      }
> @@ -915,17 +917,20 @@
>          note->setReminderDoneTime(reminderDoneTime);
>          roles << RoleReminderDone << RoleReminderDoneTime;
>      }
> +
> +    note->setLoading(false);
> +    roles << RoleLoading;
> +
>      emit noteChanged(note->guid(), note->notebookGuid());
> -
>      emit dataChanged(noteIndex, noteIndex, roles);
>  
>      if (refreshWithResourceData) {
>          qCDebug(dcSync) << "Fetching Note resources:" << note->guid();
> -        refreshNoteContent(note->guid(), FetchNoteJob::LoadResources, job->jobPriority());
> -    } else {
> -        syncToCacheFile(note); // Syncs into the list cache
> -        note->syncToCacheFile(); // Syncs note's content into notes cache
> +        EvernoteJob::JobPriority newPriority = job->jobPriority() == EvernoteJob::JobPriorityMedium ? EvernoteJob::JobPriorityLow : job->jobPriority();
> +        refreshNoteContent(note->guid(), FetchNoteJob::LoadResources, newPriority);
>      }
> +    syncToCacheFile(note); // Syncs into the list cache
> +    note->syncToCacheFile(); // Syncs note's content into notes cache
>  }
>  
>  void NotesStore::refreshNotebooks()
> @@ -1004,6 +1009,8 @@
>          }
>      }
>  
> +    qCDebug(dcSync) << "Remote notebooks merged into storage. Merging local changes to server.";
> +
>      foreach (Notebook *notebook, unhandledNotebooks) {
>          if (notebook->lastSyncedSequenceNumber() == 0) {
>              qCDebug(dcSync) << "Have a local notebook that doesn't exist on Evernote. Creating on server:" << notebook->guid();
> @@ -1027,6 +1034,8 @@
>              notebook->deleteLater();
>          }
>      }
> +
> +    qCDebug(dcSync) << "Notebooks merged.";
>  }
>  
>  void NotesStore::refreshTags()
> 
> === modified file 'src/libqtevernote/resource.cpp'
> --- src/libqtevernote/resource.cpp	2015-03-06 00:47:45 +0000
> +++ src/libqtevernote/resource.cpp	2015-03-08 21:00:47 +0000
> @@ -52,15 +52,16 @@
>      }
>  }
>  
> -bool Resource::isCached(const QString &hash)
> -{
> -    QDir cacheDir(NotesStore::instance()->storageLocation());
> -    foreach (const QString fi, cacheDir.entryList()) {
> -        if (fi.contains(hash)) {
> -            return true;
> -        }
> -    }
> -    return false;
> +Resource::Resource(const QString &hash, const QString &fileName, const QString &type, QObject *parent):
> +    Resource(QByteArray(), hash, fileName, type, parent)
> +{
> +
> +}
> +
> +bool Resource::isCached()
> +{
> +    QFileInfo fi(m_filePath);
> +    return fi.exists();
>  }
>  
>  Resource::Resource(const QString &path, QObject *parent):
> @@ -159,3 +160,13 @@
>      }
>      return QByteArray();
>  }
> +
> +void Resource::setData(const QByteArray &data)
> +{
> +    QFile file(m_filePath);
> +    if (file.open(QFile::WriteOnly | QFile::Truncate)) {
> +        file.write(data);
> +    } else {
> +        qCDebug(dcNotesStore) << "Error saving data for resource:" << m_hash;
> +    }
> +}
> 
> === modified file 'src/libqtevernote/resource.h'
> --- src/libqtevernote/resource.h	2015-02-16 21:57:16 +0000
> +++ src/libqtevernote/resource.h	2015-03-08 21:00:47 +0000
> @@ -37,10 +37,12 @@
>  public:
>      Resource(const QString &path, QObject *parent = 0);
>      Resource(const QByteArray &data, const QString &hash, const QString &fileName, const QString &type, QObject *parent = 0);
> +    Resource(const QString &hash, const QString &fileName, const QString &type, QObject *parent = 0);
>  
> -    static bool isCached(const QString &hash);
> +    bool isCached();
>  
>      QByteArray data() const;
> +    void setData(const QByteArray &data);
>      QString hash() const;
>      QString fileName() const;
>      QString type() const;
> 
> === modified file 'src/libqtevernote/resourceimageprovider.cpp'
> --- src/libqtevernote/resourceimageprovider.cpp	2015-03-06 00:47:45 +0000
> +++ src/libqtevernote/resourceimageprovider.cpp	2015-03-08 21:00:47 +0000
> @@ -5,6 +5,7 @@
>  #include <note.h>
>  
>  #include <QUrlQuery>
> +#include <QFileInfo>
>  
>  ResourceImageProvider::ResourceImageProvider():
>      QQuickImageProvider(QQuickImageProvider::Image)
> @@ -18,6 +19,7 @@
>      QUrlQuery arguments(id.split('?').last());
>      QString noteGuid = arguments.queryItemValue("noteGuid");
>      QString resourceHash = arguments.queryItemValue("hash");
> +    bool isLoaded = arguments.queryItemValue("loaded") == "true";
>      Note *note = NotesStore::instance()->note(noteGuid);
>      if (!note) {
>          qCWarning(dcNotesStore) << "Unable to find note for resource:" << id;
> @@ -26,19 +28,47 @@
>  
>      QImage image;
>      if (mediaType.startsWith("image")) {
> -        QSize tmpSize = requestedSize;
> -        if (!requestedSize.isValid() || requestedSize.width() > 1024 || requestedSize.height() > 1024) {
> -            tmpSize = QSize(1024, 1024);
> +        if (isLoaded) {
> +            QSize tmpSize = requestedSize;
> +            if (!requestedSize.isValid() || requestedSize.width() > 1024 || requestedSize.height() > 1024) {
> +                tmpSize = QSize(1024, 1024);
> +            }
> +            image = QImage::fromData(NotesStore::instance()->note(noteGuid)->resource(resourceHash)->imageData(tmpSize));
> +        } else {
> +            image = loadIcon("image-x-generic-symbolic", requestedSize);
>          }
> -        image = QImage::fromData(NotesStore::instance()->note(noteGuid)->resource(resourceHash)->imageData(tmpSize));
>      } else if (mediaType.startsWith("audio")) {
> -        image.load("/usr/share/icons/suru/mimetypes/scalable/audio-x-generic-symbolic.svg");
> +        image = loadIcon("audio-x-generic-symbolic", requestedSize);
>      } else if (mediaType == "application/pdf") {
> -        image.load("/usr/share/icons/suru/mimetypes/scalable/application-pdf-symbolic.svg");
> +        image = loadIcon("application-pdf-symbolic", requestedSize);
>      } else {
> -        image.load("/usr/share/icons/suru/mimetypes/scalable/empty-symbolic.svg");
> +        image = loadIcon("empty-symbolic", requestedSize);
>      }
>  
>      *size = image.size();
>      return image;
>  }
> +
> +QImage ResourceImageProvider::loadIcon(const QString &name, const QSize &size)
> +{
> +    QString path = QString("/home/phablet/.cache/com.ubuntu.reminders/%1_%2x%3.png").arg(name).arg(size.width()).arg(size.height());

Shouldn't better to use QStandardPaths::CacheLocation, so in a future, when the system will support multiuser, every user could use his home folder?

Also, this break compatibility with destkop if username != phablet

> +    QFileInfo fi(path);
> +    if (fi.exists()) {
> +        QImage image;
> +        image.load(path);
> +        return image;
> +    }
> +
> +    QString svgPath = "/usr/share/icons/suru/mimetypes/scalable/" + name + ".svg";
> +    QImage image;
> +    image.load(svgPath);
> +    if (size.height() > 0 && size.width() > 0) {
> +        image = image.scaled(size, Qt::KeepAspectRatio, Qt::SmoothTransformation);
> +    } else if (size.height() > 0) {
> +        image = image.scaledToHeight(size.height(), Qt::SmoothTransformation);
> +    } else if (size.width() > 0) {
> +        image = image.scaledToWidth(size.width(), Qt::SmoothTransformation);
> +    }
> +    image.save(path);
> +    return image;
> +}
> 
> === modified file 'src/libqtevernote/resourceimageprovider.h'
> --- src/libqtevernote/resourceimageprovider.h	2014-10-23 21:27:46 +0000
> +++ src/libqtevernote/resourceimageprovider.h	2015-03-08 21:00:47 +0000
> @@ -9,6 +9,9 @@
>      explicit ResourceImageProvider();
>  
>      QImage requestImage(const QString &id, QSize *size, const QSize &requestedSize);
> +
> +    void scale(QImage &image, const QSize &size);
> +    QImage loadIcon(const QString &name, const QSize &size);
>  };
>  
>  #endif // RESOURCEIMAGEPROVIDER_H
> 
> === modified file 'src/libqtevernote/utils/enmldocument.cpp'
> --- src/libqtevernote/utils/enmldocument.cpp	2015-03-08 16:01:23 +0000
> +++ src/libqtevernote/utils/enmldocument.cpp	2015-03-08 21:00:47 +0000
> @@ -296,6 +296,7 @@
>      QUrlQuery arguments;
>      arguments.addQueryItem("noteGuid", noteGuid);
>      arguments.addQueryItem("hash", hash);
> +    arguments.addQueryItem("loaded", NotesStore::instance()->note(noteGuid)->resource(hash)->isCached() ? "true" : "false");
>      url.setQuery(arguments);
>      return url.toString();
>  }
> 


-- 
https://code.launchpad.net/~mzanetti/reminders-app/two-job-queues/+merge/252175
Your team Ubuntu Reminders app developers is subscribed to branch lp:reminders-app.