[afs] Fix provider ignores FilterFids requests and returns

incorrect features
qgis · Feb 20, 2018 · 87207dd · 87207dd
1 parent 394e1a1
commit 87207dd
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 23 deletions.
diff --git a/src/providers/arcgisrest/qgsafsfeatureiterator.cpp b/src/providers/arcgisrest/qgsafsfeatureiterator.cpp
@@ -53,6 +53,18 @@ QgsAfsFeatureIterator::QgsAfsFeatureIterator( QgsAfsFeatureSource *source, bool
     close();
     return;
   }
+
+  if ( mRequest.filterType() == QgsFeatureRequest::FilterFids )
+  {
+    mUsingFeatureIdList = true;
+    mFeatureIdList = mRequest.filterFids();
+    mRemainingFeatureIds = mFeatureIdList;
+  }
+  else if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
+  {
+    mFeatureIdList.insert( mRequest.filterFid() );
+    mRemainingFeatureIds = mFeatureIdList;
+  }
 }
 
 QgsAfsFeatureIterator::~QgsAfsFeatureIterator()
@@ -70,27 +82,48 @@ bool QgsAfsFeatureIterator::fetchFeature( QgsFeature &f )
   if ( mFeatureIterator >= mSource->sharedData()->featureCount() )
     return false;
 
-  if ( mRequest.filterType() == QgsFeatureRequest::FilterFid )
+  switch ( mRequest.filterType() )
   {
-    bool result = mSource->sharedData()->getFeature( mRequest.filterFid(), f );
-    geometryToDestinationCrs( f, mTransform );
-    f.setValid( result );
-    return result;
-  }
-  else
-  {
-    QgsRectangle filterRect = mSource->sharedData()->extent();
-    if ( !mRequest.filterRect().isNull() )
-      filterRect = filterRect.intersect( &mFilterRect );
-    while ( mFeatureIterator < mSource->sharedData()->featureCount() )
+    case QgsFeatureRequest::FilterFid:
     {
-      bool success = mSource->sharedData()->getFeature( mFeatureIterator, f, filterRect );
-      ++mFeatureIterator;
-      if ( !success )
-        continue;
+      if ( mRemainingFeatureIds.empty() )
+        return false;
+
+      bool result = mSource->sharedData()->getFeature( mRequest.filterFid(), f );
       geometryToDestinationCrs( f, mTransform );
-      f.setValid( true );
-      return true;
+      f.setValid( result );
+      mRemainingFeatureIds.remove( f.id() );
+      return result;
+    }
+
+    case QgsFeatureRequest::FilterFids:
+    case QgsFeatureRequest::FilterExpression:
+    case QgsFeatureRequest::FilterNone:
+    {
+      QgsRectangle filterRect;
+      if ( !mRequest.filterRect().isNull() )
+      {
+        filterRect = mSource->sharedData()->extent();
+        filterRect = filterRect.intersect( &mFilterRect );
+      }
+      while ( mFeatureIterator < mSource->sharedData()->featureCount() )
+      {
+        bool success = mSource->sharedData()->getFeature( mFeatureIterator, f, filterRect );
+        ++mFeatureIterator;
+        if ( !success )
+          continue;
+        if ( mUsingFeatureIdList )
+        {
+          if ( !mRemainingFeatureIds.contains( f.id() ) )
+            continue;
+          else
+            mRemainingFeatureIds.remove( f.id() );
+        }
+        geometryToDestinationCrs( f, mTransform );
+        f.setValid( true );
+        return true;
+      }
+      return false;
     }
   }
   return false;
@@ -101,6 +134,7 @@ bool QgsAfsFeatureIterator::rewind()
   if ( mClosed )
     return false;
   mFeatureIterator = 0;
+  mRemainingFeatureIds = mFeatureIdList;
   return true;
 }
 

diff --git a/src/providers/arcgisrest/qgsafsfeatureiterator.h b/src/providers/arcgisrest/qgsafsfeatureiterator.h
@@ -21,6 +21,7 @@
 
 class QgsSpatialIndex;
 
+typedef QMap<QgsFeatureId, QgsFeature> QgsFeatureMap;
 
 class QgsAfsFeatureSource : public QgsAbstractFeatureSource
 {
@@ -49,6 +50,11 @@ class QgsAfsFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsAfs
 
   private:
     QgsFeatureId mFeatureIterator = 0;
+
+    bool mUsingFeatureIdList = false;
+    QgsFeatureIds mFeatureIdList;
+    QgsFeatureIds mRemainingFeatureIds;
+
     QgsCoordinateTransform mTransform;
     QgsRectangle mFilterRect;
 };

diff --git a/src/providers/arcgisrest/qgsarcgisrestutils.cpp b/src/providers/arcgisrest/qgsarcgisrestutils.cpp
@@ -396,7 +396,7 @@ QVariantMap QgsArcGisRestUtils::getObjects( const QString &layerurl, const QList
   }
   queryUrl.addQueryItem( QStringLiteral( "returnM" ), fetchM ? "true" : "false" );
   queryUrl.addQueryItem( QStringLiteral( "returnZ" ), fetchZ ? "true" : "false" );
-  if ( !filterRect.isEmpty() )
+  if ( !filterRect.isNull() )
   {
     queryUrl.addQueryItem( QStringLiteral( "geometry" ), QStringLiteral( "%1,%2,%3,%4" )
                            .arg( filterRect.xMinimum(), 0, 'f', -1 ).arg( filterRect.yMinimum(), 0, 'f', -1 )
@@ -483,7 +483,7 @@ QUrl QgsArcGisRestUtils::parseUrl( const QUrl &url )
     QgsDebugMsg( QString( "Get %1" ).arg( modifiedUrlString ) );
     modifiedUrlString = modifiedUrlString.mid( QStringLiteral( "http://" ).size() );
     QString args = modifiedUrlString.mid( modifiedUrlString.indexOf( '?' ) );
-    if ( modifiedUrlString.size() > 256 )
+    if ( modifiedUrlString.size() > 150 )
     {
       args = QCryptographicHash::hash( args.toUtf8(), QCryptographicHash::Md5 ).toHex();
     }

diff --git a/tests/src/python/test_provider_afs.py b/tests/src/python/test_provider_afs.py
@@ -43,7 +43,7 @@ def sanitize(endpoint, x):
         x = x[len('/query'):]
         endpoint = endpoint + '_query'
 
-    if len(endpoint + x) > 256:
+    if len(endpoint + x) > 150:
         ret = endpoint + hashlib.md5(x.encode()).hexdigest()
         # print('Before: ' + endpoint + x)
         # print('After:  ' + ret)
@@ -130,8 +130,97 @@ def setUpClass(cls):
         assert cls.vl.isValid()
         cls.source = cls.vl.dataProvider()
 
-        '"f=json&objectIds=5,3,1,2,4&inSR=&outSR=&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false&geometry=-71.123000,66.330000,-65.320000,78.300000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects"'
-        '"f=json&objectIds=5,3,1,2,4&inSR=&outSR=&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false&geometry=-71.123000,66.330000,-65.320000,78.300000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects"'
+        with open(sanitize(endpoint,
+                           '/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false'),
+                  'wb') as f:
+            f.write("""
+        {
+         "displayFieldName": "name",
+         "fieldAliases": {
+          "name": "name"
+         },
+         "geometryType": "esriGeometryPoint",
+         "spatialReference": {
+          "wkid": 4326,
+          "latestWkid": 4326
+         },
+         "fields":[{"name":"OBJECTID","type":"esriFieldTypeOID","alias":"OBJECTID","domain":null},
+        {"name":"pk","type":"esriFieldTypeInteger","alias":"pk","domain":null},
+        {"name":"cnt","type":"esriFieldTypeInteger","alias":"cnt","domain":null},
+        {"name":"name","type":"esriFieldTypeString","alias":"name","length":100,"domain":null},
+        {"name":"name2","type":"esriFieldTypeString","alias":"name2","length":100,"domain":null},
+        {"name":"num_char","type":"esriFieldTypeString","alias":"num_char","length":100,"domain":null},
+        {"name":"Shape","type":"esriFieldTypeGeometry","alias":"Shape","domain":null}],
+         "features": [
+          {
+           "attributes": {
+            "OBJECTID": 5,
+            "pk": 5,
+            "cnt": -200,
+            "name": null,
+            "name2":"NuLl",
+            "num_char":"5"    
+           },
+           "geometry": {
+            "x": -71.123,
+            "y": 78.23
+           }
+          },
+          {
+           "attributes": {
+            "OBJECTID": 3,
+            "pk": 3,
+            "cnt": 300,
+            "name": "Pear",
+            "name2":"PEaR",
+            "num_char":"3"   
+           },
+           "geometry": null
+          },
+          {
+           "attributes": {
+            "OBJECTID": 1,
+            "pk": 1,
+            "cnt": 100,
+            "name": "Orange",
+            "name2":"oranGe",
+            "num_char":"1"    
+           },
+           "geometry": {
+            "x": -70.332,
+            "y": 66.33
+           }
+          },
+          {
+           "attributes": {
+            "OBJECTID": 2,
+            "pk": 2,
+            "cnt": 200,
+            "name": "Apple",
+            "name2":"Apple",
+            "num_char":"2"    
+           },
+           "geometry": {
+            "x": -68.2,
+            "y": 70.8
+           }
+          },
+          {
+           "attributes": {
+            "OBJECTID": 4,
+            "pk": 4,
+            "cnt": 400,
+            "name": "Honey",
+            "name2":"Honey",
+            "num_char":"4"    
+           },
+           "geometry": {
+            "x": -65.32,
+            "y": 78.3
+           }
+          }
+         ]
+        }""".encode('UTF-8'))
 
         with open(sanitize(endpoint, '/query?f=json&objectIds=5,3,1,2,4&inSR=4326&outSR=4326&returnGeometry=true&outFields=OBJECTID,pk,cnt,name,name2,num_char&returnM=false&returnZ=false&geometry=-71.123000,66.330000,-65.320000,78.300000&geometryType=esriGeometryEnvelope&spatialRel=esriSpatialRelEnvelopeIntersects'), 'wb') as f:
             f.write("""