Use QgsField::convertCompatible to check for errors

qgis · Jun 19, 2020 · f3bbc4d · f3bbc4d
1 parent 39068c1
commit f3bbc4d
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 82 deletions.
diff --git a/python/core/auto_generated/qgsvectorlayerutils.sip.in b/python/core/auto_generated/qgsvectorlayerutils.sip.in
@@ -175,17 +175,6 @@ The optional seed value can be used as a basis for generated values.
 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 strength or origin parameter is set then only constraints with a matching strength/origin will be checked.
-%End
-
-    static bool canConvert( const QVariant &value, const QVariant::Type destinationType );
-%Docstring
-Tests an attribute ``value`` for type compatibility, i.e. checks whether it can be converted
-to the ``destinationType``.
-Will always return ``True`` for NULL and invalid QVariants.
-
-.. seealso:: :py:func:`validateAttribute`
-
-.. versionadded:: 3.14
 %End
 
     static QgsFeature createFeature( const QgsVectorLayer *layer,

diff --git a/src/core/providers/memory/qgsmemoryprovider.cpp b/src/core/providers/memory/qgsmemoryprovider.cpp
@@ -423,7 +423,8 @@ bool QgsMemoryProvider::addFeatures( QgsFeatureList &flist, Flags )
     bool conversionError { false };
     for ( int i = 0; i < mFields.count() && ! conversionError; ++i )
     {
-      if ( ! QgsVectorLayerUtils::canConvert( it->attribute( i ), mFields.at( i ).type() ) )
+      QVariant attrValue { it->attribute( i ) };
+      if ( ! mFields.at( i ).convertCompatible( attrValue ) )
       {
         pushError( tr( "Could not add feature with attribute %1 having type %2, cannot convert to type %3" )
                    .arg( mFields.at( i ).name(), it->attribute( i ).typeName(), mFields.at( i ).typeName() ) );

diff --git a/src/core/qgsvectorfilewriter.cpp b/src/core/qgsvectorfilewriter.cpp
@@ -2414,8 +2414,8 @@ gdal::ogr_feature_unique_ptr QgsVectorFileWriter::createFeature( const QgsFeatur
       attrValue = mFieldValueConverter->convert( fldIdx, attrValue );
     }
 
-    // Check for QVariant conversion before passing attribute value to OGR
-    if ( ! QgsVectorLayerUtils::canConvert( attrValue, field.type() ) )
+    // Check for conversion before passing attribute value to OGR
+    if ( ! field.convertCompatible( attrValue ) )
     {
       mErrorMessage = QObject::tr( "Invalid variant type for field %1[%2]: received %3 with type %4" )
                       .arg( mFields.at( fldIdx ).name() )

diff --git a/src/core/qgsvectorlayerutils.cpp b/src/core/qgsvectorlayerutils.cpp
@@ -472,18 +472,6 @@ bool QgsVectorLayerUtils::validateAttribute( const QgsVectorLayer *layer, const
   return valid;
 }
 
-bool QgsVectorLayerUtils::canConvert( const QVariant &value, const QVariant::Type destinationType )
-{
-  if ( value.isNull() )
-  {
-    return true;
-  }
-
-  QVariant converted { value };
-  const bool ok { converted.canConvert( destinationType ) &&converted.convert( destinationType ) };
-  return ok && ( converted.toString() == value.toString() );
-}
-
 QgsFeature QgsVectorLayerUtils::createFeature( const QgsVectorLayer *layer, const QgsGeometry &geometry,
     const QgsAttributeMap &attributes, QgsExpressionContext *context )
 {

diff --git a/src/core/qgsvectorlayerutils.h b/src/core/qgsvectorlayerutils.h
@@ -176,16 +176,6 @@ class CORE_EXPORT QgsVectorLayerUtils
                                    QgsFieldConstraints::ConstraintStrength strength = QgsFieldConstraints::ConstraintStrengthNotSet,
                                    QgsFieldConstraints::ConstraintOrigin origin = QgsFieldConstraints::ConstraintOriginNotSet );
 
-    /**
-     * Tests an attribute \a value for type compatibility, i.e. checks whether it can be converted
-     * to the \a destinationType.
-     * Will always return TRUE for NULL and invalid QVariants.
-     *
-     * \see validateAttribute()
-     * \since QGIS 3.14
-     */
-    static bool canConvert( const QVariant &value, const QVariant::Type destinationType );
-
     /**
      * Creates a new feature ready for insertion into a layer. Default values and constraints
      * (e.g., unique constraints) will automatically be handled. An optional attribute map can be

diff --git a/tests/src/python/test_qgsfields.py b/tests/src/python/test_qgsfields.py
@@ -12,8 +12,9 @@
 
 import qgis  # NOQA
 
-from qgis.core import QgsVectorLayer
+from qgis.core import QgsVectorLayer, NULL
 from qgis.testing import start_app, unittest
+from qgis.PyQt.QtCore import QVariant, QDate, QDateTime, QTime
 
 start_app()
 
@@ -103,10 +104,62 @@ def test_names(self):
 
         expected_fields = ['id', 'value', 'crazy']
 
-        self.assertEquals(fields.names(), expected_fields)
+        self.assertEqual(fields.names(), expected_fields)
         fields.remove(1)
         expected_fields = ['id', 'crazy']
-        self.assertEquals(fields.names(), expected_fields)
+        self.assertEqual(fields.names(), expected_fields)
+
+    def test_convert_compatible(self):
+        """Test convertCompatible"""
+
+        vl = QgsVectorLayer('Point?crs=epsg:4326&field=int:integer', 'test', 'memory')
+
+        # Valid values
+        self.assertTrue(vl.fields()[0].convertCompatible(123.0))
+        self.assertTrue(vl.fields()[0].convertCompatible(123))
+        # Check NULL/invalid
+        self.assertIsNone(vl.fields()[0].convertCompatible(None))
+        self.assertEqual(vl.fields()[0].convertCompatible(QVariant(QVariant.Int)), NULL)
+        # Not valid
+        with self.assertRaises(Exception):
+            vl.fields()[0].convertCompatible('QGIS Rocks!')
+        with self.assertRaises(Exception):
+            self.assertFalse(vl.fields()[0].convertCompatible(QDate(2020, 6, 30)))
+        # Not valid: overflow
+        with self.assertRaises(Exception):
+            self.assertFalse(vl.fields()[0].convertCompatible(2147483647 + 1))
+        # Valid: narrow cast with loss of precision (!)
+        self.assertTrue(vl.fields()[0].convertCompatible(123.123))
+
+        vl = QgsVectorLayer('Point?crs=epsg:4326&field=date:date', 'test', 'memory')
+        self.assertTrue(vl.fields()[0].convertCompatible(QDate(2020, 6, 30)))
+        # Not valid
+        with self.assertRaises(Exception):
+            self.assertFalse(vl.fields()[0].convertCompatible('QGIS Rocks!'))
+        with self.assertRaises(Exception):
+            self.assertFalse(vl.fields()[0].convertCompatible(123))
+
+        # Strings can store almost anything
+        vl = QgsVectorLayer('Point?crs=epsg:4326&field=text:text', 'test', 'memory')
+        self.assertTrue(vl.fields()[0].convertCompatible(QDate(2020, 6, 30)))
+        self.assertTrue(vl.fields()[0].convertCompatible('QGIS Rocks!'))
+        self.assertTrue(vl.fields()[0].convertCompatible(123))
+        self.assertTrue(vl.fields()[0].convertCompatible(123.456))
+
+        vl = QgsVectorLayer('Point?crs=epsg:4326&field=double:double', 'test', 'memory')
+
+        # Valid values
+        self.assertTrue(vl.fields()[0].convertCompatible(123.0))
+        self.assertTrue(vl.fields()[0].convertCompatible(123))
+        # Check NULL/invalid
+        self.assertIsNone(vl.fields()[0].convertCompatible(None))
+        self.assertEqual(vl.fields()[0].convertCompatible(NULL), NULL)
+        self.assertTrue(vl.fields()[0].convertCompatible(QVariant.Double))
+        # Not valid
+        with self.assertRaises(Exception):
+            self.assertFalse(vl.fields()[0].convertCompatible('QGIS Rocks!'))
+        with self.assertRaises(Exception):
+            self.assertFalse(vl.fields()[0].convertCompatible(QDate(2020, 6, 30)))
 
 
 if __name__ == '__main__':

diff --git a/tests/src/python/test_qgsvectorlayerutils.py b/tests/src/python/test_qgsvectorlayerutils.py
@@ -690,49 +690,6 @@ def test_unique_pk_when_subset(self):
         vl.addFeatures(features)
         self.assertTrue(vl.commitChanges())
 
-    def test_check_attribute_type(self):
-        """Test checkAttributeType"""
-
-        vl = QgsVectorLayer('Point?crs=epsg:4326&field=int:integer', 'test', 'memory')
-
-        # Valid values
-        self.assertTrue(QgsVectorLayerUtils.canConvert(123.0, vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert(123, vl.fields()[0].type()))
-        # Check NULL/invalid
-        self.assertTrue(QgsVectorLayerUtils.canConvert(None, vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert(QVariant(QVariant.Int), vl.fields()[0].type()))
-        # Not valid
-        self.assertFalse(QgsVectorLayerUtils.canConvert('QGIS Rocks!', vl.fields()[0].type()))
-        self.assertFalse(QgsVectorLayerUtils.canConvert(QDate(2020, 6, 30), vl.fields()[0].type()))
-        # Not valid: overflow and narrow cast!
-        self.assertFalse(QgsVectorLayerUtils.canConvert(2147483647 + 1, vl.fields()[0].type()))
-        self.assertFalse(QgsVectorLayerUtils.canConvert(123.123, vl.fields()[0].type()))
-
-        vl = QgsVectorLayer('Point?crs=epsg:4326&field=date:date', 'test', 'memory')
-        self.assertTrue(QgsVectorLayerUtils.canConvert(QDate(2020, 6, 30), vl.fields()[0].type()))
-        # Not valid
-        self.assertFalse(QgsVectorLayerUtils.canConvert('QGIS Rocks!', vl.fields()[0].type()))
-        self.assertFalse(QgsVectorLayerUtils.canConvert(123, vl.fields()[0].type()))
-
-        # Strings can store almost anything
-        vl = QgsVectorLayer('Point?crs=epsg:4326&field=text:text', 'test', 'memory')
-        self.assertTrue(QgsVectorLayerUtils.canConvert(QDate(2020, 6, 30), vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert('QGIS Rocks!', vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert(123, vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert(123.456, vl.fields()[0].type()))
-
-        vl = QgsVectorLayer('Point?crs=epsg:4326&field=double:double', 'test', 'memory')
-
-        # Valid values
-        self.assertTrue(QgsVectorLayerUtils.canConvert(123.0, vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert(123, vl.fields()[0].type()))
-        # Check NULL/invalid
-        self.assertTrue(QgsVectorLayerUtils.canConvert(None, vl.fields()[0].type()))
-        self.assertTrue(QgsVectorLayerUtils.canConvert(QVariant.Double, vl.fields()[0].type()))
-        # Not valid
-        self.assertFalse(QgsVectorLayerUtils.canConvert('QGIS Rocks!', vl.fields()[0].type()))
-        self.assertFalse(QgsVectorLayerUtils.canConvert(QDate(2020, 6, 30), vl.fields()[0].type()))
-
 
 if __name__ == '__main__':
     unittest.main()