Merge pull request #5022 from boundlessgeo/gpkg-show-attributes

Geopackage fix mixed geometry layers and attributes
qgis · Aug 15, 2017 · 3922efc · 3922efc
2 parents 7d4f81d + 7d074de
commit 3922efc
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 35 deletions.
diff --git a/python/gui/qgsnewgeopackagelayerdialog.sip b/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
 
 };

diff --git a/src/gui/qgsnewgeopackagelayerdialog.h b/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();

diff --git a/src/providers/ogr/qgsgeopackageconnection.cpp b/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;
 }

diff --git a/src/providers/ogr/qgsgeopackageconnection.h b/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;

diff --git a/src/providers/ogr/qgsgeopackagedataitems.cpp b/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()
 {

diff --git a/src/providers/ogr/qgsgeopackagedataitems.h b/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 );
 
 };