Skip to content

Commit

Permalink
Merge pull request #3324 from uklotzde/cue-dbid
Browse files Browse the repository at this point in the history 
Cue/CueDAO: Improve type-safety and reduce code duplication
  • Loading branch information
@Holzhaus
Holzhaus authored Nov 15, 2020
2 parents 7bc9c1b + c9170a8 commit e3fab0c
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 72 deletions.
8 changes: 4 additions & 4 deletions src/dialog/dlgreplacecuecolor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void setButtonColor(QPushButton* button, const QColor& color) {
}

typedef struct {
int id;
DbId id;
TrackId trackId;
mixxx::RgbColor color;
} CueDatabaseRow;
Expand Down Expand Up @@ -339,7 +339,7 @@ void DlgReplaceCueColor::slotApply() {
VERIFY_OR_DEBUG_ASSERT(color) {
continue;
}
CueDatabaseRow row = {selectQuery.value(idColumn).toInt(),
CueDatabaseRow row = {DbId(selectQuery.value(idColumn)),
TrackId(selectQuery.value(trackIdColumn).toInt()),
*color};
rows << row;
Expand Down Expand Up @@ -383,14 +383,14 @@ void DlgReplaceCueColor::slotApply() {

bool canceled = false;

QMultiMap<TrackPointer, int> cues;
QMultiMap<TrackPointer, DbId> cues;
for (const auto& row : qAsConst(rows)) {
QCoreApplication::processEvents();
if (progress.wasCanceled()) {
canceled = true;
break;
}
query.bindValue(":id", row.id);
query.bindValue(":id", row.id.toVariant());
query.bindValue(":track_id", row.trackId.value());
query.bindValue(":current_color", mixxx::RgbColor::toQVariant(row.color));
if (!query.exec()) {
Expand Down
98 changes: 47 additions & 51 deletions src/library/dao/cuedao.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ inline QString labelFromQVariant(const QVariant& value) {
}

CuePointer cueFromRow(const QSqlRecord& row) {
int id = row.value(row.indexOf("id")).toInt();
const auto id = DbId(row.value(row.indexOf("id")));
TrackId trackId(row.value(row.indexOf("track_id")));
int type = row.value(row.indexOf("type")).toInt();
int position = row.value(row.indexOf("position")).toInt();
Expand Down Expand Up @@ -142,28 +142,11 @@ bool CueDAO::saveCue(Cue* cue) const {
VERIFY_OR_DEBUG_ASSERT(cue) {
return false;
}
if (cue->getId() == -1) {
// New cue
QSqlQuery query(m_database);
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", static_cast<int>(cue->getType()));
query.bindValue(":position", cue->getPosition());
query.bindValue(":length", cue->getLength());
query.bindValue(":hotcue", cue->getHotCue());
query.bindValue(":label", labelToQVariant(cue->getLabel()));
query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor()));

if (query.exec()) {
int id = query.lastInsertId().toInt();
cue->setId(id);
cue->setDirty(false);
return true;
}
qDebug() << query.executedQuery() << query.lastError();
} else {
// Prepare query
QSqlQuery query(m_database);
if (cue->getId().isValid()) {
// Update cue
QSqlQuery query(m_database);
query.prepare(QStringLiteral("UPDATE " CUE_TABLE " SET "
"track_id=:track_id,"
"type=:type,"
Expand All @@ -173,40 +156,53 @@ bool CueDAO::saveCue(Cue* cue) const {
"label=:label,"
"color=:color"
" WHERE id=:id"));
query.bindValue(":id", cue->getId());
query.bindValue(":track_id", cue->getTrackId().toVariant());
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", labelToQVariant(cue->getLabel()));
query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor()));
query.bindValue(":id", cue->getId().toVariant());
} else {
// New cue
query.prepare(
QStringLiteral("INSERT INTO " CUE_TABLE
" (track_id, type, position, length, hotcue, "
"label, color) VALUES (:track_id, :type, "
":position, :length, :hotcue, :label, :color)"));
}

if (query.exec()) {
cue->setDirty(false);
return true;
} else {
LOG_FAILED_QUERY(query);
}
// Bind values and execute query
query.bindValue(":track_id", cue->getTrackId().toVariant());
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", labelToQVariant(cue->getLabel()));
query.bindValue(":color", mixxx::RgbColor::toQVariant(cue->getColor()));
if (!query.exec()) {
LOG_FAILED_QUERY(query);
return false;
}
return false;

if (!cue->getId().isValid()) {
// New cue
const auto newId = DbId(query.lastInsertId());
DEBUG_ASSERT(newId.isValid());
cue->setId(newId);
}
DEBUG_ASSERT(cue->getId().isValid());
cue->setDirty(false);
return true;
}

bool CueDAO::deleteCue(Cue* cue) const {
//qDebug() << "CueDAO::deleteCue" << QThread::currentThread() << m_database.connectionName();
if (cue->getId() != -1) {
QSqlQuery query(m_database);
query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE id=:id"));
query.bindValue(":id", cue->getId());
if (query.exec()) {
return true;
} else {
LOG_FAILED_QUERY(query);
}
} else {
return true;
if (!cue->getId().isValid()) {
return false;
}
return false;
QSqlQuery query(m_database);
query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE id=:id"));
query.bindValue(":id", cue->getId().toVariant());
if (!query.exec()) {
LOG_FAILED_QUERY(query);
return false;
}
return true;
}

void CueDAO::saveTrackCues(
Expand All @@ -219,16 +215,16 @@ void CueDAO::saveTrackCues(
pCue->setTrackId(trackId);
}
// New cues (without an id) must always be marked as dirty
DEBUG_ASSERT(pCue->getId() >= 0 || pCue->isDirty());
DEBUG_ASSERT(pCue->getId().isValid() || pCue->isDirty());
// Update or save cue
if (pCue->isDirty()) {
saveCue(pCue.get());
}
// After saving each cue must have a valid id
VERIFY_OR_DEBUG_ASSERT(pCue->getId() >= 0) {
VERIFY_OR_DEBUG_ASSERT(pCue->getId().isValid()) {
continue;
}
cueIds.append(QString::number(pCue->getId()));
cueIds.append(pCue->getId().toString());
}

// Delete orphaned cues
Expand Down
19 changes: 10 additions & 9 deletions src/track/cue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,32 @@ void CuePointer::deleteLater(Cue* pCue) {

Cue::Cue()
: m_bDirty(false),
m_iId(-1),
m_type(mixxx::CueType::Invalid),
m_sampleStartPosition(Cue::kNoPosition),
m_sampleEndPosition(Cue::kNoPosition),
m_iHotCue(Cue::kNoHotCue),
m_color(mixxx::PredefinedColorPalettes::kDefaultCueColor) {
DEBUG_ASSERT(!m_dbId.isValid());
}

Cue::Cue(
int id,
DbId id,
TrackId trackId,
mixxx::CueType type,
double position,
double length,
int hotCue,
QString label,
mixxx::RgbColor color)
: m_bDirty(false),
m_iId(id),
: m_bDirty(false), // clear flag after loading from database
m_dbId(id),
m_trackId(trackId),
m_type(type),
m_sampleStartPosition(position),
m_iHotCue(hotCue),
m_label(label),
m_color(color) {
DEBUG_ASSERT(m_dbId.isValid());
if (length != 0) {
if (position != Cue::kNoPosition) {
m_sampleEndPosition = position + length;
Expand All @@ -90,7 +91,6 @@ Cue::Cue(
mixxx::audio::SampleRate sampleRate,
bool setDirty)
: m_bDirty(setDirty),
m_iId(-1),
m_type(cueInfo.getType()),
m_sampleStartPosition(
positionMillisToSamples(
Expand All @@ -103,6 +103,7 @@ Cue::Cue(
m_iHotCue(cueInfo.getHotCueNumber() ? *cueInfo.getHotCueNumber() : kNoHotCue),
m_label(cueInfo.getLabel()),
m_color(cueInfo.getColor().value_or(mixxx::PredefinedColorPalettes::kDefaultCueColor)) {
DEBUG_ASSERT(!m_dbId.isValid());
}

mixxx::CueInfo Cue::getCueInfo(
Expand All @@ -117,14 +118,14 @@ mixxx::CueInfo Cue::getCueInfo(
m_color);
}

int Cue::getId() const {
DbId Cue::getId() const {
QMutexLocker lock(&m_mutex);
return m_iId;
return m_dbId;
}

void Cue::setId(int cueId) {
void Cue::setId(DbId cueId) {
QMutexLocker lock(&m_mutex);
m_iId = cueId;
m_dbId = cueId;
// Neither mark as dirty nor do emit the updated() signal.
// This function is only called after adding the Cue object
// to the database. The id is not visible for anyone else.
Expand Down
13 changes: 7 additions & 6 deletions src/track/cue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
#include <QColor>
#include <QMutex>
#include <QObject>
#include <memory>

#include "audio/types.h"
#include "track/cueinfo.h"
#include "track/trackid.h"
#include "util/color/rgbcolor.h"
#include "util/memory.h"
#include "util/db/dbid.h"

class CuePosition;
class CueDAO;
class Track;

Expand All @@ -26,8 +26,9 @@ class Cue : public QObject {
const mixxx::CueInfo& cueInfo,
mixxx::audio::SampleRate sampleRate,
bool setDirty);
/// Load entity from database.
Cue(
int id,
DbId id,
TrackId trackId,
mixxx::CueType type,
double position,
Expand All @@ -38,7 +39,7 @@ class Cue : public QObject {
~Cue() override = default;

bool isDirty() const;
int getId() const;
DbId getId() const;
TrackId getTrackId() const;

mixxx::CueType getType() const;
Expand Down Expand Up @@ -75,13 +76,13 @@ class Cue : public QObject {
private:
void setDirty(bool dirty);

void setId(int id);
void setId(DbId dbId);
void setTrackId(TrackId trackId);

mutable QMutex m_mutex;

bool m_bDirty;
int m_iId;
DbId m_dbId;
TrackId m_trackId;
mixxx::CueType m_type;
double m_sampleStartPosition;
Expand Down
2 changes: 1 addition & 1 deletion src/track/track.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ CuePointer Track::findCueByType(mixxx::CueType type) const {
return CuePointer();
}

CuePointer Track::findCueById(int id) const {
CuePointer Track::findCueById(DbId id) const {
QMutexLocker lock(&m_qMutex);
for (const CuePointer& pCue : m_cuePoints) {
if (pCue->getId() == id) {
Expand Down
2 changes: 1 addition & 1 deletion src/track/track.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ class Track : public QObject {
// Calls for managing the track's cue points
CuePointer createAndAddCue();
CuePointer findCueByType(mixxx::CueType type) const; // NOTE: Cannot be used for hotcues.
CuePointer findCueById(int id) const;
CuePointer findCueById(DbId id) const;
void removeCue(const CuePointer& pCue);
void removeCuesOfType(mixxx::CueType);
QList<CuePointer> getCuePoints() const {
Expand Down

0 comments on commit e3fab0c

Please sign in to comment.