Skip to content

Commit

Permalink
Merge pull request #6316 from nyalldawson/locator_thread
Browse files Browse the repository at this point in the history 
Make QgsLocator more thread safe
  • Loading branch information
@nyalldawson
nyalldawson committed Feb 16, 2018
2 parents 838bde3 + 2c6100e commit a6a36ac
Show file tree
Hide file tree
Showing 12 changed files with 238 additions and 71 deletions.
3 changes: 0 additions & 3 deletions .ci/travis/linux/blacklist.txt
View file
Expand Up @@ -30,9 +30,6 @@ PyQgsSpatialiteProvider
# Flaky, see https://travis-ci.org/qgis/QGIS/jobs/297708174
PyQgsServerAccessControl

# Flaky, see https://dash.orfeo-toolbox.org/testDetails.php?test=61670866&build=297397
PyQgsLocator

# Need a local postgres installation
PyQgsAuthManagerPKIPostgresTest
PyQgsAuthManagerPasswordPostgresTest
Expand Down
37 changes: 37 additions & 0 deletions python/core/locator/qgslocatorfilter.sip.in
View file
Expand Up @@ -69,9 +69,22 @@ Abstract base class for filters which collect locator results.
Lowest
};

enum Flag
{
FlagFast,
};
typedef QFlags<QgsLocatorFilter::Flag> Flags;


QgsLocatorFilter( QObject *parent = 0 );
%Docstring
Constructor for QgsLocatorFilter.
%End

virtual QgsLocatorFilter *clone() const = 0 /Factory/;
%Docstring
Creates a clone of the filter. New requests are always executed in a
clone of the original filter.
%End

virtual QString name() const = 0;
Expand All @@ -86,6 +99,11 @@ Returns the unique name for the filter. This should be an untranslated string id
Returns a translated, user-friendly name for the filter.

.. seealso:: :py:func:`name`
%End

virtual QgsLocatorFilter::Flags flags() const;
%Docstring
Returns flags which specify the filter's behavior.
%End

virtual Priority priority() const;
Expand All @@ -108,6 +126,14 @@ results from this filter.
be ignored.
%End

virtual void prepare( const QString &string, const QgsLocatorContext &context );
%Docstring
Prepares the filter instance for an upcoming search for the specified ``string``. This method is always called
from the main thread, and individual filter subclasses should perform whatever
tasks are required in order to allow a subsequent search to safely execute
on a background thread.
%End

virtual void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) = 0;
%Docstring
Retrieves the filter results for a specified search ``string``. The ``context``
Expand All @@ -120,6 +146,9 @@ signal whenever they encounter a matching result.
Subclasses should periodically check the ``feedback`` object to determine
whether the query has been canceled. If so, the subclass should return
from this method as soon as possible.

This will be called from a background thread unless flags() returns the
QgsLocatorFilter.FlagFast flag.
%End

virtual void triggerResult( const QgsLocatorResult &result ) = 0;
Expand Down Expand Up @@ -190,6 +219,11 @@ custom configuration widget.

signals:

void finished();
%Docstring
Emitted when the filter finishes fetching results.
%End

void resultFetched( const QgsLocatorResult &result );
%Docstring
Should be emitted by filters whenever they encounter a matching result
Expand All @@ -198,6 +232,9 @@ during within their fetchResults() implementation.

};

QFlags<QgsLocatorFilter::Flag> operator|(QgsLocatorFilter::Flag f1, QFlags<QgsLocatorFilter::Flag> f2);





Expand Down
12 changes: 9 additions & 3 deletions python/plugins/processing/gui/AlgorithmLocatorFilter.py
View file
Expand Up @@ -40,6 +40,9 @@ class AlgorithmLocatorFilter(QgsLocatorFilter):
def __init__(self, parent=None):
super(AlgorithmLocatorFilter, self).__init__(parent)

def clone(self):
return AlgorithmLocatorFilter()

def name(self):
return 'processing_alg'

Expand All @@ -52,10 +55,13 @@ def priority(self):
def prefix(self):
return 'a'

def flags(self):
return QgsLocatorFilter.FlagFast

def fetchResults(self, string, context, feedback):
# collect results in main thread, since this method is inexpensive and
# accessing the processing registry is not thread safe
for a in QgsApplication.processingRegistry().algorithms():
if feedback.isCanceled():
return
if a.flags() & QgsProcessingAlgorithm.FlagHideFromToolbox:
continue

Expand All @@ -81,7 +87,7 @@ def triggerResult(self, result):
dlg.setMessage(message)
dlg.exec_()
return
dlg = alg.createCustomParametersWidget()
dlg = alg.createCustomParametersWidget(None)
if not dlg:
dlg = AlgorithmDialog(alg)
canvas = iface.mapCanvas()
Expand Down
83 changes: 50 additions & 33 deletions src/app/locator/qgsinbuiltlocatorfilters.cpp
View file
Expand Up @@ -31,19 +31,20 @@ QgsLayerTreeLocatorFilter::QgsLayerTreeLocatorFilter( QObject *parent )
: QgsLocatorFilter( parent )
{}

void QgsLayerTreeLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsLayerTreeLocatorFilter *QgsLayerTreeLocatorFilter::clone() const
{
return new QgsLayerTreeLocatorFilter();
}

void QgsLayerTreeLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback * )
{
QgsLayerTree *tree = QgsProject::instance()->layerTreeRoot();
QList<QgsLayerTreeLayer *> layers = tree->findLayers();
Q_FOREACH ( QgsLayerTreeLayer *layer, layers )
const QList<QgsLayerTreeLayer *> layers = tree->findLayers();
for ( QgsLayerTreeLayer *layer : layers )
{
if ( feedback->isCanceled() )
return;

if ( layer->layer() && stringMatches( layer->layer()->name(), string ) )
{
QgsLocatorResult result;
result.filter = this;
result.displayString = layer->layer()->name();
result.userData = layer->layerId();
result.icon = QgsMapLayerModel::iconForLayer( layer->layer() );
Expand All @@ -68,18 +69,19 @@ QgsLayoutLocatorFilter::QgsLayoutLocatorFilter( QObject *parent )
: QgsLocatorFilter( parent )
{}

void QgsLayoutLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsLayoutLocatorFilter *QgsLayoutLocatorFilter::clone() const
{
return new QgsLayoutLocatorFilter();
}

void QgsLayoutLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback * )
{
const QList< QgsMasterLayoutInterface * > layouts = QgsProject::instance()->layoutManager()->layouts();
for ( QgsMasterLayoutInterface *layout : layouts )
{
if ( feedback->isCanceled() )
return;

if ( layout && stringMatches( layout->name(), string ) )
{
QgsLocatorResult result;
result.filter = this;
result.displayString = layout->name();
result.userData = layout->name();
//result.icon = QgsMapLayerModel::iconForLayer( layer->layer() );
Expand Down Expand Up @@ -109,15 +111,20 @@ QgsActionLocatorFilter::QgsActionLocatorFilter( const QList<QWidget *> &parentOb
setUseWithoutPrefix( false );
}

void QgsActionLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsActionLocatorFilter *QgsActionLocatorFilter::clone() const
{
return new QgsActionLocatorFilter( mActionParents );
}

void QgsActionLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback * )
{
// collect results in main thread, since this method is inexpensive and
// accessing the gui actions is not thread safe

QList<QAction *> found;

Q_FOREACH ( QWidget *object, mActionParents )
for ( QWidget *object : qgis::as_const( mActionParents ) )
{
if ( feedback->isCanceled() )
return;

searchActions( string, object, found );
}
}
Expand All @@ -131,8 +138,8 @@ void QgsActionLocatorFilter::triggerResult( const QgsLocatorResult &result )

void QgsActionLocatorFilter::searchActions( const QString &string, QWidget *parent, QList<QAction *> &found )
{
QList< QWidget *> children = parent->findChildren<QWidget *>();
Q_FOREACH ( QWidget *widget, children )
const QList< QWidget *> children = parent->findChildren<QWidget *>();
for ( QWidget *widget : children )
{
searchActions( string, widget, found );
}
Expand All @@ -154,7 +161,6 @@ void QgsActionLocatorFilter::searchActions( const QString &string, QWidget *pare
if ( stringMatches( searchText, string ) )
{
QgsLocatorResult result;
result.filter = this;
result.displayString = searchText;
result.userData = QVariant::fromValue( action );
result.icon = action->icon();
Expand All @@ -171,7 +177,12 @@ QgsActiveLayerFeaturesLocatorFilter::QgsActiveLayerFeaturesLocatorFilter( QObjec
setUseWithoutPrefix( false );
}

void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
QgsActiveLayerFeaturesLocatorFilter *QgsActiveLayerFeaturesLocatorFilter::clone() const
{
return new QgsActiveLayerFeaturesLocatorFilter();
}

void QgsActiveLayerFeaturesLocatorFilter::prepare( const QString &string, const QgsLocatorContext & )
{
if ( string.length() < 3 )
return;
Expand All @@ -183,11 +194,9 @@ void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, c
if ( !layer )
return;

int found = 0;
QgsExpression dispExpression( layer->displayExpression() );
QgsExpressionContext context;
context.appendScopes( QgsExpressionContextUtils::globalProjectLayerScopes( layer ) );
dispExpression.prepare( &context );
mDispExpression = QgsExpression( layer->displayExpression() );
mContext.appendScopes( QgsExpressionContextUtils::globalProjectLayerScopes( layer ) );
mDispExpression.prepare( &mContext );

// build up request expression
QStringList expressionParts;
Expand All @@ -211,17 +220,25 @@ void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, c
req.setFlags( QgsFeatureRequest::NoGeometry );
req.setFilterExpression( expression );
req.setLimit( 30 );
mIterator = layer->getFeatures( req );

mLayerId = layer->id();
mLayerIcon = QgsMapLayerModel::iconForLayer( layer );
}

void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
{
int found = 0;
QgsFeature f;
QgsFeatureIterator it = layer->getFeatures( req );
while ( it.nextFeature( f ) )

while ( mIterator.nextFeature( f ) )
{
if ( feedback->isCanceled() )
return;

QgsLocatorResult result;
result.filter = this;

context.setFeature( f );
mContext.setFeature( f );

// find matching field content
Q_FOREACH ( const QVariant &var, f.attributes() )
Expand All @@ -236,10 +253,10 @@ void QgsActiveLayerFeaturesLocatorFilter::fetchResults( const QString &string, c
if ( result.displayString.isEmpty() )
continue; //not sure how this result slipped through...

result.description = dispExpression.evaluate( &context ).toString();
result.description = mDispExpression.evaluate( &mContext ).toString();

result.userData = QVariantList() << f.id() << layer->id();
result.icon = QgsMapLayerModel::iconForLayer( layer );
result.userData = QVariantList() << f.id() << mLayerId;
result.icon = mLayerIcon;
result.score = static_cast< double >( string.length() ) / result.displayString.size();
emit resultFetched( result );

Expand Down
19 changes: 19 additions & 0 deletions src/app/locator/qgsinbuiltlocatorfilters.h
View file
Expand Up @@ -19,6 +19,9 @@
#define QGSINBUILTLOCATORFILTERS_H

#include "qgslocatorfilter.h"
#include "qgsexpressioncontext.h"
#include "qgsfeatureiterator.h"

class QAction;

class QgsLayerTreeLocatorFilter : public QgsLocatorFilter
Expand All @@ -28,10 +31,12 @@ class QgsLayerTreeLocatorFilter : public QgsLocatorFilter
public:

QgsLayerTreeLocatorFilter( QObject *parent = nullptr );
QgsLayerTreeLocatorFilter *clone() const override;
QString name() const override { return QStringLiteral( "layertree" ); }
QString displayName() const override { return tr( "Project Layers" ); }
Priority priority() const override { return Highest; }
QString prefix() const override { return QStringLiteral( "l" ); }
QgsLocatorFilter::Flags flags() const override { return QgsLocatorFilter::FlagFast; }

void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;
Expand All @@ -45,10 +50,12 @@ class QgsLayoutLocatorFilter : public QgsLocatorFilter
public:

QgsLayoutLocatorFilter( QObject *parent = nullptr );
QgsLayoutLocatorFilter *clone() const override;
QString name() const override { return QStringLiteral( "layouts" ); }
QString displayName() const override { return tr( "Project Layouts" ); }
Priority priority() const override { return Highest; }
QString prefix() const override { return QStringLiteral( "pl" ); }
QgsLocatorFilter::Flags flags() const override { return QgsLocatorFilter::FlagFast; }

void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;
Expand All @@ -62,10 +69,12 @@ class QgsActionLocatorFilter : public QgsLocatorFilter
public:

QgsActionLocatorFilter( const QList<QWidget *> &parentObjectsForActions, QObject *parent = nullptr );
QgsActionLocatorFilter *clone() const override;
QString name() const override { return QStringLiteral( "actions" ); }
QString displayName() const override { return tr( "Actions" ); }
Priority priority() const override { return Lowest; }
QString prefix() const override { return QStringLiteral( "." ); }
QgsLocatorFilter::Flags flags() const override { return QgsLocatorFilter::FlagFast; }

void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;
Expand All @@ -84,13 +93,23 @@ class QgsActiveLayerFeaturesLocatorFilter : public QgsLocatorFilter
public:

QgsActiveLayerFeaturesLocatorFilter( QObject *parent = nullptr );
QgsActiveLayerFeaturesLocatorFilter *clone() const override;
QString name() const override { return QStringLiteral( "features" ); }
QString displayName() const override { return tr( "Active Layer Features" ); }
Priority priority() const override { return Medium; }
QString prefix() const override { return QStringLiteral( "f" ); }

void prepare( const QString &string, const QgsLocatorContext &context ) override;
void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;

private:

QgsExpression mDispExpression;
QgsExpressionContext mContext;
QgsFeatureIterator mIterator;
QString mLayerId;
QIcon mLayerIcon;
};


Expand Down

0 comments on commit a6a36ac

Please sign in to comment.