From 29d0c98854ad04dce1cd3e5c653a45ffd954466b Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Mon, 7 Oct 2024 14:19:12 +0200 Subject: [PATCH 1/5] Fix style constraint save when strength is NotSet Fix #58431 I feel that the underlying issue remains (which is that setting the strength to NotSet removes it from the field but not from the layer and that the default strength is Hard) but since it was clearly made in purpose 8 years ago I am not keen to change it lightly. --- src/core/vector/qgsvectorlayer.cpp | 51 +++++++++++++++++++++++-- tests/src/python/test_qgsvectorlayer.py | 29 ++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp index 7810f5273e64..eaa62e0edce1 100644 --- a/src/core/vector/qgsvectorlayer.cpp +++ b/src/core/vector/qgsvectorlayer.cpp @@ -3141,10 +3141,53 @@ bool QgsVectorLayer::writeSymbology( QDomNode &node, QDomDocument &doc, QString { QDomElement constraintElem = doc.createElement( QStringLiteral( "constraint" ) ); constraintElem.setAttribute( QStringLiteral( "field" ), field.name() ); - constraintElem.setAttribute( QStringLiteral( "constraints" ), field.constraints().constraints() ); - constraintElem.setAttribute( QStringLiteral( "unique_strength" ), field.constraints().constraintStrength( QgsFieldConstraints::ConstraintUnique ) ); - constraintElem.setAttribute( QStringLiteral( "notnull_strength" ), field.constraints().constraintStrength( QgsFieldConstraints::ConstraintNotNull ) ); - constraintElem.setAttribute( QStringLiteral( "exp_strength" ), field.constraints().constraintStrength( QgsFieldConstraints::ConstraintExpression ) ); + + // This manipulation is required because when the strength is set to NotSet the constraint is removed from + // the field but when the field is queried for the strength of a not-existing constraint it returns Hard by default, + // see issue #58431 + QgsFieldConstraints::Constraints constraints { mFieldConstraints.value( field.name() ) }; + QgsFieldConstraints::ConstraintStrength uniqueStrength { QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet }; + QgsFieldConstraints::ConstraintStrength notNullStrength { QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet }; + QgsFieldConstraints::ConstraintStrength expressionStrength { QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet }; + + QPair strengthKey { qMakePair( field.name(), QgsFieldConstraints::ConstraintUnique ) }; + + if ( mFieldConstraintStrength.contains( strengthKey ) && mFieldConstraintStrength.value( strengthKey ) == QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet ) + { + constraints.setFlag( QgsFieldConstraints::Constraint::ConstraintUnique, false ); + } + else + { + uniqueStrength = mFieldConstraintStrength.value( strengthKey ); + } + + strengthKey = qMakePair( field.name(), QgsFieldConstraints::ConstraintNotNull ); + + if ( mFieldConstraintStrength.contains( strengthKey ) && mFieldConstraintStrength.value( strengthKey ) == QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet ) + { + constraints.setFlag( QgsFieldConstraints::Constraint::ConstraintNotNull, false ); + } + else + { + notNullStrength = mFieldConstraintStrength.value( strengthKey ); + } + + strengthKey = qMakePair( field.name(), QgsFieldConstraints::ConstraintExpression ); + + if ( mFieldConstraintStrength.contains( strengthKey ) && mFieldConstraintStrength.value( strengthKey ) == QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet ) + { + constraints.setFlag( QgsFieldConstraints::Constraint::ConstraintExpression, false ); + } + else + { + expressionStrength = mFieldConstraintStrength.value( strengthKey ); + } + + + constraintElem.setAttribute( QStringLiteral( "constraints" ), constraints ); + constraintElem.setAttribute( QStringLiteral( "unique_strength" ), uniqueStrength ); + constraintElem.setAttribute( QStringLiteral( "notnull_strength" ), notNullStrength ); + constraintElem.setAttribute( QStringLiteral( "exp_strength" ), expressionStrength ); constraintsElem.appendChild( constraintElem ); } node.appendChild( constraintsElem ); diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index 881578a0bb0f..14d6c3efc260 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -4675,6 +4675,35 @@ def test_selection_properties(self): self.assertEqual(vl2.selectionProperties().selectionColor(), QColor(255, 0, 0)) + def testConstraintsStrengthNotSet(self): + """Test issue GH #58431 when strength NotSet becomes Hard""" + + # Create a memory layer with unique constraints + layer = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer", "test_unique", "memory") + self.assertTrue(layer.isValid()) + + # Se not constraint on fldtxt + layer.setFieldConstraint(0, QgsFieldConstraints.Constraint.ConstraintNotNull, QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) + layer.setFieldConstraint(0, QgsFieldConstraints.Constraint.ConstraintUnique, QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) + + # Export the style to QML + style = QgsMapLayerStyle() + temp_file = tempfile.mktemp(suffix='.qml') + layer.saveNamedStyle(temp_file) + layer.loadNamedStyle(temp_file) + + # Check constraints at the field level + field = layer.fields().at(0) + constraints = field.constraints() + self.assertEqual(constraints.constraintStrength(QgsFieldConstraints.Constraint.ConstraintNotNull), QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) + self.assertEqual(constraints.constraintStrength(QgsFieldConstraints.Constraint.ConstraintUnique), QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) + + # Check constraints at the layer level + constraints = layer.fieldConstraintsAndStrength(0) + self.assertEqual(constraints[QgsFieldConstraints.Constraint.ConstraintNotNull], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) + self.assertEqual(constraints[QgsFieldConstraints.Constraint.ConstraintUnique], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) + + # TODO: # - fetch rect: feat with changed geometry: 1. in rect, 2. out of rect # - more join tests From 6ab87a17fee9ff00d3379391f6d0bed297fd82ac Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 8 Oct 2024 09:52:42 +0200 Subject: [PATCH 2/5] Return NotSet when not set --- src/core/qgsfieldconstraints.cpp | 4 +- src/core/vector/qgsvectorlayer.cpp | 50 ++----------------------- tests/src/python/test_qgsvectorlayer.py | 20 ++++++++-- 3 files changed, 22 insertions(+), 52 deletions(-) diff --git a/src/core/qgsfieldconstraints.cpp b/src/core/qgsfieldconstraints.cpp index c9007529145f..371ea825b199 100644 --- a/src/core/qgsfieldconstraints.cpp +++ b/src/core/qgsfieldconstraints.cpp @@ -29,8 +29,8 @@ QgsFieldConstraints::ConstraintStrength QgsFieldConstraints::constraintStrength( if ( !( mConstraints & constraint ) ) return ConstraintStrengthNotSet; - // defaults to hard strength unless explicitly set - return mConstraintStrengths.value( constraint, ConstraintStrengthHard ); + // defaults to not set + return mConstraintStrengths.value( constraint, ConstraintStrengthNotSet ); } void QgsFieldConstraints::setConstraintStrength( QgsFieldConstraints::Constraint constraint, QgsFieldConstraints::ConstraintStrength strength ) diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp index eaa62e0edce1..e41fcc853292 100644 --- a/src/core/vector/qgsvectorlayer.cpp +++ b/src/core/vector/qgsvectorlayer.cpp @@ -3141,53 +3141,11 @@ bool QgsVectorLayer::writeSymbology( QDomNode &node, QDomDocument &doc, QString { QDomElement constraintElem = doc.createElement( QStringLiteral( "constraint" ) ); constraintElem.setAttribute( QStringLiteral( "field" ), field.name() ); + constraintElem.setAttribute( QStringLiteral( "constraints" ), field.constraints().constraints() ); + constraintElem.setAttribute( QStringLiteral( "unique_strength" ), field.constraints().constraintStrength( QgsFieldConstraints::ConstraintUnique ) ); + constraintElem.setAttribute( QStringLiteral( "notnull_strength" ), field.constraints().constraintStrength( QgsFieldConstraints::ConstraintNotNull ) ); + constraintElem.setAttribute( QStringLiteral( "exp_strength" ), field.constraints().constraintStrength( QgsFieldConstraints::ConstraintExpression ) ); - // This manipulation is required because when the strength is set to NotSet the constraint is removed from - // the field but when the field is queried for the strength of a not-existing constraint it returns Hard by default, - // see issue #58431 - QgsFieldConstraints::Constraints constraints { mFieldConstraints.value( field.name() ) }; - QgsFieldConstraints::ConstraintStrength uniqueStrength { QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet }; - QgsFieldConstraints::ConstraintStrength notNullStrength { QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet }; - QgsFieldConstraints::ConstraintStrength expressionStrength { QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet }; - - QPair strengthKey { qMakePair( field.name(), QgsFieldConstraints::ConstraintUnique ) }; - - if ( mFieldConstraintStrength.contains( strengthKey ) && mFieldConstraintStrength.value( strengthKey ) == QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet ) - { - constraints.setFlag( QgsFieldConstraints::Constraint::ConstraintUnique, false ); - } - else - { - uniqueStrength = mFieldConstraintStrength.value( strengthKey ); - } - - strengthKey = qMakePair( field.name(), QgsFieldConstraints::ConstraintNotNull ); - - if ( mFieldConstraintStrength.contains( strengthKey ) && mFieldConstraintStrength.value( strengthKey ) == QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet ) - { - constraints.setFlag( QgsFieldConstraints::Constraint::ConstraintNotNull, false ); - } - else - { - notNullStrength = mFieldConstraintStrength.value( strengthKey ); - } - - strengthKey = qMakePair( field.name(), QgsFieldConstraints::ConstraintExpression ); - - if ( mFieldConstraintStrength.contains( strengthKey ) && mFieldConstraintStrength.value( strengthKey ) == QgsFieldConstraints::ConstraintStrength::ConstraintStrengthNotSet ) - { - constraints.setFlag( QgsFieldConstraints::Constraint::ConstraintExpression, false ); - } - else - { - expressionStrength = mFieldConstraintStrength.value( strengthKey ); - } - - - constraintElem.setAttribute( QStringLiteral( "constraints" ), constraints ); - constraintElem.setAttribute( QStringLiteral( "unique_strength" ), uniqueStrength ); - constraintElem.setAttribute( QStringLiteral( "notnull_strength" ), notNullStrength ); - constraintElem.setAttribute( QStringLiteral( "exp_strength" ), expressionStrength ); constraintsElem.appendChild( constraintElem ); } node.appendChild( constraintsElem ); diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index 14d6c3efc260..8a630a13878d 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -4675,18 +4675,30 @@ def test_selection_properties(self): self.assertEqual(vl2.selectionProperties().selectionColor(), QColor(255, 0, 0)) - def testConstraintsStrengthNotSet(self): - """Test issue GH #58431 when strength NotSet becomes Hard""" + def testConstraintsotSet(self): + """Test that a NotSet constraint does not become a Hard constraint + when saving and loading a layer style, see issue GH #58431""" # Create a memory layer with unique constraints layer = QgsVectorLayer("Point?field=fldtxt:string&field=fldint:integer", "test_unique", "memory") self.assertTrue(layer.isValid()) - # Se not constraint on fldtxt + # Se not constraints on fldtxt layer.setFieldConstraint(0, QgsFieldConstraints.Constraint.ConstraintNotNull, QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) layer.setFieldConstraint(0, QgsFieldConstraints.Constraint.ConstraintUnique, QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) - # Export the style to QML + # Check constraints at the field level + field = layer.fields().at(0) + constraints = field.constraints() + self.assertEqual(constraints.constraintStrength(QgsFieldConstraints.Constraint.ConstraintNotNull), QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) + self.assertEqual(constraints.constraintStrength(QgsFieldConstraints.Constraint.ConstraintUnique), QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) + + # Check constraints at the layer level + constraints = layer.fieldConstraintsAndStrength(0) + self.assertEquals(constraints[QgsFieldConstraints.Constraint.ConstraintNotNull], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) + self.assertEquals(constraints[QgsFieldConstraints.Constraint.ConstraintUnique], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) + + # Export the style to QML and reload it style = QgsMapLayerStyle() temp_file = tempfile.mktemp(suffix='.qml') layer.saveNamedStyle(temp_file) From e67e7776a0f110dac4f13785ff3ed84be3d77f08 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Tue, 8 Oct 2024 15:33:35 +0200 Subject: [PATCH 3/5] Exception for constraint expression --- python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in | 3 ++- python/core/auto_generated/qgsfieldconstraints.sip.in | 3 ++- src/core/qgsfieldconstraints.cpp | 4 ++-- src/core/qgsfieldconstraints.h | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in b/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in index e96ba5a1d631..8c27d2e82162 100644 --- a/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in +++ b/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in @@ -72,7 +72,8 @@ is not present on this field. ConstraintStrength constraintStrength( Constraint constraint ) const; %Docstring Returns the strength of a field constraint, or ConstraintStrengthNotSet if the constraint -is not present on this field. +is not present on this field. If the strength is not set returns ConstraintStrengthNotSet +for anything but ConstraintExpression when returns ConstraintStrengthHard. .. seealso:: :py:func:`constraints` diff --git a/python/core/auto_generated/qgsfieldconstraints.sip.in b/python/core/auto_generated/qgsfieldconstraints.sip.in index 51ef4a9b8ba8..f26ffa6f6e28 100644 --- a/python/core/auto_generated/qgsfieldconstraints.sip.in +++ b/python/core/auto_generated/qgsfieldconstraints.sip.in @@ -72,7 +72,8 @@ is not present on this field. ConstraintStrength constraintStrength( Constraint constraint ) const; %Docstring Returns the strength of a field constraint, or ConstraintStrengthNotSet if the constraint -is not present on this field. +is not present on this field. If the strength is not set returns ConstraintStrengthNotSet +for anything but ConstraintExpression when returns ConstraintStrengthHard. .. seealso:: :py:func:`constraints` diff --git a/src/core/qgsfieldconstraints.cpp b/src/core/qgsfieldconstraints.cpp index 371ea825b199..4954916497a3 100644 --- a/src/core/qgsfieldconstraints.cpp +++ b/src/core/qgsfieldconstraints.cpp @@ -29,8 +29,8 @@ QgsFieldConstraints::ConstraintStrength QgsFieldConstraints::constraintStrength( if ( !( mConstraints & constraint ) ) return ConstraintStrengthNotSet; - // defaults to not set - return mConstraintStrengths.value( constraint, ConstraintStrengthNotSet ); + // defaults to not set for all but expressions (which does not use strength) + return mConstraintStrengths.value( constraint, constraint == ConstraintExpression ? ConstraintStrengthHard : ConstraintStrengthNotSet ); } void QgsFieldConstraints::setConstraintStrength( QgsFieldConstraints::Constraint constraint, QgsFieldConstraints::ConstraintStrength strength ) diff --git a/src/core/qgsfieldconstraints.h b/src/core/qgsfieldconstraints.h index a5e0ee5752a8..f23b4cc5005a 100644 --- a/src/core/qgsfieldconstraints.h +++ b/src/core/qgsfieldconstraints.h @@ -89,7 +89,8 @@ class CORE_EXPORT QgsFieldConstraints /** * Returns the strength of a field constraint, or ConstraintStrengthNotSet if the constraint - * is not present on this field. + * is not present on this field. If the strength is not set returns ConstraintStrengthNotSet + * for anything but ConstraintExpression when returns ConstraintStrengthHard. * \see constraints() * \see setConstraintStrength() */ From 5728a7bbc0b2f8f92a2a6c6089002dcc8179f85e Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 9 Oct 2024 08:20:21 +0200 Subject: [PATCH 4/5] Update test_qgsvectorlayer.py --- tests/src/python/test_qgsvectorlayer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index 8a630a13878d..c6b4918d00a1 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -4695,8 +4695,8 @@ def testConstraintsotSet(self): # Check constraints at the layer level constraints = layer.fieldConstraintsAndStrength(0) - self.assertEquals(constraints[QgsFieldConstraints.Constraint.ConstraintNotNull], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) - self.assertEquals(constraints[QgsFieldConstraints.Constraint.ConstraintUnique], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) + self.assertEqual(constraints[QgsFieldConstraints.Constraint.ConstraintNotNull], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthSoft) + self.assertEqual(constraints[QgsFieldConstraints.Constraint.ConstraintUnique], QgsFieldConstraints.ConstraintStrength.ConstraintStrengthNotSet) # Export the style to QML and reload it style = QgsMapLayerStyle() From 34531f7ef65c000bdabbcebdc9ce99c9b8948358 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 10 Oct 2024 14:53:20 +0200 Subject: [PATCH 5/5] Fix docstring --- python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in | 2 +- python/core/auto_generated/qgsfieldconstraints.sip.in | 2 +- src/core/qgsfieldconstraints.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in b/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in index 8c27d2e82162..adf14b5af385 100644 --- a/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in +++ b/python/PyQt6/core/auto_generated/qgsfieldconstraints.sip.in @@ -73,7 +73,7 @@ is not present on this field. %Docstring Returns the strength of a field constraint, or ConstraintStrengthNotSet if the constraint is not present on this field. If the strength is not set returns ConstraintStrengthNotSet -for anything but ConstraintExpression when returns ConstraintStrengthHard. +for anything but ConstraintExpression which returns ConstraintStrengthHard. .. seealso:: :py:func:`constraints` diff --git a/python/core/auto_generated/qgsfieldconstraints.sip.in b/python/core/auto_generated/qgsfieldconstraints.sip.in index f26ffa6f6e28..c9530a1a482c 100644 --- a/python/core/auto_generated/qgsfieldconstraints.sip.in +++ b/python/core/auto_generated/qgsfieldconstraints.sip.in @@ -73,7 +73,7 @@ is not present on this field. %Docstring Returns the strength of a field constraint, or ConstraintStrengthNotSet if the constraint is not present on this field. If the strength is not set returns ConstraintStrengthNotSet -for anything but ConstraintExpression when returns ConstraintStrengthHard. +for anything but ConstraintExpression which returns ConstraintStrengthHard. .. seealso:: :py:func:`constraints` diff --git a/src/core/qgsfieldconstraints.h b/src/core/qgsfieldconstraints.h index f23b4cc5005a..8bd87ac260ae 100644 --- a/src/core/qgsfieldconstraints.h +++ b/src/core/qgsfieldconstraints.h @@ -90,7 +90,7 @@ class CORE_EXPORT QgsFieldConstraints /** * Returns the strength of a field constraint, or ConstraintStrengthNotSet if the constraint * is not present on this field. If the strength is not set returns ConstraintStrengthNotSet - * for anything but ConstraintExpression when returns ConstraintStrengthHard. + * for anything but ConstraintExpression which returns ConstraintStrengthHard. * \see constraints() * \see setConstraintStrength() */