From 691fb348752d3372a5302b4fc53a012ebf7587ff Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 27 Sep 2020 13:52:48 +0200 Subject: [PATCH] Demonstrate that ColumnCache is seriously broken --- src/library/columncache.cpp | 215 ++++++++++++++++++++++-------------- src/library/columncache.h | 24 ++-- 2 files changed, 145 insertions(+), 94 deletions(-) diff --git a/src/library/columncache.cpp b/src/library/columncache.cpp index 590112915b1..ba520eae904 100644 --- a/src/library/columncache.cpp +++ b/src/library/columncache.cpp @@ -3,8 +3,15 @@ #include "library/dao/trackschema.h" #include "library/dao/playlistdao.h" +namespace { - ColumnCache::ColumnCache(const QStringList& columns) { +const QString kSortInt = QStringLiteral("cast(%1 as integer)"); + +const QString kSortNoCase = QStringLiteral("lower(%1)"); + +} // anonymous namespace + +ColumnCache::ColumnCache(const QStringList& columns) { m_pKeyNotationCP = new ControlProxy("[Library]", "key_notation", this); m_pKeyNotationCP->connectValueChanged(this, &ColumnCache::slotSetKeySortOrder); @@ -15,96 +22,136 @@ setColumns(columns); } -void ColumnCache::setColumns(const QStringList& columns) { - m_columnsByIndex.clear(); - m_columnsByIndex.append(columns); - - m_columnIndexByName.clear(); - for (int i = 0; i < columns.size(); ++i) { - QString column = columns[i]; - m_columnIndexByName[column] = i; +void ColumnCache::initColumnIndexByEnum( + Column columnEnum, + const QString& columnName) { + // Verify valid and uninitialized + DEBUG_ASSERT(!columnName.isEmpty()); + DEBUG_ASSERT(m_columnIndexByEnum[columnEnum] == -1); + const int columnIndex = fieldIndex(columnName); + // Verify unambiguous + VERIFY_OR_DEBUG_ASSERT(columnIndex == -1 || + std::count(m_columnIndexByEnum, + m_columnIndexByEnum + + sizeof(m_columnIndexByEnum) / + sizeof(m_columnIndexByEnum[0]), + columnIndex) == 0) { + qCritical() << "ColumnCache: Column name" << columnName << "with index" + << columnIndex << "is already mapped to column enum" + << (std::find(m_columnIndexByEnum, + m_columnIndexByEnum + + sizeof(m_columnIndexByEnum) / + sizeof(m_columnIndexByEnum[0]), + columnIndex) - + m_columnIndexByEnum) + << "and cannot be remapped to column enum" << columnEnum; + return; } + m_columnIndexByEnum[columnEnum] = columnIndex; +} - for (int i = 0; i < NUM_COLUMNS; ++i) { - m_columnIndexByEnum[i] = -1; +void ColumnCache::setColumns(const QStringList& columnNames) { + m_columnsByIndex = columnNames; + + m_columnIndexByName.clear(); + for (int i = 0; i < columnNames.size(); ++i) { + const QString& columnName = columnNames[i]; + VERIFY_OR_DEBUG_ASSERT(!columnName.isEmpty()) { + continue; + } + VERIFY_OR_DEBUG_ASSERT(!m_columnIndexByName.contains(columnName)) { + qCritical() + << "ColumnCache: Skipping duplicate column name" + << columnName + << "column index" + << i + << "that is used for column index" + << m_columnIndexByName[columnName]; + continue; + } + m_columnIndexByName[columnName] = i; } - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ID] = fieldIndex(LIBRARYTABLE_ID); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ARTIST] = fieldIndex(LIBRARYTABLE_ARTIST); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TITLE] = fieldIndex(LIBRARYTABLE_TITLE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ALBUM] = fieldIndex(LIBRARYTABLE_ALBUM); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ALBUMARTIST] = fieldIndex(LIBRARYTABLE_ALBUMARTIST); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_YEAR] = fieldIndex(LIBRARYTABLE_YEAR); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_GENRE] = fieldIndex(LIBRARYTABLE_GENRE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COMPOSER] = fieldIndex(LIBRARYTABLE_COMPOSER); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_GROUPING] = fieldIndex(LIBRARYTABLE_GROUPING); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TRACKNUMBER] = fieldIndex(LIBRARYTABLE_TRACKNUMBER); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_FILETYPE] = fieldIndex(LIBRARYTABLE_FILETYPE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_NATIVELOCATION] = fieldIndex(LIBRARYTABLE_LOCATION); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COMMENT] = fieldIndex(LIBRARYTABLE_COMMENT); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_DURATION] = fieldIndex(LIBRARYTABLE_DURATION); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_BITRATE] = fieldIndex(LIBRARYTABLE_BITRATE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_BPM] = fieldIndex(LIBRARYTABLE_BPM); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_REPLAYGAIN] = fieldIndex(LIBRARYTABLE_REPLAYGAIN); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_CUEPOINT] = fieldIndex(LIBRARYTABLE_CUEPOINT); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_URL] = fieldIndex(LIBRARYTABLE_URL); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_SAMPLERATE] = fieldIndex(LIBRARYTABLE_SAMPLERATE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_WAVESUMMARYHEX] = fieldIndex(LIBRARYTABLE_WAVESUMMARYHEX); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_CHANNELS] = fieldIndex(LIBRARYTABLE_CHANNELS); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_MIXXXDELETED] = fieldIndex(LIBRARYTABLE_MIXXXDELETED); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_DATETIMEADDED] = fieldIndex(LIBRARYTABLE_DATETIMEADDED); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_HEADERPARSED] = fieldIndex(LIBRARYTABLE_HEADERPARSED); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TIMESPLAYED] = fieldIndex(LIBRARYTABLE_TIMESPLAYED); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_PLAYED] = fieldIndex(LIBRARYTABLE_PLAYED); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_RATING] = fieldIndex(LIBRARYTABLE_RATING); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_KEY] = fieldIndex(LIBRARYTABLE_KEY); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_KEY_ID] = fieldIndex(LIBRARYTABLE_KEY_ID); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_BPM_LOCK] = fieldIndex(LIBRARYTABLE_BPM_LOCK); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_PREVIEW] = fieldIndex(LIBRARYTABLE_PREVIEW); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COLOR] = fieldIndex(LIBRARYTABLE_COLOR); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART] = fieldIndex(LIBRARYTABLE_COVERART); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_SOURCE] = fieldIndex(LIBRARYTABLE_COVERART_SOURCE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_TYPE] = fieldIndex(LIBRARYTABLE_COVERART_TYPE); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_LOCATION] = fieldIndex(LIBRARYTABLE_COVERART_LOCATION); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_COLOR] = - fieldIndex(LIBRARYTABLE_COVERART_COLOR); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_DIGEST] = - fieldIndex(LIBRARYTABLE_COVERART_DIGEST); - m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COVERART_HASH] = fieldIndex(LIBRARYTABLE_COVERART_HASH); - - m_columnIndexByEnum[COLUMN_TRACKLOCATIONSTABLE_FSDELETED] = fieldIndex(TRACKLOCATIONSTABLE_FSDELETED); - - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_TRACKID] = fieldIndex(PLAYLISTTRACKSTABLE_TRACKID); - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_POSITION] = fieldIndex(PLAYLISTTRACKSTABLE_POSITION); - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_PLAYLISTID] = fieldIndex(PLAYLISTTRACKSTABLE_PLAYLISTID); - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_LOCATION] = fieldIndex(PLAYLISTTRACKSTABLE_LOCATION); - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_ARTIST] = fieldIndex(PLAYLISTTRACKSTABLE_ARTIST); - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_TITLE] = fieldIndex(PLAYLISTTRACKSTABLE_TITLE); - m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_DATETIMEADDED] = fieldIndex(PLAYLISTTRACKSTABLE_DATETIMEADDED); - - m_columnIndexByEnum[COLUMN_REKORDBOX_ANALYZE_PATH] = fieldIndex(REKORDBOX_ANALYZE_PATH); - - const QString sortInt("cast(%1 as integer)"); - const QString sortNoCase("lower(%1)"); + std::fill( + m_columnIndexByEnum, + m_columnIndexByEnum + sizeof(m_columnIndexByEnum) / sizeof(m_columnIndexByEnum[0]), + -1); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_ID, LIBRARYTABLE_ID); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_ARTIST, LIBRARYTABLE_ARTIST); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_TITLE, LIBRARYTABLE_TITLE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_ALBUM, LIBRARYTABLE_ALBUM); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_ALBUMARTIST, LIBRARYTABLE_ALBUMARTIST); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_YEAR, LIBRARYTABLE_YEAR); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_GENRE, LIBRARYTABLE_GENRE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COMPOSER, LIBRARYTABLE_COMPOSER); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_GROUPING, LIBRARYTABLE_GROUPING); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_TRACKNUMBER, LIBRARYTABLE_TRACKNUMBER); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_FILETYPE, LIBRARYTABLE_FILETYPE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_NATIVELOCATION, LIBRARYTABLE_LOCATION); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COMMENT, LIBRARYTABLE_COMMENT); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_DURATION, LIBRARYTABLE_DURATION); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_BITRATE, LIBRARYTABLE_BITRATE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_BPM, LIBRARYTABLE_BPM); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_REPLAYGAIN, LIBRARYTABLE_REPLAYGAIN); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_CUEPOINT, LIBRARYTABLE_CUEPOINT); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_URL, LIBRARYTABLE_URL); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_SAMPLERATE, LIBRARYTABLE_SAMPLERATE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_WAVESUMMARYHEX, LIBRARYTABLE_WAVESUMMARYHEX); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_CHANNELS, LIBRARYTABLE_CHANNELS); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_MIXXXDELETED, LIBRARYTABLE_MIXXXDELETED); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_DATETIMEADDED, LIBRARYTABLE_DATETIMEADDED); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_HEADERPARSED, LIBRARYTABLE_HEADERPARSED); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_TIMESPLAYED, LIBRARYTABLE_TIMESPLAYED); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_PLAYED, LIBRARYTABLE_PLAYED); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_RATING, LIBRARYTABLE_RATING); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_KEY, LIBRARYTABLE_KEY); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_KEY_ID, LIBRARYTABLE_KEY_ID); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_BPM_LOCK, LIBRARYTABLE_BPM_LOCK); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_PREVIEW, LIBRARYTABLE_PREVIEW); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COLOR, LIBRARYTABLE_COLOR); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART, LIBRARYTABLE_COVERART); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART_SOURCE, LIBRARYTABLE_COVERART_SOURCE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART_TYPE, LIBRARYTABLE_COVERART_TYPE); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART_LOCATION, LIBRARYTABLE_COVERART_LOCATION); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART_COLOR, LIBRARYTABLE_COVERART_COLOR); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART_DIGEST, LIBRARYTABLE_COVERART_DIGEST); + initColumnIndexByEnum(COLUMN_LIBRARYTABLE_COVERART_HASH, LIBRARYTABLE_COVERART_HASH); + + initColumnIndexByEnum(COLUMN_TRACKLOCATIONSTABLE_FSDELETED, TRACKLOCATIONSTABLE_FSDELETED); + + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_TRACKID, PLAYLISTTRACKSTABLE_TRACKID); + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_POSITION, PLAYLISTTRACKSTABLE_POSITION); + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_PLAYLISTID, PLAYLISTTRACKSTABLE_PLAYLISTID); + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_LOCATION, PLAYLISTTRACKSTABLE_LOCATION); + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_ARTIST, PLAYLISTTRACKSTABLE_ARTIST); + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_TITLE, PLAYLISTTRACKSTABLE_TITLE); + initColumnIndexByEnum(COLUMN_PLAYLISTTRACKSTABLE_DATETIMEADDED, + PLAYLISTTRACKSTABLE_DATETIMEADDED); + + initColumnIndexByEnum(COLUMN_REKORDBOX_ANALYZE_PATH, REKORDBOX_ANALYZE_PATH); m_columnSortByIndex.clear(); // Add the columns that requires a special sort - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ARTIST], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TITLE], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ALBUM], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ALBUMARTIST], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_YEAR], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_GENRE], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COMPOSER], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_GROUPING], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TRACKNUMBER], sortInt); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_FILETYPE], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_NATIVELOCATION], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COMMENT], sortNoCase); - - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_LOCATION], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_ARTIST], sortNoCase); - m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_TITLE], sortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ARTIST], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TITLE], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ALBUM], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_ALBUMARTIST], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_YEAR], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_GENRE], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COMPOSER], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_GROUPING], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_TRACKNUMBER], kSortInt); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_FILETYPE], kSortNoCase); + m_columnSortByIndex.insert( + m_columnIndexByEnum[COLUMN_LIBRARYTABLE_NATIVELOCATION], + kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_LIBRARYTABLE_COMMENT], kSortNoCase); + + m_columnSortByIndex.insert( + m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_LOCATION], + kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_ARTIST], kSortNoCase); + m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_TITLE], kSortNoCase); slotSetKeySortOrder(m_pKeyNotationCP->get()); } diff --git a/src/library/columncache.h b/src/library/columncache.h index de32e1767b6..8b4c073f074 100644 --- a/src/library/columncache.h +++ b/src/library/columncache.h @@ -1,5 +1,4 @@ -#ifndef COLUMNCACHE_H -#define COLUMNCACHE_H +#pragma once #include #include @@ -77,29 +76,32 @@ class ColumnCache : public QObject { void setColumns(const QStringList& columns); - inline int fieldIndex(Column column) const { - if (column < 0 || column >= NUM_COLUMNS) { + int fieldIndex(Column column) const { + if (column <= COLUMN_LIBRARYTABLE_INVALID) { + return -1; + } + VERIFY_OR_DEBUG_ASSERT(column < NUM_COLUMNS) { return -1; } return m_columnIndexByEnum[column]; } - inline int fieldIndex(const QString& columnName) const { + int fieldIndex(const QString& columnName) const { return m_columnIndexByName.value(columnName, -1); } - inline QString columnName(Column column) const { + QString columnName(Column column) const { return columnNameForFieldIndex(fieldIndex(column)); } - inline QString columnNameForFieldIndex(int index) const { + QString columnNameForFieldIndex(int index) const { if (index < 0 || index >= m_columnsByIndex.size()) { return QString(); } return m_columnsByIndex.at(index); } - inline QString columnSortForFieldIndex(int index) const { + QString columnSortForFieldIndex(int index) const { // Check if there is a special sort clause QString format = m_columnSortByIndex.value(index, "%1"); return format.arg(columnNameForFieldIndex(index)); @@ -117,10 +119,12 @@ class ColumnCache : public QObject { } private: + void initColumnIndexByEnum( + Column columnEnum, + const QString& columnName); + ControlProxy* m_pKeyNotationCP; private slots: void slotSetKeySortOrder(double); }; - -#endif /* COLUMNCACHE_H */