Generalize Result<> class, add Optional<>

To make it nicer to use outside of HTTP results.

src/common/result.h

@@ -14,21 +14,16 @@
 
 #pragma once
 
+#include "asserts.h"
+
 namespace OCC {
 
 /**
- * A Result of type T, or an Error that contains a code and a string
- *
- * The code is an HTTP error code.
- **/
-template <typename T>
+ * A Result of type T, or an Error
+ */
+template <typename T, typename Error>
 class Result
 {
-    struct Error
-    {
-        QString string;
-        int code;
-    };
     union {
         T _result;
         Error _error;
@@ -38,12 +33,16 @@ class Result
 public:
     Result(T value)
         : _result(std::move(value))
-        , _isError(false){};
-    Result(int code, QString str)
-        : _error({ std::move(str), code })
+        , _isError(false)
+    {
+    }
+    // TODO: This doesn't work if T and Error are too similar
+    Result(Error error)
+        : _error(std::move(error))
         , _isError(true)
     {
     }
+
     ~Result()
     {
         if (_isError)
@@ -62,14 +61,31 @@ public:
         ASSERT(!_isError);
         return std::move(_result);
     }
-    QString errorMessage() const
+    const Error &error() const &
     {
         ASSERT(_isError);
-        return _error.string;
+        return _error;
     }
-    int errorCode() const
+    Error error() &&
+    {
+        ASSERT(_isError);
+        return std::move(_error);
+    }
+};
+
+namespace detail {
+struct OptionalNoErrorData{};
+}
+
+template <typename T>
+class Optional : public Result<T, detail::OptionalNoErrorData>
+{
+public:
+    using Result<T, detail::OptionalNoErrorData>::Result;
+
+    Optional()
+        : Optional(detail::OptionalNoErrorData{})
     {
-        return _isError ? _error.code : 0;
     }
 };
 

src/libsync/discovery.cpp

@@ -522,10 +522,10 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
             // we need to make a request to the server to know that the original file is deleted on the server
             _pendingAsyncJobs++;
             auto job = new RequestEtagJob(_discoveryData->_account, originalPath, this);
-            connect(job, &RequestEtagJob::finishedWithResult, this, [=](const Result<QString> &etag) mutable {
+            connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult<QString> &etag) mutable {
                 _pendingAsyncJobs--;
                 QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
-                if (etag.errorCode() != 404 ||
+                if (etag || etag.error().code != 404 ||
                     // Somehow another item claimed this original path, consider as if it existed
                     _discoveryData->_renamedItems.contains(originalPath)) {
                     // If the file exist or if there is another error, consider it is a new file.
@@ -828,7 +828,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
             if (base.isVirtualFile() && isVfsWithSuffix())
                 chopVirtualFileSuffix(serverOriginalPath);
             auto job = new RequestEtagJob(_discoveryData->_account, serverOriginalPath, this);
-            connect(job, &RequestEtagJob::finishedWithResult, this, [=](const Result<QString> &etag) mutable {
+            connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult<QString> &etag) mutable {
                 if (!etag || (*etag != base._etag && !item->isDirectory()) || _discoveryData->_renamedItems.contains(originalPath)) {
                     qCInfo(lcDisco) << "Can't rename because the etag has changed or the directory is gone" << originalPath;
                     // Can't be a rename, leave it as a new.
@@ -1220,17 +1220,17 @@ DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery()
             if (_localQueryDone)
                 process();
         } else {
-            if (results.errorCode() == 403) {
+            if (results.error().code == 403) {
                 // 403 Forbidden can be sent by the server if the file firewall is active.
                 // A file or directory should be ignored and sync must continue. See #3490
                 qCWarning(lcDisco, "Directory access Forbidden (File Firewall?)");
                 if (_dirItem) {
                     _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
-                    _dirItem->_errorString = results.errorMessage();
+                    _dirItem->_errorString = results.error().message;
                     emit finished();
                     return;
                 }
-            } else if (results.errorCode() == 503) {
+            } else if (results.error().code == 503) {
                 // The server usually replies with the custom "503 Storage not available"
                 // if some path is temporarily unavailable. But in some cases a standard 503
                 // is returned too. Thus we can't distinguish the two and will treat any
@@ -1238,13 +1238,13 @@ DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery()
                 qCWarning(lcDisco(), "Storage was not available!");
                 if (_dirItem) {
                     _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
-                    _dirItem->_errorString = results.errorMessage();
+                    _dirItem->_errorString = results.error().message;
                     emit finished();
                     return;
                 }
             }
             emit _discoveryData->fatalError(tr("Server replied with an error while reading directory '%1' : %2")
-                .arg(_currentFolder._server, results.errorMessage()));
+                .arg(_currentFolder._server, results.error().message));
         }
     });
     connect(serverJob, &DiscoverySingleDirectoryJob::firstDirectoryPermissions, this,

src/libsync/discoveryphase.cpp

@@ -355,11 +355,11 @@ void DiscoverySingleDirectoryJob::lsJobFinishedWithoutErrorSlot()
     if (!_ignoredFirst) {
         // This is a sanity check, if we haven't _ignoredFirst then it means we never received any directoryListingIteratedSlot
         // which means somehow the server XML was bogus
-        emit finished({ 0, tr("Server error: PROPFIND reply is not XML formatted!") });
+        emit finished(HttpError{ 0, tr("Server error: PROPFIND reply is not XML formatted!") });
         deleteLater();
         return;
     } else if (!_error.isEmpty()) {
-        emit finished({ 0, _error });
+        emit finished(HttpError{ 0, _error });
         deleteLater();
         return;
     }
@@ -379,7 +379,7 @@ void DiscoverySingleDirectoryJob::lsJobFinishedWithErrorSlot(QNetworkReply *r)
         && !contentType.contains("application/xml; charset=utf-8")) {
         msg = tr("Server error: PROPFIND reply is not XML formatted!");
     }
-    emit finished({httpCode, msg});
+    emit finished(HttpError{ httpCode, msg });
     deleteLater();
 }
 }

src/libsync/discoveryphase.h

@@ -97,7 +97,7 @@ public:
 signals:
     void firstDirectoryPermissions(RemotePermissions);
     void etag(const QString &);
-    void finished(const Result<QVector<RemoteInfo>> &result);
+    void finished(const HttpResult<QVector<RemoteInfo>> &result);
 private slots:
     void directoryListingIteratedSlot(QString, const QMap<QString, QString> &);
     void lsJobFinishedWithoutErrorSlot();

src/libsync/networkjobs.cpp

@@ -107,7 +107,7 @@ bool RequestEtagJob::finished()
         emit etagRetrieved(etag);
         emit finishedWithResult(etag);
     } else {
-        emit finishedWithResult({ httpCode, errorString() });
+        emit finishedWithResult(HttpError{ httpCode, errorString() });
     }
     return true;
 }

src/libsync/networkjobs.h

@@ -18,7 +18,7 @@
 
 #include "abstractnetworkjob.h"
 
-#include "result.h"
+#include "common/result.h"
 
 #include <QBuffer>
 #include <QUrlQuery>
@@ -29,6 +29,15 @@ class QJsonObject;
 
 namespace OCC {
 
+struct HttpError
+{
+    int code; // HTTP error code
+    QString message;
+};
+
+template <typename T>
+using HttpResult = Result<T, HttpError>;
+
 /**
  * @brief The EntityExistsJob class
  * @ingroup libsync
@@ -337,7 +346,7 @@ public:
 
 signals:
     void etagRetrieved(const QString &etag);
-    void finishedWithResult(const Result<QString> &etag);
+    void finishedWithResult(const HttpResult<QString> &etag);
 
 private slots:
     bool finished() override;