Martchus
/
tagparser
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Don't use global variable for backup directory

This commit is contained in:
Martchus 2018-07-10 16:34:57 +02:00
parent 1a9f71a3ea
commit a87ad5f5ec
8 changed files with 41 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
backuphelper.cpp
View File

@ -33,20 +33,6 @@ namespace TagParser {
namespace BackupHelper {
/*!
* \brief Returns the directory used to store backup files.
*
* Setting this value allows creation of backup files in a custom location
* instead of the directory of the file being modified.
*
* \todo Add this as member variable to MediaFileInfo to avoid global.
*/
string &backupDirectory()
{
static string backupDir;
return backupDir;
}
/*!
* \brief Restores the original file from the specified backup file.
* \param originalPath Specifies the path to the original file.
@ -111,6 +97,8 @@ static bool isRelative(const std::string &path)
/*!
* \brief Creates a backup file for the specified file.
* \param backupDir Specifies the directory to store backup files. If empty, the directory of the file
* to be backuped is used.
* \param originalPath Specifies the path of the file to be backuped.
* \param backupPath Contains the path of the created backup file when this function returns.
* \param originalStream Specifies a std::fstream for the original file.
@ -132,10 +120,10 @@ static bool isRelative(const std::string &path)
* \throws Throws std::ios_base::failure on failure.
* \todo Implement callback for progress updates (copy).
*/
void createBackupFile(const std::string &originalPath, std::string &backupPath, NativeFileStream &originalStream, NativeFileStream &backupStream)
void createBackupFile(const std::string &backupDir, const std::string &originalPath, std::string &backupPath, NativeFileStream &originalStream,
NativeFileStream &backupStream)
{
// determine dirs
const auto &backupDir(backupDirectory());
const auto backupDirRelative(isRelative(backupDir));
const auto originalDir(backupDirRelative ? BasicFileInfo::containingDirectory(originalPath) : string());

5
backuphelper.h
View File

@ -12,11 +12,10 @@ class Diagnostics;
namespace BackupHelper {
TAG_PARSER_EXPORT std::string &backupDirectory();
TAG_PARSER_EXPORT void restoreOriginalFileFromBackupFile(const std::string &originalPath, const std::string &backupPath,
IoUtilities::NativeFileStream &originalStream, IoUtilities::NativeFileStream &backupStream);
TAG_PARSER_EXPORT void createBackupFile(const std::string &originalPath, std::string &backupPath, IoUtilities::NativeFileStream &originalStream,
IoUtilities::NativeFileStream &backupStream);
TAG_PARSER_EXPORT void createBackupFile(const std::string &backupDir, const std::string &originalPath, std::string &backupPath,
IoUtilities::NativeFileStream &originalStream, IoUtilities::NativeFileStream &backupStream);
TAG_PARSER_EXPORT void handleFailureAfterFileModified(MediaFileInfo &mediaFileInfo, const std::string &backupPath,
IoUtilities::NativeFileStream &outputStream, IoUtilities::NativeFileStream &backupStream, Diagnostics &diag,
const std::string &context = "making file");

2
matroska/matroskacontainer.cpp
View File

@ -1497,7 +1497,7 @@ void MatroskaContainer::internalMakeFile(Diagnostics &diag, AbortableProgressFee
if (fileInfo().saveFilePath().empty()) {
// move current file to temp dir and reopen it as backupStream, recreate original file
try {
BackupHelper::createBackupFile(fileInfo().path(), backupPath, outputStream, backupStream);
BackupHelper::createBackupFile(fileInfo().backupDirectory(), fileInfo().path(), backupPath, outputStream, backupStream);
// recreate original file, define buffer variables
outputStream.open(fileInfo().path(), ios_base::out | ios_base::binary | ios_base::trunc);
} catch (...) {

2
mediafileinfo.cpp
View File

@ -1608,7 +1608,7 @@ void MediaFileInfo::makeMp3File(Diagnostics &diag, AbortableProgressFeedback &pr
if (m_saveFilePath.empty()) {
// move current file to temp dir and reopen it as backupStream, recreate original file
try {
BackupHelper::createBackupFile(path(), backupPath, outputStream, backupStream);
BackupHelper::createBackupFile(backupDirectory(), path(), backupPath, outputStream, backupStream);
// recreate original file, define buffer variables
outputStream.open(path(), ios_base::out | ios_base::binary | ios_base::trunc);
} catch (...) {

22
mediafileinfo.h
View File

@ -120,6 +120,8 @@ public:
void clearParsingResults();
// methods to get, set object behaviour
const std::string &backupDirectory() const;
void setBackupDirectory(const std::string &backupDirectory);
const std::string &saveFilePath() const;
void setSaveFilePath(const std::string &saveFilePath);
const std::string writingApplication() const;
@ -176,6 +178,7 @@ private:
ParsingStatus m_attachmentsParsingStatus;
// fields specifying object behaviour
std::string m_backupDirectory;
std::string m_saveFilePath;
std::string m_writingApplication;
size_t m_minPadding;
@ -339,6 +342,25 @@ inline const std::vector<std::unique_ptr<Id3v2Tag>> &MediaFileInfo::id3v2Tags()
return m_id3v2Tags;
}
/*!
* \brief Returns the directory used to store backup files.
* \remarks If empty, backup files will be stored in the same directory of the file being modified.
* \sa setBackupDirectory()
*/
inline const std::string &MediaFileInfo::backupDirectory() const
{
return m_backupDirectory;
}
/*!
* \brief Sets the directory used to store backup files.
* \remarks If empty, backup files will be stored in the same directory of the file being modified.
*/
inline void MediaFileInfo::setBackupDirectory(const std::string &backupDirectory)
{
m_backupDirectory = backupDirectory;
}
/*!
* \brief Returns the "save file path" which has been set using setSaveFilePath().
* \sa setSaveFilePath()

2
mp4/mp4container.cpp
View File

@ -517,7 +517,7 @@ calculatePadding:
if (fileInfo().saveFilePath().empty()) {
// move current file to temp dir and reopen it as backupStream, recreate original file
try {
BackupHelper::createBackupFile(fileInfo().path(), backupPath, outputStream, backupStream);
BackupHelper::createBackupFile(fileInfo().backupDirectory(), fileInfo().path(), backupPath, outputStream, backupStream);
// recreate original file, define buffer variables
outputStream.open(fileInfo().path(), ios_base::out | ios_base::binary | ios_base::trunc);
} catch (...) {

2
ogg/oggcontainer.cpp
View File

@ -376,7 +376,7 @@ void OggContainer::internalMakeFile(Diagnostics &diag, AbortableProgressFeedback
if (fileInfo().saveFilePath().empty()) {
// move current file to temp dir and reopen it as backupStream, recreate original file
try {
BackupHelper::createBackupFile(fileInfo().path(), backupPath, fileInfo().stream(), backupStream);
BackupHelper::createBackupFile(fileInfo().backupDirectory(), fileInfo().path(), backupPath, fileInfo().stream(), backupStream);
// recreate original file, define buffer variables
fileInfo().stream().open(fileInfo().path(), ios_base::out | ios_base::binary | ios_base::trunc);
} catch (...) {

25
tests/utils.cpp
View File

@ -300,19 +300,16 @@ void UtilitiesTests::testBackupFile()
{
using namespace BackupHelper;
// ensure backup directory is empty, so backups will be created in the same directory
// as the original file
backupDirectory().clear();
// setup testfile
MediaFileInfo file(workingCopyPath("unsupported.bin"));
file.setBackupDirectory(string()); // ensure backup directory is empty, so backups will be created in the same directory as the original file
const auto workingDir(file.containingDirectory());
file.open();
// create backup file
string backupPath1, backupPath2;
NativeFileStream backupStream1, backupStream2;
createBackupFile(file.path(), backupPath1, file.stream(), backupStream1);
createBackupFile(string(), file.path(), backupPath1, file.stream(), backupStream1);
CPPUNIT_ASSERT_EQUAL(workingDir + "/unsupported.bin.bak", backupPath1);
// recreate original file (like the 'make' methods would do to apply changes)
@ -320,7 +317,7 @@ void UtilitiesTests::testBackupFile()
file.stream() << "test1" << endl;
// create a 2nd backup which should not override the first one
createBackupFile(file.path(), backupPath2, file.stream(), backupStream2);
createBackupFile(string(), file.path(), backupPath2, file.stream(), backupStream2);
CPPUNIT_ASSERT_EQUAL(workingDir + "/unsupported.bin.1.bak", backupPath2);
// get rid of 2nd backup, recreate original file
@ -330,9 +327,8 @@ void UtilitiesTests::testBackupFile()
file.stream() << "test2" << endl;
// create backup under another location
backupDirectory() = "bak";
try {
createBackupFile(file.path(), backupPath2, file.stream(), backupStream2);
createBackupFile("bak", file.path(), backupPath2, file.stream(), backupStream2);
CPPUNIT_FAIL("renaming failed because backup dir does not exist");
} catch (...) {
const char *what = catchIoFailure();
@ -340,13 +336,13 @@ void UtilitiesTests::testBackupFile()
}
backupStream2.clear();
workingCopyPathMode("bak/unsupported.bin", WorkingCopyMode::NoCopy);
createBackupFile(file.path(), backupPath2, file.stream(), backupStream2);
createBackupFile("bak", file.path(), backupPath2, file.stream(), backupStream2);
CPPUNIT_ASSERT_EQUAL(workingDir + "/bak/unsupported.bin", backupPath2);
// get rid of 2nd backup (again)
backupStream2.close();
CPPUNIT_ASSERT_EQUAL(0, remove(backupPath2.data()));
CPPUNIT_ASSERT_EQUAL(0, remove(argsToString(workingDir % '/' + backupDirectory()).data()));
CPPUNIT_ASSERT_EQUAL(0, remove((workingDir + "/bak").data()));
// should be able to use backup stream, eg. seek to the end
backupStream1.seekg(0, ios_base::end);
@ -361,11 +357,8 @@ void UtilitiesTests::testBackupFile()
CPPUNIT_ASSERT_EQUAL(0x34_st, static_cast<size_t>(file.stream().get()));
file.close();
// reset backup dir again
backupDirectory().clear();
// restore after user aborted
createBackupFile(file.path(), backupPath1, file.stream(), backupStream1);
createBackupFile(string(), file.path(), backupPath1, file.stream(), backupStream1);
try {
throw OperationAbortedException();
} catch (...) {
@ -379,7 +372,7 @@ void UtilitiesTests::testBackupFile()
}
// restore after error
createBackupFile(file.path(), backupPath1, file.stream(), backupStream1);
createBackupFile(string(), file.path(), backupPath1, file.stream(), backupStream1);
try {
throw Failure();
} catch (...) {
@ -391,7 +384,7 @@ void UtilitiesTests::testBackupFile()
}
// restore after io failure
createBackupFile(file.path(), backupPath1, file.stream(), backupStream1);
createBackupFile(string(), file.path(), backupPath1, file.stream(), backupStream1);
try {
throwIoFailure("simulated IO failure");
} catch (...) {