Merge pull request #12427 from huffman/fix/http-connection-crash

Fix rolling backups, delete backup, and recovery id

domain-server/src/AssetsBackupHandler.cpp

@@ -306,7 +306,7 @@ void AssetsBackupHandler::recoverBackup(QuaZip& zip) {
     restoreAllAssets();
 }
 
-void AssetsBackupHandler::deleteBackup(QuaZip& zip) {
+void AssetsBackupHandler::deleteBackup(const QString& absoluteFilePath) {
     Q_ASSERT(QThread::currentThread() == thread());
 
     if (operationInProgress()) {
@@ -315,10 +315,10 @@ void AssetsBackupHandler::deleteBackup(QuaZip& zip) {
     }
 
     auto it = find_if(begin(_backups), end(_backups), [&](const std::vector<AssetServerBackup>::value_type& value) {
-        return value.filePath == zip.getZipName();
+        return value.filePath == absoluteFilePath;
     });
     if (it == end(_backups)) {
-        qCDebug(asset_backup) << "Could not find backup" << zip.getZipName() << "to delete.";
+        qCDebug(asset_backup) << "Could not find backup" << absoluteFilePath << "to delete.";
         return;
     }
 

domain-server/src/AssetsBackupHandler.h

@@ -37,7 +37,7 @@ public:
     void loadBackup(QuaZip& zip) override;
     void createBackup(QuaZip& zip) override;
     void recoverBackup(QuaZip& zip) override;
-    void deleteBackup(QuaZip& zip) override;
+    void deleteBackup(const QString& absoluteFilePath) override;
     void consolidateBackup(QuaZip& zip) override;
 
     bool operationInProgress() { return getRecoveryStatus().first; }

domain-server/src/BackupHandler.h

@@ -30,7 +30,7 @@ public:
     virtual void loadBackup(QuaZip& zip) = 0;
     virtual void createBackup(QuaZip& zip) = 0;
     virtual void recoverBackup(QuaZip& zip) = 0;
-    virtual void deleteBackup(QuaZip& zip) = 0;
+    virtual void deleteBackup(const QString& absoluteFilePath) = 0;
     virtual void consolidateBackup(QuaZip& zip) = 0;
 };
 using BackupHandlerPointer = std::unique_ptr<BackupHandlerInterface>;

domain-server/src/ContentSettingsBackupHandler.h

@@ -28,7 +28,7 @@ public:
 
     void recoverBackup(QuaZip& zip) override;
 
-    void deleteBackup(QuaZip& zip) override {}
+    void deleteBackup(const QString& absoluteFilePath) override {}
 
     void consolidateBackup(QuaZip& zip) override {}
 private:

domain-server/src/DomainContentBackupManager.cpp

@@ -89,8 +89,7 @@ void DomainContentBackupManager::parseSettings(const QJsonObject& settings) {
             }
 
             auto name = obj["Name"].toString();
-            auto format = obj["format"].toString();
+            auto format = name.replace(" ", "_").toLower();
-            format = name.replace(" ", "_").toLower();
 
             qCDebug(domain_server) << "    Name:" << name;
             qCDebug(domain_server) << "        format:" << format;
@@ -116,6 +115,12 @@ void DomainContentBackupManager::parseSettings(const QJsonObject& settings) {
     }
 }
 
+void DomainContentBackupManager::refreshBackupRules() {
+    for (auto& backup : _backupRules) {
+        backup.lastBackupSeconds = getMostRecentBackupTimeInSecs(backup.extensionFormat);
+    }
+}
+
 int64_t DomainContentBackupManager::getMostRecentBackupTimeInSecs(const QString& format) {
     int64_t mostRecentBackupInSecs = 0;
 
@@ -155,6 +160,7 @@ bool DomainContentBackupManager::process() {
 
                 if (!isStillRecovering) {
                     _isRecovering = false;
+                    _recoveryFilename = "";
                     emit recoveryCompleted();
                 }
             }
@@ -235,8 +241,16 @@ void DomainContentBackupManager::deleteBackup(MiniPromise::Promise promise, cons
     }
 
     QDir backupDir { _backupDirectory };
-    QFile backupFile { backupDir.filePath(backupName) };
+    auto absoluteFilePath { backupDir.filePath(backupName) };
+    QFile backupFile { absoluteFilePath };
     auto success = backupFile.remove();
+
+    refreshBackupRules();
+
+    for (auto& handler : _backupHandlers) {
+        handler->deleteBackup(absoluteFilePath);
+    }
+
     promise->resolve({
         { "success", success }
     });
@@ -268,6 +282,7 @@ void DomainContentBackupManager::recoverFromBackup(MiniPromise::Promise promise,
             success = false;
         } else {
             _isRecovering = true;
+            _recoveryFilename = backupName;
             for (auto& handler : _backupHandlers) {
                 handler->recoverBackup(zip);
             }
@@ -286,12 +301,7 @@ void DomainContentBackupManager::recoverFromBackup(MiniPromise::Promise promise,
     });
 }
 
-void DomainContentBackupManager::getAllBackupInformation(MiniPromise::Promise promise) {
+std::vector<BackupItemInfo> DomainContentBackupManager::getAllBackups() {
-    if (QThread::currentThread() != thread()) {
-        QMetaObject::invokeMethod(this, "getAllBackupInformation", Q_ARG(MiniPromise::Promise, promise));
-        return;
-    }
-
     QDir backupDir { _backupDirectory };
     auto matchingFiles =
             backupDir.entryInfoList({ AUTOMATIC_BACKUP_PREFIX + "*.zip", MANUAL_BACKUP_PREFIX + "*.zip" },
@@ -301,7 +311,7 @@ void DomainContentBackupManager::getAllBackupInformation(MiniPromise::Promise pr
     QString dateTimeFormat = "(" + DATETIME_FORMAT_RE + ")";
     QRegExp backupNameFormat { prefixFormat + nameFormat + "-" + dateTimeFormat + "\\.zip" };
 
-    QVariantList backups;
+    std::vector<BackupItemInfo> backups;
 
     for (const auto& fileInfo : matchingFiles) {
         auto fileName = fileInfo.fileName();
@@ -324,17 +334,53 @@ void DomainContentBackupManager::getAllBackupInformation(MiniPromise::Promise pr
                 availabilityProgress += progress / _backupHandlers.size();
             }
 
-            backups.push_back(QVariantMap({
+            backups.emplace_back(fileInfo.fileName(), name, fileInfo.absoluteFilePath(), createdAt,
-                { "id", fileInfo.fileName() },
+                                 type == MANUAL_BACKUP_PREFIX);
-                { "name", name },
-                { "createdAtMillis", createdAt.toMSecsSinceEpoch() },
-                { "isAvailable", isAvailable },
-                { "availabilityProgress", availabilityProgress },
-                { "isManualBackup", type == MANUAL_BACKUP_PREFIX }
-            }));
         }
     }
 
+    return backups;
+}
+
+void DomainContentBackupManager::getAllBackupsAndStatus(MiniPromise::Promise promise) {
+    if (QThread::currentThread() != thread()) {
+        QMetaObject::invokeMethod(this, "getAllBackupsAndStatus", Q_ARG(MiniPromise::Promise, promise));
+        return;
+    }
+
+    QDir backupDir { _backupDirectory };
+    auto matchingFiles =
+            backupDir.entryInfoList({ AUTOMATIC_BACKUP_PREFIX + "*.zip", MANUAL_BACKUP_PREFIX + "*.zip" },
+                                    QDir::Files | QDir::NoSymLinks, QDir::Name);
+    QString prefixFormat = "(" + QRegExp::escape(AUTOMATIC_BACKUP_PREFIX) + "|" + QRegExp::escape(MANUAL_BACKUP_PREFIX) + ")";
+    QString nameFormat = "(.+)";
+    QString dateTimeFormat = "(" + DATETIME_FORMAT_RE + ")";
+    QRegExp backupNameFormat { prefixFormat + nameFormat + "-" + dateTimeFormat + "\\.zip" };
+
+    auto backups = getAllBackups();
+
+    QVariantList variantBackups;
+
+    for (auto& backup : backups) {
+        bool isAvailable { true };
+        float availabilityProgress { 0.0f };
+        for (auto& handler : _backupHandlers) {
+            bool handlerIsAvailable { true };
+            float progress { 0.0f };
+            std::tie(handlerIsAvailable, progress) = handler->isAvailable(backup.absolutePath);
+            isAvailable &= handlerIsAvailable;
+            availabilityProgress += progress / _backupHandlers.size();
+        }
+        variantBackups.push_back(QVariantMap({
+            { "id", backup.id },
+            { "name", backup.name },
+            { "createdAtMillis", backup.createdAt.toMSecsSinceEpoch() },
+            { "isAvailable", isAvailable },
+            { "availabilityProgress", availabilityProgress },
+            { "isManualBackup", backup.isManualBackup }
+        }));
+    }
+
     float recoveryProgress = 0.0f;
     bool isRecovering = _isRecovering.load();
     if (_isRecovering) {
@@ -351,7 +397,7 @@ void DomainContentBackupManager::getAllBackupInformation(MiniPromise::Promise pr
     };
 
     QVariantMap info {
-        { "backups", backups },
+        { "backups", variantBackups },
         { "status", status }
     };
 
@@ -390,32 +436,27 @@ void DomainContentBackupManager::removeOldBackupVersions(const BackupRule& rule)
 }
 
 void DomainContentBackupManager::load() {
-    QDir backupDir { _backupDirectory };
+    auto backups = getAllBackups();
-    if (backupDir.exists()) {
+    for (auto& backup : backups) {
-
+        QFile backupFile { backup.absolutePath };
-        auto matchingFiles = backupDir.entryInfoList({ "backup-*.zip" }, QDir::Files | QDir::NoSymLinks, QDir::Name);
+        if (!backupFile.open(QIODevice::ReadOnly)) {
-
+            qCritical() << "Could not open file:" << backup.absolutePath;
-        for (const auto& file : matchingFiles) {
+            qCritical() << "    ERROR:" << backupFile.errorString();
-            QFile backupFile { file.absoluteFilePath() };
+            continue;
-            if (!backupFile.open(QIODevice::ReadOnly)) {
-                qCritical() << "Could not open file:" << file.absoluteFilePath();
-                qCritical() << "    ERROR:" << backupFile.errorString();
-                continue;
-            }
-
-            QuaZip zip { &backupFile };
-            if (!zip.open(QuaZip::mdUnzip)) {
-                qCritical() << "Could not open backup archive:" << file.absoluteFilePath();
-                qCritical() << "    ERROR:" << zip.getZipError();
-                continue;
-            }
-
-            for (auto& handler : _backupHandlers) {
-                handler->loadBackup(zip);
-            }
-
-            zip.close();
         }
+
+        QuaZip zip { &backupFile };
+        if (!zip.open(QuaZip::mdUnzip)) {
+            qCritical() << "Could not open backup archive:" << backup.absolutePath;
+            qCritical() << "    ERROR:" << zip.getZipError();
+            continue;
+        }
+
+        for (auto& handler : _backupHandlers) {
+            handler->loadBackup(zip);
+        }
+
+        zip.close();
     }
 }
 

domain-server/src/DomainContentBackupManager.h

@@ -24,6 +24,17 @@
 
 #include <shared/MiniPromises.h>
 
+struct BackupItemInfo {
+    BackupItemInfo(QString pId, QString pName, QString pAbsolutePath, QDateTime pCreatedAt, bool pIsManualBackup) :
+        id(pId), name(pName), absolutePath(pAbsolutePath), createdAt(pCreatedAt), isManualBackup(pIsManualBackup) { };
+
+    QString id;
+    QString name;
+    QString absolutePath;
+    QDateTime createdAt;
+    bool isManualBackup;
+};
+
 class DomainContentBackupManager : public GenericThread {
     Q_OBJECT
 public:
@@ -43,14 +54,13 @@ public:
                                int persistInterval = DEFAULT_PERSIST_INTERVAL,
                                bool debugTimestampNow = false);
 
+    std::vector<BackupItemInfo> getAllBackups();
     void addBackupHandler(BackupHandlerPointer handler);
-
     void aboutToFinish();  /// call this to inform the persist thread that the owner is about to finish to support final persist
-
     void replaceData(QByteArray data);
 
 public slots:
-    void getAllBackupInformation(MiniPromise::Promise promise);
+    void getAllBackupsAndStatus(MiniPromise::Promise promise);
     void createManualBackup(MiniPromise::Promise promise, const QString& name);
     void recoverFromBackup(MiniPromise::Promise promise, const QString& backupName);
     void deleteBackup(MiniPromise::Promise promise, const QString& backupName);
@@ -68,6 +78,7 @@ protected:
     void load();
     void backup();
     void removeOldBackupVersions(const BackupRule& rule);
+    void refreshBackupRules();
     bool getMostRecentBackup(const QString& format, QString& mostRecentBackupFileName, QDateTime& mostRecentBackupTime);
     int64_t getMostRecentBackupTimeInSecs(const QString& format);
     void parseSettings(const QJsonObject& settings);

domain-server/src/DomainServer.cpp

@@ -2128,13 +2128,13 @@ bool DomainServer::handleHTTPRequest(HTTPConnection* connection, const QUrl& url
 
             return true;
         } else if (url.path() == URI_API_BACKUPS) {
-            auto deferred = makePromise("getAllBackupInformation");
+            auto deferred = makePromise("getAllBackupsAndStatus");
             deferred->then([connection, JSON_MIME_TYPE](QString error, QVariantMap result) {
                 QJsonDocument docJSON(QJsonObject::fromVariantMap(result));
 
                 connection->respond(HTTPConnection::StatusCode200, docJSON.toJson(), JSON_MIME_TYPE.toUtf8());
             });
-            _contentManager->getAllBackupInformation(deferred);
+            _contentManager->getAllBackupsAndStatus(deferred);
             return true;
         } else if (url.path().startsWith(URI_API_BACKUPS_ID)) {
             auto id = url.path().mid(QString(URI_API_BACKUPS_ID).length());

domain-server/src/EntitiesBackupHandler.h

@@ -30,7 +30,7 @@ public:
     void recoverBackup(QuaZip& zip) override;
 
     // Delete a skeleton backup
-    void deleteBackup(QuaZip& zip) override {}
+    void deleteBackup(const QString& absoluteFilePath) override {}
 
     // Create a full backup
     void consolidateBackup(QuaZip& zip) override {}

libraries/embedded-webserver/src/HTTPConnection.cpp

@@ -133,57 +133,33 @@ QList<FormData> HTTPConnection::parseFormData() const {
 }
 
 void HTTPConnection::respond(const char* code, const QByteArray& content, const char* contentType, const Headers& headers) {
-    QByteArray data(content);
+    respondWithStatusAndHeaders(code, contentType, headers, content.size());
-    auto device { std::unique_ptr<QBuffer>(new QBuffer()) };
+
-    device->setBuffer(new QByteArray(content));
+    _socket->write(content);
-    if (device->open(QIODevice::ReadOnly)) {
+
-        respond(code, std::move(device), contentType, headers);
+    _socket->disconnectFromHost();
-    } else {
+
-        qCritical() << "Error opening QBuffer to respond to " << _requestUrl.path();
+    // make sure we receive no further read notifications
-    }
+    disconnect(_socket, &QTcpSocket::readyRead, this, nullptr);
 }
 
 void HTTPConnection::respond(const char* code, std::unique_ptr<QIODevice> device, const char* contentType, const Headers& headers) {
     _responseDevice = std::move(device);
 
-    _socket->write("HTTP/1.1 ");
-
     if (_responseDevice->isSequential()) {
         qWarning() << "Error responding to HTTPConnection: sequential IO devices not supported";
-        _socket->write(StatusCode500);
+        respondWithStatusAndHeaders(StatusCode500, contentType, headers, 0);
-        _socket->write("\r\n");
         _socket->disconnect(SIGNAL(readyRead()), this);
         _socket->disconnectFromHost();
         return;
     }
 
-    _socket->write(code);
+    int totalToBeWritten = _responseDevice->size();
-    _socket->write("\r\n");
+    respondWithStatusAndHeaders(code, contentType, headers, totalToBeWritten);
-
-    for (Headers::const_iterator it = headers.constBegin(), end = headers.constEnd();
-            it != end; it++) {
-        _socket->write(it.key());
-        _socket->write(": ");
-        _socket->write(it.value());
-        _socket->write("\r\n");
-    }
-
-    int csize = _responseDevice->size();
-    if (csize > 0) {
-        _socket->write("Content-Length: ");
-        _socket->write(QByteArray::number(csize));
-        _socket->write("\r\n");
-
-        _socket->write("Content-Type: ");
-        _socket->write(contentType);
-        _socket->write("\r\n");
-    }
-    _socket->write("Connection: close\r\n\r\n");
 
     if (_responseDevice->atEnd()) {
         _socket->disconnectFromHost();
     } else {
-        int totalToBeWritten = csize;
         connect(_socket, &QTcpSocket::bytesWritten, this, [this, totalToBeWritten](size_t bytes) mutable {
             constexpr size_t HTTP_RESPONSE_CHUNK_SIZE = 1024 * 10;
             if (!_responseDevice->atEnd()) {
@@ -201,6 +177,32 @@ void HTTPConnection::respond(const char* code, std::unique_ptr<QIODevice> device
     disconnect(_socket, &QTcpSocket::readyRead, this, nullptr);
 }
 
+void HTTPConnection::respondWithStatusAndHeaders(const char* code, const char* contentType, const Headers& headers, qint64 contentLength) {
+    _socket->write("HTTP/1.1 ");
+
+    _socket->write(code);
+    _socket->write("\r\n");
+
+    for (Headers::const_iterator it = headers.constBegin(), end = headers.constEnd();
+            it != end; it++) {
+        _socket->write(it.key());
+        _socket->write(": ");
+        _socket->write(it.value());
+        _socket->write("\r\n");
+    }
+
+    if (contentLength > 0) {
+        _socket->write("Content-Length: ");
+        _socket->write(QByteArray::number(contentLength));
+        _socket->write("\r\n");
+
+        _socket->write("Content-Type: ");
+        _socket->write(contentType);
+        _socket->write("\r\n");
+    }
+    _socket->write("Connection: close\r\n\r\n");
+}
+
 void HTTPConnection::readRequest() {
     if (!_socket->canReadLine()) {
         return;

libraries/embedded-webserver/src/HTTPConnection.h

@@ -105,6 +105,7 @@ protected slots:
     void readContent ();
 
 protected:
+    void respondWithStatusAndHeaders(const char* code, const char* contentType, const Headers& headers, qint64 size);
 
     /// The parent HTTP manager
     HTTPManager* _parentManager;