From 98150beb38f73b56c7bc76f95dcc1d72290e4ac7 Mon Sep 17 00:00:00 2001 From: Alexander Tkachev Date: Fri, 27 May 2016 15:21:06 +0600 Subject: CLOUD: Refactor ConnectionManager/Requests system ConnectionManager now storages Request * (not generates ids for it), Requests have control on their RequestState, RequestIdPair is now called Response and storages Request * with some response together. All related classes are changed to use it in more clean and understandable way. Request, RequestState and Response are carefully commented/documented. --- backends/networking/curl/connectionmanager.cpp | 54 ++++-------- backends/networking/curl/connectionmanager.h | 30 ++----- backends/networking/curl/curljsonrequest.cpp | 23 ++++-- backends/networking/curl/curljsonrequest.h | 11 ++- backends/networking/curl/curlrequest.cpp | 8 +- backends/networking/curl/curlrequest.h | 16 +++- backends/networking/curl/networkreadstream.h | 2 +- backends/networking/curl/request.h | 110 ++++++++++++++++++++++--- 8 files changed, 160 insertions(+), 94 deletions(-) (limited to 'backends/networking/curl') diff --git a/backends/networking/curl/connectionmanager.cpp b/backends/networking/curl/connectionmanager.cpp index 9d88c59b25..ef2afc2655 100644 --- a/backends/networking/curl/connectionmanager.cpp +++ b/backends/networking/curl/connectionmanager.cpp @@ -34,7 +34,7 @@ DECLARE_SINGLETON(Networking::ConnectionManager); namespace Networking { -ConnectionManager::ConnectionManager(): _multi(0), _timerStarted(false), _nextId(0) { +ConnectionManager::ConnectionManager(): _multi(0), _timerStarted(false) { curl_global_init(CURL_GLOBAL_ALL); _multi = curl_multi_init(); } @@ -48,16 +48,10 @@ void ConnectionManager::registerEasyHandle(CURL *easy) { curl_multi_add_handle(_multi, easy); } -int32 ConnectionManager::addRequest(Request *request) { - int32 newId = _nextId++; - _requests[newId] = RequestInfo(newId, request); - request->setId(newId); +Request *ConnectionManager::addRequest(Request *request) { + _requests.push_back(request); if (!_timerStarted) startTimer(); - return newId; -} - -RequestInfo &ConnectionManager::getRequestInfo(int32 id) { - return _requests[id]; + return request; } //private goes here: @@ -91,36 +85,22 @@ void ConnectionManager::handle() { void ConnectionManager::interateRequests() { //call handle() of all running requests (so they can do their work) - debug("handling %d request(s)", _requests.size()); - Common::Array idsToRemove; - for (Common::HashMap::iterator i = _requests.begin(); i != _requests.end(); ++i) { - RequestInfo &info = _requests[i->_key]; + debug("handling %d request(s)", _requests.size()); + for (Common::Array::iterator i = _requests.begin(); i != _requests.end();) { + Request *request = *i; + if (!request || request->state() == FINISHED) { + delete (*i); + _requests.erase(i); + continue; + } - switch(info.state) { - case FINISHED: - delete info.request; - info.request = 0; - idsToRemove.push_back(info.id); - break; - - case PROCESSING: - info.request->handle(); - break; - - case RETRY: - if (info.retryInSeconds > 0) --info.retryInSeconds; - else { - info.state = PROCESSING; - info.request->restart(); - debug("request restarted"); - } - - default: - ; //nothing to do + if (request) { + if (request->state() == PROCESSING) request->handle(); + else if (request->state() == RETRY) request->handleRetry(); } + + ++i; } - for (uint32 i = 0; i < idsToRemove.size(); ++i) - _requests.erase(idsToRemove[i]); if (_requests.empty()) stopTimer(); } diff --git a/backends/networking/curl/connectionmanager.h b/backends/networking/curl/connectionmanager.h index 15327a28b2..2d37c0595c 100644 --- a/backends/networking/curl/connectionmanager.h +++ b/backends/networking/curl/connectionmanager.h @@ -36,30 +36,12 @@ namespace Networking { class NetworkReadStream; -enum RequestState { - PROCESSING, - PAUSED, - RETRY, - FINISHED -}; - -struct RequestInfo { - int32 id; - Request *request; - RequestState state; - uint32 retryInSeconds; - - RequestInfo() : id(-1), request(0), state(FINISHED), retryInSeconds(0) {} - RequestInfo(int32 rqId, Request *rq) : id(rqId), request(rq), state(PROCESSING), retryInSeconds(0) {} -}; - class ConnectionManager : public Common::Singleton { friend void connectionsThread(void *); //calls handle() CURLM *_multi; bool _timerStarted; - Common::HashMap _requests; - int32 _nextId; + Common::Array _requests; void startTimer(int interval = 1000000); //1 second is the default interval void stopTimer(); @@ -81,15 +63,15 @@ public: /** * Use this method to add new Request into manager's queue. * Manager will periodically call handle() method of these - * Requests until they return true. + * Requests until they set their state to FINISHED. + * + * If Request's state is RETRY, handleRetry() is called instead. * * @note This method starts the timer if it's not started yet. * - * @return generated Request's id, which might be used to get its status + * @return the same Request pointer, just as a shortcut */ - int32 addRequest(Request *request); - - RequestInfo &getRequestInfo(int32 id); + Request *addRequest(Request *request); }; /** Shortcut for accessing the connection manager. */ diff --git a/backends/networking/curl/curljsonrequest.cpp b/backends/networking/curl/curljsonrequest.cpp index 326d8e27a0..3c598d7f18 100644 --- a/backends/networking/curl/curljsonrequest.cpp +++ b/backends/networking/curl/curljsonrequest.cpp @@ -69,14 +69,11 @@ void CurlJsonRequest::handle() { if (_stream->httpResponseCode() != 200) warning("HTTP response code is not 200 OK (it's %ld)", _stream->httpResponseCode()); - ConnMan.getRequestInfo(_id).state = Networking::FINISHED; - if (_jsonCallback) { - char *contents = getPreparedContents(); - if (_stream->httpResponseCode() != 200) - debug("%s", contents); - Common::JSONValue *json = Common::JSON::parse(contents); - (*_jsonCallback)(RequestJsonPair(_id, json)); //potential memory leak, free it in your callbacks! - } + char *contents = getPreparedContents(); + if (_stream->httpResponseCode() != 200) + debug("%s", contents); + Common::JSONValue *json = Common::JSON::parse(contents); + finishJson(json); } } } @@ -88,4 +85,14 @@ void CurlJsonRequest::restart() { //with no stream available next handle() will create another one } +void CurlJsonRequest::finishJson(Common::JSONValue *json) { + Request::finish(); + if (_jsonCallback) (*_jsonCallback)(JsonResponse(this, json)); //potential memory leak, free it in your callbacks! + else delete json; +} + +void CurlJsonRequest::finish() { + finishJson(0); +} + } //end of namespace Networking diff --git a/backends/networking/curl/curljsonrequest.h b/backends/networking/curl/curljsonrequest.h index 5e78bd1965..0a560f93f4 100644 --- a/backends/networking/curl/curljsonrequest.h +++ b/backends/networking/curl/curljsonrequest.h @@ -29,10 +29,8 @@ namespace Networking { -class NetworkReadStream; - -typedef RequestIdPair RequestJsonPair; -typedef Common::BaseCallback *JsonCallback; +typedef Response JsonResponse; +typedef Common::BaseCallback *JsonCallback; class CurlJsonRequest: public CurlRequest { JsonCallback _jsonCallback; @@ -41,12 +39,17 @@ class CurlJsonRequest: public CurlRequest { /** Prepares raw bytes from _contentsStream to be parsed with Common::JSON::parse(). */ char *getPreparedContents(); +protected: + /** Sets FINISHED state and passes the JSONValue * into user's callback in JsonResponse. */ + virtual void finishJson(Common::JSONValue *json); + public: CurlJsonRequest(JsonCallback cb, Common::String url); virtual ~CurlJsonRequest(); virtual void handle(); virtual void restart(); + virtual void finish(); }; } //end of namespace Networking diff --git a/backends/networking/curl/curlrequest.cpp b/backends/networking/curl/curlrequest.cpp index f01a430b87..a8eb425412 100644 --- a/backends/networking/curl/curlrequest.cpp +++ b/backends/networking/curl/curlrequest.cpp @@ -40,10 +40,10 @@ CurlRequest::~CurlRequest() { void CurlRequest::handle() { if (!_stream) _stream = new NetworkReadStream(_url.c_str(), _headersList, _postFields); - if (_stream && _stream->eos()) { + if (_stream && _stream->eos()) { if (_stream->httpResponseCode() != 200) warning("HTTP response code is not 200 OK (it's %ld)", _stream->httpResponseCode()); - ConnMan.getRequestInfo(_id).state = Networking::FINISHED; + finish(); } } @@ -71,13 +71,13 @@ void CurlRequest::addPostField(Common::String keyValuePair) { _postFields += "&" + keyValuePair; } -Cloud::Storage::RequestReadStreamPair CurlRequest::execute() { +NetworkReadStreamResponse CurlRequest::execute() { if (!_stream) { _stream = new NetworkReadStream(_url.c_str(), _headersList, _postFields); ConnMan.addRequest(this); } - return Cloud::Storage::RequestReadStreamPair(_id, _stream); + return NetworkReadStreamResponse(this, _stream); } } //end of namespace Networking diff --git a/backends/networking/curl/curlrequest.h b/backends/networking/curl/curlrequest.h index 18a41a1c06..5677720b1d 100644 --- a/backends/networking/curl/curlrequest.h +++ b/backends/networking/curl/curlrequest.h @@ -24,7 +24,6 @@ #define BACKENDS_NETWORKING_CURL_CURLREQUEST_H #include "backends/networking/curl/request.h" -#include "backends/cloud/storage.h" #include "common/str.h" #include "common/array.h" @@ -34,6 +33,9 @@ namespace Networking { class NetworkReadStream; +typedef Response NetworkReadStreamResponse; +typedef Common::BaseCallback *NetworkReadStreamCallback; + class CurlRequest: public Request { protected: Common::String _url; @@ -48,12 +50,20 @@ public: virtual void handle(); virtual void restart(); + /** Replaces all headers with the passed array of headers. */ virtual void setHeaders(Common::Array &headers); + + /** Adds a header into headers list. */ virtual void addHeader(Common::String header); + + /** Adds a post field (key=value pair). */ virtual void addPostField(Common::String field); - /** Start this Request with ConnMan. Returns its ReadStream and request id. */ - virtual Cloud::Storage::RequestReadStreamPair execute(); + /** + * Starts this Request with ConnMan. + * @return its NetworkReadStream in NetworkReadStreamResponse. + */ + virtual NetworkReadStreamResponse execute(); }; } //end of namespace Networking diff --git a/backends/networking/curl/networkreadstream.h b/backends/networking/curl/networkreadstream.h index 6431a01fee..a0c87460cb 100644 --- a/backends/networking/curl/networkreadstream.h +++ b/backends/networking/curl/networkreadstream.h @@ -83,6 +83,6 @@ public: long httpResponseCode(); }; -} //end of namespace Cloud +} //end of namespace Networking #endif diff --git a/backends/networking/curl/request.h b/backends/networking/curl/request.h index d81fe903b8..ff919e02f1 100644 --- a/backends/networking/curl/request.h +++ b/backends/networking/curl/request.h @@ -28,40 +28,124 @@ namespace Networking { -template struct RequestIdPair { - int32 id; +class Request; + +/** +* Response is a struct to be returned from Request +* to user's callbacks. It's a type safe way to indicate +* which "return value" Request has and user awaits. +* +* It just keeps a Request pointer together with +* some T value (which might be a pointer, a reference +* or a plain type (copied by value)). +* +* To make it more convenient, typedefs are used. +* For example, Response is called DataResponse +* and corresponding callback pointer is DataCallback. +*/ + +template struct Response { + Request *request; T value; - RequestIdPair(int32 rid, T v) : id(rid), value(v) {} + Response(Request *rq, T v) : request(rq), value(v) {} }; -typedef RequestIdPair RequestDataPair; -typedef Common::BaseCallback *DataCallback; +typedef Response DataReponse; +typedef Common::BaseCallback *DataCallback; + +/** +* RequestState is used to indicate current Request state. +* ConnectionManager uses it to decide what to do with the Request. +* +* PROCESSING state indicates that Request is working. +* ConnectionManager calls handle() method of Requests in that state. +* +* PAUSED state indicates that Request is not working. +* ConnectionManager keeps Requests in that state and doesn't call any methods of those. +* +* RETRY state indicates that Request must restart after a few seconds. +* ConnectionManager calls handleRetry() method of Requests in that state. +* Default handleRetry() implementation decreases _retryInSeconds value +* until it reaches zero. When it does, Request's restart() method is called. +* +* FINISHED state indicates that Request did the work and might be deleted. +* ConnectionManager deletes Requests in that state. +* After this state is set, but before ConnectionManager deletes the Request, +* Request calls user's callback. User can ask Request to change its state +* by calling retry() or pause() methods and Request won't be deleted. +*/ + +enum RequestState { + PROCESSING, + PAUSED, + RETRY, + FINISHED +}; class Request { protected: /** - * Callback, which should be called before Request returns true in handle(). + * Callback, which should be called when Request is finished. * That's the way Requests pass the result to the code which asked to create this request. + * + * @note some Requests use their own callbacks to return something but void *. + * @note callback must be called in finish() or similar method. */ DataCallback _callback; - int32 _id; + /** + * Request state, which is used by ConnectionManager to determine + * whether request might be deleted or it's still working. + * + * State might be changed from outside with finish(), pause() or + * retry() methods. Override these if you want to react to these + * changes correctly. + */ + + RequestState _state; + + /** In RETRY state this indicates whether it's time to call restart(). */ + uint32 _retryInSeconds; public: - Request(DataCallback cb): _callback(cb), _id(-1) {} + Request(DataCallback cb): _callback(cb), _state(PROCESSING), _retryInSeconds(0) {} virtual ~Request() { delete _callback; } - /** - * Method, which does actual work. Depends on what this Request is doing. - */ - + /** Method, which does actual work. Depends on what this Request is doing. */ virtual void handle() = 0; + /** Method, which is called by ConnectionManager when Request's state is RETRY. */ + virtual void handleRetry() { + if (_retryInSeconds > 0) --_retryInSeconds; + else { + _state = PROCESSING; + restart(); + } + } + + /** Method, which is used to restart the Request. */ virtual void restart() = 0; - void setId(int32 id) { _id = id; } + /** Method, which is called to pause the Request. */ + virtual void pause() { _state = PAUSED; } + + /** + * Method, which is called to *interrupt* the Request. + * When it's called, Request must stop its work and + * call the callback to notify user of failure. + */ + virtual void finish() { _state = FINISHED; } + + /** Method, which is called to retry the Request. */ + virtual void retry(uint32 seconds) { + _state = RETRY; + _retryInSeconds = seconds; + } + + /** Returns Request's current state. */ + RequestState state() const { return _state; } }; } //end of namespace Cloud -- cgit v1.2.3