Jelajahi Sumber

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

Tatsuhiro Tsujikawa 12 tahun lalu
induk
melakukan
cb205a207c

+ 2 - 2
src/AbstractProxyRequestCommand.cc

@@ -74,12 +74,12 @@ AbstractProxyRequestCommand::~AbstractProxyRequestCommand() {}
 bool AbstractProxyRequestCommand::executeInternal() {
   //socket->setBlockingMode();
   if(httpConnection_->sendBufferIsEmpty()) {
-    std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
+    auto httpRequest = make_unique<HttpRequest>();
     httpRequest->setUserAgent(getOption()->get(PREF_USER_AGENT));
     httpRequest->setRequest(getRequest());
     httpRequest->setProxyRequest(proxyRequest_);
 
-    httpConnection_->sendProxyRequest(httpRequest);
+    httpConnection_->sendProxyRequest(std::move(httpRequest));
   } else {
     httpConnection_->sendPendingData();
   }

+ 2 - 2
src/FtpNegotiationCommand.cc

@@ -734,7 +734,7 @@ bool FtpNegotiationCommand::sendTunnelRequest()
         }
       }
     }
-    std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
+    auto httpRequest = make_unique<HttpRequest>();
     httpRequest->setUserAgent(getOption()->get(PREF_USER_AGENT));
     std::shared_ptr<Request> req(new Request());
     // Construct fake URI in order to use HttpRequest
@@ -749,7 +749,7 @@ bool FtpNegotiationCommand::sendTunnelRequest()
     }
     httpRequest->setRequest(req);
     httpRequest->setProxyRequest(createProxyRequest());
-    http_->sendProxyRequest(httpRequest);
+    http_->sendProxyRequest(std::move(httpRequest));
   } else {
     http_->sendPendingData();
   }

+ 13 - 19
src/HttpConnection.cc

@@ -62,12 +62,15 @@
 namespace aria2 {
 
 HttpRequestEntry::HttpRequestEntry
-(const std::shared_ptr<HttpRequest>& httpRequest)
-  : httpRequest_{httpRequest},
+(std::unique_ptr<HttpRequest> httpRequest)
+  : httpRequest_{std::move(httpRequest)},
     proc_{make_unique<HttpHeaderProcessor>(HttpHeaderProcessor::CLIENT_PARSER)}
 {}
 
-HttpRequestEntry::~HttpRequestEntry() {}
+std::unique_ptr<HttpRequest> HttpRequestEntry::popHttpRequest()
+{
+  return std::move(httpRequest_);
+}
 
 const std::unique_ptr<HttpHeaderProcessor>&
 HttpRequestEntry::getHttpHeaderProcessor() const
@@ -106,7 +109,7 @@ std::string HttpConnection::eraseConfidentialInfo(const std::string& request)
 }
 
 void HttpConnection::sendRequest
-(const std::shared_ptr<HttpRequest>& httpRequest, std::string request)
+(std::unique_ptr<HttpRequest> httpRequest, std::string request)
 {
   A2_LOG_INFO(fmt(MSG_SENDING_REQUEST,
                   cuid_,
@@ -114,19 +117,19 @@ void HttpConnection::sendRequest
   socketBuffer_.pushStr(std::move(request));
   socketBuffer_.send();
   outstandingHttpRequests_.push_back(make_unique<HttpRequestEntry>
-                                     (httpRequest));
+                                     (std::move(httpRequest)));
 }
 
 void HttpConnection::sendRequest
-(const std::shared_ptr<HttpRequest>& httpRequest)
+(std::unique_ptr<HttpRequest> httpRequest)
 {
-  sendRequest(httpRequest, httpRequest->createRequest());
+  sendRequest(std::move(httpRequest), httpRequest->createRequest());
 }
 
 void HttpConnection::sendProxyRequest
-(const std::shared_ptr<HttpRequest>& httpRequest)
+(std::unique_ptr<HttpRequest> httpRequest)
 {
-  sendRequest(httpRequest, httpRequest->createProxyRequest());
+  sendRequest(std::move(httpRequest), httpRequest->createProxyRequest());
 }
 
 std::unique_ptr<HttpResponse> HttpConnection::receiveResponse()
@@ -150,7 +153,7 @@ std::unique_ptr<HttpResponse> HttpConnection::receiveResponse()
     httpResponse->setCuid(cuid_);
     httpResponse->setHttpHeader(proc->getResult());
     httpResponse->setHttpRequest(outstandingHttpRequests_.front()->
-                                 getHttpRequest());
+                                 popHttpRequest());
     socketRecvBuffer_->shiftBuffer(proc->getLastBytesProcessed());
     outstandingHttpRequests_.pop_front();
     return httpResponse;
@@ -170,15 +173,6 @@ bool HttpConnection::isIssued(const std::shared_ptr<Segment>& segment) const
   return false;
 }
 
-std::shared_ptr<HttpRequest> HttpConnection::getFirstHttpRequest() const
-{
-  if(outstandingHttpRequests_.empty()) {
-    return std::shared_ptr<HttpRequest>();
-  } else {
-    return outstandingHttpRequests_.front()->getHttpRequest();
-  }
-}
-
 bool HttpConnection::sendBufferIsEmpty() const
 {
   return socketBuffer_.sendBufferIsEmpty();

+ 8 - 10
src/HttpConnection.h

@@ -56,18 +56,18 @@ class SocketRecvBuffer;
 
 class HttpRequestEntry {
 private:
-  std::shared_ptr<HttpRequest> httpRequest_;
+  std::unique_ptr<HttpRequest> httpRequest_;
   std::unique_ptr<HttpHeaderProcessor> proc_;
 public:
-  HttpRequestEntry(const std::shared_ptr<HttpRequest>& httpRequest);
+  HttpRequestEntry(std::unique_ptr<HttpRequest> httpRequest);
 
-  ~HttpRequestEntry();
-
-  const std::shared_ptr<HttpRequest>& getHttpRequest() const
+  const std::unique_ptr<HttpRequest>& getHttpRequest() const
   {
     return httpRequest_;
   }
 
+  std::unique_ptr<HttpRequest> popHttpRequest();
+
   const std::unique_ptr<HttpHeaderProcessor>& getHttpHeaderProcessor() const;
 };
 
@@ -84,7 +84,7 @@ private:
 
   std::string eraseConfidentialInfo(const std::string& request);
   void sendRequest
-  (const std::shared_ptr<HttpRequest>& httpRequest, std::string request);
+  (std::unique_ptr<HttpRequest> httpRequest, std::string request);
 public:
   HttpConnection
   (cuid_t cuid,
@@ -99,12 +99,12 @@ public:
    * HTTP proxy(GET method).
    * @param segment indicates starting postion of the file for downloading
    */
-  void sendRequest(const std::shared_ptr<HttpRequest>& httpRequest);
+  void sendRequest(std::unique_ptr<HttpRequest> httpRequest);
 
   /**
    * Sends Http proxy request using CONNECT method.
    */
-  void sendProxyRequest(const std::shared_ptr<HttpRequest>& httpRequest);
+  void sendProxyRequest(std::unique_ptr<HttpRequest> httpRequest);
 
   /**
    * Receives HTTP response from the server and returns HttpResponseHandle
@@ -119,8 +119,6 @@ public:
    */
   std::unique_ptr<HttpResponse> receiveResponse();
 
-  std::shared_ptr<HttpRequest> getFirstHttpRequest() const;
-
   bool isIssued(const std::shared_ptr<Segment>& segment) const;
 
   bool sendBufferIsEmpty() const;

+ 19 - 25
src/HttpRequestCommand.cc

@@ -82,7 +82,7 @@ HttpRequestCommand::HttpRequestCommand
 HttpRequestCommand::~HttpRequestCommand() {}
 
 namespace {
-std::shared_ptr<HttpRequest>
+std::unique_ptr<HttpRequest>
 createHttpRequest(const std::shared_ptr<Request>& req,
                   const std::shared_ptr<FileEntry>& fileEntry,
                   const std::shared_ptr<Segment>& segment,
@@ -92,7 +92,7 @@ createHttpRequest(const std::shared_ptr<Request>& req,
                   const std::shared_ptr<Request>& proxyRequest,
                   int64_t endOffset = 0)
 {
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
+  auto httpRequest = make_unique<HttpRequest>();
   httpRequest->setUserAgent(option->get(PREF_USER_AGENT));
   httpRequest->setRequest(req);
   httpRequest->setFileEntry(fileEntry);
@@ -135,14 +135,13 @@ bool HttpRequestCommand::executeInternal() {
     }
 #endif // ENABLE_SSL
     if(getSegments().empty()) {
-      std::shared_ptr<HttpRequest> httpRequest
-        (createHttpRequest(getRequest(),
-                           getFileEntry(),
-                           std::shared_ptr<Segment>(),
-                           getOption(),
-                           getRequestGroup(),
-                           getDownloadEngine(),
-                           proxyRequest_));
+      auto httpRequest = createHttpRequest(getRequest(),
+                                           getFileEntry(),
+                                           std::shared_ptr<Segment>(),
+                                           getOption(),
+                                           getRequestGroup(),
+                                           getDownloadEngine(),
+                                           proxyRequest_);
       if(getOption()->getAsBool(PREF_CONDITIONAL_GET) &&
          (getRequest()->getProtocol() == "http" ||
           getRequest()->getProtocol() == "https")) {
@@ -167,12 +166,9 @@ bool HttpRequestCommand::executeInternal() {
           }
         }
       }
-      httpConnection_->sendRequest(httpRequest);
+      httpConnection_->sendRequest(std::move(httpRequest));
     } else {
-      for(std::vector<std::shared_ptr<Segment> >::const_iterator itr =
-            getSegments().begin(), eoi = getSegments().end();
-          itr != eoi; ++itr) {
-        const std::shared_ptr<Segment>& segment = *itr;
+      for(auto& segment : getSegments()) {
         if(!httpConnection_->isIssued(segment)) {
           int64_t endOffset = 0;
           // FTP via HTTP proxy does not support end byte marker
@@ -185,16 +181,14 @@ bool HttpRequestCommand::executeInternal() {
                getFileEntry()->gtoloff
                (static_cast<int64_t>(segment->getSegmentLength())*nextIndex));
           }
-          std::shared_ptr<HttpRequest> httpRequest
-            (createHttpRequest(getRequest(),
-                               getFileEntry(),
-                               segment,
-                               getOption(),
-                               getRequestGroup(),
-                               getDownloadEngine(),
-                               proxyRequest_,
-                               endOffset));
-          httpConnection_->sendRequest(httpRequest);
+          httpConnection_->sendRequest(createHttpRequest(getRequest(),
+                                                         getFileEntry(),
+                                                         segment,
+                                                         getOption(),
+                                                         getRequestGroup(),
+                                                         getDownloadEngine(),
+                                                         proxyRequest_,
+                                                         endOffset));
         }
       }
     }

+ 3 - 5
src/HttpResponse.cc

@@ -68,11 +68,9 @@
 namespace aria2 {
 
 HttpResponse::HttpResponse()
-  : cuid_(0)
+  : cuid_{0}
 {}
 
-HttpResponse::~HttpResponse() {}
-
 void HttpResponse::validateResponse() const
 {
   int statusCode = getStatusCode();
@@ -266,9 +264,9 @@ const std::unique_ptr<HttpHeader>& HttpResponse::getHttpHeader() const
   return httpHeader_;
 }
 
-void HttpResponse::setHttpRequest(const std::shared_ptr<HttpRequest>& httpRequest)
+void HttpResponse::setHttpRequest(std::unique_ptr<HttpRequest> httpRequest)
 {
-  httpRequest_ = httpRequest;
+  httpRequest_ = std::move(httpRequest);
 }
 
 int HttpResponse::getStatusCode() const

+ 3 - 5
src/HttpResponse.h

@@ -56,13 +56,11 @@ class Checksum;
 class HttpResponse {
 private:
   cuid_t cuid_;
-  std::shared_ptr<HttpRequest> httpRequest_;
+  std::unique_ptr<HttpRequest> httpRequest_;
   std::unique_ptr<HttpHeader> httpHeader_;
 public:
   HttpResponse();
 
-  ~HttpResponse();
-
   void validateResponse() const;
 
   /**
@@ -109,9 +107,9 @@ public:
 
   int getStatusCode() const;
 
-  void setHttpRequest(const std::shared_ptr<HttpRequest>& httpRequest);
+  void setHttpRequest(std::unique_ptr<HttpRequest> httpRequest);
 
-  const std::shared_ptr<HttpRequest>& getHttpRequest() const
+  const std::unique_ptr<HttpRequest>& getHttpRequest() const
   {
     return httpRequest_;
   }

+ 0 - 1
src/HttpResponseCommand.cc

@@ -151,7 +151,6 @@ HttpResponseCommand::~HttpResponseCommand() {}
 
 bool HttpResponseCommand::executeInternal()
 {
-  std::shared_ptr<HttpRequest> httpRequest =httpConnection_->getFirstHttpRequest();
   auto httpResponse = httpConnection_->receiveResponse();
   if(!httpResponse) {
     // The server has not responded to our request yet.

+ 41 - 39
test/HttpResponseTest.cc

@@ -146,13 +146,13 @@ void HttpResponseTest::testGetContentType()
 void HttpResponseTest::testDeterminFilename_without_ContentDisposition()
 {
   HttpResponse httpResponse;
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Request> request(new Request());
+  auto httpRequest = make_unique<HttpRequest>();
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
 
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
 
   CPPUNIT_ASSERT_EQUAL(std::string("aria2-1.0.0.tar.bz2"),
                        httpResponse.determinFilename());
@@ -164,13 +164,13 @@ void HttpResponseTest::testDeterminFilename_with_ContentDisposition_zero_length
   HttpResponse httpResponse;
   auto httpHeader = make_unique<HttpHeader>();
   httpHeader->put(HttpHeader::CONTENT_DISPOSITION, "attachment; filename=\"\"");
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Request> request(new Request());
+  auto httpRequest = make_unique<HttpRequest>();
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
 
   httpResponse.setHttpHeader(std::move(httpHeader));
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
 
   CPPUNIT_ASSERT_EQUAL(std::string("aria2-1.0.0.tar.bz2"),
                        httpResponse.determinFilename());
@@ -182,13 +182,13 @@ void HttpResponseTest::testDeterminFilename_with_ContentDisposition()
   auto httpHeader = make_unique<HttpHeader>();
   httpHeader->put(HttpHeader::CONTENT_DISPOSITION,
                   "attachment; filename=\"aria2-current.tar.bz2\"");
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Request> request(new Request());
+  auto httpRequest = make_unique<HttpRequest>();
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
 
   httpResponse.setHttpHeader(std::move(httpHeader));
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
 
   CPPUNIT_ASSERT_EQUAL(std::string("aria2-current.tar.bz2"),
                        httpResponse.determinFilename());
@@ -351,16 +351,16 @@ void HttpResponseTest::testValidateResponse_good_range()
 
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
 
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Piece> p(new Piece(1, 1024*1024));
-  std::shared_ptr<Segment> segment(new PiecedSegment(1024*1024, p));
+  auto httpRequest = make_unique<HttpRequest>();
+  auto p = std::make_shared<Piece>(1, 1024*1024);
+  auto segment = std::make_shared<PiecedSegment>(1024*1024, p);
   httpRequest->setSegment(segment);
-  std::shared_ptr<FileEntry> fileEntry(new FileEntry("file", 1024*1024*10, 0));
+  auto fileEntry = std::make_shared<FileEntry>("file", 1024*1024*10, 0);
   httpRequest->setFileEntry(fileEntry);
-  std::shared_ptr<Request> request(new Request());
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
   httpResponse.getHttpHeader()->setStatusCode(206);
   httpResponse.getHttpHeader()->put(HttpHeader::CONTENT_RANGE,
                                     "bytes 1048576-10485760/10485760");
@@ -379,16 +379,16 @@ void HttpResponseTest::testValidateResponse_bad_range()
 
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
 
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Piece> p(new Piece(1, 1024*1024));
-  std::shared_ptr<Segment> segment(new PiecedSegment(1024*1024, p));
+  auto httpRequest = make_unique<HttpRequest>();
+  auto p = std::make_shared<Piece>(1, 1024*1024);
+  auto segment = std::make_shared<PiecedSegment>(1024*1024, p);
   httpRequest->setSegment(segment);
-  std::shared_ptr<FileEntry> fileEntry(new FileEntry("file", 1024*1024*10, 0));
+  auto fileEntry = std::make_shared<FileEntry>("file", 1024*1024*10, 0);
   httpRequest->setFileEntry(fileEntry);
-  std::shared_ptr<Request> request(new Request());
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
   httpResponse.getHttpHeader()->setStatusCode(206);
   httpResponse.getHttpHeader()->put(HttpHeader::CONTENT_RANGE,
                                     "bytes 0-10485760/10485761");
@@ -405,16 +405,16 @@ void HttpResponseTest::testValidateResponse_chunked()
   HttpResponse httpResponse;
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
 
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Piece> p(new Piece(1, 1024*1024));
-  std::shared_ptr<Segment> segment(new PiecedSegment(1024*1024, p));
+  auto httpRequest = make_unique<HttpRequest>();
+  auto p = std::make_shared<Piece>(1, 1024*1024);
+  auto segment = std::make_shared<PiecedSegment>(1024*1024, p);
   httpRequest->setSegment(segment);
-  std::shared_ptr<FileEntry> fileEntry(new FileEntry("file", 1024*1024*10, 0));
+  auto fileEntry = std::make_shared<FileEntry>("file", 1024*1024*10, 0);
   httpRequest->setFileEntry(fileEntry);
-  std::shared_ptr<Request> request(new Request());
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
   httpResponse.getHttpHeader()->setStatusCode(206);
   httpResponse.getHttpHeader()->put(HttpHeader::CONTENT_RANGE,
                                     "bytes 0-10485760/10485761");
@@ -433,14 +433,16 @@ void HttpResponseTest::testValidateResponse_withIfModifiedSince()
   HttpResponse httpResponse;
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
   httpResponse.getHttpHeader()->setStatusCode(304);
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  httpResponse.setHttpRequest(httpRequest);
+  auto httpRequest = make_unique<HttpRequest>();
+  httpResponse.setHttpRequest(std::move(httpRequest));
   try {
     httpResponse.validateResponse();
     CPPUNIT_FAIL("exception must be thrown.");
   } catch(Exception& e) {
   }
+  httpRequest = make_unique<HttpRequest>();
   httpRequest->setIfModifiedSinceHeader("Fri, 16 Jul 2010 12:56:59 GMT");
+  httpResponse.setHttpRequest(std::move(httpRequest));
   httpResponse.validateResponse();
 }
 
@@ -450,11 +452,11 @@ void HttpResponseTest::testProcessRedirect()
 
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
 
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Request> request(new Request());
+  auto httpRequest = make_unique<HttpRequest>();
+  auto request = std::make_shared<Request>();
   request->setUri("http://localhost/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
 
   httpResponse.getHttpHeader()->put(HttpHeader::LOCATION,
                                     "http://mirror/aria2-1.0.0.tar.bz2");
@@ -490,13 +492,13 @@ void HttpResponseTest::testRetrieveCookie()
 
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
 
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  std::shared_ptr<Request> request(new Request());
+  auto httpRequest = make_unique<HttpRequest>();
+  auto request = std::make_shared<Request>();
   request->setUri("http://www.aria2.org/archives/aria2-1.0.0.tar.bz2");
   httpRequest->setRequest(request);
   CookieStorage  st;
   httpRequest->setCookieStorage(&st);
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(std::move(httpRequest));
 
   httpResponse.getHttpHeader()->put
     (HttpHeader::SET_COOKIE,
@@ -523,8 +525,7 @@ void HttpResponseTest::testSupportsPersistentConnection()
 {
   HttpResponse httpResponse;
   httpResponse.setHttpHeader(make_unique<HttpHeader>());
-  std::shared_ptr<HttpRequest> httpRequest(new HttpRequest());
-  httpResponse.setHttpRequest(httpRequest);
+  httpResponse.setHttpRequest(make_unique<HttpRequest>());
 
   httpResponse.getHttpHeader()->setVersion("HTTP/1.1");
   CPPUNIT_ASSERT(httpResponse.supportsPersistentConnection());
@@ -545,8 +546,9 @@ void HttpResponseTest::testSupportsPersistentConnection()
   httpResponse.getHttpHeader()->clearField();
 
   // test proxy connection
-  std::shared_ptr<Request> proxyRequest(new Request());
-  httpRequest->setProxyRequest(proxyRequest);
+  auto httpRequest = make_unique<HttpRequest>();
+  httpRequest->setProxyRequest(std::make_shared<Request>());
+  httpResponse.setHttpRequest(std::move(httpRequest));
 
   httpResponse.getHttpHeader()->setVersion("HTTP/1.1");
   CPPUNIT_ASSERT(httpResponse.supportsPersistentConnection());