Skip to content

Commit

Permalink
Make subtle_notifications_per_minute_by_priority non-mandatory (#3477)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@jacobkeeler
jacobkeeler authored Aug 19, 2020
1 parent 4c9f873 commit 65f45c3
Show file tree
Hide file tree
Showing 15 changed files with 156 additions and 63 deletions.
7 changes: 3 additions & 4 deletions src/components/policy/policy_external/include/policy/policy_table/types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,8 @@ struct ModuleConfig : CompositeType {
ServiceEndpoints endpoints;
Optional<ServiceEndpointProperties> endpoint_properties;
NumberOfNotificationsPerMinute notifications_per_minute_by_priority;
NumberOfNotificationsPerMinute subtle_notifications_per_minute_by_priority;
Optional<NumberOfNotificationsPerMinute>
subtle_notifications_per_minute_by_priority;
Optional<String<0, 100> > vehicle_make;
Optional<String<0, 100> > vehicle_model;
Optional<String<0, 4> > vehicle_year;
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/components/policy/policy_external/include/policy/policy_table_interface_ext.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@
<param name="seconds_between_retries" type="SecondsBetweenRetries" />
<param name="endpoints" type="ServiceEndpoints" />
<param name="notifications_per_minute_by_priority" type="NumberOfNotificationsPerMinute" />
<param name="subtle_notifications_per_minute_by_priority" type="NumberOfNotificationsPerMinute" />
<param name="subtle_notifications_per_minute_by_priority" type="NumberOfNotificationsPerMinute"
mandatory="false" />
<param name="vehicle_make" type="String" maxlength="100"
mandatory="false" />
<param name="vehicle_model" type="String" maxlength="100"
Expand Down
11 changes: 6 additions & 5 deletions src/components/policy/policy_external/src/cache_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1802,11 +1802,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);

Expand Down
2 changes: 1 addition & 1 deletion src/components/policy/policy_external/src/policy_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
13 changes: 5 additions & 8 deletions src/components/policy/policy_external/src/policy_table/types.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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() {}

Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/components/policy/policy_external/src/sql_pt_representation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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;
}

Expand Down
45 changes: 45 additions & 0 deletions src/components/policy/policy_external/test/cache_manager_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
22 changes: 13 additions & 9 deletions src/components/policy/policy_external/test/sql_pt_representation_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 3 additions & 4 deletions src/components/policy/policy_regular/include/policy/policy_table/types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@ struct ModuleConfig : CompositeType {
ServiceEndpoints endpoints;
Optional<ServiceEndpointProperties> endpoint_properties;
NumberOfNotificationsPerMinute notifications_per_minute_by_priority;
NumberOfNotificationsPerMinute subtle_notifications_per_minute_by_priority;
Optional<NumberOfNotificationsPerMinute>
subtle_notifications_per_minute_by_priority;
Optional<String<1, 100> > vehicle_make;
Optional<String<1, 100> > vehicle_model;
Optional<String<4, 4> > vehicle_year;
Expand All @@ -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);
Expand Down
11 changes: 6 additions & 5 deletions src/components/policy/policy_regular/src/cache_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/components/policy/policy_regular/src/policy_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
13 changes: 5 additions & 8 deletions src/components/policy/policy_regular/src/policy_table/types.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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() {}

Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/components/policy/policy_regular/src/sql_pt_representation.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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;
}

Expand Down
45 changes: 45 additions & 0 deletions src/components/policy/policy_regular/test/cache_manager_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 65f45c3

Please sign in to comment.