diff --git a/CMakeLists.txt b/CMakeLists.txt index aee17e375f2..a3e9f89cc9c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -865,7 +865,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/util/dnd.cpp src/util/duration.cpp src/util/experiment.cpp - src/util/file.cpp + src/util/fileaccess.cpp src/util/fileinfo.cpp src/util/imageutils.cpp src/util/indexrange.cpp @@ -1146,7 +1146,7 @@ elseif (UNIX) endif() if(WIN32) - target_compile_definitions(mixxx-lib PRIVATE __WINDOWS__) + target_compile_definitions(mixxx-lib PUBLIC __WINDOWS__) # Helps prevent duplicate symbols target_compile_definitions(mixxx-lib PUBLIC _ATL_MIN_CRT) @@ -1177,7 +1177,7 @@ elseif(UNIX) if(APPLE) target_compile_definitions(mixxx-lib PUBLIC __APPLE__) else() - target_compile_definitions(mixxx-lib PRIVATE __UNIX__) + target_compile_definitions(mixxx-lib PUBLIC __UNIX__) if(CMAKE_SYSTEM_NAME STREQUAL Linux) target_compile_definitions(mixxx-lib PUBLIC __LINUX__) elseif(CMAKE_SYSTEM_NAME MATCHES "^.*BSD$") diff --git a/src/coreservices.cpp b/src/coreservices.cpp index 5eae68f0278..3a76a2f6215 100644 --- a/src/coreservices.cpp +++ b/src/coreservices.cpp @@ -18,6 +18,7 @@ #include "engine/enginemaster.h" #include "library/coverartcache.h" #include "library/library.h" +#include "library/trackcollection.h" #include "library/trackcollectionmanager.h" #include "mixer/playerinfo.h" #include "mixer/playermanager.h" @@ -284,8 +285,7 @@ void CoreServices::initialize(QApplication* pApp) { bool hasChanged_MusicDir = false; - QStringList dirs = m_pLibrary->getDirs(); - if (dirs.size() < 1) { + if (m_pTrackCollectionManager->internalCollection()->loadRootDirs().isEmpty()) { // TODO(XXX) this needs to be smarter, we can't distinguish between an empty // path return value (not sure if this is normally possible, but it is // possible with the Windows 7 "Music" library, which is what diff --git a/src/library/browse/browsefeature.cpp b/src/library/browse/browsefeature.cpp index e18fbd7dac8..560dcf86004 100644 --- a/src/library/browse/browsefeature.cpp +++ b/src/library/browse/browsefeature.cpp @@ -249,21 +249,21 @@ void BrowseFeature::activateChild(const QModelIndex& index) { QString path = item->getData().toString(); if (path == QUICK_LINK_NODE || path == DEVICE_NODE) { - m_browseModel.setPath(MDir()); + m_browseModel.setPath({}); } else { // Open a security token for this path and if we do not have access, ask // for it. - MDir dir(path); - if (!dir.canAccess()) { + auto dir = mixxx::FileAccess(mixxx::FileInfo(path)); + if (!dir.isReadable()) { if (Sandbox::askForAccess(path)) { // Re-create to get a new token. - dir = MDir(path); + dir = mixxx::FileAccess(mixxx::FileInfo(path)); } else { // TODO(rryan): Activate an info page about sandboxing? return; } } - m_browseModel.setPath(dir); + m_browseModel.setPath(std::move(dir)); } emit showTrackModel(&m_proxyModel); emit enableCoverArtDisplay(false); @@ -402,10 +402,10 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) { } else { // we assume that the path refers to a folder in the file system // populate childs - MDir dir(path); + const auto dirAccess = mixxx::FileAccess(mixxx::FileInfo(path)); - QFileInfoList all = dir.dir().entryInfoList( - QDir::Dirs | QDir::NoDotAndDotDot); + QFileInfoList all = dirAccess.info().toQDir().entryInfoList( + QDir::Dirs | QDir::NoDotAndDotDot); // loop through all the item and construct the childs foreach (QFileInfo one, all) { @@ -465,7 +465,6 @@ QString BrowseFeature::extractNameFromPath(const QString& spath) { QStringList BrowseFeature::getDefaultQuickLinks() const { // Default configuration - QStringList mixxxMusicDirs = m_pTrackCollection->getDirectoryDAO().getDirs(); QDir osMusicDir(QStandardPaths::writableLocation( QStandardPaths::MusicLocation)); QDir osDocumentsDir(QStandardPaths::writableLocation( @@ -485,10 +484,11 @@ QStringList BrowseFeature::getDefaultQuickLinks() const { bool osDownloadsDirIncluded = false; bool osDesktopDirIncluded = false; bool osDocumentsDirIncluded = false; - foreach (QString dirPath, mixxxMusicDirs) { - QDir dir(dirPath); + const auto rootDirs = m_pLibrary->trackCollections()->internalCollection()->loadRootDirs(); + for (const mixxx::FileInfo& fileInfo : rootDirs) { + const auto dir = fileInfo.toQDir(); // Skip directories we don't have permission to. - if (!Sandbox::canAccessFile(dir)) { + if (!Sandbox::canAccessDir(dir)) { continue; } if (dir == osMusicDir) { @@ -506,22 +506,22 @@ QStringList BrowseFeature::getDefaultQuickLinks() const { result << dir.canonicalPath() + "/"; } - if (!osMusicDirIncluded && Sandbox::canAccessFile(osMusicDir)) { + if (!osMusicDirIncluded && Sandbox::canAccessDir(osMusicDir)) { result << osMusicDir.canonicalPath() + "/"; } if (downloadsExists && !osDownloadsDirIncluded && - Sandbox::canAccessFile(osDownloadsDir)) { + Sandbox::canAccessDir(osDownloadsDir)) { result << osDownloadsDir.canonicalPath() + "/"; } if (!osDesktopDirIncluded && - Sandbox::canAccessFile(osDesktopDir)) { + Sandbox::canAccessDir(osDesktopDir)) { result << osDesktopDir.canonicalPath() + "/"; } if (!osDocumentsDirIncluded && - Sandbox::canAccessFile(osDocumentsDir)) { + Sandbox::canAccessDir(osDocumentsDir)) { result << osDocumentsDir.canonicalPath() + "/"; } diff --git a/src/library/browse/browsetablemodel.cpp b/src/library/browse/browsetablemodel.cpp index 36a26b5d81b..e51063a7ada 100644 --- a/src/library/browse/browsetablemodel.cpp +++ b/src/library/browse/browsetablemodel.cpp @@ -19,6 +19,7 @@ #include "moc_browsetablemodel.cpp" #include "track/track.h" #include "util/compatibility.h" +#include "util/fileaccess.h" #include "widget/wlibrarytableview.h" BrowseTableModel::BrowseTableModel(QObject* parent, @@ -185,9 +186,8 @@ void BrowseTableModel::addSearchColumn(int index) { m_searchColumns.push_back(index); } -void BrowseTableModel::setPath(const MDir& path) { - m_current_directory = path; - m_pBrowseThread->executePopulation(m_current_directory, this); +void BrowseTableModel::setPath(mixxx::FileAccess path) { + m_pBrowseThread->executePopulation(std::move(path), this); } TrackPointer BrowseTableModel::getTrack(const QModelIndex& index) const { diff --git a/src/library/browse/browsetablemodel.h b/src/library/browse/browsetablemodel.h index 4df99591e1d..e0473e01b72 100644 --- a/src/library/browse/browsetablemodel.h +++ b/src/library/browse/browsetablemodel.h @@ -5,7 +5,6 @@ #include "library/trackmodel.h" #include "recording/recordingmanager.h" -#include "util/file.h" #include "library/browse/browsethread.h" //constants @@ -33,6 +32,12 @@ const int COLUMN_REPLAYGAIN = 20; class TrackCollectionManager; +namespace mixxx { + +class FileAccess; + +} // namespace mixxx + // The BrowseTable models displays tracks // of given directory on the HDD. // Usage: Recording and Browse feature. @@ -47,7 +52,7 @@ class BrowseTableModel final : public QStandardItemModel, public virtual TrackMo BrowseTableModel(QObject* parent, TrackCollectionManager* pTrackCollectionManager, RecordingManager* pRec); virtual ~BrowseTableModel(); - void setPath(const MDir& path); + void setPath(mixxx::FileAccess path); TrackPointer getTrack(const QModelIndex& index) const override; TrackPointer getTrackByRef(const TrackRef& trackRef) const override; @@ -84,7 +89,6 @@ class BrowseTableModel final : public QStandardItemModel, public virtual TrackMo TrackCollectionManager* const m_pTrackCollectionManager; QList m_searchColumns; - MDir m_current_directory; RecordingManager* m_pRecordingManager; BrowseThreadPointer m_pBrowseThread; QString m_previewDeckGroup; diff --git a/src/library/browse/browsethread.cpp b/src/library/browse/browsethread.cpp index eac23d12431..285053aa03e 100644 --- a/src/library/browse/browsethread.cpp +++ b/src/library/browse/browsethread.cpp @@ -1,7 +1,3 @@ -/* - * browsethread.cpp (C) 2011 Tobias Rafreider - */ - #include "library/browse/browsethread.h" #include @@ -65,9 +61,9 @@ BrowseThreadPointer BrowseThread::getInstanceRef() { return strong; } -void BrowseThread::executePopulation(const MDir& path, BrowseTableModel* client) { +void BrowseThread::executePopulation(mixxx::FileAccess path, BrowseTableModel* client) { m_path_mutex.lock(); - m_path = path; + m_path = std::move(path); m_model_observer = client; m_path_mutex.unlock(); m_locationUpdated.wakeAll(); @@ -116,15 +112,16 @@ class YearItem: public QStandardItem { void BrowseThread::populateModel() { m_path_mutex.lock(); - MDir thisPath = m_path; + auto thisPath = m_path; BrowseTableModel* thisModelObserver = m_model_observer; m_path_mutex.unlock(); // Refresh the name filters in case we loaded new SoundSource plugins. QStringList nameFilters(SoundSourceProxy::getSupportedFileNamePatterns()); - QDirIterator fileIt(thisPath.dir().absolutePath(), nameFilters, - QDir::Files | QDir::NoDotAndDotDot); + QDirIterator fileIt(thisPath.info().location(), + nameFilters, + QDir::Files | QDir::NoDotAndDotDot); // remove all rows // This is a blocking operation @@ -139,10 +136,10 @@ void BrowseThread::populateModel() { // If a user quickly jumps through the folders // the current task becomes "dirty" m_path_mutex.lock(); - MDir newPath = m_path; + auto newPath = m_path; m_path_mutex.unlock(); - if (thisPath.dir() != newPath.dir()) { + if (thisPath.info() != newPath.info()) { qDebug() << "Abort populateModel()"; populateModel(); return; diff --git a/src/library/browse/browsethread.h b/src/library/browse/browsethread.h index 08c15630d21..fb24c572e38 100644 --- a/src/library/browse/browsethread.h +++ b/src/library/browse/browsethread.h @@ -4,15 +4,15 @@ #pragma once -#include -#include -#include -#include #include +#include #include +#include +#include +#include #include -#include "util/file.h" +#include "util/fileaccess.h" // This class is a singleton and represents a thread // that is used to read ID3 metadata @@ -30,7 +30,7 @@ class BrowseThread : public QThread { Q_OBJECT public: virtual ~BrowseThread(); - void executePopulation(const MDir& path, BrowseTableModel* client); + void executePopulation(mixxx::FileAccess path, BrowseTableModel* client); void run(); static BrowseThreadPointer getInstanceRef(); @@ -49,7 +49,7 @@ class BrowseThread : public QThread { // You must hold m_path_mutex to touch m_path or m_model_observer QMutex m_path_mutex; - MDir m_path; + mixxx::FileAccess m_path; BrowseTableModel* m_model_observer; static QWeakPointer m_weakInstanceRef; diff --git a/src/library/browse/foldertreemodel.cpp b/src/library/browse/foldertreemodel.cpp index e1d03d1fdd1..1cde96a1826 100644 --- a/src/library/browse/foldertreemodel.cpp +++ b/src/library/browse/foldertreemodel.cpp @@ -16,7 +16,6 @@ #include "library/browse/foldertreemodel.h" #include "library/treeitem.h" #include "moc_foldertreemodel.cpp" -#include "util/file.h" FolderTreeModel::FolderTreeModel(QObject *parent) : TreeItemModel(parent) { @@ -59,7 +58,7 @@ bool FolderTreeModel::directoryHasChildren(const QString& path) const { } // Acquire a security token for the path. - MDir dir(path); + const auto dirAccess = mixxx::FileAccess(mixxx::FileInfo(path)); /* * The following code is too expensive, general and SLOW since diff --git a/src/library/dao/directorydao.cpp b/src/library/dao/directorydao.cpp index 8ba7acae2b2..b94d7102822 100644 --- a/src/library/dao/directorydao.cpp +++ b/src/library/dao/directorydao.cpp @@ -1,80 +1,145 @@ #include "library/dao/directorydao.h" #include -#include #include "library/queryutil.h" +#include "util/db/fwdsqlquery.h" #include "util/db/sqllikewildcardescaper.h" #include "util/db/sqllikewildcards.h" +#include "util/logger.h" namespace { -bool isChildDir(const QString& testDir, const QString& dirStr) { - QDir test = QDir(testDir); - QDir dir = QDir(dirStr); - bool child = dir == test; - while (test.cdUp()) { - if (dir == test) { - child = true; +const mixxx::Logger kLogger("DirectoryDAO"); + +const QString kTable = QStringLiteral("directories"); +const QString kLocationColumn = QStringLiteral("directory"); + +} // anonymous namespace + +QList DirectoryDAO::loadAllDirectories( + bool skipInvalidOrMissing) const { + DEBUG_ASSERT(m_database.isOpen()); + const auto statement = + QStringLiteral("SELECT %1 FROM %2") + .arg( + kLocationColumn, + kTable); + FwdSqlQuery query( + m_database, + statement); + VERIFY_OR_DEBUG_ASSERT(query.execPrepared()) { + return {}; + } + + QList allDirs; + const auto locationIndex = query.fieldIndex(kLocationColumn); + while (query.next()) { + const auto locationValue = + query.fieldValue(locationIndex).toString(); + auto fileInfo = mixxx::FileInfo(locationValue); + if (skipInvalidOrMissing) { + if (!fileInfo.exists() || !fileInfo.isDir()) { + kLogger.debug() + << "Skipping to load invalid or missing directory" + << fileInfo; + continue; + } } + allDirs.append(std::move(fileInfo)); } - // qDebug() << "--- test related function ---"; - // qDebug() << "testDir " << testDir; - // qDebug() << "dir" << dirStr; - // qDebug() << "child = " << child; - // qDebug() << "-----------------------------"; - return child; + return allDirs; } -} // anonymous namespace - -int DirectoryDAO::addDirectory(const QString& newDir) const { - // Do nothing if the dir to add is a child of a directory that is already in - // the db. - QStringList dirs = getDirs(); - QString childDir; - QString parentDir; - foreach (const QString& dir, dirs) { - if (isChildDir(newDir, dir)) { - childDir = dir; +DirectoryDAO::AddResult DirectoryDAO::addDirectory( + const mixxx::FileInfo& newDir) const { + DEBUG_ASSERT(m_database.isOpen()); + if (!newDir.exists() || !newDir.isDir()) { + kLogger.warning() + << "Failed to add" + << newDir + << ": Directory does not exist or is inaccessible"; + return AddResult::InvalidOrMissingDirectory; + } + const auto newCanonicalLocation = newDir.canonicalLocation(); + DEBUG_ASSERT(!newCanonicalLocation.isEmpty()); + QList obsoleteChildDirs; + for (auto&& oldDir : loadAllDirectories()) { + if (!oldDir.exists() || !oldDir.isDir()) { + // Abort to prevent inconsistencies in the database + kLogger.warning() + << "Aborting to add" + << newDir + << ": Loaded directory" + << oldDir + << "does not exist or is inaccessible"; + return AddResult::InvalidOrMissingDirectory; } - if (isChildDir(dir, newDir)) { - parentDir = dir; + const auto oldCanonicalLocation = oldDir.canonicalLocation(); + DEBUG_ASSERT(!oldCanonicalLocation.isEmpty()); + if (mixxx::FileInfo::isRootSubCanonicalLocation( + oldCanonicalLocation, + newCanonicalLocation)) { + return AddResult::AlreadyWatching; + } + if (mixxx::FileInfo::isRootSubCanonicalLocation( + newCanonicalLocation, + oldCanonicalLocation)) { + obsoleteChildDirs.append(std::move(oldDir)); } } - if (!childDir.isEmpty()) { - qDebug() << "return already watching"; - return ALREADY_WATCHING; + const auto statement = + QStringLiteral("INSERT INTO %1 (%2) VALUES (:location)") + .arg( + kTable, + kLocationColumn); + FwdSqlQuery query( + m_database, + statement); + query.bindValue( + QStringLiteral(":location"), + newDir.location()); + VERIFY_OR_DEBUG_ASSERT(query.execPrepared()) { + return AddResult::SqlError; } - if (!parentDir.isEmpty()) { - // removing the old directory won't harm because we are adding the - // parent later in this function - removeDirectory(parentDir); + for (const auto& oldDir : obsoleteChildDirs) { + VERIFY_OR_DEBUG_ASSERT(RemoveResult::Ok == removeDirectory(oldDir)) { + kLogger.warning() + << "Failed to remove obsolete child directory" + << oldDir; + continue; + } } - QSqlQuery query(m_database); - query.prepare("INSERT INTO " % DIRECTORYDAO_TABLE % - " (" % DIRECTORYDAO_DIR % ") VALUES (:dir)"); - query.bindValue(":dir", newDir); - if (!query.exec()) { - LOG_FAILED_QUERY(query) << "Adding new dir (" % newDir % ") failed."; - return SQL_ERROR; - } - return ALL_FINE; + return AddResult::Ok; } -int DirectoryDAO::removeDirectory(const QString& dir) const { - QSqlQuery query(m_database); - query.prepare("DELETE FROM " % DIRECTORYDAO_TABLE % " WHERE " - % DIRECTORYDAO_DIR % "= :dir"); - query.bindValue(":dir", dir); - if (!query.exec()) { - LOG_FAILED_QUERY(query) << "purging dir (" % dir % ") failed"; - return SQL_ERROR; +DirectoryDAO::RemoveResult DirectoryDAO::removeDirectory( + const mixxx::FileInfo& oldDir) const { + DEBUG_ASSERT(m_database.isOpen()); + const auto statement = + QStringLiteral("DELETE FROM %1 WHERE %2=:location") + .arg( + kTable, + kLocationColumn); + FwdSqlQuery query( + m_database, + statement); + query.bindValue( + QStringLiteral(":location"), + oldDir.location()); + VERIFY_OR_DEBUG_ASSERT(query.execPrepared()) { + return RemoveResult::SqlError; } - return ALL_FINE; + + if (query.numRowsAffected() < 1) { + return RemoveResult::NotFound; + } + DEBUG_ASSERT(query.numRowsAffected() == 1); + + return RemoveResult::Ok; } QList DirectoryDAO::relocateDirectory( @@ -85,8 +150,8 @@ QList DirectoryDAO::relocateDirectory( // track location in newFolder then the replace query will fail because the // location column becomes non-unique. QSqlQuery query(m_database); - query.prepare("UPDATE " % DIRECTORYDAO_TABLE % " SET " % DIRECTORYDAO_DIR % - "=:newFolder WHERE " % DIRECTORYDAO_DIR % " = :oldFolder"); + query.prepare("UPDATE " % kTable % " SET " % kLocationColumn % + "=:newFolder WHERE " % kLocationColumn % " = :oldFolder"); query.bindValue(":newFolder", newFolder); query.bindValue(":oldFolder", oldFolder); if (!query.exec()) { @@ -98,16 +163,18 @@ QList DirectoryDAO::relocateDirectory( // on Windows the absolute path starts with the drive name // we also need to check for that QString startsWithOldFolder = SqlLikeWildcardEscaper::apply( - QDir(oldFolder).absolutePath() + "/", kSqlLikeMatchAll) + kSqlLikeMatchAll; + QDir(oldFolder).absolutePath() + "/", kSqlLikeMatchAll) + + kSqlLikeMatchAll; // Also update information in the track_locations table. This is where mixxx // gets the location information for a track. Put marks around %1 so that // this also works on windows - query.prepare(QString("SELECT library.id, track_locations.id, track_locations.location " - "FROM library INNER JOIN track_locations ON " - "track_locations.id = library.location WHERE " - "track_locations.location LIKE '%1' ESCAPE '%2'") - .arg(startsWithOldFolder, kSqlLikeMatchAll)); + query.prepare(QString( + "SELECT library.id, track_locations.id, track_locations.location " + "FROM library INNER JOIN track_locations ON " + "track_locations.id = library.location WHERE " + "track_locations.location LIKE '%1' ESCAPE '%2'") + .arg(startsWithOldFolder, kSqlLikeMatchAll)); if (!query.exec()) { LOG_FAILED_QUERY(query) << "could not relocate path of tracks"; return {}; @@ -125,13 +192,14 @@ QList DirectoryDAO::relocateDirectory( const int oldSuffixLen = oldLocation.size() - oldFolder.size(); QString newLocation = newFolder + oldLocation.right(oldSuffixLen); auto addedTrackRef = TrackRef::fromFileInfo( - TrackFile(newLocation) /*without TrackId*/); + TrackFile(newLocation) /*without TrackId*/); relocatedTracks.append(RelocatedTrack( std::move(missingTrackRef), std::move(addedTrackRef))); } - QString replacement = "UPDATE track_locations SET location = :newloc " + QString replacement = + "UPDATE track_locations SET location = :newloc " "WHERE id = :id"; query.prepare(replacement); for (int i = 0; i < loc_ids.size(); ++i) { @@ -146,17 +214,3 @@ QList DirectoryDAO::relocateDirectory( qDebug() << "Relocated tracks:" << relocatedTracks.size(); return relocatedTracks; } - -QStringList DirectoryDAO::getDirs() const { - QSqlQuery query(m_database); - query.prepare("SELECT " % DIRECTORYDAO_DIR % " FROM " % DIRECTORYDAO_TABLE); - if (!query.exec()) { - LOG_FAILED_QUERY(query) << "could not retrieve directory list from database"; - } - QStringList dirs; - const int dirColumn = query.record().indexOf(DIRECTORYDAO_DIR); - while (query.next()) { - dirs << query.value(dirColumn).toString(); - } - return dirs; -} diff --git a/src/library/dao/directorydao.h b/src/library/dao/directorydao.h index 5478cd699d4..452532f094f 100644 --- a/src/library/dao/directorydao.h +++ b/src/library/dao/directorydao.h @@ -4,25 +4,31 @@ #include "library/dao/dao.h" #include "library/relocatedtrack.h" - -const QString DIRECTORYDAO_DIR = "directory"; -const QString DIRECTORYDAO_TABLE = "directories"; - -enum ReturnCodes { - SQL_ERROR, - ALREADY_WATCHING, - ALL_FINE -}; +#include "util/fileinfo.h" class DirectoryDAO : public DAO { public: ~DirectoryDAO() override = default; - QStringList getDirs() const; - - int addDirectory(const QString& dir) const; - int removeDirectory(const QString& dir) const; - + QList loadAllDirectories( + bool skipInvalidOrMissing = false) const; + + enum class AddResult { + Ok, + AlreadyWatching, + InvalidOrMissingDirectory, + SqlError, + }; + AddResult addDirectory(const mixxx::FileInfo& newDir) const; + + enum class RemoveResult { + Ok, + NotFound, + SqlError, + }; + RemoveResult removeDirectory(const mixxx::FileInfo& oldDir) const; + + // TODO: Move this function out of the DAO QList relocateDirectory( const QString& oldFolder, const QString& newFolder) const; diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index c5fcce5670d..a811b8dd536 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -36,7 +36,7 @@ #include "util/db/sqllikewildcards.h" #include "util/db/sqlstringformatter.h" #include "util/db/sqltransaction.h" -#include "util/file.h" +#include "util/fileinfo.h" #include "util/logger.h" #include "util/math.h" #include "util/qt.h" @@ -1944,7 +1944,7 @@ void TrackDAO::hideAllTracks(const QDir& rootDir) const { } bool TrackDAO::verifyRemainingTracks( - const QStringList& libraryRootDirs, + const QList& libraryRootDirs, volatile const bool* pCancel) { // This function is called from the LibraryScanner Thread, which also has a // transaction running, so we do NOT NEED to use one here @@ -1973,8 +1973,8 @@ bool TrackDAO::verifyRemainingTracks( while (query.next()) { trackLocation = query.value(locationColumn).toString(); int fs_deleted = 0; - for (const auto& dir: libraryRootDirs) { - if (trackLocation.startsWith(dir)) { + for (const auto& rootDir : libraryRootDirs) { + if (trackLocation.startsWith(rootDir.location())) { // Track is under the library root, // but was not verified. // This happens if the track was deleted diff --git a/src/library/dao/trackdao.h b/src/library/dao/trackdao.h index a805b19f40d..e707fb3ddca 100644 --- a/src/library/dao/trackdao.h +++ b/src/library/dao/trackdao.h @@ -1,15 +1,15 @@ #pragma once #include +#include #include #include -#include #include #include -#include "preferences/usersettings.h" #include "library/dao/dao.h" #include "library/relocatedtrack.h" +#include "preferences/usersettings.h" #include "track/globaltrackcache.h" #include "util/class.h" #include "util/memory.h" @@ -21,6 +21,12 @@ class AnalysisDao; class CueDAO; class LibraryHashDAO; +namespace mixxx { + +class FileInfo; + +} // namespace mixxx + class TrackDAO : public QObject, public virtual DAO, public virtual GlobalTrackCacheRelocator { Q_OBJECT public: @@ -149,7 +155,7 @@ class TrackDAO : public QObject, public virtual DAO, public virtual GlobalTrackC void markUnverifiedTracksAsDeleted(); bool verifyRemainingTracks( - const QStringList& libraryRootDirs, + const QList& libraryRootDirs, volatile const bool* pCancel); void detectCoverArtForTracksWithoutCover(volatile const bool* pCancel, diff --git a/src/library/export/engineprimeexportjob.cpp b/src/library/export/engineprimeexportjob.cpp index 166b845ed2b..68d8697203e 100644 --- a/src/library/export/engineprimeexportjob.cpp +++ b/src/library/export/engineprimeexportjob.cpp @@ -330,14 +330,14 @@ void EnginePrimeExportJob::loadIds(const QSet& crateIds) { // of unique track refs from all directories in the library. qDebug() << "Loading all track refs and crate ids..."; QSet trackRefs; - const auto dirs = m_pTrackCollectionManager->internalCollection() - ->getDirectoryDAO() - .getDirs(); - for (const auto& dir : dirs) { + const auto dirInfos = m_pTrackCollectionManager->internalCollection() + ->getDirectoryDAO() + .loadAllDirectories(); + for (const mixxx::FileInfo& dirInfo : dirInfos) { const auto trackRefsFromDir = m_pTrackCollectionManager ->internalCollection() ->getTrackDAO() - .getAllTrackRefs(dir); + .getAllTrackRefs(dirInfo.toQDir()); for (const auto& trackRef : trackRefsFromDir) { trackRefs.insert(trackRef); } diff --git a/src/library/library.cpp b/src/library/library.cpp index 9d256159dc8..89036c59c77 100644 --- a/src/library/library.cpp +++ b/src/library/library.cpp @@ -217,12 +217,25 @@ Library::Library( // On startup we need to check if all of the user's library folders are // accessible to us. If the user is using a database from <1.12.0 with // sandboxing then we will need them to give us permission. - qDebug() << "Checking for access to user's library directories:"; - foreach (QString directoryPath, getDirs()) { - QFileInfo directory(directoryPath); - bool hasAccess = Sandbox::askForAccess(directory.canonicalFilePath()); - qDebug() << "Checking for access to" << directoryPath << ":" - << hasAccess; + const auto rootDirs = m_pTrackCollectionManager->internalCollection()->loadRootDirs(); + for (const mixxx::FileInfo& dirInfo : rootDirs) { + if (!dirInfo.exists() || !dirInfo.isDir()) { + kLogger.warning() + << "Skipping access check for missing or invalid directory" + << dirInfo; + continue; + } + if (Sandbox::askForAccess(dirInfo.canonicalLocation())) { + kLogger.info() + << "Access to directory" + << dirInfo + << "from sandbox granted"; + } else { + kLogger.warning() + << "Access to directory" + << dirInfo + << "from sandbox denied"; + } } m_iTrackTableRowHeight = m_pConfig->getValue( @@ -486,9 +499,9 @@ void Library::slotRequestAddDir(const QString& dir) { // to canonicalize the path so we first wrap the directory string with a // QDir. QDir directory(dir); - Sandbox::createSecurityToken(directory); + Sandbox::createSecurityTokenForDir(directory); - if (!m_pTrackCollectionManager->addDirectory(dir)) { + if (!m_pTrackCollectionManager->addDirectory(mixxx::FileInfo(dir))) { QMessageBox::information(nullptr, tr("Add Directory to Library"), tr("Could not add the directory to your library. Either this " @@ -503,6 +516,11 @@ void Library::slotRequestAddDir(const QString& dir) { } void Library::slotRequestRemoveDir(const QString& dir, RemovalType removalType) { + // Remove the directory from the directory list. + if (!m_pTrackCollectionManager->removeDirectory(mixxx::FileInfo(dir))) { + return; + } + switch (removalType) { case RemovalType::KeepTracks: break; @@ -519,21 +537,19 @@ void Library::slotRequestRemoveDir(const QString& dir, RemovalType removalType) DEBUG_ASSERT(!"unreachable"); } - // Remove the directory from the directory list. - m_pTrackCollectionManager->removeDirectory(dir); - // Also update the config file if necessary so that downgrading is still // possible. QString confDir = m_pConfig->getValueString(PREF_LEGACY_LIBRARY_DIR); if (QDir(dir) == QDir(confDir)) { - QStringList dirList = getDirs(); - if (!dirList.isEmpty()) { - m_pConfig->set(PREF_LEGACY_LIBRARY_DIR, dirList.first()); - } else { + const QList dirList = + m_pTrackCollectionManager->internalCollection()->loadRootDirs(); + if (dirList.isEmpty()) { // Save empty string so that an old version of mixxx knows it has to // ask for a new directory. m_pConfig->set(PREF_LEGACY_LIBRARY_DIR, QString()); + } else { + m_pConfig->set(PREF_LEGACY_LIBRARY_DIR, dirList.first().location()); } } } @@ -549,10 +565,6 @@ void Library::slotRequestRelocateDir(const QString& oldDir, const QString& newDi } } -QStringList Library::getDirs() { - return m_pTrackCollectionManager->internalCollection()->getDirectoryDAO().getDirs(); -} - void Library::setFont(const QFont& font) { QFontMetrics currMetrics(m_trackTableFont); QFontMetrics newMetrics(font); diff --git a/src/library/library.h b/src/library/library.h index bab0abe4041..8bb8617c3ca 100644 --- a/src/library/library.h +++ b/src/library/library.h @@ -75,7 +75,6 @@ class Library: public QObject { KeyboardEventFilter* pKeyboard); void addFeature(LibraryFeature* feature); - QStringList getDirs(); int getTrackTableRowHeight() const { return m_iTrackTableRowHeight; diff --git a/src/library/recording/dlgrecording.cpp b/src/library/recording/dlgrecording.cpp index 16805bc69fc..2df756d29e5 100644 --- a/src/library/recording/dlgrecording.cpp +++ b/src/library/recording/dlgrecording.cpp @@ -83,7 +83,7 @@ DlgRecording::DlgRecording( m_proxyModel.setFilterCaseSensitivity(Qt::CaseInsensitive); m_proxyModel.setSortCaseSensitivity(Qt::CaseInsensitive); - m_browseModel.setPath(m_recordingDir); + m_browseModel.setPath(mixxx::FileAccess(mixxx::FileInfo(m_recordingDir))); m_pTrackTableView->loadTrackModel(&m_proxyModel); connect(pushButtonRecording, @@ -105,7 +105,7 @@ DlgRecording::~DlgRecording() { void DlgRecording::onShow() { m_recordingDir = m_pRecordingManager->getRecordingDir(); - m_browseModel.setPath(m_recordingDir); + m_browseModel.setPath(mixxx::FileAccess(mixxx::FileInfo(m_recordingDir))); } bool DlgRecording::hasFocus() const { @@ -113,7 +113,7 @@ bool DlgRecording::hasFocus() const { } void DlgRecording::refreshBrowseModel() { - m_browseModel.setPath(m_recordingDir); + m_browseModel.setPath(mixxx::FileAccess(mixxx::FileInfo(m_recordingDir))); } void DlgRecording::onSearch(const QString& text) { @@ -168,7 +168,7 @@ void DlgRecording::slotRecordingStateChanged(bool isRecording) { labelRecStatistics->hide(); } //This will update the recorded track table view - m_browseModel.setPath(m_recordingDir); + m_browseModel.setPath(mixxx::FileAccess(mixxx::FileInfo(m_recordingDir))); } // gets number of recorded bytes and update label diff --git a/src/library/rekordbox/rekordboxfeature.cpp b/src/library/rekordbox/rekordboxfeature.cpp index c06d68b3a7f..c0dc9f8668d 100644 --- a/src/library/rekordbox/rekordboxfeature.cpp +++ b/src/library/rekordbox/rekordboxfeature.cpp @@ -26,7 +26,6 @@ #include "util/color/color.h" #include "util/db/dbconnectionpooled.h" #include "util/db/dbconnectionpooler.h" -#include "util/file.h" #include "util/sandbox.h" #include "waveform/waveform.h" #include "widget/wlibrary.h" diff --git a/src/library/scanner/libraryscanner.cpp b/src/library/scanner/libraryscanner.cpp index f75e24c2466..62a117da248 100644 --- a/src/library/scanner/libraryscanner.cpp +++ b/src/library/scanner/libraryscanner.cpp @@ -12,7 +12,7 @@ #include "util/db/dbconnectionpooled.h" #include "util/db/dbconnectionpooler.h" #include "util/db/fwdsqlquery.h" -#include "util/file.h" +#include "util/fileaccess.h" #include "util/logger.h" #include "util/performancetimer.h" #include "util/timer.h" @@ -163,7 +163,7 @@ void LibraryScanner::slotStartScan() { cleanUpDatabase(m_libraryHashDao.database()); // Recursively scan each directory in the directories table. - m_libraryRootDirs = m_directoryDao.getDirs(); + m_libraryRootDirs = m_directoryDao.loadAllDirectories(); // If there are no directories then we have nothing to do. Cleanup and // finish the scan immediately. if (m_libraryRootDirs.isEmpty()) { @@ -219,17 +219,19 @@ void LibraryScanner::slotStartScan() { this, &LibraryScanner::slotFinishHashedScan); - foreach (const QString& dirPath, m_libraryRootDirs) { + for (const mixxx::FileInfo& rootDir : qAsConst(m_libraryRootDirs)) { // Acquire a security bookmark for this directory if we are in a // sandbox. For speed we avoid opening security bookmarks when recursive // scanning so that relies on having an open bookmark for the containing // directory. - MDir dir(dirPath); - if (!m_scannerGlobal->testAndMarkDirectoryScanned(dir.dir())) { - queueTask(new RecursiveScanDirectoryTask(this, m_scannerGlobal, - dir.dir(), - dir.token(), - false)); + if (!rootDir.exists() || !rootDir.isDir()) { + qWarning() << "Skipping to scan" << rootDir; + continue; + } + auto dirAccess = mixxx::FileAccess(rootDir); + if (!m_scannerGlobal->testAndMarkDirectoryScanned(rootDir.toQDir())) { + queueTask(new RecursiveScanDirectoryTask( + this, m_scannerGlobal, std::move(dirAccess), false)); } } pWatcher->taskDone(); @@ -264,13 +266,11 @@ void LibraryScanner::slotFinishHashedScan() { this, &LibraryScanner::slotFinishUnhashedScan); - foreach (const DirInfo& dirInfo, m_scannerGlobal->unhashedDirs()) { + for (mixxx::FileAccess dirAccess : m_scannerGlobal->unhashedDirs()) { // no testAndMarkDirectoryScanned() here, because all unhashedDirs() // are already tracked - queueTask(new RecursiveScanDirectoryTask(this, m_scannerGlobal, - dirInfo.dir(), - dirInfo.token(), - true)); + queueTask(new RecursiveScanDirectoryTask( + this, m_scannerGlobal, std::move(dirAccess), true)); } pWatcher->taskDone(); } diff --git a/src/library/scanner/libraryscanner.h b/src/library/scanner/libraryscanner.h index f9a82a560a7..f89608323ad 100644 --- a/src/library/scanner/libraryscanner.h +++ b/src/library/scanner/libraryscanner.h @@ -2,10 +2,10 @@ #include +#include #include #include #include -#include #include #include @@ -119,6 +119,6 @@ class LibraryScanner : public QThread { // this is accessed main and LibraryScanner thread volatile ScannerState m_state; - QStringList m_libraryRootDirs; + QList m_libraryRootDirs; QScopedPointer m_pProgressDlg; }; diff --git a/src/library/scanner/recursivescandirectorytask.cpp b/src/library/scanner/recursivescandirectorytask.cpp index bf72cf2e04e..3c8e8152c3b 100644 --- a/src/library/scanner/recursivescandirectorytask.cpp +++ b/src/library/scanner/recursivescandirectorytask.cpp @@ -9,11 +9,12 @@ #include "util/timer.h" RecursiveScanDirectoryTask::RecursiveScanDirectoryTask( - LibraryScanner* pScanner, const ScannerGlobalPointer scannerGlobal, - const QDir& dir, SecurityTokenPointer pToken, bool scanUnhashed) + LibraryScanner* pScanner, + const ScannerGlobalPointer& scannerGlobal, + const mixxx::FileAccess&& dirAccess, + bool scanUnhashed) : ScannerTask(pScanner, scannerGlobal), - m_dir(dir), - m_pToken(pToken), + m_dirAccess(std::move(dirAccess)), m_scanUnhashed(scanUnhashed) { } @@ -32,14 +33,13 @@ void RecursiveScanDirectoryTask::run() { // a QDirIterator with a QDir instead of a QString -- but it inherits its // Filter from the QDir so we have to set it first. If the QDir has not done // any FS operations yet then this should be lightweight. - m_dir.setFilter(QDir::Dirs | QDir::Files | QDir::NoDotAndDotDot); - QDirIterator it(m_dir); + auto dir = m_dirAccess.info().toQDir(); + dir.setFilter(QDir::Dirs | QDir::Files | QDir::NoDotAndDotDot); + QDirIterator it(dir); - QString currentFile; - QFileInfo currentFileInfo; std::list filesToImport; std::list possibleCovers; - std::list dirsToScan; + std::list dirsToScan; QCryptographicHash hasher(QCryptographicHash::Sha256); @@ -51,8 +51,8 @@ void RecursiveScanDirectoryTask::run() { m_scannerGlobal->supportedCoverExtensionsRegex(); while (it.hasNext()) { - currentFile = it.next(); - currentFileInfo = it.fileInfo(); + QString currentFile = it.next(); + QFileInfo currentFileInfo = it.fileInfo(); if (currentFileInfo.isFile()) { const QString& fileName = currentFileInfo.fileName(); @@ -69,18 +69,17 @@ void RecursiveScanDirectoryTask::run() { // Art Folder since it is probably a waste of time. continue; } - const QDir currentDir(currentFile); - dirsToScan.push_back(currentDir); + dirsToScan.push_back(mixxx::FileInfo(std::move(currentFileInfo))); } } // Calculate a hash of the directory's file list. const mixxx::cache_key_t newHash = mixxx::cacheKeyFromMessageDigest(hasher.result()); - QString dirPath = m_dir.path(); + QString dirLocation = m_dirAccess.info().location(); // Try to retrieve a hash from the last time that directory was scanned. - const mixxx::cache_key_t prevHash = m_scannerGlobal->directoryHashInDatabase(dirPath); + const mixxx::cache_key_t prevHash = m_scannerGlobal->directoryHashInDatabase(dirLocation); const bool prevHashExists = mixxx::isValidCacheKey(prevHash); if (prevHashExists || m_scanUnhashed) { @@ -90,29 +89,36 @@ void RecursiveScanDirectoryTask::run() { // Rescan that mofo! If importing fails then the scan was cancelled so // we return immediately. if (!filesToImport.empty()) { - m_pScanner->queueTask( - new ImportFilesTask(m_pScanner, m_scannerGlobal, dirPath, - prevHashExists, newHash, filesToImport, - possibleCovers, m_pToken)); + m_pScanner->queueTask(new ImportFilesTask(m_pScanner, + m_scannerGlobal, + dirLocation, + prevHashExists, + newHash, + filesToImport, + possibleCovers, + m_dirAccess.token())); } else { - emit directoryHashedAndScanned(dirPath, !prevHashExists, newHash); + emit directoryHashedAndScanned(dirLocation, !prevHashExists, newHash); } } else { - emit directoryUnchanged(dirPath); + emit directoryUnchanged(dirLocation); } } else { - m_scannerGlobal->addUnhashedDir(m_dir, m_pToken); + m_scannerGlobal->addUnhashedDir(m_dirAccess); } // Process all of the sub-directories. - foreach (const QDir& nextDir, dirsToScan) { + for (const mixxx::FileInfo& dirInfo : dirsToScan) { // Atomically test and mark the directory as scanned to avoid // that the same directory is scanned multiple times by different // tasks. - if (!m_scannerGlobal->testAndMarkDirectoryScanned(nextDir)) { + if (!m_scannerGlobal->testAndMarkDirectoryScanned(dirInfo.toQDir())) { m_pScanner->queueTask( - new RecursiveScanDirectoryTask(m_pScanner, m_scannerGlobal, - nextDir, m_pToken, m_scanUnhashed)); + new RecursiveScanDirectoryTask( + m_pScanner, + m_scannerGlobal, + mixxx::FileAccess(dirInfo, m_dirAccess.token()), + m_scanUnhashed)); } } setSuccess(true); diff --git a/src/library/scanner/recursivescandirectorytask.h b/src/library/scanner/recursivescandirectorytask.h index 5a7440cb837..d22950740fe 100644 --- a/src/library/scanner/recursivescandirectorytask.h +++ b/src/library/scanner/recursivescandirectorytask.h @@ -3,7 +3,7 @@ #include #include "library/scanner/scannertask.h" -#include "util/sandbox.h" +#include "util/fileaccess.h" /// Recursively scan a music library. Doesn't import tracks for any directories /// that have already been scanned and have not changed. Changes are tracked by @@ -13,18 +13,15 @@ class RecursiveScanDirectoryTask : public ScannerTask { Q_OBJECT public: - RecursiveScanDirectoryTask(LibraryScanner* pScanner, - const ScannerGlobalPointer scannerGlobal, - const QDir& dir, - SecurityTokenPointer pToken, - bool scanUnhashed); - virtual ~RecursiveScanDirectoryTask() {} + const ScannerGlobalPointer& scannerGlobal, + const mixxx::FileAccess&& dirAccess, + bool scanUnhashed); + ~RecursiveScanDirectoryTask() override = default; - virtual void run(); + void run() override; private: - QDir m_dir; - SecurityTokenPointer m_pToken; - bool m_scanUnhashed; + const mixxx::FileAccess m_dirAccess; + const bool m_scanUnhashed; }; diff --git a/src/library/scanner/scannerglobal.h b/src/library/scanner/scannerglobal.h index d33c83eb9e5..ed89f031185 100644 --- a/src/library/scanner/scannerglobal.h +++ b/src/library/scanner/scannerglobal.h @@ -10,39 +10,17 @@ #include #include "util/cache.h" +#include "util/fileaccess.h" #include "util/performancetimer.h" -#include "util/sandbox.h" #include "util/task.h" -class DirInfo { - public: - DirInfo(const QDir& dir, - const SecurityTokenPointer& token) - : m_dir(dir), - m_token(token) { - } - - const QDir& dir() const { - return m_dir; - } - - const SecurityTokenPointer& token() const { - return m_token; - } - - private: - QDir m_dir; - SecurityTokenPointer m_token; -}; - - class ScannerGlobal { public: ScannerGlobal(const QSet& trackLocations, - const QHash& directoryHashes, - const QRegExp& supportedExtensionsMatcher, - const QRegExp& supportedCoverExtensionsMatcher, - const QStringList& directoriesBlacklist) + const QHash& directoryHashes, + const QRegExp& supportedExtensionsMatcher, + const QRegExp& supportedCoverExtensionsMatcher, + const QStringList& directoriesBlacklist) : m_trackLocations(trackLocations), m_directoryHashes(directoryHashes), m_supportedExtensionsMatcher(supportedExtensionsMatcher), @@ -59,7 +37,7 @@ class ScannerGlobal { } // Returns whether the track already exists in the database. - inline bool trackExistsInDatabase(const QString& trackLocation) const { + bool trackExistsInDatabase(const QString& trackLocation) const { return m_trackLocations.contains(trackLocation); } @@ -68,7 +46,7 @@ class ScannerGlobal { return m_directoryHashes.value(directoryPath, mixxx::invalidCacheKey()); } - inline bool directoryBlacklisted(const QString& directoryPath) const { + bool directoryBlacklisted(const QString& directoryPath) const { return m_directoriesBlacklist.contains(directoryPath); } @@ -76,7 +54,7 @@ class ScannerGlobal { return m_supportedExtensionsMatcher; } - inline bool testAndMarkDirectoryScanned(const QDir& dir) { + bool testAndMarkDirectoryScanned(const QDir& dir) { const QString canonicalPath(dir.canonicalPath()); QMutexLocker locker(&m_directoriesScannedMutex); if (m_directoriesScanned.contains(canonicalPath)) { @@ -87,20 +65,19 @@ class ScannerGlobal { } } - inline void addUnhashedDir(const QDir& dir, - const SecurityTokenPointer& token) { + void addUnhashedDir(const mixxx::FileAccess& dirAccess) { QMutexLocker locker(&m_directoriesUnhashedMutex); - m_directoriesUnhashed.append(DirInfo(dir, token)); + m_directoriesUnhashed.append(dirAccess); } - inline QList& unhashedDirs() { + const QList& unhashedDirs() const { // no need for locking here, because it is only used // when only one using thread is around. return m_directoriesUnhashed; } // TODO(rryan) test whether tasks should create their own QRegExp. - inline bool isAudioFileSupported(const QString& fileName) const { + bool isAudioFileSupported(const QString& fileName) const { QMutexLocker locker(&m_supportedExtensionsMatcherMutex); return m_supportedExtensionsMatcher.indexIn(fileName) != -1; } @@ -110,16 +87,16 @@ class ScannerGlobal { } // TODO(rryan) test whether tasks should create their own QRegExp. - inline bool isCoverFileSupported(const QString& fileName) const { + bool isCoverFileSupported(const QString& fileName) const { QMutexLocker locker(&m_supportedCoverExtensionsMatcherMutex); return m_supportedCoverExtensionsMatcher.indexIn(fileName) != -1; } - inline bool shouldCancel() const { + bool shouldCancel() const { return m_shouldCancel; } - inline volatile const bool* shouldCancelPointer() const { + volatile const bool* shouldCancelPointer() const { return &m_shouldCancel; } @@ -127,7 +104,7 @@ class ScannerGlobal { m_shouldCancel = true; } - inline bool scanFinishedCleanly() const { + bool scanFinishedCleanly() const { return m_scanFinishedCleanly; } @@ -174,7 +151,6 @@ class ScannerGlobal { m_numScannedDirectories++; } - private: TaskWatcher m_watcher; @@ -197,7 +173,7 @@ class ScannerGlobal { // discovered directories, they are scanned in a // second run to avoid swapping between duplicated tracks mutable QMutex m_directoriesUnhashedMutex; - QList m_directoriesUnhashed; + QList m_directoriesUnhashed; // Typically there are 1 to 2 entries in the blacklist so a O(n) search in a // QList may have better constant factors than a O(1) QSet check. However, diff --git a/src/library/trackcollection.cpp b/src/library/trackcollection.cpp index d1e7f163541..69286ff2fc8 100644 --- a/src/library/trackcollection.cpp +++ b/src/library/trackcollection.cpp @@ -75,6 +75,7 @@ void TrackCollection::connectDatabase(const QSqlDatabase& database) { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); kLogger.info() << "Connecting database"; + DEBUG_ASSERT(database.isOpen()); m_database = database; m_trackDao.initialize(database); m_playlistDao.initialize(database); @@ -141,34 +142,41 @@ QWeakPointer TrackCollection::disconnectTrackSource() { return pWeakPtr; } -bool TrackCollection::addDirectory(const QString& dir) { +QList TrackCollection::loadRootDirs(bool skipInvalidOrMissing) const { + return m_directoryDao.loadAllDirectories(skipInvalidOrMissing); +} + +bool TrackCollection::addDirectory(const mixxx::FileInfo& rootDir) { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); SqlTransaction transaction(m_database); - switch (m_directoryDao.addDirectory(dir)) { - case SQL_ERROR: - return false; - case ALREADY_WATCHING: - return true; - case ALL_FINE: + switch (m_directoryDao.addDirectory(rootDir)) { + case DirectoryDAO::AddResult::Ok: transaction.commit(); return true; + case DirectoryDAO::AddResult::AlreadyWatching: + return true; + case DirectoryDAO::AddResult::InvalidOrMissingDirectory: + case DirectoryDAO::AddResult::SqlError: + return false; default: DEBUG_ASSERT("unreachable"); } return false; } -bool TrackCollection::removeDirectory(const QString& dir) { +bool TrackCollection::removeDirectory(const mixxx::FileInfo& rootDir) { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); SqlTransaction transaction(m_database); - switch (m_directoryDao.removeDirectory(dir)) { - case SQL_ERROR: - return false; - case ALL_FINE: + switch (m_directoryDao.removeDirectory(rootDir)) { + case DirectoryDAO::RemoveResult::Ok: transaction.commit(); return true; + case DirectoryDAO::RemoveResult::NotFound: + return true; + case DirectoryDAO::RemoveResult::SqlError: + return false; default: DEBUG_ASSERT("unreachable"); } @@ -184,7 +192,7 @@ void TrackCollection::relocateDirectory(const QString& oldDir, const QString& ne // have permission so that we can access the folder on future runs. We need // to canonicalize the path so we first wrap the directory string with a // QDir. - Sandbox::createSecurityToken(QDir(newDir)); + Sandbox::createSecurityTokenForDir(QDir(newDir)); SqlTransaction transaction(m_database); QList relocatedTracks = diff --git a/src/library/trackcollection.h b/src/library/trackcollection.h index cefe12c6a47..5155f417fc5 100644 --- a/src/library/trackcollection.h +++ b/src/library/trackcollection.h @@ -43,6 +43,9 @@ class TrackCollection : public QObject, return m_database; } + QList loadRootDirs( + bool skipInvalidOrMissing = false) const; + const CrateStorage& crates() const { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); return m_crates; @@ -56,7 +59,7 @@ class TrackCollection : public QObject, DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); return m_playlistDao; } - DirectoryDAO& getDirectoryDAO() { + const DirectoryDAO& getDirectoryDAO() const { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); return m_directoryDao; } @@ -148,8 +151,9 @@ class TrackCollection : public QObject, bool purgeTracks(const QList& trackIds); bool purgeAllTracks(const QDir& rootDir); - bool addDirectory(const QString& dir); - bool removeDirectory(const QString& dir); + bool addDirectory(const mixxx::FileInfo& rootDir); + bool removeDirectory(const mixxx::FileInfo& rootDir); + void relocateDirectory(const QString& oldDir, const QString& newDir); void saveTrack(Track* pTrack); diff --git a/src/library/trackcollectionmanager.cpp b/src/library/trackcollectionmanager.cpp index cf8ec4319a3..6bc9b102948 100644 --- a/src/library/trackcollectionmanager.cpp +++ b/src/library/trackcollectionmanager.cpp @@ -285,16 +285,16 @@ void TrackCollectionManager::exportTrackMetadata( } } -bool TrackCollectionManager::addDirectory(const QString& dir) const { +bool TrackCollectionManager::addDirectory(const mixxx::FileInfo& newDir) const { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); - return m_pInternalCollection->addDirectory(dir); + return m_pInternalCollection->addDirectory(newDir); } -bool TrackCollectionManager::removeDirectory(const QString& dir) const { +bool TrackCollectionManager::removeDirectory(const mixxx::FileInfo& oldDir) const { DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this); - return m_pInternalCollection->removeDirectory(dir); + return m_pInternalCollection->removeDirectory(oldDir); } void TrackCollectionManager::relocateDirectory(const QString& oldDir, const QString& newDir) const { diff --git a/src/library/trackcollectionmanager.h b/src/library/trackcollectionmanager.h index 682b1f30522..42efdfc50ad 100644 --- a/src/library/trackcollectionmanager.h +++ b/src/library/trackcollectionmanager.h @@ -9,6 +9,7 @@ #include "preferences/usersettings.h" #include "track/globaltrackcache.h" #include "util/db/dbconnectionpool.h" +#include "util/fileinfo.h" #include "util/parented_ptr.h" #include "util/thread_affinity.h" @@ -53,8 +54,8 @@ class TrackCollectionManager: public QObject, void purgeTracks(const QList& trackRefs) const; void purgeAllTracks(const QDir& rootDir) const; - bool addDirectory(const QString& dir) const; - bool removeDirectory(const QString& dir) const; + bool addDirectory(const mixxx::FileInfo& newDir) const; + bool removeDirectory(const mixxx::FileInfo& oldDir) const; void relocateDirectory(const QString& oldDir, const QString& newDir) const; TrackPointer getOrAddTrack( diff --git a/src/preferences/dialog/dlgpreflibrary.cpp b/src/preferences/dialog/dlgpreflibrary.cpp index c465c8401d3..b050f0b00a3 100644 --- a/src/preferences/dialog/dlgpreflibrary.cpp +++ b/src/preferences/dialog/dlgpreflibrary.cpp @@ -12,13 +12,16 @@ #include #include "library/dlgtrackmetadataexport.h" +#include "library/trackcollection.h" +#include "library/trackcollectionmanager.h" #include "moc_dlgpreflibrary.cpp" #include "sources/soundsourceproxy.h" #include "widget/wsearchlineedit.h" namespace { - const ConfigKey kSearchDebouncingTimeoutMillisKey = ConfigKey("[Library]","SearchDebouncingTimeoutMillis"); - } // namespace +const ConfigKey kSearchDebouncingTimeoutMillisKey = + ConfigKey("[Library]", "SearchDebouncingTimeoutMillis"); +} // namespace DlgPrefLibrary::DlgPrefLibrary( QWidget* pParent, @@ -153,9 +156,9 @@ void DlgPrefLibrary::initializeDirList() { const QString selected = dirList->currentIndex().data().toString(); // clear and fill model m_dirListModel.clear(); - QStringList dirs = m_pLibrary->getDirs(); - foreach (QString dir, dirs) { - m_dirListModel.appendRow(new QStandardItem(dir)); + const auto rootDirs = m_pLibrary->trackCollections()->internalCollection()->loadRootDirs(); + for (const mixxx::FileInfo& rootDir : rootDirs) { + m_dirListModel.appendRow(new QStandardItem(rootDir.location())); } dirList->setModel(&m_dirListModel); dirList->setCurrentIndex(m_dirListModel.index(0, 0)); diff --git a/src/preferences/dialog/dlgprefrecord.cpp b/src/preferences/dialog/dlgprefrecord.cpp index c8226765eed..a3878aa4c8f 100644 --- a/src/preferences/dialog/dlgprefrecord.cpp +++ b/src/preferences/dialog/dlgprefrecord.cpp @@ -222,7 +222,7 @@ void DlgPrefRecord::slotBrowseRecordingsDir() // that we can access the folder on future runs. We need to canonicalize // the path so we first wrap the directory string with a QDir. QDir directory(fd); - Sandbox::createSecurityToken(directory); + Sandbox::createSecurityTokenForDir(directory); LineEditRecordings->setText(fd); } } diff --git a/src/preferences/upgrade.cpp b/src/preferences/upgrade.cpp index 8289003fef2..d5fd8d7776f 100644 --- a/src/preferences/upgrade.cpp +++ b/src/preferences/upgrade.cpp @@ -392,7 +392,7 @@ UserSettingsPointer Upgrade::versionUpgrade(const QString& settingsPath) { // Sandbox isn't setup yet at this point in startup because it relies on // the config settings path and this function is what loads the config // so it's not ready yet. - successful = tc.addDirectory(currentFolder); + successful = tc.addDirectory(mixxx::FileInfo(currentFolder)); tc.disconnectDatabase(); } diff --git a/src/test/coverartcache_test.cpp b/src/test/coverartcache_test.cpp index 07753080fe9..8cf21f799b8 100644 --- a/src/test/coverartcache_test.cpp +++ b/src/test/coverartcache_test.cpp @@ -14,7 +14,7 @@ class CoverArtCacheTest : public LibraryTest, public CoverArtCache { void loadCoverFromMetadata(const QString& trackLocation) { const QImage img = SoundSourceProxy::importTemporaryCoverImage( trackLocation, - Sandbox::openSecurityToken(QDir(trackLocation), true)); + Sandbox::openSecurityToken(trackLocation, true)); ASSERT_FALSE(img.isNull()); CoverInfo info; diff --git a/src/test/directorydaotest.cpp b/src/test/directorydaotest.cpp index 400929cfbe5..29f89f0ee51 100644 --- a/src/test/directorydaotest.cpp +++ b/src/test/directorydaotest.cpp @@ -20,13 +20,17 @@ class DirectoryDAOTest : public LibraryTest { protected: void TearDown() override { // make sure we clean up the db + const auto& directoryDao = internalCollection()->getDirectoryDAO(); + const auto allDirs = directoryDao.loadAllDirectories(); + for (const auto& dir : allDirs) { + ASSERT_EQ(DirectoryDAO::RemoveResult::Ok, directoryDao.removeDirectory(dir)); + } + ASSERT_TRUE(directoryDao.loadAllDirectories().isEmpty()); QSqlQuery query(dbConnection()); - query.prepare("DELETE FROM " % DIRECTORYDAO_TABLE); - query.exec(); - query.prepare("DELETE FROM library"); - query.exec(); - query.prepare("DELETE FROM track_locations"); - query.exec(); + ASSERT_TRUE(query.prepare("DELETE FROM library")); + ASSERT_TRUE(query.exec()); + ASSERT_TRUE(query.prepare("DELETE FROM track_locations")); + ASSERT_TRUE(query.exec()); } static QString getSupportedFileExt() { @@ -39,95 +43,134 @@ class DirectoryDAOTest : public LibraryTest { } }; -TEST_F(DirectoryDAOTest, addDirTest) { - DirectoryDAO m_DirectoryDao = internalCollection()->getDirectoryDAO(); - // prepend dir with '/' so that QT thinks the dir starts at the root - QTemporaryDir tempDir; +TEST_F(DirectoryDAOTest, add) { + const QTemporaryDir tempDir; ASSERT_TRUE(tempDir.isValid()); - QString testdir = QString(tempDir.path() + "/TestDir/a"); - QString testChild = QString(tempDir.path() + "/TestDir/a/child"); - QString testParent = QString(tempDir.path() + "/TestDir"); //create temp dirs - ASSERT_TRUE(QDir(tempDir.path()).mkpath(testParent)); + const QString testdir = QString(tempDir.path() + "/TestDir/test"); ASSERT_TRUE(QDir(tempDir.path()).mkpath(testdir)); + const QString testChild = QString(tempDir.path() + "/TestDir/test/child"); ASSERT_TRUE(QDir(tempDir.path()).mkpath(testChild)); + const QString testParent = QString(tempDir.path() + "/TestDir"); + ASSERT_TRUE(QDir(tempDir.path()).mkpath(testParent)); +#if !defined(__WINDOWS__) + const QString linkdir = QString(tempDir.path() + "/testLink"); + ASSERT_TRUE(QFile::link(testdir, linkdir)); + const QString linkChild = QString(tempDir.path() + "/childLink"); + ASSERT_TRUE(QFile::link(testChild, linkChild)); + const QString linkParent = QString(tempDir.path() + "/parentLink"); + ASSERT_TRUE(QFile::link(testParent, linkParent)); +#endif + + const DirectoryDAO& dao = internalCollection()->getDirectoryDAO(); // check if directory doa adds and thinks everything is ok - int success = m_DirectoryDao.addDirectory(testdir); - EXPECT_EQ(ALL_FINE, success); + EXPECT_EQ( + DirectoryDAO::AddResult::Ok, + dao.addDirectory(mixxx::FileInfo(testdir))); // check that we don't add the directory again - success = m_DirectoryDao.addDirectory(testdir); - EXPECT_EQ(ALREADY_WATCHING, success); + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(testdir))); +#if !defined(__WINDOWS__) + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(linkdir))); +#endif // check that we don't add the directory again also if the string ends with // "/". - success = m_DirectoryDao.addDirectory(testdir + "/"); - EXPECT_EQ(ALREADY_WATCHING, success); + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(testdir))); +#if !defined(__WINDOWS__) + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(linkdir))); +#endif // check that we don't add a child directory - success = m_DirectoryDao.addDirectory(testChild); - EXPECT_EQ(ALREADY_WATCHING, success); + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(testChild))); +#if !defined(__WINDOWS__) + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(linkChild))); +#endif + +#if !defined(__WINDOWS__) + // Use the link to the parent directory + const auto parentInfo = mixxx::FileInfo(linkParent); +#else + // Use the the parent directory + const auto parentInfo = mixxx::FileInfo(testParent); +#endif // check that we add the parent dir - success = m_DirectoryDao.addDirectory(testParent); - EXPECT_EQ(ALL_FINE, success); - - QSqlQuery query(dbConnection()); - query.prepare("SELECT " % DIRECTORYDAO_DIR % " FROM " % DIRECTORYDAO_TABLE); - success = query.exec(); - ASSERT_TRUE(success); - - // we do not trust what directory dao thinks and better check up on it - QStringList dirs; - while (query.next()) { - dirs << query.value(0).toString(); - } - - // the test db should be always empty when tests are started. - EXPECT_EQ(1, dirs.size()); - EXPECT_QSTRING_EQ(testParent, dirs.at(0)); + EXPECT_EQ( + DirectoryDAO::AddResult::Ok, + dao.addDirectory(parentInfo)); + + // the db should now only contain the link to the parent directory + const QList allDirs = dao.loadAllDirectories(); + ASSERT_EQ(1, allDirs.length()); + EXPECT_QSTRING_EQ(parentInfo.location(), allDirs.first().location()); + +#if !defined(__WINDOWS__) + // Verify that adding the actual dir path instead of the + // link is rejected. + EXPECT_EQ( + DirectoryDAO::AddResult::AlreadyWatching, + dao.addDirectory(mixxx::FileInfo(testParent))); +#endif } -TEST_F(DirectoryDAOTest, removeDirTest) { - DirectoryDAO m_DirectoryDao = internalCollection()->getDirectoryDAO(); - QString testdir = getTestDataDir().path(); +TEST_F(DirectoryDAOTest, remove) { + const QString testdir = getTestDataDir().path(); - // check if directory doa adds and thinks everything is ok - m_DirectoryDao.addDirectory(testdir); + const DirectoryDAO& dao = internalCollection()->getDirectoryDAO(); - int success = m_DirectoryDao.removeDirectory(testdir); - EXPECT_EQ(ALL_FINE, success); + EXPECT_EQ( + DirectoryDAO::RemoveResult::NotFound, + dao.removeDirectory(mixxx::FileInfo(testdir))); - // we do not trust what directory dao thinks and better check up on it - QSqlQuery query(dbConnection()); - query.prepare("SELECT " % DIRECTORYDAO_DIR % " FROM " % DIRECTORYDAO_TABLE); - success = query.exec(); - ASSERT_TRUE(success); + ASSERT_EQ( + DirectoryDAO::AddResult::Ok, + dao.addDirectory(mixxx::FileInfo(testdir))); - QStringList dirs; - while (query.next()) { - dirs << query.value(0).toString(); - } + EXPECT_EQ( + DirectoryDAO::RemoveResult::Ok, + dao.removeDirectory(mixxx::FileInfo(testdir))); // the db should have now no entries left anymore - EXPECT_EQ(0, dirs.size()); + EXPECT_TRUE(dao.loadAllDirectories().isEmpty()); + + EXPECT_EQ( + DirectoryDAO::RemoveResult::NotFound, + dao.removeDirectory(mixxx::FileInfo(testdir))); } -TEST_F(DirectoryDAOTest, getDirTest) { - DirectoryDAO m_DirectoryDao = internalCollection()->getDirectoryDAO(); - QString testdir = "/a/c"; - QString testdir2 = "b/d"; +TEST_F(DirectoryDAOTest, loadAll) { + const QTemporaryDir tempDir; + ASSERT_TRUE(tempDir.isValid()); + + //create temp dirs + const QString testdir = tempDir.path() + "/a/c"; + ASSERT_TRUE(QDir(tempDir.path()).mkpath(testdir)); + const QString testdir2 = tempDir.path() + "b/d"; + ASSERT_TRUE(QDir(tempDir.path()).mkpath(testdir2)); - m_DirectoryDao.addDirectory(testdir); - m_DirectoryDao.addDirectory(testdir2); + const DirectoryDAO dao = internalCollection()->getDirectoryDAO(); - QStringList dirs = m_DirectoryDao.getDirs(); + ASSERT_EQ(DirectoryDAO::AddResult::Ok, dao.addDirectory(mixxx::FileInfo(testdir))); + ASSERT_EQ(DirectoryDAO::AddResult::Ok, dao.addDirectory(mixxx::FileInfo(testdir2))); - EXPECT_EQ(2, dirs.size()); - EXPECT_QSTRING_EQ(testdir, dirs.at(0)); - EXPECT_QSTRING_EQ(testdir2, dirs.at(1)); + const QList allDirs = dao.loadAllDirectories(); + EXPECT_EQ(2, allDirs.size()); + EXPECT_THAT(allDirs, UnorderedElementsAre(mixxx::FileInfo(testdir), mixxx::FileInfo(testdir2))); } TEST_F(DirectoryDAOTest, relocateDirectory) { @@ -147,8 +190,8 @@ TEST_F(DirectoryDAOTest, relocateDirectory) { const DirectoryDAO& dao = internalCollection()->getDirectoryDAO(); - ASSERT_EQ(ALL_FINE, dao.addDirectory(testdir)); - ASSERT_EQ(ALL_FINE, dao.addDirectory(test2)); + ASSERT_EQ(DirectoryDAO::AddResult::Ok, dao.addDirectory(mixxx::FileInfo(testdir))); + ASSERT_EQ(DirectoryDAO::AddResult::Ok, dao.addDirectory(mixxx::FileInfo(test2))); // ok now lets create some tracks here ASSERT_TRUE(internalCollection() @@ -180,7 +223,7 @@ TEST_F(DirectoryDAOTest, relocateDirectory) { dao.relocateDirectory(testdir, testnew); EXPECT_EQ(2, relocatedTracks.size()); - const QStringList allDirs = dao.getDirs(); + const QList allDirs = dao.loadAllDirectories(); EXPECT_EQ(2, allDirs.size()); - EXPECT_THAT(allDirs, UnorderedElementsAre(test2, testnew)); + EXPECT_THAT(allDirs, UnorderedElementsAre(mixxx::FileInfo(test2), mixxx::FileInfo(testnew))); } diff --git a/src/track/globaltrackcache.h b/src/track/globaltrackcache.h index 56a7a8a4cab..faa3d2b8001 100644 --- a/src/track/globaltrackcache.h +++ b/src/track/globaltrackcache.h @@ -6,6 +6,7 @@ #include "track/track_decl.h" #include "track/trackref.h" +#include "util/fileaccess.h" #include "util/sandbox.h" // forward declaration(s) diff --git a/src/track/trackfile.h b/src/track/trackfile.h index 170c4d6ccfc..5a5c411caef 100644 --- a/src/track/trackfile.h +++ b/src/track/trackfile.h @@ -6,6 +6,8 @@ #include #include +#include "util/fileinfo.h" + // Wrapper class for dealing with track files and their // path/location, URL, and URI representations. // @@ -28,6 +30,7 @@ class TrackFile { return TrackFile(url.toLocalFile()); } + TrackFile() = default; // For backward-compatibility the QString single argument // constructor has not been declared as "explicit". It is // also not strictly necessary and might be removed at some @@ -37,7 +40,7 @@ class TrackFile { : m_fileInfo(filePath) { } explicit TrackFile( - QFileInfo fileInfo = QFileInfo()) + QFileInfo fileInfo) : m_fileInfo(std::move(fileInfo)) { } explicit TrackFile( @@ -45,6 +48,10 @@ class TrackFile { const QString& file = QString()) : m_fileInfo(QFileInfo(dir, file)) { } + explicit TrackFile( + const mixxx::FileInfo& fileInfo) + : m_fileInfo(fileInfo.asQFileInfo()) { + } const QFileInfo& asFileInfo() const { return m_fileInfo; diff --git a/src/util/file.cpp b/src/util/file.cpp deleted file mode 100644 index 962055c2ccf..00000000000 --- a/src/util/file.cpp +++ /dev/null @@ -1,30 +0,0 @@ -#include "util/file.h" - -MDir::MDir() { -} - -MDir::MDir(const QString& path) - : m_dirPath(path), - m_dir(path), - m_pSecurityToken(Sandbox::openSecurityToken(m_dir, true)) { -} - -MDir::MDir(const MDir& other) - : m_dirPath(other.m_dirPath), - m_dir(m_dirPath), - m_pSecurityToken(other.m_pSecurityToken) { -} - -MDir::~MDir() { -} - -MDir& MDir::operator=(const MDir& other) { - m_dirPath = other.m_dirPath; - m_dir = QDir(m_dirPath); - m_pSecurityToken = other.m_pSecurityToken; - return *this; -} - -bool MDir::canAccess() { - return Sandbox::canAccessFile(m_dir); -} diff --git a/src/util/file.h b/src/util/file.h deleted file mode 100644 index d1950d0ca95..00000000000 --- a/src/util/file.h +++ /dev/null @@ -1,35 +0,0 @@ -#pragma once - -#include -#include - -#include "util/sandbox.h" - -class MDir { - public: - MDir(); - MDir(const QString& name); - MDir(const MDir& other); - virtual ~MDir(); - - QDir& dir() { - return m_dir; - } - - const QDir& dir() const { - return m_dir; - } - - SecurityTokenPointer token() { - return m_pSecurityToken; - } - - bool canAccess(); - - MDir& operator=(const MDir& other); - - private: - QString m_dirPath; - QDir m_dir; - SecurityTokenPointer m_pSecurityToken; -}; diff --git a/src/util/fileaccess.cpp b/src/util/fileaccess.cpp new file mode 100644 index 00000000000..1ff63f61a0f --- /dev/null +++ b/src/util/fileaccess.cpp @@ -0,0 +1,29 @@ +#include "util/fileaccess.h" + +namespace mixxx { + +namespace { + +inline SecurityTokenPointer acquireSecurityToken( + SecurityTokenPointer pSecurityToken, + const FileInfo& fileInfo) { + if (pSecurityToken) { + return pSecurityToken; + } else { + return Sandbox::openSecurityToken(fileInfo.asQFileInfo(), true); + } +} + +} // anonymous namespace + +FileAccess::FileAccess( + FileInfo fileInfo, + SecurityTokenPointer pSecurityToken) + : m_fileInfo(std::move(fileInfo)), + m_pSecurityToken( + acquireSecurityToken( + std::move(pSecurityToken), + m_fileInfo)) { +} + +} // namespace mixxx diff --git a/src/util/fileaccess.h b/src/util/fileaccess.h new file mode 100644 index 00000000000..1570a33ac4c --- /dev/null +++ b/src/util/fileaccess.h @@ -0,0 +1,42 @@ +#pragma once + +#include "util/fileinfo.h" +#include "util/sandbox.h" + +namespace mixxx { + +/// Bundles `FileInfo` with the corresponding security token (if available). +/// On systems with sandboxing the latter is required to actually access the +/// corresponding file system object. +class FileAccess final { + public: + explicit FileAccess( + FileInfo fileInfo, + SecurityTokenPointer pSecurityToken = SecurityTokenPointer()); + FileAccess() = default; + FileAccess(FileAccess&&) = default; + FileAccess(const FileAccess&) = default; + FileAccess& operator=(FileAccess&&) = default; + FileAccess& operator=(const FileAccess&) = default; + + const FileInfo& info() const { + return m_fileInfo; + } + + const SecurityTokenPointer& token() const { + return m_pSecurityToken; + } + + bool isReadable() const { + return m_pSecurityToken && m_fileInfo.isReadable(); + } + bool isWritable() const { + return m_pSecurityToken && m_fileInfo.isWritable(); + } + + private: + FileInfo m_fileInfo; + SecurityTokenPointer m_pSecurityToken; +}; + +} // namespace mixxx diff --git a/src/util/sandbox.cpp b/src/util/sandbox.cpp index d228a9f65b8..bc9d22eed7a 100644 --- a/src/util/sandbox.cpp +++ b/src/util/sandbox.cpp @@ -71,7 +71,7 @@ bool Sandbox::askForAccess(const QString& canonicalPath) { QFileInfo info(canonicalPath); // We always want read/write access because we wouldn't want to have to // re-ask for access in the future if we need to write. - if (canAccessFile(info)) { + if (canAccess(info)) { return true; } @@ -193,18 +193,26 @@ bool Sandbox::createSecurityToken(const QString& canonicalPath, // static SecurityTokenPointer Sandbox::openSecurityToken(const QFileInfo& file, bool create) { - const QString& canonicalFilePath = file.canonicalFilePath(); + const QString canonicalFilePath = file.canonicalFilePath(); + if (canonicalFilePath.isEmpty()) { + return nullptr; + } + + if (file.isDir()) { + return openSecurityTokenForDir(QDir(canonicalFilePath), create); + } + if (sDebug) { - qDebug() << "openSecurityToken QFileInfo" << canonicalFilePath << create; + qDebug() << "openSecurityToken for file" << canonicalFilePath << create; } if (!enabled()) { - return SecurityTokenPointer(); + return nullptr; } QMutexLocker locker(&s_mutex); - if (s_pSandboxPermissions == nullptr) { - return SecurityTokenPointer(); + if (!s_pSandboxPermissions) { + return nullptr; } QHash::iterator it = s_activeTokens @@ -220,10 +228,6 @@ SecurityTokenPointer Sandbox::openSecurityToken(const QFileInfo& file, bool crea } } - if (file.isDir()) { - return openSecurityToken(QDir(canonicalFilePath), create); - } - // First, check for a bookmark of the key itself. ConfigKey key = keyForCanonicalPath(canonicalFilePath); if (s_pSandboxPermissions->exists(key)) { @@ -234,13 +238,13 @@ SecurityTokenPointer Sandbox::openSecurityToken(const QFileInfo& file, bool crea // Next, try to open a bookmark for an existing directory but don't create a // bookmark. - SecurityTokenPointer pDirToken = openSecurityToken(file.dir(), false); + SecurityTokenPointer pDirToken = openSecurityTokenForDir(file.dir(), false); if (!pDirToken.isNull()) { return pDirToken; } if (!create) { - return SecurityTokenPointer(); + return nullptr; } // Otherwise, try to create a token. @@ -251,27 +255,27 @@ SecurityTokenPointer Sandbox::openSecurityToken(const QFileInfo& file, bool crea canonicalFilePath, s_pSandboxPermissions->getValueString(key)); } - return SecurityTokenPointer(); + return nullptr; } // static -SecurityTokenPointer Sandbox::openSecurityToken(const QDir& dir, bool create) { +SecurityTokenPointer Sandbox::openSecurityTokenForDir(const QDir& dir, bool create) { QDir walkDir = dir; QString walkDirCanonicalPath = walkDir.canonicalPath(); if (sDebug) { - qDebug() << "openSecurityToken QDir" << walkDirCanonicalPath << create; + qDebug() << "openSecurityToken for dir" << walkDirCanonicalPath << create; } if (!enabled()) { - return SecurityTokenPointer(); + return nullptr; } QMutexLocker locker(&s_mutex); - if (s_pSandboxPermissions.isNull()) { - return SecurityTokenPointer(); + if (!s_pSandboxPermissions) { + return nullptr; } - while (true) { + while (!walkDirCanonicalPath.isEmpty()) { // Look for a valid token in the cache. QHash::iterator it = s_activeTokens .find(walkDirCanonicalPath); @@ -312,7 +316,7 @@ SecurityTokenPointer Sandbox::openSecurityToken(const QDir& dir, bool create) { dir.canonicalPath(), s_pSandboxPermissions->getValueString(key)); } - return SecurityTokenPointer(); + return nullptr; } SecurityTokenPointer Sandbox::openTokenFromBookmark(const QString& canonicalPath, @@ -359,7 +363,7 @@ SecurityTokenPointer Sandbox::openTokenFromBookmark(const QString& canonicalPath Q_UNUSED(bookmarkBase64); #endif - return SecurityTokenPointer(); + return nullptr; } QString Sandbox::migrateOldSettings() { diff --git a/src/util/sandbox.h b/src/util/sandbox.h index 48b90b62fda..0df170bac82 100644 --- a/src/util/sandbox.h +++ b/src/util/sandbox.h @@ -42,13 +42,13 @@ class Sandbox { // Prompt the user to give us access to the path with an open-file dialog. static bool askForAccess(const QString& canonicalPath); - static bool canAccessFile(const QFileInfo& file) { - SecurityTokenPointer pToken = openSecurityToken(file, true); - return file.isReadable(); + static bool canAccess(const QFileInfo& info) { + SecurityTokenPointer pToken = openSecurityToken(info, true); + return info.isReadable(); } - static bool canAccessFile(const QDir& dir) { - SecurityTokenPointer pToken = openSecurityToken(dir, true); + static bool canAccessDir(const QDir& dir) { + SecurityTokenPointer pToken = openSecurityTokenForDir(dir, true); QFileInfo info(dir.canonicalPath()); return info.isReadable(); } @@ -57,15 +57,15 @@ class Sandbox { return createSecurityToken(info.canonicalFilePath(), info.isDir()); } - static bool createSecurityToken(const QDir& dir) { + static bool createSecurityTokenForDir(const QDir& dir) { return createSecurityToken(dir.canonicalPath(), true); } static SecurityTokenPointer openSecurityToken(const QFileInfo& info, bool create); - static SecurityTokenPointer openSecurityToken(const QDir& dir, bool create); + static SecurityTokenPointer openSecurityTokenForDir(const QDir& dir, bool create); private: - Sandbox() {} + Sandbox() = delete; static ConfigKey keyForCanonicalPath(const QString& canonicalPath);