[Libreoffice-commits] online.git: 3 commits - loleaflet/src wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/protocol.txt wsd/Storage.cpp wsd/Storage.hpp

Pranav Kant pranavk at collabora.co.uk
Thu Jun 1 14:24:56 UTC 2017


 loleaflet/src/control/Toolbar.js |    4 +++-
 loleaflet/src/core/Socket.js     |   30 ++++++++++++++++++++++++++++++
 wsd/ClientSession.cpp            |   14 ++++++++++++++
 wsd/DocumentBroker.cpp           |   17 ++++++++++++-----
 wsd/DocumentBroker.hpp           |    8 +++++++-
 wsd/Storage.cpp                  |   15 ++++++++++++---
 wsd/Storage.hpp                  |    8 +++++++-
 wsd/protocol.txt                 |   12 ++++++++++++
 8 files changed, 97 insertions(+), 11 deletions(-)

New commits:
commit 41234773e3b57e1695951bddaedb4f61ad44026d
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Thu Jun 1 19:46:03 2017 +0530

    If user permits, save to storage force-fully in case of doc conflict
    
    There is one known problem still - after any user decides to overwrite
    the file to storage, other users are not informed, so their dialog keeps
    hanging on the screen until they press the cancel or reload button.
    
    Change-Id: I6dad1585e4c53eeed79cd38316892a7f239d44ef

diff --git a/loleaflet/src/control/Toolbar.js b/loleaflet/src/control/Toolbar.js
index 681a8ecc..b5f68749 100644
--- a/loleaflet/src/control/Toolbar.js
+++ b/loleaflet/src/control/Toolbar.js
@@ -131,7 +131,9 @@ L.Map.include({
 	},
 
 	save: function(dontTerminateEdit, dontSaveIfUnmodified) {
-		this._socket.sendMessage('save dontTerminateEdit=' + (dontTerminateEdit ? 1 : 0) + ' dontSaveIfUnmodified=' + (dontSaveIfUnmodified ? 1 : 0));
+		this._socket.sendMessage('save' +
+		                         ' dontTerminateEdit=' + (dontTerminateEdit ? 1 : 0) +
+		                         ' dontSaveIfUnmodified=' + (dontSaveIfUnmodified ? 1 : 0));
 	},
 
 	sendUnoCommand: function (command, json) {
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index a65a330a..1d6b884f 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -357,7 +357,7 @@ L.Socket = L.Class.extend({
 							this.sendMessage('closedocument');
 						} else {
 							// They want to overwrite
-							this.sendMessage('documentconflict.overwrite');
+							this.sendMessage('savetostorage force=1');
 						}
 					}, this)
 				});
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 4ba8988f..7af580bb 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -136,6 +136,7 @@ bool ClientSession::_handleInput(const char *buffer, int length)
              tokens[0] != "resetselection" &&
              tokens[0] != "save" &&
              tokens[0] != "saveas" &&
+             tokens[0] != "savetostorage" &&
              tokens[0] != "selectgraphic" &&
              tokens[0] != "selecttext" &&
              tokens[0] != "setclientpart" &&
@@ -221,6 +222,12 @@ bool ClientSession::_handleInput(const char *buffer, int length)
         getTokenInteger(tokens[2], "dontSaveIfUnmodified", dontSaveIfUnmodified);
         docBroker->sendUnoSave(getId(), dontTerminateEdit != 0, dontSaveIfUnmodified != 0);
     }
+    else if (tokens[0] == "savetostorage")
+    {
+        int force = 0;
+        getTokenInteger(tokens[1], "force", force);
+        docBroker->saveToStorage(getId(), true, "" /* This is irrelevant when success is true*/, true);
+    }
     else
     {
         if (!filterMessage(firstLine))
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 9b282165..981834fa 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -544,10 +544,15 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
 }
 
 bool DocumentBroker::saveToStorage(const std::string& sessionId,
-                                   bool success, const std::string& result)
+                                   bool success, const std::string& result, bool force)
 {
     assertCorrectThread();
 
+    if (force)
+    {
+        LOG_TRC("Document will be saved forcefully to storage.");
+        _storage->forceSave();
+    }
     const bool res = saveToStorageInternal(sessionId, success, result);
 
     // If marked to destroy, or session is disconnected, remove.
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 588df3fd..7f563647 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -240,7 +240,7 @@ public:
     bool isDocumentChangedInStorage() { return _documentChangedInStorage; }
 
     /// Save the document to Storage if it needs persisting.
-    bool saveToStorage(const std::string& sesionId, bool success, const std::string& result = "");
+    bool saveToStorage(const std::string& sesionId, bool success, const std::string& result = "", bool force = false);
     bool isModified() const { return _isModified; }
     void setModified(const bool value);
     /// Save the document if the document is modified.
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 7c677a6d..af31674e 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -668,13 +668,14 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& a
 
         Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
         request.set("X-WOPI-Override", "PUT");
-        if (!_forceOverwrite)
+        if (!_forceSave)
         {
             // Request WOPI host to not overwrite if timestamps mismatch
             request.set("X-LOOL-WOPI-Timestamp",
                         Poco::DateTimeFormatter::format(Poco::DateTime(_fileInfo._modifiedTime),
                                                         Poco::DateTimeFormat::ISO8601_FRAC_FORMAT));
         }
+
         request.setContentType("application/octet-stream");
         request.setContentLength(size);
         addStorageDebugCookie(request);
@@ -699,6 +700,10 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& a
                 const std::string lastModifiedTime = getJSONValue<std::string>(object, "LastModifiedTime");
                 LOG_TRC("WOPI::PutFile returns LastModifiedTime [" << lastModifiedTime << "].");
                 _fileInfo._modifiedTime = iso8601ToTimestamp(lastModifiedTime);
+
+                // Reset the force save flag now, if any, since we are done saving
+                // Next saves shouldn't be saved forcefully unless commanded
+                _forceSave = false;
             }
             else
             {
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index bb25a2d7..60c73275 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -78,7 +78,7 @@ public:
         _jailPath(jailPath),
         _fileInfo("", "lool", Poco::Timestamp::fromEpochTime(0), 0),
         _isLoaded(false),
-        _forceOverwrite(false)
+        _forceSave(false)
     {
         LOG_DBG("Storage ctor: " << uri.toString());
     }
@@ -92,7 +92,7 @@ public:
 
     /// Asks the storage object to force overwrite to storage upon next save
     /// even if document turned out to be changed in storage
-    void forceOverwrite() { _forceOverwrite = true; }
+    void forceSave() { _forceSave = true; }
 
     /// Returns the basic information about the file.
     FileInfo getFileInfo() { return _fileInfo; }
@@ -125,7 +125,7 @@ protected:
     std::string _jailedFilePath;
     FileInfo _fileInfo;
     bool _isLoaded;
-    bool _forceOverwrite;
+    bool _forceSave;
 
     static bool FilesystemEnabled;
     static bool WopiEnabled;
diff --git a/wsd/protocol.txt b/wsd/protocol.txt
index b0617b2f..e3bf12ab 100644
--- a/wsd/protocol.txt
+++ b/wsd/protocol.txt
@@ -160,6 +160,14 @@ save dontTerminateEdit=<value> dontSaveIfUnmodified=<value>
     non-zero. 'dontSaveIfUnmodified' when set to non-zero skips saving the document when it is
     unmodified.
 
+savetostorage force=<value>
+
+    Saves the files to storage. A 'save' command automatically saves the file to
+    storage almost always except in the case where there is a document conflict
+    i.e document in the storage is changed behind our back. We might need to do
+    call this command with force=1 in that case to forcefully save it to storage
+    after asking from the user.
+
 clientvisiblearea x=<x> y=<y> width=<width> height=<height>
 
     Invokes lok::Document::setClientVisibleArea().
commit 4d61cae4c8b6ecc7890fb0426c9600e1cf77bbdb
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Thu Jun 1 18:26:54 2017 +0530

    If user commands, refresh the document for all in case of doc conflict
    
    Change-Id: I42c61fb8099b0bcc60f942e602561cc97486a918

diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 37b67fd4..a65a330a 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -266,6 +266,21 @@ L.Socket = L.Class.extend({
 					}
 				}, timeoutMs);
 			}
+			else if (textMsg.startsWith('documentconflict')) {
+				var username = textMsg.substring('documentconflict '.length);
+				msg = _('%user asked to refresh the document. Document will now refresh automatically.').replace('%user', username);
+
+				// Reload the document
+				this._map._active = false;
+				map = this._map;
+				vex.timer = setInterval(function() {
+					try {
+						// Activate and cancel timer and dialogs.
+						map._activate();
+					} catch (error) {
+					}
+				}, 3000);
+			}
 
 			// Close any open dialogs first.
 			if (vex.dialogID > 0) {
@@ -334,6 +349,21 @@ L.Socket = L.Class.extend({
 			else if (command.errorKind === 'documentconflict')
 			{
 				storageError = errorMessages.storage.documentconflict;
+				vex.dialog.confirm({
+					message: _('Document has been changed in storage. Do you want to refresh the page to load the new document ? Cancelling will continue editing and overwrite.'),
+					callback: L.bind(function(value) {
+						if (value) {
+							// They want to refresh the page and load document again for all
+							this.sendMessage('closedocument');
+						} else {
+							// They want to overwrite
+							this.sendMessage('documentconflict.overwrite');
+						}
+					}, this)
+				});
+				vex.dialogID = vex.globalID - 1;
+
+				return;
 			}
 
 			// Parse the storage url as link
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 21b23d12..4ba8988f 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -173,6 +173,13 @@ bool ClientSession::_handleInput(const char *buffer, int length)
             LOG_DBG("Session [" << getId() << "] requested owner termination");
             docBroker->closeDocument("ownertermination");
         }
+        else if (docBroker->isDocumentChangedInStorage())
+        {
+            LOG_DBG("Document marked as changed in storage and user ["
+                    << getUserId() << ", " << getUserName()
+                    << "] wants to refresh the document for all.");
+            docBroker->closeDocument("documentconflict " + getUserName());
+        }
 
         return true;
     }
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index ee674f36..9b282165 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -147,6 +147,7 @@ DocumentBroker::DocumentBroker(const std::string& uri,
     _docId(Util::encodeId(DocBrokerId++, 3)),
     _childRoot(childRoot),
     _cacheRoot(getCachePath(uriPublic.toString())),
+    _documentChangedInStorage(false),
     _lastSaveTime(std::chrono::steady_clock::now()),
     _lastSaveRequestTime(std::chrono::steady_clock::now() - std::chrono::milliseconds(COMMAND_TIMEOUT_MS)),
     _markToDestroy(false),
@@ -495,6 +496,7 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
             _documentLastModifiedTime != fileInfo._modifiedTime)
         {
             LOG_WRN("Document [" << _docKey << "] has been modified behind our back. Informing all clients.");
+            _documentChangedInStorage = true;
             // Inform all clients
             for (const auto& sessionIt : _sessions)
             {
@@ -652,7 +654,7 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
     else if (storageSaveResult == StorageBase::SaveResult::DOC_CHANGED)
     {
         LOG_ERR("PutFile says that Document changed in storage");
-
+        _documentChangedInStorage = true;
         // Inform all clients
         for (const auto& sessionIt : _sessions)
         {
@@ -1334,7 +1336,7 @@ bool DocumentBroker::forwardToClient(const std::shared_ptr<Message>& payload)
 void DocumentBroker::shutdownClients(const std::string& closeReason)
 {
     assertCorrectThread();
-    LOG_INF("Terminating " << _sessions.size() << " clients of doc [" << _docKey << "].");
+    LOG_INF("Terminating " << _sessions.size() << " clients of doc [" << _docKey << "] with reason: " << closeReason);
 
     // First copy into local container, since removeSession
     // will erase from _sessions, but will leave the last.
@@ -1376,7 +1378,7 @@ void DocumentBroker::terminateChild(const std::string& closeReason, const bool r
 {
     assertCorrectThread();
 
-    LOG_INF("Terminating doc [" << _docKey << "].");
+    LOG_INF("Terminating doc [" << _docKey << "] with reason: " << closeReason);
 
     // Close all running sessions
     if (!rude)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 27d525c0..588df3fd 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -237,6 +237,8 @@ public:
     bool isLoaded() const { return _isLoaded; }
     void setLoaded();
 
+    bool isDocumentChangedInStorage() { return _documentChangedInStorage; }
+
     /// Save the document to Storage if it needs persisting.
     bool saveToStorage(const std::string& sesionId, bool success, const std::string& result = "");
     bool isModified() const { return _isModified; }
@@ -376,6 +378,10 @@ private:
     std::string _jailId;
     std::string _filename;
 
+    /// Set to true when document changed in storage and we are waiting
+    /// for user's command to act.
+    bool _documentChangedInStorage;
+
     /// The last time we tried saving, regardless of whether the
     /// document was modified and saved or not.
     std::chrono::steady_clock::time_point _lastSaveTime;
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 394b9136..7c677a6d 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -668,9 +668,13 @@ StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const std::string& a
 
         Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, uriObject.getPathAndQuery(), Poco::Net::HTTPMessage::HTTP_1_1);
         request.set("X-WOPI-Override", "PUT");
-        request.set("X-LOOL-WOPI-Timestamp",
-                    Poco::DateTimeFormatter::format(Poco::DateTime(_fileInfo._modifiedTime),
-                                                    Poco::DateTimeFormat::ISO8601_FRAC_FORMAT));
+        if (!_forceOverwrite)
+        {
+            // Request WOPI host to not overwrite if timestamps mismatch
+            request.set("X-LOOL-WOPI-Timestamp",
+                        Poco::DateTimeFormatter::format(Poco::DateTime(_fileInfo._modifiedTime),
+                                                        Poco::DateTimeFormat::ISO8601_FRAC_FORMAT));
+        }
         request.setContentType("application/octet-stream");
         request.setContentLength(size);
         addStorageDebugCookie(request);
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 89cc4de9..bb25a2d7 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -77,7 +77,8 @@ public:
         _localStorePath(localStorePath),
         _jailPath(jailPath),
         _fileInfo("", "lool", Poco::Timestamp::fromEpochTime(0), 0),
-        _isLoaded(false)
+        _isLoaded(false),
+        _forceOverwrite(false)
     {
         LOG_DBG("Storage ctor: " << uri.toString());
     }
@@ -89,6 +90,10 @@ public:
 
     bool isLoaded() const { return _isLoaded; }
 
+    /// Asks the storage object to force overwrite to storage upon next save
+    /// even if document turned out to be changed in storage
+    void forceOverwrite() { _forceOverwrite = true; }
+
     /// Returns the basic information about the file.
     FileInfo getFileInfo() { return _fileInfo; }
 
@@ -120,6 +125,7 @@ protected:
     std::string _jailedFilePath;
     FileInfo _fileInfo;
     bool _isLoaded;
+    bool _forceOverwrite;
 
     static bool FilesystemEnabled;
     static bool WopiEnabled;
diff --git a/wsd/protocol.txt b/wsd/protocol.txt
index 89c2ce43..b0617b2f 100644
--- a/wsd/protocol.txt
+++ b/wsd/protocol.txt
@@ -269,6 +269,10 @@ close: <reason>
     shutting down to let clients know they can try connecting
     after a short interval.
 
+    * documentconflict <user name> - All sessions of this document are going down
+    because file was changed in storage and one of the user ( with <user
+    name>) asked to reload the session for all.
+
 getchildid: id=<id>
 
     Returns the child id
commit d6ef241cf581f981a0b7a7cfcd60f0e62546e633
Author: Pranav Kant <pranavk at collabora.co.uk>
Date:   Thu Jun 1 18:16:20 2017 +0530

    Don't be so rude
    
    If we are rude, then we don't tell the reason behind closing the
    document to our clients.
    
    This method earlier was used to do 'ownertermination', but without this
    patch, ownertermination is not going to work. We regressed somewhere in
    the past.
    
    Change-Id: I7a2513e567f72b1adf00d5a74b118e116d6d99d3

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a97e34da..ee674f36 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1406,7 +1406,7 @@ void DocumentBroker::closeDocument(const std::string& reason)
     assertCorrectThread();
 
     LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with reason: " << reason);
-    terminateChild(reason, true);
+    terminateChild(reason, false);
 }
 
 void DocumentBroker::updateLastActivityTime()


More information about the Libreoffice-commits mailing list