address-book-service

Merge lp:~renatofilho/address-book-service/enable-debug into lp:address-book-service

enable-debug
Merge into trunk

Proposed by Renato Araujo Oliveira Filho on 2014-09-02

Status:

Merged

Approved by:

Bill Filler on 2014-09-12

Approved revision:

146

Merged at revision:

138

Proposed branch:

lp:~renatofilho/address-book-service/enable-debug

Merge into:

lp:address-book-service

Diff against target:

344 lines (+139/-10)

9 files modified

contacts/contacts-service.cpp (+1/-0)
lib/addressbook-adaptor.h (+2/-0)
lib/addressbook.cpp (+99/-4)
lib/addressbook.h (+11/-2)
lib/contacts-map.cpp (+5/-0)
lib/contacts-map.h (+2/-0)
lib/dirtycontact-notify.cpp (+6/-0)
lib/dirtycontact-notify.h (+1/-0)
upstart/address-book-service.conf (+12/-4)

To merge this branch:

bzr merge lp:~renatofilho/address-book-service/enable-debug

Critical

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Bill Filler (community)		2014-09-02	Approve on 2014-09-12
PS Jenkins bot	continuous-integration		Approve on 2014-09-11
Review via email: mp+233084@code.launchpad.net

Commit message

Enabled folks and service debug by default.
Reload contacts if EDS die.
WORKAROUND: Check for EDS status after start the service and if necessary reload folks.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/280/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-03:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/281/rebuild

review: Approve (continuous-integration)

lp:~renatofilho/address-book-service/enable-debug updated on 2014-09-04

138. By Renato Araujo Oliveira Filho on 2014-09-04: Reload contacts if EDS die.
WORKAROUND: Check for EDS status after start the service and if necessary reload folks.
139. By Renato Araujo Oliveira Filho on 2014-09-04: Merge parent branch.
140. By Renato Araujo Oliveira Filho on 2014-09-04: Does not wait 10s to check for eds, check for it just after the service be ready.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-04:

FAILED: Continuous integration, rev:140
http://jenkins.qa.ubuntu.com/job/address-book-service-ci/282/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-armhf-ci/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-i386-ci/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4482/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3332/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5734/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3629/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/282/rebuild

review: Needs Fixing (continuous-integration)

lp:~renatofilho/address-book-service/enable-debug updated on 2014-09-05

141. By Renato Araujo Oliveira Filho on 2014-09-05: Disable EDS tracker if FOLKS_BACKENDS_ALLOWED is set without eds.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-05:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/283/rebuild

review: Needs Fixing (continuous-integration)

lp:~renatofilho/address-book-service/enable-debug updated on 2014-09-05

142. By Renato Araujo Oliveira Filho on 2014-09-05: Check if EDS was running already when the service started.
Created a new "reloaded" signal to notify when the service reload all contacts.
143. By Renato Araujo Oliveira Filho on 2014-09-05: Avoid infinite loop while looking for EDS.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-05:

FAILED: Continuous integration, rev:143
http://jenkins.qa.ubuntu.com/job/address-book-service-ci/284/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-armhf-ci/56/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/address-book-service-utopic-i386-ci/56/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4537/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3369/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5789/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3671/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/284/rebuild

review: Needs Fixing (continuous-integration)

lp:~renatofilho/address-book-service/enable-debug updated on 2014-09-08

144. By Renato Araujo Oliveira Filho on 2014-09-08

Avoid crash during the addressbook service destructor.

Does not use the contacts map if it was already destroyed by previous call of function shutdown.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-08:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/285/rebuild

review: Approve (continuous-integration)

Revision history for this message

Bill Filler (bfiller) wrote on 2014-09-09:

It basically works but we should fix the restart logic a bit. Currently it tries 10 times in a row to restart EDS, without any delay. This happens very quickly, probably too quickly.

Instead, you should sleep between each retry with a timer or something. Like try immediately after first failure, then sleep 2 seconds and retry, then sleep 3 seconds and retry, then sleep 4 seconds and retry, etc.. until you've tried 10 times.

review: Needs Fixing

lp:~renatofilho/address-book-service/enable-debug updated on 2014-09-10

145. By Renato Araujo Oliveira Filho on 2014-09-10: Wait some ms before reload folks if EDS did not start.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/287/rebuild

review: Needs Fixing (continuous-integration)

lp:~renatofilho/address-book-service/enable-debug updated on 2014-09-11

146. By Renato Araujo Oliveira Filho on 2014-09-11: Removed folks debug.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/address-book-service-ci/288/rebuild

review: Approve (continuous-integration)

Revision history for this message

Bill Filler (bfiller) wrote on 2014-09-12:

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?
yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/<package-name>) on device or emulator?
yes

Did CI run pass? If not, please explain why.
yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Renato Araujo Oliveira Filho

Ubuntu Phablet Team

 === modified file 'contacts/contacts-service.cpp'
 --- contacts/contacts-service.cpp	2014-06-26 22:10:02 +0000
 +++ contacts/contacts-service.cpp	2014-09-11 18:57:11 +0000
@@ -185,6 +185,7 @@
          if (!m_iface->lastError().isValid()) {
              m_serviceIsReady = m_iface.data()->property("isReady").toBool();
              connect(m_iface.data(), SIGNAL(ready()), this, SLOT(onServiceReady()));
++            connect(m_iface.data(), SIGNAL(reloaded()), this, SIGNAL(serviceChanged()));
              connect(m_iface.data(), SIGNAL(contactsAdded(QStringList)), this, SLOT(onContactsAdded(QStringList)));
              connect(m_iface.data(), SIGNAL(contactsRemoved(QStringList)), this, SLOT(onContactsRemoved(QStringList)));
              connect(m_iface.data(), SIGNAL(contactsUpdated(QStringList)), this, SLOT(onContactsUpdated(QStringList)));
 === modified file 'lib/addressbook-adaptor.h'
 --- lib/addressbook-adaptor.h	2014-04-09 19:45:40 +0000
 +++ lib/addressbook-adaptor.h	2014-09-11 18:57:11 +0000
@@ -51,6 +51,7 @@
  "      <arg direction=\"out\" type=\"a(ss)\" name=\"errorMap\"/>\n"
  "    </signal>\n"
  "    <signal name=\"ready\"/>\n"
++"    <signal name=\"reloaded\"/>\n"
  "    <method name=\"ping\">\n"
  "      <arg direction=\"out\" type=\"b\"/>\n"
  "    </method>\n"
@@ -131,6 +132,7 @@
      void contactsUpdated(const QStringList &ids);
      void asyncOperationResult(QMap<QString, QString> errors);
      void ready();
++    void reloaded();
  private:
      AddressBook *m_addressBook;
 === modified file 'lib/addressbook.cpp'
 --- lib/addressbook.cpp	2014-08-11 21:20:38 +0000
 +++ lib/addressbook.cpp	2014-09-11 18:57:11 +0000
@@ -91,9 +91,10 @@
  AddressBook::AddressBook(QObject *parent)
      : QObject(parent),
        m_individualAggregator(0),
--      m_contacts(new ContactsMap),
++      m_contacts(0),
        m_adaptor(0),
        m_notifyContactUpdate(0),
++      m_edsIsLive(false),
        m_ready(false),
        m_individualsChangedDetailedId(0),
        m_notifyIsQuiescentHandlerId(0),
@@ -106,6 +107,7 @@
          m_serviceName = CPIM_SERVICE_NAME;
+     }
      prepareUnixSignals();
++    connectWithEDS();
+ }
  AddressBook::~AddressBook()
@@ -159,6 +161,7 @@
          prepareFolks();
          return true;
+     }
++
      return false;
+ }
@@ -167,8 +170,18 @@
      return start(QDBusConnection::sessionBus());
+ }
--void AddressBook::shutdown()
++void AddressBook::unprepareFolks()
+ {
++    // remove all contacts
++    // flusing any pending notification
++    m_notifyContactUpdate->flush();
++
++    // notify about contacts removal
++    if (m_contacts) {
++        m_notifyContactUpdate->insertRemovedContacts(m_contacts->keys().toSet());
++        m_notifyContactUpdate->flush();
++    }
++
      m_ready = false;
      Q_FOREACH(View* view, m_views) {
@@ -189,6 +202,11 @@
          m_individualsChangedDetailedId = m_notifyIsQuiescentHandlerId = 0;
          g_clear_object(&m_individualAggregator);
+     }
++}
++
++void AddressBook::shutdown()
++{
++    unprepareFolks();
      if (m_adaptor) {
          if (m_connection.interface() &&
@@ -208,6 +226,7 @@
  void AddressBook::prepareFolks()
+ {
++    m_contacts = new ContactsMap;
      m_individualAggregator = folks_individual_aggregator_dup();
      g_object_get(G_OBJECT(m_individualAggregator), "is-quiescent", &m_ready, NULL);
      if (m_ready) {
@@ -215,7 +234,7 @@
+     }
      m_notifyIsQuiescentHandlerId = g_signal_connect(m_individualAggregator,
                                            "notify::is-quiescent",
--                                          (GCallback)AddressBook::isQuiescentChanged,
++                                          (GCallback) AddressBook::isQuiescentChanged,
                                            this);
      m_individualsChangedDetailedId = g_signal_connect(m_individualAggregator,
@@ -223,12 +242,45 @@
                                            (GCallback) AddressBook::individualsChangedCb,
                                            this);
--
      folks_individual_aggregator_prepare(m_individualAggregator,
                                          (GAsyncReadyCallback) AddressBook::prepareFolksDone,
                                          this);
+ }
++void AddressBook::connectWithEDS()
++{
++    // we need to keep it update with the EDS dbus service name
++    static const QString evolutionServiceName("org.gnome.evolution.dataserver.AddressBook6");
++
++    // Check if eds was disabled manually
++    // If eds was disabled we should skip the check
++    if (qEnvironmentVariableIsSet("FOLKS_BACKENDS_ALLOWED")) {
++        QString allowedBackends = qgetenv("FOLKS_BACKENDS_ALLOWED");
++        if (!allowedBackends.contains("eds")) {
++            return;
++        }
++    }
++
++    // check if service is already registered
++    // We will try register a EDS service if its fails this mean that the service is already registered
++    m_edsIsLive = !QDBusConnection::sessionBus().registerService(evolutionServiceName);
++    if (!m_edsIsLive) {
++        // if we succeed we need to unregister it
++        QDBusConnection::sessionBus().unregisterService(evolutionServiceName);
++    }
++
++    m_edsWatcher = new QDBusServiceWatcher(evolutionServiceName,
++                                           QDBusConnection::sessionBus(),
++                                           QDBusServiceWatcher::WatchForOwnerChange,
++                                           this);
++    connect(m_edsWatcher, SIGNAL(serviceOwnerChanged(QString,QString,QString)),
++            this, SLOT(onEdsServiceOwnerChanged(QString,QString,QString)));
++
++
++    // WORKAROUND: Will ceck for EDS after the service get ready
++    connect(this, SIGNAL(ready()), SLOT(checkForEds()));
++}
++
  SourceList AddressBook::availableSources(const QDBusMessage &message)
+ {
      getSource(message, false);
@@ -606,6 +658,22 @@
      m_notifyContactUpdate->insertChangedContacts(QSet<QString>() << individual->id());
+ }
++void AddressBook::onEdsServiceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner)
++{
++    qDebug() << name << oldOwner << newOwner;
++    if (newOwner.isEmpty()) {
++        m_edsIsLive = false;
++        qWarning() << "EDS died: restarting service" << m_individualsChangedDetailedId;
++        // reset folks objects
++        unprepareFolks();
++        prepareFolks();
++
++        Q_EMIT m_adaptor->reloaded();
++    } else {
++        m_edsIsLive = true;
++    }
++}
++
  int AddressBook::removeContacts(const QStringList &contactIds, const QDBusMessage &message)
+ {
      RemoveContactsData *data = new RemoveContactsData;
@@ -920,4 +988,31 @@
      m_snQuit->setEnabled(true);
+ }
++// WORKAROUND: For some strange reason sometimes EDS does not start with the service request
++// we will try to reload folks if this happen
++void AddressBook::checkForEds()
++{
++    // Use maxRetry value to avoid infinite loop
++    static const int maxRerty = 10;
++    static int retryCount = 0;
++    if (retryCount >= maxRerty) {
++        return;
++    }
++    retryCount++;
++
++    if (!m_edsIsLive) {
++        // wait some ms to restart folks, this ms increase 500ms for each retryCount
++        QTimer::singleShot(500 * retryCount, this, SLOT(reloadFolks()));
++        qWarning() << "EDS did not start, trying to reload folks;";
++    } else {
++        retryCount = 0;
++    }
++}
++
++void AddressBook::reloadFolks()
++{
++    unprepareFolks();
++    prepareFolks();
++}
++
  } //namespace
 === modified file 'lib/addressbook.h'
 --- lib/addressbook.h	2014-04-09 19:45:40 +0000
 +++ lib/addressbook.h	2014-09-11 18:57:11 +0000
@@ -80,10 +80,15 @@
  private Q_SLOTS:
      void viewClosed();
      void individualChanged(QIndividual *individual);
++    void onEdsServiceOwnerChanged(const QString &name, const QString &oldOwner, const QString &newOwner);
      // Unix signal handlers.
      void handleSigQuit();
++    // WORKAROUND: Check if EDS was running when the service started
++    void checkForEds();
++    void reloadFolks();
++
  private:
      FolksIndividualAggregator *m_individualAggregator;
      ContactsMap *m_contacts;
@@ -91,10 +96,12 @@
      AddressBookAdaptor *m_adaptor;
      // timer to avoid send several updates at the same time
      DirtyContactsNotify *m_notifyContactUpdate;
++    QDBusServiceWatcher *m_edsWatcher;
++    bool m_edsIsLive;
      bool m_ready;
--    int m_individualsChangedDetailedId;
--    int m_notifyIsQuiescentHandlerId;
++    gulong m_individualsChangedDetailedId;
++    gulong m_notifyIsQuiescentHandlerId;
      QDBusConnection m_connection;
      // Update command
@@ -123,6 +130,8 @@
      static void quitSignalHandler(int unused);
      void prepareFolks();
++    void unprepareFolks();
++    void connectWithEDS();
      bool registerObject(QDBusConnection &connection);
      QString removeContact(FolksIndividual *individual);
      QString addContact(FolksIndividual *individual);
 === modified file 'lib/contacts-map.cpp'
 --- lib/contacts-map.cpp	2014-08-11 21:20:38 +0000
 +++ lib/contacts-map.cpp	2014-09-11 18:57:11 +0000
@@ -158,6 +158,11 @@
      return m_contacts;
+ }
++QStringList ContactsMap::keys() const
++{
++    return m_idToEntry.keys();
++}
++
  void ContactsMap::sertSort(const SortClause &clause)
+ {
      if (clause.toContactSortOrder() != m_sortClause.toContactSortOrder()) {
 === modified file 'lib/contacts-map.h'
 --- lib/contacts-map.h	2014-08-11 21:20:38 +0000
 +++ lib/contacts-map.h	2014-09-11 18:57:11 +0000
@@ -75,6 +75,8 @@
      void lock();
      void unlock();
      QList<ContactEntry*> values() const;
++    QStringList keys() const;
++
      void sertSort(const SortClause &clause);
      SortClause sort() const;
 === modified file 'lib/dirtycontact-notify.cpp'
 --- lib/dirtycontact-notify.cpp	2014-06-30 23:30:11 +0000
 +++ lib/dirtycontact-notify.cpp	2014-09-11 18:57:11 +0000
@@ -54,6 +54,12 @@
      m_timer.start();
+ }
++void DirtyContactsNotify::flush()
++{
++    m_timer.stop();
++    emitSignals();
++}
++
  void DirtyContactsNotify::insertRemovedContacts(QSet<QString> ids)
+ {
      if (!m_adaptor->isReady()) {
 === modified file 'lib/dirtycontact-notify.h'
 --- lib/dirtycontact-notify.h	2014-03-27 16:47:09 +0000
 +++ lib/dirtycontact-notify.h	2014-09-11 18:57:11 +0000
@@ -41,6 +41,7 @@
      void insertChangedContacts(QSet<QString> ids);
      void insertRemovedContacts(QSet<QString> ids);
      void insertAddedContacts(QSet<QString> ids);
++    void flush();
  private Q_SLOTS:
      void emitSignals();
 === modified file 'upstart/address-book-service.conf'
 --- upstart/address-book-service.conf	2014-08-19 13:03:47 +0000
 +++ upstart/address-book-service.conf	2014-09-11 18:57:11 +0000
@@ -1,11 +1,19 @@
  description "address-book-service"
  author "Bill Filler <bill.filler@canonical.com>"
--start on started unity8
--stop on desktop-end
++start on started dbus and xsession SESSION=ubuntu-touch
++stop on runlevel [06]
  respawn
++respawn limit unlimited
++
++pre-start script
++    echo "START `date`"
++    initctl set-env ADDRESS_BOOK_SERVICE_DEBUG="On"
++end script
++
++post-stop script
++    echo "STOP `date`"
++end script
  exec /usr/lib/arm-linux-gnueabihf/address-book-service/address-book-service
--
--pre-start exec /usr/lib/evolution/evolution-addressbook-factory

address-book-service

Merge lp:~renatofilho/address-book-service/enable-debug into lp:address-book-service

Commit message

Description of the change

Preview Diff

Subscribers