From 624167744d734ed51232b2c04a48fb5eed1ae3e9 Mon Sep 17 00:00:00 2001 From: Emil Axelsson Date: Sat, 23 Dec 2017 17:37:55 +0100 Subject: [PATCH 1/3] Report failure for http requests with response code different from 200 --- include/openspace/util/httprequest.h | 51 ++++++++++++++++++++++------ src/util/httprequest.cpp | 33 ++++++++++++++++-- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/include/openspace/util/httprequest.h b/include/openspace/util/httprequest.h index 1621b9a44a..33ebc36ee8 100644 --- a/include/openspace/util/httprequest.h +++ b/include/openspace/util/httprequest.h @@ -39,16 +39,27 @@ namespace openspace { namespace curlfunctions { - size_t writeCallback(char *ptr, - size_t size, - size_t nmemb, - void *userdata); + size_t writeCallback( + char *ptr, + size_t size, + size_t nmemb, + void *userdata + ); - int progressCallback(void *clientp, - int64_t dltotal, - int64_t dlnow, - int64_t ultotal, - int64_t ulnow); + int progressCallback( + void *clientp, + int64_t dltotal, + int64_t dlnow, + int64_t ultotal, + int64_t ulnow + ); + + size_t headerCallback( + char* ptr, + size_t size, + size_t nmemb, + void* userData + ); } // Synchronous http request @@ -57,7 +68,7 @@ public: enum class ReadyState : unsigned int { Unsent, Loading, - ReceivedHeaders, + ReceivedHeader, Fail, Success }; @@ -73,6 +84,11 @@ public: size_t size; }; + struct Header { + char* buffer; + size_t size; + }; + using ReadyStateChangeCallback = std::function; // ProgressCallback: Return non-zero value to cancel download. @@ -83,6 +99,11 @@ public: // the request will fail. using DataCallback = std::function; + // HeaderCallback: Return number of bytes successfully stored. + // If this does not match the data buffer sice reported in Header, + // the request will fail. + using HeaderCallback = std::function; + struct RequestOptions { int requestTimeoutSeconds; // 0 for no timeout }; @@ -90,6 +111,7 @@ public: HttpRequest(std::string url); void onReadyStateChange(ReadyStateChangeCallback cb); + void onHeader(HeaderCallback cb); void onProgress(ProgressCallback cb); void onData(DataCallback cb); @@ -103,6 +125,7 @@ private: ReadyStateChangeCallback _onReadyStateChange; ProgressCallback _onProgress; DataCallback _onData; + HeaderCallback _onHeader; ReadyState _readyState; Progress _progress; @@ -120,16 +143,24 @@ private: int64_t dlnow, int64_t ultotal, int64_t ulnow); + + friend size_t curlfunctions::headerCallback( + char *ptr, + size_t size, + size_t nmemb, + void *userdata); }; class HttpDownload { public: using ProgressCallback = std::function; + using HeaderCallback = std::function; HttpDownload(); HttpDownload(HttpDownload&& d) = default; virtual ~HttpDownload() = default; void onProgress(ProgressCallback progressCallback); + void onHeader(HeaderCallback headerCallback); bool hasStarted(); bool hasFailed(); bool hasSucceeded(); diff --git a/src/util/httprequest.cpp b/src/util/httprequest.cpp index 41bf672e71..be990b57ef 100644 --- a/src/util/httprequest.cpp +++ b/src/util/httprequest.cpp @@ -49,12 +49,21 @@ #endif namespace { - const char* _loggerCat = "HttpRequest"; + constexpr const long StatusCodeOk = 200; + constexpr const char* _loggerCat = "HttpRequest"; } namespace openspace { namespace curlfunctions { + +size_t headerCallback(char* ptr, size_t size, size_t nmemb, void* userData) { + HttpRequest* r = reinterpret_cast(userData); + size_t nBytes = r->_onHeader(HttpRequest::Header{ptr, size * nmemb}); + r->setReadyState(HttpRequest::ReadyState::ReceivedHeader); + return nBytes; +} + int progressCallback( void* userData, int64_t nTotalBytes, @@ -83,6 +92,7 @@ HttpRequest::HttpRequest(std::string url) , _onReadyStateChange([](ReadyState) {}) , _onProgress([](Progress) { return 0; }) , _onData([](Data d) { return d.size; }) + , _onHeader([](Header h) { return h.size; }) {} void HttpRequest::onReadyStateChange(ReadyStateChangeCallback cb) { @@ -97,6 +107,10 @@ void HttpRequest::onData(DataCallback cb) { _onData = std::move(cb); } +void HttpRequest::onHeader(HeaderCallback cb) { + _onHeader = std::move(cb); +} + void HttpRequest::perform(RequestOptions opt) { setReadyState(ReadyState::Loading); @@ -109,18 +123,31 @@ void HttpRequest::perform(RequestOptions opt) { curl_easy_setopt(curl, CURLOPT_URL, _url.c_str()); curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); + curl_easy_setopt(curl, CURLOPT_HEADERDATA, this); + curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, curlfunctions::headerCallback); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, this); curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, curlfunctions::writeCallback); curl_easy_setopt(curl, CURLOPT_XFERINFODATA, this); curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, curlfunctions::progressCallback); - + if (opt.requestTimeoutSeconds > 0) { curl_easy_setopt(curl, CURLOPT_TIMEOUT, opt.requestTimeoutSeconds); } CURLcode res = curl_easy_perform(curl); - setReadyState(res == CURLE_OK ? ReadyState::Success : ReadyState::Fail); + if (res == CURLE_OK) { + long responseCode; + curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &responseCode); + if (responseCode == StatusCodeOk) { + setReadyState(ReadyState::Success); + } else { + setReadyState(ReadyState::Fail); + } + } else { + setReadyState(ReadyState::Fail); + } curl_easy_cleanup(curl); } From fd2568fdabff5760b868090b4d2e9f15fc120565 Mon Sep 17 00:00:00 2001 From: Emil Axelsson Date: Sat, 23 Dec 2017 17:54:22 +0100 Subject: [PATCH 2/3] Let http synchronizations overwrite files --- include/openspace/util/httprequest.h | 17 +++++++++++--- modules/sync/syncs/httpsynchronization.cpp | 4 +++- src/util/httprequest.cpp | 27 +++++++++++++++------- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/include/openspace/util/httprequest.h b/include/openspace/util/httprequest.h index 33ebc36ee8..7c4cc2942d 100644 --- a/include/openspace/util/httprequest.h +++ b/include/openspace/util/httprequest.h @@ -212,8 +212,10 @@ private: class HttpFileDownload : public virtual HttpDownload { public: + using Overwrite = ghoul::Boolean; + HttpFileDownload() = default; - HttpFileDownload(std::string destination); + HttpFileDownload(std::string destination, Overwrite = Overwrite::No); HttpFileDownload(HttpFileDownload&& d) = default; virtual ~HttpFileDownload() = default; protected: @@ -224,6 +226,7 @@ protected: static std::mutex _directoryCreationMutex; private: std::string _destination; + bool _overwrite; std::ofstream _file; }; @@ -251,7 +254,11 @@ public: // Synchronous download to file class SyncHttpFileDownload : public SyncHttpDownload, public HttpFileDownload { public: - SyncHttpFileDownload(std::string url, std::string destinationPath); + SyncHttpFileDownload( + std::string url, + std::string destinationPath, + HttpFileDownload::Overwrite = Overwrite::No + ); virtual ~SyncHttpFileDownload() = default; }; @@ -265,7 +272,11 @@ public: // Asynchronous download to file class AsyncHttpFileDownload : public AsyncHttpDownload, public HttpFileDownload { public: - AsyncHttpFileDownload(std::string url, std::string destinationPath); + AsyncHttpFileDownload( + std::string url, + std::string destinationPath, + HttpFileDownload::Overwrite = Overwrite::No + ); virtual ~AsyncHttpFileDownload() = default; }; diff --git a/modules/sync/syncs/httpsynchronization.cpp b/modules/sync/syncs/httpsynchronization.cpp index aa1882dbb7..1fa36c8d2e 100644 --- a/modules/sync/syncs/httpsynchronization.cpp +++ b/modules/sync/syncs/httpsynchronization.cpp @@ -206,7 +206,9 @@ bool HttpSynchronization::trySyncFromUrl(std::string listUrl) { ghoul::filesystem::FileSystem::PathSeparator + filename; - downloads.push_back(std::make_unique(line, fileDestination)); + downloads.push_back(std::make_unique( + line, fileDestination, HttpFileDownload::Overwrite::Yes)); + auto& fileDownload = downloads.back(); ++nDownloads; diff --git a/src/util/httprequest.cpp b/src/util/httprequest.cpp index be990b57ef..675ccb2f81 100644 --- a/src/util/httprequest.cpp +++ b/src/util/httprequest.cpp @@ -306,12 +306,15 @@ size_t HttpMemoryDownload::handleData(HttpRequest::Data d) { return d.size; } -HttpFileDownload::HttpFileDownload(std::string destination) { - _destination = std::move(destination); -} +HttpFileDownload::HttpFileDownload( + std::string destination, + HttpFileDownload::Overwrite overwrite) + : _destination(std::move(destination)) + , _overwrite(overwrite) +{} bool HttpFileDownload::initDownload() { - if (FileSys.fileExists(_destination)) { + if (!_overwrite && FileSys.fileExists(_destination)) { LERROR("File " << _destination << " already exists!"); return false; } @@ -352,18 +355,26 @@ SyncHttpMemoryDownload::SyncHttpMemoryDownload(std::string url) , HttpMemoryDownload() {} -SyncHttpFileDownload::SyncHttpFileDownload(std::string url, std::string destinationPath) +SyncHttpFileDownload::SyncHttpFileDownload( + std::string url, + std::string destinationPath, + HttpFileDownload::Overwrite overwrite +) : SyncHttpDownload(std::move(url)) - , HttpFileDownload(std::move(destinationPath)) + , HttpFileDownload(std::move(destinationPath), overwrite) {} AsyncHttpMemoryDownload::AsyncHttpMemoryDownload(std::string url) : AsyncHttpDownload(std::move(url)) {} -AsyncHttpFileDownload::AsyncHttpFileDownload(std::string url, std::string destinationPath) +AsyncHttpFileDownload::AsyncHttpFileDownload( + std::string url, + std::string destinationPath, + HttpFileDownload::Overwrite overwrite +) : AsyncHttpDownload(std::move(url)) - , HttpFileDownload(std::move(destinationPath)) + , HttpFileDownload(std::move(destinationPath), overwrite) {} From 2fad096a284fdbd83a3f05f7ec442dd0697e51d5 Mon Sep 17 00:00:00 2001 From: Emil Axelsson Date: Sat, 23 Dec 2017 18:17:45 +0100 Subject: [PATCH 3/3] Add directory constant to asset table --- src/scene/assetloader.cpp | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/scene/assetloader.cpp b/src/scene/assetloader.cpp index cbc6c8ba1f..aa38f5aa34 100644 --- a/src/scene/assetloader.cpp +++ b/src/scene/assetloader.cpp @@ -39,26 +39,28 @@ #include "assetloader_lua.inl" namespace { - const char* AssetGlobalVariableName = "asset"; + constexpr const char* AssetGlobalVariableName = "asset"; - const char* RequireFunctionName = "require"; - const char* RequestFunctionName = "request"; - const char* ExportFunctionName = "export"; + constexpr const char* RequireFunctionName = "require"; + constexpr const char* RequestFunctionName = "request"; + constexpr const char* ExportFunctionName = "export"; - const char* SyncedResourceFunctionName = "syncedResource"; - const char* LocalResourceFunctionName = "localResource"; + constexpr const char* SyncedResourceFunctionName = "syncedResource"; + constexpr const char* LocalResourceFunctionName = "localResource"; - const char* OnInitializeFunctionName = "onInitialize"; - const char* OnDeinitializeFunctionName = "onDeinitialize"; + constexpr const char* OnInitializeFunctionName = "onInitialize"; + constexpr const char* OnDeinitializeFunctionName = "onDeinitialize"; - const char* ExportsTableName = "_exports"; - const char* AssetTableName = "_asset"; - const char* DependantsTableName = "_dependants"; + constexpr const char* DirectoryConstantName = "directory"; - const char* _loggerCat = "AssetLoader"; + constexpr const char* ExportsTableName = "_exports"; + constexpr const char* AssetTableName = "_asset"; + constexpr const char* DependantsTableName = "_dependants"; - const char* AssetFileSuffix = "asset"; - const char* SceneFileSuffix = "scene"; + constexpr const char* _loggerCat = "AssetLoader"; + + constexpr const char* AssetFileSuffix = "asset"; + constexpr const char* SceneFileSuffix = "scene"; enum class PathType : int { RelativeToAsset = 0, @@ -133,6 +135,7 @@ void AssetLoader::setUpAssetLuaTable(Asset* asset) { | |- export | |- onInitialize | |- onDeinitialize + | |- directory |- Dependants (table) where Dependency is a table: @@ -201,6 +204,11 @@ void AssetLoader::setUpAssetLuaTable(Asset* asset) { lua_pushcclosure(*_luaState, &assetloader::onDeinitialize, 1); lua_setfield(*_luaState, assetTableIndex, OnDeinitializeFunctionName); + // Register directory constant + // string directory + lua_pushstring(*_luaState, asset->assetDirectory().c_str()); + lua_setfield(*_luaState, assetTableIndex, DirectoryConstantName); + // Attach Asset table to AssetInfo table lua_setfield(*_luaState, assetInfoTableIndex, AssetTableName);