From bb8594f830c235399c61f63d05c39a4c099b832a Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Wed, 10 Jan 2018 15:35:49 -0500
Subject: [PATCH] datatransfer: fix bytes progress info for reception
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* modifying the API name of bytes progression
* add bytes progression reporting for incoming file transfer
* D-Bus binding adapted

Change-Id: Ife397ce7d13f248ed1df4338e62361ddbc9e2027
Reviewed-by: Anthony Léonard <anthony.leonard@savoirfairelinux.com>
---
 .../cx.ring.Ring.ConfigurationManager.xml     |  2 +-
 bin/dbus/dbusconfigurationmanager.cpp         |  4 +-
 bin/dbus/dbusconfigurationmanager.h           |  2 +-
 src/client/datatransfer.cpp                   |  4 +-
 src/data_transfer.cpp                         | 41 ++++++++++++++-----
 src/data_transfer.h                           |  8 ++--
 src/dring/datatransfer_interface.h            |  4 +-
 src/ftp_server.cpp                            |  6 ++-
 src/ftp_server.h                              |  3 +-
 9 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
index b4f1d3b9fa..b07c3ce775 100644
--- a/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
+++ b/bin/dbus/cx.ring.Ring.ConfigurationManager.xml
@@ -1426,7 +1426,7 @@
            </arg>
        </method>
 
-       <method name="dataTransferBytesSent" tp:name-for-bindings="dataTransferBytesSent">
+       <method name="dataTransferBytesProgress" tp:name-for-bindings="dataTransferBytesProgress">
            <tp:added version="4.2.0"/>
            <arg type="t" name="BytesProgress" direction="out">
            </arg>
diff --git a/bin/dbus/dbusconfigurationmanager.cpp b/bin/dbus/dbusconfigurationmanager.cpp
index 2c4c36fce7..13acf8fbf4 100644
--- a/bin/dbus/dbusconfigurationmanager.cpp
+++ b/bin/dbus/dbusconfigurationmanager.cpp
@@ -636,9 +636,9 @@ DBusConfigurationManager::dataTransferInfo(const DRing::DataTransferId& id)
 }
 
 uint64_t
-DBusConfigurationManager::dataTransferBytesSent(const uint64_t& id)
+DBusConfigurationManager::dataTransferBytesProgress(const uint64_t& id)
 {
-    return DRing::dataTransferBytesSent(id);
+    return DRing::dataTransferBytesProgress(id);
 }
 
 auto
diff --git a/bin/dbus/dbusconfigurationmanager.h b/bin/dbus/dbusconfigurationmanager.h
index a14fba54c6..4ef1f7be8f 100644
--- a/bin/dbus/dbusconfigurationmanager.h
+++ b/bin/dbus/dbusconfigurationmanager.h
@@ -156,7 +156,7 @@ class DBusConfigurationManager :
         DRing::DataTransferId sendFile(const std::string& account_id, const std::string& peer_uri,
                                        const std::string& file_path, const std::string& display_name);
         DBus::Struct<bool, uint32_t, uint64_t, uint64_t, std::string, std::string> dataTransferInfo(const DRing::DataTransferId& id);
-        uint64_t dataTransferBytesSent(const uint64_t& id);
+        uint64_t dataTransferBytesProgress(const uint64_t& id);
         std::vector<uint64_t> dataTransferList();
         void acceptFileTransfer(const uint64_t& id, const std::string& file_path, const uint64_t& offset);
         void cancelDataTransfer(const uint64_t& id);
diff --git a/src/client/datatransfer.cpp b/src/client/datatransfer.cpp
index 7b44e56de7..3c9e234153 100644
--- a/src/client/datatransfer.cpp
+++ b/src/client/datatransfer.cpp
@@ -72,9 +72,9 @@ cancelDataTransfer(const DataTransferId& id)
 }
 
 std::streamsize
-dataTransferBytesSent(const DataTransferId& id)
+dataTransferBytesProgress(const DataTransferId& id)
 {
-    return ring::Manager::instance().dataTransfers->bytesSent(id);
+    return ring::Manager::instance().dataTransfers->bytesProgress(id);
 }
 
 DataTransferInfo
diff --git a/src/data_transfer.cpp b/src/data_transfer.cpp
index 3a1f70ccc7..00edb481f9 100644
--- a/src/data_transfer.cpp
+++ b/src/data_transfer.cpp
@@ -68,12 +68,13 @@ public:
         return started_.compare_exchange_strong(expected, true);
     }
 
-    virtual std::streamsize bytesSent() const {
+    virtual std::streamsize bytesProgress() const {
         std::lock_guard<std::mutex> lk {infoMutex_};
         return info_.bytesProgress;
     }
 
     DRing::DataTransferInfo info() const {
+        bytesProgress();
         std::lock_guard<std::mutex> lk {infoMutex_};
         return info_;
     }
@@ -204,12 +205,15 @@ FileTransfer::read(std::vector<uint8_t>& buf) const
 class IncomingFileTransfer final : public DataTransfer
 {
 public:
-    IncomingFileTransfer(DRing::DataTransferId id, const std::string&, std::size_t, std::size_t);
+    IncomingFileTransfer(DRing::DataTransferId, const std::string&, std::size_t, std::size_t,
+                         std::atomic<std::size_t>&);
 
     bool start() override;
 
     void close() noexcept override;
 
+    std::streamsize bytesProgress() const override;
+
     std::string requestFilename();
 
     void accept(const std::string&, std::size_t offset) override;
@@ -217,14 +221,17 @@ public:
 private:
     IncomingFileTransfer() = delete;
 
+    std::atomic<std::size_t>& progressStorage_;
     std::promise<void> filenamePromise_;
 };
 
 IncomingFileTransfer::IncomingFileTransfer(DRing::DataTransferId id,
                                            const std::string& display_name,
                                            std::size_t total_size,
-                                           std::size_t offset)
+                                           std::size_t offset,
+                                           std::atomic<std::size_t>& progress_storage)
     : DataTransfer(id)
+    , progressStorage_ {progress_storage}
 {
     RING_WARN() << "[FTP] incoming transfert of " << total_size << " byte(s): " << display_name;
     (void)offset;
@@ -237,6 +244,14 @@ IncomingFileTransfer::IncomingFileTransfer(DRing::DataTransferId id,
     emit(DRing::DataTransferEventCode::created);
 }
 
+std::streamsize
+IncomingFileTransfer::bytesProgress() const
+{
+    std::lock_guard<std::mutex> lk {infoMutex_};
+    info_.bytesProgress = progressStorage_.load();
+    return info_.bytesProgress;
+}
+
 std::string
 IncomingFileTransfer::requestFilename()
 {
@@ -294,7 +309,8 @@ public:
                                                      const std::string& display_name);
     std::shared_ptr<IncomingFileTransfer> createIncomingFileTransfer(const std::string& display_name,
                                                                      std::size_t total_size,
-                                                                     std::size_t offset);
+                                                                     std::size_t offset,
+                                                                     std::atomic<std::size_t>& progress_storage);
 
     std::shared_ptr<DataTransfer> getTransfer(const DRing::DataTransferId& id);
 
@@ -333,10 +349,12 @@ DataTransferFacade::Impl::createFileTransfer(const std::string& file_path,
 std::shared_ptr<IncomingFileTransfer>
 DataTransferFacade::Impl::createIncomingFileTransfer(const std::string& display_name,
                                                      std::size_t total_size,
-                                                     std::size_t offset)
+                                                     std::size_t offset,
+                                                     std::atomic<std::size_t>& progress_storage)
 {
     auto id = generateUID();
-    auto transfer = std::make_shared<IncomingFileTransfer>(id, display_name, total_size, offset);
+    auto transfer = std::make_shared<IncomingFileTransfer>(id, display_name, total_size, offset,
+                                                           progress_storage);
     std::lock_guard<std::mutex> lk {mapMutex_};
     map_.emplace(id, transfer);
     return transfer;
@@ -425,10 +443,10 @@ DataTransferFacade::cancel(const DRing::DataTransferId& id)
 }
 
 std::streamsize
-DataTransferFacade::bytesSent(const DRing::DataTransferId& id) const
+DataTransferFacade::bytesProgress(const DRing::DataTransferId& id) const
 {
     if (auto transfer = pimpl_->getTransfer(id))
-        return transfer->bytesSent();
+        return transfer->bytesProgress();
     throw std::invalid_argument("not existing DataTransferId");
 }
 
@@ -441,9 +459,12 @@ DataTransferFacade::info(const DRing::DataTransferId& id) const
 }
 
 std::string
-DataTransferFacade::onIncomingFileRequest(const std::string& display_name, std::size_t total_size, std::size_t offset)
+DataTransferFacade::onIncomingFileRequest(const std::string& display_name,
+                                          std::size_t total_size,
+                                          std::size_t offset,
+                                          std::atomic<std::size_t>& progress_storage)
 {
-    auto transfer = pimpl_->createIncomingFileTransfer(display_name, total_size, offset);
+    auto transfer = pimpl_->createIncomingFileTransfer(display_name, total_size, offset, progress_storage);
     auto filename = transfer->requestFilename();
     if (!filename.empty())
         transfer->start(); // TODO: bad place, call only if file can be open
diff --git a/src/data_transfer.h b/src/data_transfer.h
index 5c5f40213a..4c9a47a503 100644
--- a/src/data_transfer.h
+++ b/src/data_transfer.h
@@ -63,14 +63,16 @@ public:
     /// \return a copy of all information about a data transfer
     DRing::DataTransferInfo info(const DRing::DataTransferId& id) const;
 
-    /// \return number of bytes sent by a data transfer
+    /// \return number of bytes sent/received by a data transfer
     /// \note this method is fatest than info()
-    std::streamsize bytesSent(const DRing::DataTransferId& id) const;
+    std::streamsize bytesProgress(const DRing::DataTransferId& id) const;
 
     /// Create an IncomingFileTransfer object.
     /// \return a filename to open where incoming data will be written or an empty string
     ///         in case of refusal.
-    std::string onIncomingFileRequest(const std::string& display_name, std::size_t total_size, std::size_t offset);
+    std::string onIncomingFileRequest(const std::string& display_name, std::size_t total_size,
+                                      std::size_t offset,
+                                      std::atomic<std::size_t>& progress_storage);
 
 private:
     class Impl;
diff --git a/src/dring/datatransfer_interface.h b/src/dring/datatransfer_interface.h
index 238b353fc0..b47ec6f3b5 100644
--- a/src/dring/datatransfer_interface.h
+++ b/src/dring/datatransfer_interface.h
@@ -118,13 +118,13 @@ void cancelDataTransfer(const DataTransferId& id);
 ///
 DataTransferInfo dataTransferInfo(const DataTransferId& id);
 
-/// Return the amount of sent bytes of an existing data transfer.
+/// Return the amount of sent/received bytes of an existing data transfer.
 ///
 /// \param id data transfer identification value as given by a DataTransferEvent signal.
 ///
 /// \return number of successfuly transfered bytes.
 ///
-std::streamsize dataTransferBytesSent(const DataTransferId& id);
+std::streamsize dataTransferBytesProgress(const DataTransferId& id);
 
 // Signal handlers registration
 void registerDataXferHandlers(const std::map<std::string, std::shared_ptr<CallbackWrapperBase>>&);
diff --git a/src/ftp_server.cpp b/src/ftp_server.cpp
index 78171640b1..7d9ee8bba3 100644
--- a/src/ftp_server.cpp
+++ b/src/ftp_server.cpp
@@ -56,7 +56,10 @@ bool
 FtpServer::startNewFile()
 {
     // Request filename from client (WARNING: synchrone call!)
-    auto filename = Manager::instance().dataTransfers->onIncomingFileRequest(displayName_, fileSize_, 0 /* TODO: offset */);
+    auto filename = Manager::instance().dataTransfers->onIncomingFileRequest(displayName_,
+                                                                             fileSize_,
+                                                                             0 /* TODO: offset */,
+                                                                             rx_);
     if (filename.empty())
         return false;
 
@@ -72,7 +75,6 @@ FtpServer::closeCurrentFile()
 {
     out_.close();
     RING_WARN() << "[FTP] File received, " << rx_ << " byte(s)";
-    rx_ = fileSize_ = 0;
 }
 
 bool
diff --git a/src/ftp_server.h b/src/ftp_server.h
index d2d9c1dcad..6313544a90 100644
--- a/src/ftp_server.h
+++ b/src/ftp_server.h
@@ -26,6 +26,7 @@
 #include <array>
 #include <fstream>
 #include <sstream>
+#include <atomic>
 
 namespace ring {
 
@@ -52,7 +53,7 @@ private:
 
     std::ofstream out_;
     std::size_t fileSize_ {0};
-    std::size_t rx_ {0};
+    std::atomic<std::size_t> rx_ {0};
     std::stringstream headerStream_;
     std::string displayName_;
     std::array<char, 1000> line_;
-- 
GitLab