From db3ed2c9a5a9bb886ab7fd473b71cda0d0f489aa Mon Sep 17 00:00:00 2001
From: Pete Peterson <petersonpf@ornl.gov>
Date: Mon, 17 Dec 2018 13:08:01 -0500
Subject: [PATCH] Get download url from json blob

Since github supplies the correct url in their json, just use it rather
than guess it from the html url. Refs #24339.
---
 .../DataHandling/src/DownloadInstrument.cpp   | 40 +++++++++++--------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/Framework/DataHandling/src/DownloadInstrument.cpp b/Framework/DataHandling/src/DownloadInstrument.cpp
index 8fccdbeda01..a3f289c69dc 100644
--- a/Framework/DataHandling/src/DownloadInstrument.cpp
+++ b/Framework/DataHandling/src/DownloadInstrument.cpp
@@ -117,16 +117,34 @@ void DownloadInstrument::exec() {
 
   for (auto &itMap : fileMap) {
     // download a file
-    doDownloadFile(itMap.first, itMap.second);
     if (boost::algorithm::ends_with(itMap.second, "Facilities.xml")) {
       g_log.notice("A new Facilities.xml file has been downloaded, this will "
                    "take effect next time Mantid is started.");
+    } else {
+      g_log.information() << "Downloading \"" << itMap.second << "\" from \""
+                          << itMap.first << "\"\n";
     }
+    doDownloadFile(itMap.first, itMap.second);
   }
 
   setProperty("FileDownloadCount", static_cast<int>(fileMap.size()));
 }
 
+namespace {
+// Converts a json chunk to a url for the raw file contents.
+std::string getDownloadUrl(Json::Value &contents) {
+  std::string url = contents.get("download_url", "").asString();
+  if (url.empty()) { // guess it from html url
+    url = contents.get("html_url", "").asString();
+    if (url.empty())
+      throw std::runtime_error("Failed to find download link");
+    url = url + "?raw=1";
+  }
+
+  return url;
+}
+} // namespace
+
 DownloadInstrument::StringToStringMap DownloadInstrument::processRepository() {
   // get the instrument directories
   auto instrumentDirs =
@@ -198,8 +216,7 @@ DownloadInstrument::StringToStringMap DownloadInstrument::processRepository() {
     if (filePath.getExtension() != "xml")
       continue;
     std::string sha = serverElement.get("sha", "").asString();
-    std::string htmlUrl =
-        getDownloadableRepoUrl(serverElement.get("html_url", "").asString());
+    std::string downloadUrl = getDownloadUrl(serverElement);
 
     // Find shas
     std::string localSha = getValueOrDefault(localShas, name, "");
@@ -208,14 +225,14 @@ DownloadInstrument::StringToStringMap DownloadInstrument::processRepository() {
     // this will also catch when file is only present on github (as local sha
     // will be "")
     if ((sha != installSha) && (sha != localSha)) {
-      fileMap.emplace(htmlUrl,
+      fileMap.emplace(downloadUrl,
                       filePath.toString()); // ACTION - DOWNLOAD to localPath
     } else if ((!localSha.empty()) && (sha == installSha) &&
                (sha != localSha)) // matches install, but different local
     {
-      fileMap.emplace(
-          htmlUrl,
-          filePath.toString()); // ACTION - DOWNLOAD to localPath and overwrite
+      fileMap.emplace(downloadUrl, filePath.toString()); // ACTION - DOWNLOAD to
+                                                         // localPath and
+                                                         // overwrite
     }
   }
 
@@ -333,15 +350,6 @@ size_t DownloadInstrument::removeOrphanedFiles(
   return filesToDelete.size();
 }
 
-/** Converts a github file page to a downloadable url for the file.
- * @param filename a github file page url
- * @returns a downloadable url for the file
- **/
-const std::string
-DownloadInstrument::getDownloadableRepoUrl(const std::string &filename) const {
-  return filename + "?raw=1";
-}
-
 /** Download a url and fetch it inside the local path given.
 This calls Kernel/InternetHelper, but is wrapped in this method to allow mocking
 in the unit tests.
-- 
GitLab