Keep invalid relations and update them when the data source changes

Added a check for layer.isValid in relation.isValid, keep relations in the manager even if they are not valid and connect dataSourceChanged with updateRelationStatus
qgis · Nov 5, 2018 · 0cd21c9 · 0cd21c9
1 parent 2bd90da
commit 0cd21c9
Show file tree

Hide file tree

Showing 8 changed files with 142 additions and 7 deletions.
diff --git a/python/core/auto_generated/qgsrelation.sip.in b/python/core/auto_generated/qgsrelation.sip.in
@@ -300,6 +300,14 @@ Gets the referenced field counterpart given a referencing field.
 Gets the referencing field counterpart given a referenced field.
 
 .. versionadded:: 3.0
+%End
+
+    void updateRelationStatus();
+%Docstring
+Updates the validity status of this relation.
+Will be called internally whenever a member is changed.
+
+.. versionadded:: 3.6
 %End
 
 };

diff --git a/python/core/auto_generated/qgsrelationmanager.sip.in b/python/core/auto_generated/qgsrelationmanager.sip.in
@@ -134,6 +134,13 @@ This signal is emitted when the relations were loaded after reading a project
 Emitted when relations are added or removed to the manager.
 
 .. versionadded:: 2.5
+%End
+
+  public slots:
+
+    void updateRelationsStatus();
+%Docstring
+Updates relations status
 %End
 
 };

diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -368,6 +368,20 @@ QgsProject::QgsProject( QObject *parent )
   connect( mLayerStore.get(), &QgsMapLayerStore::layerWasAdded, this, &QgsProject::layerWasAdded );
   if ( QgsApplication::instance() )
     connect( QgsApplication::instance(), &QgsApplication::requestForTranslatableObjects, this, &QgsProject::registerTranslatableObjects );
+  connect( mLayerStore.get(), static_cast<void ( QgsMapLayerStore::* )( const QList<QgsMapLayer *> & )>( &QgsMapLayerStore::layersWillBeRemoved ),
+           [ & ]( const QList<QgsMapLayer *> &layers )
+  {
+    for ( const auto &layer : layers )
+      disconnect( layer, &QgsMapLayer::dataSourceChanged, mRelationManager, &QgsRelationManager::updateRelationsStatus );
+  }
+         );
+  connect( mLayerStore.get(), static_cast<void ( QgsMapLayerStore::* )( const QList<QgsMapLayer *> & )>( &QgsMapLayerStore::layersAdded ),
+           [ & ]( const QList<QgsMapLayer *> &layers )
+  {
+    for ( const auto &layer : layers )
+      connect( layer, &QgsMapLayer::dataSourceChanged, mRelationManager, &QgsRelationManager::updateRelationsStatus );
+  }
+         );
 }
 
 

diff --git a/src/core/qgsrelation.cpp b/src/core/qgsrelation.cpp
@@ -325,7 +325,7 @@ QgsAttributeList QgsRelation::referencingFields() const
 
 bool QgsRelation::isValid() const
 {
-  return d->mValid && !d->mReferencingLayer.isNull() && !d->mReferencedLayer.isNull();
+  return d->mValid && !d->mReferencingLayer.isNull() && !d->mReferencedLayer.isNull() && d->mReferencingLayer.data()->isValid() && d->mReferencedLayer.data()->isValid();
 }
 
 bool QgsRelation::hasEqualDefinition( const QgsRelation &other ) const

diff --git a/src/core/qgsrelation.h b/src/core/qgsrelation.h
@@ -366,14 +366,16 @@ class CORE_EXPORT QgsRelation
      */
     Q_INVOKABLE QString resolveReferencingField( const QString &referencedField ) const;
 
-  private:
-
     /**
      * Updates the validity status of this relation.
      * Will be called internally whenever a member is changed.
+     *
+     * \since QGIS 3.6
      */
     void updateRelationStatus();
 
+  private:
+
     mutable QExplicitlySharedDataPointer<QgsRelationPrivate> d;
 };
 

diff --git a/src/core/qgsrelationmanager.cpp b/src/core/qgsrelationmanager.cpp
@@ -50,16 +50,23 @@ QMap<QString, QgsRelation> QgsRelationManager::relations() const
 
 void QgsRelationManager::addRelation( const QgsRelation &relation )
 {
-  if ( !relation.isValid() )
-    return;
-
   mRelations.insert( relation.id(), relation );
 
   if ( mProject )
     mProject->setDirty( true );
   emit changed();
 }
 
+
+void QgsRelationManager::updateRelationsStatus()
+{
+  for ( auto relation : mRelations )
+  {
+    relation.updateRelationStatus();
+  }
+}
+
+
 void QgsRelationManager::removeRelation( const QString &id )
 {
   mRelations.remove( id );

diff --git a/src/core/qgsrelationmanager.h b/src/core/qgsrelationmanager.h
@@ -141,6 +141,13 @@ class CORE_EXPORT QgsRelationManager : public QObject
      */
     void changed();
 
+  public slots:
+
+    /**
+     * Updates relations status
+     */
+    void updateRelationsStatus();
+
   private slots:
     void readProject( const QDomDocument &doc, QgsReadWriteContext &context );
     void writeProject( QDomDocument &doc );

diff --git a/tests/src/python/test_qgsprojectbadlayers.py b/tests/src/python/test_qgsprojectbadlayers.py
@@ -39,9 +39,15 @@
 
 class TestQgsProjectBadLayers(unittest.TestCase):
 
+    def setUp(self):
+        p = QgsProject.instance()
+        p.removeAllMapLayers()
+
     def test_project_roundtrip(self):
-        temp_dir = QTemporaryDir()
+        """Tests that a project with bad layers can be saved without loosing them"""
+
         p = QgsProject.instance()
+        temp_dir = QTemporaryDir()
         for ext in ('shp', 'dbf', 'shx', 'prj'):
             copyfile(os.path.join(TEST_DATA_DIR, 'lines.%s' % ext), os.path.join(temp_dir.path(), 'lines.%s' % ext))
         copyfile(os.path.join(TEST_DATA_DIR, 'raster', 'band1_byte_ct_epsg4326.tif'), os.path.join(temp_dir.path(), 'band1_byte_ct_epsg4326.tif'))
@@ -112,6 +118,90 @@ def test_project_roundtrip(self):
         self.assertTrue(raster.isValid())
         self.assertTrue(raster_copy.isValid())
 
+    def test_project_relations(self):
+        """Tests that a project with bad layers and relations can be saved without loosing the relations"""
+
+        temp_dir = QTemporaryDir()
+        p = QgsProject.instance()
+        for ext in ('qgs', 'gpkg'):
+            copyfile(os.path.join(TEST_DATA_DIR, 'projects', 'relation_reference_test.%s' % ext), os.path.join(temp_dir.path(), 'relation_reference_test.%s' % ext))
+
+        # Load the good project
+        project_path = os.path.join(temp_dir.path(), 'relation_reference_test.qgs')
+        self.assertTrue(p.read(project_path))
+        point_a = list(p.mapLayersByName('point_a'))[0]
+        point_b = list(p.mapLayersByName('point_b'))[0]
+        point_a_source = point_a.publicSource()
+        point_b_source = point_b.publicSource()
+        self.assertTrue(point_a.isValid())
+        self.assertTrue(point_b.isValid())
+
+        # Check relations
+        def _check_relations():
+            relation = list(p.relationManager().relations().values())[0]
+            self.assertTrue(relation.isValid())
+            self.assertEqual(relation.referencedLayer().id(), point_b.id())
+            self.assertEqual(relation.referencingLayer().id(), point_a.id())
+
+        _check_relations()
+
+        # Now build a bad project
+        bad_project_path = os.path.join(temp_dir.path(), 'relation_reference_test_bad.qgs')
+        with open(project_path, 'r') as infile:
+            with open(bad_project_path, 'w+') as outfile:
+                outfile.write(infile.read().replace('./relation_reference_test.gpkg', './relation_reference_test-BAD_SOURCE.gpkg'))
+
+        # Load the bad project
+        self.assertTrue(p.read(bad_project_path))
+        point_a = list(p.mapLayersByName('point_a'))[0]
+        point_b = list(p.mapLayersByName('point_b'))[0]
+        self.assertFalse(point_a.isValid())
+        self.assertFalse(point_b.isValid())
+
+        # This fails because relations are not valid anymore
+        with self.assertRaises(AssertionError):
+            _check_relations()
+
+        # Changing data source, relations should be restored:
+        point_a.setDataSource(point_a_source, 'point_a', 'ogr')
+        point_b.setDataSource(point_b_source, 'point_b', 'ogr')
+        self.assertTrue(point_a.isValid())
+        self.assertTrue(point_b.isValid())
+
+        # Check if relations were restored
+        _check_relations()
+
+        # Reload the bad project
+        self.assertTrue(p.read(bad_project_path))
+        point_a = list(p.mapLayersByName('point_a'))[0]
+        point_b = list(p.mapLayersByName('point_b'))[0]
+        self.assertFalse(point_a.isValid())
+        self.assertFalse(point_b.isValid())
+
+        # This fails because relations are not valid anymore
+        with self.assertRaises(AssertionError):
+            _check_relations()
+
+        # Save the bad project
+        bad_project_path2 = os.path.join(temp_dir.path(), 'relation_reference_test_bad2.qgs')
+        p.write(bad_project_path2)
+
+        # Now fix the bad project
+        bad_project_path_fixed = os.path.join(temp_dir.path(), 'relation_reference_test_bad_fixed.qgs')
+        with open(bad_project_path2, 'r') as infile:
+            with open(bad_project_path_fixed, 'w+') as outfile:
+                outfile.write(infile.read().replace('./relation_reference_test-BAD_SOURCE.gpkg', './relation_reference_test.gpkg'))
+
+        # Load the fixed project
+        self.assertTrue(p.read(bad_project_path_fixed))
+        point_a = list(p.mapLayersByName('point_a'))[0]
+        point_b = list(p.mapLayersByName('point_b'))[0]
+        point_a_source = point_a.publicSource()
+        point_b_source = point_b.publicSource()
+        self.assertTrue(point_a.isValid())
+        self.assertTrue(point_b.isValid())
+        _check_relations()
+
 
 if __name__ == '__main__':
     unittest.main()