Fix QzTools::ensureUniqueName again

This time it should be correct fix + added tests

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)

tests/autotests/qztoolstest.cpp

@@ -18,8 +18,24 @@
 #include "qztoolstest.h"
 #include "qztools.h"
 
+#include <QDir>
 #include <QtTest/QtTest>
 
+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<QString>("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;
+}

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