From 57a23dc1d31d5bc271a3bdeba0d5f97c4a2ec86c Mon Sep 17 00:00:00 2001
From: Duc Le <duc.le@stfc.ac.uk>
Date: Mon, 25 Nov 2019 22:41:55 +0000
Subject: [PATCH] Re #27495 - Removed canWrite checks in FileValidator
---
Framework/API/src/FileProperty.cpp | 4 +-
.../DataHandling/src/SaveNexusProcessed.cpp | 24 ++--------
.../Kernel/inc/MantidKernel/FileValidator.h | 4 +-
Framework/Kernel/src/FileValidator.cpp | 46 +------------------
Framework/Kernel/test/FileValidatorTest.h | 15 ------
Framework/Nexus/inc/MantidNexus/NexusFileIO.h | 3 +-
Framework/Nexus/src/NexusFileIO.cpp | 5 +-
7 files changed, 13 insertions(+), 88 deletions(-)
diff --git a/Framework/API/src/FileProperty.cpp b/Framework/API/src/FileProperty.cpp
index a01f64c9969..bb358cb596b 100644
--- a/Framework/API/src/FileProperty.cpp
+++ b/Framework/API/src/FileProperty.cpp
@@ -43,8 +43,8 @@ IValidator_sptr createValidator(unsigned int action,
return boost::make_shared<DirectoryValidator>(action ==
FileProperty::Directory);
} else {
- return boost::make_shared<FileValidator>(
- exts, (action == FileProperty::Load), (action == FileProperty::Save));
+ return boost::make_shared<FileValidator>(exts,
+ (action == FileProperty::Load));
}
}
diff --git a/Framework/DataHandling/src/SaveNexusProcessed.cpp b/Framework/DataHandling/src/SaveNexusProcessed.cpp
index 4aec25790e6..192a716f0f3 100644
--- a/Framework/DataHandling/src/SaveNexusProcessed.cpp
+++ b/Framework/DataHandling/src/SaveNexusProcessed.cpp
@@ -20,7 +20,6 @@
#include "MantidKernel/ArrayProperty.h"
#include "MantidKernel/BoundedValidator.h"
#include "MantidNexus/NexusFileIO.h"
-#include <Poco/File.h>
#include <boost/shared_ptr.hpp>
using namespace Mantid::API;
@@ -288,16 +287,11 @@ void SaveNexusProcessed::doExec(
// get the workspace name to write to file
const std::string wsName = inputWorkspace->getName();
- // If we don't want to append then remove the file if it already exists
+ // If not append, openNexusWrite will open with _CREATES perm and overwrite
const bool append_to_file = getProperty("Append");
- if (!append_to_file && !keepFile) {
- Poco::File file(filename);
- if (file.exists())
- file.remove();
- }
nexusFile->resetProgress(&prog_init);
- nexusFile->openNexusWrite(filename, entryNumber);
+ nexusFile->openNexusWrite(filename, entryNumber, append_to_file || keepFile);
// Equivalent C++ API handle
::NeXus::File cppFile(nexusFile->fileID);
@@ -582,18 +576,6 @@ bool SaveNexusProcessed::processGroups() {
// Then immediately open the file
auto nexusFile = boost::make_shared<Mantid::NeXus::NexusFileIO>();
- /* Unless we have explicity been asked to append to the file. We should assume
- that we can remove any existing
- files of the same name prior to processing. */
- bool append_to_file = this->getProperty("Append");
- if (!append_to_file) {
- const std::string filename = getPropertyValue("Filename");
- Poco::File file(filename);
- if (file.exists()) {
- file.remove();
- }
- }
-
// If we have arrived here then a WorkspaceGroup was passed to the
// InputWorkspace property. Pull out the unrolled workspaces and append an
// entry for each one. We only have a single input workspace property declared
@@ -606,7 +588,7 @@ bool SaveNexusProcessed::processGroups() {
throw std::runtime_error("SaveNexusProcessed: NeXus files do not "
"support nested groups of groups");
}
- this->doExec(ws, nexusFile, true /*keepFile*/, entry);
+ this->doExec(ws, nexusFile, entry > 0 /*keepFile*/, entry);
g_log.information() << "Saving group index " << entry << "\n";
}
}
diff --git a/Framework/Kernel/inc/MantidKernel/FileValidator.h b/Framework/Kernel/inc/MantidKernel/FileValidator.h
index 9f65e54be5e..36cd5fba149 100644
--- a/Framework/Kernel/inc/MantidKernel/FileValidator.h
+++ b/Framework/Kernel/inc/MantidKernel/FileValidator.h
@@ -27,7 +27,7 @@ class MANTID_KERNEL_DLL FileValidator : public TypedValidator<std::string> {
public:
explicit FileValidator(
const std::vector<std::string> &extensions = std::vector<std::string>(),
- bool testFileExists = true, bool testCanWrite = false);
+ bool testFileExists = true);
std::vector<std::string> allowedValues() const override;
IValidator_sptr clone() const override;
@@ -36,8 +36,6 @@ protected:
std::vector<std::string> m_extensions;
/// Flag indicating whether to test for existence of filename
bool m_testExist;
- /// Flag indicating whether to test for the file being writable
- bool m_testCanWrite;
private:
std::string checkValidity(const std::string &value) const override;
diff --git a/Framework/Kernel/src/FileValidator.cpp b/Framework/Kernel/src/FileValidator.cpp
index f53736631a9..0aeb2fde152 100644
--- a/Framework/Kernel/src/FileValidator.cpp
+++ b/Framework/Kernel/src/FileValidator.cpp
@@ -26,9 +26,8 @@ Logger g_log("FileValidator");
* @param testCanWrite :: Flag to check if file writing permissible.
*/
FileValidator::FileValidator(const std::vector<std::string> &extensions,
- bool testFileExists, bool testCanWrite)
- : TypedValidator<std::string>(), m_testExist(testFileExists),
- m_testCanWrite(testCanWrite) {
+ bool testFileExists)
+ : TypedValidator<std::string>(), m_testExist(testFileExists) {
for (const auto &extension : extensions) {
const std::string ext = boost::to_lower_copy(extension);
if (std::find(m_extensions.begin(), m_extensions.end(), ext) ==
@@ -92,47 +91,6 @@ std::string FileValidator::checkValidity(const std::string &value) const {
return "File \"" + abspath + "\" not found";
}
- // If the file is required to be writable...
- if (m_testCanWrite) {
- if (value.empty())
- return "Cannot write to empty filename";
-
- Poco::File file(value);
- // the check for writable is different for whether or not a version exists
- // this is taken from ConfigService near line 443
- if (file.exists()) {
- try {
- if (!file.canWrite())
- return "File \"" + abspath + "\" cannot be written";
- } catch (std::exception &e) {
- g_log.information()
- << "Encountered exception while checking for writable: "
- << e.what();
- }
- } else // if the file doesn't exist try to temporarily create one
- {
- try {
- Poco::Path direc(value);
- if (direc.isAbsolute()) {
- // see if file is writable
- if (Poco::File(direc).canWrite())
- return "";
- else
- return "Cannot write to file \"" + direc.toString() + "\"";
- }
-
- g_log.debug() << "Do not have enough information to validate \""
- << abspath << "\"\n";
- } catch (std::exception &e) {
- g_log.information()
- << "Encountered exception while checking for writable: "
- << e.what();
- } catch (...) {
- g_log.information() << "Unknown exception while checking for writable";
- }
- }
- }
-
// Otherwise we are okay, file extensions are just a suggestion so no
// validation on them is necessary
return "";
diff --git a/Framework/Kernel/test/FileValidatorTest.h b/Framework/Kernel/test/FileValidatorTest.h
index 8a8497c200c..4bf90ac16ed 100644
--- a/Framework/Kernel/test/FileValidatorTest.h
+++ b/Framework/Kernel/test/FileValidatorTest.h
@@ -104,21 +104,6 @@ public:
FileValidator file_val;
TS_ASSERT_EQUALS(file_val.isValid(""), "File \"\" not found");
}
-
- void testCanWrite() {
- std::string filename("myJunkFile_hgfvj.cpp");
-
- // file existance is optional
- FileValidator v1(std::vector<std::string>(), false, true);
- TS_ASSERT_EQUALS(v1.isValid(""), "Cannot write to empty filename");
- TS_ASSERT_EQUALS(v1.isValid(filename), "");
-
- // file existance is required
- FileValidator v2(std::vector<std::string>(), true, true);
- TS_ASSERT_EQUALS(v2.isValid(""), "File \"\" not found");
- TS_ASSERT_EQUALS(v2.isValid(filename),
- "File \"" + filename + "\" not found");
- }
};
#endif /*FILEVALIDATORTEST_H_*/
diff --git a/Framework/Nexus/inc/MantidNexus/NexusFileIO.h b/Framework/Nexus/inc/MantidNexus/NexusFileIO.h
index 41e51ee7e9c..ad2160d5909 100644
--- a/Framework/Nexus/inc/MantidNexus/NexusFileIO.h
+++ b/Framework/Nexus/inc/MantidNexus/NexusFileIO.h
@@ -53,7 +53,8 @@ public:
/// open the nexus file for writing
void openNexusWrite(const std::string &fileName,
- optional_size_t entryNumber = optional_size_t());
+ optional_size_t entryNumber = optional_size_t(),
+ const bool append_to_file = true);
/// write the header ifon for the Mantid workspace format
int writeNexusProcessedHeader(const std::string &title,
const std::string &wsName = "") const;
diff --git a/Framework/Nexus/src/NexusFileIO.cpp b/Framework/Nexus/src/NexusFileIO.cpp
index c1dd7857b63..33c950ebee2 100644
--- a/Framework/Nexus/src/NexusFileIO.cpp
+++ b/Framework/Nexus/src/NexusFileIO.cpp
@@ -81,7 +81,8 @@ void NexusFileIO::resetProgress(Progress *prog) { m_progress = prog; }
// </NXentry>
void NexusFileIO::openNexusWrite(const std::string &fileName,
- NexusFileIO::optional_size_t entryNumber) {
+ NexusFileIO::optional_size_t entryNumber,
+ const bool append_to_file) {
// open named file and entry - file may exist
// @throw Exception::FileError if cannot open Nexus file for writing
//
@@ -93,7 +94,7 @@ void NexusFileIO::openNexusWrite(const std::string &fileName,
// if so open as xml
// format otherwise as compressed hdf5
//
- if (Poco::File(m_filename).exists())
+ if ((Poco::File(m_filename).exists() && append_to_file) || m_filehandle)
mode = NXACC_RDWR;
else {
--
GitLab