Geopackage fix mixed geometry layers and attributes

In addition to geometry filtered layers for each geometry
type in a mixed-geometry type layer adds a table
layer with all attributes.

Tehre is no perfetc solution here, if the layer is created
with a filter on geometrytype, the attributes are not shown
(this is probably a pre-existing bug), if the layer is added
as is, only the first geometry type is drawn.

With this implementation at least all data (attributes and
geometries) are accessible in some way.

Note that geopackage layers added by DB Manager do not show
all geometries of a mixed type layer.

python/gui/qgsnewgeopackagelayerdialog.sip

@@ -29,6 +29,19 @@ Constructor
 %Docstring
  Sets the ``crs`` value for the new layer in the dialog.
 .. versionadded:: 3.0
+%End
+
+    QString databasePath() const;
+%Docstring
+ Returns the database path
+.. versionadded:: 3.0
+ :rtype: str
+%End
+
+    void setDatabasePath( const QString &path );
+%Docstring
+ Sets the the database ``path``
+.. versionadded:: 3.0
 %End
 
 };

src/gui/qgsnewgeopackagelayerdialog.h

@@ -41,6 +41,18 @@ class GUI_EXPORT QgsNewGeoPackageLayerDialog: public QDialog, private Ui::QgsNew
      */
     void setCrs( const QgsCoordinateReferenceSystem &crs );
 
+    /**
+     * Returns the database path
+     * \since QGIS 3.0
+     */
+    QString databasePath() const { return mDatabaseEdit->text(); }
+
+    /**
+     * Sets the the database \a path
+     * \since QGIS 3.0
+     */
+    void setDatabasePath( const QString &path ) { mDatabaseEdit->setText( path ); }
+
   private slots:
     void on_mAddAttributeButton_clicked();
     void on_mRemoveAttributeButton_clicked();

src/providers/ogr/qgsgeopackageconnection.cpp

@@ -47,7 +47,7 @@ QgsDataSourceUri QgsGeoPackageConnection::uri()
   return uri;
 }
 
-void QgsGeoPackageConnection::setPath( QString &path )
+void QgsGeoPackageConnection::setPath( const QString &path )
 {
   mPath = path;
 }

src/providers/ogr/qgsgeopackageconnection.h

@@ -47,11 +47,11 @@ class QgsGeoPackageConnection : public QObject
     //! \see QgsDataSourceUri
     QgsDataSourceUri uri();
     //! Return the path
-    QString path( ) { return mPath; }
+    QString path( ) const { return mPath; }
     //! Returns the connection name
-    QString name() { return mConnName; }
+    QString name() const { return mConnName; }
     //! Set the \a path fo the connection
-    void setPath( QString &path );
+    void setPath( const QString &path );
     //! Store the connection data in the settings
     void save();
     const static QString SETTINGS_PREFIX;

src/providers/ogr/qgsgeopackagedataitems.cpp

@@ -94,8 +94,25 @@ void QgsGeoPackageRootItem::newConnection()
 {
   // TODO use QgsFileWidget
   QString path = QFileDialog::getOpenFileName( nullptr, tr( "Open GeoPackage" ), "", tr( "GeoPackage Database (*.gpkg)" ) );
+  storeConnection( path );
+}
+
+
+#ifdef HAVE_GUI
+void QgsGeoPackageRootItem::createDatabase()
+{
+  QgsNewGeoPackageLayerDialog dialog( nullptr );
+  dialog.setCrs( QgsProject::instance()->defaultCrsForNewLayers() );
+  if ( dialog.exec() == QDialog::Accepted )
+  {
+    storeConnection( dialog.databasePath() );
+  }
+}
+#endif
+
+bool QgsGeoPackageRootItem::storeConnection( const QString &path )
+{
   QFileInfo fileInfo( path );
-  QString folder = fileInfo.path();
   QString connName = fileInfo.fileName();
   if ( ! path.isEmpty() )
   {
@@ -113,16 +130,10 @@ void QgsGeoPackageRootItem::newConnection()
       connection.setPath( path );
       connection.save();
       refreshConnections();
+      return true;
     }
   }
-}
-
-
-void QgsGeoPackageRootItem::createDatabase()
-{
-  QgsNewGeoPackageLayerDialog dialog( nullptr );
-  dialog.setCrs( QgsProject::instance()->defaultCrsForNewLayers() );
-  dialog.exec();
+  return false;
 }
 
 
@@ -145,36 +156,69 @@ QVector<QgsDataItem *> QgsGeoPackageConnectionItem::createChildren()
   }
   else
   {
+    // Collect mixed-geom layers
+    QMultiMap<int, QStringList> subLayers;
     Q_FOREACH ( const QString &descriptor, layer.dataProvider()->subLayers( ) )
     {
       QStringList pieces = descriptor.split( ':' );
-      QString layerId = pieces[0];
-      QString name = pieces[1];
-      QString featuresCount = pieces[2];
-      QString geometryType = pieces[3];
-      QgsLayerItem::LayerType layerType;
-      layerType = layerTypeFromDb( geometryType );
-      if ( geometryType.contains( QStringLiteral( "Collection" ), Qt::CaseInsensitive ) )
+      subLayers.insert( pieces[0].toInt(), pieces );
+    }
+    int prevIdx = -1;
+    Q_FOREACH ( const int &idx, subLayers.keys( ) )
+    {
+      if ( idx == prevIdx )
       {
-        QgsDebugMsgLevel( QStringLiteral( "Layer %1 is a geometry collection: skipping %2" ).arg( name, mPath ), 3 );
+        continue;
       }
-      else
+      prevIdx = idx;
+      QList<QStringList> values = subLayers.values( idx );
+      for ( int i = 0; i < values.size(); ++i )
       {
-        // example URI:    '/path/gdal_sample_v1.2_no_extensions.gpkg|layerid=7|geometrytype=Point'
+        QStringList pieces = values.at( i );
+        QString layerId = pieces[0];
+        QString name = pieces[1];
+        // QString featuresCount = pieces[2]; // Not used
+        QString geometryType = pieces[3];
+        QgsLayerItem::LayerType layerType;
+        layerType = layerTypeFromDb( geometryType );
+        // example URI for mixed-geoms geoms:    '/path/gdal_sample_v1.2_no_extensions.gpkg|layerid=7|geometrytype=Point'
+        // example URI for mixed-geoms attr table:    '/path/gdal_sample_v1.2_no_extensions.gpkg|layername=MyLayer|layerid=7'
+        // example URI for single geoms:    '/path/gdal_sample_v1.2_no_extensions.gpkg|layerid=6'
         QString uri;
-        // We do not need to add a geometry type for table layers
-        if ( layerType != QgsLayerItem::LayerType::TableLayer )
+        // Check if it's a mixed geometry type
+        if ( i == 0 && values.size() > 1 )
         {
-          uri = QStringLiteral( "%1|layerid=%2|geometrytype=%3" ).arg( mPath, layerId, geometryType );
+          uri = QStringLiteral( "%1|layerid=%2|layername=%3" ).arg( mPath, layerId, name );
+          QgsGeoPackageVectorLayerItem *item = new QgsGeoPackageVectorLayerItem( this, name, mPath, uri, QgsLayerItem::LayerType::TableLayer );
+          children.append( item );
+        }
+        if ( layerType != QgsLayerItem::LayerType::NoType )
+        {
+          if ( geometryType.contains( QStringLiteral( "Collection" ), Qt::CaseInsensitive ) )
+          {
+            QgsDebugMsgLevel( QStringLiteral( "Layer %1 is a geometry collection: skipping %2" ).arg( name, mPath ), 3 );
+          }
+          else
+          {
+            if ( values.size() > 1 )
+            {
+              uri = QStringLiteral( "%1|layerid=%2|geometrytype=%3" ).arg( mPath, layerId, geometryType );
+            }
+            else
+            {
+              uri = QStringLiteral( "%1|layerid=%2" ).arg( mPath, layerId );
+            }
+            QgsGeoPackageVectorLayerItem *item = new QgsGeoPackageVectorLayerItem( this, name, mPath, uri, layerType );
+            QgsDebugMsgLevel( QStringLiteral( "Adding GPKG Vector item %1 %2 %3" ).arg( name, uri, geometryType ), 3 );
+            children.append( item );
+          }
         }
         else
         {
-          uri = QStringLiteral( "%1|layerid=%2" ).arg( mPath, layerId );
+          QgsDebugMsgLevel( QStringLiteral( "Layer type is not a supported GeoPackage Vector layer %1" ).arg( mPath ), 3 );
         }
-        // TODO?: not sure, but if it's a collection, an expandable node would be better?
-        QgsGeoPackageVectorLayerItem *item = new QgsGeoPackageVectorLayerItem( this, name, mPath, uri, layerType );
         QgsDebugMsgLevel( QStringLiteral( "Adding GPKG Vector item %1 %2 %3" ).arg( name, uri, geometryType ), 3 );
-        children.append( item );
+        qDebug() << QStringLiteral( "Adding GPKG Vector item %1 %2 %3" ).arg( name, uri, geometryType );
       }
     }
   }
@@ -210,10 +254,16 @@ QList<QAction *> QgsGeoPackageConnectionItem::actions()
 {
   QList<QAction *> lst;
 
+  QAction *actionDeleteConnection = new QAction( tr( "Remove connection" ), this );
+  connect( actionDeleteConnection, &QAction::triggered, this, &QgsGeoPackageConnectionItem::deleteConnection );
+  lst.append( actionDeleteConnection );
+
+  // Add table to existing DB
+  QAction *actionAddTable = new QAction( tr( "Create a new layer or table..." ), this );
+  connect( actionAddTable, &QAction::triggered, this, &QgsGeoPackageConnectionItem::addTable );
+  lst.append( actionAddTable );
+
 
-  QAction *actiondeleteConnection = new QAction( tr( "Remove connection" ), this );
-  connect( actiondeleteConnection, &QAction::triggered, this, &QgsGeoPackageConnectionItem::deleteConnection );
-  lst.append( actiondeleteConnection );
   return lst;
 }
 #endif
@@ -250,6 +300,29 @@ void QgsGeoPackageConnectionItem::deleteConnection()
   mParent->refreshConnections();
 }
 
+#ifdef HAVE_GUI
+void QgsGeoPackageConnectionItem::addTable()
+{
+  QgsNewGeoPackageLayerDialog dialog( nullptr );
+  QFileInfo fileInfo( mPath );
+  QString connName = fileInfo.fileName();
+  QgsGeoPackageConnection connection( connName );
+  if ( ! connection.path().isEmpty() )
+  {
+    dialog.setDatabasePath( connection.path() );
+    dialog.setCrs( QgsProject::instance()->defaultCrsForNewLayers() );
+    if ( dialog.exec() == QMessageBox::Ok )
+    {
+      mParent->refreshConnections();
+    }
+  }
+  else
+  {
+    QgsDebugMsg( QStringLiteral( "Cannot add Table: connection %1 does not exists or the path is empy!" ).arg( connName ) );
+  }
+}
+#endif
+
 #ifdef HAVE_GUI
 QList<QAction *> QgsGeoPackageAbstractLayerItem::actions()
 {

src/providers/ogr/qgsgeopackagedataitems.h

@@ -76,7 +76,7 @@ class QgsGeoPackageConnectionItem : public QgsDataCollectionItem
 #ifdef HAVE_GUI
     void editConnection();
     void deleteConnection();
-
+    void addTable();
 #endif
 
   protected:
@@ -102,8 +102,11 @@ class QgsGeoPackageRootItem : public QgsDataCollectionItem
 #ifdef HAVE_GUI
     void newConnection();
     void connectionsChanged();
-#endif
     void createDatabase();
+#endif
+
+  private:
+    bool storeConnection( const QString &path );
 
 };