From e9bd24ee35da39c749fdc5d559db873cc969637a Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 24 Dec 2020 14:22:59 +0100 Subject: [PATCH 1/5] Fix reading/writing of last_played_at values --- CMakeLists.txt | 1 + src/library/basetracktablemodel.cpp | 14 +++++++++- src/library/dao/trackdao.cpp | 8 +++--- src/util/db/sqlite.cpp | 41 +++++++++++++++++++++++++++++ src/util/db/sqlite.h | 12 +++++++++ 5 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 src/util/db/sqlite.cpp create mode 100644 src/util/db/sqlite.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 49b00e8a7b4..d0802e0ba21 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -804,6 +804,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/util/db/fwdsqlquery.cpp src/util/db/fwdsqlqueryselectresult.cpp src/util/db/sqllikewildcardescaper.cpp + src/util/db/sqlite.cpp src/util/db/sqlqueryfinisher.cpp src/util/db/sqlstringformatter.cpp src/util/db/sqltransaction.cpp diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index 2f8da7509f2..9151d453cf9 100644 --- a/src/library/basetracktablemodel.cpp +++ b/src/library/basetracktablemodel.cpp @@ -18,6 +18,7 @@ #include "util/assert.h" #include "util/compatibility.h" #include "util/datetime.h" +#include "util/db/sqlite.h" #include "util/logger.h" #include "widget/wlibrary.h" #include "widget/wtracktableview.h" @@ -618,12 +619,23 @@ QVariant BaseTrackTableModel::roleValue( return QString("(%1)").arg(timesPlayed); } case ColumnCache::COLUMN_LIBRARYTABLE_DATETIMEADDED: - case ColumnCache::COLUMN_LIBRARYTABLE_LAST_PLAYED_AT: case ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_DATETIMEADDED: VERIFY_OR_DEBUG_ASSERT(rawValue.canConvert()) { return QVariant(); } return mixxx::localDateTimeFromUtc(rawValue.toDateTime()); + case ColumnCache::COLUMN_LIBRARYTABLE_LAST_PLAYED_AT: { + QDateTime lastPlayedAt; + if (rawValue.type() == QVariant::String) { + // column value + lastPlayedAt = sqlite::readGeneratedTimestamp(rawValue); + } else { + // cached in memory (local time) + lastPlayedAt = rawValue.toDateTime().toUTC(); + } + DEBUG_ASSERT(lastPlayedAt.timeSpec() == Qt::UTC); + return mixxx::localDateTimeFromUtc(lastPlayedAt); + } case ColumnCache::COLUMN_LIBRARYTABLE_BPM: { mixxx::Bpm bpm; if (!rawValue.isNull()) { diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 9042309c7f4..860e40e85a9 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -31,6 +31,7 @@ #include "util/compatibility.h" #include "util/datetime.h" #include "util/db/fwdsqlquery.h" +#include "util/db/sqlite.h" #include "util/db/sqllikewildcardescaper.h" #include "util/db/sqllikewildcards.h" #include "util/db/sqlstringformatter.h" @@ -554,7 +555,8 @@ void bindTrackLibraryValues( const PlayCounter& playCounter = track.getPlayCounter(); pTrackLibraryQuery->bindValue(":timesplayed", playCounter.getTimesPlayed()); - pTrackLibraryQuery->bindValue(":last_played_at", playCounter.getLastPlayedAt()); + pTrackLibraryQuery->bindValue(":last_played_at", + sqlite::writeGeneratedTimestamp(playCounter.getLastPlayedAt())); pTrackLibraryQuery->bindValue(":played", playCounter.isPlayed() ? 1 : 0); const CoverInfoRelative& coverInfo = track.getCoverInfo(); @@ -1170,8 +1172,8 @@ bool setTrackPlayed(const QSqlRecord& record, const int column, } bool setTrackLastPlayedAt(const QSqlRecord& record, const int column, TrackPointer pTrack) { - PlayCounter playCounter(pTrack->getPlayCounter()); - playCounter.setLastPlayedAt(record.value(column).toDateTime()); + auto playCounter = pTrack->getPlayCounter(); + playCounter.setLastPlayedAt(sqlite::readGeneratedTimestamp(record.value(column))); pTrack->setPlayCounter(playCounter); return false; } diff --git a/src/util/db/sqlite.cpp b/src/util/db/sqlite.cpp new file mode 100644 index 00000000000..f5d92772662 --- /dev/null +++ b/src/util/db/sqlite.cpp @@ -0,0 +1,41 @@ +#include "util/db/sqlite.h" + +#include "util/assert.h" + +namespace { + +/// Date/time formate generated by SQLite CURRENT_TIMESTAMP (UTC) that is +/// used for library.last_played_at and PlalistTracks.pl_datetime_added. +const QString kGeneratedTimestampFormat = QStringLiteral("yyyy-MM-dd hh:mm:ss"); +const QString kGeneratedTimestampDateFormat = QStringLiteral("yyyy-MM-dd"); +const QString kGeneratedTimestampTimeFormat = QStringLiteral("hh:mm:ss"); + +} // namespace + +namespace sqlite { + +QDateTime readGeneratedTimestamp(const QVariant& value) { + const auto timestamp = value.toString(); + if (timestamp.isEmpty()) { + return QDateTime(); + } + const auto parts = timestamp.split(QChar(' ')); + VERIFY_OR_DEBUG_ASSERT(parts.size() == 2) { + return QDateTime(); + } + const auto date = QDate::fromString(parts[0], kGeneratedTimestampDateFormat); + VERIFY_OR_DEBUG_ASSERT(date.isValid()) { + return QDateTime(); + } + const auto time = QTime::fromString(parts[1], kGeneratedTimestampTimeFormat); + VERIFY_OR_DEBUG_ASSERT(time.isValid()) { + return QDateTime(); + } + return QDateTime(date, time, Qt::UTC); +} + +QVariant writeGeneratedTimestamp(const QDateTime& value) { + return value.toUTC().toString(kGeneratedTimestampFormat); +} + +} // namespace sqlite diff --git a/src/util/db/sqlite.h b/src/util/db/sqlite.h new file mode 100644 index 00000000000..b6b41bef250 --- /dev/null +++ b/src/util/db/sqlite.h @@ -0,0 +1,12 @@ +#pragma once + +#include +#include + +namespace sqlite { + +QDateTime readGeneratedTimestamp(const QVariant& value); + +QVariant writeGeneratedTimestamp(const QDateTime& value); + +} // namespace sqlite From 86d15768fb78f1e8598227bb3a8ed0e38bd548a4 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Thu, 24 Dec 2020 11:34:26 +0100 Subject: [PATCH 2/5] Split database migration steps 35/36 to fix last_played_at --- res/schema.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/res/schema.xml b/res/schema.xml index 1587139718e..db570e886ba 100644 --- a/res/schema.xml +++ b/res/schema.xml @@ -524,6 +524,15 @@ METADATA Add last_played_at column to library table + + -- Add new column + ALTER TABLE library ADD COLUMN last_played_at DATETIME DEFAULT NULL; + + + + + Populate last_played_at column in library table from play history + -- Add new column ALTER TABLE library ADD COLUMN last_played_at DATETIME DEFAULT NULL; From 9de685d2b3041d14b5746d1b73adb7cd46dab269 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sat, 26 Dec 2020 17:14:41 +0100 Subject: [PATCH 3/5] Add another constant for the date/time separator QStringBuilder doesn't work for constants! --- src/util/db/sqlite.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/db/sqlite.cpp b/src/util/db/sqlite.cpp index f5d92772662..4021f833bed 100644 --- a/src/util/db/sqlite.cpp +++ b/src/util/db/sqlite.cpp @@ -6,9 +6,12 @@ namespace { /// Date/time formate generated by SQLite CURRENT_TIMESTAMP (UTC) that is /// used for library.last_played_at and PlalistTracks.pl_datetime_added. -const QString kGeneratedTimestampFormat = QStringLiteral("yyyy-MM-dd hh:mm:ss"); +/// QStringBuilder cannot be used to compose this constant at compile time! +const QString kGeneratedTimestampFormat = + QStringLiteral("yyyy-MM-dd hh:mm:ss"); // date + separator + time const QString kGeneratedTimestampDateFormat = QStringLiteral("yyyy-MM-dd"); const QString kGeneratedTimestampTimeFormat = QStringLiteral("hh:mm:ss"); +const QChar kGeneratedTimestampDateTimeSeparator = QChar(' '); } // namespace @@ -19,7 +22,7 @@ QDateTime readGeneratedTimestamp(const QVariant& value) { if (timestamp.isEmpty()) { return QDateTime(); } - const auto parts = timestamp.split(QChar(' ')); + const auto parts = timestamp.split(kGeneratedTimestampDateTimeSeparator); VERIFY_OR_DEBUG_ASSERT(parts.size() == 2) { return QDateTime(); } From 1f370b2dfc33d6e8e976eab4464b82156c3db6cb Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 27 Dec 2020 11:21:40 +0100 Subject: [PATCH 4/5] Add debug assertion for date/time formats --- src/util/db/sqlite.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/db/sqlite.cpp b/src/util/db/sqlite.cpp index 4021f833bed..61fdddc7a2f 100644 --- a/src/util/db/sqlite.cpp +++ b/src/util/db/sqlite.cpp @@ -18,6 +18,10 @@ const QChar kGeneratedTimestampDateTimeSeparator = QChar(' '); namespace sqlite { QDateTime readGeneratedTimestamp(const QVariant& value) { + DEBUG_ASSERT(kGeneratedTimestampFormat == + kGeneratedTimestampDateFormat + + kGeneratedTimestampDateTimeSeparator + + kGeneratedTimestampTimeFormat); const auto timestamp = value.toString(); if (timestamp.isEmpty()) { return QDateTime(); From 2ffbd012def4513a8e03262e063de7f44bf639d1 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 27 Dec 2020 19:18:27 +0100 Subject: [PATCH 5/5] Rename namespace mixxx::sqlite --- src/library/basetracktablemodel.cpp | 2 +- src/library/dao/trackdao.cpp | 5 +++-- src/util/db/sqlite.cpp | 4 ++++ src/util/db/sqlite.h | 3 +++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/library/basetracktablemodel.cpp b/src/library/basetracktablemodel.cpp index 9151d453cf9..c178a0eb69d 100644 --- a/src/library/basetracktablemodel.cpp +++ b/src/library/basetracktablemodel.cpp @@ -628,7 +628,7 @@ QVariant BaseTrackTableModel::roleValue( QDateTime lastPlayedAt; if (rawValue.type() == QVariant::String) { // column value - lastPlayedAt = sqlite::readGeneratedTimestamp(rawValue); + lastPlayedAt = mixxx::sqlite::readGeneratedTimestamp(rawValue); } else { // cached in memory (local time) lastPlayedAt = rawValue.toDateTime().toUTC(); diff --git a/src/library/dao/trackdao.cpp b/src/library/dao/trackdao.cpp index 860e40e85a9..954db07f63c 100644 --- a/src/library/dao/trackdao.cpp +++ b/src/library/dao/trackdao.cpp @@ -556,7 +556,7 @@ void bindTrackLibraryValues( const PlayCounter& playCounter = track.getPlayCounter(); pTrackLibraryQuery->bindValue(":timesplayed", playCounter.getTimesPlayed()); pTrackLibraryQuery->bindValue(":last_played_at", - sqlite::writeGeneratedTimestamp(playCounter.getLastPlayedAt())); + mixxx::sqlite::writeGeneratedTimestamp(playCounter.getLastPlayedAt())); pTrackLibraryQuery->bindValue(":played", playCounter.isPlayed() ? 1 : 0); const CoverInfoRelative& coverInfo = track.getCoverInfo(); @@ -1173,7 +1173,8 @@ bool setTrackPlayed(const QSqlRecord& record, const int column, bool setTrackLastPlayedAt(const QSqlRecord& record, const int column, TrackPointer pTrack) { auto playCounter = pTrack->getPlayCounter(); - playCounter.setLastPlayedAt(sqlite::readGeneratedTimestamp(record.value(column))); + playCounter.setLastPlayedAt( + mixxx::sqlite::readGeneratedTimestamp(record.value(column))); pTrack->setPlayCounter(playCounter); return false; } diff --git a/src/util/db/sqlite.cpp b/src/util/db/sqlite.cpp index 61fdddc7a2f..46f9e8e4152 100644 --- a/src/util/db/sqlite.cpp +++ b/src/util/db/sqlite.cpp @@ -15,6 +15,8 @@ const QChar kGeneratedTimestampDateTimeSeparator = QChar(' '); } // namespace +namespace mixxx { + namespace sqlite { QDateTime readGeneratedTimestamp(const QVariant& value) { @@ -46,3 +48,5 @@ QVariant writeGeneratedTimestamp(const QDateTime& value) { } } // namespace sqlite + +} // namespace mixxx diff --git a/src/util/db/sqlite.h b/src/util/db/sqlite.h index b6b41bef250..da5aec9d765 100644 --- a/src/util/db/sqlite.h +++ b/src/util/db/sqlite.h @@ -3,6 +3,7 @@ #include #include +namespace mixxx { namespace sqlite { QDateTime readGeneratedTimestamp(const QVariant& value); @@ -10,3 +11,5 @@ QDateTime readGeneratedTimestamp(const QVariant& value); QVariant writeGeneratedTimestamp(const QDateTime& value); } // namespace sqlite + +} // namespace mixxx