If the layer is NOT being edited then only check layer based constraints

and not any constraints enforced by the provider Because: 1. we want to keep browsing features nice and responsive. It's nice to give feedback as to whether the value checks out, but not if it's too slow to do so. Some constraints (eg unique) can be expensive to test. A user can freely remove a layer-based constraint if it proves to be too slow to test, but they are unlikely to have any control over provider-side constraints 2. the provider has already accepted the value, so presumably it doesn't violate the constraint and there's no point rechecking!
qgis · Nov 2, 2016 · a6319a4 · a6319a4
1 parent c98d380
commit a6319a4
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 37 deletions.
diff --git a/python/core/qgsvectorlayerutils.sip b/python/core/qgsvectorlayerutils.sip
@@ -22,7 +22,9 @@ class QgsVectorLayerUtils
     /**
      * Tests an attribute value to check whether it passes all constraints which are present on the corresponding field.
      * Returns true if the attribute value is valid for the field. Any constraint failures will be reported in the errors argument.
+     * If the origin parameter is set then only constraints with a matching origin will be checked.
      */
-    static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors /Out/ );
+    static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors /Out/,
+                                   QgsField::ConstraintOrigin origin = QgsField::ConstraintOriginNotSet );
 
 };
diff --git a/src/core/qgsvectorlayerutils.cpp b/src/core/qgsvectorlayerutils.cpp
@@ -50,7 +50,7 @@ bool QgsVectorLayerUtils::valueExists( const QgsVectorLayer* layer, int fieldInd
   return false;
 }
 
-bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors )
+bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors, QgsField::ConstraintOrigin origin )
 {
   if ( !layer )
     return false;
@@ -63,7 +63,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
   bool valid = true;
   errors.clear();
 
-  if ( field.constraints() & QgsField::ConstraintExpression && !field.constraintExpression().isEmpty() )
+  if ( field.constraints() & QgsField::ConstraintExpression && !field.constraintExpression().isEmpty()
+       && ( origin == QgsField::ConstraintOriginNotSet || origin == field.constraintOrigin( QgsField::ConstraintExpression ) ) )
   {
     QgsExpressionContext context = layer->createExpressionContext();
     context.setFeature( feature );
@@ -86,7 +87,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
     }
   }
 
-  if ( field.constraints() & QgsField::ConstraintNotNull )
+  if ( field.constraints() & QgsField::ConstraintNotNull
+       && ( origin == QgsField::ConstraintOriginNotSet || origin == field.constraintOrigin( QgsField::ConstraintNotNull ) ) )
   {
     valid = valid && !value.isNull();
 
@@ -96,7 +98,8 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer* layer, const
     }
   }
 
-  if ( field.constraints() & QgsField::ConstraintUnique )
+  if ( field.constraints() & QgsField::ConstraintUnique
+       && ( origin == QgsField::ConstraintOriginNotSet || origin == field.constraintOrigin( QgsField::ConstraintUnique ) ) )
   {
     bool alreadyExists = QgsVectorLayerUtils::valueExists( layer, attributeIndex, value, QgsFeatureIds() << feature.id() );
     valid = valid && !alreadyExists;

diff --git a/src/core/qgsvectorlayerutils.h b/src/core/qgsvectorlayerutils.h
@@ -39,8 +39,10 @@ class CORE_EXPORT QgsVectorLayerUtils
     /**
      * Tests an attribute value to check whether it passes all constraints which are present on the corresponding field.
      * Returns true if the attribute value is valid for the field. Any constraint failures will be reported in the errors argument.
+     * If the origin parameter is set then only constraints with a matching origin will be checked.
      */
-    static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors );
+    static bool validateAttribute( const QgsVectorLayer* layer, const QgsFeature& feature, int attributeIndex, QStringList& errors,
+                                   QgsField::ConstraintOrigin origin = QgsField::ConstraintOriginNotSet );
 
 };
 

diff --git a/src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp b/src/gui/editorwidgets/core/qgseditorwidgetwrapper.cpp
@@ -106,7 +106,7 @@ void QgsEditorWidgetWrapper::updateConstraintWidgetStatus( bool constraintValid
     widget()->setStyleSheet( QStringLiteral( "background-color: #dd7777;" ) );
 }
 
-void QgsEditorWidgetWrapper::updateConstraint( const QgsFeature &ft )
+void QgsEditorWidgetWrapper::updateConstraint( const QgsFeature &ft, QgsField::ConstraintOrigin constraintOrigin )
 {
   bool toEmit( false );
   QgsField field = layer()->fields().at( mFieldIdx );
@@ -150,7 +150,7 @@ void QgsEditorWidgetWrapper::updateConstraint( const QgsFeature &ft )
   }
 
   QStringList errors;
-  mValidConstraint = QgsVectorLayerUtils::validateAttribute( layer(), ft, mFieldIdx, errors );
+  mValidConstraint = QgsVectorLayerUtils::validateAttribute( layer(), ft, mFieldIdx, errors, constraintOrigin );
   mConstraintFailureReason = errors.join( ", " );
 
   if ( toEmit )

diff --git a/src/gui/editorwidgets/core/qgseditorwidgetwrapper.h b/src/gui/editorwidgets/core/qgseditorwidgetwrapper.h
@@ -118,9 +118,11 @@ class GUI_EXPORT QgsEditorWidgetWrapper : public QgsWidgetWrapper
     /**
      * Update constraint.
      * @param featureContext the feature to use to evaluate the constraint
+     * @param constraintOrigin optional origin for constraints to check. This can be used to limit the constraints tested
+     * to only provider or layer based constraints.
      * @note added in QGIS 2.16
      */
-    void updateConstraint( const QgsFeature &featureContext );
+    void updateConstraint( const QgsFeature &featureContext, QgsField::ConstraintOrigin constraintOrigin = QgsField::ConstraintOriginNotSet );
 
     /**
      * Get the current constraint status.

diff --git a/src/gui/qgsattributeform.cpp b/src/gui/qgsattributeform.cpp
@@ -674,30 +674,6 @@ void QgsAttributeForm::onAttributeChanged( const QVariant& value )
       break;
   }
 
-  if ( eww->layer()->fields().at( eww->fieldIdx() ).constraints() & QgsField::ConstraintNotNull )
-  {
-    QLabel* buddy = mBuddyMap.value( eww->widget() );
-
-    if ( buddy )
-    {
-      if ( !buddy->property( "originalText" ).isValid() )
-        buddy->setProperty( "originalText", buddy->text() );
-
-      QString text = buddy->property( "originalText" ).toString();
-
-      if ( value.isNull() )
-      {
-        // not good
-        buddy->setText( QStringLiteral( "%1<font color=\"red\">❌</font>" ).arg( text ) );
-      }
-      else
-      {
-        // good
-        buddy->setText( QStringLiteral( "%1<font color=\"green\">✔</font>" ).arg( text ) );
-      }
-    }
-  }
-
   updateConstraints( eww );
 
   // emit
@@ -720,14 +696,25 @@ void QgsAttributeForm::updateConstraints( QgsEditorWidgetWrapper *eww )
   QgsFeature ft;
   if ( currentFormFeature( ft ) )
   {
+    // if the layer is NOT being edited then we only check layer based constraints, and not
+    // any constraints enforced by the provider. Because:
+    // 1. we want to keep browsing features nice and responsive. It's nice to give feedback as to whether
+    // the value checks out, but not if it's too slow to do so. Some constraints (eg unique) can be
+    // expensive to test. A user can freely remove a layer-based constraint if it proves to be too slow
+    // to test, but they are unlikely to have any control over provider-side constraints
+    // 2. the provider has already accepted the value, so presumably it doesn't violate the constraint
+    // and there's no point rechecking!
+    QgsField::ConstraintOrigin constraintOrigin = mLayer->isEditable() ? QgsField::ConstraintOriginNotSet
+        : QgsField::ConstraintOriginLayer;
+
     // update eww constraint
-    eww->updateConstraint( ft );
+    eww->updateConstraint( ft, constraintOrigin );
 
     // update eww dependencies constraint
     QList<QgsEditorWidgetWrapper*> deps = constraintDependencies( eww );
 
     Q_FOREACH ( QgsEditorWidgetWrapper* depsEww, deps )
-      depsEww->updateConstraint( ft );
+      depsEww->updateConstraint( ft, constraintOrigin );
 
     // sync ok button status
     synchronizeEnabledState();
@@ -914,12 +901,12 @@ void QgsAttributeForm::onConstraintStatusChanged( const QString& constraint,
     if ( !ok )
     {
       // not good
-      buddy->setText( QStringLiteral( "%1<font color=\"red\">*</font>" ).arg( text ) );
+      buddy->setText( QStringLiteral( "%1<font color=\"red\">❌</font>" ).arg( text ) );
     }
     else
     {
       // good
-      buddy->setText( QStringLiteral( "%1<font color=\"green\">*</font>" ).arg( text ) );
+      buddy->setText( QStringLiteral( "%1<font color=\"green\">✔</font>" ).arg( text ) );
     }
   }
 }

diff --git a/tests/src/python/test_qgsvectorlayerutils.py b/tests/src/python/test_qgsvectorlayerutils.py
@@ -101,6 +101,10 @@ def test_validate_attribute(self):
         self.assertFalse(res)
         self.assertEqual(len(errors), 1)
         print(errors)
+        # checking only for provider constraints
+        res, errors = QgsVectorLayerUtils.validateAttribute(layer, f, 1, QgsField.ConstraintOriginProvider)
+        self.assertTrue(res)
+        self.assertEqual(len(errors), 0)
 
         # bad field expression check
         layer.setConstraintExpression(1, 'fldint>')
@@ -123,6 +127,11 @@ def test_validate_attribute(self):
         self.assertEqual(len(errors), 1)
         print(errors)
 
+        # checking only for provider constraints
+        res, errors = QgsVectorLayerUtils.validateAttribute(layer, f, 1, QgsField.ConstraintOriginProvider)
+        self.assertTrue(res)
+        self.assertEqual(len(errors), 0)
+
         # unique constraint
         f.setAttributes(["test123", 123])
         layer.setFieldConstraints(1, QgsField.Constraints())
@@ -135,6 +144,11 @@ def test_validate_attribute(self):
         self.assertEqual(len(errors), 1)
         print(errors)
 
+        # checking only for provider constraints
+        res, errors = QgsVectorLayerUtils.validateAttribute(layer, f, 1, QgsField.ConstraintOriginProvider)
+        self.assertTrue(res)
+        self.assertEqual(len(errors), 0)
+
         # check - same id should be ignored when testing for uniqueness
         f1 = QgsFeature(1)
         f1.setAttributes(["test123", 123])