sourceModel()->removeRows returns false although it removes the rows

deleted71

Hi,

while playing around with connecting QML with C++, I stumbled over this little weirdness.

I have an SQLite database which I work on with a model derived from QSQLTableModel.
Then I use a proxy model derived from QSortFilterProxyModel.
The QML TableView then uses said proxy model as model.

Everything seems to work. I can filter the rows shown in the view and delete rows from the view. However, the removeRows method call always returns false.
The documentation says

Returns true if the rows were successfully removed; otherwise returns false.

Any idea why removeRows does not return true here?
Here is the code.

=== main.qml ===

import QtQuick 2.7
import QtQuick.Controls 1.4
import QtQuick.Layouts 1.0
import QtQuick.Window 2.2
import QtQuick.Controls.Styles 1.4

ApplicationWindow {
    id: window
    visible: true
    width: 640
    height: 480
    x: Screen.width / 2 - width / 2
    y: Screen.height / 2 - height / 2
    title: "Qnergenie"

    ColumnLayout {
        anchors.fill: parent

        TextField {
            placeholderText: qsTr("Search...")
            style: TextFieldStyle {
                textColor: "black"
                background: Rectangle {
                    radius: 2
                    implicitWidth: 100
                    implicitHeight: 24
                    border.color: "black"
                    border.width: 1
                }
            }
            onEditingFinished: {
                var filterValue = parseFloat(text)
                myModel.energyFilter = isNaN(filterValue) ? 0 : filterValue
                text = myModel.energyFilter
            }
        }


        TableView {
            id: tableView
            Layout.fillWidth: true
            Layout.fillHeight: true
            selectionMode: SelectionMode.ExtendedSelection
            model: myModel

            TableViewColumn {
                role: "date"
                title: qsTr("Date")
                width: window.width * 0.19
            }
            TableViewColumn {
                role: "voltage"
                title: qsTr("Voltage")
                width: window.width * 0.19

                delegate: Text {
                    text: styleData.value + " V"
                    horizontalAlignment: Text.AlignRight
                }
            }
            TableViewColumn {
                role: "current"
                title: qsTr("Current")
                width: window.width * 0.19

                delegate: Text {
                    text: styleData.value + " A"
                    horizontalAlignment: Text.AlignRight
                }
            }
            TableViewColumn {
                role: "power"
                title: qsTr("Power")
                width: window.width * 0.19

                delegate: Text {
                    text: styleData.value + " W"
                    horizontalAlignment: Text.AlignRight
                }
            }
            TableViewColumn {
                role: "energy"
                title: qsTr("Energy")
                width: window.width * 0.19

                delegate: Text {
                    text: styleData.value + " kWh"
                    horizontalAlignment: Text.AlignRight
                }
            }

            MouseArea {
                anchors.fill: parent
                acceptedButtons: Qt.RightButton
                onClicked: {
                    if (mouse.button === Qt.RightButton)
                    {
                        contextMenu.popup()
                    }
                }
            }

            Menu {
                id: contextMenu
                MenuItem {
                    text: qsTr("Delete")
                    shortcut: "Del"
                    onTriggered: {
                        console.log("click")
                        tableView.selection.forEach(function(rowIndex) {myModel.deleteRow(rowIndex)})
                    }
                }
            }
        }
    }
}

=== main.cpp ===

#include <QApplication>
#include <QDebug>
#include <QSortFilterProxyModel>
#include <QSqlDatabase>
#include <QQmlApplicationEngine>
#include <QQmlContext>

#include "sqltablemodel.h"
#include "proxymodel.h"

int main(int argc, char *argv[])
{
    QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
    QApplication app(argc, argv);

    qmlRegisterType<ProxyModel>("com.basyskom.fsch", 1, 0, "ProxyModel");

    auto db = QSqlDatabase::addDatabase("QSQLITE");
    db.setDatabaseName("/home/fsch/projects/Qnergenie/qnergenie.sqlite3");

    auto ok = db.open();
    if (ok == false) {
        qWarning() << "Exiting...";
        return -1;
    }

    SqlTableModel tabModel;

    ProxyModel myModel;
    myModel.setSourceModel(&tabModel);

    QQmlApplicationEngine engine;
    engine.rootContext()->setContextProperty(QStringLiteral("myModel"), &myModel);
    engine.load(QUrl(QStringLiteral("qrc:/main.qml")));

    return app.exec();
}

=== proxymodel.h ===

#ifndef PROXYMODEL_H
#define PROXYMODEL_H

#include <QDate>
#include <QSortFilterProxyModel>

class ProxyModel : public QSortFilterProxyModel
{
    Q_OBJECT
    Q_PROPERTY(double energyFilter READ energyFilter WRITE setEnergyFilter NOTIFY energyFilterChanged)

public:
    ProxyModel(QObject *parent = nullptr);

    double energyFilter() const;
    void setEnergyFilter(double filterValue);

    Q_INVOKABLE void deleteRow(int row);

signals:
    void energyFilterChanged();

protected:
    bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override;
    bool lessThan(const QModelIndex &left, const QModelIndex &right) const override;

private:
    double m_energyFilter;
};

#endif // PROXYMODEL_H

=== proxymodel.cpp ===

#include "proxymodel.h"

#include <QDebug>
#include "sqltablemodel.h"

ProxyModel::ProxyModel(QObject * parent) : QSortFilterProxyModel(parent)
    , m_energyFilter(0.0)
{
}

double ProxyModel::energyFilter() const
{
    qDebug() << "energyFilter";
    return m_energyFilter;
}

void ProxyModel::setEnergyFilter(double filterValue)
{
    qDebug() << "setEnergyFilter";
    m_energyFilter = filterValue;
    invalidateFilter();
}

void ProxyModel::deleteRow(int row)
{
    QModelIndex proxyIndex = index(row, 1, QModelIndex());
    if (!proxyIndex.isValid()) {
        qDebug() << "proxyIndex invalid";
        return;
    }
    QModelIndex sourceIndex = mapToSource(proxyIndex);
    int sourceRow = sourceIndex.row();
    beginRemoveRows(QModelIndex(), row, row);
    if (sourceModel()->removeRows(sourceRow, 1) == true) {
        qDebug() << "Yay!";
    } else {
        qDebug() << "Nay!";  // <--- This is always called
    }
    endRemoveRows();
}

bool ProxyModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const
{
    if (this->sourceModel() == nullptr) {
        return false;
    }

    auto index = this->sourceModel()->index(sourceRow, 0, sourceParent);
    if (!index.isValid()) {
        return false;
    }

    double energy = sourceModel()->data(sourceModel()->index(sourceRow, 4, sourceParent)).toDouble();

    if (energy > m_energyFilter) {
        return true;
    }
    return false;
}

bool ProxyModel::lessThan(const QModelIndex &left, const QModelIndex &right) const
{
    QVariant leftData = sourceModel()->data(left);
    QVariant rightData = sourceModel()->data(right);
    return leftData.toDateTime() < rightData.toDateTime();
}

=== sqltablemodel.h ===

#ifndef SQLTABLEMODEL_H
#define SQLTABLEMODEL_H

#include <QObject>
#include <QSqlDatabase>
#include <QSqlTableModel>
#include <QHash>

class SqlTableModel : public QSqlTableModel
{
    Q_OBJECT
public:
    explicit SqlTableModel(QObject *parent = 0);
    QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
    QHash<int, QByteArray> roleNames() const override;
};

#endif // SQLTABLEMODEL_H

=== sqltablemodel.cpp ===

#include "sqltablemodel.h"

#include <QSqlTableModel>
#include <QSqlRecord>
#include <QDebug>

SqlTableModel::SqlTableModel(QObject *parent) : QSqlTableModel(parent)
{
    setTable("measurements");
    setEditStrategy(QSqlTableModel::OnFieldChange);
    select();
}

QVariant SqlTableModel::data(const QModelIndex &index, int role) const
{
    QVariant value = QSqlQueryModel::data(index, role);

    if (role < Qt::UserRole) {
        value = QSqlQueryModel::data(index, role);
    } else {
        int row = index.row();
        int col = role - Qt::UserRole - 1;

        auto modelIndex = this->index(row, col);

        value = QSqlQueryModel::data(modelIndex, Qt::DisplayRole);
    }

    return value;
}

QHash<int, QByteArray> SqlTableModel::roleNames() const
{
    QHash<int, QByteArray> roles;

    for ( int i = 0; i < record().count(); i++) {
        roles[Qt::UserRole + i + 1] = record().fieldName(i).toLocal8Bit();
    }

    return roles;
}

VRonin

any reason why you don't just use the parent implementation?

void ProxyModel::deleteRow(int row)
{    
    if (QSortFilterProxyModel::removeRow(row)) {
        qDebug() << "Yay!";
    } else {
        qDebug() << "Nay!";  
    }
}

deleted71

Thank you for your reply.

No, no special reason. Probably found it somewhere. I am a beginner in these things and am currently trying to understand how this works.

The row coming from QML is the current view's row though and not the model's row. That's why I need to call mapToSource() before and for it first create a QModelIndex.

Or am I missing something?

VRonin

QSortFilterProxyModel can take care of it, you don't have to do it manually. did you try my code?

deleted71

Yes I did but now (without the beginRemoveRows() and endRemoveRows()) the view is not updated anymore and the changes are only visible after restarting the application.

VRonin

This is strange as the source for QSortFilterProxyModel does not call beginRemoveRows() and endRemoveRows() let me test it...

EDIT:
could you attach a minimal example of the sqlite database?

deleted71

create table measurements(date integer, voltage real, current real, power real, energy real);

insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.0, 1.1, 2.2, 3.3);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.1, 1.2, 2.3, 3.4);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.2, 1.3, 2.4, 3.5);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.3, 1.4, 2.5, 3.6);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.4, 1.5, 2.6, 3.7);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.5, 1.6, 2.7, 3.8);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 0.6, 1.7, 2.8, 3.9);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.0, 2.1, 3.2, 4.3);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.1, 2.2, 3.3, 4.4);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.2, 2.3, 3.4, 4.5);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.3, 2.4, 3.5, 4.6);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.4, 2.5, 3.6, 4.7);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.5, 2.6, 3.7, 4.8);
insert into measurements(date,voltage,current,power,energy) values(strftime('%Y-%m-%d %H:%M:%S','now'), 1.6, 2.7, 3.8, 4.9);

Just save this as testdata.sql and run sqlite3 qnergenie.sqlite3 <testdata.sql ... And of course change the path in main.cpp

Thanks.

VRonin

Looks like it's a problem with the submit() of the base model.
To make it work add virtual bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex()) override; to SqlTableModel with this body:

bool SqlTableModel::removeRows(int row, int count, const QModelIndex &parent)
{
    bool success = false;
    const EditStrategy oldStartegy=editStrategy();
    setEditStrategy(QSqlTableModel::OnManualSubmit);
    if (QSqlTableModel::removeRows(row,count,parent)) {
        if (submitAll()) {
            success=true;
            qDebug() << "Yay!";
        } else {
            qDebug() << lastError();
        }
    }
    setEditStrategy(oldStartegy);
    return success;
}

and the proxy is simply:

void ProxyModel::deleteRow(int row)
{

    if (removeRow(row)) {
        qDebug() << "Yay!";
    } else {
        qDebug() << "Nay!";
    }
}

If someone has any insight why submit fails even if the rows removal is successful I'm curious now

VRonin

On a separate note ProxyModel::setEnergyFilter should become:

void ProxyModel::setEnergyFilter(double filterValue)
{
    qDebug() << "setEnergyFilter";
if(m_energyFilter==filterValue)
return;
    m_energyFilter = filterValue;
    invalidateFilter();
emit energyFilterChanged();
}

deleted71

I applied your changes to the models and it .. well, still works, just with a Yay! now. I am puzzled. :)

In the beginning it was even weirder. I could reproduce a situation where it also returned Yay! when it failed. So it was just upside-down. ... Not sure what I did at that point, though.

Regarding ProxyModel::setEnergyFilter:
Thanks for the hints. The early exit is a no-brainer. I am just in the "make it work" phase before getting to applying the best practices I picked-up so far.
But what about the signal? It works without it. Do you know why?

Thanks so far for digging into this. :)

VRonin

The filter is just for QML.
If you declare Q_PROPERTY(double energyFilter READ energyFilter WRITE setEnergyFilter NOTIFY energyFilterChanged) QML wil rely on the energyFilterChanged signal to know when the filter changes but the signal has to be emitted manually by your QObject. see http://doc.qt.io/qt-5/properties.html#a-simple-example

deleted71

Ok, I see how it is meant to be and will follow the lead. :)
Still, it works just fine without emitting the signal. Courious.

So two questions remain for me:

why submit fails even if the rows removal is successful
why the filter works fine without emitting the signal

Regards

deleted71

As for why the filter works fine without emitting the signal it's that the view is updated just fine without sending the signal but the property itself is not updated. So if I just print out the property somewhere in the QML GUI, its value remains the initial value.