diff --git a/CMakeLists.txt b/CMakeLists.txt index f11ce8c997e..637f35611db 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -526,6 +526,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/util/audiosignal.cpp src/util/autohidpi.cpp src/util/battery/battery.cpp + src/util/cache.cpp src/util/cmdlineargs.cpp src/util/color/color.cpp src/util/color/predefinedcolor.cpp @@ -913,6 +914,7 @@ add_executable(mixxx-test src/test/bpmcontrol_test.cpp src/test/broadcastprofile_test.cpp src/test/broadcastsettings_test.cpp + src/test/cache_test.cpp src/test/channelhandle_test.cpp src/test/configobject_test.cpp src/test/controller_preset_validation_test.cpp diff --git a/build/depends.py b/build/depends.py index f558598ac96..9f0a9ebd25d 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1269,6 +1269,7 @@ def sources(self, build): "src/util/xml.cpp", "src/util/tapfilter.cpp", "src/util/movinginterquartilemean.cpp", + "src/util/cache.cpp", "src/util/console.cpp", "src/util/color/color.cpp", "src/util/db/dbconnection.cpp", diff --git a/src/library/dao/libraryhashdao.cpp b/src/library/dao/libraryhashdao.cpp index 637f1f49f4b..32cc824cb0b 100644 --- a/src/library/dao/libraryhashdao.cpp +++ b/src/library/dao/libraryhashdao.cpp @@ -8,10 +8,21 @@ #include "libraryhashdao.h" #include "library/queryutil.h" -QHash LibraryHashDAO::getDirectoryHashes() { +namespace { + +// Store hash values as a signed 64-bit integer. Otherwise values greater +// than 2^63-1 would be converted into a floating point numbers while +// losing precision!! +inline constexpr mixxx::cache_key_signed_t dbHash(mixxx::cache_key_t hash) { + return mixxx::signedCacheKey(hash); +} + +} // anonymous namespace + +QHash LibraryHashDAO::getDirectoryHashes() { QSqlQuery query(m_database); query.prepare("SELECT hash, directory_path FROM LibraryHashes"); - QHash hashes; + QHash hashes; if (!query.exec()) { LOG_FAILED_QUERY(query); } @@ -20,15 +31,15 @@ QHash LibraryHashDAO::getDirectoryHashes() { int directoryPathColumn = query.record().indexOf("directory_path"); while (query.next()) { hashes[query.value(directoryPathColumn).toString()] = - query.value(hashColumn).toInt(); + query.value(hashColumn).toULongLong(); } return hashes; } -int LibraryHashDAO::getDirectoryHash(const QString& dirPath) { +mixxx::cache_key_t LibraryHashDAO::getDirectoryHash(const QString& dirPath) { //qDebug() << "LibraryHashDAO::getDirectoryHash" << QThread::currentThread() << m_database.connectionName(); - int hash = -1; + mixxx::cache_key_t hash = mixxx::invalidCacheKey(); QSqlQuery query(m_database); query.prepare("SELECT hash FROM LibraryHashes " @@ -40,7 +51,7 @@ int LibraryHashDAO::getDirectoryHash(const QString& dirPath) { } //Grab a hash for this directory from the database, from the last time it was scanned. if (query.next()) { - hash = query.value(query.record().indexOf("hash")).toInt(); + hash = query.value(query.record().indexOf("hash")).toULongLong(); //qDebug() << "prev hash exists" << hash << dirPath; } else { //qDebug() << "prev hash does not exist" << dirPath; @@ -49,13 +60,13 @@ int LibraryHashDAO::getDirectoryHash(const QString& dirPath) { return hash; } -void LibraryHashDAO::saveDirectoryHash(const QString& dirPath, const int hash) { +void LibraryHashDAO::saveDirectoryHash(const QString& dirPath, mixxx::cache_key_t hash) { //qDebug() << "LibraryHashDAO::saveDirectoryHash" << QThread::currentThread() << m_database.connectionName(); QSqlQuery query(m_database); query.prepare("INSERT INTO LibraryHashes (directory_path, hash, directory_deleted) " "VALUES (:directory_path, :hash, :directory_deleted)"); query.bindValue(":directory_path", dirPath); - query.bindValue(":hash", hash); + query.bindValue(":hash", dbHash(hash)); query.bindValue(":directory_deleted", 0); if (!query.exec()) { @@ -65,8 +76,8 @@ void LibraryHashDAO::saveDirectoryHash(const QString& dirPath, const int hash) { } void LibraryHashDAO::updateDirectoryHash(const QString& dirPath, - const int newHash, - const int dir_deleted) { + mixxx::cache_key_t newHash, + int dir_deleted) { //qDebug() << "LibraryHashDAO::updateDirectoryHash" << QThread::currentThread() << m_database.connectionName(); QSqlQuery query(m_database); // By definition if we have calculated a new hash for a directory then it @@ -75,7 +86,7 @@ void LibraryHashDAO::updateDirectoryHash(const QString& dirPath, "SET hash=:hash, directory_deleted=:directory_deleted, " "needs_verification=0 " "WHERE directory_path=:directory_path"); - query.bindValue(":hash", newHash); + query.bindValue(":hash", dbHash(newHash)); query.bindValue(":directory_deleted", dir_deleted); query.bindValue(":directory_path", dirPath); diff --git a/src/library/dao/libraryhashdao.h b/src/library/dao/libraryhashdao.h index 123a95b2faa..363c01164e2 100644 --- a/src/library/dao/libraryhashdao.h +++ b/src/library/dao/libraryhashdao.h @@ -7,6 +7,7 @@ #include #include "library/dao/dao.h" +#include "util/cache.h" class LibraryHashDAO : public DAO { public: @@ -16,11 +17,11 @@ class LibraryHashDAO : public DAO { m_database = database; }; - QHash getDirectoryHashes(); - int getDirectoryHash(const QString& dirPath); - void saveDirectoryHash(const QString& dirPath, const int hash); - void updateDirectoryHash(const QString& dirPath, const int newHash, - const int dir_deleted); + QHash getDirectoryHashes(); + mixxx::cache_key_t getDirectoryHash(const QString& dirPath); + void saveDirectoryHash(const QString& dirPath, mixxx::cache_key_t hash); + void updateDirectoryHash(const QString& dirPath, mixxx::cache_key_t newHash, + int dir_deleted); void markAsExisting(const QString& dirPath); void invalidateAllDirectories(); void markUnverifiedDirectoriesAsDeleted(); diff --git a/src/library/scanner/importfilestask.cpp b/src/library/scanner/importfilestask.cpp index efdadd80e38..86964dffe3d 100644 --- a/src/library/scanner/importfilestask.cpp +++ b/src/library/scanner/importfilestask.cpp @@ -8,7 +8,7 @@ ImportFilesTask::ImportFilesTask(LibraryScanner* pScanner, const ScannerGlobalPointer scannerGlobal, const QString& dirPath, const bool prevHashExists, - const int newHash, + const mixxx::cache_key_t newHash, const QLinkedList& filesToImport, const QLinkedList& possibleCovers, SecurityTokenPointer pToken) diff --git a/src/library/scanner/importfilestask.h b/src/library/scanner/importfilestask.h index c929be8b22f..c4d5e58f1f0 100644 --- a/src/library/scanner/importfilestask.h +++ b/src/library/scanner/importfilestask.h @@ -6,7 +6,6 @@ #include "util/sandbox.h" #include "library/scanner/scannertask.h" -#include "library/scanner/scannerglobal.h" // Import the provided files. Successful if the scan completed without being // cancelled. False if the scan was cancelled part-way through. @@ -17,7 +16,7 @@ class ImportFilesTask : public ScannerTask { const ScannerGlobalPointer scannerGlobal, const QString& dirPath, const bool prevHashExists, - const int newHash, + const mixxx::cache_key_t newHash, const QLinkedList& filesToImport, const QLinkedList& possibleCovers, SecurityTokenPointer pToken); @@ -28,7 +27,7 @@ class ImportFilesTask : public ScannerTask { private: const QString m_dirPath; const bool m_prevHashExists; - const int m_newHash; + const mixxx::cache_key_t m_newHash; const QLinkedList m_filesToImport; const QLinkedList m_possibleCovers; SecurityTokenPointer m_pToken; diff --git a/src/library/scanner/libraryscanner.cpp b/src/library/scanner/libraryscanner.cpp index e9fca5fd44b..cd48824ce35 100644 --- a/src/library/scanner/libraryscanner.cpp +++ b/src/library/scanner/libraryscanner.cpp @@ -26,12 +26,8 @@ mixxx::Logger kLogger("LibraryScanner"); QAtomicInt s_instanceCounter(0); -const QString kDeleteOrphanedLibraryScannerDirectories = - "delete from LibraryHashes where hash <> 0 and directory_path not in (select directory from track_locations)"; - // Returns the number of affected rows or -1 on error -int execCleanupQuery(QSqlDatabase database, const QString& statement) { - FwdSqlQuery query(database, statement); +int execCleanupQuery(FwdSqlQuery& query) { VERIFY_OR_DEBUG_ASSERT(query.isPrepared()) { return -1; } @@ -142,11 +138,19 @@ void LibraryScanner::run() { // Clean up the database and fix inconsistencies from previous runs. // See also: https://bugs.launchpad.net/mixxx/+bug/1846945 { - kLogger.info() << "Cleaning up database..."; + kLogger.info() + << "Cleaning up database..."; PerformanceTimer timer; timer.start(); - auto numRows = execCleanupQuery(dbConnection, - kDeleteOrphanedLibraryScannerDirectories); + const auto sqlStmt = QStringLiteral( + "DELETE FROM LibraryHashes WHERE hash <> :unequalHash " + "AND directory_path NOT IN " + "(SELECT directory FROM track_locations)"); + FwdSqlQuery query(dbConnection, sqlStmt); + query.bindValue( + QStringLiteral(":unequalHash"), + static_cast(mixxx::invalidCacheKey())); + auto numRows = execCleanupQuery(query); if (numRows < 0) { kLogger.warning() << "Failed to delete orphaned directory hashes"; @@ -189,7 +193,7 @@ void LibraryScanner::slotStartScan() { changeScannerState(SCANNING); QSet trackLocations = m_trackDao.getTrackLocations(); - QHash directoryHashes = m_libraryHashDao.getDirectoryHashes(); + QHash directoryHashes = m_libraryHashDao.getDirectoryHashes(); QRegExp extensionFilter(SoundSourceProxy::getSupportedFileNamesRegex()); QRegExp coverExtensionFilter = QRegExp(CoverArtUtils::supportedCoverArtExtensionsRegex(), @@ -515,7 +519,7 @@ void LibraryScanner::queueTask(ScannerTask* pTask) { } void LibraryScanner::slotDirectoryHashedAndScanned(const QString& directoryPath, - bool newDirectory, int hash) { + bool newDirectory, mixxx::cache_key_t hash) { ScopedTimer timer("LibraryScanner::slotDirectoryHashedAndScanned"); //kLogger.debug() << "sloDirectoryHashedAndScanned" << directoryPath // << newDirectory << hash; diff --git a/src/library/scanner/libraryscanner.h b/src/library/scanner/libraryscanner.h index 80edfb0991e..4761e7f5777 100644 --- a/src/library/scanner/libraryscanner.h +++ b/src/library/scanner/libraryscanner.h @@ -69,7 +69,7 @@ class LibraryScanner : public QThread { // ScannerTask signal handlers. void slotDirectoryHashedAndScanned(const QString& directoryPath, - bool newDirectory, int hash); + bool newDirectory, mixxx::cache_key_t hash); void slotDirectoryUnchanged(const QString& directoryPath); void slotTrackExists(const QString& trackPath); void slotAddNewTrack(const QString& trackPath); diff --git a/src/library/scanner/recursivescandirectorytask.cpp b/src/library/scanner/recursivescandirectorytask.cpp index 3b133982334..dcef6e15107 100644 --- a/src/library/scanner/recursivescandirectorytask.cpp +++ b/src/library/scanner/recursivescandirectorytask.cpp @@ -1,3 +1,4 @@ +#include #include #include "library/scanner/recursivescandirectorytask.h" @@ -38,7 +39,8 @@ void RecursiveScanDirectoryTask::run() { QLinkedList filesToImport; QLinkedList possibleCovers; QLinkedList dirsToScan; - QStringList newHashStr; + + QCryptographicHash hasher(QCryptographicHash::Sha256); // TODO(rryan) benchmark QRegExp copy versus QMutex/QRegExp in ScannerGlobal // versus slicing the extension off and checking for set/list containment. @@ -54,7 +56,7 @@ void RecursiveScanDirectoryTask::run() { if (currentFileInfo.isFile()) { const QString& fileName = currentFileInfo.fileName(); if (supportedExtensionsRegex.indexIn(fileName) != -1) { - newHashStr.append(currentFile); + hasher.addData(currentFile.toUtf8()); filesToImport.append(currentFileInfo); } else if (supportedCoverExtensionsRegex.indexIn(fileName) != -1) { possibleCovers.append(currentFileInfo); @@ -73,13 +75,13 @@ void RecursiveScanDirectoryTask::run() { // Note: A hash of "0" is a real hash if the directory contains no files! // Calculate a hash of the directory's file list. - int newHash = qHash(newHashStr.join("")); + const mixxx::cache_key_t newHash = mixxx::cacheKeyFromMessageDigest(hasher.result()); QString dirPath = m_dir.path(); // Try to retrieve a hash from the last time that directory was scanned. - int prevHash = m_scannerGlobal->directoryHashInDatabase(dirPath); - bool prevHashExists = prevHash != -1; + const mixxx::cache_key_t prevHash = m_scannerGlobal->directoryHashInDatabase(dirPath); + const bool prevHashExists = mixxx::isValidCacheKey(prevHash); if (prevHashExists || m_scanUnhashed) { // Compare the hashes, and if they don't match, rescan the files in that diff --git a/src/library/scanner/scannerglobal.h b/src/library/scanner/scannerglobal.h index ffb18465eac..b774227b3b6 100644 --- a/src/library/scanner/scannerglobal.h +++ b/src/library/scanner/scannerglobal.h @@ -9,6 +9,7 @@ #include #include +#include "util/cache.h" #include "util/task.h" #include "util/performancetimer.h" @@ -37,7 +38,7 @@ class DirInfo { class ScannerGlobal { public: ScannerGlobal(const QSet& trackLocations, - const QHash& directoryHashes, + const QHash& directoryHashes, const QRegExp& supportedExtensionsMatcher, const QRegExp& supportedCoverExtensionsMatcher, const QStringList& directoriesBlacklist) @@ -62,7 +63,7 @@ class ScannerGlobal { } // Returns the directory hash if it exists or -1 if it doesn't. - inline int directoryHashInDatabase(const QString& directoryPath) const { + inline mixxx::cache_key_t directoryHashInDatabase(const QString& directoryPath) const { return m_directoryHashes.value(directoryPath, -1); } @@ -177,7 +178,7 @@ class ScannerGlobal { TaskWatcher m_watcher; QSet m_trackLocations; - QHash m_directoryHashes; + QHash m_directoryHashes; mutable QMutex m_supportedExtensionsMatcherMutex; QRegExp m_supportedExtensionsMatcher; diff --git a/src/library/scanner/scannertask.h b/src/library/scanner/scannertask.h index 22db07c462f..1a3bc869a2f 100644 --- a/src/library/scanner/scannertask.h +++ b/src/library/scanner/scannertask.h @@ -22,7 +22,7 @@ class ScannerTask : public QObject, public QRunnable { void taskDone(bool success); void queueTask(ScannerTask* pTask); void directoryHashedAndScanned(const QString& directoryPath, - bool newDirectory, int hash); + bool newDirectory, mixxx::cache_key_t hash); void directoryUnchanged(const QString& directoryPath); void trackExists(const QString& filePath); void addNewTrack(const QString& filePath); diff --git a/src/mixxxapplication.cpp b/src/mixxxapplication.cpp index 91cc0586c96..412c342cc15 100644 --- a/src/mixxxapplication.cpp +++ b/src/mixxxapplication.cpp @@ -9,6 +9,7 @@ #include "soundio/soundmanagerutil.h" #include "track/track.h" #include "track/trackref.h" +#include "util/cache.h" #include "util/color/rgbcolor.h" #include "util/math.h" @@ -66,6 +67,7 @@ void MixxxApplication::registerMetaTypes() { qRegisterMetaType>(); qRegisterMetaType(); qRegisterMetaType("mixxx::ReplayGain"); + qRegisterMetaType("mixxx::cache_key_t"); qRegisterMetaType("mixxx::Bpm"); qRegisterMetaType("mixxx::Duration"); qRegisterMetaType>("std::optional"); diff --git a/src/test/cache_test.cpp b/src/test/cache_test.cpp new file mode 100644 index 00000000000..d6f59f7ce43 --- /dev/null +++ b/src/test/cache_test.cpp @@ -0,0 +1,41 @@ +#include "util/cache.h" + +#include + +#include + +namespace { + +class CacheTest : public testing::Test { +}; + +TEST_F(CacheTest, InvalidCacheKey) { + EXPECT_FALSE(mixxx::isValidCacheKey( + mixxx::invalidCacheKey())); +} + +TEST_F(CacheTest, InvalidCacheKeyEmptyByteArray) { + EXPECT_FALSE(mixxx::isValidCacheKey( + mixxx::cacheKeyFromMessageDigest(QByteArray()))); + EXPECT_FALSE(mixxx::isValidCacheKey( + mixxx::cacheKeyFromMessageDigest(""))); + EXPECT_FALSE(mixxx::isValidCacheKey( + mixxx::cacheKeyFromMessageDigest("\0"))); +} + +TEST_F(CacheTest, ValidCacheKey) { + EXPECT_TRUE(mixxx::isValidCacheKey( + mixxx::cacheKeyFromMessageDigest(QByteArrayLiteral("test")))); +} + +TEST_F(CacheTest, ValidCacheKeySingleZeroAscii) { + EXPECT_TRUE(mixxx::isValidCacheKey( + mixxx::cacheKeyFromMessageDigest(QByteArray("0")))); +} + +TEST_F(CacheTest, ValidCacheKeySingleZeroByte) { + EXPECT_TRUE(mixxx::isValidCacheKey( + mixxx::cacheKeyFromMessageDigest(QByteArray("\0", 1)))); +} + +} // anonymous namespace diff --git a/src/util/cache.cpp b/src/util/cache.cpp new file mode 100644 index 00000000000..021efc19b5e --- /dev/null +++ b/src/util/cache.cpp @@ -0,0 +1,38 @@ +#include "util/cache.h" + +#include "util/assert.h" +#include "util/math.h" + +namespace mixxx { + +cache_key_t cacheKeyFromMessageDigest(const QByteArray& messageDigest) { + cache_key_t key = invalidCacheKey(); + DEBUG_ASSERT(!isValidCacheKey(key)); + if (messageDigest.isEmpty()) { + return key; + } + // Source: FIPS 180-4 Secure Hash Standard (SHS) + // SP 800-107 + // 5 Hash function Usage + // 5.1 Truncated Message Digest + const auto significantByteCount = math_min( + messageDigest.size(), + static_cast(sizeof(cache_key_t))); + for (auto i = 0; i < significantByteCount; ++i) { + // Only 8 bits are relevant and we don't want the sign + // extension of a (signed) char during the conversion. + const cache_key_t nextByte = + static_cast(messageDigest.at(i)); + DEBUG_ASSERT(nextByte == (nextByte & 0xFF)); + key <<= 8; + key |= nextByte; + } + if (!isValidCacheKey(key)) { + // Unlikely but possible + key = ~key; + } + DEBUG_ASSERT(isValidCacheKey(key)); + return key; +} + +} // namespace mixxx diff --git a/src/util/cache.h b/src/util/cache.h new file mode 100644 index 00000000000..c4b4844ff72 --- /dev/null +++ b/src/util/cache.h @@ -0,0 +1,38 @@ +#pragma once + +#include +#include +#include // static_assert + +namespace mixxx { + +typedef quint64 cache_key_t; + +// A signed integer is needed for storing cache keys as +// integer numbers with full precision in the database. +typedef qint64 cache_key_signed_t; + +inline constexpr cache_key_t invalidCacheKey() { + return 0; +} + +inline constexpr bool isValidCacheKey(cache_key_t key) { + return key != invalidCacheKey(); +} + +inline constexpr cache_key_t signedCacheKey(cache_key_t cacheKey) { + static_assert( + sizeof(cache_key_t) == sizeof(cache_key_signed_t), + "size mismatch of signed/unsigned cache key types"); + return static_cast(cacheKey); +} + +// Truncate a message digest to obtain a cache key. +// +// The message digest should either be empty or contain at least as many +// bytes as the size (in bytes) of the cache key. +cache_key_t cacheKeyFromMessageDigest(const QByteArray& messageDigest); + +} // namespace mixxx + +Q_DECLARE_METATYPE(mixxx::cache_key_t);