From e6e2de96caa26ce6081093ac7ffd621d59bcc085 Mon Sep 17 00:00:00 2001 From: Nils Maier Date: Mon, 18 Jul 2016 23:11:55 +0200 Subject: [PATCH 1/2] Better auto-renaming --- src/RequestGroup.cc | 10 +++++++++- src/RequestGroup.h | 4 ++-- test/RequestGroupTest.cc | 29 +++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index c89dacbf..8c18af66 100644 --- a/src/RequestGroup.cc +++ b/src/RequestGroup.cc @@ -778,8 +778,16 @@ void RequestGroup::tryAutoFileRenaming() fmt("File renaming failed: %s", getFirstFilePath().c_str()), error_code::FILE_RENAMING_FAILED); } + auto fn = filepath; + std::string ext; + auto idx = fn.find_last_of("."); + auto slash = fn.find_last_of("\\/"); + if (idx != std::string::npos && (slash == std::string::npos || slash < idx)) { + ext = fn.substr(idx); + fn = fn.substr(0, idx); + } for (int i = 1; i < 10000; ++i) { - auto newfilename = fmt("%s.%d", filepath.c_str(), i); + auto newfilename = fmt("%s.%d%s", fn.c_str(), i, ext.c_str()); File newfile(newfilename); File ctrlfile(newfile.getPath() + DefaultBtProgressInfoFile::getSuffix()); if (!newfile.exists() || (newfile.exists() && ctrlfile.exists())) { diff --git a/src/RequestGroup.h b/src/RequestGroup.h index 8b7862eb..6698f93d 100644 --- a/src/RequestGroup.h +++ b/src/RequestGroup.h @@ -199,8 +199,6 @@ private: void initializePostDownloadHandler(); - void tryAutoFileRenaming(); - // Returns the result code of this RequestGroup. If the download // finished, then returns error_code::FINISHED. If the // download didn't finish and error result is available in @@ -219,6 +217,8 @@ public: bool isCheckIntegrityReady(); + void tryAutoFileRenaming(); + const std::shared_ptr& getSegmentMan() const { return segmentMan_; diff --git a/test/RequestGroupTest.cc b/test/RequestGroupTest.cc index 47bc6e05..65b086bd 100644 --- a/test/RequestGroupTest.cc +++ b/test/RequestGroupTest.cc @@ -39,6 +39,35 @@ void RequestGroupTest::testGetFirstFilePath() CPPUNIT_ASSERT_EQUAL(std::string("/tmp/myfile"), group.getFirstFilePath()); + // test file renaming + option_->put(PREF_AUTO_FILE_RENAMING, "false"); + try { + group.tryAutoFileRenaming(); + } + catch (const Exception& ex) { + CPPUNIT_ASSERT_EQUAL(error_code::FILE_ALREADY_EXISTS, ex.getErrorCode()); + + } + + option_->put(PREF_AUTO_FILE_RENAMING, "true"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string("/tmp/myfile.1"), group.getFirstFilePath()); + + ctx->getFirstFileEntry()->setPath("/tmp/myfile.txt"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string("/tmp/myfile.1.txt"), group.getFirstFilePath()); + + ctx->getFirstFileEntry()->setPath("/tmp.txt/myfile"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string("/tmp.txt/myfile.1"), group.getFirstFilePath()); + + ctx->getFirstFileEntry()->setPath("/tmp.txt/myfile.txt"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string("/tmp.txt/myfile.1.txt"), group.getFirstFilePath()); + + // test in-memory + ctx->getFirstFileEntry()->setPath("/tmp/myfile"); + group.markInMemoryDownload(); CPPUNIT_ASSERT_EQUAL(std::string("[MEMORY]myfile"), group.getFirstFilePath()); From 67f556f208eb4c97af3143fe939120f3b2a9f318 Mon Sep 17 00:00:00 2001 From: Nils Maier Date: Tue, 19 Jul 2016 15:58:36 +0200 Subject: [PATCH 2/2] Address better renaming review comments and suggestions pt.1 --- src/RequestGroup.cc | 18 +++++++++++++++--- test/RequestGroupTest.cc | 37 ++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index 8c18af66..748b6fd4 100644 --- a/src/RequestGroup.cc +++ b/src/RequestGroup.cc @@ -780,9 +780,21 @@ void RequestGroup::tryAutoFileRenaming() } auto fn = filepath; std::string ext; - auto idx = fn.find_last_of("."); - auto slash = fn.find_last_of("\\/"); - if (idx != std::string::npos && (slash == std::string::npos || slash < idx)) { + const auto idx = fn.find_last_of("."); + const auto slash = fn.find_last_of("\\/"); + // Do extract the extension, as in "file.ext" = "file" and ".ext", + // but do not consider ".file" to be a file name without extension instead + // of a blank file name and an extension of ".file" + if (idx != std::string::npos && + // fn has no path component and starts with a dot, but has no extension + // otherwise + idx != 0 && + // has a file path component if we found a slash. + // if slash == idx - 1 this means a form of "*/.*", so the file name + // starts with a dot, has no extension otherwise, and therefore do not + // extract an extension either + (slash == std::string::npos || slash < idx - 1) + ) { ext = fn.substr(idx); fn = fn.substr(0, idx); } diff --git a/test/RequestGroupTest.cc b/test/RequestGroupTest.cc index 65b086bd..b00ebf60 100644 --- a/test/RequestGroupTest.cc +++ b/test/RequestGroupTest.cc @@ -14,6 +14,7 @@ class RequestGroupTest : public CppUnit::TestFixture { CPPUNIT_TEST_SUITE(RequestGroupTest); CPPUNIT_TEST(testGetFirstFilePath); + CPPUNIT_TEST(testTryAutoFileRenaming); CPPUNIT_TEST(testCreateDownloadResult); CPPUNIT_TEST_SUITE_END(); @@ -24,6 +25,7 @@ public: void setUp() { option_.reset(new Option()); } void testGetFirstFilePath(); + void testTryAutoFileRenaming(); void testCreateDownloadResult(); }; @@ -39,7 +41,22 @@ void RequestGroupTest::testGetFirstFilePath() CPPUNIT_ASSERT_EQUAL(std::string("/tmp/myfile"), group.getFirstFilePath()); - // test file renaming + // test in-memory + ctx->getFirstFileEntry()->setPath("/tmp/myfile"); + + group.markInMemoryDownload(); + + CPPUNIT_ASSERT_EQUAL(std::string("[MEMORY]myfile"), group.getFirstFilePath()); +} + +void RequestGroupTest::testTryAutoFileRenaming() +{ + std::shared_ptr ctx( + new DownloadContext(1_k, 1_k, "/tmp/myfile")); + + RequestGroup group(GroupId::create(), option_); + group.setDownloadContext(ctx); + option_->put(PREF_AUTO_FILE_RENAMING, "false"); try { group.tryAutoFileRenaming(); @@ -65,12 +82,22 @@ void RequestGroupTest::testGetFirstFilePath() group.tryAutoFileRenaming(); CPPUNIT_ASSERT_EQUAL(std::string("/tmp.txt/myfile.1.txt"), group.getFirstFilePath()); - // test in-memory - ctx->getFirstFileEntry()->setPath("/tmp/myfile"); + ctx->getFirstFileEntry()->setPath(".bashrc"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string(".bashrc.1"), group.getFirstFilePath()); - group.markInMemoryDownload(); + ctx->getFirstFileEntry()->setPath(".bashrc.txt"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string(".bashrc.1.txt"), group.getFirstFilePath()); + + ctx->getFirstFileEntry()->setPath("/tmp.txt/.bashrc"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string("/tmp.txt/.bashrc.1"), group.getFirstFilePath()); + + ctx->getFirstFileEntry()->setPath("/tmp.txt/.bashrc.txt"); + group.tryAutoFileRenaming(); + CPPUNIT_ASSERT_EQUAL(std::string("/tmp.txt/.bashrc.1.txt"), group.getFirstFilePath()); - CPPUNIT_ASSERT_EQUAL(std::string("[MEMORY]myfile"), group.getFirstFilePath()); } void RequestGroupTest::testCreateDownloadResult()