Incremental commit.

Change-Id: I35f92e98d1ecda31999c7ca23c8f53c7f6a85d15
Signed-off-by: Ossama Othman <ossama.othman@intel.com>

diff --git a/plugins/connman/agent.cpp b/plugins/connman/agent.cpp
index 6bccc37..61e99cd 100644 (file)
--- a/plugins/connman/agent.cpp
+++ b/plugins/connman/agent.cpp
@@ -81,6 +81,76 @@ namespace
   // --------------------------------------------------------------
   //                   Connman Agent Handlers
   // --------------------------------------------------------------
+  /**
+   * @class agent_operation_handler
+   *
+   * @brief Handle agent operations through given callback.
+   *
+   * This class factors out common connection data retrieval code, and
+   * performs Agent operations through the given callback.
+   */
+  class agent_operation_handler
+  {
+    typedef ivi::settings::agent agent;
+    typedef agent::user_data data_type;
+
+    // Prevent copying.
+    agent_operation_handler(agent_operation_handler const &) = delete;
+    void operator=(agent_operation_handler const &) = delete;
+
+  public:
+
+    /**
+     * Constructor
+     *
+     * @param[in] service   D-Bus object path for connman service
+     * @param[in] user_data User/connection data
+     * @param[in] callback  Callback that implements Agent operation
+     */
+    agent_operation_handler(
+      gchar const * service,
+      gpointer user_data,
+      std::function<void(ivi::settings::agent::connect_data const &)>
+        callback)
+      : data_(static_cast<data_type *>(user_data))
+      , guard_(data_->lock)
+      , iterator_(data_->info.find(service))
+    {
+      // data_ should never be null!
+
+      if (iterator_ != data_->info.end())
+        callback(iterator_->second);
+    }
+
+    /// Destructor
+    ~agent_operation_handler()
+    {
+      // Done with the connection data.  Remove it from the map.
+      if (iterator_ != data_->info.end())
+        data_->info.erase(iterator_);
+    }
+
+  private:
+    /// User/connection data.
+    data_type * const data_;
+
+    /**
+     * Synchronize access to the user/connection data map.
+     *
+     * @note This lock will be held for the life of the
+     *       @c agent_operation_handler object, i.e. for the duration
+     *       of the operation.  This is needed to synchronize the map
+     *       @c erase() operation in the destructor.  The erase()
+     *       operation is performed in the destructor to ensure
+     *       element removal occurs if an exception is thrown by the
+     *       callback.
+     */
+    std::lock_guard<std::mutex> guard_;
+
+    /// Map iterator that points to the connection data.
+    agent::user_data::map_type::iterator const iterator_;
+  };
+
   bool
   on_handle_release(Agent * object,
                     GDBusMethodInvocation * invocation,
@@ -97,27 +167,13 @@ namespace
                          gchar const * error,
                          gpointer user_data)
   {
-    using namespace ivi::settings;
-    typedef agent::user_data data_type;
+    using ivi::settings::agent;
 
-    data_type * const data = static_cast<data_type *>(user_data);
-
-    {
-      std::lock_guard<std::mutex> guard(data->lock);
-
-      auto const i = data->info.find(service);
-      if (i != data->info.end()) {
-        auto & response = i->second.response;
-        response.send_error(error);
-
-        /**
-         * @todo Make sure map element removal occurs if exception is
-         *       thrown above.
-         */
-        // Done with the connection data.
-        data->info.erase(i);
-      }
-    }
+    agent_operation_handler(service,
+                            user_data,
+                            [error](agent::connect_data const & d){
+                              d.response.send_error(std::string(error));
+                            });
 
     agent_complete_report_error(object, invocation);
 
@@ -142,16 +198,14 @@ namespace
                           GVariant * fields,
                           gpointer user_data)
   {
-    using namespace ivi::settings;
-    typedef agent::user_data data_type;
+    using ivi::settings::agent;
 
-    data_type * const data = static_cast<data_type *>(user_data);
+    agent_operation_handler(
+      service,
+      user_data,
+      [fields](agent::connect_data const & /* d */){
+        using ivi::settings::unique_ptr;
 
-    {
-      std::lock_guard<std::mutex> guard(data->lock);
-
-      auto const i = data->info.find(service);
-      if (i != data->info.end()) {
         // Extract requested mandatory and alternate fields.
         GVariantIter * vi = nullptr;
         g_variant_get(fields, "(a{sv})", &vi);
@@ -166,29 +220,10 @@ namespace
 
 
         }
-
-
-        auto & response = i->second.response;
-
-        // Nothing to add to successful response.
-        response.send_response(
-          [](JsonBuilder * /* builder */) {});
-
-        /**
-         * @todo Make sure map element removal occurs if exception is
-         *       thrown above.
-         */
-        // Done with the connection data.
-        data->info.erase(i);
-      }
-    }
-
-
+      });
 
     GVariant * dictionary = nullptr;
 
-
-
     // The method return value will contain a dictionary of the
     // requested input fields.
     agent_complete_request_input(object, invocation, dictionary);
@@ -283,11 +318,13 @@ ivi::settings::agent::register_connect_data(
   JsonReader * reader,
   response_callback const & response)
 {
+  connect_data data(reader, response);
+
   std::lock_guard<std::mutex> guard(data_.lock);
 
   auto const result =
-    data_.info.emplace(std::make_pair(service_path,
-                                      connect_data(reader, response)));
+    data_.info.insert(
+      std::make_pair(service_path, std::move(data)));
 
   return result.second;
 }

diff --git a/plugins/connman/agent.hpp b/plugins/connman/agent.hpp
index 32c35dd..9b8cf17 100644 (file)
--- a/plugins/connman/agent.hpp
+++ b/plugins/connman/agent.hpp
@@ -33,7 +33,6 @@
 
 #include <settingsd/response_callback.hpp>
 
-
 #include <string>
 #include <map>
 #include <mutex>
@@ -108,8 +107,9 @@ namespace ivi
           // Nothing to do with response field.
         }
 
-        /// Disallow copy construction.
+        /// Disallow copy construction and assignment.
         connect_data(connect_data const &) = delete;
+        void operator=(connect_data const &) = delete;
 
         /**
          * Pointer to the @c JsonReader object containing the request
@@ -135,11 +135,13 @@ namespace ivi
         /// Synchronize access to the response/request data map.
         std::mutex lock;
 
+        typedef std::map<std::string, connect_data> map_type;
+
         /**
          * Map of service D-Bus object path to request/response
          * data.
          */
-        std::map<std::string, connect_data> info;
+        map_type info;
       };
 
     private:

diff --git a/plugins/connman/technology.cpp b/plugins/connman/technology.cpp
index c4706dd..9a83b01 100644 (file)
--- a/plugins/connman/technology.cpp
+++ b/plugins/connman/technology.cpp
@@ -222,30 +222,6 @@ ivi::settings::technology::connect(JsonReader * reader,
     if (manager_.register_connect_data(service_path,
                                        reader,
                                        response)) {
-      class auto_deregister
-      {
-      public:
-        auto_deregister(connman_manager & manager,
-                        char const * service_path)
-          : manager_(manager)
-          , service_path_(service_path)
-        {
-        }
-
-        ~auto_deregister()
-        {
-          manager_.deregister_connect_data(service_path_);
-        }
-
-      private:
-
-        connman_manager & manager_;
-        char const * service_path_;
-
-      };
-
-      auto_deregister const dereg(manager_, service_path);
-
       service s(service_path,
                 connman_.connection(),
                 event_callback_);