improved reliability of accept ack

and increased wait timeouts, configurable

Author: Skycoder42

src/datasync/messages/proofmessage.cpp

@@ -59,8 +59,8 @@ const QMetaObject *AcceptMessage::getMetaObject() const
 
 
 
-AcceptAckMessage::AcceptAckMessage(const AcceptMessage &message) :
-	deviceId(message.deviceId)
+AcceptAckMessage::AcceptAckMessage(const QUuid &deviceId) :
+	deviceId(deviceId)
 {}
 
 const QMetaObject *AcceptAckMessage::getMetaObject() const

src/datasync/messages/proofmessage_p.h

@@ -83,7 +83,7 @@ class Q_DATASYNC_EXPORT AcceptAckMessage : public Message
 	Q_PROPERTY(QUuid deviceId MEMBER deviceId)
 
 public:
-	AcceptAckMessage(const AcceptMessage &message = {});
+	AcceptAckMessage(const QUuid &deviceId = {});
 
 	QUuid deviceId;
 

tests/auto/datasync/TestAppServer/tst_appserver.cpp

@@ -958,12 +958,8 @@ void TestAppServer::testSendDoubleAccept()
 
 		//send another proof accept
 		client->sendSigned(accMsg, crypto);
-		//is still sent, event though no accept happend...
-		QVERIFY(client->waitForReply<AcceptAckMessage>([&](AcceptAckMessage message, bool &ok) {
-			QCOMPARE(message.deviceId, partnerDevId);
-			ok = true;
-		}));
-		QVERIFY(partner->waitForNothing());
+		QVERIFY(client->waitForNothing()); //no accept ack
+		QVERIFY(partner->waitForNothing()); //no grant
 
 		//clean partner
 		clean(partner);

tests/auto/datasync/TestLib/mockclient.cpp

@@ -1,5 +1,11 @@
 #include "mockclient.h"
 
+#ifdef Q_OS_WIN
+#define WAIT_TIMEOUT 15000
+#else
+#define WAIT_TIMEOUT 10000
+#endif
+
 MockClient::MockClient(QObject *parent) :
 	MockConnection(new QWebSocket(), parent),
 	_connectSpy(_socket, &QWebSocket::connected)
@@ -16,7 +22,7 @@ bool MockClient::waitForConnected(quint16 port)
 	auto ok = false;
 	[&]() {
 		if(_connectSpy.isEmpty())
-			QVERIFY(_connectSpy.wait());
+			QVERIFY(_connectSpy.wait(WAIT_TIMEOUT));
 		QVERIFY(!_connectSpy.isEmpty());
 		ok = true;
 	}();

tests/auto/datasync/TestLib/mockconnection.cpp

@@ -1,6 +1,10 @@
 #include "mockconnection.h"
 
-
+#ifdef Q_OS_WIN
+#define WAIT_TIMEOUT 15000
+#else
+#define WAIT_TIMEOUT 10000
+#endif
 
 MockConnection::MockConnection(QWebSocket *socket, QObject *parent) :
 	QObject(parent),
@@ -50,7 +54,7 @@ void MockConnection::close()
 bool MockConnection::waitForNothing()
 {
 	return _msgSpy.isEmpty() &&
-			!_msgSpy.wait() &&
+			!_msgSpy.wait(WAIT_TIMEOUT) &&
 			_closeSpy.isEmpty();
 }
 
@@ -64,7 +68,7 @@ bool MockConnection::waitForPing()
 	auto ok = false;
 	[&]() {
 		if(_msgSpy.isEmpty())
-			QVERIFY(_msgSpy.wait(65000)); // 1 min 5 secs
+			QVERIFY(_msgSpy.wait(60000 + WAIT_TIMEOUT)); // 1 min + WAIT_TIMEOUT
 		QVERIFY(!_msgSpy.isEmpty());
 		auto msg = _msgSpy.takeFirst()[0].toByteArray();
 		ok = (msg == QtDataSync::Message::PingMessage);
@@ -86,7 +90,7 @@ bool MockConnection::waitForDisconnect()
 	auto ok = false;
 	[&]() {
 		if(_closeSpy.isEmpty())
-			QVERIFY(_closeSpy.wait());
+			QVERIFY(_closeSpy.wait(WAIT_TIMEOUT));
 		QVERIFY(!_closeSpy.isEmpty());
 		ok = true;
 	}();
@@ -100,7 +104,7 @@ bool MockConnection::waitForReplyImpl(const std::function<void(QByteArray, bool&
 		QByteArray msg;
 		do {
 			if(_msgSpy.isEmpty())
-				QVERIFY(_msgSpy.wait());
+				QVERIFY(_msgSpy.wait(WAIT_TIMEOUT));
 			QVERIFY(!_msgSpy.isEmpty());
 			msg = _msgSpy.takeFirst()[0].toByteArray();
 			if(msg == QtDataSync::Message::PingMessage)

tests/auto/datasync/TestLib/mockserver.cpp

@@ -1,5 +1,11 @@
 #include "mockserver.h"
 
+#ifdef Q_OS_WIN
+#define WAIT_TIMEOUT 10000
+#else
+#define WAIT_TIMEOUT 5000
+#endif
+
 MockServer::MockServer(QObject *parent) :
 	QObject(parent),
 	_server(new QWebSocketServer(QStringLiteral("mockserver"), QWebSocketServer::NonSecureMode, this)),
@@ -44,7 +50,7 @@ bool MockServer::waitForConnected(MockConnection **connection, int timeout)
 	[&]() {
 		QVERIFY(connection);
 		if(_connectedSpy.isEmpty())
-			QVERIFY(_connectedSpy.wait(timeout));
+			QVERIFY(_connectedSpy.wait(WAIT_TIMEOUT + timeout));
 		QVERIFY(!_connectedSpy.isEmpty());
 		QVERIFY(_server->hasPendingConnections());
 		_connectedSpy.removeFirst();

tests/auto/datasync/TestRemoteConnector/tst_remoteconnector.cpp

@@ -680,7 +680,7 @@ void TestRemoteConnector::testAddDeviceUntrusted()
 
 			//Send from here because message needed
 			partnerConnection->send(GrantMessage(message));
-			connection->send(AcceptAckMessage(message));
+			connection->send(AcceptAckMessage(message.deviceId));
 		}));
 
 		//verify preperations are done
@@ -784,7 +784,7 @@ void TestRemoteConnector::testAddDeviceTrusted()
 
 			//Send from here because message needed
 			partnerCon->send(GrantMessage(message));
-			connection->send(AcceptAckMessage(message));
+			connection->send(AcceptAckMessage(message.deviceId));
 		}));
 
 		//verify preperations are done
@@ -1562,7 +1562,7 @@ void TestRemoteConnector::testUnexpectedMessage_data()
 									  << false;
 	QTest::newRow("ProofMessage") << create<ProofMessage>(AccessMessage(), partnerDevId)
 								  << false;
-	QTest::newRow("AcceptAckMessage") << create<AcceptAckMessage>(AcceptMessage(partnerDevId))
+	QTest::newRow("AcceptAckMessage") << create<AcceptAckMessage>(partnerDevId)
 									  << false;
 	QTest::newRow("MacUpdateAckMessage") << create<MacUpdateAckMessage>()
 										 << false;

tools/appserver/client.cpp

@@ -186,6 +186,16 @@ void Client::sendProof(const ProofMessage &message)
 	});
 }
 
+void Client::acceptDone(const QUuid &deviceId)
+{
+	run([this, deviceId]() {
+		if(_state != Idle)
+			qWarning() << "Cannot send accept ack when not in idle state";
+		else
+			sendMessage(AcceptAckMessage(deviceId));
+	});
+}
+
 void Client::binaryMessageReceived(const QByteArray &message)
 {
 	if(message == Message::PingMessage) {
@@ -552,7 +562,6 @@ void Client::onAccept(const AcceptMessage &message, QDataStream &stream)
 	}
 
 	emit proofDone(message.deviceId, true, message);
-	sendMessage(AcceptAckMessage(message));
 }
 
 void Client::onDeny(const DenyMessage &message)

tools/appserver/client.h

@@ -55,6 +55,7 @@ public Q_SLOTS:
 	void notifyChanged();
 	void proofResult(bool success, const QtDataSync::AcceptMessage &message = {}); //empty key equals denied
 	void sendProof(const QtDataSync::ProofMessage &message);
+	void acceptDone(const QUuid &deviceId);
 
 Q_SIGNALS:
 	void connected(const QUuid &deviceId);

tools/appserver/clientconnector.cpp

@@ -194,9 +194,18 @@ void ClientConnector::proofRequested(const QUuid &partner, const QtDataSync::Pro
 		connect(pClient, &Client::proofDone,
 				client, [devId, pClient, client](const QUuid &partner, bool success, const QtDataSync::AcceptMessage &message) {
 			if(devId == partner) {
-				client->proofResult(success, message);
-				if(pClient)
+				if(pClient) {
+					// remove this connections
 					pClient->disconnect(client);
+					// once client was added, notify pClient so he can ack the accept
+					connect(client, &Client::connected,
+							pClient, [pClient](const QUuid &accPartner) {
+						pClient->acceptDone(accPartner);
+						//no disconnect needed, single time emit
+					}, Qt::QueuedConnection);
+				}
+				// pass message on to client
+				client->proofResult(success, message);
 			}
 		}, Qt::QueuedConnection);
 		pClient->sendProof(message);