Merge pull request #4012 from uklotzde/source_synchronized_ms

DB: Fix 0/NULL issues when reading source_synchronized_ms

res/schema.xml

@@ -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>

src/database/mixxxdb.cpp

@@ -12,7 +12,7 @@
 const QString MixxxDb::kDefaultSchemaFile(":/schema.xml");
 
 //static
-const int MixxxDb::kRequiredSchemaVersion = 37;
+const int MixxxDb::kRequiredSchemaVersion = 38;
 
 namespace {
 

src/library/dao/trackdao.cpp

@@ -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);

src/library/export/engineprimeexportjob.cpp

@@ -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;
+    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;

src/sources/metadatasourcetaglib.cpp

@@ -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

src/track/trackrecord.cpp

@@ -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());