[Libreoffice-commits] online.git: 2 commits - loolwsd/DocumentBroker.cpp loolwsd/LOOLWSD.cpp loolwsd/protocol.txt loolwsd/Storage.cpp loolwsd/Storage.hpp

Pranav Kant pranavk at collabora.co.uk
Mon Oct 24 10:35:20 UTC 2016


 loolwsd/DocumentBroker.cpp |   42 ++++++++++++++++++++----------------------
 loolwsd/LOOLWSD.cpp        |    9 ---------
 loolwsd/Storage.cpp        |    6 +++---
 loolwsd/Storage.hpp        |   13 +++++++++----
 loolwsd/protocol.txt       |    2 +-
 5 files changed, 33 insertions(+), 39 deletions(-)

New commits:
commit 36fece8b07097d8e155f632abdf6c0f907550502
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Sun Oct 23 17:35:46 2016 +0530

    loolwsd: Separate WOPI load duration and check fileinfo duration
    
    Add both of them and send them as wopiloadduration to the client,
    if storage is WOPI.
    
    Change-Id: I5652d346d70f473624f03536469acf45470db742

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index eeb3fd7..3a5e1be 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -237,19 +237,29 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
         }
 
         // Lets load the document now
-        if (_storage->isLoaded())
+        const bool loaded = _storage->isLoaded();
+        if (!loaded)
         {
-            // Already loaded. Nothing to do.
-            return true;
-        }
+            const auto localPath = _storage->loadStorageFileToLocal();
+            _uriJailed = Poco::URI(Poco::URI("file://"), localPath);
+            _filename = fileInfo._filename;
 
-        const auto localPath = _storage->loadStorageFileToLocal();
-        _uriJailed = Poco::URI(Poco::URI("file://"), localPath);
-        _filename = fileInfo._filename;
+            // Use the local temp file's timestamp.
+            _lastFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified();
+            _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot));
+        }
 
-        // Use the local temp file's timestamp.
-        _lastFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified();
-        _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot));
+        // If its WOPI storage, send the duration that it took for WOPI calls
+        if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr)
+        {
+            // Get the time taken to load the file from storage
+            auto callDuration = dynamic_cast<WopiStorage*>(_storage.get())->getWopiLoadDuration();
+            // Add the time taken to check file info
+            callDuration += fileInfo._callDuration;
+            const std::string msg = "stats: wopiloadduration " + std::to_string(callDuration.count());
+            Log::trace("Sending to Client [" + msg + "].");
+            it->second->sendTextFrame(msg);
+        }
 
         return true;
     }
@@ -856,16 +866,6 @@ bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vecto
     return false;
 }
 
-const std::chrono::duration<double> DocumentBroker::getStorageLoadDuration() const
-{
-    if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr)
-    {
-        return dynamic_cast<WopiStorage*>(_storage.get())->getWopiLoadDuration();
-    }
-
-    return std::chrono::duration<double>::zero();
-}
-
 void DocumentBroker::childSocketTerminated()
 {
     if (!_childProcess->isAlive())
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 10b4e2f..3eefd6d 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -881,15 +881,6 @@ private:
             auto sessionsCount = docBroker->addSession(session);
             Log::trace(docKey + ", ws_sessions++: " + std::to_string(sessionsCount));
 
-            // If its a WOPI host, return time taken to make calls to it
-            const auto storageCallDuration = docBroker->getStorageLoadDuration();
-            if (storageCallDuration != std::chrono::duration<double>::zero())
-            {
-                status = "stats: wopiloadduration " + std::to_string(storageCallDuration.count());
-                Log::trace("Sending to Client [" + status + "].");
-                ws->sendFrame(status.data(), (int) status.size());
-            }
-
             LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "NewSession: " + uri);
 
             // Let messages flow.
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index a20691d..3a00066 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -306,9 +306,8 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
     Poco::StreamCopier::copyToString(rs, resMsg);
 
     const auto endTime = std::chrono::steady_clock::now();
-    const std::chrono::duration<double> diff = (endTime - startTime);
-    _wopiLoadDuration += diff;
-    Log::debug("WOPI::CheckFileInfo returned: " + resMsg + ". Call duration: " + std::to_string(diff.count()) + "s");
+    const std::chrono::duration<double> callDuration = (endTime - startTime);
+    Log::debug("WOPI::CheckFileInfo returned: " + resMsg + ". Call duration: " + std::to_string(callDuration.count()) + "s");
     const auto index = resMsg.find_first_of('{');
     if (index != std::string::npos)
     {
@@ -331,6 +330,7 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
 
     // WOPI doesn't support file last modified time.
     _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName, canWrite});
+    _fileInfo._callDuration = callDuration;
     return _fileInfo;
 }
 
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 163443c..6238a57 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -42,7 +42,8 @@ public:
               _size(size),
               _userId(userId),
               _userName(userName),
-              _canWrite(canWrite)
+              _canWrite(canWrite),
+              _callDuration(0)
         {
         }
 
@@ -57,6 +58,10 @@ public:
         std::string _userId;
         std::string _userName;
         bool _canWrite;
+
+        // Time it took to fetch fileinfo from storage
+        // Matters in case of external storage such as WOPI
+        std::chrono::duration<double> _callDuration;
     };
 
     /// localStorePath the absolute root path of the chroot.
@@ -164,11 +169,11 @@ public:
 
     bool saveLocalFileToStorage(const Poco::URI& uriPublic) override;
 
-    /// Total time taken for making WOPI calls during load (includes GetFileInfo calls)
-    const std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
+    /// Total time taken for making WOPI calls during load
+    std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
 
 private:
-    // Time spend in waiting for WOPI host during document load
+    // Time spend in loading the file from storage
     std::chrono::duration<double> _wopiLoadDuration;
 };
 
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index eb40f7a..560208f 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -334,7 +334,7 @@ redlinetablechanged:
 
 stats: <key> <value>
 
-    Contains statistical data. Eg: 'stats: wopiduration 5' means that
+    Contains statistical data. Eg: 'stats: wopiloadduration 5' means that
     wopi calls made in loading of document took 5 seconds.
 
 perm: <permission>
commit 050bf4c65ec6901749da484cd568b175e98aa0ef
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Sun Oct 23 17:06:32 2016 +0530

    loolwsd: Remove this comment - this is fixed now
    
    Change-Id: I032c7e4a1609b68882dba6cc48ebd3fb2d59b8f5

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 850d570..eeb3fd7 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -222,8 +222,6 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
 
     if (_storage)
     {
-        // Set the username for the session
-        // TODO: security: Set the permission (readonly etc.) of the session here also
         const auto fileInfo = _storage->getFileInfo(uriPublic);
         if (!fileInfo.isValid())
         {


More information about the Libreoffice-commits mailing list