From fdeb0598cc1be57216c107bfa25a0c94793dc424 Mon Sep 17 00:00:00 2001 From: jacobkeeler Date: Tue, 21 Apr 2020 12:56:06 -0400 Subject: Move cached app logic to ChoosePTUApplication Also add macro guards for uses of last_ptu_app_id_ --- .../application_manager/policies/policy_handler.h | 44 +++------------------- .../test/commands/hmi/hmi_notifications_test.cc | 6 +-- .../src/policies/policy_handler.cc | 44 +++++++++++++--------- .../test/policy_handler_test.cc | 19 +++------- .../policies/policy_handler_interface.h | 40 +++++++++++++++----- .../policies/mock_policy_handler_interface.h | 9 +++-- 6 files changed, 75 insertions(+), 87 deletions(-) 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 a0fef88b6d..2070554cd8 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 @@ -110,28 +110,8 @@ class PolicyHandler : public PolicyHandlerInterface, #else // EXTERNAL_PROPRIETARY_MODE void OnSnapshotCreated(const BinaryMessage& pt_string, const PTUIterationType iteration_type) OVERRIDE; - - /** - * @brief Get the next available PTU URL and the associated application for - * performing the PTU - * @param iteration_type The iteration type of the PTU. - * If this is a retry and a retry URL was cached, that URL will be returned - * @param app_id Filled with the ID of application used to perform the PTU on - * success - * @return The next available PTU URL on success, empty string on failure - */ std::string GetNextUpdateUrl(const PTUIterationType iteration_type, uint32_t& app_id) OVERRIDE; - - /** - * @brief Update the cached URL and app ID used for policy retries - * @param app_id The ID of the application to be used for performing PTUs. - * If 0, the existing cached application will be cleared - * @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 - */ - void CacheRetryInfo(const uint32_t app_id, const std::string url) OVERRIDE; #endif // EXTERNAL_PROPRIETARY_MODE virtual bool GetPriority(const std::string& policy_app_id, std::string* priority) const OVERRIDE; @@ -433,28 +413,14 @@ class PolicyHandler : public PolicyHandlerInterface, */ void OnSystemError(int code) OVERRIDE; - /** - * @brief Chooses and stores random application id to be used for snapshot - * sending considering HMI level - * @param iteration_type The iteration type of the request. If RetryIteration, - * the previously chosen app ID (via ChoosePTUApplication or CacheRetryInfo) - * will be returned if available - * @return Application id or 0, if there are no suitable applications - */ +#ifndef EXTERNAL_PROPRIETARY_MODE uint32_t ChoosePTUApplication( const PTUIterationType iteration_type = PTUIterationType::DefaultIteration) OVERRIDE; + void CacheRetryInfo(const uint32_t app_id, const std::string url) OVERRIDE; +#endif // EXTERNAL_PROPRIETARY_MODE - /** - * @brief Retrieve potential application id to be used for snapshot sending - * @param iteration_type The iteration type of the request. If RetryIteration, - * the previously chosen app ID (via ChoosePTUApplication or CacheRetryInfo) - * will be returned if available - * @return Application id or 0, if there are no suitable applications - */ - uint32_t GetAppIdForSending( - const PTUIterationType iteration_type = - PTUIterationType::DefaultIteration) const OVERRIDE; + uint32_t GetAppIdForSending() const OVERRIDE; /** * @brief Add application to PTU queue if no application with @@ -931,9 +897,11 @@ class PolicyHandler : public PolicyHandlerInterface, std::shared_ptr event_observer_; uint32_t last_activated_app_id_; +#ifndef EXTERNAL_PROPRIETARY_MODE // PTU retry information uint32_t last_ptu_app_id_; std::string retry_update_url_; +#endif // EXTERNAL_PROPRIETARY_MODE /** * @brief Contains device handles, which were sent for user consent to HMI diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc index 9403b862df..26f7a63872 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/hmi/hmi_notifications_test.cc @@ -1557,8 +1557,7 @@ TEST_F(HMICommandsNotificationsTest, std::shared_ptr command = CreateCommand(message); - EXPECT_CALL(mock_policy_handler_, - GetAppIdForSending(policy::PTUIterationType::DefaultIteration)) + EXPECT_CALL(mock_policy_handler_, GetAppIdForSending()) .WillOnce(Return(kAppId_)); EXPECT_CALL(app_mngr_, application(_)).WillOnce(Return(app_)); ON_CALL(app_mngr_, connection_handler()) @@ -1594,8 +1593,7 @@ TEST_F(HMICommandsNotificationsTest, std::shared_ptr command = CreateCommand(message); - EXPECT_CALL(mock_policy_handler_, - GetAppIdForSending(policy::PTUIterationType::DefaultIteration)) + EXPECT_CALL(mock_policy_handler_, GetAppIdForSending()) .WillOnce(Return(kNullApppId)); EXPECT_CALL(app_mngr_, application(_)).Times(0); EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)).Times(0); diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc index 356c7a765e..cee33b229b 100644 --- a/src/components/application_manager/src/policies/policy_handler.cc +++ b/src/components/application_manager/src/policies/policy_handler.cc @@ -299,11 +299,14 @@ PolicyHandler::PolicyHandler(const PolicySettings& settings, ApplicationManager& application_manager) : AsyncRunner("PolicyHandler async runner thread") , last_activated_app_id_(0) +#ifndef EXTERNAL_PROPRIETARY_MODE , last_ptu_app_id_(0) +#endif // EXTERNAL_PROPRIETARY_MODE , statistic_manager_impl_(std::make_shared(this)) , settings_(settings) , application_manager_(application_manager) - , last_registered_policy_app_id_(std::string()) {} + , last_registered_policy_app_id_(std::string()) { +} PolicyHandler::~PolicyHandler() {} @@ -401,8 +404,10 @@ void PolicyHandler::OnPTInited() { void PolicyHandler::StopRetrySequence() { LOG4CXX_AUTO_TRACE(logger_); POLICY_LIB_CHECK_VOID(); +#ifndef EXTERNAL_PROPRIETARY_MODE // Clear cached PTU app last_ptu_app_id_ = 0; +#endif // EXTERNAL_PROPRIETARY_MODE policy_manager_->StopRetrySequence(); } @@ -423,17 +428,11 @@ bool PolicyHandler::ClearUserConsent() { return policy_manager_->ResetUserConsent(); } +#ifndef EXTERNAL_PROPRIETARY_MODE uint32_t PolicyHandler::ChoosePTUApplication( const PTUIterationType iteration_type) { LOG4CXX_AUTO_TRACE(logger_); - last_ptu_app_id_ = GetAppIdForSending(iteration_type); - return last_ptu_app_id_; -} -uint32_t PolicyHandler::GetAppIdForSending( - const PTUIterationType iteration_type) const { - LOG4CXX_AUTO_TRACE(logger_); - POLICY_LIB_CHECK_OR_RETURN(0); // Return the previous app chosen if this is a retry for a PTU in progress if (iteration_type == PTUIterationType::RetryIteration && last_ptu_app_id_ != 0) { @@ -445,6 +444,21 @@ uint32_t PolicyHandler::GetAppIdForSending( } } + last_ptu_app_id_ = GetAppIdForSending(); + return last_ptu_app_id_; +} + +void PolicyHandler::CacheRetryInfo(const uint32_t app_id, + const std::string url) { + last_ptu_app_id_ = app_id; + retry_update_url_ = url; +} +#endif // EXTERNAL_PROPRIETARY_MODE + +uint32_t PolicyHandler::GetAppIdForSending() const { + LOG4CXX_AUTO_TRACE(logger_); + POLICY_LIB_CHECK_OR_RETURN(0); + // fix ApplicationSet access crash const ApplicationSet accessor = application_manager_.applications().GetData(); @@ -1120,8 +1134,7 @@ void PolicyHandler::OnPendingPermissionChange( bool PolicyHandler::SendMessageToSDK(const BinaryMessage& pt_string, const std::string& url) { - const uint32_t app_id = - ChoosePTUApplication(PTUIterationType::DefaultIteration); + const uint32_t app_id = GetAppIdForSending(); return SendMessageToSDK(pt_string, url, app_id); } @@ -1180,9 +1193,10 @@ bool PolicyHandler::ReceiveMessageFromSDK(const std::string& file, policy_manager_->CleanupUnpairedDevices(); SetDaysAfterEpoch(); policy_manager_->OnPTUFinished(load_pt_result); - - // Clean up retry information (used in PROPRIETARY and HTTP mode) +#ifndef EXTERNAL_PROPRIETARY_MODE + // Clean up retry information last_ptu_app_id_ = 0; +#endif // EXTERNAL_PROPRIETARY_MODE uint32_t correlation_id = application_manager_.GetNextHMICorrelationID(); event_observer_->subscribe_on_event( @@ -1686,12 +1700,6 @@ std::string PolicyHandler::GetNextUpdateUrl( const std::string& url = data.url[app_url.second]; return url; } - -void PolicyHandler::CacheRetryInfo(const uint32_t app_id, - const std::string url) { - last_ptu_app_id_ = app_id; - retry_update_url_ = url; -} #endif // EXTERNAL_PROPRIETARY_MODE bool PolicyHandler::GetPriority(const std::string& policy_app_id, diff --git a/src/components/application_manager/test/policy_handler_test.cc b/src/components/application_manager/test/policy_handler_test.cc index e444edeb52..cfaa771fa0 100644 --- a/src/components/application_manager/test/policy_handler_test.cc +++ b/src/components/application_manager/test/policy_handler_test.cc @@ -2035,9 +2035,7 @@ TEST_F(PolicyHandlerTest, GetAppIdForSending_WithoutApps) { EXPECT_CALL(app_manager_, applications()).WillRepeatedly(Return(app_set)); // Set does not include any applications EXPECT_CALL(*mock_policy_manager_, GetUserConsentForDevice(_)).Times(0); - EXPECT_EQ( - 0u, - policy_handler_.GetAppIdForSending(PTUIterationType::DefaultIteration)); + EXPECT_EQ(0u, policy_handler_.GetAppIdForSending()); } TEST_F(PolicyHandlerTest, GetAppIdForSending_GetDefaultMacAddress) { @@ -2063,9 +2061,7 @@ TEST_F(PolicyHandlerTest, GetAppIdForSending_GetDefaultMacAddress) { EXPECT_CALL(*mock_policy_manager_, GetUserConsentForDevice(_)) .WillRepeatedly(Return(kDeviceAllowed)); // Act - EXPECT_EQ( - kAppId1_, - policy_handler_.GetAppIdForSending(PTUIterationType::DefaultIteration)); + EXPECT_EQ(kAppId1_, policy_handler_.GetAppIdForSending()); } void PolicyHandlerTest::GetAppIDForSending() { @@ -2090,9 +2086,7 @@ TEST_F(PolicyHandlerTest, GetAppIdForSending_SetMacAddress) { // Arrange GetAppIDForSending(); // Act - EXPECT_EQ( - kAppId1_, - policy_handler_.GetAppIdForSending(PTUIterationType::DefaultIteration)); + EXPECT_EQ(kAppId1_, policy_handler_.GetAppIdForSending()); } TEST_F(PolicyHandlerTest, GetAppIdForSending_ExpectReturnAnyIdButNone) { @@ -2156,9 +2150,7 @@ TEST_F(PolicyHandlerTest, GetAppIdForSending_ExpectReturnAnyIdButNone) { .WillRepeatedly(Return(kDeviceAllowed)); // Act - EXPECT_NE( - app_in_none_id, - policy_handler_.GetAppIdForSending(PTUIterationType::DefaultIteration)); + EXPECT_NE(app_in_none_id, policy_handler_.GetAppIdForSending()); } TEST_F(PolicyHandlerTest, GetAppIdForSending_ExpectReturnAnyAppInNone) { @@ -2201,8 +2193,7 @@ TEST_F(PolicyHandlerTest, GetAppIdForSending_ExpectReturnAnyAppInNone) { // Act - const uint32_t app_id = - policy_handler_.GetAppIdForSending(PTUIterationType::DefaultIteration); + const uint32_t app_id = policy_handler_.GetAppIdForSending(); EXPECT_EQ(app_in_none_id_1 || app_in_none_id_2, app_id); } 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 39f3616a13..f85483f98e 100644 --- a/src/components/include/application_manager/policies/policy_handler_interface.h +++ b/src/components/include/application_manager/policies/policy_handler_interface.h @@ -100,9 +100,18 @@ class PolicyHandlerInterface : public VehicleDataItemProvider { #else // EXTERNAL_PROPRIETARY_MODE virtual void OnSnapshotCreated(const BinaryMessage& pt_string, const PTUIterationType iteration_type) = 0; + + /** + * @brief Get the next available PTU URL and the associated application for + * performing the PTU + * @param iteration_type The iteration type of the PTU. + * If this is a retry and a retry URL was cached, that URL will be returned + * @param app_id Filled with the ID of application used to perform the PTU on + * success + * @return The next available PTU URL on success, empty string on failure + */ virtual std::string GetNextUpdateUrl(const PTUIterationType iteration_type, uint32_t& app_id) = 0; - virtual void CacheRetryInfo(const uint32_t app_id, const std::string url) = 0; #endif // EXTERNAL_PROPRIETARY_MODE virtual bool GetPriority(const std::string& policy_app_id, @@ -346,22 +355,35 @@ class PolicyHandlerInterface : public VehicleDataItemProvider { */ virtual void OnSystemError(int code) = 0; +#ifndef EXTERNAL_PROPRIETARY_MODE /** - * @brief Choose application id to be used for snapshot sending - * @return Application id or 0, if there are no applications registered + * @brief Chooses and stores random application id to be used for snapshot + * sending considering HMI level + * @param iteration_type The iteration type of the request. If RetryIteration, + * the previously chosen app ID (via ChoosePTUApplication or CacheRetryInfo) + * will be returned if available + * @return Application id or 0, if there are no suitable applications */ virtual uint32_t ChoosePTUApplication( const PTUIterationType iteration_type = PTUIterationType::DefaultIteration) = 0; /** - * @brief Retrieve an application id to be used for snapshot sending - * @param iteration_type - * @return Application id or 0, if there are no applications registered + * @brief Update the cached URL and app ID used for policy retries + * @param app_id The ID of the application to be used for performing PTUs. + * If 0, the existing cached application will be cleared + * @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 */ - virtual uint32_t GetAppIdForSending( - const PTUIterationType iteration_type = - PTUIterationType::DefaultIteration) const = 0; + virtual void CacheRetryInfo(const uint32_t app_id, const std::string url) = 0; +#endif // EXTERNAL_PROPRIETARY_MODE + + /** + * @brief Retrieve potential application id to be used for snapshot sending + * @return Application id or 0, if there are no suitable applications + */ + virtual uint32_t GetAppIdForSending() const = 0; virtual utils::custom_string::CustomString GetAppName( const std::string& policy_app_id) = 0; 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 e1fb8f702f..bcff478023 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 @@ -89,8 +89,6 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface { MOCK_METHOD2(GetNextUpdateUrl, std::string(const policy::PTUIterationType iteration_type, uint32_t& app_id)); - MOCK_METHOD2(CacheRetryInfo, - void(const uint32_t app_id, const std::string url)); #endif // EXTERNAL_PROPRIETARY_MODE MOCK_CONST_METHOD2(GetPriority, @@ -192,10 +190,13 @@ class MockPolicyHandlerInterface : public policy::PolicyHandlerInterface { MOCK_METHOD1(RemoveDevice, void(const std::string& device_id)); MOCK_METHOD1(AddStatisticsInfo, void(int type)); MOCK_METHOD1(OnSystemError, void(int code)); +#ifndef EXTERNAL_PROPRIETARY_MODE MOCK_METHOD1(ChoosePTUApplication, uint32_t(const policy::PTUIterationType iteration_type)); - MOCK_CONST_METHOD1(GetAppIdForSending, - uint32_t(const policy::PTUIterationType iteration_type)); + MOCK_METHOD2(CacheRetryInfo, + void(const uint32_t app_id, const std::string url)); +#endif + MOCK_CONST_METHOD0(GetAppIdForSending, uint32_t()); MOCK_METHOD1( GetAppName, utils::custom_string::CustomString(const std::string& policy_app_id)); -- cgit v1.2.1