From 59157f0e79566a5dfcd860bbb220f76843ff519b Mon Sep 17 00:00:00 2001 From: "Andrii Kalinich (GitHub)" Date: Wed, 23 Oct 2019 07:17:00 -0700 Subject: Fix VD items subscriptions after PTU (#3081) * Fix revoked VD items subscription after PTU There was found an issue that SDL still kept subscriptions for custom vehicle data items even for case when they were removed during PTU. By that reason, SDL was crashing with DCHECK on attempt to unsubscribe from non existing custom data. There was added missing logic of internal unsubscribe in case some custom vehicle data was removed by policies. * Fix sending of UnsubscribeVehicleData Was added a missing part of logic of subscriptions control - cache manager was updated to keep the vehicle data which was removed by last PTU. Custom vehicle data manager was updated to consider the removed vehicle data and also prepare and send unsubscribe requests to HMI for case when some vehicle data was removed and some application was subscribed on it. * fixup! Fix sending of UnsubscribeVehicleData * fixup! Fix sending of UnsubscribeVehicleData * fixup! Fix sending of UnsubscribeVehicleData * fixup! Fix sending of UnsubscribeVehicleData --- .../policies/custom_vehicle_data_provider.h | 7 +++ .../application_manager/policies/policy_handler.h | 3 ++ .../custom_vehicle_data_manager.h | 9 ++++ .../custom_vehicle_data_manager_impl.h | 4 ++ .../vehicle_info_plugin/vehicle_info_plugin.h | 1 + .../src/custom_vehicle_data_manager_impl.cc | 27 ++++++++++ .../vehicle_info_plugin/src/vehicle_info_plugin.cc | 46 ++++++++++++++++ .../test/custom_vehicle_data_manager_test.cc | 2 + .../mock_custom_vehicle_data_manager.h | 2 + .../src/policies/policy_handler.cc | 5 ++ .../policy/policy_external/policy/policy_manager.h | 7 +++ .../policy/policy_regular/policy/policy_manager.h | 7 +++ .../policies/mock_custom_vehicle_data_provider.h | 3 ++ .../policies/mock_policy_handler_interface.h | 3 ++ .../policy_external/policy/mock_cache_manager.h | 2 + .../policy_external/policy/mock_policy_manager.h | 2 + .../policy_regular/policy/mock_cache_manager.h | 2 + .../policy_regular/policy/mock_policy_manager.h | 2 + .../policy_external/include/policy/cache_manager.h | 22 ++++++++ .../include/policy/cache_manager_interface.h | 7 +++ .../include/policy/policy_manager_impl.h | 3 ++ .../policy/policy_external/src/cache_manager.cc | 61 ++++++++++++++++++++- .../policy_external/src/policy_manager_impl.cc | 5 ++ .../policy_regular/include/policy/cache_manager.h | 22 ++++++++ .../include/policy/cache_manager_interface.h | 7 +++ .../include/policy/policy_manager_impl.h | 3 ++ .../policy/policy_regular/src/cache_manager.cc | 62 ++++++++++++++++++++-- .../policy_regular/src/policy_manager_impl.cc | 5 ++ 28 files changed, 327 insertions(+), 4 deletions(-) diff --git a/src/components/application_manager/include/application_manager/policies/custom_vehicle_data_provider.h b/src/components/application_manager/include/application_manager/policies/custom_vehicle_data_provider.h index edd0a29c48..307f0b93df 100644 --- a/src/components/application_manager/include/application_manager/policies/custom_vehicle_data_provider.h +++ b/src/components/application_manager/include/application_manager/policies/custom_vehicle_data_provider.h @@ -19,6 +19,13 @@ class VehicleDataItemProvider { */ virtual const std::vector GetVehicleDataItems() const = 0; + + /** + * @brief Gets vehicle data items removed by policies + * @return Structure with vehicle data items + */ + virtual std::vector + GetRemovedVehicleDataItems() const = 0; }; } // namespace policy diff --git a/src/components/application_manager/include/application_manager/policies/policy_handler.h b/src/components/application_manager/include/application_manager/policies/policy_handler.h index 61cf2cf480..e4d4cd7fa2 100644 --- a/src/components/application_manager/include/application_manager/policies/policy_handler.h +++ b/src/components/application_manager/include/application_manager/policies/policy_handler.h @@ -704,6 +704,9 @@ class PolicyHandler : public PolicyHandlerInterface, const std::vector GetVehicleDataItems() const OVERRIDE; + std::vector + GetRemovedVehicleDataItems() const OVERRIDE; + void OnLockScreenDismissalStateChanged() FINAL; protected: diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager.h b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager.h index b34a9f84b8..9cac76b954 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager.h +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager.h @@ -41,6 +41,15 @@ class CustomVehicleDataManager { virtual void OnPolicyEvent(plugin_manager::PolicyEvent policy_event) = 0; virtual bool IsValidCustomVehicleDataName(const std::string& name) const = 0; + + /** + * @brief Checks whether custom vehicle data name was removed after the last + * PTU or not + * @param name vehicle item name to check + * @return true if vehicle data with this name was removed after the last PTU + */ + virtual bool IsRemovedCustomVehicleDataName( + const std::string& name) const = 0; }; } // namespace vehicle_info_plugin #endif // SRC_COMPONENTS_APPLICATION_MANAGER_RPC_PLUGINS_VEHICLE_INFO_PLUGIN_INCLUDE_VEHICLE_INFO_PLUGIN_CUSTOM_VEHICLE_DATA_MANAGER_H_ diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager_impl.h b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager_impl.h index 701bb1ba74..239c4edb90 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager_impl.h +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/custom_vehicle_data_manager_impl.h @@ -42,6 +42,8 @@ class CustomVehicleDataManagerImpl : public CustomVehicleDataManager { bool IsValidCustomVehicleDataName(const std::string& name) const OVERRIDE; + bool IsRemovedCustomVehicleDataName(const std::string& name) const OVERRIDE; + private: class RPCParams { public: @@ -83,6 +85,8 @@ class CustomVehicleDataManagerImpl : public CustomVehicleDataManager { const OptionalDataItem FindSchemaByNameNonRecursive( const std::string& name) const; + const OptionalDataItem FindRemovedSchemaByNameNonRecursive( + const std::string& name) const; const OptionalDataItem FindSchemaByKeyNonRecursive( const std::string& key) const; const OptionalDataItem FindSchemaByNameRecursive( diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/vehicle_info_plugin.h b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/vehicle_info_plugin.h index 3f6c078522..f0f68af298 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/vehicle_info_plugin.h +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/include/vehicle_info_plugin/vehicle_info_plugin.h @@ -72,6 +72,7 @@ class VehicleInfoPlugin : public plugins::RPCPlugin { VehicleInfoAppExtension& ext); private: + void UnsubscribeFromRemovedVDItems(); smart_objects::SmartObjectSPtr GetUnsubscribeIVIRequest( const std::vector& ivi_names); void DeleteSubscriptions(app_mngr::ApplicationSharedPtr app); diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/custom_vehicle_data_manager_impl.cc b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/custom_vehicle_data_manager_impl.cc index 334c876d76..ecd6be207b 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/custom_vehicle_data_manager_impl.cc +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/custom_vehicle_data_manager_impl.cc @@ -77,6 +77,12 @@ bool CustomVehicleDataManagerImpl::IsValidCustomVehicleDataName( return schema.is_initialized(); } +bool CustomVehicleDataManagerImpl::IsRemovedCustomVehicleDataName( + const std::string& name) const { + const auto& schema = FindRemovedSchemaByNameNonRecursive(name); + return schema.is_initialized(); +} + void CustomVehicleDataManagerImpl::CreateMobileMessageParams( smart_objects::SmartObject& msg_params) { using namespace application_manager; @@ -174,6 +180,12 @@ smart_objects::SmartObject CustomVehicleDataManagerImpl::CreateHMIMessageParams( auto schema = FindSchemaByNameNonRecursive(name); if (schema.is_initialized()) { fill_param(fill_hmi_params, *schema, &out_params); + continue; + } + + auto removed_schema = FindRemovedSchemaByNameNonRecursive(name); + if (removed_schema.is_initialized()) { + fill_param(fill_hmi_params, *removed_schema, &out_params); } } @@ -497,6 +509,21 @@ CustomVehicleDataManagerImpl::FindSchemaByNameNonRecursive( return FindSchema(oem_items, SearchMethod::NON_RECURSIVE, compare_by_name); } +const OptionalDataItem +CustomVehicleDataManagerImpl::FindRemovedSchemaByNameNonRecursive( + const std::string& name) const { + LOG4CXX_AUTO_TRACE(logger_); + + const auto& removed_oem_items = + vehicle_data_provider_.GetRemovedVehicleDataItems(); + auto compare_by_name = [&name](const policy_table::VehicleDataItem& item) { + return (name == std::string(item.name)); + }; + + return FindSchema( + removed_oem_items, SearchMethod::NON_RECURSIVE, compare_by_name); +} + const OptionalDataItem CustomVehicleDataManagerImpl::FindSchemaByNameRecursive( const std::string& name) const { LOG4CXX_AUTO_TRACE(logger_); diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc index d6d1c69461..40da7501c1 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_plugin.cc @@ -81,6 +81,7 @@ app_mngr::CommandFactory& VehicleInfoPlugin::GetCommandFactory() { } void VehicleInfoPlugin::OnPolicyEvent(plugins::PolicyEvent event) { + UnsubscribeFromRemovedVDItems(); custom_vehicle_data_manager_->OnPolicyEvent(event); } @@ -96,6 +97,43 @@ void VehicleInfoPlugin::OnApplicationEvent( } } +void VehicleInfoPlugin::UnsubscribeFromRemovedVDItems() { + typedef std::vector StringsVector; + + auto get_items_to_unsubscribe = [this]() -> StringsVector { + StringsVector output_items_list; + auto applications = application_manager_->applications(); + for (auto& app : applications.GetData()) { + auto& ext = VehicleInfoAppExtension::ExtractVIExtension(*app); + auto subscription_names = ext.Subscriptions(); + for (auto& subscription_name : subscription_names) { + if (custom_vehicle_data_manager_->IsRemovedCustomVehicleDataName( + subscription_name)) { + ext.unsubscribeFromVehicleInfo(subscription_name); + if (!helpers::in_range(output_items_list, subscription_name)) { + LOG4CXX_DEBUG(logger_, + "Vehicle data item " + << subscription_name + << " has been removed by policy"); + output_items_list.push_back(subscription_name); + } + } + } + } + return output_items_list; + }; + + const StringsVector items_to_unsubscribe = get_items_to_unsubscribe(); + + if (items_to_unsubscribe.empty()) { + LOG4CXX_DEBUG(logger_, "There is no data to unsubscribe"); + return; + } + + auto message = GetUnsubscribeIVIRequest(items_to_unsubscribe); + application_manager_->GetRPCService().ManageHMICommand(message); +} + void VehicleInfoPlugin::ProcessResumptionSubscription( application_manager::Application& app, VehicleInfoAppExtension& ext) { LOG4CXX_AUTO_TRACE(logger_); @@ -103,9 +141,15 @@ void VehicleInfoPlugin::ProcessResumptionSubscription( smart_objects::SmartObject(smart_objects::SmartType_Map); msg_params[strings::app_id] = app.app_id(); const auto& subscriptions = ext.Subscriptions(); + if (subscriptions.empty()) { + LOG4CXX_DEBUG(logger_, "No vehicle data to subscribe. Exiting"); + return; + } + for (const auto& item : subscriptions) { msg_params[item] = true; } + smart_objects::SmartObjectSPtr request = application_manager::MessageHelper::CreateModuleInfoSO( hmi_apis::FunctionID::VehicleInfo_SubscribeVehicleData, @@ -151,6 +195,8 @@ smart_objects::SmartObjectSPtr VehicleInfoPlugin::GetUnsubscribeIVIRequest( if (key_name.empty()) { // the name hasn't been found in vehicle data types if (custom_vehicle_data_manager_->IsValidCustomVehicleDataName( + ivi_name) || + custom_vehicle_data_manager_->IsRemovedCustomVehicleDataName( ivi_name)) { key_name = ivi_name; } diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/custom_vehicle_data_manager_test.cc b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/custom_vehicle_data_manager_test.cc index 2ef2d285ab..7d65d989bf 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/custom_vehicle_data_manager_test.cc +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/custom_vehicle_data_manager_test.cc @@ -60,6 +60,8 @@ class CustomVehicleDataManagerTest : public ::testing::Test { ON_CALL(mock_custom_vehicle_data_provider_, GetVehicleDataItems()) .WillByDefault(Return(items)); + ON_CALL(mock_custom_vehicle_data_provider_, GetRemovedVehicleDataItems()) + .WillByDefault(Return(policy_table::VehicleDataItems())); custom_vd_manager_.reset(new CustomVehicleDataManagerImpl( mock_custom_vehicle_data_provider_, mock_rpc_service_)); } diff --git a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/include/vehicle_info_plugin/mock_custom_vehicle_data_manager.h b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/include/vehicle_info_plugin/mock_custom_vehicle_data_manager.h index 472568d378..9a7435f980 100644 --- a/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/include/vehicle_info_plugin/mock_custom_vehicle_data_manager.h +++ b/src/components/application_manager/rpc_plugins/vehicle_info_plugin/test/include/vehicle_info_plugin/mock_custom_vehicle_data_manager.h @@ -18,6 +18,8 @@ class MockCustomVehicleDataManager : public CustomVehicleDataManager { MOCK_METHOD1(OnPolicyEvent, void(plugin_manager::PolicyEvent policy_event)); MOCK_CONST_METHOD1(IsValidCustomVehicleDataName, bool(const std::string& name)); + MOCK_CONST_METHOD1(IsRemovedCustomVehicleDataName, + bool(const std::string& name)); }; } // namespace vehicle_info_plugin diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index b0a4c4586d..09dcb2c7ad 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -2325,6 +2325,11 @@ policy::PolicyHandler::GetVehicleDataItems() const { return policy_manager_->GetVehicleDataItems(); } +std::vector +policy::PolicyHandler::GetRemovedVehicleDataItems() const { + return policy_manager_->GetRemovedVehicleDataItems(); +} + #ifdef EXTERNAL_PROPRIETARY_MODE const MetaInfo PolicyHandler::GetMetaInfo() const { POLICY_LIB_CHECK(MetaInfo()); diff --git a/src/components/include/policy/policy_external/policy/policy_manager.h b/src/components/include/policy/policy_external/policy/policy_manager.h index f608b62057..248bbfe16c 100644 --- a/src/components/include/policy/policy_external/policy/policy_manager.h +++ b/src/components/include/policy/policy_external/policy/policy_manager.h @@ -588,6 +588,13 @@ class PolicyManager : public usage_statistics::StatisticsManager, virtual const std::vector GetVehicleDataItems() const = 0; + /** + * @brief Gets vehicle data items removed by policies + * @return Structure with vehicle data items + */ + virtual std::vector + GetRemovedVehicleDataItems() const = 0; + /** * @brief Gets copy of current policy table data * @return policy_table as json object diff --git a/src/components/include/policy/policy_regular/policy/policy_manager.h b/src/components/include/policy/policy_regular/policy/policy_manager.h index 7ec407bb34..ee1f5fa98f 100644 --- a/src/components/include/policy/policy_regular/policy/policy_manager.h +++ b/src/components/include/policy/policy_regular/policy/policy_manager.h @@ -563,6 +563,13 @@ class PolicyManager : public usage_statistics::StatisticsManager, virtual const std::vector GetVehicleDataItems() const = 0; + /** + * @brief Gets removed vehicle data items + * @return Structure with vehicle data items + */ + virtual std::vector + GetRemovedVehicleDataItems() const = 0; + /** * @brief Gets copy of current policy table data * @return policy_table as json object diff --git a/src/components/include/test/application_manager/policies/mock_custom_vehicle_data_provider.h b/src/components/include/test/application_manager/policies/mock_custom_vehicle_data_provider.h index b6533521ed..ba077122ce 100644 --- a/src/components/include/test/application_manager/policies/mock_custom_vehicle_data_provider.h +++ b/src/components/include/test/application_manager/policies/mock_custom_vehicle_data_provider.h @@ -13,6 +13,9 @@ class MockCustomVehicleDataProvider : public policy::VehicleDataItemProvider { MOCK_CONST_METHOD0( GetVehicleDataItems, const std::vector()); + MOCK_CONST_METHOD0( + GetRemovedVehicleDataItems, + std::vector()); }; } // namespace policy_test diff --git a/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h b/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h index deb405cbde..f74526148a 100644 --- a/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h +++ b/src/components/include/test/application_manager/policies/mock_policy_handler_interface.h @@ -73,6 +73,9 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface { MOCK_CONST_METHOD0( GetVehicleDataItems, const std::vector()); + MOCK_CONST_METHOD0( + GetRemovedVehicleDataItems, + std::vector()); #ifdef EXTERNAL_PROPRIETARY_MODE MOCK_METHOD3(OnSnapshotCreated, diff --git a/src/components/include/test/policy/policy_external/policy/mock_cache_manager.h b/src/components/include/test/policy/policy_external/policy/mock_cache_manager.h index 35b01d20c1..b74ddbd13c 100644 --- a/src/components/include/test/policy/policy_external/policy/mock_cache_manager.h +++ b/src/components/include/test/policy/policy_external/policy/mock_cache_manager.h @@ -79,6 +79,8 @@ class MockCacheManagerInterface : public ::policy::CacheManagerInterface { MOCK_CONST_METHOD0(GetPolicyTableData, Json::Value()); MOCK_CONST_METHOD0(GetVehicleDataItems, const std::vector()); + MOCK_CONST_METHOD0(GetRemovedVehicleDataItems, + std::vector()); MOCK_CONST_METHOD1(GetEnabledCloudApps, void(std::vector& enabled_apps)); MOCK_CONST_METHOD7(GetCloudAppParameters, diff --git a/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h b/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h index 99d671e4ef..8e6cc2666d 100644 --- a/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h +++ b/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h @@ -224,6 +224,8 @@ class MockPolicyManager : public PolicyManager { MOCK_CONST_METHOD0(GetPolicyTableData, Json::Value()); MOCK_CONST_METHOD0(GetVehicleDataItems, const std::vector()); + MOCK_CONST_METHOD0(GetRemovedVehicleDataItems, + std::vector()); MOCK_CONST_METHOD1(GetEnabledCloudApps, void(std::vector& enabled_apps)); MOCK_CONST_METHOD7(GetCloudAppParameters, diff --git a/src/components/include/test/policy/policy_regular/policy/mock_cache_manager.h b/src/components/include/test/policy/policy_regular/policy/mock_cache_manager.h index d3da4d74c3..e592caf3bc 100644 --- a/src/components/include/test/policy/policy_regular/policy/mock_cache_manager.h +++ b/src/components/include/test/policy/policy_regular/policy/mock_cache_manager.h @@ -65,6 +65,8 @@ class MockCacheManagerInterface : public CacheManagerInterface { MOCK_CONST_METHOD0(GetPolicyTableData, Json::Value()); MOCK_CONST_METHOD0(GetVehicleDataItems, const std::vector()); + MOCK_CONST_METHOD0(GetRemovedVehicleDataItems, + std::vector()); MOCK_CONST_METHOD1(GetEnabledCloudApps, void(std::vector& enabled_apps)); MOCK_CONST_METHOD7(GetCloudAppParameters, diff --git a/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h b/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h index d711b50bab..927f5ba5cc 100644 --- a/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h +++ b/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h @@ -219,6 +219,8 @@ class MockPolicyManager : public PolicyManager { MOCK_CONST_METHOD0(GetPolicyTableData, Json::Value()); MOCK_CONST_METHOD0(GetVehicleDataItems, const std::vector()); + MOCK_CONST_METHOD0(GetRemovedVehicleDataItems, + std::vector()); MOCK_CONST_METHOD1(GetEnabledCloudApps, void(std::vector& enabled_apps)); MOCK_CONST_METHOD7(GetCloudAppParameters, diff --git a/src/components/policy/policy_external/include/policy/cache_manager.h b/src/components/policy/policy_external/include/policy/cache_manager.h index acc7a7da51..1bac72e2c6 100644 --- a/src/components/policy/policy_external/include/policy/cache_manager.h +++ b/src/components/policy/policy_external/include/policy/cache_manager.h @@ -166,6 +166,9 @@ class CacheManager : public CacheManagerInterface { virtual const std::vector GetVehicleDataItems() const OVERRIDE; + std::vector GetRemovedVehicleDataItems() + const OVERRIDE; + /** * @brief Gets copy of current policy table data * @return policy_table as json object @@ -893,6 +896,24 @@ class CacheManager : public CacheManagerInterface { */ void CheckSnapshotInitialization(); + /** + * @brief Calculates difference between two provided custom vehicle data items + * @param items_before list of vehicle data items before PTU was applied + * @param items_after list of vehicle data items after PTU was applied + * @return list with calculated difference or empty list if two input lists + * are equal + */ + policy_table::VehicleDataItems CalculateCustomVdItemsDiff( + const policy_table::VehicleDataItems& items_before, + const policy_table::VehicleDataItems& items_after) const; + + /** + * @brief Sets the custom vehicle data items + * @param removed_items list of vehicle data items to set + */ + void SetRemovedCustomVdItems( + const policy_table::VehicleDataItems& removed_items); + void PersistData(); /** @@ -938,6 +959,7 @@ class CacheManager : public CacheManagerInterface { bool update_required; typedef std::set UnpairedDevices; UnpairedDevices is_unpaired_; + policy_table::VehicleDataItems removed_custom_vd_items_; mutable sync_primitives::RecursiveLock cache_lock_; sync_primitives::Lock unpaired_lock_; diff --git a/src/components/policy/policy_external/include/policy/cache_manager_interface.h b/src/components/policy/policy_external/include/policy/cache_manager_interface.h index ff09ed608d..2cb5f8fe33 100644 --- a/src/components/policy/policy_external/include/policy/cache_manager_interface.h +++ b/src/components/policy/policy_external/include/policy/cache_manager_interface.h @@ -174,6 +174,13 @@ class CacheManagerInterface { virtual const std::vector GetVehicleDataItems() const = 0; + /** + * @brief Gets vehicle data items removed after the last PTU + * @return List of removed vehicle data items + */ + virtual std::vector + GetRemovedVehicleDataItems() const = 0; + /** * @brief Get a list of enabled cloud applications * @param enabled_apps List filled with the policy app id of each enabled diff --git a/src/components/policy/policy_external/include/policy/policy_manager_impl.h b/src/components/policy/policy_external/include/policy/policy_manager_impl.h index 60d70a9283..b1c22ab9e6 100644 --- a/src/components/policy/policy_external/include/policy/policy_manager_impl.h +++ b/src/components/policy/policy_external/include/policy/policy_manager_impl.h @@ -655,6 +655,9 @@ class PolicyManagerImpl : public PolicyManager { const std::vector GetVehicleDataItems() const OVERRIDE; + std::vector GetRemovedVehicleDataItems() + const OVERRIDE; + /** * @brief Get a list of enabled cloud applications * @param enabled_apps List filled with the policy app id of each enabled diff --git a/src/components/policy/policy_external/src/cache_manager.cc b/src/components/policy/policy_external/src/cache_manager.cc index 3e1538ab04..e1015ea574 100644 --- a/src/components/policy/policy_external/src/cache_manager.cc +++ b/src/components/policy/policy_external/src/cache_manager.cc @@ -269,7 +269,8 @@ CacheManager::CacheManager() : CacheManagerInterface() , pt_(new policy_table::Table) , backup_(new SQLPTExtRepresentation()) - , update_required(false) { + , update_required(false) + , removed_custom_vd_items_() { InitBackupThread(); } @@ -758,6 +759,12 @@ bool CacheManager::ApplyUpdate(const policy_table::Table& update_pt) { // Apply update for vehicle data if (update_pt.policy_table.vehicle_data.is_initialized()) { + policy_table::VehicleDataItems custom_items_before_apply; + if (pt_->policy_table.vehicle_data->schema_items.is_initialized()) { + custom_items_before_apply = + CollectCustomVDItems(*pt_->policy_table.vehicle_data->schema_items); + } + if (!update_pt.policy_table.vehicle_data->schema_items.is_initialized() || update_pt.policy_table.vehicle_data->schema_items->empty()) { pt_->policy_table.vehicle_data->schema_items = @@ -771,6 +778,12 @@ bool CacheManager::ApplyUpdate(const policy_table::Table& update_pt) { pt_->policy_table.vehicle_data->schema_items = rpc::Optional(custom_items); } + + policy_table::VehicleDataItems custom_items_after_apply = + *pt_->policy_table.vehicle_data->schema_items; + const auto& items_diff = CalculateCustomVdItemsDiff( + custom_items_before_apply, custom_items_after_apply); + SetRemovedCustomVdItems(items_diff); } ResetCalculatedPermissions(); @@ -1430,6 +1443,12 @@ CacheManager::GetVehicleDataItems() const { return std::vector(); } +std::vector +CacheManager::GetRemovedVehicleDataItems() const { + CACHE_MANAGER_CHECK(std::vector()); + return removed_custom_vd_items_; +} + Json::Value CacheManager::GetPolicyTableData() const { return pt_->policy_table.ToJsonValue(); } @@ -1922,6 +1941,46 @@ void CacheManager::CheckSnapshotInitialization() { } } +policy_table::VehicleDataItems CacheManager::CalculateCustomVdItemsDiff( + const policy_table::VehicleDataItems& items_before, + const policy_table::VehicleDataItems& items_after) const { + LOG4CXX_AUTO_TRACE(logger_); + if (items_before.empty()) { + LOG4CXX_DEBUG(logger_, "No custom VD items found in policy"); + return policy_table::VehicleDataItems(); + } + + if (items_after.empty()) { + LOG4CXX_DEBUG(logger_, + "All custom VD items were removed after policy update"); + return items_before; + } + + policy_table::VehicleDataItems removed_items; + for (auto& item_to_search : items_before) { + auto item_predicate = + [&item_to_search](const policy_table::VehicleDataItem& item_to_check) { + return item_to_search.name == item_to_check.name; + }; + + auto it = + std::find_if(items_after.begin(), items_after.end(), item_predicate); + if (items_after.end() == it) { + removed_items.push_back(item_to_search); + } + } + + LOG4CXX_DEBUG(logger_, + "Found " << removed_items.size() << " removed VD items"); + return removed_items; +} + +void CacheManager::SetRemovedCustomVdItems( + const policy_table::VehicleDataItems& removed_items) { + LOG4CXX_AUTO_TRACE(logger_); + removed_custom_vd_items_ = removed_items; +} + void CacheManager::PersistData() { LOG4CXX_AUTO_TRACE(logger_); if (backup_.use_count() != 0) { diff --git a/src/components/policy/policy_external/src/policy_manager_impl.cc b/src/components/policy/policy_external/src/policy_manager_impl.cc index b9eed0be4e..6868b9060c 100644 --- a/src/components/policy/policy_external/src/policy_manager_impl.cc +++ b/src/components/policy/policy_external/src/policy_manager_impl.cc @@ -804,6 +804,11 @@ PolicyManagerImpl::GetVehicleDataItems() const { return cache_->GetVehicleDataItems(); } +std::vector +PolicyManagerImpl::GetRemovedVehicleDataItems() const { + return cache_->GetRemovedVehicleDataItems(); +} + Json::Value PolicyManagerImpl::GetPolicyTableData() const { return cache_->GetPolicyTableData(); } diff --git a/src/components/policy/policy_regular/include/policy/cache_manager.h b/src/components/policy/policy_regular/include/policy/cache_manager.h index 9b0413d7ce..6bd2c4840b 100644 --- a/src/components/policy/policy_regular/include/policy/cache_manager.h +++ b/src/components/policy/policy_regular/include/policy/cache_manager.h @@ -154,6 +154,9 @@ class CacheManager : public CacheManagerInterface { virtual const std::vector GetVehicleDataItems() const; + std::vector GetRemovedVehicleDataItems() + const OVERRIDE; + const boost::optional LockScreenDismissalEnabledState() const OVERRIDE; const boost::optional LockScreenDismissalWarningMessage( @@ -919,6 +922,24 @@ class CacheManager : public CacheManagerInterface { */ void CheckSnapshotInitialization(); + /** + * @brief Calculates difference between two provided custom vehicle data items + * @param items_before list of vehicle data items before PTU was applied + * @param items_after list of vehicle data items after PTU was applied + * @return list with calculated difference or empty list if two input lists + * are equal + */ + policy_table::VehicleDataItems CalculateCustomVdItemsDiff( + const policy_table::VehicleDataItems& items_before, + const policy_table::VehicleDataItems& items_after) const; + + /** + * @brief Sets the custom vehicle data items + * @param removed_items list of vehicle data items to set + */ + void SetRemovedCustomVdItems( + const policy_table::VehicleDataItems& removed_items); + void PersistData(); /** @@ -945,6 +966,7 @@ class CacheManager : public CacheManagerInterface { bool update_required; typedef std::set UnpairedDevices; UnpairedDevices is_unpaired_; + policy_table::VehicleDataItems removed_custom_vd_items_; mutable sync_primitives::RecursiveLock cache_lock_; sync_primitives::Lock unpaired_lock_; diff --git a/src/components/policy/policy_regular/include/policy/cache_manager_interface.h b/src/components/policy/policy_regular/include/policy/cache_manager_interface.h index bd03896bfb..a1b4af0f39 100644 --- a/src/components/policy/policy_regular/include/policy/cache_manager_interface.h +++ b/src/components/policy/policy_regular/include/policy/cache_manager_interface.h @@ -161,6 +161,13 @@ class CacheManagerInterface { virtual const std::vector GetVehicleDataItems() const = 0; + /** + * @brief Gets vehicle data items removed after the last PTU + * @return List of removed vehicle data items + */ + virtual std::vector + GetRemovedVehicleDataItems() const = 0; + /** * @brief Get a list of enabled cloud applications * @param enabled_apps List filled with the policy app id of each enabled diff --git a/src/components/policy/policy_regular/include/policy/policy_manager_impl.h b/src/components/policy/policy_regular/include/policy/policy_manager_impl.h index 178de4a8f0..fc6ec369dc 100644 --- a/src/components/policy/policy_regular/include/policy/policy_manager_impl.h +++ b/src/components/policy/policy_regular/include/policy/policy_manager_impl.h @@ -663,6 +663,9 @@ class PolicyManagerImpl : public PolicyManager { const std::vector GetVehicleDataItems() const OVERRIDE; + std::vector GetRemovedVehicleDataItems() + const OVERRIDE; + /** * @brief Gets copy of current policy table data * @return policy_table as json object diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc index 166a79b89d..9c73d88e63 100644 --- a/src/components/policy/policy_regular/src/cache_manager.cc +++ b/src/components/policy/policy_regular/src/cache_manager.cc @@ -107,6 +107,7 @@ CacheManager::CacheManager() , pt_(new policy_table::Table) , backup_(new SQLPTRepresentation()) , update_required(false) + , removed_custom_vd_items_() , settings_(nullptr) { LOG4CXX_AUTO_TRACE(logger_); backuper_ = new BackgroundBackuper(this); @@ -304,6 +305,12 @@ bool CacheManager::ApplyUpdate(const policy_table::Table& update_pt) { // Apply update for vehicle data if (update_pt.policy_table.vehicle_data.is_initialized()) { + policy_table::VehicleDataItems custom_items_before_apply; + if (pt_->policy_table.vehicle_data->schema_items.is_initialized()) { + custom_items_before_apply = + CollectCustomVDItems(*pt_->policy_table.vehicle_data->schema_items); + } + if (!update_pt.policy_table.vehicle_data->schema_items.is_initialized() || update_pt.policy_table.vehicle_data->schema_items->empty()) { pt_->policy_table.vehicle_data->schema_items = @@ -317,9 +324,12 @@ bool CacheManager::ApplyUpdate(const policy_table::Table& update_pt) { pt_->policy_table.vehicle_data->schema_items = rpc::Optional(custom_items); } - if (update_pt.policy_table.vehicle_data->schema_version.is_initialized() && - update_pt.policy_table.vehicle_data->schema_items.is_initialized()) { - } + + policy_table::VehicleDataItems custom_items_after_apply = + *pt_->policy_table.vehicle_data->schema_items; + const auto& items_diff = CalculateCustomVdItemsDiff( + custom_items_before_apply, custom_items_after_apply); + SetRemovedCustomVdItems(items_diff); } ResetCalculatedPermissions(); @@ -727,6 +737,12 @@ CacheManager::GetVehicleDataItems() const { return std::vector(); } +std::vector +CacheManager::GetRemovedVehicleDataItems() const { + CACHE_MANAGER_CHECK(std::vector()); + return removed_custom_vd_items_; +} + Json::Value CacheManager::GetPolicyTableData() const { return pt_->policy_table.ToJsonValue(); } @@ -1226,6 +1242,46 @@ void CacheManager::CheckSnapshotInitialization() { } } +policy_table::VehicleDataItems CacheManager::CalculateCustomVdItemsDiff( + const policy_table::VehicleDataItems& items_before, + const policy_table::VehicleDataItems& items_after) const { + LOG4CXX_AUTO_TRACE(logger_); + if (items_before.empty()) { + LOG4CXX_DEBUG(logger_, "No custom VD items found in policy"); + return policy_table::VehicleDataItems(); + } + + if (items_after.empty()) { + LOG4CXX_DEBUG(logger_, + "All custom VD items were removed after policy update"); + return items_before; + } + + policy_table::VehicleDataItems removed_items; + for (auto& item_to_search : items_before) { + auto item_predicate = + [&item_to_search](const policy_table::VehicleDataItem& item_to_check) { + return item_to_search.name == item_to_check.name; + }; + + auto it = + std::find_if(items_after.begin(), items_after.end(), item_predicate); + if (items_after.end() == it) { + removed_items.push_back(item_to_search); + } + } + + LOG4CXX_DEBUG(logger_, + "Found " << removed_items.size() << " removed VD items"); + return removed_items; +} + +void CacheManager::SetRemovedCustomVdItems( + const policy_table::VehicleDataItems& removed_items) { + LOG4CXX_AUTO_TRACE(logger_); + removed_custom_vd_items_ = removed_items; +} + void CacheManager::PersistData() { LOG4CXX_AUTO_TRACE(logger_); if (backup_.use_count() != 0) { diff --git a/src/components/policy/policy_regular/src/policy_manager_impl.cc b/src/components/policy/policy_regular/src/policy_manager_impl.cc index 4c3f0763bc..2fef8de1d8 100644 --- a/src/components/policy/policy_regular/src/policy_manager_impl.cc +++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc @@ -721,6 +721,11 @@ PolicyManagerImpl::GetVehicleDataItems() const { return cache_->GetVehicleDataItems(); } +std::vector +PolicyManagerImpl::GetRemovedVehicleDataItems() const { + return cache_->GetRemovedVehicleDataItems(); +} + Json::Value PolicyManagerImpl::GetPolicyTableData() const { return cache_->GetPolicyTableData(); } -- cgit v1.2.1 From 494a8a4a4bdac82f47f87d521bc4ed37aeb57c17 Mon Sep 17 00:00:00 2001 From: "Andrii Kalinich (GitHub)" Date: Wed, 23 Oct 2019 12:39:26 -0700 Subject: Fix sending of OnServiceUpdate on service NACK (#3095) There was found one corner case when SDL does not change the internal status when service start was refused due to current HMI level of application. Was added missing check to set status properly and send expected notification. --- src/components/protocol_handler/src/protocol_handler_impl.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc index 61b9afd7bc..3b216c273c 100644 --- a/src/components/protocol_handler/src/protocol_handler_impl.cc +++ b/src/components/protocol_handler/src/protocol_handler_impl.cc @@ -1816,8 +1816,15 @@ void ProtocolHandlerImpl::NotifySessionStarted( LOG4CXX_WARN(logger_, "Refused by session_observer to create service " << static_cast(service_type) << " type."); + const auto session_id = packet->session_id(); + const auto connection_key = + session_observer_.KeyFromPair(context.connection_id_, session_id); + service_status_update_handler_->OnServiceUpdate( + connection_key, + context.service_type_, + ServiceStatus::SERVICE_START_FAILED); SendStartSessionNAck(context.connection_id_, - packet->session_id(), + session_id, protocol_version, packet->service_type(), rejected_params); -- cgit v1.2.1