Skip to content
Permalink
Browse files

Providers: fix inconsistent behavior of OGR + tests

Fixes #36583

Add a base test to make sure all major DB providers will
behave the same way and return FALSE when a constraint violation
refuses an attribute change (in a batch with a valid one).

OGR with GPKG was the "bad guy" here.
  • Loading branch information
elpaso committed May 21, 2020
1 parent 7ee0fce commit 18e9bdc01023c4f10aec3f6ff1c928261ce382d2
@@ -2191,6 +2191,8 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
if ( attr_map.isEmpty() )
return true;

bool returnValue = true;

clearMinMaxCache();

setRelevantFields( true, attributeIndexes() );
@@ -2351,6 +2353,7 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
if ( mOgrLayer->SetFeature( of.get() ) != OGRERR_NONE )
{
pushError( tr( "OGR error setting feature %1: %2" ).arg( fid ).arg( CPLGetLastErrorMsg() ) );
returnValue = false;
}
}

@@ -2367,7 +2370,7 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
pushError( tr( "OGR error syncing to disk: %1" ).arg( CPLGetLastErrorMsg() ) );
}
QgsOgrConnPool::instance()->invalidateConnections( QgsOgrProviderUtils::connectionPoolId( dataSourceUri( true ), mShareSameDatasetAmongLayers ) );
return true;
return returnValue;
}

bool QgsOgrProvider::changeGeometryValues( const QgsGeometryMap &geometry_map )
@@ -315,6 +315,8 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider, public QgsFeat
/**
* Changes attribute values of existing features. This should
* succeed if the provider reports the ChangeAttributeValues capability.
* The method returns FALSE if the provider does not have ChangeAttributeValues
* capability or if any of the changes could not be successfully applied.
* \param attr_map a map containing changed attributes
* \returns TRUE in case of success and FALSE in case of failure
*/
@@ -879,6 +879,37 @@ def testChangeAttributes(self):
self.assertFalse(l.dataProvider().changeAttributeValues(changes),
'Provider reported no ChangeAttributeValues capability, but returned true to changeAttributeValues')


def testChangeAttributesConstraintViolation(self):
"""Checks that changing attributes violating a DB-level CHECK constraint returns false
the provider test case must provide an editable layer with a text field
"i_will_fail_on_no_name" having a CHECK constraint that will fail when value is "no name".
The layer must contain at least 2 features, that will be used to test the attibute change.
"""

if not getattr(self, 'getEditableLayerWithCheckConstraint', None):
return

l = self.getEditableLayerWithCheckConstraint()
self.assertTrue(l.isValid())

assert l.dataProvider().capabilities() & QgsVectorDataProvider.ChangeAttributeValues

# find the featurea to change
feature0 = [f for f in l.dataProvider().getFeatures()][0]
feature1 = [f for f in l.dataProvider().getFeatures()][1]
field_idx = l.fields().indexFromName('i_will_fail_on_no_name')
self.assertTrue(field_idx >= 0)
# changes by feature id, for changeAttributeValues call
changes = {
feature0.id(): {field_idx: 'no name'},
feature1.id(): {field_idx: 'I have a valid name'}
}
# expect failure
result = l.dataProvider().changeAttributeValues(changes)
self.assertFalse(
result, 'Provider reported success when changing an attribute value that violates a DB level CHECK constraint')

def testChangeGeometries(self):
if not getattr(self, 'getEditableLayer', None):
return
@@ -65,10 +65,13 @@ def setUpClass(cls):

srcpath = os.path.join(TEST_DATA_DIR, 'provider')
shutil.copy(os.path.join(srcpath, 'geopackage.gpkg'), cls.basetestpath)
shutil.copy(os.path.join(srcpath, 'geopackage_poly.gpkg'), cls.basetestpath)
shutil.copy(os.path.join(srcpath, 'geopackage_poly.gpkg'),
cls.basetestpath)
cls.basetestfile = os.path.join(cls.basetestpath, 'geopackage.gpkg')
cls.basetestpolyfile = os.path.join(cls.basetestpath, 'geopackage_poly.gpkg')
cls.vl = QgsVectorLayer(cls.basetestfile, 'test', 'ogr')
cls.basetestpolyfile = os.path.join(
cls.basetestpath, 'geopackage_poly.gpkg')
cls.vl = QgsVectorLayer(
cls.basetestfile + '|layername=geopackage', 'test', 'ogr')
assert(cls.vl.isValid())
cls.source = cls.vl.dataProvider()
cls.vl_poly = QgsVectorLayer(cls.basetestpolyfile, 'test', 'ogr')
@@ -77,6 +80,10 @@ def setUpClass(cls):

cls.dirs_to_cleanup = [cls.basetestpath, cls.repackfilepath]

# Create the other layer for constraints check
cls.check_constraint = QgsVectorLayer(
cls.basetestfile + '|layername=check_constraint', 'check_constraint', 'ogr')

@classmethod
def tearDownClass(cls):
"""Run after all tests"""
@@ -91,8 +98,14 @@ def getSource(self):
datasource = os.path.join(tmpdir, 'geopackage.gpkg')

vl = QgsVectorLayer(datasource, 'test', 'ogr')

return vl

def getEditableLayerWithCheckConstraint(self):
"""Returns the layer for attribute change CHECK constraint violation"""

return self.check_constraint

def enableCompiler(self):
QgsSettings().setValue('/qgis/compileExpressions', True)
return True

0 comments on commit 18e9bdc

Please sign in to comment.
You can’t perform that action at this time.