Merge pull request #7674 from elpaso/bugfix-19611-ogr-overwrite-gpkg

[bugfix] Vector file writer: also check for layername before giving up
qgis · Aug 22, 2018 · 6fba9b0 · 6fba9b0
2 parents 66dd676 + fbfb0bc
commit 6fba9b0
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 8 deletions.
diff --git a/src/core/qgsvectorfilewriter.cpp b/src/core/qgsvectorfilewriter.cpp
@@ -33,6 +33,7 @@
 #include "qgsexception.h"
 #include "qgssettings.h"
 #include "qgsgeometryengine.h"
+#include "qgsproviderregistry.h"
 
 #include <QFile>
 #include <QFileInfo>
@@ -2422,6 +2423,7 @@ QgsVectorFileWriter::WriterError QgsVectorFileWriter::prepareWriteAsVectorFormat
     details.dataSourceUri = layer->dataProvider()->dataSourceUri();
   details.storageType = layer->storageType();
   details.selectedFeatureIds = layer->selectedFeatureIds();
+  details.providerUriParams = QgsProviderRegistry::instance()->decodeUri( layer->providerType(), layer->dataProvider()->dataSourceUri() );
 
   if ( details.storageType == QLatin1String( "ESRI Shapefile" ) )
   {
@@ -2550,16 +2552,23 @@ QgsVectorFileWriter::WriterError QgsVectorFileWriter::writeAsVectorFormat( Prepa
   int lastProgressReport = 0;
   long total = details.featureCount;
 
+  // Special rules for OGR layers
   if ( details.providerType == QLatin1String( "ogr" ) && !details.dataSourceUri.isEmpty() )
   {
-    QStringList theURIParts = details.dataSourceUri.split( '|' );
-    QString srcFileName = theURIParts[0];
-
+    QString srcFileName( details.providerUriParams.value( QLatin1String( "path" ) ).toString() );
     if ( QFile::exists( srcFileName ) && QFileInfo( fileName ).canonicalFilePath() == QFileInfo( srcFileName ).canonicalFilePath() )
     {
-      if ( errorMessage )
-        *errorMessage = QObject::tr( "Cannot overwrite a OGR layer in place" );
-      return ErrCreateDataSource;
+      // Check the layer name too if it's a GPKG/SpatiaLite/SQLite OGR driver (pay attention: camel case in layerName)
+      QgsDataSourceUri uri( details.dataSourceUri );
+      if ( !( ( options.driverName == QLatin1String( "GPKG" ) ||
+                options.driverName == QLatin1String( "SpatiaLite" ) ||
+                options.driverName == QLatin1String( "SQLite" ) ) &&
+              options.layerName != details.providerUriParams.value( QLatin1String( "layerName" ) ) ) )
+      {
+        if ( errorMessage )
+          *errorMessage = QObject::tr( "Cannot overwrite a OGR layer in place" );
+        return ErrCreateDataSource;
+      }
     }
 
     // Shapefiles might contain multi types although wkbType() only reports singles
@@ -2577,7 +2586,7 @@ QgsVectorFileWriter::WriterError QgsVectorFileWriter::writeAsVectorFormat( Prepa
         if ( options.feedback )
         {
           //dedicate first 5% of progress bar to this scan
-          int newProgress = ( 5.0 * scanned ) / total;
+          int newProgress = static_cast<int>( ( 5.0 * scanned ) / total );
           if ( newProgress != lastProgressReport )
           {
             lastProgressReport = newProgress;
@@ -2675,7 +2684,7 @@ QgsVectorFileWriter::WriterError QgsVectorFileWriter::writeAsVectorFormat( Prepa
     if ( options.feedback )
     {
       //avoid spamming progress reports
-      int newProgress = initialProgress + ( ( 100.0 - initialProgress ) * saved ) / total;
+      int newProgress = static_cast<int>( initialProgress + ( ( 100.0 - initialProgress ) * saved ) / total );
       if ( newProgress < 100 && newProgress != lastProgressReport )
       {
         lastProgressReport = newProgress;

diff --git a/src/core/qgsvectorfilewriter.h b/src/core/qgsvectorfilewriter.h
@@ -760,6 +760,7 @@ class CORE_EXPORT QgsVectorFileWriter : public QgsFeatureSink
       QgsFeatureIterator sourceFeatureIterator;
       QgsGeometry filterRectGeometry;
       std::unique_ptr< QgsGeometryEngine  > filterRectEngine;
+      QVariantMap providerUriParams;
     };
 
     /**
@@ -814,6 +815,7 @@ class CORE_EXPORT QgsVectorFileWriter : public QgsFeatureSink
     static QStringList concatenateOptions( const QMap<QString, Option *> &options );
 
     friend class QgsVectorFileWriterTask;
+    friend class TestQgsVectorFileWriter;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsVectorFileWriter::EditionCapabilities )

diff --git a/tests/src/core/testqgsvectorfilewriter.cpp b/tests/src/core/testqgsvectorfilewriter.cpp
@@ -80,6 +80,8 @@ class TestQgsVectorFileWriter: public QObject
     void projectedPlygonGridTest();
     //! This is a regression test ticket 1141 (broken Polish characters support since r8592) https://issues.qgis.org/issues/1141
     void regression1141();
+    //! Test prepareWriteAsVectorFormat
+    void prepareWriteAsVectorFormat();
 
   private:
     // a little util fn used by all tests
@@ -106,6 +108,7 @@ void TestQgsVectorFileWriter::initTestCase()
   // init QGIS's paths - true means that all path will be inited from prefix
   QgsApplication::init();
   QgsApplication::showSettings();
+  QgsApplication::initQgis();
   //create some objects that will be used in all tests...
 
   mEncoding = QStringLiteral( "UTF-8" );
@@ -457,5 +460,36 @@ void TestQgsVectorFileWriter::regression1141()
   QVERIFY( QgsVectorFileWriter::deleteShapeFile( fileName ) );
 }
 
+void TestQgsVectorFileWriter::prepareWriteAsVectorFormat()
+{
+  QgsVectorFileWriter::PreparedWriterDetails details;
+  QgsVectorFileWriter::SaveVectorOptions options;
+  QgsVectorLayer ml( "Point?field=firstfield:int&field=secondfield:int", "test", "memory" );
+  QgsFeature ft( ml.fields( ) );
+  ft.setAttribute( 0, 4 );
+  ft.setAttribute( 1, -10 );
+  ml.dataProvider()->addFeature( ft );
+  QVERIFY( ml.isValid() );
+  QTemporaryFile tmpFile( QDir::tempPath() +  "/test_qgsvectorfilewriter_XXXXXX.gpkg" );
+  tmpFile.open();
+  QString fileName( tmpFile.fileName( ) );
+  options.driverName = "GPKG";
+  options.layerName = "test";
+  QString errorMessage;
+  QgsVectorFileWriter::WriterError error( QgsVectorFileWriter::writeAsVectorFormat(
+      &ml,
+      fileName,
+      options,
+      &errorMessage ) );
+
+  QCOMPARE( error, QgsVectorFileWriter::WriterError::NoError );
+  QCOMPARE( errorMessage, fileName );
+  QgsVectorLayer vl( QStringLiteral( "%1|layername=test" ).arg( fileName ), "src_test", "ogr" );
+  QVERIFY( vl.isValid() );
+  QgsVectorFileWriter::prepareWriteAsVectorFormat( &vl, options, details );
+  QCOMPARE( details.providerUriParams.value( "layerName" ).toString(), QStringLiteral( "test" ) );
+  QCOMPARE( details.providerUriParams.value( "path" ).toString(), fileName );
+}
+
 QGSTEST_MAIN( TestQgsVectorFileWriter )
 #include "testqgsvectorfilewriter.moc"
diff --git a/tests/src/python/test_qgsvectorfilewriter.py b/tests/src/python/test_qgsvectorfilewriter.py
@@ -32,6 +32,7 @@
                        )
 from qgis.PyQt.QtCore import QDate, QTime, QDateTime, QVariant, QDir
 import os
+import tempfile
 import osgeo.gdal  # NOQA
 from osgeo import gdal, ogr
 from qgis.testing import start_app, unittest
@@ -898,6 +899,46 @@ def testSupportsFeatureStyles(self):
         self.assertTrue(QgsVectorFileWriter.supportsFeatureStyles('MapInfo File'))
         self.assertTrue(QgsVectorFileWriter.supportsFeatureStyles('MapInfo MIF'))
 
+    def testOverwriteGPKG(self):
+        """Test that overwriting the same origin GPKG file works only if the layername is different"""
+
+        # Prepare test data
+        ml = QgsVectorLayer('Point?field=firstfield:int&field=secondfield:int', 'test', 'memory')
+        provider = ml.dataProvider()
+        ft = QgsFeature()
+        ft.setAttributes([4, -10])
+        provider.addFeatures([ft])
+        filehandle, filename = tempfile.mkstemp('.gpkg')
+
+        options = QgsVectorFileWriter.SaveVectorOptions()
+        options.driverName = 'GPKG'
+        options.layerName = 'test'
+        write_result, error_message = QgsVectorFileWriter.writeAsVectorFormat(
+            ml,
+            filename,
+            options)
+        self.assertEqual(write_result, QgsVectorFileWriter.NoError, error_message)
+
+        # Real test
+        vl = QgsVectorLayer("%s|layername=test" % filename, 'src_test', 'ogr')
+        self.assertTrue(vl.isValid())
+        self.assertEqual(vl.featureCount(), 1)
+
+        # This must fail
+        write_result, error_message = QgsVectorFileWriter.writeAsVectorFormat(
+            vl,
+            filename,
+            options)
+        self.assertEqual(write_result, QgsVectorFileWriter.ErrCreateDataSource)
+        self.assertEqual(error_message, 'Cannot overwrite a OGR layer in place')
+
+        options.layerName = 'test2'
+        write_result, error_message = QgsVectorFileWriter.writeAsVectorFormat(
+            vl,
+            filename,
+            options)
+        self.assertEqual(write_result, QgsVectorFileWriter.NoError, error_message)
+
 
 if __name__ == '__main__':
     unittest.main()