[WFS provider] Fix #7170 / properly take into account feature count limit (refs #18935)

rouault · rouault · commit 39f09228c89a · 2018-06-22T16:36:15.000+02:00
diff --git a/src/providers/wfs/qgswfsfeatureiterator.cpp b/src/providers/wfs/qgswfsfeatureiterator.cpp
@@ -480,6 +480,10 @@ void QgsWFSFeatureDownloader::run( bool serializeFeatures, int maxFeatures )
     success = true;
     QgsGmlStreamingParser *parser = mShared->createParser();
 
+    if ( maxTotalFeatures > 0 && mTotalDownloadedFeatureCount >= maxTotalFeatures )
+    {
+      break;
+    }
     int maxFeaturesThisRequest = static_cast<int>(
                                    std::min( maxTotalFeatures - mTotalDownloadedFeatureCount,
                                        static_cast<qint64>( std::numeric_limits<int>::max() ) ) );
diff --git a/src/providers/wfs/qgswfsprovider.cpp b/src/providers/wfs/qgswfsprovider.cpp
@@ -1684,15 +1684,15 @@ bool QgsWFSProvider::getCapabilities()
   }
 
   mShared->mWFSVersion = mShared->mCaps.version;
-  if ( mShared->mURI.maxNumFeatures() > 0 && mShared->mCaps.maxFeatures > 0 )
+  if ( mShared->mURI.maxNumFeatures() > 0 && mShared->mCaps.maxFeatures > 0 && !( mShared->mCaps.supportsPaging && mShared->mURI.pagingEnabled() ) )
   {
     mShared->mMaxFeatures = std::min( mShared->mURI.maxNumFeatures(), mShared->mCaps.maxFeatures );
   }
   else if ( mShared->mURI.maxNumFeatures() > 0 )
   {
     mShared->mMaxFeatures = mShared->mURI.maxNumFeatures();
   }
-  else if ( mShared->mCaps.maxFeatures > 0 )
+  else if ( mShared->mCaps.maxFeatures > 0 && !( mShared->mCaps.supportsPaging && mShared->mURI.pagingEnabled() ) )
   {
     mShared->mMaxFeatures = mShared->mCaps.maxFeatures;
   }
@@ -1705,20 +1705,24 @@ bool QgsWFSProvider::getCapabilities()
   {
     if ( mShared->mURI.pageSize() > 0 )
     {
-      if ( mShared->mMaxFeatures > 0 )
+      if ( mShared->mCaps.maxFeatures > 0 )
       {
-        mShared->mPageSize = std::min( mShared->mURI.pageSize(), mShared->mMaxFeatures );
+        mShared->mPageSize = std::min( mShared->mURI.pageSize(), mShared->mCaps.maxFeatures );
       }
       else
       {
         mShared->mPageSize = mShared->mURI.pageSize();
       }
     }
+    else if ( mShared->mCaps.maxFeatures > 0 )
+    {
+      mShared->mPageSize = mShared->mCaps.maxFeatures;
+    }
     else
     {
       QgsSettings settings;
       mShared->mPageSize = settings.value( QStringLiteral( "wfs/max_feature_count_if_not_provided" ), "1000" ).toInt();
-      QgsDebugMsg( QString( "Server declares paging but does not advertize max feature count and user did not specify it. Using %1" ).arg( mShared->mMaxFeatures ) );
+      QgsDebugMsg( QString( "Server declares paging but does not advertize max feature count and user did not specify it. Using %1" ).arg( mShared->mPageSize ) );
     }
   }
   else
diff --git a/src/providers/wfs/qgswfsshareddata.cpp b/src/providers/wfs/qgswfsshareddata.cpp
@@ -978,7 +978,7 @@ void QgsWFSSharedData::endOfDownload( bool success, int featureCount,
     pushError( errorMsgOut );
   }
 
-  bool bDownloadLimit = truncatedResponse || ( !mCaps.supportsPaging && featureCount == mMaxFeatures && mMaxFeatures > 0 );
+  bool bDownloadLimit = truncatedResponse || ( featureCount >= mMaxFeatures && mMaxFeatures > 0 );
 
   mDownloadFinished = true;
   if ( success && !mRect.isEmpty() )

Original file line number	Diff line number	Diff line change
`@@ -1684,15 +1684,15 @@ bool QgsWFSProvider::getCapabilities()`
`1684`	`1684`	`}`
`1685`	`1685`
`1686`	`1686`	`mShared->mWFSVersion = mShared->mCaps.version;`
`1687`		`- if ( mShared->mURI.maxNumFeatures() > 0 && mShared->mCaps.maxFeatures > 0 )`
	`1687`	`+ if ( mShared->mURI.maxNumFeatures() > 0 && mShared->mCaps.maxFeatures > 0 && !( mShared->mCaps.supportsPaging && mShared->mURI.pagingEnabled() ) )`
`1688`	`1688`	`{`
`1689`	`1689`	`mShared->mMaxFeatures = std::min( mShared->mURI.maxNumFeatures(), mShared->mCaps.maxFeatures );`
`1690`	`1690`	`}`
`1691`	`1691`	`else if ( mShared->mURI.maxNumFeatures() > 0 )`
`1692`	`1692`	`{`
`1693`	`1693`	`mShared->mMaxFeatures = mShared->mURI.maxNumFeatures();`
`1694`	`1694`	`}`
`1695`		`- else if ( mShared->mCaps.maxFeatures > 0 )`
	`1695`	`+ else if ( mShared->mCaps.maxFeatures > 0 && !( mShared->mCaps.supportsPaging && mShared->mURI.pagingEnabled() ) )`
`1696`	`1696`	`{`
`1697`	`1697`	`mShared->mMaxFeatures = mShared->mCaps.maxFeatures;`
`1698`	`1698`	`}`
`@@ -1705,20 +1705,24 @@ bool QgsWFSProvider::getCapabilities()`
`1705`	`1705`	`{`
`1706`	`1706`	`if ( mShared->mURI.pageSize() > 0 )`
`1707`	`1707`	`{`
`1708`		`- if ( mShared->mMaxFeatures > 0 )`
	`1708`	`+ if ( mShared->mCaps.maxFeatures > 0 )`
`1709`	`1709`	`{`
`1710`		`- mShared->mPageSize = std::min( mShared->mURI.pageSize(), mShared->mMaxFeatures );`
	`1710`	`+ mShared->mPageSize = std::min( mShared->mURI.pageSize(), mShared->mCaps.maxFeatures );`
`1711`	`1711`	`}`
`1712`	`1712`	`else`
`1713`	`1713`	`{`
`1714`	`1714`	`mShared->mPageSize = mShared->mURI.pageSize();`
`1715`	`1715`	`}`
`1716`	`1716`	`}`
	`1717`	`+ else if ( mShared->mCaps.maxFeatures > 0 )`
	`1718`	`+ {`
	`1719`	`+ mShared->mPageSize = mShared->mCaps.maxFeatures;`
	`1720`	`+ }`
`1717`	`1721`	`else`
`1718`	`1722`	`{`
`1719`	`1723`	`QgsSettings settings;`
`1720`	`1724`	`mShared->mPageSize = settings.value( QStringLiteral( "wfs/max_feature_count_if_not_provided" ), "1000" ).toInt();`
`1721`		`- QgsDebugMsg( QString( "Server declares paging but does not advertize max feature count and user did not specify it. Using %1" ).arg( mShared->mMaxFeatures ) );`
	`1725`	`+ QgsDebugMsg( QString( "Server declares paging but does not advertize max feature count and user did not specify it. Using %1" ).arg( mShared->mPageSize ) );`
`1722`	`1726`	`}`
`1723`	`1727`	`}`
`1724`	`1728`	`else`
Original file line number	Diff line number	Diff line change
`@@ -978,7 +978,7 @@ void QgsWFSSharedData::endOfDownload( bool success, int featureCount,`
`978`	`978`	`pushError( errorMsgOut );`
`979`	`979`	`}`
`980`	`980`
`981`		`- bool bDownloadLimit = truncatedResponse \|\| ( !mCaps.supportsPaging && featureCount == mMaxFeatures && mMaxFeatures > 0 );`
	`981`	`+ bool bDownloadLimit = truncatedResponse \|\| ( featureCount >= mMaxFeatures && mMaxFeatures > 0 );`
`982`	`982`
`983`	`983`	`mDownloadFinished = true;`
`984`	`984`	`if ( success && !mRect.isEmpty() )`