From 65f45c34d680c11a78ceea28400b1994a0c2fc33 Mon Sep 17 00:00:00 2001 From: Jacob Keeler Date: Wed, 19 Aug 2020 16:35:24 -0400 Subject: Make `subtle_notifications_per_minute_by_priority` non-mandatory (#3477) * Make `subtle_notifications_per_minute_by_priority` non-mandatory `notifications_per_minute_by_priority` will be used as a fallback if the value is not present in the policy table --- .../include/policy/policy_table/types.h | 7 ++-- .../include/policy/policy_table_interface_ext.xml | 3 +- .../policy/policy_external/src/cache_manager.cc | 11 +++--- .../policy_external/src/policy_manager_impl.cc | 2 +- .../policy_external/src/policy_table/types.cc | 13 +++---- .../policy_external/src/sql_pt_representation.cc | 8 ++-- .../policy_external/test/cache_manager_test.cc | 45 ++++++++++++++++++++++ .../test/sql_pt_representation_test.cc | 22 ++++++----- .../include/policy/policy_table/types.h | 7 ++-- .../policy/policy_regular/src/cache_manager.cc | 11 +++--- .../policy_regular/src/policy_manager_impl.cc | 2 +- .../policy_regular/src/policy_table/types.cc | 13 +++---- .../policy_regular/src/sql_pt_representation.cc | 8 ++-- .../policy_regular/test/cache_manager_test.cc | 45 ++++++++++++++++++++++ .../test/sql_pt_representation_test.cc | 22 ++++++----- 15 files changed, 156 insertions(+), 63 deletions(-) (limited to 'src/components') diff --git a/src/components/policy/policy_external/include/policy/policy_table/types.h b/src/components/policy/policy_external/include/policy/policy_table/types.h index 7736468aa0..c95802f181 100644 --- a/src/components/policy/policy_external/include/policy/policy_table/types.h +++ b/src/components/policy/policy_external/include/policy/policy_table/types.h @@ -382,7 +382,8 @@ struct ModuleConfig : CompositeType { ServiceEndpoints endpoints; Optional endpoint_properties; NumberOfNotificationsPerMinute notifications_per_minute_by_priority; - NumberOfNotificationsPerMinute subtle_notifications_per_minute_by_priority; + Optional + subtle_notifications_per_minute_by_priority; Optional > vehicle_make; Optional > vehicle_model; Optional > vehicle_year; @@ -404,9 +405,7 @@ struct ModuleConfig : CompositeType { const ServiceEndpoints& endpoints, const ServiceEndpointProperties& endpoint_properties, const NumberOfNotificationsPerMinute& - notifications_per_minute_by_priority, - const NumberOfNotificationsPerMinute& - subtle_notifications_per_minute_by_priority); + notifications_per_minute_by_priority); ~ModuleConfig(); explicit ModuleConfig(const Json::Value* value__); void SafeCopyFrom(const ModuleConfig& from); diff --git a/src/components/policy/policy_external/include/policy/policy_table_interface_ext.xml b/src/components/policy/policy_external/include/policy/policy_table_interface_ext.xml index 7fad362c57..64eab51870 100644 --- a/src/components/policy/policy_external/include/policy/policy_table_interface_ext.xml +++ b/src/components/policy/policy_external/include/policy/policy_table_interface_ext.xml @@ -176,7 +176,8 @@ - + policy_table.module_config - .subtle_notifications_per_minute_by_priority - : pt_->policy_table.module_config - .notifications_per_minute_by_priority; + const auto& module_config = pt_->policy_table.module_config; + const auto& nnpm = + is_subtle && module_config.subtle_notifications_per_minute_by_priority + .is_initialized() + ? *module_config.subtle_notifications_per_minute_by_priority + : module_config.notifications_per_minute_by_priority; auto priority_iter = nnpm.find(priority); 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 0c72aac4ad..7314fc040d 100644 --- a/src/components/policy/policy_external/src/policy_manager_impl.cc +++ b/src/components/policy/policy_external/src/policy_manager_impl.cc @@ -432,7 +432,7 @@ void FilterPolicyTable( module_config.subtle_notifications_per_minute_by_priority .is_initialized()) { FilterInvalidPriorityValues( - module_config.subtle_notifications_per_minute_by_priority); + *module_config.subtle_notifications_per_minute_by_priority); } if (pt.app_policies_section.is_initialized()) { diff --git a/src/components/policy/policy_external/src/policy_table/types.cc b/src/components/policy/policy_external/src/policy_table/types.cc index 6d50a8cf26..1f344f5263 100644 --- a/src/components/policy/policy_external/src/policy_table/types.cc +++ b/src/components/policy/policy_external/src/policy_table/types.cc @@ -830,9 +830,7 @@ ModuleConfig::ModuleConfig( const SecondsBetweenRetries& seconds_between_retries, const ServiceEndpoints& endpoints, const ServiceEndpointProperties& endpoint_properties, - const NumberOfNotificationsPerMinute& notifications_per_minute_by_priority, - const NumberOfNotificationsPerMinute& - subtle_notifications_per_minute_by_priority) + const NumberOfNotificationsPerMinute& notifications_per_minute_by_priority) : CompositeType(kUninitialized) , exchange_after_x_ignition_cycles(exchange_after_x_ignition_cycles) , exchange_after_x_kilometers(exchange_after_x_kilometers) @@ -841,9 +839,8 @@ ModuleConfig::ModuleConfig( , seconds_between_retries(seconds_between_retries) , endpoints(endpoints) , endpoint_properties(endpoint_properties) - , notifications_per_minute_by_priority(notifications_per_minute_by_priority) - , subtle_notifications_per_minute_by_priority( - subtle_notifications_per_minute_by_priority) {} + , notifications_per_minute_by_priority( + notifications_per_minute_by_priority) {} ModuleConfig::~ModuleConfig() {} @@ -885,10 +882,10 @@ void ModuleConfig::SafeCopyFrom(const ModuleConfig& from) { endpoint_properties = from.endpoint_properties; notifications_per_minute_by_priority = from.notifications_per_minute_by_priority; - subtle_notifications_per_minute_by_priority = - from.subtle_notifications_per_minute_by_priority; lock_screen_dismissal_enabled = from.lock_screen_dismissal_enabled; + subtle_notifications_per_minute_by_priority.assign_if_valid( + from.subtle_notifications_per_minute_by_priority); certificate.assign_if_valid(from.certificate); vehicle_make.assign_if_valid(from.vehicle_make); vehicle_model.assign_if_valid(from.vehicle_model); diff --git a/src/components/policy/policy_external/src/sql_pt_representation.cc b/src/components/policy/policy_external/src/sql_pt_representation.cc index d368707efa..a7658740c4 100644 --- a/src/components/policy/policy_external/src/sql_pt_representation.cc +++ b/src/components/policy/policy_external/src/sql_pt_representation.cc @@ -569,9 +569,9 @@ void SQLPTRepresentation::GatherModuleConfig( "Incorrect select statement for subtle notifications"); } else { while (subtle_notifications.Next()) { - config->subtle_notifications_per_minute_by_priority[subtle_notifications - .GetString(0)] = - subtle_notifications.GetInteger(1); + (*config->subtle_notifications_per_minute_by_priority) + [subtle_notifications.GetString(0)] = + subtle_notifications.GetInteger(1); } } utils::dbms::SQLQuery seconds(db()); @@ -1546,7 +1546,7 @@ bool SQLPTRepresentation::SaveModuleConfig( } if (!SaveNumberOfSubtleNotificationsPerMinute( - config.subtle_notifications_per_minute_by_priority)) { + *config.subtle_notifications_per_minute_by_priority)) { return false; } diff --git a/src/components/policy/policy_external/test/cache_manager_test.cc b/src/components/policy/policy_external/test/cache_manager_test.cc index 440df431db..746d0c7221 100644 --- a/src/components/policy/policy_external/test/cache_manager_test.cc +++ b/src/components/policy/policy_external/test/cache_manager_test.cc @@ -242,6 +242,51 @@ TEST_F(CacheManagerTest, EXPECT_EQ(0u, notif_number); } +TEST_F(CacheManagerTest, + GetNotificationsNumber_Subtle_FieldNotPresent_UseFallback) { + const std::string string_table( + "{" + "\"policy_table\": {" + "\"module_config\": {" + "\"notifications_per_minute_by_priority\": {" + "\"EMERGENCY\": 1," + "\"NAVIGATION\": 2," + "\"VOICECOM\": 3," + "\"COMMUNICATION\": 4," + "\"NORMAL\": 5," + "\"NONE\": 6" + "}" + "}" + "}" + "}"); + *pt_ = CreateCustomPT(string_table); + + std::string priority = "EMERGENCY"; + uint32_t notif_number = + cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(1u, notif_number); + + priority = "NAVIGATION"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(2u, notif_number); + + priority = "VOICECOM"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(3u, notif_number); + + priority = "COMMUNICATION"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(4u, notif_number); + + priority = "NORMAL"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(5u, notif_number); + + priority = "NONE"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(6u, notif_number); +} + TEST_F(CacheManagerTest, GetConsentsPriority_DeviceIDNotFound_ReturnkExternalConsentPrio) { const auto priority = diff --git a/src/components/policy/policy_external/test/sql_pt_representation_test.cc b/src/components/policy/policy_external/test/sql_pt_representation_test.cc index ae3000fe7c..5495ef65b0 100644 --- a/src/components/policy/policy_external/test/sql_pt_representation_test.cc +++ b/src/components/policy/policy_external/test/sql_pt_representation_test.cc @@ -1726,7 +1726,7 @@ TEST_F(SQLPTRepresentationTest, EXPECT_EQ(0u, config.seconds_between_retries.size()); EXPECT_EQ(0u, config.endpoints.size()); EXPECT_EQ(0u, config.notifications_per_minute_by_priority.size()); - EXPECT_EQ(0u, config.subtle_notifications_per_minute_by_priority.size()); + EXPECT_EQ(0u, (*config.subtle_notifications_per_minute_by_priority).size()); policy_table::ConsumerFriendlyMessages messages; GatherConsumerFriendlyMessages(&messages); @@ -1833,15 +1833,19 @@ TEST_F(SQLPTRepresentationTest, ASSERT_EQ(4, config.notifications_per_minute_by_priority["communication"]); ASSERT_EQ(5, config.notifications_per_minute_by_priority["normal"]); ASSERT_EQ(6, config.notifications_per_minute_by_priority["none"]); - ASSERT_EQ(6u, config.subtle_notifications_per_minute_by_priority.size()); - ASSERT_EQ(7, config.subtle_notifications_per_minute_by_priority["emergency"]); - ASSERT_EQ(8, - config.subtle_notifications_per_minute_by_priority["navigation"]); - ASSERT_EQ(9, config.subtle_notifications_per_minute_by_priority["VOICECOMM"]); + ASSERT_EQ(6u, (*config.subtle_notifications_per_minute_by_priority).size()); + ASSERT_EQ(7, + (*config.subtle_notifications_per_minute_by_priority)["emergency"]); ASSERT_EQ( - 10, config.subtle_notifications_per_minute_by_priority["communication"]); - ASSERT_EQ(11, config.subtle_notifications_per_minute_by_priority["normal"]); - ASSERT_EQ(12, config.subtle_notifications_per_minute_by_priority["none"]); + 8, (*config.subtle_notifications_per_minute_by_priority)["navigation"]); + ASSERT_EQ(9, + (*config.subtle_notifications_per_minute_by_priority)["VOICECOMM"]); + ASSERT_EQ( + 10, + (*config.subtle_notifications_per_minute_by_priority)["communication"]); + ASSERT_EQ(11, + (*config.subtle_notifications_per_minute_by_priority)["normal"]); + ASSERT_EQ(12, (*config.subtle_notifications_per_minute_by_priority)["none"]); EXPECT_EQ(1u, config.endpoints.size()); policy_table::ServiceEndpoints& service_endpoints = config.endpoints; EXPECT_EQ("0x00", service_endpoints.begin()->first); diff --git a/src/components/policy/policy_regular/include/policy/policy_table/types.h b/src/components/policy/policy_regular/include/policy/policy_table/types.h index 8ef42c822e..38d49bcc7c 100644 --- a/src/components/policy/policy_regular/include/policy/policy_table/types.h +++ b/src/components/policy/policy_regular/include/policy/policy_table/types.h @@ -323,7 +323,8 @@ struct ModuleConfig : CompositeType { ServiceEndpoints endpoints; Optional endpoint_properties; NumberOfNotificationsPerMinute notifications_per_minute_by_priority; - NumberOfNotificationsPerMinute subtle_notifications_per_minute_by_priority; + Optional + subtle_notifications_per_minute_by_priority; Optional > vehicle_make; Optional > vehicle_model; Optional > vehicle_year; @@ -343,9 +344,7 @@ struct ModuleConfig : CompositeType { const ServiceEndpoints& endpoints, const ServiceEndpointProperties& endpoint_properties, const NumberOfNotificationsPerMinute& - notifications_per_minute_by_priority, - const NumberOfNotificationsPerMinute& - subtle_notifications_per_minute_by_priority); + notifications_per_minute_by_priority); ~ModuleConfig(); explicit ModuleConfig(const Json::Value* value__); void SafeCopyFrom(const ModuleConfig& from); diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc index 97e0aa3b4d..ed2bbefcd7 100644 --- a/src/components/policy/policy_regular/src/cache_manager.cc +++ b/src/components/policy/policy_regular/src/cache_manager.cc @@ -1105,11 +1105,12 @@ CacheManager::GetNotificationsNumber(const std::string& priority, CACHE_MANAGER_CHECK(0); sync_primitives::AutoLock auto_lock(cache_lock_); - const auto& nnpm = is_subtle - ? pt_->policy_table.module_config - .subtle_notifications_per_minute_by_priority - : pt_->policy_table.module_config - .notifications_per_minute_by_priority; + const auto& module_config = pt_->policy_table.module_config; + const auto& nnpm = + is_subtle && module_config.subtle_notifications_per_minute_by_priority + .is_initialized() + ? *module_config.subtle_notifications_per_minute_by_priority + : module_config.notifications_per_minute_by_priority; auto priority_iter = nnpm.find(priority); 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 8f3f32e9cb..174d0d5d12 100644 --- a/src/components/policy/policy_regular/src/policy_manager_impl.cc +++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc @@ -295,7 +295,7 @@ void FilterPolicyTable( module_config.subtle_notifications_per_minute_by_priority .is_initialized()) { FilterInvalidPriorityValues( - module_config.subtle_notifications_per_minute_by_priority); + *module_config.subtle_notifications_per_minute_by_priority); } if (pt.app_policies_section.is_initialized()) { diff --git a/src/components/policy/policy_regular/src/policy_table/types.cc b/src/components/policy/policy_regular/src/policy_table/types.cc index df59afe3d8..017a7e75cb 100644 --- a/src/components/policy/policy_regular/src/policy_table/types.cc +++ b/src/components/policy/policy_regular/src/policy_table/types.cc @@ -720,9 +720,7 @@ ModuleConfig::ModuleConfig( const SecondsBetweenRetries& seconds_between_retries, const ServiceEndpoints& endpoints, const ServiceEndpointProperties& endpoint_properties, - const NumberOfNotificationsPerMinute& notifications_per_minute_by_priority, - const NumberOfNotificationsPerMinute& - subtle_notifications_per_minute_by_priority) + const NumberOfNotificationsPerMinute& notifications_per_minute_by_priority) : CompositeType(kUninitialized) , exchange_after_x_ignition_cycles(exchange_after_x_ignition_cycles) , exchange_after_x_kilometers(exchange_after_x_kilometers) @@ -731,9 +729,8 @@ ModuleConfig::ModuleConfig( , seconds_between_retries(seconds_between_retries) , endpoints(endpoints) , endpoint_properties(endpoint_properties) - , notifications_per_minute_by_priority(notifications_per_minute_by_priority) - , subtle_notifications_per_minute_by_priority( - subtle_notifications_per_minute_by_priority) {} + , notifications_per_minute_by_priority( + notifications_per_minute_by_priority) {} ModuleConfig::~ModuleConfig() {} @@ -777,11 +774,11 @@ void ModuleConfig::SafeCopyFrom(const ModuleConfig& from) { endpoint_properties = from.endpoint_properties; notifications_per_minute_by_priority = from.notifications_per_minute_by_priority; - subtle_notifications_per_minute_by_priority = - from.subtle_notifications_per_minute_by_priority; lock_screen_dismissal_enabled = from.lock_screen_dismissal_enabled; + subtle_notifications_per_minute_by_priority.assign_if_valid( + from.subtle_notifications_per_minute_by_priority); vehicle_make.assign_if_valid(from.vehicle_make); vehicle_model.assign_if_valid(from.vehicle_model); vehicle_year.assign_if_valid(from.vehicle_year); diff --git a/src/components/policy/policy_regular/src/sql_pt_representation.cc b/src/components/policy/policy_regular/src/sql_pt_representation.cc index 8ad278ec72..043b413c13 100644 --- a/src/components/policy/policy_regular/src/sql_pt_representation.cc +++ b/src/components/policy/policy_regular/src/sql_pt_representation.cc @@ -557,9 +557,9 @@ void SQLPTRepresentation::GatherModuleConfig( "Incorrect select statement for subtle notifications"); } else { while (subtle_notifications.Next()) { - config->subtle_notifications_per_minute_by_priority[subtle_notifications - .GetString(0)] = - subtle_notifications.GetInteger(1); + (*config->subtle_notifications_per_minute_by_priority) + [subtle_notifications.GetString(0)] = + subtle_notifications.GetInteger(1); } } utils::dbms::SQLQuery seconds(db()); @@ -1492,7 +1492,7 @@ bool SQLPTRepresentation::SaveModuleConfig( } if (!SaveNumberOfSubtleNotificationsPerMinute( - config.subtle_notifications_per_minute_by_priority)) { + *config.subtle_notifications_per_minute_by_priority)) { return false; } diff --git a/src/components/policy/policy_regular/test/cache_manager_test.cc b/src/components/policy/policy_regular/test/cache_manager_test.cc index 5666de03b0..31166b8f43 100644 --- a/src/components/policy/policy_regular/test/cache_manager_test.cc +++ b/src/components/policy/policy_regular/test/cache_manager_test.cc @@ -242,6 +242,51 @@ TEST_F(CacheManagerTest, EXPECT_EQ(0u, notif_number); } +TEST_F(CacheManagerTest, + GetNotificationsNumber_Subtle_FieldNotPresent_UseFallback) { + const std::string string_table( + "{" + "\"policy_table\": {" + "\"module_config\": {" + "\"notifications_per_minute_by_priority\": {" + "\"EMERGENCY\": 1," + "\"NAVIGATION\": 2," + "\"VOICECOM\": 3," + "\"COMMUNICATION\": 4," + "\"NORMAL\": 5," + "\"NONE\": 6" + "}" + "}" + "}" + "}"); + *pt_ = CreateCustomPT(string_table); + + std::string priority = "EMERGENCY"; + uint32_t notif_number = + cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(1u, notif_number); + + priority = "NAVIGATION"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(2u, notif_number); + + priority = "VOICECOM"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(3u, notif_number); + + priority = "COMMUNICATION"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(4u, notif_number); + + priority = "NORMAL"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(5u, notif_number); + + priority = "NONE"; + notif_number = cache_manager_->GetNotificationsNumber(priority, true); + EXPECT_EQ(6u, notif_number); +} + TEST_F(CacheManagerTest, HeartBeatTimeout_ValueInitialized_ReturnValue) { *pt_->policy_table.app_policies_section.apps[kValidAppId] .heart_beat_timeout_ms = 100u; diff --git a/src/components/policy/policy_regular/test/sql_pt_representation_test.cc b/src/components/policy/policy_regular/test/sql_pt_representation_test.cc index 8da9158999..880e9ca4f9 100644 --- a/src/components/policy/policy_regular/test/sql_pt_representation_test.cc +++ b/src/components/policy/policy_regular/test/sql_pt_representation_test.cc @@ -1893,7 +1893,7 @@ TEST_F(SQLPTRepresentationTest, Save_SetPolicyTableThenSave_ExpectSavedToPT) { EXPECT_EQ(0u, config.seconds_between_retries.size()); EXPECT_EQ(0u, config.endpoints.size()); EXPECT_EQ(0u, config.notifications_per_minute_by_priority.size()); - EXPECT_EQ(0u, config.subtle_notifications_per_minute_by_priority.size()); + EXPECT_EQ(0u, (*config.subtle_notifications_per_minute_by_priority).size()); policy_table::ConsumerFriendlyMessages messages; GatherConsumerFriendlyMessages(&messages); @@ -2001,15 +2001,19 @@ TEST_F(SQLPTRepresentationTest, Save_SetPolicyTableThenSave_ExpectSavedToPT) { ASSERT_EQ(4, config.notifications_per_minute_by_priority["communication"]); ASSERT_EQ(5, config.notifications_per_minute_by_priority["normal"]); ASSERT_EQ(6, config.notifications_per_minute_by_priority["none"]); - ASSERT_EQ(6u, config.subtle_notifications_per_minute_by_priority.size()); - ASSERT_EQ(7, config.subtle_notifications_per_minute_by_priority["emergency"]); - ASSERT_EQ(8, - config.subtle_notifications_per_minute_by_priority["navigation"]); - ASSERT_EQ(9, config.subtle_notifications_per_minute_by_priority["VOICECOMM"]); + ASSERT_EQ(6u, (*config.subtle_notifications_per_minute_by_priority).size()); + ASSERT_EQ(7, + (*config.subtle_notifications_per_minute_by_priority)["emergency"]); ASSERT_EQ( - 10, config.subtle_notifications_per_minute_by_priority["communication"]); - ASSERT_EQ(11, config.subtle_notifications_per_minute_by_priority["normal"]); - ASSERT_EQ(12, config.subtle_notifications_per_minute_by_priority["none"]); + 8, (*config.subtle_notifications_per_minute_by_priority)["navigation"]); + ASSERT_EQ(9, + (*config.subtle_notifications_per_minute_by_priority)["VOICECOMM"]); + ASSERT_EQ( + 10, + (*config.subtle_notifications_per_minute_by_priority)["communication"]); + ASSERT_EQ(11, + (*config.subtle_notifications_per_minute_by_priority)["normal"]); + ASSERT_EQ(12, (*config.subtle_notifications_per_minute_by_priority)["none"]); EXPECT_EQ(1u, config.endpoints.size()); policy_table::ServiceEndpoints& service_endpoints = config.endpoints; EXPECT_EQ("0x00", service_endpoints.begin()->first); -- cgit v1.2.1