[BUGFIX] Fix import of 3D shapefile into spatialite

Fixes #33883 - QgsOgrProvider::wkbType(): autopromote Z/M/ZM linestring/polygon to multi - QgsVectorLayerExporter::exportLayer(): remove unneeded hack for shapefile - QgsSpatiaLiteProvider::createEmptyLayer(): deal with xxxZ geometry types (refs #qgis/qgis4.0_api#107)
qgis · Jan 21, 2020 · 7938130 · 7938130
1 parent 0a9c9e6
commit 7938130
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 31 deletions.
diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp
@@ -1445,7 +1445,8 @@ size_t QgsOgrProvider::layerCount() const
 QgsWkbTypes::Type QgsOgrProvider::wkbType() const
 {
   QgsWkbTypes::Type wkb = QgsOgrUtils::ogrGeometryTypeToQgsWkbType( mOGRGeomType );
-  if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) && ( wkb == QgsWkbTypes::LineString || wkb == QgsWkbTypes::Polygon ) )
+  const QgsWkbTypes::Type wkbFlat = QgsWkbTypes::flatType( wkb );
+  if ( mGDALDriverName == QLatin1String( "ESRI Shapefile" ) && ( wkbFlat == QgsWkbTypes::LineString || wkbFlat == QgsWkbTypes::Polygon ) )
   {
     wkb = QgsWkbTypes::multiType( wkb );
   }

diff --git a/src/core/qgsvectorlayerexporter.cpp b/src/core/qgsvectorlayerexporter.cpp
@@ -278,34 +278,6 @@ QgsVectorLayerExporter::exportLayer( QgsVectorLayer *layer,
     {
       fields[fldIdx].setName( fields.at( fldIdx ).name().toLower() );
     }
-
-    // This code does not make much sense anymore except for POINT
-    // because since commit 1aa0091e7a368ded all POLYGON and LINESTRING are
-    // reported as MULTI from the OGR provider and shapefiles.
-    if ( !forceSinglePartGeom )
-    {
-      // convert wkbtype to multipart (see #5547)
-      switch ( wkbType )
-      {
-        case QgsWkbTypes::LineString:
-          wkbType = QgsWkbTypes::MultiLineString;
-          break;
-        case QgsWkbTypes::Polygon:
-          wkbType = QgsWkbTypes::MultiPolygon;
-          break;
-        case QgsWkbTypes::Point25D:
-          wkbType = QgsWkbTypes::MultiPoint25D;
-          break;
-        case QgsWkbTypes::LineString25D:
-          wkbType = QgsWkbTypes::MultiLineString25D;
-          break;
-        case QgsWkbTypes::Polygon25D:
-          wkbType = QgsWkbTypes::MultiPolygon25D;
-          break;
-        default:
-          break;
-      }
-    }
   }
 
   bool convertGeometryToSinglePart = false;

diff --git a/src/providers/spatialite/qgsspatialiteprovider.cpp b/src/providers/spatialite/qgsspatialiteprovider.cpp
@@ -255,41 +255,47 @@ QgsSpatiaLiteProvider::createEmptyLayer( const QString &uri,
       switch ( wkbType )
       {
         case QgsWkbTypes::Point25D:
+        case QgsWkbTypes::PointZ:
           dim = 3;
           FALLTHROUGH
         case QgsWkbTypes::Point:
           geometryType = QStringLiteral( "POINT" );
           break;
 
         case QgsWkbTypes::LineString25D:
+        case QgsWkbTypes::LineStringZ:
           dim = 3;
           FALLTHROUGH
         case QgsWkbTypes::LineString:
           geometryType = QStringLiteral( "LINESTRING" );
           break;
 
         case QgsWkbTypes::Polygon25D:
+        case QgsWkbTypes::PolygonZ:
           dim = 3;
           FALLTHROUGH
         case QgsWkbTypes::Polygon:
           geometryType = QStringLiteral( "POLYGON" );
           break;
 
         case QgsWkbTypes::MultiPoint25D:
+        case QgsWkbTypes::MultiPointZ:
           dim = 3;
           FALLTHROUGH
         case QgsWkbTypes::MultiPoint:
           geometryType = QStringLiteral( "MULTIPOINT" );
           break;
 
         case QgsWkbTypes::MultiLineString25D:
+        case QgsWkbTypes::MultiLineStringZ:
           dim = 3;
           FALLTHROUGH
         case QgsWkbTypes::MultiLineString:
           geometryType = QStringLiteral( "MULTILINESTRING" );
           break;
 
         case QgsWkbTypes::MultiPolygon25D:
+        case QgsWkbTypes::MultiPolygonZ:
           dim = 3;
           FALLTHROUGH
         case QgsWkbTypes::MultiPolygon:

diff --git a/tests/src/analysis/testqgsprocessing.cpp b/tests/src/analysis/testqgsprocessing.cpp
@@ -1917,7 +1917,7 @@ void TestQgsProcessing::parameters()
   layer = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( destId, context ) );
   QVERIFY( layer );
   QVERIFY( layer->isValid() );
-  QCOMPARE( layer->wkbType(), wkbType );
+  QCOMPARE( layer->wkbType(), QgsWkbTypes::MultiPolygonM ); // shapefile Polygon[XX] get promoted to Multi
   QCOMPARE( layer->crs(), crs );
 
   // make sure layer was automatically added to list to load on completion

diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
@@ -897,7 +897,7 @@ def testWriteShapefileWithSingleConversion(self):
         # Failing case: add a real multi to the shapefile and try to force to single
         self.assertTrue(shapefile_layer.startEditing())
         ft = QgsFeature()
-        ft.setGeometry(QgsGeometry.fromWkt('MultiPolygon (((0 0, 0 1, 1 1, 1 0, 0 0)), ((0 0, 0 1.5, 1 1.5, 1 0, 0 0)))'))
+        ft.setGeometry(QgsGeometry.fromWkt('MultiPolygon (((0 0, 0 1, 1 1, 1 0, 0 0)), ((-10 -10,-10 -9,-9 -9,-10 -10)))'))
         ft.setAttributes([2])
         self.assertTrue(shapefile_layer.addFeatures([ft]))
         self.assertTrue(shapefile_layer.commitChanges())
@@ -915,6 +915,53 @@ def testWriteShapefileWithSingleConversion(self):
         self.assertTrue(QgsWkbTypes.isMultiType(multi_layer.wkbType()))
         self.assertEqual(write_result, QgsVectorLayerExporter.ErrFeatureWriteFailed, "Failed to transform a feature with ID '1' to single part. Writing stopped.")
 
+    def testReadingLayerGeometryTypes(self):
+
+        tests = [(osgeo.ogr.wkbPoint, 'Point (0 0)', QgsWkbTypes.Point, 'Point (0 0)'),
+                 (osgeo.ogr.wkbPoint25D, 'Point Z (0 0 1)', QgsWkbTypes.PointZ, 'PointZ (0 0 1)'),
+                 (osgeo.ogr.wkbPointM, 'Point M (0 0 1)', QgsWkbTypes.PointM, 'PointM (0 0 1)'),
+                 (osgeo.ogr.wkbPointZM, 'Point ZM (0 0 1 2)', QgsWkbTypes.PointZM, 'PointZM (0 0 1 2)'),
+                 (osgeo.ogr.wkbLineString, 'LineString (0 0, 1 1)', QgsWkbTypes.MultiLineString, 'MultiLineString ((0 0, 1 1))'),
+                 (osgeo.ogr.wkbLineString25D, 'LineString Z (0 0 10, 1 1 10)', QgsWkbTypes.MultiLineStringZ, 'MultiLineStringZ ((0 0 10, 1 1 10))'),
+                 (osgeo.ogr.wkbLineStringM, 'LineString M (0 0 10, 1 1 10)', QgsWkbTypes.MultiLineStringM, 'MultiLineStringM ((0 0 10, 1 1 10))'),
+                 (osgeo.ogr.wkbLineStringZM, 'LineString ZM (0 0 10 20, 1 1 10 20)', QgsWkbTypes.MultiLineStringZM, 'MultiLineStringZM ((0 0 10 20, 1 1 10 20))'),
+                 (osgeo.ogr.wkbPolygon, 'Polygon ((0 0,0 1,1 1,0 0))', QgsWkbTypes.MultiPolygon, 'MultiPolygon (((0 0, 0 1, 1 1, 0 0)))'),
+                 (osgeo.ogr.wkbPolygon25D, 'Polygon Z ((0 0 10, 0 1 10, 1 1 10, 0 0 10))', QgsWkbTypes.MultiPolygonZ, 'MultiPolygonZ (((0 0 10, 0 1 10, 1 1 10, 0 0 10)))'),
+                 (osgeo.ogr.wkbPolygonM, 'Polygon M ((0 0 10, 0 1 10, 1 1 10, 0 0 10))', QgsWkbTypes.MultiPolygonM, 'MultiPolygonM (((0 0 10, 0 1 10, 1 1 10, 0 0 10)))'),
+                 (osgeo.ogr.wkbPolygonZM, 'Polygon ZM ((0 0 10 20, 0 1 10 20, 1 1 10 20, 0 0 10 20))', QgsWkbTypes.MultiPolygonZM, 'MultiPolygonZM (((0 0 10 20, 0 1 10 20, 1 1 10 20, 0 0 10 20)))'),
+                 (osgeo.ogr.wkbMultiPoint, 'MultiPoint (0 0,1 1)', QgsWkbTypes.MultiPoint, 'MultiPoint ((0 0),(1 1))'),
+                 (osgeo.ogr.wkbMultiPoint25D, 'MultiPoint Z ((0 0 10), (1 1 10))', QgsWkbTypes.MultiPointZ, 'MultiPointZ ((0 0 10),(1 1 10))'),
+                 (osgeo.ogr.wkbMultiPointM, 'MultiPoint M ((0 0 10), (1 1 10))', QgsWkbTypes.MultiPointM, 'MultiPointM ((0 0 10),(1 1 10))'),
+                 (osgeo.ogr.wkbMultiPointZM, 'MultiPoint ZM ((0 0 10 20), (1 1 10 20))', QgsWkbTypes.MultiPointZM, 'MultiPointZM ((0 0 10 20),(1 1 10 20))'),
+                 (osgeo.ogr.wkbMultiLineString, 'MultiLineString ((0 0, 1 1))', QgsWkbTypes.MultiLineString, 'MultiLineString ((0 0, 1 1))'),
+                 (osgeo.ogr.wkbMultiLineString25D, 'MultiLineString Z ((0 0 10, 1 1 10))', QgsWkbTypes.MultiLineStringZ, 'MultiLineStringZ ((0 0 10, 1 1 10))'),
+                 (osgeo.ogr.wkbMultiLineStringM, 'MultiLineString M ((0 0 10, 1 1 10))', QgsWkbTypes.MultiLineStringM, 'MultiLineStringM ((0 0 10, 1 1 10))'),
+                 (osgeo.ogr.wkbMultiLineStringZM, 'MultiLineString ZM ((0 0 10 20, 1 1 10 20))', QgsWkbTypes.MultiLineStringZM, 'MultiLineStringZM ((0 0 10 20, 1 1 10 20))'),
+                 (osgeo.ogr.wkbMultiPolygon, 'MultiPolygon (((0 0,0 1,1 1,0 0)))', QgsWkbTypes.MultiPolygon, 'MultiPolygon (((0 0, 0 1, 1 1, 0 0)))'),
+                 (osgeo.ogr.wkbMultiPolygon25D, 'MultiPolygon Z (((0 0 10, 0 1 10, 1 1 10, 0 0 10)))', QgsWkbTypes.MultiPolygonZ, 'MultiPolygonZ (((0 0 10, 0 1 10, 1 1 10, 0 0 10)))'),
+                 (osgeo.ogr.wkbMultiPolygonM, 'MultiPolygon M (((0 0 10, 0 1 10, 1 1 10, 0 0 10)))', QgsWkbTypes.MultiPolygonM, 'MultiPolygonM (((0 0 10, 0 1 10, 1 1 10, 0 0 10)))'),
+                 (osgeo.ogr.wkbMultiPolygonZM, 'MultiPolygon ZM (((0 0 10 20, 0 1 10 20, 1 1 10 20, 0 0 10 20)))', QgsWkbTypes.MultiPolygonZM, 'MultiPolygonZM (((0 0 10 20, 0 1 10 20, 1 1 10 20, 0 0 10 20)))'),
+                 ]
+        for ogr_type, wkt, qgis_type, expected_wkt in tests:
+
+            filename = 'testPromoteToMulti'
+            tmpfile = os.path.join(self.basetestpath, filename)
+            ds = osgeo.ogr.GetDriverByName('ESRI Shapefile').CreateDataSource(tmpfile)
+            lyr = ds.CreateLayer(filename, geom_type=ogr_type)
+            f = osgeo.ogr.Feature(lyr.GetLayerDefn())
+            f.SetGeometry(osgeo.ogr.CreateGeometryFromWkt(wkt))
+            lyr.CreateFeature(f)
+            ds = None
+
+            vl = QgsVectorLayer(tmpfile, 'test', 'ogr')
+            self.assertTrue(vl.isValid())
+            self.assertEqual(vl.wkbType(), qgis_type)
+            f = next(vl.getFeatures())
+            self.assertEqual(f.geometry().constGet().asWkt(), expected_wkt)
+            del vl
+
+            osgeo.ogr.GetDriverByName('ESRI Shapefile').DeleteDataSource(tmpfile)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/src/python/test_provider_spatialite.py b/tests/src/python/test_provider_spatialite.py
@@ -31,6 +31,7 @@
                        QgsDefaultValue,
                        QgsFeatureRequest,
                        QgsRectangle,
+                       QgsVectorLayerExporter,
                        QgsWkbTypes)
 
 from qgis.testing import start_app, unittest
@@ -1066,6 +1067,68 @@ def _test_db(testPath):
         testPath = "dbname=%s table='test_pg'" % dbname
         _test_db(testPath)
 
+    def testGeometryTypes(self):
+        """Test creating db with various geometry types"""
+
+        # create test db
+        dbname = os.path.join(tempfile.gettempdir(), "testGeometryTypes.sqlite")
+        if os.path.exists(dbname):
+            os.remove(dbname)
+        con = spatialite_connect(dbname, isolation_level=None)
+        cur = con.cursor()
+        cur.execute("BEGIN")
+        sql = "SELECT InitSpatialMetadata()"
+        cur.execute(sql)
+
+        cur.execute("COMMIT")
+        con.close()
+
+        tests = [('Point', 'Point (0 0)', QgsWkbTypes.Point),
+                 ('PointZ', 'PointZ (0 0 10)', QgsWkbTypes.PointZ),
+                 ('Point25D', 'PointZ (0 0 10)', QgsWkbTypes.PointZ),
+                 ('MultiPoint', 'MultiPoint (0 0, 0 1)', QgsWkbTypes.MultiPoint),
+                 ('MultiPointZ', 'MultiPointZ ((0 0 10, 0 1 10))', QgsWkbTypes.MultiPointZ),
+                 ('MultiPoint25D', 'MultiPointZ ((0 0 10, 0 1 10))', QgsWkbTypes.MultiPointZ),
+                 ('LineString', 'LineString (0 0, 0 1)', QgsWkbTypes.LineString),
+                 ('LineStringZ', 'LineStringZ (0 0 10, 0 1 10)', QgsWkbTypes.LineStringZ),
+                 ('LineString25D', 'LineStringZ (0 0 10, 0 1 10)', QgsWkbTypes.LineStringZ),
+                 ('MultiLineString', 'MultiLineString (0 0, 0 1)', QgsWkbTypes.MultiLineString),
+                 ('MultiLineStringZ', 'MultiLineStringZ ((0 0 10, 0 1 10))', QgsWkbTypes.MultiLineStringZ),
+                 ('MultiLineString25D', 'MultiLineStringZ ((0 0 10, 0 1 10))', QgsWkbTypes.MultiLineStringZ),
+                 ('Polygon', 'Polygon ((0 0, 0 1, 1 1, 1 0, 0 0))', QgsWkbTypes.Polygon),
+                 ('PolygonZ', 'PolygonZ ((0 0 10, 0 1 10, 1 1 10, 1 0 10, 0 0 10))', QgsWkbTypes.PolygonZ),
+                 ('Polygon25D', 'PolygonZ ((0 0 10, 0 1 10, 1 1 10, 1 0 10, 0 0 10))', QgsWkbTypes.PolygonZ),
+                 ('MultiPolygon', 'MultiPolygon (((0 0, 0 1, 1 1, 1 0, 0 0)))', QgsWkbTypes.MultiPolygon),
+                 ('MultiPolygonZ', 'MultiPolygonZ (((0 0 10, 0 1 10, 1 1 10, 1 0 10, 0 0 10)))', QgsWkbTypes.MultiPolygonZ),
+                 ('MultiPolygon25D', 'MultiPolygonZ (((0 0 10, 0 1 10, 1 1 10, 1 0 10, 0 0 10)))', QgsWkbTypes.MultiPolygonZ)
+                 ]
+        for typeStr, wkt, qgisType in tests:
+
+            ml = QgsVectorLayer(
+                (typeStr + '?crs=epsg:4326&field=id:int'),
+                typeStr,
+                'memory')
+
+            provider = ml.dataProvider()
+            ft = QgsFeature()
+            ft.setGeometry(QgsGeometry.fromWkt(wkt))
+            res, features = provider.addFeatures([ft])
+
+            layer = typeStr
+            uri = "dbname=%s table='%s' (geometry)" % (dbname, layer)
+            write_result, error_message = QgsVectorLayerExporter.exportLayer(ml,
+                                                                             uri,
+                                                                             'spatialite',
+                                                                             ml.crs(),
+                                                                             False,
+                                                                             {},
+                                                                             )
+            self.assertEqual(write_result, QgsVectorLayerExporter.NoError, error_message)
+
+            vl = QgsVectorLayer(uri, typeStr, 'spatialite')
+            self.assertTrue(vl.isValid())
+            self.assertEqual(vl.wkbType(), qgisType)
+
 
 if __name__ == '__main__':
     unittest.main()