fixup! Fix redundant OnPermissionChanged notificationfix/SDL_sends_OnPermissionsChange_twice

4 files changed, 46 insertions, 29 deletions

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 c6066a9ae9..0bc311ba9f 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
@@ -136,16 +136,20 @@ class PolicyManagerImpl : public PolicyManager {
 
   /**
    * @brief Notifies system by sending OnAppPermissionChanged notification
+   * @param device_id device identifier
    * @param app_policy Reference to application policy
    */
-  void NotifySystem(const AppPoliciesValueType& app_policy) const;
+  void NotifySystem(const std::string& device_id,
+                    const AppPoliciesValueType& app_policy) const;
 
   /**
    * @brief Sends OnPermissionChange notification to application if its
    * currently registered
+   * @param device_id device identifier
    * @param app_policy Reference to application policy
    */
-  void SendPermissionsToApp(const AppPoliciesValueType& app_policy);
+  void SendPermissionsToApp(const std::string& device_id,
+                            const AppPoliciesValueType& app_policy);
 
   /**
    * @brief Resets Policy Table
diff --git a/src/components/policy/policy_regular/src/policy_helper.cc b/src/components/policy/policy_regular/src/policy_helper.cc
index d9c329d5f6..c6116584f8 100644
--- a/src/components/policy/policy_regular/src/policy_helper.cc
+++ b/src/components/policy/policy_regular/src/policy_helper.cc
@@ -453,11 +453,13 @@ bool CheckAppPolicy::operator()(const AppPoliciesValueType& app_policy) {
     return true;
   }
 
+  SetPendingPermissions(app_policy, result);
   if (RESULT_CONSENT_NOT_REQUIRED != result) {
-    SetPendingPermissions(app_policy, result);
     AddResult(app_id, RESULT_CONSENT_NEEDED);
+    return true;
   }
 
+  AddResult(app_id, result);
   return true;
 }
 
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 9f20f23c45..0e1cfd267a 100644
--- a/src/components/policy/policy_regular/src/policy_manager_impl.cc
+++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc
@@ -455,40 +455,45 @@ void PolicyManagerImpl::ProcessActionsForAppPolicies(
       continue;
     }
 
-    if (it_actions->second.is_consent_needed) {
-      // Post-check after ExternalConsent consent changes
-      const std::string& policy_app_id = app_policy->first;
-      if (!IsConsentNeeded(last_device_id_, policy_app_id)) {
-        sync_primitives::AutoLock lock(app_permissions_diff_lock_);
-
-        PendingPermissions::iterator app_id_diff =
-            app_permissions_diff_.find(policy_app_id);
-
-        if (app_permissions_diff_.end() != app_id_diff) {
-          app_id_diff->second.appPermissionsConsentNeeded = false;
+    const auto devices_ids = listener()->GetDevicesIds(app_policy->first);
+    for (const auto& device_id : devices_ids) {
+      if (it_actions->second.is_consent_needed) {
+        // Post-check after ExternalConsent consent changes
+        const std::string& policy_app_id = app_policy->first;
+        if (!IsConsentNeeded(device_id, policy_app_id)) {
+          sync_primitives::AutoLock lock(app_permissions_diff_lock_);
+
+          PendingPermissions::iterator app_id_diff =
+              app_permissions_diff_.find(policy_app_id);
+
+          if (app_permissions_diff_.end() != app_id_diff) {
+            app_id_diff->second.appPermissionsConsentNeeded = false;
+          }
         }
       }
-    }
-    if (it_actions->second.is_notify_system) {
-      NotifySystem(*app_policy);
-    }
-    if (it_actions->second.is_send_permissions_to_app) {
-      SendPermissionsToApp(*app_policy);
+      if (it_actions->second.is_notify_system) {
+        NotifySystem(device_id, *app_policy);
+      }
+      if (it_actions->second.is_send_permissions_to_app) {
+        SendPermissionsToApp(device_id, *app_policy);
+      }
     }
   }
 }
 
 void PolicyManagerImpl::NotifySystem(
+    const std::string& device_id,
     const PolicyManagerImpl::AppPoliciesValueType& app_policy) const {
-  listener()->OnPendingPermissionChange(last_device_id_, app_policy.first);
+  listener()->OnPendingPermissionChange(device_id, app_policy.first);
 }
 
 void PolicyManagerImpl::SendPermissionsToApp(
+    const std::string& device_id,
     const PolicyManagerImpl::AppPoliciesValueType& app_policy) {
   const std::string app_id = app_policy.first;
 
   std::vector<FunctionalGroupPermission> group_permissons;
-  GetPermissionsForApp(last_device_id_, app_id, group_permissons);
+  GetPermissionsForApp(device_id, app_id, group_permissons);
 
   Permissions notification_data;
 
@@ -504,7 +509,7 @@ void PolicyManagerImpl::SendPermissionsToApp(
   std::string default_hmi;
   default_hmi = "NONE";
   listener()->OnPermissionsUpdated(
-      last_device_id_, app_id, notification_data, default_hmi);
+      device_id, app_id, notification_data, default_hmi);
 }
 
 CheckAppPolicyResults PolicyManagerImpl::CheckPermissionsChanges(
diff --git a/src/components/policy/policy_regular/test/policy_manager_impl_test.cc b/src/components/policy/policy_regular/test/policy_manager_impl_test.cc
index ce0d3f5941..273656565e 100644
--- a/src/components/policy/policy_regular/test/policy_manager_impl_test.cc
+++ b/src/components/policy/policy_regular/test/policy_manager_impl_test.cc
@@ -1013,7 +1013,8 @@ TEST_F(PolicyManagerImplTest, LoadPT_FunctionalGroup_removeRPC_SendUpdate) {
   // Assert
   EXPECT_CALL(*cache_manager, GetVehicleDataItems())
       .WillOnce(Return(std::vector<policy_table::VehicleDataItem>()));
-  EXPECT_CALL(*cache_manager, GenerateSnapshot()).WillOnce(Return(snapshot));
+  EXPECT_CALL(*cache_manager, GenerateSnapshot())
+      .WillRepeatedly(Return(snapshot));
   EXPECT_CALL(*cache_manager, ApplyUpdate(_)).WillOnce(Return(true));
   ExpectOnPermissionsUpdated();
 
@@ -1045,7 +1046,8 @@ TEST_F(PolicyManagerImplTest,
   ::policy::BinaryMessage msg(json.begin(), json.end());
 
   // Assert
-  EXPECT_CALL(*cache_manager, GenerateSnapshot()).WillOnce(Return(snapshot));
+  EXPECT_CALL(*cache_manager, GenerateSnapshot())
+      .WillRepeatedly(Return(snapshot));
   EXPECT_CALL(*cache_manager, GetVehicleDataItems())
       .WillOnce(Return(std::vector<policy_table::VehicleDataItem>()));
   EXPECT_CALL(*cache_manager, ApplyUpdate(_)).WillOnce(Return(true));
@@ -1079,7 +1081,8 @@ TEST_F(PolicyManagerImplTest,
   ::policy::BinaryMessage msg(json.begin(), json.end());
 
   // Assert
-  EXPECT_CALL(*cache_manager, GenerateSnapshot()).WillOnce(Return(snapshot));
+  EXPECT_CALL(*cache_manager, GenerateSnapshot())
+      .WillRepeatedly(Return(snapshot));
   EXPECT_CALL(*cache_manager, GetVehicleDataItems())
       .WillOnce(Return(std::vector<policy_table::VehicleDataItem>()));
   EXPECT_CALL(*cache_manager, ApplyUpdate(_)).WillOnce(Return(true));
@@ -1114,7 +1117,8 @@ TEST_F(PolicyManagerImplTest,
   ::policy::BinaryMessage msg(json.begin(), json.end());
 
   // Assert
-  EXPECT_CALL(*cache_manager, GenerateSnapshot()).WillOnce(Return(snapshot));
+  EXPECT_CALL(*cache_manager, GenerateSnapshot())
+      .WillRepeatedly(Return(snapshot));
   EXPECT_CALL(*cache_manager, GetVehicleDataItems())
       .WillOnce(Return(std::vector<policy_table::VehicleDataItem>()));
   EXPECT_CALL(*cache_manager, ApplyUpdate(_)).WillOnce(Return(true));
@@ -1148,7 +1152,8 @@ TEST_F(PolicyManagerImplTest, LoadPT_FunctionalGroup_addRPCParams_SendUpdate) {
   ::policy::BinaryMessage msg(json.begin(), json.end());
 
   // Assert
-  EXPECT_CALL(*cache_manager, GenerateSnapshot()).WillOnce(Return(snapshot));
+  EXPECT_CALL(*cache_manager, GenerateSnapshot())
+      .WillRepeatedly(Return(snapshot));
   EXPECT_CALL(*cache_manager, GetVehicleDataItems())
       .WillOnce(Return(std::vector<policy_table::VehicleDataItem>()));
   EXPECT_CALL(*cache_manager, ApplyUpdate(_)).WillOnce(Return(true));
@@ -1175,7 +1180,8 @@ TEST_F(PolicyManagerImplTest, LoadPT_FunctionalGroup_NoUpdate_DONT_SendUpdate) {
   ::policy::BinaryMessage msg(json.begin(), json.end());
 
   // Assert
-  EXPECT_CALL(*cache_manager, GenerateSnapshot()).WillOnce(Return(snapshot));
+  EXPECT_CALL(*cache_manager, GenerateSnapshot())
+      .WillRepeatedly(Return(snapshot));
   EXPECT_CALL(*cache_manager, GetVehicleDataItems())
       .WillOnce(Return(std::vector<policy_table::VehicleDataItem>()));
   EXPECT_CALL(*cache_manager, ApplyUpdate(_)).WillOnce(Return(true));