Add unit test for importing/exporting bookmarks

python/core/auto_generated/qgsbookmarkmanager.sip.in

@@ -240,9 +240,9 @@ Moves the bookmark with matching ``id`` from this manager to a ``destination`` m
 Returns ``True`` if the bookmark was successfully moved.
 %End
 
-    bool exportToFile( const QString &path ) const;
+    static bool exportToFile( const QString &path, QList<const QgsBookmarkManager *> &managers );
 %Docstring
-Exports the bookmarks to an xml file at the specified ``path``.
+Exports all bookmarks from a list of ``managers`` to an xml file at the specified ``path``.
 
 Returns ``True`` if the export was successful.
 

src/app/qgsbookmarks.cpp

@@ -283,8 +283,16 @@ void QgsBookmarks::exportToXml()
     fileName += QLatin1String( ".xml" );
   }
 
-  // TODO - how to handle project bookmarks too?
+  if ( !QgsBookmarkManager::exportToFile( fileName, QList< const QgsBookmarkManager * >() << QgsApplication::bookmarkManager()
-  QgsApplication::bookmarkManager()->exportToFile( fileName );
+                                          << QgsProject::instance()->bookmarkManager() ) )
+  {
+    QgisApp::instance()->messageBar()->pushWarning( tr( "Export Bookmarks" ), tr( "Error exporting bookmark file" ) );
+  }
+  else
+  {
+    QgisApp::instance()->messageBar()->pushSuccess( tr( "Export Bookmarks" ), tr( "Successfully exported bookmarks to <a href=\"%1\">%2</a>" )
+        .arg( QUrl::fromLocalFile( fileName ).toString(), QDir::toNativeSeparators( fileName ) ) );
+  }
 
   settings.setValue( QStringLiteral( "Windows/Bookmarks/LastUsedDirectory" ), QFileInfo( fileName ).path() );
 }
@@ -577,7 +585,7 @@ QVariant QgsMergedBookmarksTableModel::data( const QModelIndex &index, int role
     }
     else // ... it is a project stored bookmark
     {
-      value = mProjectTableModel.data( this->index( 0, index.column() ), role );
+      value = mProjectTableModel.data( this->index( index.row() - mQgisTableModel.rowCount(), index.column() ), role );
     }
   }
   return value;

src/core/qgsbookmarkmanager.cpp

@@ -303,7 +303,7 @@ bool QgsBookmarkManager::moveBookmark( const QString &id, QgsBookmarkManager *de
   return ok;
 }
 
-bool QgsBookmarkManager::exportToFile( const QString &path ) const
+bool QgsBookmarkManager::exportToFile( const QString &path, QList<const QgsBookmarkManager *> &managers )
 {
   // note - we don't use the other writeXml implementation, to maintain older format compatibility
   QDomDocument doc( QStringLiteral( "qgis_bookmarks" ) );
@@ -319,39 +319,43 @@ bool QgsBookmarkManager::exportToFile( const QString &path ) const
       << QStringLiteral( "ymax" )
       << QStringLiteral( "sr_id" );
 
-  for ( const QgsBookmark &b : mBookmarks )
+  for ( const QgsBookmarkManager *manager : managers )
   {
-    QDomElement bookmark = doc.createElement( QStringLiteral( "bookmark" ) );
+    const QList< QgsBookmark > bookmarks = manager->bookmarks();
-    root.appendChild( bookmark );
+    for ( const QgsBookmark &b : bookmarks )
+    {
+      QDomElement bookmark = doc.createElement( QStringLiteral( "bookmark" ) );
+      root.appendChild( bookmark );
 
-    QDomElement id = doc.createElement( QStringLiteral( "id" ) );
+      QDomElement id = doc.createElement( QStringLiteral( "id" ) );
-    id.appendChild( doc.createTextNode( b.id() ) );
+      id.appendChild( doc.createTextNode( b.id() ) );
-    bookmark.appendChild( id );
+      bookmark.appendChild( id );
 
-    QDomElement name = doc.createElement( QStringLiteral( "name" ) );
+      QDomElement name = doc.createElement( QStringLiteral( "name" ) );
-    name.appendChild( doc.createTextNode( b.name() ) );
+      name.appendChild( doc.createTextNode( b.name() ) );
-    bookmark.appendChild( name );
+      bookmark.appendChild( name );
 
-    QDomElement group = doc.createElement( QStringLiteral( "project" ) );
+      QDomElement group = doc.createElement( QStringLiteral( "project" ) );
-    group.appendChild( doc.createTextNode( b.group() ) );
+      group.appendChild( doc.createTextNode( b.group() ) );
-    bookmark.appendChild( group );
+      bookmark.appendChild( group );
 
-    QDomElement xMin = doc.createElement( QStringLiteral( "xmin" ) );
+      QDomElement xMin = doc.createElement( QStringLiteral( "xmin" ) );
-    xMin.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().xMinimum() ) ) );
+      xMin.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().xMinimum() ) ) );
-    bookmark.appendChild( xMin );
+      bookmark.appendChild( xMin );
-    QDomElement yMin = doc.createElement( QStringLiteral( "ymin" ) );
+      QDomElement yMin = doc.createElement( QStringLiteral( "ymin" ) );
-    yMin.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().yMinimum() ) ) );
+      yMin.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().yMinimum() ) ) );
-    bookmark.appendChild( yMin );
+      bookmark.appendChild( yMin );
-    QDomElement xMax = doc.createElement( QStringLiteral( "xmax" ) );
+      QDomElement xMax = doc.createElement( QStringLiteral( "xmax" ) );
-    xMax.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().xMaximum() ) ) );
+      xMax.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().xMaximum() ) ) );
-    bookmark.appendChild( xMax );
+      bookmark.appendChild( xMax );
-    QDomElement yMax = doc.createElement( QStringLiteral( "ymax" ) );
+      QDomElement yMax = doc.createElement( QStringLiteral( "ymax" ) );
-    yMax.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().yMaximum() ) ) );
+      yMax.appendChild( doc.createTextNode( qgsDoubleToString( b.extent().yMaximum() ) ) );
-    bookmark.appendChild( yMax );
+      bookmark.appendChild( yMax );
 
-    QDomElement crs = doc.createElement( QStringLiteral( "sr_id" ) );
+      QDomElement crs = doc.createElement( QStringLiteral( "sr_id" ) );
-    crs.appendChild( doc.createTextNode( QString::number( b.extent().crs().srsid() ) ) );
+      crs.appendChild( doc.createTextNode( QString::number( b.extent().crs().srsid() ) ) );
-    bookmark.appendChild( crs );
+      bookmark.appendChild( crs );
+    }
   }
 
   QFile f( path );

src/core/qgsbookmarkmanager.h

@@ -246,13 +246,13 @@ class CORE_EXPORT QgsBookmarkManager : public QObject
     bool moveBookmark( const QString &id, QgsBookmarkManager *destination );
 
     /**
-     * Exports the bookmarks to an xml file at the specified \a path.
+     * Exports all bookmarks from a list of \a managers to an xml file at the specified \a path.
      *
      * Returns TRUE if the export was successful.
      *
      * \see importFile()
      */
-    bool exportToFile( const QString &path ) const;
+    static bool exportToFile( const QString &path, QList<const QgsBookmarkManager *> &managers );
 
 
     /**

tests/src/python/test_qgsbookmarkmanager.py

@@ -13,7 +13,7 @@ __copyright__ = 'Copyright 2019, The QGIS Project'
 import qgis  # NOQA
 import os
 
-from qgis.PyQt.QtCore import QCoreApplication, QLocale, QTemporaryDir
+from qgis.PyQt.QtCore import QCoreApplication, QLocale, QTemporaryDir, QEvent
 from qgis.PyQt.QtXml import QDomDocument
 
 from qgis.core import (QgsBookmark,
@@ -398,6 +398,7 @@ class TestQgsBookmarkManager(unittest.TestCase):
 
         # destroy manager, causes write to disk
         manager.deleteLater()
+        QCoreApplication.sendPostedEvents(None, QEvent.DeferredDelete)
         del manager
 
         # create another new manager with same key, should contain existing bookmarks
@@ -487,6 +488,56 @@ class TestQgsBookmarkManager(unittest.TestCase):
         self.assertEqual(manager.bookmarks(), [b2, b3])
         self.assertEqual(manager2.bookmarks(), [b])
 
+    def testExportImport(self):
+        p = QgsProject()
+        manager = QgsBookmarkManager.createProjectBasedManager(p)
+        manager2 = QgsBookmarkManager.createProjectBasedManager(p)
+        manager3 = QgsBookmarkManager.createProjectBasedManager(p)
+
+        tmpDir = QTemporaryDir()
+        tmpFile = "{}/bookmarks.xml".format(tmpDir.path())
+
+        # no managers
+        self.assertTrue(QgsBookmarkManager.exportToFile(tmpFile, []))
+        self.assertTrue(manager3.importFromFile(tmpFile))
+        self.assertFalse(manager3.bookmarks())
+
+        # no bookmarks
+        self.assertTrue(QgsBookmarkManager.exportToFile(tmpFile, [manager]))
+        self.assertTrue(manager3.importFromFile(tmpFile))
+        self.assertFalse(manager3.bookmarks())
+
+        # add a bunch of bookmarks
+        b = QgsBookmark()
+        b.setId('1')
+        b.setName('b1')
+        b.setExtent(QgsReferencedRectangle(QgsRectangle(11, 21, 31, 41), QgsCoordinateReferenceSystem('EPSG:4326')))
+
+        b2 = QgsBookmark()
+        b2.setId('2')
+        b2.setName('b2')
+        b2.setExtent(QgsReferencedRectangle(QgsRectangle(12, 22, 32, 42), QgsCoordinateReferenceSystem('EPSG:4326')))
+
+        b3 = QgsBookmark()
+        b3.setId('3')
+        b3.setName('b3')
+        b3.setExtent(QgsReferencedRectangle(QgsRectangle(32, 32, 33, 43), QgsCoordinateReferenceSystem('EPSG:4326')))
+
+        manager.addBookmark(b)
+        manager.addBookmark(b2)
+        manager2.addBookmark(b3)
+
+        # export one manager's bookmarks
+        self.assertTrue(QgsBookmarkManager.exportToFile(tmpFile, [manager]))
+        self.assertTrue(manager3.importFromFile(tmpFile))
+        self.assertEqual([(b.name(), b.extent()) for b in manager3.bookmarks()], [(b.name(), b.extent()) for b in [b, b2]])
+
+        manager3.clear()
+        # export both manager's bookmarks
+        self.assertTrue(QgsBookmarkManager.exportToFile(tmpFile, [manager, manager2]))
+        self.assertTrue(manager3.importFromFile(tmpFile))
+        self.assertEqual([(b.name(), b.extent()) for b in manager3.bookmarks()], [(b.name(), b.extent()) for b in [b, b2, b3]])
+
 
 if __name__ == '__main__':
     unittest.main()