[Libreoffice-commits] online.git: 3 commits - loolwsd/DocumentBroker.cpp loolwsd/Storage.cpp loolwsd/Storage.hpp loolwsd/test loolwsd/Unit.hpp

Pranav Kant pranavk at collabora.co.uk
Fri Oct 14 11:49:39 UTC 2016


 loolwsd/DocumentBroker.cpp   |   37 ++++++++++++++--------
 loolwsd/Storage.cpp          |   65 ++++++++++++++++++++++-----------------
 loolwsd/Storage.hpp          |   70 +++++++++++++++++++++++--------------------
 loolwsd/Unit.hpp             |    4 +-
 loolwsd/test/UnitStorage.cpp |    4 +-
 5 files changed, 103 insertions(+), 77 deletions(-)

New commits:
commit edfd3266f89708afd8cbec0adf88bac63f90cd24
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Fri Oct 14 15:39:43 2016 +0530

    loolwsd: Reuse storage object; kill superfluous WOPI calls
    
    This reduces the number of fileinfo calls made to storage. These calls can
    be expensive in storage such as WOPI where loolwsd needs to
    interact with another server to get the file information. Use the
    same storage object once created so that fileinfo can be
    cached and returned quickly for subsequent such calls.
    
    3 GetFileInfo WOPI calls are now merged into 1.
    
    Change-Id: I56c3d23d3d6d7dc3a4b42433f51304dac28a12e8

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 0884163..5ab7790 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -158,8 +158,8 @@ const StorageBase::FileInfo DocumentBroker::validate(const Poco::URI& uri)
     Log::info("Validating: " + uriString);
     try
     {
-        auto storage = StorageBase::create("", "", uri);
-        auto fileinfo = storage->getFileInfo();
+        _storage = StorageBase::create(uri, "", "");
+        auto fileinfo = _storage->getFileInfo();
         Log::info("After checkfileinfo: " + uriString + " -> " + fileinfo._filename);
         if (!fileinfo.isValid())
         {
@@ -188,12 +188,6 @@ bool DocumentBroker::load(const std::string& jailId)
         return false;
     }
 
-    if (_storage)
-    {
-        // Already loaded. Nothing to do.
-        return true;
-    }
-
     _jailId = jailId;
 
     // The URL is the publicly visible one, not visible in the chroot jail.
@@ -208,20 +202,35 @@ bool DocumentBroker::load(const std::string& jailId)
     if (LOOLWSD::NoCapsForKit)
         jailRoot = jailPath.toString() + "/" + getJailRoot();
 
-    auto storage = StorageBase::create(jailRoot, jailPath.toString(), _uriPublic);
-    if (storage)
+    if (_storage == nullptr)
+    {
+        _storage = StorageBase::create(_uriPublic, jailRoot, jailPath.toString());
+    }
+    else
+    {
+        if (_storage->isLoaded())
+        {
+            // Already loaded. Nothing to do.
+            return true;
+        }
+
+        _storage->setLocalStorePath(jailRoot);
+        _storage->setJailPath(jailPath.toString());
+    }
+
+
+    if (_storage)
     {
-        const auto fileInfo = storage->getFileInfo();
+        const auto fileInfo = _storage->getFileInfo();
         _filename = fileInfo._filename;
 
-        const auto localPath = storage->loadStorageFileToLocal();
+        const auto localPath = _storage->loadStorageFileToLocal();
         _uriJailed = Poco::URI(Poco::URI("file://"), localPath);
 
         // Use the local temp file's timestamp.
-        _lastFileModifiedTime = Poco::File(storage->getLocalRootPath()).getLastModified();
+        _lastFileModifiedTime = Poco::File(_storage->getLocalRootPath()).getLastModified();
         _tileCache.reset(new TileCache(_uriPublic.toString(), _lastFileModifiedTime, _cacheRoot));
 
-        _storage = std::move(storage);
         return true;
     }
 
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index 11cead8..7e5d222 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -128,11 +128,11 @@ bool isLocalhost(const std::string& targetHost)
     return false;
 }
 
-std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, const std::string& jailPath, const Poco::URI& uri)
+std::unique_ptr<StorageBase> StorageBase::create(const Poco::URI& uri, const std::string& jailRoot, const std::string& jailPath)
 {
     std::unique_ptr<StorageBase> storage;
 
-    if (UnitWSD::get().createStorage(jailRoot, jailPath, uri, storage))
+    if (UnitWSD::get().createStorage(uri, jailRoot, jailPath, storage))
     {
         Log::info("Storage load hooked.");
         if (storage)
@@ -143,7 +143,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, co
         Log::info("Public URI [" + uri.toString() + "] is a file.");
         if (FilesystemEnabled)
         {
-            return std::unique_ptr<StorageBase>(new LocalStorage(jailRoot, jailPath, uri));
+            return std::unique_ptr<StorageBase>(new LocalStorage(uri, jailRoot, jailPath));
         }
 
         Log::error("Local Storage is disabled by default. Enable in the config file or on the command-line to enable.");
@@ -154,7 +154,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, co
         const auto& targetHost = uri.getHost();
         if (WopiHosts.match(targetHost) || isLocalhost(targetHost))
         {
-            return std::unique_ptr<StorageBase>(new WopiStorage(jailRoot, jailPath, uri));
+            return std::unique_ptr<StorageBase>(new WopiStorage(uri, jailRoot, jailPath));
         }
 
         Log::error("No acceptable WOPI hosts found matching the target host [" + targetHost + "] in config.");
@@ -217,6 +217,7 @@ std::string LocalStorage::loadStorageFileToLocal()
         throw;
     }
 
+    _isLoaded = true;
     // Now return the jailed path.
     return Poco::Path(_jailPath, filename).toString();
 }
@@ -256,6 +257,12 @@ StorageBase::FileInfo WopiStorage::getFileInfo()
 {
     Log::debug("Getting info for wopi uri [" + _uri.toString() + "].");
 
+    if (_fileInfo.isValid())
+    {
+        Log::debug("Returning cached fileinfo for wopi uri [" + _uri.toString() + "].");
+        return _fileInfo;
+    }
+
     std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(_uri));
 
     Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, _uri.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
@@ -298,7 +305,8 @@ StorageBase::FileInfo WopiStorage::getFileInfo()
     }
 
     // WOPI doesn't support file last modified time.
-    return FileInfo({filename, Poco::Timestamp(), size, userId, userName});
+    _fileInfo = FileInfo({filename, Poco::Timestamp(), size, userId, userName});
+    return _fileInfo;
 }
 
 /// uri format: http://server/<...>/wopi*/files/<id>/content
@@ -306,8 +314,8 @@ std::string WopiStorage::loadStorageFileToLocal()
 {
     Log::info("Downloading URI [" + _uri.toString() + "].");
 
-    _fileInfo = getFileInfo();
-    if (_fileInfo._size == 0 && _fileInfo._filename.empty())
+    getFileInfo();
+    if (!_fileInfo.isValid())
     {
         //TODO: Should throw a more appropriate exception.
         throw std::runtime_error("Failed to load file from storage.");
@@ -348,6 +356,7 @@ std::string WopiStorage::loadStorageFileToLocal()
                 << "] -> " << _jailedFilePath << ": "
                 << response.getStatus() << " " << response.getReason() << Log::end;
 
+    _isLoaded = true;
     // Now return the jailed path.
     return Poco::Path(_jailPath, _fileInfo._filename).toString();
 }
@@ -396,6 +405,7 @@ StorageBase::FileInfo WebDAVStorage::getFileInfo()
 std::string WebDAVStorage::loadStorageFileToLocal()
 {
     // TODO: implement webdav GET.
+    _isLoaded = true;
     return _uri.toString();
 }
 
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 7a9849f..4050906 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -58,13 +58,14 @@ public:
 
     /// localStorePath the absolute root path of the chroot.
     /// jailPath the path within the jail that the child uses.
-    StorageBase(const std::string& localStorePath,
-                const std::string& jailPath,
-                const Poco::URI& uri) :
+    StorageBase(const Poco::URI& uri,
+                const std::string& localStorePath,
+                const std::string& jailPath) :
+        _uri(uri),
         _localStorePath(localStorePath),
         _jailPath(jailPath),
-        _uri(uri),
-        _fileInfo("", Poco::Timestamp(), 0, "", "")
+        _fileInfo("", Poco::Timestamp(), 0, "", ""),
+        _isLoaded(false)
     {
         Log::debug("Storage ctor: " + uri.toString());
     }
@@ -73,6 +74,12 @@ public:
 
     const std::string getUri() const { return _uri.toString(); }
 
+    bool isLoaded() const { return _isLoaded; }
+
+    void setLocalStorePath(const std::string& localStorePath) { _localStorePath = localStorePath; }
+
+    void setJailPath(const std::string& jailPath) { _jailPath = jailPath; }
+
     /// Returns information about the file.
     virtual FileInfo getFileInfo() = 0;
 
@@ -93,16 +100,17 @@ public:
     static void initialize();
 
     /// Storage object creation factory.
-    static std::unique_ptr<StorageBase> create(const std::string& jailRoot,
-                                               const std::string& jailPath,
-                                               const Poco::URI& uri);
+    static std::unique_ptr<StorageBase> create(const Poco::URI& uri,
+                                               const std::string& jailRoot,
+                                               const std::string& jailPath);
 
 protected:
-    const std::string _localStorePath;
-    const std::string _jailPath;
     const Poco::URI _uri;
+    std::string _localStorePath;
+    std::string _jailPath;
     std::string _jailedFilePath;
     FileInfo _fileInfo;
+    bool _isLoaded;
 
     static bool FilesystemEnabled;
     static bool WopiEnabled;
@@ -114,10 +122,10 @@ protected:
 class LocalStorage : public StorageBase
 {
 public:
-    LocalStorage(const std::string& localStorePath,
-                 const std::string& jailPath,
-                 const Poco::URI& uri) :
-        StorageBase(localStorePath, jailPath, uri),
+    LocalStorage(const Poco::URI& uri,
+                 const std::string& localStorePath,
+                 const std::string& jailPath) :
+        StorageBase(uri, localStorePath, jailPath),
         _isCopy(false),
         _localStorageId(LocalStorage::LastLocalStorageId++)
     {
@@ -143,10 +151,10 @@ private:
 class WopiStorage : public StorageBase
 {
 public:
-    WopiStorage(const std::string& localStorePath,
-                const std::string& jailPath,
-                const Poco::URI& uri) :
-        StorageBase(localStorePath, jailPath, uri)
+    WopiStorage(const Poco::URI& uri,
+                const std::string& localStorePath,
+                const std::string& jailPath) :
+        StorageBase(uri, localStorePath, jailPath)
     {
         Log::info("WopiStorage ctor with localStorePath: [" + localStorePath +
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
@@ -164,11 +172,11 @@ public:
 class WebDAVStorage : public StorageBase
 {
 public:
-    WebDAVStorage(const std::string& localStorePath,
+    WebDAVStorage(const Poco::URI& uri,
+                  const std::string& localStorePath,
                   const std::string& jailPath,
-                  const Poco::URI& uri,
                   std::unique_ptr<AuthBase> authAgent) :
-        StorageBase(localStorePath, jailPath, uri),
+        StorageBase(uri, localStorePath, jailPath),
         _authAgent(std::move(authAgent))
     {
         Log::info("WebDAVStorage ctor with localStorePath: [" + localStorePath +
diff --git a/loolwsd/Unit.hpp b/loolwsd/Unit.hpp
index f32047b..05b2126 100644
--- a/loolwsd/Unit.hpp
+++ b/loolwsd/Unit.hpp
@@ -122,9 +122,9 @@ public:
     /// When a new child kit process reports
     virtual void newChild(const std::shared_ptr<Poco::Net::WebSocket> & /* socket */) {}
     /// Intercept createStorage
-    virtual bool createStorage(const std::string& /* jailRoot */,
+    virtual bool createStorage(const Poco::URI& /* uri */,
+                               const std::string& /* jailRoot */,
                                const std::string& /* jailPath */,
-                               const Poco::URI& /* uri */,
                                std::unique_ptr<StorageBase> & /*rStorage */)
         { return false; }
     /// Intercept incoming requests, so unit tests can silently communicate
diff --git a/loolwsd/test/UnitStorage.cpp b/loolwsd/test/UnitStorage.cpp
index 306c1c7..94b913d 100644
--- a/loolwsd/test/UnitStorage.cpp
+++ b/loolwsd/test/UnitStorage.cpp
@@ -13,9 +13,9 @@
 class UnitStorage : public UnitWSD
 {
 public:
-    virtual bool createStorage(const std::string& /* jailRoot */,
+    virtual bool createStorage(const Poco::URI& /* uri */,
+                               const std::string& /* jailRoot */,
                                const std::string& /* jailPath */,
-                               const Poco::URI& /* uri */,
                                std::unique_ptr<StorageBase> & /* rStorage */)
     {
         // leave rStorage empty - fail to return anything
commit 66fa578fb31f63518d6749e307d9db046892cfa9
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Fri Oct 14 15:32:07 2016 +0530

    loolwsd: No need of passing URI again to getFileInfo
    
    Lets use the internal URI stored in storage object.
    
    Change-Id: I2bfefc8d41db86a940060fadaa8629e50ed14d7c

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3189944..0884163 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -159,7 +159,7 @@ const StorageBase::FileInfo DocumentBroker::validate(const Poco::URI& uri)
     try
     {
         auto storage = StorageBase::create("", "", uri);
-        auto fileinfo = storage->getFileInfo(uri);
+        auto fileinfo = storage->getFileInfo();
         Log::info("After checkfileinfo: " + uriString + " -> " + fileinfo._filename);
         if (!fileinfo.isValid())
         {
@@ -211,7 +211,7 @@ bool DocumentBroker::load(const std::string& jailId)
     auto storage = StorageBase::create(jailRoot, jailPath.toString(), _uriPublic);
     if (storage)
     {
-        const auto fileInfo = storage->getFileInfo(_uriPublic);
+        const auto fileInfo = storage->getFileInfo();
         _filename = fileInfo._filename;
 
         const auto localPath = storage->loadStorageFileToLocal();
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index 23a924e..11cead8 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -165,10 +165,10 @@ std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, co
 
 unsigned LocalStorage::LastLocalStorageId = 0;
 
-StorageBase::FileInfo LocalStorage::getFileInfo(const Poco::URI& uri)
+StorageBase::FileInfo LocalStorage::getFileInfo()
 {
-    const auto path = Poco::Path(uri.getPath());
-    Log::debug("Getting info for local uri [" + uri.toString() + "], path [" + path.toString() + "].");
+    const auto path = Poco::Path(_uri.getPath());
+    Log::debug("Getting info for local uri [" + _uri.toString() + "], path [" + path.toString() + "].");
     const auto& filename = path.getFileName();
     const auto file = Poco::File(path);
     const auto lastModified = file.getLastModified();
@@ -252,13 +252,13 @@ Poco::Net::HTTPClientSession* getHTTPClientSession(const Poco::URI& uri)
 
 } // anonymous namespace
 
-StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uri)
+StorageBase::FileInfo WopiStorage::getFileInfo()
 {
-    Log::debug("Getting info for wopi uri [" + uri.toString() + "].");
+    Log::debug("Getting info for wopi uri [" + _uri.toString() + "].");
 
-    std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(uri));
+    std::unique_ptr<Poco::Net::HTTPClientSession> psession(getHTTPClientSession(_uri));
 
-    Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, uri.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
+    Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, _uri.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
     request.set("User-Agent", "LOOLWSD WOPI Agent");
     psession->sendRequest(request);
 
@@ -266,7 +266,7 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uri)
     std::istream& rs = psession->receiveResponse(response);
 
     auto logger = Log::trace();
-    logger << "WOPI::CheckFileInfo header for URI [" << uri.toString() << "]:\n";
+    logger << "WOPI::CheckFileInfo header for URI [" << _uri.toString() << "]:\n";
     for (auto& pair : response)
     {
         logger << '\t' + pair.first + ": " + pair.second << " / ";
@@ -306,7 +306,7 @@ std::string WopiStorage::loadStorageFileToLocal()
 {
     Log::info("Downloading URI [" + _uri.toString() + "].");
 
-    _fileInfo = getFileInfo(_uri);
+    _fileInfo = getFileInfo();
     if (_fileInfo._size == 0 && _fileInfo._filename.empty())
     {
         //TODO: Should throw a more appropriate exception.
@@ -386,10 +386,9 @@ bool WopiStorage::saveLocalFileToStorage()
     return success;
 }
 
-StorageBase::FileInfo WebDAVStorage::getFileInfo(const Poco::URI& uri)
+StorageBase::FileInfo WebDAVStorage::getFileInfo()
 {
-    Log::debug("Getting info for webdav uri [" + uri.toString() + "].");
-    (void)uri;
+    Log::debug("Getting info for webdav uri [" + _uri.toString() + "].");
     assert(false && "Not Implemented!");
     return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin"});
 }
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 285b9a9..7a9849f 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -74,7 +74,7 @@ public:
     const std::string getUri() const { return _uri.toString(); }
 
     /// Returns information about the file.
-    virtual FileInfo getFileInfo(const Poco::URI& uri) = 0;
+    virtual FileInfo getFileInfo() = 0;
 
     /// Returns a local file path for the given URI.
     /// If necessary copies the file locally first.
@@ -125,7 +125,7 @@ public:
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
-    FileInfo getFileInfo(const Poco::URI& uri) override;
+    FileInfo getFileInfo() override;
 
     std::string loadStorageFileToLocal() override;
 
@@ -152,7 +152,7 @@ public:
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
-    FileInfo getFileInfo(const Poco::URI& uri) override;
+    FileInfo getFileInfo() override;
 
     /// uri format: http://server/<...>/wopi*/files/<id>/content
     std::string loadStorageFileToLocal() override;
@@ -175,7 +175,7 @@ public:
                   "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
-    FileInfo getFileInfo(const Poco::URI& uri) override;
+    FileInfo getFileInfo() override;
 
     std::string loadStorageFileToLocal() override;
 
commit e4466b418b62a435948ac6c422dd37fc7427a88d
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Fri Oct 14 14:58:19 2016 +0530

    loolwsd: Storage class: std::string -> Poco::URI
    
    This saves us from encoding/decoding mess of URIs. Poco::URI is
    flexible enough and can give encoded or decoded version whenever
    required, so lets avoid storing the URI in std::string where we
    have no information about whether its encoded or not.
    
    Change-Id: I4a6979e13b20d9f56fc5a0baa630cb1b35ca33b0

diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index 7cf5693..23a924e 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -143,7 +143,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, co
         Log::info("Public URI [" + uri.toString() + "] is a file.");
         if (FilesystemEnabled)
         {
-            return std::unique_ptr<StorageBase>(new LocalStorage(jailRoot, jailPath, uri.getPath()));
+            return std::unique_ptr<StorageBase>(new LocalStorage(jailRoot, jailPath, uri));
         }
 
         Log::error("Local Storage is disabled by default. Enable in the config file or on the command-line to enable.");
@@ -154,7 +154,7 @@ std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, co
         const auto& targetHost = uri.getHost();
         if (WopiHosts.match(targetHost) || isLocalhost(targetHost))
         {
-            return std::unique_ptr<StorageBase>(new WopiStorage(jailRoot, jailPath, uri.toString()));
+            return std::unique_ptr<StorageBase>(new WopiStorage(jailRoot, jailPath, uri));
         }
 
         Log::error("No acceptable WOPI hosts found matching the target host [" + targetHost + "] in config.");
@@ -181,15 +181,15 @@ std::string LocalStorage::loadStorageFileToLocal()
     const auto rootPath = getLocalRootPath();
 
     // /chroot/jailId/user/doc/childId/file.ext
-    const auto filename = Poco::Path(_uri).getFileName();
+    const auto filename = Poco::Path(_uri.getPath()).getFileName();
     _jailedFilePath = Poco::Path(rootPath, filename).toString();
 
-    Log::info("Public URI [" + _uri +
+    Log::info("Public URI [" + _uri.getPath() +
               "] jailed to [" + _jailedFilePath + "].");
 
     // Despite the talk about URIs it seems that _uri is actually just a pathname here
 
-    const auto publicFilePath = _uri;
+    const auto publicFilePath = _uri.getPath();
 
     if (!Util::checkDiskSpace(publicFilePath))
         throw StorageSpaceLowException("Low disk space for " + publicFilePath);
@@ -228,13 +228,13 @@ bool LocalStorage::saveLocalFileToStorage()
         // Copy the file back.
         if (_isCopy && Poco::File(_jailedFilePath).exists())
         {
-            Log::info("Copying " + _jailedFilePath + " to " + _uri);
-            Poco::File(_jailedFilePath).copyTo(_uri);
+            Log::info("Copying " + _jailedFilePath + " to " + _uri.getPath());
+            Poco::File(_jailedFilePath).copyTo(_uri.getPath());
         }
     }
     catch (const Poco::Exception& exc)
     {
-        Log::error("copyTo(\"" + _jailedFilePath + "\", \"" + _uri + "\") failed: " + exc.displayText());
+        Log::error("copyTo(\"" + _jailedFilePath + "\", \"" + _uri.getPath() + "\") failed: " + exc.displayText());
         throw;
     }
 
@@ -304,9 +304,9 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const Poco::URI& uri)
 /// uri format: http://server/<...>/wopi*/files/<id>/content
 std::string WopiStorage::loadStorageFileToLocal()
 {
-    Log::info("Downloading URI [" + _uri + "].");
+    Log::info("Downloading URI [" + _uri.toString() + "].");
 
-    _fileInfo = getFileInfo(Poco::URI(_uri));
+    _fileInfo = getFileInfo(_uri);
     if (_fileInfo._size == 0 && _fileInfo._filename.empty())
     {
         //TODO: Should throw a more appropriate exception.
@@ -329,7 +329,7 @@ std::string WopiStorage::loadStorageFileToLocal()
     std::istream& rs = psession->receiveResponse(response);
 
     auto logger = Log::trace();
-    logger << "WOPI::GetFile header for URI [" << _uri << "]:\n";
+    logger << "WOPI::GetFile header for URI [" << _uri.toString() << "]:\n";
     for (auto& pair : response)
     {
         logger << '\t' + pair.first + ": " + pair.second << " / ";
@@ -354,7 +354,7 @@ std::string WopiStorage::loadStorageFileToLocal()
 
 bool WopiStorage::saveLocalFileToStorage()
 {
-    Log::info("Uploading URI [" + _uri + "] from [" + _jailedFilePath + "].");
+    Log::info("Uploading URI [" + _uri.toString() + "] from [" + _jailedFilePath + "].");
     const auto size = getFileSize(_jailedFilePath);
 
     Poco::URI uriObject(_uri);
@@ -397,7 +397,7 @@ StorageBase::FileInfo WebDAVStorage::getFileInfo(const Poco::URI& uri)
 std::string WebDAVStorage::loadStorageFileToLocal()
 {
     // TODO: implement webdav GET.
-    return _uri;
+    return _uri.toString();
 }
 
 bool WebDAVStorage::saveLocalFileToStorage()
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 23bc7cd..285b9a9 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -60,18 +60,18 @@ public:
     /// jailPath the path within the jail that the child uses.
     StorageBase(const std::string& localStorePath,
                 const std::string& jailPath,
-                const std::string& uri) :
+                const Poco::URI& uri) :
         _localStorePath(localStorePath),
         _jailPath(jailPath),
         _uri(uri),
         _fileInfo("", Poco::Timestamp(), 0, "", "")
     {
-        Log::debug("Storage ctor: " + uri);
+        Log::debug("Storage ctor: " + uri.toString());
     }
 
     std::string getLocalRootPath() const;
 
-    const std::string& getUri() const { return _uri; }
+    const std::string getUri() const { return _uri.toString(); }
 
     /// Returns information about the file.
     virtual FileInfo getFileInfo(const Poco::URI& uri) = 0;
@@ -100,7 +100,7 @@ public:
 protected:
     const std::string _localStorePath;
     const std::string _jailPath;
-    const std::string _uri;
+    const Poco::URI _uri;
     std::string _jailedFilePath;
     FileInfo _fileInfo;
 
@@ -116,13 +116,13 @@ class LocalStorage : public StorageBase
 public:
     LocalStorage(const std::string& localStorePath,
                  const std::string& jailPath,
-                 const std::string& uri) :
+                 const Poco::URI& uri) :
         StorageBase(localStorePath, jailPath, uri),
         _isCopy(false),
         _localStorageId(LocalStorage::LastLocalStorageId++)
     {
         Log::info("LocalStorage ctor with localStorePath: [" + localStorePath +
-                  "], jailPath: [" + jailPath + "], uri: [" + uri + "].");
+                  "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
     FileInfo getFileInfo(const Poco::URI& uri) override;
@@ -145,11 +145,11 @@ class WopiStorage : public StorageBase
 public:
     WopiStorage(const std::string& localStorePath,
                 const std::string& jailPath,
-                const std::string& uri) :
+                const Poco::URI& uri) :
         StorageBase(localStorePath, jailPath, uri)
     {
         Log::info("WopiStorage ctor with localStorePath: [" + localStorePath +
-                  "], jailPath: [" + jailPath + "], uri: [" + uri + "].");
+                  "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
     FileInfo getFileInfo(const Poco::URI& uri) override;
@@ -166,13 +166,13 @@ class WebDAVStorage : public StorageBase
 public:
     WebDAVStorage(const std::string& localStorePath,
                   const std::string& jailPath,
-                  const std::string& uri,
+                  const Poco::URI& uri,
                   std::unique_ptr<AuthBase> authAgent) :
         StorageBase(localStorePath, jailPath, uri),
         _authAgent(std::move(authAgent))
     {
         Log::info("WebDAVStorage ctor with localStorePath: [" + localStorePath +
-                  "], jailPath: [" + jailPath + "], uri: [" + uri + "].");
+                  "], jailPath: [" + jailPath + "], uri: [" + uri.toString() + "].");
     }
 
     FileInfo getFileInfo(const Poco::URI& uri) override;


More information about the Libreoffice-commits mailing list