From 15ac92ca2c3a0bfb66288fd270b61891569a9016 Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Sat, 1 Dec 2012 17:37:46 -0300 Subject: QMenuModel now keeps cache of any link element. --- libqmenumodel/src/qmenumodel.cpp | 69 +++++++++++++-- libqmenumodel/src/qmenumodel.h | 6 ++ tests/client/CMakeLists.txt | 1 + tests/client/cachetest.cpp | 175 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 6 deletions(-) create mode 100644 tests/client/cachetest.cpp diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index 53dc966..b18cc45 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -26,6 +26,24 @@ extern "C" { #include +class CacheData +{ +public: + CacheData(QMenuModel *link, int pos) + : link(link), + pos(pos) + { + } + + ~CacheData() + { + delete link; + } + + QMenuModel *link; + int pos; +}; + /*! \qmltype QMenuModel \brief The QMenuModel class implements the base list model for menus @@ -131,10 +149,10 @@ void QMenuModel::clearModel() m_menuModel = NULL; } - QList list = findChildren(QString(), Qt::FindDirectChildrenOnly); - Q_FOREACH(QMenuModel *model, list) { - delete model; + Q_FOREACH(CacheData *data, m_cache) { + delete data; } + m_cache.clear(); } /*! \internal */ @@ -226,8 +244,23 @@ QVariant QMenuModel::getLink(const QModelIndex &index, linkName.toUtf8().data()); if (link) { - QMenuModel *other = new QMenuModel(link, const_cast(this)); - return QVariant::fromValue(other); + QMenuModel *result = 0; + Q_FOREACH(CacheData *cache, m_cache) { + if ((cache->link->menuModel() == link) && + (cache->pos == index.row())) { + result = cache->link; + break; + } + } + + if (result == 0) { + QMenuModel *self = const_cast(this); + result = new QMenuModel(link, self); + self->m_cache << new CacheData(result, index.row()); + } + + g_object_unref(link); + return QVariant::fromValue(result); } return QVariant(); @@ -265,7 +298,13 @@ QVariant QMenuModel::getExtraProperties(const QModelIndex &index) const } /*! \internal */ -void QMenuModel::onItemsChanged(GMenuModel *, +QList QMenuModel::cache() const +{ + return m_cache; +} + +/*! \internal */ +void QMenuModel::onItemsChanged(GMenuModel *model, gint position, gint removed, gint added, @@ -275,11 +314,29 @@ void QMenuModel::onItemsChanged(GMenuModel *, if (removed > 0) { self->beginRemoveRows(QModelIndex(), position, position + removed - 1); + for(int i=position, iMax=position+removed; i < iMax; i++) { + QList lst = self->m_cache; + Q_FOREACH(CacheData* data, lst) { + if (data->pos == position) { + self->m_cache.removeOne(data); + delete data; + } else if (data->pos >= position) { + data->pos -= removed; + } + } + } self->endRemoveRows(); } if (added > 0) { self->beginInsertRows(QModelIndex(), position, position + added - 1); + for(int i=position, iMax=position+added; i < iMax; i++) { + Q_FOREACH(CacheData* data, self->m_cache) { + if (data->pos >= position) { + data->pos += added; + } + } + } self->endInsertRows(); } } diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index 9371bd8..de49368 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -28,6 +28,8 @@ typedef void* gpointer; typedef struct _GMenuModel GMenuModel; typedef struct _GObject GObject; +class CacheData; + class QMenuModel : public QAbstractListModel { Q_OBJECT @@ -60,7 +62,11 @@ protected: void setMenuModel(GMenuModel *model); GMenuModel *menuModel() const; + // help function for test + QList cache() const; + private: + QList m_cache; GMenuModel *m_menuModel; guint m_signalChangedId; diff --git a/tests/client/CMakeLists.txt b/tests/client/CMakeLists.txt index 392437c..0fcac9e 100644 --- a/tests/client/CMakeLists.txt +++ b/tests/client/CMakeLists.txt @@ -58,6 +58,7 @@ declare_test(modeltest) declare_test(actiongrouptest) declare_test(qmltest) declare_simple_test(convertertest) +declare_simple_test(cachetest) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/qmlfiles.h.in ${CMAKE_CURRENT_BINARY_DIR}/qmlfiles.h) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp new file mode 100644 index 0000000..16b3a6a --- /dev/null +++ b/tests/client/cachetest.cpp @@ -0,0 +1,175 @@ +/* + * Copyright 2012 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * Authors: + * Renato Araujo Oliveira Filho + */ + +#include "qmenumodel.h" + +extern "C" { +#include +} + +#include +#include + + +class MenuModelTestClass : public QMenuModel +{ + Q_OBJECT +public: + MenuModelTestClass() + : QMenuModel(0) + { + } + + void loadMenu() + { + GMenu *menu3 = g_menu_new(); + g_menu_append(menu3, "menu4", NULL); + g_menu_append(menu3, "menu5", NULL); + g_menu_append(menu3, "menu6", NULL); + + GMenu *menu = g_menu_new(); + g_menu_append(menu, "menu0", NULL); + g_menu_append(menu, "menu1", NULL); + g_menu_append(menu, "menu2", NULL); + g_menu_append_section(menu, "menu3", G_MENU_MODEL(menu3)); + + setMenuModel(G_MENU_MODEL(menu)); + + m_menus << menu << menu3; + } + + void removeItem(int section, int index) + { + GMenu *menu = m_menus[section]; + g_menu_remove(menu, index); + } + + void insertItem(int section, int index, const QString &label) + { + GMenu *menu = m_menus[section]; + g_menu_insert(menu, index, label.toUtf8().data(), NULL); + } + + int cacheSize() const + { + return cache().size(); + } + +private: + QList m_menus; +}; + +class CacheTest : public QObject +{ + Q_OBJECT +private Q_SLOTS: + void initTestCase() + { + g_type_init(); + } + + // + // Test if the link property always returns the same element + // + void testStaticMenuCache() + { + MenuModelTestClass menu; + menu.loadMenu(); + + QModelIndex index = menu.index(3); + + QVariant data = menu.data(index, QMenuModel::LinkSection); + QCOMPARE(menu.cacheSize(), 1); + + QVariant data2 = menu.data(index, QMenuModel::LinkSection); + QCOMPARE(menu.cacheSize(), 1); + + QVERIFY(data.value() == data2.value()); + + QMenuModel *section = qvariant_cast(data); + + index = section->index(1); + data = menu.data(index, QMenuModel::LinkSection); + data2 = menu.data(index, QMenuModel::LinkSection); + QVERIFY(data.value() == data2.value()); + } + + + // + // Test if cache works after add a new item + // + void testAddItem() + { + MenuModelTestClass menu; + menu.loadMenu(); + + QModelIndex index = menu.index(3); + QVariant data = menu.data(index, QMenuModel::LinkSection); + + menu.insertItem(0, 1, "newMenu"); + + index = menu.index(4); + QVariant data2 = menu.data(index, QMenuModel::LinkSection); + + QCOMPARE(menu.cacheSize(), 1); + QVERIFY(data.value() == data2.value()); + } + + + // + // Test if cache works after remove a item + // + void testRemoveItem() + { + MenuModelTestClass menu; + menu.loadMenu(); + + QModelIndex index = menu.index(3); + QVariant data = menu.data(index, QMenuModel::LinkSection); + + menu.removeItem(0, 1); + + index = menu.index(2); + QVariant data2 = menu.data(index, QMenuModel::LinkSection); + + QCOMPARE(menu.cacheSize(), 1); + QVERIFY(data.value() == data2.value()); + } + + // + // Test if cached item is removed after removed from the menu + // + void testRemoveCachedItem() + { + MenuModelTestClass menu; + menu.loadMenu(); + + QModelIndex index = menu.index(3); + QVariant data = menu.data(index, QMenuModel::LinkSection); + + QCOMPARE(menu.cacheSize(), 1); + menu.removeItem(0, 3); + QCOMPARE(menu.cacheSize(), 0); + } +}; + +QTEST_MAIN(CacheTest) + +#include "cachetest.moc" + -- cgit v1.2.3 From 5c529632253bb8e864b3cc81eaabbd0a423112aa Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Mon, 3 Dec 2012 09:36:59 -0300 Subject: Changed getLink function to not be const. --- libqmenumodel/src/qmenumodel.cpp | 17 +++++++++++------ libqmenumodel/src/qmenumodel.h | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index b18cc45..51b6863 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -185,11 +185,17 @@ QVariant QMenuModel::data(const QModelIndex &index, int role) const attribute = getStringAttribute(index, G_MENU_ATTRIBUTE_LABEL); break; case LinkSection: - attribute = getLink(index, G_MENU_LINK_SECTION); + { + QMenuModel *self = const_cast(this); + attribute = self->getLink(index, G_MENU_LINK_SECTION); break; + } case LinkSubMenu: - attribute = getLink(index, G_MENU_LINK_SUBMENU); + { + QMenuModel *self = const_cast(this); + attribute = self->getLink(index, G_MENU_LINK_SUBMENU); break; + } case Extra: attribute = getExtraProperties(index); break; @@ -235,7 +241,7 @@ QVariant QMenuModel::getStringAttribute(const QModelIndex &index, /*! \internal */ QVariant QMenuModel::getLink(const QModelIndex &index, - const QString &linkName) const + const QString &linkName) { GMenuModel *link; @@ -254,9 +260,8 @@ QVariant QMenuModel::getLink(const QModelIndex &index, } if (result == 0) { - QMenuModel *self = const_cast(this); - result = new QMenuModel(link, self); - self->m_cache << new CacheData(result, index.row()); + result = new QMenuModel(link, this); + m_cache << new CacheData(result, index.row()); } g_object_unref(link); diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index de49368..e0f69f2 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -71,7 +71,7 @@ private: guint m_signalChangedId; QVariant getStringAttribute(const QModelIndex &index, const QString &attribute) const; - QVariant getLink(const QModelIndex &index, const QString &linkName) const; + QVariant getLink(const QModelIndex &index, const QString &linkName); QVariant getExtraProperties(const QModelIndex &index) const; QString parseExtraPropertyName(const QString &name) const; void clearModel(); -- cgit v1.2.3 From d39a6e122441d79d4e448a26bb6ece48376ab157 Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Mon, 3 Dec 2012 10:12:02 -0300 Subject: Udated ignored files list. --- .bzrignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.bzrignore b/.bzrignore index 9038e8d..7c2d57a 100644 --- a/.bzrignore +++ b/.bzrignore @@ -16,6 +16,7 @@ coverage/ tests/client/*.moc tests/client/actiongrouptest +tests/client/cachetest tests/client/convertertest tests/client/menuchangestest tests/client/modeltest -- cgit v1.2.3 From 2f3299465de87c5801a5016c39b0fdb129347895 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Mon, 3 Dec 2012 18:52:59 +0100 Subject: Simplify the caching mechanism using a a QHash that associates indexes (int) to QMenuModel*. --- libqmenumodel/src/qmenumodel.cpp | 84 ++++++++++++++-------------------------- libqmenumodel/src/qmenumodel.h | 7 ++-- 2 files changed, 31 insertions(+), 60 deletions(-) diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index 51b6863..e9ce5fc 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -24,26 +24,6 @@ extern "C" { #include "qmenumodel.h" #include "converter.h" -#include - -class CacheData -{ -public: - CacheData(QMenuModel *link, int pos) - : link(link), - pos(pos) - { - } - - ~CacheData() - { - delete link; - } - - QMenuModel *link; - int pos; -}; - /*! \qmltype QMenuModel \brief The QMenuModel class implements the base list model for menus @@ -149,8 +129,8 @@ void QMenuModel::clearModel() m_menuModel = NULL; } - Q_FOREACH(CacheData *data, m_cache) { - delete data; + Q_FOREACH(QMenuModel* child, m_cache) { + delete child; } m_cache.clear(); } @@ -243,31 +223,25 @@ QVariant QMenuModel::getStringAttribute(const QModelIndex &index, QVariant QMenuModel::getLink(const QModelIndex &index, const QString &linkName) { - GMenuModel *link; - - link = g_menu_model_get_item_link(m_menuModel, - index.row(), - linkName.toUtf8().data()); - + GMenuModel *link = g_menu_model_get_item_link(m_menuModel, + index.row(), + linkName.toUtf8().data()); if (link) { - QMenuModel *result = 0; - Q_FOREACH(CacheData *cache, m_cache) { - if ((cache->link->menuModel() == link) && - (cache->pos == index.row())) { - result = cache->link; - break; + QMenuModel* child = 0; + int key = index.row(); + if (m_cache.contains(key)) { + QMenuModel* cached = m_cache.value(key); + if (cached->menuModel() == link) { + child = cached; } } - - if (result == 0) { - result = new QMenuModel(link, this); - m_cache << new CacheData(result, index.row()); + if (child == 0) { + child = new QMenuModel(link, this); + m_cache.insert(key, child); } - g_object_unref(link); - return QVariant::fromValue(result); + return QVariant::fromValue(child); } - return QVariant(); } @@ -303,7 +277,7 @@ QVariant QMenuModel::getExtraProperties(const QModelIndex &index) const } /*! \internal */ -QList QMenuModel::cache() const +QHash QMenuModel::cache() const { return m_cache; } @@ -317,17 +291,17 @@ void QMenuModel::onItemsChanged(GMenuModel *model, { QMenuModel *self = reinterpret_cast(data); + int prevcount = g_menu_model_get_n_items(model) + removed - added; if (removed > 0) { self->beginRemoveRows(QModelIndex(), position, position + removed - 1); - for(int i=position, iMax=position+removed; i < iMax; i++) { - QList lst = self->m_cache; - Q_FOREACH(CacheData* data, lst) { - if (data->pos == position) { - self->m_cache.removeOne(data); - delete data; - } else if (data->pos >= position) { - data->pos -= removed; - } + for (int i = position, iMax = position + removed; i < iMax; ++i) { + if (self->m_cache.contains(i)) { + delete self->m_cache.take(i); + } + } + for (int i = position + removed; i < prevcount; ++i) { + if (self->m_cache.contains(i)) { + self->m_cache.insert(i - removed, self->m_cache.take(i)); } } self->endRemoveRows(); @@ -335,11 +309,9 @@ void QMenuModel::onItemsChanged(GMenuModel *model, if (added > 0) { self->beginInsertRows(QModelIndex(), position, position + added - 1); - for(int i=position, iMax=position+added; i < iMax; i++) { - Q_FOREACH(CacheData* data, self->m_cache) { - if (data->pos >= position) { - data->pos += added; - } + for (int i = prevcount - removed - 1; i >= position; --i) { + if (self->m_cache.contains(i)) { + self->m_cache.insert(i + added, self->m_cache.take(i)); } } self->endInsertRows(); diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index e0f69f2..fef75e4 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -21,6 +21,7 @@ #define QMENUMODEL_H #include +#include typedef int gint; typedef unsigned int guint; @@ -28,8 +29,6 @@ typedef void* gpointer; typedef struct _GMenuModel GMenuModel; typedef struct _GObject GObject; -class CacheData; - class QMenuModel : public QAbstractListModel { Q_OBJECT @@ -63,10 +62,10 @@ protected: GMenuModel *menuModel() const; // help function for test - QList cache() const; + QHash cache() const; private: - QList m_cache; + QHash m_cache; GMenuModel *m_menuModel; guint m_signalChangedId; -- cgit v1.2.3 From 7b2badf6f2c2d5d4596ca3778b4cf924d498e2d7 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 08:30:24 +0100 Subject: Make the cache a pointer, to enforce const correctness. --- libqmenumodel/src/qmenumodel.cpp | 41 +++++++++++++++++++--------------------- libqmenumodel/src/qmenumodel.h | 5 ++--- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index e9ce5fc..524953f 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -39,6 +39,7 @@ QMenuModel::QMenuModel(GMenuModel *other, QObject *parent) m_menuModel(0), m_signalChangedId(0) { + m_cache = new QHash; setMenuModel(other); connect(this, SIGNAL(rowsInserted(const QModelIndex &, int, int)), SIGNAL(countChanged())); @@ -50,6 +51,7 @@ QMenuModel::QMenuModel(GMenuModel *other, QObject *parent) QMenuModel::~QMenuModel() { clearModel(); + delete m_cache; } /*! @@ -129,10 +131,10 @@ void QMenuModel::clearModel() m_menuModel = NULL; } - Q_FOREACH(QMenuModel* child, m_cache) { + Q_FOREACH(QMenuModel* child, *m_cache) { delete child; } - m_cache.clear(); + m_cache->clear(); } /*! \internal */ @@ -165,17 +167,11 @@ QVariant QMenuModel::data(const QModelIndex &index, int role) const attribute = getStringAttribute(index, G_MENU_ATTRIBUTE_LABEL); break; case LinkSection: - { - QMenuModel *self = const_cast(this); - attribute = self->getLink(index, G_MENU_LINK_SECTION); + attribute = getLink(index, G_MENU_LINK_SECTION); break; - } case LinkSubMenu: - { - QMenuModel *self = const_cast(this); - attribute = self->getLink(index, G_MENU_LINK_SUBMENU); + attribute = getLink(index, G_MENU_LINK_SUBMENU); break; - } case Extra: attribute = getExtraProperties(index); break; @@ -221,7 +217,7 @@ QVariant QMenuModel::getStringAttribute(const QModelIndex &index, /*! \internal */ QVariant QMenuModel::getLink(const QModelIndex &index, - const QString &linkName) + const QString &linkName) const { GMenuModel *link = g_menu_model_get_item_link(m_menuModel, index.row(), @@ -229,15 +225,15 @@ QVariant QMenuModel::getLink(const QModelIndex &index, if (link) { QMenuModel* child = 0; int key = index.row(); - if (m_cache.contains(key)) { - QMenuModel* cached = m_cache.value(key); + if (m_cache->contains(key)) { + QMenuModel* cached = m_cache->value(key); if (cached->menuModel() == link) { child = cached; } } if (child == 0) { - child = new QMenuModel(link, this); - m_cache.insert(key, child); + child = new QMenuModel(link); + m_cache->insert(key, child); } g_object_unref(link); return QVariant::fromValue(child); @@ -279,7 +275,7 @@ QVariant QMenuModel::getExtraProperties(const QModelIndex &index) const /*! \internal */ QHash QMenuModel::cache() const { - return m_cache; + return *m_cache; } /*! \internal */ @@ -290,18 +286,19 @@ void QMenuModel::onItemsChanged(GMenuModel *model, gpointer data) { QMenuModel *self = reinterpret_cast(data); + QHash* cache = self->m_cache; int prevcount = g_menu_model_get_n_items(model) + removed - added; if (removed > 0) { self->beginRemoveRows(QModelIndex(), position, position + removed - 1); for (int i = position, iMax = position + removed; i < iMax; ++i) { - if (self->m_cache.contains(i)) { - delete self->m_cache.take(i); + if (cache->contains(i)) { + delete cache->take(i); } } for (int i = position + removed; i < prevcount; ++i) { - if (self->m_cache.contains(i)) { - self->m_cache.insert(i - removed, self->m_cache.take(i)); + if (cache->contains(i)) { + cache->insert(i - removed, cache->take(i)); } } self->endRemoveRows(); @@ -310,8 +307,8 @@ void QMenuModel::onItemsChanged(GMenuModel *model, if (added > 0) { self->beginInsertRows(QModelIndex(), position, position + added - 1); for (int i = prevcount - removed - 1; i >= position; --i) { - if (self->m_cache.contains(i)) { - self->m_cache.insert(i + added, self->m_cache.take(i)); + if (cache->contains(i)) { + cache->insert(i + added, cache->take(i)); } } self->endInsertRows(); diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index fef75e4..f77f472 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -21,7 +21,6 @@ #define QMENUMODEL_H #include -#include typedef int gint; typedef unsigned int guint; @@ -65,12 +64,12 @@ protected: QHash cache() const; private: - QHash m_cache; + QHash* m_cache; GMenuModel *m_menuModel; guint m_signalChangedId; QVariant getStringAttribute(const QModelIndex &index, const QString &attribute) const; - QVariant getLink(const QModelIndex &index, const QString &linkName); + QVariant getLink(const QModelIndex &index, const QString &linkName) const; QVariant getExtraProperties(const QModelIndex &index) const; QString parseExtraPropertyName(const QString &name) const; void clearModel(); -- cgit v1.2.3 From 517c21c4f3b4ff3e4e07b0e0f9cc41c3cefccba7 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 08:32:13 +0100 Subject: Add myself to the list of authors. --- libqmenumodel/src/qmenumodel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index 524953f..e9f6acc 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -15,6 +15,7 @@ * * Authors: * Renato Araujo Oliveira Filho + * Olivier Tilloy */ extern "C" { -- cgit v1.2.3 From 4b57869f28ed1721a00e3d498c3fb268d1704ac6 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 08:32:24 +0100 Subject: Remove useless include. --- tests/client/cachetest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index 16b3a6a..245ad9b 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -24,7 +24,6 @@ extern "C" { } #include -#include class MenuModelTestClass : public QMenuModel -- cgit v1.2.3 From af2608a0a41a8d3092035f823257dc2274079a94 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 08:34:20 +0100 Subject: Simplify test code. --- tests/client/cachetest.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index 245ad9b..bd29dfe 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -32,10 +32,6 @@ class MenuModelTestClass : public QMenuModel public: MenuModelTestClass() : QMenuModel(0) - { - } - - void loadMenu() { GMenu *menu3 = g_menu_new(); g_menu_append(menu3, "menu4", NULL); @@ -89,7 +85,6 @@ private Q_SLOTS: void testStaticMenuCache() { MenuModelTestClass menu; - menu.loadMenu(); QModelIndex index = menu.index(3); @@ -116,7 +111,6 @@ private Q_SLOTS: void testAddItem() { MenuModelTestClass menu; - menu.loadMenu(); QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); @@ -137,7 +131,6 @@ private Q_SLOTS: void testRemoveItem() { MenuModelTestClass menu; - menu.loadMenu(); QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); @@ -157,7 +150,6 @@ private Q_SLOTS: void testRemoveCachedItem() { MenuModelTestClass menu; - menu.loadMenu(); QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); -- cgit v1.2.3 From 50d449d489262f644a33d2d2fb1479b235bc23c2 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 08:53:19 +0100 Subject: Compare the exact cache indexes instead of just comparing the size of the cache. --- tests/client/cachetest.cpp | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index bd29dfe..381761f 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -26,12 +26,11 @@ extern "C" { #include -class MenuModelTestClass : public QMenuModel +class TestModel : public QMenuModel { Q_OBJECT public: - MenuModelTestClass() - : QMenuModel(0) + TestModel() : QMenuModel(0) { GMenu *menu3 = g_menu_new(); g_menu_append(menu3, "menu4", NULL); @@ -61,9 +60,11 @@ public: g_menu_insert(menu, index, label.toUtf8().data(), NULL); } - int cacheSize() const + QList cacheIndexes() const { - return cache().size(); + QList indexes = cache().keys(); + qSort(indexes); + return indexes; } private: @@ -73,6 +74,7 @@ private: class CacheTest : public QObject { Q_OBJECT + private Q_SLOTS: void initTestCase() { @@ -84,15 +86,15 @@ private Q_SLOTS: // void testStaticMenuCache() { - MenuModelTestClass menu; + TestModel menu; QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); - QCOMPARE(menu.cacheSize(), 1); + QCOMPARE(menu.cacheIndexes(), QList() << 3); QVariant data2 = menu.data(index, QMenuModel::LinkSection); - QCOMPARE(menu.cacheSize(), 1); + QCOMPARE(menu.cacheIndexes(), QList() << 3); QVERIFY(data.value() == data2.value()); @@ -101,6 +103,7 @@ private Q_SLOTS: index = section->index(1); data = menu.data(index, QMenuModel::LinkSection); data2 = menu.data(index, QMenuModel::LinkSection); + QCOMPARE(menu.cacheIndexes(), QList() << 3); QVERIFY(data.value() == data2.value()); } @@ -110,17 +113,19 @@ private Q_SLOTS: // void testAddItem() { - MenuModelTestClass menu; + TestModel menu; QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); + QCOMPARE(menu.cacheIndexes(), QList() << 3); menu.insertItem(0, 1, "newMenu"); + QCOMPARE(menu.cacheIndexes(), QList() << 4); index = menu.index(4); QVariant data2 = menu.data(index, QMenuModel::LinkSection); - QCOMPARE(menu.cacheSize(), 1); + QCOMPARE(menu.cacheIndexes(), QList() << 4); QVERIFY(data.value() == data2.value()); } @@ -130,17 +135,19 @@ private Q_SLOTS: // void testRemoveItem() { - MenuModelTestClass menu; + TestModel menu; QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); + QCOMPARE(menu.cacheIndexes(), QList() << 3); menu.removeItem(0, 1); + QCOMPARE(menu.cacheIndexes(), QList() << 2); index = menu.index(2); QVariant data2 = menu.data(index, QMenuModel::LinkSection); - QCOMPARE(menu.cacheSize(), 1); + QCOMPARE(menu.cacheIndexes(), QList() << 2); QVERIFY(data.value() == data2.value()); } @@ -149,14 +156,14 @@ private Q_SLOTS: // void testRemoveCachedItem() { - MenuModelTestClass menu; + TestModel menu; QModelIndex index = menu.index(3); QVariant data = menu.data(index, QMenuModel::LinkSection); + QCOMPARE(menu.cacheIndexes(), QList() << 3); - QCOMPARE(menu.cacheSize(), 1); menu.removeItem(0, 3); - QCOMPARE(menu.cacheSize(), 0); + QVERIFY(menu.cacheIndexes().isEmpty()); } }; -- cgit v1.2.3 From bf8e579d5da42dc554a6889402a62c19ce02f2bc Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 08:57:38 +0100 Subject: Updated comments. --- tests/client/cachetest.cpp | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index 381761f..9719023 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -81,9 +81,7 @@ private Q_SLOTS: g_type_init(); } - // - // Test if the link property always returns the same element - // + // Test if the link attribute always returns the same cached menu void testStaticMenuCache() { TestModel menu; @@ -107,10 +105,7 @@ private Q_SLOTS: QVERIFY(data.value() == data2.value()); } - - // - // Test if cache works after add a new item - // + // Test if the cache is correctly updated after inserting a new item void testAddItem() { TestModel menu; @@ -129,10 +124,7 @@ private Q_SLOTS: QVERIFY(data.value() == data2.value()); } - - // - // Test if cache works after remove a item - // + // Test if the cache is correctly updated after removing an item that wasn’t cached void testRemoveItem() { TestModel menu; @@ -151,9 +143,7 @@ private Q_SLOTS: QVERIFY(data.value() == data2.value()); } - // - // Test if cached item is removed after removed from the menu - // + // Test if the cache is correctly updated after removing a cached item void testRemoveCachedItem() { TestModel menu; -- cgit v1.2.3 From e8927d8a16a211c9994653bbb9ebbfd451e61c2b Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 09:11:18 +0100 Subject: Add a test to verify that the cache is correctly updated after multiple insertions and removals. --- tests/client/cachetest.cpp | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index 9719023..abd3331 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -81,7 +81,7 @@ private Q_SLOTS: g_type_init(); } - // Test if the link attribute always returns the same cached menu + // Verify that the link attribute always returns the same cached menu void testStaticMenuCache() { TestModel menu; @@ -105,8 +105,8 @@ private Q_SLOTS: QVERIFY(data.value() == data2.value()); } - // Test if the cache is correctly updated after inserting a new item - void testAddItem() + // Verify that the cache is correctly updated after inserting a new item + void testInsertItems() { TestModel menu; @@ -114,6 +114,9 @@ private Q_SLOTS: QVariant data = menu.data(index, QMenuModel::LinkSection); QCOMPARE(menu.cacheIndexes(), QList() << 3); + menu.insertItem(0, 4, "newMenu"); + QCOMPARE(menu.cacheIndexes(), QList() << 3); + menu.insertItem(0, 1, "newMenu"); QCOMPARE(menu.cacheIndexes(), QList() << 4); @@ -124,8 +127,8 @@ private Q_SLOTS: QVERIFY(data.value() == data2.value()); } - // Test if the cache is correctly updated after removing an item that wasn’t cached - void testRemoveItem() + // Verify that the cache is correctly updated after removing an item that wasn’t cached + void testRemoveNonCachedItem() { TestModel menu; @@ -143,7 +146,7 @@ private Q_SLOTS: QVERIFY(data.value() == data2.value()); } - // Test if the cache is correctly updated after removing a cached item + // Verify that the cache is correctly updated after removing a cached item void testRemoveCachedItem() { TestModel menu; @@ -155,6 +158,31 @@ private Q_SLOTS: menu.removeItem(0, 3); QVERIFY(menu.cacheIndexes().isEmpty()); } + + // Verify that the cache is correctly updated after multiple insertions and removals + void testMultiplesUpdates() + { + TestModel menu; + QVERIFY(menu.cacheIndexes().isEmpty()); + + menu.data(menu.index(3), QMenuModel::LinkSection); + QCOMPARE(menu.cacheIndexes(), QList() << 3); + + menu.insertItem(0, 1, "newMenu"); + menu.insertItem(0, 2, "newMenu"); + menu.insertItem(0, 6, "newMenu"); + menu.insertItem(0, 3, "newMenu"); + menu.insertItem(0, 7, "newMenu"); + QCOMPARE(menu.cacheIndexes(), QList() << 6); + + menu.removeItem(0, 4); + menu.removeItem(0, 6); + menu.removeItem(0, 2); + QCOMPARE(menu.cacheIndexes(), QList() << 4); + + menu.removeItem(0, 4); + QVERIFY(menu.cacheIndexes().isEmpty()); + } }; QTEST_MAIN(CacheTest) -- cgit v1.2.3 From 02cfba5c7f3781bb2d7cd2a4a55948efa4ed7918 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 09:14:04 +0100 Subject: Add a test to verify that normal menu items are not cached (only sub-menus are). --- tests/client/cachetest.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index abd3331..8d5bd14 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -81,6 +81,19 @@ private Q_SLOTS: g_type_init(); } + // Verify that normal menu items are not cached (only sub-menus are) + void testCacheContents() + { + TestModel menu; + QVERIFY(menu.cacheIndexes().isEmpty()); + + menu.data(menu.index(1), QMenuModel::Label); + QVERIFY(menu.cacheIndexes().isEmpty()); + + menu.data(menu.index(2), QMenuModel::Action); + QVERIFY(menu.cacheIndexes().isEmpty()); + } + // Verify that the link attribute always returns the same cached menu void testStaticMenuCache() { -- cgit v1.2.3 From 395a8c64acee78f206b021b3218de4641c4d95d6 Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 09:21:06 +0100 Subject: Add comments to explain non-trivial cache updates. --- libqmenumodel/src/qmenumodel.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index e9f6acc..1326d77 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -292,11 +292,13 @@ void QMenuModel::onItemsChanged(GMenuModel *model, int prevcount = g_menu_model_get_n_items(model) + removed - added; if (removed > 0) { self->beginRemoveRows(QModelIndex(), position, position + removed - 1); + // Remove invalidated menus from the cache for (int i = position, iMax = position + removed; i < iMax; ++i) { if (cache->contains(i)) { delete cache->take(i); } } + // Update the indexes of other cached menus to account for the removals for (int i = position + removed; i < prevcount; ++i) { if (cache->contains(i)) { cache->insert(i - removed, cache->take(i)); @@ -307,6 +309,7 @@ void QMenuModel::onItemsChanged(GMenuModel *model, if (added > 0) { self->beginInsertRows(QModelIndex(), position, position + added - 1); + // Update the indexes of cached menus to account for the insertions for (int i = prevcount - removed - 1; i >= position; --i) { if (cache->contains(i)) { cache->insert(i + added, cache->take(i)); -- cgit v1.2.3 From f9ced66a11b879cf86239d06ed92ca2863dd83ac Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 09:31:33 +0100 Subject: Add myself to the list of authors. --- tests/client/cachetest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/client/cachetest.cpp b/tests/client/cachetest.cpp index 8d5bd14..22fe5d3 100644 --- a/tests/client/cachetest.cpp +++ b/tests/client/cachetest.cpp @@ -15,6 +15,7 @@ * * Authors: * Renato Araujo Oliveira Filho + * Olivier Tilloy */ #include "qmenumodel.h" -- cgit v1.2.3 From ac9450d43d09d817d0f8855ac8c691cab11eddbe Mon Sep 17 00:00:00 2001 From: Olivier Tilloy Date: Tue, 4 Dec 2012 09:31:46 +0100 Subject: Better comment. --- libqmenumodel/src/qmenumodel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index f77f472..54a5c42 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -60,7 +60,7 @@ protected: void setMenuModel(GMenuModel *model); GMenuModel *menuModel() const; - // help function for test + // helper getter intended for use in tests only QHash cache() const; private: -- cgit v1.2.3