Skip to content

Commit

Permalink
Cancel previous fetch when current feature changes and an external (#…
Browse files Browse the repository at this point in the history 
…44900)

storage is set
  • Loading branch information
@troopa81
troopa81 committed Sep 16, 2021
1 parent b60400c commit 469b145
Show file tree
Hide file tree
Showing 4 changed files with 191 additions and 40 deletions.
4 changes: 3 additions & 1 deletion src/core/network/qgsnetworkcontentfetcher.cpp
View file
Expand Up @@ -74,7 +74,9 @@ void QgsNetworkContentFetcher::fetchContent( const QNetworkRequest &r, const QSt

auto onError = [ = ]( QNetworkReply::NetworkError code )
{
emit errorOccurred( code, mReply->errorString() );
// could have been canceled in the meantime
if ( mReply )
emit errorOccurred( code, mReply->errorString() );
};

#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
Expand Down
76 changes: 42 additions & 34 deletions src/gui/qgsexternalresourcewidget.cpp
View file
Expand Up @@ -317,37 +317,12 @@ void QgsExternalResourceWidget::loadDocument( const QString &path )

if ( mFileWidget->externalStorage() )
{
QgsExternalStorageFetchedContent *content = mFileWidget->externalStorage()->fetch( resolvedPath, storageAuthConfigId() );

auto onFetchFinished = [ = ]
if ( mContent )
{
if ( content->status() == Qgis::ContentStatus::Failed )
{
#ifdef WITH_QTWEBKIT
mWebView->setVisible( false );
#endif
mPixmapLabel->setVisible( false );
mLoadingLabel->setVisible( false );
mLoadingMovie->stop();
mErrorLabel->setVisible( true );

if ( messageBar() )
{
messageBar()->pushWarning( tr( "Fetching External Resource" ),
tr( "Error while fetching external resource '%1' : %2" ).arg( path, content->errorString() ) );
}
}
else if ( content->status() == Qgis::ContentStatus::Finished )
{
const QString filePath = mDocumentViewerContent == Web
? QString( "file://%1" ).arg( content->filePath() )
: content->filePath();

updateDocumentContent( filePath );
}

content->deleteLater();
};
mContent->cancel();
}

mContent = mFileWidget->externalStorage()->fetch( resolvedPath, storageAuthConfigId() );

#ifdef WITH_QTWEBKIT
mWebView->setVisible( false );
Expand All @@ -356,15 +331,48 @@ void QgsExternalResourceWidget::loadDocument( const QString &path )
mErrorLabel->setVisible( false );
mLoadingLabel->setVisible( true );
mLoadingMovie->start();
connect( content, &QgsExternalStorageFetchedContent::fetched, onFetchFinished );
connect( content, &QgsExternalStorageFetchedContent::errorOccurred, onFetchFinished );
connect( content, &QgsExternalStorageFetchedContent::canceled, onFetchFinished );
connect( mContent, &QgsExternalStorageFetchedContent::fetched, this, &QgsExternalResourceWidget::onFetchFinished );
connect( mContent, &QgsExternalStorageFetchedContent::errorOccurred, this, &QgsExternalResourceWidget::onFetchFinished );
connect( mContent, &QgsExternalStorageFetchedContent::canceled, this, &QgsExternalResourceWidget::onFetchFinished );

content->fetch();
mContent->fetch();
}
else
{
updateDocumentContent( resolvedPath );
}
}
}

void QgsExternalResourceWidget::onFetchFinished()
{
QgsExternalStorageFetchedContent *content = qobject_cast<QgsExternalStorageFetchedContent *>( sender() );

if ( content == mContent && mContent->status() == Qgis::ContentStatus::Failed )
{
#ifdef WITH_QTWEBKIT
mWebView->setVisible( false );
#endif
mPixmapLabel->setVisible( false );
mLoadingLabel->setVisible( false );
mLoadingMovie->stop();
mErrorLabel->setVisible( true );

if ( messageBar() )
{
messageBar()->pushWarning( tr( "Fetching External Resource" ),
tr( "Error while fetching external resource '%1' : %2" ).arg(
mFileWidget->filePath(), mContent->errorString() ) );
}
}
else if ( content == mContent && mContent->status() == Qgis::ContentStatus::Finished )
{
const QString filePath = mDocumentViewerContent == Web
? QString( "file://%1" ).arg( mContent->filePath() )
: mContent->filePath();

updateDocumentContent( filePath );
}

content->deleteLater();
}
4 changes: 4 additions & 0 deletions src/gui/qgsexternalresourcewidget.h
View file
Expand Up @@ -21,9 +21,11 @@ class QWebView;
class QgsPixmapLabel;
class QgsMessageBar;
class QgsExternalStorageFileWidget;
class QgsExternalStorageFetchedContent;

#include <QWidget>
#include <QVariant>
#include <QPointer>

#include "qgsfilewidget.h"
#include "qgis_gui.h"
Expand Down Expand Up @@ -196,6 +198,7 @@ class GUI_EXPORT QgsExternalResourceWidget : public QWidget

private slots:
void loadDocument( const QString &path );
void onFetchFinished();

private:
void updateDocumentViewer();
Expand Down Expand Up @@ -230,6 +233,7 @@ class GUI_EXPORT QgsExternalResourceWidget : public QWidget
QLabel *mLoadingLabel = nullptr;
QLabel *mErrorLabel = nullptr;
QMovie *mLoadingMovie = nullptr;
QPointer<QgsExternalStorageFetchedContent> mContent;

friend class TestQgsExternalResourceWidgetWrapper;
};
Expand Down
147 changes: 142 additions & 5 deletions tests/src/gui/testqgsexternalresourcewidgetwrapper.cpp
View file
Expand Up @@ -34,6 +34,9 @@
#include <QWebView>
#endif

#define SAMPLE_IMAGE QStringLiteral( "%1/sample_image.png" ).arg( TEST_DATA_DIR )
#define OTHER_SAMPLE_IMAGE QStringLiteral( "%1/sample_image2.png" ).arg( TEST_DATA_DIR )

/**
* @ingroup UnitTests
* This is a unit test for the external resource widget wrapper
Expand Down Expand Up @@ -63,6 +66,8 @@ class TestQgsExternalResourceWidgetWrapper : public QObject
void testStoreExternalDocumentCancel();
void testStoreExternalDocumentNoExpression_data();
void testStoreExternalDocumentNoExpression();
void testChangeValueBeforeLoaded();
void testChangeValueBeforeLoaded_data();

private:
std::unique_ptr<QgsVectorLayer> vl;
Expand All @@ -75,9 +80,10 @@ class QgsTestExternalStorageFetchedContent

public:

QgsTestExternalStorageFetchedContent( bool cached )
QgsTestExternalStorageFetchedContent( QString url )
: QgsExternalStorageFetchedContent()
, mCached( cached )
, mCached( url.endsWith( QStringLiteral( "cached.txt" ) ) )
, mUrl( mCached ? SAMPLE_IMAGE : url )
{
}

Expand All @@ -95,7 +101,7 @@ class QgsTestExternalStorageFetchedContent

QString filePath() const override
{
return TEST_DATA_DIR + QStringLiteral( "/sample_image.png" );
return mUrl;
}

void emitFetched()
Expand All @@ -116,6 +122,7 @@ class QgsTestExternalStorageFetchedContent
private:

bool mCached = false;
QString mUrl;
};

class QgsTestExternalStorageStoredContent
Expand Down Expand Up @@ -188,7 +195,7 @@ class QgsTestExternalStorage : public QgsExternalStorage
{
Q_UNUSED( authcfg );

sFetchContent = new QgsTestExternalStorageFetchedContent( url.endsWith( QStringLiteral( "cached.txt" ) ) );
sFetchContent = new QgsTestExternalStorageFetchedContent( url );

return sFetchContent;
}
Expand Down Expand Up @@ -380,7 +387,7 @@ void TestQgsExternalResourceWidgetWrapper::testLoadExternalDocument()
// ----------------------------------------------------
// load url
// ----------------------------------------------------
ww.setValues( QStringLiteral( "/home/test/myfile.txt" ), QVariantList() );
ww.setValues( SAMPLE_IMAGE, QVariantList() );

// content still null, fetching in progress...
QVERIFY( !ww.mQgsWidget->mPixmapLabel->isVisible() );
Expand Down Expand Up @@ -999,7 +1006,137 @@ void TestQgsExternalResourceWidgetWrapper::testStoreExternalDocumentNoExpression
delete messageBar;
}

void TestQgsExternalResourceWidgetWrapper::testChangeValueBeforeLoaded_data()
{
QTest::addColumn<int>( "documentType" );

QTest::newRow( "image" ) << static_cast<int>( QgsExternalResourceWidget::Image );
#ifdef WITH_QTWEBKIT
QTest::newRow( "webview" ) << static_cast<int>( QgsExternalResourceWidget::Web );
#endif
}

void TestQgsExternalResourceWidgetWrapper::testChangeValueBeforeLoaded()
{
// test to change value before loading of a previous value document has finished
QEventLoop loop;

QFETCH( int, documentType );

QgsMessageBar *messageBar = new QgsMessageBar;
QgsExternalResourceWidgetWrapper ww( vl.get(), 0, nullptr, messageBar, nullptr );

QWidget *widget = ww.createWidget( nullptr );
QVERIFY( widget );

QVariantMap config;
config.insert( QStringLiteral( "StorageType" ), QStringLiteral( "test" ) );
config.insert( QStringLiteral( "DocumentViewer" ), documentType );
ww.setConfig( config );

QgsFeature feat = vl->getFeature( 1 );
QVERIFY( feat.isValid() );
ww.setFeature( feat );

ww.initWidget( widget );
QVERIFY( ww.mQgsWidget );
QgsExternalStorageFileWidget *fileWidget = ww.mQgsWidget->fileWidget();
QVERIFY( fileWidget );
QCOMPARE( fileWidget->storageType(), QStringLiteral( "test" ) );

widget->show();
if ( documentType == QgsExternalResourceWidget::Image )
{
QVERIFY( ww.mQgsWidget->mPixmapLabel->isVisible() );
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
QVERIFY( !ww.mQgsWidget->mPixmapLabel->pixmap() );
#else
QVERIFY( ww.mQgsWidget->mPixmapLabel->pixmap( Qt::ReturnByValue ).isNull() );
#endif
}
#ifdef WITH_QTWEBKIT
else if ( documentType == QgsExternalResourceWidget::Web )

{
QVERIFY( ww.mQgsWidget->mWebView->isVisible() );
QCOMPARE( ww.mQgsWidget->mWebView->url().toString(), QStringLiteral( "about:blank" ) );
}
#endif

QVERIFY( !ww.mQgsWidget->mLoadingLabel->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingMovie->state() == QMovie::NotRunning );
QVERIFY( !ww.mQgsWidget->mErrorLabel->isVisible() );

// load url
ww.setValues( SAMPLE_IMAGE, QVariantList() );

// content still null, fetching in progress...
QVERIFY( !ww.mQgsWidget->mPixmapLabel->isVisible() );
QVERIFY( !ww.mQgsWidget->mWebView->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingLabel->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingMovie->state() == QMovie::Running );
QVERIFY( !ww.mQgsWidget->mErrorLabel->isVisible() );
QVERIFY( !messageBar->currentItem() );

QVERIFY( QgsTestExternalStorage::sFetchContent );

QPointer firstFetchContent = QgsTestExternalStorage::sFetchContent;

// first fetch is not over, load another file
ww.setValues( OTHER_SAMPLE_IMAGE, QVariantList() );
QVERIFY( firstFetchContent != QgsTestExternalStorage::sFetchContent );

// content still null, fetching in progress...
QVERIFY( !ww.mQgsWidget->mPixmapLabel->isVisible() );
QVERIFY( !ww.mQgsWidget->mWebView->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingLabel->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingMovie->state() == QMovie::Running );
QVERIFY( !ww.mQgsWidget->mErrorLabel->isVisible() );
QVERIFY( !messageBar->currentItem() );

firstFetchContent->emitFetched();

connect( firstFetchContent, &QObject::destroyed, &loop, &QEventLoop::quit );
loop.exec();

// content still null, fetching in progress...
QVERIFY( !ww.mQgsWidget->mPixmapLabel->isVisible() );
QVERIFY( !ww.mQgsWidget->mWebView->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingLabel->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingMovie->state() == QMovie::Running );
QVERIFY( !ww.mQgsWidget->mErrorLabel->isVisible() );
QVERIFY( !messageBar->currentItem() );

QgsTestExternalStorage::sFetchContent->emitFetched();
QCoreApplication::processEvents();

if ( documentType == QgsExternalResourceWidget::Image )
{
QVERIFY( ww.mQgsWidget->mPixmapLabel->isVisible() );
#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
QVERIFY( ww.mQgsWidget->mPixmapLabel->pixmap() );
#else
QVERIFY( !ww.mQgsWidget->mPixmapLabel->pixmap( Qt::ReturnByValue ).isNull() );
#endif
}
#ifdef WITH_QTWEBKIT
else if ( documentType == QgsExternalResourceWidget::Web )
{
QVERIFY( ww.mQgsWidget->mWebView->isVisible() );
QVERIFY( ww.mQgsWidget->mWebView->url().isValid() );
QCOMPARE( ww.mQgsWidget->mWebView->url().toString(), QStringLiteral( "file://%1" ).arg( OTHER_SAMPLE_IMAGE ) );
}
#endif

QVERIFY( !ww.mQgsWidget->mLoadingLabel->isVisible() );
QVERIFY( ww.mQgsWidget->mLoadingMovie->state() == QMovie::NotRunning );
QVERIFY( !ww.mQgsWidget->mErrorLabel->isVisible() );
QVERIFY( !messageBar->currentItem() );

// wait for the fetch content object to be destroyed
connect( QgsTestExternalStorage::sFetchContent, &QObject::destroyed, &loop, &QEventLoop::quit );
loop.exec();
}

QGSTEST_MAIN( TestQgsExternalResourceWidgetWrapper )
#include "testqgsexternalresourcewidgetwrapper.moc"

0 comments on commit 469b145

Please sign in to comment.