[Libreoffice-commits] online.git: wsd/Storage.cpp wsd/Storage.hpp

Michael Meeks (via logerrit) logerrit at kemper.freedesktop.org
Thu Jun 11 15:38:50 UTC 2020


 wsd/Storage.cpp |  239 ++++++++++++++++++++++++++------------------------------
 wsd/Storage.hpp |   95 +---------------------
 2 files changed, 120 insertions(+), 214 deletions(-)

New commits:
commit 33a5813d84f24910c36adc3615b0d017f13f6e4d
Author:     Michael Meeks <michael.meeks at collabora.com>
AuthorDate: Thu Jun 11 15:54:27 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Thu Jun 11 17:38:32 2020 +0200

    WOPI: pure re-factor, remove rampant duplication.
    
    Dung out lots of pointless intermediate variables, and overly
    verbose code. Vertical space is not a renewable resource.
    
    Most variables had a consistent pattern, except these:
    
    caller var          c'tor parameter         member name
    
    Change-Id: I7910b713b8c4f6950b1e7be9c3a8e4eb4f54e249
    ----------------------------------------------------------
    userId              userid                  _userId
    userName            username                _username
    canWrite            userCanWrite            _userCanWriter
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/96129
    Tested-by: Jenkins
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 95046acac..3960544c2 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -12,6 +12,7 @@
 #include "Storage.hpp"
 
 #include <algorithm>
+#include <memory>
 #include <cassert>
 #include <errno.h>
 #include <fstream>
@@ -602,40 +603,6 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au
         LOG_ERR("Cannot get file info from WOPI storage uri [" << uriAnonym << "]. Error:  Failed HTPP request authorization");
     }
 
-    // Parse the response.
-    std::string filename;
-    size_t size = 0;
-    std::string ownerId;
-    std::string userId;
-    std::string userName;
-    std::string obfuscatedUserId;
-    std::string userExtraInfo;
-    std::string watermarkText;
-    std::string templateSaveAs;
-    std::string templateSource;
-    bool canWrite = false;
-    bool enableOwnerTermination = false;
-    std::string postMessageOrigin;
-    bool hidePrintOption = false;
-    bool hideSaveOption = false;
-    bool hideExportOption = false;
-    bool disablePrint = false;
-    bool disableExport = false;
-    bool disableCopy = false;
-    bool disableInactiveMessages = false;
-    bool downloadAsPostMessage = false;
-    std::string lastModifiedTime;
-    bool userCanNotWriteRelative = true;
-    bool enableInsertRemoteImage = false;
-    bool enableShare = false;
-    bool supportsLocks = false;
-    bool supportsRename = false;
-    bool userCanRename = false;
-    std::string hideUserList("false");
-    WOPIFileInfo::TriState disableChangeTrackingRecord = WOPIFileInfo::TriState::Unset;
-    WOPIFileInfo::TriState disableChangeTrackingShow = WOPIFileInfo::TriState::Unset;
-    WOPIFileInfo::TriState hideChangeTrackingControls = WOPIFileInfo::TriState::Unset;
-
     Poco::JSON::Object::Ptr object;
     if (JsonUtil::parseJSON(wopiResponse, object))
     {
@@ -644,88 +611,26 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au
         else
             LOG_DBG("WOPI::CheckFileInfo (" << callDuration.count() * 1000. << " ms): " << wopiResponse);
 
-        JsonUtil::findJSONValue(object, "BaseFileName", filename);
+        size_t size = 0;
+        std::string filename, ownerId, lastModifiedTime;
+
+        JsonUtil::findJSONValue(object, "Size", size);
         JsonUtil::findJSONValue(object, "OwnerId", ownerId);
-        JsonUtil::findJSONValue(object, "UserId", userId);
-        JsonUtil::findJSONValue(object, "UserFriendlyName", userName);
-        JsonUtil::findJSONValue(object, "TemplateSaveAs", templateSaveAs);
-        JsonUtil::findJSONValue(object, "TemplateSource", templateSource);
+        JsonUtil::findJSONValue(object, "BaseFileName", filename);
+        JsonUtil::findJSONValue(object, "LastModifiedTime", lastModifiedTime);
+
+        const std::chrono::system_clock::time_point modifiedTime = Util::iso8601ToTimestamp(lastModifiedTime, "LastModifiedTime");
+        FileInfo fileInfo = FileInfo({filename, ownerId, modifiedTime, size});
+        setFileInfo(fileInfo);
 
-        // Anonymize key values.
         if (LOOLWSD::AnonymizeUserData)
-        {
             Util::mapAnonymized(Util::getFilenameFromURL(filename), Util::getFilenameFromURL(getUri().toString()));
 
-            JsonUtil::findJSONValue(object, "ObfuscatedUserId", obfuscatedUserId, false);
-            if (!obfuscatedUserId.empty())
-            {
-                Util::mapAnonymized(ownerId, obfuscatedUserId);
-                Util::mapAnonymized(userId, obfuscatedUserId);
-                Util::mapAnonymized(userName, obfuscatedUserId);
-            }
-
-            // Set anonymized version of the above fields before logging.
-            // Note: anonymization caches the result, so we don't need to store here.
-            if (LOOLWSD::AnonymizeUserData)
-                object->set("BaseFileName", LOOLWSD::anonymizeUrl(filename));
-
-            // If obfuscatedUserId is provided, then don't log the originals and use it.
-            if (LOOLWSD::AnonymizeUserData && obfuscatedUserId.empty())
-            {
-                object->set("OwnerId", LOOLWSD::anonymizeUsername(ownerId));
-                object->set("UserId", LOOLWSD::anonymizeUsername(userId));
-                object->set("UserFriendlyName", LOOLWSD::anonymizeUsername(userName));
-            }
-
-            std::ostringstream oss;
-            object->stringify(oss);
-            wopiResponse = oss.str();
-
-            // Remove them for performance reasons; they aren't needed anymore.
-            object->remove("ObfuscatedUserId");
-
-            if (LOOLWSD::AnonymizeUserData)
-            {
-                object->remove("BaseFileName");
-                object->remove("TemplateSaveAs");
-                object->remove("TemplateSource");
-                object->remove("OwnerId");
-                object->remove("UserId");
-                object->remove("UserFriendlyName");
-            }
-
-            LOG_DBG("WOPI::CheckFileInfo (" << callDuration.count() * 1000. << " ms): " << wopiResponse);
-        }
+        auto wopiInfo = std::unique_ptr<WopiStorage::WOPIFileInfo>(new WOPIFileInfo(fileInfo, callDuration, object));
+        if (wopiInfo->getSupportsLocks())
+            lockCtx.initSupportsLocks();
 
-        JsonUtil::findJSONValue(object, "Size", size);
-        JsonUtil::findJSONValue(object, "UserExtraInfo", userExtraInfo);
-        JsonUtil::findJSONValue(object, "WatermarkText", watermarkText);
-        JsonUtil::findJSONValue(object, "UserCanWrite", canWrite);
-        JsonUtil::findJSONValue(object, "PostMessageOrigin", postMessageOrigin);
-        JsonUtil::findJSONValue(object, "HidePrintOption", hidePrintOption);
-        JsonUtil::findJSONValue(object, "HideSaveOption", hideSaveOption);
-        JsonUtil::findJSONValue(object, "HideExportOption", hideExportOption);
-        JsonUtil::findJSONValue(object, "EnableOwnerTermination", enableOwnerTermination);
-        JsonUtil::findJSONValue(object, "DisablePrint", disablePrint);
-        JsonUtil::findJSONValue(object, "DisableExport", disableExport);
-        JsonUtil::findJSONValue(object, "DisableCopy", disableCopy);
-        JsonUtil::findJSONValue(object, "DisableInactiveMessages", disableInactiveMessages);
-        JsonUtil::findJSONValue(object, "DownloadAsPostMessage", downloadAsPostMessage);
-        JsonUtil::findJSONValue(object, "LastModifiedTime", lastModifiedTime);
-        JsonUtil::findJSONValue(object, "UserCanNotWriteRelative", userCanNotWriteRelative);
-        JsonUtil::findJSONValue(object, "EnableInsertRemoteImage", enableInsertRemoteImage);
-        JsonUtil::findJSONValue(object, "EnableShare", enableShare);
-        JsonUtil::findJSONValue(object, "HideUserList", hideUserList);
-        JsonUtil::findJSONValue(object, "SupportsLocks", supportsLocks);
-        JsonUtil::findJSONValue(object, "SupportsRename", supportsRename);
-        JsonUtil::findJSONValue(object, "UserCanRename", userCanRename);
-        bool booleanFlag = false;
-        if (JsonUtil::findJSONValue(object, "DisableChangeTrackingRecord", booleanFlag))
-            disableChangeTrackingRecord = (booleanFlag ? WOPIFileInfo::TriState::True : WOPIFileInfo::TriState::False);
-        if (JsonUtil::findJSONValue(object, "DisableChangeTrackingShow", booleanFlag))
-            disableChangeTrackingShow = (booleanFlag ? WOPIFileInfo::TriState::True : WOPIFileInfo::TriState::False);
-        if (JsonUtil::findJSONValue(object, "HideChangeTrackingControls", booleanFlag))
-            hideChangeTrackingControls = (booleanFlag ? WOPIFileInfo::TriState::True : WOPIFileInfo::TriState::False);
+        return wopiInfo;
     }
     else
     {
@@ -738,25 +643,111 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> WopiStorage::getWOPIFileInfo(const Au
 
         throw UnauthorizedRequestException("Access denied. WOPI::CheckFileInfo failed on: " + uriAnonym);
     }
+}
+
+void WopiStorage::WOPIFileInfo::init()
+{
+    _userCanWrite = false;
+    _enableOwnerTermination = false;
+    _hidePrintOption = false;
+    _hideSaveOption = false;
+    _hideExportOption = false;
+    _disablePrint = false;
+    _disableExport = false;
+    _disableCopy = false;
+    _disableInactiveMessages = false;
+    _downloadAsPostMessage = false;
+    _userCanNotWriteRelative = true;
+    _enableInsertRemoteImage = false;
+    _enableShare = false;
+    _supportsLocks = false;
+    _supportsRename = false;
+    _userCanRename = false;
+    _hideUserList = "false";
+    _disableChangeTrackingRecord = WOPIFileInfo::TriState::Unset;
+    _disableChangeTrackingShow = WOPIFileInfo::TriState::Unset;
+    _hideChangeTrackingControls = WOPIFileInfo::TriState::Unset;
+}
+
+WopiStorage::WOPIFileInfo::WOPIFileInfo(const FileInfo &fileInfo,
+                                        std::chrono::duration<double> callDuration,
+                                        Poco::JSON::Object::Ptr &object)
+{
+    init();
+
+    const std::string &filename = fileInfo.getFilename();
+    const std::string &ownerId = fileInfo.getOwnerId();
+
+    JsonUtil::findJSONValue(object, "UserId", _userId);
+    JsonUtil::findJSONValue(object, "UserFriendlyName", _username);
+    JsonUtil::findJSONValue(object, "TemplateSaveAs", _templateSaveAs);
+    JsonUtil::findJSONValue(object, "TemplateSource", _templateSource);
 
-    const std::chrono::system_clock::time_point modifiedTime = Util::iso8601ToTimestamp(lastModifiedTime, "LastModifiedTime");
-    setFileInfo(FileInfo({filename, ownerId, modifiedTime, size}));
+    std::ostringstream wopiResponse;
 
-    if (supportsLocks)
-        lockCtx.initSupportsLocks();
+    // Anonymize key values.
+    if (LOOLWSD::AnonymizeUserData)
+    {
+        JsonUtil::findJSONValue(object, "ObfuscatedUserId", _obfuscatedUserId, false);
+        if (!_obfuscatedUserId.empty())
+        {
+            Util::mapAnonymized(ownerId, _obfuscatedUserId);
+            Util::mapAnonymized(_userId, _obfuscatedUserId);
+            Util::mapAnonymized(_username, _obfuscatedUserId);
+        }
+
+        Poco::JSON::Object::Ptr anonObject(object);
+
+        // Set anonymized version of the above fields before logging.
+        // Note: anonymization caches the result, so we don't need to store here.
+        if (LOOLWSD::AnonymizeUserData)
+            anonObject->set("BaseFileName", LOOLWSD::anonymizeUrl(filename));
+
+        // If obfuscatedUserId is provided, then don't log the originals and use it.
+        if (LOOLWSD::AnonymizeUserData && _obfuscatedUserId.empty())
+        {
+            anonObject->set("OwnerId", LOOLWSD::anonymizeUsername(ownerId));
+            anonObject->set("UserId", LOOLWSD::anonymizeUsername(_userId));
+            anonObject->set("UserFriendlyName", LOOLWSD::anonymizeUsername(_username));
+        }
+        anonObject->stringify(wopiResponse);
+    }
+    else
+        object->stringify(wopiResponse);
+
+    LOG_DBG("WOPI::CheckFileInfo (" << callDuration.count() * 1000. << " ms): " << wopiResponse.str());
+
+    JsonUtil::findJSONValue(object, "UserExtraInfo", _userExtraInfo);
+    JsonUtil::findJSONValue(object, "WatermarkText", _watermarkText);
+    JsonUtil::findJSONValue(object, "UserCanWrite", _userCanWrite);
+    JsonUtil::findJSONValue(object, "PostMessageOrigin", _postMessageOrigin);
+    JsonUtil::findJSONValue(object, "HidePrintOption", _hidePrintOption);
+    JsonUtil::findJSONValue(object, "HideSaveOption", _hideSaveOption);
+    JsonUtil::findJSONValue(object, "HideExportOption", _hideExportOption);
+    JsonUtil::findJSONValue(object, "EnableOwnerTermination", _enableOwnerTermination);
+    JsonUtil::findJSONValue(object, "DisablePrint", _disablePrint);
+    JsonUtil::findJSONValue(object, "DisableExport", _disableExport);
+    JsonUtil::findJSONValue(object, "DisableCopy", _disableCopy);
+    JsonUtil::findJSONValue(object, "DisableInactiveMessages", _disableInactiveMessages);
+    JsonUtil::findJSONValue(object, "DownloadAsPostMessage", _downloadAsPostMessage);
+    JsonUtil::findJSONValue(object, "UserCanNotWriteRelative", _userCanNotWriteRelative);
+    JsonUtil::findJSONValue(object, "EnableInsertRemoteImage", _enableInsertRemoteImage);
+    JsonUtil::findJSONValue(object, "EnableShare", _enableShare);
+    JsonUtil::findJSONValue(object, "HideUserList", _hideUserList);
+    JsonUtil::findJSONValue(object, "SupportsLocks", _supportsLocks);
+    JsonUtil::findJSONValue(object, "SupportsRename", _supportsRename);
+    JsonUtil::findJSONValue(object, "UserCanRename", _userCanRename);
+    bool booleanFlag = false;
+    if (JsonUtil::findJSONValue(object, "DisableChangeTrackingRecord", booleanFlag))
+        _disableChangeTrackingRecord = (booleanFlag ? WOPIFileInfo::TriState::True : WOPIFileInfo::TriState::False);
+    if (JsonUtil::findJSONValue(object, "DisableChangeTrackingShow", booleanFlag))
+        _disableChangeTrackingShow = (booleanFlag ? WOPIFileInfo::TriState::True : WOPIFileInfo::TriState::False);
+    if (JsonUtil::findJSONValue(object, "HideChangeTrackingControls", booleanFlag))
+        _hideChangeTrackingControls = (booleanFlag ? WOPIFileInfo::TriState::True : WOPIFileInfo::TriState::False);
 
     std::string overrideWatermarks = LOOLWSD::getConfigValue<std::string>("watermark.text", "");
     if (!overrideWatermarks.empty())
-        watermarkText = overrideWatermarks;
-
-    return std::unique_ptr<WopiStorage::WOPIFileInfo>(new WOPIFileInfo(
-        {userId, obfuscatedUserId, userName, userExtraInfo, watermarkText, templateSaveAs, templateSource,
-         canWrite, postMessageOrigin, hidePrintOption, hideSaveOption, hideExportOption,
-         enableOwnerTermination, disablePrint, disableExport, disableCopy,
-         disableInactiveMessages, downloadAsPostMessage, userCanNotWriteRelative, enableInsertRemoteImage, enableShare,
-         hideUserList, disableChangeTrackingShow, disableChangeTrackingRecord,
-         hideChangeTrackingControls, supportsLocks, supportsRename,
-         userCanRename, callDuration}));
+        _watermarkText = overrideWatermarks;
 }
 
 bool WopiStorage::updateLockState(const Authorization& auth, const std::string& cookies,
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 84d77a41b..5adb2da0c 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -18,6 +18,7 @@
 #include <Poco/URI.h>
 #include <Poco/Util/Application.h>
 #include <Poco/Net/HTTPClientSession.h>
+#include <Poco/JSON/Object.h>
 
 #include "Auth.hpp"
 #include "LOOLWSD.hpp"
@@ -366,6 +367,7 @@ public:
 
     class WOPIFileInfo
     {
+        void init();
     public:
         enum class TriState
         {
@@ -374,127 +376,40 @@ public:
             Unset
         };
 
-        WOPIFileInfo(const std::string& userid,
-                     const std::string& obfuscatedUserId,
-                     const std::string& username,
-                     const std::string& userExtraInfo,
-                     const std::string& watermarkText,
-                     const std::string& templateSaveAs,
-                     const std::string& templateSource,
-                     const bool userCanWrite,
-                     const std::string& postMessageOrigin,
-                     const bool hidePrintOption,
-                     const bool hideSaveOption,
-                     const bool hideExportOption,
-                     const bool enableOwnerTermination,
-                     const bool disablePrint,
-                     const bool disableExport,
-                     const bool disableCopy,
-                     const bool disableInactiveMessages,
-                     const bool downloadAsPostMessage,
-                     const bool userCanNotWriteRelative,
-                     const bool enableInsertRemoteImage,
-                     const bool enableShare,
-                     const std::string& hideUserList,
-                     const TriState disableChangeTrackingShow,
-                     const TriState disableChangeTrackingRecord,
-                     const TriState hideChangeTrackingControls,
-                     const bool supportsLocks,
-                     const bool supportsRename,
-                     const bool userCanRename,
-                     const std::chrono::duration<double> callDuration)
-            : _userId(userid),
-              _obfuscatedUserId(obfuscatedUserId),
-              _username(username),
-              _watermarkText(watermarkText),
-              _templateSaveAs(templateSaveAs),
-              _templateSource(templateSource),
-              _userCanWrite(userCanWrite),
-              _postMessageOrigin(postMessageOrigin),
-              _hidePrintOption(hidePrintOption),
-              _hideSaveOption(hideSaveOption),
-              _hideExportOption(hideExportOption),
-              _enableOwnerTermination(enableOwnerTermination),
-              _disablePrint(disablePrint),
-              _disableExport(disableExport),
-              _disableCopy(disableCopy),
-              _disableInactiveMessages(disableInactiveMessages),
-              _downloadAsPostMessage(downloadAsPostMessage),
-              _userCanNotWriteRelative(userCanNotWriteRelative),
-              _enableInsertRemoteImage(enableInsertRemoteImage),
-              _enableShare(enableShare),
-              _hideUserList(hideUserList),
-              _disableChangeTrackingShow(disableChangeTrackingShow),
-              _disableChangeTrackingRecord(disableChangeTrackingRecord),
-              _hideChangeTrackingControls(hideChangeTrackingControls),
-              _supportsLocks(supportsLocks),
-              _supportsRename(supportsRename),
-              _userCanRename(userCanRename),
-              _callDuration(callDuration)
-            {
-                _userExtraInfo = userExtraInfo;
-            }
+        /// warning - removes items from object.
+        WOPIFileInfo(const FileInfo &fileInfo, std::chrono::duration<double> callDuration,
+                     Poco::JSON::Object::Ptr &object);
 
         const std::string& getUserId() const { return _userId; }
-
         const std::string& getUsername() const { return _username; }
-
         const std::string& getUserExtraInfo() const { return _userExtraInfo; }
-
         const std::string& getWatermarkText() const { return _watermarkText; }
-
         const std::string& getTemplateSaveAs() const { return _templateSaveAs; }
-
         const std::string& getTemplateSource() const { return _templateSource; }
-
         bool getUserCanWrite() const { return _userCanWrite; }
-
         std::string& getPostMessageOrigin() { return _postMessageOrigin; }
-
         void setHidePrintOption(bool hidePrintOption) { _hidePrintOption = hidePrintOption; }
-
         bool getHidePrintOption() const { return _hidePrintOption; }
-
         bool getHideSaveOption() const { return _hideSaveOption; }
-
         void setHideExportOption(bool hideExportOption) { _hideExportOption = hideExportOption; }
-
         bool getHideExportOption() const { return _hideExportOption; }
-
         bool getEnableOwnerTermination() const { return _enableOwnerTermination; }
-
         bool getDisablePrint() const { return _disablePrint; }
-
         bool getDisableExport() const { return _disableExport; }
-
         bool getDisableCopy() const { return _disableCopy; }
-
         bool getDisableInactiveMessages() const { return _disableInactiveMessages; }
-
         bool getDownloadAsPostMessage() const { return _downloadAsPostMessage; }
-
         bool getUserCanNotWriteRelative() const { return _userCanNotWriteRelative; }
-
         bool getEnableInsertRemoteImage() const { return _enableInsertRemoteImage; }
-
         bool getEnableShare() const { return _enableShare; }
-
         bool getSupportsRename() const { return _supportsRename; }
-
         bool getSupportsLocks() const { return _supportsLocks; }
-
         bool getUserCanRename() const { return _userCanRename; }
-
         std::string& getHideUserList() { return _hideUserList; }
-
         TriState getDisableChangeTrackingShow() const { return _disableChangeTrackingShow; }
-
         TriState getDisableChangeTrackingRecord() const { return _disableChangeTrackingRecord; }
-
         TriState getHideChangeTrackingControls() const { return _hideChangeTrackingControls; }
-
         std::chrono::duration<double> getCallDuration() const { return _callDuration; }
-
     private:
         /// User id of the user accessing the file
         std::string _userId;


More information about the Libreoffice-commits mailing list