Przeglądaj źródła

Use std::unique_ptr for URISelector instead of std::shared_ptr

Tatsuhiro Tsujikawa 12 lat temu
rodzic
commit
a479473949

+ 1 - 1
src/CreateRequestCommand.cc

@@ -79,7 +79,7 @@ bool CreateRequestCommand::executeInternal()
     getDownloadEngine()->getRequestGroupMan()->getUsedHosts(usedHosts);
   }
   setRequest
-    (getFileEntry()->getRequest(getRequestGroup()->getURISelector(),
+    (getFileEntry()->getRequest(getRequestGroup()->getURISelector().get(),
                                 getOption()->getAsBool(PREF_REUSE_URI),
                                 usedHosts,
                                 getOption()->get(PREF_REFERER),

+ 1 - 1
src/FileEntry.cc

@@ -141,7 +141,7 @@ OutputIterator enumerateInFlightHosts
 
 std::shared_ptr<Request>
 FileEntry::getRequest
-(const std::shared_ptr<URISelector>& selector,
+(URISelector* selector,
  bool uriReuse,
  const std::vector<std::pair<size_t, std::string> >& usedHosts,
  const std::string& referer,

+ 1 - 1
src/FileEntry.h

@@ -179,7 +179,7 @@ public:
   // are not be usable because maxConnectionPerServer_ limit, then
   // reuse used URIs and do selection again.
   std::shared_ptr<Request> getRequest
-  (const std::shared_ptr<URISelector>& selector,
+  (URISelector* selector,
    bool uriReuse,
    const std::vector<std::pair<size_t, std::string> >& usedHosts,
    const std::string& referer = A2STR::NIL,

+ 3 - 3
src/RequestGroup.cc

@@ -136,7 +136,7 @@ RequestGroup::RequestGroup(const std::shared_ptr<GroupId>& gid,
     forceHaltRequested_(false),
     haltReason_(RequestGroup::NONE),
     pauseRequested_(false),
-    uriSelector_(new InorderURISelector()),
+    uriSelector_(make_unique<InorderURISelector>()),
     lastModifiedTime_(Time::null()),
     fileNotFoundCount_(0),
     timeout_(option->getAsInt(PREF_TIMEOUT)),
@@ -1212,9 +1212,9 @@ void RequestGroup::reportDownloadFinished()
 #endif // ENABLE_BITTORRENT
 }
 
-void RequestGroup::setURISelector(const std::shared_ptr<URISelector>& uriSelector)
+void RequestGroup::setURISelector(std::unique_ptr<URISelector> uriSelector)
 {
-  uriSelector_ = uriSelector;
+  uriSelector_ = std::move(uriSelector);
 }
 
 void RequestGroup::applyLastModifiedTimeToLocalFiles()

+ 3 - 3
src/RequestGroup.h

@@ -135,7 +135,7 @@ private:
 
   std::vector<std::shared_ptr<PostDownloadHandler> > postDownloadHandlers_;
 
-  std::shared_ptr<URISelector> uriSelector_;
+  std::unique_ptr<URISelector> uriSelector_;
 
   Time lastModifiedTime_;
 
@@ -414,9 +414,9 @@ public:
 
   void reportDownloadFinished();
 
-  void setURISelector(const std::shared_ptr<URISelector>& uriSelector);
+  void setURISelector(std::unique_ptr<URISelector> uriSelector);
 
-  const std::shared_ptr<URISelector>& getURISelector() const
+  const std::unique_ptr<URISelector>& getURISelector() const
   {
     return uriSelector_;
   }

+ 5 - 7
src/RequestGroupMan.cc

@@ -424,16 +424,14 @@ void RequestGroupMan::configureRequestGroup
 {
   const std::string& uriSelectorValue =
     requestGroup->getOption()->get(PREF_URI_SELECTOR);
-  std::shared_ptr<URISelector> sel;
   if(uriSelectorValue == V_FEEDBACK) {
-    sel.reset(new FeedbackURISelector(serverStatMan_));
+    requestGroup->setURISelector(make_unique<FeedbackURISelector>
+                                 (serverStatMan_));
   } else if(uriSelectorValue == V_INORDER) {
-    sel.reset(new InorderURISelector());
+    requestGroup->setURISelector(make_unique<InorderURISelector>());
   } else if(uriSelectorValue == V_ADAPTIVE) {
-    sel.reset(new AdaptiveURISelector(serverStatMan_, requestGroup.get()));
-  }
-  if(sel) {
-    requestGroup->setURISelector(sel);
+    requestGroup->setURISelector(make_unique<AdaptiveURISelector>
+                                 (serverStatMan_, requestGroup.get()));
   }
 }
 

+ 52 - 68
test/FileEntryTest.cc

@@ -46,18 +46,18 @@ CPPUNIT_TEST_SUITE_REGISTRATION( FileEntryTest );
 namespace {
 std::shared_ptr<FileEntry> createFileEntry()
 {
-  const char* uris[] = { "http://localhost/aria2.zip",
-                         "ftp://localhost/aria2.zip",
-                         "http://mirror/aria2.zip" };
-  std::shared_ptr<FileEntry> fileEntry(new FileEntry());
-  fileEntry->setUris(std::vector<std::string>(&uris[0], &uris[3]));
+  auto fileEntry = std::make_shared<FileEntry>();
+  fileEntry->setUris(std::vector<std::string>{
+      "http://localhost/aria2.zip",
+      "ftp://localhost/aria2.zip",
+        "http://mirror/aria2.zip" });
   return fileEntry;
 }
 } // namespace
 
 void FileEntryTest::testRemoveURIWhoseHostnameIs()
 {
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
+  auto fileEntry = createFileEntry();
   fileEntry->removeURIWhoseHostnameIs("localhost");
   CPPUNIT_ASSERT_EQUAL((size_t)1, fileEntry->getRemainingUris().size());
   CPPUNIT_ASSERT_EQUAL(std::string("http://mirror/aria2.zip"),
@@ -94,89 +94,76 @@ void FileEntryTest::testExtractURIResult()
 
 void FileEntryTest::testGetRequest()
 {
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
-  std::vector<std::pair<size_t, std::string> > usedHosts;
-  std::shared_ptr<Request> req =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto fileEntry = createFileEntry();
+  InorderURISelector selector{};
+  std::vector<std::pair<size_t, std::string>> usedHosts;
+  auto req = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("localhost"), req->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req->getProtocol());
   fileEntry->poolRequest(req);
 
-  std::shared_ptr<Request> req2nd =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req2nd = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("localhost"), req2nd->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req2nd->getProtocol());
 
-  std::shared_ptr<Request> req3rd =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req3rd = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("mirror"), req3rd->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req3rd->getProtocol());
 
-  std::shared_ptr<Request> req4th =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req4th = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT(!req4th);
 
   fileEntry->setMaxConnectionPerServer(2);
 
-  std::shared_ptr<Request> req5th =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req5th = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("localhost"), req5th->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("ftp"), req5th->getProtocol());
 
-  std::shared_ptr<Request> req6th =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req6th = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("mirror"), req6th->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req6th->getProtocol());
 
-  std::shared_ptr<Request> req7th =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req7th = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT(!req7th);
 }
 
 void FileEntryTest::testGetRequest_withoutUriReuse()
 {
-  std::vector<std::pair<size_t, std::string> > usedHosts;
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
+  std::vector<std::pair<size_t, std::string>> usedHosts;
+  auto fileEntry = createFileEntry();
   fileEntry->setMaxConnectionPerServer(2);
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
-  std::shared_ptr<Request> req = fileEntry->getRequest(selector, false, usedHosts);
+  InorderURISelector selector{};
+  auto req = fileEntry->getRequest(&selector, false, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("localhost"), req->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req->getProtocol());
 
-  std::shared_ptr<Request> req2nd =
-    fileEntry->getRequest(selector, false, usedHosts);
+  auto req2nd = fileEntry->getRequest(&selector, false, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("localhost"), req2nd->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("ftp"), req2nd->getProtocol());
 
-  std::shared_ptr<Request> req3rd =
-    fileEntry->getRequest(selector, false, usedHosts);
+  auto req3rd = fileEntry->getRequest(&selector, false, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("mirror"), req3rd->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req3rd->getProtocol());
 
-  std::shared_ptr<Request> req4th =
-    fileEntry->getRequest(selector, false, usedHosts);
+  auto req4th = fileEntry->getRequest(&selector, false, usedHosts);
   CPPUNIT_ASSERT(!req4th);
 }
 
 void FileEntryTest::testGetRequest_withUniqueProtocol()
 {
-  std::vector<std::pair<size_t, std::string> > usedHosts;
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
+  std::vector<std::pair<size_t, std::string>> usedHosts;
+  auto fileEntry = createFileEntry();
   fileEntry->setUniqueProtocol(true);
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
-  std::shared_ptr<Request> req =
-    fileEntry->getRequest(selector, true, usedHosts);
+  InorderURISelector selector{};
+  auto req = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("localhost"), req->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req->getProtocol());
 
-  std::shared_ptr<Request> req2nd =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req2nd = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT_EQUAL(std::string("mirror"), req2nd->getHost());
   CPPUNIT_ASSERT_EQUAL(std::string("http"), req2nd->getProtocol());
 
-  std::shared_ptr<Request> req3rd =
-    fileEntry->getRequest(selector, true, usedHosts);
+  auto req3rd = fileEntry->getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT(!req3rd);
 
   CPPUNIT_ASSERT_EQUAL((size_t)2, fileEntry->getRemainingUris().size());
@@ -188,26 +175,26 @@ void FileEntryTest::testGetRequest_withUniqueProtocol()
 
 void FileEntryTest::testGetRequest_withReferer()
 {
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
-  std::vector<std::pair<size_t, std::string> > usedHosts;
-  std::shared_ptr<Request> req =
-    fileEntry->getRequest(selector, true, usedHosts, "http://referer");
+  auto fileEntry = createFileEntry();
+  InorderURISelector selector{};
+  std::vector<std::pair<size_t, std::string>> usedHosts;
+  auto req = fileEntry->getRequest(&selector, true, usedHosts,
+                                   "http://referer");
   CPPUNIT_ASSERT_EQUAL(std::string("http://referer"), req->getReferer());
   // URI is used as referer if "*" is given.
-  req = fileEntry->getRequest(selector, true, usedHosts, "*");
+  req = fileEntry->getRequest(&selector, true, usedHosts, "*");
   CPPUNIT_ASSERT_EQUAL(req->getUri(), req->getReferer());
 }
 
 void FileEntryTest::testReuseUri()
 {
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
+  InorderURISelector selector{};
+  auto fileEntry = createFileEntry();
   fileEntry->setMaxConnectionPerServer(3);
   size_t numUris = fileEntry->getRemainingUris().size();
-  std::vector<std::pair<size_t, std::string> > usedHosts;
+  std::vector<std::pair<size_t, std::string>> usedHosts;
   for(size_t i = 0; i < numUris; ++i) {
-    fileEntry->getRequest(selector, false, usedHosts);
+    fileEntry->getRequest(&selector, false, usedHosts);
   }
   CPPUNIT_ASSERT_EQUAL((size_t)0, fileEntry->getRemainingUris().size());
   fileEntry->addURIResult("http://localhost/aria2.zip",
@@ -215,11 +202,11 @@ void FileEntryTest::testReuseUri()
   std::vector<std::string> ignore;
   fileEntry->reuseUri(ignore);
   CPPUNIT_ASSERT_EQUAL((size_t)2, fileEntry->getRemainingUris().size());
-  std::deque<std::string> uris = fileEntry->getRemainingUris();
+  auto uris = fileEntry->getRemainingUris();
   CPPUNIT_ASSERT_EQUAL(std::string("ftp://localhost/aria2.zip"), uris[0]);
   CPPUNIT_ASSERT_EQUAL(std::string("http://mirror/aria2.zip"), uris[1]);
   for(size_t i = 0; i < 2; ++i) {
-    fileEntry->getRequest(selector, false, usedHosts);
+    fileEntry->getRequest(&selector, false, usedHosts);
   }
   CPPUNIT_ASSERT_EQUAL((size_t)0, fileEntry->getRemainingUris().size());
   ignore.clear();
@@ -260,7 +247,7 @@ void FileEntryTest::testInsertUri()
   CPPUNIT_ASSERT(file.insertUri("http://example.org/2", 0));
   CPPUNIT_ASSERT(file.insertUri("http://example.org/3", 1));
   CPPUNIT_ASSERT(file.insertUri("http://example.org/4", 5));
-  std::deque<std::string> uris = file.getRemainingUris();
+  auto& uris = file.getRemainingUris();
   CPPUNIT_ASSERT_EQUAL(std::string("http://example.org/2"), uris[0]);
   CPPUNIT_ASSERT_EQUAL(std::string("http://example.org/3"), uris[1]);
   CPPUNIT_ASSERT_EQUAL(std::string("http://example.org/1"), uris[2]);
@@ -278,8 +265,8 @@ void FileEntryTest::testInsertUri()
 
 void FileEntryTest::testRemoveUri()
 {
-  std::vector<std::pair<size_t, std::string> > usedHosts;
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
+  std::vector<std::pair<size_t, std::string>> usedHosts;
+  InorderURISelector selector{};
   FileEntry file;
   file.addUri("http://example.org/");
   CPPUNIT_ASSERT(file.removeUri("http://example.org/"));
@@ -287,8 +274,7 @@ void FileEntryTest::testRemoveUri()
   CPPUNIT_ASSERT(!file.removeUri("http://example.org/"));
 
   file.addUri("http://example.org/");
-  std::shared_ptr<Request> exampleOrgReq =
-    file.getRequest(selector, true, usedHosts);
+  auto exampleOrgReq = file.getRequest(&selector, true, usedHosts);
   CPPUNIT_ASSERT(!exampleOrgReq->removalRequested());
   CPPUNIT_ASSERT_EQUAL((size_t)1, file.getSpentUris().size());
   CPPUNIT_ASSERT(file.removeUri("http://example.org/"));
@@ -298,7 +284,7 @@ void FileEntryTest::testRemoveUri()
   CPPUNIT_ASSERT_EQUAL((size_t)0, file.countPooledRequest());
 
   file.addUri("http://example.org/");
-  exampleOrgReq = file.getRequest(selector, true, usedHosts);
+  exampleOrgReq = file.getRequest(&selector, true, usedHosts);
   file.poolRequest(exampleOrgReq);
   CPPUNIT_ASSERT_EQUAL((size_t)1, file.countPooledRequest());
   CPPUNIT_ASSERT(file.removeUri("http://example.org/"));
@@ -311,17 +297,15 @@ void FileEntryTest::testRemoveUri()
 
 void FileEntryTest::testPutBackRequest()
 {
-  std::shared_ptr<FileEntry> fileEntry = createFileEntry();
-  std::shared_ptr<InorderURISelector> selector(new InorderURISelector());
-  std::vector<std::pair<size_t, std::string> > usedHosts;
-  std::shared_ptr<Request> req1 =
-    fileEntry->getRequest(selector, false, usedHosts);
-  std::shared_ptr<Request> req2 =
-    fileEntry->getRequest(selector, false, usedHosts);
+  auto fileEntry = createFileEntry();
+  InorderURISelector selector{};
+  std::vector<std::pair<size_t, std::string>> usedHosts;
+  auto req1 = fileEntry->getRequest(&selector, false, usedHosts);
+  auto req2 = fileEntry->getRequest(&selector, false, usedHosts);
   CPPUNIT_ASSERT_EQUAL((size_t)1, fileEntry->getRemainingUris().size());
   fileEntry->poolRequest(req2);
   fileEntry->putBackRequest();
-  const std::deque<std::string>& uris = fileEntry->getRemainingUris();
+  auto& uris = fileEntry->getRemainingUris();
   CPPUNIT_ASSERT_EQUAL((size_t)3, uris.size());
   CPPUNIT_ASSERT_EQUAL(std::string("http://localhost/aria2.zip"), uris[0]);
   CPPUNIT_ASSERT_EQUAL(std::string("http://mirror/aria2.zip"), uris[1]);