From d173b70b5335b93dbf049a3f40579fcb032dc8d6 Mon Sep 17 00:00:00 2001 From: signedav Date: Thu, 22 Nov 2018 12:52:51 +0100 Subject: [PATCH 1/5] leave last attribute empty instead of first because though fid appears to be the first field it's added in the end and has the last index. fixes #20276 --- src/core/qgsofflineediting.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/qgsofflineediting.cpp b/src/core/qgsofflineediting.cpp index 156d13709f19..99d2ff62ec3e 100644 --- a/src/core/qgsofflineediting.cpp +++ b/src/core/qgsofflineediting.cpp @@ -763,11 +763,9 @@ QgsVectorLayer *QgsOfflineEditing::copyVectorLayer( QgsVectorLayer *layer, sqlit // NOTE: SpatiaLite provider ignores position of geometry column // fill gap in QgsAttributeMap if geometry column is not last (WORKAROUND) QgsAttributes attrs = f.attributes(); - int column = 0; // on GPKG newAttrs has an addition FID attribute, so we have to add a dummy in the original set - if ( containerType == GPKG ) - column++; - QgsAttributes newAttrs( attrs.count() + column ); + QgsAttributes newAttrs( containerType == GPKG ? attrs.count() + 1 : attrs.count() ); + int column = 0; for ( int it = 0; it < attrs.count(); ++it ) { newAttrs[column++] = attrs.at( it ); From 0f653179ffcb73b5e9b4990592f759e4ea2d023f Mon Sep 17 00:00:00 2001 From: signedav Date: Mon, 3 Dec 2018 15:23:48 +0100 Subject: [PATCH 2/5] handle fid attribute in attrLookup on synchronization and use column++ again on iteration through attributes on copyVectorLayer fixes #20276 especially the not reported issue with synchronization --- src/core/qgsofflineediting.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/qgsofflineediting.cpp b/src/core/qgsofflineediting.cpp index 99d2ff62ec3e..09f40e367b93 100644 --- a/src/core/qgsofflineediting.cpp +++ b/src/core/qgsofflineediting.cpp @@ -762,10 +762,10 @@ QgsVectorLayer *QgsOfflineEditing::copyVectorLayer( QgsVectorLayer *layer, sqlit // NOTE: SpatiaLite provider ignores position of geometry column // fill gap in QgsAttributeMap if geometry column is not last (WORKAROUND) + int column = 0; QgsAttributes attrs = f.attributes(); // on GPKG newAttrs has an addition FID attribute, so we have to add a dummy in the original set QgsAttributes newAttrs( containerType == GPKG ? attrs.count() + 1 : attrs.count() ); - int column = 0; for ( int it = 0; it < attrs.count(); ++it ) { newAttrs[column++] = attrs.at( it ); @@ -1137,13 +1137,14 @@ void QgsOfflineEditing::updateLayerOrder( QgsVectorLayer *sourceLayer, QgsVector QMap QgsOfflineEditing::attributeLookup( QgsVectorLayer *offlineLayer, QgsVectorLayer *remoteLayer ) { const QgsAttributeList &offlineAttrs = offlineLayer->attributeList(); - const QgsAttributeList &remoteAttrs = remoteLayer->attributeList(); QMap < int /*offline attr*/, int /*remote attr*/ > attrLookup; - // NOTE: use size of remoteAttrs, as offlineAttrs can have new attributes not yet synced - for ( int i = 0; i < remoteAttrs.size(); i++ ) + // NOTE: though offlineAttrs can have new attributes not yet synced, we take the amount of offlineAttrs + // because we anyway only add mapping for the fields existing in remoteLayer (this because it could contain fid on 0) + for ( int i = 0; i < offlineAttrs.size(); i++ ) { - attrLookup.insert( offlineAttrs.at( i ), remoteAttrs.at( i ) ); + if ( remoteLayer->fields().lookupField( offlineLayer->fields().field( i ).name() ) >= 0 ) + attrLookup.insert( offlineAttrs.at( i ), remoteLayer->fields().indexOf( offlineLayer->fields().field( i ).name() ) ); } return attrLookup; From be9f6bb97ad99c0baf25e82afd1f202074b47005 Mon Sep 17 00:00:00 2001 From: signedav Date: Mon, 3 Dec 2018 18:10:27 +0100 Subject: [PATCH 3/5] tests if gpkg back synchronization provides the same values --- tests/src/core/testqgsofflineediting.cpp | 27 +++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/src/core/testqgsofflineediting.cpp b/tests/src/core/testqgsofflineediting.cpp index c8d8e361fe48..c58a553d16f9 100644 --- a/tests/src/core/testqgsofflineediting.cpp +++ b/tests/src/core/testqgsofflineediting.cpp @@ -124,6 +124,9 @@ void TestQgsOfflineEditing::createGeopackageAndSynchronizeBack() QCOMPARE( mpLayer->name(), QStringLiteral( "points" ) ); QCOMPARE( mpLayer->featureCount(), numberOfFeatures ); QCOMPARE( mpLayer->fields().size(), numberOfFields ); + QgsFeature firstFeatureBeforeAction; + QgsFeatureIterator it = mpLayer->getFeatures(); + it.nextFeature( firstFeatureBeforeAction ); connect( mOfflineEditing, &QgsOfflineEditing::warning, this, []( const QString & title, const QString & message ) { qDebug() << title << message; } ); //convert @@ -135,13 +138,35 @@ void TestQgsOfflineEditing::createGeopackageAndSynchronizeBack() //comparing with the number +1 because GPKG created an fid QCOMPARE( mpLayer->fields().size(), numberOfFields + 1 ); + QgsFeature firstFeatureInAction; + it = mpLayer->getFeatures(); + it.nextFeature( firstFeatureInAction ); + + //compare some values + QCOMPARE( firstFeatureInAction.attribute( "Class" ).toString(), firstFeatureBeforeAction.attribute( "Class" ).toString() ); + QCOMPARE( firstFeatureInAction.attribute( "Heading" ).toString(), firstFeatureBeforeAction.attribute( "Heading" ).toString() ); + QCOMPARE( firstFeatureInAction.attribute( "Cabin Crew" ).toString(), firstFeatureBeforeAction.attribute( "Cabin Crew" ).toString() ); + + QgsFeature newFeature( mpLayer->fields() ); + mpLayer->startEditing(); + mpLayer->dataProvider()->addFeature( newFeature ); + mpLayer->commitChanges(); + QCOMPARE( mpLayer->featureCount(), numberOfFeatures + 1 ); + //synchronize back mOfflineEditing->synchronize(); mpLayer = qobject_cast( QgsProject::instance()->mapLayers().first() ); QCOMPARE( mpLayer->name(), QStringLiteral( "points" ) ); - QCOMPARE( mpLayer->featureCount(), numberOfFeatures ); + //following it's failing and I don't know why + //QCOMPARE( mpLayer->dataProvider()->featureCount(), numberOfFeatures + 1 ); QCOMPARE( mpLayer->fields().size(), numberOfFields ); + + QgsFeature firstFeatureAfterAction; + it = mpLayer->getFeatures(); + it.nextFeature( firstFeatureAfterAction ); + + QCOMPARE( firstFeatureAfterAction, firstFeatureBeforeAction ); } QGSTEST_MAIN( TestQgsOfflineEditing ) From 73e3eadfc91e51ea1051cdbc931c2da26bb7bd78 Mon Sep 17 00:00:00 2001 From: signedav Date: Tue, 4 Dec 2018 10:37:49 +0100 Subject: [PATCH 4/5] fix test and add test cases --- tests/src/core/testqgsofflineediting.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/src/core/testqgsofflineediting.cpp b/tests/src/core/testqgsofflineediting.cpp index c58a553d16f9..26f516c5edb4 100644 --- a/tests/src/core/testqgsofflineediting.cpp +++ b/tests/src/core/testqgsofflineediting.cpp @@ -143,13 +143,14 @@ void TestQgsOfflineEditing::createGeopackageAndSynchronizeBack() it.nextFeature( firstFeatureInAction ); //compare some values - QCOMPARE( firstFeatureInAction.attribute( "Class" ).toString(), firstFeatureBeforeAction.attribute( "Class" ).toString() ); - QCOMPARE( firstFeatureInAction.attribute( "Heading" ).toString(), firstFeatureBeforeAction.attribute( "Heading" ).toString() ); - QCOMPARE( firstFeatureInAction.attribute( "Cabin Crew" ).toString(), firstFeatureBeforeAction.attribute( "Cabin Crew" ).toString() ); + QCOMPARE( firstFeatureInAction.attribute( QStringLiteral( "Class" ) ).toString(), firstFeatureBeforeAction.attribute( QStringLiteral( "Class" ) ).toString() ); + QCOMPARE( firstFeatureInAction.attribute( QStringLiteral( "Heading" ) ).toString(), firstFeatureBeforeAction.attribute( QStringLiteral( "Heading" ) ).toString() ); + QCOMPARE( firstFeatureInAction.attribute( QStringLiteral( "Cabin Crew" ) ).toString(), firstFeatureBeforeAction.attribute( QStringLiteral( "Cabin Crew" ) ).toString() ); - QgsFeature newFeature( mpLayer->fields() ); + QgsFeature newFeature( mpLayer->dataProvider()->fields() ); + newFeature.setAttribute( QStringLiteral( "Class" ), QStringLiteral( "Superjet" ) ); mpLayer->startEditing(); - mpLayer->dataProvider()->addFeature( newFeature ); + mpLayer->addFeature( newFeature ); mpLayer->commitChanges(); QCOMPARE( mpLayer->featureCount(), numberOfFeatures + 1 ); @@ -158,15 +159,24 @@ void TestQgsOfflineEditing::createGeopackageAndSynchronizeBack() mpLayer = qobject_cast( QgsProject::instance()->mapLayers().first() ); QCOMPARE( mpLayer->name(), QStringLiteral( "points" ) ); - //following it's failing and I don't know why - //QCOMPARE( mpLayer->dataProvider()->featureCount(), numberOfFeatures + 1 ); + QCOMPARE( mpLayer->dataProvider()->featureCount(), numberOfFeatures + 1 ); QCOMPARE( mpLayer->fields().size(), numberOfFields ); + //get last feature + QgsFeature f = mpLayer->getFeature( mpLayer->dataProvider()->featureCount() - 1 ); + qDebug() << "FID:" << f.id() << "Class:" << f.attribute( "Class" ).toString(); + QCOMPARE( f.attribute( QStringLiteral( "Class" ) ).toString(), QStringLiteral( "Superjet" ) ); QgsFeature firstFeatureAfterAction; it = mpLayer->getFeatures(); it.nextFeature( firstFeatureAfterAction ); QCOMPARE( firstFeatureAfterAction, firstFeatureBeforeAction ); + + //and delete the feature again + QgsFeatureIds idsToClean; + idsToClean << f.id(); + mpLayer->dataProvider()->deleteFeatures( idsToClean ); + QCOMPARE( mpLayer->dataProvider()->featureCount(), numberOfFeatures ); } QGSTEST_MAIN( TestQgsOfflineEditing ) From 8fda2b7df81a0b568e2282321b87861e5e750c19 Mon Sep 17 00:00:00 2001 From: signedav Date: Mon, 10 Dec 2018 17:53:01 +0100 Subject: [PATCH 5/5] use temp copy of points.shp --- tests/src/core/testqgsofflineediting.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/src/core/testqgsofflineediting.cpp b/tests/src/core/testqgsofflineediting.cpp index 26f516c5edb4..8e76281e5fb0 100644 --- a/tests/src/core/testqgsofflineediting.cpp +++ b/tests/src/core/testqgsofflineediting.cpp @@ -41,6 +41,7 @@ class TestQgsOfflineEditing : public QObject QStringList layerIds; long numberOfFeatures; int numberOfFields; + QTemporaryDir tempDir; private slots: void initTestCase();// will be called before the first testfunction is executed. @@ -75,8 +76,12 @@ void TestQgsOfflineEditing::cleanupTestCase() void TestQgsOfflineEditing::init() { QString myFileName( TEST_DATA_DIR ); //defined in CmakeLists.txt - myFileName = myFileName + "/points.shp"; - QFileInfo myMapFileInfo( myFileName ); + QString myTempDirName = tempDir.path(); + QFile::copy( myFileName + "/points.shp", myTempDirName + "/points.shp" ); + QFile::copy( myFileName + "/points.shx", myTempDirName + "/points.shx" ); + QFile::copy( myFileName + "/points.dbf", myTempDirName + "/points.dbf" ); + QString myTempFileName = myTempDirName + "/points.shp"; + QFileInfo myMapFileInfo( myTempFileName ); mpLayer = new QgsVectorLayer( myMapFileInfo.filePath(), myMapFileInfo.completeBaseName(), QStringLiteral( "ogr" ) ); QgsProject::instance()->addMapLayer( mpLayer );