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

lp1861431: Fix debug assertion for invalid crate names #2477

Merged
merged 2 commits into from
Feb 2, 2020
Merged

lp1861431: Fix debug assertion for invalid crate names #2477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ce4bad
Fix debug assertion for crate names with leading/trailing ws
uklotzde Feb 1, 2020
97127ed
Use low-level string operations instead of semantic abstraction
uklotzde Feb 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 15 additions & 12 deletions src/library/crate/cratefeature.cpp
View file
Open in desktop
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."));
daschuer marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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