Revert "Merge pull request #9164 from elpaso/bugfix-21227-layer-renam…

…e-styles" This reverts commit 0d95ec8.
qgis · Feb 20, 2019 · 5adb34d · 5adb34d
1 parent ab60a3e
commit 5adb34d
Show file tree

Hide file tree

Showing 12 changed files with 26 additions and 163 deletions.
diff --git a/python/plugins/db_manager/db_manager.py b/python/plugins/db_manager/db_manager.py
@@ -88,8 +88,6 @@ def itemChanged(self, item):
         with OverrideCursor(Qt.WaitCursor):
             try:
                 self.reloadButtons()
-                # Force-reload information on the layer
-                self.info.setDirty()
                 # clear preview, this will delete the layer in preview tab
                 self.preview.loadPreview(None)
                 self.refreshTabs()

diff --git a/python/plugins/db_manager/db_plugins/gpkg/connector.py b/python/plugins/db_manager/db_plugins/gpkg/connector.py
@@ -30,8 +30,6 @@
 from ..plugin import ConnectionError, DbError, Table
 
 from qgis.utils import spatialite_connect
-from qgis.core import QgsApplication
-
 import sqlite3
 
 from osgeo import gdal, ogr, osr
@@ -592,26 +590,28 @@ def emptyTable(self, table):
         self._execute_and_commit(sql)
 
     def renameTable(self, table, new_table):
-        """Renames the table
-
-        :param table: tuple with schema and table names
-        :type table: tuple (str, str)
-        :param new_table: new table name
-        :type new_table: str
-        :return: true on success
-        :rtype: bool
-        """
+        """ rename a table """
+
+        if self.isRasterTable(table):
+            return False
 
-        table_name = table[1]
-        provider = [p for p in QgsApplication.dataItemProviderRegistry().providers() if p.name() == 'OGR'][0]
-        collection_item = provider.createDataItem(self.dbname, None)
-        data_item = [c for c in collection_item.createChildren() if c.name() == table_name][0]
-        result = data_item.rename(new_table)
+        _, tablename = self.getSchemaTableName(table)
+        if new_table == tablename:
+            return True
+
+        if tablename.find('"') >= 0:
+            tablename = self.quoteId(tablename)
+        if new_table.find('"') >= 0:
+            new_table = self.quoteId(new_table)
+
+        gdal.ErrorReset()
+        self.gdal_ds.ExecuteSQL('ALTER TABLE %s RENAME TO %s' % (tablename, new_table))
+        if gdal.GetLastErrorMsg() != '':
+            return False
         # we need to reopen after renaming since OGR doesn't update its
         # internal state
-        if result:
-            self._opendb()
-        return result
+        self._opendb()
+        return True
 
     def moveTable(self, table, new_table, new_schema=None):
         return self.renameTable(table, new_table)

diff --git a/python/plugins/db_manager/db_plugins/plugin.py b/python/plugins/db_manager/db_plugins/plugin.py
@@ -594,7 +594,6 @@ def rename(self, new_name):
         ret = self.database().connector.renameSchema(self.name, new_name)
         if ret is not False:
             self.name = new_name
-            # FIXME: refresh triggers
             self.refresh()
         return ret
 
@@ -653,9 +652,6 @@ def rename(self, new_name):
         ret = self.database().connector.renameTable((self.schemaName(), self.name), new_name)
         if ret is not False:
             self.name = new_name
-            self._triggers = None
-            self._rules = None
-            self._constraints = None
             self.refresh()
         return ret
 

diff --git a/python/plugins/db_manager/layer_preview.py b/python/plugins/db_manager/layer_preview.py
@@ -40,7 +40,7 @@ def __init__(self, parent=None):
 
         self.item = None
         self.dirty = False
-        self.currentLayerId = None
+        self.currentLayer = None
 
         # reuse settings from QGIS
         settings = QgsSettings()
@@ -118,9 +118,9 @@ def _loadTablePreview(self, table, limit=False):
                     vl = None
 
             # remove old layer (if any) and set new
-            if self.currentLayerId:
-                if not QgsProject.instance().layerTreeRoot().findLayer(self.currentLayerId):
-                    QgsProject.instance().removeMapLayers([self.currentLayerId])
+            if self.currentLayer:
+                if not QgsProject.instance().layerTreeRoot().findLayer(self.currentLayer.id()):
+                    QgsProject.instance().removeMapLayers([self.currentLayer.id()])
 
             if vl and vl.isValid():
                 self.setLayers([vl])
@@ -129,7 +129,7 @@ def _loadTablePreview(self, table, limit=False):
             else:
                 self.setLayers([])
 
-            self.currentLayerId = vl.id()
+            self.currentLayer = vl
 
             self.freeze(False)
             super().refresh()
diff --git a/src/providers/ogr/CMakeLists.txt b/src/providers/ogr/CMakeLists.txt
@@ -63,26 +63,18 @@ 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
@@ -39,7 +39,6 @@
 #include "qgstaskmanager.h"
 #include "qgsproviderregistry.h"
 #include "qgsproxyprogresstask.h"
-#include "qgssqliteutils.h"
 
 QGISEXTERN bool deleteLayer( const QString &uri, const QString &errCause );
 
@@ -744,86 +743,3 @@ bool QgsGeoPackageVectorLayerItem::executeDeleteLayer( QString &errCause )
   return ::deleteLayer( mUri, errCause );
 }
 
-bool QgsGeoPackageVectorLayerItem::rename( const QString &name )
-{
-  // Checks that name does not exist yet
-  if ( tableNames().contains( name ) )
-  {
-    return false;
-  }
-  // Check if the layer(s) are in the registry
-  const QList<QgsMapLayer *> layersList( layersInProject() );
-  if ( ! layersList.isEmpty( ) )
-  {
-    if ( QMessageBox::question( nullptr, QObject::tr( "Rename Layer" ), QObject::tr( "The layer <b>%1</b> is loaded in the current project with name <b>%2</b>,"
-                                " do you want to remove it from the project and rename it?" ).arg( mName, layersList.at( 0 )->name() ), QMessageBox::Yes | QMessageBox::No, QMessageBox::No ) != QMessageBox::Yes )
-    {
-      return false;
-    }
-  }
-  if ( ! layersList.isEmpty() )
-  {
-    QgsProject::instance()->removeMapLayers( layersList );
-  }
-
-  const QVariantMap parts = QgsProviderRegistry::instance()->decodeUri( mProviderKey, mUri );
-  QString errCause;
-  if ( parts.empty() || parts.value( QStringLiteral( "path" ) ).isNull() || parts.value( QStringLiteral( "layerName" ) ).isNull() )
-  {
-    errCause = QObject::tr( "Layer URI %1 is not valid!" ).arg( mUri );
-  }
-  else
-  {
-    QString filePath = parts.value( QStringLiteral( "path" ) ).toString();
-    const QList<QgsMapLayer *> layersList( layersInProject() );
-    if ( ! layersList.isEmpty( ) )
-    {
-      if ( QMessageBox::question( nullptr, QObject::tr( "Rename Layer" ), QObject::tr( "The layer <b>%1</b> exists in the current project <b>%2</b>,"
-                                  " do you want to remove it from the project and rename it?" ).arg( mName, layersList.at( 0 )->name() ), QMessageBox::Yes | QMessageBox::No, QMessageBox::No ) != QMessageBox::Yes )
-      {
-        return false;
-      }
-    }
-    if ( ! layersList.isEmpty() )
-    {
-      QgsProject::instance()->removeMapLayers( layersList );
-    }
-
-    // TODO: maybe an index?
-    QString oldName = parts.value( QStringLiteral( "layerName" ) ).toString();
-
-    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( QgsSqliteUtils::quotedIdentifier( oldName ),
-                         QgsSqliteUtils::quotedIdentifier( name ) ) );
-      OGRLayerH ogrLayer( GDALDatasetExecuteSQL( hDS, sql.toUtf8().constData(), nullptr, nullptr ) );
-      if ( ogrLayer )
-        GDALDatasetReleaseResultSet( hDS, ogrLayer );
-      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
-    {
-      errCause = QObject::tr( "There was an error opening %1!" ).arg( filePath );
-    }
-  }
-
-  if ( ! errCause.isEmpty() )
-    QMessageBox::critical( nullptr, QObject::tr( "Error renaming layer" ), errCause );
-  else if ( mParent )
-    mParent->refreshConnections();
-
-  return errCause.isEmpty();
-}
-
diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -6455,4 +6455,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
@@ -16,8 +16,6 @@ INCLUDE_DIRECTORIES(${CMAKE_CURRENT_SOURCE_DIR}
   ${CMAKE_SOURCE_DIR}/src/providers/wms
   ${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,7 +72,6 @@ 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,8 +24,6 @@
 #include <qgsproviderregistry.h>
 #include <qgsvectorlayer.h>
 #include <qgsnetworkaccessmanager.h>
-#include <qgsgeopackagedataitems.h>
-#include <qgsdataitem.h>
 
 #include <QObject>
 
@@ -48,8 +46,6 @@ class TestQgsOgrProvider : public QObject
 
     void setupProxy();
     void testThread();
-    //! Test GPKG data items rename
-    void testGpkgDataItemRename();
 
   private:
     QString mTestDataDir;
@@ -205,37 +201,6 @@ 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/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py
@@ -1296,8 +1296,8 @@ def testJson(self):
     def test_quote_identifier(self):
         """Regression #21100"""
 
-        tmpfile = os.path.join(self.basetestpath, 'bug_21100-wierd_field_names.gpkg')  # spellok
-        shutil.copy(os.path.join(unitTestDataPath(''), 'bug_21100-wierd_field_names.gpkg'), tmpfile) # spellok
+        tmpfile = os.path.join(self.basetestpath, 'bug21100-wierd_field_names.gpkg')  # spellok
+        shutil.copy(os.path.join(unitTestDataPath(''), 'bug21100-wierd_field_names.gpkg'), tmpfile) # spellok
         vl = QgsVectorLayer('{}|layerid=0'.format(tmpfile), 'foo', 'ogr')
         self.assertTrue(vl.isValid())
         for i in range(1, len(vl.fields())):

diff --git a/...testdata/bug_21100-wierd_field_names.gpkg → .../testdata/bug21100-wierd_field_names.gpkg b/...testdata/bug_21100-wierd_field_names.gpkg → .../testdata/bug21100-wierd_field_names.gpkg
diff --git a/tests/testdata/provider/bug_21227-rename-styles.gpkg b/tests/testdata/provider/bug_21227-rename-styles.gpkg