[Libreoffice-commits] online.git: 2 commits - loolwsd/ChildSession.hpp loolwsd/LOOLKit.cpp loolwsd/LOOLKit.hpp loolwsd/test

Miklos Vajna vmiklos at collabora.co.uk
Fri Sep 30 14:00:20 UTC 2016


 loolwsd/ChildSession.hpp       |    4 ++
 loolwsd/LOOLKit.cpp            |   42 ++++++++++++++++++++----------
 loolwsd/LOOLKit.hpp            |   11 +++++++
 loolwsd/test/Makefile.am       |    1 
 loolwsd/test/WhiteBoxTests.cpp |   57 +++++++++++++++++++++++++++++++++++++++++
 loolwsd/test/httpwstest.cpp    |   23 ----------------
 6 files changed, 101 insertions(+), 37 deletions(-)

New commits:
commit d0abdee6d5bef7ca24a8042445f4ea2a071d0575
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Fri Sep 30 15:46:28 2016 +0200

    Move HTTPWSTest::testEmptyCellCursor() to WhiteBoxTests
    
    Which is a unit test, so should be faster.
    
    Change-Id: Id5c2ed705576a1467756ac8dba6e5e434b56f725

diff --git a/loolwsd/test/WhiteBoxTests.cpp b/loolwsd/test/WhiteBoxTests.cpp
index 3e43437..af4e34f 100644
--- a/loolwsd/test/WhiteBoxTests.cpp
+++ b/loolwsd/test/WhiteBoxTests.cpp
@@ -11,7 +11,9 @@
 
 #include <cppunit/extensions/HelperMacros.h>
 
+#include <ChildSession.hpp>
 #include <Common.hpp>
+#include <LOOLKit.hpp>
 #include <LOOLProtocol.hpp>
 #include <MessageQueue.hpp>
 #include <Util.hpp>
@@ -24,12 +26,14 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testLOOLProtocolFunctions);
     CPPUNIT_TEST(testRegexListMatcher);
     CPPUNIT_TEST(testRegexListMatcher_Init);
+    CPPUNIT_TEST(testEmptyCellCursor);
 
     CPPUNIT_TEST_SUITE_END();
 
     void testLOOLProtocolFunctions();
     void testRegexListMatcher();
     void testRegexListMatcher_Init();
+    void testEmptyCellCursor();
 };
 
 void WhiteBoxTests::testLOOLProtocolFunctions()
@@ -145,6 +149,59 @@ void WhiteBoxTests::testRegexListMatcher_Init()
     CPPUNIT_ASSERT(matcher.match("192.168.."));
 }
 
+/// A stub IDocumentManager implementation for unit test purposes.
+class DummyDocument : public IDocumentManager
+{
+    std::shared_ptr<TileQueue> _tileQueue;
+    std::mutex _mutex;
+public:
+    DummyDocument()
+        : _tileQueue(new TileQueue()),
+        _mutex()
+    {
+    }
+    std::shared_ptr<lok::Document> onLoad(const std::string& /*sessionId*/,
+                                          const std::string& /*jailedFilePath*/,
+                                          const std::string& /*userName*/,
+                                          const std::string& /*docPassword*/,
+                                          const std::string& /*renderOpts*/,
+                                          const bool /*haveDocPassword*/) override
+    {
+        return nullptr;
+    }
+
+    void onUnload(const ChildSession& /*session*/) override
+    {
+    }
+
+    void notifyViewInfo() override
+    {
+    }
+
+    std::map<int, std::string> getViewInfo() override
+    {
+        return {};
+    }
+
+    std::mutex& getMutex() override
+    {
+        return _mutex;
+    }
+
+    std::shared_ptr<TileQueue>& getTileQueue() override
+    {
+        return _tileQueue;
+    }
+};
+
+void WhiteBoxTests::testEmptyCellCursor()
+{
+    DummyDocument document;
+    CallbackDescriptor callbackDescriptor{&document, 0};
+    // This failed as stoi raised an std::invalid_argument exception.
+    documentViewCallback(LOK_CALLBACK_CELL_CURSOR, "EMPTY", &callbackDescriptor);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(WhiteBoxTests);
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 5e8f50c..91dc79a 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -82,7 +82,6 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testInactiveClient);
     CPPUNIT_TEST(testMaxColumn);
     CPPUNIT_TEST(testMaxRow);
-    CPPUNIT_TEST(testEmptyCellCursor);
     CPPUNIT_TEST(testInsertAnnotationWriter);
     CPPUNIT_TEST(testEditAnnotationWriter);  // Broken with multiview.
     CPPUNIT_TEST(testInsertAnnotationCalc);
@@ -126,7 +125,6 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
     void testInactiveClient();
     void testMaxColumn();
     void testMaxRow();
-    void testEmptyCellCursor();
     void testInsertAnnotationWriter();
     void testEditAnnotationWriter();
     void testInsertAnnotationCalc();
@@ -1127,27 +1125,6 @@ void HTTPWSTest::testMaxRow()
     }
 }
 
-void HTTPWSTest::testEmptyCellCursor()
-{
-    // Load a document, and make sure a cell cursor shows up.
-    std::string docPath;
-    std::string docURL;
-    getDocumentPathAndURL("setclientpart.ods", docPath, docURL);
-    Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, docURL);
-    std::shared_ptr<Poco::Net::WebSocket> socket = loadDocAndGetSocket(_uri, docURL);
-    std::string response;
-    getResponseMessage(socket, "cellcursor:", response, false);
-
-    // Begin text edit of a cell.
-    sendTextFrame(socket, "key type=input char=115 key=97");
-
-    // Make sure the cell cursor is now hidden.
-    getResponseMessage(socket, "cellcursor: ", response, false);
-    // This failed as response was "", not "EMPTY", because code in
-    // Document::ViewCallback() crashed.
-    CPPUNIT_ASSERT_EQUAL(std::string("EMPTY"), response);
-}
-
 void HTTPWSTest::testNoExtraLoolKitsLeft()
 {
     const auto countNow = countLoolKitProcesses(InitialLoolKitCount);
commit 38577b74fd25d0f1f5b775d8ed815124c4514bea
Author: Miklos Vajna <vmiklos at collabora.co.uk>
Date:   Fri Sep 30 15:42:27 2016 +0200

    Refactor to be able to unit-test Document::ViewCallback()
    
    Change-Id: If07eb89e5d2f45737a8af10803511ab2ee3a07b3

diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index dfc5bf2..1ee6ecd 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -46,6 +46,10 @@ public:
     /// Get a view ID <-> user name map.
     virtual
     std::map<int, std::string> getViewInfo() = 0;
+    virtual
+    std::mutex& getMutex() = 0;
+    virtual
+    std::shared_ptr<TileQueue>& getTileQueue() = 0;
 };
 
 /// Represents a session to the WSD process, in a Kit process. Note that this is not a singleton.
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index d34fe47..f73de13 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -53,6 +53,7 @@
 #include "Common.hpp"
 #include "IoUtil.hpp"
 #include "LOKitHelper.hpp"
+#include "LOOLKit.hpp"
 #include "LOOLProtocol.hpp"
 #include "LibreOfficeKit.hpp"
 #include "Log.hpp"
@@ -95,6 +96,7 @@ static std::shared_ptr<Document> document;
 
 namespace
 {
+#ifndef BUILDING_TESTS
     typedef enum { COPY_ALL, COPY_LO, COPY_NO_USR } LinkOrCopyType;
     LinkOrCopyType linkOrCopyType;
     std::string sourceForLinkOrCopy;
@@ -251,6 +253,7 @@ namespace
             throw Exception("symlink() failed");
         }
     }
+#endif
 }
 
 /// Connection thread with a client (via WSD).
@@ -385,14 +388,6 @@ public:
     /// 2) Document which require password to modify
     enum class PasswordType { ToView, ToModify };
 
-    /// Descriptor class used to link a LOK
-    /// callback to a specific view.
-    struct CallbackDescriptor
-    {
-        Document* const Doc;
-        const int ViewId;
-    };
-
 public:
     Document(const std::shared_ptr<lok::Office>& loKit,
              const std::string& jailId,
@@ -811,8 +806,6 @@ public:
         _ws->sendFrame(message.data(), message.size());
     }
 
-private:
-
     static void GlobalCallback(const int nType, const char* pPayload, void* pData)
     {
         if (TerminationFlag)
@@ -853,7 +846,7 @@ private:
                      << "] [" << LOKitHelper::kitCallbackTypeToString(nType)
                      << "] [" << payload << "]." << Log::end;
 
-        std::unique_lock<std::mutex> lock(pDescr->Doc->_mutex);
+        std::unique_lock<std::mutex> lock(pDescr->Doc->getMutex());
 
         if (nType == LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR ||
             nType == LOK_CALLBACK_CELL_CURSOR)
@@ -867,7 +860,7 @@ private:
                 auto cursorWidth = std::stoi(tokens[2]);
                 auto cursorHeight = std::stoi(tokens[3]);
 
-                pDescr->Doc->_tileQueue->updateCursorPosition(0, 0, cursorX, cursorY, cursorWidth, cursorHeight);
+                pDescr->Doc->getTileQueue()->updateCursorPosition(0, 0, cursorX, cursorY, cursorWidth, cursorHeight);
             }
         }
         else if (nType == LOK_CALLBACK_INVALIDATE_VIEW_CURSOR ||
@@ -888,13 +881,15 @@ private:
                 auto cursorWidth = std::stoi(tokens[2]);
                 auto cursorHeight = std::stoi(tokens[3]);
 
-                pDescr->Doc->_tileQueue->updateCursorPosition(std::stoi(viewId), std::stoi(part), cursorX, cursorY, cursorWidth, cursorHeight);
+                pDescr->Doc->getTileQueue()->updateCursorPosition(std::stoi(viewId), std::stoi(part), cursorX, cursorY, cursorWidth, cursorHeight);
             }
         }
 
-        pDescr->Doc->_tileQueue->put("callback " + std::to_string(pDescr->ViewId) + " " + std::to_string(nType) + " " + payload);
+        pDescr->Doc->getTileQueue()->put("callback " + std::to_string(pDescr->ViewId) + " " + std::to_string(nType) + " " + payload);
     }
 
+private:
+
     static void DocumentCallback(const int nType, const char* pPayload, void* pData)
     {
         if (TerminationFlag)
@@ -1014,6 +1009,16 @@ private:
         return viewInfo;
     };
 
+    std::mutex& getMutex() override
+    {
+        return _mutex;
+    }
+
+    std::shared_ptr<TileQueue>& getTileQueue() override
+    {
+        return _tileQueue;
+    }
+
     /// Notify all views of viewId and their associated usernames
     void notifyViewInfo() override
     {
@@ -1335,6 +1340,12 @@ private:
     std::atomic_size_t _clientViews;
 };
 
+void documentViewCallback(const int nType, const char* pPayload, void* pData)
+{
+    Document::ViewCallback(nType, pPayload, pData);
+}
+
+#ifndef BUILDING_TESTS
 void lokit_main(const std::string& childRoot,
                 const std::string& sysTemplate,
                 const std::string& loTemplate,
@@ -1636,6 +1647,7 @@ void lokit_main(const std::string& childRoot,
     Log::info("Process finished.");
     std::_Exit(Application::EXIT_OK);
 }
+#endif
 
 /// Initializes LibreOfficeKit for cross-fork re-use.
 bool globalPreinit(const std::string &loTemplate)
@@ -1696,10 +1708,12 @@ bool globalPreinit(const std::string &loTemplate)
 namespace Util
 {
 
+#ifndef BUILDING_TESTS
 void alertAllUsers(const std::string& cmd, const std::string& kind)
 {
     document->sendTextFrame("errortoall: cmd=" + cmd + " kind=" + kind);
 }
+#endif
 
 }
 
diff --git a/loolwsd/LOOLKit.hpp b/loolwsd/LOOLKit.hpp
index 4206133..a7e0dab 100644
--- a/loolwsd/LOOLKit.hpp
+++ b/loolwsd/LOOLKit.hpp
@@ -17,7 +17,18 @@ void lokit_main(const std::string& childRoot,
                 bool queryVersionInfo);
 
 bool globalPreinit(const std::string &loTemplate);
+/// Wrapper around private Document::ViewCallback().
+void documentViewCallback(const int nType, const char* pPayload, void* pData);
 
+class IDocumentManager;
+
+/// Descriptor class used to link a LOK
+/// callback to a specific view.
+struct CallbackDescriptor
+{
+    IDocumentManager* const Doc;
+    const int ViewId;
+};
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/test/Makefile.am b/loolwsd/test/Makefile.am
index cbe1135..232872c 100644
--- a/loolwsd/test/Makefile.am
+++ b/loolwsd/test/Makefile.am
@@ -31,6 +31,7 @@ AM_CPPFLAGS = -pthread -I$(top_srcdir) -DBUILDING_TESTS
 wsd_sources = \
             ../IoUtil.cpp \
             ../Log.cpp \
+            ../LOOLKit.cpp \
             ../LOOLProtocol.cpp \
             ../LOOLSession.cpp \
             ../TileCache.cpp \


More information about the Libreoffice-commits mailing list