Browse Source

[5189] Addressed review comments.

Marcin Siodelski 8 years ago
parent
commit
085a5708e4

+ 31 - 26
src/lib/asiolink/tests/unix_domain_socket_unittest.cc

@@ -171,41 +171,36 @@ TEST_F(UnixDomainSocketTest, asyncSendReceive) {
     ASSERT_GT(sent_size, 0);
 
     // Receive response from the socket.
-    std::array<char, 1024> read_buf;
+    std::array<char, 2> read_buf;
     size_t bytes_read = 0;
-    bool receive_handler_invoked = false;
-    ASSERT_NO_THROW(socket.asyncReceive(&read_buf[0], read_buf.size(),
-        [this, &receive_handler_invoked, &bytes_read]
-            (const boost::system::error_code& ec, size_t length) mutable {
-        // Indicate that the handler has been called to interrupt the
-        // loop below.
-        receive_handler_invoked = true;
+    std::string response;
+    std::string expected_response = "received foo";
+    // Run IO service until we get the full response from the server.
+    while ((bytes_read < expected_response.size()) && !test_socket_->isStopped()) {
+        ASSERT_NO_THROW(socket.asyncReceive(&read_buf[0], read_buf.size(),
+            [this, &read_buf, &response, &bytes_read]
+                (const boost::system::error_code& ec, size_t length) {
+            // If we have been successful receiving the data, record the number of
+            // bytes received.
+            if (!ec) {
+                bytes_read += length;
+                response.append(&read_buf[0], length);
+
+            } else if (ec.value() != boost::asio::error::operation_aborted) {
+                ADD_FAILURE() << "error occurred while asynchronously receiving"
+                    " data via unix domain socket: " << ec.message();
+            }
 
-        // If we have been successful receiving the data, record the number of
-        // bytes received.
-        if (!ec) {
-            bytes_read = length;
+        }));
 
-        } else if (ec.value() != boost::asio::error::operation_aborted) {
-            ADD_FAILURE() << "error occurred while asynchronously receiving"
-                " data via unix domain socket: " << ec.message();
-        }
-
-    }));
-    // Run IO service until we get some response from the server.
-    while (!receive_handler_invoked && !test_socket_->isStopped()) {
         io_service_.run_one();
     }
 
     // Make sure we have received something.
     ASSERT_GT(bytes_read, 0);
 
-    std::string response(&read_buf[0], bytes_read);
-
-    // What we have received should be a substring of the sent data prepended
-    // with 'received'. For such short chunks of data it is usually 'received foo'
-    // that we receive but there is no guarantee.
-    EXPECT_EQ(0, std::string("received foo").find(response));
+    // Check that the entire response has been received and is correct.
+    EXPECT_EQ(expected_response, response);
 }
 
 // This test verifies that UnixDomainSocketError exception is thrown
@@ -227,6 +222,16 @@ TEST_F(UnixDomainSocketTest, clientErrors) {
 TEST_F(UnixDomainSocketTest, asyncClientErrors) {
     UnixDomainSocket socket(io_service_);
 
+    // Asynchronous operations signal errors through boost::system::error_code
+    // object passed to the handler function. This object casts to boolean.
+    // In case of success the object casts to false. In case of an error it
+    // casts to true. The actual error codes can be retrieved by comparing the
+    // ec objects to predefined error objects. We don't check for the actual
+    // errors here, because it is not certain that the same error codes would
+    // be returned on various operating systems.
+
+    // In the following tests we use C++11 lambdas as callbacks.
+
     // Connect
     bool connect_handler_invoked = false;
     socket.asyncConnect(unixSocketFilePath(),

+ 8 - 0
src/lib/asiolink/testutils/test_server_unix_socket.h

@@ -145,9 +145,17 @@ private:
     bool running_;
 
     /// @brief Mutex used by the server.
+    ///
+    /// Mutex is used in situations when server's IO service is being run in a
+    /// thread to synchronize this thread with a main thread using
+    /// @ref signalRunning and @ref waitForRunning.
     isc::util::thread::Mutex mutex_;
 
     /// @brief Conditional variable used by the server.
+    ///
+    /// Conditional variable is used in situations when server's IO service is
+    /// being run in a thread to synchronize this thread with a main thread
+    /// using @ref signalRunning and @ref waitForRunning.
     isc::util::thread::CondVar condvar_;
 };
 

+ 2 - 0
src/lib/cc/json_feed.h

@@ -156,6 +156,8 @@ public:
 
     /// @brief Returns processed data as a structure of @ref isc::data::Element
     /// objects.
+    ///
+    /// @throw JSONFeedError if the received JSON is not well formed.
     data::ElementPtr toElement() const;
 
     /// @brief Receives additional data read from a data stream.

+ 2 - 1
src/lib/config/client_connection.cc

@@ -53,7 +53,8 @@ public:
     ///
     /// @param buffer Pointer to the buffer holding input data.
     /// @param length Length of the data in the input buffer.
-    /// @param handler User supplied callback.
+    /// @param handler User supplied callback invoked after the chunk of data
+    /// has been sent.
     void doSend(const void* buffer, const size_t length,
                 ClientConnection::Handler handler);
 

+ 1 - 1
src/lib/config/client_connection.h

@@ -139,7 +139,7 @@ public:
     /// occurred during the transaction.
     /// @param timeout Connection timeout in milliseconds.
     void start(const SocketPath& socket_path, const ControlCommand& command,
-               Handler handler, const Timeout& timeout = Timeout(10000));
+               Handler handler, const Timeout& timeout = Timeout(5000));
 
 private:
 

+ 8 - 0
src/lib/config/tests/client_connection_unittests.cc

@@ -52,6 +52,14 @@ public:
     /// If the KEA_SOCKET_TEST_DIR environment variable is specified, the
     /// socket file is created in the location pointed to by this variable.
     /// Otherwise, it is created in the build directory.
+    ///
+    /// The KEA_SOCKET_TEST_DIR is typically used to overcome the problem of
+    /// a system limit on the unix socket file path (usually 102 or 103 characters).
+    /// When Kea build is located in the nested directories with absolute path
+    /// exceeding this limit, the test system should be configured to set
+    /// the KEA_SOCKET_TEST_DIR environmental variable to point to an alternative
+    /// location, e.g. /tmp, with an absolute path length being within the
+    /// allowed range.
     static std::string unixSocketFilePath() {
         std::ostringstream s;
         const char* env = getenv("KEA_SOCKET_TEST_DIR");