[feature] Allow to specify timeout when requesting features

m-kuhn · m-kuhn · commit 49de489c09db · 2017-10-25T00:53:52.000+02:00
This is potentially interesting for race conditions, but whenever it is specified,
iterators will need to be carefully checked for validity and error handling needs
to be taken care of.
diff --git a/python/core/qgsfeatureiterator.sip b/python/core/qgsfeatureiterator.sip
@@ -63,6 +63,21 @@ end of iterating: free the resources / lock
  :rtype: CompileStatus
 %End
 
+    virtual bool isValid() const;
+%Docstring
+ Returns if this iterator is valid.
+ An invalid feature iterator is not able to provide a reliable source for data.
+ If an iterator is invalid, either give up or try to send the request again (preferably
+ after a timeout to give the system some time to stay responsive).
+
+ If you want to check if the iterator successfully completed, better use QgsFeatureIterator.isClosed().
+
+.. note::
+
+   Added in QGIS 3.0
+ :rtype: bool
+%End
+
   protected:
 
     virtual bool fetchFeature( QgsFeature &f ) = 0;
@@ -145,6 +160,7 @@ Setup the simplification of geometries to fetch using the specified simplify met
  :rtype: bool
 %End
 
+
 };
 
 
@@ -233,6 +249,20 @@ destructor deletes the iterator if it has no more references
  :rtype: bool
 %End
 
+    virtual bool isValid() const;
+%Docstring
+ Will return if this iterator is valid.
+ An invalid iterator was probably introduced by a failed attempt to acquire a connection
+ or is a default constructed iterator.
+
+.. seealso:: isClosed to check if the iterator successfully completed and returned all the features.
+
+.. note::
+
+   Added in QGIS 3.0
+ :rtype: bool
+%End
+
     bool isClosed() const;
 %Docstring
 find out whether the iterator is still valid or closed already
diff --git a/python/core/qgsfeaturerequest.sip b/python/core/qgsfeaturerequest.sip
@@ -625,6 +625,19 @@ Set a subset of attributes by names that will be fetched
  :rtype: bool
 %End
 
+    int connectionTimeout() const;
+%Docstring
+ The timeout for how long we should wait for a connection if none is available from the pool
+ at this moment. A negative value (which is set by default) will wait forever.
+ :rtype: int
+%End
+
+    void setConnectionTimeout( int connectionTimeout );
+%Docstring
+ The timeout for how long we should wait for a connection if none is available from the pool
+ at this moment. A negative value (which is set by default) will wait forever.
+%End
+
   protected:
 };
 
diff --git a/python/core/qgsvectorlayerfeatureiterator.sip b/python/core/qgsvectorlayerfeatureiterator.sip
@@ -107,6 +107,8 @@ end of iterating: free the resources / lock
     };
 
 
+    virtual bool isValid() const;
+
   protected:
     virtual bool fetchFeature( QgsFeature &feature );
 %Docstring
diff --git a/src/core/qgsconnectionpool.h b/src/core/qgsconnectionpool.h
@@ -85,10 +85,11 @@ class QgsConnectionPoolGroup
     //! QgsConnectionPoolGroup cannot be copied
     QgsConnectionPoolGroup &operator=( const QgsConnectionPoolGroup &other ) = delete;
 
-    T acquire()
+    T acquire( int timeout )
     {
       // we are going to acquire a resource - if no resource is available, we will block here
-      sem.acquire();
+      if ( !sem.tryAcquire( 1, timeout ) )
+        return nullptr;
 
       // quick (preferred) way - use cached connection
       {
@@ -259,10 +260,13 @@ class QgsConnectionPool
     }
 
     /**
-     * Try to acquire a connection: if no connections are available, the thread will get blocked.
-     * \returns initialized connection or null on error
+     * Try to acquire a connection for a maximum of \a timeout milliseconds.
+     * If \a timeout is a negative value the calling thread will be blocked
+     * until a connection becomes available. This is the default behavior.
+     *
+     * \returns initialized connection or nullptr if unsuccessful
      */
-    T acquireConnection( const QString &connInfo )
+    T acquireConnection( const QString &connInfo, int timeout = -1 )
     {
       mMutex.lock();
       typename T_Groups::iterator it = mGroups.find( connInfo );
@@ -273,7 +277,7 @@ class QgsConnectionPool
       T_Group *group = *it;
       mMutex.unlock();
 
-      return group->acquire();
+      return group->acquire( timeout );
     }
 
     //! Release an existing connection so it will get back into the pool and can be reused
diff --git a/src/core/qgsfeatureiterator.cpp b/src/core/qgsfeatureiterator.cpp
@@ -235,3 +235,8 @@ QgsFeatureIterator &QgsFeatureIterator::operator=( const QgsFeatureIterator &oth
   }
   return *this;
 }
+
+bool QgsFeatureIterator::isValid() const
+{
+  return mIter && mIter->isValid();
+}
diff --git a/src/core/qgsfeatureiterator.h b/src/core/qgsfeatureiterator.h
@@ -82,6 +82,21 @@ class CORE_EXPORT QgsAbstractFeatureIterator
      */
     CompileStatus compileStatus() const { return mCompileStatus; }
 
+    /**
+     * Returns if this iterator is valid.
+     * An invalid feature iterator is not able to provide a reliable source for data.
+     * If an iterator is invalid, either give up or try to send the request again (preferably
+     * after a timeout to give the system some time to stay responsive).
+     *
+     * If you want to check if the iterator successfully completed, better use QgsFeatureIterator::isClosed().
+     *
+     * @note Added in QGIS 3.0
+     */
+    virtual bool isValid() const
+    {
+      return mValid;
+    }
+
   protected:
 
     /**
@@ -175,6 +190,16 @@ class CORE_EXPORT QgsAbstractFeatureIterator
     //! Setup the simplification of geometries to fetch using the specified simplify method
     virtual bool prepareSimplification( const QgsSimplifyMethod &simplifyMethod );
 
+    /**
+     * An invalid state of a feature iterator indicates that there was a problem with
+     * even getting it up and running.
+     * This should be set to false by subclasses if they have problems connecting to
+     * the provider.
+     * Do NOT set this to false when the feature iterator closes or has no features but
+     * we are sure, that it's just an empty dataset.
+     */
+    bool mValid = true;
+
   private:
     bool mUseCachedFeatures;
     QList<QgsIndexedFeature> mCachedFeatures;
@@ -279,6 +304,17 @@ class CORE_EXPORT QgsFeatureIterator
     bool rewind();
     bool close();
 
+    /**
+     * Will return if this iterator is valid.
+     * An invalid iterator was probably introduced by a failed attempt to acquire a connection
+     * or is a default constructed iterator.
+     *
+     * \see isClosed to check if the iterator successfully completed and returned all the features.
+     *
+     * @note Added in QGIS 3.0
+     */
+    virtual bool isValid() const;
+
     //! find out whether the iterator is still valid or closed already
     bool isClosed() const;
 
diff --git a/src/core/qgsfeaturerequest.cpp b/src/core/qgsfeaturerequest.cpp
@@ -85,6 +85,7 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
   mOrderBy = rh.mOrderBy;
   mCrs = rh.mCrs;
   mTransformErrorCallback = rh.mTransformErrorCallback;
+  mConnectionTimeout = rh.mConnectionTimeout;
   return *this;
 }
 
@@ -281,6 +282,16 @@ bool QgsFeatureRequest::acceptFeature( const QgsFeature &feature )
   return true;
 }
 
+int QgsFeatureRequest::connectionTimeout() const
+{
+  return mConnectionTimeout;
+}
+
+void QgsFeatureRequest::setConnectionTimeout( int connectionTimeout )
+{
+  mConnectionTimeout = connectionTimeout;
+}
+
 
 #include "qgsfeatureiterator.h"
 #include "qgslogger.h"
diff --git a/src/core/qgsfeaturerequest.h b/src/core/qgsfeaturerequest.h
@@ -601,6 +601,18 @@ class CORE_EXPORT QgsFeatureRequest
      */
     bool acceptFeature( const QgsFeature &feature );
 
+    /**
+     * The timeout for how long we should wait for a connection if none is available from the pool
+     * at this moment. A negative value (which is set by default) will wait forever.
+     */
+    int connectionTimeout() const;
+
+    /**
+     * The timeout for how long we should wait for a connection if none is available from the pool
+     * at this moment. A negative value (which is set by default) will wait forever.
+     */
+    void setConnectionTimeout( int connectionTimeout );
+
   protected:
     FilterType mFilter = FilterNone;
     QgsRectangle mFilterRect;
@@ -617,6 +629,7 @@ class CORE_EXPORT QgsFeatureRequest
     std::function< void( const QgsFeature & ) > mInvalidGeometryCallback;
     std::function< void( const QgsFeature & ) > mTransformErrorCallback;
     QgsCoordinateReferenceSystem mCrs;
+    int mConnectionTimeout = -1;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsFeatureRequest::Flags )
diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp
@@ -211,12 +211,8 @@ QgsVectorLayerFeatureIterator::QgsVectorLayerFeatureIterator( QgsVectorLayerFeat
   if ( mSource->mHasEditBuffer )
   {
     mChangedFeaturesRequest = mProviderRequest;
-    QgsFeatureIds changedIds;
-    QgsChangedAttributesMap::const_iterator attIt = mSource->mChangedAttributeValues.constBegin();
-    for ( ; attIt != mSource->mChangedAttributeValues.constEnd(); ++attIt )
-    {
-      changedIds << attIt.key();
-    }
+    const QgsFeatureIds changedIds = mSource->mChangedAttributeValues.keys().toSet();
+
     mChangedFeaturesRequest.setFilterFids( changedIds );
 
     if ( mChangedFeaturesRequest.limit() > 0 )
@@ -408,6 +404,11 @@ void QgsVectorLayerFeatureIterator::setInterruptionChecker( QgsInterruptionCheck
   mInterruptionChecker = interruptionChecker;
 }
 
+bool QgsVectorLayerFeatureIterator::isValid() const
+{
+  return mProviderIterator.isValid();
+}
+
 bool QgsVectorLayerFeatureIterator::fetchNextAddedFeature( QgsFeature &f )
 {
   while ( mFetchAddedFeaturesIt-- != mSource->mAddedFeatures.constBegin() )
diff --git a/src/core/qgsvectorlayerfeatureiterator.h b/src/core/qgsvectorlayerfeatureiterator.h
@@ -139,6 +139,8 @@ class CORE_EXPORT QgsVectorLayerFeatureIterator : public QgsAbstractFeatureItera
     };
 
 
+    virtual bool isValid() const override;
+
   protected:
     //! fetch next feature, return true on success
     virtual bool fetchFeature( QgsFeature &feature ) override;

Original file line number	Diff line number	Diff line change
`@@ -235,3 +235,8 @@ QgsFeatureIterator &QgsFeatureIterator::operator=( const QgsFeatureIterator &oth`
`235`	`235`	`}`
`236`	`236`	`return *this;`
`237`	`237`	`}`
	`238`	`+`
	`239`	`+bool QgsFeatureIterator::isValid() const`
	`240`	`+{`
	`241`	`+ return mIter && mIter->isValid();`
	`242`	`+}`