[Libreoffice-commits] online.git: 5 commits - loolwsd/ChildSession.hpp loolwsd/ClientSession.hpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLKit.cpp loolwsd/LOOLSession.hpp loolwsd/LOOLWSD.cpp loolwsd/MessageQueue.cpp loolwsd/MessageQueue.hpp loolwsd/test loolwsd/Util.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Mon Oct 10 06:31:19 UTC 2016


 loolwsd/ChildSession.hpp                 |   17 ++++++++++++++
 loolwsd/ClientSession.hpp                |    5 ++--
 loolwsd/DocumentBroker.cpp               |   17 +++++++++++---
 loolwsd/DocumentBroker.hpp               |    1 
 loolwsd/LOOLKit.cpp                      |   36 ++++++++++++++++++++++++++-----
 loolwsd/LOOLSession.hpp                  |    2 +
 loolwsd/LOOLWSD.cpp                      |   29 ++++++++++++++++++------
 loolwsd/MessageQueue.cpp                 |   17 ++++++++++++--
 loolwsd/MessageQueue.hpp                 |    3 +-
 loolwsd/Util.hpp                         |   24 ++++++++++++++++++++
 loolwsd/test/WhiteBoxTests.cpp           |    5 ++++
 loolwsd/test/httpwstest.cpp              |   16 +------------
 loolwsd/test/integration-http-server.cpp |    1 
 13 files changed, 137 insertions(+), 36 deletions(-)

New commits:
commit 1edd37ea2dbe1e3d140115396b85bc09be90af4b
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Oct 9 13:42:30 2016 -0400

    loolwsd: fix convert-to handling
    
    Change-Id: I24b8c0b7129bee2692696809613ee49a38024c98
    Reviewed-on: https://gerrit.libreoffice.org/29649
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 2cbfd88..2b3b205 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -432,7 +432,9 @@ private:
 
                     auto uriPublic = DocumentBroker::sanitizeURI(fromPath);
                     const auto docKey = DocumentBroker::getDocKey(uriPublic);
+                    Log::debug("New DocumentBroker for docKey [" + docKey + "].");
                     auto docBroker = std::make_shared<DocumentBroker>(uriPublic, docKey, LOOLWSD::ChildRoot, child);
+                    child->setDocumentBroker(docBroker);
 
                     // This lock could become a bottleneck.
                     // In that case, we can use a pool and index by publicPath.
@@ -474,14 +476,24 @@ private:
                     session->handleInput(saveas.data(), saveas.size());
 
                     // Send it back to the client.
-                    Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS));
-                    if (!resultURL.getPath().empty())
+                    try
+                    {
+                        Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS));
+                        Log::trace("Save-as URL: " + resultURL.toString());
+
+                        if (!resultURL.getPath().empty())
+                        {
+                            const std::string mimeType = "application/octet-stream";
+                            std::string encodedFilePath;
+                            URI::encode(resultURL.getPath(), "", encodedFilePath);
+                            Log::trace("Sending file: " + encodedFilePath);
+                            response.sendFile(encodedFilePath, mimeType);
+                            sent = true;
+                        }
+                    }
+                    catch (const std::exception& ex)
                     {
-                        const std::string mimeType = "application/octet-stream";
-                        std::string encodedFilePath;
-                        URI::encode(resultURL.getPath(), "", encodedFilePath);
-                        response.sendFile(encodedFilePath, mimeType);
-                        sent = true;
+                        Log::error(std::string("Failed to get save-as url: ") + ex.what());
                     }
 
                     lock.lock();
@@ -495,6 +507,8 @@ private:
                     {
                         Log::error("Multiple sessions during conversion. " + std::to_string(sessionsCount) + " sessions remain.");
                     }
+
+                    session->shutdownPeer(WebSocket::WS_NORMAL_CLOSE, "");
                 }
 
                 // Clean up the temporary directory the HTMLForm ctor created.
commit 366e0e21d5733808fda7080a013bbb6c4096b669
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Oct 9 11:59:00 2016 -0400

    loolwsd: support timeout on MessageQueue get
    
    Change-Id: Iaad39aaa06c59cdacdd4a864599ef6a4a12976f8
    Reviewed-on: https://gerrit.libreoffice.org/29648
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index e9bc176..3be6277 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -39,9 +39,9 @@ public:
      * Return the URL of the saved-as document when it's ready. If called
      * before it's ready, the call blocks till then.
      */
-    std::string getSaveAsUrl()
+    std::string getSaveAsUrl(const unsigned timeoutMs)
     {
-        const auto payload = _saveAsQueue.get();
+        const auto payload = _saveAsQueue.get(timeoutMs);
         return std::string(payload.data(), payload.size());
     }
 
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 1889987..2cbfd88 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -474,8 +474,7 @@ private:
                     session->handleInput(saveas.data(), saveas.size());
 
                     // Send it back to the client.
-                    //TODO: Should have timeout to avoid waiting forever.
-                    Poco::URI resultURL(session->getSaveAsUrl());
+                    Poco::URI resultURL(session->getSaveAsUrl(COMMAND_TIMEOUT_MS));
                     if (!resultURL.getPath().empty())
                     {
                         const std::string mimeType = "application/octet-stream";
diff --git a/loolwsd/MessageQueue.cpp b/loolwsd/MessageQueue.cpp
index ffa5f79..3729018 100644
--- a/loolwsd/MessageQueue.cpp
+++ b/loolwsd/MessageQueue.cpp
@@ -32,10 +32,23 @@ void MessageQueue::put(const Payload& value)
     _cv.notify_one();
 }
 
-MessageQueue::Payload MessageQueue::get()
+MessageQueue::Payload MessageQueue::get(const unsigned timeoutMs)
 {
     std::unique_lock<std::mutex> lock(_mutex);
-    _cv.wait(lock, [this] { return wait_impl(); });
+
+    if (timeoutMs > 0)
+    {
+        if (!_cv.wait_for(lock, std::chrono::milliseconds(timeoutMs),
+                          [this] { return wait_impl(); }))
+        {
+            throw std::runtime_error("Timed out waiting to get queue item.");
+        }
+    }
+    else
+    {
+        _cv.wait(lock, [this] { return wait_impl(); });
+    }
+
     return get_impl();
 }
 
diff --git a/loolwsd/MessageQueue.hpp b/loolwsd/MessageQueue.hpp
index 8ce2287..95a5654 100644
--- a/loolwsd/MessageQueue.hpp
+++ b/loolwsd/MessageQueue.hpp
@@ -43,7 +43,8 @@ public:
     }
 
     /// Thread safe obtaining of the message.
-    Payload get();
+    /// timeoutMs can be 0 to signify infinity.
+    Payload get(const unsigned timeoutMs = 0);
 
     /// Thread safe removal of all the pending messages.
     void clear();
diff --git a/loolwsd/test/integration-http-server.cpp b/loolwsd/test/integration-http-server.cpp
index 6899476..25aea88 100644
--- a/loolwsd/test/integration-http-server.cpp
+++ b/loolwsd/test/integration-http-server.cpp
@@ -243,6 +243,7 @@ void HTTPServerTest::testConvertTo()
 {
     const auto srcPath = Util::getTempFilePath(TDOC, "hello.odt");
     std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
+    session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds.
 
     Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to");
     Poco::Net::HTMLForm form;
commit 128cd73154894cc63cb16cc1d47c5b905cb1f7a6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Oct 9 11:28:22 2016 -0400

    loolwsd: send child messages to client via unified wsd-kit WS
    
    Change-Id: I237120e5a81a2e6d8772a2b6f1e98b1ba567f97e
    Reviewed-on: https://gerrit.libreoffice.org/29647
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index e8e23ac..d8d68bd 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -50,6 +50,9 @@ public:
     std::mutex& getMutex() = 0;
     virtual
     std::shared_ptr<TileQueue>& getTileQueue() = 0;
+
+    virtual
+    bool sendTextFrame(const std::string& message) = 0;
 };
 
 /// Represents a session to the WSD process, in a Kit process. Note that this is not a singleton.
@@ -77,6 +80,20 @@ public:
 
     void loKitCallback(const int nType, const std::string& rPayload);
 
+    bool sendTextFrame(const char* buffer, const int length) override
+    {
+        const auto msg = "client-" + getId() + ' ' + std::string(buffer, length);
+
+        const auto lock = getLock();
+
+        return _docManager.sendTextFrame(msg);
+    }
+
+    bool sendTextFrame(const std::string& text)
+    {
+        return sendTextFrame(text.data(), text.size());
+    }
+
 private:
     bool loadDocument(const char *buffer, int length, Poco::StringTokenizer& tokens);
 
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index 7508802..e9bc176 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -30,6 +30,7 @@ public:
     bool isReadOnly() const { return _isReadOnly; }
 
     void setPeer(const std::shared_ptr<PrisonerSession>& peer) { _peer = peer; }
+    std::shared_ptr<PrisonerSession> getPeer() const { return _peer.lock(); }
     bool shutdownPeer(Poco::UInt16 statusCode, const std::string& message);
 
     void setUserName(const std::string& userName) { _userName = userName; }
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3201170..02686fa 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -756,7 +756,15 @@ bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vecto
         const auto it = _sessions.find(sid);
         if (it != _sessions.end())
         {
-            return it->second->sendTextFrame(message);
+            const auto peer = it->second->getPeer();
+            if (peer)
+            {
+                return peer->handleInput(message.data(), message.size());
+            }
+            else
+            {
+                Log::warn() << "Client session [" << sid << "] has no peer to forward message: " << message << Log::end;
+            }
         }
         else
         {
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 66ec29c..d8ccee4 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -64,6 +64,7 @@ public:
 
     void setDocumentBroker(const std::shared_ptr<DocumentBroker>& docBroker)
     {
+        assert(docBroker && "Invalid DocumentBroker instance.");
         _docBroker = docBroker;
     }
 
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 65f8fa2..3bb689d 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -40,6 +40,7 @@
 #include <Poco/Net/HTTPRequest.h>
 #include <Poco/Net/HTTPResponse.h>
 #include <Poco/Net/NetException.h>
+#include <Poco/Net/Socket.h>
 #include <Poco/Net/WebSocket.h>
 #include <Poco/NotificationQueue.h>
 #include <Poco/Process.h>
@@ -78,6 +79,7 @@ using Poco::Net::HTTPClientSession;
 using Poco::Net::HTTPRequest;
 using Poco::Net::HTTPResponse;
 using Poco::Net::NetException;
+using Poco::Net::Socket;
 using Poco::Net::WebSocket;
 using Poco::Path;
 using Poco::Process;
@@ -799,11 +801,34 @@ public:
         ws->sendFrame(response.data(), length, WebSocket::FRAME_BINARY);
     }
 
-    void sendTextFrame(const std::string& message)
+    bool sendTextFrame(const std::string& message) override
     {
-        std::lock_guard<std::mutex> lock(_mutex);
+        try
+        {
+            if (!_ws || _ws->poll(Poco::Timespan(0), Socket::SelectMode::SELECT_ERROR))
+            {
+                Log::error("Child Doc: Bad socket while sending [" + getAbbreviatedMessage(message) + "].");
+                return false;
+            }
+
+            const auto length = message.size();
+            if (length > SMALL_MESSAGE_SIZE)
+            {
+                const std::string nextmessage = "nextmessage: size=" + std::to_string(length);
+                _ws->sendFrame(nextmessage.data(), nextmessage.size());
+            }
 
-        _ws->sendFrame(message.data(), message.size());
+            _ws->sendFrame(message.data(), length);
+            return true;
+        }
+        catch (const Exception& exc)
+        {
+            Log::error() << "Document::sendTextFrame: "
+                         << "Exception: " << exc.displayText()
+                         << (exc.nested() ? "( " + exc.nested()->displayText() + ")" : "");
+        }
+
+        return false;
     }
 
     static void GlobalCallback(const int nType, const char* pPayload, void* pData)
diff --git a/loolwsd/LOOLSession.hpp b/loolwsd/LOOLSession.hpp
index 0606f4e..b07b2c6 100644
--- a/loolwsd/LOOLSession.hpp
+++ b/loolwsd/LOOLSession.hpp
@@ -42,7 +42,9 @@ public:
     const std::string& getName() const { return _name; }
     bool isDisconnected() const { return _disconnected; }
 
+    virtual
     bool sendBinaryFrame(const char *buffer, int length);
+    virtual
     bool sendTextFrame(const char* buffer, const int length);
     bool sendTextFrame(const std::string& text)
     {
diff --git a/loolwsd/test/WhiteBoxTests.cpp b/loolwsd/test/WhiteBoxTests.cpp
index d17cc22..45db538 100644
--- a/loolwsd/test/WhiteBoxTests.cpp
+++ b/loolwsd/test/WhiteBoxTests.cpp
@@ -192,6 +192,11 @@ public:
     {
         return _tileQueue;
     }
+
+    bool sendTextFrame(const std::string& /*message*/) override
+    {
+        return true;
+    }
 };
 
 void WhiteBoxTests::testEmptyCellCursor()
commit 22a5db2fc09a123f9c00135f7fc44c4d257c7a9d
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Oct 9 11:27:01 2016 -0400

    loolwsd: trim forwarded messages to avoid leading whitespace
    
    Change-Id: I932baf3ec41789d89bf897fcbf25a1ee1d27f89d
    Reviewed-on: https://gerrit.libreoffice.org/29646
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 6c6cbb8..3201170 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -725,7 +725,7 @@ void DocumentBroker::setModified(const bool value)
 bool DocumentBroker::forwardToChild(const std::string& viewId, const char *buffer, int length)
 {
     const auto message = std::string(buffer, length);
-    Log::warn() << "Forwarding payload to child [" << viewId << "]: " << message << Log::end;
+    Log::trace() << "Forwarding payload to child [" << viewId << "]: " << message << Log::end;
 
     const auto it = _sessions.find(viewId);
     if (it != _sessions.end())
@@ -745,8 +745,9 @@ bool DocumentBroker::forwardToChild(const std::string& viewId, const char *buffe
 
 bool DocumentBroker::forwardToClient(const std::string& prefix, const std::vector<char>& payload)
 {
-    const std::string message(payload.data() + prefix.size(), payload.size() - prefix.size());
-    Log::warn("Forwarding payload to client: " + message);
+    std::string message(payload.data() + prefix.size(), payload.size() - prefix.size());
+    Util::ltrim(message);
+    Log::trace("Forwarding payload to " + prefix + ' ' + message);
 
     std::string name;
     std::string sid;
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 41d39fc..65f8fa2 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -1227,8 +1227,9 @@ private:
 
     bool forwardToChild(const std::string& prefix, const std::vector<char>& payload)
     {
-        const std::string message(payload.data() + prefix.size(), payload.size() - prefix.size());
-        Log::trace("Forwarding payload to client: " + message);
+        std::string message(payload.data() + prefix.size(), payload.size() - prefix.size());
+        Util::ltrim(message);
+        Log::trace("Forwarding payload to " + prefix + ' ' + message);
 
         std::string name;
         std::string value;
diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp
index bd1a5a3..df4f60c 100644
--- a/loolwsd/Util.hpp
+++ b/loolwsd/Util.hpp
@@ -153,6 +153,30 @@ namespace Util
     /// Return a string that is unique across processes and calls.
     std::string UniqueId();
 
+    /// Trim spaces from the left. Just spaces.
+    inline
+    void ltrim(std::string& s)
+    {
+        const auto pos = s.find_first_not_of(" ");
+        if (pos != std::string::npos)
+        {
+            s = s.substr(pos);
+        }
+    }
+
+    /// Trim spaces from the left and copy. Just spaces.
+    inline
+    std::string ltrimmed(const std::string& s)
+    {
+        const auto pos = s.find_first_not_of(" ");
+        if (pos != std::string::npos)
+        {
+            return s.substr(pos);
+        }
+
+        return s;
+    }
+
     /// Given one or more patterns to allow, and one or more to deny,
     /// the match member will return true if, and only if, the subject
     /// matches the allowed list, but not the deny.
commit 0f2f49823ef02c9cebb32f32d4bcd4dd44e9351c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sat Oct 8 23:51:07 2016 -0400

    loolwsd: unittest cleanup
    
    Change-Id: I90e4fa7d5377b7ecf427c87ce068efdb4bffe933
    Reviewed-on: https://gerrit.libreoffice.org/29645
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index a6f0654..6876218 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -338,21 +338,9 @@ void HTTPWSTest::loadDoc(const std::string& documentURL, const std::string& test
         // Don't replace with helpers, so we catch status.
         Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, documentURL);
         auto socket = connectLOKit(_uri, request, _response, testname);
-        sendTextFrame(*socket, "load url=" + documentURL, testname);
+        sendTextFrame(socket, "load url=" + documentURL, testname);
 
-        SocketProcessor(testname, socket, [&](const std::string& msg)
-                {
-                    const std::string prefix = "status: ";
-                    if (msg.find(prefix) == 0)
-                    {
-                        const auto status = msg.substr(prefix.length());
-                        // Might be too strict, consider something flexible instread.
-                        CPPUNIT_ASSERT_EQUAL(std::string("type=text parts=1 current=0 width=12808 height=16408 viewid=0"), status);
-                        return false;
-                    }
-
-                    return true;
-                });
+        assertResponseString(socket, "status:", testname);
     }
     catch (const Poco::Exception& exc)
     {


More information about the Libreoffice-commits mailing list