QSqlTableModel::submitAll(): avoid resubmiting successful changes

Consider what happens the 1st change succeeds and the 2nd fails. No
select will be done. When submitAll() is called again, the 1st will
still seem to be pending. It will fail or have unexpected effects if
the primary values were changed.

The solution is to avoid resubmitting successful changes. We leave
them in the cache so they stay visible. Submitted changes cannot
be reverted of course.

Change-Id: Ibf400555effa1c3801d02f8713b4b69856ede23a
Reviewed-by: Yunqiao Yin <charles.yin@nokia.com>

2 files changed, 20 insertions, 2 deletions

diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp
index 12561ee189..b21a965c21 100644
--- a/src/sql/models/qsqltablemodel.cpp
+++ b/src/sql/models/qsqltablemodel.cpp
@@ -121,6 +121,11 @@ void QSqlTableModelPrivate::revertCachedRow(int row)
 {
     Q_Q(QSqlTableModel);
     ModifiedRow r = cache.value(row);
+
+    // cannot revert a committed change
+    if (r.submitted())
+        return;
+
     switch (r.op()) {
     case QSqlTableModelPrivate::None:
         Q_ASSERT_X(false, "QSqlTableModelPrivate::revertCachedRow()", "Invalid entry in cache map");
@@ -373,6 +378,12 @@ bool QSqlTableModel::select()
     if (query.isEmpty())
         return false;
 
+    // clear the submitted flags so revertAll can do its job
+    for (QSqlTableModelPrivate::CacheMap::Iterator it = d->cache.begin();
+         it != d->cache.constEnd();
+         ++it)
+        it.value().setSubmitted(false);
+
     revertAll();
     QSqlQuery qu(query, d->db);
     setQuery(qu);
@@ -657,8 +668,11 @@ bool QSqlTableModel::submitAll()
 {
     Q_D(QSqlTableModel);
 
-    for (QSqlTableModelPrivate::CacheMap::ConstIterator it = d->cache.constBegin();
+    for (QSqlTableModelPrivate::CacheMap::Iterator it = d->cache.begin();
          it != d->cache.constEnd(); ++it) {
+        if (it.value().submitted())
+            continue;
+
         switch (it.value().op()) {
         case QSqlTableModelPrivate::Insert:
             if (!insertRowIntoTable(it.value().rec()))
@@ -676,6 +690,7 @@ bool QSqlTableModel::submitAll()
             Q_ASSERT_X(false, "QSqlTableModel::submitAll()", "Invalid cache operation");
             break;
         }
+        it.value().setSubmitted(true);
     }
 
     // all changes have been committed
diff --git a/src/sql/models/qsqltablemodel_p.h b/src/sql/models/qsqltablemodel_p.h
index 052f99005f..0ae6b53742 100644
--- a/src/sql/models/qsqltablemodel_p.h
+++ b/src/sql/models/qsqltablemodel_p.h
@@ -101,7 +101,7 @@ public:
     {
     public:
         inline ModifiedRow(Op o = None, const QSqlRecord &r = QSqlRecord())
-            : m_op(o), m_rec(r)
+            : m_op(o), m_rec(r), m_submitted(false)
         {
             for (int i = m_rec.count() - 1; i >= 0; --i)
                 m_rec.setGenerated(i, false);
@@ -114,9 +114,12 @@ public:
             m_rec.setValue(c, v);
             m_rec.setGenerated(c, true);
         }
+        inline bool submitted() const { return m_submitted; }
+        inline void setSubmitted(bool b) { m_submitted = b; }
     private:
         Op m_op;
         QSqlRecord m_rec;
+        bool m_submitted;
     };
 
     typedef QMap<int, ModifiedRow> CacheMap;