Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DB: Fix 0/NULL issues when reading source_synchronized_ms #4012

Merged
merged 7 commits into from
Jun 17, 2021
8 changes: 8 additions & 0 deletions res/schema.xml
Original file line number Diff line number Diff line change
Expand Up @@ -566,4 +566,12 @@ reapplying those migrations.
ALTER TABLE library ADD COLUMN source_synchronized_ms INTEGER DEFAULT NULL;
</sql>
</revision>
<revision version="38" min_compatible="3">
<description>
Fix 0/NULL issue after upgrade to schema version 37.
</description>
<sql>
UPDATE library SET source_synchronized_ms=NULL WHERE source_synchronized_ms=0;
</sql>
</revision>
</schema>
2 changes: 1 addition & 1 deletion src/database/mixxxdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
const QString MixxxDb::kDefaultSchemaFile(":/schema.xml");

//static
const int MixxxDb::kRequiredSchemaVersion = 37;
const int MixxxDb::kRequiredSchemaVersion = 38;

namespace {

Expand Down
6 changes: 5 additions & 1 deletion src/library/dao/trackdao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,11 @@ bool setTrackHeaderParsed(const QSqlRecord& record, const int column, TrackPoint
bool setTrackSourceSynchronizedAt(const QSqlRecord& record, const int column, TrackPointer pTrack) {
QDateTime sourceSynchronizedAt;
const QVariant value = record.value(column);
if (value.isValid() && value.canConvert<quint64>()) {
// Observation (Qt 5.15): QVariant::isValid() may return true even
// if the column value is NULL and then converts to 0 (zero). This
// is NOT desired and therefore we MUST USE QVariant::isNull() instead!
if (!value.isNull() && value.canConvert<quint64>()) {
DEBUG_ASSERT(value.isValid());
const quint64 msecsSinceEpoch = qvariant_cast<quint64>(value);
sourceSynchronizedAt.setTimeSpec(Qt::UTC);
sourceSynchronizedAt.setMSecsSinceEpoch(msecsSinceEpoch);
Expand Down
8 changes: 6 additions & 2 deletions src/library/export/engineprimeexportjob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,12 @@ void exportMetadata(djinterop::database* pDatabase,
snapshot.comment = pTrack->getComment().toStdString();
snapshot.composer = pTrack->getComposer().toStdString();
snapshot.key = toDjinteropKey(pTrack->getKey());
int64_t lastModifiedMillisSinceEpoch =
pTrack->getFileInfo().lastModified().toMSecsSinceEpoch();
int64_t lastModifiedMillisSinceEpoch = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mr-smidge I fixed a possible edge case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you 👍

const QDateTime fileLastModified = pTrack->getFileInfo().lastModified();
if (fileLastModified.isValid()) {
// Only defined if valid
lastModifiedMillisSinceEpoch = fileLastModified.toMSecsSinceEpoch();
}
std::chrono::system_clock::time_point lastModifiedAt{
std::chrono::milliseconds{lastModifiedMillisSinceEpoch}};
snapshot.last_modified_at = lastModifiedAt;
Expand Down
11 changes: 10 additions & 1 deletion src/sources/metadatasourcetaglib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,16 @@ class AiffFile : public TagLib::RIFF::AIFF::File {
};

inline QDateTime getSourceSynchronizedAt(const QFileInfo& fileInfo) {
return fileInfo.lastModified().toUTC();
const QDateTime lastModifiedUtc = fileInfo.lastModified().toUTC();
// Ignore bogus values like 1970-01-01T00:00:00.000 UTC
// that are obviously incorrect and don't provide any
// information.
if (lastModifiedUtc.isValid() &&
// Only defined if valid
lastModifiedUtc.toMSecsSinceEpoch() == 0) {
return QDateTime{};
}
return lastModifiedUtc;
}

} // anonymous namespace
Expand Down
8 changes: 8 additions & 0 deletions src/track/trackrecord.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ bool TrackRecord::updateSourceSynchronizedAt(
if (getSourceSynchronizedAt() == sourceSynchronizedAt) {
return false; // unchanged
}
if (getSourceSynchronizedAt().isValid() &&
getSourceSynchronizedAt() > sourceSynchronizedAt) {
kLogger.warning()
<< "Backdating source synchronization time from"
<< getSourceSynchronizedAt()
<< "to"
<< sourceSynchronizedAt;
}
setSourceSynchronizedAt(sourceSynchronizedAt);
m_headerParsed = sourceSynchronizedAt.isValid();
DEBUG_ASSERT(isSourceSynchronized());
Expand Down