MemoryProvider fix strange fieldName suffix in cloned layer

If fields added with QgsMemoryProvider::addAttributes have an empty typeName, the URI returned by vl.publicSource() misses the type information for fields.
qgis · Jul 24, 2021 · 0191af7 · 0191af7
1 parent 2755615
commit 0191af7
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 38 deletions.
diff --git a/.flake8 b/.flake8
@@ -8,8 +8,6 @@ ignore =
 	E501,
 	# do not use variables named ‘l’, ‘O’, or ‘I’
 	E741,
-	# redefinition of unused name from line N
-	F811,
 	# local variable name is assigned to but never used
 	F841,
 	# class names should use CapWords convention

diff --git a/src/core/providers/memory/qgsmemoryprovider.cpp b/src/core/providers/memory/qgsmemoryprovider.cpp
@@ -104,6 +104,7 @@ QgsMemoryProvider::QgsMemoryProvider( const QString &uri, const ProviderOptions
 
                   // boolean
                   << QgsVectorDataProvider::NativeType( tr( "Boolean" ), QStringLiteral( "bool" ), QVariant::Bool )
+                  << QgsVectorDataProvider::NativeType( tr( "Boolean" ), QStringLiteral( "boolean" ), QVariant::Bool )
 
                   // blob
                   << QgsVectorDataProvider::NativeType( tr( "Binary object (BLOB)" ), QStringLiteral( "binary" ), QVariant::ByteArray )
@@ -556,28 +557,39 @@ bool QgsMemoryProvider::deleteFeatures( const QgsFeatureIds &id )
 
 bool QgsMemoryProvider::addAttributes( const QList<QgsField> &attributes )
 {
-  for ( QList<QgsField>::const_iterator it = attributes.begin(); it != attributes.end(); ++it )
+  for ( QgsField field : attributes )
   {
-    switch ( it->type() )
+    if ( !supportedType( field ) )
+      continue;
+
+    // Make sure added attributes typeName correspond to a native type name
+    bool isNativeTypeName = false;
+    NativeType nativeTypeCandidate( QString(), QString(), QVariant::Invalid );
+    for ( const NativeType &nativeType : nativeTypes() )
     {
-      case QVariant::Int:
-      case QVariant::Double:
-      case QVariant::String:
-      case QVariant::Date:
-      case QVariant::Time:
-      case QVariant::DateTime:
-      case QVariant::LongLong:
-      case QVariant::StringList:
-      case QVariant::List:
-      case QVariant::Bool:
-      case QVariant::ByteArray:
+      if ( nativeType.mTypeName.toLower() == field.typeName().toLower() )
+      {
+        isNativeTypeName = true;
         break;
-      default:
-        QgsDebugMsg( "Field type not supported: " + it->typeName() );
+      }
+
+      if ( nativeType.mType == field.type()
+           && nativeTypeCandidate.mType == QVariant::Invalid )
+        nativeTypeCandidate = nativeType;
+    }
+    if ( !isNativeTypeName )
+    {
+      if ( nativeTypeCandidate.mType == QVariant::Invalid )
+      {
+        QgsDebugMsg( "Field type not supported: " + field.typeName() );
         continue;
+      }
+
+      field.setTypeName( nativeTypeCandidate.mTypeName );
     }
+
     // add new field as a last one
-    mFields.append( *it );
+    mFields.append( field );
 
     for ( QgsFeatureMap::iterator fit = mFeatures.begin(); fit != mFeatures.end(); ++fit )
     {

diff --git a/tests/src/python/test_provider_memory.py b/tests/src/python/test_provider_memory.py
@@ -238,7 +238,7 @@ def testAddFeatures(self):
 
             assert compareWkt(str(geom.asWkt()), "Point (10 10)"), myMessage
 
-    def testClone(self):
+    def testCloneFeatures(self):
         """
         Test that cloning a memory layer also clones features
         """
@@ -263,6 +263,25 @@ def testClone(self):
         self.assertTrue([f for f in features if f['f1'] == 3])
         self.assertTrue([f for f in features if f['f1'] == 1])
 
+    def testCloneId(self):
+        """Test that a cloned layer has a single new id and
+        the same fields as the source layer"""
+
+        vl = QgsVectorLayer(
+            'Point?crs=epsg:4326',
+            'test', 'memory')
+        self.assertTrue(vl.isValid)
+        dp = vl.dataProvider()
+        self.assertTrue(dp.addAttributes([QgsField("name", QVariant.String),
+                                          QgsField("age", QVariant.Int),
+                                          QgsField("size", QVariant.Double)]))
+        vl2 = vl.clone()
+        self.assertTrue(
+            'memory?geometry=Point&crs=EPSG:4326&field=name:string(0,0)&field=age:integer(0,0)&field=size:double(0,0)' in vl2.publicSource())
+        self.assertEqual(len(parse_qs(vl.publicSource())['uid']), 1)
+        self.assertEqual(len(parse_qs(vl2.publicSource())['uid']), 1)
+        self.assertNotEqual(parse_qs(vl2.publicSource())['uid'][0], parse_qs(vl.publicSource())['uid'][0])
+
     def testGetFields(self):
         layer = QgsVectorLayer("Point", "test", "memory")
         provider = layer.dataProvider()
@@ -784,25 +803,6 @@ def testSpatialIndex(self):
         vl.dataProvider().createSpatialIndex()
         self.assertEqual(vl.hasSpatialIndex(), QgsFeatureSource.SpatialIndexPresent)
 
-    def testClone(self):
-        """Test that a cloned layer has a single new id and
-        the same fields as the source layer"""
-
-        vl = QgsVectorLayer(
-            'Point?crs=epsg:4326',
-            'test', 'memory')
-        self.assertTrue(vl.isValid)
-        dp = vl.dataProvider()
-        self.assertTrue(dp.addAttributes([QgsField("name", QVariant.String),
-                                          QgsField("age", QVariant.Int),
-                                          QgsField("size", QVariant.Double)]))
-        vl2 = vl.clone()
-        self.assertTrue(
-            'memory?geometry=Point&crs=EPSG:4326&field=name:(0,0)&field=age:(0,0)&field=size:(0,0)' in vl2.publicSource())
-        self.assertEqual(len(parse_qs(vl.publicSource())['uid']), 1)
-        self.assertEqual(len(parse_qs(vl2.publicSource())['uid']), 1)
-        self.assertNotEqual(parse_qs(vl2.publicSource())['uid'][0], parse_qs(vl.publicSource())['uid'][0])
-
     def testTypeValidation(self):
         """Test that incompatible types in attributes raise errors"""
 
@@ -915,6 +915,21 @@ def testTypeValidation(self):
         f = vl.getFeature(1)
         self.assertEqual(f.attribute('int'), 123)
 
+    def testAddAttributes(self):
+        """Test that fields with empty/invalid typenames are updated to native type names"""
+
+        vl = QgsVectorLayer("Point", "temporary_points", "memory")
+        pr = vl.dataProvider()
+
+        # add fields
+        pr.addAttributes([QgsField("name", QVariant.String),
+                          QgsField("age", QVariant.Int, "invalidInteger"),
+                          QgsField("size", QVariant.Double)])
+
+        self.assertEqual(pr.fields()[0].typeName(), "string")
+        self.assertEqual(pr.fields()[1].typeName(), "integer")
+        self.assertEqual(pr.fields()[2].typeName(), "double")
+
 
 class TestPyQgsMemoryProviderIndexed(unittest.TestCase, ProviderTestCase):
     """Runs the provider test suite against an indexed memory layer"""