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.

Also, the commit handles a situation when user waits for already decoded
future using `result` argument. Before the commit, `wait` would return
zero return code (success) but the `result` wouldn't be set - user still
had to check if it's not in the map of futures. After the commit,
already decoded future is moved to `result` as well, so if the function
returns successfully, `result` is guaranteed to be set.

Part of #112

Author: drewdzzz

src/Client/Connector.hpp

@@ -212,8 +212,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: ") +
@@ -222,6 +229,10 @@ 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;
 	}
@@ -233,6 +244,8 @@ Connector<BUFFER, NetProvider>::wait(Connection<BUFFER, NetProvider> &conn,
 		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);
 }