Merge lp:~renatofilho/address-book-service/fix-1387763-rtm into lp:address-book-service/rtm-14.09

[Renato Araujo Oliveira Filho]

Avoid create sources with duplicated Id;
Remove address book sources based on source name instead of source id.

[Bill Filler (bfiller)]

approved

review: Approve

=== modified file 'lib/addressbook.cpp'
--- lib/addressbook.cpp	2014-09-30 16:42:48 +0000
+++ lib/addressbook.cpp	2014-12-02 19:11:11 +0000
@@ -69,10 +69,12 @@
 class CreateSourceData
 {
 public:
+    QString m_sourceId;
     QString m_sourceName;
     bool m_setAsPrimary;
     galera::AddressBook *m_addressbook;
     QDBusMessage m_message;
+    ESource *m_source;
 };
 
 class RemoveSourceData
@@ -82,6 +84,32 @@
     QDBusMessage m_message;
 };
 
+ESource* create_esource_from_data(const CreateSourceData &data, ESourceRegistry **registry)
+{
+    GError *error = NULL;
+    ESource *source = e_source_new_with_uid(data.m_sourceId.toUtf8().data(), NULL, &error);
+    if (error) {
+        qWarning() << "Fail to create source" << error->message;
+        g_error_free(error);
+        return 0;
+    }
+
+    e_source_set_parent(source, "local-stub");
+    e_source_set_display_name(source, data.m_sourceName.toUtf8().data());
+    ESourceAddressBook *ext = E_SOURCE_ADDRESS_BOOK(e_source_get_extension(source, E_SOURCE_EXTENSION_ADDRESS_BOOK));
+    e_source_backend_set_backend_name(E_SOURCE_BACKEND(ext), "local");
+
+    *registry = e_source_registry_new_sync(NULL, &error);
+    if (error) {
+        qWarning() << "Fail to change default contact address book" << error->message;
+        g_error_free(error);
+        g_object_unref(source);
+        return 0;
+    }
+
+    return source;
+}
+
 }
 
 namespace galera
@@ -335,12 +363,12 @@
     return Source();
 }
 
-Source AddressBook::createSource(const QString &sourceId, bool setAsPrimary, const QDBusMessage &message)
+Source AddressBook::createSource(const QString &sourceName, bool setAsPrimary, const QDBusMessage &message)
 {
     CreateSourceData *data = new CreateSourceData;
     data->m_addressbook = this;
     data->m_message = message;
-    data->m_sourceName = sourceId;
+    data->m_sourceName = sourceName;
     data->m_setAsPrimary = setAsPrimary;
 
     FolksPersonaStore *store = folks_individual_aggregator_get_primary_store(m_individualAggregator);
@@ -359,22 +387,37 @@
                                                        NULL, NULL, NULL, NULL, NULL, NULL);
 
         gee_collection_add_all(GEE_COLLECTION(storesIds), GEE_COLLECTION(storesKeys));
-        gee_collection_add(GEE_COLLECTION(storesIds), sourceId.toUtf8().constData());
+        gee_collection_add(GEE_COLLECTION(storesIds), sourceName.toUtf8().constData());
         folks_backend_set_persona_stores(dummy, storesIds);
 
         g_object_unref(storesIds);
         g_object_unref(backendStore);
         g_object_unref(dummy);
 
-        Source src(sourceId, sourceId, false, false);
+        Source src(sourceName, sourceName, false, false);
         QDBusMessage reply = message.createReply(QVariant::fromValue<Source>(src));
         QDBusConnection::sessionBus().send(reply);
     } else if (personaStoreTypeId == "eds") {
-        edsf_persona_store_create_address_book(sourceId.toUtf8().data(),
-                                               (GAsyncReadyCallback) AddressBook::createSourceDone,
-                                               data);
+        data->m_sourceId = QUuid::createUuid().toString().remove("{").remove("}");
+        ESourceRegistry *registry = NULL;
+        ESource *source = create_esource_from_data(*data, &registry);
+        if (source) {
+            data->m_source = source;
+            e_source_registry_commit_source(registry,
+                                            source,
+                                            NULL,
+                                            (GAsyncReadyCallback) AddressBook::createSourceDone,
+                                            data);
+        } else {
+            delete data;
+            QDBusMessage reply = message.createReply(QVariant::fromValue<Source>(Source()));
+            QDBusConnection::sessionBus().send(reply);
+        }
     } else {
         qWarning() << "Not supported, create sources on persona store with type id:" << personaStoreTypeId;
+        delete data;
+        QDBusMessage reply = message.createReply(QVariant::fromValue<Source>(Source()));
+        QDBusConnection::sessionBus().send(reply);
     }
     return Source();
 }
@@ -386,19 +429,30 @@
     bool error = false;
     if (backend) {
         GeeMap *storesMap = folks_backend_get_persona_stores(backend);
-        if (gee_map_has_key(storesMap, sourceId.toUtf8().constData())) {
-            EdsfPersonaStore *ps = EDSF_PERSONA_STORE(gee_map_get(storesMap, sourceId.toUtf8().constData()));
-
-            RemoveSourceData *rData = new RemoveSourceData;
-            rData->m_addressbook = this;
-            rData->m_message = message;
-            edsf_persona_store_remove_address_book(ps, AddressBook::removeSourceDone, rData);
+        GeeCollection *stores = gee_map_get_values(storesMap);
+        GeeIterator *i = gee_iterable_iterator(GEE_ITERABLE(stores));
+        RemoveSourceData *rData = 0;
+        while (gee_iterator_next(i)) {
+            FolksPersonaStore *ps = FOLKS_PERSONA_STORE(gee_iterator_get(i));
+            // We need to compare using source name due the missing API to handle sources diff from contacts
+            if (g_strcmp0(folks_persona_store_get_display_name(ps), sourceId.toUtf8().constData()) == 0) {
+                rData = new RemoveSourceData;
+                rData->m_addressbook = this;
+                rData->m_message = message;
+                edsf_persona_store_remove_address_book(EDSF_PERSONA_STORE(ps), AddressBook::removeSourceDone, rData);
+                g_object_unref(ps);
+                break;
+            }
             g_object_unref(ps);
-        } else {
+        }
+
+        g_object_unref(backend);
+        g_object_unref(stores);
+
+        if (!rData) {
             qWarning() << "Source not found to remove:" << sourceId;
             error = true;
         }
-        g_object_unref(backend);
     } else {
         qWarning() << "Fail to create eds backend during the source removal:" << sourceId;
         error = true;
@@ -486,36 +540,18 @@
     CreateSourceData *cData = static_cast<CreateSourceData*>(data);
     GError *error = 0;
     Source src;
-    edsf_persona_store_create_address_book_finish(res, &error);
+    e_source_registry_commit_source_finish(E_SOURCE_REGISTRY(source), res, &error);
     if (error) {
         qWarning() << "Fail to create source" << error->message;
         g_error_free(error);
     } else {
-        src = Source(cData->m_sourceName, cData->m_sourceName, false, cData->m_setAsPrimary);
+        // set as primary if necessary
         if (cData->m_setAsPrimary) {
-            ESourceRegistry *r = e_source_registry_new_sync(NULL, &error);
-            if (error) {
-                qWarning() << "Fail to change default contact address book" << error->message;
-                g_error_free(error);
-            } else {
-                ESource *edsSource = 0;
-                GList *sources = e_source_registry_list_sources(r, E_SOURCE_EXTENSION_ADDRESS_BOOK);
-                for(GList *i = sources; i != -0; i = sources->next) {
-                    edsSource = E_SOURCE(i->data);
-                    if (strcmp(cData->m_sourceName.toUtf8().constData(), e_source_get_uid(edsSource)) == 0) {
-                        break;
-                    }
-                }
-                if (edsSource) {
-                    e_source_registry_set_default_address_book(r, edsSource);
-                } else {
-                    qWarning() << "Fail to find source:" << cData->m_sourceName;
-                }
-                g_list_free_full(sources, g_object_unref);
-            }
-            g_object_unref(r);
+            e_source_registry_set_default_address_book(E_SOURCE_REGISTRY(source), cData->m_source);
         }
+        src = Source(cData->m_sourceId, cData->m_sourceName, false, cData->m_setAsPrimary);
     }
+    g_object_unref(source);
     QDBusMessage reply = cData->m_message.createReply(QVariant::fromValue<Source>(src));
     QDBusConnection::sessionBus().send(reply);
     delete cData;
@@ -630,7 +666,6 @@
             }
 
             result << Source(id, displayName, !canWrite, isPrimary);
-
             g_object_unref(store);
         }
 

=== modified file 'lib/addressbook.h'
--- lib/addressbook.h	2014-09-30 16:23:53 +0000
+++ lib/addressbook.h	2014-12-02 19:11:11 +0000
@@ -70,7 +70,7 @@
     void shutdown();
     SourceList availableSources(const QDBusMessage &message);
     Source source(const QDBusMessage &message);
-    Source createSource(const QString &sourceId, bool setAsPrimary, const QDBusMessage &message);
+    Source createSource(const QString &sourceName, bool setAsPrimary, const QDBusMessage &message);
     void removeSource(const QString &sourceId, const QDBusMessage &message);
     QString createContact(const QString &contact, const QString &source, const QDBusMessage &message = QDBusMessage());
     int removeContacts(const QStringList &contactIds, const QDBusMessage &message);

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2013-05-21 20:11:19 +0000
+++ tests/CMakeLists.txt	2014-12-02 19:11:11 +0000
@@ -1,2 +1,5 @@
+find_program(EVOLUTION_ADDRESSBOOK_FACTORY_BIN evolution-addressbook-factory
+             PATHS /usr/lib/evolution/)
+set(EVOLUTION_ADDRESSBOOK_SERVICE_NAME "org.gnome.evolution.dataserver.AddressBook6")
 add_subdirectory(data)
 add_subdirectory(unittest)

=== modified file 'tests/unittest/CMakeLists.txt'
--- tests/unittest/CMakeLists.txt	2014-05-27 14:41:55 +0000
+++ tests/unittest/CMakeLists.txt	2014-12-02 19:11:11 +0000
@@ -36,6 +36,57 @@
                           TIMEOUT ${CTEST_TESTING_TIMEOUT})
 endmacro()
 
+macro(declare_eds_test TESTNAME)
+    add_executable(${TESTNAME}
+                   ${ARGN}
+                   ${TESTNAME}.cpp
+    )
+    qt5_use_modules(${TESTNAME} Core Contacts Versit Test DBus)
+
+    if(TEST_XML_OUTPUT)
+        set(TEST_ARGS -p -xunitxml -p -o -p test_${testname}.xml)
+    else()
+        set(TEST_ARGS "")
+    endif()
+
+    target_link_libraries(${TESTNAME}
+                          address-book-service-lib
+                          ${CONTACTS_SERVICE_LIB}
+                          ${GLIB_LIBRARIES}
+                          ${GIO_LIBRARIES}
+                          ${FOLKS_LIBRARIES}
+    )
+
+    add_test(${TESTNAME}
+             ${DBUS_RUNNER}
+             --keep-env
+             --task ${EVOLUTION_ADDRESSBOOK_FACTORY_BIN} --task-name "evolution-addressbook" -r
+             --task ${address-book-service_BINARY_DIR}/address-book-service --wait-for=${EVOLUTION_ADDRESSBOOK_SERVICE_NAME} -r
+             --task ${CMAKE_CURRENT_BINARY_DIR}/${TESTNAME} ${TEST_ARGS} --wait-for=com.canonical.pim --wait-until-complete)
+
+    execute_process(COMMAND mktemp -d /tmp/${TESTNAME}_XXXX
+                    OUTPUT_VARIABLE TMP_HOME_DIR)
+    list(APPEND TEST_ENVIRONMENT_LIST "QT_QPA_PLATFORM=minimal")
+    list(APPEND TEST_ENVIRONMENT_LIST "FOLKS_BACKENDS_ALLOWED=eds")
+    list(APPEND TEST_ENVIRONMENT_LIST "HOME=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_RUNTIME_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_CACHE_HOME=${TMP_HOME_DIR}/.cache")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_CONFIG_HOME=${TMP_HOME_DIR}/.config")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_CONFIG_HOME=${TMP_HOME_DIR}/.local/share")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_DOCUMENTS_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_DOWNLOAD_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_MUSIC_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_PICTURES_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_PUBLICSHARE_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_TEMPLATES_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "XDG_VIDEOS_DIR=${TMP_HOME_DIR}")
+    list(APPEND TEST_ENVIRONMENT_LIST "QORGANIZER_EDS_DEBUG=On")
+
+    set_tests_properties(${TESTNAME} PROPERTIES
+                          ENVIRONMENT ${TEST_ENVIRONMENT_LIST}
+                          TIMEOUT ${CTEST_TESTING_TIMEOUT})
+endmacro()
+
 include_directories(
     ${CMAKE_SOURCE_DIR}
     ${CMAKE_BINARY_DIR}
@@ -77,6 +128,8 @@
     declare_test(qcontacts-test True ${BASE_CLIENT_TEST_SRC})
     declare_test(qcontacts-create-source-test True ${BASE_CLIENT_TEST_SRC})
     declare_test(qcontacts-async-request-test True ${BASE_CLIENT_TEST_SRC})
+
+    declare_eds_test(contact-collection-test)
 elseif()
     message(STATUS "DBus test runner not found. Some tests will be disabled")
 endif()
@@ -101,3 +154,4 @@
                       ${FOLKS_LIBRARIES}
 )
 
+

=== added file 'tests/unittest/contact-collection-test.cpp'
--- tests/unittest/contact-collection-test.cpp	1970-01-01 00:00:00 +0000
+++ tests/unittest/contact-collection-test.cpp	2014-12-02 19:11:11 +0000
@@ -0,0 +1,142 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This file is part of contact-service-app.
+ *
+ * contact-service-app 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; version 3.
+ *
+ * contact-service-app 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 <QObject>
+#include <QtTest>
+#include <QDebug>
+#include <QtContacts>
+
+#include "config.h"
+
+using namespace QtContacts;
+
+class ContactCollectionTest : public QObject
+{
+    Q_OBJECT
+private:
+    QContactManager *m_manager;
+
+private Q_SLOTS:
+    void initTestCase()
+    {
+        QCoreApplication::setLibraryPaths(QStringList() << QT_PLUGINS_BINARY_DIR);
+        // wait for address-book-service
+        QTest::qWait(1000);
+    }
+
+    void init()
+    {
+        m_manager = new QContactManager("galera");
+    }
+
+    void cleanup()
+    {
+        delete m_manager;
+    }
+
+    /*
+     * Test create a new collection
+     */
+    void testCreateAddressBook()
+    {
+        QContact c;
+        c.setType(QContactType::TypeGroup);
+        QContactDisplayLabel label;
+        QString uniqueName = QString("source@%1").arg(QUuid::createUuid().toString().remove("{").remove("}"));
+        label.setLabel(uniqueName);
+        c.saveDetail(&label);
+
+        bool saveResult = m_manager->saveContact(&c);
+        QVERIFY(saveResult);
+
+        QContactDetailFilter filter;
+        filter.setDetailType(QContactDetail::TypeType, QContactType::FieldType);
+        filter.setValue(QContactType::TypeGroup);
+
+        QList<QContact> contacts = m_manager->contacts(filter);
+        Q_FOREACH(const QContact &contact, contacts) {
+            if ((contact.detail<QContactDisplayLabel>().label() == uniqueName) &&
+                (contact.id() == c.id())) {
+                return;
+            }
+        }
+        QFAIL("New collection not found");
+    }
+
+    /*
+     * Test remove a collection
+     */
+    void testRemoveAddressBook()
+    {
+        // create a source
+        QContact c;
+        c.setType(QContactType::TypeGroup);
+        QContactDisplayLabel label;
+        QString uniqueName = QString("source@%1").arg(QUuid::createUuid().toString().remove("{").remove("}"));
+        label.setLabel(uniqueName);
+        c.saveDetail(&label);
+
+        bool saveResult = m_manager->saveContact(&c);
+        QVERIFY(saveResult);
+
+        // try to remove new source
+        // WORKAROUND: Since qcontacts does not cotains a API to remove address book we use the contact label as id
+        // for addressbook. This Id must contain a "@" to be handled as address book name.
+        QContactId addressBookId = QContactId::fromString(QString("qtcontacts:galera::%1").arg(uniqueName));
+        bool removeResult = m_manager->removeContact(addressBookId);
+        QVERIFY(removeResult);
+
+        // check if the source was removed
+        QContactDetailFilter filter;
+        filter.setDetailType(QContactDetail::TypeType, QContactType::FieldType);
+        filter.setValue(QContactType::TypeGroup);
+        QList<QContact> contacts = m_manager->contacts(filter);
+        Q_FOREACH(const QContact &contact, contacts) {
+            if (contact.id() == c.id()) {
+                QFAIL("Collection not removed");
+            }
+        }
+    }
+
+    /*
+     * Test query for collections using the contact group type
+     */
+    void testQueryAddressBook()
+    {
+        // filter all contact groups/addressbook
+        QContactDetailFilter filter;
+        filter.setDetailType(QContactDetail::TypeType, QContactType::FieldType);
+        filter.setValue(QContactType::TypeGroup);
+
+        // check result for the default source
+        QList<QContact> contacts = m_manager->contacts(filter);
+        Q_FOREACH(const QContact &c, contacts) {
+            QCOMPARE(c.type(), QContactType::TypeGroup);
+            if (c.id().toString() == QStringLiteral("qtcontacts:galera::system-address-book")) {
+                QContactDisplayLabel label = c.detail(QContactDisplayLabel::Type);
+                QCOMPARE(label.label(), QStringLiteral("Personal"));
+                return;
+            }
+        }
+        QFAIL("Fail to query for collections");
+    }
+};
+
+QTEST_MAIN(ContactCollectionTest)
+
+#include "contact-collection-test.moc"