Store vector layer wkb type in xml

nyalldawson · nyalldawson · commit 6bd5d079d62b · 2019-04-18T06:45:12.000Z
We can use this when restoring the layer, if the uri turns out
to be invalid at that stage (e.g. a file has moved). By storing
and falling back to the last known wkb type, we avoid unnecessarily
discarding the existing layer renderer, and can still show the
expected layer type in the layer tree.
diff --git a/python/core/auto_additions/qgswkbtypes.py b/python/core/auto_additions/qgswkbtypes.py
@@ -1,2 +1,3 @@
 # The following has been generated automatically from src/core/geometry/qgswkbtypes.h
+QgsWkbTypes.Type.baseClass = QgsWkbTypes
 QgsWkbTypes.GeometryType.baseClass = QgsWkbTypes
diff --git a/src/core/geometry/qgswkbtypes.h b/src/core/geometry/qgswkbtypes.h
@@ -128,6 +128,7 @@ class CORE_EXPORT QgsWkbTypes
       MultiLineString25D,
       MultiPolygon25D
     };
+    Q_ENUM( Type )
 
     /**
      * The geometry types are used to group QgsWkbTypes::Type in a
diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -1803,8 +1803,9 @@ bool QgsProject::writeProjectFile( const QString &filename )
       if ( emIt == mEmbeddedLayers.constEnd() )
       {
         QDomElement maplayerElem;
-        // If layer is not valid, let's try to restore saved properties from invalidLayerProperties
-        if ( ml->isValid() )
+        // If layer is not valid, prefer to restore saved properties from invalidLayerProperties. But if that's
+        // not available, just write what we DO have
+        if ( ml->isValid() || ml->originalXmlProperties().isEmpty() )
         {
           // general layer metadata
           maplayerElem = doc->createElement( QStringLiteral( "maplayer" ) );
diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -1405,6 +1405,11 @@ bool QgsVectorLayer::readXml( const QDomNode &layer_node, QgsReadWriteContext &c
   if ( !setDataProvider( mProviderKey, options ) )
   {
     QgsDebugMsg( QStringLiteral( "Could not set data provider for layer %1" ).arg( publicSource() ) );
+    const QDomElement elem = layer_node.toElement();
+
+    // for invalid layer sources, we fallback to stored wkbType if available
+    if ( elem.hasAttribute( QStringLiteral( "wkbType" ) ) )
+      mWkbType = qgsEnumKeyToValue( elem.attribute( QStringLiteral( "wkbType" ) ), mWkbType );
   }
 
   QDomElement pkeyElem = pkeyNode.toElement();
@@ -1691,6 +1696,7 @@ bool QgsVectorLayer::writeXml( QDomNode &layer_node,
 
   // set the geometry type
   mapLayerNode.setAttribute( QStringLiteral( "geometry" ), QgsWkbTypes::geometryDisplayString( geometryType() ) );
+  mapLayerNode.setAttribute( QStringLiteral( "wkbType" ), qgsEnumValueToKey( wkbType() ) );
 
   // add provider node
   if ( mDataProvider )
diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py
@@ -15,9 +15,11 @@
 import qgis  # NOQA
 
 import os
+import tempfile
+import shutil
 
 from qgis.PyQt.QtCore import QVariant, Qt
-from qgis.PyQt.QtGui import QPainter
+from qgis.PyQt.QtGui import QPainter, QColor
 from qgis.PyQt.QtXml import QDomDocument
 
 from qgis.core import (QgsWkbTypes,
@@ -392,6 +394,45 @@ def testSetDataSourceInvalidToValid(self):
         # should STILL have kept renderer!
         self.assertEqual(layer.renderer(), r)
 
+    def testStoreWkbTypeInvalidLayers(self):
+        """
+        Test that layer wkb types are restored for projects with invalid layer paths
+        """
+        layer = createLayerWithOnePoint()
+        layer.setName('my test layer')
+        r = QgsSingleSymbolRenderer(QgsSymbol.defaultSymbol(QgsWkbTypes.PointGeometry))
+        r.symbol().setColor(QColor('#123456'))
+        layer.setRenderer(r)
+        self.assertEqual(layer.renderer().symbol().color().name(), '#123456')
+
+        p = QgsProject()
+        p.addMapLayer(layer)
+
+        # reset layer to a bad path
+        options = QgsDataProvider.ProviderOptions()
+        layer.setDataSource('nothing', 'new name', 'ogr', options)
+        # should have kept the same renderer and wkb type!
+        self.assertEqual(layer.wkbType(), QgsWkbTypes.Point)
+        self.assertEqual(layer.renderer().symbol().color().name(), '#123456')
+
+        # save project to a temporary file
+        temp_path = tempfile.mkdtemp()
+        temp_project_path = os.path.join(temp_path, 'temp.qgs')
+        self.assertTrue(p.write(temp_project_path))
+
+        # restore project
+        p2 = QgsProject()
+        self.assertTrue(p2.read(temp_project_path))
+
+        l2 = p2.mapLayersByName('new name')[0]
+        self.assertFalse(l2.isValid())
+
+        # should have kept the same renderer and wkb type!
+        self.assertEqual(l2.wkbType(), QgsWkbTypes.Point)
+        self.assertEqual(l2.renderer().symbol().color().name(), '#123456')
+
+        shutil.rmtree(temp_path, True)
+
     def test_layer_crs(self):
         """
         Test that spatial layers have CRS, and non-spatial don't

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`# The following has been generated automatically from src/core/geometry/qgswkbtypes.h`
	`2`	`+QgsWkbTypes.Type.baseClass = QgsWkbTypes`
`2`	`3`	`QgsWkbTypes.GeometryType.baseClass = QgsWkbTypes`
Original file line number	Diff line number	Diff line change
`@@ -1803,8 +1803,9 @@ bool QgsProject::writeProjectFile( const QString &filename )`
`1803`	`1803`	`if ( emIt == mEmbeddedLayers.constEnd() )`
`1804`	`1804`	`{`
`1805`	`1805`	`QDomElement maplayerElem;`
`1806`		`- // If layer is not valid, let's try to restore saved properties from invalidLayerProperties`
`1807`		`- if ( ml->isValid() )`
	`1806`	`+ // If layer is not valid, prefer to restore saved properties from invalidLayerProperties. But if that's`
	`1807`	`+ // not available, just write what we DO have`
	`1808`	`+ if ( ml->isValid() \|\| ml->originalXmlProperties().isEmpty() )`
`1808`	`1809`	`{`
`1809`	`1810`	`// general layer metadata`
`1810`	`1811`	`maplayerElem = doc->createElement( QStringLiteral( "maplayer" ) );`