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.

11 files changed, 90 insertions, 41 deletions

diff --git a/src/core/Database.cpp b/src/core/Database.cpp
index 2fe208909..20fe8cbc7 100644
--- a/src/core/Database.cpp
+++ b/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()
diff --git a/src/core/Database.h b/src/core/Database.h
index 8876b8382..dc7fa3bbb 100644
--- a/src/core/Database.h
+++ b/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);
 
diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp
index ac47a2fc3..0d7c1848d 100644
--- a/src/core/Entry.cpp
+++ b/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);
 }
 
diff --git a/src/core/Entry.h b/src/core/Entry.h
index 248bb71a4..319b0da04 100644
--- a/src/core/Entry.h
+++ b/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;
 };
diff --git a/src/core/Group.cpp b/src/core/Group.cpp
index 543cdb75c..e5732c8f1 100644
--- a/src/core/Group.cpp
+++ b/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();
+    }
+}
diff --git a/src/core/Group.h b/src/core/Group.h
index 54febf9ce..64734a7ad 100644
--- a/src/core/Group.h
+++ b/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);
 };
 
diff --git a/src/format/KeePass2XmlReader.cpp b/src/format/KeePass2XmlReader.cpp
index aa2549218..af3b7d450 100644
--- a/src/format/KeePass2XmlReader.cpp
+++ b/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") {
@@ -302,7 +304,7 @@ Group* KeePass2XmlReader::parseGroup()
             }
             else {
                 group = getGroup(uuid);
-           }
+            }
         }
         else if (m_xml.name() == "Name") {
             group->setName(readString());
diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp
index a5c0225f8..0c89652d8 100644
--- a/tests/TestGroup.cpp
+++ b/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);
diff --git a/tests/TestGroup.h b/tests/TestGroup.h
index 4f0995088..7edd3577d 100644
--- a/tests/TestGroup.h
+++ b/tests/TestGroup.h
@@ -29,6 +29,7 @@ private Q_SLOTS:
     void testParenting();
     void testSignals();
     void testEntries();
+    void testDeleteSignals();
 };
 
 #endif // KEEPASSX_TESTGROUP_H
diff --git a/tests/TestGroupModel.cpp b/tests/TestGroupModel.cpp
index 96e2317fd..1b9a89843 100644
--- a/tests/TestGroupModel.cpp
+++ b/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;
diff --git a/tests/TestKeePass2Writer.cpp b/tests/TestKeePass2Writer.cpp
index ba097d454..6955ba5c4 100644
--- a/tests/TestKeePass2Writer.cpp
+++ b/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);