Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #36620 from elpaso/bugfix-gh36583-change-attr-values
Bugfix gh36583 change attr values
  • Loading branch information
elpaso committed May 22, 2020
2 parents 93648dd + 34ec263 commit 5aea8ed
Show file tree
Hide file tree
Showing 10 changed files with 672 additions and 296 deletions.
2 changes: 2 additions & 0 deletions python/core/auto_generated/qgsvectordataprovider.sip.in
Expand Up @@ -304,6 +304,8 @@ manually to ensure that the layer's field are correctly reported.
%Docstring
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

Expand Down
5 changes: 4 additions & 1 deletion src/core/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -2191,6 +2191,8 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
if ( attr_map.isEmpty() )
return true;

bool returnValue = true;

clearMinMaxCache();

setRelevantFields( true, attributeIndexes() );
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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 )
Expand Down
2 changes: 2 additions & 0 deletions src/core/qgsvectordataprovider.h
Expand Up @@ -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
*/
Expand Down
30 changes: 30 additions & 0 deletions tests/src/python/providertestbase.py
Expand Up @@ -879,6 +879,36 @@ 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
Expand Down
19 changes: 16 additions & 3 deletions tests/src/python/test_provider_gpkg.py
Expand Up @@ -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')
Expand All @@ -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"""
Expand All @@ -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
Expand Down

0 comments on commit 5aea8ed

Please sign in to comment.