~aleteoryx/muditaos: [CP-599] Add crc hash to the last chunk of sent file

6 files changed, 52 insertions(+), 91 deletions(-)

M module-services/service-desktop/endpoints/filesystem/FileContext.cpp
M module-services/service-desktop/endpoints/filesystem/FileOperations.cpp
M module-services/service-desktop/endpoints/filesystem/FilesystemEndpoint.cpp
M module-services/service-desktop/endpoints/include/endpoints/filesystem/FileContext.hpp
M module-services/service-desktop/endpoints/include/endpoints/filesystem/FileOperations.hpp
M test/harness

M module-services/service-desktop/endpoints/filesystem/FileContext.cpp => module-services/service-desktop/endpoints/filesystem/FileContext.cpp +18 -42

@@ 3,9 3,13 @@
 
 #include <endpoints/filesystem/FileContext.hpp>
 #include <log/log.hpp>
+#include <utility>
 
-FileContext::FileContext(
-    std::filesystem::path path, std::size_t size, std::size_t chunkSize, std::string openMode, std::size_t offset)
+FileContext::FileContext(const std::filesystem::path &path,
+                         std::size_t size,
+                         std::size_t chunkSize,
+                         const std::string &openMode,
+                         std::size_t offset)
     : path(path), size(size), offset(offset), chunkSize(chunkSize)
 {
     if (!size || !chunkSize) {


@@ 29,7 33,7 @@ FileContext::~FileContext()
     std::fclose(file);
 }
 
-FileReadContext::FileReadContext(std::filesystem::path path,
+FileReadContext::FileReadContext(const std::filesystem::path &path,
                                  std::size_t size,
                                  std::size_t chunkSize,
                                  std::size_t offset)


@@ 39,9 43,12 @@ FileReadContext::FileReadContext(std::filesystem::path path,
 FileReadContext::~FileReadContext()
 {}
 
-FileWriteContext::FileWriteContext(
-    std::filesystem::path path, std::size_t size, std::size_t chunkSize, std::string crc32Digest, std::size_t offset)
-    : FileContext(path, size, chunkSize, "wb", offset), crc32Digest(crc32Digest)
+FileWriteContext::FileWriteContext(const std::filesystem::path &path,
+                                   std::size_t size,
+                                   std::size_t chunkSize,
+                                   std::string crc32Digest,
+                                   std::size_t offset)
+    : FileContext(path, size, chunkSize, "wb", offset), crc32Digest(std::move(crc32Digest))
 {}
 
 FileWriteContext::~FileWriteContext()


@@ 77,34 84,9 @@ auto FileContext::validateChunkRequest(std::uint32_t chunkNo) const -> bool
     return !(chunkNo < 1 || chunkNo > totalChunksInFile() || chunkNo != expectedChunkInFile());
 }
 
-auto FileReadContext::getFileHash() -> std::string
+auto FileContext::fileHash() const -> std::string
 {
-    std::vector<std::uint8_t> buffer(chunkSize);
-
-    auto dataSize = size;
-
-    while (dataSize) {
-
-        auto dataLeft = std::min(static_cast<std::size_t>(chunkSize), dataSize);
-        auto dataRead = std::fread(buffer.data(), sizeof(int8_t), dataLeft, file);
-
-        if (dataRead != dataLeft) {
-            LOG_ERROR("File %s read error", path.c_str());
-            throw std::runtime_error("File read error");
-        }
-
-        runningCrc32Digest.add(buffer.data(), dataRead);
-
-        dataSize -= dataRead;
-    }
-
-    std::fseek(file, 0, SEEK_SET);
-
-    auto fileHash = runningCrc32Digest.getHash();
-
-    runningCrc32Digest.reset();
-
-    return fileHash;
+    return runningCrc32Digest.getHash();
 }
 
 auto FileReadContext::read() -> std::vector<std::uint8_t>


@@ 124,8 106,9 @@ auto FileReadContext::read() -> std::vector<std::uint8_t>
         throw std::runtime_error("File read error");
     }
 
-    LOG_DEBUG("Read %u bytes", static_cast<unsigned int>(dataRead));
+    runningCrc32Digest.add(buffer.data(), dataRead);
 
+    LOG_DEBUG("Read %u bytes", static_cast<unsigned int>(dataRead));
     advanceFileOffset(dataRead);
 
     if (reachedEOF()) {


@@ 159,19 142,12 @@ auto FileWriteContext::write(const std::vector<std::uint8_t> &data) -> void
     if (reachedEOF()) {
         LOG_INFO("Reached EOF of %s", path.c_str());
     }
-
-    return;
-}
-
-auto FileWriteContext::fileHash() const -> std::string
-{
-    return runningCrc32Digest.getHash();
 }
 
 auto FileWriteContext::crc32Matches() const -> bool
 {
     LOG_DEBUG("Hash: %s", fileHash().c_str());
-    return crc32Digest.compare(fileHash()) == 0;
+    return crc32Digest == fileHash();
 }
 
 auto FileWriteContext::removeFile() -> void

M module-services/service-desktop/endpoints/filesystem/FileOperations.cpp => module-services/service-desktop/endpoints/filesystem/FileOperations.cpp +6 -26

@@ 75,7 75,7 @@ auto FileOperations::createFileReadContextFor(const std::filesystem::path &file,
 
 auto FileOperations::createFileWriteContextFor(const std::filesystem::path &file,
                                                std::size_t fileSize,
-                                               const std::string Crc32,
+                                               const std::string &Crc32,
                                                transfer_id xfrId) -> void
 {
     writeTransfers.insert(


@@ 120,32 120,12 @@ auto FileOperations::decodeDataFromBase64(const std::string &encodedData) const 
     return decodedData;
 }
 
-auto FileOperations::getFileHashForReceiveID(transfer_id rxID) -> std::string
-{
-    LOG_DEBUG("Getting hash for rxID %u", static_cast<unsigned>(rxID));
-
-    const auto fileCtxEntry = readTransfers.find(rxID);
-
-    if (fileCtxEntry == readTransfers.end()) {
-        LOG_ERROR("Invalid rxID %u", static_cast<unsigned>(rxID));
-        return {};
-    }
-
-    auto fileCtx = fileCtxEntry->second.get();
-
-    if (!fileCtx) {
-        LOG_ERROR("Invalid fileCtx for rxID %u", static_cast<unsigned>(rxID));
-        return {};
-    }
-
-    return fileCtx->getFileHash();
-}
-
-auto FileOperations::getDataForReceiveID(transfer_id rxID, std::uint32_t chunkNo) -> std::string
+auto FileOperations::getDataForReceiveID(transfer_id rxID, std::uint32_t chunkNo) -> DataWithCrc32
 {
     LOG_DEBUG("Getting chunk %u for rxID %u", static_cast<unsigned>(chunkNo), static_cast<unsigned>(rxID));
 
     const auto fileCtxEntry = readTransfers.find(rxID);
+    std::string fileCrc32   = {};
 
     if (fileCtxEntry == readTransfers.end()) {
         LOG_ERROR("Invalid rxID %u", static_cast<unsigned>(rxID));


@@ 173,16 153,16 @@ auto FileOperations::getDataForReceiveID(transfer_id rxID, std::uint32_t chunkNo
 
     if (fileCtx->reachedEOF()) {
         LOG_INFO("Reached EOF for rxID %u", static_cast<unsigned>(rxID));
-
+        fileCrc32 = fileCtx->fileHash();
         writeTransfers.erase(rxID);
     }
 
-    return encodeDataAsBase64(data);
+    return {std::move(encodeDataAsBase64(data)), std::move(fileCrc32)};
 }
 
 auto FileOperations::createTransmitIDForFile(const std::filesystem::path &file,
                                              std::size_t size,
-                                             const std::string Crc32) -> transfer_id
+                                             const std::string &Crc32) -> transfer_id
 {
     cancelTimedOutWriteTransfer();
     const auto txID = ++runningXfrId;

M module-services/service-desktop/endpoints/filesystem/FilesystemEndpoint.cpp => module-services/service-desktop/endpoints/filesystem/FilesystemEndpoint.cpp +12 -9

@@ 75,13 75,10 @@ namespace sdesktop::endpoints
         try {
             auto [rxID, fileSize] = fileOps.createReceiveIDForFile(filePath);
 
-            auto fileHash = fileOps.getFileHashForReceiveID(rxID);
-
             context.setResponseStatus(http::Code::OK);
             context.setResponseBody(
                 json11::Json::object({{json::filesystem::rxID, static_cast<int>(rxID)},
                                       {json::fileSize, static_cast<int>(fileSize)},
-                                      {json::fileCrc32, fileHash},
                                       {json::filesystem::chunkSize, static_cast<int>(FileOperations::ChunkSize)}}));
         }
         catch (std::runtime_error &e) {


@@ 107,10 104,10 @@ namespace sdesktop::endpoints
         auto returnCode    = sys::ReturnCodes::Failure;
         const auto rxID    = context.getBody()[json::filesystem::rxID].int_value();
         const auto chunkNo = context.getBody()[json::filesystem::chunkNo].int_value();
-        std::string data{};
+        FileOperations::DataWithCrc32 dataWithCrc32;
 
         try {
-            data = fileOps.getDataForReceiveID(rxID, chunkNo);
+            dataWithCrc32 = fileOps.getDataForReceiveID(rxID, chunkNo);
         }
         catch (std::exception &e) {
             LOG_ERROR("%s", e.what());


@@ 120,11 117,17 @@ namespace sdesktop::endpoints
             return sys::ReturnCodes::Failure;
         }
 
-        if (data.length()) {
+        if (dataWithCrc32.data.length()) {
             context.setResponseStatus(http::Code::OK);
-            context.setResponseBody(json11::Json::object({{json::filesystem::rxID, static_cast<int>(rxID)},
-                                                          {json::filesystem::chunkNo, static_cast<int>(chunkNo)},
-                                                          {json::filesystem::data, data}}));
+            json11::Json::object contextJsonObject =
+                json11::Json::object({{json::filesystem::rxID, static_cast<int>(rxID)},
+                                      {json::filesystem::chunkNo, static_cast<int>(chunkNo)},
+                                      {json::filesystem::data, dataWithCrc32.data}});
+
+            if (dataWithCrc32.crc32.length()) {
+                contextJsonObject[json::fileCrc32] = dataWithCrc32.crc32;
+            }
+            context.setResponseBody(contextJsonObject);
 
             returnCode = sys::ReturnCodes::Success;
         }

M module-services/service-desktop/endpoints/include/endpoints/filesystem/FileContext.hpp => module-services/service-desktop/endpoints/include/endpoints/filesystem/FileContext.hpp +6 -8

@@ 16,10 16,10 @@
 class FileContext
 {
   public:
-    explicit FileContext(std::filesystem::path path,
+    explicit FileContext(const std::filesystem::path &path,
                          std::size_t size,
                          std::size_t chunkSize,
-                         std::string openMode,
+                         const std::string &openMode,
                          std::size_t offset = 0);
 
     virtual ~FileContext();


@@ 36,6 36,8 @@ class FileContext
 
     auto expectedChunkInFile() const -> std::size_t;
 
+    auto fileHash() const -> std::string;
+
   protected:
     std::filesystem::path path{};
     std::FILE *file{};


@@ 49,22 51,20 @@ class FileContext
 class FileReadContext : public FileContext
 {
   public:
-    explicit FileReadContext(std::filesystem::path path,
+    explicit FileReadContext(const std::filesystem::path &path,
                              std::size_t size,
                              std::size_t chunkSize,
                              std::size_t offset = 0);
 
     ~FileReadContext();
 
-    auto getFileHash() -> std::string;
-
     auto read() -> std::vector<std::uint8_t>;
 };
 
 class FileWriteContext : public FileContext
 {
   public:
-    explicit FileWriteContext(std::filesystem::path path,
+    explicit FileWriteContext(const std::filesystem::path &path,
                               std::size_t size,
                               std::size_t chunkSize,
                               std::string crc32Digest,


@@ 72,8 72,6 @@ class FileWriteContext : public FileContext
 
     ~FileWriteContext();
 
-    auto fileHash() const -> std::string;
-
     auto crc32Matches() const -> bool;
 
     auto removeFile() -> void;

M module-services/service-desktop/endpoints/include/endpoints/filesystem/FileOperations.hpp => module-services/service-desktop/endpoints/include/endpoints/filesystem/FileOperations.hpp +9 -5

@@ 29,7 29,7 @@ class FileOperations
 
     auto createFileWriteContextFor(const std::filesystem::path &file,
                                    std::size_t fileSize,
-                                   const std::string Crc32,
+                                   const std::string &Crc32,
                                    transfer_id xfrId) -> void;
 
     auto encodeDataAsBase64(const std::vector<std::uint8_t> &binaryData) const -> std::string;


@@ 53,15 53,19 @@ class FileOperations
     static constexpr auto ChunkSizeMultiplier = 12u;
     static constexpr auto ChunkSize           = ChunkSizeMultiplier * SingleChunkSize;
 
+    struct DataWithCrc32
+    {
+        std::string data;
+        std::string crc32;
+    };
+
     static FileOperations &instance();
 
     auto createReceiveIDForFile(const std::filesystem::path &file) -> std::pair<transfer_id, std::size_t>;
 
-    auto getDataForReceiveID(transfer_id, std::uint32_t chunkNo) -> std::string;
-
-    auto getFileHashForReceiveID(transfer_id rxID) -> std::string;
+    auto getDataForReceiveID(transfer_id, std::uint32_t chunkNo) -> DataWithCrc32;
 
-    auto createTransmitIDForFile(const std::filesystem::path &file, std::size_t size, const std::string Crc32)
+    auto createTransmitIDForFile(const std::filesystem::path &file, std::size_t size, const std::string &Crc32)
         -> transfer_id;
 
     auto sendDataForTransmitID(transfer_id, std::uint32_t chunkNo, const std::string &data) -> sys::ReturnCodes;

M test/harness => test/harness +1 -1

@@ 1,1 1,1 @@
-Subproject commit 109327d1a7a73afad9f741e3c17d1e161e8a9c01
+Subproject commit 8a33fa2634c6e24b9099e23404ab5104678ac426