From b891d9273660597d3a716d5347410cd0d3fdc7fa Mon Sep 17 00:00:00 2001 From: David Rosca Date: Fri, 9 Oct 2015 12:26:13 +0200 Subject: [PATCH] Fix QzTools::ensureUniqueName again This time it should be correct fix + added tests --- src/lib/tools/qztools.cpp | 34 +++++---- tests/autotests/qztoolstest.cpp | 131 ++++++++++++++++++++++++++++++++ tests/autotests/qztoolstest.h | 10 +++ 3 files changed, 160 insertions(+), 15 deletions(-) diff --git a/src/lib/tools/qztools.cpp b/src/lib/tools/qztools.cpp index 7e3773397..83caae0d3 100644 --- a/src/lib/tools/qztools.cpp +++ b/src/lib/tools/qztools.cpp @@ -219,27 +219,31 @@ QString QzTools::escapeSqlString(QString urlString) QString QzTools::ensureUniqueFilename(const QString &name, const QString &appendFormat) { - if (!QFile::exists(name)) { + Q_ASSERT(appendFormat.contains(QL1S("%1"))); + + QFileInfo info(name); + + if (!info.exists()) return name; - } - QString tmpPath = name; + const QDir dir = info.absoluteDir(); + const QString fileName = info.fileName(); + int i = 1; - while (QFile::exists(tmpPath)) { - tmpPath = name; - int fileNameIndex = tmpPath.lastIndexOf(QL1C('/')); - int index = tmpPath.lastIndexOf(QL1C('.'), fileNameIndex); - QString appendString = appendFormat.arg(i); - if (index == -1) { - tmpPath.append(appendString); - } - else { - tmpPath = tmpPath.left(index) + appendString + tmpPath.mid(index); - } + while (info.exists()) { + QString file = fileName; + int index = file.lastIndexOf(QL1C('.')); + const QString appendString = appendFormat.arg(i); + if (index == -1) + file.append(appendString); + else + file = file.left(index) + appendString + file.mid(index); + info.setFile(dir, file); i++; } - return tmpPath; + + return info.absoluteFilePath(); } QString QzTools::getFileNameFromUrl(const QUrl &url) diff --git a/tests/autotests/qztoolstest.cpp b/tests/autotests/qztoolstest.cpp index fe2cc96b4..88d55bb4d 100644 --- a/tests/autotests/qztoolstest.cpp +++ b/tests/autotests/qztoolstest.cpp @@ -18,8 +18,24 @@ #include "qztoolstest.h" #include "qztools.h" +#include #include +void QzToolsTest::initTestCase() +{ + m_tmpPath = QDir::tempPath() + QL1S("/qupzilla-test/qztoolstest"); + QDir().mkpath(m_tmpPath); + + QVERIFY(QDir(m_tmpPath).exists()); +} + +void QzToolsTest::cleanupTestCase() +{ + QDir().rmpath(m_tmpPath); + + QVERIFY(!QDir(m_tmpPath).exists()); +} + void QzToolsTest::samePartOfStrings_data() { QTest::addColumn("string1"); @@ -117,3 +133,118 @@ void QzToolsTest::splitCommandArguments() QCOMPARE(QzTools::splitCommandArguments(command), result); } + +class TempFile +{ + QString name; + +public: + explicit TempFile(const QString &name) + : name(name) + { + QFile file(name); + file.open(QFile::WriteOnly); + file.write(QByteArrayLiteral("qupzilla-test")); + file.close(); + } + + ~TempFile() + { + QFile::remove(name); + } +}; + +void QzToolsTest::ensureUniqueFilename() +{ + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out")), createPath("test.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test")), createPath("test")); + + // default appendFormat = (%1) + { + TempFile f1(createPath("test.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out")), createPath("test(1).out")); + TempFile f2(createPath("test(1).out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out")), createPath("test(2).out")); + TempFile f3(createPath("test(2).out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out")), createPath("test(3).out")); + } + { + TempFile f1(createPath("test")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test")), createPath("test(1)")); + TempFile f2(createPath("test(1)")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test")), createPath("test(2)")); + TempFile f3(createPath("test(2)")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test")), createPath("test(3)")); + } + { + TempFile f1(createPath("test(1)")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test")), createPath("test")); + TempFile f2(createPath("test")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test")), createPath("test(2)")); + } + + // appendFormat = %1 + { + QString appendFormat = QSL("%1"); + + TempFile f1(createPath("test.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out"), appendFormat), createPath("test1.out")); + TempFile f2(createPath("test1.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out"), appendFormat), createPath("test2.out")); + TempFile f3(createPath("test2.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out"), appendFormat), createPath("test3.out")); + } + { + QString appendFormat = QSL("%1"); + + TempFile f1(createPath("test")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test1")); + TempFile f2(createPath("test1")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test2")); + TempFile f3(createPath("test2")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test3")); + } + { + QString appendFormat = QSL("%1"); + + TempFile f1(createPath("test1")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test")); + TempFile f2(createPath("test")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test2")); + } + + // appendFormat = .%1 + { + QString appendFormat = QSL(".%1"); + + TempFile f1(createPath("test.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out"), appendFormat), createPath("test.1.out")); + TempFile f2(createPath("test.1.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out"), appendFormat), createPath("test.2.out")); + TempFile f3(createPath("test.2.out")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test.out"), appendFormat), createPath("test.3.out")); + } + { + QString appendFormat = QSL(".%1"); + + TempFile f1(createPath("test")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test.1")); + TempFile f2(createPath("test.1")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test.2")); + TempFile f3(createPath("test.2")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test.3")); + } + { + QString appendFormat = QSL(".%1"); + + TempFile f1(createPath("test.1")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test")); + TempFile f2(createPath("test")); + QCOMPARE(QzTools::ensureUniqueFilename(createPath("test"), appendFormat), createPath("test.2")); + } +} + +QString QzToolsTest::createPath(const char *file) const +{ + return m_tmpPath + QL1S("/") + file; +} diff --git a/tests/autotests/qztoolstest.h b/tests/autotests/qztoolstest.h index 6d76ae1d5..1aa1f730f 100644 --- a/tests/autotests/qztoolstest.h +++ b/tests/autotests/qztoolstest.h @@ -25,6 +25,9 @@ class QzToolsTest : public QObject Q_OBJECT private slots: + void initTestCase(); + void cleanupTestCase(); + void samePartOfStrings_data(); void samePartOfStrings(); @@ -34,6 +37,13 @@ private slots: void splitCommandArguments_data(); void splitCommandArguments(); + void ensureUniqueFilename(); + +private: + QString createPath(const char *file) const; + + QString m_tmpPath; + }; #endif // QZTOOLSTEST_H