From 95a76a27a3527d94a699d76c20c1b03a7cbee42e Mon Sep 17 00:00:00 2001
From: Pete Peterson <petersonpf@ornl.gov>
Date: Thu, 11 Dec 2014 14:37:36 -0500
Subject: [PATCH] Re #10559. Refactored code to consolidate work in
 InternetHelper.

---
 .../DataHandling/test/DownloadFileTest.h      |  10 +-
 .../Kernel/inc/MantidKernel/InternetHelper.h  |  29 +--
 .../Framework/Kernel/src/InternetHelper.cpp   | 201 +++++++++---------
 .../Kernel/test/InternetHelperTest.h          |  17 +-
 4 files changed, 120 insertions(+), 137 deletions(-)

diff --git a/Code/Mantid/Framework/DataHandling/test/DownloadFileTest.h b/Code/Mantid/Framework/DataHandling/test/DownloadFileTest.h
index 32dc28481c0..400f80554d3 100644
--- a/Code/Mantid/Framework/DataHandling/test/DownloadFileTest.h
+++ b/Code/Mantid/Framework/DataHandling/test/DownloadFileTest.h
@@ -27,20 +27,16 @@ namespace
   {
   protected:
     virtual int sendHTTPSRequest(const std::string& url, 
-                          std::ostream& responseStream,
-                          const StringToStringMap& headers = StringToStringMap())
+                          std::ostream& responseStream)
     {
       UNUSED_ARG(url);
-      UNUSED_ARG(headers);
       responseStream << "HTTPS request succeeded";
       return 200;
     }
     virtual int sendHTTPRequest(const std::string& url, 
-                              std::ostream& responseStream,
-                              const StringToStringMap& headers = StringToStringMap())
+                              std::ostream& responseStream)
     {
       UNUSED_ARG(url);
-      UNUSED_ARG(headers);
       responseStream << "HTTP request succeeded";
       return 200;
     }
@@ -143,4 +139,4 @@ public:
 };
 
 
-#endif /* MANTID_DATAHANDLING_DOWNLOADFILETEST_H_ */
\ No newline at end of file
+#endif /* MANTID_DATAHANDLING_DOWNLOADFILETEST_H_ */
diff --git a/Code/Mantid/Framework/Kernel/inc/MantidKernel/InternetHelper.h b/Code/Mantid/Framework/Kernel/inc/MantidKernel/InternetHelper.h
index 0dd90ed7a6b..d90c55c32bb 100644
--- a/Code/Mantid/Framework/Kernel/inc/MantidKernel/InternetHelper.h
+++ b/Code/Mantid/Framework/Kernel/inc/MantidKernel/InternetHelper.h
@@ -10,8 +10,11 @@
 namespace Poco {
 // forward declaration
 class URI;
+
 namespace Net {
 // forward declaration
+class HTTPClientSession;
+// forward declaration
 class HTTPResponse;
 // forward declaration
 class HTTPRequest;
@@ -73,26 +76,28 @@ public:
   void setTimeout(int seconds);
 
 protected:
-  virtual int
-  sendHTTPSRequest(const std::string &url, std::ostream &responseStream,
-                   const StringToStringMap &headers = StringToStringMap(),
-                   const std::string &method = std::string(),
-                   const std::string &body = std::string());
-  virtual int
-  sendHTTPRequest(const std::string &url, std::ostream &responseStream,
-                  const StringToStringMap &headers = StringToStringMap(),
-                  const std::string &method = std::string(),
-                  const std::string &body = std::string());
+  virtual int sendHTTPSRequest(const std::string &url,
+                               std::ostream &responseStream);
+  virtual int sendHTTPRequest(const std::string &url,
+                              std::ostream &responseStream);
   virtual int processErrorStates(const Poco::Net::HTTPResponse &res,
                                  std::istream &rs, const std::string &url);
 
 private:
-  void createRequest(Poco::URI &uri, const std::string method,
-                     const StringToStringMap &headers);
+  void setupProxyOnSession(Poco::Net::HTTPClientSession &session,
+                           const std::string &proxyUrl);
+  void createRequest(Poco::URI &uri);
+  int sendRequestAndProcess(Poco::Net::HTTPClientSession &session,
+                            Poco::URI &uri, std::ostream &responseStream);
+  int processRelocation(const Poco::Net::HTTPResponse &response,
+                        std::ostream &responseStream);
   Kernel::ProxyInfo m_proxyInfo;
   bool m_isProxySet;
   int m_timeout;
+  std::string m_method;
   std::string m_contentType;
+  std::string m_body;
+  StringToStringMap m_headers;
   Poco::Net::HTTPRequest *m_request;
 };
 
diff --git a/Code/Mantid/Framework/Kernel/src/InternetHelper.cpp b/Code/Mantid/Framework/Kernel/src/InternetHelper.cpp
index dc3977ba64c..d092cd52bcb 100644
--- a/Code/Mantid/Framework/Kernel/src/InternetHelper.cpp
+++ b/Code/Mantid/Framework/Kernel/src/InternetHelper.cpp
@@ -47,14 +47,24 @@ namespace {
 /// static Logger object
 Logger g_log("InternetHelper");
 
-/// Convert the method into the proper get/post string
-/// @returns Poco::Net::HTTPRequest::HTTP_GET if it is empty
-std::string getMethod(const std::string &method) {
-  if (method.empty()) {
-    return HTTPRequest::HTTP_GET;
-  } else {
-    return method;
-  }
+/// Throw an exception occurs when the computer
+/// is not connected to the internet
+void throwNotConnected(const std::string &url,
+                       const HostNotFoundException &ex) {
+  std::stringstream info;
+  info << "Failed to download " << url
+       << " because there is no connection to the host " << ex.message()
+       << ".\nHint: Check your connection following this link: <a href=\""
+       << url << "\">" << url << "</a> ";
+  throw Exception::InternetError(info.str() + ex.displayText());
+}
+
+/// @returns true if the return code is considered a relocation
+bool isRelocated(const int response) {
+  return ((response == HTTPResponse::HTTP_FOUND) ||
+          (response == HTTPResponse::HTTP_MOVED_PERMANENTLY) ||
+          (response == HTTPResponse::HTTP_TEMPORARY_REDIRECT) ||
+          (response == HTTPResponse::HTTP_SEE_OTHER));
 }
 }
 
@@ -63,14 +73,16 @@ std::string getMethod(const std::string &method) {
 */
 InternetHelper::InternetHelper()
     : m_proxyInfo(), m_isProxySet(false), m_timeout(30),
-      m_contentType("application/json"), m_request(NULL) {}
+      m_method(HTTPRequest::HTTP_GET), m_contentType("application/json"),
+      m_request(NULL) {}
 
 //----------------------------------------------------------------------------------------------
 /** Constructor
 */
 InternetHelper::InternetHelper(const Kernel::ProxyInfo &proxy)
     : m_proxyInfo(proxy), m_isProxySet(true), m_timeout(30),
-      m_contentType("application/json"), m_request(NULL) {}
+      m_method(HTTPRequest::HTTP_GET), m_contentType("application/json"),
+      m_request(NULL) {}
 
 //----------------------------------------------------------------------------------------------
 /** Destructor
@@ -81,22 +93,65 @@ InternetHelper::~InternetHelper() {
   }
 }
 
-void InternetHelper::createRequest(Poco::URI &uri, const std::string method,
-                                   const std::map<string, string> &headers) {
-  string reqMethod = getMethod(method);
+void InternetHelper::setupProxyOnSession(HTTPClientSession &session,
+                                         const std::string &proxyUrl) {
+  auto proxy = this->getProxy(proxyUrl);
+  if (!proxy.emptyProxy()) {
+    session.setProxyHost(proxy.host());
+    session.setProxyPort(static_cast<Poco::UInt16>(proxy.port()));
+  }
+}
 
-  m_request = new HTTPRequest(getMethod(method), uri.getPathAndQuery(),
-                              HTTPMessage::HTTP_1_1);
-  if (reqMethod == HTTPRequest::HTTP_POST) {
+void InternetHelper::createRequest(Poco::URI &uri) {
+  m_request =
+      new HTTPRequest(m_method, uri.getPathAndQuery(), HTTPMessage::HTTP_1_1);
+  if (!m_contentType.empty()) {
     m_request->setContentType(m_contentType);
   }
   m_request->set("User-Agent", "MANTID");
-  for (auto itHeaders = headers.begin(); itHeaders != headers.end();
+  for (auto itHeaders = m_headers.begin(); itHeaders != m_headers.end();
        ++itHeaders) {
     m_request->set(itHeaders->first, itHeaders->second);
   }
 }
 
+int InternetHelper::sendRequestAndProcess(HTTPClientSession &session,
+                                          Poco::URI &uri,
+                                          std::ostream &responseStream) {
+  // create a request
+  this->createRequest(uri);
+  m_request->setContentLength(m_body.length());
+  session.sendRequest(*m_request) << m_body;
+
+  HTTPResponse res;
+  std::istream &rs = session.receiveResponse(res);
+  int retStatus = res.getStatus();
+  g_log.debug() << "Answer from web: " << retStatus << " " << res.getReason()
+                << std::endl;
+
+  if (retStatus == HTTPResponse::HTTP_OK ||
+      retStatus == HTTPResponse::HTTP_CREATED) {
+    Poco::StreamCopier::copyStream(rs, responseStream);
+    return retStatus;
+  } else if (isRelocated(retStatus)) {
+    return this->processRelocation(res, responseStream);
+  } else {
+    return processErrorStates(res, rs, uri.toString());
+  }
+}
+
+int InternetHelper::processRelocation(const HTTPResponse &response,
+                                      std::ostream &responseStream) {
+  std::string newLocation = response.get("location", "");
+  if (!newLocation.empty()) {
+    g_log.information() << "url relocated to " << newLocation;
+    return this->sendRequest(newLocation, responseStream);
+  } else {
+    g_log.warning("Apparent relocation did not give new location\n");
+    return response.getStatus();
+  }
+}
+
 /** Performs a request using http or https depending on the url
 * @param url the address to the network resource
 * @param responseStream The stream to fill with the reply on success
@@ -108,11 +163,23 @@ int InternetHelper::sendRequest(const std::string &url,
                                 const StringToStringMap &headers,
                                 const std::string &method,
                                 const std::string &body) {
+  // set instance variables from the input as appropriate
+  if (!method.empty()) {
+    m_method = method;
+  }
+  if (!headers.empty()) {
+    m_headers = headers;
+  }
+  if (!body.empty()) {
+    m_body = body;
+  }
+
+  // send the request
   Poco::URI uri(url);
   if ((uri.getScheme() == "https") || (uri.getPort() == 443)) {
-    return sendHTTPSRequest(url, responseStream, headers, method, body);
+    return sendHTTPSRequest(url, responseStream);
   } else {
-    return sendHTTPRequest(url, responseStream, headers, method, body);
+    return sendHTTPRequest(url, responseStream);
   }
 }
 
@@ -123,10 +190,7 @@ int InternetHelper::sendRequest(const std::string &url,
 *include in the request.
 **/
 int InternetHelper::sendHTTPRequest(const std::string &url,
-                                    std::ostream &responseStream,
-                                    const StringToStringMap &headers,
-                                    const std::string &method,
-                                    const std::string &body) {
+                                    std::ostream &responseStream) {
   int retStatus = 0;
   g_log.debug() << "Sending request to: " << url << "\n";
 
@@ -137,45 +201,13 @@ int InternetHelper::sendHTTPRequest(const std::string &url,
     session.setTimeout(Poco::Timespan(m_timeout, 0)); // m_timeout seconds
 
     // configure proxy
-    if (!getProxy(url).emptyProxy()) {
-      session.setProxyHost(getProxy(url).host());
-      session.setProxyPort(static_cast<Poco::UInt16>(getProxy(url).port()));
-    }
+    setupProxyOnSession(session, url);
 
-    this->createRequest(uri, method, headers);
-    m_request->setContentLength(body.length());
-    session.sendRequest(*m_request) << body;
-
-    HTTPResponse res;
-    std::istream &rs = session.receiveResponse(res);
-    retStatus = res.getStatus();
-    g_log.debug() << "Answer from web: " << retStatus << " "
-                  << res.getReason() << std::endl;
-
-    if (retStatus == HTTPResponse::HTTP_OK || retStatus == HTTPResponse::HTTP_CREATED) {
-      Poco::StreamCopier::copyStream(rs, responseStream);
-      return retStatus;
-    } else if ((retStatus == HTTPResponse::HTTP_FOUND) ||
-               (retStatus == HTTPResponse::HTTP_MOVED_PERMANENTLY) ||
-               (retStatus == HTTPResponse::HTTP_TEMPORARY_REDIRECT) ||
-               (retStatus == HTTPResponse::HTTP_SEE_OTHER)) {
-      // extract the new location
-      std::string newLocation = res.get("location", "");
-      if (newLocation != "") {
-        return sendRequest(newLocation, responseStream, headers, method, body);
-      }
-    } else {
-      retStatus = processErrorStates(res, rs, url);
-    }
+    // low level sending the request
+    retStatus = this->sendRequestAndProcess(session, uri, responseStream);
   }
   catch (HostNotFoundException &ex) {
-    // this exception occurs when the pc is not connected to the internet
-    std::stringstream info;
-    info << "Failed to download " << url
-         << " because there is no connection to the host " << ex.message()
-         << ".\nHint: Check your connection following this link: <a href=\""
-         << url << "\">" << url << "</a> ";
-    throw Exception::InternetError(info.str() + ex.displayText());
+    throwNotConnected(url, ex);
   }
   catch (Poco::Exception &ex) {
     throw Exception::InternetError("Connection and request failed " +
@@ -191,13 +223,9 @@ int InternetHelper::sendHTTPRequest(const std::string &url,
 *include in the request.
 **/
 int InternetHelper::sendHTTPSRequest(const std::string &url,
-                                     std::ostream &responseStream,
-                                     const StringToStringMap &headers,
-                                     const std::string &method,
-                                     const std::string &body) {
+                                     std::ostream &responseStream) {
   int retStatus = 0;
-  std::string reqMethod = getMethod(method);
-  g_log.debug() << "Sending " << reqMethod << " request : " << url << "\n";
+  g_log.debug() << "Sending request to: " << url << "\n";
 
   Poco::URI uri(url);
   try {
@@ -226,48 +254,13 @@ int InternetHelper::sendHTTPSRequest(const std::string &url,
     if (urlforProxy.empty()) {
       urlforProxy = "http://" + uri.getHost();
     }
+    setupProxyOnSession(session, urlforProxy);
 
-    auto proxy = getProxy(urlforProxy);
-    if (!proxy.emptyProxy()) {
-      session.setProxyHost(proxy.host());
-      session.setProxyPort(static_cast<Poco::UInt16>(proxy.port()));
-    }
-
-    // create a request
-    this->createRequest(uri, method, headers);
-    m_request->setContentLength(body.length());
-    session.sendRequest(*m_request) << body;
-
-    HTTPResponse res;
-    std::istream &rs = session.receiveResponse(res);
-    retStatus = res.getStatus();
-    g_log.debug() << "Answer from web: " << retStatus << " "
-                  << res.getReason() << std::endl;
-
-    if (retStatus == HTTPResponse::HTTP_OK || retStatus == HTTPResponse::HTTP_CREATED) {
-      Poco::StreamCopier::copyStream(rs, responseStream);
-      return retStatus;
-    } else if ((retStatus == HTTPResponse::HTTP_FOUND) ||
-               (retStatus == HTTPResponse::HTTP_MOVED_PERMANENTLY) ||
-               (retStatus == HTTPResponse::HTTP_TEMPORARY_REDIRECT) ||
-               (retStatus == HTTPResponse::HTTP_SEE_OTHER)) {
-      // extract the new location
-      std::string newLocation = res.get("location", "");
-      if (newLocation != "") {
-        return sendRequest(newLocation, responseStream, headers, method, body);
-      }
-    } else {
-      retStatus = processErrorStates(res, rs, url);
-    }
+    // low level sending the request
+    retStatus = this->sendRequestAndProcess(session, uri, responseStream);
   }
   catch (HostNotFoundException &ex) {
-    // this exception occurs when the pc is not connected to the internet
-    std::stringstream info;
-    info << "Failed to download " << url
-         << " because there is no connection to the host " << ex.message()
-         << ".\nHint: Check your connection following this link: <a href=\""
-         << url << "\">" << url << "</a> ";
-    throw Exception::InternetError(info.str() + ex.displayText());
+    throwNotConnected(url, ex);
   }
   catch (Poco::Exception &ex) {
     throw Exception::InternetError("Connection and request failed " +
diff --git a/Code/Mantid/Framework/Kernel/test/InternetHelperTest.h b/Code/Mantid/Framework/Kernel/test/InternetHelperTest.h
index 4169e5a9ef1..8ad8c31e512 100644
--- a/Code/Mantid/Framework/Kernel/test/InternetHelperTest.h
+++ b/Code/Mantid/Framework/Kernel/test/InternetHelperTest.h
@@ -23,26 +23,15 @@ namespace {
 class MockedInternetHelper : public InternetHelper {
 protected:
   virtual int
-  sendHTTPSRequest(const std::string &url, std::ostream &responseStream,
-                   const StringToStringMap &headers = StringToStringMap(),
-                   const std::string &method = std::string(),
-                   const std::string &body = std::string()) {
+  sendHTTPSRequest(const std::string &url, std::ostream &responseStream) {
     UNUSED_ARG(url);
-    UNUSED_ARG(headers);
-    UNUSED_ARG(method);
-    UNUSED_ARG(body);
+
     responseStream << "HTTPS request succeeded";
     return 200;
   }
   virtual int
-  sendHTTPRequest(const std::string &url, std::ostream &responseStream,
-                  const StringToStringMap &headers = StringToStringMap(),
-                  const std::string &method = std::string(),
-                  const std::string &body = std::string()) {
+  sendHTTPRequest(const std::string &url, std::ostream &responseStream) {
     UNUSED_ARG(url);
-    UNUSED_ARG(headers);
-    UNUSED_ARG(method);
-    UNUSED_ARG(body);
     responseStream << "HTTP request succeeded";
     return 200;
   }
-- 
GitLab