Added API methods to retrieve only valid maplayers

... and to count them Tests updated accordingly, note that from now on the assumption that a layer store or a layer project contains only valid layers will not be true anymore. To be honest it has never been true, because a layer can become invalid at any time during its life cycle, so better never assume that a layer from the store or from the project is vaid.
qgis · Nov 5, 2018 · 497a7da · 497a7da
1 parent f928980
commit 497a7da
Show file tree

Hide file tree

Showing 8 changed files with 156 additions and 41 deletions.
diff --git a/python/core/auto_generated/qgsmaplayerstore.sip.in b/python/core/auto_generated/qgsmaplayerstore.sip.in
@@ -37,6 +37,13 @@ Constructor for QgsMapLayerStore.
 Returns the number of layers contained in the store.
 %End
 
+    int validCount() const;
+%Docstring
+Returns the number of valid layers contained in the store.
+
+.. versionadded:: 3.6
+%End
+
 
     int __len__() const;
 %Docstring
@@ -89,6 +96,19 @@ Returns a map of all layers by layer ID.
 .. seealso:: :py:func:`layers`
 %End
 
+    QMap<QString, QgsMapLayer *> validMapLayers() const;
+%Docstring
+Returns a map of all valid layers by layer ID.
+
+.. seealso:: :py:func:`mapLayer`
+
+.. seealso:: :py:func:`mapLayersByName`
+
+.. seealso:: :py:func:`layers`
+
+.. versionadded:: 3.6
+%End
+
 
     QList<QgsMapLayer *> addMapLayers( const QList<QgsMapLayer *> &layers /Transfer/);
 

diff --git a/python/core/auto_generated/qgsproject.sip.in b/python/core/auto_generated/qgsproject.sip.in
@@ -700,6 +700,11 @@ Returns a pointer to the project's internal layer store.
     int count() const;
 %Docstring
 Returns the number of registered layers.
+%End
+
+    int validCount() const;
+%Docstring
+Returns the number of registered valid layers.
 %End
 
     QgsMapLayer *mapLayer( const QString &layerId ) const;
@@ -737,6 +742,19 @@ Returns a map of all registered layers by layer ID.
 .. seealso:: :py:func:`mapLayersByName`
 
 .. seealso:: :py:func:`layers`
+%End
+
+    QMap<QString, QgsMapLayer *> validMapLayers() const;
+%Docstring
+Returns a map of all registered valid layers by layer ID.
+
+.. seealso:: :py:func:`mapLayer`
+
+.. seealso:: :py:func:`mapLayersByName`
+
+.. seealso:: :py:func:`layers`
+
+.. versionadded:: 3.6
 %End
 
     bool isZipped() const;

diff --git a/src/core/qgsmaplayerstore.cpp b/src/core/qgsmaplayerstore.cpp
@@ -16,7 +16,9 @@
  ***************************************************************************/
 
 #include "qgsmaplayerstore.h"
+#include "qgsmaplayer.h"
 #include "qgslogger.h"
+#include <QList>
 
 QgsMapLayerStore::QgsMapLayerStore( QObject *parent )
   : QObject( parent )
@@ -32,6 +34,18 @@ int QgsMapLayerStore::count() const
   return mMapLayers.size();
 }
 
+int QgsMapLayerStore::validCount() const
+{
+  int i = 0;
+  const QList<QgsMapLayer *> cLayers = mMapLayers.values();
+  for ( const auto l : cLayers )
+  {
+    if ( l->isValid() )
+      i++;
+  }
+  return i;
+}
+
 QgsMapLayer *QgsMapLayerStore::mapLayer( const QString &layerId ) const
 {
   return mMapLayers.value( layerId );
@@ -212,3 +226,14 @@ QMap<QString, QgsMapLayer *> QgsMapLayerStore::mapLayers() const
 {
   return mMapLayers;
 }
+
+QMap<QString, QgsMapLayer *> QgsMapLayerStore::validMapLayers() const
+{
+  QMap<QString, QgsMapLayer *> validLayers;
+  for ( const auto &id : mMapLayers.keys() )
+  {
+    if ( mMapLayers[id]->isValid() )
+      validLayers[id] = mMapLayers[id];
+  }
+  return validLayers;
+}
diff --git a/src/core/qgsmaplayerstore.h b/src/core/qgsmaplayerstore.h
@@ -50,6 +50,12 @@ class CORE_EXPORT QgsMapLayerStore : public QObject
      */
     int count() const;
 
+    /**
+     * Returns the number of valid layers contained in the store.
+     * \since QGIS 3.6
+     */
+    int validCount() const;
+
 #ifdef SIP_RUN
 
     /**
@@ -93,6 +99,15 @@ class CORE_EXPORT QgsMapLayerStore : public QObject
      */
     QMap<QString, QgsMapLayer *> mapLayers() const;
 
+    /**
+     * Returns a map of all valid layers by layer ID.
+     * \see mapLayer()
+     * \see mapLayersByName()
+     * \see layers()
+     * \since QGIS 3.6
+     */
+    QMap<QString, QgsMapLayer *> validMapLayers() const;
+
 #ifndef SIP_RUN
 
     /**

diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -1499,43 +1499,46 @@ void QgsProject::onMapLayersAdded( const QList<QgsMapLayer *> &layers )
 
   Q_FOREACH ( QgsMapLayer *layer, layers )
   {
-    QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
-    if ( vlayer )
+    if ( layer->isValid() )
     {
-      if ( autoTransaction() )
+      QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
+      if ( vlayer )
       {
-        if ( QgsTransaction::supportsTransaction( vlayer ) )
+        if ( autoTransaction() )
         {
-          QString connString = QgsDataSourceUri( vlayer->source() ).connectionInfo();
-          QString key = vlayer->providerType();
-
-          QgsTransactionGroup *tg = mTransactionGroups.value( qMakePair( key, connString ) );
-
-          if ( !tg )
+          if ( QgsTransaction::supportsTransaction( vlayer ) )
           {
-            tg = new QgsTransactionGroup();
-            mTransactionGroups.insert( qMakePair( key, connString ), tg );
-            tgChanged = true;
+            QString connString = QgsDataSourceUri( vlayer->source() ).connectionInfo();
+            QString key = vlayer->providerType();
+
+            QgsTransactionGroup *tg = mTransactionGroups.value( qMakePair( key, connString ) );
+
+            if ( !tg )
+            {
+              tg = new QgsTransactionGroup();
+              mTransactionGroups.insert( qMakePair( key, connString ), tg );
+              tgChanged = true;
+            }
+            tg->addLayer( vlayer );
           }
-          tg->addLayer( vlayer );
         }
+        vlayer->dataProvider()->setProviderProperty( QgsVectorDataProvider::EvaluateDefaultValues, evaluateDefaultValues() );
       }
-      vlayer->dataProvider()->setProviderProperty( QgsVectorDataProvider::EvaluateDefaultValues, evaluateDefaultValues() );
-    }
 
-    if ( tgChanged )
-      emit transactionGroupsChanged();
+      if ( tgChanged )
+        emit transactionGroupsChanged();
 
-    connect( layer, &QgsMapLayer::configChanged, this, [ = ] { setDirty(); } );
+      connect( layer, &QgsMapLayer::configChanged, this, [ = ] { setDirty(); } );
 
-    // check if we have to update connections for layers with dependencies
-    for ( QMap<QString, QgsMapLayer *>::iterator it = existingMaps.begin(); it != existingMaps.end(); it++ )
-    {
-      QSet<QgsMapLayerDependency> deps = it.value()->dependencies();
-      if ( deps.contains( layer->id() ) )
+      // check if we have to update connections for layers with dependencies
+      for ( QMap<QString, QgsMapLayer *>::iterator it = existingMaps.begin(); it != existingMaps.end(); it++ )
       {
-        // reconnect to change signals
-        it.value()->setDependencies( deps );
+        QSet<QgsMapLayerDependency> deps = it.value()->dependencies();
+        if ( deps.contains( layer->id() ) )
+        {
+          // reconnect to change signals
+          it.value()->setDependencies( deps );
+        }
       }
     }
   }
@@ -2550,6 +2553,11 @@ int QgsProject::count() const
   return mLayerStore->count();
 }
 
+int QgsProject::validCount() const
+{
+  return mLayerStore->validCount();
+}
+
 QgsMapLayer *QgsProject::mapLayer( const QString &layerId ) const
 {
   return mLayerStore->mapLayer( layerId );
@@ -2744,6 +2752,11 @@ QMap<QString, QgsMapLayer *> QgsProject::mapLayers() const
   return mLayerStore->mapLayers();
 }
 
+QMap<QString, QgsMapLayer *> QgsProject::validMapLayers() const
+{
+  return mLayerStore->validMapLayers();
+}
+
 QgsTransactionGroup *QgsProject::transactionGroup( const QString &providerKey, const QString &connString )
 {
   return mTransactionGroups.value( qMakePair( providerKey, connString ) );

diff --git a/src/core/qgsproject.h b/src/core/qgsproject.h
@@ -690,6 +690,9 @@ class CORE_EXPORT QgsProject : public QObject, public QgsExpressionContextGenera
     //! Returns the number of registered layers.
     int count() const;
 
+    //! Returns the number of registered valid layers.
+    int validCount() const;
+
     /**
      * Retrieve a pointer to a registered layer by layer ID.
      * \param layerId ID of layer to retrieve
@@ -716,6 +719,15 @@ class CORE_EXPORT QgsProject : public QObject, public QgsExpressionContextGenera
      */
     QMap<QString, QgsMapLayer *> mapLayers() const;
 
+    /**
+     * Returns a map of all registered valid layers by layer ID.
+     * \see mapLayer()
+     * \see mapLayersByName()
+     * \see layers()
+     * \since QGIS 3.6
+     */
+    QMap<QString, QgsMapLayer *> validMapLayers() const;
+
     /**
      * Returns true if the project comes from a zip archive, false otherwise.
      */

diff --git a/tests/src/python/test_qgsmaplayerstore.py b/tests/src/python/test_qgsmaplayerstore.py
@@ -66,9 +66,11 @@ def test_addMapLayerInvalid(self):
         """ test that invalid map layers can't be added to store """
         store = QgsMapLayerStore()
 
-        self.assertEqual(store.addMapLayer(QgsVectorLayer("Point?field=x:string", 'test', "xxx")), None)
-        self.assertEqual(len(store.mapLayersByName('test')), 0)
-        self.assertEqual(store.count(), 0)
+        vl = QgsVectorLayer("Point?field=x:string", 'test', "xxx")
+        self.assertEqual(store.addMapLayer(vl), vl)
+        self.assertEqual(len(store.mapLayersByName('test')), 1)
+        self.assertEqual(store.count(), 1)
+        self.assertEqual(store.validCount(), 0)
 
     def test_addMapLayerSignals(self):
         """ test that signals are correctly emitted when adding map layer"""
@@ -120,12 +122,14 @@ def test_addMapLayers(self):
         store.removeAllMapLayers()
 
     def test_addMapLayersInvalid(self):
-        """ test that invalid map layersd can't be added to store """
+        """ test that invalid map layers can be added to store """
         store = QgsMapLayerStore()
 
-        self.assertEqual(store.addMapLayers([QgsVectorLayer("Point?field=x:string", 'test', "xxx")]), [])
-        self.assertEqual(len(store.mapLayersByName('test')), 0)
-        self.assertEqual(store.count(), 0)
+        vl = QgsVectorLayer("Point?field=x:string", 'test', "xxx")
+        self.assertEqual(store.addMapLayers([vl]), [vl])
+        self.assertEqual(len(store.mapLayersByName('test')), 1)
+        self.assertEqual(store.count(), 1)
+        self.assertEqual(store.validCount(), 0)
 
     def test_addMapLayersAlreadyAdded(self):
         """ test that already added layers can't be readded to store """

diff --git a/tests/src/python/test_qgsproject.py b/tests/src/python/test_qgsproject.py
@@ -249,12 +249,17 @@ def test_addMapLayerAlreadyAdded(self):
         QgsProject.instance().removeAllMapLayers()
 
     def test_addMapLayerInvalid(self):
-        """ test that invalid map layersd can't be added to registry """
+        """ test that invalid map layers can be added to registry """
         QgsProject.instance().removeAllMapLayers()
 
-        self.assertEqual(QgsProject.instance().addMapLayer(QgsVectorLayer("Point?field=x:string", 'test', "xxx")), None)
-        self.assertEqual(len(QgsProject.instance().mapLayersByName('test')), 0)
-        self.assertEqual(QgsProject.instance().count(), 0)
+        vl = QgsVectorLayer("Point?field=x:string", 'test', "xxx")
+        self.assertEqual(QgsProject.instance().addMapLayer(vl), vl)
+        self.assertFalse(vl in QgsProject.instance().validMapLayers().values())
+        self.assertEqual(len(QgsProject.instance().mapLayersByName('test')), 1)
+        self.assertEqual(QgsProject.instance().count(), 1)
+        self.assertEqual(QgsProject.instance().validCount(), 0)
+
+        self.assertEqual(len(QgsProject.instance().validMapLayers()), 0)
 
         QgsProject.instance().removeAllMapLayers()
 
@@ -313,12 +318,15 @@ def test_addMapLayers(self):
         QgsProject.instance().removeAllMapLayers()
 
     def test_addMapLayersInvalid(self):
-        """ test that invalid map layersd can't be added to registry """
+        """ test that invalid map layers can be added to registry """
         QgsProject.instance().removeAllMapLayers()
 
-        self.assertEqual(QgsProject.instance().addMapLayers([QgsVectorLayer("Point?field=x:string", 'test', "xxx")]), [])
-        self.assertEqual(len(QgsProject.instance().mapLayersByName('test')), 0)
-        self.assertEqual(QgsProject.instance().count(), 0)
+        vl = QgsVectorLayer("Point?field=x:string", 'test', "xxx")
+        self.assertEqual(QgsProject.instance().addMapLayers([vl]), [vl])
+        self.assertFalse(vl in QgsProject.instance().validMapLayers().values())
+        self.assertEqual(len(QgsProject.instance().mapLayersByName('test')), 1)
+        self.assertEqual(QgsProject.instance().count(), 1)
+        self.assertEqual(QgsProject.instance().validCount(), 0)
 
         QgsProject.instance().removeAllMapLayers()