From 72c368ea0edd13294e3dae1777c9ecd57e764c2f Mon Sep 17 00:00:00 2001 From: Ashod Nakashian Date: Sun, 31 Mar 2024 08:33:05 -0400 Subject: [PATCH] wsd: refactor the download logic in DocumentBroker Change-Id: Ic916a8373664f2b4de8d8649bf160effb1fe3dfa Signed-off-by: Ashod Nakashian --- wsd/DocumentBroker.cpp | 230 +++++++++++++++++------------------------ wsd/DocumentBroker.hpp | 7 ++ 2 files changed, 102 insertions(+), 135 deletions(-) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 44ee1fbab..9b15924b9 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -32,6 +32,7 @@ #include #include "Admin.hpp" +#include "Authorization.hpp" #include "ClientSession.hpp" #include "Exceptions.hpp" #include "COOLWSD.hpp" @@ -881,70 +882,13 @@ bool DocumentBroker::downloadAdvance(const std::string& jailId, const Poco::URI& std::chrono::milliseconds getFileCallDurationMs = std::chrono::milliseconds::zero(); if (!_storage->isDownloaded()) { - LOG_DBG("Download file for docKey [" << _docKey << ']'); - std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); - std::string localPath = _storage->downloadStorageFileToLocal( - Authorization::create(uriPublic), *_lockCtx, templateSource); - if (localPath.empty()) + const Authorization auth = Authorization::create(uriPublic); + if (!doDownloadDocument(/*session=*/nullptr, auth, templateSource, fileInfo.getFilename(), + getFileCallDurationMs)) { - throw std::runtime_error("Failed to retrieve document from storage"); - } - - getFileCallDurationMs = std::chrono::duration_cast( - std::chrono::steady_clock::now() - start); - - _docState.setStatus(DocumentState::Status::Loading); // Done downloading. - -#if !MOBILEAPP - if (!processPlugins(localPath)) - { - // FIXME: Why don't we resume anyway? - LOG_WRN("Failed to process plugins on file [" << localPath << ']'); + LOG_DBG("Failed to download or process downloaded document"); return false; } -#endif //!MOBILEAPP - - const std::string localFilePath = Poco::Path(getJailRoot(), localPath).toString(); - std::ifstream istr(localFilePath, std::ios::binary); - Poco::SHA1Engine sha1; - Poco::DigestOutputStream dos(sha1); - Poco::StreamCopier::copyStream(istr, dos); - dos.close(); - LOG_INF("SHA1 for DocKey [" << _docKey << "] of [" << COOLWSD::anonymizeUrl(localPath) - << "]: " << Poco::DigestEngine::digestToHex(sha1.digest())); - - std::string localPathEncoded; - Poco::URI::encode(localPath, "#?", localPathEncoded); - _uriJailed = Poco::URI(Poco::URI("file://"), localPathEncoded).toString(); - _uriJailedAnonym = - Poco::URI(Poco::URI("file://"), COOLWSD::anonymizeUrl(localPathEncoded)).toString(); - - _filename = fileInfo.getFilename(); -#if !MOBILEAPP - _quarantine = std::make_unique(*this, _filename); -#endif - - if (!templateSource.empty()) - { - // Invalid timestamp for templates, to force uploading once we save-after-loading. - _saveManager.setLastModifiedTime(std::chrono::system_clock::time_point()); - _storageManager.setLastUploadedFileModifiedTime( - std::chrono::system_clock::time_point()); - } - else - { - // Use the local temp file's timestamp. - const auto timepoint = FileUtil::Stat(localFilePath).modifiedTimepoint(); - _saveManager.setLastModifiedTime(timepoint); - _storageManager.setLastUploadedFileModifiedTime( - timepoint); // Used to detect modifications. - } - - bool dontUseCache = Util::isMobileApp(); - - _tileCache = std::make_unique(_storage->getUri().toString(), - _saveManager.getLastModifiedTime(), dontUseCache); - _tileCache->setThreadOwner(std::this_thread::get_id()); } #if !MOBILEAPP @@ -1147,83 +1091,14 @@ bool DocumentBroker::download( std::chrono::milliseconds getFileCallDurationMs = std::chrono::milliseconds::zero(); if (!_storage->isDownloaded()) { - LOG_DBG("Download file for docKey [" << _docKey << ']'); - std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); - std::string localPath = _storage->downloadStorageFileToLocal(session->getAuthorization(), - *_lockCtx, templateSource); - if (localPath.empty()) + const Authorization auth = + session ? session->getAuthorization() : Authorization::create(uriPublic); + if (!doDownloadDocument(session, auth, templateSource, fileInfo.getFilename(), + getFileCallDurationMs)) { - throw std::runtime_error("Failed to retrieve document from storage"); - } - - getFileCallDurationMs = std::chrono::duration_cast( - std::chrono::steady_clock::now() - start); - - _docState.setStatus(DocumentState::Status::Loading); // Done downloading. - - // Only lock the document on storage for editing sessions - // FIXME: why not lock before downloadStorageFileToLocal? Would also prevent race conditions - if (!session->isReadOnly()) - { - std::string error; - if (!updateStorageLockState(*session, /*lock=*/true, error)) - { - LOG_ERR("Failed to lock docKey [" << _docKey << "] with session [" - << session->getId() - << "] after downloading: " << error); - } - } - -#if !MOBILEAPP - if (!processPlugins(localPath)) - { - // FIXME: Why don't we resume anyway? - LOG_WRN("Failed to process plugins on file [" << localPath << ']'); + LOG_DBG("Failed to download or process downloaded document"); return false; } -#endif //!MOBILEAPP - - const std::string localFilePath = Poco::Path(getJailRoot(), localPath).toString(); - std::ifstream istr(localFilePath, std::ios::binary); - Poco::SHA1Engine sha1; - Poco::DigestOutputStream dos(sha1); - Poco::StreamCopier::copyStream(istr, dos); - dos.close(); - LOG_INF("SHA1 for DocKey [" << _docKey << "] of [" << COOLWSD::anonymizeUrl(localPath) - << "]: " << Poco::DigestEngine::digestToHex(sha1.digest())); - - std::string localPathEncoded; - Poco::URI::encode(localPath, "#?", localPathEncoded); - _uriJailed = Poco::URI(Poco::URI("file://"), localPathEncoded).toString(); - _uriJailedAnonym = - Poco::URI(Poco::URI("file://"), COOLWSD::anonymizeUrl(localPathEncoded)).toString(); - - _filename = fileInfo.getFilename(); -#if !MOBILEAPP - _quarantine = std::make_unique(*this, _filename); -#endif - - if (!templateSource.empty()) - { - // Invalid timestamp for templates, to force uploading once we save-after-loading. - _saveManager.setLastModifiedTime(std::chrono::system_clock::time_point()); - _storageManager.setLastUploadedFileModifiedTime( - std::chrono::system_clock::time_point()); - } - else - { - // Use the local temp file's timestamp. - const auto timepoint = FileUtil::Stat(localFilePath).modifiedTimepoint(); - _saveManager.setLastModifiedTime(timepoint); - _storageManager.setLastUploadedFileModifiedTime( - timepoint); // Used to detect modifications. - } - - bool dontUseCache = Util::isMobileApp(); - - _tileCache = std::make_unique(_storage->getUri().toString(), - _saveManager.getLastModifiedTime(), dontUseCache); - _tileCache->setThreadOwner(std::this_thread::get_id()); } #if !MOBILEAPP @@ -1244,6 +1119,91 @@ bool DocumentBroker::download( return true; } +bool DocumentBroker::doDownloadDocument(const std::shared_ptr& session, + const Authorization& auth, + const std::string& templateSource, + const std::string& filename, + std::chrono::milliseconds& getFileCallDurationMs) +{ + assert(_storage && !_storage->isDownloaded()); + + LOG_DBG("Download file for docKey [" << _docKey << ']'); + std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); + std::string localPath = _storage->downloadStorageFileToLocal(auth, *_lockCtx, templateSource); + if (localPath.empty()) + { + throw std::runtime_error("Failed to retrieve document from storage"); + } + + getFileCallDurationMs = std::chrono::duration_cast( + std::chrono::steady_clock::now() - start); + + _docState.setStatus(DocumentState::Status::Loading); // Done downloading. + + // Only lock the document on storage for editing sessions + // FIXME: why not lock before downloadStorageFileToLocal? Would also prevent race conditions + if (session && !session->isReadOnly()) + { + std::string error; + if (!updateStorageLockState(*session, /*lock=*/true, error)) + { + LOG_ERR("Failed to lock docKey [" << _docKey << "] with session [" << session->getId() + << "] after downloading: " << error); + } + } + +#if !MOBILEAPP + if (!processPlugins(localPath)) + { + // FIXME: Why don't we resume anyway? + LOG_WRN("Failed to process plugins on file [" << localPath << ']'); + return false; + } +#endif //!MOBILEAPP + + const std::string localFilePath = Poco::Path(getJailRoot(), localPath).toString(); + std::ifstream istr(localFilePath, std::ios::binary); + Poco::SHA1Engine sha1; + Poco::DigestOutputStream dos(sha1); + Poco::StreamCopier::copyStream(istr, dos); + dos.close(); + LOG_INF("SHA1 for DocKey [" << _docKey << "] of [" << COOLWSD::anonymizeUrl(localPath) + << "]: " << Poco::DigestEngine::digestToHex(sha1.digest())); + + std::string localPathEncoded; + Poco::URI::encode(localPath, "#?", localPathEncoded); + _uriJailed = Poco::URI(Poco::URI("file://"), localPathEncoded).toString(); + _uriJailedAnonym = + Poco::URI(Poco::URI("file://"), COOLWSD::anonymizeUrl(localPathEncoded)).toString(); + + _filename = filename; +#if !MOBILEAPP + _quarantine = std::make_unique(*this, _filename); +#endif + + if (!templateSource.empty()) + { + // Invalid timestamp for templates, to force uploading once we save-after-loading. + _saveManager.setLastModifiedTime(std::chrono::system_clock::time_point()); + _storageManager.setLastUploadedFileModifiedTime(std::chrono::system_clock::time_point()); + } + else + { + // Use the local temp file's timestamp. + const auto timepoint = FileUtil::Stat(localFilePath).modifiedTimepoint(); + _saveManager.setLastModifiedTime(timepoint); + _storageManager.setLastUploadedFileModifiedTime(timepoint); // Used to detect modifications. + } + + const bool dontUseCache = Util::isMobileApp(); + + _tileCache = std::make_unique(_storage->getUri().toString(), + _saveManager.getLastModifiedTime(), dontUseCache); + _tileCache->setThreadOwner(std::this_thread::get_id()); + + return true; +} + #if !MOBILEAPP std::string DocumentBroker::updateSessionWithWopiInfo(const std::shared_ptr& session, diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index fee617f6b..337b3fc73 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -594,6 +594,13 @@ private: bool download(const std::shared_ptr& session, const std::string& jailId, std::unique_ptr wopiFileInfo); + /// Actual document download and post-download processing. + /// Must be called only when creating the storage for the first time. + bool doDownloadDocument(const std::shared_ptr& session, + const Authorization& auth, const std::string& templateSource, + const std::string& filename, + std::chrono::milliseconds& getFileCallDurationMs); + #if !MOBILEAPP /// Updates the Session with the wopiFileInfo given. /// Returns the templateSource, if any.