(fix) Computer feature: update removable devices on Linux

src/library/browse/browsefeature.cpp

@@ -19,8 +19,21 @@
 
 namespace {
 
+const QString kViewName = QStringLiteral("BROWSEHOME");
+
 const QString kQuickLinksSeparator = QStringLiteral("-+-");
 
+#if defined(__LINUX__)
+const QStringList removableDriveRootPaths() {
+    QStringList paths;
+    const QString user = QString::fromLocal8Bit(qgetenv("USER"));
+    paths.append("/media");
+    paths.append(QStringLiteral("/media/") + user);
+    paths.append(QStringLiteral("/run/media/") + user);
+    return paths;
+}
+#endif
+
 } // anonymous namespace
 
 BrowseFeature::BrowseFeature(
@@ -79,7 +92,7 @@ BrowseFeature::BrowseFeature(
     // show drive letters
     QFileInfoList drives = QDir::drives();
     // show drive letters
-    foreach (QFileInfo drive, drives) {
+    for (const QFileInfo& drive : std::as_const(drives)) {
         // Using drive.filePath() to get path to display instead of drive.canonicalPath()
         // as it delay the startup too much if there is a network share mounted
         // (drive letter assigned) but unavailable
@@ -123,7 +136,7 @@ BrowseFeature::BrowseFeature(
 
     loadQuickLinks();
 
-    foreach (QString quickLinkPath, m_quickLinkList) {
+    for (const QString& quickLinkPath : std::as_const(m_quickLinkList)) {
         QString name = extractNameFromPath(quickLinkPath);
         qDebug() << "Appending Quick Link: " << name << "---" << quickLinkPath;
         m_pQuickLinkItem->appendChild(name, quickLinkPath);
@@ -222,7 +235,7 @@ void BrowseFeature::bindLibraryWidget(WLibrary* libraryWidget,
     Q_UNUSED(keyboard);
     WLibraryTextBrowser* edit = new WLibraryTextBrowser(libraryWidget);
     edit->setHtml(getRootViewHtml());
-    libraryWidget->registerView("BROWSEHOME", edit);
+    libraryWidget->registerView(kViewName, edit);
 }
 
 void BrowseFeature::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) {
@@ -231,7 +244,7 @@ void BrowseFeature::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) {
 }
 
 void BrowseFeature::activate() {
-    emit switchToView("BROWSEHOME");
+    emit switchToView(kViewName);
     emit disableSearch();
     emit enableCoverArtDisplay(false);
 }
@@ -246,6 +259,7 @@ void BrowseFeature::activateChild(const QModelIndex& index) {
     QString path = item->getData().toString();
     if (path == QUICK_LINK_NODE || path == DEVICE_NODE) {
         emit saveModelState();
+        // Clear the tracks view
         m_browseModel.setPath({});
     } else {
         // Open a security token for this path and if we do not have access, ask
@@ -274,10 +288,6 @@ void BrowseFeature::activateChild(const QModelIndex& index) {
 
 void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex& index) {
     TreeItem *item = static_cast<TreeItem*>(index.internalPointer());
-
-    // Make sure that this is reset when the related TreeItem is deleted.
-    m_pLastRightClickedItem = item;
-
     if (!item) {
         return;
     }
@@ -288,18 +298,22 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex
         return;
     }
 
+    // Make sure that this is reset when TreeItems are deleted in onLazyChildExpandation()
+    m_pLastRightClickedItem = item;
+
     QMenu menu(m_pSidebarWidget);
+
+    // If this a QuickLink show only the Remove action
     if (item->parent()->getData().toString() == QUICK_LINK_NODE) {
         menu.addAction(m_pRemoveQuickLinkAction);
         menu.exec(globalPos);
         onLazyChildExpandation(index);
         return;
     }
 
+    // If path is in the QuickLinks list show only the Remove action
     foreach (const QString& str, m_quickLinkList) {
         if (str == path) {
-            // if path is a QuickLink:
-            // show remove action
             menu.addAction(m_pRemoveQuickLinkAction);
             menu.exec(globalPos);
             onLazyChildExpandation(index);
@@ -319,9 +333,9 @@ std::vector<std::unique_ptr<TreeItem>> createRemovableDevices() {
     std::vector<std::unique_ptr<TreeItem>> ret;
 #if defined(__WINDOWS__)
     // Repopulate drive list
-    QFileInfoList drives = QDir::drives();
+    const QFileInfoList drives = QDir::drives();
     // show drive letters
-    foreach (QFileInfo drive, drives) {
+    for (const QFileInfo& drive : std::as_const(drives)) {
         // Using drive.filePath() instead of drive.canonicalPath() as it
         // freezes interface too much if there is a network share mounted
         // (drive letter assigned) but unavailable
@@ -339,21 +353,19 @@ std::vector<std::unique_ptr<TreeItem>> createRemovableDevices() {
                 drive.filePath())); // Displays C:/
     }
 #elif defined(__LINUX__)
-    // To get devices on Linux, we look for directories under /media and
-    // /run/media/$USER.
     QFileInfoList devices;
-
-    // Add folders under /media to devices.
-    devices += QDir("/media").entryInfoList(
-        QDir::AllDirs | QDir::NoDotAndDotDot);
-
-    // Add folders under /run/media/$USER to devices.
-    QDir run_media_user_dir(QStringLiteral("/run/media/") + QString::fromLocal8Bit(qgetenv("USER")));
-    devices += run_media_user_dir.entryInfoList(
-        QDir::AllDirs | QDir::NoDotAndDotDot);
+    for (const QString& path : removableDriveRootPaths()) {
+        devices += QDir(path).entryInfoList(QDir::AllDirs | QDir::NoDotAndDotDot);
+    }
 
     // Convert devices into a QList<TreeItem*> for display.
-    foreach(QFileInfo device, devices) {
+    for (const QFileInfo& device : std::as_const(devices)) {
+        // On Linux, devices can be mounted in /media and /media/user and /run/media/[user]
+        // but there's no benefit of displaying the [user] dir in Devices.
+        // Show its children but skip the dir itself.
+        if (removableDriveRootPaths().contains(device.absoluteFilePath())) {
+            continue;
+        }
         ret.push_back(std::make_unique<TreeItem>(
                 device.fileName(),
                 QVariant(device.filePath() + QStringLiteral("/"))));
@@ -402,6 +414,13 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) {
 
     // If we are on the special device node
     if (path == DEVICE_NODE) {
+#if defined(__LINUX__)
+        // Tell the model to remove the cached 'hasChildren' states of all sub-
+        // directories when we expand the Device node.
+        // This ensures we show the real dir tree. This is relevant when devices
+        // were unmounted, changed and mounted again.
+        m_pSidebarModel->removeChildDirsFromCache(removableDriveRootPaths());
+#endif
         folders = createRemovableDevices();
     } else {
         // we assume that the path refers to a folder in the file system

src/library/browse/foldertreemodel.cpp

@@ -20,20 +20,18 @@ FolderTreeModel::FolderTreeModel(QObject *parent)
 FolderTreeModel::~FolderTreeModel() {
 }
 
-/* A tree model of the filesystem should be initialized lazy.
- * It will take the universe to iterate over all files over filesystem
- * hasChildren() returns true if a folder has subfolders although
- * we do not know the precise number of subfolders.
- *
- * Note that BrowseFeature inserts folder trees dynamically and rowCount()
- * is only called if necessary.
- */
+// A tree model of the filesystem should be initialized lazy.
+// It will take the universe to iterate over all files over filesystem
+// hasChildren() returns true if a folder has subfolders although
+// we do not know the precise number of subfolders.
+//
+// Note that BrowseFeature inserts folder trees dynamically and rowCount()
+// is only called if necessary.
 bool FolderTreeModel::hasChildren(const QModelIndex& parent) const {
     TreeItem *item = static_cast<TreeItem*>(parent.internalPointer());
-    /* Usually the child count is 0 because we do lazy initialization
-     * However, for, buid-in items such as 'Quick Links' there exist
-     * child items at init time
-     */
+    // Usually the child count is 0 because we do lazy initialization
+    // However, for, buid-in items such as 'Quick Links' there exist
+    // child items at init time
     if (item->getData().toString() == QUICK_LINK_NODE) {
         return true;
     }
@@ -56,18 +54,15 @@ bool FolderTreeModel::directoryHasChildren(const QString& path) const {
     // Acquire a security token for the path.
     const auto dirAccess = mixxx::FileAccess(mixxx::FileInfo(path));
 
-    /*
-     *  The following code is too expensive, general and SLOW since
-     *  QDIR::EntryInfoList returns a full QFileInfolist
-     *
-     *
-     *  QDir dir(item->getData().toString());
-     *  QFileInfoList all = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot);
-     *  return (all.count() > 0);
-     *
-     *  We can benefit from low-level filesystem APIs, i.e.,
-     *  Windows API or SystemCalls
-     */
+    // The following code is too expensive, general and SLOW since
+    // QDIR::EntryInfoList returns a full QFileInfolist
+    //
+    // QDir dir(item->getData().toString());
+    // QFileInfoList all = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot);
+    // return (all.count() > 0);
+    //
+    // We can benefit from low-level filesystem APIs, i.e.,
+    // Windows API or SystemCalls
 
     bool has_children = false;
 
@@ -116,3 +111,38 @@ bool FolderTreeModel::directoryHasChildren(const QString& path) const {
     m_directoryCache[path] = has_children;
     return has_children;
 }
+
+void FolderTreeModel::removeChildDirsFromCache(const QStringList& rootPaths) {
+    // PerformanceTimer time;
+    // const auto start = time.elapsed();
+    if (rootPaths.isEmpty()) {
+        return;
+    }
+
+    // Just a quick check that prevents iterating the cache pointlessly
+    for (const auto& rootPath : rootPaths) {
+        VERIFY_OR_DEBUG_ASSERT(!rootPath.isEmpty()) {
+            // List contains at least one non-empty path
+            break;
+        }
+    }
+
+    // int checked = 0;
+    // int removed = 0;
+    QHashIterator<QString, bool> it(m_directoryCache);
+    while (it.hasNext()) {
+        it.next();
+        // checked++;
+        const QString cachedPath = it.key();
+        for (const auto& rootPath : rootPaths) {
+            if (!rootPath.isEmpty() && cachedPath.startsWith(rootPath)) {
+                m_directoryCache.remove(cachedPath);
+                // removed++;
+            }
+        }
+    }
+
+    // qWarning() << "     checked:" << checked << "| removed:" << removed;
+    // qWarning() << "     elapsed:" << mixxx::Duration(time.elapsed() -
+    // start).debugMicrosWithUnit();
+}

src/library/browse/foldertreemodel.h

@@ -16,8 +16,13 @@ class FolderTreeModel : public TreeItemModel {
     virtual ~FolderTreeModel();
     virtual bool hasChildren(const QModelIndex& parent = QModelIndex()) const;
     bool directoryHasChildren(const QString& path) const;
+    void removeChildDirsFromCache(const QStringList& rootPaths);
 
   private:
-    // Used for memoizing the results of directoryHasChildren
+    // Used for memorizing the results of directoryHasChildren.
+    // Note: this means we won't see directory tree changes after the initial
+    // tree population after first expansion. I.e. newly added directories won't
+    // be displayed and removed dirs will remain in the sidebar tree.
+    // removeChildDirsFromCache() can be used to reset selected directories.
     mutable QHash<QString, bool> m_directoryCache;
 };

src/library/treeitemmodel.cpp

@@ -3,30 +3,29 @@
 #include "library/treeitem.h"
 #include "moc_treeitemmodel.cpp"
 
-/*
- * Just a word about how the TreeItem objects and TreeItemModels are used in general:
- * TreeItems are used by the TreeItemModel class to display tree
- * structures in the sidebar.
- *
- * The constructor has 4 arguments:
- * 1. argument represents a name shown in the sidebar view later on
- * 2. argument represents the absolute path of this tree item
- * 3. argument is a library feature object.
- *    This is necessary because in sidebar.cpp we handle 'activateChid' events
- * 4. the parent TreeItem object
- *    The constructor does not add this TreeItem object to the parent's child list
- *
- * In case of no arguments, the standard constructor creates a
- * root item that is not visible in the sidebar.
- *
- * Once the TreeItem objects are inserted to models, the models take care of their
- * deletion.
- *
- * Examples on how to use TreeItem and TreeItemModels can be found in
- * - playlistfeature.cpp
- * - cratefeature.cpp
- * - *feature.cpp
- */
+// Just a word about how the TreeItem objects and TreeItemModels are used in general:
+// TreeItems are used by the TreeItemModel class to display tree
+// structures in the sidebar.
+//
+// The constructor has 4 arguments:
+// 1. argument represents a name shown in the sidebar view later on
+// 2. argument represents the absolute path of this tree item
+// 3. argument is a library feature object.
+//    This is necessary because in sidebar.cpp we handle 'activateChid' events
+// 4. the parent TreeItem object
+//    The constructor does not add this TreeItem object to the parent's child list
+//
+// In case of no arguments, the standard constructor creates a
+// root item that is not visible in the sidebar.
+//
+// Once the TreeItem objects are inserted to models, the models take care of their
+// deletion.
+//
+// Examples on how to use TreeItem and TreeItemModels can be found in
+// - playlistfeature.cpp
+// - cratefeature.cpp
+// - *feature.cpp
+
 TreeItemModel::TreeItemModel(QObject *parent)
         : QAbstractItemModel(parent),
           m_pRootItem(std::make_unique<TreeItem>()) {
@@ -35,7 +34,7 @@ TreeItemModel::TreeItemModel(QObject *parent)
 TreeItemModel::~TreeItemModel() {
 }
 
-//Our Treeview Model supports exactly a single column
+// Our Treeview Model supports exactly a single column
 int TreeItemModel::columnCount(const QModelIndex &parent) const {
     Q_UNUSED(parent);
     return 1;
@@ -153,10 +152,8 @@ int TreeItemModel::rowCount(const QModelIndex& parent) const {
     return parentItem->childRows();
 }
 
-/**
- * Populates the model and notifies the view.
- * Call this method first, before you do call any other methods.
- */
+// Populates the model and notifies the view.
+// Call this method first, before you do call any other methods.
 TreeItem* TreeItemModel::setRootItem(std::unique_ptr<TreeItem> pRootItem) {
     beginResetModel();
     m_pRootItem = std::move(pRootItem);
@@ -168,10 +165,8 @@ const QModelIndex TreeItemModel::getRootIndex() {
     return createIndex(0, 0, getRootItem());
 }
 
-/**
- * Before you can resize the data model dynamically by using 'insertRows' and 'removeRows'
- * make sure you have initialized.
- */
+// Before you can resize the data model dynamically by using 'insertRows' and 'removeRows'
+// make sure you have initialized.
 void TreeItemModel::insertTreeItemRows(
         std::vector<std::unique_ptr<TreeItem>>&& rows,
         int position,