From 0bbc62ee1fbfd107a935131d5d93994e87865afa Mon Sep 17 00:00:00 2001 From: Ricardo Quesada Date: Fri, 7 Aug 2015 15:46:55 -0700 Subject: [PATCH] more tests... and works but there is huge bug. ProgData needs the position of DownloadUnits but does not guaranty the order. So in my next commit I"ll merge DownloadUnit with ProgressData --- cocos/network/CCDownloader.cpp | 31 ++--- cocos/network/CCDownloader.h | 12 +- cocos/network/CCDownloaderImpl.cpp | 17 +-- .../Classes/DownloaderTest/DownloaderTest.cpp | 111 ++++++++++++++++++ .../Classes/DownloaderTest/DownloaderTest.h | 20 ++++ 5 files changed, 161 insertions(+), 30 deletions(-) diff --git a/cocos/network/CCDownloader.cpp b/cocos/network/CCDownloader.cpp index 708083afe9d9..15e514b097f3 100644 --- a/cocos/network/CCDownloader.cpp +++ b/cocos/network/CCDownloader.cpp @@ -121,10 +121,6 @@ std::string Downloader::getFileNameFromUrl(const std::string& srcUrl) void Downloader::clearBatchDownloadData() { _progDatas.clear(); - - for(const auto& fp: _files) - fclose(fp); - _files.clear(); } void Downloader::prepareDownload(const std::string& srcUrl, const std::string& storagePath, const std::string& customId, bool resumeDownload, FILE **fp, ProgressData *pData) @@ -156,6 +152,10 @@ void Downloader::prepareDownload(const std::string& srcUrl, const std::string& s *fp = nullptr; return; } + + // create possible subdirectories + if (!FileUtils::getInstance()->isDirectoryExist(pData->path)) + FileUtils::getInstance()->createDirectory(pData->path); // Create a file to save file. const std::string outFileName = storagePath + TEMP_EXT; @@ -273,6 +273,7 @@ void Downloader::downloadToFP(const std::string& srcUrl, const std::string& cust DownloadUnit unit; unit.srcUrl = srcUrl; unit.customId = customId; + unit.storagePath = storagePath; unit.fp = fp; _downloaderImpl->init(srcUrl); @@ -387,16 +388,13 @@ void Downloader::groupBatchDownload(const DownloadUnits& units) std::placeholders::_1, std::placeholders::_2, std::placeholders::_3); CC_ASSERT(_progDatas.size() ==0 && "_progsData must be 0"); - CC_ASSERT(_files.size() ==0 && "_files must be 0"); - _progDatas.resize(units.size()); - _files.resize(units.size()); int i=0; for (const auto& entry: units) { auto&& unit = entry.second; - prepareDownload(unit.srcUrl, unit.storagePath, unit.customId, unit.resumeDownload, &_files[i], &_progDatas[i]); + prepareDownload(unit.srcUrl, unit.storagePath, unit.customId, unit.resumeDownload, (FILE**)&unit.fp, &_progDatas[i]); i++; } _downloaderImpl->performBatchDownload(units, @@ -417,6 +415,14 @@ void Downloader::groupBatchDownload(const DownloadUnits& units) _fileUtils->renameFile(data.path, data.name + TEMP_EXT, data.name); } } + + // close opened FPs + for(const auto& entry: units) + { + const auto& unit = entry.second; + if (unit.fp) + fclose((FILE*)unit.fp); + } clearBatchDownloadData(); } @@ -425,8 +431,9 @@ void Downloader::groupBatchDownload(const DownloadUnits& units) size_t Downloader::fileWriteFunc(void *ptr, size_t size, size_t nmemb, void *userdata) { CC_ASSERT(userdata && "Invalid userdata"); - FILE *fp = (FILE*)((DownloadUnit*)userdata)->fp; - + DownloadUnit* unit = (DownloadUnit*)userdata; + FILE *fp = (FILE*)(unit->fp); + CC_ASSERT(fp && "Invalid FP"); size_t written = fwrite(ptr, size, nmemb, fp); return written; @@ -477,8 +484,6 @@ void Downloader::reportProgressInProgress(double totalToDownload, double nowDown // This is only for batchDownload process, will notify file succeed event in progress function int Downloader::batchDownloadProgressFunc(void *userdata, double totalToDownload, double nowDownloaded) { - CCLOG("batchDownloadProgressFunc: %d", (int)((nowDownloaded/totalToDownload)*100)); - CC_ASSERT(userdata && "Invalid userdata"); ProgressData* ptr = (ProgressData*) userdata; @@ -537,8 +542,6 @@ int Downloader::downloadProgressFunc(void *userdata, double totalToDownload, dou { CC_ASSERT(userdata && "Invalid userdata"); - CCLOG("downloadProgressFunc: %d", (int)((nowDownloaded/totalToDownload)*100)); - ProgressData* ptr = (ProgressData*)userdata; if (ptr->totalToDownload == 0) { diff --git a/cocos/network/CCDownloader.h b/cocos/network/CCDownloader.h index 09bf34a62687..4db423d2039a 100644 --- a/cocos/network/CCDownloader.h +++ b/cocos/network/CCDownloader.h @@ -140,23 +140,17 @@ class CC_EX_DLL Downloader : public std::enable_shared_from_this private: - int _connectionTimeout; + std::string getFileNameFromUrl(const std::string& srcUrl); + void clearBatchDownloadData(); ErrorCallback _onError; ProgressCallback _onProgress; SuccessCallback _onSuccess; - std::string getFileNameFromUrl(const std::string& srcUrl); - - void clearBatchDownloadData(); - - std::vector _files; + int _connectionTimeout; ProgressDatas _progDatas; - FileUtils* _fileUtils; - bool _supportResuming; - DownloaderImpl* _downloaderImpl; }; diff --git a/cocos/network/CCDownloaderImpl.cpp b/cocos/network/CCDownloaderImpl.cpp index b800e3f15e8b..063dbfb6baf9 100644 --- a/cocos/network/CCDownloaderImpl.cpp +++ b/cocos/network/CCDownloaderImpl.cpp @@ -45,7 +45,8 @@ static size_t _fileWriteFunc(void *ptr, size_t size, size_t nmemb, void* userdat { DownloadUnit *unit = (DownloadUnit*)userdata; DownloaderImpl* this_ = (DownloaderImpl*)unit->__reserved; - return this_->getWriterCallback()(ptr, size, nmemb, unit); + int ret = this_->getWriterCallback()(ptr, size, nmemb, unit); + return ret; } static int _downloadProgressFunc(void* userdata, double totalToDownload, double nowDownloaded, double totalToUpLoad, double nowUpLoaded) @@ -54,6 +55,8 @@ static int _downloadProgressFunc(void* userdata, double totalToDownload, double DownloaderImpl* this_ = (DownloaderImpl*)progressData->__reserved; this_->getProgressCallback()(progressData, totalToDownload, nowDownloaded); + + // must return 0, otherwise download will get cancelled return 0; } @@ -150,12 +153,12 @@ int DownloaderImpl::performBatchDownload(const DownloadUnits& units, int i=0; for (const auto& unitEntry: units) { - // HACK: Needed for callbacks. "this" + "unit" + "data" are needed - unitEntry.second.__reserved = this; - datas[i].__reserved = this; - const auto& unit = unitEntry.second; - const auto& data = datas[i]; + const ProgressData* data = &datas[i++]; + + // HACK: Needed for callbacks. "this" + "unit" + "data" are needed + unit.__reserved = this; + data->__reserved = this; if (unit.fp != NULL) { @@ -165,7 +168,7 @@ int DownloaderImpl::performBatchDownload(const DownloadUnits& units, curl_easy_setopt(curl, CURLOPT_WRITEDATA, &unit); curl_easy_setopt(curl, CURLOPT_NOPROGRESS, false); curl_easy_setopt(curl, CURLOPT_PROGRESSFUNCTION, _downloadProgressFunc); - curl_easy_setopt(curl, CURLOPT_PROGRESSDATA, &data); + curl_easy_setopt(curl, CURLOPT_PROGRESSDATA, data); curl_easy_setopt(curl, CURLOPT_FAILONERROR, true); if (_connectionTimeout) curl_easy_setopt(_curlHandle, CURLOPT_CONNECTTIMEOUT, _connectionTimeout); diff --git a/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.cpp b/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.cpp index b2e8a5fb776d..8179a57805d5 100644 --- a/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.cpp +++ b/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.cpp @@ -35,6 +35,8 @@ DownloaderTests::DownloaderTests() { ADD_TEST_CASE(DownloaderSyncTest); ADD_TEST_CASE(DownloaderAsyncTest); + ADD_TEST_CASE(DownloaderBatchSyncTest); + ADD_TEST_CASE(DownloaderBatchAsyncTest); }; // @@ -140,5 +142,114 @@ std::string DownloaderAsyncTest::subtitle() const return "Async test"; } +//------------------------------------------------------------------ +// +// DownloaderBatchSyncTest +// +//------------------------------------------------------------------ +void DownloaderBatchSyncTest::onEnter() +{ + DownloaderBaseTest::onEnter(); + + auto menuItem = MenuItemFont::create("start download", [=](Ref* sender){ + + if (_downloader) + { + std::vector images = { + "http://www.cocos2d-x.org/attachments/802/cocos2dx_landscape.png", + "http://www.cocos2d-x.org/docs/manual/framework/native/wiki/logo-resources-of-cocos2d-x/res/2dx_icon_512_rounded.png", + "http://www.cocos2d-x.org/attachments/1503/Cocos2CoordinateRelease.png" + }; + + std::vector names = { + "cocos2dx_landscape.png", + "2dx_icon_512_rounded.png", + "Cocos2CoordinateRelease.png" + }; + + network::DownloadUnits units; + + int i=0; + for(const auto& image: images) + { + network::DownloadUnit unit; + unit.storagePath = FileUtils::getInstance()->getWritablePath() + "CppTests/DownloaderTest/" + names[i]; + unit.srcUrl = image; + unit.customId = image; + i++; + units[image] = unit; + } + + _downloader->batchDownloadSync(units, "sync_download"); + cocos2d::log("Downloading..."); + } + }); + auto menu = Menu::create(menuItem, nullptr); + addChild(menu); + menu->setNormalizedPosition(Vec2(0.5, 0.5)); +} + +std::string DownloaderBatchSyncTest::title() const +{ + return "Downloader"; +} + +std::string DownloaderBatchSyncTest::subtitle() const +{ + return "Batch Sync test"; +} + + +//------------------------------------------------------------------ +// +// DownloaderBatchAsyncTest +// +//------------------------------------------------------------------ +void DownloaderBatchAsyncTest::onEnter() +{ + DownloaderBaseTest::onEnter(); + + auto menuItem = MenuItemFont::create("start download", [=](Ref* sender){ + + if (_downloader) + { + std::vector images = { + "http://www.cocos2d-x.org/attachments/802/cocos2dx_landscape.png", + "http://www.cocos2d-x.org/docs/manual/framework/native/wiki/logo-resources-of-cocos2d-x/res/2dx_icon_512_rounded.png", + "http://www.cocos2d-x.org/attachments/1503/Cocos2CoordinateRelease.png" + }; + + network::DownloadUnits units; + + int i=0; + for(const auto& image: images) + { + network::DownloadUnit unit; + unit.storagePath = FileUtils::getInstance()->getWritablePath(); + unit.srcUrl = image; + unit.customId = image; + i++; + units[image] = unit; + } + + _downloader->batchDownloadAsync(units, "sync_download"); + cocos2d::log("Downloading..."); + } + }); + auto menu = Menu::create(menuItem, nullptr); + addChild(menu); + menu->setNormalizedPosition(Vec2(0.5, 0.5)); +} + +std::string DownloaderBatchAsyncTest::title() const +{ + return "Downloader"; +} + +std::string DownloaderBatchAsyncTest::subtitle() const +{ + return "Batch Async test"; +} + diff --git a/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.h b/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.h index 66b0062446f6..b7063822c2ee 100644 --- a/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.h +++ b/tests/cpp-tests/Classes/DownloaderTest/DownloaderTest.h @@ -69,3 +69,23 @@ class DownloaderAsyncTest : public DownloaderBaseTest virtual std::string subtitle() const override; }; +class DownloaderBatchSyncTest : public DownloaderBaseTest +{ +public: + CREATE_FUNC(DownloaderBatchSyncTest); + + virtual void onEnter() override; + virtual std::string title() const override; + virtual std::string subtitle() const override; +}; + +class DownloaderBatchAsyncTest : public DownloaderBaseTest +{ +public: + CREATE_FUNC(DownloaderBatchAsyncTest); + + virtual void onEnter() override; + virtual std::string title() const override; + virtual std::string subtitle() const override; +}; +