17 files changed, 800 insertions, 956 deletions
diff --git a/src/library/dao/analysisdao.cpp b/src/library/dao/analysisdao.cpp
index 58121296e8..f1a43c2810 100644
--- a/src/library/dao/analysisdao.cpp
+++ b/src/library/dao/analysisdao.cpp
@@ -125,7 +125,6 @@ bool AnalysisDao::saveAnalysis(AnalysisDao::AnalysisInfo* info) {
             "VALUES (:trackId,:type,:description,:version,:data_checksum)")
                       .arg(s_analysisTableName));
 
-        QByteArray waveformBytes;
         query.bindValue(":trackId", info->trackId.toVariant());
         query.bindValue(":type", info->type);
         query.bindValue(":description", info->description);
diff --git a/src/library/dao/autodjcratesdao.cpp b/src/library/dao/autodjcratesdao.cpp
index 43557dc81c..58809c2baa 100644
--- a/src/library/dao/autodjcratesdao.cpp
+++ b/src/library/dao/autodjcratesdao.cpp
@@ -1,16 +1,19 @@
+#include "library/dao/autodjcratesdao.h"
+
+#if (QT_VERSION >= QT_VERSION_CHECK(5, 10, 0))
+#include <QRandomGenerator>
+#endif
 #include <QtDebug>
 #include <QtSql>
 
-#include "library/dao/autodjcratesdao.h"
-
-#include "mixer/playerinfo.h"
-#include "mixer/playermanager.h"
 #include "library/crate/crateschema.h"
 #include "library/dao/settingsdao.h"
 #include "library/dao/trackdao.h"
 #include "library/dao/trackschema.h"
 #include "library/queryutil.h"
 #include "library/trackcollection.h"
+#include "mixer/playerinfo.h"
+#include "mixer/playermanager.h"
 
 #define AUTODJCRATESTABLE_TRACKID "track_id"
 #define AUTODJCRATESTABLE_CRATEREFS "craterefs"
@@ -34,8 +37,21 @@
 namespace {
 // Percentage of most and least played tracks to ignore [0,50)
 const int kLeastPreferredPercent = 15;
+
+// These consts are only used for DEBUG_ASSERTs
+#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED
 const int kLeastPreferredPercentMin = 0;
 const int kLeastPreferredPercentMax = 50;
+#endif
+
+int bounded_rand(int highest) {
+#if (QT_VERSION >= QT_VERSION_CHECK(5, 10, 0))
+    return QRandomGenerator::global()->bounded(highest);
+#else
+    return qrand() % highest;
+#endif
+}
+
 } // anonymous namespace
 
 AutoDJCratesDAO::AutoDJCratesDAO(
@@ -194,41 +210,62 @@ void AutoDJCratesDAO::createAndConnectAutoDjCratesDatabase() {
 
     // Be notified when a track is modified.
     // We only care when the number of times it's been played changes.
-    connect(&m_pTrackCollection->getTrackDAO(), SIGNAL(trackDirty(TrackId)),
-            this, SLOT(slotTrackDirty(TrackId)));
+    connect(m_pTrackCollection,
+            &TrackCollection::trackDirty,
+            this,
+            &AutoDJCratesDAO::slotTrackDirty);
 
     // Be notified when the status of crates changes.
-    connect(m_pTrackCollection, SIGNAL(crateInserted(CrateId)),
-            this, SLOT(slotCrateInserted(CrateId)));
-    connect(m_pTrackCollection, SIGNAL(crateDeleted(CrateId)),
-            this, SLOT(slotCrateDeleted(CrateId)));
-    connect(m_pTrackCollection, SIGNAL(crateUpdated(CrateId)),
-            this, SLOT(slotCrateUpdated(CrateId)));
-    connect(m_pTrackCollection, SIGNAL(crateTracksChanged(CrateId,QList<TrackId>,QList<TrackId>)),
-            this, SLOT(slotCrateTracksChanged(CrateId,QList<TrackId>,QList<TrackId>)));
+    connect(m_pTrackCollection,
+            &TrackCollection::crateInserted,
+            this,
+            &AutoDJCratesDAO::slotCrateInserted);
+    connect(m_pTrackCollection,
+            &TrackCollection::crateDeleted,
+            this,
+            &AutoDJCratesDAO::slotCrateDeleted);
+    connect(m_pTrackCollection,
+            &TrackCollection::crateUpdated,
+            this,
+            &AutoDJCratesDAO::slotCrateUpdated);
+    connect(m_pTrackCollection,
+            &TrackCollection::crateTracksChanged,
+            this,
+            &AutoDJCratesDAO::slotCrateTracksChanged);
 
     // Be notified when playlists are added/removed.
     // We only care about set-log playlists.
-    connect(&m_pTrackCollection->getPlaylistDAO(), SIGNAL(added(int)),
-            this, SLOT(slotPlaylistAdded(int)));
-    connect(&m_pTrackCollection->getPlaylistDAO(), SIGNAL(deleted(int)),
-            this, SLOT(slotPlaylistDeleted(int)));
+    connect(&m_pTrackCollection->getPlaylistDAO(),
+            &PlaylistDAO::added,
+            this,
+            &AutoDJCratesDAO::slotPlaylistAdded);
+    connect(&m_pTrackCollection->getPlaylistDAO(),
+            &PlaylistDAO::deleted,
+            this,
+            &AutoDJCratesDAO::slotPlaylistDeleted);
 
     // Be notified when tracks are added/removed from playlists.
     // We only care about the auto-DJ playlist and the set-log playlists.
-    connect(&m_pTrackCollection->getPlaylistDAO(), SIGNAL(trackAdded(int,TrackId,int)),
-            this, SLOT(slotPlaylistTrackAdded(int,TrackId,int)));
-    connect(&m_pTrackCollection->getPlaylistDAO(), SIGNAL(trackRemoved(int,TrackId,int)),
-            this, SLOT(slotPlaylistTrackRemoved(int,TrackId,int)));
+    connect(&m_pTrackCollection->getPlaylistDAO(),
+            &PlaylistDAO::trackAdded,
+            this,
+            &AutoDJCratesDAO::slotPlaylistTrackAdded);
+    connect(&m_pTrackCollection->getPlaylistDAO(),
+            &PlaylistDAO::trackRemoved,
+            this,
+            &AutoDJCratesDAO::slotPlaylistTrackRemoved);
 
     // Be notified when tracks are loaded to, or unloaded from, a deck.
     // These count as auto-DJ references, i.e. prevent the track from being
     // selected randomly.
-    connect(&PlayerInfo::instance(), SIGNAL(trackLoaded(QString,TrackPointer)),
-            this, SLOT(slotPlayerInfoTrackLoaded(QString,TrackPointer)));
     connect(&PlayerInfo::instance(),
-            SIGNAL(trackUnloaded(QString,TrackPointer)),
-            this, SLOT(slotPlayerInfoTrackUnloaded(QString,TrackPointer)));
+            &PlayerInfo::trackLoaded,
+            this,
+            &AutoDJCratesDAO::slotPlayerInfoTrackLoaded);
+    connect(&PlayerInfo::instance(),
+            &PlayerInfo::trackUnloaded,
+            this,
+            &AutoDJCratesDAO::slotPlayerInfoTrackUnloaded);
 
     // Remember that the auto-DJ-crates database has been created.
     m_bAutoDjCratesDbCreated = true;
@@ -689,8 +726,8 @@ TrackId AutoDJCratesDAO::getRandomTrackIdFromAutoDj(int percentActive) {
 // Signaled by the track DAO when a track's information is updated.
 void AutoDJCratesDAO::slotTrackDirty(TrackId trackId) {
     // Update our record of the number of times played, if that changed.
-    TrackPointer pTrack = m_pTrackCollection->getTrackDAO().getTrack(trackId);
-    if (pTrack == NULL) {
+    TrackPointer pTrack = m_pTrackCollection->getTrackById(trackId);
+    if (!pTrack) {
         return;
     }
     const PlayCounter playCounter(pTrack->getPlayCounter());
@@ -1147,25 +1184,25 @@ TrackId AutoDJCratesDAO::getRandomTrackIdFromLibrary(int iPlaylistId) {
         // Least Preferred is not disabled
         iIgnoreIndex1 = (kLeastPreferredPercent * iTotalTracks) / 100;
         iIgnoreIndex2 = iTotalTracks - iIgnoreIndex1;
-        int iRandomNo = qrand() % 16 ;
+        int iRandomNo = bounded_rand(16);
         if(iRandomNo == 0 && iIgnoreIndex1 != 0) {
             // Select a track from the first [1, iIgnoredIndex1]
             beginIndex = 0;
-            offset = qrand() % iIgnoreIndex1 + 1 ;
+            offset = bounded_rand(iIgnoreIndex1) + 1;
         } else if(iRandomNo == 1 && iTotalTracks > iIgnoreIndex2){
             // Select from [iIgnoredIndex2 + 1, iTotalTracks];
             beginIndex = iIgnoreIndex2;
             // We need a number between [1, Total - iIgnoreIndex2]
-            offset = qrand() % (iTotalTracks - iIgnoreIndex2) + 1;
+            offset = bounded_rand(iTotalTracks - iIgnoreIndex2) + 1;
         } else {
             // Select from [iIgnoreIndex1 + 1, iIgnoreIndex2];
             beginIndex = iIgnoreIndex1;
             // We need a number between [1, iIgnoreIndex2 - iIgnoreIndex1]
-            offset = qrand() % (iIgnoreIndex2 - iIgnoreIndex1) + 1;
+            offset = bounded_rand(iIgnoreIndex2 - iIgnoreIndex1) + 1;
         }
         offset = beginIndex + offset;
         // In case we end up doing a qRand()%1 above
-        if( offset >= iTotalTracks) {
+        if (offset >= iTotalTracks) {
             offset= 0 ;
         }
     }
diff --git a/src/library/dao/autodjcratesdao.h b/src/library/dao/autodjcratesdao.h
index 0d4882940d..2aeb73096a 100644
--- a/src/library/dao/autodjcratesdao.h
+++ b/src/library/dao/autodjcratesdao.h
@@ -1,5 +1,4 @@
-#ifndef AUTODJCRATESDAO_H
-#define AUTODJCRATESDAO_H
+#pragma once
 
 #include <QObject>
 #include <QSqlDatabase>
@@ -120,5 +119,3 @@ class AutoDJCratesDAO : public QObject {
     // The ID of every set-log playlist.
     QList<int> m_lstSetLogPlaylistIds;
 };
-
-#endif // AUTODJCRATESDAO_H
diff --git a/src/library/dao/cue.cpp b/src/library/dao/cue.cpp
deleted file mode 100644
index 8efc6f9a27..0000000000
--- a/src/library/dao/cue.cpp
+++ /dev/null
@@ -1,162 +0,0 @@
-// cue.cpp
-// Created 10/26/2009 by RJ Ryan (rryan@mit.edu)
-
-#include <QMutexLocker>
-#include <QtDebug>
-
-#include "library/dao/cue.h"
-#include "util/assert.h"
-
-namespace {
-    const QColor kDefaultColor = QColor("#FF0000");
-    const QString kDefaultLabel = ""; // empty string, not null
-}
-
-Cue::~Cue() {
-    //qDebug() << "~Cue()" << m_iId;
-}
-
-Cue::Cue(TrackId trackId)
-        : m_bDirty(false),
-          m_iId(-1),
-          m_trackId(trackId),
-          m_type(INVALID),
-          m_samplePosition(-1.0),
-          m_length(0.0),
-          m_iHotCue(-1),
-          m_label(kDefaultLabel),
-          m_color(kDefaultColor) {
-    DEBUG_ASSERT(!m_label.isNull());
-}
-
-
-Cue::Cue(int id, TrackId trackId, Cue::CueType type, double position, double length,
-         int hotCue, QString label, QColor color)
-        : m_bDirty(false),
-          m_iId(id),
-          m_trackId(trackId),
-          m_type(type),
-          m_samplePosition(position),
-          m_length(length),
-          m_iHotCue(hotCue),
-          m_label(label),
-          m_color(color) {
-    DEBUG_ASSERT(!m_label.isNull());
-}
-
-int Cue::getId() const {
-    QMutexLocker lock(&m_mutex);
-    return m_iId;
-}
-
-void Cue::setId(int cueId) {
-    QMutexLocker lock(&m_mutex);
-    m_iId = cueId;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-TrackId Cue::getTrackId() const {
-    QMutexLocker lock(&m_mutex);
-    return m_trackId;
-}
-
-void Cue::setTrackId(TrackId trackId) {
-    QMutexLocker lock(&m_mutex);
-    m_trackId = trackId;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-Cue::CueType Cue::getType() const {
-    QMutexLocker lock(&m_mutex);
-    return m_type;
-}
-
-void Cue::setType(Cue::CueType type) {
-    QMutexLocker lock(&m_mutex);
-    m_type = type;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-double Cue::getPosition() const {
-    QMutexLocker lock(&m_mutex);
-    return m_samplePosition;
-}
-
-void Cue::setPosition(double samplePosition) {
-    QMutexLocker lock(&m_mutex);
-    m_samplePosition = samplePosition;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-double Cue::getLength() const {
-    QMutexLocker lock(&m_mutex);
-    return m_length;
-}
-
-void Cue::setLength(double length) {
-    QMutexLocker lock(&m_mutex);
-    m_length = length;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-int Cue::getHotCue() const {
-    QMutexLocker lock(&m_mutex);
-    return m_iHotCue;
-}
-
-void Cue::setHotCue(int hotCue) {
-    QMutexLocker lock(&m_mutex);
-    // TODO(XXX) enforce uniqueness?
-    m_iHotCue = hotCue;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-QString Cue::getLabel() const {
-    QMutexLocker lock(&m_mutex);
-    return m_label;
-}
-
-void Cue::setLabel(const QString label) {
-    //qDebug() << "setLabel()" << m_label << "-" << label;
-    DEBUG_ASSERT(!label.isNull());
-    QMutexLocker lock(&m_mutex);
-    m_label = label;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-QColor Cue::getColor() const {
-    QMutexLocker lock(&m_mutex);
-    return m_color;
-}
-
-void Cue::setColor(const QColor color) {
-    QMutexLocker lock(&m_mutex);
-    m_color = color;
-    m_bDirty = true;
-    lock.unlock();
-    emit(updated());
-}
-
-bool Cue::isDirty() const {
-    QMutexLocker lock(&m_mutex);
-    return m_bDirty;
-}
-
-void Cue::setDirty(bool dirty) {
-    QMutexLocker lock(&m_mutex);
-    m_bDirty = dirty;
-}
diff --git a/src/library/dao/cue.h b/src/library/dao/cue.h
deleted file mode 100644
index 796dba0493..0000000000
--- a/src/library/dao/cue.h
+++ /dev/null
@@ -1,92 +0,0 @@
-#ifndef MIXXX_CUE_H
-#define MIXXX_CUE_H
-
-#include <QObject>
-#include <QMutex>
-#include <QColor>
-
-#include "track/trackid.h"
-#include "util/memory.h"
-
-class CueDAO;
-class Track;
-
-class Cue : public QObject {
-  Q_OBJECT
-  public:
-    enum CueType {
-        INVALID = 0,
-        CUE     = 1, // hot cue
-        LOAD    = 2, // the cue
-        BEAT    = 3,
-        LOOP    = 4,
-        JUMP    = 5,
-    };
-
-    ~Cue() override;
-
-    bool isDirty() const;
-    int getId() const;
-    TrackId getTrackId() const;
-
-    CueType getType() const;
-    void setType(CueType type);
-
-    double getPosition() const;
-    void setPosition(double samplePosition);
-
-    double getLength() const;
-    void setLength(double length);
-
-    int getHotCue() const;
-    void setHotCue(int hotCue);
-
-    QString getLabel() const;
-    void setLabel(QString label);
-
-    QColor getColor() const;
-    void setColor(QColor color);
-
-  signals:
-    void updated();
-
-  private:
-    explicit Cue(TrackId trackId);
-    Cue(int id, TrackId trackId, CueType type, double position, double length,
-        int hotCue, QString label, QColor color);
-    void setDirty(bool dirty);
-    void setId(int id);
-    void setTrackId(TrackId trackId);
-
-    mutable QMutex m_mutex;
-
-    bool m_bDirty;
-    int m_iId;
-    TrackId m_trackId;
-    CueType m_type;
-    double m_samplePosition;
-    double m_length;
-    int m_iHotCue;
-    QString m_label;
-    QColor m_color;
-
-    friend class Track;
-    friend class CueDAO;
-};
-
-class CuePointer: public std::shared_ptr<Cue> {
-  public:
-    CuePointer() {}
-    explicit CuePointer(Cue* pCue)
-          : std::shared_ptr<Cue>(pCue, deleteLater) {
-    }
-
-  private:
-    static void deleteLater(Cue* pCue) {
-        if (pCue != nullptr) {
-            pCue->deleteLater();
-        }
-    }
-};
-
-#endif // MIXXX_CUE_H
diff --git a/src/library/dao/cuedao.cpp b/src/library/dao/cuedao.cpp
index 66a4e440c9..2cba410bcd 100644
--- a/src/library/dao/cuedao.cpp
+++ b/src/library/dao/cuedao.cpp
@@ -1,107 +1,116 @@
 // cuedao.cpp
 // Created 10/26/2009 by RJ Ryan (rryan@mit.edu)
 
+#include "library/dao/cuedao.h"
+
+#include <QVariant>
 #include <QtDebug>
 #include <QtSql>
-#include <QVariant>
 
-#include "library/dao/cuedao.h"
-#include "library/dao/cue.h"
-#include "track/track.h"
 #include "library/queryutil.h"
+#include "track/cue.h"
+#include "track/track.h"
 #include "util/assert.h"
+#include "util/color/rgbcolor.h"
+#include "util/db/fwdsqlquery.h"
+#include "util/logger.h"
 #include "util/performancetimer.h"
 
-int CueDAO::cueCount() {
-    qDebug() << "CueDAO::cueCount" << QThread::currentThread() << m_database.connectionName();
-    QSqlQuery query(m_database);
-    query.prepare("SELECT COUNT(*) FROM " CUE_TABLE);
-    if (query.exec()) {
-        if (query.next()) {
-            return query.value(0).toInt();
-        }
+namespace {
+
+const mixxx::Logger kLogger = mixxx::Logger("CueDAO");
+
+// The label column is not nullable!
+const QVariant kEmptyLabel = QVariant(QStringLiteral(""));
+
+inline const QVariant labelToQVariant(const QString& label) {
+    if (label.isNull()) {
+        return kEmptyLabel; // null -> empty
     } else {
-        LOG_FAILED_QUERY(query);
+        return label;
     }
-    //query.finish();
-    return 0;
 }
 
-int CueDAO::numCuesForTrack(TrackId trackId) {
-    qDebug() << "CueDAO::numCuesForTrack" << QThread::currentThread() << m_database.connectionName();
-    QSqlQuery query(m_database);
-    query.prepare("SELECT COUNT(*) FROM " CUE_TABLE " WHERE track_id = :id");
-    query.bindValue(":id", trackId.toVariant());
-    if (query.exec()) {
-        if (query.next()) {
-            return query.value(0).toInt();
-        }
+// Empty labels are read as null strings
+inline QString labelFromQVariant(const QVariant& value) {
+    const auto label = value.toString();
+    if (label.isEmpty()) {
+        return QString(); // empty -> null
     } else {
-        LOG_FAILED_QUERY(query);
+        return label;
     }
-    return 0;
 }
 
-CuePointer CueDAO::cueFromRow(const QSqlQuery& query) const {
-    QSqlRecord record = query.record();
-    int id = record.value(record.indexOf("id")).toInt();
-    TrackId trackId(record.value(record.indexOf("track_id")));
-    int type = record.value(record.indexOf("type")).toInt();
-    int position = record.value(record.indexOf("position")).toInt();
-    int length = record.value(record.indexOf("length")).toInt();
-    int hotcue = record.value(record.indexOf("hotcue")).toInt();
-    QString label = record.value(record.indexOf("label")).toString();
-    QColor color = QColor::fromRgba(record.value(record.indexOf("color")).toInt());
-    CuePointer pCue(new Cue(id, trackId, (Cue::CueType)type,
-                       position, length, hotcue, label, color));
-    m_cues[id] = pCue;
+CuePointer cueFromRow(const QSqlRecord& row) {
+    int id = row.value(row.indexOf("id")).toInt();
+    TrackId trackId(row.value(row.indexOf("track_id")));
+    int type = row.value(row.indexOf("type")).toInt();
+    int position = row.value(row.indexOf("position")).toInt();
+    int length = row.value(row.indexOf("length")).toInt();
+    int hotcue = row.value(row.indexOf("hotcue")).toInt();
+    QString label = labelFromQVariant(row.value(row.indexOf("label")));
+    mixxx::RgbColor::optional_t color = mixxx::RgbColor::fromQVariant(row.value(row.indexOf("color")));
+    VERIFY_OR_DEBUG_ASSERT(color) {
+        return CuePointer();
+    }
+    CuePointer pCue(new Cue(id,
+            trackId,
+            static_cast<mixxx::CueType>(type),
+            position,
+            length,
+            hotcue,
+            label,
+            *color));
     return pCue;
 }
 
+} // namespace
+
 QList<CuePointer> CueDAO::getCuesForTrack(TrackId trackId) const {
     //qDebug() << "CueDAO::getCuesForTrack" << QThread::currentThread() << m_database.connectionName();
     QList<CuePointer> cues;
-    // A hash from hotcue index to cue id and cue*, used to detect if more
-    // than one cue has been assigned to a single hotcue id.
-    QMap<int, QPair<int, CuePointer> > dupe_hotcues;
 
-    QSqlQuery query(m_database);
-    query.prepare("SELECT * FROM " CUE_TABLE " WHERE track_id = :id");
+    FwdSqlQuery query(
+            m_database,
+            QStringLiteral("SELECT * FROM " CUE_TABLE " WHERE track_id=:id"));
+    DEBUG_ASSERT(
+            query.isPrepared() &&
+            !query.hasError());
     query.bindValue(":id", trackId.toVariant());
-    if (query.exec()) {
-        const int idColumn = query.record().indexOf("id");
-        const int hotcueIdColumn = query.record().indexOf("hotcue");
-        while (query.next()) {
-            CuePointer pCue;
-            int cueId = query.value(idColumn).toInt();
-            if (m_cues.contains(cueId)) {
-                pCue = m_cues[cueId];
-            }
-            if (!pCue) {
-                pCue = cueFromRow(query);
-            }
-            int hotcueId = query.value(hotcueIdColumn).toInt();
-            if (hotcueId != -1) {
-                if (dupe_hotcues.contains(hotcueId)) {
-                    m_cues.remove(dupe_hotcues[hotcueId].first);
-                    cues.removeOne(dupe_hotcues[hotcueId].second);
-                }
-                dupe_hotcues[hotcueId] = qMakePair(cueId, pCue);
-            }
-            if (pCue) {
-                cues.push_back(pCue);
+    VERIFY_OR_DEBUG_ASSERT(query.execPrepared()) {
+        kLogger.warning()
+                << "Failed to load cues of track"
+                << trackId;
+        return cues;
+    }
+    QMap<int, CuePointer> hotCuesByNumber;
+    while (query.next()) {
+        CuePointer pCue = cueFromRow(query.record());
+        VERIFY_OR_DEBUG_ASSERT(pCue) {
+            continue;
+        }
+        int hotCueNumber = pCue->getHotCue();
+        if (hotCueNumber != Cue::kNoHotCue) {
+            const auto pDuplicateCue = hotCuesByNumber.take(hotCueNumber);
+            if (pDuplicateCue) {
+                kLogger.warning()
+                        << "Dropping hot cue"
+                        << pDuplicateCue->getId()
+                        << "with duplicate number"
+                        << hotCueNumber;
+                cues.removeOne(pDuplicateCue);
             }
+            hotCuesByNumber.insert(hotCueNumber, pCue);
         }
-    } else {
-        LOG_FAILED_QUERY(query);
+        cues.push_back(pCue);
     }
     return cues;
 }
 
-bool CueDAO::deleteCuesForTrack(TrackId trackId) {
+bool CueDAO::deleteCuesForTrack(TrackId trackId) const {
     qDebug() << "CueDAO::deleteCuesForTrack" << QThread::currentThread() << m_database.connectionName();
     QSqlQuery query(m_database);
-    query.prepare("DELETE FROM " CUE_TABLE " WHERE track_id = :track_id");
+    query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE track_id=:track_id"));
     query.bindValue(":track_id", trackId.toVariant());
     if (query.exec()) {
         return true;
@@ -111,7 +120,7 @@ bool CueDAO::deleteCuesForTrack(TrackId trackId) {
     return false;
 }
 
-bool CueDAO::deleteCuesForTracks(const QList<TrackId>& trackIds) {
+bool CueDAO::deleteCuesForTracks(const QList<TrackId>& trackIds) const {
     qDebug() << "CueDAO::deleteCuesForTracks" << QThread::currentThread() << m_database.connectionName();
 
     QStringList idList;
@@ -120,7 +129,7 @@ bool CueDAO::deleteCuesForTracks(const QList<TrackId>& trackIds) {
     }
 
     QSqlQuery query(m_database);
-    query.prepare(QString("DELETE FROM " CUE_TABLE " WHERE track_id in (%1)")
+    query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE track_id in (%1)")
                   .arg(idList.join(",")));
     if (query.exec()) {
         return true;
@@ -130,7 +139,7 @@ bool CueDAO::deleteCuesForTracks(const QList<TrackId>& trackIds) {
     return false;
 }
 
-bool CueDAO::saveCue(Cue* cue) {
+bool CueDAO::saveCue(Cue* cue) const {
     //qDebug() << "CueDAO::saveCue" << QThread::currentThread() << m_database.connectionName();
     VERIFY_OR_DEBUG_ASSERT(cue) {
         return false;
@@ -138,14 +147,14 @@ bool CueDAO::saveCue(Cue* cue) {
     if (cue->getId() == -1) {
         // New cue
         QSqlQuery query(m_database);
-        query.prepare("INSERT INTO " CUE_TABLE " (track_id, type, position, length, hotcue, label, color) VALUES (:track_id, :type, :position, :length, :hotcue, :label, :color)");
+        query.prepare(QStringLiteral("INSERT INTO " CUE_TABLE " (track_id, type, position, length, hotcue, label, color) VALUES (:track_id, :type, :position, :length, :hotcue, :label, :color)"));
         query.bindValue(":track_id", cue->getTrackId().toVariant());
-        query.bindValue(":type", cue->getType());
+        query.bindValue(":type", static_cast<int>(cue->getType()));
         query.bindValue(":position", cue->getPosition());
         query.bindValue(":length", cue->getLength());
         query.bindValue(":hotcue", cue->getHotCue());
-        query.bindValue(":label", cue->getLabel());
-        query.bindValue(":color", cue->getColor().rgba());
+        query.bindValue(":label", labelToQVariant(cue->getLabel()));
+        query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor()));
 
         if (query.exec()) {
             int id = query.lastInsertId().toInt();
@@ -157,23 +166,23 @@ bool CueDAO::saveCue(Cue* cue) {
     } else {
         // Update cue
         QSqlQuery query(m_database);
-        query.prepare("UPDATE " CUE_TABLE " SET "
-                        "track_id = :track_id,"
-                        "type = :type,"
-                        "position = :position,"
-                        "length = :length,"
-                        "hotcue = :hotcue,"
-                        "label = :label,"
-                        "color = :color"
-                        " WHERE id = :id");
+        query.prepare(QStringLiteral("UPDATE " CUE_TABLE " SET "
+                        "track_id=:track_id,"
+                        "type=:type,"
+                        "position=:position,"
+                        "length=:length,"
+                        "hotcue=:hotcue,"
+                        "label=:label,"
+                        "color=:color"
+                        " WHERE id=:id"));
         query.bindValue(":id", cue->getId());
         query.bindValue(":track_id", cue->getTrackId().toVariant());
-        query.bindValue(":type", cue->getType());
+        query.bindValue(":type", static_cast<int>(cue->getType()));
         query.bindValue(":position", cue->getPosition());
         query.bindValue(":length", cue->getLength());
         query.bindValue(":hotcue", cue->getHotCue());
-        query.bindValue(":label", cue->getLabel());
-        query.bindValue(":color", cue->getColor().rgba());
+        query.bindValue(":label", labelToQVariant(cue->getLabel()));
+        query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor()));
 
         if (query.exec()) {
             cue->setDirty(false);
@@ -185,11 +194,11 @@ bool CueDAO::saveCue(Cue* cue) {
     return false;
 }
 
-bool CueDAO::deleteCue(Cue* cue) {
+bool CueDAO::deleteCue(Cue* cue) const {
     //qDebug() << "CueDAO::deleteCue" << QThread::currentThread() << m_database.connectionName();
     if (cue->getId() != -1) {
         QSqlQuery query(m_database);
-        query.prepare("DELETE FROM " CUE_TABLE " WHERE id = :id");
+        query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE id=:id"));
         query.bindValue(":id", cue->getId());
         if (query.exec()) {
             return true;
@@ -202,56 +211,48 @@ bool CueDAO::deleteCue(Cue* cue) {
     return false;
 }
 
-void CueDAO::saveTrackCues(TrackId trackId, const QList<CuePointer>& cueList) {
-    //qDebug() << "CueDAO::saveTrackCues" << QThread::currentThread() << m_database.connectionName();
-    // TODO(XXX) transaction, but people who are already in a transaction call
-    // this.
-    PerformanceTimer time;
-
-    // qDebug() << "CueDAO::saveTrackCues old size:" << oldCueList.size()
-    //          << "new size:" << cueList.size();
-
-    QString list = "";
-
-    time.start();
-    // For each id still in the TIO, save or delete it.
-    QListIterator<CuePointer> cueIt(cueList);
-    while (cueIt.hasNext()) {
-        CuePointer pCue(cueIt.next());
-        int cueId = pCue->getId();
-        bool newCue = cueId == -1;
-        if (newCue) {
-            // New cue
+void CueDAO::saveTrackCues(
+        TrackId trackId,
+        const QList<CuePointer>& cueList) const {
+    QStringList cueIds;
+    cueIds.reserve(cueList.size());
+    for (const auto& pCue : cueList) {
+        VERIFY_OR_DEBUG_ASSERT(pCue->getTrackId() == trackId) {
             pCue->setTrackId(trackId);
-        } else {
-            //idList.append(QString("%1").arg(cueId));
-            list.append(QString("%1,").arg(cueId));
         }
+        // New cues (without an id) must always be marked as dirty
+        DEBUG_ASSERT(pCue->getId() >= 0 || pCue->isDirty());
         // Update or save cue
         if (pCue->isDirty()) {
             saveCue(pCue.get());
-
-            // Since this cue didn't have an id until now, add it to the list of
-            // cues not to delete.
-            if (newCue)
-                list.append(QString("%1,").arg(pCue->getId()));
         }
+        // After saving each cue must have a valid id
+        VERIFY_OR_DEBUG_ASSERT(pCue->getId() >= 0) {
+            continue;
+        }
+        cueIds.append(QString::number(pCue->getId()));
     }
-    //qDebug() << "Saving cues took " << time.formatMillisWithUnit();
-    time.start();
-
-    // Strip the last ,
-    if (list.count() > 0)
-        list.truncate(list.count()-1);
 
-    // Delete cues that are no longer on the track.
-    QSqlQuery query(m_database);
-    query.prepare(QString("DELETE FROM cues where track_id=:track_id and not id in (%1)")
-                  .arg(list));
+    // Delete orphaned cues
+    FwdSqlQuery query(
+            m_database,
+            QStringLiteral("DELETE FROM " CUE_TABLE " WHERE track_id=:track_id AND id NOT IN (%1)")
+                    .arg(cueIds.join(QChar(','))));
+    DEBUG_ASSERT(
+            query.isPrepared() &&
+            !query.hasError());
     query.bindValue(":track_id", trackId.toVariant());
-
-    if (!query.exec()) {
-        LOG_FAILED_QUERY(query) << "Delete cues failed.";
+    VERIFY_OR_DEBUG_ASSERT(query.execPrepared()) {
+        kLogger.warning()
+                << "Failed to delete orphaned cues of track"
+                << trackId;
+        return;
+    }
+    if (query.numRowsAffected() > 0) {
+        kLogger.debug()
+                << "Deleted"
+                << query.numRowsAffected()
+                << "orphaned cue(s) of track"
+                << trackId;
     }
-    //qDebug() << "Deleting cues took " << time.formatMillisWithUnit();
 }
diff --git a/src/library/dao/cuedao.h b/src/library/dao/cuedao.h
index 3f0a9cb163..d912c2daa0 100644
--- a/src/library/dao/cuedao.h
+++ b/src/library/dao/cuedao.h
@@ -1,10 +1,5 @@
-// cuedao.h
-// Created 10/26/2009 by RJ Ryan (rryan@mit.edu)
+#pragma once
 
-#ifndef CUEDAO_H
-#define CUEDAO_H
-
-#include <QMap>
 #include <QSqlDatabase>
 
 #include "track/track.h"
@@ -16,27 +11,21 @@ class Cue;