5d43ac0 · unity8 · git.iko.soy

git.iko.soy/unity8

Fix insertions/removals on the qlimitproxymodel

. Fixes: https://bugs.launchpad.net/bugs/1213959. Approved by PS Jenkins bot, Michal Hruby.

commit5d43ac0c04b64ba61446833f05cc1afe14c2d9e6 authorAlbert Astals <albert.astals@canonical.com> dateWed 21 Aug 2013, 13:58 (13 years ago) parent9847f82 dc12ab7

3 files changed+225−12

compare with xenial

unifiedsplit

plugins/Utils/qlimitproxymodelqml.cpp+90−4 plugins/Utils/qlimitproxymodelqml.h+2 tests/plugins/Utils/qlimitproxymodeltest.cpp+133−8

plugins/Utils/qlimitproxymodelqml.cpp+90−4

@@ -25,6 +25,8 @@ QLimitProxyModelQML::QLimitProxyModelQML(QObject *parent)

2525 , m_limit(-1)

2626 , m_sourceInserting(false)

2727 , m_sourceRemoving(false)

28+ , m_dataChangedBegin(-1)

29+ , m_dataChangedEnd(-1)

2830{

2931 connect(this, SIGNAL(modelReset()), SIGNAL(countChanged()));

3032 connect(this, SIGNAL(rowsInserted(QModelIndex,int,int)), SIGNAL(countChanged()));

@@ -130,8 +132,45 @@ QLimitProxyModelQML::sourceRowsAboutToBeInserted(const QModelIndex &parent, int

130132 beginInsertRows(mapFromSource(parent), start, end);

131133 m_sourceInserting = true;

132134 } else if (start < m_limit) {

133− beginInsertRows(mapFromSource(parent), start, qMin(m_limit - 1, end));

134− m_sourceInserting = true;

135+ const int nSourceAddedItems = end - start + 1;

136+ const int currentCount = QIdentityProxyModel::rowCount();

137+ if (currentCount + nSourceAddedItems <= m_limit) {

138+ // After Inserting items we will be under the limit

139+ // so just proceed with the insertion normally

140+ beginInsertRows(mapFromSource(parent), start, end);

141+ m_sourceInserting = true;

142+ } else if (currentCount >= m_limit) {

143+ // We are already over the limit so to our users we are not adding items, just

144+ // changing it's data, i.e we had something like

145+ // A B C D E

146+ // with a limit of 5

147+ // after inserting (let's say three 'F' at position 1) we will have

148+ // A F F F B

149+ // so we just need to signal a dataChanged from 1 to 4

150+ m_dataChangedBegin = start;

151+ m_dataChangedEnd = m_limit - 1;

152+ } else { // currentCount < m_limit && currentCount + nSourceAddedItems > m_limit

153+ // We have less items than the limit but after adding them we will be over

154+ // To our users this means we need to insert some items and change the

155+ // data of some others, i.e we had something like

156+ // A B C

157+ // with a limit of 5

158+ // after inserting (let's say three 'F' at position 1) we will have

159+ // A F F F B

160+ // so we need to signal an insetion from position 1 to 2, instead of from

161+ // position 1 to 3 and a after that a data changed from 3 to 4

162+ const int nItemsToInsert = m_limit - currentCount;

163+ beginInsertRows(mapFromSource(parent), start, start + nItemsToInsert - 1);

164+ m_sourceInserting = true;

165+ m_dataChangedBegin = start + nItemsToInsert;

166+ m_dataChangedEnd = m_limit - 1;

167+ if (m_dataChangedBegin > m_dataChangedEnd) {

168+ // Just in case we were empty and insert 6 items with a limit of 5

169+ // We don't want to signal a dataChanged from 5 to 4

170+ m_dataChangedBegin = -1;

171+ m_dataChangedEnd = -1;

172+ }

173+ }

135174 }

136175}

137176

@@ -142,8 +181,45 @@ QLimitProxyModelQML::sourceRowsAboutToBeRemoved(const QModelIndex &parent, int s

142181 beginRemoveRows(mapFromSource(parent), start, end);

143182 m_sourceRemoving = true;

144183 } else if (start < m_limit) {

145− beginRemoveRows(mapFromSource(parent), start, qMin(m_limit - 1, end));

146− m_sourceRemoving = true;

184+ const int nSourceRemovedItems = end - start + 1;

185+ const int currentCount = QIdentityProxyModel::rowCount();

186+ if (currentCount <= m_limit) {

187+ // We are already under the limit so

188+ // so just proceed with the removal normally

189+ beginRemoveRows(mapFromSource(parent), start, end);

190+ m_sourceRemoving = true;

191+ } else if (currentCount - nSourceRemovedItems >= m_limit) {

192+ // Even after removing items we will be at or over the limit

193+ // So to our users we are not removing anything, just changing the data

194+ // i.e. we had a internal model with

195+ // A B C D E F G H

196+ // and a limit of 5, our users just see

197+ // A B C D E

198+ // so if we remove 3 items starting at 1 we have to expose

199+ // A E F G H

200+ // that is, a dataChanged from 1 to 4

201+ m_dataChangedBegin = start;

202+ m_dataChangedEnd = m_limit - 1;

203+ } else { // currentCount > m_limit && currentCount - nSourceRemovedItems < m_limit

204+ // We have more items than the limit but after removing we will be below it

205+ // So to our users we both removing and changing the data

206+ // i.e. we had a internal model with

207+ // A B C D E F G

208+ // and a limit of 5, our users just see

209+ // A B C D E

210+ // so if we remove items from 1 to 3 we have to expose

211+ // A E F G

212+ // that is, a remove from 4 to 4 and a dataChanged from 1 to 3

213+ const int nItemsToRemove = m_limit - (currentCount - nSourceRemovedItems);

214+ beginRemoveRows(mapFromSource(parent), m_limit - nItemsToRemove, m_limit - 1);

215+ m_sourceRemoving = true;

216+ m_dataChangedBegin = start;

217+ m_dataChangedEnd = m_limit - nItemsToRemove - 1;

218+ if (m_dataChangedBegin > m_dataChangedEnd) {

219+ m_dataChangedBegin = -1;

220+ m_dataChangedEnd = -1;

221+ }

222+ }

147223 }

148224}

149225

@@ -154,6 +230,11 @@ QLimitProxyModelQML::sourceRowsInserted(const QModelIndex & /*parent*/, int /*st

154230 endInsertRows();

155231 m_sourceInserting = false;

156232 }

233+ if (m_dataChangedBegin != -1 && m_dataChangedEnd != -1) {

234+ dataChanged(index(m_dataChangedBegin, 0), index(m_dataChangedEnd, 0));

235+ m_dataChangedBegin = -1;

236+ m_dataChangedEnd = -1;

237+ }

157238}

158239

159240void

@@ -163,4 +244,9 @@ QLimitProxyModelQML::sourceRowsRemoved(const QModelIndex & /*parent*/, int /*sta

163244 endRemoveRows();

164245 m_sourceRemoving = false;

165246 }

247+ if (m_dataChangedBegin != -1 && m_dataChangedEnd != -1) {

248+ dataChanged(index(m_dataChangedBegin, 0), index(m_dataChangedEnd, 0));

249+ m_dataChangedBegin = -1;

250+ m_dataChangedEnd = -1;

251+ }

166252}

plugins/Utils/qlimitproxymodelqml.h+2

@@ -55,6 +55,8 @@ private:

5555 int m_limit;

5656 bool m_sourceInserting;

5757 bool m_sourceRemoving;

58+ int m_dataChangedBegin;

59+ int m_dataChangedEnd;

5860};

5961

6062#endif // QLIMITPROXYMODELQML_H

tests/plugins/Utils/qlimitproxymodeltest.cpp+133−8

@@ -58,10 +58,17 @@ public:

5858 m_roles = roles;

5959 }

6060

61+ void clear()

62+ {

63+ beginRemoveRows(QModelIndex(), 0, m_list.count() - 1);

64+ m_list.clear();

65+ endRemoveRows();

66+ }

67+

6168 bool insertRows(int row, int count, const QModelIndex &parent=QModelIndex()) {

6269 beginInsertRows(parent, row, row+count-1);

6370 for (int i=0; i<count; i++) {

64− m_list.insert(i+row, "test"+i);

71+ m_list.insert(i+row, QString::number(i));

6572 }

6673 endInsertRows();

6774 return true;

@@ -88,6 +95,60 @@ private:

8895 QHash<int, QByteArray> m_roles;

8996};

9097

98+class MirrorModel : public QObject

99+{

100+ Q_OBJECT

101+

102+public:

103+ MirrorModel(QAbstractItemModel *model) : m_mirror(model)

104+ {

105+ connect(model, SIGNAL(rowsInserted(QModelIndex,int,int)), this, SLOT(rowsInserted(QModelIndex,int,int)));

106+ connect(model, SIGNAL(rowsRemoved(QModelIndex,int,int)), this, SLOT(rowsRemoved(QModelIndex,int,int)));

107+ connect(model, SIGNAL(dataChanged(QModelIndex,QModelIndex)), this, SLOT(dataChanged(QModelIndex,QModelIndex)));

108+ }

109+

110+ void check()

111+ {

112+ QCOMPARE(m_list.count(), m_mirror->rowCount());

113+ for (int i = 0; i < m_list.count(); ++i) {

114+ QCOMPARE(m_list[i], m_mirror->data(m_mirror->index(i, 0)).toString());

115+ }

116+ }

117+

118+private Q_SLOTS:

119+ void rowsInserted(const QModelIndex &parent, int start, int end)

120+ {

121+ QVERIFY(!parent.isValid());

122+ for (int i = start; i <= end; ++i) {

123+ m_list.insert(i, m_mirror->data(m_mirror->index(i, 0)).toString());

124+ }

125+ }

126+

127+ void rowsRemoved(const QModelIndex &parent, int start, int end)

128+ {

129+ QVERIFY(!parent.isValid());

130+ for (int i = end; i >= start; --i) {

131+ m_list.removeAt(i);

132+ }

133+ }

134+

135+ void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight)

136+ {

137+ QVERIFY(topLeft.isValid());

138+ QVERIFY(bottomRight.isValid());

139+ QVERIFY(!topLeft.parent().isValid());

140+ QVERIFY(!bottomRight.parent().isValid());

141+ for (int i = topLeft.row(); i <= bottomRight.row(); ++i) {

142+ m_list[i] = m_mirror->data(m_mirror->index(i, 0)).toString();

143+ }

144+ }

145+

146+

147+private:

148+ QStringList m_list;

149+ QAbstractItemModel *m_mirror;

150+};

151+

91152class QLimitProxyModelTest : public QObject

92153{

93154 Q_OBJECT

@@ -401,15 +462,10 @@ private Q_SLOTS:

401462 QCOMPARE(spyOnCountChanged.count(), 0);

402463

403464 model.removeRows(2, 2);

404− QCOMPARE(spyOnRowsRemoved.count(), 1);

465+ QCOMPARE(spyOnRowsRemoved.count(), 0);

405466 QCOMPARE(spyOnRowsInserted.count(), 0);

406− QCOMPARE(spyOnCountChanged.count(), 1);

407− arguments = spyOnRowsRemoved.takeFirst();

408− QCOMPARE(arguments.at(1).toInt(), 2);

409− QCOMPARE(arguments.at(2).toInt(), 3);

467+ QCOMPARE(spyOnCountChanged.count(), 0);

410468 QCOMPARE(proxy.rowCount(), 7);

411− spyOnRowsInserted.clear();

412− spyOnCountChanged.clear();

413469

414470 model.removeRows(0, 7);

415471 QCOMPARE(spyOnRowsRemoved.count(), 1);

@@ -479,6 +535,75 @@ private Q_SLOTS:

479535 QCOMPARE(proxy.rowCount(), 5);

480536 QCOMPARE(spyOnCountChanged.count(), 1);

481537 }

538+

539+ void testMirrorModel() {

540+ QLimitProxyModelQML proxy;

541+ MockListModel model;

542+ MirrorModel mirror(&proxy);

543+

544+ proxy.setModel(&model);

545+ proxy.setLimit(5);

546+

547+ // Checks what happens when the model already has

548+ // more items than the limit and we add stuff to

549+ // the front

550+ model.insertRows(0, 7);

551+ mirror.check();

552+ model.insertRows(1, 3);

553+ mirror.check();

554+ model.clear();

555+ mirror.check();

556+

557+ // Checks what happens when the model does

558+ // not has more items than limit but adding

559+ // stuff to its front makes it go past the limit

560+ model.insertRows(0, 3);

561+ mirror.check();

562+ model.insertRows(1, 3);

563+ mirror.check();

564+ model.clear();

565+ mirror.check();

566+

567+ // Checks what happens when the model does

568+ // not has more items than limit and adding

569+ // stuff to its front makes it not go past the limit

570+ model.insertRows(0, 1);

571+ mirror.check();

572+ model.insertRows(1, 3);

573+ mirror.check();

574+ model.clear();

575+ mirror.check();

576+

577+ // Checks what happens when the model already has

578+ // more items than the limit and we remove stuff from

579+ // the front but it still has more than the limit

580+ model.insertRows(0, 10);

581+ mirror.check();

582+ model.removeRows(1, 3);

583+ mirror.check();

584+ model.clear();

585+ mirror.check();

586+

587+ // Checks what happens when the model already has

588+ // more items than the limit and we remove stuff from

589+ // the front but it has less than the limit

590+ model.insertRows(0, 6);

591+ mirror.check();

592+ model.removeRows(1, 3);

593+ mirror.check();

594+ model.clear();

595+ mirror.check();

596+

597+ // Checks what happens when the model has

598+ // less items than the limit and we remove stuff from

599+ // the front

600+ model.insertRows(0, 4);

601+ mirror.check();

602+ model.removeRows(1, 3);

603+ mirror.check();

604+ model.clear();

605+ mirror.check();

606+ }

482607};

483608

484609QTEST_GUILESS_MAIN(QLimitProxyModelTest)