GPKG: Rename styles when layers are renamed

elpaso · elpaso · commit 61d361d6746e · 2019-02-12T19:37:01.000+01:00
Partially fixes #21227 TODO: - DB manager - Other providers
diff --git a/src/providers/ogr/CMakeLists.txt b/src/providers/ogr/CMakeLists.txt
@@ -64,18 +64,26 @@ INCLUDE_DIRECTORIES(SYSTEM
 
 
 ADD_LIBRARY(ogrprovider MODULE ${OGR_SRCS} ${OGR_MOC_SRCS})
+ADD_LIBRARY(ogrprovider_a STATIC ${OGR_SRCS} ${OGR_MOC_SRCS})
 
 TARGET_LINK_LIBRARIES(ogrprovider
   qgis_core
 )
 
+TARGET_LINK_LIBRARIES(ogrprovider_a
+  qgis_core
+)
 
 IF (WITH_GUI)
   TARGET_LINK_LIBRARIES (ogrprovider
     qgis_gui
   )
+  TARGET_LINK_LIBRARIES (ogrprovider_a
+    qgis_gui
+  )
 ENDIF ()
 
+
 IF (MSVC)
   #needed for linking to gdal which needs odbc
   SET(TARGET_LINK_LIBRARIES ${TARGET_LINK_LIBRARIE} odbc32 odbccp32)
diff --git a/src/providers/ogr/qgsgeopackagedataitems.cpp b/src/providers/ogr/qgsgeopackagedataitems.cpp
@@ -41,6 +41,7 @@
 #include "qgstaskmanager.h"
 #include "qgsproviderregistry.h"
 #include "qgsproxyprogresstask.h"
+#include "qgssqliteutils.h"
 
 
 QGISEXTERN bool deleteLayer( const QString &uri, const QString &errCause );
@@ -887,12 +888,23 @@ bool QgsGeoPackageVectorLayerItem::rename( const QString &name )
     GDALDatasetH hDS = GDALOpenEx( filePath.toUtf8().constData(), GDAL_OF_VECTOR | GDAL_OF_UPDATE, nullptr, nullptr, nullptr );
     if ( hDS )
     {
-      QString sql( QStringLiteral( "ALTER TABLE \"%1\" RENAME TO \"%2\"" ).arg( oldName, name ) );
+      QString sql( QStringLiteral( "ALTER TABLE %1 RENAME TO %2" )
+                   .arg( QgsSqliteUtils::quotedIdentifier( oldName ),
+                         QgsSqliteUtils::quotedIdentifier( name ) ) );
       OGRLayerH ogrLayer( GDALDatasetExecuteSQL( hDS, sql.toUtf8().constData(), nullptr, nullptr ) );
       if ( ogrLayer )
         GDALDatasetReleaseResultSet( hDS, ogrLayer );
-      GDALClose( hDS );
       errCause = CPLGetLastErrorMsg( );
+      if ( errCause.isEmpty() )
+      {
+        sql = QStringLiteral( "UPDATE layer_styles SET f_table_name = %2 WHERE f_table_name = %1" )
+              .arg( QgsSqliteUtils::quotedString( oldName ),
+                    QgsSqliteUtils::quotedString( name ) );
+        ogrLayer = GDALDatasetExecuteSQL( hDS, sql.toUtf8().constData(), nullptr, nullptr );
+        if ( ogrLayer )
+          GDALDatasetReleaseResultSet( hDS, ogrLayer );
+      }
+      GDALClose( hDS );
     }
     else
     {
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -6574,4 +6574,3 @@ QGISEXTERN QgsTransaction *createTransaction( const QString &connString )
   return new QgsOgrTransaction( connString, ds );
 }
 
-
diff --git a/tests/src/providers/CMakeLists.txt b/tests/src/providers/CMakeLists.txt
@@ -17,6 +17,7 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}
   ${CMAKE_SOURCE_DIR}/src/providers/postgres
   ${CMAKE_SOURCE_DIR}/src/providers/arcgisrest
   ${CMAKE_SOURCE_DIR}/src/providers/mdal
+  ${CMAKE_SOURCE_DIR}/src/providers/ogr
   ${CMAKE_SOURCE_DIR}/src/test
   ${CMAKE_BINARY_DIR}/src/core
 )
@@ -74,6 +75,7 @@ ADD_QGIS_TEST(gdalprovidertest testqgsgdalprovider.cpp)
 
 SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread")
 ADD_QGIS_TEST(ogrprovidertest testqgsogrprovider.cpp)
+TARGET_LINK_LIBRARIES(qgis_ogrprovidertest ogrprovider_a)
 
 ADD_QGIS_TEST(wmscapabilitiestest
               testqgswmscapabilities.cpp)
diff --git a/tests/src/providers/testqgsogrprovider.cpp b/tests/src/providers/testqgsogrprovider.cpp
@@ -24,6 +24,8 @@
 #include <qgsproviderregistry.h>
 #include <qgsvectorlayer.h>
 #include <qgsnetworkaccessmanager.h>
+#include <qgsgeopackagedataitems.h>
+#include <qgsdataitem.h>
 
 #include <QObject>
 
@@ -47,6 +49,8 @@ class TestQgsOgrProvider : public QObject
     void setupProxy();
     void decodeUri();
     void testThread();
+    //! Test GPKG data items rename
+    void testGpkgDataItemRename();
 
   private:
     QString mTestDataDir;
@@ -213,6 +217,37 @@ void TestQgsOgrProvider::testThread()
 
 }
 
+void TestQgsOgrProvider::testGpkgDataItemRename()
+{
+  QTemporaryFile f( QStringLiteral( "qgis-XXXXXX.gpkg" ) );
+  f.open();
+  f.close();
+  QString fileName { f.fileName( ) };
+  f.remove();
+  QVERIFY( QFile::copy( QStringLiteral( "%1/provider/bug_21227-rename-styles.gpkg" ).arg( mTestDataDir ),  fileName ) );
+  QgsGeoPackageVectorLayerItem item( nullptr,
+                                     QStringLiteral( "Layer 1" ),
+                                     QStringLiteral( "gpkg:/%1|layername=layer 1" )
+                                     .arg( fileName ),
+                                     QStringLiteral( "%1|layername=layer 1" ).arg( fileName ),
+                                     QgsLayerItem::LayerType::TableLayer );
+  item.rename( "layer 3" );
+  // Check that the style is still available
+  QgsVectorLayer metadataLayer( QStringLiteral( "/%1|layername=layer_styles" ).arg( fileName ) );
+  QVERIFY( metadataLayer.isValid() );
+  QgsFeature feature;
+  QgsFeatureIterator it = metadataLayer.getFeatures( QgsFeatureRequest( QgsExpression( QStringLiteral( "\"f_table_name\" = 'layer 3'" ) ) ) );
+  QVERIFY( it.nextFeature( feature ) );
+  QVERIFY( feature.isValid() );
+  QCOMPARE( feature.attribute( QStringLiteral( "styleName" ) ).toString(), QString( "style for layer 1" ) );
+  it = metadataLayer.getFeatures( QgsFeatureRequest( QgsExpression( QStringLiteral( "\"f_table_name\" = 'layer 1' " ) ) ) );
+  QVERIFY( !it.nextFeature( feature ) );
+  it = metadataLayer.getFeatures( QgsFeatureRequest( QgsExpression( QStringLiteral( "\"f_table_name\" = 'layer 2' " ) ) ) );
+  QVERIFY( it.nextFeature( feature ) );
+  QVERIFY( feature.isValid() );
+  QCOMPARE( feature.attribute( QStringLiteral( "styleName" ) ).toString(), QString( "style for layer 2" ) );
+}
+
 
 QGSTEST_MAIN( TestQgsOgrProvider )
 #include "testqgsogrprovider.moc"
diff --git a/tests/testdata/provider/bug_21227-rename-styles.gpkg b/tests/testdata/provider/bug_21227-rename-styles.gpkg

Original file line number	Diff line number	Diff line change
`@@ -6574,4 +6574,3 @@ QGISEXTERN QgsTransaction *createTransaction( const QString &connString )`
`6574`	`6574`	`return new QgsOgrTransaction( connString, ds );`
`6575`	`6575`	`}`
`6576`	`6576`
`6577`		`-`