Answer to PR comments.

10 files changed, 76 insertions, 51 deletions

diff --git a/src/components/application_manager/include/application_manager/application.h b/src/components/application_manager/include/application_manager/application.h
index cb25a56175..b76fc3efb3 100644
--- a/src/components/application_manager/include/application_manager/application.h
+++ b/src/components/application_manager/include/application_manager/application.h
@@ -394,22 +394,22 @@ class DynamicApplicationData {
   virtual bool is_reset_global_properties_active() const = 0;
 
   /**
-  * @brief Set allow mode for choice_set allow: allowed/disallowed.
-  * Allow mode means, than choice_set not fully stored or not fully deleted
-  * (in both times, request processes on HMI side, HMI didn't send yet
-  * response with result)
+  * @brief Set allowed mode for specified choice_set_id.
   * @param choice_set_id Choice set id.
   * @param is_allowed TRUE if choice set is have to be allowed to perform,
   * otherwise FALSE.
+  * Allow mode means, than choice_set not fully processed or not fully deleted
+  * (in both times, request processes on HMI side, HMI didn't send yet response
+  * with result)
   */
-  virtual void set_choice_set_allow_mode(const std::uint32_t choice_set_id,
+  virtual void set_choice_set_allow_mode(const uint32_t choice_set_id,
                                          const bool is_allowed) = 0;
   /**
-  * @brief Check choice set allowing.
+  * @brief Check if choice set allowed.
   * @param choice_set_id Choice set id.
+  * @return TRUE if choice set is allowed to perform, otherwise ELSE.
   */
-  virtual bool is_choice_set_allowed_to_perform(
-      std::uint32_t choice_set_id) const = 0;
+  virtual bool is_choice_set_allowed(const uint32_t choice_set_id) const = 0;
 };
 
 class Application : public virtual InitialApplicationData,
diff --git a/src/components/application_manager/include/application_manager/application_data_impl.h b/src/components/application_manager/include/application_manager/application_data_impl.h
index 0e0ffb319e..1e96035d7f 100644
--- a/src/components/application_manager/include/application_manager/application_data_impl.h
+++ b/src/components/application_manager/include/application_manager/application_data_impl.h
@@ -36,6 +36,7 @@
 #include <string>
 #include <map>
 #include <cstdint>
+
 #include "utils/lock.h"
 #include "utils/semantic_version.h"
 #include "smart_objects/smart_object.h"
@@ -45,7 +46,7 @@
 namespace application_manager {
 
 namespace mobile_api = mobile_apis;
-typedef std::map<std::uint32_t, bool> ChoiceSetAllowedMap;
+typedef std::map<uint32_t, bool> ChoiceSetAllowedMap;
 
 class InitialApplicationDataImpl : public virtual Application {
  public:
@@ -266,9 +267,9 @@ class DynamicApplicationDataImpl : public virtual Application {
    */
   inline bool is_reset_global_properties_active() const;
 
-  virtual void set_choice_set_allow_mode(const std::uint32_t choice_set_id,
+  virtual void set_choice_set_allow_mode(const uint32_t choice_set_id,
                                          const bool is_allowed);
-  bool is_choice_set_allowed_to_perform(std::uint32_t choice_set_id) const;
+  bool is_choice_set_allowed(const uint32_t choice_set_id) const;
 
  protected:
   smart_objects::SmartObject* help_prompt_;
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/perform_interaction_request.h b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/perform_interaction_request.h
index 26b48a95c6..43fc9a76b8 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/perform_interaction_request.h
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/perform_interaction_request.h
@@ -235,14 +235,12 @@ class PerformInteractionRequest
   /**
    * @brief Checks for all choice set ids are allowed to perform.
    * @param app Contains pointer to application.
-   * @param choice_set_id_list_length Contains amount of choice set ids.
    * @param choice_set_id_list Array of choice set ids
    * @return If all at-least one choice list is disallowed to perform returns
    * false, otherwise returns true.
    */
-  bool IsAllChoiceSetAllowedToPerform(
+  bool IsAllChoiceSetIdsAllowed(
       app_mngr::ApplicationSharedPtr app,
-      const size_t choice_set_id_list_length,
       const smart_objects::SmartObject& choice_set_id_list) const;
 
   mobile_apis::InteractionMode::eType interaction_mode_;
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc
index c3b6775a00..ee8eb61494 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/create_interaction_choice_set_request.cc
@@ -152,6 +152,11 @@ void CreateInteractionChoiceSetRequest::Run() {
     // we have VR commands
     SendVRAddCommandRequests(app);
   } else {
+    if (vr_status == MessageHelper::ChoiceSetVRCommandsStatus::NONE) {
+      // We not wait for calling on_event and choice set is allowed just after
+      // added.
+      app->set_choice_set_allow_mode(choice_set_id_, true);
+    }
     // we have none, just return with success
     SendResponse(true, Result::SUCCESS);
   }
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc
index de809f286e..bb5d9c219a 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc
@@ -87,6 +87,18 @@ void DeleteInteractionChoiceSetRequest::Run() {
     return;
   }
   SendVrDeleteCommand(app);
+
+  smart_objects::SmartObject msg_params =
+      smart_objects::SmartObject(smart_objects::SmartType_Map);
+
+  msg_params[strings::interaction_choice_set_id] = choice_set_id;
+  msg_params[strings::app_id] = app->app_id();
+
+  app->RemoveChoiceSet(choice_set_id);
+
+  // Checking of HMI responses will be implemented with APPLINK-14600
+  const bool result = true;
+  SendResponse(result, mobile_apis::Result::SUCCESS);
 }
 
 bool DeleteInteractionChoiceSetRequest::Init() {
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc
index b4865b0083..7dcfd3de17 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/perform_interaction_request.cc
@@ -164,12 +164,10 @@ void PerformInteractionRequest::Run() {
     return;
   }
 
-  if (!IsAllChoiceSetAllowedToPerform(
-          app,
-          choice_set_id_list_length,
-          msg_params[strings::interaction_choice_set_id_list])) {
-    LOG4CXX_ERROR(logger_, "Choice set is not ready to perform.");
+  if (!IsAllChoiceSetIdsAllowed(
+          app, msg_params[strings::interaction_choice_set_id_list])) {
     SendResponse(false, mobile_apis::Result::REJECTED);
+    return;
   }
 
   if (msg_params.keyExists(strings::vr_help)) {
@@ -1071,16 +1069,19 @@ void PerformInteractionRequest::SendBothModeResponse(
                response_params);
 }
 
-bool PerformInteractionRequest::IsAllChoiceSetAllowedToPerform(
+bool PerformInteractionRequest::IsAllChoiceSetIdsAllowed(
     ApplicationSharedPtr app,
-    const size_t choice_set_id_list_length,
     const ns_smart_device_link::ns_smart_objects::SmartObject&
         choice_set_id_list) const {
   LOG4CXX_AUTO_TRACE(logger_);
+  const size_t choice_set_id_list_length = choice_set_id_list.length();
   for (size_t i = 0; i < choice_set_id_list_length; ++i) {
     const bool is_allowed =
-        app->is_choice_set_allowed_to_perform(choice_set_id_list[i].asInt());
+        app->is_choice_set_allowed(choice_set_id_list[i].asInt());
     if (!is_allowed) {
+      LOG4CXX_DEBUG(logger_,
+                    "Choice set with id " << choice_set_id_list[i].asInt()
+                                          << "dosn't allowed.");
       return false;
     }
   }
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc
index 0ff557d2cf..303e83da76 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc
@@ -242,7 +242,6 @@ TEST_F(DeleteInteractionChoiceSetRequestTest, Run_SendVrDeleteCommand_SUCCESS) {
         .WillOnce(Return(choice_set_id));
 
     EXPECT_CALL(*app_, set_choice_set_allow_mode(kChoiceSetId, false));
-    EXPECT_CALL(*app_, app_id()).WillOnce(Return(kConnectionKey));
   }
 
   EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_)).WillOnce(Return(true));
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/perform_interaction_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/perform_interaction_test.cc
index 4050c563ea..b1c369a9bd 100644
--- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/perform_interaction_test.cc
+++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/perform_interaction_test.cc
@@ -70,6 +70,11 @@ using ::test::components::application_manager_test::MockApplication;
 namespace strings = ::application_manager::strings;
 namespace hmi_response = ::application_manager::hmi_response;
 
+MATCHER_P(CheckMessageSuccess, success, "") {
+  return success ==
+         (*arg)[am::strings::msg_params][am::strings::success].asBool();
+}
+
 namespace {
 const int32_t kCommandId = 1;
 const uint32_t kCmdId = 1u;
@@ -258,45 +263,32 @@ TEST_F(PerformInteractionRequestTest,
 }
 
 TEST_F(PerformInteractionRequestTest, ChoiceProcessesOnHHMI_REJECT) {
-  MessageSharedPtr message = CreateMessage(smart_objects::SmartType_Map);
-  (*message)[strings::params][strings::connection_key] = kConnectionKey;
+  MessageSharedPtr message = CreateMessage(smart_objects::SmartType_Null);
   (*message)[strings::msg_params][strings::interaction_choice_set_id_list][0] =
       kChoiceSetId;
   smart_objects::SmartObject* choice_set_id =
       &((*message)[am::strings::msg_params]
                   [am::strings::interaction_choice_set_id]);
-  std::shared_ptr<PerformInteractionRequest> command =
-      CreateCommand<PerformInteractionRequest>(message);
 
-  smart_objects::SmartObject& response = *message;
-
-  response[strings::params][strings::message_type] = am::MessageType::kResponse;
-  response[strings::params][strings::correlation_id] = kCorrelationId;
-  response[strings::params][strings::protocol_type] = 0;
-  response[strings::params][strings::protocol_version] = 3;
-  response[strings::params][strings::connection_key] = kConnectionKey;
-  response[strings::params][strings::function_id] =
-      mobile_apis::FunctionID::PerformInteractionID;
-  response[strings::msg_params][strings::success] = false;
-  response[strings::msg_params][strings::result_code] =
-      mobile_apis::Result::REJECTED;
-
-  EXPECT_CALL(app_mngr_, application(kConnectionKey))
-      .WillRepeatedly(Return(mock_app_));
+  EXPECT_CALL(app_mngr_, application(_)).WillRepeatedly(Return(mock_app_));
+
   {
     InSequence seq;
     EXPECT_CALL(*mock_app_, is_perform_interaction_active())
         .WillOnce(Return(false));
     EXPECT_CALL(*mock_app_, FindChoiceSet(kChoiceSetId))
         .WillOnce(Return(choice_set_id));
-    EXPECT_CALL(*mock_app_, is_choice_set_allowed_to_perform(kChoiceSetId))
+    EXPECT_CALL(*mock_app_, is_choice_set_allowed(kChoiceSetId))
         .WillOnce(Return(false));
-    EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _))
-        .WillOnce(Return(false));
-    EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _))
+    EXPECT_CALL(mock_rpc_service_,
+                ManageMobileCommand(
+                    _, am::commands::Command::CommandSource::SOURCE_SDL))
         .WillOnce(Return(true));
   }
 
+  std::shared_ptr<PerformInteractionRequest> command =
+      CreateCommand<PerformInteractionRequest>(message);
+
   command->Init();
   command->Run();
 }
diff --git a/src/components/application_manager/src/application_data_impl.cc b/src/components/application_manager/src/application_data_impl.cc
index 6e720425ce..3ff3b3fd92 100644
--- a/src/components/application_manager/src/application_data_impl.cc
+++ b/src/components/application_manager/src/application_data_impl.cc
@@ -576,6 +576,12 @@ void DynamicApplicationDataImpl::RemoveChoiceSet(uint32_t choice_set_id) {
 
   ChoiceSetAllowedMap::iterator choise_id_it =
       choice_set_allowed_map_.find(choice_set_id);
+  if (choice_set_allowed_map_.end() == choise_id_it) {
+    LOG4CXX_WARN(logger_,
+                 "Not exists info about choice set " << choice_set_id
+                                                     << " allow");
+    return;
+  }
   choice_set_allowed_map_.erase(choise_id_it);
 }
 
@@ -624,16 +630,27 @@ void DynamicApplicationDataImpl::set_reset_global_properties_active(
 void DynamicApplicationDataImpl::set_choice_set_allow_mode(
     const uint32_t choice_set_id, const bool is_allowed) {
   auto choice_set = choice_set_allowed_map_.find(choice_set_id);
-  if (choice_set == choice_set_allowed_map_.end()) {
+  if (choice_set_allowed_map_.end() == choice_set) {
     LOG4CXX_WARN(logger_,
                  "Choice set with id " << choice_set_id << " is not found.");
+    return;
   }
   choice_set->second = is_allowed;
+  LOG4CXX_DEBUG(logger_,
+                "choice_set_id: " << choice_set_id << " is_allowed: "
+                                  << std::boolalpha << is_allowed);
 }
 
-bool DynamicApplicationDataImpl::is_choice_set_allowed_to_perform(
-    uint32_t choice_set_id) const {
-  return choice_set_allowed_map_.find(choice_set_id)->second;
+bool DynamicApplicationDataImpl::is_choice_set_allowed(
+    const uint32_t choice_set_id) const {
+  LOG4CXX_DEBUG(logger_, "Choice setID: " << choice_set_id);
+  auto it = choice_set_allowed_map_.find(choice_set_id);
+  if (choice_set_allowed_map_.end() == it) {
+    LOG4CXX_ERROR(logger_,
+                  "Choice set with id " << choice_set_id << " is not found.");
+    return false;
+  }
+  return it->second;
 }
 
 void DynamicApplicationDataImpl::set_perform_interaction_mode(int32_t mode) {
diff --git a/src/components/application_manager/test/include/application_manager/mock_application.h b/src/components/application_manager/test/include/application_manager/mock_application.h
index 26e06b5453..5d5de6c701 100644
--- a/src/components/application_manager/test/include/application_manager/mock_application.h
+++ b/src/components/application_manager/test/include/application_manager/mock_application.h
@@ -300,7 +300,7 @@ class MockApplication : public ::application_manager::Application {
   MOCK_METHOD1(set_reset_global_properties_active, void(bool active));
   MOCK_CONST_METHOD0(is_reset_global_properties_active, bool());
   MOCK_METHOD2(set_choice_set_allow_mode, void(std::uint32_t, bool));
-  MOCK_CONST_METHOD1(is_choice_set_allowed_to_perform, bool(std::uint32_t));
+  MOCK_CONST_METHOD1(is_choice_set_allowed, bool(std::uint32_t));
   MOCK_CONST_METHOD0(app_id, uint32_t());
   MOCK_CONST_METHOD0(mac_address, const std::string&());
   MOCK_CONST_METHOD0(bundle_id, const std::string&());