diff options
author | JackLivio <jack@livio.io> | 2020-05-28 14:05:27 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-05-28 17:05:27 -0400 |
commit | 6c6ee382c87cfbffbb8a1b2112a44815ef95cca9 (patch) | |
tree | a2e6903be70c719cc15efd5d1b70e2af2a5351e6 | |
parent | 97620cd2b73a65d5a7d826563fa8e44966bef56c (diff) | |
download | sdl_core-6c6ee382c87cfbffbb8a1b2112a44815ef95cca9.tar.gz |
Fix onsystemrequest retry logic (#3400)
* Fix onsystemrequest retry logic
* Cache policy update file from HMI for retries
* Fix issues with previous commit
Co-authored-by: jacobkeeler <jacob.keeler@livioradio.com>
11 files changed, 100 insertions, 25 deletions
diff --git a/src/components/application_manager/include/application_manager/message_helper.h b/src/components/application_manager/include/application_manager/message_helper.h index 5d95a2468d..09d225a120 100644 --- a/src/components/application_manager/include/application_manager/message_helper.h +++ b/src/components/application_manager/include/application_manager/message_helper.h @@ -491,9 +491,23 @@ class MessageHelper { * @brief Send notification for Update of Policy Table * with PT Snapshot. * @param connection_key Id of application to send message to + * @param snapshot_file_path path to PT Snapshot + * @param url If empty string, no URL is provided + * @param timeout If -1 no timeout is provided + */ + static void SendPolicySnapshotNotification( + uint32_t connection_key, + const std::string& snapshot_file_path, + const std::string& url, + ApplicationManager& app_mngr); + + /* + * @brief Send notification for Update of Policy Table + * with PT Snapshot. + * @param connection_key Id of application to send message to * @param policy_data PT Snapshot * @param url If empty string, no URL is provided - * @param timeout If -1 no timeout is provdied + * @param timeout If -1 no timeout is provided */ static void SendPolicySnapshotNotification( uint32_t connection_key, 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 2c7b878023..8559e77061 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 @@ -419,7 +419,9 @@ class PolicyHandler : public PolicyHandlerInterface, uint32_t ChoosePTUApplication( const PTUIterationType iteration_type = PTUIterationType::DefaultIteration) OVERRIDE; - void CacheRetryInfo(const uint32_t app_id, const std::string url) OVERRIDE; + void CacheRetryInfo(const uint32_t app_id = 0, + const std::string url = std::string(), + const std::string snapshot_path = std::string()) OVERRIDE; #endif // EXTERNAL_PROPRIETARY_MODE uint32_t GetAppIdForSending() const OVERRIDE; @@ -903,6 +905,7 @@ class PolicyHandler : public PolicyHandlerInterface, // PTU retry information uint32_t last_ptu_app_id_; std::string retry_update_url_; + std::string policy_snapshot_path_; #endif // EXTERNAL_PROPRIETARY_MODE /** diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc index d96d2bb90e..4f83e3e653 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_system_request_notification.cc @@ -84,10 +84,12 @@ void OnSystemRequestNotification::Run() { policy_handler_.CacheRetryInfo(msg_params.keyExists(strings::app_id) ? msg_params[strings::app_id].asUInt() : 0, - msg_params[strings::url].asString()); + msg_params[strings::url].asString(), + msg_params[strings::file_name].asString()); } else { // Clear cached retry info - policy_handler_.CacheRetryInfo(0, std::string()); + policy_handler_.CacheRetryInfo( + 0, std::string(), msg_params[strings::file_name].asString()); // URL and app are chosen by Core for PROPRIETARY mode normally uint32_t app_id = 0; diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/on_system_request_notification.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/on_system_request_notification.cc index 991144d7cf..9c868172f5 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/on_system_request_notification.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/on_system_request_notification.cc @@ -118,10 +118,14 @@ void OnSystemRequestNotification::Run() { mobile_apis::RequestType::OEM_SPECIFIC); BinaryMessage binary_data; - if (binary_data_is_required) { + if (binary_data_is_required && + (*message_)[strings::msg_params].keyExists(strings::file_name)) { const std::string filename = (*message_)[strings::msg_params][strings::file_name].asString(); file_system::ReadBinaryFile(filename, binary_data); + } else if ((*message_)[strings::params].keyExists(strings::binary_data)) { + // Binary data may already be attached to the message + binary_data = (*message_)[strings::params][strings::binary_data].asBinary(); } if (mobile_apis::RequestType::OEM_SPECIFIC == request_type) { diff --git a/src/components/application_manager/src/message_helper/message_helper.cc b/src/components/application_manager/src/message_helper/message_helper.cc index 9b12eb2889..d6fbed115b 100644 --- a/src/components/application_manager/src/message_helper/message_helper.cc +++ b/src/components/application_manager/src/message_helper/message_helper.cc @@ -2526,6 +2526,33 @@ bool MessageHelper::SendUnsubscribedWayPoints(ApplicationManager& app_mngr) { void MessageHelper::SendPolicySnapshotNotification( uint32_t connection_key, + const std::string& snapshot_file_path, + const std::string& url, + ApplicationManager& app_mngr) { + LOG4CXX_AUTO_TRACE(logger_); + smart_objects::SmartObject content(smart_objects::SmartType_Map); + const auto request_type = +#if defined(PROPRIETARY_MODE) || defined(EXTERNAL_PROPRIETARY_MODE) + mobile_apis::RequestType::PROPRIETARY; +#else + mobile_apis::RequestType::HTTP; +#endif // PROPRIETARY || EXTERNAL_PROPRIETARY_MODE + + content[strings::msg_params][strings::request_type] = request_type; + + if (!url.empty()) { + content[strings::msg_params][strings::url] = url; + } else { + LOG4CXX_WARN(logger_, "No service URLs"); + } + + content[strings::msg_params][strings::file_name] = snapshot_file_path; + + SendSystemRequestNotification(connection_key, content, app_mngr); +} + +void MessageHelper::SendPolicySnapshotNotification( + uint32_t connection_key, const std::vector<uint8_t>& policy_data, const std::string& url, ApplicationManager& app_mngr) { diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 678916f7eb..5032c2884c 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -449,9 +449,11 @@ uint32_t PolicyHandler::ChoosePTUApplication( } void PolicyHandler::CacheRetryInfo(const uint32_t app_id, - const std::string url) { + const std::string url, + const std::string snapshot_path) { last_ptu_app_id_ = app_id; retry_update_url_ = url; + policy_snapshot_path_ = snapshot_path; } #endif // EXTERNAL_PROPRIETARY_MODE @@ -1627,11 +1629,10 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string, uint32_t app_id_for_sending = 0; const std::string& url = GetNextUpdateUrl(PTUIterationType::RetryIteration, app_id_for_sending); - if (0 != url.length()) { + if (0 != url.length() && !policy_snapshot_path_.empty()) { MessageHelper::SendPolicySnapshotNotification( - app_id_for_sending, pt_string, url, application_manager_); + app_id_for_sending, policy_snapshot_path_, url, application_manager_); } - } else { std::string policy_snapshot_full_path; if (!SaveSnapshot(pt_string, policy_snapshot_full_path)) { diff --git a/src/components/application_manager/test/include/application_manager/mock_message_helper.h b/src/components/application_manager/test/include/application_manager/mock_message_helper.h index eb3c97bfff..5f6e613123 100644 --- a/src/components/application_manager/test/include/application_manager/mock_message_helper.h +++ b/src/components/application_manager/test/include/application_manager/mock_message_helper.h @@ -162,6 +162,11 @@ class MockMessageHelper { const bool require_encryption)); MOCK_METHOD4(SendPolicySnapshotNotification, void(uint32_t connection_key, + const std::string& snapshot_file_path, + const std::string& url, + ApplicationManager& app_mngr)); + MOCK_METHOD4(SendPolicySnapshotNotification, + void(uint32_t connection_key, const std::vector<uint8_t>& policy_data, const std::string& url, ApplicationManager& app_mngr)); diff --git a/src/components/application_manager/test/mock_message_helper.cc b/src/components/application_manager/test/mock_message_helper.cc index 959bbf5bd0..a001e852b8 100644 --- a/src/components/application_manager/test/mock_message_helper.cc +++ b/src/components/application_manager/test/mock_message_helper.cc @@ -220,6 +220,15 @@ void MessageHelper::SendOnPermissionsChangeNotification( void MessageHelper::SendPolicySnapshotNotification( uint32_t connection_key, + const std::string& snapshot_file_path, + const std::string& url, + ApplicationManager& app_mngr) { + MockMessageHelper::message_helper_mock()->SendPolicySnapshotNotification( + connection_key, snapshot_file_path, url, app_mngr); +} + +void MessageHelper::SendPolicySnapshotNotification( + uint32_t connection_key, const std::vector<uint8_t>& policy_data, const std::string& url, ApplicationManager& app_mngr) { diff --git a/src/components/include/application_manager/policies/policy_handler_interface.h b/src/components/include/application_manager/policies/policy_handler_interface.h index 735f44a314..1c039d6a88 100644 --- a/src/components/include/application_manager/policies/policy_handler_interface.h +++ b/src/components/include/application_manager/policies/policy_handler_interface.h @@ -375,8 +375,13 @@ class PolicyHandlerInterface : public VehicleDataItemProvider { * @param url The URL provided by the HMI to be used for performing PTU * retries. If empty, the existing cached URL will be cleared and Core will * choose which URLs to use on retry + * @param snapshot_path The PT snapshot path provided by the HMI. If empty, + * the existing cached snapshot path will be cleared. */ - virtual void CacheRetryInfo(const uint32_t app_id, const std::string url) = 0; + virtual void CacheRetryInfo( + const uint32_t app_id = 0, + const std::string url = std::string(), + const std::string snapshot_path = std::string()) = 0; #endif // EXTERNAL_PROPRIETARY_MODE /** 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 0dde38bc79..38c1cca69e 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 @@ -193,8 +193,10 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface { #ifndef EXTERNAL_PROPRIETARY_MODE MOCK_METHOD1(ChoosePTUApplication, uint32_t(const policy::PTUIterationType iteration_type)); - MOCK_METHOD2(CacheRetryInfo, - void(const uint32_t app_id, const std::string url)); + MOCK_METHOD3(CacheRetryInfo, + void(const uint32_t app_id, + const std::string url, + const std::string snapshot_path)); #endif MOCK_CONST_METHOD0(GetAppIdForSending, uint32_t()); MOCK_METHOD1( 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 c6389b503b..0cdca26f2e 100644 --- a/src/components/policy/policy_regular/src/policy_manager_impl.cc +++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc @@ -610,23 +610,26 @@ void PolicyManagerImpl::GetUpdateUrls(const uint32_t service_type, bool PolicyManagerImpl::RequestPTUpdate(const PTUIterationType iteration_type) { LOG4CXX_AUTO_TRACE(logger_); - std::shared_ptr<policy_table::Table> policy_table_snapshot = - cache_->GenerateSnapshot(); - if (!policy_table_snapshot) { - LOG4CXX_ERROR(logger_, "Failed to create snapshot of policy table"); - return false; - } + BinaryMessage update; + if (PTUIterationType::DefaultIteration == iteration_type) { + std::shared_ptr<policy_table::Table> policy_table_snapshot = + cache_->GenerateSnapshot(); + if (!policy_table_snapshot) { + LOG4CXX_ERROR(logger_, "Failed to create snapshot of policy table"); + return false; + } - IsPTValid(policy_table_snapshot, policy_table::PT_SNAPSHOT); + IsPTValid(policy_table_snapshot, policy_table::PT_SNAPSHOT); - Json::Value value = policy_table_snapshot->ToJsonValue(); - Json::StreamWriterBuilder writer_builder; - writer_builder["indentation"] = ""; - std::string message_string = Json::writeString(writer_builder, value); + Json::Value value = policy_table_snapshot->ToJsonValue(); + Json::StreamWriterBuilder writer_builder; + writer_builder["indentation"] = ""; + std::string message_string = Json::writeString(writer_builder, value); - LOG4CXX_DEBUG(logger_, "Snapshot contents is : " << message_string); + LOG4CXX_DEBUG(logger_, "Snapshot contents is : " << message_string); - BinaryMessage update(message_string.begin(), message_string.end()); + update = BinaryMessage(message_string.begin(), message_string.end()); + } ptu_requested_ = true; listener_->OnSnapshotCreated(update, iteration_type); return true; |