Pass notification of failure of watches onto the caller.

This is particularly useful for situations where the user might really want to
be notified about a failure, for instance, in a backup application.

Empty paths are not treated as an error in calling, as the user code cannot
really do anything sensible to handle this error, but empty paths should not be
used.

Change-Id: Iddb44fd39f4e3fac5c3f9f60fb7999e1833280a8
Reviewed-by: João Abecasis <joao.abecasis@nokia.com>
Reviewed-by: Denis Dzyubenko <denis.dzyubenko@nokia.com>
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>

diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0
index 84add0d..3dec253 100644 (file)
--- a/dist/changes-5.0.0
+++ b/dist/changes-5.0.0
@@ -170,6 +170,10 @@ QtCore
 * The default value of the property QSortFilterProxyModel::dynamicSortFilter was
   changed from false to true.
 
+* QFileSystemWatcher is now able to return failure in case of errors whilst
+  altering the watchlist in both the singular and QStringList overloads of
+  addPath and removePath.
+
 QtGui
 -----
 * Accessibility has been refactored. The hierachy of accessible objects is implemented via

diff --git a/src/corelib/io/qfilesystemwatcher.cpp b/src/corelib/io/qfilesystemwatcher.cpp
index efa9029..29abf96 100644 (file)
--- a/src/corelib/io/qfilesystemwatcher.cpp
+++ b/src/corelib/io/qfilesystemwatcher.cpp
@@ -425,20 +425,28 @@ QFileSystemWatcher::~QFileSystemWatcher()
     otherwise the fileChanged() signal is emitted when \a path is
     modified, renamed or removed.
 
-    \note There is a system dependent limit to the number of files and
-    directories that can be monitored simultaneously. If this limit
-    has been reached, \a path will not be added to the file system
-    watcher, and a warning message will be printed to \e{stderr}.
+    If the watch was successful, true is returned.
+
+    Reasons for a watch failure are generally system-dependent, but
+    may include the resource not existing, access failures, or the
+    total watch count limit, if the platform has one.
+
+    \note There may be a system dependent limit to the number of
+    files and directories that can be monitored simultaneously.
+    If this limit is been reached, \a path will not be monitored,
+    and false is returned.
 
     \sa addPaths(), removePath()
 */
-void QFileSystemWatcher::addPath(const QString &path)
+bool QFileSystemWatcher::addPath(const QString &path)
 {
     if (path.isEmpty()) {
         qWarning("QFileSystemWatcher::addPath: path is empty");
-        return;
+        return true;
     }
-    addPaths(QStringList(path));
+
+    QStringList paths = addPaths(QStringList(path));
+    return paths.isEmpty();
 }
 
 /*!
@@ -451,23 +459,37 @@ void QFileSystemWatcher::addPath(const QString &path)
     otherwise the fileChanged() signal is emitted when the path is
     modified, renamed, or removed.
 
-    \note There is a system dependent limit to the number of files and
-    directories that can be monitored simultaneously. If this limit
-    has been reached, the excess \a paths will not be added to the
-    file system watcher, and a warning message will be printed to
-    \e{stderr} for each path that could not be added.
+    The return value is a list of paths that could not be watched.
+
+    Reasons for a watch failure are generally system-dependent, but
+    may include the resource not existing, access failures, or the
+    total watch count limit, if the platform has one.
+
+    \note There may be a system dependent limit to the number of
+    files and directories that can be monitored simultaneously.
+    If this limit has been reached, the excess \a paths will not
+    be monitored, and they will be added to the returned QStringList.
 
     \sa addPath(), removePaths()
 */
-void QFileSystemWatcher::addPaths(const QStringList &paths)
+QStringList QFileSystemWatcher::addPaths(const QStringList &paths)
 {
     Q_D(QFileSystemWatcher);
-    if (paths.isEmpty()) {
+
+    QStringList p = paths;
+    QMutableListIterator<QString> it(p);
+
+    while (it.hasNext()) {
+        const QString &path = it.next();
+        if (path.isEmpty())
+            it.remove();
+    }
+
+    if (p.isEmpty()) {
         qWarning("QFileSystemWatcher::addPaths: list is empty");
-        return;
+        return QStringList();
     }
 
-    QStringList p = paths;
     QFileSystemWatcherEngine *engine = 0;
 
     if(!objectName().startsWith(QLatin1String("_qt_autotest_force_engine_"))) {
@@ -495,42 +517,65 @@ void QFileSystemWatcher::addPaths(const QStringList &paths)
     if(engine)
         p = engine->addPaths(p, &d->files, &d->directories);
 
-    if (!p.isEmpty())
-        qWarning("QFileSystemWatcher: failed to add paths: %s",
-                 qPrintable(p.join(QLatin1String(", "))));
+    return p;
 }
 
 /*!
     Removes the specified \a path from the file system watcher.
 
+    If the watch is successfully removed, true is returned.
+
+    Reasons for watch removal failing are generally system-dependent,
+    but may be due to the path having already been deleted, for example.
+
     \sa removePaths(), addPath()
 */
-void QFileSystemWatcher::removePath(const QString &path)
+bool QFileSystemWatcher::removePath(const QString &path)
 {
     if (path.isEmpty()) {
         qWarning("QFileSystemWatcher::removePath: path is empty");
-        return;
+        return true;
     }
-    removePaths(QStringList(path));
+
+    QStringList paths = removePaths(QStringList(path));
+    return paths.isEmpty();
 }
 
 /*!
     Removes the specified \a paths from the file system watcher.
 
+    The return value is a list of paths which were not able to be
+    unwatched successfully.
+
+    Reasons for watch removal failing are generally system-dependent,
+    but may be due to the path having already been deleted, for example.
+
     \sa removePath(), addPaths()
 */
-void QFileSystemWatcher::removePaths(const QStringList &paths)
+QStringList QFileSystemWatcher::removePaths(const QStringList &paths)
 {
-    if (paths.isEmpty()) {
-        qWarning("QFileSystemWatcher::removePaths: list is empty");
-        return;
-    }
     Q_D(QFileSystemWatcher);
+
     QStringList p = paths;
+    QMutableListIterator<QString> it(p);
+
+    while (it.hasNext()) {
+        const QString &path = it.next();
+        if (path.isEmpty())
+            it.remove();
+    }
+
+    if (p.isEmpty()) {
+        qWarning("QFileSystemWatcher::removePaths: list is empty");
+        return QStringList();
+    }
+
     if (d->native)
         p = d->native->removePaths(p, &d->files, &d->directories);
     if (d->poller)
         p = d->poller->removePaths(p, &d->files, &d->directories);
+
+    return p;
 }
 
 /*!

diff --git a/src/corelib/io/qfilesystemwatcher.h b/src/corelib/io/qfilesystemwatcher.h
index 763c8de..a695485 100644 (file)
--- a/src/corelib/io/qfilesystemwatcher.h
+++ b/src/corelib/io/qfilesystemwatcher.h
@@ -64,10 +64,10 @@ public:
     QFileSystemWatcher(const QStringList &paths, QObject *parent = 0);
     ~QFileSystemWatcher();
 
-    void addPath(const QString &file);
-    void addPaths(const QStringList &files);
-    void removePath(const QString &file);
-    void removePaths(const QStringList &files);
+    bool addPath(const QString &file);
+    QStringList addPaths(const QStringList &files);
+    bool removePath(const QString &file);
+    QStringList removePaths(const QStringList &files);
 
     QStringList files() const;
     QStringList directories() const;

diff --git a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp
index d0eb360..ad29c57 100644 (file)
--- a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp
+++ b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp
@@ -105,8 +105,7 @@ void tst_QFileSystemWatcher::basicTest()
     // create watcher, forcing it to use a specific backend
     QFileSystemWatcher watcher;
     watcher.setObjectName(QLatin1String("_qt_autotest_force_engine_") + backend);
-    watcher.removePath(testFile.fileName());
-    watcher.addPath(testFile.fileName());
+    QVERIFY(watcher.addPath(testFile.fileName()));
 
     QSignalSpy changedSpy(&watcher, SIGNAL(fileChanged(const QString &)));
     QVERIFY(changedSpy.isValid());
@@ -140,7 +139,7 @@ void tst_QFileSystemWatcher::basicTest()
     changedSpy.clear();
 
     // remove the watch and modify the file, should not get a signal from the watcher
-    watcher.removePath(testFile.fileName());
+    QVERIFY(watcher.removePath(testFile.fileName()));
     testFile.open(QIODevice::WriteOnly | QIODevice::Truncate);
     testFile.write(QByteArray("hello universe!"));
     testFile.close();
@@ -152,19 +151,19 @@ void tst_QFileSystemWatcher::basicTest()
     QCOMPARE(changedSpy.count(), 0);
 
     // readd the file watch with a relative path
-    watcher.addPath(testFile.fileName().prepend("./"));
+    QVERIFY(watcher.addPath(testFile.fileName().prepend("./")));
     testFile.open(QIODevice::WriteOnly | QIODevice::Truncate);
     testFile.write(QByteArray("hello multiverse!"));
     testFile.close();
 
     QTRY_VERIFY(changedSpy.count() > 0);
 
-    watcher.removePath(testFile.fileName().prepend("./"));
+    QVERIFY(watcher.removePath(testFile.fileName().prepend("./")));
 
     changedSpy.clear();
 
     // readd the file watch
-    watcher.addPath(testFile.fileName());
+    QVERIFY(watcher.addPath(testFile.fileName()));
 
     // change the permissions, should get a signal from the watcher
     testFile.setPermissions(QFile::ReadOwner);
@@ -179,7 +178,7 @@ void tst_QFileSystemWatcher::basicTest()
     changedSpy.clear();
 
     // remove the watch and modify file permissions, should not get a signal from the watcher
-    watcher.removePath(testFile.fileName());
+    QVERIFY(watcher.removePath(testFile.fileName()));
     testFile.setPermissions(QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOther);
 
     // waiting max 5 seconds for notification for file modification to trigger
@@ -189,7 +188,7 @@ void tst_QFileSystemWatcher::basicTest()
     QCOMPARE(changedSpy.count(), 0);
 
     // readd the file watch
-    watcher.addPath(testFile.fileName());
+    QVERIFY(watcher.addPath(testFile.fileName()));
 
     // remove the file, should get a signal from the watcher
     QVERIFY(testFile.remove());
@@ -231,7 +230,7 @@ void tst_QFileSystemWatcher::watchDirectory()
 
     QFileSystemWatcher watcher;
     watcher.setObjectName(QLatin1String("_qt_autotest_force_engine_") + backend);
-    watcher.addPath(testDir.dirName());
+    QVERIFY(watcher.addPath(testDir.dirName()));
 
     QSignalSpy changedSpy(&watcher, SIGNAL(directoryChanged(const QString &)));
     QVERIFY(changedSpy.isValid());
@@ -247,7 +246,7 @@ void tst_QFileSystemWatcher::watchDirectory()
     QString fileName;
 
     // remove the watch, should not get notification of a new file
-    watcher.removePath(testDir.dirName());
+    QVERIFY(watcher.removePath(testDir.dirName()));
     QVERIFY(testFile.open(QIODevice::WriteOnly | QIODevice::Truncate));
     testFile.close();
 
@@ -257,7 +256,7 @@ void tst_QFileSystemWatcher::watchDirectory()
 
     QCOMPARE(changedSpy.count(), 0);
 
-    watcher.addPath(testDir.dirName());
+    QVERIFY(watcher.addPath(testDir.dirName()));
 
     // remove the file again, should get a signal from the watcher
     QVERIFY(testFile.remove());
@@ -300,30 +299,32 @@ void tst_QFileSystemWatcher::addPath()
 {
     QFileSystemWatcher watcher;
     QString home = QDir::homePath();
-    watcher.addPath(home);
+    QVERIFY(watcher.addPath(home));
     QCOMPARE(watcher.directories().count(), 1);
     QCOMPARE(watcher.directories().first(), home);
-    watcher.addPath(home);
+
+    // second watch on an already-watched path should fail
+    QVERIFY(!watcher.addPath(home));
     QCOMPARE(watcher.directories().count(), 1);
 
     // With empty string
     QTest::ignoreMessage(QtWarningMsg, "QFileSystemWatcher::addPath: path is empty");
-    watcher.addPath(QString());
+    QVERIFY(watcher.addPath(QString()));
 }
 
 void tst_QFileSystemWatcher::removePath()
 {
     QFileSystemWatcher watcher;
     QString home = QDir::homePath();
-    watcher.addPath(home);
-    watcher.removePath(home);
+    QVERIFY(watcher.addPath(home));
+    QVERIFY(watcher.removePath(home));
     QCOMPARE(watcher.directories().count(), 0);
-    watcher.removePath(home);
+    QVERIFY(!watcher.removePath(home));
     QCOMPARE(watcher.directories().count(), 0);
 
     // With empty string
     QTest::ignoreMessage(QtWarningMsg, "QFileSystemWatcher::removePath: path is empty");
-    watcher.removePath(QString());
+    QVERIFY(watcher.removePath(QString()));
 }
 
 void tst_QFileSystemWatcher::addPaths()
@@ -331,13 +332,13 @@ void tst_QFileSystemWatcher::addPaths()
     QFileSystemWatcher watcher;
     QStringList paths;
     paths << QDir::homePath() << QDir::currentPath();
-    watcher.addPaths(paths);
+    QCOMPARE(watcher.addPaths(paths), QStringList());
     QCOMPARE(watcher.directories().count(), 2);
 
     // With empty list
     paths.clear();
     QTest::ignoreMessage(QtWarningMsg, "QFileSystemWatcher::addPaths: list is empty");
-    watcher.addPaths(paths);
+    QCOMPARE(watcher.addPaths(paths), QStringList());
 }
 
 void tst_QFileSystemWatcher::removePaths()
@@ -345,9 +346,9 @@ void tst_QFileSystemWatcher::removePaths()
     QFileSystemWatcher watcher;
     QStringList paths;
     paths << QDir::homePath() << QDir::currentPath();
-    watcher.addPaths(paths);
+    QCOMPARE(watcher.addPaths(paths), QStringList());
     QCOMPARE(watcher.directories().count(), 2);
-    watcher.removePaths(paths);
+    QCOMPARE(watcher.removePaths(paths), QStringList());
     QCOMPARE(watcher.directories().count(), 0);
 
     //With empty list
@@ -377,8 +378,8 @@ void tst_QFileSystemWatcher::watchFileAndItsDirectory()
     QFileSystemWatcher watcher;
     watcher.setObjectName(QLatin1String("_qt_autotest_force_engine_") + backend);
 
-    watcher.addPath(testDir.dirName());
-    watcher.addPath(testFileName);
+    QVERIFY(watcher.addPath(testDir.dirName()));
+    QVERIFY(watcher.addPath(testFileName));
 
     QSignalSpy fileChangedSpy(&watcher, SIGNAL(fileChanged(const QString &)));
     QSignalSpy dirChangedSpy(&watcher, SIGNAL(directoryChanged(const QString &)));
@@ -433,7 +434,8 @@ void tst_QFileSystemWatcher::watchFileAndItsDirectory()
     fileChangedSpy.clear();
     dirChangedSpy.clear();
 
-    watcher.removePath(testFileName);
+    // removing a deleted file should fail
+    QVERIFY(!watcher.removePath(testFileName));
     QFile::remove(secondFileName);
 
     timer.start(3000);
@@ -458,8 +460,17 @@ void tst_QFileSystemWatcher::nonExistingFile()
 {
     // Don't crash...
     QFileSystemWatcher watcher;
-    watcher.addPath("file_that_does_not_exist.txt");
-    QVERIFY(true);
+    QVERIFY(!watcher.addPath("file_that_does_not_exist.txt"));
+
+    // Test that the paths returned in error aren't messed with
+    QCOMPARE(watcher.addPaths(QStringList() << "../..//./does-not-exist"),
+                              QStringList() << "../..//./does-not-exist");
+
+    // empty path is not actually a failure
+    QCOMPARE(watcher.addPaths(QStringList() << QString()), QStringList());
+
+    // empty path is not actually a failure
+    QCOMPARE(watcher.removePaths(QStringList() << QString()), QStringList());
 }
 
 void tst_QFileSystemWatcher::removeFileAndUnWatch()
@@ -472,17 +483,17 @@ void tst_QFileSystemWatcher::removeFileAndUnWatch()
         testFile.open(QIODevice::WriteOnly);
         testFile.close();
     }
-    watcher.addPath(filename);
+    QVERIFY(watcher.addPath(filename));
 
     QFile::remove(filename);
-    watcher.removePath(filename);
+    QVERIFY(watcher.removePath(filename));
 
     {
         QFile testFile(filename);
         testFile.open(QIODevice::WriteOnly);
         testFile.close();
     }
-    watcher.addPath(filename);
+    QVERIFY(watcher.addPath(filename));
 }
 
 class SomeSingleton : public QObject