loleaflet/reference.html | 15 +++++++++------
loleaflet/src/core/Socket.js | 4 +++-
wsd/ClientSession.cpp | 10 ++--------
wsd/DocumentBroker.cpp | 19 +++++++++++++++++++
wsd/DocumentBroker.hpp | 8 ++++++++
wsd/Storage.cpp | 1 +
wsd/Storage.hpp | 11 +++++++++++
7 files changed, 53 insertions(+), 15 deletions(-)
New commits:
commit 9f8fdb7bd7b12c1591e2c118e59ea3154844c960
Author: Samuel Mehrbrodt <samuel.mehrbr...@cib.de>
AuthorDate: Wed Jul 1 12:46:58 2020 +0200
Commit: Samuel Mehrbrodt <samuel.mehrbr...@cib.de>
CommitDate: Wed Jul 1 17:57:53 2020 +0200
tdf#131123 Report back real save result
665b1629de30a4a402c6b10dd542de158db1f428 was not correct, as it reported
back
the save result of the internal save (which usually succeeds).
Instead we want to know the save result of the remote storage (WOPI/Webdav).
So report that back instead.
Change-Id: Iaaa42b8c817a19c2c77935a6f81c1951fdf2216c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97637
Tested-by: Jenkins
Reviewed-by: Samuel Mehrbrodt <samuel.mehrbr...@cib.de>
diff --git a/loleaflet/reference.html b/loleaflet/reference.html
index b52d34688..99cae5ec6 100644
--- a/loleaflet/reference.html
+++ b/loleaflet/reference.html
@@ -3040,18 +3040,21 @@ Editor to WOPI host
<td><code>
<nobr>success: <boolean></nobr>
<nobr>result: <string></nobr>
+ <nobr>errorMsg: <string></nobr>
</code></td>
<td>Acknowledgement when save finishes.<br/>
+ <i>This response is only emitted if <code>Notify</code>
parameter
+ is mentioned by <code>Action_Save</code> PostMessage API.</i>
+ <br/>
<code>success</code> tells if LOOL was able to save the document
- successfully. When this is false, then another
- parameter, <code>result</code> is present which contains the
- reason that document was not saved.
+ successfully.<br/>
+ <code>result</code> contains the reason the document was not
saved.<br/>
In case, document was not saved because it was not modified,
then this parameter contains the string 'unmodified'. In this
case, WOPI hosts can be sure that there are no changes pending
- in the document to be saved to the storage.
- This response is only emitted if <code>Notify</code> parameter
- is mentioned by <code>Action_Save</code> PostMessage API.
+ in the document to be saved to the storage.<br/>
+ <code>errorMsg</code> contains a detailed error message in case
saving failed.
+ Probably it will contain the error message returned from the
WOPI/Webdav host.
</td>
</tr>
<tr>
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 5a5568424..c3798a774 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -365,7 +365,9 @@ L.Socket = L.Class.extend({
}
var postMessageObj = {
- success: commandresult['success']
+ success: commandresult['success'],
+ result: commandresult['result'],
+ errorMsg: commandresult['errorMsg']
};
this._map.fire('postMessage', {msgId:
'Action_Save_Resp', args: postMessageObj});
}
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 5f79a07fa..7762089aa 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -553,11 +553,8 @@ bool ClientSession::_handleInput(const char *buffer, int
length)
constexpr bool isAutosave = false;
constexpr bool isExitSave = false;
- bool result = docBroker->sendUnoSave(getId(), dontTerminateEdit !=
0, dontSaveIfUnmodified != 0,
+ docBroker->sendUnoSave(getId(), dontTerminateEdit != 0,
dontSaveIfUnmodified != 0,
isAutosave, isExitSave, extendedData);
- std::string resultstr = result ? "true" : "false";
- std::string msg = "commandresult: { \"command\": \"save\",
\"success\": " + resultstr + " }";
- docBroker->broadcastMessage(msg);
}
}
else if (tokens.equals(0, "savetostorage"))
@@ -566,10 +563,7 @@ bool ClientSession::_handleInput(const char *buffer, int
length)
if (tokens.size() > 1)
getTokenInteger(tokens[1], "force", force);
- bool result = docBroker->saveToStorage(getId(), true, "" /* This is
irrelevant when success is true*/, true);
- std::string resultstr = result ? "true" : "false";
- std::string msg = "commandresult: { \"command\": \"savetostorage\",
\"success\": " + resultstr + " }";
- docBroker->broadcastMessage(msg);
+ docBroker->saveToStorage(getId(), true, "" /* This is irrelevant when
success is true*/, true);
}
else if (tokens.equals(0, "clientvisiblearea"))
{
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 8a41a6dd9..7753d87ac 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -969,6 +969,7 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
LOG_DBG("Save skipped as document [" << _docKey << "] was not
modified.");
_lastSaveTime = std::chrono::steady_clock::now();
_poll->wakeup();
+ broadcastSaveResult(true, "unmodified");
return true;
}
@@ -978,6 +979,7 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
LOG_ERR("Session with sessionId ["
<< sessionId << "] not found while storing document docKey ["
<< _docKey
<< "]. The document will not be uploaded to storage at this
time.");
+ broadcastSaveResult(false, "Session not found");
return false;
}
@@ -986,6 +988,7 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
{
LOG_ERR("Cannot store docKey [" << _docKey << "] as .uno:Save has
failed in LOK.");
it->second->sendTextFrameAndLogError("error: cmd=storage
kind=savefailed");
+ broadcastSaveResult(false, "Could not save document in
LibreOfficeKit");
return false;
}
@@ -1021,6 +1024,7 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
LOG_DBG("Skipping unnecessary saving to URI [" << uriAnonym << "] with
docKey [" << _docKey <<
"]. File last modified " << timeInSec.count() << " seconds
ago, timestamp unchanged.");
_poll->wakeup();
+ broadcastSaveResult(true, "unmodified");
return true;
}
@@ -1103,12 +1107,14 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
{
sessionIt.second->sendTextFrameAndLogError("error: cmd=storage
kind=savediskfull");
}
+ broadcastSaveResult(false, "Disk full",
storageSaveResult.getErrorMsg());
}
else if (storageSaveResult.getResult() ==
StorageBase::SaveResult::UNAUTHORIZED)
{
LOG_ERR("Cannot save docKey [" << _docKey << "] to storage URI [" <<
uriAnonym <<
"]. Invalid or expired access token. Notifying client.");
it->second->sendTextFrameAndLogError("error: cmd=storage
kind=saveunauthorized");
+ broadcastSaveResult(false, "Invalid or expired access token",
storageSaveResult.getErrorMsg());
}
else if (storageSaveResult.getResult() == StorageBase::SaveResult::FAILED)
{
@@ -1117,6 +1123,7 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
std::ostringstream oss;
oss << "error: cmd=storage kind=" << (isRename ? "renamefailed" :
"savefailed");
it->second->sendTextFrame(oss.str());
+ broadcastSaveResult(false, "Save failed",
storageSaveResult.getErrorMsg());
}
else if (storageSaveResult.getResult() ==
StorageBase::SaveResult::DOC_CHANGED
|| storageSaveResult.getResult() ==
StorageBase::SaveResult::CONFLICT)
@@ -1127,11 +1134,23 @@ bool DocumentBroker::saveToStorageInternal(const
std::string& sessionId, bool su
= isModified() ? "error: cmd=storage kind=documentconflict" :
"close: documentconflict";
broadcastMessage(message);
+ broadcastSaveResult(false, "Conflict: Document changed in storage",
storageSaveResult.getErrorMsg());
}
return false;
}
+void DocumentBroker::broadcastSaveResult(bool success, const std::string&
result, const std::string& errorMsg)
+{
+ std::string resultstr = success ? "true" : "false";
+ // Some sane limit, otherwise we get problems transfering this to the
client with large strings (can be a whole webpage)
+ std::string errorMsgFormatted = errorMsg.substr(0, 1000);
+ // Replace reserverd characters
+ errorMsgFormatted = Poco::translate(errorMsgFormatted, "\"", "'");
+ broadcastMessage("commandresult: { \"command\": \"save\", \"success\": " +
resultstr +
+ ", \"result\": \"" + result + "\", \"errorMsg\": \"" +
errorMsgFormatted + "\"}");
+}
+
void DocumentBroker::setLoaded()
{
if (!_isLoaded)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 4d09dff32..ce5bf7b7d 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -335,6 +335,14 @@ private:
const std::string& saveAsFilename =
std::string(),
const bool isRename = false, const bool force =
false);
+ /**
+ * Report back the save result to PostMessage users (Action_Save_Resp)
+ * @param success: Whether saving was successful
+ * @param result: Short message why saving was (not) successful
+ * @param errorMsg: Long error msg (Error message from WOPI host if any)
+ */
+ void broadcastSaveResult(bool success, const std::string& result = "",
const std::string& errorMsg = "");
+
/// True iff a save is in progress (requested but not completed).
bool isSaving() const { return _lastSaveResponseTime <
_lastSaveRequestTime; }
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index 0d616f23c..e15f1abcb 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -1048,6 +1048,7 @@ WopiStorage::saveLocalFileToStorage(const Authorization&
auth, const std::string
std::ostringstream oss;
Poco::StreamCopier::copyStream(rs, oss);
std::string responseString = oss.str();
+ saveResult.setErrorMsg(responseString);
const std::string wopiLog(isSaveAs ? "WOPI::PutRelativeFile" :
(isRename ? "WOPI::RenameFile":"WOPI::PutFile"));
diff --git a/wsd/Storage.hpp b/wsd/Storage.hpp
index 8f07b9229..d087fb174 100644
--- a/wsd/Storage.hpp
+++ b/wsd/Storage.hpp
@@ -132,10 +132,21 @@ public:
return _saveAsUrl;
}
+ void setErrorMsg(const std::string &msg)
+ {
+ _errorMsg = msg;
+ }
+
+ const std::string &getErrorMsg() const
+ {
+ return _errorMsg;
+ }
+
private:
Result _result;
std::string _saveAsName;
std::string _saveAsUrl;
+ std::string _errorMsg;
};
enum class LOOLStatusCode
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
