From 215e3ecbd114edbb9a5998fc5c9cbba4400de37c Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 12 Nov 2019 18:16:48 +0100 Subject: [PATCH 1/2] [feature] Delete PG rasters from browser I also fixed a bunch of small issues while I was there (parent item didn't refresh), misnames table/file (wrong assumption that all ogr/gdal datasources are filesystem based). Fixes #32809 --- .../browser/qgsinbuiltdataitemproviders.cpp | 5 +- src/gui/providers/gdal/qgsgdalguiprovider.cpp | 95 ++++++++++++++++--- .../qgspostgresproviderconnection.cpp | 1 + 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/src/app/browser/qgsinbuiltdataitemproviders.cpp b/src/app/browser/qgsinbuiltdataitemproviders.cpp index 75d6ba37a7b..0853298208a 100644 --- a/src/app/browser/qgsinbuiltdataitemproviders.cpp +++ b/src/app/browser/qgsinbuiltdataitemproviders.cpp @@ -434,7 +434,10 @@ void QgsLayerItemGuiProvider::populateContextMenu( QgsDataItem *item, QMenu *men bool isFile = false; if ( layerItem ) { - isFile = layerItem->providerKey() == QStringLiteral( "ogr" ) || layerItem->providerKey() == QStringLiteral( "gdal" ); + // Also check for postgres layers (rasters are handled by GDAL) + isFile = ( layerItem->providerKey() == QStringLiteral( "ogr" ) || + layerItem->providerKey() == QStringLiteral( "gdal" ) ) && + ! layerItem->uri().startsWith( QStringLiteral( "PG:" ) ); } else { diff --git a/src/gui/providers/gdal/qgsgdalguiprovider.cpp b/src/gui/providers/gdal/qgsgdalguiprovider.cpp index f604a17ad18..51fee83d1a1 100644 --- a/src/gui/providers/gdal/qgsgdalguiprovider.cpp +++ b/src/gui/providers/gdal/qgsgdalguiprovider.cpp @@ -25,6 +25,7 @@ #include #include "qgsproject.h" +#include "qgsrasterlayer.h" #include "qgssourceselectprovider.h" #include "qgsgdalsourceselect.h" #include "qgsapplication.h" @@ -50,12 +51,17 @@ void QgsGdalItemGuiProvider::onDeleteLayer() QVariantMap data = s->data().toMap(); const QString uri = data[QStringLiteral( "uri" )].toString(); const QString path = data[QStringLiteral( "path" )].toString(); - QPointer< QgsDataItem > parent = data[QStringLiteral( "parent" )].value>(); + QPointer< QgsDataItem > parent = data[QStringLiteral( "parentItem" )].value>(); + + // Messages are different for files and tables + bool isPostgresRaster { uri.startsWith( QStringLiteral( "PG:" ) ) }; + const QString title = isPostgresRaster ? + tr( "Delete Table" ) : + tr( "Delete File" ); - const QString title = QObject::tr( "Delete File" ); // Check if the layer is in the project const QgsMapLayer *projectLayer = nullptr; - const auto mapLayers = QgsProject::instance()->mapLayers(); + const QMap mapLayers = QgsProject::instance()->mapLayers(); for ( auto it = mapLayers.constBegin(); it != mapLayers.constEnd(); ++it ) { if ( it.value()->publicSource() == uri ) @@ -63,30 +69,90 @@ void QgsGdalItemGuiProvider::onDeleteLayer() projectLayer = it.value(); } } + if ( ! projectLayer ) { - const QString confirmMessage = QObject::tr( "Are you sure you want to delete file '%1'?" ).arg( path ); + const QString confirmMessage = isPostgresRaster ? tr( "Are you sure you want to delete table “%1”?" ).arg( path ) : + tr( "Are you sure you want to delete file “%1”?" ).arg( path ); if ( QMessageBox::question( nullptr, title, confirmMessage, QMessageBox::Yes | QMessageBox::No, QMessageBox::No ) != QMessageBox::Yes ) return; - - if ( !QFile::remove( path ) ) + if ( isPostgresRaster ) { - QMessageBox::warning( nullptr, title, tr( "Could not delete file." ) ); + QString errorMessage; + bool deleted { false }; + QgsProviderMetadata *postgresMetadata { QgsProviderRegistry::instance()->providerMetadata( QLatin1String( "postgres" ) ) }; + if ( postgresMetadata ) + { + QgsAbstractDatabaseProviderConnection *connection = static_cast( postgresMetadata->createConnection( uri, {} ) ); + const QgsDataSourceUri dsUri { QgsDataSourceUri( uri ) }; + if ( connection ) + { + try + { + // Try hard to get the schema + QString schema = dsUri.schema(); + if ( schema.isEmpty() ) + { + schema = dsUri.param( QStringLiteral( "schema" ) ); + } + if ( schema.isEmpty() ) + { + schema = QStringLiteral( "public" ); + } + + connection->dropRasterTable( schema, dsUri.table() ); + deleted = true; + + } + catch ( QgsProviderConnectionException &ex ) + { + errorMessage = ex.what(); + } + } + else + { + errorMessage = tr( "could not create a connection to the database" ); + } + } + else + { + errorMessage = tr( "could not retrieve provider metadata" ); + } + + if ( deleted ) + { + QMessageBox::information( nullptr, title, tr( "Table deleted successfully." ) ); + if ( parent ) + parent->refresh(); + } + else + { + QMessageBox::warning( nullptr, title, errorMessage.isEmpty() ? + tr( "Could not delete table." ) : + tr( "Could not delete table, reason: %1." ).arg( errorMessage ) ); + } } else { - QMessageBox::information( nullptr, title, tr( "File deleted successfully." ) ); - if ( parent ) - parent->refresh(); + if ( !QFile::remove( path ) ) + { + QMessageBox::warning( nullptr, title, tr( "Could not delete file." ) ); + } + else + { + QMessageBox::information( nullptr, title, tr( "File deleted successfully." ) ); + if ( parent ) + parent->refresh(); + } } } else { - QMessageBox::warning( nullptr, title, QObject::tr( "The layer '%1' cannot be deleted because it is in the current project as '%2'," + QMessageBox::warning( nullptr, title, QObject::tr( "The layer “%1” cannot be deleted because it is in the current project as “%2”," " remove it from the project and retry." ).arg( path, projectLayer->name() ) ); } } @@ -99,11 +165,14 @@ void QgsGdalItemGuiProvider::populateContextMenu( QgsDataItem *item, QMenu *menu if ( QgsGdalLayerItem *layerItem = qobject_cast< QgsGdalLayerItem * >( item ) ) { // Messages are different for files and tables - const QString message = QObject::tr( "Delete File “%1”…" ).arg( layerItem->name() ); + bool isPostgresRaster { layerItem->uri().startsWith( QStringLiteral( "PG:" ) ) }; + const QString message = isPostgresRaster ? + QObject::tr( "Delete Table “%1”…" ).arg( layerItem->name() ) : + QObject::tr( "Delete File “%1”…" ).arg( layerItem->name() ); QAction *actionDeleteLayer = new QAction( message, menu ); QVariantMap data; data.insert( QStringLiteral( "uri" ), layerItem->uri() ); - data.insert( QStringLiteral( "path" ), layerItem->path() ); + data.insert( QStringLiteral( "path" ), isPostgresRaster ? layerItem->name() : layerItem->path() ); data.insert( QStringLiteral( "parentItem" ), QVariant::fromValue( QPointer< QgsDataItem >( layerItem->parent() ) ) ); actionDeleteLayer->setData( data ); connect( actionDeleteLayer, &QAction::triggered, this, &QgsGdalItemGuiProvider::onDeleteLayer ); diff --git a/src/providers/postgres/qgspostgresproviderconnection.cpp b/src/providers/postgres/qgspostgresproviderconnection.cpp index 5bc337d0829..ec7f3911cc5 100644 --- a/src/providers/postgres/qgspostgresproviderconnection.cpp +++ b/src/providers/postgres/qgspostgresproviderconnection.cpp @@ -48,6 +48,7 @@ void QgsPostgresProviderConnection::setDefaultCapabilities() mCapabilities = { Capability::DropVectorTable, + Capability::DropRasterTable, Capability::CreateVectorTable, Capability::RenameSchema, Capability::DropSchema, From 91ee3b40780a8b954456d729917366705f093ef0 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 13 Nov 2019 08:50:00 +0100 Subject: [PATCH 2/2] Don't leak the connection, please --- src/gui/providers/gdal/qgsgdalguiprovider.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/providers/gdal/qgsgdalguiprovider.cpp b/src/gui/providers/gdal/qgsgdalguiprovider.cpp index 51fee83d1a1..e4b458a7baa 100644 --- a/src/gui/providers/gdal/qgsgdalguiprovider.cpp +++ b/src/gui/providers/gdal/qgsgdalguiprovider.cpp @@ -87,7 +87,7 @@ void QgsGdalItemGuiProvider::onDeleteLayer() QgsProviderMetadata *postgresMetadata { QgsProviderRegistry::instance()->providerMetadata( QLatin1String( "postgres" ) ) }; if ( postgresMetadata ) { - QgsAbstractDatabaseProviderConnection *connection = static_cast( postgresMetadata->createConnection( uri, {} ) ); + std::unique_ptr connection { static_cast( postgresMetadata->createConnection( uri, {} ) ) }; const QgsDataSourceUri dsUri { QgsDataSourceUri( uri ) }; if ( connection ) {