[Libreoffice-commits] online.git: 2 commits - loolwsd/DocumentBroker.cpp loolwsd/LOOLWSD.cpp loolwsd/Makefile.am loolwsd/Storage.cpp loolwsd/Storage.hpp loolwsd/test loolwsd/Unit.hpp loolwsd/UnitHTTP.hpp
Michael Meeks
michael.meeks at collabora.com
Thu Apr 7 21:21:36 UTC 2016
loolwsd/DocumentBroker.cpp | 22 ++++++++-----
loolwsd/LOOLWSD.cpp | 36 +++++++++++++++++++---
loolwsd/Makefile.am | 3 +
loolwsd/Storage.cpp | 28 +++++++++++++++++
loolwsd/Storage.hpp | 28 ++---------------
loolwsd/Unit.hpp | 26 +++++++++++++++
loolwsd/UnitHTTP.hpp | 70 +++++++++++++++++++++++++++++++++++++++++++
loolwsd/test/Makefile.am | 9 +++--
loolwsd/test/UnitStorage.cpp | 46 ++++++++++++++++++++++++++++
loolwsd/test/run_unit.sh | 2 -
10 files changed, 227 insertions(+), 43 deletions(-)
New commits:
commit 7d62c74b8388170b3284e7128f19618b49853e10
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Thu Apr 7 21:59:27 2016 +0100
Fix segv on failure to create a storage, and add unit test infra.
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 90f60bc..b8cfa61 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -81,7 +81,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
void DocumentBroker::validate(const Poco::URI& uri)
{
Log::info("Validating: " + uri.toString());
- auto storage = createStorage("", "", uri);
+ auto storage = StorageBase::create("", "", uri);
if (storage == nullptr || !storage->getFileInfo(uri).isValid())
{
throw std::runtime_error("Invalid URI or access denied.");
@@ -111,15 +111,21 @@ bool DocumentBroker::load(const std::string& jailId)
Log::info("jailPath: " + jailPath.toString() + ", jailRoot: " + jailRoot);
- auto storage = createStorage("", "", _uriPublic);
- const auto fileInfo = storage->getFileInfo(_uriPublic);
- _tileCache.reset(new TileCache(_uriPublic.toString(), fileInfo.ModifiedTime, _cacheRoot));
+ auto storage = StorageBase::create("", "", _uriPublic);
+ if (storage)
+ {
+ const auto fileInfo = storage->getFileInfo(_uriPublic);
+ _tileCache.reset(new TileCache(_uriPublic.toString(), fileInfo.ModifiedTime, _cacheRoot));
+
+ _storage = StorageBase::create(jailRoot, jailPath.toString(), _uriPublic);
- _storage = createStorage(jailRoot, jailPath.toString(), _uriPublic);
+ const auto localPath = _storage->loadStorageFileToLocal();
+ _uriJailed = Poco::URI(Poco::URI("file://"), localPath);
- const auto localPath = _storage->loadStorageFileToLocal();
- _uriJailed = Poco::URI(Poco::URI("file://"), localPath);
- return true;
+ return true;
+ }
+ else
+ return false;
}
bool DocumentBroker::save()
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 3c497db..08076c2 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -50,7 +50,6 @@
#include <Poco/Net/HTTPRequestHandlerFactory.h>
#include <Poco/Net/HTTPServer.h>
#include <Poco/Net/HTTPServerParams.h>
-#include <Poco/Net/HTTPServerParams.h>
#include <Poco/Net/HTTPServerRequest.h>
#include <Poco/Net/HTTPServerResponse.h>
#include <Poco/Net/InvalidCertificateHandler.h>
@@ -93,6 +92,7 @@
#include "IoUtil.hpp"
#include "Util.hpp"
#include "Unit.hpp"
+#include "UnitHTTP.hpp"
using namespace LOOLProtocol;
@@ -240,7 +240,7 @@ class ClientRequestHandler: public HTTPRequestHandler
{
private:
- void handlePostRequest(HTTPServerRequest& request, HTTPServerResponse& response, const std::string& id)
+ static void handlePostRequest(HTTPServerRequest& request, HTTPServerResponse& response, const std::string& id)
{
Log::info("Post request: [" + request.getURI() + "]");
StringTokenizer tokens(request.getURI(), "/?");
@@ -425,7 +425,7 @@ private:
}
}
- void handleGetRequest(HTTPServerRequest& request, HTTPServerResponse& response, const std::string& id)
+ static void handleGetRequest(HTTPServerRequest& request, HTTPServerResponse& response, const std::string& id)
{
Log::info("Starting GET request handler for session [" + id + "].");
@@ -546,7 +546,7 @@ private:
}
}
- void handleGetDiscovery(HTTPServerRequest& request, HTTPServerResponse& response)
+ static void handleGetDiscovery(HTTPServerRequest& request, HTTPServerResponse& response)
{
DOMParser parser;
DOMWriter writer;
@@ -583,6 +583,11 @@ public:
void handleRequest(HTTPServerRequest& request, HTTPServerResponse& response) override
{
+ handleClientRequest(request,response);
+ }
+
+ static void handleClientRequest(HTTPServerRequest& request, HTTPServerResponse& response)
+ {
const auto id = LOOLWSD::GenSessionId();
Util::setThreadName("client_ws_" + id);
@@ -632,6 +637,11 @@ public:
void handleRequest(HTTPServerRequest& request, HTTPServerResponse& response) override
{
+ handlePrisonerRequest(request, response);
+ }
+
+ static void handlePrisonerRequest(HTTPServerRequest& request, HTTPServerResponse& response)
+ {
Util::setThreadName("prison_ws");
Log::debug("Child connection with URI [" + request.getURI() + "].");
@@ -1395,6 +1405,8 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
int status = 0;
while (!TerminationFlag && !LOOLWSD::DoTest)
{
+ UnitHooks::get().invokeTest();
+
const pid_t pid = waitpid(forKitPid, &status, WUNTRACED | WNOHANG);
if (pid > 0)
{
@@ -1552,6 +1564,22 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
return returnValue;
}
+void UnitHooks::testHandleRequest(TestRequest type, UnitHTTPServerRequest& request, UnitHTTPServerResponse& response)
+{
+ switch (type)
+ {
+ case TestRequest::TEST_REQ_CLIENT:
+ ClientRequestHandler::handleClientRequest(request, response);
+ break;
+ case TestRequest::TEST_REQ_PRISONER:
+ PrisonerRequestHandler::handlePrisonerRequest(request, response);
+ break;
+ default:
+ assert(false);
+ break;
+ }
+}
+
POCO_SERVER_MAIN(LOOLWSD)
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/Makefile.am b/loolwsd/Makefile.am
index fc29ed5..943331c 100644
--- a/loolwsd/Makefile.am
+++ b/loolwsd/Makefile.am
@@ -75,6 +75,7 @@ noinst_HEADERS = Admin.hpp \
Storage.hpp \
TileCache.hpp \
Unit.hpp \
+ UnitHTTP.hpp \
Util.hpp \
bundled/include/LibreOfficeKit/LibreOfficeKit.h \
bundled/include/LibreOfficeKit/LibreOfficeKitEnums.h \
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index f649a79..b15253a 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -23,6 +23,7 @@
#include "Auth.hpp"
#include "Storage.hpp"
#include "Util.hpp"
+#include "Unit.hpp"
///////////////////
// StorageBase Impl
@@ -48,6 +49,33 @@ size_t StorageBase::getFileSize(const std::string& filename)
return std::ifstream(filename, std::ifstream::ate | std::ifstream::binary).tellg();
}
+std::unique_ptr<StorageBase> StorageBase::create(const std::string& jailRoot, const std::string& jailPath, const Poco::URI& uri)
+{
+ std::unique_ptr<StorageBase> storage;
+
+ if (UnitHooks::get().createStorage(jailRoot, jailPath, uri, storage))
+ Log::info("Storage load hooked");
+ else if (uri.isRelative() || uri.getScheme() == "file")
+ {
+ if (!Poco::Util::Application::instance().config().getBool("storage.filesystem[@allow]", false))
+ {
+ Log::error("Local Storage is disabled by default. Specify allowlocalstorage on the command-line to enable.");
+ return nullptr;
+ }
+
+ Log::info("Public URI [" + uri.toString() + "] is a file.");
+ storage = std::unique_ptr<StorageBase>(new LocalStorage(jailRoot, jailPath, uri.getPath()));
+ }
+ else
+ {
+ Log::info("Public URI [" + uri.toString() +
+ "] assuming cloud storage.");
+ //TODO: Configure the storage to use. For now, assume it's WOPI.
+ storage = std::unique_ptr<StorageBase>(new WopiStorage(jailRoot, jailPath, uri.toString()));
+ }
+ return storage;
+}
+
////////////////////
// LocalStorage Impl
/////////////////////
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index 24023a2..5a34d29 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -69,6 +69,10 @@ public:
static
size_t getFileSize(const std::string& filename);
+ static std::unique_ptr<StorageBase> create(const std::string& jailRoot,
+ const std::string& jailPath,
+ const Poco::URI& uri);
+
protected:
const std::string _localStorePath;
const std::string _jailPath;
@@ -135,33 +139,9 @@ public:
std::string loadStorageFileToLocal() override;
bool saveLocalFileToStorage() override;
-
private:
std::unique_ptr<AuthBase> _authAgent;
};
-inline
-std::unique_ptr<StorageBase> createStorage(const std::string& jailRoot, const std::string& jailPath, const Poco::URI& uri)
-{
- if (uri.isRelative() || uri.getScheme() == "file")
- {
- if (!Poco::Util::Application::instance().config().getBool("storage.filesystem[@allow]", false))
- {
- Log::error("Local Storage is disabled by default. Specify allowlocalstorage on the command-line to enable.");
- return nullptr;
- }
-
- Log::info("Public URI [" + uri.toString() + "] is a file.");
- return std::unique_ptr<StorageBase>(new LocalStorage(jailRoot, jailPath, uri.getPath()));
- }
- else
- {
- Log::info("Public URI [" + uri.toString() +
- "] assuming cloud storage.");
- //TODO: Configure the storage to use. For now, assume it's WOPI.
- return std::unique_ptr<StorageBase>(new WopiStorage(jailRoot, jailPath, uri.toString()));
- }
-}
-
#endif
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/Unit.hpp b/loolwsd/Unit.hpp
index dc53d9f..75b2b0f 100644
--- a/loolwsd/Unit.hpp
+++ b/loolwsd/Unit.hpp
@@ -10,8 +10,13 @@
#define LOOL_UNIT_HPP
#include <string>
+#include <memory>
class UnitHooks;
+class UnitHTTPServerRequest;
+class UnitHTTPServerResponse;
+
+class StorageBase;
#define CREATE_UNIT_HOOKS_SYMBOL "unit_create"
typedef UnitHooks *(CreateUnitHooksFunction)();
@@ -28,9 +33,19 @@ class UnitHooks
void setHandle(void *dlHandle) { _dlHandle = dlHandle; }
static UnitHooks *linkAndCreateUnit(const std::string &unitLibPath);
protected:
+
+ // ---------------- Helper API ----------------
+
enum TestResult { TEST_FAILED, TEST_OK, TEST_TIMED_OUT };
/// Encourages loolwsd to exit with this value (unless hooked)
void exitTest(TestResult result);
+
+ enum TestRequest { TEST_REQ_CLIENT, TEST_REQ_PRISONER };
+ /// Simulate an incoming request
+ void testHandleRequest(TestRequest type,
+ UnitHTTPServerRequest& request,
+ UnitHTTPServerResponse& response);
+
public:
UnitHooks();
virtual ~UnitHooks();
@@ -38,14 +53,23 @@ public:
/// Load unit test hook shared library from this path
static bool init(const std::string &unitLibPath);
+ // ---------------- Hooks ----------------
+
+ /// Main-loop reached, time for testing
+ virtual void invokeTest() {}
/// Tweak the count of pre-spawned kits.
virtual void preSpawnCount(int & /* numPrefork */) {}
/// Tweak the return value from LOOLWSD.
virtual void returnValue(int & /* retValue */);
/// When a new child kit process reports
virtual void newChild() {}
- /// If the test times out
+ /// If the test times out this gets invoked
virtual void timeout();
+ /// Intercept createStorage
+ virtual bool createStorage(const std::string& /* jailRoot */,
+ const std::string& /* jailPath */,
+ const Poco::URI& /* uri */,
+ std::unique_ptr<StorageBase> & /*rStorage */) { return false; }
};
#endif // LOOL_UNIT_HPP
diff --git a/loolwsd/UnitHTTP.hpp b/loolwsd/UnitHTTP.hpp
new file mode 100644
index 0000000..ea00b85
--- /dev/null
+++ b/loolwsd/UnitHTTP.hpp
@@ -0,0 +1,70 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * 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/.
+ */
+#ifndef LOOL_UNIT_HTTP_HPP
+#define LOOL_UNIT_HTTP_HPP
+
+#include <Poco/Net/HTTPRequest.h>
+#include <Poco/Net/HTTPServerParams.h>
+#include <Poco/Net/HTTPServerRequest.h>
+#include <Poco/Net/HTTPServerResponse.h>
+#include <Poco/Net/SocketAddress.h>
+
+#include "Common.hpp"
+
+using Poco::Net::SocketAddress;
+using Poco::Net::HTTPServerParams;
+
+/// Unit test stub for a server response
+class UnitHTTPServerResponse : public Poco::Net::HTTPServerResponse
+{
+ bool _sent;
+public:
+ UnitHTTPServerResponse() : _sent (false) {}
+ virtual void sendContinue() override {}
+ virtual std::ostream& send() override
+ { _sent = true; return *(static_cast<std::ostream *>(nullptr)); }
+ virtual void sendFile(const std::string& /* path */,
+ const std::string& /* mediaType */) override {}
+ virtual void sendBuffer(const void* /* pBuffer */,
+ std::size_t /* length */) override {}
+ virtual void redirect(const std::string& /* uri */,
+ HTTPStatus /* status = HTTP_FOUND */) override {}
+ virtual void requireAuthentication(const std::string& /* realm */) override {}
+ virtual bool sent() const override { return _sent; }
+};
+
+/// Unit test stub for a server request
+class UnitHTTPServerRequest : public Poco::Net::HTTPServerRequest
+{
+protected:
+ UnitHTTPServerResponse &_response;
+ Poco::Net::SocketAddress _clientAddress;
+ Poco::Net::SocketAddress _serverAddress;
+public:
+ UnitHTTPServerRequest(UnitHTTPServerResponse &inResponse,
+ const std::string &uri)
+ : _response(inResponse),
+ _clientAddress(),
+ _serverAddress(MASTER_PORT_NUMBER)
+ { setURI(uri); }
+ virtual std::istream& stream() override
+ { return *(static_cast<std::istream *>(nullptr)); }
+ virtual bool expectContinue() const override
+ { return false; }
+ virtual const SocketAddress& clientAddress() const override
+ { return _clientAddress; }
+ virtual const SocketAddress& serverAddress() const override
+ { return _serverAddress; }
+ virtual const HTTPServerParams& serverParams() const override
+ { return *(static_cast<HTTPServerParams *>(nullptr)); }
+ virtual Poco::Net::HTTPServerResponse& response() const override
+ { return _response; }
+};
+
+#endif // LOOL_UNIT_HTTP_HPP
diff --git a/loolwsd/test/Makefile.am b/loolwsd/test/Makefile.am
index d7a17a3..03f3718 100644
--- a/loolwsd/test/Makefile.am
+++ b/loolwsd/test/Makefile.am
@@ -11,7 +11,7 @@ check_PROGRAMS = test
AM_CXXFLAGS = $(CPPUNIT_CFLAGS)
-lib_LTLIBRARIES = unit-prefork.la
+lib_LTLIBRARIES = unit-prefork.la unit-storage.la
AM_CPPFLAGS = -pthread -I$(top_srcdir)
@@ -22,9 +22,10 @@ test_LDADD = $(CPPUNIT_LIBS)
test_SOURCES = httpposttest.cpp httpwstest.cpp test.cpp ../LOOLProtocol.cpp
# unit test modules:
-unit_prefork_la_SOURCES = \
- UnitPrefork.cpp
+unit_prefork_la_SOURCES = UnitPrefork.cpp
unit_prefork_la_LDFLAGS = -module
+unit_storage_la_SOURCES = UnitStorage.cpp
+unit_storage_la_LDFLAGS = -module
${systemplate}/system_stamp :
rm -rf ${systemplate}
@@ -38,7 +39,7 @@ ${top_builddir}/test/run_unit.sh.log ${top_builddir}/test/run_unit.sh.trs : \
${top_srcdir}/test/run_unit.sh \
${top_builddir}/loolwsd \
${top_builddir}/loolforkit \
- unit-prefork.la
+ unit-prefork.la unit-storage.la
if ${top_srcdir}/test/run_unit.sh; then \
touch ${top_builddir}/test/run_unit.sh.log; \
fi
diff --git a/loolwsd/test/UnitStorage.cpp b/loolwsd/test/UnitStorage.cpp
new file mode 100644
index 0000000..91989f4
--- /dev/null
+++ b/loolwsd/test/UnitStorage.cpp
@@ -0,0 +1,46 @@
+/* -*- 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 <dlfcn.h>
+#include <ftw.h>
+#include <cassert>
+#include <iostream>
+
+#include "Util.hpp"
+#include "Unit.hpp"
+#include "UnitHTTP.hpp"
+
+class UnitStorage : public UnitHooks
+{
+public:
+ virtual bool createStorage(const std::string& /* jailRoot */,
+ const std::string& /* jailPath */,
+ const Poco::URI& /* uri */,
+ std::unique_ptr<StorageBase> & /* rStorage */)
+ {
+ // leave rStorage empty - fail to return anything
+ return true;
+ }
+ virtual void invokeTest()
+ {
+ // FIXME: push through to the right place to exercise this.
+ exitTest(TestResult::TEST_OK);
+ UnitHTTPServerResponse response;
+ UnitHTTPServerRequest request(response, std::string(CHILD_URI));
+ UnitHooks::testHandleRequest(TestRequest::TEST_REQ_PRISONER,
+ request, response);
+ }
+};
+
+UnitHooks *unit_create(void)
+{
+ return new UnitStorage();
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/test/run_unit.sh b/loolwsd/test/run_unit.sh
index 8c67750..22f7363 100755
--- a/loolwsd/test/run_unit.sh
+++ b/loolwsd/test/run_unit.sh
@@ -7,7 +7,7 @@ mkdir -p test_output
# result logging
echo > run_unit.sh.trs
-for tst in prefork; do
+for tst in storage prefork; do
tst_log="test_output/$tst.log"
echo "Running test: $tst | $tst_log ...";
if ../loolwsd --systemplate=${systemplate} --lotemplate="${LO_PATH}" --childroot="${jails}" --unitlib=".libs/unit-$tst.so" 2> "$tst_log"; then
commit 86ebefce50bc86ff2671f2299e9f058d6ed0041f
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Thu Apr 7 22:12:23 2016 +0100
Get subdirs build order right for tests.
diff --git a/loolwsd/Makefile.am b/loolwsd/Makefile.am
index c555b88..fc29ed5 100644
--- a/loolwsd/Makefile.am
+++ b/loolwsd/Makefile.am
@@ -1,4 +1,4 @@
-SUBDIRS = test
+SUBDIRS = . test
bin_PROGRAMS = loolwsd loolforkit loolmap loolmount
More information about the Libreoffice-commits
mailing list