common/Unit.hpp | 4 - net/Socket.hpp | 2 test/Makefile.am | 10 ++ test/UnitWOPIDocumentConflict.cpp | 136 ++++++++++++++++++++++++++++++++++++++ test/WopiTestServer.hpp | 53 +++++++++++++- wsd/LOOLWSD.cpp | 2 6 files changed, 196 insertions(+), 11 deletions(-)
New commits: commit 39f11ab4f7be708acc5e913d62e99321d8675357 Author: Pranav Kant <pran...@collabora.co.uk> Date: Fri Feb 9 00:21:54 2018 +0530 document conflict: unit test Change-Id: I4ea310fe5adb198bc7b5e083f6bd4b0431c0cdef diff --git a/common/Unit.hpp b/common/Unit.hpp index 97db1c39..5844cc3b 100644 --- a/common/Unit.hpp +++ b/common/Unit.hpp @@ -29,6 +29,8 @@ class WebSocketHandler; // Forward declaration to avoid pulling the world here. namespace Poco { + class MemoryInputStream; + namespace Net { class HTTPServerRequest; @@ -117,7 +119,7 @@ public: } /// Custom response to a http request. - virtual bool handleHttpRequest(const Poco::Net::HTTPRequest& /*request*/, std::shared_ptr<StreamSocket>& /*socket*/) + virtual bool handleHttpRequest(const Poco::Net::HTTPRequest& /*request*/, Poco::MemoryInputStream& /*message*/,std::shared_ptr<StreamSocket>& /*socket*/) { return false; } diff --git a/net/Socket.hpp b/net/Socket.hpp index da34b5e3..7e5035b4 100644 --- a/net/Socket.hpp +++ b/net/Socket.hpp @@ -501,7 +501,7 @@ public: } while (rc == -1 && errno == EINTR); if (rc == -1 && errno != EAGAIN && errno != EWOULDBLOCK) - LOG_SYS("wakeup socket #" << fd << " is closd at wakeup?"); + LOG_SYS("wakeup socket #" << fd << " is closed at wakeup?"); } /// Wakeup the main polling loop in another thread diff --git a/test/Makefile.am b/test/Makefile.am index 1184f31f..b417a857 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -18,7 +18,8 @@ noinst_LTLIBRARIES = \ unit-admin.la unit-tilecache.la \ unit-fuzz.la unit-oob.la unit-oauth.la \ unit-wopi.la unit-wopi-saveas.la \ - unit-wopi-ownertermination.la unit-wopi-versionrestore.la + unit-wopi-ownertermination.la unit-wopi-versionrestore.la \ + unit-wopi-documentconflict.la MAGIC_TO_FORCE_SHLIB_CREATION = -rpath /dummy @@ -89,6 +90,8 @@ unit_wopi_ownertermination_la_SOURCES = UnitWopiOwnertermination.cpp unit_wopi_ownertermination_la_LIBADD = $(CPPUNIT_LIBS) unit_wopi_versionrestore_la_SOURCES = UnitWOPIVersionRestore.cpp unit_wopi_versionrestore_la_LIBADD = $(CPPUNIT_LIBS) +unit_wopi_documentconflict_la_SOURCES = UnitWOPIDocumentConflict.cpp +unit_wopi_documentconflict_la_LIBADD = $(CPPUNIT_LIBS) if HAVE_LO_PATH SYSTEM_STAMP = @SYSTEMPLATE_PATH@/system_stamp @@ -102,7 +105,10 @@ 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 unit-wopi-ownertermination.la unit-wopi-versionrestore.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 unit-wopi-versionrestore.la \ + unit-wopi-documentconflict.la # TESTS = unit-client.la # TESTS += unit-admin.la # TESTS += unit-storage.la diff --git a/test/UnitWOPIDocumentConflict.cpp b/test/UnitWOPIDocumentConflict.cpp new file mode 100644 index 00000000..0b1cbb8e --- /dev/null +++ b/test/UnitWOPIDocumentConflict.cpp @@ -0,0 +1,136 @@ +/* -*- 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/Timestamp.h> + +/** + * This test asserts that the unsaved changes in the opened document are + * discarded in case document is changed in storage behind our back. We don't + * want to overwrite the document which is in storage when the user asks us to + * do so. + */ +class UnitWOPIDocumentConflict : public WopiTestServer +{ + enum class Phase + { + Load, + ModifyAndChangeStorageDoc, + LoadNewDocument, + Polling + } _phase; + + enum class DocLoaded + { + Doc1, + Doc2 + } _docLoaded; + + const std::string _testName = "UnitWOPIDocumentConflict"; + +public: + UnitWOPIDocumentConflict() : + _phase(Phase::Load) + { + } + + void assertGetFileRequest(const Poco::Net::HTTPRequest& /*request*/) override + { + if (_docLoaded == DocLoaded::Doc2) + { + // On second doc load, we should have the document in storage which + // was changed beneath us, not the one which we modified by pressing 'a' + if (_fileContent != "Modified content in storage") + exitTest(TestResult::Failed); + else + exitTest(TestResult::Ok); + } + } + + bool filterSendMessage(const char* data, const size_t len, const WSOpCode /* code */, const bool /* flush */, int& /*unitReturn*/) override + { + std::string message(data, len); + if (message == "error: cmd=storage kind=documentconflict") + { + // we don't want to save current changes because doing so would + // overwrite the document which was changed underneath us + helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "closedocument", _testName.c_str()); + _phase = Phase::LoadNewDocument; + } + + return false; + } + + void invokeTest() override + { + switch (_phase) + { + case Phase::Load: + { + initWebsocket("/wopi/files/0?access_token=anything"); + _docLoaded = DocLoaded::Doc1; + + helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "load url=" + _wopiSrc, _testName.c_str()); + + _phase = Phase::ModifyAndChangeStorageDoc; + break; + } + case Phase::ModifyAndChangeStorageDoc: + { + // modify the currently opened document; type 'a' + helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "key type=input char=97 key=0", _testName.c_str()); + helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "key type=up char=0 key=512", _testName.c_str()); + SocketPoll::wakeupWorld(); + + // ModifiedStatus=true is a bit slow; let's sleep and hope that + // it is received before we wake up + std::this_thread::sleep_for(std::chrono::milliseconds(POLL_TIMEOUT_MS)); + + // change the document underneath, in storage + setFileContent("Modified content in storage"); + + // save the document; wsd should detect now that document has + // been changed underneath it and send us: + // "error: cmd=storage kind=documentconflict" + helpers::sendTextFrame(*_ws->getLOOLWebSocket(), "save", _testName.c_str()); + + _phase = Phase::Polling; + + break; + } + case Phase::LoadNewDocument: + { + initWebsocket("/wopi/files/0?access_token=anything"); + _docLoaded = DocLoaded::Doc2; + _phase = Phase::Polling; + break; + } + case Phase::Polling: + { + // just wait for the results + break; + } + } + } +}; + +UnitBase *unit_create_wsd(void) +{ + return new UnitWOPIDocumentConflict(); +} + +/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/WopiTestServer.hpp b/test/WopiTestServer.hpp index 3baf17ba..5013d8af 100644 --- a/test/WopiTestServer.hpp +++ b/test/WopiTestServer.hpp @@ -18,12 +18,18 @@ #include <Poco/DateTimeFormat.h> #include <Poco/DateTimeFormatter.h> #include <Poco/JSON/Object.h> +#include <Poco/MemoryStream.h> #include <Poco/Net/HTTPRequest.h> #include <Poco/URI.h> #include <Poco/Timestamp.h> class WopiTestServer : public UnitWSD { + enum class LOOLStatusCode + { + DocChanged = 1010 + }; + protected: /// The WOPISrc URL. std::string _wopiSrc; @@ -37,6 +43,13 @@ protected: /// Last modified time of the file Poco::Timestamp _fileLastModifiedTime; + /// Sets the file content to a given value and update the last file modified time + void setFileContent(const std::string& fileContent) + { + _fileContent = fileContent; + _fileLastModifiedTime = Poco::Timestamp(); + } + public: WopiTestServer(std::string fileContent = "Hello, world") : UnitWSD() @@ -76,7 +89,7 @@ public: protected: /// Here we act as a WOPI server, so that we have a server that responds to /// the wopi requests without additional expensive setup. - virtual bool handleHttpRequest(const Poco::Net::HTTPRequest& request, std::shared_ptr<StreamSocket>& socket) override + virtual bool handleHttpRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message, std::shared_ptr<StreamSocket>& socket) override { Poco::URI uriReq(request.getURI()); LOG_INF("Fake wopi host request: " << uriReq.toString()); @@ -173,6 +186,31 @@ protected: { LOG_INF("Fake wopi host request, handling PutFile: " << uriReq.getPath()); + std::string wopiTimestamp = request.get("X-LOOL-WOPI-Timestamp"); + if (!wopiTimestamp.empty()) + { + const std::string fileModifiedTime = + Poco::DateTimeFormatter::format(Poco::DateTime(_fileLastModifiedTime), + Poco::DateTimeFormat::ISO8601_FRAC_FORMAT); + if (wopiTimestamp != fileModifiedTime) + { + std::ostringstream oss; + oss << "HTTP/1.1 409 Conflict\r\n" + << "User-Agent: " << WOPI_AGENT_STRING << "\r\n" + << "\r\n" + << "{\"LOOLStatusCode\":" << LOOLStatusCode::DocChanged << "}"; + + socket->send(oss.str()); + socket->shutdown(); + return true; + } + } + + std::streamsize size = request.getContentLength(); + char buffer[size]; + message.read(buffer, size); + setFileContent(buffer); + assertPutFileRequest(request); std::ostringstream oss; diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 5ae13552..97c0953f 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -1792,7 +1792,7 @@ private: std::vector<std::string> reqPathSegs; requestUri.getPathSegments(reqPathSegs); - if (UnitWSD::get().handleHttpRequest(request, socket)) + if (UnitWSD::get().handleHttpRequest(request, message, socket)) { // Unit testing, nothing to do here } commit cb0d65289907dca851c0a5386483b709c3dcd2df Author: Pranav Kant <pran...@collabora.co.uk> Date: Thu Feb 8 20:15:47 2018 +0530 Try to act like a wopi test server bit more realistically Change-Id: If10a65a3e7740a752057f63072f1ada1be3552e7 diff --git a/test/WopiTestServer.hpp b/test/WopiTestServer.hpp index feaf0c17..3baf17ba 100644 --- a/test/WopiTestServer.hpp +++ b/test/WopiTestServer.hpp @@ -34,6 +34,9 @@ protected: /// Content of the file. std::string _fileContent; + /// Last modified time of the file + Poco::Timestamp _fileLastModifiedTime; + public: WopiTestServer(std::string fileContent = "Hello, world") : UnitWSD() @@ -95,7 +98,7 @@ protected: fileInfo->set("UserFriendlyName", "test"); fileInfo->set("UserCanWrite", "true"); fileInfo->set("PostMessageOrigin", "localhost"); - fileInfo->set("LastModifiedTime", Poco::DateTimeFormatter::format(now, Poco::DateTimeFormat::ISO8601_FORMAT)); + fileInfo->set("LastModifiedTime", Poco::DateTimeFormatter::format(_fileLastModifiedTime, Poco::DateTimeFormat::ISO8601_FORMAT)); fileInfo->set("EnableOwnerTermination", "true"); std::ostringstream jsonStream; @@ -106,7 +109,7 @@ protected: std::ostringstream oss; oss << "HTTP/1.1 200 OK\r\n" - << "Last-Modified: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" + << "Last-Modified: " << Poco::DateTimeFormatter::format(_fileLastModifiedTime, Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" << "User-Agent: " << WOPI_AGENT_STRING << "\r\n" << "Content-Length: " << responseString.size() << "\r\n" << "Content-Type: " << mimeType << "\r\n" @@ -129,7 +132,7 @@ protected: std::ostringstream oss; oss << "HTTP/1.1 200 OK\r\n" - << "Last-Modified: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" + << "Last-Modified: " << Poco::DateTimeFormatter::format(_fileLastModifiedTime, Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" << "User-Agent: " << WOPI_AGENT_STRING << "\r\n" << "Content-Length: " << _fileContent.size() << "\r\n" << "Content-Type: " << mimeType << "\r\n" @@ -154,7 +157,7 @@ protected: std::ostringstream oss; oss << "HTTP/1.1 200 OK\r\n" - << "Last-Modified: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" + << "Last-Modified: " << Poco::DateTimeFormatter::format(_fileLastModifiedTime, Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" << "User-Agent: " << WOPI_AGENT_STRING << "\r\n" << "Content-Length: " << content.size() << "\r\n" << "Content-Type: application/json\r\n" @@ -174,7 +177,7 @@ protected: std::ostringstream oss; oss << "HTTP/1.1 200 OK\r\n" - << "Last-Modified: " << Poco::DateTimeFormatter::format(Poco::Timestamp(), Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" + << "Last-Modified: " << Poco::DateTimeFormatter::format(_fileLastModifiedTime, Poco::DateTimeFormat::HTTP_FORMAT) << "\r\n" << "User-Agent: " << WOPI_AGENT_STRING << "\r\n" << "\r\n"; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits