From 25b769bbf12502c536ba79ab9e3e486294f9eaf6 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 14 Jan 2017 00:04:52 +0100 Subject: [PATCH] More refactoring --- QtClient/JoplinQtClient/MainPage.qml | 2 +- QtClient/JoplinQtClient/dispatcher.cpp | 12 ++++ QtClient/JoplinQtClient/dispatcher.h | 6 ++ .../models/abstractlistmodel.cpp | 17 +++++- .../JoplinQtClient/models/abstractlistmodel.h | 2 + QtClient/JoplinQtClient/models/basemodel.cpp | 28 ++++++---- QtClient/JoplinQtClient/models/basemodel.h | 1 - .../JoplinQtClient/models/foldermodel.cpp | 32 ++++++----- QtClient/JoplinQtClient/models/foldermodel.h | 3 +- QtClient/JoplinQtClient/models/notemodel.cpp | 55 +++++++++++++++++++ QtClient/JoplinQtClient/models/notemodel.h | 4 ++ 11 files changed, 132 insertions(+), 30 deletions(-) diff --git a/QtClient/JoplinQtClient/MainPage.qml b/QtClient/JoplinQtClient/MainPage.qml index 7778a68c9..cc8c9ab69 100755 --- a/QtClient/JoplinQtClient/MainPage.qml +++ b/QtClient/JoplinQtClient/MainPage.qml @@ -21,7 +21,7 @@ Item { list.model.addData(text) list.selectItemById(list.model.lastInsertId()); } else { - list.model.setTitle(index, text) + list.model.setData(index, text, "title") } } diff --git a/QtClient/JoplinQtClient/dispatcher.cpp b/QtClient/JoplinQtClient/dispatcher.cpp index a90f7d54e..d84591866 100755 --- a/QtClient/JoplinQtClient/dispatcher.cpp +++ b/QtClient/JoplinQtClient/dispatcher.cpp @@ -20,6 +20,18 @@ void Dispatcher::emitAllFoldersDeleted() { emit allFoldersDeleted(); } +void Dispatcher::emitNoteCreated(const QString ¬eId) { + emit noteCreated(noteId); +} + +void Dispatcher::emitNoteUpdated(const QString ¬eId) { + emit noteUpdated(noteId); +} + +void Dispatcher::emitNoteDeleted(const QString ¬eId) { + emit noteDeleted(noteId); +} + void Dispatcher::emitLoginClicked(const QString &apiBaseUrl, const QString &email, const QString &password) { emit loginClicked(apiBaseUrl, email, password); } diff --git a/QtClient/JoplinQtClient/dispatcher.h b/QtClient/JoplinQtClient/dispatcher.h index 64e2ac3d8..bc7d6550d 100755 --- a/QtClient/JoplinQtClient/dispatcher.h +++ b/QtClient/JoplinQtClient/dispatcher.h @@ -19,6 +19,9 @@ public slots: void emitFolderUpdated(const QString& folderId); void emitFolderDeleted(const QString& folderId); void emitAllFoldersDeleted(); + void emitNoteCreated(const QString& noteId); + void emitNoteUpdated(const QString& noteId); + void emitNoteDeleted(const QString& noteId); void emitLoginClicked(const QString& domain, const QString& email, const QString &password); void emitLogoutClicked(); void emitLoginStarted(); @@ -31,6 +34,9 @@ signals: void folderUpdated(const QString& folderId); void folderDeleted(const QString& folderId); void allFoldersDeleted(); + void noteCreated(const QString& noteId); + void noteUpdated(const QString& noteId); + void noteDeleted(const QString& noteId); void loginClicked(const QString& domain, const QString& email, const QString& password); void logoutClicked(); void loginStarted(); diff --git a/QtClient/JoplinQtClient/models/abstractlistmodel.cpp b/QtClient/JoplinQtClient/models/abstractlistmodel.cpp index 991d73b57..9c420dd7b 100755 --- a/QtClient/JoplinQtClient/models/abstractlistmodel.cpp +++ b/QtClient/JoplinQtClient/models/abstractlistmodel.cpp @@ -44,8 +44,8 @@ bool AbstractListModel::setData(const QModelIndex &index, const QVariant &value, BaseModel* model = atIndex(index.row()); if (!model) return false; - if (role == Qt::EditRole) { - model->setValue("title", value); + if (role == TitleRole) { + model->setValue("title", value.toString()); if (!model->save()) return false; cacheClear(); return true; @@ -55,6 +55,10 @@ bool AbstractListModel::setData(const QModelIndex &index, const QVariant &value, return false; } +bool AbstractListModel::setData(int index, const QVariant &value, const QString& role) { + return setData(this->index(index), value, roleNameToId(role)); +} + int AbstractListModel::baseModelCount() const { qFatal("AbstractListModel::baseModelCount() not implemented"); return 0; @@ -101,6 +105,15 @@ QHash AbstractListModel::roleNames() const { return roles; } +int AbstractListModel::roleNameToId(const QString &name) const { + QHash roles = roleNames(); + for (QHash::const_iterator it = roles.begin(); it != roles.end(); ++it) { + if (it.value() == name) return it.key(); + } + qCritical() << "Unknown role" << name; + return 0; +} + QString AbstractListModel::indexToId(int index) const { return data(this->index(index), IdRole).toString(); } diff --git a/QtClient/JoplinQtClient/models/abstractlistmodel.h b/QtClient/JoplinQtClient/models/abstractlistmodel.h index 555a2a6b6..9c497ae04 100755 --- a/QtClient/JoplinQtClient/models/abstractlistmodel.h +++ b/QtClient/JoplinQtClient/models/abstractlistmodel.h @@ -48,9 +48,11 @@ public slots: bool virtualItemShown() const; void hideVirtualItem(); QHash roleNames() const; + int roleNameToId(const QString& name) const; QString indexToId(int index) const; virtual int idToIndex(const QString& id) const; QString lastInsertId() const; + bool setData(int index, const QVariant &value, const QString& role = "edit"); }; diff --git a/QtClient/JoplinQtClient/models/basemodel.cpp b/QtClient/JoplinQtClient/models/basemodel.cpp index befc1315d..8b3462a36 100755 --- a/QtClient/JoplinQtClient/models/basemodel.cpp +++ b/QtClient/JoplinQtClient/models/basemodel.cpp @@ -14,10 +14,6 @@ BaseModel::BaseModel() { isNew_ = -1; } -//BaseModel::BaseModel(const BaseModel &baseModel) : BaseModel() { - -//} - QStringList BaseModel::changedFields() const { QStringList output; for (QHash::const_iterator it = changedFields_.begin(); it != changedFields_.end(); ++it) { @@ -132,11 +128,20 @@ bool BaseModel::save(bool trackChanges) { jop::db().commit(); - if (isSaved && table() == jop::FoldersTable) { - if (isNew) { - dispatcher().emitFolderCreated(id().toString()); - } else { - dispatcher().emitFolderUpdated(id().toString()); + if (isSaved) { + if (table() == jop::FoldersTable) { + if (isNew) { + dispatcher().emitFolderCreated(idString()); + } else { + dispatcher().emitFolderUpdated(idString()); + } + } + if (table() == jop::NotesTable) { + if (isNew) { + dispatcher().emitNoteCreated(idString()); + } else { + dispatcher().emitNoteUpdated(idString()); + } } } @@ -164,8 +169,9 @@ bool BaseModel::dispose() { change.save(); } - if (isDeleted && table() == jop::FoldersTable) { - dispatcher().emitFolderDeleted(id().toString()); + if (isDeleted) { + if (table() == jop::FoldersTable) dispatcher().emitFolderDeleted(idString()); + if (table() == jop::NotesTable) dispatcher().emitNoteDeleted(idString()); } return isDeleted; diff --git a/QtClient/JoplinQtClient/models/basemodel.h b/QtClient/JoplinQtClient/models/basemodel.h index b976a2265..d91a65f39 100755 --- a/QtClient/JoplinQtClient/models/basemodel.h +++ b/QtClient/JoplinQtClient/models/basemodel.h @@ -40,7 +40,6 @@ public: }; BaseModel(); - //BaseModel(const BaseModel& baseModel); QStringList changedFields() const; static int count(jop::Table table); bool load(const QString& id); diff --git a/QtClient/JoplinQtClient/models/foldermodel.cpp b/QtClient/JoplinQtClient/models/foldermodel.cpp index c9af6d3ee..114e9333a 100755 --- a/QtClient/JoplinQtClient/models/foldermodel.cpp +++ b/QtClient/JoplinQtClient/models/foldermodel.cpp @@ -5,10 +5,19 @@ using namespace jop; FolderModel::FolderModel() : AbstractListModel(), orderBy_("title") { - connect(&dispatcher(), SIGNAL(folderCreated(QString)), this, SLOT(dispatcher_folderCreated(QString))); - connect(&dispatcher(), SIGNAL(folderUpdated(QString)), this, SLOT(dispatcher_folderUpdated(QString))); - connect(&dispatcher(), SIGNAL(folderDeleted(QString)), this, SLOT(dispatcher_folderDeleted(QString))); - connect(&dispatcher(), SIGNAL(allFoldersDeleted()), this, SLOT(dispatcher_allFoldersDeleted())); + // Qt::QueuedConnection needs to be used here because in the dispatcher_XXX slots + // the object that is being worked on might get deleted via cacheClear(). For example: + // 1. setData() requests a model from the cache + // 2. it updates it and call model->save() + // 3. save() emits "folderUpdated" + // 4. in dispatcher_folderUpdated() (which, without QueuedConnection is called immediately) the cache is cleared, including the model + // 5. the model is now an invalid pointer and the rest of the code in save() crashes + // This is solved using QueuedConnection, as it means the cache will be cleared only once model is no longer in use. + + connect(&dispatcher(), SIGNAL(folderCreated(QString)), this, SLOT(dispatcher_folderCreated(QString)), Qt::QueuedConnection); + connect(&dispatcher(), SIGNAL(folderUpdated(QString)), this, SLOT(dispatcher_folderUpdated(QString)), Qt::QueuedConnection); + connect(&dispatcher(), SIGNAL(folderDeleted(QString)), this, SLOT(dispatcher_folderDeleted(QString)), Qt::QueuedConnection); + connect(&dispatcher(), SIGNAL(allFoldersDeleted()), this, SLOT(dispatcher_allFoldersDeleted()), Qt::QueuedConnection); } BaseModel* FolderModel::atIndex(int index) const { @@ -33,10 +42,6 @@ BaseModel* FolderModel::atIndex(int index) const { } } -//QString FolderModel::indexToId(int index) const { -// return data(this->index(index), IdRole).toString(); -//} - int FolderModel::idToIndex(const QString &id) const { int count = this->rowCount(); for (int i = 0; i < count; i++) { @@ -46,14 +51,13 @@ int FolderModel::idToIndex(const QString &id) const { return -1; } -//QString FolderModel::lastInsertId() const { -// return lastInsertId_; +//bool FolderModel::setTitle(int index, const QVariant &value, int role) { +// return setData(this->index(index), value, role); //} - -bool FolderModel::setTitle(int index, const QVariant &value, int role) { - return setData(this->index(index), value, role); -} +//bool FolderModel::setData(int index, const QVariant &value, int role) { +// return BaseModel::setData(this->index(index), value, role); +//} void FolderModel::addData(const QString &title) { Folder folder; diff --git a/QtClient/JoplinQtClient/models/foldermodel.h b/QtClient/JoplinQtClient/models/foldermodel.h index 5d28aa577..7fb9ff0b2 100755 --- a/QtClient/JoplinQtClient/models/foldermodel.h +++ b/QtClient/JoplinQtClient/models/foldermodel.h @@ -38,7 +38,8 @@ public slots: void addData(const QString& title); void deleteData(const int index); - bool setTitle(int index, const QVariant &value, int role = Qt::EditRole); + //bool setTitle(int index, const QVariant &value, int role = Qt::EditRole); + //bool setData(int index, const QVariant &value, int role = Qt::EditRole); int idToIndex(const QString& id) const; void dispatcher_folderCreated(const QString& folderId); diff --git a/QtClient/JoplinQtClient/models/notemodel.cpp b/QtClient/JoplinQtClient/models/notemodel.cpp index 5eec57876..1acf9ca53 100755 --- a/QtClient/JoplinQtClient/models/notemodel.cpp +++ b/QtClient/JoplinQtClient/models/notemodel.cpp @@ -1,10 +1,15 @@ #include "notemodel.h" +#include "dispatcher.h" namespace jop { NoteModel::NoteModel() : AbstractListModel() { folderId_ = ""; orderBy_ = "title"; + + connect(&dispatcher(), SIGNAL(noteCreated(QString)), this, SLOT(dispatcher_noteCreated(QString)), Qt::QueuedConnection); + connect(&dispatcher(), SIGNAL(noteUpdated(QString)), this, SLOT(dispatcher_noteUpdated(QString)), Qt::QueuedConnection); + connect(&dispatcher(), SIGNAL(noteDeleted(QString)), this, SLOT(dispatcher_noteDeleted(QString)), Qt::QueuedConnection); } Note *NoteModel::atIndex(int index) const { @@ -53,6 +58,15 @@ int NoteModel::idToIndex(const QString &id) const { return f.noteIndexById(orderBy_, id); } +void NoteModel::addData(const QString &title) { + Note note; + note.setValue("title", title); + note.setValue("parent_id", folderId_); + if (!note.save()) return; + + lastInsertId_ = note.idString(); +} + int NoteModel::baseModelCount() const { return folder().noteCount(); } @@ -73,4 +87,45 @@ void NoteModel::cacheClear() const { cache_.clear(); } +void NoteModel::dispatcher_noteCreated(const QString ¬eId) { + qDebug() << "NoteModel Folder created" << noteId; + + cacheClear(); + + int from = 0; + int to = rowCount() - 1; + + QVector roles; + roles << Qt::DisplayRole; + + // Necessary to make sure a new item is added to the view, even + // though it might not be positioned there due to sorting + beginInsertRows(QModelIndex(), to, to); + endInsertRows(); + + emit dataChanged(this->index(from), this->index(to), roles); +} + +void NoteModel::dispatcher_noteUpdated(const QString ¬eId) { + qDebug() << "NoteModel Folder udpated" << noteId; + + cacheClear(); + + QVector roles; + roles << Qt::DisplayRole; + emit dataChanged(this->index(0), this->index(rowCount() - 1), roles); +} + +void NoteModel::dispatcher_noteDeleted(const QString ¬eId) { + qDebug() << "NoteModel Folder deleted" << noteId; + + int index = idToIndex(noteId); + if (index < 0) return; + + cacheClear(); + + beginRemoveRows(QModelIndex(), index, index); + endRemoveRows(); +} + } diff --git a/QtClient/JoplinQtClient/models/notemodel.h b/QtClient/JoplinQtClient/models/notemodel.h index 00ef25dca..b4331b07c 100755 --- a/QtClient/JoplinQtClient/models/notemodel.h +++ b/QtClient/JoplinQtClient/models/notemodel.h @@ -23,6 +23,10 @@ public: public slots: int idToIndex(const QString& id) const; + void addData(const QString& title); + void dispatcher_noteCreated(const QString& noteId); + void dispatcher_noteUpdated(const QString& noteId); + void dispatcher_noteDeleted(const QString& noteId); protected: