Add signal when a map theme changes, add tests

Author: nyalldawson

python/core/qgsmapthemecollection.sip

@@ -160,9 +160,10 @@ class QgsMapThemeCollection : QObject
     void setProject( QgsProject* project );
   signals:
 
-    /** Emitted when map themes within the collection are changed.
-     */
     void mapThemesChanged();
+
+    void mapThemeChanged( const QString &theme );
+
     void projectChanged();
 };
 

src/core/qgsmapthemecollection.cpp

@@ -29,7 +29,7 @@
 QgsMapThemeCollection::QgsMapThemeCollection( QgsProject *project )
   : mProject( project )
 {
-  connect( project, &QgsProject::layersRemoved, this, &QgsMapThemeCollection::registryLayersRemoved );
+  connect( project, static_cast<void ( QgsProject::* )( const QStringList & )>( &QgsProject::layersWillBeRemoved ), this, &QgsMapThemeCollection::registryLayersRemoved );
 }
 
 QgsMapThemeCollection::MapThemeLayerRecord QgsMapThemeCollection::createThemeLayerRecord( QgsLayerTreeLayer *nodeLayer, QgsLayerTreeModel *model )
@@ -169,9 +169,9 @@ void QgsMapThemeCollection::setProject( QgsProject *project )
   if ( project == mProject )
     return;
 
-  disconnect( mProject, &QgsProject::layersRemoved, this, &QgsMapThemeCollection::registryLayersRemoved );
+  disconnect( mProject, static_cast<void ( QgsProject::* )( const QStringList & )>( &QgsProject::layersWillBeRemoved ), this, &QgsMapThemeCollection::registryLayersRemoved );
   mProject = project;
-  connect( mProject, &QgsProject::layersRemoved, this, &QgsMapThemeCollection::registryLayersRemoved );
+  connect( mProject, static_cast<void ( QgsProject::* )( const QStringList & )>( &QgsProject::layersWillBeRemoved ), this, &QgsMapThemeCollection::registryLayersRemoved );
   emit projectChanged();
 }
 
@@ -186,6 +186,7 @@ void QgsMapThemeCollection::insert( const QString &name, const QgsMapThemeCollec
   mMapThemes.insert( name, state );
 
   reconnectToLayersStyleManager();
+  emit mapThemeChanged( name );
   emit mapThemesChanged();
 }
 
@@ -197,6 +198,7 @@ void QgsMapThemeCollection::update( const QString &name, const MapThemeRecord &s
   mMapThemes[name] = state;
 
   reconnectToLayersStyleManager();
+  emit mapThemeChanged( name );
   emit mapThemesChanged();
 }
 
@@ -316,7 +318,7 @@ void QgsMapThemeCollection::reconnectToLayersStyleManager()
 
   Q_FOREACH ( QgsMapLayer *ml, layers )
   {
-    connect( ml->styleManager(), SIGNAL( styleRenamed( QString, QString ) ), this, SLOT( layerStyleRenamed( QString, QString ) ) );
+    connect( ml->styleManager(), &QgsMapLayerStyleManager::styleRenamed, this, &QgsMapThemeCollection::layerStyleRenamed );
   }
 }
 
@@ -375,6 +377,7 @@ void QgsMapThemeCollection::readXml( const QDomDocument &doc )
     MapThemeRecord rec;
     rec.setLayerRecords( layerRecords.values() );
     mMapThemes.insert( presetName, rec );
+    emit mapThemeChanged( presetName );
 
     visPresetElem = visPresetElem.nextSiblingElement( QStringLiteral( "visibility-preset" ) );
   }
@@ -426,8 +429,9 @@ void QgsMapThemeCollection::writeXml( QDomDocument &doc )
 
 void QgsMapThemeCollection::registryLayersRemoved( const QStringList &layerIDs )
 {
-  // TODO: this should not be necessary - layers are stored as weak pointers
-
+  // while layers are stored as weak pointers, this triggers the mapThemeChanged signal for
+  // affected themes
+  QSet< QString > changedThemes;
   MapThemeRecordMap::iterator it = mMapThemes.begin();
   for ( ; it != mMapThemes.end(); ++it )
   {
@@ -436,9 +440,17 @@ void QgsMapThemeCollection::registryLayersRemoved( const QStringList &layerIDs )
     {
       MapThemeLayerRecord &layerRec = rec.mLayerRecords[i];
       if ( layerRec.layer() && layerIDs.contains( layerRec.layer()->id() ) )
+      {
         rec.mLayerRecords.removeAt( i-- );
+        changedThemes << it.key();
+      }
     }
   }
+
+  Q_FOREACH ( const QString &theme, changedThemes )
+  {
+    emit mapThemeChanged( theme );
+  }
   emit mapThemesChanged();
 }
 
@@ -448,6 +460,8 @@ void QgsMapThemeCollection::layerStyleRenamed( const QString &oldName, const QSt
   if ( !styleMgr )
     return;
 
+  QSet< QString > changedThemes;
+
   MapThemeRecordMap::iterator it = mMapThemes.begin();
   for ( ; it != mMapThemes.end(); ++it )
   {
@@ -458,10 +472,17 @@ void QgsMapThemeCollection::layerStyleRenamed( const QString &oldName, const QSt
       if ( layerRec.layer() == styleMgr->layer() )
       {
         if ( layerRec.currentStyle == oldName )
+        {
           layerRec.currentStyle = newName;
+          changedThemes << it.key();
+        }
       }
     }
   }
+  Q_FOREACH ( const QString &theme, changedThemes )
+  {
+    emit mapThemeChanged( theme );
+  }
   emit mapThemesChanged();
 }
 

src/core/qgsmapthemecollection.h

@@ -250,6 +250,12 @@ class CORE_EXPORT QgsMapThemeCollection : public QObject
      */
     void mapThemesChanged();
 
+    /**
+     * Emitted when a map theme changes definition.
+     * @note added in QGIS 3.0
+     */
+    void mapThemeChanged( const QString &theme );
+
     /**
      * Emitted when the project changes
      *

tests/src/python/CMakeLists.txt

@@ -70,6 +70,7 @@ ADD_PYTHON_TEST(PyQgsMapLayer test_qgsmaplayer.py)
 ADD_PYTHON_TEST(PyQgsMapLayerModel test_qgsmaplayermodel.py)
 ADD_PYTHON_TEST(PyQgsMapRenderer test_qgsmaprenderer.py)
 ADD_PYTHON_TEST(PyQgsMapRendererCache test_qgsmaprenderercache.py)
+ADD_PYTHON_TEST(PyQgsMapThemeCollection test_qgsmapthemecollection.py)
 ADD_PYTHON_TEST(PyQgsMapUnitScale test_qgsmapunitscale.py)
 ADD_PYTHON_TEST(PyQgsMargins test_qgsmargins.py)
 ADD_PYTHON_TEST(PyQgsMemoryProvider test_provider_memory.py)

tests/src/python/test_qgsmapthemecollection.py

@@ -0,0 +1,102 @@
+# -*- coding: utf-8 -*-
+"""QGIS Unit tests for QgsMapThemeCollection.
+
+.. note:: This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+"""
+__author__ = 'Nyall Dawson'
+__date__ = '8/03/2017'
+__copyright__ = 'Copyright 2017, The QGIS Project'
+# This will get replaced with a git SHA1 when you do a git archive
+__revision__ = '$Format:%H$'
+
+import qgis  # NOQA
+
+from qgis.core import (QgsMapThemeCollection,
+                       QgsProject,
+                       QgsVectorLayer)
+from qgis.testing import start_app, unittest
+from qgis.PyQt.QtTest import QSignalSpy
+
+app = start_app()
+
+
+class TestQgsMapThemeCollection(unittest.TestCase):
+
+    def setUp(self):
+        pass
+
+    def testThemeChanged(self):
+        """
+        Test that the mapTheme(s)Changed signals are correctly emitted in all relevant situations
+        """
+        project = QgsProject()
+        collection = QgsMapThemeCollection(project)
+
+        record = QgsMapThemeCollection.MapThemeRecord()
+
+        theme_changed_spy = QSignalSpy(collection.mapThemeChanged)
+        themes_changed_spy = QSignalSpy(collection.mapThemesChanged)
+
+        collection.insert('theme1', record)
+        self.assertEqual(len(theme_changed_spy), 1)
+        self.assertEqual(theme_changed_spy[-1][0], 'theme1')
+        self.assertEqual(len(themes_changed_spy), 1)
+
+        # reinsert
+        collection.insert('theme1', record)
+        self.assertEqual(len(theme_changed_spy), 2)
+        self.assertEqual(theme_changed_spy[-1][0], 'theme1')
+        self.assertEqual(len(themes_changed_spy), 2)
+
+        # update
+        collection.update('theme1', record)
+        self.assertEqual(len(theme_changed_spy), 3)
+        self.assertEqual(theme_changed_spy[-1][0], 'theme1')
+        self.assertEqual(len(themes_changed_spy), 3)
+
+        # remove invalid
+        collection.removeMapTheme('i wish i was a slave to an age old trade... like riding around on rail cars and working long days')
+        self.assertEqual(len(theme_changed_spy), 3)
+        self.assertEqual(len(themes_changed_spy), 3)
+        # remove valid
+        collection.removeMapTheme('theme1')
+        self.assertEqual(len(theme_changed_spy), 3) # not changed - removed!
+        self.assertEqual(len(themes_changed_spy), 4)
+
+        # reinsert
+        collection.insert('theme1', record)
+        self.assertEqual(len(theme_changed_spy), 4)
+        self.assertEqual(len(themes_changed_spy), 5)
+
+        # clear
+        collection.clear()
+        self.assertEqual(len(theme_changed_spy), 4)  # not changed - removed!
+        self.assertEqual(len(themes_changed_spy), 6)
+
+        # check that mapThemeChanged is emitted if layer is removed
+        layer = QgsVectorLayer("Point?field=fldtxt:string",
+                               "layer1", "memory")
+        layer2 = QgsVectorLayer("Point?field=fldtxt:string",
+                                "layer2", "memory")
+        project.addMapLayers([layer, layer2])
+
+        # record for layer1
+        record.addLayerRecord(QgsMapThemeCollection.MapThemeLayerRecord(layer))
+        collection.insert('theme1', record)
+        self.assertEqual(len(theme_changed_spy), 5)
+        self.assertEqual(len(themes_changed_spy), 7)
+
+        # now kill layer 2
+        project.removeMapLayer(layer2)
+        self.assertEqual(len(theme_changed_spy), 5) # signal should not be emitted - layer is not in record
+        # now kill layer 1
+        project.removeMapLayer(layer)
+        app.processEvents()
+        self.assertEqual(len(theme_changed_spy), 6) # signal should be emitted - layer is in record
+
+
+if __name__ == '__main__':
+    unittest.main()