Skip to content

Commit

Permalink
Merge pull request #2477 from uklotzde/lp1861431_crate_names
Browse files Browse the repository at this point in the history 
lp1861431: Fix debug assertion for invalid crate names
  • Loading branch information
@daschuer
daschuer authored Feb 2, 2020
2 parents f3b926c + 97127ed commit 3047062
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 49 deletions.
27 changes: 15 additions & 12 deletions src/library/crate/cratefeature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,34 +371,36 @@ void CrateFeature::slotRenameCrate() {
if (readLastRightClickedCrate(&crate)) {
const QString oldName = crate.getName();
crate.resetName();
while (!crate.hasName()) {
for (;;) {
bool ok = false;
crate.parseName(
auto newName =
QInputDialog::getText(
nullptr,
tr("Rename Crate"),
tr("Enter new name for crate:"),
QLineEdit::Normal,
oldName,
&ok));
if (!ok || (crate.getName() == oldName)) {
&ok).trimmed();
if (!ok || newName.isEmpty()) {
return;
}
if (!crate.hasName()) {
if (newName.isEmpty()) {
QMessageBox::warning(
nullptr,
tr("Renaming Crate Failed"),
tr("A crate cannot have a blank name."));
continue;
}
if (m_pTrackCollection->crates().readCrateByName(crate.getName())) {
if (m_pTrackCollection->crates().readCrateByName(newName)) {
QMessageBox::warning(
nullptr,
tr("Renaming Crate Failed"),
tr("A crate by that name already exists."));
crate.resetName();
continue;
}
crate.setName(std::move(newName));
DEBUG_ASSERT(crate.hasName());
break;
}

if (!m_pTrackCollection->updateCrate(crate)) {
Expand Down Expand Up @@ -594,15 +596,16 @@ void CrateFeature::slotCreateImportCrate() {
// Get a valid name
QString baseName = fileName.baseName();
for (int i = 0;; ++i) {
QString name = baseName;
auto name = baseName;
if (i > 0) {
name += QString(" %1").arg(i);
}

if (crate.parseName(name)) {
DEBUG_ASSERT(crate.hasName());
if (!m_pTrackCollection->crates().readCrateByName(crate.getName())) {
name = name.trimmed();
if (!name.isEmpty()) {
if (!m_pTrackCollection->crates().readCrateByName(name)) {
// unused crate name found
crate.setName(std::move(name));
DEBUG_ASSERT(crate.hasName());
break; // terminate loop
}
}
Expand Down
29 changes: 16 additions & 13 deletions src/library/crate/cratefeaturehelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,35 +35,36 @@ CrateId CrateFeatureHelper::createEmptyCrate() {
const QString proposedCrateName =
proposeNameForNewCrate(tr("New Crate"));
Crate newCrate;
while (!newCrate.hasName()) {
for (;;) {
bool ok = false;
newCrate.parseName(
auto newName =
QInputDialog::getText(
nullptr,
tr("Create New Crate"),
tr("Enter name for new crate:"),
QLineEdit::Normal,
proposedCrateName,
&ok));
&ok).trimmed();
if (!ok) {
return CrateId();
}
if (!newCrate.hasName()) {
if (newName.isEmpty()) {
QMessageBox::warning(
nullptr,
tr("Creating Crate Failed"),
tr("A crate cannot have a blank name."));
continue;
}

if (m_pTrackCollection->crates().readCrateByName(newCrate.getName())) {
if (m_pTrackCollection->crates().readCrateByName(newName)) {
QMessageBox::warning(
nullptr,
tr("Creating Crate Failed"),
tr("A crate by that name already exists."));
newCrate.resetName();
continue;
}
newCrate.setName(std::move(newName));
DEBUG_ASSERT(newCrate.hasName());
break;
}

CrateId newCrateId;
Expand All @@ -89,34 +90,36 @@ CrateId CrateFeatureHelper::duplicateCrate(const Crate& oldCrate) {
QString("%1 %2").arg(
oldCrate.getName(), tr("copy" , "[noun]")));
Crate newCrate;
while (!newCrate.hasName()) {
for (;;) {
bool ok = false;
newCrate.parseName(
auto newName =
QInputDialog::getText(
nullptr,
tr("Duplicate Crate"),
tr("Enter name for new crate:"),
QLineEdit::Normal,
proposedCrateName,
&ok));
&ok).trimmed();
if (!ok) {
return CrateId();
}
if (!newCrate.hasName()) {
if (newName.isEmpty()) {
QMessageBox::warning(
nullptr,
tr("Duplicating Crate Failed"),
tr("A crate cannot have a blank name."));
continue;
}
if (m_pTrackCollection->crates().readCrateByName(newCrate.getName())) {
if (m_pTrackCollection->crates().readCrateByName(newName)) {
QMessageBox::warning(
nullptr,
tr("Duplicating Crate Failed"),
tr("A crate by that name already exists."));
newCrate.resetName();
continue;
}
newCrate.setName(std::move(newName));
DEBUG_ASSERT(newCrate.hasName());
break;
}

CrateId newCrateId;
Expand Down
31 changes: 7 additions & 24 deletions src/util/db/dbnamedentity.h
View file
Original file line number Diff line number Diff line change
@@ -1,44 +1,30 @@
#ifndef MIXXX_DBNAMEDENTITY_H
#define MIXXX_DBNAMEDENTITY_H

#pragma once

#include "util/db/dbentity.h"


// Base class for database entities with a non-empty name.
template<typename T> // where T is derived from DbId
class DbNamedEntity: public DbEntity<T> {
public:
~DbNamedEntity() override = default;

static QString normalizeName(const QString& name) {
return name.trimmed();
}
bool hasName() const {
DEBUG_ASSERT(normalizeName(m_name) == m_name); // already normalized
return !m_name.isEmpty();
}
const QString& getName() const {
return m_name;
}
void setName(const QString& name) {
DEBUG_ASSERT(normalizeName(name) == name); // already normalized
m_name = name;
void setName(QString name) {
// Due to missing trimming names with only whitespaces
// may occur in the database and can't we assert on
// this here!
DEBUG_ASSERT(!name.isEmpty());
m_name = std::move(name);
}
void resetName() {
m_name.clear();
DEBUG_ASSERT(!hasName());
}
bool parseName(const QString& name) {
QString normalizedName(normalizeName(name));
if (name.isEmpty()) {
return false;
} else {
setName(name);
DEBUG_ASSERT(hasName());
return true;
}
}

protected:
DbNamedEntity() = default;
Expand All @@ -54,6 +40,3 @@ template<typename T>
QDebug operator<<(QDebug debug, const DbNamedEntity<T>& entity) {
return debug << QString("%1 '%2'").arg(entity.getId().toString(), entity.getName());
}


#endif // MIXXX_DBNAMEDENTITY_H

0 comments on commit 3047062

Please sign in to comment.