Deprecate QgsBrowserModel::connectItem

This method should only be private, and is completely dangerous to call from external code (it breaks the model!). Deprecate and replace with a no-op
qgis · Oct 18, 2018 · 72bf08e · 72bf08e
1 parent ac3fe8f
commit 72bf08e
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 15 deletions.
diff --git a/python/core/auto_generated/qgsbrowsermodel.sip.in b/python/core/auto_generated/qgsbrowsermodel.sip.in
@@ -117,7 +117,13 @@ items, i.e. it does not fetch children.
 %End
 
 
-    void connectItem( QgsDataItem *item );
+ void connectItem( QgsDataItem *item ) /Deprecated/;
+%Docstring
+
+.. deprecated:: since QGIS 3.4 -- this method has no effect, and is dangerous to call
+in earlier QGIS versions. Any usage should be removed (and will have no harmful
+side-effects!).
+%End
 
     bool initialized() const;
 %Docstring

diff --git a/src/core/qgsbrowsermodel.cpp b/src/core/qgsbrowsermodel.cpp
@@ -75,7 +75,7 @@ void QgsBrowserModel::updateProjectHome()
   mProjectHome = home.isNull() ? nullptr : new QgsProjectHomeItem( nullptr, tr( "Project Home" ), home, QStringLiteral( PROJECT_HOME_PREFIX ) + home );
   if ( mProjectHome )
   {
-    connectItem( mProjectHome );
+    setupItemConnections( mProjectHome );
 
     beginInsertRows( QModelIndex(), 0, 0 );
     mRootItems.insert( 0, mProjectHome );
@@ -90,14 +90,14 @@ void QgsBrowserModel::addRootItems()
   // give the home directory a prominent third place
   QgsDirectoryItem *item = new QgsDirectoryItem( nullptr, tr( "Home" ), QDir::homePath(), QStringLiteral( HOME_PREFIX ) + QDir::homePath() );
   item->setSortKey( QStringLiteral( " 2" ) );
-  connectItem( item );
+  setupItemConnections( item );
   mRootItems << item;
 
   // add favorite directories
   mFavorites = new QgsFavoritesItem( nullptr, tr( "Favorites" ) );
   if ( mFavorites )
   {
-    connectItem( mFavorites );
+    setupItemConnections( mFavorites );
     mRootItems << mFavorites;
   }
 
@@ -113,7 +113,7 @@ void QgsBrowserModel::addRootItems()
     item->setSortKey( QStringLiteral( " 3 %1" ).arg( path ) );
     mDriveItems.insert( path, item );
 
-    connectItem( item );
+    setupItemConnections( item );
     mRootItems << item;
   }
 
@@ -142,7 +142,7 @@ void QgsBrowserModel::addRootItems()
       // Forward the signal from the root items to the model (and then to the app)
       connect( item, &QgsDataItem::connectionsChanged, this, &QgsBrowserModel::connectionsChanged );
       QgsDebugMsgLevel( "Add new top level item : " + item->name(), 4 );
-      connectItem( item );
+      setupItemConnections( item );
       providerMap.insertMulti( capabilities, item );
     }
   }
@@ -367,6 +367,11 @@ QModelIndex QgsBrowserModel::findPath( QAbstractItemModel *model, const QString
   return QModelIndex(); // not found
 }
 
+void QgsBrowserModel::connectItem( QgsDataItem * )
+{
+  // deprecated, no use
+}
+
 void QgsBrowserModel::reload()
 {
   // TODO: put items creating currently children in threads to deleteLater (does not seem urget because reload() is not used in QGIS)
@@ -415,7 +420,7 @@ void QgsBrowserModel::refreshDrives()
       item->setSortKey( QStringLiteral( " 3 %1" ).arg( path ) );
 
       mDriveItems.insert( path, item );
-      connectItem( item );
+      setupItemConnections( item );
 
       beginInsertRows( QModelIndex(), mRootItems.count(), mRootItems.count() );
       mRootItems << item;
@@ -508,7 +513,7 @@ void QgsBrowserModel::itemStateChanged( QgsDataItem *item, QgsDataItem::State ol
   emit stateChanged( idx, oldState );
 }
 
-void QgsBrowserModel::connectItem( QgsDataItem *item )
+void QgsBrowserModel::setupItemConnections( QgsDataItem *item )
 {
   connect( item, &QgsDataItem::beginInsertItems,
            this, &QgsBrowserModel::beginInsertItems );

diff --git a/src/core/qgsbrowsermodel.h b/src/core/qgsbrowsermodel.h
@@ -136,7 +136,12 @@ class CORE_EXPORT QgsBrowserModel : public QAbstractItemModel
     //! \note not available in Python bindings
     static QModelIndex findPath( QAbstractItemModel *model, const QString &path, Qt::MatchFlag matchFlag = Qt::MatchExactly ) SIP_SKIP;
 
-    void connectItem( QgsDataItem *item );
+    /**
+     * \deprecated since QGIS 3.4 -- this method has no effect, and is dangerous to call
+     * in earlier QGIS versions. Any usage should be removed (and will have no harmful
+     * side-effects!).
+     */
+    Q_DECL_DEPRECATED void connectItem( QgsDataItem *item ) SIP_DEPRECATED;
 
     /**
      * Returns true if the model has been initialized.
@@ -216,6 +221,8 @@ class CORE_EXPORT QgsBrowserModel : public QAbstractItemModel
     bool mInitialized = false;
     QMap< QString, QgsDataItem * > mDriveItems;
 
+    void setupItemConnections( QgsDataItem *item );
+
     void removeRootItem( QgsDataItem *item );
 
     friend class TestQgsBrowserModel;

diff --git a/tests/src/core/testqgsbrowsermodel.cpp b/tests/src/core/testqgsbrowsermodel.cpp
@@ -76,7 +76,7 @@ void TestQgsBrowserModel::testModel()
   // add a root child
   QgsDataCollectionItem *rootItem1 = new QgsDataCollectionItem( nullptr, QStringLiteral( "Test" ), QStringLiteral( "root1" ) );
   QVERIFY( !model.findItem( rootItem1 ).isValid() );
-  model.connectItem( rootItem1 );
+  model.setupItemConnections( rootItem1 );
   model.mRootItems.append( rootItem1 );
 
   QCOMPARE( model.rowCount(), 1 );
@@ -99,7 +99,7 @@ void TestQgsBrowserModel::testModel()
 
   // second root item
   QgsDataCollectionItem *rootItem2 = new QgsDataCollectionItem( nullptr, QStringLiteral( "Test2" ), QStringLiteral( "root2" ) );
-  model.connectItem( rootItem2 );
+  model.setupItemConnections( rootItem2 );
   model.mRootItems.append( rootItem2 );
 
   QCOMPARE( model.rowCount(), 2 );
@@ -115,7 +115,7 @@ void TestQgsBrowserModel::testModel()
 
   // child item
   QgsDataCollectionItem *childItem1 = new QgsDataCollectionItem( nullptr, QStringLiteral( "Child1" ), QStringLiteral( "child1" ) );
-  model.connectItem( childItem1 );
+  model.setupItemConnections( childItem1 );
   rootItem1->addChild( childItem1 );
 
   QCOMPARE( model.rowCount(), 2 );
@@ -135,16 +135,13 @@ void TestQgsBrowserModel::testModel()
 
   // more children
   QgsDataCollectionItem *childItem2 = new QgsDataCollectionItem( nullptr, QStringLiteral( "Child2" ), QStringLiteral( "child2" ) );
-  model.connectItem( childItem2 );
   rootItem1->addChildItem( childItem2, true );
 
   QgsDataCollectionItem *childItem3 = new QgsDataCollectionItem( nullptr, QStringLiteral( "Child3" ), QStringLiteral( "child3" ) );
-  model.connectItem( childItem3 );
   childItem2->addChildItem( childItem3, true );
   QCOMPARE( childItem2->rowCount(), 1 );
 
   QgsDataCollectionItem *childItem4 = new QgsDataCollectionItem( nullptr, QStringLiteral( "Child4" ), QStringLiteral( "child4" ) );
-  model.connectItem( childItem4 );
   rootItem2->addChildItem( childItem4, true );
 
   QCOMPARE( model.rowCount(), 2 );