post review additions

app/test/testmerginapi.cpp

@@ -2945,60 +2945,49 @@ void TestMerginApi::testParseVersion()
 
 void TestMerginApi::testDownloadWithNetworkError()
 {
-  QString projectName = "testDownloadRetry";
+  // Store original manager
+  QNetworkAccessManager *originalManager = mApi->networkManager();
 
-  // create the project on the server (the content is not important)
+  QString projectName = "testDownloadRetry";
   createRemoteProject( mApiExtra, mWorkspaceName, projectName, mTestDataPath + "/" + TEST_PROJECT_NAME + "/" );
 
   // Create mock manager - initially not failing
-  MockNetworkManager *mockManager = new MockNetworkManager( this );
-  mApi->setNetworkManager( mockManager );
+  MockNetworkManager *failingManager = new MockNetworkManager( this );
+  mApi->setNetworkManager( failingManager );
 
-  // Create signal spies
   QSignalSpy startSpy( mApi, &MerginApi::downloadItemsStarted );
   QSignalSpy retrySpy( mApi, &MerginApi::downloadItemRetried );
   QSignalSpy finishSpy( mApi, &MerginApi::syncProjectFinished );
 
-  // Connect to downloadItemsStarted to trigger failure when items download start
-  connect( mApi, &MerginApi::downloadItemsStarted, this, [this]()
+  // First attempt - TimeoutError
+  connect( mApi, &MerginApi::downloadItemsStarted, this, [this, failingManager]()
   {
-    MockNetworkManager *failingManager = new MockNetworkManager( this );
-    failingManager->setShouldFail( true );
-    mApi->setNetworkManager( failingManager );
+    failingManager->setShouldFail( true, QNetworkReply::TimeoutError );
   } );
 
-  // Try to download the project
   mApi->pullProject( mWorkspaceName, projectName );
-
-  // Verify a transaction was created
-  QCOMPARE( mApi->transactions().count(), 1 );
-
-  // Wait for the 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 transaction.MAX_RETRY_COUNT retry attempts were made
-  int maxRetries = TransactionStatus::MAX_RETRY_COUNT;
-  QCOMPARE( retrySpy.count(), maxRetries );
+  disconnect( mApi, &MerginApi::downloadItemsStarted, this, nullptr );
 
-  // Verify the sync failed
-  QList<QVariant> arguments = finishSpy.takeFirst();
-  QVERIFY( !arguments.at( 1 ).toBool() );
+  // Second attempt - TemporaryNetworkFailureError
+  connect( mApi, &MerginApi::downloadItemsStarted, this, [this, failingManager]()
+  {
+    failingManager->setShouldFail( true, QNetworkReply::TemporaryNetworkFailureError );
+  } );
 
-  // Verify no local project was created
-  LocalProject localProject = mApi->localProjectsManager().projectFromMerginName( mWorkspaceName, projectName );
-  QVERIFY( !localProject.isValid() );
+  mApi->pullProject( mWorkspaceName, projectName );
+  QVERIFY( startSpy.wait( TestUtils::LONG_REPLY ) );
+  QVERIFY( finishSpy.wait( TestUtils::LONG_REPLY ) );
 
-  // Disconnect all signals
   disconnect( mApi, &MerginApi::downloadItemsStarted, this, nullptr );
 
-  // Restore the original manager
-  QNetworkAccessManager *originalManager = new QNetworkAccessManager( this );
+  QVERIFY( startSpy.count() > 0 );
+  QVERIFY( retrySpy.count() > 0 );
+  QCOMPARE( finishSpy.count(), 3 );
+
+  // Restore original manager and clear old one
   mApi->setNetworkManager( originalManager );
+  delete failingManager;
 }
-

app/test/testmerginapi.h

@@ -24,17 +24,22 @@
 class MockReply : public QNetworkReply
 {
   public:
-    explicit MockReply( const QNetworkRequest &request, QNetworkAccessManager::Operation operation, QObject *parent = nullptr )
+    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() );
 
-      setError( QNetworkReply::TimeoutError, "Mock network failure" );
+      if ( errorCode != QNetworkReply::NoError )
+      {
+        setError( errorCode, "Mock network failure" );
+        QMetaObject::invokeMethod( this, "errorOccurred", Qt::QueuedConnection, Q_ARG( QNetworkReply::NetworkError, errorCode ) );
+      }
 
-      QMetaObject::invokeMethod( this, "errorOccurred", Qt::QueuedConnection, Q_ARG( QNetworkReply::NetworkError, QNetworkReply::TimeoutError ) );
       QMetaObject::invokeMethod( this, "finished", Qt::QueuedConnection );
+      open( QIODevice::ReadOnly );
     }
 
     void abort() override {}
@@ -58,29 +63,29 @@ class MockNetworkManager : public QNetworkAccessManager
     explicit MockNetworkManager( QObject *parent = nullptr )
       : QNetworkAccessManager( parent )
       , mShouldFail( false )
+      , mErrorCode( QNetworkReply::NoError )
     {}
 
-    void setShouldFail( bool shouldFail )
+    void setShouldFail( bool shouldFail, QNetworkReply::NetworkError errorCode = QNetworkReply::NoError )
     {
       mShouldFail = shouldFail;
+      mErrorCode = errorCode;
     }
 
-    bool shouldFail() const { return mShouldFail; }
-
   protected:
     QNetworkReply *createRequest( Operation op, const QNetworkRequest &request, QIODevice *outgoingData = nullptr ) override
     {
       if ( mShouldFail )
       {
-        MockReply *reply = new MockReply( request, op, this );
+        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

core/merginapi.cpp

@@ -406,12 +406,15 @@ void MerginApi::downloadItemReplyFinished( DownloadQueueItem item )
     transaction.transferedSize += data.size();
     emit syncProjectStatusChanged( projectFullName, transaction.transferedSize / transaction.totalSize );
     transaction.replyPullItems.remove( r );
+
     r->deleteLater();
+
     if ( !transaction.downloadQueue.isEmpty() )
     {
       // 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
@@ -3978,10 +3981,7 @@ void MerginApi::setNetworkManager( QNetworkAccessManager *manager )
   if ( mManager == manager )
     return;
 
-  delete mManager;
   mManager = manager;
-  if ( mManager )
-    mManager->setParent( this );
 
   emit networkManagerChanged();
 }