From f671c96f84e884f1ef546c9b00e71f300ae0e65e Mon Sep 17 00:00:00 2001 From: Vitor Vieira <155513369+VitorVieiraZ@users.noreply.github.com> Date: Mon, 20 Jan 2025 14:39:05 -0300 Subject: [PATCH 1/2] Retry failed download requests implementation (#3673) --- app/test/testmerginapi.cpp | 142 ++++++++++++++++++++++++++++++++++++- app/test/testmerginapi.h | 69 ++++++++++++++++++ core/merginapi.cpp | 111 +++++++++++++++++++---------- core/merginapi.h | 29 +++++++- 4 files changed, 312 insertions(+), 39 deletions(-) diff --git a/app/test/testmerginapi.cpp b/app/test/testmerginapi.cpp index 815563bef..8e7f648c3 100644 --- a/app/test/testmerginapi.cpp +++ b/app/test/testmerginapi.cpp @@ -2687,7 +2687,7 @@ void TestMerginApi::deleteRemoteProjectNow( MerginApi *api, const QString &proje QUrl url( api->mApiRoot + QStringLiteral( "/v2/projects/%1" ).arg( projectId ) ); request.setUrl( url ); qDebug() << "Trying to delete project " << projectName << ", id: " << projectId << " (" << url << ")"; - QNetworkReply *r = api->mManager.deleteResource( request ); + QNetworkReply *r = api->mManager->deleteResource( request ); QSignalSpy spy( r, &QNetworkReply::finished ); spy.wait( TestUtils::SHORT_REPLY ); @@ -2942,3 +2942,143 @@ void TestMerginApi::testParseVersion() QCOMPARE( major, 2024 ); QCOMPARE( minor, 4 ); } + +void TestMerginApi::testDownloadWithNetworkError() +{ + // Store original manager + QNetworkAccessManager *originalManager = mApi->networkManager(); + + QString projectName = "testDownloadRetry"; + createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" ); + + // Errors to test + QList errorsToTest = + { + QNetworkReply::TimeoutError, + QNetworkReply::NetworkSessionFailedError + }; + + foreach ( QNetworkReply::NetworkError networkError, errorsToTest ) + { + // Create mock manager - initially not failing + MockNetworkManager *failingManager = new MockNetworkManager( this ); + mApi->setNetworkManager( failingManager ); + + // Create signal spies + QSignalSpy startSpy( mApi, &MerginApi::pullFilesStarted ); + QSignalSpy retrySpy( mApi, &MerginApi::downloadItemRetried ); + QSignalSpy finishSpy( mApi, &MerginApi::syncProjectFinished ); + + // Trigger the current network error when download starts + connect( mApi, &MerginApi::pullFilesStarted, this, [this, failingManager, networkError]() + { + failingManager->setShouldFail( true, networkError ); + } ); + + mApi->pullProject( mWorkspaceName, projectName ); + + // Verify a transaction was created + QCOMPARE( mApi->transactions().count(), 1 ); + + // Wait for download to start and then fail + QVERIFY( startSpy.wait( TestUtils::LONG_REPLY ) ); + QVERIFY( finishSpy.wait( TestUtils::LONG_REPLY ) ); + + // Verify signals were emitted + QVERIFY( startSpy.count() > 0 ); + QVERIFY( retrySpy.count() > 0 ); + QCOMPARE( finishSpy.count(), 1 ); + + // Verify that MAX_RETRY_COUNT retry attempts were made + int maxRetries = TransactionStatus::MAX_RETRY_COUNT; + QCOMPARE( retrySpy.count(), maxRetries ); + + // Verify sync failed + QList arguments = finishSpy.takeFirst(); + QVERIFY( !arguments.at( 1 ).toBool() ); + + // Verify no local project was created + LocalProject localProject = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName ); + QVERIFY( !localProject.isValid() ); + + // Disconnect all signals + disconnect( mApi, &MerginApi::pullFilesStarted, this, nullptr ); + + // Clean up + mApi->setNetworkManager( originalManager ); + delete failingManager; + } +} + +void TestMerginApi::testDownloadWithNetworkErrorRecovery() +{ + // Store original manager + QNetworkAccessManager *originalManager = mApi->networkManager(); + + QString projectName = "testDownloadRetryRecovery"; + createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" ); + + // Create mock manager - initially not failing + MockNetworkManager *failingManager = new MockNetworkManager( this ); + mApi->setNetworkManager( failingManager ); + + // Create signal spies + QSignalSpy startSpy( mApi, &MerginApi::pullFilesStarted ); + QSignalSpy retrySpy( mApi, &MerginApi::downloadItemRetried ); + QSignalSpy finishSpy( mApi, &MerginApi::syncProjectFinished ); + + // Counter to track retry attempts + int retryCount = 0; + QNetworkReply::NetworkError networkError = QNetworkReply::TimeoutError; + + // Reset network after two retries + connect( mApi, &MerginApi::downloadItemRetried, this, [&retryCount, failingManager, this]() + { + retryCount++; + if ( retryCount == 2 ) + { + failingManager->setShouldFail( false ); + disconnect( mApi, &MerginApi::pullFilesStarted, nullptr, nullptr ); + disconnect( mApi, &MerginApi::downloadItemRetried, nullptr, nullptr ); + } + } ); + + // Trigger network error when download starts + connect( mApi, &MerginApi::pullFilesStarted, this, [failingManager, networkError]() + { + failingManager->setShouldFail( true, networkError ); + } ); + + mApi->pullProject( mWorkspaceName, projectName ); + + // Verify a transaction was created + QCOMPARE( mApi->transactions().count(), 1 ); + + // Wait for download to start, retry twice, and then complete successfully + QVERIFY( startSpy.wait( TestUtils::LONG_REPLY ) ); + QVERIFY( finishSpy.wait( TestUtils::LONG_REPLY ) ); + + // Verify signals were emitted + QVERIFY( startSpy.count() > 0 ); + QCOMPARE( retrySpy.count(), 2 ); // Should have exactly 2 retries + QCOMPARE( finishSpy.count(), 1 ); + + // Verify sync succeeded + QList arguments = finishSpy.takeFirst(); + QVERIFY( arguments.at( 1 ).toBool() ); + + // Verify local project was created successfully + LocalProject localProject = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName ); + QVERIFY( localProject.isValid() ); + + // Verify project files were downloaded correctly + QString projectDir = mApi->projectsPath() + "/" + projectName; + QStringList projectFiles = QDir( projectDir ).entryList( QDir::Files ); + QVERIFY( projectFiles.count() > 0 ); + QVERIFY( projectFiles.contains( "project.qgs" ) ); + + // Clean up + mApi->setNetworkManager( originalManager ); + delete failingManager; +} + diff --git a/app/test/testmerginapi.h b/app/test/testmerginapi.h index 25292fcbd..9a9640447 100644 --- a/app/test/testmerginapi.h +++ b/app/test/testmerginapi.h @@ -21,6 +21,73 @@ #include +class MockReply : public QNetworkReply +{ + public: + explicit MockReply( const QNetworkRequest &request, QNetworkAccessManager::Operation operation, + QObject *parent = nullptr, QNetworkReply::NetworkError errorCode = QNetworkReply::NoError ) + : QNetworkReply( parent ) + { + setRequest( request ); + setOperation( operation ); + setUrl( request.url() ); + + if ( errorCode != QNetworkReply::NoError ) + { + setError( errorCode, "Mock network failure" ); + QMetaObject::invokeMethod( this, "errorOccurred", Qt::QueuedConnection, Q_ARG( QNetworkReply::NetworkError, errorCode ) ); + } + + QMetaObject::invokeMethod( this, "finished", Qt::QueuedConnection ); + open( QIODevice::ReadOnly ); + } + + void abort() override {} + + qint64 readData( char *data, qint64 maxlen ) override + { + Q_UNUSED( data ); + Q_UNUSED( maxlen ); + return -1; + } + + qint64 bytesAvailable() const override + { + return 0; + } +}; + +class MockNetworkManager : public QNetworkAccessManager +{ + public: + explicit MockNetworkManager( QObject *parent = nullptr ) + : QNetworkAccessManager( parent ) + , mShouldFail( false ) + , mErrorCode( QNetworkReply::NoError ) + {} + + void setShouldFail( bool shouldFail, QNetworkReply::NetworkError errorCode = QNetworkReply::NoError ) + { + mShouldFail = shouldFail; + mErrorCode = errorCode; + } + + protected: + QNetworkReply *createRequest( Operation op, const QNetworkRequest &request, QIODevice *outgoingData = nullptr ) override + { + if ( mShouldFail ) + { + auto *reply = new MockReply( request, op, this, mErrorCode ); + return reply; + } + return QNetworkAccessManager::createRequest( op, request, outgoingData ); + } + + private: + bool mShouldFail; + QNetworkReply::NetworkError mErrorCode; +}; + class TestMerginApi: public QObject { Q_OBJECT @@ -40,6 +107,8 @@ class TestMerginApi: public QObject void testListProject(); void testListProjectsByName(); void testDownloadProject(); + void testDownloadWithNetworkError(); + void testDownloadWithNetworkErrorRecovery(); void testDownloadProjectSpecChars(); void testCancelDownloadProject(); void testCreateProjectTwice(); diff --git a/core/merginapi.cpp b/core/merginapi.cpp index ab5e4a618..5a6804903 100644 --- a/core/merginapi.cpp +++ b/core/merginapi.cpp @@ -49,6 +49,7 @@ MerginApi::MerginApi( LocalProjectsManager &localProjects, QObject *parent ) , mWorkspaceInfo( new MerginWorkspaceInfo ) , mSubscriptionInfo( new MerginSubscriptionInfo ) , mUserAuth( new MerginUserAuth ) + , mManager( new QNetworkAccessManager( this ) ) { // load cached data if there are any QSettings cache; @@ -199,7 +200,7 @@ QString MerginApi::listProjects( const QString &searchExpression, const QString QString requestId = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "list projects", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, [this, requestId]() {this->listProjectsReplyFinished( requestId );} ); @@ -248,7 +249,7 @@ QString MerginApi::listProjectsByName( const QStringList &projectNames ) QString requestId = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); - QNetworkReply *reply = mManager.post( request, body.toJson() ); + QNetworkReply *reply = mManager->post( request, body.toJson() ); CoreUtils::log( "list projects by name", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, [this, requestId]() {this->listProjectsByNameReplyFinished( requestId );} ); @@ -286,8 +287,9 @@ void MerginApi::downloadNextItem( const QString &projectFullName ) request.setRawHeader( "Range", range.toUtf8() ); } - QNetworkReply *reply = mManager.get( request ); - connect( reply, &QNetworkReply::finished, this, &MerginApi::downloadItemReplyFinished ); + QNetworkReply *reply = mManager->get( request ); + connect( reply, &QNetworkReply::finished, this, [this, item]() { downloadItemReplyFinished( item ); } ); + transaction.replyPullItems.insert( reply ); CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Requesting item: " ) + url.toString() + @@ -373,14 +375,12 @@ QString MerginApi::getApiKey( const QString &serverName ) return "not-secret-key"; } -void MerginApi::downloadItemReplyFinished() +void MerginApi::downloadItemReplyFinished( DownloadQueueItem item ) { QNetworkReply *r = qobject_cast( sender() ); Q_ASSERT( r ); - QString projectFullName = r->request().attribute( static_cast( AttrProjectFullName ) ).toString(); QString tempFileName = r->request().attribute( static_cast( AttrTempFileName ) ).toString(); - Q_ASSERT( mTransactionalStatus.contains( projectFullName ) ); TransactionStatus &transaction = mTransactionalStatus[projectFullName]; Q_ASSERT( transaction.replyPullItems.contains( r ) ); @@ -388,13 +388,10 @@ void MerginApi::downloadItemReplyFinished() if ( r->error() == QNetworkReply::NoError ) { QByteArray data = r->readAll(); - CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Downloaded item (%1 bytes)" ).arg( data.size() ) ); - QString tempFolder = getTempProjectDir( projectFullName ); QString tempFilePath = tempFolder + "/" + tempFileName; createPathIfNotExists( tempFilePath ); - // save to a tmp file, assemble at the end QFile file( tempFilePath ); if ( file.open( QIODevice::WriteOnly ) ) @@ -406,11 +403,10 @@ void MerginApi::downloadItemReplyFinished() { CoreUtils::log( "pull " + projectFullName, "Failed to open for writing: " + file.fileName() ); } - transaction.transferedSize += data.size(); emit syncProjectStatusChanged( projectFullName, transaction.transferedSize / transaction.totalSize ); - transaction.replyPullItems.remove( r ); + r->deleteLater(); if ( !transaction.downloadQueue.isEmpty() ) @@ -418,6 +414,7 @@ void MerginApi::downloadItemReplyFinished() // one request finished, let's start another one downloadNextItem( projectFullName ); } + else if ( transaction.replyPullItems.isEmpty() ) { // nothing else to download and all requests are finished, we're done @@ -428,6 +425,20 @@ void MerginApi::downloadItemReplyFinished() // no more requests to start, but there are pending requests - let's do nothing and wait } } + else if ( transaction.retryCount < transaction.MAX_RETRY_COUNT && isRetryableNetworkError( r ) ) + { + transaction.retryCount++; + transaction.downloadQueue.append( item ); + + CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Retrying download (attempt %1 of %2)" ).arg( transaction.retryCount ) + .arg( transaction.MAX_RETRY_COUNT ) ); + + downloadNextItem( projectFullName ); + + emit downloadItemRetried( projectFullName, transaction.retryCount ); + transaction.replyPullItems.remove( r ); + r->deleteLater(); + } else { QString serverMsg = extractServerErrorMsg( r->readAll() ); @@ -439,15 +450,12 @@ void MerginApi::downloadItemReplyFinished() serverMsg = r->errorString(); } CoreUtils::log( "pull " + projectFullName, QStringLiteral( "FAILED - %1. %2" ).arg( r->errorString(), serverMsg ) ); - transaction.replyPullItems.remove( r ); r->deleteLater(); - if ( !transaction.pullItemsAborting ) { // the first failed request will abort all the other pending requests too, and finish pull with error abortPullItems( projectFullName ); - // signal a networking error - we may retry int httpCode = r->attribute( QNetworkRequest::HttpStatusCodeAttribute ).toInt(); emit networkErrorOccurred( serverMsg, QStringLiteral( "Mergin API error: downloadFile" ), httpCode, projectFullName ); @@ -567,7 +575,7 @@ void MerginApi::pushFile( const QString &projectFullName, const QString &transac request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPushFile ); - transaction.replyPushFile = mManager.post( request, data ); + transaction.replyPushFile = mManager->post( request, data ); connect( transaction.replyPushFile, &QNetworkReply::finished, this, &MerginApi::pushFileReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Uploading item: " ) + url.toString() ); @@ -590,7 +598,7 @@ void MerginApi::pushStart( const QString &projectFullName, const QByteArray &jso request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPushStart ); - transaction.replyPushStart = mManager.post( request, json ); + transaction.replyPushStart = mManager->post( request, json ); connect( transaction.replyPushStart, &QNetworkReply::finished, this, &MerginApi::pushStartReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Starting push request: " ) + url.toString() ); @@ -653,7 +661,7 @@ void MerginApi::sendPushCancelRequest( const QString &projectFullName, const QSt request.setRawHeader( "Content-Type", "application/json" ); request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); - QNetworkReply *reply = mManager.post( request, QByteArray() ); + QNetworkReply *reply = mManager->post( request, QByteArray() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::pushCancelReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Requesting upload transaction cancel: " ) + url.toString() ); } @@ -707,7 +715,7 @@ void MerginApi::pushFinish( const QString &projectFullName, const QString &trans request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPushFinish ); - transaction.replyPushFinish = mManager.post( request, QByteArray() ); + transaction.replyPushFinish = mManager->post( request, QByteArray() ); connect( transaction.replyPushFinish, &QNetworkReply::finished, this, &MerginApi::pushFinishReplyFinished ); CoreUtils::log( "push " + projectFullName, QStringLiteral( "Requesting transaction finish: " ) + transactionUUID ); @@ -803,7 +811,7 @@ void MerginApi::authorize( const QString &login, const QString &password ) jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, &MerginApi::authorizeFinished ); CoreUtils::log( "auth", QStringLiteral( "Requesting authorization: " ) + url.toString() ); } @@ -878,7 +886,7 @@ void MerginApi::registerUser( const QString &username, jsonObject.insert( QStringLiteral( "api_key" ), getApiKey( mApiRoot ) ); jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, [ = ]() { this->registrationFinished( username, password ); } ); CoreUtils::log( "auth", QStringLiteral( "Requesting registration: " ) + url.toString() ); } @@ -914,7 +922,7 @@ void MerginApi::postRegisterUser( const QString &marketingChannel, const QString jsonObject.insert( QStringLiteral( "subscribe" ), wantsNewsletter ); jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, [ = ]() { this->postRegistrationFinished(); } ); CoreUtils::log( "auth", QStringLiteral( "Requesting post-registration: " ) + url.toString() ); } @@ -940,7 +948,7 @@ void MerginApi::getUserInfo() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "user info", QStringLiteral( "Requesting user info: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getUserInfoFinished ); } @@ -967,7 +975,7 @@ void MerginApi::getWorkspaceInfo() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "workspace info", QStringLiteral( "Requesting workspace info: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getWorkspaceInfoReplyFinished ); } @@ -998,7 +1006,7 @@ void MerginApi::getServiceInfo() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getServiceInfoReplyFinished ); @@ -1103,7 +1111,7 @@ bool MerginApi::createProject( const QString &projectNamespace, const QString &p jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, &MerginApi::createProjectFinished ); CoreUtils::log( "create " + projectFullName, QStringLiteral( "Requesting project creation: " ) + url.toString() ); @@ -1123,7 +1131,7 @@ void MerginApi::deleteProject( const QString &projectNamespace, const QString &p QUrl url( mApiRoot + QStringLiteral( "/v1/project/%1" ).arg( projectFullName ) ); request.setUrl( url ); request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); - QNetworkReply *reply = mManager.deleteResource( request ); + QNetworkReply *reply = mManager->deleteResource( request ); connect( reply, &QNetworkReply::finished, this, [this, informUser]() { this->deleteProjectFinished( informUser );} ); CoreUtils::log( "delete " + projectFullName, QStringLiteral( "Requesting project deletion: " ) + url.toString() ); } @@ -1434,7 +1442,7 @@ QNetworkReply *MerginApi::getProjectInfo( const QString &projectFullName, bool w request.setUrl( url ); request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); - return mManager.get( request ); + return mManager->get( request ); } bool MerginApi::validateAuth() @@ -1653,7 +1661,7 @@ void MerginApi::pingMergin() QUrl url( mApiRoot + QStringLiteral( "/ping" ) ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "ping", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::pingMerginReplyFinished ); } @@ -2558,7 +2566,7 @@ void MerginApi::requestServerConfig( const QString &projectFullName ) request.setAttribute( static_cast( AttrProjectFullName ), projectFullName ); Q_ASSERT( !transaction.replyPullServerConfig ); - transaction.replyPullServerConfig = mManager.get( request ); + transaction.replyPullServerConfig = mManager->get( request ); connect( transaction.replyPullServerConfig, &QNetworkReply::finished, this, &MerginApi::cacheServerConfig ); CoreUtils::log( "pull " + projectFullName, QStringLiteral( "Requesting mergin config: " ) + url.toString() ); @@ -3519,7 +3527,7 @@ void MerginApi::deleteAccount() QNetworkRequest request = getDefaultRequest(); QUrl url( mApiRoot + QStringLiteral( "/v1/user" ) ); request.setUrl( url ); - QNetworkReply *reply = mManager.deleteResource( request ); + QNetworkReply *reply = mManager->deleteResource( request ); connect( reply, &QNetworkReply::finished, this, [this]() { this->deleteAccountFinished();} ); CoreUtils::log( "delete account " + mUserAuth->username(), QStringLiteral( "Requesting account deletion: " ) + url.toString() ); } @@ -3571,7 +3579,7 @@ void MerginApi::getServerConfig() QUrl url( urlString ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); connect( reply, &QNetworkReply::finished, this, &MerginApi::getServerConfigReplyFinished ); CoreUtils::log( "Config", QStringLiteral( "Requesting server configuration: " ) + url.toString() ); @@ -3687,7 +3695,7 @@ void MerginApi::listWorkspaces() QNetworkRequest request = getDefaultRequest( mUserAuth->hasAuthData() ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "list workspaces", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::listWorkspacesReplyFinished ); } @@ -3743,7 +3751,7 @@ void MerginApi::listInvitations() QNetworkRequest request = getDefaultRequest( mUserAuth->hasAuthData() ); request.setUrl( url ); - QNetworkReply *reply = mManager.get( request ); + QNetworkReply *reply = mManager->get( request ); CoreUtils::log( "list invitations", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::listInvitationsReplyFinished ); } @@ -3806,7 +3814,7 @@ void MerginApi::processInvitation( const QString &uuid, bool accept ) jsonObject.insert( QStringLiteral( "accept" ), accept ); jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); CoreUtils::log( "process invitation", QStringLiteral( "Requesting: " ) + url.toString() ); connect( reply, &QNetworkReply::finished, this, &MerginApi::processInvitationReplyFinished ); } @@ -3868,7 +3876,7 @@ bool MerginApi::createWorkspace( const QString &workspaceName ) jsonDoc.setObject( jsonObject ); QByteArray json = jsonDoc.toJson( QJsonDocument::Compact ); - QNetworkReply *reply = mManager.post( request, json ); + QNetworkReply *reply = mManager->post( request, json ); connect( reply, &QNetworkReply::finished, this, &MerginApi::createWorkspaceReplyFinished ); CoreUtils::log( "create " + workspaceName, QStringLiteral( "Requesting workspace creation: " ) + url.toString() ); @@ -3945,3 +3953,34 @@ DownloadQueueItem::DownloadQueueItem( const QString &fp, qint64 s, int v, qint64 tempFileName = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); } +bool MerginApi::isRetryableNetworkError( QNetworkReply *reply ) +{ + Q_ASSERT( reply ); + + QNetworkReply::NetworkError err = reply->error(); + + bool isRetryableError = ( err == QNetworkReply::TimeoutError || + err == QNetworkReply::TemporaryNetworkFailureError || + err == QNetworkReply::NetworkSessionFailedError || + err == QNetworkReply::UnknownNetworkError || + err == QNetworkReply::RemoteHostClosedError || + err == QNetworkReply::ProxyConnectionClosedError || + err == QNetworkReply::ProxyTimeoutError || + err == QNetworkReply::UnknownProxyError || + err == QNetworkReply::ServiceUnavailableError ); + + return isRetryableError; +} + +void MerginApi::setNetworkManager( QNetworkAccessManager *manager ) +{ + if ( !manager ) + return; + + if ( mManager == manager ) + return; + + mManager = manager; + + emit networkManagerChanged(); +} diff --git a/core/merginapi.h b/core/merginapi.h index 0e5380bb2..4646e9361 100644 --- a/core/merginapi.h +++ b/core/merginapi.h @@ -166,6 +166,10 @@ struct TransactionStatus QList pushQueue; //!< pending list of files to push (at the end of transaction it is empty) QList pushDiffFiles; //!< these are just diff files for push - we don't remove them when pushing chunks (needed for finalization) + // retry handling + int retryCount = 0; //!< current number of retry attempts for failed network requests + static const int MAX_RETRY_COUNT = 5; //!< maximum number of retry attempts for failed network requests + QString projectDir; QByteArray projectMetadata; //!< metadata of the new project (not parsed) bool firstTimeDownload = false; //!< only for update. whether this is first time to download the project (on failure we would also remove the project folder) @@ -573,6 +577,17 @@ class MerginApi: public QObject */ bool apiSupportsWorkspaces(); + /** + * Returns the network manager used for Mergin API requests + */ + QNetworkAccessManager *networkManager() const { return mManager; } + + /** + * Sets the network manager to be used for Mergin API requests + * Function will return early if manager is null. + */ + void setNetworkManager( QNetworkAccessManager *manager ); + signals: void apiSupportsSubscriptionsChanged(); void supportsSelectiveSyncChanged(); @@ -650,6 +665,9 @@ class MerginApi: public QObject void apiSupportsWorkspacesChanged(); void serverWasUpgraded(); + void networkManagerChanged(); + + void downloadItemRetried( const QString &projectFullName, int retryCount ); private slots: void listProjectsReplyFinished( QString requestId ); @@ -657,7 +675,7 @@ class MerginApi: public QObject // Pull slots void pullInfoReplyFinished(); - void downloadItemReplyFinished(); + void downloadItemReplyFinished( DownloadQueueItem item ); void cacheServerConfig(); // Push slots @@ -785,11 +803,18 @@ class MerginApi: public QObject //! Works only when login, password and token is set in UserAuth void refreshAuthToken(); + /** + * Checks if a network error should trigger a retry attempt. + * \param reply Network reply to check for retryable errors + * \returns True if the error should trigger a retry, false otherwise + */ + bool isRetryableNetworkError( QNetworkReply *reply ); + QNetworkRequest getDefaultRequest( bool withAuth = true ); bool projectFileHasBeenUpdated( const ProjectDiff &diff ); - QNetworkAccessManager mManager; + QNetworkAccessManager *mManager = nullptr; QString mApiRoot; LocalProjectsManager &mLocalProjects; QString mDataDir; // dir with all projects From 6115084334ada95e29792b97d317241674d646d9 Mon Sep 17 00:00:00 2001 From: Vitor Vieira <155513369+VitorVieiraZ@users.noreply.github.com> Date: Thu, 23 Jan 2025 13:23:27 -0300 Subject: [PATCH 2/2] Improving UX by hiding editing buttons for readers of a project (#3682) --- app/activeproject.cpp | 18 +++++ app/activeproject.h | 12 ++- app/main.cpp | 22 +++++ app/qml/form/MMFormPage.qml | 4 +- app/qml/form/MMPreviewDrawer.qml | 2 +- .../components/MMFeaturesListPageDrawer.qml | 1 + app/qml/layers/MMFeaturesListPage.qml | 2 +- app/qml/main.qml | 1 + app/test/testcoreutils.cpp | 80 +++++++++++++++++++ app/test/testcoreutils.h | 1 + app/test/testmerginapi.cpp | 37 ++++++++- app/test/testmerginapi.h | 1 + core/coreutils.cpp | 42 ++++++++++ core/coreutils.h | 10 +++ core/merginapi.cpp | 69 ++++++++++++++++ core/merginapi.h | 21 ++++- core/merginprojectmetadata.cpp | 1 + core/merginprojectmetadata.h | 1 + 18 files changed, 318 insertions(+), 7 deletions(-) diff --git a/app/activeproject.cpp b/app/activeproject.cpp index acfb53093..2614e9f6a 100644 --- a/app/activeproject.cpp +++ b/app/activeproject.cpp @@ -184,6 +184,9 @@ bool ActiveProject::forceLoad( const QString &filePath, bool force ) CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Could not find project in local projects: " ) + filePath ); } + QString role = MerginProjectMetadata::fromCachedJson( CoreUtils::getProjectMetadataPath( mLocalProject.projectDir ) ).role; + setProjectRole( role ); + updateMapTheme(); updateRecordingLayers(); updateActiveLayer(); @@ -553,3 +556,18 @@ bool ActiveProject::positionTrackingSupported() const return mQgsProject->readBoolEntry( QStringLiteral( "Mergin" ), QStringLiteral( "PositionTracking/Enabled" ), false ); } + +QString ActiveProject::projectRole() const +{ + return mProjectRole; +} + +void ActiveProject::setProjectRole( const QString &role ) +{ + if ( mProjectRole != role ) + { + mProjectRole = role; + + emit projectRoleChanged(); + } +} diff --git a/app/activeproject.h b/app/activeproject.h index b775303f6..c9a828fee 100644 --- a/app/activeproject.h +++ b/app/activeproject.h @@ -22,6 +22,7 @@ #include "localprojectsmanager.h" #include "autosynccontroller.h" #include "inputmapsettings.h" +#include "merginprojectmetadata.h" /** * \brief The ActiveProject class can load a QGIS project and holds its data. @@ -33,6 +34,7 @@ class ActiveProject: public QObject Q_PROPERTY( QgsProject *qgsProject READ qgsProject NOTIFY qgsProjectChanged ) // QgsProject instance of active project, never changes Q_PROPERTY( AutosyncController *autosyncController READ autosyncController NOTIFY autosyncControllerChanged ) Q_PROPERTY( InputMapSettings *mapSettings READ mapSettings WRITE setMapSettings NOTIFY mapSettingsChanged ) + Q_PROPERTY( QString projectRole READ projectRole WRITE setProjectRole NOTIFY projectRoleChanged ) Q_PROPERTY( QString mapTheme READ mapTheme WRITE setMapTheme NOTIFY mapThemeChanged ) Q_PROPERTY( bool positionTrackingSupported READ positionTrackingSupported NOTIFY positionTrackingSupportedChanged ) @@ -118,6 +120,12 @@ class ActiveProject: public QObject bool positionTrackingSupported() const; + /** + * Returns role/permission level of current user for this project + */ + Q_INVOKABLE QString projectRole() const; + void setProjectRole( const QString &role ); + signals: void qgsProjectChanged(); void localProjectChanged( LocalProject project ); @@ -145,6 +153,8 @@ class ActiveProject: public QObject // Emited when the app (UI) should show tracking because there is a running tracking service void startPositionTracking(); + void projectRoleChanged(); + public slots: // Reloads project if current project path matches given path (its the same project) bool reloadProject( QString projectDir ); @@ -182,10 +192,10 @@ class ActiveProject: public QObject LayersProxyModel &mRecordingLayerPM; LocalProjectsManager &mLocalProjectsManager; InputMapSettings *mMapSettings = nullptr; - std::unique_ptr mAutosyncController; QString mProjectLoadingLog; + QString mProjectRole; /** * Reloads project. diff --git a/app/main.cpp b/app/main.cpp index 2ff044e83..6f741af8f 100644 --- a/app/main.cpp +++ b/app/main.cpp @@ -495,6 +495,7 @@ int main( int argc, char *argv[] ) ActiveLayer al; ActiveProject activeProject( as, al, recordingLpm, localProjectsManager ); + std::unique_ptr vm( new VariablesManager( ma.get() ) ); vm->registerInputExpressionFunctions(); @@ -569,6 +570,27 @@ int main( int argc, char *argv[] ) syncManager.syncProject( project, SyncOptions::Authorized, SyncOptions::Retry ); } ); + QObject::connect( &activeProject, &ActiveProject::projectReloaded, &lambdaContext, [merginApi = ma.get(), &activeProject]() + { + merginApi->reloadProjectRole( activeProject.projectFullName() ); + } ); + + QObject::connect( ma.get(), &MerginApi::authChanged, &lambdaContext, [merginApi = ma.get(), &activeProject]() + { + if ( activeProject.isProjectLoaded() ) + { + merginApi->reloadProjectRole( activeProject.projectFullName() ); + } + } ); + + QObject::connect( ma.get(), &MerginApi::projectRoleUpdated, &activeProject, [&activeProject]( const QString & projectFullName, const QString & role ) + { + if ( projectFullName == activeProject.projectFullName() ) + { + activeProject.setProjectRole( role ); + } + } ); + QObject::connect( ma.get(), &MerginApi::notifyInfo, &lambdaContext, [¬ificationModel]( const QString & message ) { notificationModel.addInfo( message ); diff --git a/app/qml/form/MMFormPage.qml b/app/qml/form/MMFormPage.qml index f12a9647f..1168a06d0 100644 --- a/app/qml/form/MMFormPage.qml +++ b/app/qml/form/MMFormPage.qml @@ -201,7 +201,7 @@ Page { footer: MMComponents.MMToolbar { - visible: !root.layerIsReadOnly + visible: !root.layerIsReadOnly && __activeProject.projectRole !== "reader" ObjectModel { id: readStateButtons @@ -231,7 +231,7 @@ Page { id: editGeometry text: qsTr( "Edit geometry" ) iconSource: __style.editIcon - visible: root.layerIsSpatial + visible: root.layerIsSpatial && __activeProject.projectRole !== "reader" onClicked: root.editGeometryRequested( root.controller.featureLayerPair ) } } diff --git a/app/qml/form/MMPreviewDrawer.qml b/app/qml/form/MMPreviewDrawer.qml index ca6753fc0..328d55ebb 100644 --- a/app/qml/form/MMPreviewDrawer.qml +++ b/app/qml/form/MMPreviewDrawer.qml @@ -295,7 +295,7 @@ Item { property bool isHTMLType: root.controller.type === MM.AttributePreviewController.HTML property bool isEmptyType: root.controller.type === MM.AttributePreviewController.Empty - property bool showEditButton: !root.layerIsReadOnly + property bool showEditButton: !root.layerIsReadOnly && __activeProject.projectRole !== "reader" property bool showStakeoutButton: __inputUtils.isPointLayerFeature( controller.featureLayerPair ) property bool showButtons: showEditButton || showStakeoutButton diff --git a/app/qml/form/components/MMFeaturesListPageDrawer.qml b/app/qml/form/components/MMFeaturesListPageDrawer.qml index c6f3a266a..ce9c22933 100644 --- a/app/qml/form/components/MMFeaturesListPageDrawer.qml +++ b/app/qml/form/components/MMFeaturesListPageDrawer.qml @@ -103,6 +103,7 @@ Drawer { } text: qsTr( "Add feature" ) + visible: __activeProject.projectRole !== "reader" onClicked: root.buttonClicked() } diff --git a/app/qml/layers/MMFeaturesListPage.qml b/app/qml/layers/MMFeaturesListPage.qml index 04b8bb256..5bf9c3f9f 100644 --- a/app/qml/layers/MMFeaturesListPage.qml +++ b/app/qml/layers/MMFeaturesListPage.qml @@ -89,7 +89,7 @@ MMComponents.MMPage { anchors.bottom: parent.bottom anchors.bottomMargin: root.hasToolbar ? __style.margin20 : ( __style.safeAreaBottom + __style.margin8 ) - visible: __inputUtils.isNoGeometryLayer( root.selectedLayer ) && !root.layerIsReadOnly + visible: __inputUtils.isNoGeometryLayer( root.selectedLayer ) && !root.layerIsReadOnly && __activeProject.projectRole !== "reader" text: qsTr("Add feature") diff --git a/app/qml/main.qml b/app/qml/main.qml index d6b7ea116..4cb127ddb 100644 --- a/app/qml/main.qml +++ b/app/qml/main.qml @@ -276,6 +276,7 @@ ApplicationWindow { MMToolbarButton { text: qsTr("Add") iconSource: __style.addIcon + visible: __activeProject.projectRole !== "reader" onClicked: { if ( __recordingLayersModel.rowCount() > 0 ) { stateManager.state = "map" diff --git a/app/test/testcoreutils.cpp b/app/test/testcoreutils.cpp index 4eb099e1b..23d6d78e7 100644 --- a/app/test/testcoreutils.cpp +++ b/app/test/testcoreutils.cpp @@ -272,3 +272,83 @@ void TestCoreUtils::testNameAbbr() QCOMPARE( CoreUtils::nameAbbr( name, email ), test.second ); } } + +void TestCoreUtils::testReplaceValueInJson() +{ + // temporary test file + QString testFilePath = QDir::tempPath() + "/test_replace_value.json"; + + // basic replacement in valid JSON with int value + { + QFile file( testFilePath ); + QVERIFY( file.open( QIODevice::WriteOnly ) ); + file.write( R"({"name": "test", "value": 123})" ); + file.close(); + + QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "value", 456 ) ); + + // verify + QVERIFY( file.open( QIODevice::ReadOnly ) ); + QJsonDocument doc = QJsonDocument::fromJson( file.readAll() ); + file.close(); + QVERIFY( doc.isObject() ); + QJsonObject obj = doc.object(); + QCOMPARE( obj["value"].toInt(), 456 ); + QCOMPARE( obj["name"].toString(), QString( "test" ) ); + } + // valid JSON with string value + { + QFile file( testFilePath ); + QVERIFY( file.open( QIODevice::WriteOnly ) ); + file.write( R"({"name": "test", "status": "active"})" ); + file.close(); + + QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "status", "inactive" ) ); + + // verify replacement + QVERIFY( file.open( QIODevice::ReadOnly ) ); + QJsonDocument doc = QJsonDocument::fromJson( file.readAll() ); + file.close(); + QVERIFY( doc.isObject() ); + QJsonObject obj = doc.object(); + QCOMPARE( obj["status"].toString(), QString( "inactive" ) ); + QCOMPARE( obj["name"].toString(), QString( "test" ) ); + } + + // add new key-value pair + { + QFile file( testFilePath ); + QVERIFY( file.open( QIODevice::WriteOnly ) ); + file.write( R"({"name": "test"})" ); + file.close(); + + QVERIFY( CoreUtils::replaceValueInJson( testFilePath, "newKey", "newValue" ) ); + + // verify the addition + QVERIFY( file.open( QIODevice::ReadOnly ) ); + QJsonDocument doc = QJsonDocument::fromJson( file.readAll() ); + file.close(); + QVERIFY( doc.isObject() ); + QJsonObject obj = doc.object(); + QCOMPARE( obj["newKey"].toString(), QString( "newValue" ) ); + QCOMPARE( obj["name"].toString(), QString( "test" ) ); + } + + // invalid JSON file + { + QFile file( testFilePath ); + QVERIFY( file.open( QIODevice::WriteOnly ) ); + file.write( "invalid json content" ); + file.close(); + + QVERIFY( !CoreUtils::replaceValueInJson( testFilePath, "key", "value" ) ); + } + + // non-existent file + { + QString nonExistentPath = QDir::tempPath() + "/non_existent.json"; + QVERIFY( !CoreUtils::replaceValueInJson( nonExistentPath, "key", "value" ) ); + } + + QFile::remove( testFilePath ); +} diff --git a/app/test/testcoreutils.h b/app/test/testcoreutils.h index 5f78b067c..8f4541b78 100644 --- a/app/test/testcoreutils.h +++ b/app/test/testcoreutils.h @@ -27,6 +27,7 @@ class TestCoreUtils : public QObject void testHasProjectFileExtension(); void testNameValidation(); void testNameAbbr(); + void testReplaceValueInJson(); }; #endif // TESTCOREUTILS_H diff --git a/app/test/testmerginapi.cpp b/app/test/testmerginapi.cpp index 8e7f648c3..a7ee5a4be 100644 --- a/app/test/testmerginapi.cpp +++ b/app/test/testmerginapi.cpp @@ -2943,6 +2943,42 @@ void TestMerginApi::testParseVersion() QCOMPARE( minor, 4 ); } +void TestMerginApi::testUpdateProjectMetadataRole() +{ + QString projectName = "testUpdateProjectMetadataRole"; + + createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" ); + downloadRemoteProject( mApi, mWorkspaceName, projectName ); + + LocalProject projectInfo = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName ); + QVERIFY( projectInfo.isValid() ); + + QString fullProjectName = MerginApi::getFullProjectName( mWorkspaceName, projectName ); + + // Test 1: Initial role should be 'owner' + QString cachedRole = mApi->getCachedProjectRole( fullProjectName ); + QCOMPARE( cachedRole, QString( "owner" ) ); + + // Test 2: Update cached role to 'reader' + QString newRole = "reader"; + bool updateSuccess = mApi->updateCachedProjectRole( fullProjectName, newRole ); + QVERIFY( updateSuccess ); + + // Verify role was updated in cache + cachedRole = mApi->getCachedProjectRole( fullProjectName ); + QCOMPARE( cachedRole, QString( "reader" ) ); + + // Role in server wasn't updated and stills "owner" => let's reload it from server and see if it updates in cached + QSignalSpy spy( mApi, &MerginApi::projectRoleUpdated ); + mApi->reloadProjectRole( fullProjectName ); + QVERIFY( spy.wait() ); + cachedRole = mApi->getCachedProjectRole( fullProjectName ); + QCOMPARE( cachedRole, QString( "owner" ) ); + + // Clean up + deleteRemoteProjectNow( mApi, mWorkspaceName, projectName ); +} + void TestMerginApi::testDownloadWithNetworkError() { // Store original manager @@ -3081,4 +3117,3 @@ void TestMerginApi::testDownloadWithNetworkErrorRecovery() mApi->setNetworkManager( originalManager ); delete failingManager; } - diff --git a/app/test/testmerginapi.h b/app/test/testmerginapi.h index 9a9640447..bb8b56591 100644 --- a/app/test/testmerginapi.h +++ b/app/test/testmerginapi.h @@ -148,6 +148,7 @@ class TestMerginApi: public QObject void testSynchronizationViaManager(); void testAutosync(); void testAutosyncFailure(); + void testUpdateProjectMetadataRole(); void testRegisterAndDelete(); void testCreateWorkspace(); diff --git a/core/coreutils.cpp b/core/coreutils.cpp index 834e24c12..5d6976768 100644 --- a/core/coreutils.cpp +++ b/core/coreutils.cpp @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include "qcoreapplication.h" @@ -335,3 +337,43 @@ QString CoreUtils::bytesToHumanSize( double bytes ) return QString::number( bytes / 1024.0 / 1024.0 / 1024.0 / 1024.0, 'f', precision ) + " TB"; } } + +QString CoreUtils::getProjectMetadataPath( QString projectDir ) +{ + if ( projectDir.isEmpty() ) + return QString(); + + return projectDir + "/.mergin/mergin.json"; +} + +bool CoreUtils::replaceValueInJson( const QString &filePath, const QString &key, const QJsonValue &value ) +{ + QFile file( filePath ); + if ( !file.open( QIODevice::ReadOnly ) ) + { + return false; + } + + QByteArray data = file.readAll(); + file.close(); + + QJsonDocument doc = QJsonDocument::fromJson( data ); + if ( !doc.isObject() ) + { + return false; + } + + QJsonObject obj = doc.object(); + obj[key] = value; + doc.setObject( obj ); + + if ( !file.open( QIODevice::WriteOnly ) ) + { + return false; + } + + bool success = ( file.write( doc.toJson() ) != -1 ); + file.close(); + + return success; +} diff --git a/core/coreutils.h b/core/coreutils.h index f39754737..bcacaa91a 100644 --- a/core/coreutils.h +++ b/core/coreutils.h @@ -125,6 +125,16 @@ class CoreUtils */ static QString bytesToHumanSize( double bytes ); + /** + * Returns path to project metadata file for a given project directory + */ + static QString getProjectMetadataPath( QString projectDir ); + + /** + * Updates a value in a JSON file at the specified top-level key + */ + static bool replaceValueInJson( const QString &filePath, const QString &key, const QJsonValue &value ); + private: static QString sLogFile; static int CHECKSUM_CHUNK_SIZE; diff --git a/core/merginapi.cpp b/core/merginapi.cpp index 5a6804903..efb1bd1dd 100644 --- a/core/merginapi.cpp +++ b/core/merginapi.cpp @@ -3457,6 +3457,17 @@ bool MerginApi::writeData( const QByteArray &data, const QString &path ) return true; } +bool MerginApi::updateCachedProjectRole( const QString &projectFullName, const QString &newRole ) +{ + LocalProject project = mLocalProjects.projectFromMerginName( projectFullName ); + if ( !project.isValid() ) + { + return false; + } + + QString metadataPath = project.projectDir + "/" + sMetadataFile; + return CoreUtils::replaceValueInJson( metadataPath, "role", newRole ); +} void MerginApi::createPathIfNotExists( const QString &filePath ) { @@ -3953,6 +3964,64 @@ DownloadQueueItem::DownloadQueueItem( const QString &fp, qint64 s, int v, qint64 tempFileName = CoreUtils::uuidWithoutBraces( QUuid::createUuid() ); } +void MerginApi::reloadProjectRole( const QString &projectFullName ) +{ + if ( projectFullName.isEmpty() ) + { + return; + } + + QNetworkReply *reply = getProjectInfo( projectFullName ); + if ( !reply ) + return; + + reply->request().setAttribute( static_cast( AttrProjectFullName ), projectFullName ); + connect( reply, &QNetworkReply::finished, this, &MerginApi::reloadProjectRoleReplyFinished ); +} + +void MerginApi::reloadProjectRoleReplyFinished() +{ + QNetworkReply *r = qobject_cast( sender() ); + Q_ASSERT( r ); + + QString projectFullName = r->request().attribute( static_cast( AttrProjectFullName ) ).toString(); + QString cachedRole = MerginApi::getCachedProjectRole( projectFullName ); + + if ( r->error() == QNetworkReply::NoError ) + { + QByteArray data = r->readAll(); + MerginProjectMetadata serverProject = MerginProjectMetadata::fromJson( data ); + QString role = serverProject.role; + + if ( role != cachedRole ) + { + if ( updateCachedProjectRole( projectFullName, role ) ) + emit projectRoleUpdated( projectFullName, role ); + } + } + else + { + CoreUtils::log( "metadata", QString( "Failed to update cached role for project %1" ).arg( projectFullName ) ); + } + + r->deleteLater(); +} + +QString MerginApi::getCachedProjectRole( const QString &projectFullName ) const +{ + if ( projectFullName.isEmpty() ) + return QString(); + + QString projectDir = mLocalProjects.projectFromMerginName( projectFullName ).projectDir; + + if ( projectDir.isEmpty() ) + return QString(); + + MerginProjectMetadata cachedProjectMetadata = MerginProjectMetadata::fromCachedJson( projectDir + "/" + sMetadataFile ); + + return cachedProjectMetadata.role; +} + bool MerginApi::isRetryableNetworkError( QNetworkReply *reply ) { Q_ASSERT( reply ); diff --git a/core/merginapi.h b/core/merginapi.h index 4646e9361..f36874b4a 100644 --- a/core/merginapi.h +++ b/core/merginapi.h @@ -577,6 +577,11 @@ class MerginApi: public QObject */ bool apiSupportsWorkspaces(); + /** + * Reloads project metadata role by fetching latest information from server. + */ + Q_INVOKABLE void reloadProjectRole( const QString &projectFullName ); + /** * Returns the network manager used for Mergin API requests */ @@ -665,6 +670,9 @@ class MerginApi: public QObject void apiSupportsWorkspacesChanged(); void serverWasUpgraded(); + + void projectRoleUpdated( const QString &projectFullName, const QString &role ); + void networkManagerChanged(); void downloadItemRetried( const QString &projectFullName, int retryCount ); @@ -814,7 +822,18 @@ class MerginApi: public QObject bool projectFileHasBeenUpdated( const ProjectDiff &diff ); + //! Checks if retrieving the project role from the server was successful and + //! if it differs from the current project role, emits a signal with new project role + void reloadProjectRoleReplyFinished(); + + //! Updates project role in metadata file + bool updateCachedProjectRole( const QString &projectFullName, const QString &newRole ); + + //! Retrieves cached role from metadata file + QString getCachedProjectRole( const QString &projectFullName ) const; + QNetworkAccessManager *mManager = nullptr; + QString mApiRoot; LocalProjectsManager &mLocalProjects; QString mDataDir; // dir with all projects @@ -829,7 +848,7 @@ class MerginApi: public QObject AttrProjectFullName = QNetworkRequest::User, AttrTempFileName = QNetworkRequest::User + 1, AttrWorkspaceName = QNetworkRequest::User + 2, - AttrAcceptFlag = QNetworkRequest::User + 3, + AttrAcceptFlag = QNetworkRequest::User + 3 }; Transactions mTransactionalStatus; //projectFullname -> transactionStatus diff --git a/core/merginprojectmetadata.cpp b/core/merginprojectmetadata.cpp index ba1ff95c3..7b4f2610e 100644 --- a/core/merginprojectmetadata.cpp +++ b/core/merginprojectmetadata.cpp @@ -99,6 +99,7 @@ MerginProjectMetadata MerginProjectMetadata::fromJson( const QByteArray &data ) project.name = docObj.value( QStringLiteral( "name" ) ).toString(); project.projectNamespace = docObj.value( QStringLiteral( "namespace" ) ).toString(); + project.role = docObj.value( QStringLiteral( "role" ) ).toString(); QString versionStr = docObj.value( QStringLiteral( "version" ) ).toString(); if ( versionStr.isEmpty() ) diff --git a/core/merginprojectmetadata.h b/core/merginprojectmetadata.h index b4bd178c8..00705add7 100644 --- a/core/merginprojectmetadata.h +++ b/core/merginprojectmetadata.h @@ -59,6 +59,7 @@ struct MerginProjectMetadata { QString name; QString projectNamespace; + QString role; int version = -1; QList files; QString projectId; //!< unique project ID (only available in API that supports project IDs)