diff --git a/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in b/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in index 819d4c05dbe..6b993df616d 100644 --- a/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in +++ b/python/gui/auto_generated/processing/qgsprocessingtoolboxmodel.sip.in @@ -67,6 +67,12 @@ Returns a list of children belonging to the node. %End + QgsProcessingToolboxModelNode *takeChild( QgsProcessingToolboxModelNode *node ); +%Docstring +Removes the specified ``node`` from this node's children, and gives +ownership back to the caller. +%End + QgsProcessingToolboxModelGroupNode *getChildGroupNode( const QString &id ); %Docstring Tries to find a child node belonging to this node, which corresponds to @@ -115,6 +121,11 @@ specified ``provider``. QgsProcessingProvider *provider(); %Docstring Returns the provider associated with this node. +%End + + QString providerId() const; +%Docstring +Returns the provider ID. %End }; @@ -258,6 +269,16 @@ a None if the index does not represent a provider. .. seealso:: :py:func:`algorithmForIndex` +.. seealso:: :py:func:`indexForProvider` +%End + + QString providerIdForIndex( const QModelIndex &index ) const; +%Docstring +Returns the provider ID which corresponds to a given ``index``, or +an empty string if the index does not represent a provider. + +.. seealso:: :py:func:`algorithmForIndex` + .. seealso:: :py:func:`indexForProvider` %End @@ -278,9 +299,9 @@ Returns true if ``index`` corresponds to an algorithm. .. seealso:: :py:func:`algorithmForIndex` %End - QModelIndex indexForProvider( QgsProcessingProvider *provider ) const; + QModelIndex indexForProvider( const QString &providerId ) const; %Docstring -Returns the index corresponding to the specified ``provider``. +Returns the index corresponding to the specified ``providerId``. .. seealso:: :py:func:`providerForIndex` %End diff --git a/python/gui/processing/qgsprocessingtoolboxmodel.sip.in b/python/gui/processing/qgsprocessingtoolboxmodel.sip.in index 819d4c05dbe..91af6ce466e 100644 --- a/python/gui/processing/qgsprocessingtoolboxmodel.sip.in +++ b/python/gui/processing/qgsprocessingtoolboxmodel.sip.in @@ -67,6 +67,12 @@ Returns a list of children belonging to the node. %End + QgsProcessingToolboxModelNode* takeChild( QgsProcessingToolboxModelNode* node ); +%Docstring +Removes the specified ``node`` from this node's children, and gives +ownership back to the caller. +%End + QgsProcessingToolboxModelGroupNode *getChildGroupNode( const QString &id ); %Docstring Tries to find a child node belonging to this node, which corresponds to @@ -115,6 +121,11 @@ specified ``provider``. QgsProcessingProvider *provider(); %Docstring Returns the provider associated with this node. +%End + + QString providerId() const; +%Docstring +Returns the provider ID. %End }; @@ -258,6 +269,16 @@ a None if the index does not represent a provider. .. seealso:: :py:func:`algorithmForIndex` +.. seealso:: :py:func:`indexForProvider` +%End + + QString providerIdForIndex( const QModelIndex &index ) const; +%Docstring +Returns the provider ID which corresponds to a given ``index``, or +an empty string if the index does not represent a provider. + +.. seealso:: :py:func:`algorithmForIndex` + .. seealso:: :py:func:`indexForProvider` %End @@ -278,9 +299,9 @@ Returns true if ``index`` corresponds to an algorithm. .. seealso:: :py:func:`algorithmForIndex` %End - QModelIndex indexForProvider( QgsProcessingProvider *provider ) const; + QModelIndex indexForProvider(const QString &providerId ) const; %Docstring -Returns the index corresponding to the specified ``provider``. +Returns the index corresponding to the specified ``providerId``. .. seealso:: :py:func:`providerForIndex` %End diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.cpp b/src/gui/processing/qgsprocessingtoolboxmodel.cpp index fbc03b41c69..5be3a98f87f 100644 --- a/src/gui/processing/qgsprocessingtoolboxmodel.cpp +++ b/src/gui/processing/qgsprocessingtoolboxmodel.cpp @@ -31,6 +31,11 @@ QgsProcessingToolboxModelNode::~QgsProcessingToolboxModelNode() deleteChildren(); } +QgsProcessingToolboxModelNode *QgsProcessingToolboxModelNode::takeChild( QgsProcessingToolboxModelNode *node ) +{ + return mChildren.takeAt( mChildren.indexOf( node ) ); +} + QgsProcessingToolboxModelGroupNode *QgsProcessingToolboxModelNode::getChildGroupNode( const QString &groupId ) { for ( QgsProcessingToolboxModelNode *node : qgis::as_const( mChildren ) ) @@ -67,7 +72,8 @@ void QgsProcessingToolboxModelNode::deleteChildren() // QgsProcessingToolboxModelProviderNode::QgsProcessingToolboxModelProviderNode( QgsProcessingProvider *provider ) - : mProvider( provider ) + : mProviderId( provider->id() ) + , mProvider( provider ) {} QgsProcessingProvider *QgsProcessingToolboxModelProviderNode::provider() @@ -109,7 +115,7 @@ QgsProcessingToolboxModel::QgsProcessingToolboxModel( QObject *parent, QgsProces rebuild(); connect( mRegistry, &QgsProcessingRegistry::providerAdded, this, &QgsProcessingToolboxModel::providerAdded ); - connect( mRegistry, &QgsProcessingRegistry::providerRemoved, this, &QgsProcessingToolboxModel::rebuild ); + connect( mRegistry, &QgsProcessingRegistry::providerRemoved, this, &QgsProcessingToolboxModel::providerRemoved ); } void QgsProcessingToolboxModel::rebuild() @@ -132,9 +138,44 @@ void QgsProcessingToolboxModel::providerAdded( const QString &id ) if ( !provider ) return; - beginResetModel(); - addProvider( provider ); - endResetModel(); + if ( !isTopLevelProvider( id ) ) + { + int previousRowCount = rowCount(); + beginInsertRows( QModelIndex(), previousRowCount + 1, previousRowCount + 1 ); + addProvider( provider ); + endInsertRows(); + } + else + { + //native providers use top level groups - that's too hard for us to + //work out exactly what's going to change, so just reset the model + beginResetModel(); + addProvider( provider ); + endResetModel(); + } +} + +void QgsProcessingToolboxModel::providerRemoved( const QString &id ) +{ + if ( isTopLevelProvider( id ) ) + { + // native providers use top level groups - so we can't + // work out what to remove. Just rebuild the whole model instead. + rebuild(); + } + else + { + // can't retrieve the provider - it's been deleted! + // so find node by id + QModelIndex index = indexForProvider( id ); + QgsProcessingToolboxModelNode *node = index2node( index ); + if ( !node ) + return; + + beginRemoveRows( QModelIndex(), index.row(), index.row() ); + delete mRootNode->takeChild( node ); + endRemoveRows(); + } } QgsProcessingToolboxModelNode *QgsProcessingToolboxModel::index2node( const QModelIndex &index ) const @@ -163,7 +204,7 @@ void QgsProcessingToolboxModel::addProvider( QgsProcessingProvider *provider ) connect( provider, &QgsProcessingProvider::algorithmsLoaded, this, &QgsProcessingToolboxModel::rebuild, Qt::UniqueConnection ); QgsProcessingToolboxModelNode *parentNode = nullptr; - if ( !isTopLevelProvider( provider ) ) + if ( !isTopLevelProvider( provider->id() ) ) { std::unique_ptr< QgsProcessingToolboxModelProviderNode > node = qgis::make_unique< QgsProcessingToolboxModelProviderNode >( provider ); parentNode = node.get(); @@ -198,11 +239,11 @@ void QgsProcessingToolboxModel::addProvider( QgsProcessingProvider *provider ) } } -bool QgsProcessingToolboxModel::isTopLevelProvider( QgsProcessingProvider *provider ) +bool QgsProcessingToolboxModel::isTopLevelProvider( const QString &providerId ) { - return provider->id() == QLatin1String( "qgis" ) || - provider->id() == QLatin1String( "native" ) || - provider->id() == QLatin1String( "3d" ); + return providerId == QLatin1String( "qgis" ) || + providerId == QLatin1String( "native" ) || + providerId == QLatin1String( "3d" ); } QString QgsProcessingToolboxModel::toolTipForAlgorithm( const QgsProcessingAlgorithm *algorithm ) @@ -368,6 +409,15 @@ QgsProcessingProvider *QgsProcessingToolboxModel::providerForIndex( const QModel return qobject_cast< QgsProcessingToolboxModelProviderNode * >( n )->provider(); } +QString QgsProcessingToolboxModel::providerIdForIndex( const QModelIndex &index ) const +{ + QgsProcessingToolboxModelNode *n = index2node( index ); + if ( !n || n->nodeType() != QgsProcessingToolboxModelNode::NodeProvider ) + return nullptr; + + return qobject_cast< QgsProcessingToolboxModelProviderNode * >( n )->providerId(); +} + const QgsProcessingAlgorithm *QgsProcessingToolboxModel::algorithmForIndex( const QModelIndex &index ) const { QgsProcessingToolboxModelNode *n = index2node( index ); @@ -383,27 +433,24 @@ bool QgsProcessingToolboxModel::isAlgorithm( const QModelIndex &index ) const return ( n && n->nodeType() == QgsProcessingToolboxModelNode::NodeAlgorithm ); } -QModelIndex QgsProcessingToolboxModel::indexForProvider( QgsProcessingProvider *provider ) const +QModelIndex QgsProcessingToolboxModel::indexForProvider( const QString &providerId ) const { - if ( !provider ) - return QModelIndex(); - - std::function< QModelIndex( const QModelIndex &parent, QgsProcessingProvider *provider ) > findIndex = [&]( const QModelIndex & parent, QgsProcessingProvider * provider )->QModelIndex + std::function< QModelIndex( const QModelIndex &parent, const QString &providerId ) > findIndex = [&]( const QModelIndex & parent, const QString & providerId )->QModelIndex { for ( int row = 0; row < rowCount( parent ); ++row ) { QModelIndex current = index( row, 0, parent ); - if ( providerForIndex( current ) == provider ) + if ( providerIdForIndex( current ) == providerId ) return current; - QModelIndex checkChildren = findIndex( current, provider ); + QModelIndex checkChildren = findIndex( current, providerId ); if ( checkChildren.isValid() ) return checkChildren; } return QModelIndex(); }; - return findIndex( QModelIndex(), provider ); + return findIndex( QModelIndex(), providerId ); } QModelIndex QgsProcessingToolboxModel::indexOfParentTreeNode( QgsProcessingToolboxModelNode *parentNode ) const diff --git a/src/gui/processing/qgsprocessingtoolboxmodel.h b/src/gui/processing/qgsprocessingtoolboxmodel.h index 0e066b42d7b..a780147e266 100644 --- a/src/gui/processing/qgsprocessingtoolboxmodel.h +++ b/src/gui/processing/qgsprocessingtoolboxmodel.h @@ -89,6 +89,12 @@ class GUI_EXPORT QgsProcessingToolboxModelNode : public QObject */ QList children() const { return mChildren; } SIP_SKIP + /** + * Removes the specified \a node from this node's children, and gives + * ownership back to the caller. + */ + QgsProcessingToolboxModelNode *takeChild( QgsProcessingToolboxModelNode *node ); + /** * Tries to find a child node belonging to this node, which corresponds to * a group node with the given group \a id. Returns nullptr if no matching @@ -140,8 +146,14 @@ class GUI_EXPORT QgsProcessingToolboxModelProviderNode : public QgsProcessingToo */ QgsProcessingProvider *provider(); + /** + * Returns the provider ID. + */ + QString providerId() const { return mProviderId; } + private: + QString mProviderId; QgsProcessingProvider *mProvider = nullptr; }; @@ -278,6 +290,15 @@ class GUI_EXPORT QgsProcessingToolboxModel : public QAbstractItemModel */ QgsProcessingProvider *providerForIndex( const QModelIndex &index ) const; + /** + * Returns the provider ID which corresponds to a given \a index, or + * an empty string if the index does not represent a provider. + * + * \see algorithmForIndex() + * \see indexForProvider() + */ + QString providerIdForIndex( const QModelIndex &index ) const; + /** * Returns the algorithm which corresponds to a given \a index, or * a nullptr if the index does not represent an algorithm. @@ -295,10 +316,10 @@ class GUI_EXPORT QgsProcessingToolboxModel : public QAbstractItemModel bool isAlgorithm( const QModelIndex &index ) const; /** - * Returns the index corresponding to the specified \a provider. + * Returns the index corresponding to the specified \a providerId. * \see providerForIndex() */ - QModelIndex indexForProvider( QgsProcessingProvider *provider ) const; + QModelIndex indexForProvider( const QString &providerId ) const; /** * Returns the index corresponding to the parent of a given node. @@ -309,6 +330,7 @@ class GUI_EXPORT QgsProcessingToolboxModel : public QAbstractItemModel void rebuild(); void providerAdded( const QString &id ); + void providerRemoved( const QString &id ); private: @@ -319,10 +341,10 @@ class GUI_EXPORT QgsProcessingToolboxModel : public QAbstractItemModel void addProvider( QgsProcessingProvider *provider ); /** - * Returns true if \a provider is a "top-level" provider, which shows + * Returns true if \a providerId is a "top-level" provider, which shows * groups directly under the root node and not under a provider child node. */ - static bool isTopLevelProvider( QgsProcessingProvider *provider ); + static bool isTopLevelProvider( const QString &providerId ); /** * Returns a formatted tooltip for an \a algorithm. diff --git a/tests/src/gui/testqgsprocessingmodel.cpp b/tests/src/gui/testqgsprocessingmodel.cpp index d028dc657b7..1b03f6e75a6 100644 --- a/tests/src/gui/testqgsprocessingmodel.cpp +++ b/tests/src/gui/testqgsprocessingmodel.cpp @@ -133,7 +133,7 @@ void TestQgsProcessingModel::testModel() QVERIFY( model.index( 0, 0, QModelIndex() ).isValid() ); QCOMPARE( model.providerForIndex( model.index( 0, 0, QModelIndex() ) ), p1 ); QVERIFY( !model.providerForIndex( model.index( 1, 0, QModelIndex() ) ) ); - QCOMPARE( model.indexForProvider( p1 ), model.index( 0, 0, QModelIndex() ) ); + QCOMPARE( model.indexForProvider( p1->id() ), model.index( 0, 0, QModelIndex() ) ); QVERIFY( !model.indexForProvider( nullptr ).isValid() ); QCOMPARE( model.rowCount( model.index( 0, 0, QModelIndex() ) ), 0 ); QVERIFY( !model.hasChildren( model.index( 0, 0, QModelIndex() ) ) ); @@ -152,8 +152,8 @@ void TestQgsProcessingModel::testModel() QCOMPARE( model.providerForIndex( model.index( 0, 0, QModelIndex() ) ), p1 ); QCOMPARE( model.providerForIndex( model.index( 1, 0, QModelIndex() ) ), p2 ); QVERIFY( !model.providerForIndex( model.index( 2, 0, QModelIndex() ) ) ); - QCOMPARE( model.indexForProvider( p1 ), model.index( 0, 0, QModelIndex() ) ); - QCOMPARE( model.indexForProvider( p2 ), model.index( 1, 0, QModelIndex() ) ); + QCOMPARE( model.indexForProvider( p1->id() ), model.index( 0, 0, QModelIndex() ) ); + QCOMPARE( model.indexForProvider( p2->id() ), model.index( 1, 0, QModelIndex() ) ); QVERIFY( !model.indexForProvider( nullptr ).isValid() ); QVERIFY( !model.hasChildren( model.index( 1, 0, QModelIndex() ) ) ); @@ -174,9 +174,9 @@ void TestQgsProcessingModel::testModel() QVERIFY( model.hasChildren() ); QVERIFY( model.index( 2, 0, QModelIndex() ).isValid() ); QCOMPARE( model.providerForIndex( model.index( 2, 0, QModelIndex() ) ), p3 ); - QCOMPARE( model.indexForProvider( p1 ), model.index( 0, 0, QModelIndex() ) ); - QCOMPARE( model.indexForProvider( p2 ), model.index( 1, 0, QModelIndex() ) ); - QCOMPARE( model.indexForProvider( p3 ), model.index( 2, 0, QModelIndex() ) ); + QCOMPARE( model.indexForProvider( p1->id() ), model.index( 0, 0, QModelIndex() ) ); + QCOMPARE( model.indexForProvider( p2->id() ), model.index( 1, 0, QModelIndex() ) ); + QCOMPARE( model.indexForProvider( p3->id() ), model.index( 2, 0, QModelIndex() ) ); QCOMPARE( model.rowCount( model.index( 1, 0, QModelIndex() ) ), 0 ); QCOMPARE( model.rowCount( model.index( 2, 0, QModelIndex() ) ), 2 ); QVERIFY( !model.hasChildren( model.index( 1, 0, QModelIndex() ) ) ); @@ -216,7 +216,7 @@ void TestQgsProcessingModel::testModel() DummyAlgorithm *a4 = new DummyAlgorithm( "a4", "group1" ); DummyProvider *p4 = new DummyProvider( "p4", "provider4", QList< QgsProcessingAlgorithm * >() << a3 << a4 ); registry.addProvider( p4 ); - QModelIndex p4ProviderIndex = model.indexForProvider( p4 ); + QModelIndex p4ProviderIndex = model.indexForProvider( p4->id() ); QModelIndex groupIndex = model.index( 0, 0, p4ProviderIndex ); QCOMPARE( model.rowCount( groupIndex ), 2 ); QCOMPARE( model.algorithmForIndex( model.index( 0, 0, groupIndex ) )->id(), QStringLiteral( "p4:a3" ) ); @@ -229,7 +229,7 @@ void TestQgsProcessingModel::testModel() DummyProvider *p5 = new DummyProvider( "p5", "provider5", QList< QgsProcessingAlgorithm * >() << a5 << a6 << a7 ); registry.addProvider( p5 ); QCOMPARE( model.rowCount(), 5 ); - QModelIndex p5ProviderIndex = model.indexForProvider( p5 ); + QModelIndex p5ProviderIndex = model.indexForProvider( p5->id() ); QCOMPARE( model.rowCount( p5ProviderIndex ), 3 ); groupIndex = model.index( 0, 0, p5ProviderIndex ); @@ -247,7 +247,7 @@ void TestQgsProcessingModel::testModel() // reload provider p5->refreshAlgorithms(); QCOMPARE( model.rowCount(), 5 ); - p5ProviderIndex = model.indexForProvider( p5 ); + p5ProviderIndex = model.indexForProvider( p5->id() ); QCOMPARE( model.rowCount( p5ProviderIndex ), 3 ); groupIndex = model.index( 0, 0, p5ProviderIndex ); @@ -261,7 +261,7 @@ void TestQgsProcessingModel::testModel() QCOMPARE( model.algorithmForIndex( model.index( 0, 0, groupIndex ) )->id(), QStringLiteral( "p5:a7" ) ); p3->refreshAlgorithms(); - providerIndex = model.indexForProvider( p3 ); + providerIndex = model.indexForProvider( p3->id() ); group1Index = model.index( 0, 0, providerIndex ); QCOMPARE( model.rowCount( group1Index ), 1 ); alg1Index = model.index( 0, 0, group1Index ); @@ -276,21 +276,15 @@ void TestQgsProcessingModel::testModel() QCOMPARE( model.rowCount(), 4 ); QVERIFY( model.index( 0, 0, QModelIndex() ).isValid() ); QCOMPARE( model.providerForIndex( model.index( 0, 0, QModelIndex() ) ), p2 ); - //shhh - p1 is actually a dangling pointer here. That's fine for tests! - QVERIFY( !model.indexForProvider( p1 ).isValid() ); QCOMPARE( model.data( model.index( 0, 0, QModelIndex() ), Qt::DisplayRole ).toString(), QStringLiteral( "provider2" ) ); registry.removeProvider( p5 ); QCOMPARE( model.rowCount(), 3 ); - QVERIFY( !model.indexForProvider( p5 ).isValid() ); registry.removeProvider( p2 ); QCOMPARE( model.rowCount(), 2 ); - QVERIFY( !model.indexForProvider( p2 ).isValid() ); registry.removeProvider( p3 ); QCOMPARE( model.rowCount(), 1 ); - QVERIFY( !model.indexForProvider( p3 ).isValid() ); registry.removeProvider( p4 ); QCOMPARE( model.rowCount(), 0 ); - QVERIFY( !model.indexForProvider( p4 ).isValid() ); QCOMPARE( model.columnCount(), 1 ); QVERIFY( !model.hasChildren() ); QVERIFY( !model.providerForIndex( model.index( 0, 0, QModelIndex() ) ) );