CLOUD: Refactor Request

Added ErrorResponse and ErrorCallback. Each Request now has an
ErrorCallback, which should be called instead of usual callback in case
of failure.

6 files changed, 156 insertions, 43 deletions

diff --git a/backends/networking/curl/curljsonrequest.cpp b/backends/networking/curl/curljsonrequest.cpp
index fee0932129..df982bc814 100644
--- a/backends/networking/curl/curljsonrequest.cpp
+++ b/backends/networking/curl/curljsonrequest.cpp
@@ -31,8 +31,8 @@
 
 namespace Networking {
 
-CurlJsonRequest::CurlJsonRequest(JsonCallback cb, Common::String url):
-	CurlRequest(0, url), _jsonCallback(cb), _contentsStream(DisposeAfterUse::YES) {}
+CurlJsonRequest::CurlJsonRequest(JsonCallback cb, ErrorCallback ecb, Common::String url):
+	CurlRequest(nullptr, ecb, url), _jsonCallback(cb), _contentsStream(DisposeAfterUse::YES) {}
 
 CurlJsonRequest::~CurlJsonRequest() { delete _jsonCallback; }
 
@@ -65,34 +65,32 @@ void CurlJsonRequest::handle() {
 			if (_contentsStream.write(buf, readBytes) != readBytes)
 				warning("MemoryWriteStreamDynamic was unable to write all the bytes");
 
-		if (_stream->eos()) {			
-			if (_stream->httpResponseCode() != 200)
-				warning("HTTP response code is not 200 OK (it's %ld)", _stream->httpResponseCode());
-
+		if (_stream->eos()) {
 			char *contents = getPreparedContents();
-			if (_stream->httpResponseCode() != 200)
-				debug("%s", contents);
 			Common::JSONValue *json = Common::JSON::parse(contents);
-			finishJson(json);
+			if (json) {
+				finishSuccess(json); //it's JSON even if's not 200 OK? That's fine!..
+			} else {
+				if (_stream->httpResponseCode() == 200) //no JSON, but 200 OK? That's fine!..
+					finishSuccess(nullptr);
+				else
+					finishError(ErrorResponse(this, false, true, contents, _stream->httpResponseCode()));
+			}
 		}
 	}
 }
 
 void CurlJsonRequest::restart() {
 	if (_stream) delete _stream;
-	_stream = 0;
+	_stream = nullptr;
 	_contentsStream = Common::MemoryWriteStreamDynamic(DisposeAfterUse::YES);
 	//with no stream available next handle() will create another one
 }
 
-void CurlJsonRequest::finishJson(Common::JSONValue *json) {
-	Request::finish();
+void CurlJsonRequest::finishSuccess(Common::JSONValue *json) {
+	Request::finishSuccess();
 	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 5e08be24c9..83005a79d5 100644
--- a/backends/networking/curl/curljsonrequest.h
+++ b/backends/networking/curl/curljsonrequest.h
@@ -41,15 +41,14 @@ protected:
 	char *getPreparedContents();
 
 	/** Sets FINISHED state and passes the JSONValue * into user's callback in JsonResponse. */
-	virtual void finishJson(Common::JSONValue *json);
+	virtual void finishSuccess(Common::JSONValue *json);
 
 public:
-	CurlJsonRequest(JsonCallback cb, Common::String url);
+	CurlJsonRequest(JsonCallback cb, ErrorCallback ecb, 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 64f6c26fb9..861546b906 100644
--- a/backends/networking/curl/curlrequest.cpp
+++ b/backends/networking/curl/curlrequest.cpp
@@ -30,8 +30,8 @@
 
 namespace Networking {
 
-CurlRequest::CurlRequest(DataCallback cb, Common::String url):
-	Request(cb), _url(url), _stream(nullptr), _headersList(nullptr), _bytesBuffer(nullptr), _bytesBufferSize(0) {}
+CurlRequest::CurlRequest(DataCallback cb, ErrorCallback ecb, Common::String url):
+	Request(cb, ecb), _url(url), _stream(nullptr), _headersList(nullptr), _bytesBuffer(nullptr), _bytesBufferSize(0) {}
 
 CurlRequest::~CurlRequest() {
 	delete _stream;
@@ -49,9 +49,14 @@ void CurlRequest::handle() {
 	if (!_stream) _stream = makeStream();	
 
 	if (_stream && _stream->eos()) {
-		if (_stream->httpResponseCode() != 200)
+		if (_stream->httpResponseCode() != 200) {
 			warning("HTTP response code is not 200 OK (it's %ld)", _stream->httpResponseCode());
-		finish();
+			ErrorResponse error(this, false, true, "", _stream->httpResponseCode());
+			finishError(error);
+			return;
+		}
+
+		finishSuccess(); //note that this Request doesn't call its callback on success (that's because it has nothing to return)
 	}
 }
 
@@ -101,4 +106,6 @@ NetworkReadStreamResponse CurlRequest::execute() {
 	return NetworkReadStreamResponse(this, _stream);
 }
 
+const NetworkReadStream *CurlRequest::getNetworkReadStream() const { return _stream; }
+
 } // End of namespace Networking
diff --git a/backends/networking/curl/curlrequest.h b/backends/networking/curl/curlrequest.h
index 461f153b9d..660479e34a 100644
--- a/backends/networking/curl/curlrequest.h
+++ b/backends/networking/curl/curlrequest.h
@@ -48,7 +48,7 @@ protected:
 	virtual NetworkReadStream *makeStream();
 
 public:
-	CurlRequest(DataCallback cb, Common::String url);
+	CurlRequest(DataCallback cb, ErrorCallback ecb, Common::String url);
 	virtual ~CurlRequest();
 
 	virtual void handle();
@@ -73,7 +73,7 @@ public:
 	virtual NetworkReadStreamResponse execute();
 
 	/** Returns Request's NetworkReadStream. */
-	const NetworkReadStream *getNetworkReadStream() const { return _stream; }
+	const NetworkReadStream *getNetworkReadStream() const;
 };
 
 } // End of namespace Networking
diff --git a/backends/networking/curl/request.cpp b/backends/networking/curl/request.cpp
new file mode 100644
index 0000000000..d2f91586a0
--- /dev/null
+++ b/backends/networking/curl/request.cpp
@@ -0,0 +1,70 @@
+/* ScummVM - Graphic Adventure Engine
+*
+* ScummVM is the legal property of its developers, whose names
+* are too numerous to list here. Please refer to the COPYRIGHT
+* file distributed with this source distribution.
+*
+* This program is free software; you can redistribute it and/or
+* modify it under the terms of the GNU General Public License
+* as published by the Free Software Foundation; either version 2
+* of the License, or (at your option) any later version.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU General Public License for more details.
+*
+* You should have received a copy of the GNU General Public License
+* along with this program; if not, write to the Free Software
+* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+*
+*/
+
+#include "backends/networking/curl/request.h"
+
+namespace Networking {
+
+ErrorResponse::ErrorResponse(Request *rq):
+	request(rq), interrupted(false), failed(true), response(""), httpResponseCode(-1) {}
+
+ErrorResponse::ErrorResponse(Request *rq, bool interrupt, bool failure, Common::String resp, long httpCode):
+	request(rq), interrupted(interrupt), failed(failure), response(resp), httpResponseCode(httpCode) {}
+
+Request::Request(DataCallback cb, ErrorCallback ecb):
+	_callback(cb), _errorCallback(ecb), _state(PROCESSING), _retryInSeconds(0) {}
+
+Request::~Request() {
+	delete _callback;
+	delete _errorCallback;
+}
+
+void Request::handleRetry() {
+	if (_retryInSeconds > 0) --_retryInSeconds;
+	else {
+		_state = PROCESSING;
+		restart();
+	}
+}
+
+void Request::pause() { _state = PAUSED; }
+
+void Request::finish() {
+	ErrorResponse error(this, true, false, "", -1);
+	finishError(error);
+}
+
+void Request::retry(uint32 seconds) {
+	_state = RETRY;
+	_retryInSeconds = seconds;
+}
+
+RequestState Request::state() const { return _state; }
+
+void Request::finishError(ErrorResponse error) {
+	_state = FINISHED;
+	if (_errorCallback) (*_errorCallback)(error);
+}
+
+void Request::finishSuccess() { _state = FINISHED; }
+
+} // End of namespace Networking
diff --git a/backends/networking/curl/request.h b/backends/networking/curl/request.h
index a3c723d218..de5308fa52 100644
--- a/backends/networking/curl/request.h
+++ b/backends/networking/curl/request.h
@@ -25,6 +25,7 @@
 
 #include "common/callback.h"
 #include "common/scummsys.h"
+#include "common/str.h"
 
 namespace Networking {
 
@@ -51,8 +52,39 @@ template<typename T> struct Response {
 	Response(Request *rq, T v) : request(rq), value(v) {}
 };
 
+/**
+ * ErrorResponse is a struct to be returned from Request
+ * to user's failure callbacks.
+ *
+ * It keeps a Request pointer together with some useful
+ * information fields, which would explain why failure
+ * callback was called.
+ *
+ * <interrupted> flag is set when Request was interrupted,
+ * i.e. finished by user with finish() call.
+ *
+ * <failed> flag is set when Request has failed because of
+ * some error (bad server response, for example).
+ *
+ * <response> contains server's original response.
+ *
+ * <httpResponseCode> contains server's HTTP response code.
+ */
+
+struct ErrorResponse {
+	Request *request;
+	bool interrupted;
+	bool failed;
+	Common::String response;
+	long httpResponseCode;
+
+	ErrorResponse(Request *rq);
+	ErrorResponse(Request *rq, bool interrupt, bool failure, Common::String resp, long httpCode);
+};
+
 typedef Response<void *> DataReponse;
 typedef Common::BaseCallback<DataReponse> *DataCallback;
+typedef Common::BaseCallback<ErrorResponse> *ErrorCallback;
 
 /**
  * RequestState is used to indicate current Request state.
@@ -74,6 +106,9 @@ typedef Common::BaseCallback<DataReponse> *DataCallback;
  * 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.
+ *
+ * Request get a success and failure callbacks. Request must call one
+ * (and only one!) of these callbacks when it sets FINISHED state.
  */
 enum RequestState {
 	PROCESSING,
@@ -94,6 +129,13 @@ protected:
 	DataCallback _callback;
 
 	/**
+	* Callback, which should be called when Request is failed/interrupted.
+	* That's the way Requests pass error information to the code which asked to create this request.
+	* @note callback must be called in finish() or similar method.
+	*/
+	ErrorCallback _errorCallback;
+
+	/**
 	 * Request state, which is used by ConnectionManager to determine
 	 * whether request might be deleted or it's still working.
 	 *
@@ -106,45 +148,42 @@ protected:
 	/** In RETRY state this indicates whether it's time to call restart(). */
 	uint32 _retryInSeconds;
 
+	/** Sets FINISHED state and calls the _errorCallback with given error. */
+	virtual void finishError(ErrorResponse error);
+
+	/** Sets FINISHED state. Implementations might extend it if needed. */
+	virtual void finishSuccess();
+
 public:
-	Request(DataCallback cb): _callback(cb), _state(PROCESSING), _retryInSeconds(0) {}
-	virtual ~Request() { delete _callback; }
+	Request(DataCallback cb, ErrorCallback ecb);
+	virtual ~Request();
 
 	/** 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();
-		}
-	}
+	virtual void handleRetry();
 
 	/** Method, which is used to restart the Request. */
 	virtual void restart() = 0;
 
 	/** Method, which is called to pause the Request. */
-	virtual void pause() { _state = PAUSED; }
+	virtual void pause();
 
 	/**
 	 * 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.
+	 * call the failure callback to notify user.
 	 */
-	virtual void finish() { _state = FINISHED; }
+	virtual void finish();
 
 	/** Method, which is called to retry the Request. */
-	virtual void retry(uint32 seconds) {		
-		_state = RETRY;
-		_retryInSeconds = seconds;
-	}
+	virtual void retry(uint32 seconds);
 
 	/** Returns Request's current state. */
-	RequestState state() const { return _state; }
+	RequestState state() const;
 };
 
-} // End of namespace Cloud
+} // End of namespace Networking
 
 #endif