A few UX enhancements for error reporting on GPKGs

This fixes #36574 by showing an error in the data
items when the layer cannot be opened.

In the same way, similar errors now bubble up
to the message bar from data source select dialog.
This commit is contained in:
Alessandro Pasotti 2020-06-07 12:53:34 +02:00 committed by Nyall Dawson
parent 62bd026743
commit cd4785de9e
9 changed files with 157 additions and 77 deletions

View File

@ -138,6 +138,13 @@ Emitted when a progress dialog is shown by the provider dialog
Emitted when the ok/add buttons should be enabled/disabled
%End
void pushMessage( const QString &title, const QString &message, const Qgis::MessageLevel level = Qgis::MessageLevel::Info );
%Docstring
Emitted when a ``message`` with ``title`` and ``level`` must be shown to the user using the parent visible message bar
.. versionadded:: 3.14
%End
protected:

View File

@ -15865,7 +15865,7 @@ void QgisApp::onLayerError( const QString &msg )
Q_ASSERT( layer );
mInfoBar->pushCritical( tr( "Layer %1" ).arg( layer->name() ), msg );
visibleMessageBar()->pushCritical( tr( "Layer %1" ).arg( layer->name() ), msg );
}
bool QgisApp::gestureEvent( QGestureEvent *event )

View File

@ -112,28 +112,35 @@ QgsGeoPackageCollectionItem::QgsGeoPackageCollectionItem( QgsDataItem *parent, c
QVector<QgsDataItem *> QgsGeoPackageCollectionItem::createChildren()
{
QVector<QgsDataItem *> children;
const auto layers = QgsOgrLayerItem::subLayers( mPath.remove( QLatin1String( "gpkg:/" ) ), QStringLiteral( "GPKG" ) );
for ( const QgsOgrDbLayerInfo *info : layers )
try
{
if ( info->layerType() == QgsLayerItem::LayerType::Raster )
const auto layers = QgsOgrLayerItem::subLayers( mPath.remove( QLatin1String( "gpkg:/" ) ), QStringLiteral( "GPKG" ) );
for ( const QgsOgrDbLayerInfo *info : layers )
{
children.append( new QgsGeoPackageRasterLayerItem( this, info->name(), info->path(), info->uri() ) );
if ( info->layerType() == QgsLayerItem::LayerType::Raster )
{
children.append( new QgsGeoPackageRasterLayerItem( this, info->name(), info->path(), info->uri() ) );
}
else
{
children.append( new QgsGeoPackageVectorLayerItem( this, info->name(), info->path(), info->uri(), info->layerType( ) ) );
}
}
else
qDeleteAll( layers );
QgsProjectStorage *storage = QgsApplication::projectStorageRegistry()->projectStorageFromType( "geopackage" );
if ( storage )
{
children.append( new QgsGeoPackageVectorLayerItem( this, info->name(), info->path(), info->uri(), info->layerType( ) ) );
const QStringList projectNames = storage->listProjects( mPath );
for ( const QString &projectName : projectNames )
{
QgsGeoPackageProjectUri projectUri { true, mPath, projectName };
children.append( new QgsProjectItem( this, projectName, QgsGeoPackageProjectStorage::encodeUri( projectUri ) ) );
}
}
}
qDeleteAll( layers );
QgsProjectStorage *storage = QgsApplication::projectStorageRegistry()->projectStorageFromType( "geopackage" );
if ( storage )
catch ( QgsOgrLayerNotValidException &ex )
{
const QStringList projectNames = storage->listProjects( mPath );
for ( const QString &projectName : projectNames )
{
QgsGeoPackageProjectUri projectUri { true, mPath, projectName };
children.append( new QgsProjectItem( this, projectName, QgsGeoPackageProjectStorage::encodeUri( projectUri ) ) );
}
children.append( new QgsErrorItem( this, ex.what(), mPath + "/error" ) );
}
return children;
}

View File

@ -107,11 +107,7 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
// Vector layers
const QgsVectorLayer::LayerOptions layerOptions { QgsProject::instance()->transformContext() };
QgsVectorLayer layer( path, QStringLiteral( "ogr_tmp" ), QStringLiteral( "ogr" ), layerOptions );
if ( ! layer.isValid( ) )
{
QgsDebugMsgLevel( QStringLiteral( "Layer is not a valid %1 Vector layer %2" ).arg( path ), 3 );
}
else
if ( layer.isValid( ) )
{
// Collect mixed-geom layers
QMultiMap<int, QStringList> subLayersMap;
@ -188,6 +184,7 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
}
}
}
// Raster layers
QgsRasterLayer::LayerOptions options;
options.loadDefaultStyle = false;
@ -200,7 +197,7 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
// Split on ':' since this is what comes out from the provider
QStringList pieces = uri.split( ':' );
QString name = pieces.value( pieces.length() - 1 );
QgsDebugMsgLevel( QStringLiteral( "Adding GeoPackage Raster item %1 %2 %3" ).arg( name, uri ), 3 );
QgsDebugMsgLevel( QStringLiteral( "Adding GeoPackage Raster item %1 %2" ).arg( name, uri ), 3 );
children.append( new QgsOgrDbLayerInfo( path, uri, name, QString(), QStringLiteral( "Raster" ), QgsLayerItem::LayerType::Raster ) );
}
}
@ -238,6 +235,24 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
children.append( new QgsOgrDbLayerInfo( path, uri, name, QString(), QStringLiteral( "Raster" ), QgsLayerItem::LayerType::Raster ) );
}
}
// There were problems in reading the file: throw
if ( ! layer.isValid() && ! rlayer.isValid() )
{
QString errorMessage;
// If it is file based and the file exists, there might be a permission error, let's change
// the message to give the user a hint about this possiblity.
if ( QFile::exists( path ) )
{
errorMessage = tr( "Error opening file, check file and directory permissions on\n%1" ).arg( path );
}
else
{
errorMessage = tr( "Layer is not valid (%1)" ).arg( path );
}
throw QgsOgrLayerNotValidException( errorMessage );
}
return children;
}

View File

@ -55,6 +55,20 @@ class CORE_EXPORT QgsOgrDbLayerInfo
QgsLayerItem::LayerType mLayerType = QgsLayerItem::LayerType::NoType;
};
/**
* The QgsOgrLayerException class is thrown by QgsOgrLayerItem when the layer is not valid
*/
class CORE_EXPORT QgsOgrLayerNotValidException: public QgsException
{
public:
/**
* Constructor for QgsOgrLayerNotValidException, with the specified error \a message.
*/
QgsOgrLayerNotValidException( const QString &message )
: QgsException( message )
{}
};
class CORE_EXPORT QgsOgrLayerItem final: public QgsLayerItem
{
@ -63,7 +77,11 @@ class CORE_EXPORT QgsOgrLayerItem final: public QgsLayerItem
QgsOgrLayerItem( QgsDataItem *parent, const QString &name, const QString &path, const QString &uri, LayerType layerType, bool isSubLayer = false );
QString layerName() const override;
//! Retrieve sub layers from a DB ogr layer \a path with the specified \a driver
/**
* Retrieve sub layers from a DB ogr layer \a path with the specified \a driver
* If the layer is not valid, throw a std::exception
*/
static QList<QgsOgrDbLayerInfo *> subLayers( const QString &path, const QString &driver );
//! Returns a LayerType from a geometry type string
static QgsLayerItem::LayerType layerTypeFromDb( const QString &geometryType );

View File

@ -359,24 +359,34 @@ void QgsVectorLayerSaveAsDialog::accept()
}
else if ( mActionOnExistingFile == QgsVectorFileWriter::CreateOrOverwriteFile )
{
const QList<QgsOgrDbLayerInfo *> subLayers = QgsOgrLayerItem::subLayers( filename(), format() );
QStringList layerList;
for ( const QgsOgrDbLayerInfo *layer : subLayers )
try
{
layerList.append( layer->name() );
const QList<QgsOgrDbLayerInfo *> subLayers = QgsOgrLayerItem::subLayers( filename(), format() );
QStringList layerList;
for ( const QgsOgrDbLayerInfo *layer : subLayers )
{
layerList.append( layer->name() );
}
qDeleteAll( subLayers );
if ( layerList.length() > 1 )
{
layerList.sort( Qt::CaseInsensitive );
QMessageBox msgBox;
msgBox.setIcon( QMessageBox::Warning );
msgBox.setWindowTitle( tr( "Overwrite File" ) );
msgBox.setText( tr( "This file contains %1 layers that will be lost!\n" ).arg( QString::number( layerList.length() ) ) );
msgBox.setDetailedText( tr( "The following layers will be permanently lost:\n\n%1" ).arg( layerList.join( "\n" ) ) );
msgBox.setStandardButtons( QMessageBox::Ok | QMessageBox::Cancel );
if ( msgBox.exec() == QMessageBox::Cancel )
return;
}
}
qDeleteAll( subLayers );
if ( layerList.length() > 1 )
catch ( QgsOgrLayerNotValidException &ex )
{
layerList.sort( Qt::CaseInsensitive );
QMessageBox msgBox;
msgBox.setIcon( QMessageBox::Warning );
msgBox.setWindowTitle( tr( "Overwrite File" ) );
msgBox.setText( tr( "This file contains %1 layers that will be lost!\n" ).arg( QString::number( layerList.length() ) ) );
msgBox.setDetailedText( tr( "The following layers will be permanently lost:\n\n%1" ).arg( layerList.join( "\n" ) ) );
msgBox.setStandardButtons( QMessageBox::Ok | QMessageBox::Cancel );
if ( msgBox.exec() == QMessageBox::Cancel )
return;
QMessageBox::critical( this,
tr( "Save Vector Layer As" ),
tr( "Error opening destination file: %1" ).arg( ex.what() ) );
return;
}
}

View File

@ -332,41 +332,49 @@ void QgsOgrDbSourceSelect::btnConnect_clicked()
QgsOgrDbConnection conn( subKey, ogrDriverName() );
mPath = conn.path();
const QList<QgsOgrDbLayerInfo *> layers = QgsOgrLayerItem::subLayers( mPath, ogrDriverName() );
QModelIndex rootItemIndex = mTableModel.indexFromItem( mTableModel.invisibleRootItem() );
mTableModel.removeRows( 0, mTableModel.rowCount( rootItemIndex ), rootItemIndex );
// populate the table list
// get the list of suitable tables and columns and populate the UI
mTableModel.setPath( mPath );
for ( const QgsOgrDbLayerInfo *table : layers )
try
{
if ( cbxAllowGeometrylessTables->isChecked() || table->geometryType() != QStringLiteral( "None" ) )
const QList<QgsOgrDbLayerInfo *> layers = QgsOgrLayerItem::subLayers( mPath, ogrDriverName() );
QModelIndex rootItemIndex = mTableModel.indexFromItem( mTableModel.invisibleRootItem() );
mTableModel.removeRows( 0, mTableModel.rowCount( rootItemIndex ), rootItemIndex );
// populate the table list
// get the list of suitable tables and columns and populate the UI
mTableModel.setPath( mPath );
for ( const QgsOgrDbLayerInfo *table : layers )
{
mTableModel.addTableEntry( table->layerType(), table->name(), table->uri(), table->geometryColumn(), table->geometryType(), QString() );
if ( cbxAllowGeometrylessTables->isChecked() || table->geometryType() != QStringLiteral( "None" ) )
{
mTableModel.addTableEntry( table->layerType(), table->name(), table->uri(), table->geometryColumn(), table->geometryType(), QString() );
}
}
mTablesTreeView->sortByColumn( 0, Qt::AscendingOrder );
//expand all the toplevel items
int numTopLevelItems = mTableModel.invisibleRootItem()->rowCount();
for ( int i = 0; i < numTopLevelItems; ++i )
{
mTablesTreeView->expand( mProxyModel.mapFromSource( mTableModel.indexFromItem( mTableModel.invisibleRootItem()->child( i ) ) ) );
}
mTablesTreeView->resizeColumnToContents( 0 );
mTablesTreeView->resizeColumnToContents( 1 );
cbxAllowGeometrylessTables->setEnabled( true );
// Store selected connection
QgsOgrDbConnection::setSelectedConnection( subKey, ogrDriverName() );
qDeleteAll( layers );
}
mTablesTreeView->sortByColumn( 0, Qt::AscendingOrder );
//expand all the toplevel items
int numTopLevelItems = mTableModel.invisibleRootItem()->rowCount();
for ( int i = 0; i < numTopLevelItems; ++i )
catch ( QgsOgrLayerNotValidException &ex )
{
mTablesTreeView->expand( mProxyModel.mapFromSource( mTableModel.indexFromItem( mTableModel.invisibleRootItem()->child( i ) ) ) );
pushMessage( tr( "Error opening layer" ), ex.what(), Qgis::MessageLevel::Critical );
}
mTablesTreeView->resizeColumnToContents( 0 );
mTablesTreeView->resizeColumnToContents( 1 );
cbxAllowGeometrylessTables->setEnabled( true );
// Store selected connection
QgsOgrDbConnection::setSelectedConnection( subKey, ogrDriverName() );
qDeleteAll( layers );
}

View File

@ -142,6 +142,12 @@ class GUI_EXPORT QgsAbstractDataSourceWidget : public QDialog
//! Emitted when the ok/add buttons should be enabled/disabled
void enableButtons( bool enable );
/**
* Emitted when a \a message with \a title and \a level must be shown to the user using the parent visible message bar
* \since QGIS 3.14
*/
void pushMessage( const QString &title, const QString &message, const Qgis::MessageLevel level = Qgis::MessageLevel::Info );
protected:

View File

@ -166,10 +166,10 @@ void QgsDataSourceManagerDialog::addProviderDialog( QgsAbstractDataSourceWidget
void QgsDataSourceManagerDialog::makeConnections( QgsAbstractDataSourceWidget *dlg, const QString &providerKey )
{
// DB
connect( dlg, SIGNAL( addDatabaseLayers( QStringList const &, QString const & ) ),
this, SIGNAL( addDatabaseLayers( QStringList const &, QString const & ) ) );
connect( dlg, SIGNAL( progressMessage( QString ) ),
this, SIGNAL( showStatusMessage( QString ) ) );
connect( dlg, &QgsAbstractDataSourceWidget::addDatabaseLayers,
this, &QgsDataSourceManagerDialog::addDatabaseLayers );
connect( dlg, &QgsAbstractDataSourceWidget::progressMessage,
this, &QgsDataSourceManagerDialog::showStatusMessage );
// Vector
connect( dlg, &QgsAbstractDataSourceWidget::addVectorLayer, this, [ = ]( const QString & vectorLayerPath, const QString & baseName, const QString & specifiedProvider )
{
@ -179,20 +179,29 @@ void QgsDataSourceManagerDialog::makeConnections( QgsAbstractDataSourceWidget *d
);
connect( dlg, &QgsAbstractDataSourceWidget::addVectorLayers,
this, &QgsDataSourceManagerDialog::vectorLayersAdded );
connect( dlg, SIGNAL( connectionsChanged() ), this, SIGNAL( connectionsChanged() ) );
connect( dlg, &QgsAbstractDataSourceWidget::connectionsChanged, this, &QgsDataSourceManagerDialog::connectionsChanged );
// Raster
connect( dlg, SIGNAL( addRasterLayer( QString const &, QString const &, QString const & ) ),
this, SIGNAL( addRasterLayer( QString const &, QString const &, QString const & ) ) );
connect( dlg, &QgsAbstractDataSourceWidget::addRasterLayer,
this, [ = ]( const QString & uri, const QString & baseName, const QString & providerKey )
{
addRasterLayer( uri, baseName, providerKey );
} );
// Mesh
connect( dlg, &QgsAbstractDataSourceWidget::addMeshLayer, this, &QgsDataSourceManagerDialog::addMeshLayer );
// Vector tile
connect( dlg, &QgsAbstractDataSourceWidget::addVectorTileLayer, this, &QgsDataSourceManagerDialog::addVectorTileLayer );
// Virtual
connect( dlg, SIGNAL( replaceVectorLayer( QString, QString, QString, QString ) ),
this, SIGNAL( replaceSelectedVectorLayer( QString, QString, QString, QString ) ) );
connect( dlg, &QgsAbstractDataSourceWidget::replaceVectorLayer,
this, &QgsDataSourceManagerDialog::replaceSelectedVectorLayer );
// Common
connect( dlg, SIGNAL( connectionsChanged() ), this, SIGNAL( connectionsChanged() ) );
connect( this, SIGNAL( providerDialogsRefreshRequested() ), dlg, SLOT( refresh() ) );
connect( dlg, &QgsAbstractDataSourceWidget::connectionsChanged, this, &QgsDataSourceManagerDialog::connectionsChanged );
connect( this, &QgsDataSourceManagerDialog::providerDialogsRefreshRequested, dlg, &QgsAbstractDataSourceWidget::refresh );
// Message
connect( dlg, &QgsAbstractDataSourceWidget::pushMessage, this, [ = ]( const QString & title, const QString & message, const Qgis::MessageLevel level )
{
mMessageBar->pushMessage( title, message, level );
} );
}
void QgsDataSourceManagerDialog::showEvent( QShowEvent *e )