[Libreoffice-commits] online.git: 2 commits - loleaflet/dist loleaflet/src loolwsd/DocumentBroker.cpp loolwsd/PrisonerSession.cpp loolwsd/protocol.txt loolwsd/Storage.cpp loolwsd/Storage.hpp
Pranav Kant
pranavk at collabora.co.uk
Wed Nov 23 12:31:53 UTC 2016
loleaflet/dist/errormessages.js | 5 +++++
loleaflet/src/control/Control.Dialog.js | 4 +++-
loleaflet/src/core/Socket.js | 11 +++++++++++
loolwsd/DocumentBroker.cpp | 17 ++++++++++++++++-
loolwsd/PrisonerSession.cpp | 2 +-
loolwsd/Storage.cpp | 23 ++++++++++++++++-------
loolwsd/Storage.hpp | 15 +++++++++++----
loolwsd/protocol.txt | 11 ++++++++---
8 files changed, 71 insertions(+), 17 deletions(-)
New commits:
commit a149a0c2e164bd42677aec3a50c9a8a0421d1105
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed Nov 23 17:52:10 2016 +0530
tdf#103679: Don't always change document permission on error
Have a new event 'warn' which doesn't change the document
permission but just show the user dialog with some message.
Change-Id: I455168e4f7315acdcccfb31fc8c70b86bbc6caad
diff --git a/loleaflet/src/control/Control.Dialog.js b/loleaflet/src/control/Control.Dialog.js
index 41836ae..733c5cd 100644
--- a/loleaflet/src/control/Control.Dialog.js
+++ b/loleaflet/src/control/Control.Dialog.js
@@ -5,11 +5,13 @@
/* global vex */
L.Control.Dialog = L.Control.extend({
onAdd: function (map) {
+ // TODO: Better distinction between warnings and errors
map.on('error', this._onError, this);
+ map.on('warn', this._onError, this);
map.on('print', this._onPrint, this);
},
- _onError: function (e) {
+ _onError: function(e) {
if (vex.dialogID > 0 && !this._map._fatal) {
// TODO. queue message errors and pop-up dialogs
// Close other dialogs before presenting a new one.
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index 5f72068..5c0d834 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -241,11 +241,11 @@ L.Socket = L.Class.extend({
}
else if (textMsg.startsWith('error:') && command.errorCmd === 'storage') {
if (command.errorKind === 'savediskfull') {
- // Just warn the user
this._map.fire('error', {msg: errorMessages.storage.savediskfull});
}
else if (command.errorKind === 'savefailed') {
- this._map.fire('error', {msg: errorMessages.storage.savefailed});
+ // Just warn the user
+ this._map.fire('warn', {msg: errorMessages.storage.savefailed});
}
return;
commit f8e0b8c11e94810fdde73f8b4d27b80a989670cb
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Wed Nov 23 17:39:54 2016 +0530
tdf#103679: Handle storage diskfull & other PutFile errors
The new behavior is to warn the user when we try to save to
storage and set all the sessions of
the opened document to readonly, if storage server has no disk
space left. In case of WOPI, this is intimated by HTTP response code 413 -
request entity too large.
If save operation to storage failed due to reasons other than
413, just warn the user and let it continue editing the document.
We can add more reasons of failure and act accordingly in future.
Change-Id: I4b046fc38bbc0d752c89d90acb5991a958b76670
diff --git a/loleaflet/dist/errormessages.js b/loleaflet/dist/errormessages.js
index 348c6c1..036f965 100644
--- a/loleaflet/dist/errormessages.js
+++ b/loleaflet/dist/errormessages.js
@@ -4,3 +4,8 @@ exports.limitreached = _('This development build is limited to %0 documents, and
exports.serviceunavailable = _('Service is unavailable. Please try again later and report to your administrator if the issue persists.');
exports.unauthorized = _('Unauthorized WOPI host. Please try again later and report to your administrator if the issue persists.');
exports.wrongwopisrc = _('Wrong WOPISrc, usage: WOPISrc=valid encoded URI, or file_path, usage: file_path=/path/to/doc/');
+
+exports.storage = {
+ savediskfull: _('Save failed due to no disk space left on storage server. Document will now be read-only. Please contact the server administrator to continue editing.'),
+ savefailed: _('Document cannot be saved to storage. Check your permissions or contact the storage server administrator.')
+};
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index d88961b..5f72068 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -239,6 +239,17 @@ L.Socket = L.Class.extend({
return;
}
+ else if (textMsg.startsWith('error:') && command.errorCmd === 'storage') {
+ if (command.errorKind === 'savediskfull') {
+ // Just warn the user
+ this._map.fire('error', {msg: errorMessages.storage.savediskfull});
+ }
+ else if (command.errorKind === 'savefailed') {
+ this._map.fire('error', {msg: errorMessages.storage.savefailed});
+ }
+
+ return;
+ }
else if (textMsg.startsWith('error:') && command.errorCmd === 'internal') {
this._map._fatal = true;
if (command.errorKind === 'diskfull') {
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 14bbd4b..fcd4d7a 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -366,7 +366,8 @@ bool DocumentBroker::save(const std::string& sessionId, bool success, const std:
LOG_DBG("Saving to URI [" << uri << "].");
assert(_storage && _tileCache);
- if (_storage->saveLocalFileToStorage(uriPublic))
+ StorageBase::SaveResult storageSaveResult = _storage->saveLocalFileToStorage(uriPublic);
+ if (storageSaveResult == StorageBase::SaveResult::OK)
{
_isModified = false;
_tileCache->setUnsavedChanges(false);
@@ -377,6 +378,20 @@ bool DocumentBroker::save(const std::string& sessionId, bool success, const std:
_saveCV.notify_all();
return true;
}
+ else if (storageSaveResult == StorageBase::SaveResult::DISKFULL)
+ {
+ // Make everyone readonly and tell everyone that storage is low on diskspace
+ for (auto& sessionIt : _sessions)
+ {
+ sessionIt.second->setReadOnly();
+ sessionIt.second->sendTextFrame("perm: readonly");
+ sessionIt.second->sendTextFrame("error: cmd=storage kind=savediskfull");
+ }
+ }
+ else if (storageSaveResult == StorageBase::SaveResult::FAILED)
+ {
+ it->second->sendTextFrame("error: cmd=storage kind=savefailed");
+ }
LOG_ERR("Failed to save to URI [" << uri << "].");
return false;
diff --git a/loolwsd/PrisonerSession.cpp b/loolwsd/PrisonerSession.cpp
index bb352cc..28e4ac6 100644
--- a/loolwsd/PrisonerSession.cpp
+++ b/loolwsd/PrisonerSession.cpp
@@ -87,7 +87,7 @@ bool PrisonerSession::_handleInput(const char *buffer, int length)
}
if (!_docBroker->save(getId(), success, result))
- peer->sendTextFrame("error: cmd=internal kind=diskfull");
+ LOG_ERR("Saving document to storage failed.");
return true;
}
}
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index 51abe69..21800ae 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -251,7 +251,7 @@ std::string LocalStorage::loadStorageFileToLocal()
return Poco::Path(_jailPath, filename).toString();
}
-bool LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
+StorageBase::SaveResult LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
{
try
{
@@ -269,7 +269,7 @@ bool LocalStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
throw;
}
- return true;
+ return StorageBase::SaveResult::OK;
}
namespace {
@@ -472,7 +472,7 @@ std::string WopiStorage::loadStorageFileToLocal()
return Poco::Path(_jailPath, _fileInfo._filename).toString();
}
-bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
+StorageBase::SaveResult WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
{
LOG_INF("Uploading URI [" << uriPublic.toString() << "] from [" << _jailedFilePath + "].");
// TODO: Check if this URI has write permission (canWrite = true)
@@ -499,12 +499,21 @@ bool WopiStorage::saveLocalFileToStorage(const Poco::URI& uriPublic)
Poco::StreamCopier::copyStream(rs, oss);
LOG_INF("WOPI::PutFile response: " << oss.str());
- const auto success = (response.getStatus() == Poco::Net::HTTPResponse::HTTP_OK);
LOG_INF("WOPI::PutFile uploaded " << size << " bytes from [" << _jailedFilePath <<
"] -> [" << uriObject.toString() << "]: " <<
response.getStatus() << " " << response.getReason());
- return success;
+ StorageBase::SaveResult saveResult = StorageBase::SaveResult::FAILED;
+ if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_OK)
+ {
+ saveResult = StorageBase::SaveResult::OK;
+ }
+ else if (response.getStatus() == Poco::Net::HTTPResponse::HTTP_REQUESTENTITYTOOLARGE)
+ {
+ saveResult = StorageBase::SaveResult::DISKFULL;
+ }
+
+ return saveResult;
}
std::string WebDAVStorage::loadStorageFileToLocal()
@@ -514,10 +523,10 @@ std::string WebDAVStorage::loadStorageFileToLocal()
return _uri.toString();
}
-bool WebDAVStorage::saveLocalFileToStorage(const Poco::URI& /*uriPublic*/)
+StorageBase::SaveResult WebDAVStorage::saveLocalFileToStorage(const Poco::URI& /*uriPublic*/)
{
// TODO: implement webdav PUT.
- return false;
+ return StorageBase::SaveResult::OK;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index d116eec..684488f 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -52,6 +52,13 @@ public:
size_t _size;
};
+ enum SaveResult
+ {
+ OK,
+ DISKFULL,
+ FAILED
+ };
+
/// localStorePath the absolute root path of the chroot.
/// jailPath the path within the jail that the child uses.
StorageBase(const Poco::URI& uri,
@@ -80,7 +87,7 @@ public:
virtual std::string loadStorageFileToLocal() = 0;
/// Writes the contents of the file back to the source.
- virtual bool saveLocalFileToStorage(const Poco::URI& uriPublic) = 0;
+ virtual SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) = 0;
static size_t getFileSize(const std::string& filename);
@@ -140,7 +147,7 @@ public:
std::string loadStorageFileToLocal() override;
- bool saveLocalFileToStorage(const Poco::URI& uriPublic) override;
+ SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override;
private:
/// True if the jailed file is not linked but copied.
@@ -214,7 +221,7 @@ public:
/// uri format: http://server/<...>/wopi*/files/<id>/content
std::string loadStorageFileToLocal() override;
- bool saveLocalFileToStorage(const Poco::URI& uriPublic) override;
+ SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override;
/// Total time taken for making WOPI calls during load
std::chrono::duration<double> getWopiLoadDuration() const { return _wopiLoadDuration; }
@@ -244,7 +251,7 @@ public:
std::string loadStorageFileToLocal() override;
- bool saveLocalFileToStorage(const Poco::URI& uriPublic) override;
+ SaveResult saveLocalFileToStorage(const Poco::URI& uriPublic) override;
private:
std::unique_ptr<AuthBase> _authAgent;
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index c316b32..5ac2e0e 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -220,9 +220,14 @@ error: cmd=<command> kind=<kind> [code=<error_code>] [params=1,2,3,...,N]
<freeErrorText>
<command> is the command part of the corresponding client->server
- message that caused the error. <command> can also be 'internal'
- for errors generated without directly corresponding to a client
- message. <kind> is some single-word classification
+ message that caused the error.
+
+ <command> can also take following values:
+ 'internal': for errors generated without directly corresponding to a client
+ message.
+ 'storage': for errors pertaining to storage (filesysytem, wopi etc.)
+
+ <kind> is some single-word classification
<code> (when provided) further specifies the error as forwarded from
LibreOffice
More information about the Libreoffice-commits
mailing list