Skip to content

Commit d3d8f4d

Browse files
authoredSep 27, 2017
Merge pull request #5223 from boundlessgeo/geom_compatibility_check_release-2_18-fix15741
On behalf of Giovanni: Geom compatibility check release 2.18 fixes #15741 #16927
2 parents 03a444c + 278a88c commit d3d8f4d

20 files changed

+343
-118
lines changed
 

‎python/core/qgsvectordataprovider.sip

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,17 @@ class QgsVectorDataProvider : QgsDataProvider
390390

391391
void pushError( const QString& msg );
392392

393-
/** Converts the geometry to the provider type if possible / necessary
394-
@return the converted geometry or nullptr if no conversion was necessary or possible*/
393+
/** \brief Converts the geometry to the provider type if possible / necessary
394+
* this is the list of possible modifications:
395+
* - convert compoundcurve to circularstring
396+
* (possible if compoundcurve consists of one circular string)
397+
* - convert to multitype if necessary
398+
* - convert to curved type if necessary
399+
* - convert to linear type from curved type
400+
* - Add z/m 0 default values
401+
* - Remove z/m
402+
* \ref QgsVectorLayerEditBuffer::adaptGeometry()
403+
* \param geom Geometry to convert
404+
* \returns the converted geometry or nullptr if no conversion was necessary or possible*/
395405
QgsGeometry* convertToProviderType( const QgsGeometry* geom ) const /Factory/;
396406
};

‎python/core/qgsvectorlayereditbuffer.sip

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,17 @@ class QgsVectorLayerEditBuffer : QObject
148148
void updateAttributeMapIndex( QgsAttributeMap& attrs, int index, int offset ) const;
149149

150150
void updateLayerFields();
151+
152+
/** \brief Apply geometry modification basing on provider geometry type.
153+
* Geometry is modified only if successful conversion is possible.
154+
* adaptGeometry calls \ref QgsVectorDataProvider::convertToProviderType()
155+
* if necessary and that apply the modifications.
156+
* \param geometry pointer to the geometry that should be adapted to provider
157+
* \returns bool true if success.
158+
* True: Input geometry is changed because conversion is applied or
159+
* geometry is untouched if geometry conversion is not necessary.
160+
* False: Conversion is not possible and geometry is untouched.
161+
* \note added in QGIS 2.18.14
162+
*/
163+
bool adaptGeometry( QgsGeometry* geometry );
151164
};

‎src/app/qgisapp.cpp

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7162,7 +7162,7 @@ void QgisApp::mergeSelectedFeatures()
71627162
if ( !isDefaultValue && !vl->fields().at( i ).convertCompatible( val ) )
71637163
{
71647164
messageBar()->pushMessage(
7165-
tr( "Invalid result" ),
7165+
tr( "Merge features" ),
71667166
tr( "Could not store value '%1' in field of type %2" ).arg( attrs.at( i ).toString(), vl->fields().at( i ).typeName() ),
71677167
QgsMessageBar::WARNING );
71687168
}
@@ -7176,9 +7176,15 @@ void QgisApp::mergeSelectedFeatures()
71767176
vl->deleteFeature( *feature_it );
71777177
}
71787178

7179-
vl->addFeature( newFeature, false );
7180-
7181-
vl->endEditCommand();
7179+
// addFeature can fail if newFeature has no compatibile geometry
7180+
if ( !vl->addFeature( newFeature, false ) )
7181+
{
7182+
vl->destroyEditCommand();
7183+
}
7184+
else
7185+
{
7186+
vl->endEditCommand();
7187+
}
71827188

71837189
if ( mapCanvas() )
71847190
{
@@ -7533,8 +7539,14 @@ void QgisApp::editPaste( QgsMapLayer *destinationLayer )
75337539
++featureIt;
75347540
}
75357541

7536-
pasteVectorLayer->addFeatures( features );
7537-
pasteVectorLayer->endEditCommand();
7542+
if ( !pasteVectorLayer->addFeatures( features ) )
7543+
{
7544+
pasteVectorLayer->destroyEditCommand();
7545+
}
7546+
else
7547+
{
7548+
pasteVectorLayer->endEditCommand();
7549+
}
75387550

75397551
int nCopiedFeatures = features.count();
75407552
if ( nCopiedFeatures == 0 )
@@ -7721,7 +7733,7 @@ QgsVectorLayer *QgisApp::pasteToNewMemoryVector()
77217733
feature.geometry()->convertToMultiType();
77227734
}
77237735
}
7724-
if ( ! layer->addFeatures( features, false ) || !layer->commitChanges() )
7736+
if ( !layer->addFeatures( features, false ) || !layer->commitChanges() )
77257737
{
77267738
QgsDebugMsg( "Cannot add features or commit changes" );
77277739
delete layer;

‎src/app/qgsfieldcalculator.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,12 @@ void QgsFieldCalculator::accept()
295295
if ( value.canConvert< QgsGeometry >() )
296296
{
297297
QgsGeometry geom = value.value< QgsGeometry >();
298-
mVectorLayer->changeGeometry( feature.id(), &geom );
298+
if ( !mVectorLayer->changeGeometry( feature.id(), &geom ) )
299+
{
300+
calculationSuccess = false;
301+
error = tr( "Can not change geometry for feature: %1", "Field calculator" ).arg( feature.id() );
302+
break;
303+
}
299304
}
300305
}
301306
else

‎src/app/qgsmaptooladdpart.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,10 @@ void QgsMapToolAddPart::cadCanvasReleaseEvent( QgsMapMouseEvent * e )
219219
case 6:
220220
errorMessage = tr( "Selected geometry could not be found" );
221221
break;
222+
223+
case 7:
224+
errorMessage = tr( "Update geometry error" );
225+
break;
222226
}
223227

224228
emit messageEmitted( errorMessage, QgsMessageBar::WARNING );

‎src/app/qgsmaptooldeletepart.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,10 @@ void QgsMapToolDeletePart::canvasReleaseEvent( QgsMapMouseEvent* e )
105105
if ( g->deletePart( mPressedPartNum ) )
106106
{
107107
vlayer->beginEditCommand( tr( "Part of multipart feature deleted" ) );
108-
vlayer->changeGeometry( f.id(), g );
109-
vlayer->endEditCommand();
108+
if ( !vlayer->changeGeometry( f.id(), g ) )
109+
vlayer->destroyEditCommand();
110+
else
111+
vlayer->endEditCommand();
110112
vlayer->triggerRepaint();
111113
}
112114
else

‎src/app/qgsmaptooldeletering.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,11 @@ void QgsMapToolDeleteRing::canvasReleaseEvent( QgsMapMouseEvent* e )
113113
if ( g->deleteRing( mPressedRingNum, mPressedPartNum ) )
114114
{
115115
vlayer->beginEditCommand( tr( "Ring deleted" ) );
116-
vlayer->changeGeometry( mPressedFid, g );
116+
if ( !vlayer->changeGeometry( mPressedFid, g ) )
117+
{
118+
vlayer->destroyEditCommand();
119+
return;
120+
}
117121
vlayer->endEditCommand();
118122
vlayer->triggerRepaint();
119123
}

‎src/app/qgsmaptoolsimplify.cpp

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,17 +176,30 @@ void QgsMapToolSimplify::storeSimplified()
176176
double layerTolerance = QgsTolerance::toleranceInMapUnits( mTolerance, vlayer, mCanvas->mapSettings(), mToleranceUnits );
177177

178178
vlayer->beginEditCommand( tr( "Geometry simplified" ) );
179+
bool success = true;
179180
Q_FOREACH ( const QgsFeature& feat, mSelectedFeatures )
180181
{
181182
if ( QgsGeometry* g = feat.constGeometry()->simplify( layerTolerance ) )
182183
{
183-
vlayer->changeGeometry( feat.id(), g );
184+
if ( !vlayer->changeGeometry( feat.id(), g ) )
185+
{
186+
success = false;
187+
}
184188
delete g;
189+
190+
if ( !success )
191+
break;
185192
}
186193
}
187-
vlayer->endEditCommand();
188-
189-
clearSelection();
194+
if ( success )
195+
{
196+
vlayer->endEditCommand();
197+
clearSelection();
198+
}
199+
else
200+
{
201+
vlayer->destroyEditCommand();
202+
}
190203

191204
vlayer->triggerRepaint();
192205
}

‎src/core/qgsofflineediting.cpp

Lines changed: 66 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ extern "C"
5454
#define PROJECT_ENTRY_SCOPE_OFFLINE "OfflineEditingPlugin"
5555
#define PROJECT_ENTRY_KEY_OFFLINE_DB_PATH "/OfflineDbPath"
5656

57-
QgsOfflineEditing::QgsOfflineEditing()
57+
QgsOfflineEditing::QgsOfflineEditing():
58+
syncError( false )
5859
{
5960
connect( QgsMapLayerRegistry::instance(), SIGNAL( layerWasAdded( QgsMapLayer* ) ), this, SLOT( layerAdded( QgsMapLayer* ) ) );
6061
}
@@ -261,6 +262,7 @@ void QgsOfflineEditing::synchronize()
261262
updateVisibilityPresets( offlineLayer, remoteLayer );
262263

263264
// apply layer edit log
265+
syncError = false;
264266
QString qgisLayerId = layer->id();
265267
QString sql = QString( "SELECT \"id\" FROM 'log_layer_ids' WHERE \"qgis_id\" = '%1'" ).arg( qgisLayerId );
266268
int layerId = sqlQueryInt( db, sql, -1 );
@@ -276,37 +278,49 @@ void QgsOfflineEditing::synchronize()
276278
QgsDebugMsgLevel( "Apply commits chronologically", 4 );
277279
// apply commits chronologically
278280
applyAttributesAdded( remoteLayer, db, layerId, i );
279-
applyAttributeValueChanges( offlineLayer, remoteLayer, db, layerId, i );
280-
applyGeometryChanges( remoteLayer, db, layerId, i );
281+
if ( !syncError )
282+
applyAttributeValueChanges( offlineLayer, remoteLayer, db, layerId, i );
283+
if ( !syncError )
284+
applyGeometryChanges( remoteLayer, db, layerId, i );
281285
}
282286

283-
applyFeaturesAdded( offlineLayer, remoteLayer, db, layerId );
284-
applyFeaturesRemoved( remoteLayer, db, layerId );
287+
if ( !syncError )
288+
applyFeaturesAdded( offlineLayer, remoteLayer, db, layerId );
289+
if ( !syncError )
290+
applyFeaturesRemoved( remoteLayer, db, layerId );
285291

286-
if ( remoteLayer->commitChanges() )
292+
if ( !syncError )
287293
{
288-
// update fid lookup
289-
updateFidLookup( remoteLayer, db, layerId );
290-
291-
// clear edit log for this layer
292-
sql = QString( "DELETE FROM 'log_added_attrs' WHERE \"layer_id\" = %1" ).arg( layerId );
293-
sqlExec( db, sql );
294-
sql = QString( "DELETE FROM 'log_added_features' WHERE \"layer_id\" = %1" ).arg( layerId );
295-
sqlExec( db, sql );
296-
sql = QString( "DELETE FROM 'log_removed_features' WHERE \"layer_id\" = %1" ).arg( layerId );
297-
sqlExec( db, sql );
298-
sql = QString( "DELETE FROM 'log_feature_updates' WHERE \"layer_id\" = %1" ).arg( layerId );
299-
sqlExec( db, sql );
300-
sql = QString( "DELETE FROM 'log_geometry_updates' WHERE \"layer_id\" = %1" ).arg( layerId );
301-
sqlExec( db, sql );
302-
303-
// reset commitNo
304-
QString sql = QString( "UPDATE 'log_indices' SET 'last_index' = 0 WHERE \"name\" = 'commit_no'" );
305-
sqlExec( db, sql );
294+
if ( remoteLayer->commitChanges() )
295+
{
296+
// update fid lookup
297+
updateFidLookup( remoteLayer, db, layerId );
298+
299+
// clear edit log for this layer
300+
sql = QString( "DELETE FROM 'log_added_attrs' WHERE \"layer_id\" = %1" ).arg( layerId );
301+
sqlExec( db, sql );
302+
sql = QString( "DELETE FROM 'log_added_features' WHERE \"layer_id\" = %1" ).arg( layerId );
303+
sqlExec( db, sql );
304+
sql = QString( "DELETE FROM 'log_removed_features' WHERE \"layer_id\" = %1" ).arg( layerId );
305+
sqlExec( db, sql );
306+
sql = QString( "DELETE FROM 'log_feature_updates' WHERE \"layer_id\" = %1" ).arg( layerId );
307+
sqlExec( db, sql );
308+
sql = QString( "DELETE FROM 'log_geometry_updates' WHERE \"layer_id\" = %1" ).arg( layerId );
309+
sqlExec( db, sql );
310+
311+
// reset commitNo
312+
QString sql = QString( "UPDATE 'log_indices' SET 'last_index' = 0 WHERE \"name\" = 'commit_no'" );
313+
sqlExec( db, sql );
314+
}
315+
else
316+
{
317+
showWarning( remoteLayer->commitErrors().join( "\n" ) );
318+
}
306319
}
307320
else
308321
{
309-
showWarning( remoteLayer->commitErrors().join( "\n" ) );
322+
remoteLayer->rollBack();
323+
showWarning( tr( "Syncronization failed" ) );
310324
}
311325
}
312326
else
@@ -688,7 +702,8 @@ QgsVectorLayer* QgsOfflineEditing::copyVectorLayer( QgsVectorLayer* layer, sqlit
688702
f.geometry()->setGeometry( geom );
689703
}
690704

691-
newLayer->addFeature( f, false );
705+
if ( !newLayer->addFeature( f, false ) )
706+
return nullptr;
692707

693708
emit progressUpdated( featureCount++ );
694709
}
@@ -762,7 +777,11 @@ void QgsOfflineEditing::applyAttributesAdded( QgsVectorLayer* remoteLayer, sqlit
762777
{
763778
QString typeName = typeNameLookup[ field.type()];
764779
field.setTypeName( typeName );
765-
remoteLayer->addAttribute( field );
780+
if ( !remoteLayer->addAttribute( field ) )
781+
{
782+
syncError = true;
783+
return;
784+
}
766785
}
767786
else
768787
{
@@ -830,7 +849,11 @@ void QgsOfflineEditing::applyFeaturesAdded( QgsVectorLayer* offlineLayer, QgsVec
830849

831850
f.setAttributes( newAttrs );
832851

833-
remoteLayer->addFeature( f, false );
852+
if ( !remoteLayer->addFeature( f, false ) )
853+
{
854+
syncError = true;
855+
return;
856+
}
834857

835858
emit progressUpdated( i++ );
836859
}
@@ -847,7 +870,11 @@ void QgsOfflineEditing::applyFeaturesRemoved( QgsVectorLayer* remoteLayer, sqlit
847870
for ( QgsFeatureIds::const_iterator it = values.begin(); it != values.end(); ++it )
848871
{
849872
QgsFeatureId fid = remoteFid( db, layerId, *it );
850-
remoteLayer->deleteFeature( fid );
873+
if ( !remoteLayer->deleteFeature( fid ) )
874+
{
875+
syncError = true;
876+
return;
877+
}
851878

852879
emit progressUpdated( i++ );
853880
}
@@ -866,7 +893,11 @@ void QgsOfflineEditing::applyAttributeValueChanges( QgsVectorLayer* offlineLayer
866893
{
867894
QgsFeatureId fid = remoteFid( db, layerId, values.at( i ).fid );
868895
QgsDebugMsgLevel( QString( "Offline changeAttributeValue %1 = %2" ).arg( QString( attrLookup[ values.at( i ).attr ] ), values.at( i ).value ), 4 );
869-
remoteLayer->changeAttributeValue( fid, attrLookup[ values.at( i ).attr ], values.at( i ).value );
896+
if ( !remoteLayer->changeAttributeValue( fid, attrLookup[ values.at( i ).attr ], values.at( i ).value ) )
897+
{
898+
syncError = true;
899+
return;
900+
}
870901

871902
emit progressUpdated( i + 1 );
872903
}
@@ -882,7 +913,11 @@ void QgsOfflineEditing::applyGeometryChanges( QgsVectorLayer* remoteLayer, sqlit
882913
for ( int i = 0; i < values.size(); i++ )
883914
{
884915
QgsFeatureId fid = remoteFid( db, layerId, values.at( i ).fid );
885-
remoteLayer->changeGeometry( fid, QgsGeometry::fromWkt( values.at( i ).geom_wkt ) );
916+
if ( !remoteLayer->changeGeometry( fid, QgsGeometry::fromWkt( values.at( i ).geom_wkt ) ) )
917+
{
918+
syncError = true;
919+
return;
920+
}
886921

887922
emit progressUpdated( i + 1 );
888923
}

‎src/core/qgsofflineediting.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ class CORE_EXPORT QgsOfflineEditing : public QObject
153153
};
154154
typedef QList<GeometryChange> GeometryChanges;
155155
GeometryChanges sqlQueryGeometryChanges( sqlite3* db, const QString& sql );
156+
/**
157+
* in case of sync error the flag is set to true
158+
*/
159+
bool syncError;
160+
156161

157162
private slots:
158163
void layerAdded( QgsMapLayer* layer );

‎src/core/qgsvectordataprovider.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,10 +714,37 @@ QgsGeometry* QgsVectorDataProvider::convertToProviderType( const QgsGeometry* ge
714714
outputGeom->addMValue();
715715
}
716716

717+
// remove Z if provider does not have
718+
// control added to fix https://issues.qgis.org/issues/16927
719+
if ( !QgsWKBTypes::hasZ( providerGeomType ) && QgsWKBTypes::hasZ( geometry->wkbType() ) )
720+
{
721+
if ( !outputGeom )
722+
{
723+
outputGeom = geometry->clone();
724+
}
725+
outputGeom->dropZValue();
726+
}
727+
728+
// remove M if provider does not have
729+
// control added as follow-up of https://issues.qgis.org/issues/16927
730+
if ( !QgsWKBTypes::hasM( providerGeomType ) && QgsWKBTypes::hasM( geometry->wkbType() ) )
731+
{
732+
if ( !outputGeom )
733+
{
734+
outputGeom = geometry->clone();
735+
}
736+
outputGeom->dropMValue();
737+
}
738+
717739
if ( outputGeom )
718740
{
719741
return new QgsGeometry( outputGeom );
720742
}
743+
744+
QString msg = tr( "Geometry type %1 not compatible with provider type %2.", "not compatible geometry" )
745+
.arg( QgsWKBTypes::displayString( geometry->wkbType() ) )
746+
.arg( QgsWKBTypes::displayString( providerGeomType ) );
747+
const_cast<QgsVectorDataProvider*>( this )->pushError( msg );
721748
return nullptr;
722749
}
723750

‎src/core/qgsvectordataprovider.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,8 +468,18 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider
468468
/** Old-style mapping of index to name for QgsPalLabeling fix */
469469
QgsAttrPalIndexNameHash mAttrPalIndexName;
470470

471-
/** Converts the geometry to the provider type if possible / necessary
472-
@return the converted geometry or nullptr if no conversion was necessary or possible*/
471+
/** \brief Converts the geometry to the provider type if possible / necessary
472+
* this is the list of possible modifications:
473+
* - convert compoundcurve to circularstring
474+
* (possible if compoundcurve consists of one circular string)
475+
* - convert to multitype if necessary
476+
* - convert to curved type if necessary
477+
* - convert to linear type from curved type
478+
* - Add z/m 0 default values
479+
* - Remove z/m
480+
* \ref QgsVectorLayerEditBuffer::adaptGeometry()
481+
* \param geom Geometry to convert
482+
* \returns the converted geometry or nullptr if no conversion was necessary or possible*/
473483
QgsGeometry* convertToProviderType( const QgsGeometry* geom ) const;
474484

475485
private:

‎src/core/qgsvectorlayereditbuffer.cpp

Lines changed: 40 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "qgsgeometry.h"
1818
#include "qgslogger.h"
19+
#include <qgsmessagelog.h>
1920
#include "qgsvectorlayerundocommand.h"
2021
#include "qgsvectordataprovider.h"
2122
#include "qgsvectorlayer.h"
@@ -112,33 +113,34 @@ void QgsVectorLayerEditBuffer::updateChangedAttributes( QgsFeature &f )
112113
f.setAttributes( attrs );
113114
}
114115

115-
116-
117-
118116
bool QgsVectorLayerEditBuffer::addFeature( QgsFeature& f )
119117
{
120-
if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::AddFeatures ) )
118+
QgsVectorDataProvider* provider = L->dataProvider();
119+
120+
if ( !( provider->capabilities() & QgsVectorDataProvider::AddFeatures ) )
121121
{
122122
return false;
123123
}
124124
if ( L->mUpdatedFields.count() != f.attributes().count() )
125125
return false;
126126

127-
// TODO: check correct geometry type
127+
// if not then try to convert to a compatible geometry type
128+
if ( !adaptGeometry( f.geometry() ) )
129+
return false;
128130

129131
L->undoStack()->push( new QgsVectorLayerUndoCommandAddFeature( this, f ) );
130132
return true;
131133
}
132134

133-
134135
bool QgsVectorLayerEditBuffer::addFeatures( QgsFeatureList& features )
135136
{
136137
if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::AddFeatures ) )
137138
return false;
138139

139140
for ( QgsFeatureList::iterator iter = features.begin(); iter != features.end(); ++iter )
140141
{
141-
addFeature( *iter );
142+
if ( !addFeature( *iter ) )
143+
return false;
142144
}
143145

144146
L->updateExtents();
@@ -178,6 +180,32 @@ bool QgsVectorLayerEditBuffer::deleteFeatures( const QgsFeatureIds& fids )
178180
return true;
179181
}
180182

183+
bool QgsVectorLayerEditBuffer::adaptGeometry( QgsGeometry* geometry )
184+
{
185+
/* this routine was introduced to fi the following issue:
186+
* https://issues.qgis.org/issues/15741
187+
* and later used in changeGeometry and addFeature to
188+
* fix also the following issue
189+
* https://issues.qgis.org/issues/16948
190+
* https://issues.qgis.org/issues/15741
191+
*/
192+
QgsVectorDataProvider* provider = L->dataProvider();
193+
if ( geometry && geometry->geometry() &&
194+
!geometry->isEmpty() &&
195+
geometry->wkbType() != provider->geometryType() )
196+
{
197+
// check if provider do strict geometry control
198+
// otherwise leave to the commit to rise provider errors
199+
if ( provider->doesStrictFeatureTypeCheck() )
200+
{
201+
QgsGeometry* newGeom = provider->convertToProviderType( geometry );
202+
if ( !newGeom )
203+
return false;
204+
geometry = newGeom;
205+
}
206+
}
207+
return true;
208+
}
181209

182210
bool QgsVectorLayerEditBuffer::changeGeometry( QgsFeatureId fid, QgsGeometry* geom )
183211
{
@@ -194,7 +222,11 @@ bool QgsVectorLayerEditBuffer::changeGeometry( QgsFeatureId fid, QgsGeometry* ge
194222
else if ( !( L->dataProvider()->capabilities() & QgsVectorDataProvider::ChangeGeometries ) )
195223
return false;
196224

197-
// TODO: check compatible geometry
225+
// if not then try to convert to a compatible geometry type
226+
if ( !adaptGeometry( geom ) )
227+
{
228+
return false;
229+
}
198230

199231
L->undoStack()->push( new QgsVectorLayerUndoCommandChangeGeometry( this, fid, geom ) );
200232
return true;
@@ -302,40 +334,6 @@ bool QgsVectorLayerEditBuffer::commitChanges( QStringList& commitErrors )
302334
// no yes => changeAttributeValues
303335
// yes yes => changeFeatures
304336

305-
// to fix https://issues.qgis.org/issues/15741
306-
// first of all check if feature to add is compatible with provider type
307-
// this check have to be done before all checks to avoid to clear internal
308-
// buffer if some of next steps success.
309-
if ( success && !mAddedFeatures.isEmpty() )
310-
{
311-
if ( cap & QgsVectorDataProvider::AddFeatures )
312-
{
313-
if ( provider->doesStrictFeatureTypeCheck() )
314-
{
315-
QgsFeatureMap::iterator featureIt = mAddedFeatures.begin();
316-
for ( ; featureIt != mAddedFeatures.end(); ++featureIt )
317-
{
318-
if ( !featureIt->geometry() ||
319-
featureIt->geometry()->isEmpty() ||
320-
featureIt->geometry()->wkbType() == provider->geometryType() )
321-
continue;
322-
323-
if ( !provider->convertToProviderType( featureIt->geometry() ) )
324-
{
325-
commitErrors << tr( "ERROR: %n feature(s) not added - geometry type is not compatible with the current layer.", "not added features count", mAddedFeatures.size() );
326-
success = false;
327-
break;
328-
}
329-
}
330-
}
331-
}
332-
else
333-
{
334-
commitErrors << tr( "ERROR: %n feature(s) not added - provider doesn't support adding features.", "not added features count", mAddedFeatures.size() );
335-
success = false;
336-
}
337-
}
338-
339337
//
340338
// update geometries
341339
//

‎src/core/qgsvectorlayereditbuffer.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,19 @@ class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject
178178

179179
void updateLayerFields();
180180

181+
/** \brief Apply geometry modification basing on provider geometry type.
182+
* Geometry is modified only if successful conversion is possible.
183+
* adaptGeometry calls \ref QgsVectorDataProvider::convertToProviderType()
184+
* if necessary and that apply the modifications.
185+
* \param geometry pointer to the geometry that should be adapted to provider
186+
* \returns bool true if success.
187+
* True: Input geometry is changed because conversion is applied or
188+
* geometry is untouched if geometry conversion is not necessary.
189+
* False: Conversion is not possible and geometry is untouched.
190+
* \note added in QGIS 2.18.14
191+
*/
192+
bool adaptGeometry( QgsGeometry* geometry );
193+
181194
protected:
182195
QgsVectorLayer* L;
183196
friend class QgsVectorLayer;

‎src/core/qgsvectorlayereditutils.cpp

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ bool QgsVectorLayerEditUtils::insertVertex( double x, double y, QgsFeatureId atF
5050

5151
geometry.insertVertex( x, y, beforeVertex );
5252

53-
L->editBuffer()->changeGeometry( atFeatureId, &geometry );
54-
return true;
53+
return L->editBuffer()->changeGeometry( atFeatureId, &geometry );
5554
}
5655

5756

@@ -79,8 +78,7 @@ bool QgsVectorLayerEditUtils::moveVertex( const QgsPointV2& p, QgsFeatureId atFe
7978

8079
geometry.moveVertex( p, atVertex );
8180

82-
L->editBuffer()->changeGeometry( atFeatureId, &geometry );
83-
return true;
81+
return L->editBuffer()->changeGeometry( atFeatureId, &geometry );
8482
}
8583

8684

@@ -115,7 +113,9 @@ QgsVectorLayer::EditResult QgsVectorLayerEditUtils::deleteVertexV2( QgsFeatureId
115113
geometry.setGeometry( nullptr );
116114
}
117115

118-
L->editBuffer()->changeGeometry( featureId, &geometry );
116+
if ( !L->editBuffer()->changeGeometry( featureId, &geometry ) )
117+
return QgsVectorLayer::EditFailed;
118+
119119
return !geometry.isEmpty() ? QgsVectorLayer::Success : QgsVectorLayer::EmptyGeometry;
120120
}
121121

@@ -166,7 +166,11 @@ int QgsVectorLayerEditUtils::addRing( QgsCurveV2* ring, const QgsFeatureIds& tar
166166
addRingReturnCode = f.geometry()->addRing( static_cast< QgsCurveV2* >( ring->clone() ) );
167167
if ( addRingReturnCode == 0 )
168168
{
169-
L->editBuffer()->changeGeometry( f.id(), f.geometry() );
169+
if ( !L->editBuffer()->changeGeometry( f.id(), f.geometry() ) )
170+
{
171+
addRingReturnCode = 5;
172+
break;
173+
}
170174
if ( modifiedFeatureId )
171175
*modifiedFeatureId = f.id();
172176

@@ -223,7 +227,8 @@ int QgsVectorLayerEditUtils::addPart( const QgsPointSequenceV2 &points, QgsFeatu
223227
//convert back to single part if required by layer
224228
geometry.convertToSingleType();
225229
}
226-
L->editBuffer()->changeGeometry( featureId, &geometry );
230+
if ( !L->editBuffer()->changeGeometry( featureId, &geometry ) )
231+
errorCode = 7; // update geometry error
227232
}
228233
return errorCode;
229234
}
@@ -262,7 +267,8 @@ int QgsVectorLayerEditUtils::addPart( QgsCurveV2* ring, QgsFeatureId featureId )
262267
//convert back to single part if required by layer
263268
geometry.convertToSingleType();
264269
}
265-
L->editBuffer()->changeGeometry( featureId, &geometry );
270+
if ( !L->editBuffer()->changeGeometry( featureId, &geometry ) )
271+
errorCode = 7; // update geometry error
266272
}
267273
return errorCode;
268274
}
@@ -287,7 +293,8 @@ int QgsVectorLayerEditUtils::translateFeature( QgsFeatureId featureId, double dx
287293
int errorCode = geometry.translate( dx, dy );
288294
if ( errorCode == 0 )
289295
{
290-
L->editBuffer()->changeGeometry( featureId, &geometry );
296+
if ( !L->editBuffer()->changeGeometry( featureId, &geometry ) )
297+
errorCode = 1;
291298
}
292299
return errorCode;
293300
}
@@ -369,7 +376,8 @@ int QgsVectorLayerEditUtils::splitFeatures( const QList<QgsPoint>& splitLine, bo
369376
if ( splitFunctionReturn == 0 )
370377
{
371378
//change this geometry
372-
L->editBuffer()->changeGeometry( feat.id(), feat.geometry() );
379+
if ( !L->editBuffer()->changeGeometry( feat.id(), feat.geometry() ) )
380+
continue;
373381

374382
//insert new features
375383
for ( int i = 0; i < newGeometries.size(); ++i )
@@ -424,7 +432,8 @@ int QgsVectorLayerEditUtils::splitFeatures( const QList<QgsPoint>& splitLine, bo
424432

425433

426434
//now add the new features to this vectorlayer
427-
L->editBuffer()->addFeatures( newFeatures );
435+
if ( !L->editBuffer()->addFeatures( newFeatures ) )
436+
returnCode = 7; // "No feature split done" + "The geometry is invalid. Please repair before trying to split it."
428437

429438
return returnCode;
430439
}
@@ -513,11 +522,7 @@ int QgsVectorLayerEditUtils::splitParts( const QList<QgsPoint>& splitLine, bool
513522
// For test only: Exception already thrown here...
514523
// feat.geometry()->asWkb();
515524

516-
if ( !addPartRet )
517-
{
518-
L->editBuffer()->changeGeometry( feat.id(), feat.geometry() );
519-
}
520-
else
525+
if ( addPartRet )
521526
{
522527
// Test addPartRet
523528
switch ( addPartRet )
@@ -535,7 +540,11 @@ int QgsVectorLayerEditUtils::splitParts( const QList<QgsPoint>& splitLine, bool
535540
break;
536541
}
537542
}
538-
L->editBuffer()->changeGeometry( feat.id(), feat.geometry() );
543+
if ( !L->editBuffer()->changeGeometry( feat.id(), feat.geometry() ) )
544+
{
545+
splitFunctionReturn = 2; // value > 1 means no split and error
546+
break;
547+
}
539548

540549
if ( topologicalEditing )
541550
{

‎src/gui/qgsrelationeditorwidget.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ void QgsRelationEditorWidget::linkFeature()
379379
newFeatures << linkFeature;
380380
}
381381

382+
// TODO: what to do in case of addFeature fail?
382383
mRelation.referencingLayer()->addFeatures( newFeatures );
383384

384385
updateUi();

‎src/plugins/geometry_checker/ui/qgsgeometrycheckerresulttab.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,11 @@ bool QgsGeometryCheckerResultTab::exportErrorsDo( const QString& file )
272272
f.setAttribute( fieldFeatureId, error->featureId() );
273273
f.setAttribute( fieldErrDesc, error->description() );
274274
f.setGeometry( new QgsGeometry( error->location().clone() ) );
275-
layer->dataProvider()->addFeatures( QgsFeatureList() << f );
275+
if ( !layer->dataProvider()->addFeatures( QgsFeatureList() << f ) )
276+
{
277+
delete layer;
278+
return false;
279+
}
276280
}
277281

278282
// Remove existing layer with same uri

‎src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,11 @@ void QgsGeometryCheckerSetupTab::runChecks()
248248
features.append( feature );
249249
}
250250
}
251-
newlayer->dataProvider()->addFeatures( features );
251+
if ( !newlayer->dataProvider()->addFeatures( features ) )
252+
{
253+
QMessageBox::critical( this, tr( "Populate output Layer" ), tr( "Can not add features to output layer: %1." ).arg( filename ) );
254+
return;
255+
}
252256

253257
// Set selected features
254258
newlayer->selectByIds( selectedFeatures );

‎tests/src/core/testqgstracer.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ static QgsFeature make_feature( const QString& wkt )
5959
return f;
6060
}
6161

62-
static QgsVectorLayer* make_layer( const QStringList& wkts )
62+
static QgsVectorLayer* make_layer( const QStringList& wkts, const QString& type = "LineString" )
6363
{
64-
QgsVectorLayer* vl = new QgsVectorLayer( "LineString", "x", "memory" );
64+
QgsVectorLayer* vl = new QgsVectorLayer( type, "x", "memory" );
6565
Q_ASSERT( vl->isValid() );
6666

6767
vl->startEditing();
@@ -163,7 +163,7 @@ void TestQgsTracer::testPolygon()
163163
QStringList wkts;
164164
wkts << "POLYGON((0 0, 0 10, 20 10, 10 0, 0 0))";
165165

166-
QgsVectorLayer* vl = make_layer( wkts );
166+
QgsVectorLayer* vl = make_layer( wkts, "Polygon" );
167167

168168
QgsTracer tracer;
169169
tracer.setLayers( QList<QgsVectorLayer*>() << vl );

‎tests/src/python/test_qgsvectorlayereditbuffer.py

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
QgsFeature,
2121
QgsGeometry,
2222
QgsPoint,
23-
QgsField)
23+
QgsField,
24+
QgsWKBTypes,
25+
QGis)
2426
from qgis.testing import start_app, unittest
2527
start_app()
2628

@@ -51,6 +53,13 @@ def createEmptyLinestringLayer():
5153
return layer
5254

5355

56+
def createEmptyMultiLinestringLayer():
57+
layer = QgsVectorLayer("MultiLinestring?field=fldtxt:string&field=fldint:integer",
58+
"addfeat", "memory")
59+
assert layer.isValid()
60+
return layer
61+
62+
5463
class TestQgsVectorLayerEditBuffer(unittest.TestCase):
5564

5665
def testAddFeatures(self):
@@ -100,11 +109,58 @@ def testAddFeatures(self):
100109
[QgsPoint(1, 1), QgsPoint(2, 2)],
101110
[QgsPoint(3, 3), QgsPoint(4, 4)],
102111
]
112+
geom = QgsGeometry.fromMultiPolyline(multiline)
113+
f1 = QgsFeature(layer.fields(), 1)
114+
f1.setGeometry(geom)
115+
f1.setAttributes(["test", 123])
116+
117+
self.assertTrue(QgsWKBTypes.isMultiType(QGis.fromOldWkbType(geom.wkbType())))
118+
self.assertFalse((layer.editBuffer().addFeatures([f1])))
119+
120+
# check is possibile to adapt single to multi
121+
# This test should belong to vectordataprovider test
122+
layer = createEmptyMultiLinestringLayer()
123+
self.assertTrue(layer.startEditing())
124+
self.assertEqual(layer.editBuffer().addedFeatures(), {})
125+
line = [
126+
QgsPoint(1, 1), QgsPoint(2, 2), QgsPoint(3, 3)
127+
]
128+
geom = QgsGeometry.fromPolyline(line)
129+
f1 = QgsFeature(layer.fields(), 1)
130+
f1.setGeometry(geom)
131+
f1.setAttributes(["test", 123])
132+
133+
self.assertTrue(QgsWKBTypes.isSingleType(QGis.fromOldWkbType(geom.wkbType())))
134+
self.assertTrue((layer.editBuffer().addFeatures([f1])))
135+
136+
# check that is NOT possibile to adapt Multi to single if only one simple geometry
137+
# This because to avoid to have a bunch of successful import for that
138+
# thac can be converted and other feature that fails => better leave to
139+
# the user the explicit work to convert to singletype
140+
layer = createEmptyLinestringLayer()
141+
self.assertTrue(layer.startEditing())
142+
self.assertEqual(layer.editBuffer().addedFeatures(), {})
143+
multiline = [
144+
[QgsPoint(1, 1), QgsPoint(2, 2), QgsPoint(3, 3)]
145+
]
146+
geom = QgsGeometry.fromMultiPolyline(multiline)
147+
self.assertTrue(QgsWKBTypes.isMultiType(QGis.fromOldWkbType(geom.wkbType())))
148+
f1 = QgsFeature(layer.fields(), 1)
149+
f1.setGeometry(geom)
150+
f1.setAttributes(["test", 123])
151+
self.assertFalse((layer.editBuffer().addFeatures([f1])))
152+
153+
# check is possibile to adapt M geom to NOT M provider type
154+
layer = createEmptyLayer()
155+
self.assertTrue(layer.startEditing())
156+
self.assertEqual(layer.editBuffer().addedFeatures(), {})
157+
geom = QgsGeometry.fromPoint(QgsPoint(1, 1))
158+
geom.geometry().addMValue(1)
159+
self.assertTrue(QgsWKBTypes.hasM(geom.geometry().wkbType()))
103160
f1 = QgsFeature(layer.fields(), 1)
104-
f1.setGeometry(QgsGeometry.fromMultiPolyline(multiline))
161+
f1.setGeometry(geom)
105162
f1.setAttributes(["test", 123])
106-
self.assertTrue(layer.addFeatures([f1]))
107-
self.assertFalse(layer.commitChanges())
163+
self.assertTrue((layer.editBuffer().addFeatures([f1])))
108164

109165
def testAddMultipleFeatures(self):
110166
# test adding multiple features to an edit buffer

0 commit comments

Comments
 (0)
Please sign in to comment.