From c46340e7835e5381a7c309546e2d4a7a2f902130 Mon Sep 17 00:00:00 2001 From: "Yana Chernysheva (GitHub)" <59469418+ychernysheva@users.noreply.github.com> Date: Tue, 25 Jan 2022 16:41:52 +0200 Subject: Fix PerformInteractionChoiceSet request behavior (#3826) * Fix PerformInteractionChoiceSet request behavior * Update src/components/application_manager/src/application_data_impl.cc Co-authored-by: Collin Co-authored-by: Iryna Lytvynenko (GitHub) Co-authored-by: Collin --- .../include/application_manager/application.h | 19 +++++++ .../application_manager/application_data_impl.h | 7 +++ .../create_interaction_choice_set_request.cc | 13 +++++ .../delete_interaction_choice_set_request.cc | 1 + .../commands/mobile/perform_interaction_request.cc | 21 +++++++ .../mobile/create_interaction_choice_set_test.cc | 4 +- .../commands/mobile/perform_interaction_test.cc | 32 +++++++++++ .../src/application_data_impl.cc | 64 +++++++++++++++++++--- .../include/application_manager/mock_application.h | 2 + 9 files changed, 152 insertions(+), 11 deletions(-) (limited to 'src/components') diff --git a/src/components/application_manager/include/application_manager/application.h b/src/components/application_manager/include/application_manager/application.h index 86b26f24ef..33fa1ae914 100644 --- a/src/components/application_manager/include/application_manager/application.h +++ b/src/components/application_manager/include/application_manager/application.h @@ -520,6 +520,25 @@ class DynamicApplicationData { * @return TRUE if perform interaction active, otherwise FALSE */ virtual bool is_reset_global_properties_active() const = 0; + + /** + * @brief Set allowed mode for specified choice_set_id. + * @param choice_set_id Choice set id. + * @param is_allowed TRUE if the choice set has to be allowed to perform, + * otherwise FALSE. + * Allow mode means that the choice_set is not fully processed or not fully + * deleted (in both cases request is being processed on HMI side, and HMI + * hasn't sent response with result yet) + */ + virtual void set_choice_set_allow_mode(const uint32_t choice_set_id, + const bool is_allowed) = 0; + + /** + * @brief Check if choice set allowed. + * @param choice_set_id Choice set id. + * @return TRUE if the choice set is allowed to perform, otherwise FALSE. + */ + 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 086e71b48b..e349fd7d4c 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 @@ -315,6 +315,11 @@ class DynamicApplicationDataImpl : public virtual Application { */ inline bool is_reset_global_properties_active() const; + void set_choice_set_allow_mode(const uint32_t choice_set_id, + const bool is_allowed); + + bool is_choice_set_allowed(const uint32_t choice_set_id) const; + protected: smart_objects::SmartObject* help_prompt_; smart_objects::SmartObject* timeout_prompt_; @@ -346,6 +351,8 @@ class DynamicApplicationDataImpl : public virtual Application { uint32_t is_perform_interaction_active_; bool is_reset_global_properties_active_; int32_t perform_interaction_mode_; + mutable sync_primitives::Lock allowed_choice_sets_lock_; + std::set allowed_choice_sets_; DisplayCapabilitiesBuilder display_capabilities_builder_; private: 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 98e26e1aa9..70f462c05d 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 @@ -156,6 +156,11 @@ void CreateInteractionChoiceSetRequest::Run() { // we have VR commands SendVRAddCommandRequests(app); } else { + if (MessageHelper::ChoiceSetVRCommandsStatus::NONE == vr_status) { + // Because on_event will not be called for this choice, we set is allowed + // right after added. + app->set_choice_set_allow_mode(choice_set_id_, true); + } // we have none, just return with success SendResponse(true, Result::SUCCESS); } @@ -462,6 +467,14 @@ void CreateInteractionChoiceSetRequest::DeleteChoices() { void CreateInteractionChoiceSetRequest::OnAllHMIResponsesReceived() { SDL_LOG_AUTO_TRACE(); + ApplicationSharedPtr application = + application_manager_.application(connection_key()); + if (!error_from_hmi_) { + application->set_choice_set_allow_mode(choice_set_id_, true); + SDL_LOG_DEBUG("Choice set with id " << choice_set_id_ + << " is allowed to perform."); + } + if (!error_from_hmi_ && should_send_warnings_) { SendResponse(true, mobile_apis::Result::WARNINGS, kInvalidImageWarningInfo); } else if (!error_from_hmi_) { 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 eac43bb22b..dccf2e9fdd 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 @@ -173,6 +173,7 @@ void DeleteInteractionChoiceSetRequest::SendVrDeleteCommand( return; } + app->set_choice_set_allow_mode(choice_set_id, false); smart_objects::SmartObject msg_params = smart_objects::SmartObject(smart_objects::SmartType_Map); msg_params[strings::app_id] = app->app_id(); 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 bf0f05ba32..7708df9363 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 @@ -173,6 +173,27 @@ void PerformInteractionRequest::Run() { return; } + auto choice_sets_are_disallowed_to_perform = + [](ApplicationSharedPtr app, + const smart_objects::SmartObject& choice_set_id_list) { + auto choice_set_id_array = choice_set_id_list.asArray(); + + return std::any_of( + choice_set_id_array->begin(), + choice_set_id_array->end(), + [app](smart_objects::SmartObject& choice_set_id) { + return !app->is_choice_set_allowed(choice_set_id.asInt()); + }); + }; + + if (choice_sets_are_disallowed_to_perform( + app, msg_params[strings::interaction_choice_set_id_list])) { + const std::string message = "One of choice sets is not available yet"; + SDL_LOG_WARN(message); + SendResponse(false, mobile_apis::Result::REJECTED, message.c_str()); + return; + } + if (msg_params.keyExists(strings::vr_help)) { if (mobile_apis::Result::INVALID_DATA == MessageHelper::VerifyImageVrHelpItems( diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/create_interaction_choice_set_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/create_interaction_choice_set_test.cc index 3f8ac0028b..951a206dff 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/create_interaction_choice_set_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/create_interaction_choice_set_test.cc @@ -836,8 +836,8 @@ TEST_F(CreateInteractionChoiceSetRequestTest, smart_objects::SmartObject choice_set_id(smart_objects::SmartType_Null); - EXPECT_CALL(app_mngr_, application(kConnectionKey)) - .WillOnce(Return(mock_app_)); + ON_CALL(app_mngr_, application(kConnectionKey)) + .WillByDefault(Return(mock_app_)); ON_CALL(mock_message_helper_, CheckChoiceSetVRCommands(_)) .WillByDefault(Return(am::MessageHelper::ChoiceSetVRCommandsStatus::ALL)); 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 196c83d144..ee2eb720e0 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 @@ -703,6 +703,38 @@ TEST_F( "UI warning message, VR error message"); } +TEST_F(PerformInteractionRequestTest, + ChoiceSetIsNotAllowed_UnsuccessWithREJECT) { + MessageSharedPtr msg_from_mobile = + CreateMessage(smart_objects::SmartType_Map); + const uint32_t kDissalowedChoiceSetId = 11u; + + (*msg_from_mobile)[strings::msg_params] + [strings::interaction_choice_set_id_list][0] = + kDissalowedChoiceSetId; + (*msg_from_mobile)[strings::msg_params][strings::interaction_choice_set_id] = + kDissalowedChoiceSetId; + smart_objects::SmartObject choice_set_id = + (*msg_from_mobile)[am::strings::msg_params] + [am::strings::interaction_choice_set_id]; + + ON_CALL(app_mngr_, application(_)).WillByDefault(Return(mock_app_)); + EXPECT_CALL(*mock_app_, FindChoiceSet(kDissalowedChoiceSetId)) + .WillOnce(Return(choice_set_id)); + EXPECT_CALL(*mock_app_, is_choice_set_allowed(kDissalowedChoiceSetId)) + .WillOnce(Return(false)); + EXPECT_CALL( + mock_rpc_service_, + ManageMobileCommand(MobileResultCodeIs(mobile_apis::Result::REJECTED), _)) + .WillOnce(Return(true)); + + auto command = + CreateCommand(msg_from_mobile); + + command->Init(); + command->Run(); +} + } // namespace perform_interaction_request } // namespace mobile_commands_test } // namespace commands_test diff --git a/src/components/application_manager/src/application_data_impl.cc b/src/components/application_manager/src/application_data_impl.cc index 57f7932246..7e5eaf43a0 100644 --- a/src/components/application_manager/src/application_data_impl.cc +++ b/src/components/application_manager/src/application_data_impl.cc @@ -926,20 +926,40 @@ void DynamicApplicationDataImpl::RemoveWindowInfo(const WindowID window_id) { void DynamicApplicationDataImpl::AddChoiceSet( uint32_t choice_set_id, const smart_objects::SmartObject& choice_set) { - sync_primitives::AutoLock lock(choice_set_map_lock_ptr_); - ChoiceSetMap::const_iterator it = choice_set_map_.find(choice_set_id); - if (choice_set_map_.end() == it) { - choice_set_map_[choice_set_id] = new smart_objects::SmartObject(choice_set); + bool is_choice_set_added = false; + { + sync_primitives::AutoLock lock(choice_set_map_lock_ptr_); + ChoiceSetMap::const_iterator it = choice_set_map_.find(choice_set_id); + if (choice_set_map_.end() == it) { + choice_set_map_[choice_set_id] = + new smart_objects::SmartObject(choice_set); + is_choice_set_added = true; + } + } + if (is_choice_set_added) { + set_choice_set_allow_mode(choice_set_id, true); } } void DynamicApplicationDataImpl::RemoveChoiceSet(uint32_t choice_set_id) { - sync_primitives::AutoLock lock(choice_set_map_lock_ptr_); - ChoiceSetMap::iterator it = choice_set_map_.find(choice_set_id); + { + sync_primitives::AutoLock choice_set_map_lock(choice_set_map_lock_ptr_); + ChoiceSetMap::iterator it = choice_set_map_.find(choice_set_id); - if (choice_set_map_.end() != it) { - delete it->second; - choice_set_map_.erase(choice_set_id); + if (choice_set_map_.end() != it) { + delete it->second; + choice_set_map_.erase(choice_set_id); + } + } + { + sync_primitives::AutoLock allowed_choice_sets_lock( + allowed_choice_sets_lock_); + auto choice_id_it = allowed_choice_sets_.find(choice_set_id); + if (allowed_choice_sets_.end() == choice_id_it) { + SDL_LOG_WARN("Choice set with id " << choice_set_id << " is not found"); + return; + } + allowed_choice_sets_.erase(choice_id_it); } } @@ -986,6 +1006,32 @@ void DynamicApplicationDataImpl::set_reset_global_properties_active( is_reset_global_properties_active_ = active; } +void DynamicApplicationDataImpl::set_choice_set_allow_mode( + const uint32_t choice_set_id, const bool is_allowed) { + SDL_LOG_DEBUG("Choice setID: " + << choice_set_id + << (is_allowed ? " is allowed" : " disallowed")); + sync_primitives::AutoLock lock(allowed_choice_sets_lock_); + if (is_allowed) { + allowed_choice_sets_.insert(choice_set_id); + } else { + allowed_choice_sets_.erase(choice_set_id); + } +} + +bool DynamicApplicationDataImpl::is_choice_set_allowed( + const uint32_t choice_set_id) const { + SDL_LOG_DEBUG("Choice setID: " << choice_set_id); + sync_primitives::AutoLock lock(allowed_choice_sets_lock_); + const auto it = allowed_choice_sets_.find(choice_set_id); + if (allowed_choice_sets_.end() == it) { + SDL_LOG_ERROR("Choice set with id " << choice_set_id + << " is not allowed to perform now"); + return false; + } + return true; +} + void DynamicApplicationDataImpl::set_perform_interaction_mode(int32_t mode) { perform_interaction_mode_ = 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 ac2a534929..93755ef8f4 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 @@ -361,6 +361,8 @@ class MockApplication : public ::application_manager::Application { mobile_apis::LayoutMode::eType()); 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, 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&()); -- cgit v1.2.1