Finished downloads using PurpleXfer

file-transfer.cpp

@@ -133,6 +133,7 @@ static void nop(PurpleXfer *xfer)
 static void cancelDownload(PurpleXfer *xfer)
 {
     std::unique_ptr<DownloadData> data(static_cast<DownloadData *>(xfer->data));
+    xfer->data = NULL;
     if (!data) return;
 
     int32_t fileId;
@@ -212,7 +213,7 @@ static void updateDownloadProgress(const td::td_api::file &file, PurpleXfer *xfe
             if (downloadReq->tempFd >= 0)
                 close(downloadReq->tempFd);
             downloadReq->tempFd = -1;
-            //!!!! if (purple_xfer_get_status(xfer) != PURPLE_XFER_STATUS_STARTED)
+            if (purple_xfer_get_status(xfer) != PURPLE_XFER_STATUS_STARTED)
                 purple_xfer_start(xfer, -1, NULL, 0);
         }
 
@@ -244,6 +245,7 @@ void finishInlineDownloadProgress(DownloadRequest &downloadReq, TdAccountData& a
 
     if (account.getFileTransfer(downloadReq.fileId, download, chatId)) {
         std::unique_ptr<DownloadData> data(static_cast<DownloadData *>(download->data));
+        download->data = NULL;
         purple_xfer_set_bytes_sent(download, downloadReq.fileSize);
         purple_xfer_set_completed(download, TRUE);
         purple_xfer_end(download);
@@ -291,6 +293,8 @@ static void standardDownloadResponse(TdAccountData *account, uint64_t requestId,
 
     if (account->getFileTransfer(request->fileId, download, chatId)) {
         std::unique_ptr<DownloadData> data(static_cast<DownloadData *>(download->data));
+        download->data = NULL;
+
         gchar *content = NULL;
         gsize fileSize = 0;
         GError *error  = NULL;
@@ -306,12 +310,18 @@ static void standardDownloadResponse(TdAccountData *account, uint64_t requestId,
                 purple_xfer_end(download);
             }
             purple_xfer_unref(download);
-            // !!!! account->removeFileTransfer(request->fileId);
+            account->removeFileTransfer(request->fileId);
         } else {
             if (error) {
-                purple_xfer_error(PURPLE_XFER_RECEIVE, account->purpleAccount, download->who, error->message);
+                // Unlikely error message not worth translating
+                std::string message = formatMessage("Failed to read {}: {}", {path, std::string(error->message)});
+                purple_debug_misc(config::pluginId, "%s\n", message.c_str());
+                purple_xfer_error(PURPLE_XFER_RECEIVE, account->purpleAccount, download->who, message.c_str());
                 g_error_free(error);
             }
+            if (path.empty())
+                purple_debug_warning(config::pluginId, "Incomplete file in download response for %s\n",
+                                     purple_xfer_get_local_filename(download));
             purple_xfer_cancel_remote(download);
         }
 
@@ -345,7 +355,7 @@ static void startStandardDownload(PurpleXfer *xfer)
         data->account->addPendingRequest<DownloadRequest>(requestId, std::move(request));
         // Start immediately, because standardDownloadResponse will call purple_xfer_write_file, which
         // will fail if purple_xfer_start hasn't been called
-        // !!!! purple_xfer_start(xfer, -1, NULL, 0);
+        purple_xfer_start(xfer, -1, NULL, 0);
     }
 }
 

test/file-transfer-test.cpp

@@ -907,11 +907,13 @@ TEST_F(FileTransferTest, SendFileToNonContact_TurboCancel)
     ));
 }
 
-TEST_F(FileTransferTest, ReceiveDocument_StandardTransfer)
+TEST_F(FileTransferTest, ReceiveDocument_StandardTransfer_TinyFile)
 {
     const int64_t messageId = 1;
     const int32_t date      = 10001;
     const int32_t fileId    = 1234;
+    uint8_t       data[]    = {1, 2, 3, 4, 5};
+    const char *outputFileName = ".test_download";
 
     setUiName("spectrum"); // No longer pidgin - now downloads will use libpurple transfers
     loginWithOneContact();
@@ -943,9 +945,117 @@ TEST_F(FileTransferTest, ReceiveDocument_StandardTransfer)
         XferRequestEvent(PURPLE_XFER_RECEIVE, "doc.file.name")
     );
 
-    purple_xfer_request_accepted(prpl.getLastXfer(), ".test_download");
+    purple_xfer_request_accepted(prpl.getLastXfer(), outputFileName);
     prpl.verifyEvents(
-        XferAcceptedEvent(".test_download"),
+        XferAcceptedEvent(outputFileName),
-        XferLocalCancelEvent(".test_download")
+        XferStartEvent(outputFileName)
     );
+
+    tgl.verifyRequest(downloadFile(fileId, 1, 0, 0, true));
+
+    char *tdlibFileName = NULL;
+    int fd = g_file_open_tmp("tdlib_test_XXXXXX", &tdlibFileName, NULL);
+    ASSERT_TRUE(fd >= 0);
+    ASSERT_EQ((ssize_t)sizeof(data), write(fd, data, sizeof(data)));
+    ::close(fd);
+
+    tgl.reply(make_object<file>(
+        fileId, 10000, 10000,
+        make_object<localFile>(tdlibFileName, true, true, false, true, 0, 10000, 10000),
+        make_object<remoteFile>("beh", "bleh", false, true, 10000)
+    ));
+    tgl.update(make_object<file>(
+        fileId, 10000, 10000,
+        make_object<localFile>(tdlibFileName, true, true, false, true, 0, 10000, 10000),
+        make_object<remoteFile>("beh", "bleh", false, true, 10000)
+    ));
+
+    prpl.verifyEvents(
+        XferWriteFileEvent(outputFileName, data, sizeof(data)),
+        XferCompletedEvent(outputFileName, TRUE, sizeof(data)),
+        XferEndEvent(outputFileName)
+    );
+
+    remove(tdlibFileName);
+    g_free(tdlibFileName);
+}
+
+TEST_F(FileTransferTest, ReceiveDocument_StandardTransfer_Progress)
+{
+    const int64_t messageId = 1;
+    const int32_t date      = 10001;
+    const int32_t fileId    = 1234;
+    uint8_t       data[]    = {1, 2, 3, 4, 5};
+    const char *outputFileName = ".test_download";
+
+    setUiName("spectrum"); // No longer pidgin - now downloads will use libpurple transfers
+    loginWithOneContact();
+
+    tgl.update(make_object<updateNewMessage>(makeMessage(
+        messageId,
+        userIds[0],
+        chatIds[0],
+        false,
+        date,
+        make_object<messageDocument>(
+            make_object<document>(
+                "doc.file.name", "mime/type", nullptr, nullptr,
+                make_object<file>(
+                    fileId, 10000, 10000,
+                    make_object<localFile>("", true, true, false, false, 0, 0, 0),
+                    make_object<remoteFile>("beh", "bleh", false, true, 10000)
+                )
+            ),
+            make_object<formattedText>("document", std::vector<object_ptr<textEntity>>())
+        )
+    )));
+    tgl.verifyRequest(viewMessages(
+        chatIds[0],
+        std::vector<int64_t>(1, messageId),
+        true
+    ));
+    prpl.verifyEvents(
+        XferRequestEvent(PURPLE_XFER_RECEIVE, "doc.file.name")
+    );
+
+    purple_xfer_request_accepted(prpl.getLastXfer(), outputFileName);
+    prpl.verifyEvents(
+        XferAcceptedEvent(outputFileName),
+        XferStartEvent(outputFileName)
+    );
+
+    tgl.verifyRequest(downloadFile(fileId, 1, 0, 0, true));
+
+    char *tdlibFileName = NULL;
+    int fd = g_file_open_tmp("tdlib_test_XXXXXX", &tdlibFileName, NULL);
+    ASSERT_TRUE(fd >= 0);
+    ASSERT_EQ((ssize_t)sizeof(data), write(fd, data, sizeof(data)));
+    ::close(fd);
+
+    tgl.update(make_object<updateFile>(make_object<file>(
+        fileId, 10000, 10000,
+        make_object<localFile>(tdlibFileName, true, true, true, false, 0, 0, 2000),
+        make_object<remoteFile>("beh", "bleh", false, true, 10000)
+    )));
+    prpl.verifyEvents(XferProgressEvent(outputFileName, 2000));
+
+    tgl.reply(make_object<file>(
+        fileId, 10000, 10000,
+        make_object<localFile>(tdlibFileName, true, true, false, true, 0, 10000, 10000),
+        make_object<remoteFile>("beh", "bleh", false, true, 10000)
+    ));
+    tgl.update(make_object<file>(
+        fileId, 10000, 10000,
+        make_object<localFile>(tdlibFileName, true, true, false, true, 0, 10000, 10000),
+        make_object<remoteFile>("beh", "bleh", false, true, 10000)
+    ));
+
+    prpl.verifyEvents(
+        XferWriteFileEvent(outputFileName, data, sizeof(data)),
+        XferCompletedEvent(outputFileName, TRUE, sizeof(data)),
+        XferEndEvent(outputFileName)
+    );
+
+    remove(tdlibFileName);
+    g_free(tdlibFileName);
 }

test/fixture.cpp

@@ -184,7 +184,7 @@ void checkFile(const char *filename, void *content, unsigned size)
     gchar *actualContent;
     gsize  actualSize;
     ASSERT_TRUE(g_file_get_contents(filename, &actualContent, &actualSize, NULL)) << filename << " does not exist";
-    ASSERT_EQ(actualSize, size);
+    ASSERT_EQ(actualSize, size) << "Wrong file size for " << filename;
-    ASSERT_EQ(0, memcmp(content, actualContent, size));
+    ASSERT_EQ(0, memcmp(content, actualContent, size)) << "Wrong content for " << filename;
     g_free(actualContent);
 }

test/libpurple-mock.cpp

@@ -1052,6 +1052,14 @@ size_t purple_xfer_get_size(const PurpleXfer *xfer)
 gboolean
 purple_xfer_write_file(PurpleXfer *xfer, const guchar *buffer, gsize size)
 {
+    if (xfer->status != PURPLE_XFER_STATUS_STARTED) {
+        purple_debug_misc("purple_xfer_write_file", "write_file requires a transfer in progress\n");
+        purple_xfer_cancel_local(xfer);
+        return FALSE;
+    }
+    EXPECT_LE(xfer->bytes_sent + size, xfer->size);
+    xfer->bytes_sent += size;
+    EVENT(XferWriteFileEvent, xfer->local_filename, buffer, size);
     return TRUE;
 }
 

test/purple-events.cpp

@@ -276,6 +276,12 @@ static void compare(const XferRequestEvent &actual, const XferRequestEvent &expe
     COMPARE(filename);
 }
 
+static void compare(const XferWriteFileEvent &actual, const XferWriteFileEvent &expected)
+{
+    COMPARE(filename);
+    COMPARE(data);
+}
+
 static void compare(const RoomlistInProgressEvent &actual, const RoomlistInProgressEvent &expected)
 {
     COMPARE(list);
@@ -345,6 +351,7 @@ static void compareEvents(const PurpleEvent &actual, const PurpleEvent &expected
         C(XferLocalCancel)
         C(XferRemoteCancel)
         C(XferRequest)
+        C(XferWriteFile)
         C(RoomlistInProgress)
         C(RoomlistAddRoom)
         default:
@@ -491,6 +498,7 @@ std::string PurpleEvent::toString() const
     C(XferLocalCancel)
     C(XferRemoteCancel)
     C(XferRequest)
+    C(XferWriteFile)
     C(SetUserPhoto)
     C(RoomlistInProgress)
     C(RoomlistAddRoom)

test/purple-events.h

@@ -97,6 +97,7 @@ enum class PurpleEventType: uint8_t {
     XferLocalCancel,
     XferRemoteCancel,
     XferRequest,
+    XferWriteFile,
     SetUserPhoto,
     RoomlistInProgress,
     RoomlistAddRoom
@@ -507,6 +508,18 @@ struct XferRequestEvent: PurpleEvent {
       filename(filename ? filename : "") {}
 };
 
+struct XferWriteFileEvent: PurpleEvent {
+    std::string          filename;
+    std::vector<uint8_t> data;
+
+    XferWriteFileEvent(const char *filename, const uint8_t *content, unsigned size)
+    : PurpleEvent(PurpleEventType::XferWriteFile), filename(filename)
+    {
+        data.resize(size);
+        memmove(data.data(), content, size);
+    }
+};
+
 struct SetUserPhotoEvent: PurpleEvent {
     PurpleAccount *account;
     std::string    username;