[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-3-0' - 3 commits - test/Makefile.am test/UnitWopiOwnertermination.cpp test/WopiTestServer.hpp wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp
Pranav Kant
pranavk at collabora.co.uk
Thu Feb 15 13:12:27 UTC 2018
test/Makefile.am | 6 +-
test/UnitWopiOwnertermination.cpp | 89 ++++++++++++++++++++++++++++++++++++++
test/WopiTestServer.hpp | 1
wsd/ClientSession.cpp | 2
wsd/DocumentBroker.cpp | 14 +++--
wsd/DocumentBroker.hpp | 4 +
6 files changed, 107 insertions(+), 9 deletions(-)
New commits:
commit d53d1dd467ef897d6a77d696a08a5470e127dc3c
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Fri Feb 2 13:17:45 2018 +0530
Document stop() vs closeDocument()
Change-Id: I52483c0300d1d4c5b1def80b71ccd0661590bfab
Reviewed-on: https://gerrit.libreoffice.org/49753
Reviewed-by: Andras Timar <andras.timar at collabora.com>
Tested-by: Andras Timar <andras.timar at collabora.com>
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index a5345f67..443d8ab5 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -226,7 +226,7 @@ public:
/// Start processing events
void startThread();
- /// Flag for termination.
+ /// Flag for termination. Note that this doesn't save any unsaved changes in the document
void stop(const std::string& reason);
/// Thread safe termination of this broker if it has a lingering thread
@@ -321,6 +321,7 @@ public:
int getRenderedTileCount() { return _debugRenderedTileCount; }
+ /// Ask the document broker to close. Makes sure that the document is saved.
void closeDocument(const std::string& reason);
/// Called by the ChildProcess object to notify
commit 8a3f8bd352dffccc078f6fa9acf63dac13b0d5c4
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Fri Feb 2 13:08:25 2018 +0530
wsd: Don't request closing here, just stop the broker
Asking the doc broker to close means that it will save the current
document to storage before shutting down. But we don't want that because
storing the current document to storage means overwriting the document
which has been changed underneath us.
Change-Id: I8df45d2f0cf3f65936dc3030f483e52fd1703146
Reviewed-on: https://gerrit.libreoffice.org/49752
Reviewed-by: Andras Timar <andras.timar at collabora.com>
Tested-by: Andras Timar <andras.timar at collabora.com>
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 8d7ea60e..e23ec418 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -190,7 +190,7 @@ bool ClientSession::_handleInput(const char *buffer, int length)
LOG_DBG("Document marked as changed in storage and user ["
<< getUserId() << ", " << getUserName()
<< "] wants to refresh the document for all.");
- docBroker->closeDocument("documentconflict " + getUserName());
+ docBroker->stop("documentconflict " + getUserName());
}
return true;
commit cdaf6bdfb04b2371fc84a5a71b60fcf017c81297
Author: Pranav Kant <pranavk at collabora.co.uk>
Date: Fri Feb 2 11:34:48 2018 +0530
wsd: Use a close request flag to break out of doc broker poll thread
... instead of directly stop()ing it. The close request approach also
makes sure that outgoing document is saved to storage.
Change-Id: I44f61db00dbd326dec80f59f4a2cbb617048aa94
Reviewed-on: https://gerrit.libreoffice.org/49751
Reviewed-by: Andras Timar <andras.timar at collabora.com>
Tested-by: Andras Timar <andras.timar at collabora.com>
diff --git a/test/Makefile.am b/test/Makefile.am
index aa54e0cb..4808573a 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -17,7 +17,7 @@ noinst_LTLIBRARIES = \
unit-storage.la unit-client.la \
unit-admin.la unit-tilecache.la \
unit-fuzz.la unit-oob.la unit-oauth.la \
- unit-wopi.la unit-wopi-saveas.la
+ unit-wopi.la unit-wopi-saveas.la unit-wopi-ownertermination.la
MAGIC_TO_FORCE_SHLIB_CREATION = -rpath /dummy
AM_LDFLAGS = -pthread -module $(MAGIC_TO_FORCE_SHLIB_CREATION) $(ZLIB_LIBS)
@@ -82,6 +82,8 @@ unit_wopi_la_SOURCES = UnitWOPI.cpp
unit_wopi_la_LIBADD = $(CPPUNIT_LIBS)
unit_wopi_saveas_la_SOURCES = UnitWOPISaveAs.cpp
unit_wopi_saveas_la_LIBADD = $(CPPUNIT_LIBS)
+unit_wopi_ownertermination_la_SOURCES = UnitWopiOwnertermination.cpp
+unit_wopi_ownertermination_la_LIBADD = $(CPPUNIT_LIBS)
if HAVE_LO_PATH
SYSTEM_STAMP = @SYSTEMPLATE_PATH@/system_stamp
@@ -95,7 +97,7 @@ check-local:
./run_unit.sh --log-file test.log --trs-file test.trs
# FIXME 2: unit-oob.la fails with symbol undefined:
# UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) ,
-TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la unit-oauth.la unit-wopi.la unit-wopi-saveas.la
+TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la unit-oauth.la unit-wopi.la unit-wopi-saveas.la unit-wopi-ownertermination.la
# TESTS = unit-client.la
# TESTS += unit-admin.la
# TESTS += unit-storage.la
diff --git a/test/UnitWopiOwnertermination.cpp b/test/UnitWopiOwnertermination.cpp
new file mode 100644
index 00000000..d356ab0e
--- /dev/null
+++ b/test/UnitWopiOwnertermination.cpp
@@ -0,0 +1,89 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include "config.h"
+
+#include "WopiTestServer.hpp"
+#include "Log.hpp"
+#include "Unit.hpp"
+#include "UnitHTTP.hpp"
+#include "helpers.hpp"
+#include <Poco/Net/HTTPRequest.h>
+#include <Poco/Util/LayeredConfiguration.h>
+
+class UnitWopiOwnertermination : public WopiTestServer
+{
+ enum class Phase
+ {
+ Load,
+ Modify,
+ OwnerTermination,
+ Polling
+ } _phase;
+
+public:
+ UnitWopiOwnertermination() :
+ _phase(Phase::Load)
+ {
+ }
+
+ void assertPutFileRequest(const Poco::Net::HTTPRequest& /*request*/) override
+ {
+ if (_phase == Phase::Polling)
+ {
+ // Document got saved, that's what we wanted
+ exitTest(TestResult::Ok);
+ }
+ }
+
+ void invokeTest() override
+ {
+ constexpr char testName[] = "UnitWopiOwnertermination";
+
+ switch (_phase)
+ {
+ case Phase::Load:
+ {
+ initWebsocket("/wopi/files/0?access_token=anything");
+
+ helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "load url=" + _wopiSrc, testName);
+
+ _phase = Phase::Modify;
+ break;
+ }
+ case Phase::Modify:
+ {
+ helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "key type=input char=97 key=0", testName);
+ helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "key type=up char=0 key=512", testName);
+
+ _phase = Phase::OwnerTermination;
+ SocketPoll::wakeupWorld();
+ break;
+ }
+ case Phase::OwnerTermination:
+ {
+ _phase = Phase::Polling;
+ helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "closedocument", testName);
+ break;
+ }
+ case Phase::Polling:
+ {
+ // just wait for the results
+ break;
+ }
+ }
+ }
+};
+
+UnitBase *unit_create_wsd(void)
+{
+ return new UnitWopiOwnertermination();
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/WopiTestServer.hpp b/test/WopiTestServer.hpp
index b20aa032..f5cc4c82 100644
--- a/test/WopiTestServer.hpp
+++ b/test/WopiTestServer.hpp
@@ -94,6 +94,7 @@ protected:
fileInfo->set("UserCanWrite", "true");
fileInfo->set("PostMessageOrigin", "localhost");
fileInfo->set("LastModifiedTime", Poco::DateTimeFormatter::format(now, Poco::DateTimeFormat::ISO8601_FORMAT));
+ fileInfo->set("EnableOwnerTermination", "true");
std::ostringstream jsonStream;
fileInfo->stringify(jsonStream);
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 7f04412b..38440005 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -155,6 +155,7 @@ DocumentBroker::DocumentBroker(const std::string& uri,
_lastSaveTime(std::chrono::steady_clock::now()),
_lastSaveRequestTime(std::chrono::steady_clock::now() - std::chrono::milliseconds(COMMAND_TIMEOUT_MS)),
_markToDestroy(false),
+ _closeRequest(false),
_isLoaded(false),
_isModified(false),
_cursorPosX(0),
@@ -276,13 +277,14 @@ void DocumentBroker::pollThread()
continue;
}
- if (ShutdownRequestFlag)
+ if (ShutdownRequestFlag || _closeRequest)
{
- LOG_INF("Autosaving DocumentBroker for docKey [" << getDocKey() << "] to recycle server.");
+ const std::string reason = ShutdownRequestFlag ? "recycling" : _closeReason;
+ LOG_INF("Autosaving DocumentBroker for docKey [" << getDocKey() << "] for " << reason);
if (!autoSave(isPossiblyModified()))
{
- LOG_INF("Terminating DocumentBroker for docKey [" << getDocKey() << "] to recycle server.");
- stop("recycling");
+ LOG_INF("Terminating DocumentBroker for docKey [" << getDocKey() << "].");
+ stop(reason);
}
}
else if (AutoSaveEnabled && !_stop &&
@@ -862,6 +864,7 @@ bool DocumentBroker::autoSave(const bool force)
{
assertCorrectThread();
+ LOG_TRC("autoSave(): forceful? " << force);
if (_sessions.empty() || _storage == nullptr || !_isLoaded ||
!_childProcess->isAlive() || (!_isModified && !force))
{
@@ -1594,7 +1597,8 @@ void DocumentBroker::closeDocument(const std::string& reason)
assertCorrectThread();
LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with reason: " << reason);
- stop(reason);
+ _closeReason = reason;
+ _closeRequest = true;
}
void DocumentBroker::broadcastMessage(const std::string& message)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index a398e701..a5345f67 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -431,6 +431,7 @@ private:
std::unique_ptr<StorageBase> _storage;
std::unique_ptr<TileCache> _tileCache;
std::atomic<bool> _markToDestroy;
+ std::atomic<bool> _closeRequest;
std::atomic<bool> _isLoaded;
std::atomic<bool> _isModified;
int _cursorPosX;
More information about the Libreoffice-commits
mailing list