provider: "Multiple subscribe" and "unsubscribe without subscribe" are no longer errors.boottimewarnings

Sending these messages does no actual harm; the intention of the client
is clear. "Multiple subscribe" can actually happen when the provider is not
running when the subscriber starts. Contextkit recovers from that situation,
and treating "multiple subscribe" as an error provides little value.

3 files changed, 35 insertions, 21 deletions

diff --git a/libcontextprovider/customer-tests/service/servicetest.cpp b/libcontextprovider/customer-tests/service/servicetest.cpp
index c27b80ac..378f6bbf 100644
--- a/libcontextprovider/customer-tests/service/servicetest.cpp
+++ b/libcontextprovider/customer-tests/service/servicetest.cpp
@@ -179,14 +179,25 @@ void ServiceTest::multiStart()
     QString expected = "Subscribe returned: Unknown";
     QCOMPARE(actual.simplified(), expected.simplified());
 
+    // Start listening to signals (we already have one subscriber)
+    QSignalSpy firstSpy(property,
+                        SIGNAL(firstSubscriberAppeared(const QString&)));
+    QSignalSpy lastSpy(property,
+                        SIGNAL(firstSubscriberAppeared(const QString&)));
+
     // Test: start the service again (even though it's started)
     service->start();
 
     // Expected result: the service is still there, and remembers the client
     actual = writeToClient("subscribe service Test.Property\n");
-    expected = "Subscribe error: org.maemo.contextkit.Error.MultipleSubscribe";
+    expected = "Subscribe returned: Unknown";
     QCOMPARE(actual.simplified(), expected.simplified());
 
+    // (so we don't get a signal)
+    QTest::qWait(1000);
+    QCOMPARE(firstSpy.count(), 0);
+    QCOMPARE(lastSpy.count(), 0);
+
     delete service;
     delete property;
 }
diff --git a/libcontextprovider/customer-tests/subscription/subscriptiontests.cpp b/libcontextprovider/customer-tests/subscription/subscriptiontests.cpp
index 335f604e..e610524b 100644
--- a/libcontextprovider/customer-tests/subscription/subscriptiontests.cpp
+++ b/libcontextprovider/customer-tests/subscription/subscriptiontests.cpp
@@ -258,16 +258,19 @@ void SubscriptionTests::multiSubscribe()
     // Doing this only in init() is not enough; doesn't stop the test case.
     QVERIFY(clientStarted);
 
+    test_int->setValue(567);
+
     // Ask the client to call Subscribe for a property twice. The
-    // property exists and is currently unknown since we haven't set a
-    // value for it.
-    writeToClient("subscribe service1 Test.Int\n");
+    // property exists and has a value.
+    QString actual1 = writeToClient("subscribe service1 Test.Int\n");
 
-    QString actual = writeToClient("subscribe service1 Test.Int\n");
+    QString actual2 = writeToClient("subscribe service1 Test.Int\n");
 
-    // Expected result: Unsubscribe returns a D-Bus error
-    QString expected("Subscribe error: org.maemo.contextkit.Error.MultipleSubscribe");
-    QCOMPARE(actual.simplified(), expected.simplified());
+    // Expected result: both Subscribes return the same value.
+    QString expected("Subscribe returned: int:567");
+
+    QCOMPARE(actual1.simplified(), expected.simplified());
+    QCOMPARE(actual2.simplified(), expected.simplified());
 }
 
 void SubscriptionTests::illegalUnsubscribe()
@@ -281,8 +284,8 @@ void SubscriptionTests::illegalUnsubscribe()
     // since we haven't set a value for it.
     QString actual = writeToClient("unsubscribe service1 Test.Int\n");
 
-    // Expected result: Unsubscribe returns a D-Bus error
-    QString expected("Unsubscribe error: org.maemo.contextkit.Error.IllegalUnsubscribe");
+    // Expected result: Unsubscribe succeeds.
+    QString expected("Unsubscribe called");
     QCOMPARE(actual.simplified(), expected.simplified());
 }
 
diff --git a/libcontextprovider/src/propertyadaptor.cpp b/libcontextprovider/src/propertyadaptor.cpp
index f82b3055..e90a44d4 100644
--- a/libcontextprovider/src/propertyadaptor.cpp
+++ b/libcontextprovider/src/propertyadaptor.cpp
@@ -65,8 +65,8 @@ void PropertyAdaptor::Subscribe(const QDBusMessage &msg, QVariantList& values, q
 {
     contextDebug() << "Subscribe called";
 
-    // Store the information of the subscription. For each client, we
-    // record how many times the client has subscribed.
+    // Store the information of the subscription. For each property, we record
+    // which clients have subscribed.
     QString client = msg.service();
 
     if (clientServiceNames.contains(client) == false) {
@@ -77,11 +77,12 @@ void PropertyAdaptor::Subscribe(const QDBusMessage &msg, QVariantList& values, q
         serviceWatcher.addWatchedService(client);
     }
     else {
-        contextWarning() << "Client" << client << "subscribed to property" << propertyPrivate->key << "multiple times";
-        QDBusMessage error = msg.createErrorReply("org.maemo.contextkit.Error.MultipleSubscribe",
-                             "Subscribing to " + propertyPrivate->key + " multiple times");
-        connection->send(error);
+        contextDebug() << "Client" << client << "subscribed to property" << propertyPrivate->key << "multiple times";
     }
+    // Don't treat the "client sends 2 Subscribe calls" as an error.  When the
+    // provider is not running (e.g., during boot), the subscriber might send 2
+    // Subscribe calls: one of them before the provider is running, and the
+    // other when it notices that the provider has started.
 
     // Construct the return values
     Get(values, timestamp);
@@ -100,12 +101,11 @@ void PropertyAdaptor::Unsubscribe(const QDBusMessage &msg)
         serviceWatcher.removeWatchedService(client);
     }
     else {
-        contextWarning() << "Client" << client << "unsubscribed from property" <<
+        contextDebug() << "Client" << client << "unsubscribed from property" <<
             propertyPrivate->key << "without subscribing";
-        QDBusMessage error = msg.createErrorReply("org.maemo.contextkit.Error.IllegalUnsubscribe",
-                                                  "Unsubscribing from a non-subscribed property "
-                                                  + propertyPrivate->key);
-        connection->send(error);
+        // Don't treat the "client sends Unsubscribe for a non-subscribed
+        // property" as an error.  The intention of the client is to not be
+        // subscribed, so there's nothing to do.
     }
 }