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.

Sign up for GitHub

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

Jump to bottom

Cue/CueDAO: Improve type-safety and reduce code duplication #3324

Merged
merged 1 commit into from
Nov 15, 2020
Merged

Cue/CueDAO: Improve type-safety and reduce code duplication #3324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/dialog/dlgreplacecuecolor.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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