Fix label masking settings get dropped for layers with "," or ";"

characters in the text Fixes #37473
qgis · May 24, 2021 · 3d4e2f4 · 3d4e2f4
1 parent 5ded8b6
commit 3d4e2f4
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 12 deletions.
diff --git a/src/core/symbology/qgssymbollayerreference.cpp b/src/core/symbology/qgssymbollayerreference.cpp
@@ -15,17 +15,23 @@
 
 #include "qgssymbollayerreference.h"
 #include "qgis.h"
+#include <QRegularExpression>
 
 QString symbolLayerReferenceListToString( const QgsSymbolLayerReferenceList &lst )
 {
   QStringList slst;
+  slst.reserve( lst.size() );
   for ( const QgsSymbolLayerReference &ref : lst )
   {
     QStringList indexPathStr;
-    for ( int index : ref.symbolLayerId().symbolLayerIndexPath() )
+    const QVector<int> indexPath = ref.symbolLayerId().symbolLayerIndexPath();
+    indexPathStr.reserve( indexPath.size() );
+    for ( int index : indexPath )
     {
       indexPathStr.append( QString::number( index ) );
     }
+    // this is BAD BAD BAD -- it assumes that the component parts eg the symbolKey has no commas!
+    // a more unique string should have been used as a concatenator here, but it's too late to fix that without breaking projects...
     slst.append( QStringLiteral( "%1,%2,%3" ).arg( ref.layerId(), ref.symbolLayerId().symbolKey(), indexPathStr.join( ',' ) ) );
   }
   return slst.join( ';' );
@@ -34,21 +40,37 @@ QString symbolLayerReferenceListToString( const QgsSymbolLayerReferenceList &lst
 QgsSymbolLayerReferenceList stringToSymbolLayerReferenceList( const QString &str )
 {
   QgsSymbolLayerReferenceList lst;
-  QStringList slst;
-  const QStringList split = str.split( ';' );
-  for ( QString tuple : split )
+
+  // when saving we used ; as a concatenator... but that was silly, cos maybe the symbol keys contain this string!
+  // try to handle this gracefully via regex...
+  const QRegularExpression partsRx( QStringLiteral( "((?:.*?),(?:.*?),(?:(?:\\d+,)+)?(?:\\d+);)" ) );
+  QRegularExpressionMatchIterator partsIt = partsRx.globalMatch( str + ';' );
+
+  while ( partsIt.hasNext() )
   {
+    QRegularExpressionMatch partMatch = partsIt.next();
+    const QString tuple = partMatch.captured( 1 );
+
     // We should have "layer_id,symbol_key,symbol_layer_index0,symbol_layer_index1,..."
-    QStringList elements = tuple.split( ',' );
-    if ( elements.size() >= 3 )
+    // EXCEPT that the symbol_key CAN have commas, so this whole logic is extremely broken.
+    // Let's see if a messy regex can save the day!
+    const QRegularExpression rx( QStringLiteral( "(.*?),(.*?),((?:\\d+,)+)?(\\d+)" ) );
+
+    const QRegularExpressionMatch match = rx.match( tuple );
+    if ( !match.hasMatch() )
+      continue;
+
+    const QString layerId = match.captured( 1 );
+    const QString symbolKey = match.captured( 2 );
+    const QStringList indices = QString( match.captured( 3 ) + match.captured( 4 ) ).split( ',' );
+
+    QVector<int> indexPath;
+    indexPath.reserve( indices.size() );
+    for ( const QString &index : indices )
     {
-      QVector<int> indexPath;
-      for ( int i = 2; i < elements.size(); i++ )
-      {
-        indexPath.append( elements[i].toInt() );
-      }
-      lst.append( QgsSymbolLayerReference( elements[0], QgsSymbolLayerId( elements[1], indexPath ) ) );
+      indexPath.append( index.toInt() );
     }
+    lst.append( QgsSymbolLayerReference( layerId, QgsSymbolLayerId( symbolKey, indexPath ) ) );
   }
   return lst;
 }
diff --git a/tests/src/python/test_selective_masking.py b/tests/src/python/test_selective_masking.py
@@ -179,6 +179,62 @@ def check_renderings(self, map_settings, control_name):
 
                 print("=== Rendering took {}s".format(float(t) / 1000.0))
 
+    def test_save_restore_references(self):
+        """
+        Test saving and restoring symbol layer references
+        """
+
+        # simple ids
+        mask_layer = QgsMaskMarkerSymbolLayer()
+        mask_layer.setMasks([
+            QgsSymbolLayerReference(self.lines_layer.id(), QgsSymbolLayerId("", 0)),
+            QgsSymbolLayerReference(self.lines_layer2.id(), QgsSymbolLayerId("some_id", [1, 3, 5, 19])),
+            QgsSymbolLayerReference(self.polys_layer.id(), QgsSymbolLayerId("some_other_id", [4, 5])),
+        ])
+
+        props = mask_layer.properties()
+
+        mask_layer2 = QgsMaskMarkerSymbolLayer.create(props)
+        self.assertEqual(mask_layer2.masks(), [
+            QgsSymbolLayerReference(self.lines_layer.id(), QgsSymbolLayerId("", 0)),
+            QgsSymbolLayerReference(self.lines_layer2.id(), QgsSymbolLayerId("some_id", [1, 3, 5, 19])),
+            QgsSymbolLayerReference(self.polys_layer.id(), QgsSymbolLayerId("some_other_id", [4, 5])),
+        ])
+
+        # complex ids
+        mask_layer = QgsMaskMarkerSymbolLayer()
+        mask_layer.setMasks([
+            QgsSymbolLayerReference(self.lines_layer.id(), QgsSymbolLayerId("", 0)),
+            QgsSymbolLayerReference(self.lines_layer2.id(), QgsSymbolLayerId("some id, #1", [1, 3, 5, 19])),
+            QgsSymbolLayerReference(self.polys_layer.id(), QgsSymbolLayerId("some other id, like, this", [4, 5])),
+        ])
+
+        props = mask_layer.properties()
+
+        mask_layer2 = QgsMaskMarkerSymbolLayer.create(props)
+        self.assertEqual(mask_layer2.masks(), [
+            QgsSymbolLayerReference(self.lines_layer.id(), QgsSymbolLayerId("", 0)),
+            QgsSymbolLayerReference(self.lines_layer2.id(), QgsSymbolLayerId("some id, #1", [1, 3, 5, 19])),
+            QgsSymbolLayerReference(self.polys_layer.id(), QgsSymbolLayerId("some other id, like, this", [4, 5])),
+        ])
+
+        # complex ids, v2
+        mask_layer = QgsMaskMarkerSymbolLayer()
+        mask_layer.setMasks([
+            QgsSymbolLayerReference(self.lines_layer.id(), QgsSymbolLayerId("a string; with bits", 0)),
+            QgsSymbolLayerReference(self.lines_layer2.id(), QgsSymbolLayerId("some; id, #1", [1, 3, 5, 19])),
+            QgsSymbolLayerReference(self.polys_layer.id(), QgsSymbolLayerId("some other; id, lik;e, this", [4, 5])),
+        ])
+
+        props = mask_layer.properties()
+
+        mask_layer2 = QgsMaskMarkerSymbolLayer.create(props)
+        self.assertEqual(mask_layer2.masks(), [
+            QgsSymbolLayerReference(self.lines_layer.id(), QgsSymbolLayerId("a string; with bits", 0)),
+            QgsSymbolLayerReference(self.lines_layer2.id(), QgsSymbolLayerId("some; id, #1", [1, 3, 5, 19])),
+            QgsSymbolLayerReference(self.polys_layer.id(), QgsSymbolLayerId("some other; id, lik;e, this", [4, 5])),
+        ])
+
     def test_label_mask(self):
         # modify labeling settings
         label_settings = self.polys_layer.labeling().settings()