client: handle result argument of wait properly

Method `wait` of `Connector` allows to obtain result directly with
`result` argument - it can be useful when the user cares about
performance and don't want to perform an unneeded insertion to a map of
ready futures. However, we check that the future is ready by looking to
the map of ready futures, and the future is not inserted there when the
argument is used. The commit fixes the problem by checking the argument
as well.

Part of #112

Author: drewdzzz

src/Client/Connector.hpp

@@ -210,8 +210,15 @@ Connector<BUFFER, NetProvider>::wait(Connection<BUFFER, NetProvider> &conn,
 	LOG_DEBUG("Waiting for the future ", future, " with timeout ", timeout);
 	Timer timer{timeout};
 	timer.start();
+	static constexpr int INVALID_SYNC = -1;
+	if (result != NULL)
+		result->header.sync = INVALID_SYNC;
 	if (connectionDecodeResponses(conn, result) != 0)
 		return -1;
+	if (result != NULL && result->header.sync != INVALID_SYNC) {
+		LOG_DEBUG("Future ", future, " is ready and decoded");
+		return 0;
+	}
 	while (!conn.hasError() && !conn.futureIsReady(future)) {
 		if (m_NetProvider.wait(timer.timeLeft()) != 0) {
 			conn.setError(std::string("Failed to poll: ") +
@@ -220,17 +227,23 @@ Connector<BUFFER, NetProvider>::wait(Connection<BUFFER, NetProvider> &conn,
 		}
 		if (connectionDecodeResponses(conn, result) != 0)
 			return -1;
+		if (result != NULL && result->header.sync != INVALID_SYNC) {
+			LOG_DEBUG("Future ", future, " is ready and decoded");
+			return 0;
+		}
 		if (timer.isExpired())
 			break;
 	}
 	if (conn.hasError()) {
 		LOG_ERROR("Connection got an error: ", conn.getError().msg);
 		return -1;
 	}
-	if (! conn.futureIsReady(future)) {
+	if (!conn.futureIsReady(future)) {
 		LOG_DEBUG("Connection has been timed out: future ", future,
 			  " is not ready");
 		return -1;
+	} else if (result != NULL) {
+		*result = std::move(conn.getResponse(future));
 	}
 	LOG_DEBUG("Feature ", future, " is ready and decoded");
 	return 0;

test/ClientTest.cpp

@@ -1280,6 +1280,29 @@ test_wait(Connector<BUFFER, NetProvider> &client)
 	client.close(conn2);
 	client.close(conn3);
 
+	TEST_CASE("wait with argument result");
+	f = conn.ping();
+	fail_unless(!conn.futureIsReady(f));
+	Response<BUFFER> result;
+	fail_unless(client.wait(conn, f, WAIT_TIMEOUT, &result) == 0);
+	/* The result was consumed, so the future is not ready. */
+	fail_unless(!conn.futureIsReady(f));
+	/* The future is actually request sync - check if the result is valid. */
+	fail_unless(result.header.sync == static_cast<int>(f));
+	fail_unless(result.header.code == 0);
+
+	TEST_CASE("wait with argument result for decoded future");
+	f = conn.ping();
+	fail_unless(!conn.futureIsReady(f));
+	fail_unless(client.wait(conn, f, WAIT_TIMEOUT) == 0);
+	fail_unless(conn.futureIsReady(f));
+	fail_unless(client.wait(conn, f, WAIT_TIMEOUT, &result) == 0);
+	/* The result was consumed, so the future is not ready. */
+	fail_unless(!conn.futureIsReady(f));
+	/* The future is actually request sync - check if the result is valid. */
+	fail_unless(result.header.sync == static_cast<int>(f));
+	fail_unless(result.header.code == 0);
+
 	client.close(conn);
 }