Add delimited provider to common provider tests, fix records missing

from delimited layers when geometry was null.
qgis · Jun 23, 2015 · 99150dd · 99150dd
1 parent 89af061
commit 99150dd
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 14 deletions.
diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.cpp
@@ -288,17 +288,21 @@ bool QgsDelimitedTextFeatureIterator::nextFeatureInternal( QgsFeature& feature )
 
     if ( mLoadGeometry )
     {
+      bool nullGeom = false;
       if ( mSource->mGeomRep == QgsDelimitedTextProvider::GeomAsWkt )
       {
-        geom = loadGeometryWkt( tokens );
+        geom = loadGeometryWkt( tokens, nullGeom );
       }
       else if ( mSource->mGeomRep == QgsDelimitedTextProvider::GeomAsXy )
       {
-        geom = loadGeometryXY( tokens );
+        geom = loadGeometryXY( tokens, nullGeom );
       }
 
-      if ( ! geom )
+      if (( !geom && !nullGeom ) || ( nullGeom && mTestGeometry ) )
       {
+        // if we didn't get a geom and not because it's null, or we got a null
+        // geom and we are testing for intersecting geometries then ignore this
+        // record
         continue;
       }
     }
@@ -355,11 +359,17 @@ bool QgsDelimitedTextFeatureIterator::setNextFeatureId( qint64 fid )
 
 
 
-QgsGeometry* QgsDelimitedTextFeatureIterator::loadGeometryWkt( const QStringList& tokens )
+QgsGeometry* QgsDelimitedTextFeatureIterator::loadGeometryWkt( const QStringList& tokens, bool &isNull )
 {
   QgsGeometry* geom = 0;
   QString sWkt = tokens[mSource->mWktFieldIndex];
+  if ( sWkt.isEmpty() )
+  {
+    isNull = true;
+    return 0;
+  }
 
+  isNull = false;
   geom = QgsDelimitedTextProvider::geomFromWkt( sWkt, mSource->mWktHasPrefix, mSource->mWktHasZM );
 
   if ( geom && geom->type() != mSource->mGeometryType )
@@ -375,10 +385,16 @@ QgsGeometry* QgsDelimitedTextFeatureIterator::loadGeometryWkt( const QStringList
   return geom;
 }
 
-QgsGeometry* QgsDelimitedTextFeatureIterator::loadGeometryXY( const QStringList& tokens )
+QgsGeometry* QgsDelimitedTextFeatureIterator::loadGeometryXY( const QStringList& tokens, bool &isNull )
 {
   QString sX = tokens[mSource->mXFieldIndex];
   QString sY = tokens[mSource->mYFieldIndex];
+  if ( sX.isEmpty() && sY.isEmpty() )
+  {
+    isNull = true;
+    return 0;
+  }
+  isNull = false;
   QgsPoint pt;
   bool ok = QgsDelimitedTextProvider::pointFromXY( sX, sY, pt, mSource->mDecimalPoint, mSource->mXyDms );
 

diff --git a/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.h b/src/providers/delimitedtext/qgsdelimitedtextfeatureiterator.h
@@ -84,8 +84,8 @@ class QgsDelimitedTextFeatureIterator : public QgsAbstractFeatureIteratorFromSou
     bool setNextFeatureId( qint64 fid );
 
     bool nextFeatureInternal( QgsFeature& feature );
-    QgsGeometry* loadGeometryWkt( const QStringList& tokens );
-    QgsGeometry* loadGeometryXY( const QStringList& tokens );
+    QgsGeometry* loadGeometryWkt( const QStringList& tokens, bool &isNull );
+    QgsGeometry* loadGeometryXY( const QStringList& tokens, bool &isNull );
     void fetchAttribute( QgsFeature& feature, int fieldIdx, const QStringList& tokens );
 
     QList<QgsFeatureId> mFeatureIds;

diff --git a/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp b/src/providers/delimitedtext/qgsdelimitedtextprovider.cpp
@@ -436,7 +436,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes )
       if ( mWktFieldIndex >= parts.size() || parts[mWktFieldIndex].isEmpty() )
       {
         nEmptyGeometry++;
-        geomValid = false;
+        mNumberFeatures++;
       }
       else
       {
@@ -504,12 +504,12 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes )
       // Get the x and y values, first checking to make sure they
       // aren't null.
 
-      QString sX = mXFieldIndex < parts.size() ? parts[mXFieldIndex] : "";
-      QString sY = mYFieldIndex < parts.size() ? parts[mYFieldIndex] : "";
+      QString sX = mXFieldIndex < parts.size() ? parts[mXFieldIndex] : QString();
+      QString sY = mYFieldIndex < parts.size() ? parts[mYFieldIndex] : QString();
       if ( sX.isEmpty() && sY.isEmpty() )
       {
-        geomValid = false;
         nEmptyGeometry++;
+        mNumberFeatures++;
       }
       else
       {
@@ -682,7 +682,7 @@ void QgsDelimitedTextProvider::scanFile( bool buildIndexes )
   if ( nBadFormatRecords > 0 )
     warnings.append( tr( "%1 records discarded due to invalid format" ).arg( nBadFormatRecords ) );
   if ( nEmptyGeometry > 0 )
-    warnings.append( tr( "%1 records discarded due to missing geometry definitions" ).arg( nEmptyGeometry ) );
+    warnings.append( tr( "%1 records have missing geometry definitions" ).arg( nEmptyGeometry ) );
   if ( nInvalidGeometry > 0 )
     warnings.append( tr( "%1 records discarded due to invalid geometry definitions" ).arg( nInvalidGeometry ) );
   if ( nIncompatibleGeometry > 0 )

diff --git a/tests/src/python/providertestbase.py b/tests/src/python/providertestbase.py
@@ -92,4 +92,4 @@ def testUnique(self):
         assert set([u'Apple', u'Honey', u'Orange', u'Pear', NULL]) == set(self.provider.uniqueValues(2)), 'Got {}'.format(set(self.provider.uniqueValues(2)))
 
     def testFeatureCount(self):
-        assert self.provider.featureCount() == 5
+        assert self.provider.featureCount() == 5, 'Got {}'.format( self.provider.featureCount() )
diff --git a/tests/src/python/test_qgsdelimitedtextprovider.py b/tests/src/python/test_qgsdelimitedtextprovider.py
@@ -53,7 +53,10 @@
                        compareWkt
                        )
 
+from providertestbase import ProviderTestCase
+
 QGISAPP, CANVAS, IFACE, PARENT = getQgisTestApp()
+TEST_DATA_DIR = unitTestDataPath()
 
 import sip
 sipversion=str(sip.getapi('QVariant'))
@@ -299,7 +302,41 @@ def runTest( file, requests, **params ):
 
     assert len(failures) == 0,"\n".join(failures)
 
-class TestQgsDelimitedTextProvider(TestCase):
+class TestQgsDelimitedTextProviderXY(TestCase, ProviderTestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        """Run before all tests"""
+        # Create test layer
+        srcpath = os.path.join(TEST_DATA_DIR, 'provider')
+        cls.basetestfile = os.path.join(srcpath, 'delimited_xy.csv')
+
+        cls.vl = QgsVectorLayer(u'{}?crs=epsg:4326&type=csv&xField=X&yField=Y&spatialIndex=no&subsetIndex=no&watchFile=no'.format(cls.basetestfile), u'test', u'delimitedtext')
+        assert (cls.vl.isValid())
+        cls.provider = cls.vl.dataProvider()
+
+    @classmethod
+    def tearDownClass(cls):
+        """Run after all tests"""
+
+class TestQgsDelimitedTextProviderWKT(TestCase, ProviderTestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        """Run before all tests"""
+        # Create test layer
+        srcpath = os.path.join(TEST_DATA_DIR, 'provider')
+        cls.basetestfile = os.path.join(srcpath, 'delimited_wkt.csv')
+
+        cls.vl = QgsVectorLayer(u'{}?crs=epsg:4326&type=csv&wktField=wkt&spatialIndex=no&subsetIndex=no&watchFile=no'.format(cls.basetestfile), u'test', u'delimitedtext')
+        assert (cls.vl.isValid())
+        cls.provider = cls.vl.dataProvider()
+
+    @classmethod
+    def tearDownClass(cls):
+        """Run after all tests"""
+
+class TestQgsDelimitedTextProviderOther(TestCase):
 
     def test_001_provider_defined( self ):
         registry=QgsProviderRegistry.instance()

diff --git a/tests/testdata/provider/delimited_wkt.csv b/tests/testdata/provider/delimited_wkt.csv
@@ -0,0 +1,6 @@
+pk,cnt,name,wkt
+5,-200,,"Point(-71.123 78.23)"
+3,300,Pear,
+1,100,Orange,"Point(-70.332 66.33)"
+2,200,Apple,"Point(-68.2 70.8)"
+4,400,Honey,"Point(-65.32 78.3)"
diff --git a/tests/testdata/provider/delimited_xy.csv b/tests/testdata/provider/delimited_xy.csv
@@ -0,0 +1,6 @@
+pk,cnt,name,X,Y
+5,-200,,-71.123,78.23
+3,300,Pear,,
+1,100,Orange,-70.332,66.33
+2,200,Apple,-68.2,70.8
+4,400,Honey,-65.32,78.3