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

[Samuel Mehrbrodt (via logerrit)]

 wsd/ClientSession.cpp  |    6 ++++++
 wsd/DocumentBroker.cpp |    7 +++++++
 wsd/DocumentBroker.hpp |    5 +++++
 3 files changed, 18 insertions(+)

New commits:
commit 8c602e179e8afe44f2a1fe513bdc7640cd57ed2a
Author:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
AuthorDate: Mon Aug 17 11:00:38 2020 +0200
Commit:     Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Mon Aug 24 14:34:10 2020 +0200

    Revert "Revert "Don't update modified status after saving to storage fails""
    
    This reverts commit e83e36bd9b96fe38ac1f53f56d9754e26bd131e0.
    
    Unit test failure was fixed
    
    Change-Id: I2176368278725c1711df3b23eef95de6526c68d5
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100859
    Tested-by: Jenkins
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 7c4ac6fdb..60733a0c5 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -1317,6 +1317,12 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
         StringVector stateTokens(Util::tokenize(tokens[1], '='));
         if (stateTokens.size() == 2 && stateTokens.equals(0, ".uno:ModifiedStatus"))
         {
+            // When the document is saved internally, but saving to storage failed,
+            // don't update the client's modified status
+            // (otherwise client thinks document is unmodified b/c saving was successful)
+            if (!docBroker->isLastStorageSaveSuccessful())
+                return false;
+
             docBroker->setModified(stateTokens.equals(1, "true"));
         }
         else
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a5e62fcc4..4218b9fb4 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -176,6 +176,7 @@ DocumentBroker::DocumentBroker(ChildType type,
     _docKey(docKey),
     _docId(Util::encodeId(DocBrokerId++, 3)),
     _documentChangedInStorage(false),
+    _lastStorageSaveSuccessful(true),
     _lastSaveTime(std::chrono::steady_clock::now()),
     _lastSaveRequestTime(std::chrono::steady_clock::now() - std::chrono::milliseconds(COMMAND_TIMEOUT_MS)),
     _markToDestroy(false),
@@ -1049,6 +1050,11 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId, bool su
     assert(_storage && _tileCache);
     const StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(
         auth, it->second->getCookies(), *_lockCtx, saveAsPath, saveAsFilename, isRename);
+    // Storage save is considered successful when either storage returns OK or the document on the storage
+    // was changed and it was used to overwrite local changes
+    _lastStorageSaveSuccessful
+        = storageSaveResult.getResult() == StorageBase::SaveResult::OK ||
+        storageSaveResult.getResult() == StorageBase::SaveResult::DOC_CHANGED;
     if (storageSaveResult.getResult() == StorageBase::SaveResult::OK)
     {
 #if !MOBILEAPP
@@ -2474,6 +2480,7 @@ void DocumentBroker::dumpState(std::ostream& os)
     os << "\n  last saved: " << Util::getSteadyClockAsString(_lastSaveTime);
     os << "\n  last save request: " << Util::getSteadyClockAsString(_lastSaveRequestTime);
     os << "\n  last save response: " << Util::getSteadyClockAsString(_lastSaveResponseTime);
+    os << "\n  last storage save was successful: " << isLastStorageSaveSuccessful();
     os << "\n  last modified: " << Util::getHttpTime(_documentLastModifiedTime);
     os << "\n  file last modified: " << Util::getHttpTime(_lastFileModifiedTime);
     if (_limitLifeSeconds)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index b8e176b3c..e59a2d6b4 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -175,6 +175,8 @@ public:
 
     bool isDocumentChangedInStorage() { return _documentChangedInStorage; }
 
+    bool isLastStorageSaveSuccessful() { return _lastStorageSaveSuccessful; }
+
     /// Save the document to Storage if it needs persisting.
     bool saveToStorage(const std::string& sesionId, bool success, const std::string& result = "", bool force = false);
 
@@ -404,6 +406,9 @@ private:
     /// for user's command to act.
     bool _documentChangedInStorage;
 
+    /// Indicates whether the last saveToStorage operation was successful.
+    bool _lastStorageSaveSuccessful;
+
     /// The last time we tried saving, regardless of whether the
     /// document was modified and saved or not.
     std::chrono::steady_clock::time_point _lastSaveTime;