From 3ee61f7918cad8ff8ecc834b4fa85380c57afef6 Mon Sep 17 00:00:00 2001
From: Paolo Angelelli <paolo.angelelli@qt.io>
Date: Mon, 3 Dec 2018 11:22:43 +0100
Subject: [PATCH] Expose manager-specific errors from QGeoServiceProvider

So that there's no risk to mix failures when creating multiple
managers.

Task-number: QTBUG-72180
Change-Id: I5c3b18ba17094e1480b2376e37b58d47029ca8f4
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
---
 .../qdeclarativegeocodemodel.cpp              |   6 +-
 .../declarativemaps/qdeclarativegeomap.cpp    |   4 +-
 .../qdeclarativegeoroutemodel.cpp             |   6 +-
 src/location/labs/qdeclarativenavigator.cpp   | 104 +++++++++++---
 src/location/labs/qdeclarativenavigator_p.h   |  26 +++-
 src/location/labs/qdeclarativenavigator_p_p.h |   3 +
 src/location/maps/qgeoserviceprovider.cpp     | 135 +++++++++++++++++-
 src/location/maps/qgeoserviceprovider.h       |  13 ++
 8 files changed, 260 insertions(+), 37 deletions(-)

diff --git a/src/location/declarativemaps/qdeclarativegeocodemodel.cpp b/src/location/declarativemaps/qdeclarativegeocodemodel.cpp
index a73f93416..dcff0d445 100644
--- a/src/location/declarativemaps/qdeclarativegeocodemodel.cpp
+++ b/src/location/declarativemaps/qdeclarativegeocodemodel.cpp
@@ -293,9 +293,9 @@ void QDeclarativeGeocodeModel::pluginReady()
     QGeoServiceProvider *serviceProvider = plugin_->sharedGeoServiceProvider();
     QGeoCodingManager *geocodingManager = serviceProvider->geocodingManager();
 
-    if (serviceProvider->error() != QGeoServiceProvider::NoError) {
+    if (serviceProvider->geocodingError() != QGeoServiceProvider::NoError) {
         QDeclarativeGeocodeModel::GeocodeError newError = UnknownError;
-        switch (serviceProvider->error()) {
+        switch (serviceProvider->geocodingError()) {
         case QGeoServiceProvider::NotSupportedError:
             newError = EngineNotSetError; break;
         case QGeoServiceProvider::UnknownParameterError:
@@ -308,7 +308,7 @@ void QDeclarativeGeocodeModel::pluginReady()
             break;
         }
 
-        setError(newError, serviceProvider->errorString());
+        setError(newError, serviceProvider->geocodingErrorString());
         return;
     }
 
diff --git a/src/location/declarativemaps/qdeclarativegeomap.cpp b/src/location/declarativemaps/qdeclarativegeomap.cpp
index 3c73ca127..542b478ea 100644
--- a/src/location/declarativemaps/qdeclarativegeomap.cpp
+++ b/src/location/declarativemaps/qdeclarativegeomap.cpp
@@ -387,8 +387,8 @@ void QDeclarativeGeoMap::pluginReady()
     QGeoServiceProvider *provider = m_plugin->sharedGeoServiceProvider();
     m_mappingManager = provider->mappingManager();
 
-    if (provider->error() != QGeoServiceProvider::NoError) {
-        setError(provider->error(), provider->errorString());
+    if (provider->mappingError() != QGeoServiceProvider::NoError) {
+        setError(provider->mappingError(), provider->mappingErrorString());
         return;
     }
 
diff --git a/src/location/declarativemaps/qdeclarativegeoroutemodel.cpp b/src/location/declarativemaps/qdeclarativegeoroutemodel.cpp
index 20ccfdd2f..96f4dad12 100644
--- a/src/location/declarativemaps/qdeclarativegeoroutemodel.cpp
+++ b/src/location/declarativemaps/qdeclarativegeoroutemodel.cpp
@@ -345,9 +345,9 @@ void QDeclarativeGeoRouteModel::pluginReady()
     QGeoServiceProvider *serviceProvider = plugin_->sharedGeoServiceProvider();
     QGeoRoutingManager *routingManager = serviceProvider->routingManager();
 
-    if (serviceProvider->error() != QGeoServiceProvider::NoError) {
+    if (serviceProvider->routingError() != QGeoServiceProvider::NoError) {
         QDeclarativeGeoRouteModel::RouteError newError = UnknownError;
-        switch (serviceProvider->error()) {
+        switch (serviceProvider->routingError()) {
         case QGeoServiceProvider::NotSupportedError:
             newError = EngineNotSetError; break;
         case QGeoServiceProvider::UnknownParameterError:
@@ -360,7 +360,7 @@ void QDeclarativeGeoRouteModel::pluginReady()
             break;
         }
 
-        setError(newError, serviceProvider->errorString());
+        setError(newError, serviceProvider->routingErrorString());
         return;
     }
 
diff --git a/src/location/labs/qdeclarativenavigator.cpp b/src/location/labs/qdeclarativenavigator.cpp
index 86c2b85a6..dd3eaabfb 100644
--- a/src/location/labs/qdeclarativenavigator.cpp
+++ b/src/location/labs/qdeclarativenavigator.cpp
@@ -37,6 +37,7 @@
 #include <QtLocation/private/qdeclarativegeoroute_p.h>
 #include <QtLocation/private/qdeclarativegeoroutemodel_p.h>
 #include <QtLocation/private/qdeclarativegeoroutesegment_p.h>
+#include <QtLocation/qgeoserviceprovider.h>
 #include <QtPositioningQuick/private/qdeclarativepositionsource_p.h>
 #include <QtQml/qqmlinfo.h>
 
@@ -186,6 +187,23 @@ QT_BEGIN_NAMESPACE
     has been reached.
 */
 
+/*!
+    \qmlproperty enumeration Qt.labs.location::Navigator::error
+
+    This read-only property holds the latest error value of the geocoding request.
+
+    \list
+    \li Navigator.NoError - No error has occurred.
+    \li GeocodeModel.NotSupportedError - Navigation is not supported by the service provider.
+    \li GeocodeModel.ConnectionError - An error occurred while communicating with the service provider.
+    \li GeocodeModel.LoaderError - The geoservice provider library could not be loaded. Setting QT_DEBUG_PLUGINS environment variable may help diagnosing the reason.
+    \li GeocodeModel.UnknownParameterError - An unknown parameter was specified
+    \li GeocodeModel.MissingRequiredParameterError -  required parameter was not specified.
+    \li GeocodeModel.UnknownError - An error occurred which does not fit into any of the other categories.
+
+    \endlist
+*/
+
 QDeclarativeNavigatorPrivate::QDeclarativeNavigatorPrivate(QParameterizableObject *q_)
     : q(q_), m_params(new QDeclarativeNavigatorParams)
 {
@@ -360,6 +378,16 @@ int QDeclarativeNavigator::currentSegment() const
     return d_ptr->m_currentSegment;
 }
 
+QDeclarativeNavigator::NavigationError QDeclarativeNavigator::error() const
+{
+    return d_ptr->m_error;
+}
+
+QString QDeclarativeNavigator::errorString() const
+{
+    return d_ptr->m_errorString;
+}
+
 bool QDeclarativeNavigator::active() const
 {
     return d_ptr->m_active;
@@ -439,27 +467,56 @@ bool QDeclarativeNavigator::ensureEngine()
     if (!d_ptr->m_completed || !d_ptr->m_plugin->isAttached())
         return false;
 
-    auto manager = d_ptr->m_plugin->sharedGeoServiceProvider()->navigationManager();
-    if (manager) {
-        d_ptr->m_navigator.reset(manager->createNavigator(d_ptr->m_params));
-        if (!d_ptr->m_navigator)
-            return false;
-        d_ptr->m_navigator->setLocale(manager->locale());
-        d_ptr->m_navigator->setMeasurementSystem(manager->measurementSystem());
-        connect(d_ptr->m_navigator.get(), &QAbstractNavigator::waypointReached, this, &QDeclarativeNavigator::waypointReached);
-        connect(d_ptr->m_navigator.get(), &QAbstractNavigator::destinationReached, this, &QDeclarativeNavigator::destinationReached);
-        connect(d_ptr->m_navigator.get(), &QAbstractNavigator::currentRouteChanged, this, &QDeclarativeNavigator::onCurrentRouteChanged);
-        connect(d_ptr->m_navigator.get(), &QAbstractNavigator::currentRouteLegChanged, this, &QDeclarativeNavigator::onCurrentRouteLegChanged);
-        connect(d_ptr->m_navigator.get(), &QAbstractNavigator::currentSegmentChanged, this, &QDeclarativeNavigator::onCurrentSegmentChanged);
-        connect(d_ptr->m_navigator.get(), &QAbstractNavigator::activeChanged, this, [this](bool active){
-            d_ptr->m_active = active;
-            emit activeChanged(active);
-        });
-        connect(this, &QDeclarativeNavigator::trackPositionSourceChanged, d_ptr->m_navigator.get(), &QAbstractNavigator::setTrackPosition);
-        emit navigatorReadyChanged(true);
-        return true;
+    QGeoServiceProvider *serviceProvider = d_ptr->m_plugin->sharedGeoServiceProvider();
+    // if m_plugin->isAttached(), serviceProvider cannot be null
+    QNavigationManager *manager = serviceProvider->navigationManager();
+
+    if (serviceProvider->navigationError() != QGeoServiceProvider::NoError) {
+        QDeclarativeNavigator::NavigationError newError = UnknownError;
+        switch (serviceProvider->navigationError()) {
+        case QGeoServiceProvider::NotSupportedError:
+            newError = NotSupportedError; break;
+        case QGeoServiceProvider::UnknownParameterError:
+            newError = UnknownParameterError; break;
+        case QGeoServiceProvider::MissingRequiredParameterError:
+            newError = MissingRequiredParameterError; break;
+        case QGeoServiceProvider::ConnectionError:
+            newError = ConnectionError; break;
+        case QGeoServiceProvider::LoaderError:
+            newError = LoaderError; break;
+        default:
+            break;
+        }
+
+        setError(newError, serviceProvider->navigationErrorString());
+        return false;
+    }
+
+    if (!manager) {
+        setError(NotSupportedError, tr("Plugin does not support navigation."));
+        return false;
+    }
+
+    d_ptr->m_navigator.reset(manager->createNavigator(d_ptr->m_params));
+    if (!d_ptr->m_navigator) {
+        setError(UnknownError, tr("Failed to create a navigator object."));
+        return false;
     }
-    return false;
+
+    d_ptr->m_navigator->setLocale(manager->locale());
+    d_ptr->m_navigator->setMeasurementSystem(manager->measurementSystem());
+    connect(d_ptr->m_navigator.get(), &QAbstractNavigator::waypointReached, this, &QDeclarativeNavigator::waypointReached);
+    connect(d_ptr->m_navigator.get(), &QAbstractNavigator::destinationReached, this, &QDeclarativeNavigator::destinationReached);
+    connect(d_ptr->m_navigator.get(), &QAbstractNavigator::currentRouteChanged, this, &QDeclarativeNavigator::onCurrentRouteChanged);
+    connect(d_ptr->m_navigator.get(), &QAbstractNavigator::currentRouteLegChanged, this, &QDeclarativeNavigator::onCurrentRouteLegChanged);
+    connect(d_ptr->m_navigator.get(), &QAbstractNavigator::currentSegmentChanged, this, &QDeclarativeNavigator::onCurrentSegmentChanged);
+    connect(d_ptr->m_navigator.get(), &QAbstractNavigator::activeChanged, this, [this](bool active){
+        d_ptr->m_active = active;
+        emit activeChanged(active);
+    });
+    connect(this, &QDeclarativeNavigator::trackPositionSourceChanged, d_ptr->m_navigator.get(), &QAbstractNavigator::setTrackPosition);
+    emit navigatorReadyChanged(true);
+    return true;
 }
 
 void QDeclarativeNavigator::updateReadyState() {
@@ -473,6 +530,13 @@ void QDeclarativeNavigator::updateReadyState() {
         emit navigatorReadyChanged(d_ptr->m_ready);
 }
 
+void QDeclarativeNavigator::setError(QDeclarativeNavigator::NavigationError error, const QString &errorString)
+{
+    d_ptr->m_error = error;
+    d_ptr->m_errorString = errorString;
+    emit errorChanged();
+}
+
 void QDeclarativeNavigator::onCurrentRouteChanged(const QGeoRoute &route)
 {
     if (d_ptr->m_currentRoute)
diff --git a/src/location/labs/qdeclarativenavigator_p.h b/src/location/labs/qdeclarativenavigator_p.h
index 2a425e707..b99494310 100644
--- a/src/location/labs/qdeclarativenavigator_p.h
+++ b/src/location/labs/qdeclarativenavigator_p.h
@@ -52,6 +52,7 @@
 #include <QtQml/qqml.h>
 #include <QSharedPointer>
 #include <QtLocation/private/qparameterizableobject_p.h>
+#include <QtLocation/qgeoserviceprovider.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -81,9 +82,25 @@ class Q_LOCATION_PRIVATE_EXPORT QDeclarativeNavigator : public QParameterizableO
     Q_PROPERTY(QDeclarativeGeoRoute *currentRoute READ currentRoute NOTIFY currentRouteChanged)
     Q_PROPERTY(QDeclarativeGeoRouteLeg *currentRouteLeg READ currentRouteLeg NOTIFY currentRouteChanged)
     Q_PROPERTY(int currentSegment READ currentSegment NOTIFY currentSegmentChanged)
+    Q_PROPERTY(NavigationError error READ error NOTIFY errorChanged)
+    Q_PROPERTY(QString errorString READ errorString NOTIFY errorChanged)
     Q_INTERFACES(QQmlParserStatus)
 
 public:
+    enum NavigationError {
+        //QGeoServiceProvider related errors start here
+        NoError = QGeoServiceProvider::NoError,
+        NotSupportedError = QGeoServiceProvider::NotSupportedError, //TODO Qt6 consider merge with NotSupportedError
+        ConnectionError = QGeoServiceProvider::ConnectionError, //TODO Qt6 merge with Map's ConnectionError
+        LoaderError = QGeoServiceProvider::LoaderError,
+        UnknownParameterError = QGeoServiceProvider::UnknownParameterError, //TODO Qt6 consider rename UnsupportedOperationError
+        MissingRequiredParameterError = QGeoServiceProvider::MissingRequiredParameterError,
+        //we leave gap for future QGeoCodeReply errors
+
+        // Navigation-specific error should start at 100
+        UnknownError = 100
+    };
+
     explicit QDeclarativeNavigator(QObject *parent = nullptr);
     ~QDeclarativeNavigator();
 
@@ -119,6 +136,9 @@ public:
     QDeclarativeGeoRouteLeg *currentRouteLeg() const;
     int currentSegment() const;
 
+    NavigationError error() const;
+    QString errorString() const;
+
 signals:
     void navigatorReadyChanged(bool ready);
     void trackPositionSourceChanged(bool trackPositionSource);
@@ -133,13 +153,15 @@ signals:
     void currentRouteChanged();
     void currentRouteLegChanged();
     void currentSegmentChanged();
+    void errorChanged();
 
-private:
+protected:
     void pluginReady();
     bool ensureEngine();
     void updateReadyState();
+    void setError(NavigationError error, const QString &errorString);
 
-private slots:
+protected slots:
     void onCurrentRouteChanged(const QGeoRoute &route);
     void onCurrentRouteLegChanged(const QGeoRouteLeg &routeLeg);
     void onCurrentSegmentChanged(int segment);
diff --git a/src/location/labs/qdeclarativenavigator_p_p.h b/src/location/labs/qdeclarativenavigator_p_p.h
index 8849ec7a0..74bc3a19f 100644
--- a/src/location/labs/qdeclarativenavigator_p_p.h
+++ b/src/location/labs/qdeclarativenavigator_p_p.h
@@ -52,6 +52,7 @@
 #include <QtLocation/private/qlocationglobal_p.h>
 #include <QtCore/qpointer.h>
 #include <QtLocation/qgeoroute.h>
+#include <QtLocation/private/qdeclarativenavigator_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -94,6 +95,8 @@ public:
     bool m_active = false;
     bool m_completed = false;
     bool m_ready = false;
+    QDeclarativeNavigator::NavigationError m_error = QDeclarativeNavigator::NoError;
+    QString m_errorString;
 };
 
 QT_END_NAMESPACE
diff --git a/src/location/maps/qgeoserviceprovider.cpp b/src/location/maps/qgeoserviceprovider.cpp
index d25c379a7..d2f98b4d2 100644
--- a/src/location/maps/qgeoserviceprovider.cpp
+++ b/src/location/maps/qgeoserviceprovider.cpp
@@ -446,9 +446,12 @@ Manager *QGeoServiceProviderPrivate::manager(QGeoServiceProvider::Error *_error,
 */
 QGeoCodingManager *QGeoServiceProvider::geocodingManager() const
 {
-    return d_ptr->manager<QGeoCodingManager, QGeoCodingManagerEngine>(
+    QGeoCodingManager *mgr = d_ptr->manager<QGeoCodingManager, QGeoCodingManagerEngine>(
                &(d_ptr->geocodeError), &(d_ptr->geocodeErrorString),
                &(d_ptr->geocodingManager));
+    if (!mgr)
+        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    return mgr;
 }
 
 /*!
@@ -475,9 +478,12 @@ QGeoCodingManager *QGeoServiceProvider::geocodingManager() const
 */
 QGeoMappingManager *QGeoServiceProvider::mappingManager() const
 {
-    return d_ptr->manager<QGeoMappingManager, QGeoMappingManagerEngine>(
+    QGeoMappingManager *mgr = d_ptr->manager<QGeoMappingManager, QGeoMappingManagerEngine>(
                &(d_ptr->mappingError), &(d_ptr->mappingErrorString),
                &(d_ptr->mappingManager));
+    if (!mgr)
+        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    return mgr;
 }
 
 /*!
@@ -502,9 +508,12 @@ QGeoMappingManager *QGeoServiceProvider::mappingManager() const
 */
 QGeoRoutingManager *QGeoServiceProvider::routingManager() const
 {
-    return d_ptr->manager<QGeoRoutingManager, QGeoRoutingManagerEngine>(
+    QGeoRoutingManager *mgr = d_ptr->manager<QGeoRoutingManager, QGeoRoutingManagerEngine>(
                &(d_ptr->routingError), &(d_ptr->routingErrorString),
                &(d_ptr->routingManager));
+    if (!mgr)
+        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    return mgr;
 }
 
 /*!
@@ -525,9 +534,12 @@ QGeoRoutingManager *QGeoServiceProvider::routingManager() const
 */
 QPlaceManager *QGeoServiceProvider::placeManager() const
 {
-    return d_ptr->manager<QPlaceManager, QPlaceManagerEngine>(
+    QPlaceManager *mgr = d_ptr->manager<QPlaceManager, QPlaceManagerEngine>(
                &(d_ptr->placeError), &(d_ptr->placeErrorString),
                 &(d_ptr->placeManager));
+    if (!mgr)
+        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
+    return mgr;
 }
 
 /*!
@@ -541,9 +553,8 @@ QNavigationManager *QGeoServiceProvider::navigationManager() const
     QNavigationManager * mgr = d_ptr->manager<QNavigationManager, QNavigationManagerEngine>(
                &(d_ptr->navigationError), &(d_ptr->navigationErrorString),
                 &(d_ptr->navigationManager));
-    if (!mgr) {
-        qDebug() << d_ptr->navigationError << d_ptr->navigationErrorString;
-    }
+    if (!mgr)
+        qDebug() << d_ptr->error << ", " << d_ptr->errorString;
     return mgr;
 }
 
@@ -565,6 +576,116 @@ QString QGeoServiceProvider::errorString() const
     return d_ptr->errorString;
 }
 
+/*!
+    Returns an error code describing the error which occurred during the
+    last attempt to create a mapping manager.
+
+    \since 5.13
+*/
+QGeoServiceProvider::Error QGeoServiceProvider::mappingError() const
+{
+    return d_ptr->mappingError;
+}
+
+/*!
+    Returns a string describing the error which occurred during the
+    last attempt to create a mapping manager.
+
+    \since 5.13
+*/
+QString QGeoServiceProvider::mappingErrorString() const
+{
+    return d_ptr->mappingErrorString;
+}
+
+/*!
+    Returns an error code describing the error which occurred during the
+    last attempt to create a geocoding manager.
+
+    \since 5.13
+*/
+QGeoServiceProvider::Error QGeoServiceProvider::geocodingError() const
+{
+    return d_ptr->geocodeError;
+}
+
+/*!
+    Returns a string describing the error which occurred during the
+    last attempt to create a geocoding manager.
+
+    \since 5.13
+*/
+QString QGeoServiceProvider::geocodingErrorString() const
+{
+    return d_ptr->geocodeErrorString;
+}
+
+/*!
+    Returns an error code describing the error which occurred during the
+    last attempt to create a routing manager.
+
+    \since 5.13
+*/
+QGeoServiceProvider::Error QGeoServiceProvider::routingError() const
+{
+    return d_ptr->routingError;
+}
+
+/*!
+    Returns a string describing the error which occurred during the
+    last attempt to create a routing manager.
+
+    \since 5.13
+*/
+QString QGeoServiceProvider::routingErrorString() const
+{
+    return d_ptr->routingErrorString;
+}
+
+/*!
+    Returns an error code describing the error which occurred during the
+    last attempt to create a places manager.
+
+    \since 5.13
+*/
+QGeoServiceProvider::Error QGeoServiceProvider::placesError() const
+{
+    return d_ptr->placeError;
+}
+
+/*!
+    Returns a string describing the error which occurred during the
+    last attempt to create a places manager.
+
+    \since 5.13
+*/
+QString QGeoServiceProvider::placesErrorString() const
+{
+    return d_ptr->placeErrorString;
+}
+
+/*!
+    Returns an error code describing the error which occurred during the
+    last attempt to create a navigation manager.
+
+    \since 5.13
+*/
+QGeoServiceProvider::Error QGeoServiceProvider::navigationError() const
+{
+    return d_ptr->navigationError;
+}
+
+/*!
+    Returns a string describing the error which occurred during the
+    last attempt to create a navigation manager.
+
+    \since 5.13
+*/
+QString QGeoServiceProvider::navigationErrorString() const
+{
+    return d_ptr->navigationErrorString;
+}
+
 /*!
     Sets whether experimental plugins are considered when locating the
     correct plugin library for this service provider to \a allow.
diff --git a/src/location/maps/qgeoserviceprovider.h b/src/location/maps/qgeoserviceprovider.h
index b2e0be053..fe4e5b6af 100644
--- a/src/location/maps/qgeoserviceprovider.h
+++ b/src/location/maps/qgeoserviceprovider.h
@@ -160,6 +160,19 @@ public:
 
     Error error() const;
     QString errorString() const;
+    Error loaderError() const;
+    QString loaderErrorString() const;
+
+    Error mappingError() const;
+    QString mappingErrorString() const;
+    Error geocodingError() const;
+    QString geocodingErrorString() const;
+    Error routingError() const;
+    QString routingErrorString() const;
+    Error placesError() const;
+    QString placesErrorString() const;
+    Error navigationError() const;
+    QString navigationErrorString() const;
 
     void setParameters(const QVariantMap &parameters);
     void setLocale(const QLocale &locale);
-- 
GitLab