[Libreoffice-commits] online.git: 2 commits - test/httpwstest.cpp wsd/DocumentBroker.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Thu Apr 20 04:44:54 UTC 2017
test/httpwstest.cpp | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
wsd/DocumentBroker.cpp | 19 ++++++-------
2 files changed, 79 insertions(+), 10 deletions(-)
New commits:
commit 876e4098357df8319bf6c721aca359cb94be9fb7
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Apr 20 00:13:32 2017 -0400
wsd: unittest for correctly saving in presence of passive clients
Passive clients that don't load the document can
be disruptive as they are not useful for saving
the document, so we need to ignore them and
use reliable sessions.
Change-Id: I162ec00823ba5af776fcb55f6d58149f2a56d7bb
Reviewed-on: https://gerrit.libreoffice.org/36712
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 78d717fb..917a41dd 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -70,6 +70,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testReload);
CPPUNIT_TEST(testGetTextSelection);
CPPUNIT_TEST(testSaveOnDisconnect);
+ CPPUNIT_TEST(testSavePassiveOnDisconnect);
CPPUNIT_TEST(testReloadWhileDisconnecting);
CPPUNIT_TEST(testExcelLoad);
CPPUNIT_TEST(testPaste);
@@ -123,6 +124,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
void testReload();
void testGetTextSelection();
void testSaveOnDisconnect();
+ void testSavePassiveOnDisconnect();
void testReloadWhileDisconnecting();
void testExcelLoad();
void testPaste();
@@ -711,6 +713,74 @@ void HTTPWSTest::testSaveOnDisconnect()
}
}
+void HTTPWSTest::testSavePassiveOnDisconnect()
+{
+ const auto testname = "saveOnPassiveDisconnect ";
+
+ const auto text = helpers::genRandomString(40);
+ std::cerr << "Test string: [" << text << "]." << std::endl;
+
+ std::string documentPath, documentURL;
+ getDocumentPathAndURL("hello.odt", documentPath, documentURL, testname);
+
+ int kitcount = -1;
+ try
+ {
+ auto socket = loadDocAndGetSocket(_uri, documentURL, testname);
+
+ Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, documentURL);
+ auto socket2 = connectLOKit(_uri, request, _response);
+
+ sendTextFrame(socket, "uno .uno:SelectAll", testname);
+ sendTextFrame(socket, "uno .uno:Delete", testname);
+ sendTextFrame(socket, "paste mimetype=text/plain;charset=utf-8\n" + text, testname);
+
+ // Check if the document contains the pasted text.
+ sendTextFrame(socket, "uno .uno:SelectAll", testname);
+ sendTextFrame(socket, "gettextselection mimetype=text/plain;charset=utf-8", testname);
+ const auto selection = assertResponseString(socket, "textselectioncontent:", testname);
+ CPPUNIT_ASSERT_EQUAL("textselectioncontent: " + text, selection);
+
+ // Closing connection too fast might not flush buffers.
+ // Often nothing more than the SelectAll reaches the server before
+ // the socket is closed, when the doc is not even modified yet.
+ getResponseMessage(socket, "statechanged", testname);
+
+ kitcount = getLoolKitProcessCount();
+
+ // Shutdown abruptly.
+ std::cerr << "Closing connection after pasting." << std::endl;
+ socket->shutdown(); // Should trigger saving.
+ socket2->shutdown();
+ }
+ catch (const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText());
+ }
+
+ // Allow time to save and destroy before we connect again.
+ testNoExtraLoolKitsLeft();
+ std::cerr << "Loading again." << std::endl;
+ try
+ {
+ // Load the same document and check that the last changes (pasted text) is saved.
+ auto socket = loadDocAndGetSocket(_uri, documentURL, testname);
+
+ // Should have no new instances.
+ CPPUNIT_ASSERT_EQUAL(kitcount, countLoolKitProcesses(kitcount));
+
+ // Check if the document contains the pasted text.
+ sendTextFrame(socket, "uno .uno:SelectAll", testname);
+ sendTextFrame(socket, "gettextselection mimetype=text/plain;charset=utf-8", testname);
+ const auto selection = assertResponseString(socket, "textselectioncontent:", testname);
+ CPPUNIT_ASSERT_EQUAL("textselectioncontent: " + text, selection);
+ }
+ catch (const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText());
+ }
+}
+
void HTTPWSTest::testReloadWhileDisconnecting()
{
const auto testname = "reloadWhileDisconnecting ";
commit 2a0253b76be3e329af55d0ab80157544eea2253e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Apr 20 00:10:30 2017 -0400
wsd: stop the DocBroker when saving session is last
When a session is disconnecting and we use it to save
(because the other sessions are not viable for saving),
then we need to correctly detect if by the time
saving is done there are no other sessions left.
Otherwise, we end up thinking there are other sessions
when the others had been disconnected during saving.
Change-Id: I55687376f5237a495ae163b53f51ee1d2414d770
Reviewed-on: https://gerrit.libreoffice.org/36711
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index bb76ea33..fd3a6ea9 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -562,16 +562,13 @@ bool DocumentBroker::saveToStorage(const std::string& sessionId,
const bool res = saveToStorageInternal(sessionId, success, result);
+ // We've saved and can safely destroy this session.
+ removeSessionInternal(sessionId);
+
// If marked to destroy, then this was the last session.
- // FIXME: If during that last save another client connects
- // to this doc, the _markToDestroy will be reset and we
- // will leak the last session. Need to mark the session as
- // dead and cleanup somehow.
- if (_markToDestroy)
+ // Otherwise, check that we are (which we might be by now).
+ if (_markToDestroy || _sessions.empty())
{
- // We've saved and can safely destroy.
- removeSessionInternal(sessionId);
-
// Stop so we get cleaned up and removed.
_stop = true;
}
@@ -866,7 +863,8 @@ size_t DocumentBroker::removeSession(const std::string& id, bool destroyIfLast)
try
{
LOG_INF("Removing session [" << id << "] on docKey [" << _docKey <<
- "]. Have " << _sessions.size() << " sessions.");
+ "]. Have " << _sessions.size() << " sessions. markToDestroy: " << _markToDestroy <<
+ ", LastEditableSession: " << _lastEditableSession);
if (!_lastEditableSession || !autoSave(true))
return removeSessionInternal(id);
@@ -1231,7 +1229,8 @@ void DocumentBroker::destroyIfLastEditor(const std::string& id)
// Last view going away, can destroy.
_markToDestroy = (_sessions.size() <= 1);
LOG_DBG("startDestroy on session [" << id << "] on docKey [" << _docKey <<
- "], markToDestroy: " << _markToDestroy << ", lastEditableSession: " << _lastEditableSession);
+ "], sessions: " << _sessions.size() << " markToDestroy: " << _markToDestroy <<
+ ", lastEditableSession: " << _lastEditableSession);
}
void DocumentBroker::setModified(const bool value)
More information about the Libreoffice-commits
mailing list