Split ContainerConnection into two classes
authorPiotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
Fri, 25 Apr 2014 14:04:31 +0000 (16:04 +0200)
committerJan Olszak <j.olszak@samsung.com>
Mon, 19 May 2014 11:47:16 +0000 (13:47 +0200)
[Bug/Feature]   ContainerConnection had two separate functionalities
[Cause]         N/A
[Solution]      Extract transport class from connection class
[Verification]  Build, install, run tests, run server

Change-Id: I165089d861a40e94f13bba31d61bce3b7571ff4e

server/container-connection-transport.cpp [new file with mode: 0644]
server/container-connection-transport.hpp [new file with mode: 0644]
server/container-connection.cpp
server/container-connection.hpp
server/container.cpp
server/container.hpp
unit_tests/server/ut-container-connection.cpp

diff --git a/server/container-connection-transport.cpp b/server/container-connection-transport.cpp
new file mode 100644 (file)
index 0000000..e1012eb
--- /dev/null
@@ -0,0 +1,91 @@
+/*
+ *  Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *  Contact: Piotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License
+ */
+
+/**
+ * @file
+ * @author  Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com)
+ * @brief   Implementation of a class for communication transport between container and server
+ */
+
+#include "container-connection-transport.hpp"
+#include "exception.hpp"
+
+#include "utils/file-wait.hpp"
+#include "utils/fs.hpp"
+#include "log/logger.hpp"
+
+
+namespace security_containers {
+
+namespace {
+
+// Timeout in ms for waiting for dbus transport.
+// Should be very long to ensure dbus in container is ready.
+const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000;
+
+} // namespace
+
+
+ContainerConnectionTransport::ContainerConnectionTransport(const std::string& runMountPoint)
+    : mRunMountPoint(runMountPoint)
+{
+    if (runMountPoint.empty()) {
+        return;
+    }
+    if (!utils::createDirectories(runMountPoint, 0755)) {
+        LOGE("Initialization failed: could not create " << runMountPoint);
+        throw ContainerConnectionException("Could not create: " + runMountPoint);
+    }
+
+    // try to umount if already mounted
+    utils::umount(runMountPoint);
+
+    if (!utils::mountTmpfs(runMountPoint)) {
+        LOGE("Initialization failed: could not mount " << runMountPoint);
+        throw ContainerConnectionException("Could not mount: " + runMountPoint);
+    }
+}
+
+
+ContainerConnectionTransport::~ContainerConnectionTransport()
+{
+    if (!mRunMountPoint.empty()) {
+        if (!utils::umount(mRunMountPoint)) {
+            LOGE("Deinitialization failed: could not umount " << mRunMountPoint);
+        }
+    }
+}
+
+
+std::string ContainerConnectionTransport::acquireAddress()
+{
+    if (mRunMountPoint.empty()) {
+        return std::string();
+    }
+
+    const std::string dbusPath = mRunMountPoint + "/dbus/system_bus_socket";
+
+    // TODO This should be done asynchronously.
+    LOGT("Waiting for " << dbusPath);
+    utils::waitForFile(dbusPath, TRANSPORT_READY_TIMEOUT);
+
+    return "unix:path=" + dbusPath;
+}
+
+
+} // namespace security_containers
diff --git a/server/container-connection-transport.hpp b/server/container-connection-transport.hpp
new file mode 100644 (file)
index 0000000..989ba96
--- /dev/null
@@ -0,0 +1,57 @@
+/*
+ *  Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *  Contact: Piotr Bartosiewicz <p.bartosiewi@partner.samsung.com>
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License
+ */
+
+/**
+ * @file
+ * @author  Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com)
+ * @brief   Declaration of a class for communication transport between container and server
+ */
+
+
+#ifndef SERVER_CONTAINER_CONNECTION_TRANSPORT_HPP
+#define SERVER_CONTAINER_CONNECTION_TRANSPORT_HPP
+
+#include "dbus/connection.hpp"
+
+
+namespace security_containers {
+
+
+/**
+ * This class provides a communication transport between container and server.
+ * The object lifecycle should cover lifecycle of a container.
+ */
+class ContainerConnectionTransport {
+public:
+    ContainerConnectionTransport(const std::string& runMountPoint);
+    ~ContainerConnectionTransport();
+
+    /**
+     * Gets dbus addres. Will block until address is available.
+     */
+    std::string acquireAddress();
+
+private:
+    std::string mRunMountPoint;
+};
+
+
+} // namespace security_containers
+
+
+#endif // SERVER_CONTAINER_CONNECTION_TRANSPORT_HPP
index 7a3e973..ed6bdd8 100644 (file)
 /**
  * @file
  * @author  Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com)
- * @brief   Implementation of class for communication between container and server
+ * @brief   Implementation of class for communication between container and server
  */
 
 #include "container-connection.hpp"
 #include "container-dbus-definitions.hpp"
 #include "exception.hpp"
 
-#include "utils/file-wait.hpp"
-#include "utils/fs.hpp"
-#include "utils/paths.hpp"
 #include "log/logger.hpp"
 
 
@@ -36,10 +33,6 @@ namespace security_containers {
 
 namespace {
 
-// Timeout in ms for waiting for dbus transport.
-// Should be very long to ensure dbus in container is ready.
-const unsigned int TRANSPORT_READY_TIMEOUT = 2 * 60 * 1000;
-
 // Timeout in ms for waiting for dbus name.
 // Can happen if glib loop is busy or not present.
 const unsigned int NAME_ACQUIRED_TIMEOUT = 5 * 1000;
@@ -56,56 +49,18 @@ ContainerConnection::ContainerConnection()
 
 ContainerConnection::~ContainerConnection()
 {
-    deinitialize();
-}
-
-
-void ContainerConnection::initialize(const std::string& runMountPoint)
-{
-    if (runMountPoint.empty()) {
-        return;
-    }
-    if (!utils::createDirectories(runMountPoint, 0755)) {
-        LOGE("Initialization failed: could not create " << runMountPoint);
-        throw ContainerConnectionException("Could not create: " + runMountPoint);
-    }
-
-    // try to umount if already mounted
-    utils::umount(runMountPoint);
-
-    if (!utils::mountTmpfs(runMountPoint)) {
-        LOGE("Initialization failed: could not mount " << runMountPoint);
-        throw ContainerConnectionException("Could not mount: " + runMountPoint);
-    }
-
-    mRunMountPoint = runMountPoint;
-}
-
-void ContainerConnection::deinitialize()
-{
-    if (!mRunMountPoint.empty()) {
-        if (!utils::umount(mRunMountPoint)) {
-            LOGE("Deinitialization failed: could not umount " << mRunMountPoint);
-        }
-        mRunMountPoint.clear();
-    }
 }
 
 
-void ContainerConnection::connect()
+void ContainerConnection::connect(const std::string& address)
 {
-    if (mRunMountPoint.empty()) {
+    if (address.empty()) {
         LOGW("The connection to the container is disabled");
         return;
     }
 
-    const std::string dbusPath = mRunMountPoint + "/dbus/system_bus_socket";
-
-    // TODO This should be done asynchronously.
-    LOGT("Waiting for " << dbusPath);
-    utils::waitForFile(dbusPath, TRANSPORT_READY_TIMEOUT);
-    LOGT("Connecting to DBUS");
-    mDbusConnection = dbus::DbusConnection::create("unix:path=" + dbusPath);
+    LOGT("Connecting to DBUS on " << address);
+    mDbusConnection = dbus::DbusConnection::create(address);
     LOGT("Setting DBUS name");
 
     mDbusConnection->setName(api::BUS_NAME,
index e825be2..f3c6f1d 100644 (file)
@@ -19,7 +19,7 @@
 /**
  * @file
  * @author  Piotr Bartosiewicz (p.bartosiewi@partner.samsung.com)
- * @brief   Declaration of the class for communication between container and server
+ * @brief   Declaration of a class for communication between container and server
  */
 
 
@@ -41,22 +41,10 @@ public:
     ContainerConnection();
     ~ContainerConnection();
 
-    //TODO Move initialize, deinitialize and related stuff to separate class
-
-    /**
-     * Should be called every time just before container is created.
-     */
-    void initialize(const std::string& runMountPoint);
-
-    /**
-     * Should be called every time after container is destroyed.
-     */
-    void deinitialize();
-
     /**
      * Connect to container.
      */
-    void connect();
+    void connect(const std::string& address);
 
     /**
      * Disconnect from container.
@@ -82,7 +70,6 @@ public:
                           const std::string& message);
 
 private:
-    std::string mRunMountPoint;
     dbus::DbusConnection::Pointer mDbusConnection;
     std::mutex mNameMutex;
     std::condition_variable mNameCondition;
index 28ed3a6..6474fb1 100644 (file)
@@ -65,9 +65,9 @@ int Container::getPrivilege()
 
 void Container::start()
 {
-    mConnection.initialize(mConfig.runMountPoint);
+    mConnectionTransport.reset(new ContainerConnectionTransport(mConfig.runMountPoint));
     mAdmin->start();
-    mConnection.connect();
+    mConnection.connect(mConnectionTransport->acquireAddress());
 
     // Send to the background only after we're connected,
     // otherwise it'd take ages.
@@ -79,7 +79,7 @@ void Container::stop()
 {
     mConnection.disconnect();
     mAdmin->stop();
-    mConnection.deinitialize();
+    mConnectionTransport.reset();
 }
 
 void Container::goForeground()
index 3774c6b..179a3bb 100644 (file)
@@ -29,6 +29,7 @@
 #include "container-config.hpp"
 #include "container-admin.hpp"
 #include "container-connection.hpp"
+#include "container-connection-transport.hpp"
 
 #include <string>
 #include <memory>
@@ -98,6 +99,7 @@ public:
 private:
     ContainerConfig mConfig;
     std::unique_ptr<ContainerAdmin> mAdmin;
+    std::unique_ptr<ContainerConnectionTransport> mConnectionTransport;
     ContainerConnection mConnection;
 };
 
index 679cf60..95203b0 100644 (file)
@@ -26,6 +26,7 @@
 #include "ut.hpp"
 
 #include "container-connection.hpp"
+#include "container-connection-transport.hpp"
 #include "container-dbus-definitions.hpp"
 
 #include "dbus/connection.hpp"
@@ -51,19 +52,24 @@ const char* const DBUS_DAEMON_ARGS[] = {
     NULL
 };
 
-// TODO fix destruction order - move transport stuff to separate raii class
 const std::string TRANSPORT_MOUNT_POINT = "/tmp/ut-container-connection";
-const std::string DBUS_ADDRESS = "unix:path=" + TRANSPORT_MOUNT_POINT + "/dbus/system_bus_socket";
 const int EVENT_TIMEOUT = 1000;
 
-class ScopedDbusDaemon : public ScopedDaemon {
+class ScopedDbusDaemon {
 public:
-    void start()
+    ScopedDbusDaemon()
+        : mTransport(TRANSPORT_MOUNT_POINT)
     {
         utils::createDirectory(TRANSPORT_MOUNT_POINT + "/dbus", 0755);
         mDaemon.start(DBUS_DAEMON_PROC, DBUS_DAEMON_ARGS);
     }
+
+    std::string acquireAddress()
+    {
+        return mTransport.acquireAddress();
+    }
 private:
+    ContainerConnectionTransport mTransport;
     ScopedDaemon mDaemon;
 };
 
@@ -77,28 +83,24 @@ BOOST_AUTO_TEST_CASE(ConstructorDestructorTest)
 
 BOOST_AUTO_TEST_CASE(ConnectTest)
 {
-    ScopedDbusDaemon dbus;
     ScopedGlibLoop loop;
+    ScopedDbusDaemon dbus;
 
     ContainerConnection connection;
-    connection.initialize(TRANSPORT_MOUNT_POINT);
-    dbus.start();
 
-    BOOST_REQUIRE_NO_THROW(connection.connect());
+    BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress()));
     BOOST_REQUIRE_NO_THROW(connection.disconnect());
 }
 
 BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest)
 {
-    ScopedDbusDaemon dbus;
     ScopedGlibLoop loop;
+    ScopedDbusDaemon dbus;
 
     Latch notifyCalled;
     ContainerConnection connection;
-    connection.initialize(TRANSPORT_MOUNT_POINT);
-    dbus.start();
 
-    BOOST_REQUIRE_NO_THROW(connection.connect());
+    BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress()));
 
     auto callback = [&](const std::string& application, const std::string& message) {
         if (application == "testapp" && message == "testmessage") {
@@ -107,7 +109,7 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest)
     };
     connection.setNotifyActiveContainerCallback(callback);
 
-    DbusConnection::Pointer client = DbusConnection::create(DBUS_ADDRESS);
+    DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress());
     client->callMethod(api::BUS_NAME,
                        api::OBJECT_PATH,
                        api::INTERFACE,
@@ -119,17 +121,15 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest)
 
 BOOST_AUTO_TEST_CASE(SignalNotificationApiTest)
 {
-    ScopedDbusDaemon dbus;
     ScopedGlibLoop loop;
+    ScopedDbusDaemon dbus;
 
     Latch signalEmitted;
     ContainerConnection connection;
-    connection.initialize(TRANSPORT_MOUNT_POINT);
-    dbus.start();
 
-    BOOST_REQUIRE_NO_THROW(connection.connect());
+    BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress()));
 
-    DbusConnection::Pointer client = DbusConnection::create(DBUS_ADDRESS);
+    DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress());
 
     auto handler = [&](const std::string& /*senderBusName*/,
                        const std::string& objectPath,