Merge branch 'feature/cloud_app_connection_status' into feature/secure_websocket_connection

8 files changed, 32 insertions, 24 deletions

diff --git a/src/components/application_manager/include/application_manager/application_manager_impl.h b/src/components/application_manager/include/application_manager/application_manager_impl.h
index 08421615d4..8c062f9016 100644
--- a/src/components/application_manager/include/application_manager/application_manager_impl.h
+++ b/src/components/application_manager/include/application_manager/application_manager_impl.h
@@ -1468,6 +1468,8 @@ class ApplicationManagerImpl
 
   DeviceMap secondary_transport_devices_cache_;
 
+  mutable std::shared_ptr<sync_primitives::RecursiveLock>
+      pending_device_map_lock_ptr_;
   std::map<std::string, std::string> pending_device_map_;
 
 #ifdef TELEMETRY_MONITOR
diff --git a/src/components/application_manager/src/application_manager_impl.cc b/src/components/application_manager/src/application_manager_impl.cc
index cc49a93ebe..9d8185a1e2 100644
--- a/src/components/application_manager/src/application_manager_impl.cc
+++ b/src/components/application_manager/src/application_manager_impl.cc
@@ -168,6 +168,8 @@ ApplicationManagerImpl::ApplicationManagerImpl(
     , navi_close_app_timeout_(am_settings.stop_streaming_timeout())
     , navi_end_stream_timeout_(am_settings.stop_streaming_timeout())
     , state_ctrl_(*this)
+    , pending_device_map_lock_ptr_(
+          std::make_shared<sync_primitives::RecursiveLock>())
     , application_list_update_timer_(
           "AM ListUpdater",
           new TimerTaskImpl<ApplicationManagerImpl>(
@@ -810,8 +812,8 @@ void ApplicationManagerImpl::RefreshCloudAppInformation() {
   LOG4CXX_AUTO_TRACE(logger_);
   std::vector<std::string> enabled_apps;
   GetPolicyHandler().GetEnabledCloudApps(enabled_apps);
-  std::vector<std::string>::iterator it = enabled_apps.begin();
-  std::vector<std::string>::iterator end = enabled_apps.end();
+  std::vector<std::string>::iterator enabled_it = enabled_apps.begin();
+  std::vector<std::string>::iterator enabled_end = enabled_apps.end();
   std::string endpoint = "";
   std::string certificate = "";
   std::string auth_token = "";
@@ -820,10 +822,12 @@ void ApplicationManagerImpl::RefreshCloudAppInformation() {
   bool enabled = true;
 
   // Store old device map and clear the current map
+  pending_device_map_lock_ptr_->Acquire();
   std::map<std::string, std::string> old_device_map = pending_device_map_;
   pending_device_map_ = std::map<std::string, std::string>();
-  for (; it != end; ++it) {
-    GetPolicyHandler().GetCloudAppParameters(*it,
+  // Create a device for each newly enabled cloud app
+  for (; enabled_it != enabled_end; ++enabled_it) {
+    GetPolicyHandler().GetCloudAppParameters(*enabled_it,
                                              enabled,
                                              endpoint,
                                              certificate,
@@ -832,10 +836,12 @@ void ApplicationManagerImpl::RefreshCloudAppInformation() {
                                              hybrid_app_preference);
 
     pending_device_map_.insert(
-        std::pair<std::string, std::string>(endpoint, *it));
-    auto old_device = old_device_map.find(endpoint);
-    if (old_device_map.find(endpoint) != old_device_map.end()) {
-      old_device_map.erase(old_device);
+        std::pair<std::string, std::string>(endpoint, *enabled_it));
+    // Determine which endpoints were disabled by erasing all enabled apps from
+    // the old device list
+    auto old_device_it = old_device_map.find(endpoint);
+    if (old_device_it != old_device_map.end()) {
+      old_device_map.erase(old_device_it);
       continue;
     }
 
@@ -846,31 +852,34 @@ void ApplicationManagerImpl::RefreshCloudAppInformation() {
         auth_token,
         cloud_transport_type,
         hybrid_app_preference};
-    connection_handler().AddCloudAppDevice(*it, properties);
+    connection_handler().AddCloudAppDevice(properties);
   }
+  pending_device_map_lock_ptr_->Release();
 
   int removed_app_count = 0;
   // Clear out devices for existing cloud apps that were disabled
   for (auto& device : old_device_map) {
     std::string policy_app_id = device.second;
+    // First search for the disabled app within the registered apps
     ApplicationSharedPtr app = application_by_policy_id(policy_app_id);
     if (app.use_count() == 0) {
       sync_primitives::AutoLock lock(apps_to_register_list_lock_ptr_);
+      // If the disabled app is not present in the registered app list, check
+      // the apps awaiting registration
       PolicyAppIdPredicate finder(policy_app_id);
       ApplicationSet::iterator it = std::find_if(
           apps_to_register_.begin(), apps_to_register_.end(), finder);
       if (it == apps_to_register_.end()) {
+        LOG4CXX_DEBUG(logger_,
+                      "Unable to find app to remove (" << policy_app_id
+                                                       << "), skipping");
         continue;
       }
       app = *it;
       apps_to_register_.erase(it);
     }
-    if (app.use_count() == 0) {
-      LOG4CXX_DEBUG(logger_,
-                    "Unable to find app to remove (" << policy_app_id
-                                                     << "), skipping");
-      continue;
-    }
+    // If the disabled app is registered, unregistered it before destroying the
+    // device
     if (app->IsRegistered() && app->is_cloud_app()) {
       LOG4CXX_DEBUG(logger_, "Disabled app is registered, unregistering now");
       GetRPCService().ManageMobileCommand(
@@ -881,6 +890,7 @@ void ApplicationManagerImpl::RefreshCloudAppInformation() {
 
       OnAppUnauthorized(app->app_id());
     }
+    // Delete the cloud device
     connection_handler().RemoveCloudAppDevice(app->device());
     removed_app_count++;
   }
@@ -905,10 +915,12 @@ void ApplicationManagerImpl::CreatePendingApplication(
   std::string hybrid_app_preference_str = "";
   bool enabled = true;
   std::string name = device_info.name();
+  pending_device_map_lock_ptr_->Acquire();
   auto it = pending_device_map_.find(name);
   if (it == pending_device_map_.end()) {
     return;
   }
+  pending_device_map_lock_ptr_->Release();
 
   const std::string policy_app_id = it->second;
 
@@ -959,7 +971,7 @@ void ApplicationManagerImpl::CreatePendingApplication(
       mobile_apis::HybridAppPreference::eType>::
       StringToEnum(hybrid_app_preference_str, &hybrid_app_preference_enum);
 
-  if (!convert_result) {
+  if (!hybrid_app_preference_str.empty() && !convert_result) {
     LOG4CXX_ERROR(
         logger_,
         "Could not convert string to enum: " << hybrid_app_preference_str);
diff --git a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
index dd817b871b..3209250660 100644
--- a/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
+++ b/src/components/connection_handler/include/connection_handler/connection_handler_impl.h
@@ -131,7 +131,6 @@ class ConnectionHandlerImpl
   void ConnectToAllDevices() OVERRIDE;
 
   void AddCloudAppDevice(
-      const std::string& policy_app_id,
       const transport_manager::transport_adapter::CloudAppProperties&
           cloud_properties) OVERRIDE;
 
diff --git a/src/components/connection_handler/src/connection_handler_impl.cc b/src/components/connection_handler/src/connection_handler_impl.cc
index e296c048e2..9125b071ae 100644
--- a/src/components/connection_handler/src/connection_handler_impl.cc
+++ b/src/components/connection_handler/src/connection_handler_impl.cc
@@ -1324,7 +1324,6 @@ void ConnectionHandlerImpl::ConnectToAllDevices() {
 }
 
 void ConnectionHandlerImpl::AddCloudAppDevice(
-    const std::string& policy_app_id,
     const transport_manager::transport_adapter::CloudAppProperties&
         cloud_properties) {
   transport_manager_.AddCloudDevice(cloud_properties);
diff --git a/src/components/include/connection_handler/connection_handler.h b/src/components/include/connection_handler/connection_handler.h
index 1fe48ab98f..fe3c625dfe 100644
--- a/src/components/include/connection_handler/connection_handler.h
+++ b/src/components/include/connection_handler/connection_handler.h
@@ -104,7 +104,6 @@ class ConnectionHandler {
   virtual void ConnectToAllDevices() = 0;
 
   virtual void AddCloudAppDevice(
-      const std::string& policy_app_id,
       const transport_manager::transport_adapter::CloudAppProperties&
           cloud_properties) = 0;
 
diff --git a/src/components/include/test/connection_handler/mock_connection_handler.h b/src/components/include/test/connection_handler/mock_connection_handler.h
index 735c9ae016..b4f06e1ec2 100644
--- a/src/components/include/test/connection_handler/mock_connection_handler.h
+++ b/src/components/include/test/connection_handler/mock_connection_handler.h
@@ -68,8 +68,7 @@ class MockConnectionHandler : public connection_handler::ConnectionHandler {
   MOCK_METHOD0(ConnectToAllDevices, void());
   MOCK_METHOD2(
       AddCloudAppDevice,
-      void(const std::string& policy_app_id,
-           const transport_manager::transport_adapter::CloudAppProperties&));
+      void(const transport_manager::transport_adapter::CloudAppProperties&));
   MOCK_METHOD1(RemoveCloudAppDevice, void(const DeviceHandle device_id));
   MOCK_METHOD1(CloseRevokedConnection, void(uint32_t connection_key));
   MOCK_METHOD1(CloseConnection, void(ConnectionHandle connection_handle));
diff --git a/src/components/transport_manager/test/include/transport_manager/transport_adapter/mock_transport_adapter_controller.h b/src/components/transport_manager/test/include/transport_manager/transport_adapter/mock_transport_adapter_controller.h
index bd8c5d2ed3..642c193d63 100644
--- a/src/components/transport_manager/test/include/transport_manager/transport_adapter/mock_transport_adapter_controller.h
+++ b/src/components/transport_manager/test/include/transport_manager/transport_adapter/mock_transport_adapter_controller.h
@@ -44,7 +44,7 @@ using namespace ::transport_manager::transport_adapter;
 
 class MockTransportAdapterController : public TransportAdapterController {
  public:
-  MOCK_METHOD1(AddDevice, DeviceSptr(std::string device));
+  MOCK_METHOD1(AddDevice, DeviceSptr(DeviceSptr device));
   MOCK_METHOD1(SearchDeviceDone, void(DeviceVector device));
   MOCK_METHOD1(ApplicationListUpdated,
                ApplicationListUpdated(const DeviceUID& device_handle));
diff --git a/src/components/transport_manager/test/transport_adapter_test.cc b/src/components/transport_manager/test/transport_adapter_test.cc
index 72cf0ba76e..dafabf4504 100644
--- a/src/components/transport_manager/test/transport_adapter_test.cc
+++ b/src/components/transport_manager/test/transport_adapter_test.cc
@@ -504,7 +504,6 @@ TEST_F(TransportAdapterTest, DisconnectDevice_DeviceAddedConnectionCreated) {
       .WillOnce(Return(TransportAdapter::OK));
   EXPECT_CALL(transport_adapter, FindDevice(uniq_id))
       .WillRepeatedly(Return(mockdev));
-  // EXPECT_CALL(transport_adapter, ConnectionStatusUpdated(_, _));
   TransportAdapter::Error res = transport_adapter.ConnectDevice(uniq_id);
   EXPECT_EQ(TransportAdapter::OK, res);
   EXPECT_EQ(ConnectionStatus::CONNECTED, mockdev->connection_status());
@@ -515,7 +514,6 @@ TEST_F(TransportAdapterTest, DisconnectDevice_DeviceAddedConnectionCreated) {
   EXPECT_CALL(*mock_connection, Disconnect())
       .WillOnce(Return(TransportAdapter::OK));
 
-  // EXPECT_CALL(transport_adapter, ConnectionStatusUpdated(_, _));
   TransportAdapter::Error new_res = transport_adapter.DisconnectDevice(uniq_id);
   EXPECT_EQ(TransportAdapter::OK, new_res);
   EXPECT_EQ(ConnectionStatus::CLOSING, mockdev->connection_status());