QDeclarativeGeoLocation: add property bindings

Signals of QDeclarativeGeoLocation class are removed
because they were only needed for QML. With the new
bindable properties we do not need them any more.

Task-number: QTBUG-89874
Change-Id: I084fbde129d0bf3a1d7ad496f7fa25274593989f
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>

6 files changed, 290 insertions, 44 deletions

diff --git a/src/positioningquick/qdeclarativegeolocation.cpp b/src/positioningquick/qdeclarativegeolocation.cpp
index 97f6829c..b62ab7ca 100644
--- a/src/positioningquick/qdeclarativegeolocation.cpp
+++ b/src/positioningquick/qdeclarativegeolocation.cpp
@@ -106,19 +106,22 @@ QDeclarativeGeoLocation::~QDeclarativeGeoLocation()
 
     For details on how to use this property to interface between C++ and QML see
     "\l {Location - QGeoLocation} {Interfaces between C++ and QML Code}".
+
+    \note This property updates the whole geo location information, so using it
+    will result in breaking of all the bindings for all other properties.
 */
 void QDeclarativeGeoLocation::setLocation(const QGeoLocation &src)
 {
     if (m_address && m_address->parent() == this) {
         m_address->setAddress(src.address());
     } else if (!m_address || m_address->parent() != this) {
-        m_address = new QDeclarativeGeoAddress(src.address(), this);
-        emit addressChanged();
+        m_address.setValue(new QDeclarativeGeoAddress(src.address(), this));
+        m_address.notify();
     }
 
     setCoordinate(src.coordinate());
     setBoundingShape(src.boundingShape());
-    setProperty("extendedAttributes", src.extendedAttributes());
+    setExtendedAttributes(src.extendedAttributes());
 }
 
 QGeoLocation QDeclarativeGeoLocation::location() const
@@ -138,14 +141,25 @@ QGeoLocation QDeclarativeGeoLocation::location() const
 */
 void QDeclarativeGeoLocation::setAddress(QDeclarativeGeoAddress *address)
 {
+    m_address.removeBindingUnlessInWrapper();
     if (m_address == address)
         return;
 
+    // implicitly deleting m_address.value() will force the QML bindings to
+    // be reevaluated by the QML engine. So we defer the deletion of the old
+    // address until a new value is assigned to the m_address.
+    QDeclarativeGeoAddress *oldAddress = nullptr;
     if (m_address && m_address->parent() == this)
-        delete m_address;
+        oldAddress = m_address.value();
+
+    m_address.setValueBypassingBindings(address);
+    m_address.notify();
+    delete oldAddress;
+}
 
-    m_address = address;
-    emit addressChanged();
+QBindable<QDeclarativeGeoAddress *> QDeclarativeGeoLocation::bindableAddress()
+{
+    return QBindable<QDeclarativeGeoAddress *>(&m_address);
 }
 
 QDeclarativeGeoAddress *QDeclarativeGeoLocation::address() const
@@ -163,11 +177,12 @@ QDeclarativeGeoAddress *QDeclarativeGeoLocation::address() const
 */
 void QDeclarativeGeoLocation::setCoordinate(const QGeoCoordinate coordinate)
 {
-    if (m_coordinate == coordinate)
-        return;
-
     m_coordinate = coordinate;
-    emit coordinateChanged();
+}
+
+QBindable<QGeoCoordinate> QDeclarativeGeoLocation::bindableCoordinate()
+{
+    return QBindable<QGeoCoordinate>(&m_coordinate);
 }
 
 QGeoCoordinate QDeclarativeGeoLocation::coordinate() const
@@ -200,11 +215,27 @@ QGeoCoordinate QDeclarativeGeoLocation::coordinate() const
 */
 void QDeclarativeGeoLocation::setBoundingShape(const QGeoShape &boundingShape)
 {
-    if (m_boundingShape == boundingShape)
-        return;
-
     m_boundingShape = boundingShape;
-    emit boundingShapeChanged();
+}
+
+QBindable<QGeoShape> QDeclarativeGeoLocation::bindableBoundingShape()
+{
+    return QBindable<QGeoShape>(&m_boundingShape);
+}
+
+QVariantMap QDeclarativeGeoLocation::extendedAttributes() const
+{
+    return m_extendedAttributes;
+}
+
+void QDeclarativeGeoLocation::setExtendedAttributes(const QVariantMap &attributes)
+{
+    m_extendedAttributes = attributes;
+}
+
+QBindable<QVariantMap> QDeclarativeGeoLocation::bindableExtendedAttributes()
+{
+    return QBindable<QVariantMap>(&m_extendedAttributes);
 }
 
 QGeoShape QDeclarativeGeoLocation::boundingShape() const
diff --git a/src/positioningquick/qdeclarativegeolocation_p.h b/src/positioningquick/qdeclarativegeolocation_p.h
index e9310229..c1083952 100644
--- a/src/positioningquick/qdeclarativegeolocation_p.h
+++ b/src/positioningquick/qdeclarativegeolocation_p.h
@@ -53,6 +53,7 @@
 
 #include <QtCore/QObject>
 #include <QtCore/QVariantMap>
+#include <QtCore/private/qproperty_p.h>
 #include <QtPositioning/QGeoLocation>
 #include <QtPositioning/QGeoShape>
 #include <QtPositioningQuick/private/qdeclarativegeoaddress_p.h>
@@ -67,10 +68,14 @@ class Q_POSITIONINGQUICK_PRIVATE_EXPORT QDeclarativeGeoLocation : public QObject
     QML_ADDED_IN_VERSION(5, 0)
 
     Q_PROPERTY(QGeoLocation location READ location WRITE setLocation)
-    Q_PROPERTY(QDeclarativeGeoAddress *address READ address WRITE setAddress NOTIFY addressChanged)
-    Q_PROPERTY(QGeoCoordinate coordinate READ coordinate WRITE setCoordinate NOTIFY coordinateChanged)
-    Q_PROPERTY(QGeoShape boundingShape READ boundingShape WRITE setBoundingShape NOTIFY boundingShapeChanged REVISION(6, 2))
-    Q_PROPERTY(QVariantMap extendedAttributes MEMBER m_extendedAttributes NOTIFY extendedAttributesChanged REVISION(5, 13))
+    Q_PROPERTY(QDeclarativeGeoAddress *address READ address WRITE setAddress BINDABLE
+                       bindableAddress)
+    Q_PROPERTY(QGeoCoordinate coordinate READ coordinate WRITE setCoordinate BINDABLE
+                       bindableCoordinate)
+    Q_PROPERTY(QGeoShape boundingShape READ boundingShape WRITE setBoundingShape BINDABLE
+                       bindableBoundingShape REVISION(6, 2))
+    Q_PROPERTY(QVariantMap extendedAttributes READ extendedAttributes WRITE setExtendedAttributes
+                       BINDABLE bindableExtendedAttributes REVISION(5, 13))
 
 public:
     explicit QDeclarativeGeoLocation(QObject *parent = 0);
@@ -82,23 +87,26 @@ public:
 
     QDeclarativeGeoAddress *address() const;
     void setAddress(QDeclarativeGeoAddress *address);
+    QBindable<QDeclarativeGeoAddress *> bindableAddress();
+
     QGeoCoordinate coordinate() const;
     void setCoordinate(const QGeoCoordinate coordinate);
+    QBindable<QGeoCoordinate> bindableCoordinate();
 
     QGeoShape boundingShape() const;
     void setBoundingShape(const QGeoShape &boundingShape);
+    QBindable<QGeoShape> bindableBoundingShape();
 
-Q_SIGNALS:
-    void addressChanged();
-    void coordinateChanged();
-    void boundingShapeChanged();
-    void extendedAttributesChanged();
+    QVariantMap extendedAttributes() const;
+    void setExtendedAttributes(const QVariantMap &attributes);
+    QBindable<QVariantMap> bindableExtendedAttributes();
 
 private:
-    QDeclarativeGeoAddress *m_address = nullptr;
-    QGeoShape m_boundingShape;
-    QGeoCoordinate m_coordinate;
-    QVariantMap m_extendedAttributes;
+    Q_OBJECT_COMPAT_PROPERTY_WITH_ARGS(QDeclarativeGeoLocation, QDeclarativeGeoAddress *, m_address,
+                                       &QDeclarativeGeoLocation::setAddress, nullptr)
+    Q_OBJECT_BINDABLE_PROPERTY(QDeclarativeGeoLocation, QGeoShape, m_boundingShape)
+    Q_OBJECT_BINDABLE_PROPERTY(QDeclarativeGeoLocation, QGeoCoordinate, m_coordinate)
+    Q_OBJECT_BINDABLE_PROPERTY(QDeclarativeGeoLocation, QVariantMap, m_extendedAttributes)
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/CMakeLists.txt b/tests/auto/CMakeLists.txt
index 02eff897..9fd84dd9 100644
--- a/tests/auto/CMakeLists.txt
+++ b/tests/auto/CMakeLists.txt
@@ -77,6 +77,7 @@ if(TARGET Qt::Quick AND NOT ANDROID)
     add_subdirectory(dummypositionplugin)
     add_subdirectory(declarative_positioning_core)
     add_subdirectory(declarative_geolocation)
+    add_subdirectory(qdeclarativegeolocation)
     add_subdirectory(qdeclarativeposition)
     add_subdirectory(qdeclarativepositionsource)
     add_subdirectory(qquickgeocoordinateanimation)
diff --git a/tests/auto/declarative_geolocation/tst_declarativegeolocation.qml b/tests/auto/declarative_geolocation/tst_declarativegeolocation.qml
index 74aa36ee..47596304 100644
--- a/tests/auto/declarative_geolocation/tst_declarativegeolocation.qml
+++ b/tests/auto/declarative_geolocation/tst_declarativegeolocation.qml
@@ -45,10 +45,30 @@ Item {
         id: location
     }
 
-    SignalSpy { id: addressSpy; target: location; signalName: "addressChanged" }
-    SignalSpy { id: coordinateSpy; target: location; signalName: "coordinateChanged" }
-    SignalSpy { id: boundingShapeSpy; target: location; signalName: "boundingShapeChanged" }
-    SignalSpy { id: attributesSpy; target: location; signalName: "extendedAttributesChanged" }
+    // Use property bindings insetead of signal spies
+    property var addressObserver: location.address
+    property int addressChangedCount: 0
+    onAddressObserverChanged: {
+        ++addressChangedCount
+    }
+
+    property var coordObserver: location.coordinate
+    property int coordChangedCount: 0
+    onCoordObserverChanged: {
+        ++coordChangedCount
+    }
+
+    property var shapeObserver: location.boundingShape
+    property int shapeChangedCount: 0
+    onShapeObserverChanged: {
+        ++shapeChangedCount
+    }
+
+    property var attrsObserver: location.extendedAttributes
+    property int attrsChangedCount: 0
+    onAttrsObserverChanged: {
+        ++attrsChangedCount
+    }
 
     Address {
         id: emptyAddress
@@ -84,40 +104,37 @@ Item {
         }
 
         function test_address_changed() {
-            verify(addressSpy.valid)
-            addressSpy.clear()
+            addressChangedCount = 0
             location.address = addr1
-            compare(addressSpy.count, 1)
+            compare(addressChangedCount, 1)
             compare(location.address.address, addr1.address)
         }
 
         function test_coordinate_changed() {
             var coord1 = QtPositioning.coordinate(1.0, 2.0)
             var emptyCoord = QtPositioning.coordinate()
-            verify(coordinateSpy.valid)
-            coordinateSpy.clear()
+            coordChangedCount = 0
             compare(location.coordinate, emptyCoord)
             location.coordinate = coord1
-            compare(coordinateSpy.count, 1)
+            compare(coordChangedCount, 1)
             compare(location.coordinate, coord1)
         }
 
         function test_bounding_box_changed() {
             var emptyShape = QtPositioning.shape()
-            verify(boundingShapeSpy.valid)
-            boundingShapeSpy.clear()
+            shapeChangedCount = 0
             compare(location.boundingShape, emptyShape)
 
             var box = QtPositioning.rectangle(topLeft, bottomRight)
             location.boundingShape = box
-            compare(boundingShapeSpy.count, 1)
+            compare(shapeChangedCount, 1)
             compare(QtPositioning.shapeToRectangle(location.boundingShape), box)
             // verify that shape's boundingGeoRectangle() matches the box.
             compare(location.boundingShape.boundingGeoRectangle(), box)
 
             var circle = QtPositioning.circle(centerPoint, 100)
             location.boundingShape = circle
-            compare(boundingShapeSpy.count, 2)
+            compare(shapeChangedCount, 2)
             compare(QtPositioning.shapeToCircle(location.boundingShape), circle)
         }
 
@@ -130,13 +147,12 @@ Item {
         }
 
         function test_extended_attributes() {
-            verify(attributesSpy.valid)
-            attributesSpy.clear()
+            attrsChangedCount = 0
 
             var attributes = { "foo" : 42 }
             location.extendedAttributes = attributes;
             compare(location.extendedAttributes, attributes)
-            compare(attributesSpy.count, 1)
+            compare(attrsChangedCount, 1)
 
             attributes["bar"] = 41
             verify(location.extendedAttributes !== attributes)
diff --git a/tests/auto/qdeclarativegeolocation/CMakeLists.txt b/tests/auto/qdeclarativegeolocation/CMakeLists.txt
new file mode 100644
index 00000000..ebf65896
--- /dev/null
+++ b/tests/auto/qdeclarativegeolocation/CMakeLists.txt
@@ -0,0 +1,9 @@
+qt_internal_add_test(tst_qdeclarativegeolocation
+    SOURCES
+        tst_qdeclarativegeolocation.cpp
+    LIBRARIES
+        Qt::Core
+        Qt::Positioning
+        Qt::PositioningQuickPrivate
+        Qt::TestPrivate
+)
diff --git a/tests/auto/qdeclarativegeolocation/tst_qdeclarativegeolocation.cpp b/tests/auto/qdeclarativegeolocation/tst_qdeclarativegeolocation.cpp
new file mode 100644
index 00000000..e6818209
--- /dev/null
+++ b/tests/auto/qdeclarativegeolocation/tst_qdeclarativegeolocation.cpp
@@ -0,0 +1,181 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+#include <QtTest/QtTest>
+#include <QtTest/private/qpropertytesthelper_p.h>
+#include <QtPositioning/QGeoCircle>
+#include <QtPositioning/QGeoRectangle>
+#include <QtPositioningQuick/private/qdeclarativegeolocation_p.h>
+
+QT_USE_NAMESPACE
+
+class tst_DeclarativeGeoLocation : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void locationProperty();
+    void addressBinding();
+    void coordinateBinding();
+    void shapeBinding();
+    void attributesBinding();
+};
+
+void tst_DeclarativeGeoLocation::locationProperty()
+{
+    // This test calls setLocation() with different preconditions, providing
+    // coverage for different branches of the setLocation() method.
+
+    QGeoAddress addr;
+    addr.setCountryCode("DEU");
+    addr.setCountry("Germany");
+    addr.setCity("Berlin");
+    addr.setStreet("Erich-Thilo-Str");
+    addr.setStreetNumber("10");
+    addr.setPostalCode("12489");
+
+    const QGeoCoordinate c(52.43, 13.53);
+
+    const QVariantMap attrs { { "string_proprty", "value" }, { "int_property", 5 } };
+
+    QGeoLocation loc1;
+    loc1.setAddress(addr);
+    loc1.setCoordinate(c);
+    loc1.setBoundingShape(QGeoCircle(c, 100));
+    loc1.setExtendedAttributes(attrs);
+
+    QDeclarativeGeoLocation location;
+    qsizetype addrChangedCount = 0;
+    qsizetype coordChangedCount = 0;
+    qsizetype shapeChangedCount = 0;
+    qsizetype attrChangedCount = 0;
+
+    auto addrChangedHandler = location.bindableAddress().onValueChanged(
+            [&addrChangedCount]() { ++addrChangedCount; });
+    Q_UNUSED(addrChangedHandler)
+    auto coordChangedHandler = location.bindableCoordinate().onValueChanged(
+            [&coordChangedCount]() { ++coordChangedCount; });
+    Q_UNUSED(coordChangedHandler)
+    auto shapeChangedHandler = location.bindableBoundingShape().onValueChanged(
+            [&shapeChangedCount]() { ++shapeChangedCount; });
+    Q_UNUSED(shapeChangedHandler)
+    auto attrChangedHandler = location.bindableExtendedAttributes().onValueChanged(
+            [&attrChangedCount]() { ++attrChangedCount; });
+    Q_UNUSED(attrChangedHandler)
+
+    // By default an empty m_address is created in the default constructor.
+    // So m_address contains a valid pointer, m_address->parent() == this.
+    location.setLocation(loc1);
+    QCOMPARE(addrChangedCount, 0); // the pointer didn't change
+    QCOMPARE(coordChangedCount, 1);
+    QCOMPARE(shapeChangedCount, 1);
+    QCOMPARE(attrChangedCount, 1);
+    QCOMPARE(location.location(), loc1);
+    QCOMPARE(location.address()->parent(), &location);
+
+    // m_address contains a nullptr
+    location.setAddress(nullptr);
+    QVERIFY(!location.address());
+    addrChangedCount = 0;
+    coordChangedCount = 0;
+    shapeChangedCount = 0;
+    attrChangedCount = 0;
+
+    location.setLocation(loc1);
+    QCOMPARE(addrChangedCount, 1); // only the pointer has changed
+    QCOMPARE(coordChangedCount, 0);
+    QCOMPARE(shapeChangedCount, 0);
+    QCOMPARE(attrChangedCount, 0);
+    QCOMPARE(location.location(), loc1);
+    QCOMPARE(location.address()->parent(), &location);
+
+    // m_address contains a valid pointer, m_address->parent() != this
+    QGeoAddress addr1;
+    addr1.setCountryCode("USA");
+    addr1.setCountry("United States");
+    addr1.setPostalCode("8900");
+    addr1.setState("Oregon");
+    addr1.setCity("Springfield");
+    addr1.setDistrict("Pressboard Estates");
+    addr1.setStreet("Evergreen Tce");
+    addr1.setStreetNumber("742");
+
+    QDeclarativeGeoAddress geoAddr(addr1);
+
+    location.setAddress(&geoAddr);
+    QVERIFY(location.address());
+    QCOMPARE(location.address()->parent(), nullptr);
+    addrChangedCount = 0;
+
+    location.setLocation(loc1);
+    QCOMPARE(addrChangedCount, 1); // only the pointer has changed
+    QCOMPARE(coordChangedCount, 0);
+    QCOMPARE(shapeChangedCount, 0);
+    QCOMPARE(attrChangedCount, 0);
+    QCOMPARE(location.location(), loc1);
+    QCOMPARE(location.address()->parent(), &location);
+}
+
+void tst_DeclarativeGeoLocation::addressBinding()
+{
+    QDeclarativeGeoLocation location;
+    QDeclarativeGeoAddress addr1;
+    QDeclarativeGeoAddress addr2;
+    QTestPrivate::testReadWritePropertyBasics<QDeclarativeGeoLocation, QDeclarativeGeoAddress *>(
+            location, &addr1, &addr2, "address");
+}
+
+void tst_DeclarativeGeoLocation::coordinateBinding()
+{
+    QDeclarativeGeoLocation location;
+    const QGeoCoordinate c1(2.0, 1.0);
+    const QGeoCoordinate c2(1.0, 2.0);
+    QTestPrivate::testReadWritePropertyBasics<QDeclarativeGeoLocation, QGeoCoordinate>(
+            location, c1, c2, "coordinate");
+}
+
+void tst_DeclarativeGeoLocation::shapeBinding()
+{
+    QDeclarativeGeoLocation location;
+    const QGeoCircle circle(QGeoCoordinate(2.0, 1.0), 10);
+    const QGeoRectangle rectangle(QGeoCoordinate(2.0, 1.0), QGeoCoordinate(1.0, 2.0));
+    QTestPrivate::testReadWritePropertyBasics<QDeclarativeGeoLocation, QGeoShape>(
+            location, circle, rectangle, "boundingShape");
+}
+
+void tst_DeclarativeGeoLocation::attributesBinding()
+{
+    QDeclarativeGeoLocation location;
+    const QVariantMap m1 { { "string_proprty", "value" }, { "int_property", 5 } };
+    const QVariantMap m2 { { "int_property", 10 }, { "double_property", 15.5 } };
+    QTestPrivate::testReadWritePropertyBasics<QDeclarativeGeoLocation, QVariantMap>(
+            location, m1, m2, "extendedAttributes");
+}
+
+QTEST_APPLESS_MAIN(tst_DeclarativeGeoLocation)
+
+#include "tst_qdeclarativegeolocation.moc"