diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index a6a51440985..282b05a86f7 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -33,12 +33,15 @@ #include "track/tracknumbers.h" #include "util/assert.h" #include "util/file.h" +#include "util/logger.h" #include "util/timer.h" #include "util/math.h" namespace { +mixxx::Logger kLogger("TrackDAO"); + enum { UndefinedRecordIndex = -2 }; void markTrackLocationsAsDeleted(QSqlDatabase database, const QString& directory) { @@ -1241,11 +1244,15 @@ TrackPointer TrackDAO::getTrackFromDB(TrackId trackId) const { "INNER JOIN track_locations ON library.location = track_locations.id " "WHERE library.id = %2").arg(columnsStr, trackId.toString())); - if (!query.exec() || !query.next()) { + if (!query.exec()) { LOG_FAILED_QUERY(query) << QString("getTrack(%1)").arg(trackId.toString()); return TrackPointer(); } + if (!query.next()) { + qDebug() << "Track with id =" << trackId << "not found"; + return TrackPointer(); + } QSqlRecord queryRecord = query.record(); int recordCount = queryRecord.count(); @@ -1531,6 +1538,22 @@ void TrackDAO::markUnverifiedTracksAsDeleted() { } } +namespace { + // Computed the longest match from the right of both strings + int matchStringSuffix(const QString& str1, const QString& str2) { + int matchLength = 0; + int minLength = math_min(str1.length(), str2.length()); + while (matchLength < minLength) { + if (str1[str1.length() - matchLength - 1] != str2[str2.length() - matchLength - 1]) { + // first mismatch + break; + } + ++matchLength; + } + return matchLength; + } +} + // Look for moved files. Look for files that have been marked as // "deleted on disk" and see if another "file" with the same name and // files size exists in the track_locations table. That means the file has @@ -1545,7 +1568,7 @@ bool TrackDAO::detectMovedTracks(QSet* pTracksMovedSetOld, // When it's called from libraryscanner.cpp, there already is a transaction // started! - if (!addedTracks.size()) { + if (addedTracks.isEmpty()) { // We have no moved track. // We can only guarantee for new tracks that the user has not // edited metadata, which we have to preserve @@ -1553,135 +1576,150 @@ bool TrackDAO::detectMovedTracks(QSet* pTracksMovedSetOld, return true; } - QSqlQuery deletedTrackQuery(m_database); - QSqlQuery newTrackQuery(m_database); - QSqlQuery query(m_database); - QString filename; - // rather use duration then filesize as an indicator of changes. The filesize - // can change by adding more ID3v2 tags - int duration = -1; - - // Query tracks, where we need a successor for - deletedTrackQuery.prepare("SELECT track_locations.id, filename, duration FROM track_locations " - "INNER JOIN library ON track_locations.id=library.location " - "WHERE fs_deleted=1"); - - if (!deletedTrackQuery.exec()) { - LOG_FAILED_QUERY(deletedTrackQuery); - } - // Query possible successors // NOTE: Successors are identified by filename and duration (in seconds). // Since duration is stored as double-precision floating-point and since it // is sometimes truncated to nearest integer, tolerance of 1 second is used. - newTrackQuery.prepare( - QString("SELECT track_locations.id FROM track_locations " - "INNER JOIN library ON track_locations.id=library.location " - "WHERE track_locations.location IN (%1) AND " - "filename=:filename AND " - "ABS(duration - :duration) < 1").arg( - SqlStringFormatter::formatList(m_database, addedTracks))); - - QSqlRecord queryRecord = deletedTrackQuery.record(); - const int idColumn = queryRecord.indexOf("id"); - const int filenameColumn = queryRecord.indexOf("filename"); - const int durationColumn = queryRecord.indexOf("duration"); + QSqlQuery newTrackQuery(m_database); + newTrackQuery.prepare(QString( + "SELECT library.id as track_id, track_locations.id as location_id, " + "track_locations.location " + "FROM library INNER JOIN track_locations ON library.location=track_locations.id " + "WHERE track_locations.location IN (%1) AND " + "filename=:filename AND " + "ABS(duration - :duration) < 1 AND " + "fs_deleted=0").arg( + SqlStringFormatter::formatList(m_database, addedTracks))); + + // Query tracks, where we need a successor for + QSqlQuery oldTrackQuery(m_database); + oldTrackQuery.prepare( + "SELECT library.id as track_id, track_locations.id as location_id, " + "track_locations.location, filename, duration " + "FROM library INNER JOIN track_locations ON library.location=track_locations.id " + "WHERE fs_deleted=1"); + VERIFY_OR_DEBUG_ASSERT(oldTrackQuery.exec()) { + LOG_FAILED_QUERY(oldTrackQuery); + return false; + } + QSqlRecord oldTrackQueryRecord = oldTrackQuery.record(); + const int oldTrackIdColumn = oldTrackQueryRecord.indexOf("track_id"); + const int oldLocationIdColumn = oldTrackQueryRecord.indexOf("location_id"); + const int oldLocationColumn = oldTrackQueryRecord.indexOf("location"); + const int filenameColumn = oldTrackQueryRecord.indexOf("filename"); + const int durationColumn = oldTrackQueryRecord.indexOf("duration"); // For each track that's been "deleted" on disk... - while (deletedTrackQuery.next()) { + while (oldTrackQuery.next()) { if (*pCancel) { return false; } - DbId newTrackLocationId; - DbId oldTrackLocationId(deletedTrackQuery.value(idColumn)); - filename = deletedTrackQuery.value(filenameColumn).toString(); - duration = deletedTrackQuery.value(durationColumn).toInt(); + QString oldTrackLocation = oldTrackQuery.value(oldLocationColumn).toString(); + QString filename = oldTrackQuery.value(filenameColumn).toString(); + // rather use duration then filesize as an indicator of changes. The filesize + // can change by adding more ID3v2 tags + const int duration = oldTrackQuery.value(durationColumn).toInt(); - qDebug() << "TrackDAO::detectMovedTracks looking for a successor of" << filename << duration; + kLogger.info() + << "Looking for substitute of missing track location" + << oldTrackLocation; newTrackQuery.bindValue(":filename", filename); newTrackQuery.bindValue(":duration", duration); - if (!newTrackQuery.exec()) { - // Should not happen! + VERIFY_OR_DEBUG_ASSERT(newTrackQuery.exec()) { LOG_FAILED_QUERY(newTrackQuery); + continue; } - // WTF duplicate tracks? - if (newTrackQuery.size() > 1) { - LOG_FAILED_QUERY(newTrackQuery) << "Result size was greater than 1."; - } - - const int query2idColumn = newTrackQuery.record().indexOf("id"); + const auto newTrackIdColumn = newTrackQuery.record().indexOf("track_id"); + const auto newTrackLocationIdColumn = newTrackQuery.record().indexOf("location_id"); + const auto newTrackLocationColumn = newTrackQuery.record().indexOf("location"); + int newTrackLocationSuffixMatch = 0; + TrackId newTrackId; + DbId newTrackLocationId; + QString newTrackLocation; while (newTrackQuery.next()) { - newTrackLocationId = DbId(newTrackQuery.value(query2idColumn)); - } - - //If we found a moved track... - if (newTrackLocationId.isValid()) { - qDebug() << "Found moved track!" << filename; - - // Remove old row from track_locations table - query.prepare("DELETE FROM track_locations WHERE id=:id"); - query.bindValue(":id", oldTrackLocationId.toVariant()); - if (!query.exec()) { - // Should not happen! - LOG_FAILED_QUERY(query); - } - - // The library scanner will have added a new row to the Library - // table which corresponds to the track in the new location. We need - // to remove that so we don't end up with two rows in the library - // table for the same track. - query.prepare("SELECT id FROM library WHERE location=:location"); - query.bindValue(":location", newTrackLocationId.toVariant()); - if (!query.exec()) { - // Should not happen! - LOG_FAILED_QUERY(query); + const auto nextTrackLocation = + newTrackQuery.value(newTrackLocationColumn).toString(); + VERIFY_OR_DEBUG_ASSERT(nextTrackLocation != oldTrackLocation) { + continue; } - - const int query3idColumn = query.record().indexOf("id"); - if (query.next()) { - TrackId newTrackId(query.value(query3idColumn)); - query.prepare("DELETE FROM library WHERE id=:newid"); - query.bindValue(":newid", newTrackLocationId.toVariant()); - if (!query.exec()) { - // Should not happen! - LOG_FAILED_QUERY(query); - } - // We collect all the new tracks the where added to BaseTrackCache as well - pTracksMovedSetNew->insert(newTrackId); + kLogger.info() + << "Found potential moved track location:" + << nextTrackLocation; + const auto nextSuffixMatch = + matchStringSuffix(nextTrackLocation, oldTrackLocation); + DEBUG_ASSERT(nextSuffixMatch >= filename.length()); + if (newTrackLocationSuffixMatch < nextSuffixMatch) { + newTrackLocationSuffixMatch = nextSuffixMatch; + newTrackId = TrackId(newTrackQuery.value(newTrackIdColumn)); + newTrackLocationId = DbId(newTrackQuery.value(newTrackLocationIdColumn)); + newTrackLocation = nextTrackLocation; } - // Delete the track + } + if (newTrackLocation.isEmpty()) { + kLogger.info() + << "Found no substitute for missing track location" + << oldTrackLocation; + continue; + } + DEBUG_ASSERT(newTrackId.isValid()); + DEBUG_ASSERT(newTrackLocationId.isValid()); + kLogger.info() + << "Found moved track location:" + << oldTrackLocation + << "->" + << newTrackLocation; + + TrackId oldTrackId(oldTrackQuery.value(oldTrackIdColumn)); + DEBUG_ASSERT(oldTrackId.isValid()); + DbId oldTrackLocationId(oldTrackQuery.value(oldLocationIdColumn)); + DEBUG_ASSERT(oldTrackLocationId.isValid()); + + // The queries ensure that the following assertions are always true (fs_deleted=0/1)! + DEBUG_ASSERT(oldTrackId != newTrackId); + DEBUG_ASSERT(oldTrackLocationId != newTrackLocationId); + + // The library scanner will have added a new row to the Library + // table which corresponds to the track in the new location. We need + // to remove that so we don't end up with two rows in the library + // table for the same track. + { + QSqlQuery query(m_database); query.prepare("DELETE FROM library WHERE id=:newid"); - query.bindValue(":newid", newTrackLocationId.toVariant()); - if (!query.exec()) { - // Should not happen! + query.bindValue(":newid", newTrackId.toVariant()); + VERIFY_OR_DEBUG_ASSERT(query.exec()) { LOG_FAILED_QUERY(query); + // Last chance to skip this entry, i.e. nothing has been + // deleted or updated yet! + continue; } - - // Update the location foreign key for the existing row in the - // library table to point to the correct row in the track_locations - // table. - query.prepare("SELECT id FROM library WHERE location=:location"); - query.bindValue(":location", oldTrackLocationId.toVariant()); - if (!query.exec()) { - // Should not happen! + } + // Update the location foreign key for the existing row in the + // library table to point to the correct row in the track_locations + // table. + { + QSqlQuery query(m_database); + query.prepare("UPDATE library SET location=:newloc WHERE id=:oldid"); + query.bindValue(":newloc", newTrackLocationId.toVariant()); + query.bindValue(":oldid", oldTrackId.toVariant()); + VERIFY_OR_DEBUG_ASSERT(query.exec()) { LOG_FAILED_QUERY(query); } - - if (query.next()) { - TrackId oldTrackId(query.value(query3idColumn)); - query.prepare("UPDATE library SET location=:newloc WHERE id=:oldid"); - query.bindValue(":newloc", newTrackLocationId.toVariant()); - query.bindValue(":oldid", oldTrackId.toVariant()); - if (!query.exec()) { - // Should not happen! - LOG_FAILED_QUERY(query); - } - - // We collect all the old tracks that has to be updated in BaseTrackCache as well - pTracksMovedSetOld->insert(oldTrackId); + } + // Remove old, orphaned row from track_locations table + { + QSqlQuery query(m_database); + query.prepare("DELETE FROM track_locations WHERE id=:id"); + query.bindValue(":id", oldTrackLocationId.toVariant()); + VERIFY_OR_DEBUG_ASSERT(query.exec()) { + LOG_FAILED_QUERY(query); } } + + // We collect all the old tracks that has to be updated in BaseTrackCache + pTracksMovedSetOld->insert(oldTrackId); + // We collect collect all the new tracks the where added and deleted to BaseTrackCache + pTracksMovedSetNew->insert(newTrackId); } return true; } diff --git a/src/library/queryutil.h b/src/library/queryutil.h index e7ab94cdfea..fe28dba7779 100644 --- a/src/library/queryutil.h +++ b/src/library/queryutil.h @@ -5,7 +5,7 @@ #include -#define LOG_FAILED_QUERY(query) qDebug() << __FILE__ << __LINE__ << "FAILED QUERY [" \ +#define LOG_FAILED_QUERY(query) qWarning() << __FILE__ << __LINE__ << "FAILED QUERY [" \ << (query).executedQuery() << "]" << (query).lastError() class ScopedTransaction { diff --git a/src/test/trackdao_test.cpp b/src/test/trackdao_test.cpp index 2a8a0344a30..2e43ee87a13 100644 --- a/src/test/trackdao_test.cpp +++ b/src/test/trackdao_test.cpp @@ -14,19 +14,23 @@ TEST_F(TrackDAOTest, detectMovedTracks) { QString filename("file.mp3"); - QString oldFile(QDir::tempPath() + "/old/" + filename); - QString newFile(QDir::tempPath() + "/new/" + filename); + QString oldFile(QDir::tempPath() + "/old/dir1" + filename); + QString newFile(QDir::tempPath() + "/new/dir1" + filename); + QString otherFile(QDir::tempPath() + "/new" + filename); TrackPointer pOldTrack = Track::newTemporary(oldFile); TrackPointer pNewTrack = Track::newTemporary(newFile); + TrackPointer pOtherTrack = Track::newTemporary(otherFile); // Arbitrary duration pOldTrack->setDuration(135); pNewTrack->setDuration(135.7); + pOtherTrack->setDuration(135.7); trackDAO.addTracksPrepare(); TrackId oldId = trackDAO.addTracksAddTrack(pOldTrack, false); TrackId newId = trackDAO.addTracksAddTrack(pNewTrack, false); + trackDAO.addTracksAddTrack(pOtherTrack, false); trackDAO.addTracksFinish(false); // Mark as missing @@ -45,5 +49,5 @@ TEST_F(TrackDAOTest, detectMovedTracks) { EXPECT_THAT(tracksMovedSetNew, UnorderedElementsAre(newId)); QSet trackLocations = trackDAO.getTrackLocations(); - EXPECT_THAT(trackLocations, UnorderedElementsAre(newFile)); + EXPECT_THAT(trackLocations, UnorderedElementsAre(newFile, otherFile)); }