Add place add/updated/removed signals + refactor Remove

These signals indicate changes to the database.  These 3 signals
are provided rather than a single placesChanged() signal because
it gives greater granularity.  E.g. if a place is removed,
we may want to know precisely which one it is so we can remove
it from a model.

For the SearchResultmodel whenever a place is added, updated, or
deleted we reload the model by executing the query again.

For now in the jsondb plugin implement signal emission
manually whenever a request is finished.  In a future change
we will use proper jsondb notifications.

For the online plugins, we do not expect these signals to be emitted.

Also place removal now uses the QPlaceIdReply.

Change-Id: I29f5aaaa6b8152abe1f5305a4d52abc721e6915e
Reviewed-on: http://codereview.qt.nokia.com/4109
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: abcd <qt_abcd1@ovi.com>

14 files changed, 260 insertions, 45 deletions

diff --git a/src/imports/location/declarativeplaces/qdeclarativeplace.cpp b/src/imports/location/declarativeplaces/qdeclarativeplace.cpp
index 22b129ee..1a8ebda8 100644
--- a/src/imports/location/declarativeplaces/qdeclarativeplace.cpp
+++ b/src/imports/location/declarativeplaces/qdeclarativeplace.cpp
@@ -418,17 +418,29 @@ void QDeclarativePlace::finished()
     if (m_reply->error() == QPlaceReply::NoError) {
         switch (m_reply->type()) {
         case (QPlaceReply::IdReply) : {
-                QPlaceIdReply *saveReply = qobject_cast<QPlaceIdReply *>(m_reply);
-                setPlaceId(saveReply->id());
+            QPlaceIdReply *idReply = qobject_cast<QPlaceIdReply *>(m_reply);
+
+            switch (idReply->operationType()) {
+            case QPlaceIdReply::SavePlace:
+                setPlaceId(idReply->id());
                 break;
-            }
-        case (QPlaceReply::PlaceDetailsReply): {
-                QPlaceDetailsReply *detailsReply = qobject_cast<QPlaceDetailsReply *>(m_reply);
-                setPlace(detailsReply->result());
+            case QPlaceIdReply::RemovePlace:
+                setPlaceId(QString());
+                break;
+            default:
+                //Other operation types shouldn't ever be received.
                 break;
             }
-        default: //must've been a removal operation
-            setPlaceId(QString());
+            break;
+        }
+        case (QPlaceReply::PlaceDetailsReply): {
+            QPlaceDetailsReply *detailsReply = qobject_cast<QPlaceDetailsReply *>(m_reply);
+            setPlace(detailsReply->result());
+            break;
+        }
+        default:
+            //other types of replies shouldn't ever be received.
+            break;
         }
 
         m_errorString.clear();
diff --git a/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.cpp b/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.cpp
index 21204715..c6f57ecd 100644
--- a/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.cpp
+++ b/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.cpp
@@ -68,6 +68,8 @@ void QDeclarativeSearchModelBase::setPlugin(QDeclarativeGeoServiceProvider *plug
     if (m_plugin == plugin)
         return;
 
+    initializePlugin(m_plugin, plugin);
+
     reset(); // reset the model
     m_plugin = plugin;
     if (m_complete)
@@ -193,6 +195,13 @@ void QDeclarativeSearchModelBase::componentComplete()
     m_complete = true;
 }
 
+void QDeclarativeSearchModelBase::initializePlugin(QDeclarativeGeoServiceProvider *oldPlugin,
+                                                   QDeclarativeGeoServiceProvider *newPlugin)
+{
+    Q_UNUSED(oldPlugin);
+    Q_UNUSED(newPlugin);
+}
+
 void QDeclarativeSearchModelBase::queryFinished()
 {
     if (!m_reply)
diff --git a/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.h b/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.h
index be0e738f..ec822547 100644
--- a/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.h
+++ b/src/imports/location/declarativeplaces/qdeclarativesearchmodelbase.h
@@ -103,6 +103,10 @@ signals:
     void limitChanged();
     void executingChanged();
 
+protected:
+    virtual void initializePlugin(QDeclarativeGeoServiceProvider *oldPlugin,
+                                  QDeclarativeGeoServiceProvider *newPlugin);
+
 private slots:
     void queryFinished();
     void queryError(QPlaceReply::Error error, const QString &errorString);
diff --git a/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel.cpp b/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel.cpp
index 80892263..cf1c6e11 100644
--- a/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel.cpp
+++ b/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel.cpp
@@ -138,7 +138,7 @@ QT_USE_NAMESPACE
 */
 
 QDeclarativeSearchResultModel::QDeclarativeSearchResultModel(QObject *parent)
-:   QDeclarativeSearchModelBase(parent)
+    : QDeclarativeSearchModelBase(parent), m_placeManager(0)
 {
     QHash<int, QByteArray> roleNames;
     roleNames = QAbstractItemModel::roleNames();
@@ -329,5 +329,45 @@ QVariant QDeclarativeSearchResultModel::data(const QModelIndex &index, int role)
 QPlaceReply *QDeclarativeSearchResultModel::sendQuery(QPlaceManager *manager,
                                                       const QPlaceSearchRequest &request)
 {
+    Q_ASSERT(manager);
     return manager->searchForPlaces(request);
 }
+
+void QDeclarativeSearchResultModel::initializePlugin(QDeclarativeGeoServiceProvider *oldPlugin,
+                                                     QDeclarativeGeoServiceProvider *newPlugin)
+{
+    //The purpose of initialization is to connect to the place manager's signals
+    //for place notifications so we can rexecute the query
+
+    //disconnect the manager of the old plugin if we have one
+    if (oldPlugin) {
+        QGeoServiceProvider *serviceProvider = oldPlugin->sharedGeoServiceProvider();
+        if (serviceProvider) {
+            QPlaceManager *placeManager = serviceProvider->placeManager();
+            if (placeManager) {
+                disconnect(placeManager, SIGNAL(placeAdded(QString)),
+                           this, SLOT(executeQuery()));
+                disconnect(placeManager, SIGNAL(placeUpdated(QString)),
+                           this, SLOT(executeQuery()));
+                disconnect(placeManager, SIGNAL(placeRemoved(QString)),
+                           this, SLOT(executeQuery()));
+            }
+        }
+    }
+
+    //connect to the manager of the new plugin.
+    if (newPlugin) {
+        QGeoServiceProvider *serviceProvider = newPlugin->sharedGeoServiceProvider();
+        if (serviceProvider) {
+            QPlaceManager *placeManager = serviceProvider->placeManager();
+            if (placeManager) {
+                connect(placeManager, SIGNAL(placeAdded(QString)),
+                           this, SLOT(executeQuery()));
+                connect(placeManager, SIGNAL(placeUpdated(QString)),
+                           this, SLOT(executeQuery()));
+                connect(placeManager, SIGNAL(placeRemoved(QString)),
+                           this, SLOT(executeQuery()));
+            }
+        }
+    }
+}
diff --git a/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel_p.h b/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel_p.h
index 76d7539d..90a0da2f 100644
--- a/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel_p.h
+++ b/src/imports/location/declarativeplaces/qdeclarativesearchresultmodel_p.h
@@ -47,6 +47,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QDeclarativeGeoServiceProvider;
+
 class QDeclarativeSearchResultModel : public QDeclarativeSearchModelBase
 {
     Q_OBJECT
@@ -109,11 +111,14 @@ signals:
 
 protected:
     QPlaceReply *sendQuery(QPlaceManager *manager, const QPlaceSearchRequest &request);
+    virtual void initializePlugin(QDeclarativeGeoServiceProvider *oldPlugin,
+                                  QDeclarativeGeoServiceProvider *newPlugin);
 
 private:
     QList<QPlaceSearchResult> m_results;
     QMap<QString, QDeclarativePlace *> m_places;
     QDeclarativeCategory m_category;
+    QPlaceManager *m_placeManager;
 };
 
 QT_END_NAMESPACE
diff --git a/src/location/places/qplacemanager.cpp b/src/location/places/qplacemanager.cpp
index b66cd111..d2f55e20 100644
--- a/src/location/places/qplacemanager.cpp
+++ b/src/location/places/qplacemanager.cpp
@@ -106,6 +106,13 @@ QPlaceManager::QPlaceManager(QPlaceManagerEngine *engine, QObject *parent)
                 this, SIGNAL(error(QPlaceReply*,QPlaceReply::Error)));
         connect(d->engine,SIGNAL(authenticationRequired(QAuthenticator*)),
                 this, SIGNAL(authenticationRequired(QAuthenticator*)));
+
+        connect(d->engine, SIGNAL(placeAdded(QString)),
+                this, SIGNAL(placeAdded(QString)), Qt::QueuedConnection);
+        connect(d->engine, SIGNAL(placeUpdated(QString)),
+                this, SIGNAL(placeUpdated(QString)), Qt::QueuedConnection);
+        connect(d->engine, SIGNAL(placeRemoved(QString)),
+                this, SIGNAL(placeRemoved(QString)), Qt::QueuedConnection);
     } else {
         qFatal("The place manager engine that was set for this place manager was NULL.");
     }
@@ -211,7 +218,7 @@ QPlaceIdReply *QPlaceManager::savePlace(const QGeoPlace &place, VisibilityScope
 /*!
     Removes a \a place from the manager
 */
-QPlaceReply *QPlaceManager::removePlace(const QGeoPlace &place)
+QPlaceIdReply *QPlaceManager::removePlace(const QGeoPlace &place)
 {
     return d->engine->removePlace(place);
 }
@@ -307,3 +314,28 @@ Use deleteLater() instead.
 
     If authentication is unsuccessful, the manager will emit the signal again.
 */
+
+/*!
+    \fn void QPlaceManager::placeAdded(const QString&placeId)
+
+    This signal is emitted if a place has been added to the manager's datastore.
+
+    It is generally only emitted by managers that store places locally.
+
+*/
+
+/*!
+    \fn void QPlaceManager::placeUpdated(const QString&placeId)
+
+    This signal is emitted if a place has been modified in the manager's datastore.
+
+    It is generally only emitted by managers that store places locally.
+*/
+
+/*!
+    \fn void QPlaceManager::placeRemoved(const QString&placeId)
+
+    This signal is emitted if a place has been removed from the manager's datastore.
+
+    It is generally only emitted by managers that store places locally.
+*/
diff --git a/src/location/places/qplacemanager.h b/src/location/places/qplacemanager.h
index e3881d70..984ecd42 100644
--- a/src/location/places/qplacemanager.h
+++ b/src/location/places/qplacemanager.h
@@ -125,7 +125,7 @@ public:
     QPlaceIdReply *savePlace(const QGeoPlace &place, VisibilityScope scope = QPlaceManager::NoScope);
     VisibilityScopes supportedSaveVisibilityScopes();
 
-    QPlaceReply *removePlace(const QGeoPlace &place);
+    QPlaceIdReply *removePlace(const QGeoPlace &place);
 
     QPlaceReply *initializeCategories();
     QList<QPlaceCategory> categories(const QPlaceCategory &parent = QPlaceCategory()) const;
@@ -140,6 +140,10 @@ Q_SIGNALS:
     void error(QPlaceReply *, QPlaceReply::Error error, const QString &errorString = QString());
     void authenticationRequired(QAuthenticator *authenticator);
 
+    void placeAdded(const QString &placeId);
+    void placeUpdated(const QString &placeId);
+    void placeRemoved(const QString &placeId);
+
 private:
     QPlaceManager(QPlaceManagerEngine *engine, QObject *parent = 0);
 
diff --git a/src/location/places/qplacemanagerengine.cpp b/src/location/places/qplacemanagerengine.cpp
index 6b5155fc..77c87f4f 100644
--- a/src/location/places/qplacemanagerengine.cpp
+++ b/src/location/places/qplacemanagerengine.cpp
@@ -153,6 +153,31 @@ QPlaceManagerEnginePrivate::~QPlaceManagerEnginePrivate()
 {
 }
 
+/*!
+    \fn void QPlaceManagerEngine::placeAdded(const QString&placeId)
+
+    This signal is emitted if a place has been added to the manager engine's datastore.
+
+    It is generally only emitted by managers that store places locally.
+
+*/
+
+/*!
+    \fn void QPlaceManagerEngine::placeUpdated(const QString&placeId)
+
+    This signal is emitted if a place has been modified in the manager engine's datastore.
+
+    It is generally only emitted by managers that store places locally.
+*/
+
+/*!
+    \fn void QPlaceManagerEngine::placeRemoved(const QString&placeId)
+
+    This signal is emitted if a place has been removed from the manager engine's datastore.
+
+    It is generally only emitted by managers that store places locally.
+*/
+
 #include "moc_qplacemanagerengine.cpp"
 
 QT_END_NAMESPACE
diff --git a/src/location/places/qplacemanagerengine.h b/src/location/places/qplacemanagerengine.h
index 82312368..6902a151 100644
--- a/src/location/places/qplacemanagerengine.h
+++ b/src/location/places/qplacemanagerengine.h
@@ -77,7 +77,7 @@ public:
     virtual QPlaceIdReply *savePlace(const QGeoPlace &place, QPlaceManager::VisibilityScope scope) = 0;
     virtual QPlaceManager::VisibilityScopes supportedSaveVisibilityScopes() const = 0;
 
-    virtual QPlaceReply *removePlace(const QGeoPlace &place) = 0;
+    virtual QPlaceIdReply *removePlace(const QGeoPlace &place) = 0;
 
     virtual QPlaceReply *initializeCategories() = 0;
     virtual QList<QPlaceCategory> categories(const QPlaceCategory &parent) const = 0;
@@ -90,6 +90,10 @@ Q_SIGNALS:
     void error(QPlaceReply *, QPlaceReply::Error error, QString errorString = QString());
     void authenticationRequired(QAuthenticator *authenticator);
 
+    void placeAdded(const QString &placeId);
+    void placeUpdated(const QString &placeId);
+    void placeRemoved(const QString &placeId);
+
 private:
     void setManagerName(const QString &managerName);
     void setManagerVersion(int managerVersion);
diff --git a/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.cpp b/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.cpp
index a736b6cc..08f30556 100644
--- a/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.cpp
+++ b/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.cpp
@@ -260,7 +260,7 @@ QPlaceManager::VisibilityScopes QPlaceManagerEngineNokia::supportedSaveVisibilit
     return QPlaceManager::NoScope;
 }
 
-QPlaceReply *QPlaceManagerEngineNokia::removePlace(const QGeoPlace &place)
+QPlaceIdReply *QPlaceManagerEngineNokia::removePlace(const QGeoPlace &place)
 {
     Q_UNUSED(place)
 
diff --git a/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.h b/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.h
index 400baaad..cb2e9a11 100644
--- a/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.h
+++ b/src/plugins/geoservices/nokia/qplacemanagerengine_nokia.h
@@ -84,7 +84,7 @@ public:
     QPlaceIdReply *savePlace(const QGeoPlace &place, QPlaceManager::VisibilityScope scope);
     QPlaceManager::VisibilityScopes supportedSaveVisibilityScopes() const;
 
-    QPlaceReply *removePlace(const QGeoPlace &place);
+    QPlaceIdReply *removePlace(const QGeoPlace &place);
 
     QPlaceReply *initializeCategories();
     QList<QPlaceCategory> categories(const QPlaceCategory &parent) const;
diff --git a/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.cpp b/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.cpp
index 495b6b15..5f963c92 100644
--- a/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.cpp
+++ b/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.cpp
@@ -199,18 +199,19 @@ QPlaceManager::VisibilityScopes QPlaceManagerEngineJsonDb::supportedSaveVisibili
     return QPlaceManager::NoScope | QPlaceManager::PrivateScope;
 }
 
-QPlaceReply *QPlaceManagerEngineJsonDb::removePlace(const QGeoPlace &place)
+QPlaceIdReply *QPlaceManagerEngineJsonDb::removePlace(const QGeoPlace &place)
 {
-    Reply *reply = new Reply(this);
+    IdReply *removeReply = new IdReply(QPlaceIdReply::RemovePlace, this);
 
     if (!m_jsonDbHandler.isConnected()) {
-        reply->triggerDone(QPlaceReply::CommunicationError, "No connection to jsondb database");
-        return reply;
+        removeReply->triggerDone(QPlaceReply::CommunicationError, "No connection to jsondb database");
+        return removeReply;
     }
 
     int reqId = m_jsonDbHandler.remove(place.placeId());
-    m_idReplyMap.insert(reqId, reply);
-    return reply;
+    removeReply->setId(place.placeId());
+    m_idReplyMap.insert(reqId, removeReply);
+    return removeReply;
 }
 
 QPlaceReply *QPlaceManagerEngineJsonDb::initializeCategories()
@@ -242,17 +243,76 @@ void QPlaceManagerEngineJsonDb::processJsonDbResponse(int id, const QVariant &da
     if (reply) {
         switch (reply->type()) {
         case QPlaceReply::IdReply: {
+                IdReply *idReply = qobject_cast<IdReply *>(reply);
+
+                switch (idReply->operationType()) {
+                case QPlaceIdReply::SavePlace:
+                case QPlaceIdReply::SaveCategory:
+                        /*
+                        Expected data format
+                        {
+                        "uuid":<uuid>,
+                        "_version": <version>
+                        }*/
+                    idReply->setId(data.toMap().value(UUID).toString());
+                    break;
+                case QPlaceIdReply::RemovePlace:
+                case QPlaceIdReply::RemoveCategory: {
+                    /*
+                    Expected data format example
+                    {
+                    "count":1,
+                    "data":[
+                        {
+                        "_uuid":"8c196304-509c-5c45-0a07-0ea25a280f10523d"
+                        }],
+                    "error":[]
+                    }*/
+                    QVariantMap jsonResponse = data.toMap();
+                    if (jsonResponse.value(QLatin1String("count")).toInt() >= 0) {
+                        QVariantMap jsonResponse = data.toMap();
+                        QString uuid = jsonResponse.value(QLatin1String("data"))
+                                .toList().at(0).toMap().value(UUID).toString();
+                        if (uuid != idReply->id()) {
+                            idReply->triggerDone(QPlaceReply::UnknownError,
+                                                 tr("JsonDb Response UUID does not match that in request"
+                                                    "for a removal operation\n"
+                                                    "JsonDb UUID: %1"
+                                                    "Request UUID: %2").arg(uuid).arg(idReply->id()));
+                        }
+                    } else {
+                        idReply->triggerDone(QPlaceReply::UnknownError,
+                                             tr("JsonDb response does not contain a uuid"
+                                                "for a removal request"));
+                    }
+
+                    if (!jsonResponse.value(QLatin1String("error")).toStringList().isEmpty()
+                            && idReply->error() == QPlaceReply::NoError) {
+                        idReply->triggerDone(QPlaceReply::UnknownError,
+                                             tr("JsonDb response had unexpected errors: %1")
+                                             .arg(jsonResponse.value(QLatin1String("error"))
+                                                    .toStringList().join(QLatin1String(","))));
+                    }
 
-                /*
-                Expected data format
-                {
-                "uuid":<uuid>,
-                "_version": <version>
+                    //id should've already been set
+                    break;
+                }
+                default:
+                    //Other types should not be possible
+                    break;
+                }
+
+                if (idReply->error() == QPlaceReply::NoError)
+                    idReply->triggerDone();
+
+                //TODO: This is workaround code, it is intended that we use
+                //       jsondb notifications to emit the added, updated
+                //       and removed signals.
+                if (idReply->operationType() == QPlaceIdReply::RemovePlace) {
+                    emit placeRemoved(idReply->id());
+                } else if (idReply->operationType() == QPlaceIdReply::SavePlace) {
+                    emit placeAdded(idReply->id());
                 }
-            */
-                IdReply *idReply = qobject_cast<IdReply *>(reply);
-                idReply->setId(data.toMap().value(UUID).toString());
-                idReply->triggerDone();
                 break;
             }
         case QPlaceReply::PlaceSearchReply: {
@@ -349,17 +409,16 @@ void QPlaceManagerEngineJsonDb::processJsonDbError(int id, int code, const QStri
         switch (placeReply->type()) {
         case QPlaceReply::IdReply: {
             IdReply *idReply = qobject_cast<IdReply *>(placeReply);
-            switch (idReply->operationType()) {
-            case (QPlaceIdReply::SavePlace):
-                switch (code) {
-                case JsonDbError::MissingObject:
+            switch (code) {
+            case JsonDbError::MissingObject:
+                switch (idReply->operationType()) {
+                case QPlaceIdReply::SavePlace:
+                case QPlaceIdReply::RemovePlace:
                     error = QPlaceReply::PlaceDoesNotExistError;
-                    errorString = tr("Trying to update place which does not exist");
-                    break;
+                    errorString = tr("Place does not exist");
                 }
-                idReply->triggerDone(error, errorString);
-                break;
             }
+            idReply->triggerDone(error, errorString);
             break;
         }
         case QPlaceReply::PlaceSearchReply: {
diff --git a/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.h b/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.h
index 0003c8ce..d6ece1f1 100644
--- a/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.h
+++ b/src/plugins/geoservices/nokia_places_jsondb/qplacemanagerengine_jsondb.h
@@ -76,7 +76,7 @@ public:
     QPlaceIdReply *savePlace(const QGeoPlace &place, QPlaceManager::VisibilityScope scope);
     QPlaceManager::VisibilityScopes supportedSaveVisibilityScopes() const;
 
-    QPlaceReply *removePlace(const QGeoPlace &place);
+    QPlaceIdReply *removePlace(const QGeoPlace &place);
 
     QPlaceReply *initializeCategories();
     QList<QPlaceCategory> categories(const QPlaceCategory &parent) const;
diff --git a/tests/auto/qplacemanager_jsondb/tst_qplacemanager_jsondb.cpp b/tests/auto/qplacemanager_jsondb/tst_qplacemanager_jsondb.cpp
index db05e061..0c3870c5 100644
--- a/tests/auto/qplacemanager_jsondb/tst_qplacemanager_jsondb.cpp
+++ b/tests/auto/qplacemanager_jsondb/tst_qplacemanager_jsondb.cpp
@@ -72,6 +72,10 @@ private:
                 QString *placeId = 0,
                 QPlaceManager::VisibilityScope = QPlaceManager::NoScope);
     void doSavePlaces(QList<QGeoPlace> &places);
+
+    bool doRemovePlace(const QGeoPlace &place,
+                       QPlaceReply::Error expectedError = QPlaceReply::NoError);
+
     bool doSearch(const QPlaceSearchRequest &request,
                   QList<QPlaceSearchResult> *results,
              QPlaceReply::Error expectedError = QPlaceReply::NoError);
@@ -150,15 +154,16 @@ void tst_QPlaceManagerJsonDb::saveAndRemove()
     place.setPlaceId(placeId);
     QVERIFY(retrievedPlace == place);
 
-    //try remvoing a place
-    QPlaceReply *reply = placeManager->removePlace(place);
-    QSignalSpy removeSpy(reply, SIGNAL(finished()));
-    QTRY_VERIFY(removeSpy.count() == 1);
+    //try removing a place
+    QVERIFY(doRemovePlace(place, QPlaceReply::NoError));
 
     //ensure it is actually deleted
     QVERIFY(doFetchDetails(placeId, &retrievedPlace, QPlaceReply::PlaceDoesNotExistError));
     QCOMPARE(retrievedPlace, QGeoPlace());
 
+    //try removing a place that does not exist;
+    QVERIFY(doRemovePlace(place, QPlaceReply::PlaceDoesNotExistError));
+
     QVERIFY(doSavePlace(place, QPlaceReply::UnsupportedError,0, QPlaceManager::PublicScope));
 }
 
@@ -650,10 +655,16 @@ bool tst_QPlaceManagerJsonDb::doSavePlace(const QGeoPlace &place,
                                           QPlaceManager::VisibilityScope scope)
 {
     QPlaceIdReply *saveReply = placeManager->savePlace(place,scope);
-    bool isSuccessful = false;
-    isSuccessful = checkSignals(saveReply, expectedError);
-    if (placeId != 0)
+    bool isSuccessful = checkSignals(saveReply, expectedError);
+    if (placeId != 0) {
         *placeId = saveReply->id();
+    }
+
+    if (saveReply->id().isEmpty() && expectedError == QPlaceReply::NoError) {
+        qWarning("ID is empty in reply for save operation");
+        isSuccessful = false;
+    }
+
     return isSuccessful;
 }
 
@@ -670,6 +681,16 @@ void tst_QPlaceManagerJsonDb::doSavePlaces(QList<QGeoPlace> &places)
     }
 }
 
+bool tst_QPlaceManagerJsonDb::doRemovePlace(const QGeoPlace &place,
+                                            QPlaceReply::Error expectedError)
+{
+    QPlaceIdReply *removeReply = placeManager->removePlace(place);
+    bool isSuccessful = false;
+    isSuccessful = checkSignals(removeReply, expectedError)
+                    && (removeReply->id() == place.placeId());
+    return isSuccessful;
+}
+
 bool tst_QPlaceManagerJsonDb::doSearch(const QPlaceSearchRequest &request,
                                        QList<QPlaceSearchResult> *results, QPlaceReply::Error expectedError)
 {