use QgsXmlUtils::read/writeVariant to save QgsObjectCustomProperties (#43099)

3nids · web-flow · commit 05f55f08d991 · 2021-05-11T08:28:35.000+02:00
* use QgsXmlUtils to save QgsObjectCustomProperties

This allows to have more complex properties such as maps

* add test

* code layout
diff --git a/src/core/qgsobjectcustomproperties.cpp b/src/core/qgsobjectcustomproperties.cpp
@@ -17,6 +17,7 @@
 
 #include "qgsobjectcustomproperties.h"
 #include "qgis.h"
+#include "qgsxmlutils.h"
 
 #include <QDomNode>
 #include <QStringList>
@@ -77,39 +78,53 @@ void QgsObjectCustomProperties::readXml( const QDomNode &parentNode, const QStri
     mMap.clear();
   }
 
-  QDomNodeList nodes = propsNode.childNodes();
-
-  for ( int i = 0; i < nodes.size(); i++ )
+  const QVariant newProps = QgsXmlUtils::readVariant( propsNode.firstChildElement() );
+  if ( newProps.type() == QVariant::Map )
+  {
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+    const QVariantMap propsMap = newProps.toMap();
+    for ( auto it = propsMap.constBegin(); it != propsMap.constEnd(); ++it )
+      mMap.insert( it.key(), it.value() );
+#else
+    mMap.insert( newProps.toMap() );
+#endif
+  }
+  else
   {
-    QDomNode propNode = nodes.at( i );
-    if ( propNode.isNull() || propNode.nodeName() != QLatin1String( "property" ) )
-      continue;
-    QDomElement propElement = propNode.toElement();
+    // backward compatibility code for QGIS < 3.20
+    QDomNodeList nodes = propsNode.childNodes();
 
-    QString key = propElement.attribute( QStringLiteral( "key" ) );
-    if ( key.isEmpty() || key.startsWith( keyStartsWith ) )
+    for ( int i = 0; i < nodes.size(); i++ )
     {
-      if ( propElement.hasAttribute( QStringLiteral( "value" ) ) )
-      {
-        QString value = propElement.attribute( QStringLiteral( "value" ) );
-        mMap[key] = QVariant( value );
-      }
-      else
-      {
-        QStringList list;
+      QDomNode propNode = nodes.at( i );
+      if ( propNode.isNull() || propNode.nodeName() != QLatin1String( "property" ) )
+        continue;
+      QDomElement propElement = propNode.toElement();
 
-        for ( QDomElement itemElement = propElement.firstChildElement( QStringLiteral( "value" ) );
-              !itemElement.isNull();
-              itemElement = itemElement.nextSiblingElement( QStringLiteral( "value" ) ) )
+      QString key = propElement.attribute( QStringLiteral( "key" ) );
+      if ( key.isEmpty() || key.startsWith( keyStartsWith ) )
+      {
+        if ( propElement.hasAttribute( QStringLiteral( "value" ) ) )
         {
-          list << itemElement.text();
+          QString value = propElement.attribute( QStringLiteral( "value" ) );
+          mMap[key] = QVariant( value );
         }
+        else
+        {
+          QStringList list;
 
-        mMap[key] = QVariant( list );
+          for ( QDomElement itemElement = propElement.firstChildElement( QStringLiteral( "value" ) );
+                !itemElement.isNull();
+                itemElement = itemElement.nextSiblingElement( QStringLiteral( "value" ) ) )
+          {
+            list << itemElement.text();
+          }
+
+          mMap[key] = QVariant( list );
+        }
       }
     }
   }
-
 }
 
 void QgsObjectCustomProperties::writeXml( QDomNode &parentNode, QDomDocument &doc ) const
@@ -122,32 +137,7 @@ void QgsObjectCustomProperties::writeXml( QDomNode &parentNode, QDomDocument &do
   }
 
   QDomElement propsElement = doc.createElement( QStringLiteral( "customproperties" ) );
-
-  auto keys = mMap.keys();
-
-  std::sort( keys.begin(), keys.end() );
-
-  for ( const auto &key : std::as_const( keys ) )
-  {
-    QDomElement propElement = doc.createElement( QStringLiteral( "property" ) );
-    propElement.setAttribute( QStringLiteral( "key" ), key );
-    const QVariant value = mMap.value( key );
-    if ( value.canConvert<QString>() )
-    {
-      propElement.setAttribute( QStringLiteral( "value" ), value.toString() );
-    }
-    else if ( value.canConvert<QStringList>() )
-    {
-      const auto constToStringList = value.toStringList();
-      for ( const QString &valueStr : constToStringList )
-      {
-        QDomElement itemElement = doc.createElement( QStringLiteral( "value" ) );
-        itemElement.appendChild( doc.createTextNode( valueStr ) );
-        propElement.appendChild( itemElement );
-      }
-    }
-    propsElement.appendChild( propElement );
-  }
+  propsElement.appendChild( QgsXmlUtils::writeVariant( mMap, doc ) );
 
   parentNode.appendChild( propsElement );
 }
diff --git a/tests/src/python/test_qgsobjectcustomproperties.py b/tests/src/python/test_qgsobjectcustomproperties.py
@@ -79,6 +79,21 @@ def testSaveRestore(self):
         self.assertEqual(props2.value('a', defaultValue=6), '7')
         self.assertEqual(props2.value('b', defaultValue='yy'), 'xx')
 
+    def testCompatibilityRestore(self):
+        # for pre 3.20
+        legacy_xml = '<test>\n <customproperties>\n  <property key="a" value="7"/>\n  <property key="b" value="xx"/>\n </customproperties>\n</test>\n'
+        doc = QDomDocument()
+        doc.setContent(legacy_xml)
+
+        props = QgsObjectCustomProperties()
+        props.readXml(doc.documentElement())
+
+        self.assertCountEqual(props.keys(), ['a', 'b'])
+        self.assertTrue(props.contains('a'))
+        self.assertTrue(props.contains('b'))
+        self.assertEqual(props.value('a', defaultValue=6), '7')
+        self.assertEqual(props.value('b', defaultValue='yy'), 'xx')
+
 
 if __name__ == '__main__':
     unittest.main()
