diff --git a/python/core/qgsnetworkcontentfetcherregistry.sip.in b/python/core/qgsnetworkcontentfetcherregistry.sip.in index 3507466ccbac..6946771ec01e 100644 --- a/python/core/qgsnetworkcontentfetcherregistry.sip.in +++ b/python/core/qgsnetworkcontentfetcherregistry.sip.in @@ -35,7 +35,7 @@ FetchedContent holds useful information about a network content being fetched Failed }; - explicit QgsFetchedContent( QTemporaryFile *file = 0, ContentStatus status = NotStarted ); + explicit QgsFetchedContent( const QString &url, QTemporaryFile *file = 0, ContentStatus status = NotStarted ); %Docstring Constructs a FetchedContent with pointer to the downloaded file and status of the download %End @@ -76,21 +76,6 @@ Return the potential error of the download void fetched(); %Docstring Emitted when the file is fetched and accessible -%End - - void downloadStarted( const bool redownload ); -%Docstring -Emitted when the download actually starts -%End - - void cancelTriggered(); -%Docstring -Emitted when download is canceled. -%End - - void taskCompleted(); -%Docstring -Emitted when the download is finished (although file not accessible yet) %End }; @@ -125,7 +110,7 @@ Create the registry for temporary downloaded files ~QgsNetworkContentFetcherRegistry(); - const QgsFetchedContent *fetch( const QUrl &url, const FetchingMode fetchingMode = DownloadLater ); + const QgsFetchedContent *fetch( const QString &url, const FetchingMode fetchingMode = DownloadLater ); %Docstring Initialize a download for the given URL diff --git a/src/core/qgsnetworkcontentfetcherregistry.cpp b/src/core/qgsnetworkcontentfetcherregistry.cpp index 1aa805869a9c..f55e8bf0ea14 100644 --- a/src/core/qgsnetworkcontentfetcherregistry.cpp +++ b/src/core/qgsnetworkcontentfetcherregistry.cpp @@ -27,7 +27,7 @@ QgsNetworkContentFetcherRegistry::QgsNetworkContentFetcherRegistry() QgsNetworkContentFetcherRegistry::~QgsNetworkContentFetcherRegistry() { - QMap::const_iterator it = mFileRegistry.constBegin(); + QMap::const_iterator it = mFileRegistry.constBegin(); for ( ; it != mFileRegistry.constEnd(); ++it ) { delete it.value(); @@ -35,96 +35,22 @@ QgsNetworkContentFetcherRegistry::~QgsNetworkContentFetcherRegistry() mFileRegistry.clear(); } -const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QUrl &url, const FetchingMode fetchingMode ) +const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QString &url, const FetchingMode fetchingMode ) { - QMutexLocker locker( &mMutex ); + if ( mFileRegistry.contains( url ) ) { return mFileRegistry.value( url ); } - QgsFetchedContent *content = new QgsFetchedContent( nullptr, QgsFetchedContent::NotStarted ); - - // start - QObject::connect( content, &QgsFetchedContent::downloadStarted, this, [ = ]( const bool redownload ) - { - if ( redownload && content->status() == QgsFetchedContent::Downloading ) - { - { - QMutexLocker locker( &mMutex ); - if ( content->mFetchingTask ) - disconnect( content->mFetchingTask, &QgsNetworkContentFetcherTask::fetched, content, &QgsFetchedContent::taskCompleted ); - } - // no locker when calling cancel! - content->cancel(); - } - QMutexLocker locker( &mMutex ); - if ( redownload || - content->status() == QgsFetchedContent::NotStarted || - content->status() == QgsFetchedContent::Failed ) - { - content->mFetchingTask = new QgsNetworkContentFetcherTask( url ); - connect( content->mFetchingTask, &QgsNetworkContentFetcherTask::fetched, content, &QgsFetchedContent::taskCompleted ); - QgsApplication::instance()->taskManager()->addTask( content->mFetchingTask ); - content->mStatus = QgsFetchedContent::Downloading; - } - } ); - - // cancel - QObject::connect( content, &QgsFetchedContent::cancelTriggered, this, [ = ]() - { - QMutexLocker locker( &mMutex ); - if ( content->mFetchingTask && content->mFetchingTask->canCancel() ) - { - content->mFetchingTask->cancel(); - } - if ( content->mFile ) - { - content->mFile->deleteLater(); - content->mFilePath = QString(); - } - } ); - - // finished - connect( content, &QgsFetchedContent::taskCompleted, this, [ = ]() - { - QMutexLocker locker( &mMutex ); - if ( !content->mFetchingTask || !content->mFetchingTask->reply() ) - { - // if no reply, it has been canceled - content->mStatus = QgsFetchedContent::Failed; - content->mError = QNetworkReply::OperationCanceledError; - content->mFilePath = QString(); - } - else - { - QNetworkReply *reply = content->mFetchingTask->reply(); - if ( reply->error() == QNetworkReply::NoError ) - { - QTemporaryFile *tf = new QTemporaryFile( QStringLiteral( "XXXXXX" ) ); - content->mFile = tf; - tf->open(); - content->mFile->write( reply->readAll() ); - // Qt docs notes that on some system if fileName is not called before close, file might get deleted - content->mFilePath = tf->fileName(); - tf->close(); - content->mStatus = QgsFetchedContent::Finished; - } - else - { - content->mStatus = QgsFetchedContent::Failed; - content->mError = reply->error(); - content->mFilePath = QString(); - } - } - content->emitFetched(); - } ); + QgsFetchedContent *content = new QgsFetchedContent( url, nullptr, QgsFetchedContent::NotStarted ); mFileRegistry.insert( url, content ); if ( fetchingMode == DownloadImmediately ) content->download(); + return content; } @@ -135,11 +61,10 @@ QFile *QgsNetworkContentFetcherRegistry::localFile( const QString &filePathOrUrl if ( !QUrl::fromUserInput( filePathOrUrl ).isLocalFile() ) { - QMutexLocker locker( &mMutex ); - if ( mFileRegistry.contains( QUrl( path ) ) ) + if ( mFileRegistry.contains( path ) ) { - const QgsFetchedContent *content = mFileRegistry.value( QUrl( path ) ); - if ( content->status() == QgsFetchedContent::Finished && !content->file() ) + const QgsFetchedContent *content = mFileRegistry.value( path ); + if ( content && content->status() == QgsFetchedContent::Finished && content->file() ) { file = content->file(); } @@ -166,10 +91,9 @@ QString QgsNetworkContentFetcherRegistry::localPath( const QString &filePathOrUr if ( !QUrl::fromUserInput( filePathOrUrl ).isLocalFile() ) { - QMutexLocker locker( &mMutex ); - if ( mFileRegistry.contains( QUrl( path ) ) ) + if ( mFileRegistry.contains( path ) ) { - const QgsFetchedContent *content = mFileRegistry.value( QUrl( path ) ); + const QgsFetchedContent *content = mFileRegistry.value( path ); if ( content->status() == QgsFetchedContent::Finished && !content->filePath().isEmpty() ) { path = content->filePath(); @@ -190,3 +114,75 @@ QString QgsNetworkContentFetcherRegistry::localPath( const QString &filePathOrUr + +void QgsFetchedContent::download( bool redownload ) +{ + + if ( redownload && status() == QgsFetchedContent::Downloading ) + { + { + if ( mFetchingTask ) + disconnect( mFetchingTask, &QgsNetworkContentFetcherTask::taskCompleted, this, &QgsFetchedContent::taskCompleted ); + } + cancel(); + } + if ( redownload || + status() == QgsFetchedContent::NotStarted || + status() == QgsFetchedContent::Failed ) + { + mFetchingTask = new QgsNetworkContentFetcherTask( mUrl ); + // use taskCompleted which is main thread rather than fetched signal in worker thread + connect( mFetchingTask, &QgsNetworkContentFetcherTask::taskCompleted, this, &QgsFetchedContent::taskCompleted ); + QgsApplication::instance()->taskManager()->addTask( mFetchingTask ); + mStatus = QgsFetchedContent::Downloading; + } + +} + +void QgsFetchedContent::cancel() +{ + if ( mFetchingTask && mFetchingTask->canCancel() ) + { + mFetchingTask->cancel(); + } + if ( mFile ) + { + mFile->deleteLater(); + mFilePath = QString(); + } +} + + +void QgsFetchedContent::taskCompleted() +{ + if ( !mFetchingTask || !mFetchingTask->reply() ) + { + // if no reply, it has been canceled + mStatus = QgsFetchedContent::Failed; + mError = QNetworkReply::OperationCanceledError; + mFilePath = QString(); + } + else + { + QNetworkReply *reply = mFetchingTask->reply(); + if ( reply->error() == QNetworkReply::NoError ) + { + QTemporaryFile *tf = new QTemporaryFile( QStringLiteral( "XXXXXX" ) ); + mFile = tf; + tf->open(); + mFile->write( reply->readAll() ); + // Qt docs notes that on some system if fileName is not called before close, file might get deleted + mFilePath = tf->fileName(); + tf->close(); + mStatus = QgsFetchedContent::Finished; + } + else + { + mStatus = QgsFetchedContent::Failed; + mError = reply->error(); + mFilePath = QString(); + } + } + + emit fetched(); +} diff --git a/src/core/qgsnetworkcontentfetcherregistry.h b/src/core/qgsnetworkcontentfetcherregistry.h index ce37b7cfe199..5d6968845f65 100644 --- a/src/core/qgsnetworkcontentfetcherregistry.h +++ b/src/core/qgsnetworkcontentfetcherregistry.h @@ -54,8 +54,8 @@ class CORE_EXPORT QgsFetchedContent : public QObject }; //! Constructs a FetchedContent with pointer to the downloaded file and status of the download - explicit QgsFetchedContent( QTemporaryFile *file = nullptr, ContentStatus status = NotStarted ) - : QObject(), mFile( file ), mStatus( status ) {} + explicit QgsFetchedContent( const QString &url, QTemporaryFile *file = nullptr, ContentStatus status = NotStarted ) + : QObject(), mUrl( url ), mFile( file ), mStatus( status ) {} ~QgsFetchedContent() { @@ -84,36 +84,27 @@ class CORE_EXPORT QgsFetchedContent : public QObject * \brief Start the download * \param redownload if set to true, it will restart any achieved or pending download. */ - void download( bool redownload = false ) {emit downloadStarted( redownload );} + void download( bool redownload = false ); /** * @brief Cancel the download operation */ - void cancel() {emit cancelTriggered();} + void cancel(); signals: //! Emitted when the file is fetched and accessible void fetched(); - //! Emitted when the download actually starts - void downloadStarted( const bool redownload ); - - //! Emitted when download is canceled. - void cancelTriggered(); - - //! Emitted when the download is finished (although file not accessible yet) + private slots: void taskCompleted(); private: - void emitFetched() {emit fetched();} + QString mUrl; QTemporaryFile *mFile = nullptr; QString mFilePath; QgsNetworkContentFetcherTask *mFetchingTask = nullptr; ContentStatus mStatus = NotStarted; QNetworkReply::NetworkError mError = QNetworkReply::NoError; - - // allow modification of task and file from main class - friend class QgsNetworkContentFetcherRegistry; }; /** @@ -151,7 +142,7 @@ class CORE_EXPORT QgsNetworkContentFetcherRegistry : public QObject * \param fetchingMode defines if the download will start immediately or shall be manually triggered * \note If the download starts immediately, it will not redownload any already fetched or currently fetching file. */ - const QgsFetchedContent *fetch( const QUrl &url, const FetchingMode fetchingMode = DownloadLater ); + const QgsFetchedContent *fetch( const QString &url, const FetchingMode fetchingMode = DownloadLater ); #ifndef SIP_RUN @@ -169,10 +160,7 @@ class CORE_EXPORT QgsNetworkContentFetcherRegistry : public QObject QString localPath( const QString &filePathOrUrl ); private: - QMap mFileRegistry; - - //! Mutex to prevent concurrent access to the class from multiple threads at once (may corrupt the entries otherwise). - mutable QMutex mMutex; + QMap mFileRegistry; }; diff --git a/src/gui/qgsattributeform.cpp b/src/gui/qgsattributeform.cpp index 89a09e4c12c8..43498a12008e 100644 --- a/src/gui/qgsattributeform.cpp +++ b/src/gui/qgsattributeform.cpp @@ -1121,7 +1121,7 @@ void QgsAttributeForm::init() QgsDebugMsg( QString( "loading form: %1" ).arg( mLayer->editFormConfig().uiForm() ) ); const QString path = mLayer->editFormConfig().uiForm(); QFile *file = QgsApplication::instance()->networkContentFetcherRegistry()->localFile( path ); - if ( file->isReadable() && file->open( QFile::ReadOnly ) ) + if ( file && file->open( QFile::ReadOnly ) ) { QUiLoader loader; diff --git a/tests/src/python/test_qgsnetworkcontentfetcherregistry.py b/tests/src/python/test_qgsnetworkcontentfetcherregistry.py index ba901968eab3..fb8bbe6f6506 100644 --- a/tests/src/python/test_qgsnetworkcontentfetcherregistry.py +++ b/tests/src/python/test_qgsnetworkcontentfetcherregistry.py @@ -22,7 +22,6 @@ from qgis.testing import unittest, start_app from qgis.core import QgsNetworkContentFetcherRegistry, QgsFetchedContent, QgsApplication from utilities import unitTestDataPath -from qgis.PyQt.QtCore import QUrl from qgis.PyQt.QtNetwork import QNetworkReply, QNetworkRequest import socketserver import threading @@ -55,7 +54,7 @@ def __init__(self, methodName): def testFetchBadUrl(self): registry = QgsApplication.networkContentFetcherRegistry() - content = registry.fetch(QUrl('http://x')) + content = registry.fetch('http://x') self.loaded = False def check_reply(): @@ -72,7 +71,7 @@ def check_reply(): def testFetchGoodUrl(self): url = 'http://localhost:' + str(self.port) + '/qgis_local_server/index.html' registry = QgsApplication.networkContentFetcherRegistry() - content = registry.fetch(QUrl(url)) + content = registry.fetch(url) self.loaded = False def check_reply(): @@ -89,7 +88,7 @@ def check_reply(): self.assertEqual(registry.localPath(url), content.filePath()) # create new content with same URL - contentV2 = registry.fetch(QUrl(url)) + contentV2 = registry.fetch(url) self.assertEqual(contentV2.status(), QgsFetchedContent.Finished) def testFetchReloadUrl(self): @@ -99,7 +98,7 @@ def writeSimpleFile(content): self.file_content = content registry = QgsApplication.networkContentFetcherRegistry() - content = registry.fetch(QUrl('http://localhost:' + str(self.port) + '/qgis_local_server/simple_content.txt')) + content = registry.fetch('http://localhost:' + str(self.port) + '/qgis_local_server/simple_content.txt') self.loaded = False writeSimpleFile('my initial content') @@ -136,7 +135,7 @@ def testLocalPath(self): self.assertEqual(registry.localPath('xxxx'), 'xxxx') # an existent but unfinished download should return an empty path - content = registry.fetch(QUrl('xxxx')) + content = registry.fetch('xxxx') self.assertEqual(registry.localPath('xxxx'), '')