Move helper functions into a class

This means we can hide the parsing function that takes an xml string from the public interface and add a friend class to test this.
It also means that getRuns() can be called multiple times with different criteria without having to re-parse the file.

Re #26897

Framework/DataHandling/inc/MantidDataHandling/ISISJournal.h

@@ -8,37 +8,58 @@
 
 #include "MantidKernel/System.h"
 
+#include <Poco/AutoPtr.h>
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
+namespace Poco::XML {
+class Document;
+}
+
 namespace Mantid {
+namespace Kernel {
+class InternetHelper;
+}
+
 namespace DataHandling {
 namespace ISISJournal {
 /**
-  Defines functions to aid in fetching ISIS specific run information from
-  journal files
+ * ISISJournal: Helper class to aid in fetching ISIS specific run information
+ * from journal files
  */
 
-using ISISJournalTags = std::vector<std::string>;
-using ISISJournalFilters = std::map<std::string, std::string>;
-using ISISJournalData = std::map<std::string, std::string>;
+class DLLExport ISISJournal {
+public:
+  using RunData = std::map<std::string, std::string>;
 
-std::vector<ISISJournalData> DLLExport
-getRunDataFromFile(std::string const &fileContents,
-                   ISISJournalTags const &tags = ISISJournalTags(),
-                   ISISJournalFilters const &filters = ISISJournalFilters());
+  ISISJournal(std::string const &instrument, std::string const &cycle,
+              std::unique_ptr<Kernel::InternetHelper> internetHelper =
+                  std::make_unique<Kernel::InternetHelper>());
+  virtual ~ISISJournal();
 
-std::vector<ISISJournalData> DLLExport
-getRunData(std::string const &instrument, std::string const &cycle,
-           ISISJournalTags const &tags = ISISJournalTags(),
-           ISISJournalFilters const &filters = ISISJournalFilters());
+  ISISJournal(ISISJournal const &rhs) = delete;
+  ISISJournal(ISISJournal &&rhs);
+  ISISJournal const &operator=(ISISJournal const &rhs) = delete;
+  ISISJournal &operator=(ISISJournal &&rhs);
 
-std::vector<std::string>
-    DLLExport getCycleListFromFile(std::string const &fileContents);
+  /// Get the list of cycle names
+  std::vector<std::string> getCycleNames();
+  /// Get data for runs that match the given filters
+  std::vector<RunData>
+  getRuns(std::vector<std::string> const &valuesToLookup = {},
+          RunData const &filters = RunData());
 
-std::vector<std::string> DLLExport getCycleList(std::string const &instrument);
+private:
+  std::unique_ptr<Kernel::InternetHelper> m_internetHelper;
+  std::string m_runsFileURL;
+  std::string m_indexFileURL;
+  Poco::AutoPtr<Poco::XML::Document> m_runsDocument;
+  Poco::AutoPtr<Poco::XML::Document> m_indexDocument;
 
+  std::string getURLContents(std::string const &url);
+};
 } // namespace ISISJournal
 } // namespace DataHandling
 } // namespace Mantid

Framework/DataHandling/src/ISISJournal.cpp

@@ -10,7 +10,6 @@
 #include "MantidKernel/InternetHelper.h"
 
 #include "Poco/SAX/SAXException.h"
-#include <Poco/AutoPtr.h>
 #include <Poco/DOM/DOMParser.h>
 #include <Poco/DOM/Document.h>
 #include <Poco/DOM/NodeFilter.h>
@@ -34,59 +33,41 @@ using Poco::XML::NodeFilter;
 using Poco::XML::TreeWalker;
 
 namespace {
-static constexpr const char *URL_PREFIX = "http://data.isis.rl.ac.uk/";
-static constexpr const char *INSTRUMENT_PREFIX = "journals/ndx";
-static constexpr const char *JOURNAL_INDEX_FILE = "main";
-static constexpr const char *JOURNAL_PREFIX = "journal_";
+static constexpr const char *URL_PREFIX =
+    "http://data.isis.rl.ac.uk/journals/ndx";
+static constexpr const char *INDEX_FILE_NAME = "main";
+static constexpr const char *JOURNAL_PREFIX = "/journal_";
 static constexpr const char *JOURNAL_EXT = ".xml";
 static constexpr const char *NXROOT_TAG = "NXroot";
 static constexpr const char *NXENTRY_TAG = "NXentry";
 static constexpr const char *JOURNAL_TAG = "journal";
 static constexpr const char *FILE_TAG = "file";
 
-/* Construct the URL for a journal file containing run data for a particular
+/** Construct the URL for a journal file containing run data for a particular
  * instrument and cycle, e.g.
  *   http://data.isis.rl.ac.uk/journals/ndxinter/journal_19_4.xml
+ * @param instrument : the ISIS instrument name (case insensitive)
+ * @param name : the file name excluding the "journal_" prefix and extension,
+ * e.g. "19_4" for "journal_19_4.xml"
+ * @returns : the URL as a string
  */
-std::string constructRunsFileURL(std::string const &instrument,
-                                 std::string const &cycle) {
-  std::stringstream url;
-  url << URL_PREFIX << INSTRUMENT_PREFIX << instrument << "/" << JOURNAL_PREFIX
-      << cycle << JOURNAL_EXT;
+std::string constructURL(std::string instrument, std::string const &name) {
+  boost::algorithm::to_lower(instrument);
+  std::ostringstream url;
+  url << URL_PREFIX << instrument << JOURNAL_PREFIX << name << JOURNAL_EXT;
   return url.str();
 }
 
-/* Construct the URL for the journal index file for a particular instrument,
- * e.g. http://data.isis.rl.ac.uk/journals/ndxinter/journal_main.xml
+/** Parse a given XML string into a Document object
+ * @param xmlString : the XML file contents as a string
+ * @returns : the XML Document
+ * @throws : if there was an error parsing the file
  */
-std::string constructIndexFileURL(std::string const &instrument) {
-  std::stringstream url;
-  url << URL_PREFIX << INSTRUMENT_PREFIX << instrument << "/" << JOURNAL_PREFIX
-      << JOURNAL_INDEX_FILE << JOURNAL_EXT;
-  return url.str();
-}
-
-/* Get the contents of a file at a given URL
- */
-std::string getURLContents(std::string const &url) {
-  Kernel::InternetHelper inetHelper;
-  std::stringstream serverReply;
-  auto const statusCode = inetHelper.sendRequest(url, serverReply);
-  if (statusCode != Poco::Net::HTTPResponse::HTTP_OK) {
-    throw Kernel::Exception::InternetError(
-        std::string("Failed to access journal file: ") +
-        std::to_string(statusCode));
-  }
-  return serverReply.str();
-}
-
-/* Parse a given XML file contents into a Document object
- */
-Poco::AutoPtr<Document> parse(std::string const &fileContents) {
+Poco::AutoPtr<Document> parse(std::string const &xmlString) {
   DOMParser domParser;
   Poco::AutoPtr<Document> xmldoc;
   try {
-    xmldoc = domParser.parseString(fileContents);
+    xmldoc = domParser.parseString(xmlString);
   } catch (Poco::XML::SAXParseException const &ex) {
     std::ostringstream msg;
     msg << "ISISJournal: Error parsing file: " << ex.what();
@@ -95,26 +76,30 @@ Poco::AutoPtr<Document> parse(std::string const &fileContents) {
   return xmldoc;
 }
 
-/* Check the given element is not null and has the given name and throw if not
+/** Check the given element is valid.
+ * @param expectedName : the node name that the element is expected to have
+ * @throws : if the element is null or does not match the expected name
  */
-void validateElement(Element *element, const char *expectedName) {
+void throwIfNotValid(Element *element, const char *expectedName) {
   if (!element) {
     std::ostringstream msg;
-    msg << "ISISJournal::validateElement() - invalid element for '"
-        << NXROOT_TAG << "'\n";
+    msg << "ISISJournal::throwIfNotValid() - invalid element for '"
+        << expectedName << "'\n";
     throw std::invalid_argument(msg.str());
   }
 
   if (element->nodeName() != expectedName) {
     std::ostringstream msg;
-    msg << "ISISJournal::validateElement() - Element tag does not match '"
-        << NXROOT_TAG << "'. Found " << element->nodeName() << "\n";
+    msg << "ISISJournal::throwIfNotValid() - Element name does not match '"
+        << expectedName << "'. Found " << element->nodeName() << "\n";
     throw std::invalid_argument(msg.str());
   }
 }
 
-/* Return the text value contained in the given node, or an empty string if it
- * does not contain a text value.
+/** Get the text contained in a node.
+ * @param node : the node
+ * @returns : the value contained in the child #text node, or an empty string if
+ * it does not contain a text value.
  */
 std::string getTextValue(Node *node) {
   if (!node)
@@ -130,12 +115,14 @@ std::string getTextValue(Node *node) {
   return std::string();
 }
 
-/* Check if an element matches a set of filter criteria. Checks for a child
- * element with the filter name and checks that its value matches the given
- * filter value.
+/** Check if an element matches a set of filter criteria.
+ * @param element : the element to check
+ * @param filters : a map of names and values to check against
+ * @returns : true if, for all filters, the element has a child node with
+ * the filter name that contains text matching the filter value
  */
-bool matchesAllFilters(Element *element, ISISJournalFilters const &filters) {
-  for (auto filterKvp : filters) {
+bool matchesAllFilters(Element *element, ISISJournal::RunData const &filters) {
+  for (auto const &filterKvp : filters) {
     auto const childElement = element->getChildElement(filterKvp.first);
     if (getTextValue(childElement) != filterKvp.second)
       return false;
@@ -143,61 +130,87 @@ bool matchesAllFilters(Element *element, ISISJournalFilters const &filters) {
   return true;
 }
 
-/* Extract a list of named "tag" values for the given element. Gets the text
- * values of the child elements with the given names and returns a map of the
- * tag name to the value. Also always adds the element name to the list so there
- * is always a results for every run.
+/** Utility function to create the run data for an element.
+ * @param element : the element to get run data for
+ * @returns : a map of name->value pairs initialised with the mandatory data
+ * that is returned for all runs (currently just the name attribute, e.g.
+ * "name": "INTER00013460")
  */
-ISISJournalData getTagsForNode(Element *element, ISISJournalTags const &tags) {
-  auto result = ISISJournalData{};
-  // Add the run name
-  result["name"] = element->getAttribute("name");
-  // Add any tags in the tags list
-  for (auto &tag : tags)
-    result[tag] = getTextValue(element->getChildElement(tag));
-  return result;
+ISISJournal::RunData createRunDataForElement(Element *element) {
+  return ISISJournal::RunData{{"name", element->getAttribute("name")}};
 }
 
-/* Extract a list of "tag" values for all child nodes in the given "root" or
- * parent element. Here, a "tag" is a name for a child element in the element
- * we're checking i.e. a grandchild of the root node.
+/** Extract a list of named values for the given element.
+ * @param element : the element to extract values for
+ * @param valuesToLookup : a list of values to extract (which may be empty if
+ * nothing is requested)
+ * @param result : a map of names to values to which we add key-value pairs for
+ * each requested value
  */
-std::vector<ISISJournalData>
-getTagsForAllNodes(Element *root, ISISJournalTags const &tags,
-                   ISISJournalFilters const &filters) {
-  auto results = std::vector<ISISJournalData>{};
+void addValuesForElement(Element *element,
+                         std::vector<std::string> const &valuesToLookup,
+                         ISISJournal::RunData &result) {
+  for (auto &name : valuesToLookup)
+    result[name] = getTextValue(element->getChildElement(name));
+}
 
-  auto nodeIter = TreeWalker(root, NodeFilter::SHOW_ELEMENT);
+/** Extract a list of named values for all child nodes in the given parent
+ * element.
+ * @param parentElement : the element containing all child nodes we want to
+ * check
+ * @param valuesToLookup : the names of the values to extract from within the
+ * child nodes
+ * @param filters : a map of names and values to filter the results by
+ * @returns : a map of name->value pairs containing mandatory run data as well
+ * as the values that were requested
+ */
+std::vector<ISISJournal::RunData>
+getValuesForAllElements(Element *parentElement,
+                        std::vector<std::string> const &valuesToLookup,
+                        ISISJournal::RunData const &filters) {
+  auto results = std::vector<ISISJournal::RunData>{};
+
+  auto nodeIter = TreeWalker(parentElement, NodeFilter::SHOW_ELEMENT);
   for (auto node = nodeIter.nextNode(); node; node = nodeIter.nextSibling()) {
     auto element = dynamic_cast<Element *>(node);
-    validateElement(element, NXENTRY_TAG);
-    if (matchesAllFilters(element, filters))
-      results.emplace_back(getTagsForNode(element, tags));
+    throwIfNotValid(element, NXENTRY_TAG);
+
+    if (matchesAllFilters(element, filters)) {
+      auto result = createRunDataForElement(element);
+      addValuesForElement(element, valuesToLookup, result);
+      results.emplace_back(std::move(result));
+    }
   }
 
   return results;
 }
 
-/* Get the text values for all direct child elements of a given parent
- * element. The child elements should all have the given tag name - throws if
- * not.
+/** Extract an attribute value for all direct children of a given element.
+ * @param parentElement : the element containing the child nodes to check
+ * @param childElementName : the name that the child elements should have
+ * @returns : the attribute values for all of the child elements
+ * @throws : if any of the child elements does not have the expected name
  */
-std::vector<std::string> getTagValuesForChildElements(Element *root,
-                                                      const char *tag) {
+std::vector<std::string>
+getAttributeForAllChildElements(Element *parentElement,
+                                const char *childElementName,
+                                const char *attributeName) {
   auto results = std::vector<std::string>{};
 
-  auto nodeIter = TreeWalker(root, NodeFilter::SHOW_ELEMENT);
+  auto nodeIter = TreeWalker(parentElement, NodeFilter::SHOW_ELEMENT);
   for (auto node = nodeIter.nextNode(); node; node = nodeIter.nextSibling()) {
     auto element = dynamic_cast<Element *>(node);
-    validateElement(element, tag);
-    results.emplace_back(element->getAttribute("name"));
+    throwIfNotValid(element, childElementName);
+    results.emplace_back(element->getAttribute(attributeName));
   }
 
   return results;
 }
 
-/* Convert a cycle filename to the cycle name or return an empty string if it
- * doesn't match the required pattern
+/** Convert a cycle filename to the cycle name
+ * @param filename : a filename e.g. journal_19_4.xml
+ * @return the cycle name e.g. 19_4, or an empty string if the name
+ * does not match the required pattern
  */
 std::string convertFilenameToCycleName(std::string const &filename) {
   boost::regex pattern("[0-9]+_[0-9]+");
@@ -210,9 +223,11 @@ std::string convertFilenameToCycleName(std::string const &filename) {
   return std::string();
 }
 
-/* Convert a list of cycle filenames to a list of cycle names e.g.
- * journal_19_4.xml -> 19_4. Also exlcudes files from the list if they do not
- * match the required pattern.
+/** Convert a list of cycle filenames to a list of cycle names.
+ * @param filenames : the list of filenames e.g. journal_main.xml,
+ * journal_19_4.xml
+ * @returns : the list of cycle names for all valid journal files e.g. 19_4
+ * (note that this excludes files that do not match the journal-file pattern).
  */
 std::vector<std::string>
 convertFilenamesToCycleNames(std::vector<std::string> const &filenames) {
@@ -227,62 +242,78 @@ convertFilenamesToCycleNames(std::vector<std::string> const &filenames) {
 }
 } // namespace
 
-/** Get specified data from a journal file for all runs that match given filter
- * criteria.
- *
- * @param fileContents : the XML journal file contents
- * @param tags : the tag names of the required values to be returned e.g.
- * "run_number"
- * @param filters : optional tag names and values to filter the results by
+/** Construct the journal class for a specific instrument and cycle
+ * @param instrument : the ISIS instrument name to request data for e.g. "INTER"
+ * (case insensitive)
+ * @param cycle : the ISIS cycle the required data is from e.g. "19_4"
+ * @param internetHelper : class for sending internet requests
  */
-std::vector<ISISJournalData>
-getRunDataFromFile(std::string const &fileContents, ISISJournalTags const &tags,
-                   ISISJournalFilters const &filters) {
-  auto xmldoc = parse(fileContents);
-  auto root = xmldoc->documentElement();
-  validateElement(root, NXROOT_TAG);
-  return getTagsForAllNodes(root, tags, filters);
-}
+ISISJournal::ISISJournal(std::string const &instrument,
+                         std::string const &cycle,
+                         std::unique_ptr<InternetHelper> internetHelper)
+    : m_internetHelper(std::move(internetHelper)),
+      m_runsFileURL(constructURL(instrument, cycle)),
+      m_indexFileURL(constructURL(instrument, INDEX_FILE_NAME)) {}
+
+ISISJournal::~ISISJournal() = default;
+
+ISISJournal::ISISJournal(ISISJournal &&rhs) = default;
+
+ISISJournal &ISISJournal::operator=(ISISJournal &&rhs) = default;
 
-/** Get specified data for all runs in a specific instrument and cycle that
- * match given filter criteria.
+/** Get the cycle names
  *
- * @param instrument : the instrument to request data for
- * @param cycle : the ISIS cycle the required data is from e.g. "19_4"
- * @param tags : the tag names of the required values to be returned e.g.
- * "run_number"
- * @param filters : optional tag names and values to filter the results by
+ * @returns : a list of all ISIS cycle names for the instrument
+ * @throws : if there was an error fetching the runs
  */
-std::vector<ISISJournalData> getRunData(std::string const &instrument,
-                                        std::string const &cycle,
-                                        ISISJournalTags const &tags,
-                                        ISISJournalFilters const &filters) {
-  auto const url = constructRunsFileURL(instrument, cycle);
-  auto fileContents = getURLContents(url);
-  return getRunDataFromFile(fileContents, tags, filters);
+std::vector<std::string> ISISJournal::getCycleNames() {
+  if (!m_indexDocument) {
+    auto xmlString = getURLContents(m_indexFileURL);
+    m_indexDocument = parse(xmlString);
+  }
+  auto rootElement = m_indexDocument->documentElement();
+  throwIfNotValid(rootElement, JOURNAL_TAG);
+  auto filenames =
+      getAttributeForAllChildElements(rootElement, FILE_TAG, "name");
+  return convertFilenamesToCycleNames(filenames);
 }
 
-/** Get the list of cycle names from the given index file
- * @param fileContents : the contents of the XML index file
- * @returns : a list of cycle names
+/** Get run names and other specified data for all runs that match the given
+ * filter criteria.
+ *
+ * @param valuesToLookup : optional list of additional values to be returned
+ * e.g. "run_number", "title"
+ * @param filters : optional element names and values to filter the results by
+ * @throws : if there was an error fetching the runs
  */
-std::vector<std::string> getCycleListFromFile(std::string const &fileContents) {
-  auto xmldoc = parse(fileContents);
-  auto root = xmldoc->documentElement();
-  validateElement(root, JOURNAL_TAG);
-  auto filenames = getTagValuesForChildElements(root, FILE_TAG);
-  return convertFilenamesToCycleNames(filenames);
+std::vector<ISISJournal::RunData>
+ISISJournal::getRuns(std::vector<std::string> const &valuesToLookup,
+                     ISISJournal::RunData const &filters) {
+  if (!m_runsDocument) {
+    auto xmlString = getURLContents(m_runsFileURL);
+    m_runsDocument = parse(xmlString);
+  }
+  auto rootElement = m_runsDocument->documentElement();
+  throwIfNotValid(rootElement, NXROOT_TAG);
+  return getValuesForAllElements(rootElement, valuesToLookup, filters);
 }
 
-/** Get the list of cycle names for the given instrument
- * @param instrument : the instrument name
- * @returns : a list of cycle names
+/** Get the contents of a file at a given URL
+ * @param url : the URL to fetch
+ * @returns : the contents of the file as a string
+ * @throws : if there was an error fetching the file
  */
-std::vector<std::string> getCycleList(std::string const &instrument) {
-  auto const url = constructIndexFileURL(instrument);
-  auto fileContents = getURLContents(url);
-  return getCycleList(fileContents);
+std::string ISISJournal::getURLContents(std::string const &url) {
+  std::ostringstream serverReply;
+  auto const statusCode = m_internetHelper->sendRequest(url, serverReply);
+  if (statusCode != Poco::Net::HTTPResponse::HTTP_OK) {
+    throw Kernel::Exception::InternetError(
+        std::string("Failed to access journal file: HTTP Code: ") +
+        std::to_string(statusCode));
+  }
+  return serverReply.str();
 }
+
 } // namespace ISISJournal
 } // namespace DataHandling
 } // namespace Mantid

Framework/DataHandling/test/CMakeLists.txt

@@ -33,7 +33,8 @@ if(CXXTEST_FOUND)
                         HistogramData
                         ${NEXUS_LIBRARIES}
                         ${HDF5_LIBRARIES}
-                        ${HDF5_HL_LIBRARIES})
+                        ${HDF5_HL_LIBRARIES}
+                        gmock)
   add_dependencies(DataHandlingTest Algorithms MDAlgorithms)
   add_dependencies(FrameworkTests DataHandlingTest)
   # Test data

Framework/DataHandling/test/ISISJournalTest.h

@@ -6,171 +6,297 @@
 // SPDX - License - Identifier: GPL - 3.0 +
 #pragma once
 
-#include <cxxtest/TestSuite.h>
-
 #include "MantidDataHandling/ISISJournal.h"
+#include "MantidKernel/Exception.h"
+#include "MantidKernel/InternetHelper.h"
+#include <Poco/Net/HTTPResponse.h>
+
+#include <cxxtest/TestSuite.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
 
 using namespace Mantid::DataHandling::ISISJournal;
+using Mantid::Kernel::InternetHelper;
+using Mantid::Kernel::Exception::InternetError;
+using testing::_;
+using testing::NiceMock;
+using testing::Return;
+
+namespace {
+static constexpr const char *emptyFile = "";
+
+static constexpr const char *badFile = "<NXroot";
+
+static constexpr const char *emptyJournalFile = "\
+<?xml version=\"1.0\" encoding=\"UTF-8\"?>\
+  <NXroot NeXus_version=\"4.3.0\" XML_version=\"mxml\"></NXroot>";
+
+static constexpr const char *emptyIndexFile = "<journal></journal>";
+
+static constexpr const char *invalidIndexFile = "<journal><badtag/></journal>";
+
+static constexpr const char *indexFile = "\
+<journal>\
+  <file name=\"journal.xml\" />\
+  <file name=\"journal_17_1.xml\" />\
+  <file name=\"journal_18_1.xml\" />\
+  <file name=\"journal_19_1.xml\" />\
+  <file name=\"journal_19_2.xml\" />\
+</journal>";
+
+static constexpr const char *journalFile = "\
+  <NXroot>\
+    <NXentry name=\"INTER22345\">\
+      <title>Experiment 2 run 1</title>\
+      <experiment_id>200001</experiment_id>\
+      <run_number> 22345</run_number>\
+      <count> 5  </count>\
+    </NXentry>\
+    <NXentry name=\"INTER12345\">\
+      <title>Experiment 1 run 1</title>\
+      <experiment_id>100001</experiment_id>\
+      <run_number> 12345</run_number>\
+      <count> 3  </count>\
+    </NXentry>\
+    <NXentry name=\"INTER12346\">\
+      <title>Experiment 1 run 2</title>\
+      <experiment_id>100001</experiment_id>\
+      <run_number> 12346</run_number>\
+      <count> 5  </count>\
+    </NXentry>\
+  </NXroot>";
+} // namespace
+
+class MockInternetHelper : public InternetHelper {
+public:
+  // The constructor specifies the string that we want sendRequest to return
+  MockInternetHelper(std::string const &returnString)
+      : m_returnString(returnString){};
+
+  MOCK_METHOD2(sendRequestProxy, int(std::string const &, std::ostream &));
+
+  int sendRequest(std::string const &url, std::ostream &serverReply) override {
+    serverReply << m_returnString;
+    return sendRequestProxy(url, serverReply);
+  }
+
+private:
+  std::string m_returnString;
+};
 
 class ISISJournalTest : public CxxTest::TestSuite {
 public:
-  void test_getRunData_with_empty_file_throws() {
-    TS_ASSERT_THROWS(
-        getRunDataFromFile(emptyFile, defaultTags(), defaultFilters()),
-        std::runtime_error const &);
+  void test_getRuns_requests_correct_url() {
+    auto journal = makeJournal();
+    auto url = std::string(
+        "http://data.isis.rl.ac.uk/journals/ndxinter/journal_19_4.xml");
+    EXPECT_CALL(*m_internetHelper, sendRequestProxy(url, _)).Times(1);
+    journal.getRuns();
+    verifyAndClear();
   }
 
-  void test_getRunData_with_bad_xml_throws() {
-    TS_ASSERT_THROWS(
-        getRunDataFromFile(badFile, defaultTags(), defaultFilters()),
-        std::runtime_error const &);
+  void test_getCycleNames_requests_correct_url() {
+    auto journal = makeJournal(indexFile);
+    auto url = std::string(
+        "http://data.isis.rl.ac.uk/journals/ndxinter/journal_main.xml");
+    EXPECT_CALL(*m_internetHelper, sendRequestProxy(url, _)).Times(1);
+    journal.getCycleNames();
+    verifyAndClear();
   }
 
-  void test_getRunData_with_empty_xml_file_returns_empty_results() {
-    auto results =
-        getRunDataFromFile(emptyJournalFile, defaultTags(), defaultFilters());
-    TS_ASSERT_EQUALS(results, std::vector<ISISJournalData>{});
+  void test_getRuns_when_server_returns_journalFile() {
+    auto journal = makeJournal(journalFile);
+    auto results = journal.getRuns();
+    auto const expected =
+        std::vector<ISISJournal::RunData>{{{"name", "INTER22345"}},
+                                          {{"name", "INTER12345"}},
+                                          {{"name", "INTER12346"}}};
+    TS_ASSERT_EQUALS(results, expected);
+    verifyAndClear();
   }
 
-  void test_getRunData_still_returns_run_names_when_tags_list_is_empty() {
-    auto results =
-        getRunDataFromFile(journalFile, emptyTags(), defaultFilters());
-    auto const expected = std::vector<ISISJournalData>{
+  void test_getCycleNames_when_server_returns_indexFile() {
+    auto journal = makeJournal(indexFile);
+    auto results = journal.getCycleNames();
+    auto const expected =
+        std::vector<std::string>{"17_1", "18_1", "19_1", "19_2"};
+    TS_ASSERT_EQUALS(results, expected);
+    verifyAndClear();
+  }
+
+  void test_getRuns_throws_if_url_not_found() {
+    auto journal = makeJournal();
+    expectURLNotFound();
+    TS_ASSERT_THROWS(journal.getRuns(), InternetError const &);
+    verifyAndClear();
+  }
+
+  void test_getCycleNames_throws_if_url_not_found() {
+    auto journal = makeJournal(indexFile);
+    expectURLNotFound();
+    TS_ASSERT_THROWS(journal.getCycleNames(), InternetError const &);
+    verifyAndClear();
+  }
+
+  void test_getRuns_with_empty_file_throws() {
+    auto journal = makeJournal(emptyFile);
+    TS_ASSERT_THROWS(journal.getRuns(), std::runtime_error const &);
+    verifyAndClear();
+  }
+
+  void test_getRuns_with_bad_xml_throws() {
+    auto journal = makeJournal(badFile);
+    TS_ASSERT_THROWS(journal.getRuns(), std::runtime_error const &);
+    verifyAndClear();
+  }
+
+  void test_getRuns_with_empty_xml_file_returns_empty_results() {
+    auto journal = makeJournal(emptyJournalFile);
+    auto results = journal.getRuns(valuesToLookup(), filters());
+    TS_ASSERT_EQUALS(results, std::vector<ISISJournal::RunData>{});
+    verifyAndClear();
+  }
+
+  void
+  test_getRuns_still_returns_run_names_when_requested_values_list_is_empty() {
+    auto journal = makeJournal();
+    auto results = journal.getRuns(emptyValueNames(), filters());
+    auto const expected = std::vector<ISISJournal::RunData>{
         {{"name", "INTER12345"}}, {{"name", "INTER12346"}}};
   }
 
-  void test_getRunData_returns_all_run_names_when_tags_and_filters_are_empty() {
-    auto results = getRunDataFromFile(journalFile);
+  void
+  test_getRuns_returns_all_run_names_when_values_list_and_filters_are_empty() {
+    auto journal = makeJournal();
+    auto results = journal.getRuns();
     auto const expected =
-        std::vector<ISISJournalData>{{{"name", "INTER22345"}},
-                                     {{"name", "INTER12345"}},
-                                     {{"name", "INTER12346"}}};
+        std::vector<ISISJournal::RunData>{{{"name", "INTER22345"}},
+                                          {{"name", "INTER12345"}},
+                                          {{"name", "INTER12346"}}};
+    verifyAndClear();
   }
 
-  void test_getRunData_returns_requested_tags_filtered_by_one_filter() {
-    auto results =
-        getRunDataFromFile(journalFile, defaultTags(), defaultFilters());
+  void test_getRuns_returns_requested_values_filtered_by_one_filter() {
+    auto journal = makeJournal();
+    auto results = journal.getRuns(valuesToLookup(), filters());
     auto const expected =
-        std::vector<ISISJournalData>{{{"name", "INTER12345"},
-                                      {"run_number", "12345"},
-                                      {"title", "Experiment 1 run 1"}},
-                                     {{"name", "INTER12346"},
-                                      {"run_number", "12346"},
-                                      {"title", "Experiment 1 run 2"}}};
+        std::vector<ISISJournal::RunData>{{{"name", "INTER12345"},
+                                           {"run_number", "12345"},
+                                           {"title", "Experiment 1 run 1"}},
+                                          {{"name", "INTER12346"},
+                                           {"run_number", "12346"},
+                                           {"title", "Experiment 1 run 2"}}};
     TS_ASSERT_EQUALS(results, expected);
+    verifyAndClear();
   }
 
-  void test_getRunData_returns_requested_tags_filtered_by_multiple_filters() {
-    auto results =
-        getRunDataFromFile(journalFile, defaultTags(), multipleFilters());
+  void test_getRuns_returns_requested_values_filtered_by_multiple_filters() {
+    auto journal = makeJournal();
+    auto results = journal.getRuns(valuesToLookup(), multipleFilters());
     auto const expected =
-        std::vector<ISISJournalData>{{{"name", "INTER12346"},
-                                      {"run_number", "12346"},
-                                      {"title", "Experiment 1 run 2"}}};
+        std::vector<ISISJournal::RunData>{{{"name", "INTER12346"},
+                                           {"run_number", "12346"},
+                                           {"title", "Experiment 1 run 2"}}};
     TS_ASSERT_EQUALS(results, expected);
+    verifyAndClear();
   }
 
   void
-  test_getRunData_returns_requested_tags_for_all_entries_when_no_filter_is_set() {
-    auto results =
-        getRunDataFromFile(journalFile, defaultTags(), emptyFilters());
+  test_getRuns_returns_requested_values_for_all_entries_when_no_filter_is_set() {
+    auto journal = makeJournal();
+    auto results = journal.getRuns(valuesToLookup(), emptyFilters());
     auto const expected =
-        std::vector<ISISJournalData>{{{"name", "INTER22345"},
-                                      {"run_number", "22345"},
-                                      {"title", "Experiment 2 run 1"}},
-                                     {{"name", "INTER12345"},
-                                      {"run_number", "12345"},
-                                      {"title", "Experiment 1 run 1"}},
-                                     {{"name", "INTER12346"},
-                                      {"run_number", "12346"},
-                                      {"title", "Experiment 1 run 2"}}};
+        std::vector<ISISJournal::RunData>{{{"name", "INTER22345"},
+                                           {"run_number", "22345"},
+                                           {"title", "Experiment 2 run 1"}},
+                                          {{"name", "INTER12345"},
+                                           {"run_number", "12345"},
+                                           {"title", "Experiment 1 run 1"}},
+                                          {{"name", "INTER12346"},
+                                           {"run_number", "12346"},
+                                           {"title", "Experiment 1 run 2"}}};
     TS_ASSERT_EQUALS(results, expected);
+    verifyAndClear();
   }
 
   void test_getCycleList_with_empty_file_throws() {
-    TS_ASSERT_THROWS(getCycleListFromFile(emptyFile),
-                     std::runtime_error const &);
+    auto journal = makeJournal(emptyFile);
+    TS_ASSERT_THROWS(journal.getCycleNames(), std::runtime_error const &);
+    verifyAndClear();
   }
 
   void test_getCycleList_with_bad_xml_throws() {
-    TS_ASSERT_THROWS(getCycleListFromFile(badFile), std::runtime_error const &);
+    auto journal = makeJournal(badFile);
+    TS_ASSERT_THROWS(journal.getCycleNames(), std::runtime_error const &);
+    verifyAndClear();
   }
 
   void test_getCycleList_with_empty_xml_file_returns_empty_results() {
-    auto results = getCycleListFromFile(emptyIndexFile);
+    auto journal = makeJournal(emptyIndexFile);
+    auto results = journal.getCycleNames();
     TS_ASSERT_EQUALS(results, std::vector<std::string>{});
+    verifyAndClear();
   }
 
   void test_getCycleList_throws_when_invalid_element_names() {
-    TS_ASSERT_THROWS(getCycleListFromFile(invalidIndexFile),
-                     std::invalid_argument const &);
+    auto journal = makeJournal(invalidIndexFile);
+    TS_ASSERT_THROWS(journal.getCycleNames(), std::invalid_argument const &);
+    verifyAndClear();
   }
 
   void test_getCycleList_returns_all_valid_cycles() {
-    auto results = getCycleListFromFile(indexFile);
+    auto journal = makeJournal(indexFile);
+    auto results = journal.getCycleNames();
     auto const expected =
         std::vector<std::string>{"17_1", "18_1", "19_1", "19_2"};
     TS_ASSERT_EQUALS(results, expected);
+    verifyAndClear();
   }
 
 private:
-  ISISJournalTags defaultTags() {
-    return ISISJournalTags{"run_number", "title"};
-  }
+  NiceMock<MockInternetHelper> *m_internetHelper;
 
-  ISISJournalTags emptyTags() { return ISISJournalTags{}; }
-
-  ISISJournalFilters defaultFilters() {
-    return ISISJournalFilters{{"experiment_id", "100001"}};
+  ISISJournal
+  makeJournal(std::string const &xmlContents = std::string(journalFile)) {
+    // Inject a mock internet helper. The journal takes ownership of the
+    // unique_ptr but we store a raw pointer to use in our test.
+    auto internetHelper =
+        std::make_unique<NiceMock<MockInternetHelper>>(xmlContents);
+    m_internetHelper = internetHelper.get();
+    auto journal = ISISJournal("INTER", "19_4", std::move(internetHelper));
+    // Ensure the internet helper returns an ok status by default.
+    auto status = Poco::Net::HTTPResponse::HTTP_OK;
+    ON_CALL(*m_internetHelper, sendRequestProxy(_, _))
+        .WillByDefault(Return(status));
+    return journal;
   }
 
-  ISISJournalFilters multipleFilters() {
-    return ISISJournalFilters{{"experiment_id", "100001"}, {"count", "5"}};
+  std::vector<std::string> valuesToLookup() {
+    return std::vector<std::string>{"run_number", "title"};
   }
 
-  ISISJournalFilters emptyFilters() { return ISISJournalFilters{}; }
-
-  static constexpr const char *emptyFile = "";
-
-  static constexpr const char *badFile = "<NXroot";
+  std::vector<std::string> emptyValueNames() {
+    return std::vector<std::string>{};
+  }
 
-  static constexpr const char *emptyJournalFile = "\
-<?xml version=\"1.0\" encoding=\"UTF-8\"?>\
-  <NXroot NeXus_version=\"4.3.0\" XML_version=\"mxml\"></NXroot>";
+  ISISJournal::RunData filters() {
+    return ISISJournal::RunData{{"experiment_id", "100001"}};
+  }
 
-  static constexpr const char *emptyIndexFile = "<journal></journal>";
+  ISISJournal::RunData multipleFilters() {
+    return ISISJournal::RunData{{"experiment_id", "100001"}, {"count", "5"}};
+  }
 
-  static constexpr const char *invalidIndexFile =
-      "<journal><badtag/></journal>";
+  ISISJournal::RunData emptyFilters() { return ISISJournal::RunData{}; }
 
-  static constexpr const char *indexFile = "\
-<journal>\
-  <file name=\"journal.xml\" />\
-  <file name=\"journal_17_1.xml\" />\
-  <file name=\"journal_18_1.xml\" />\
-  <file name=\"journal_19_1.xml\" />\
-  <file name=\"journal_19_2.xml\" />\
-</journal>";
+  void expectURLNotFound() {
+    auto status = Poco::Net::HTTPResponse::HTTP_NOT_FOUND;
+    EXPECT_CALL(*m_internetHelper, sendRequestProxy(_, _))
+        .Times(1)
+        .WillOnce(Return(status));
+  }
 
-  static constexpr const char *journalFile = "\
-  <NXroot>\
-    <NXentry name=\"INTER22345\">\
-      <title>Experiment 2 run 1</title>\
-      <experiment_id>200001</experiment_id>\
-      <run_number> 22345</run_number>\
-      <count> 5  </count>\
-    </NXentry>\
-    <NXentry name=\"INTER12345\">\
-      <title>Experiment 1 run 1</title>\
-      <experiment_id>100001</experiment_id>\
-      <run_number> 12345</run_number>\
-      <count> 3  </count>\
-    </NXentry>\
-    <NXentry name=\"INTER12346\">\
-      <title>Experiment 1 run 2</title>\
-      <experiment_id>100001</experiment_id>\
-      <run_number> 12346</run_number>\
-      <count> 5  </count>\
-    </NXentry>\
-  </NXroot>";
+  void verifyAndClear() {
+    TS_ASSERT(testing::Mock::VerifyAndClearExpectations(m_internetHelper));
+  }
 };