Repository: nuttyartist/notes
File: src/dbmanager.cpp (C++)

Code before:
    qint64 epochTimeDateModified = note.lastModificationdateTime().toMSecsSinceEpoch();
    QString content = note.content().replace(QChar('\x0'), emptyStr);
    QString fullTitle = note.fullTitle().replace(QChar('\x0'), emptyStr);
    
    query.prepare(QStringLiteral("UPDATE node_table SET modification_date = :modification_date, content = :content, "
                                 "title = :title, scrollbar_position = :scrollbar_position WHERE id = :id AND node_type = :node_type;"));
    query.bindValue(QStringLiteral(":modification_date"), epochTimeDateModified);
    query.bindValue(QStringLiteral(":content"), content);
    query.bindValue(QStringLiteral(":title"), fullTitle);
    query.bindValue(QStringLiteral(":id"), id);
    query.bindValue(QStringLiteral(":scrollbar_position"), note.scrollBarPosition());
    query.bindValue(QStringLiteral(":node_type"), static_cast<int>(NodeData::Note));
    if (!query.exec()) {
        qDebug() << __FUNCTION__ << __LINE__ << query.lastError();
    }
    return (query.numRowsAffected() == 1);
}

QList<NodeData> DBManager::readOldNBK(const QString fileName)
{
    QList<NodeData> noteList;
    {
        QFile file(fileName);
        file.open(QIODevice::ReadOnly);
        QDataStream in(&file);
#if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)
        in.setVersion(QDataStream::Qt_5_6);
#elif QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
        in.setVersion(QDataStream::Qt_5_4);
#elif QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)
        in.setVersion(QDataStream::Qt_5_2);
#endif

        try {
            in >> noteList;
        } catch (...) {
            // Any exception deserializing will result in an empty note list and  the user will be notified
        }
        file.close();
    }
    if (noteList.isEmpty()) {
        QFile file(fileName);
        file.open(QIODevice::ReadOnly);
        QDataStream in(&file);
#if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)
        in.setVersion(QDataStream::Qt_5_6);
#elif QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
        in.setVersion(QDataStream::Qt_5_4);
#elif QT_VERSION >= QT_VERSION_CHECK(5, 2, 0)
        in.setVersion(QDataStream::Qt_5_2);
#endif

Review comment (bug):
Judging by the name of the function (`readOldNBK`), this function is for reading old backup files. As such, changing the version of the data stream seems like the wrong thing to do to me. Even changing it based on the Qt version, as was done originally, was already wrong. The files on disk will not change, and to be able to read them, we should keep using the same version. See https://doc.qt.io/qt-6/qdatastream.html#versioning for an explanation.

The proper fix may be to figure out which QDataStream version was likely used for writing most of the files out there, and specify only that version. If we want to switch to newer QDataStream versions, we need to introduce our own file version, based on which we can set a QDataStream version.

Code after:
    qint64 epochTimeDateModified = note.lastModificationdateTime().toMSecsSinceEpoch();
    QString content = note.content().replace(QChar('\x0'), emptyStr);
    QString fullTitle = note.fullTitle().replace(QChar('\x0'), emptyStr);
    
    query.prepare(QStringLiteral("UPDATE node_table SET modification_date = :modification_date, content = :content, "
                                 "title = :title, scrollbar_position = :scrollbar_position WHERE id = :id AND node_type = :node_type;"));
    query.bindValue(QStringLiteral(":modification_date"), epochTimeDateModified);
    query.bindValue(QStringLiteral(":content"), content);
    query.bindValue(QStringLiteral(":title"), fullTitle);
    query.bindValue(QStringLiteral(":id"), id);
    query.bindValue(QStringLiteral(":scrollbar_position"), note.scrollBarPosition());
    query.bindValue(QStringLiteral(":node_type"), static_cast<int>(NodeData::Note));
    if (!query.exec()) {
        qDebug() << __FUNCTION__ << __LINE__ << query.lastError();
    }
    return (query.numRowsAffected() == 1);
}

QList<NodeData> DBManager::readOldNBK(const QString fileName)
{
    QList<NodeData> noteList;
    {
        QFile file(fileName);
        file.open(QIODevice::ReadOnly);
        QDataStream in(&file);
        in.setVersion(QDataStream::Qt_5_9);

        try {
            in >> noteList;
        } catch (...) {
            // Any exception deserializing will result in an empty note list and  the user will be notified
        }
        file.close();
    }
    if (noteList.isEmpty()) {
        QFile file(fileName);
        file.open(QIODevice::ReadOnly);
        QDataStream in(&file);
        in.setVersion(QDataStream::Qt_5_9);

        QList<NodeData*> nl;
        try {
            in >> nl;
        } catch (...) {
            // Any exception deserializing will result in an empty note list and  the user will be notified
        }
        if (!nl.isEmpty()) {
            for (const auto& n : QT_AS_CONST(nl)) {
                noteList.append(*n);
            }
        }