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.me...@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.me...@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
 
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to