Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions debian/changelog
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mercury (4.3.3-0) unstable; urgency=low
* Compiling project with C++14 standard
* Fixed issue with KanoWorld, KESDLT, KESMP clients when ran
under sudo
* Added a timeout capability to HTTP class

-- Team Kano <dev@kano.me> Fri, 3 Oct 2019 14:25:00 +0100

Expand Down
9 changes: 8 additions & 1 deletion include/mercury/http/http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,19 @@

#include "mercury/http/http_client_interface.h"


namespace Mercury {
namespace HTTP {

#define DEFAULT_HTTP_CLIENT_TIMEOUT 15 // seconds
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define DEFAULT_HTTP_CLIENT_TIMEOUT 15 // seconds
const unsigned int DEFAULT_HTTP_CLIENT_TIMEOUT = 15; // seconds

Also, is 15 seconds too long?


/**
* \class HTTPClient
* \brief Simple implementation of IHTTPClient
*/
class HTTPClient : public IHTTPClient {
public:
HTTPClient();
HTTPClient(int timeout_secs);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine these two constructors into one?

Suggested change
HTTPClient(int timeout_secs);
HTTPClient(int timeout_secs = DEFAULT_HTTP_CLIENT_TIMEOUT);

std::shared_ptr<JSON_Value> POST(
const std::string& url,
const std::string& body,
Expand All @@ -50,6 +52,9 @@ class HTTPClient : public IHTTPClient {
const std::string& url,
const std::string& path) override;

void set_http_client_timeout(int seconds);
int get_http_client_timeout();

protected:
/**
* \brief Builds and sends a request to a server
Expand All @@ -69,6 +74,8 @@ class HTTPClient : public IHTTPClient {
const std::map<std::string, std::string>& headers =
std::map<std::string, std::string>(),
const std::string& body = "");

int http_client_timeout;
};

} // namespace HTTP
Expand Down
19 changes: 18 additions & 1 deletion src/http/http_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ using Poco::Net::SSLManager;
using Mercury::HTTP::HTTPClient;


HTTPClient::HTTPClient() {
HTTPClient::HTTPClient(int timeout_secs = DEFAULT_HTTP_CLIENT_TIMEOUT) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default arguments in header definition, not implementation.

HTTPSessionInstantiator::registerInstantiator();
HTTPSSessionInstantiator::registerInstantiator();

// set a default timeout, can be overriden after class instantiation
set_http_client_timeout(timeout_secs);

// Prepare for SSLManager
Poco::SharedPtr<AcceptCertificateHandler> cert =
new AcceptCertificateHandler(false);
Expand All @@ -62,6 +65,18 @@ HTTPClient::HTTPClient() {
}


void HTTPClient::set_http_client_timeout(int seconds)
{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ on same line as definition.

http_client_timeout = seconds;
}


int HTTPClient::get_http_client_timeout()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise here

{
return http_client_timeout;
}


std::shared_ptr<JSON_Value> HTTPClient::send_request(
const std::string& method,
const std::string& url,
Expand Down Expand Up @@ -89,6 +104,8 @@ std::shared_ptr<JSON_Value> HTTPClient::send_request(
request.set(header.first, header.second);
}

session->setTimeout(Poco::Timespan(this->get_http_client_timeout(), 0));

std::ostream& os = session->sendRequest(request);

if (!body.empty()) {
Expand Down
3 changes: 3 additions & 0 deletions test/kw/kw_apis.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ TEST_P(ParentalConsentAPI, TestAccountVerification)
}


/* TODO: Implement tests for HTTP class with a timeout */


/**
* KanoWorld::refresh_account_verified() should return false when the
* HTTPClient throws an exception (for example, when there is a server error)
Expand Down