Notify entry/group parent on deletion.

Also make the root group pseudo static, i.e. it shouldn't be changed
after the database has been fully constructed.

src/core/Database.cpp

@@ -28,7 +28,7 @@
 Database::Database()
 {
     m_metadata = new Metadata(this);
-    m_rootGroup = 0;
+    setRootGroup(new Group());
 
     m_cipher = KeePass2::CIPHER_AES;
     m_compressionAlgo = CompressionGZip;
@@ -47,11 +47,10 @@ const Group* Database::rootGroup() const
 
 void Database::setRootGroup(Group* group)
 {
-    if (group != 0) {
-        group->setParent(this);
-    }
+    Q_ASSERT(group);
 
     m_rootGroup = group;
+    m_rootGroup->setParent(this);
 }
 
 Metadata* Database::metadata()

src/core/Database.h

@@ -53,6 +53,10 @@ public:
 
     /**
      * Sets group as the root group and takes ownership of it.
+     * Warning: Be careful when calling this method as it doesn't
+     *          emit any notifications so e.g. models aren't updated.
+     *          The caller is responsible for cleaning up the pervious
+                root group.
      */
     void setRootGroup(Group* group);
 

src/core/Entry.cpp

@@ -40,7 +40,10 @@ Entry::Entry()
 
 Entry::~Entry()
 {
-    // TODO notify group
+    if (m_group) {
+        m_group->removeEntry(this);
+    }
+
     qDeleteAll(m_history);
 }
 

src/core/Entry.h

@@ -19,6 +19,7 @@
 #define KEEPASSX_ENTRY_H
 
 #include <QtCore/QHash>
+#include <QtCore/QPointer>
 #include <QtCore/QSet>
 #include <QtCore/QUrl>
 #include <QtGui/QColor>
@@ -122,7 +123,7 @@ private:
     QSet<QString> m_protectedAttachments;
 
     QList<Entry*> m_history;
-    Group* m_group;
+    QPointer<Group> m_group;
     const Database* m_db;
     const static QStringList m_defaultAttibutes;
 };

src/core/Group.cpp

@@ -37,7 +37,7 @@ Group::Group()
 
 Group::~Group()
 {
-    // TODO notify parent
+    cleanupParent();
 }
 
 Uuid Group::uuid() const
@@ -186,22 +186,14 @@ void Group::setParent(Group* parent, int index)
 {
     Q_ASSERT(parent);
     Q_ASSERT(index >= -1 && index <= parent->children().size());
+    // setting a new parent for root groups is not allowed
+    Q_ASSERT(!m_db || (m_db->rootGroup() != this));
 
     if (index == -1) {
         index = parent->children().size();
     }
 
-    Q_EMIT aboutToRemove(this);
-
-    if (m_parent) {
-        m_parent->m_children.removeAll(this);
-    }
-    else if (m_db) {
-        // parent was a Database
-        m_db->setRootGroup(0);
-    }
-
-    Q_EMIT removed();
+    cleanupParent();
 
     m_parent = parent;
 
@@ -222,16 +214,7 @@ void Group::setParent(Database* db)
 {
     Q_ASSERT(db);
 
-    Q_EMIT aboutToRemove(this);
-
-    if (m_parent) {
-        m_parent->m_children.removeAll(this);
-    }
-    else if (m_db) {
-        m_db->setRootGroup(0);
-    }
-
-    Q_EMIT removed();
+    cleanupParent();
 
     m_parent = 0;
     recSetDatabase(db);
@@ -295,6 +278,7 @@ void Group::recSetDatabase(Database* db)
     disconnect(SIGNAL(added()), m_db);
 
     connect(this, SIGNAL(dataChanged(Group*)), db, SIGNAL(groupDataChanged(Group*)));
+
     connect(this, SIGNAL(aboutToRemove(Group*)), db, SIGNAL(groupAboutToRemove(Group*)));
     connect(this, SIGNAL(removed()), db, SIGNAL(groupRemoved()));
     connect(this, SIGNAL(aboutToAdd(Group*,int)), db, SIGNAL(groupAboutToAdd(Group*,int)));
@@ -306,3 +290,12 @@ void Group::recSetDatabase(Database* db)
         group->recSetDatabase(db);
     }
 }
+
+void Group::cleanupParent()
+{
+    if (m_parent) {
+        Q_EMIT aboutToRemove(this);
+        m_parent->m_children.removeAll(this);
+        Q_EMIT removed();
+    }
+}

src/core/Group.h

@@ -18,6 +18,7 @@
 #ifndef KEEPASSX_GROUP_H
 #define KEEPASSX_GROUP_H
 
+#include <QtCore/QPointer>
 #include <QtGui/QIcon>
 
 #include "core/Database.h"
@@ -90,8 +91,9 @@ private:
     void setParent(Database* db);
 
     void recSetDatabase(Database* db);
+    void cleanupParent();
 
-    Database* m_db;
+    QPointer<Database> m_db;
     Uuid m_uuid;
     QString m_name;
     QString m_notes;
@@ -106,9 +108,10 @@ private:
     QList<Group*> m_children;
     QList<Entry*> m_entries;
 
-    Group* m_parent;
+    QPointer<Group> m_parent;
 
     friend void Database::setRootGroup(Group* group);
+    friend Entry::~Entry();
     friend void Entry::setGroup(Group *group);
 };
 

src/format/KeePass2XmlReader.cpp

@@ -41,7 +41,6 @@ void KeePass2XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2Ra
     m_randomStream = randomStream;
 
     m_tmpParent = new Group();
-    m_db->setRootGroup(m_tmpParent);
 
     if (!m_xml.error() && m_xml.readNextStartElement()) {
         if (m_xml.name() == "KeePassFile") {
@@ -49,6 +48,7 @@ void KeePass2XmlReader::readDatabase(QIODevice* device, Database* db, KeePass2Ra
         }
     }
 
+    // TODO check if m_tmpParent doesn't have entries
     if (!m_xml.error() && !m_tmpParent->children().isEmpty()) {
         raiseError();
     }
@@ -277,7 +277,9 @@ void KeePass2XmlReader::parseRoot()
         if (m_xml.name() == "Group") {
             Group* rootGroup = parseGroup();
             if (rootGroup) {
+                Group* oldRoot = m_db->rootGroup();
                 m_db->setRootGroup(rootGroup);
+                delete oldRoot;
             }
         }
         else if (m_xml.name() == "DeletedObjects") {

tests/TestGroup.cpp

@@ -32,6 +32,7 @@ void TestGroup::initTestCase()
 void TestGroup::testParenting()
 {
     Database* db = new Database();
+    QPointer<Group> rootGroup = db->rootGroup();
     Group* tmpRoot = new Group();
 
     QPointer<Group> g1 = new Group();
@@ -47,9 +48,9 @@ void TestGroup::testParenting()
     g2->setParent(g1);
     g4->setParent(g3);
     g3->setParent(g1);
-    db->setRootGroup(g1);
+    g1->setParent(db->rootGroup());
 
-    QVERIFY(g1->parent() == db);
+    QVERIFY(g1->parent() == rootGroup);
     QVERIFY(g2->parent() == g1);
     QVERIFY(g3->parent() == g1);
     QVERIFY(g4->parent() == g3);
@@ -60,11 +61,13 @@ void TestGroup::testParenting()
     QVERIFY(g4->database() == db);
 
     QCOMPARE(tmpRoot->children().size(), 0);
+    QCOMPARE(rootGroup->children().size(), 1);
     QCOMPARE(g1->children().size(), 2);
     QCOMPARE(g2->children().size(), 0);
     QCOMPARE(g3->children().size(), 1);
     QCOMPARE(g4->children().size(), 0);
 
+    QVERIFY(rootGroup->children().at(0) == g1);
     QVERIFY(g1->children().at(0) == g2);
     QVERIFY(g1->children().at(1) == g3);
     QVERIFY(g3->children().contains(g4));
@@ -80,6 +83,7 @@ void TestGroup::testParenting()
 
     delete db;
 
+    QVERIFY(rootGroup.isNull());
     QVERIFY(g1.isNull());
     QVERIFY(g2.isNull());
     QVERIFY(g3.isNull());
@@ -91,8 +95,7 @@ void TestGroup::testParenting()
 void TestGroup::testSignals()
 {
     Database* db = new Database();
-    QPointer<Group> root = new Group();
-    db->setRootGroup(root);
+    QPointer<Group> root = db->rootGroup();
 
     Group* g1 = new Group();
     Group* g2 = new Group();
@@ -136,4 +139,38 @@ void TestGroup::testEntries()
     QVERIFY(entry2.isNull());
 }
 
+void TestGroup::testDeleteSignals()
+{
+    Database* db = new Database();
+    Group* groupRoot = db->rootGroup();
+    Group* groupChild = new Group();
+    Group* groupChildChild = new Group();
+    groupRoot->setObjectName("groupRoot");
+    groupChild->setObjectName("groupChild");
+    groupChildChild->setObjectName("groupChildChild");
+    groupChild->setParent(groupRoot);
+    groupChildChild->setParent(groupChild);
+    QSignalSpy spyAboutToRemove(db, SIGNAL(groupAboutToRemove(Group*)));
+    QSignalSpy spyRemoved(db, SIGNAL(groupRemoved()));
+
+    delete groupChild;
+    QVERIFY(groupRoot->children().isEmpty());
+    QCOMPARE(spyAboutToRemove.count(), 1);
+    QCOMPARE(spyRemoved.count(), 1);
+    delete db;
+
+
+    Group* group = new Group();
+    Entry* entry = new Entry();
+    entry->setGroup(group);
+    QSignalSpy spyEntryAboutToRemove(group, SIGNAL(entryAboutToRemove(Entry*)));
+    QSignalSpy spyEntryRemoved(group, SIGNAL(entryRemoved()));
+
+    delete entry;
+    QVERIFY(group->entries().isEmpty());
+    QCOMPARE(spyEntryAboutToRemove.count(), 1);
+    QCOMPARE(spyEntryRemoved.count(), 1);
+    delete group;
+}
+
 QTEST_MAIN(TestGroup);

tests/TestGroup.h

@@ -29,6 +29,7 @@ private Q_SLOTS:
     void testParenting();
     void testSignals();
     void testEntries();
+    void testDeleteSignals();
 };
 
 #endif // KEEPASSX_TESTGROUP_H

tests/TestGroupModel.cpp

@@ -34,27 +34,32 @@ void TestGroupModel::test()
 {
     Database* db = new Database();
 
-    Group* groupRoot = new Group();
-    db->setRootGroup(groupRoot);
+    Group* groupRoot = db->rootGroup();
+    groupRoot->setObjectName("groupRoot");
     groupRoot->setName("groupRoot");
 
     Group* group1 = new Group();
+    group1->setObjectName("group1");
     group1->setName("group1");
     group1->setParent(groupRoot);
 
     Group* group11 = new Group();
+    group1->setObjectName("group11");
     group11->setName("group11");
     group11->setParent(group1);
 
     Group* group12 = new Group();
+    group1->setObjectName("group12");
     group12->setName("group12");
     group12->setParent(group1);
 
     Group* group121 = new Group();
+    group1->setObjectName("group121");
     group121->setName("group121");
     group121->setParent(group12);
 
     Group* group2 = new Group();
+    group1->setObjectName("group2");
     group2->setName("group2");
     group2->setParent(groupRoot);
 
@@ -93,7 +98,9 @@ void TestGroupModel::test()
     QCOMPARE(spyAboutToRemove.count(), 1);
     QCOMPARE(spyRemoved.count(), 1);
 
-    delete groupRoot;
+    // test removing a group that has children
+    delete group1;
+
     delete db;
 
     delete modelTest;

tests/TestKeePass2Writer.cpp

@@ -38,9 +38,8 @@ void TestKeePass2Writer::initTestCase()
     m_dbOrg = new Database();
     m_dbOrg->setKey(key);
     m_dbOrg->metadata()->setName("TESTDB");
-    Group* group = new Group();
+    Group* group = m_dbOrg->rootGroup();
     group->setUuid(Uuid::random());
-    m_dbOrg->setRootGroup(group);
     Entry* entry = new Entry();
     entry->setUuid(Uuid::random());
     entry->addAttribute("test", "protectedTest", true);