Modularise ISISDataArchive further

Mock out web service modules and add unit tests

Refs #24408

Framework/DataHandling/inc/MantidDataHandling/ISISDataArchive.h

@@ -13,6 +13,7 @@
 #include "MantidAPI/IArchiveSearch.h"
 #include "MantidKernel/System.h"
 
+#include <sstream>
 #include <string>
 
 namespace Mantid {
@@ -30,14 +31,16 @@ public:
   std::string
   getArchivePath(const std::set<std::string> &filenames,
                  const std::vector<std::string> &exts) const override;
+
   /// Public and virtual for testing purposes
   virtual std::string
   getCorrectExtension(const std::string &path,
                       const std::vector<std::string> &exts) const;
+  std::string getPath(const std::string &fName) const;
 
-private:
+protected:
   /// Queries the archive & returns the path to a single file.
-  std::string getPath(const std::string &fName) const;
+  virtual std::ostringstream sendRequest(const std::string &fName) const;
 };
 } // namespace DataHandling
 } // namespace Mantid

Framework/DataHandling/src/ISISDataArchive.cpp

@@ -17,8 +17,6 @@
 #include <Poco/File.h>
 #include <Poco/Path.h>
 
-#include <sstream>
-
 namespace Mantid {
 namespace DataHandling {
 namespace {
@@ -47,11 +45,13 @@ const char *URL_PREFIX = "http://data.isis.rl.ac.uk/where.py/unixdir?name=";
 std::string
 ISISDataArchive::getArchivePath(const std::set<std::string> &filenames,
                                 const std::vector<std::string> &exts) const {
-  for (const auto &filename : filenames) {
-    g_log.debug() << filename << ")\n";
-  }
-  for (const auto &ext : exts) {
-    g_log.debug() << ext << ")\n";
+  if (g_log.is(Kernel::Logger::Priority::PRIO_DEBUG)) {
+    for (const auto &filename : filenames) {
+      g_log.debug() << filename << ")\n";
+    }
+    for (const auto &ext : exts) {
+      g_log.debug() << ext << ")\n";
+    }
   }
 
   for (const auto &filename : filenames) {
@@ -66,32 +66,39 @@ ISISDataArchive::getArchivePath(const std::set<std::string> &filenames,
 }
 
 /**
- * Calls a web service to get a path to a file.
- * Only returns a path string if the file exists.
- * The ISIS web service return a path independent of the file extension of the
- * file provided. Thus the call to web service uses a file WITHOUT an extension.
+ * Gets the path to the file, or most recent set of files.
  * @param fName :: The file name.
- * @return The path to the file or an empty string in case of error/non-existing
- * file.
+ * @return The path to the file or an empty string in case of empty filename
  */
 std::string ISISDataArchive::getPath(const std::string &fName) const {
   g_log.debug() << "ISISDataArchive::getPath() - fName=" << fName << "\n";
   if (fName.empty())
     return ""; // Avoid pointless call to service
 
+  std::ostringstream os = sendRequest(fName);
+  os << Poco::Path::separator() << fName;
+  const std::string expectedPath = os.str();
+  return expectedPath;
+}
+
+/** Calls a web service to get a path to a file.
+ * If the file does not exist, returns path to most recent run.
+ * The ISIS web service return a path independent of the file extension of the
+ * file provided. Thus the call to web service uses a file WITHOUT an extension.
+ * @param fName :: The file name.
+ * @return ostringstream object containing path to file (without filename
+ * itself)
+ */
+std::ostringstream
+ISISDataArchive::sendRequest(const std::string &fName) const {
   Kernel::InternetHelper inetHelper;
   std::ostringstream os;
   try {
     inetHelper.sendRequest(URL_PREFIX + fName, os);
-
-    os << Poco::Path::separator() << fName;
-    const std::string expectedPath = os.str();
-    return expectedPath;
   } catch (Kernel::Exception::InternetError &ie) {
-    g_log.warning() << "Could not access archive index " << ie.what();
+    g_log.warning() << "Could not access archive index." << ie.what();
   }
-
-  return "";
+  return os;
 }
 
 /**
@@ -105,7 +112,7 @@ std::string ISISDataArchive::getPath(const std::string &fName) const {
  */
 std::string ISISDataArchive::getCorrectExtension(
     const std::string &path, const std::vector<std::string> &exts) const {
-  for (auto ext : exts) {
+  for (const auto &ext : exts) {
     std::string temp_path = path + ext;
     try {
       if (Poco::File(temp_path).exists())

Framework/DataHandling/test/ISISDataArchiveTest.h

@@ -9,6 +9,7 @@
 
 #include <cxxtest/TestSuite.h>
 #include <fstream>
+#include <sstream>
 
 #include "MantidAPI/ArchiveSearchFactory.h"
 #include "MantidDataHandling/ISISDataArchive.h"
@@ -24,44 +25,97 @@ const char *URL_PREFIX = "http://data.isis.rl.ac.uk/where.py/unixdir?name=";
 #endif
 }
 
-class MockgetCorrectExtension : public ISISDataArchive {
+class MockOutRequests : public ISISDataArchive {
 public:
+  MockOutRequests()
+      : m_sendRequestReturnVal("/archive/default/path"),
+        m_acceptableExts(".RAW") {}
+
+  void setSendRequestReturnVal(std::string &return_val) {
+    m_sendRequestReturnVal = return_val;
+  }
+
+  void setAcceptableExts(std::string &exts) { m_acceptableExts = exts; }
+
+  /** This mocks getCorrectExtension, which searches data archive for
+   * complete file path
+   * m_acceptableExts is a string containing acceptable extensions
+   * e.g. ".RAW,.txt"
+   * this mocked method will return the path + ext if ext is in
+   * m_acceptableExts, otherwise return empty string.
+   * This is a bit simpler than the actual method, as in practice
+   * e.g. hrpd273 would be changed to HRP000273
+   */
   std::string
   getCorrectExtension(const std::string &path,
                       const std::vector<std::string> &exts) const override {
-    (void)exts;
-    return path;
+    for (const auto &ext : exts) {
+      if (m_acceptableExts.find(ext) != std::string::npos) {
+        return path + ext;
+      }
+    }
+    return "";
   }
+
+protected:
+  std::ostringstream sendRequest(const std::string &fName) const override {
+    (void)fName;
+    std::ostringstream os;
+    os << m_sendRequestReturnVal;
+    return os;
+  }
+
+private:
+  // Mimics the directory tree returned by sendRequest
+  // e.g. /archive/ndxloq/Instrument/data/cycle_98_0
+  std::string m_sendRequestReturnVal;
+
+  // A string of allowed extensions. Used to mock getCorrectExtension
+  std::string m_acceptableExts;
 };
 
 class ISISDataArchiveTest : public CxxTest::TestSuite {
 public:
-  /**
-   * Un-'x' this test name to test locally.
-   * This tests the filename loop part of `getArchivePath`
-   * i.e. for each of the filenames in the set, it makes a call to the archive
-   * and `getCorrectExtension` is mocked out.
-   */
-  void xtestFilenameLoop() {
-    MockgetCorrectExtension arch;
-
-    std::set<std::string> filename;
-    filename.insert("hrpd273");
-    const std::vector<std::string> extension = std::vector<std::string>(1, "");
-    const std::string path = arch.getArchivePath(filename, extension);
-    if (path.empty()) {
-      TS_FAIL("Returned path was empty.");
-    } else {
-      TS_ASSERT_EQUALS(path.substr(path.size() - 18, 10), "cycle_98_0");
-    }
+  void testFilenameLoopWithCorrectExtensions() {
+    std::vector<std::string> exts = {".txt", ".RAW"};
+    std::set<std::string> filenames = {"hrpd273"};
+
+    MockOutRequests arch;
+    const std::string actualPath = arch.getArchivePath(filenames, exts);
+    TS_ASSERT_EQUALS(actualPath, "/archive/default/path/hrpd273.RAW");
   }
 
+  void testFilenameLoopWithInCorrectExtensions() {
+    std::vector<std::string> exts = {".log", ".txt"};
+    std::set<std::string> filenames = {"hrpd273", "hrpd280"};
+
+    MockOutRequests arch;
+    const std::string actualPath = arch.getArchivePath(filenames, exts);
+    TS_ASSERT_EQUALS(actualPath, "");
+  }
+
+  void testFilenameLoopIgnoresEmptyFilenames() {
+    std::vector<std::string> exts = {".RAW"};
+    std::set<std::string> filenames = {"", "", "", "hrpd273"};
+
+    MockOutRequests arch;
+    const std::string actualPath = arch.getArchivePath(filenames, exts);
+    TS_ASSERT_EQUALS(actualPath, "/archive/default/path/hrpd273.RAW");
+  }
+
+  void testFactory() {
+    boost::shared_ptr<IArchiveSearch> arch =
+        ArchiveSearchFactory::Instance().create("ISISDataSearch");
+    TS_ASSERT(arch);
+  }
+
+  /*****UN 'x' THE FOLLOWING TESTS WHEN TESTING LOCALLY*****/
   /**
    * Un-'x' this test name to test locally.
    * This tests the file extensions loop.
    * To run, this tests requires that the ISIS archive is on your local machine.
    */
-  void xtestgetCorrectExtension() {
+  void xtestgetCorrectExtensionWithCorrectExtension() {
     std::string path;
     if (strcmp(URL_PREFIX, "http://data.isis.rl.ac.uk/where.py/windir?name=") ==
         0) {
@@ -73,20 +127,26 @@ public:
     ISISDataArchive arch;
 
     const std::vector<std::string> correct_exts = {".RAW"};
-    const std::string rawExtension =
+    const std::string actualResult =
         arch.getCorrectExtension(path, correct_exts);
-    TS_ASSERT_EQUALS(rawExtension, path + ".RAW");
+    TS_ASSERT_EQUALS(actualResult, path + ".RAW");
+  }
+
+  void xtestgetCorrectExtensionWithInCorrectExtensions() {
+    std::string path;
+    if (strcmp(URL_PREFIX, "http://data.isis.rl.ac.uk/where.py/windir?name=") ==
+        0) {
+      path = "\\isis.cclrc.ac.uk\\inst$\\ndxhrpd\\instrument\\data\\cycle_98_"
+             "0\\HRP00273";
+    } else {
+      path = "/archive/ndxhrpd/Instrument/data/cycle_98_0/HRP00273";
+    }
+    ISISDataArchive arch;
 
     const std::vector<std::string> incorrect_exts = {".so", ".txt"};
-    const std::string emptyString =
+    const std::string actualResult =
         arch.getCorrectExtension(path, incorrect_exts);
-    TS_ASSERT_EQUALS(emptyString, "");
-  }
-
-  void testFactory() {
-    boost::shared_ptr<IArchiveSearch> arch =
-        ArchiveSearchFactory::Instance().create("ISISDataSearch");
-    TS_ASSERT(arch);
+    TS_ASSERT_EQUALS(actualResult, "");
   }
 };