Skip to content

Commit

Permalink
Merge pull request #2197 from uklotzde/2.2_fix_detection_of_moved_tracks
Browse files Browse the repository at this point in the history 
Fix detection of moved tracks
  • Loading branch information
@daschuer
daschuer authored Jul 7, 2019
2 parents 3ae5a7b + 726a216 commit ff98230
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 110 deletions.
250 changes: 144 additions & 106 deletions src/library/dao/trackdao.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -1545,143 +1568,158 @@ bool TrackDAO::detectMovedTracks(QSet<TrackId>* 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
// TODO(xxx) resolve old duplicates
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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/library/queryutil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <QtSql>


#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 {
Expand Down
10 changes: 7 additions & 3 deletions src/test/trackdao_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -45,5 +49,5 @@ TEST_F(TrackDAOTest, detectMovedTracks) {
EXPECT_THAT(tracksMovedSetNew, UnorderedElementsAre(newId));

QSet<QString> trackLocations = trackDAO.getTrackLocations();
EXPECT_THAT(trackLocations, UnorderedElementsAre(newFile));
EXPECT_THAT(trackLocations, UnorderedElementsAre(newFile, otherFile));
}

0 comments on commit ff98230

Please sign in to comment.