[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [libkpeople/dave] src: Disallow changing URI of PersonData after creation
From:       David Edmundson <kde () davidedmundson ! co ! uk>
Date:       2013-06-30 22:37:55
Message-ID: E1UtQFn-000446-Oa () scm ! kde ! org
[Download RAW message or body]

Git commit 56ecc360e55baa997ee011f8cab2b701cdab0ba1 by David Edmundson.
Committed on 29/06/2013 at 13:49.
Pushed by davidedmundson into branch 'dave'.

Disallow changing URI of PersonData after creation

Currently setUri and setContactId are public which would cause a lot of bugs if \
changed. Instead replace with two clear static methods; loadFromUri and \
loadFromContactId

A declarative wrapper using QDeclarativeParserStatus provides the same functionality
in QML.

M  +33   -44   src/autotests/tests/persondatatests.cpp
M  +4    -1    src/declarative/CMakeLists.txt
A  +56   -0    src/declarative/declarativepersondata.cpp     [License: GPL (v3+)]
A  +50   -0    src/declarative/declarativepersondata.h     [License: GPL (v3+)]
M  +2    -1    src/declarative/peopleqmlplugin.cpp
M  +2    -3    src/examples/personwidget.cpp
M  +1    -2    src/personactionsmodel.cpp
M  +19   -30   src/persondata.cpp
M  +14   -17   src/persondata.h
M  +1    -1    src/widgets/persondetailsview.cpp

http://commits.kde.org/libkpeople/56ecc360e55baa997ee011f8cab2b701cdab0ba1

diff --git a/src/autotests/tests/persondatatests.cpp \
b/src/autotests/tests/persondatatests.cpp index 623fb5f..6cf40be 100644
--- a/src/autotests/tests/persondatatests.cpp
+++ b/src/autotests/tests/persondatatests.cpp
@@ -134,69 +134,58 @@ void PersonDataTests::init()
 void PersonDataTests::contactProperties()
 {
     //create a simple contact with name + email
-    PersonData personData;
-    personData.setUri(m_contact1Uri.toString());
-    QCOMPARE(personData.isValid(), true);
-    QCOMPARE(personData.uri(), m_contact1Uri.toString());
-    QCOMPARE(personData.isPerson(), false);
-    QCOMPARE(personData.contactResources().size(), 1);
-
-    QCOMPARE(personData.name(), QLatin1String("Contact 1"));
-    QCOMPARE(personData.emails(), QStringList() << \
QLatin1String("contact1@example.com")); +    QScopedPointer<PersonData> \
personData(PersonData::loadFromUri(m_contact1Uri)); +    \
QCOMPARE(personData->isValid(), true); +    QCOMPARE(personData->uri(), \
m_contact1Uri); +    QCOMPARE(personData->isPerson(), false);
+    QCOMPARE(personData->contactResources().size(), 1);
+
+    QCOMPARE(personData->name(), QLatin1String("Contact 1"));
+    QCOMPARE(personData->emails(), QStringList() << \
QLatin1String("contact1@example.com"));  }
 
 void PersonDataTests::personProperties()
 {
-    PersonData personData;
-    personData.setUri(m_personAUri.toString());
-    QCOMPARE(personData.isValid(), true);
-    QCOMPARE(personData.isPerson(), true);
-    QCOMPARE(personData.contactResources().size(), 2);
-    QCOMPARE(personData.name(), QLatin1String("Person A"));
-    QCOMPARE(personData.emails(), QStringList() << \
QLatin1String("contact3@example.com") << QLatin1String("contact2@example.com")); +    \
QScopedPointer<PersonData> personData(PersonData::loadFromUri(m_personAUri)); +    \
QCOMPARE(personData->isValid(), true); +    QCOMPARE(personData->isPerson(), true);
+    QCOMPARE(personData->contactResources().size(), 2);
+    QCOMPARE(personData->name(), QLatin1String("Person A"));
+    QCOMPARE(personData->emails(), QStringList() << \
QLatin1String("contact3@example.com") << QLatin1String("contact2@example.com"));  }
 
 void PersonDataTests::personFromContactID()
 {
-    PersonData personData;
-    personData.setContactId(QLatin1String("contact2@example.com"));
+    QScopedPointer<PersonData> \
personData(PersonData::loadFromContactId(QLatin1String("contact2@example.com")));  \
                //This should load PersonA NOT Contact2
-    QCOMPARE(personData.uri(), m_personAUri.toString());
+    QCOMPARE(personData->uri(), m_personAUri);
 }
 
 void PersonDataTests::contactFromContactID()
 {
-    PersonData personData;
-    personData.setContactId(QLatin1String("contact1@example.com"));
-    QCOMPARE(personData.uri(), m_contact1Uri.toString());
+    QScopedPointer<PersonData> \
personData(PersonData::loadFromContactId(QLatin1String("contact1@example.com"))); +   \
QCOMPARE(personData->uri(), m_contact1Uri);  }
 
 void PersonDataTests::miscTests()
 {
-    PersonData personData;
-    personData.uri();
-    personData.name();
-    personData.avatar();
-    personData.birthday();
-    personData.contactResources();
-    personData.emails();
-    personData.groups();
-    personData.imAccounts();
-    personData.isPerson();
-    personData.phones();
-    QCOMPARE(personData.isValid(), false);
-
-    PersonData test2;
-    test2.setContactId(QLatin1String("IDThatDoesNotExist"));
-    QCOMPARE(test2.isValid(), false);
-
-    test2.name();
+    QScopedPointer<PersonData> \
personData(PersonData::loadFromContactId(QLatin1String("NOTEXIST"))); +    \
personData->uri(); +    personData->name();
+    personData->avatar();
+    personData->birthday();
+    personData->contactResources();
+    personData->emails();
+    personData->groups();
+    personData->imAccounts();
+    personData->isPerson();
+    personData->phones();
+    QCOMPARE(personData->isValid(), false);
 }
 
 void PersonDataTests::contactChanged()
 {
-    PersonData personData;
-    personData.setUri(m_contact1Uri.toString());
+    QScopedPointer<PersonData> personData(PersonData::loadFromUri(m_contact1Uri));
 
     Nepomuk2::SimpleResourceGraph graph;
 
@@ -213,7 +202,7 @@ void PersonDataTests::contactChanged()
     job->exec();
     QVERIFY(!job->error());
 
-    QVERIFY(QTest::kWaitForSignal(&personData, SIGNAL(dataChanged()), 5000));
+    QVERIFY(QTest::kWaitForSignal(personData.data(), SIGNAL(dataChanged()), 5000));
 
 
     QCoreApplication::processEvents();
@@ -223,7 +212,7 @@ void PersonDataTests::contactChanged()
 
     //for some reason this fails...
     //FIXME investigate
-    QCOMPARE(personData.emails().size(), 2);
+    QCOMPARE(personData->emails().size(), 2);
 }
 
 
diff --git a/src/declarative/CMakeLists.txt b/src/declarative/CMakeLists.txt
index 9b33b85..815615c 100644
--- a/src/declarative/CMakeLists.txt
+++ b/src/declarative/CMakeLists.txt
@@ -5,7 +5,10 @@ include_directories(
         ${CMAKE_BINARY_DIR}
 )
 
-kde4_add_library(kpeopledeclarative SHARED declarativepersonsmodel.cpp \
peopleqmlplugin.cpp) +kde4_add_library(kpeopledeclarative SHARED
+                    declarativepersonsmodel.cpp
+                    declarativepersondata.cpp
+                    peopleqmlplugin.cpp)
 
 target_link_libraries(kpeopledeclarative ${QT_QTCORE_LIBRARY} ${QT_QTGUI_LIBRARY} \
${QT_QTDECLARATIVE_LIBRARY} ${KDE4_KDECORE_LIBRARY} kpeople)  
diff --git a/src/declarative/declarativepersondata.cpp \
b/src/declarative/declarativepersondata.cpp new file mode 100644
index 0000000..718e8b4
--- /dev/null
+++ b/src/declarative/declarativepersondata.cpp
@@ -0,0 +1,56 @@
+/*
+    <one line to give the program's name and a brief idea of what it does.>
+    Copyright (C) 2013  David Edmundson <D.Edmundson@lboro.ac.uk>
+
+    This program is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    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 General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+
+#include "declarativepersondata.h"
+
+#include <KDebug>
+
+DeclarativePersonData::DeclarativePersonData(QObject* parent)
+    :KPeople::PersonData(parent)
+{
+
+}
+
+void DeclarativePersonData::classBegin()
+{
+
+}
+
+void DeclarativePersonData::componentComplete()
+{
+    if (!m_uri.isEmpty()) {
+        loadUri(m_uri);
+    } else if (!m_contactId.isEmpty()) {
+        loadContact(m_contactId);
+    } else {
+        kWarning() << "item has no uri or contactId set";
+    }
+}
+
+void DeclarativePersonData::setContactId(const QString& contactId)
+{
+    m_contactId = contactId;
+}
+
+void DeclarativePersonData::setUri(const QString& uri)
+{
+    m_uri = uri;
+}
+
+
diff --git a/src/declarative/declarativepersondata.h \
b/src/declarative/declarativepersondata.h new file mode 100644
index 0000000..72c6fec
--- /dev/null
+++ b/src/declarative/declarativepersondata.h
@@ -0,0 +1,50 @@
+/*
+    <one line to give the program's name and a brief idea of what it does.>
+    Copyright (C) 2013  David Edmundson <D.Edmundson@lboro.ac.uk>
+
+    This program is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    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 General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+
+#ifndef DECLARATIVEPERSONDATA_H
+#define DECLARATIVEPERSONDATA_H
+
+#include "persondata.h"
+
+#include <QDeclarativeParserStatus>
+
+class DeclarativePersonData : public KPeople::PersonData, public \
QDeclarativeParserStatus +{
+    Q_OBJECT
+    Q_INTERFACES(QDeclarativeParserStatus)
+
+    Q_PROPERTY(QString uri WRITE setUri);
+    Q_PROPERTY(QString contactId WRITE setContactId);
+
+public:
+    DeclarativePersonData(QObject *parent=0);
+
+    void setUri(const QString &uri);
+    void setContactId(const QString &uri);
+
+
+    void classBegin();
+    void componentComplete();
+
+private:
+    QString m_uri;
+    QString m_contactId;
+};
+
+#endif // DECLARATIVEPERSONDATA_H
diff --git a/src/declarative/peopleqmlplugin.cpp \
b/src/declarative/peopleqmlplugin.cpp index c991064..4bd4b45 100644
--- a/src/declarative/peopleqmlplugin.cpp
+++ b/src/declarative/peopleqmlplugin.cpp
@@ -24,6 +24,7 @@
 #include <persondata.h>
 
 #include "declarativepersonsmodel.h"
+#include "declarativepersondata.h"
 
 #include <QtDeclarative/QDeclarativeItem>
 
@@ -31,7 +32,7 @@ void PeopleQMLPlugin::registerTypes(const char* uri)
 {
     qmlRegisterType<DeclarativePersonsModel>(uri, 0, 1, "PersonsModel");
     qmlRegisterType<KPeople::PersonActionsModel>(uri, 0, 1, "PersonActions");
-    qmlRegisterType<KPeople::PersonData>(uri, 0, 1, "PersonData");
+    qmlRegisterType<DeclarativePersonData>(uri, 0, 1, "PersonData");
     qmlRegisterType<QAbstractItemModel>();
 
     qRegisterMetaType<QModelIndex>("QModelIndex");
diff --git a/src/examples/personwidget.cpp b/src/examples/personwidget.cpp
index bc2eaf2..595ccf2 100644
--- a/src/examples/personwidget.cpp
+++ b/src/examples/personwidget.cpp
@@ -35,11 +35,10 @@ int main(int argc, char** argv)
         return 1;
     }
 
-    KPeople::PersonData person;
-    person.setContactId(app.arguments()[1]);
+    QScopedPointer<KPeople::PersonData> \
person(KPeople::PersonData::loadFromContactId(app.arguments()[1]));  
     KPeople::PersonDetailsView w;
-    w.setPerson(&person);
+    w.setPerson(person.data());
 
     w.show();
 
diff --git a/src/personactionsmodel.cpp b/src/personactionsmodel.cpp
index 4580062..5dcd25c 100644
--- a/src/personactionsmodel.cpp
+++ b/src/personactionsmodel.cpp
@@ -69,8 +69,7 @@ void PersonActionsModel::setPerson(const QPersistentModelIndex& \
index)  if (d->person) {
         d->person->deleteLater();
     }
-    d->person = new PersonData(this);
-    d->person->setUri(index.data(PersonsModel::UriRole).toString());
+    d->person = PersonData::loadFromUri(index.data(PersonsModel::UriRole).toString(), \
this);  
     d->actions.append(PersonPluginManager::actionsForPerson(d->person, this));
 
diff --git a/src/persondata.cpp b/src/persondata.cpp
index 2341bd4..e564efa 100644
--- a/src/persondata.cpp
+++ b/src/persondata.cpp
@@ -39,6 +39,7 @@
 #include <Soprano/QueryResultIterator>
 
 #include <KDebug>
+
 #include <QPointer>
 
 using namespace Nepomuk2::Vocabulary;
@@ -55,8 +56,7 @@ public:
         q->connect(dataSourceWatcher, SIGNAL(contactChanged(QUrl)), q, \
SIGNAL(dataChanged()));  }
 
-    QString uri;
-    QString id;
+    QUrl uri;
     QPointer<Nepomuk2::ResourceWatcher> watcher;
     DataSourceWatcher *dataSourceWatcher;
     Nepomuk2::Resource personResource;
@@ -64,17 +64,24 @@ public:
 };
 }
 
-PersonData::PersonData(QObject *parent)
-    : QObject(parent),
-    d_ptr(new PersonDataPrivate(this))
+PersonData* PersonData::loadFromContactId(const QString& contactId, QObject* parent)
 {
+    PersonData *person = new PersonData(parent);
+    person->loadContact(contactId);
+    return person;
 }
 
-PersonData::PersonData(const QString &uri, QObject *parent)
+PersonData* PersonData::loadFromUri(const QUrl& uri, QObject* parent)
+{
+    PersonData *person = new PersonData(parent);
+    person->loadUri(uri);
+    return person;
+}
+
+PersonData::PersonData(QObject *parent)
     : QObject(parent),
-      d_ptr(new PersonDataPrivate(this))
+    d_ptr(new PersonDataPrivate(this))
 {
-    setUri(uri);
 }
 
 PersonData::~PersonData()
@@ -82,14 +89,9 @@ PersonData::~PersonData()
     delete d_ptr;
 }
 
-void PersonData::setContactId(const QString &id)
+void PersonData::loadContact(const QString &id)
 {
     Q_D(PersonData);
-    if (d->id == id) {
-        return;
-    }
-
-    d->id = id;
 
     QString query = QString::fromUtf8(
         "select DISTINCT ?uri "
@@ -108,37 +110,25 @@ void PersonData::setContactId(const QString &id)
     }
 
     if (d->uri != uri) {
-        setUri(uri);
+        loadUri(uri);
     }
 }
 
-QString PersonData::contactId() const
-{
-    Q_D(const PersonData);
-    return d->id;
-}
-
-QString PersonData::uri() const
+QUrl PersonData::uri() const
 {
     Q_D(const PersonData);
     return d->uri;
 }
 
-void PersonData::setUri(const QString &uri)
+void PersonData::loadUri(const QUrl &uri)
 {
     Q_D(PersonData);
 
     d->uri = uri;
-    d->contactResources.clear();
     d->personResource = Nepomuk2::Resource();
-    d->dataSourceWatcher->clearWatchedContacts();
 
     Nepomuk2::Resource r(uri);
 
-    if (!d->watcher.isNull()) {
-        delete d->watcher;
-    }
-
     d->watcher = new Nepomuk2::ResourceWatcher(this);
 
     if (r.type() == PIMO::Person()) {
@@ -166,7 +156,6 @@ void PersonData::setUri(const QString &uri)
         }
     }
 
-    emit uriChanged();
     emit dataChanged();
 }
 
diff --git a/src/persondata.h b/src/persondata.h
index 177f245..86ea74d 100644
--- a/src/persondata.h
+++ b/src/persondata.h
@@ -37,7 +37,6 @@ struct PersonDataPrivate;
 class KPEOPLE_EXPORT PersonData : public QObject
 {
     Q_OBJECT
-    Q_PROPERTY(QString uri READ uri WRITE setUri NOTIFY uriChanged)
     Q_PROPERTY(QUrl avatar READ avatar NOTIFY dataChanged)
     Q_PROPERTY(QString name READ name NOTIFY dataChanged)
     Q_PROPERTY(QString status READ status NOTIFY dataChanged)
@@ -45,26 +44,19 @@ class KPEOPLE_EXPORT PersonData : public QObject
     Q_PROPERTY(QStringList imAccounts READ imAccounts NOTIFY dataChanged)
     Q_PROPERTY(QStringList phones READ phones NOTIFY dataChanged)
     Q_PROPERTY(bool isPerson READ isPerson)
+    Q_PROPERTY(bool isValid READ isValid)
 
     public:
-        PersonData(QObject *parent = 0);
-
-        PersonData(const QString &uri, QObject *parent = 0);
+        static PersonData* loadFromUri(const QUrl &url, QObject *parent=0);
+        static PersonData* loadFromContactId(const QString &contactId, QObject \
*parent=0);  
         virtual ~PersonData();
 
-        /** @returns the uri of the current person */
-        QString uri() const;
-
-        /** sets new contact uri, all data are refetched */
-        void setUri(const QString &uri);
-
         /** Returns if the URI represents a valid person or contact*/
         bool isValid() const;
 
-        /** @p id will specify the person we're offering by finding the pimo:Person \
                related to it */
-        void setContactId(const QString &id);
-        QString contactId() const;
+        /** @returns the uri of the current person */
+        QUrl uri() const;
 
         /** @returns a url pointing to the avatar image */
         QUrl avatar() const;
@@ -95,18 +87,23 @@ class KPEOPLE_EXPORT PersonData : public QObject
         QList<Nepomuk2::Resource> contactResources() const;
 
     Q_SIGNALS:
-        /** the person has changed */
-        void uriChanged();
-
         /** Some of the person's data we're offering has changed */
         void dataChanged();
 
+    protected:
+        PersonData(QObject *parent);
+
+        /** sets new contact uri, all data are refetched */
+        void loadUri(const QUrl &uri);
+
+        /** @p id will specify the person we're offering by finding the pimo:Person \
related to it */ +        void loadContact(const QString &id);
+
     private:
         Q_DECLARE_PRIVATE(PersonData)
         PersonDataPrivate * d_ptr;
 
         QString findMostOnlinePresence(const QStringList &presences) const;
-
 };
 }
 
diff --git a/src/widgets/persondetailsview.cpp b/src/widgets/persondetailsview.cpp
index f655eb2..6a1e8c3 100644
--- a/src/widgets/persondetailsview.cpp
+++ b/src/widgets/persondetailsview.cpp
@@ -120,7 +120,7 @@ PersonDetailsView::PersonDetailsView(QWidget *parent)
 
     m_mainLayout->addSpacerItem(new QSpacerItem(32, 32, QSizePolicy::Minimum, \
QSizePolicy::Expanding));  
-    m_person = new PersonData(this);
+    m_person = 0;
 
     setLayout(m_mainLayout);
 }


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic