Rework layer dependencies to work also with layer definition files

python/core/qgslayerdefinition.sip

@@ -19,5 +19,26 @@ class QgsLayerDefinition
     static bool exportLayerDefinition( QString path, const QList<QgsLayerTreeNode*>& selectedTreeNodes, QString &errorMessage /Out/ );
     /** Export the selected layer tree nodes to a QLR-XML document */
     static bool exportLayerDefinition( QDomDocument doc, const QList<QgsLayerTreeNode*>& selectedTreeNodes, QString &errorMessage, const QString& relativeBasePath = QString::null );
+
+    /**
+     * Class used to work with layer dependencies stored in a XML project or layer definition file
+     */
+    class DependencySorter
+    {
+    public:
+      /** Constructor
+       * @param doc The XML document containing maplayer elements
+       */
+      DependencySorter( QDomDocument doc );
+
+      /** Get the layer nodes in an order where they can be loaded incrementally without dependency break */
+      QVector<QDomNode> sortedLayerNodes() const;
+
+      /** Whether some cyclic dependency has been detected */
+      bool hasCycle() const;
+
+      /** Whether some dependency is missing */
+      bool hasMissingDependency() const;
+    };
 };
 

python/core/qgsmaplayer.sip

@@ -317,7 +317,7 @@ class QgsMapLayer : QObject
 
     /** Creates a new layer from a layer defininition document
     */
-    static QList<QgsMapLayer*> fromLayerDefinition( QDomDocument& document );
+    static QList<QgsMapLayer*> fromLayerDefinition( QDomDocument& document, bool addToRegistry = false, bool addToLegend = false );
     static QList<QgsMapLayer*> fromLayerDefinitionFile( const QString &qlrfile );
 
     /** Set a custom property for layer. Properties are stored in a map and saved in project file. */

src/core/qgslayerdefinition.cpp

@@ -40,6 +40,33 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsLayerTreeGrou
 
   QgsLayerTreeGroup *root = new QgsLayerTreeGroup();
 
+  // reorder maplayer nodes based on dependencies
+  // dependencies have to be resolved before IDs get changed
+  DependencySorter depSorter( doc );
+  if ( !depSorter.hasMissingDependency() )
+  {
+    QVector<QDomNode> sortedLayerNodes = depSorter.sortedLayerNodes();
+    QVector<QDomNode> clonedSorted;
+    foreach ( QDomNode node, sortedLayerNodes )
+    {
+      clonedSorted << node.cloneNode();
+    }
+    QDomNode layersNode = doc.elementsByTagName( "maplayers" ).at( 0 );
+    // remove old children
+    QDomNodeList childNodes = layersNode.childNodes();
+    for ( int i = 0; i < childNodes.size(); i++ )
+    {
+      layersNode.removeChild( childNodes.at( i ) );
+    }
+    // replace with new ones
+    foreach ( QDomNode node, clonedSorted )
+    {
+      layersNode.appendChild( node );
+    }
+  }
+  // if a dependency is missing, we still try to load layers, since dependencies may already be loaded
+
+  // IDs of layers should be changed otherwise we may have more then one layer with the same id
   // We have to replace the IDs before we load them because it's too late once they are loaded
   QDomNodeList ids = doc.elementsByTagName( "id" );
   for ( int i = 0; i < ids.size(); ++i )
@@ -85,8 +112,7 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsLayerTreeGrou
     loadInLegend = false;
   }
 
-  QList<QgsMapLayer*> layers = QgsMapLayer::fromLayerDefinition( doc );
-  QgsMapLayerRegistry::instance()->addMapLayers( layers, loadInLegend );
+  QList<QgsMapLayer*> layers = QgsMapLayer::fromLayerDefinition( doc, /*addToRegistry*/ true, loadInLegend );
 
   // Now that all layers are loaded, refresh the vectorjoins to get the joined fields
   Q_FOREACH ( QgsMapLayer* layer, layers )
@@ -159,3 +185,93 @@ bool QgsLayerDefinition::exportLayerDefinition( QDomDocument doc, const QList<Qg
   qgiselm.appendChild( layerselm );
   return true;
 }
+
+QgsLayerDefinition::DependencySorter::DependencySorter( QDomDocument doc ) :
+    mHasCycle( false ), mHasMissingDependency( false )
+{
+  // Determine a loading order of layers based on a graph of dependencies
+  QMap< QString, QVector< QString > > dependencies;
+  QVector<QString> sortedLayers;
+  QList< QPair<QString, QDomNode> > layersToSort;
+
+  QDomNodeList nl = doc.elementsByTagName( "maplayer" );
+  for ( int i = 0; i < nl.count(); i++ )
+  {
+    QVector<QString> deps;
+    QDomNode node = nl.item( i );
+    QDomElement element = node.toElement();
+
+    QString id = node.namedItem( "id" ).toElement().text();
+
+    // dependencies for this layer
+    QDomElement layerDependenciesElem = node.firstChildElement( "layerDependencies" );
+    if ( !layerDependenciesElem.isNull() )
+    {
+      QDomNodeList dependencyList = layerDependenciesElem.elementsByTagName( "layer" );
+      for ( int j = 0; j < dependencyList.size(); ++j )
+      {
+        QDomElement depElem = dependencyList.at( j ).toElement();
+        deps << depElem.attribute( "id" );
+      }
+    }
+    dependencies[id] = deps;
+
+    if ( deps.empty() )
+    {
+      sortedLayers << id;
+      mSortedLayerNodes << node;
+    }
+    else
+      layersToSort << qMakePair( id, node );
+  }
+
+  // check that all dependencies are present
+  foreach ( QString id, dependencies.keys() )
+  {
+    foreach ( QString depId, dependencies[id] )
+    {
+      if ( !dependencies.contains( depId ) )
+      {
+        // some dependencies are not satisfied
+        mHasMissingDependency = true;
+        return;
+      }
+    }
+  }
+
+  // cycles should be very rare, since layers with cyclic dependencies may only be created by
+  // manually modifying the project file
+  mHasCycle = false;
+
+  while ( !layersToSort.empty() && !mHasCycle )
+  {
+    QList< QPair<QString, QDomNode> >::iterator it = layersToSort.begin();
+    while ( it != layersToSort.end() )
+    {
+      QString idToSort = it->first;
+      QDomNode node = it->second;
+      mHasCycle = true;
+      bool resolved = true;
+      foreach ( QString dep, dependencies[idToSort] )
+      {
+        if ( !sortedLayers.contains( dep ) )
+        {
+          resolved = false;
+          break;
+        }
+      }
+      if ( resolved ) // dependencies for this layer are resolved
+      {
+        sortedLayers << idToSort;
+        mSortedLayerNodes << node;
+        it = layersToSort.erase( it ); // erase and go to the next
+        mHasCycle = false;
+      }
+      else
+      {
+        it++;
+      }
+    }
+  }
+}
+

src/core/qgslayerdefinition.h

@@ -21,6 +21,32 @@ class CORE_EXPORT QgsLayerDefinition
     static bool exportLayerDefinition( QString path, const QList<QgsLayerTreeNode*>& selectedTreeNodes, QString &errorMessage );
     /** Export the selected layer tree nodes to a QLR-XML document */
     static bool exportLayerDefinition( QDomDocument doc, const QList<QgsLayerTreeNode*>& selectedTreeNodes, QString &errorMessage, const QString& relativeBasePath = QString::null );
+
+    /**
+     * Class used to work with layer dependencies stored in a XML project or layer definition file
+     */
+    class CORE_EXPORT DependencySorter
+    {
+      public:
+        /** Constructor
+         * @param doc The XML document containing maplayer elements
+         */
+        DependencySorter( QDomDocument doc );
+
+        /** Get the layer nodes in an order where they can be loaded incrementally without dependency break */
+        QVector<QDomNode> sortedLayerNodes() const { return mSortedLayerNodes; }
+
+        /** Whether some cyclic dependency has been detected */
+        bool hasCycle() const { return mHasCycle; }
+
+        /** Whether some dependency is missing */
+        bool hasMissingDependency() const { return mHasMissingDependency; }
+
+      private:
+        QVector<QDomNode> mSortedLayerNodes;
+        bool mHasCycle;
+        bool mHasMissingDependency;
+    };
 };
 
 #endif // QGSLAYERDEFINITION_H

src/core/qgsmaplayer.cpp

@@ -47,6 +47,7 @@
 #include "qgsrectangle.h"
 #include "qgsvectorlayer.h"
 #include "qgsvectordataprovider.h"
+#include "qgsmaplayerregistry.h"
 
 
 QgsMapLayer::QgsMapLayer( QgsMapLayer::LayerType type,
@@ -788,7 +789,7 @@ QDomDocument QgsMapLayer::asLayerDefinition( const QList<QgsMapLayer *>& layers,
   return doc;
 }
 
-QList<QgsMapLayer*> QgsMapLayer::fromLayerDefinition( QDomDocument& document )
+QList<QgsMapLayer*> QgsMapLayer::fromLayerDefinition( QDomDocument& document, bool addToRegistry, bool addToLegend )
 {
   QList<QgsMapLayer*> layers;
   QDomNodeList layernodes = document.elementsByTagName( "maplayer" );
@@ -820,7 +821,11 @@ QList<QgsMapLayer*> QgsMapLayer::fromLayerDefinition( QDomDocument& document )
 
     bool ok = layer->readLayerXML( layerElem );
     if ( ok )
+    {
       layers << layer;
+      if ( addToRegistry )
+        QgsMapLayerRegistry::instance()->addMapLayer( layer, addToLegend );
+    }
   }
   return layers;
 }

src/core/qgsmaplayer.h

@@ -333,7 +333,7 @@ class CORE_EXPORT QgsMapLayer : public QObject
 
     /** Creates a new layer from a layer defininition document
     */
-    static QList<QgsMapLayer*> fromLayerDefinition( QDomDocument& document );
+    static QList<QgsMapLayer*> fromLayerDefinition( QDomDocument& document, bool addToRegistry = false, bool addToLegend = false );
     static QList<QgsMapLayer*> fromLayerDefinitionFile( const QString &qlrfile );
 
     /** Set a custom property for layer. Properties are stored in a map and saved in project file. */

src/core/qgsproject.cpp

@@ -35,6 +35,7 @@
 #include "qgsrelationmanager.h"
 #include "qgsvectorlayer.h"
 #include "qgsvisibilitypresetcollection.h"
+#include "qgslayerdefinition.h"
 
 #include <QApplication>
 #include <QFileInfo>
@@ -672,83 +673,19 @@ QPair< bool, QList<QDomNode> > QgsProject::_getMapLayers( QDomDocument const &do
 
   emit layerLoaded( 0, nl.count() );
 
-  // Determine a loading order of layers based on a graph of dependencies
-  QMap< QString, QVector< QString > > dependencies;
-  QVector<QString> sortedLayers;
-  QMap<QString, int> layerIdIdx;
-  QList<QString> layersToSort;
-
-  for ( int i = 0; i < nl.count(); i++ )
-  {
-    QVector<QString> deps;
-    QDomNode node = nl.item( i );
-    QDomElement element = node.toElement();
-
-    QString id = node.namedItem( "id" ).toElement().text();
-
-    // dependencies for this layer
-    QDomElement layerDependenciesElem = node.firstChildElement( "layerDependencies" );
-    if ( !layerDependenciesElem.isNull() )
-    {
-      QDomNodeList dependencyList = layerDependenciesElem.elementsByTagName( "layer" );
-      for ( int j = 0; j < dependencyList.size(); ++j )
-      {
-        QDomElement depElem = dependencyList.at( j ).toElement();
-        deps << depElem.attribute( "id" );
-      }
-    }
-    dependencies[id] = deps;
-
-    if ( deps.empty() )
-      sortedLayers << id;
-    else
-      layersToSort << id;
-    layerIdIdx[id] = i;
-  }
-
-  bool hasCycle = false;
-  while ( !layersToSort.empty() && !hasCycle )
-  {
-    QList<QString>::iterator it = layersToSort.begin();
-    while ( it != layersToSort.end() )
-    {
-      hasCycle = true;
-      bool resolved = true;
-      foreach ( QString dep, dependencies[*it] )
-      {
-        if ( !sortedLayers.contains( dep ) )
-        {
-          resolved = false;
-          break;
-        }
-      }
-      if ( resolved ) // dependencies for this layer are resolved
-      {
-        sortedLayers << *it;
-        it = layersToSort.erase( it ); // erase and go to the next
-        hasCycle = false;
-      }
-      else
-      {
-        it++;
-      }
-    }
-  }
-
-  if ( hasCycle )
-  {
-    // should not happen, since layers with cyclic dependencies may only be created by
-    // manually modifying the project file
+  // order layers based on their dependencies
+  QgsLayerDefinition::DependencySorter depSorter( doc );
+  if ( depSorter.hasCycle() || depSorter.hasMissingDependency() )
     return qMakePair( false, QList<QDomNode>() );
-  }
+
+  QVector<QDomNode> sortedLayerNodes = depSorter.sortedLayerNodes();
 
   // Collect vector layers with joins.
   // They need to refresh join caches and symbology infos after all layers are loaded
   QList< QPair< QgsVectorLayer*, QDomElement > > vLayerList;
-  foreach ( QString id, sortedLayers )
+  int i = 0;
+  foreach ( QDomNode node, sortedLayerNodes )
   {
-    int i = layerIdIdx[id];
-    QDomNode node = nl.item( i );
     QDomElement element = node.toElement();
 
     QString name = node.namedItem( "layername" ).toElement().text();
@@ -768,6 +705,7 @@ QPair< bool, QList<QDomNode> > QgsProject::_getMapLayers( QDomDocument const &do
       }
     }
     emit layerLoaded( i + 1, nl.count() );
+    i++;
   }
 
   // Update field map of layers with joins and create join caches if necessary

tests/src/python/CMakeLists.txt

@@ -67,6 +67,7 @@ ADD_PYTHON_TEST(PyQgsZonalStatistics test_qgszonalstatistics.py)
 ADD_PYTHON_TEST(PyQgsMapLayerRegistry test_qgsmaplayerregistry.py)
 ADD_PYTHON_TEST(PyQgsVirtualLayerProvider test_provider_virtual.py)
 ADD_PYTHON_TEST(PyQgsVirtualLayerDefinition test_qgsvirtuallayerdefinition.py)
+ADD_PYTHON_TEST(PyQgsLayerDefinition test_qgslayerdefinition.py)
 
 IF (NOT WIN32)
   ADD_PYTHON_TEST(PyQgsLogger test_qgslogger.py)

tests/src/python/test_provider_virtual.py

@@ -29,7 +29,7 @@
                        QgsErrorMessage,
                        QgsProviderRegistry,
                        QgsVirtualLayerDefinition,
-                       QgsWKBTypes
+                       QgsWKBTypes,
                        QgsProject
                        )