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

Pranav Kant pranavk at collabora.co.uk
Wed Oct 26 14:45:47 UTC 2016


 loolwsd/DocumentBroker.cpp |   41 ++++++++++++++++------
 loolwsd/Storage.cpp        |   42 ++++++++++++-----------
 loolwsd/Storage.hpp        |   82 +++++++++++++++++++++++++++++----------------
 3 files changed, 107 insertions(+), 58 deletions(-)

New commits:
commit 4269dfdf846f17e500a70bd7b55e175ac207a593
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Wed Oct 26 18:17:42 2016 +0530

    loolwsd: unsigned -> std::atomic<unsigned>
    
    Change-Id: I41da65c7c5a43c2b1aafd8620042b0a3d34cc3af

diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index 0f617d6..451387e 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -175,7 +175,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
     throw BadRequestException("No Storage configured or invalid URI.");
 }
 
-unsigned LocalStorage::LastLocalStorageId = 0;
+std::atomic<unsigned> LocalStorage::LastLocalStorageId;
 
 LocalStorage::LocalFileInfo LocalStorage::getLocalFileInfo(const Poco::URI& uriPublic)
 {
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 1bd8784..e9ed98c 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -144,7 +144,7 @@ public:
 private:
     /// True if the jailed file is not linked but copied.
     bool _isCopy;
-    static unsigned LastLocalStorageId;
+    static std::atomic<unsigned> LastLocalStorageId;
 };
 
 /// WOPI protocol backed storage.
commit cc184ea2c225024abca746dd02197ff211ae50dd
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Wed Oct 26 16:45:28 2016 +0530

    loolwsd: Separate FileInfo classes for different storages
    
    The idea, for now, is to call the storage specific fileinfo
    functions (getLocalFileInfo, getWOPIFileInfo) which returns the
    file + user specific data for given URI. By user specific, it
    means data which is token (provided in URI) dependent.
    For WOPI, it would mean various user permissions
    (editable/viewable etc.)
    
    For our current storages, these calls also include the basic file
    information which is common across all tokens. This can be
    retreived by calling getFileInfo on the storage object.
    
    Change-Id: Ibbd3b74b011d8bb6fe4730c33276ef9ac6c47103

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3a5e1be..38846ac 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -222,19 +222,38 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
 
     if (_storage)
     {
-        const auto fileInfo = _storage->getFileInfo(uriPublic);
+        // Call the storage specific file info functions
+        std::string username;
+        std::chrono::duration<double> getInfoCallDuration;
+        if (dynamic_cast<WopiStorage*>(_storage.get()) != nullptr)
+        {
+            const WopiStorage::WOPIFileInfo wopifileinfo = static_cast<WopiStorage*>(_storage.get())->getWOPIFileInfo(uriPublic);
+            username = wopifileinfo._username;
+
+            if (!wopifileinfo._userCanWrite)
+            {
+                Log::debug("Setting the session as readonly");
+                it->second->setReadOnly();
+            }
+
+            getInfoCallDuration = wopifileinfo._callDuration;
+        }
+        else if (dynamic_cast<LocalStorage*>(_storage.get()) != nullptr)
+        {
+            const LocalStorage::LocalFileInfo localfileinfo = static_cast<LocalStorage*>(_storage.get())->getLocalFileInfo(uriPublic);
+            username = localfileinfo._username;
+        }
+
+        Log::debug("Setting username of the session to: " + username);
+        it->second->setUserName(username);
+
+        // Get basic file information from the storage
+        const auto fileInfo = _storage->getFileInfo();
         if (!fileInfo.isValid())
         {
             Log::error("Invalid fileinfo for URI [" + uriPublic.toString() + "].");
             return false;
         }
-        Log::debug("Setting username of the session to: " + fileInfo._userName);
-        it->second->setUserName(fileInfo._userName);
-        if (!fileInfo._canWrite)
-        {
-            Log::debug("Setting the session as readonly");
-            it->second->setReadOnly();
-        }
 
         // Lets load the document now
         const bool loaded = _storage->isLoaded();
@@ -249,13 +268,13 @@ bool DocumentBroker::load(const std::string& sessionId, const std::string& jailI
             _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot));
         }
 
-        // If its WOPI storage, send the duration that it took for WOPI calls
+        // Since document has been loaded, send the stats if its WOPI
         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();
+            auto callDuration = static_cast<WopiStorage*>(_storage.get())->getWopiLoadDuration();
             // Add the time taken to check file info
-            callDuration += fileInfo._callDuration;
+            callDuration += getInfoCallDuration;
             const std::string msg = "stats: wopiloadduration " + std::to_string(callDuration.count());
             Log::trace("Sending to Client [" + msg + "].");
             it->second->sendTextFrame(msg);
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index 3a00066..0f617d6 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -177,15 +177,23 @@ std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std
 
 unsigned LocalStorage::LastLocalStorageId = 0;
 
-StorageBase::FileInfo LocalStorage::getFileInfo(const Poco::URI& uriPublic)
+LocalStorage::LocalFileInfo LocalStorage::getLocalFileInfo(const Poco::URI& uriPublic)
 {
     const auto path = Poco::Path(uriPublic.getPath());
     Log::debug("Getting info for local uri [" + uriPublic.toString() + "], path [" + path.toString() + "].");
-    const auto& filename = path.getFileName();
-    const auto file = Poco::File(path);
-    const auto lastModified = file.getLastModified();
-    const auto size = file.getSize();
-    return FileInfo({filename, lastModified, size, "localhost", std::string("Local Host #") + std::to_string(_localStorageId), true});
+
+    if (!_fileInfo.isValid())
+    {
+        const auto& filename = path.getFileName();
+        const auto file = Poco::File(path);
+        const auto lastModified = file.getLastModified();
+        const auto size = file.getSize();
+
+        _fileInfo = FileInfo({filename, lastModified, size});
+    }
+
+    // Set automatic userid and username
+    return LocalFileInfo({"localhost", std::string("Local Host #") + std::to_string(LastLocalStorageId++)});
 }
 
 std::string LocalStorage::loadStorageFileToLocal()
@@ -273,7 +281,7 @@ Poco::Dynamic::Var getOrWarn(const Poco::JSON::Object::Ptr &object, const char *
 
 } // anonymous namespace
 
-StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
+WopiStorage::WOPIFileInfo WopiStorage::getWOPIFileInfo(const Poco::URI& uriPublic)
 {
     Log::debug("Getting info for wopi uri [" + uriPublic.toString() + "].");
 
@@ -328,10 +336,13 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uriPublic)
     else
         Log::error("WOPI::CheckFileInfo is missing JSON payload");
 
-    // WOPI doesn't support file last modified time.
-    _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName, canWrite});
-    _fileInfo._callDuration = callDuration;
-    return _fileInfo;
+    if (!_fileInfo.isValid())
+    {
+        // WOPI doesn't support file last modified time.
+        _fileInfo = FileInfo({filename, Poco::Timestamp(), size});
+    }
+
+    return WOPIFileInfo({userId, userName, canWrite, callDuration});
 }
 
 /// uri format: http://server/<...>/wopi*/files/<id>/content
@@ -417,13 +428,6 @@ bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
     return success;
 }
 
-StorageBase::FileInfo WebDAVStorage::getFileInfo(const Poco::URI& uriPublic)
-{
-    Log::debug("Getting info for webdav uri [" + uriPublic.toString() + "].");
-    assert(false && "Not Implemented!");
-    return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin", false});
-}
-
 std::string WebDAVStorage::loadStorageFileToLocal()
 {
     // TODO: implement webdav GET.
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 6238a57..1bd8784 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -26,24 +26,17 @@ class StorageBase
 {
 public:
 
-    /// Represents a file's attributes.
+    /// Represents basic file's attributes.
     /// Used for local and network files.
     class FileInfo
     {
     public:
         FileInfo(const std::string& filename,
                  const Poco::Timestamp& modifiedTime,
-                 size_t size,
-                 const std::string& userId,
-                 const std::string& userName,
-                 const bool canWrite)
+                 size_t size)
             : _filename(filename),
               _modifiedTime(modifiedTime),
-              _size(size),
-              _userId(userId),
-              _userName(userName),
-              _canWrite(canWrite),
-              _callDuration(0)
+              _size(size)
         {
         }
 
@@ -55,13 +48,6 @@ public:
         std::string _filename;
         Poco::Timestamp _modifiedTime;
         size_t _size;
-        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.
@@ -72,7 +58,7 @@ public:
         _uri(uri),
         _localStorePath(localStorePath),
         _jailPath(jailPath),
-        _fileInfo("", Poco::Timestamp(), 0, "", "", false),
+        _fileInfo("", Poco::Timestamp(), 0),
         _isLoaded(false)
     {
         Log::debug("Storage ctor: " + uri.toString());
@@ -84,8 +70,8 @@ public:
 
     bool isLoaded() const { return _isLoaded; }
 
-    /// Returns information about the file.
-    virtual FileInfo getFileInfo(const Poco::URI& uriPublic) = 0;
+    /// Returns the basic information about the file.
+    virtual FileInfo getFileInfo() { return _fileInfo; }
 
     /// Returns a local file path for the given URI.
     /// If necessary copies the file locally first.
@@ -127,14 +113,29 @@ public:
                  const std::string& localStorePath,
                  const std::string& jailPath) :
         StorageBase(uri, localStorePath, jailPath),
-        _isCopy(false),
-        _localStorageId(LocalStorage::LastLocalStorageId++)
+        _isCopy(false)
     {
         Log::info("LocalStorage ctor with localStorePath: [" + localStorePath +
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
-    FileInfo getFileInfo(const Poco::URI& uriPublic) override;
+    class LocalFileInfo {
+    public:
+        LocalFileInfo(const std::string& userid,
+                      const std::string& username)
+            : _userid(userid),
+              _username(username)
+        {
+        }
+
+        std::string _userid;
+        std::string _username;
+    };
+
+    /// Returns the URI specific file data
+    /// Also stores the basic file information which can then be
+    /// obtained using getFileInfo method
+    LocalFileInfo getLocalFileInfo(const Poco::URI& uriPublic);
 
     std::string loadStorageFileToLocal() override;
 
@@ -144,8 +145,6 @@ private:
     /// True if the jailed file is not linked but copied.
     bool _isCopy;
     static unsigned LastLocalStorageId;
-    /// Used to differentiate between multiple localhost users.
-    const unsigned _localStorageId;
 };
 
 /// WOPI protocol backed storage.
@@ -162,7 +161,33 @@ public:
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
-    FileInfo getFileInfo(const Poco::URI& uriPublic) override;
+    class WOPIFileInfo {
+    public:
+        WOPIFileInfo(const std::string& userid,
+                     const std::string& username,
+                     const bool userCanWrite,
+                     const std::chrono::duration<double> callDuration)
+            : _userid(userid),
+              _username(username),
+              _userCanWrite(userCanWrite),
+              _callDuration(callDuration)
+            {
+            }
+
+        /// User id of the user accessing the file
+        std::string _userid;
+        /// Display Name of user accessing the file
+        std::string _username;
+        /// If user accessing the file has write permission
+        bool _userCanWrite;
+        /// Time it took to call WOPI's CheckFileInfo
+        std::chrono::duration<double> _callDuration;
+    };
+
+    /// Returns the response of CheckFileInfo WOPI call for given URI
+    /// Also extracts the basic file information from the response
+    /// which can then be obtained using getFileInfo()
+    WOPIFileInfo getWOPIFileInfo(const Poco::URI& uriPublic);
 
     /// uri format: http://server/<...>/wopi*/files/<id>/content
     std::string loadStorageFileToLocal() override;
@@ -192,7 +217,8 @@ public:
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
-    FileInfo getFileInfo(const Poco::URI& uriPublic) override;
+    // Implement me
+    // WebDAVFileInfo getWebDAVFileInfo(const Poco::URI& uriPublic);
 
     std::string loadStorageFileToLocal() override;
 


More information about the Libreoffice-commits mailing list