Fix PTU applying for default app policy section

There was a problem with applying changes for applications
which is registered with "default" policies and this
policy group was updated after PTU. In this case permissions
for already registered applications which is using these groups
still have permissions as before update.
Also default policy section was not updated after PTU with changes
in this section.

This issue is reproduced on EXTERNAL_PROPRIETARY flow only.

To fix this issue there was removed code in CacheManager, which
incorrectly assigns default policies to apps with "default" policies.

Also there was a redundant code because default policies is unwrapped in
PTU before its applying, so all specific application policies is already
have actual new default policy permissions. In this case it is correct to
assign to every app his own policies from PTU.

Also there was updated logic in ProcessAppPolicyCheckResults() to perform
all needed actions once per app, because its possible that results could
contain sever results for one app_id.

7 files changed, 135 insertions, 71 deletions

diff --git a/src/components/policy/policy_external/include/policy/policy_helper.h b/src/components/policy/policy_external/include/policy/policy_helper.h
index 42c1ec0b46..6945f45b45 100644
--- a/src/components/policy/policy_external/include/policy/policy_helper.h
+++ b/src/components/policy/policy_external/include/policy/policy_helper.h
@@ -199,6 +199,23 @@ struct CheckAppPolicy {
   CheckAppPolicyResults& out_results_;
 };
 
+/**
+ * @brief Helper struct for filling actions to be done for processed application
+ * using CheckAppPolicyResults data as a source
+ */
+struct FillActionsForAppPolicies {
+  FillActionsForAppPolicies(
+      ApplicationsPoliciesActions& actions,
+      const policy_table::ApplicationPolicies& app_policies)
+      : actions_(actions), app_policies_(app_policies) {}
+
+  void operator()(const policy::CheckAppPolicyResults::value_type& value);
+
+ private:
+  ApplicationsPoliciesActions& actions_;
+  const policy_table::ApplicationPolicies& app_policies_;
+};
+
 /*
  * @brief Fill permissions data with merged rpc permissions for hmi levels and
  * parameters
diff --git a/src/components/policy/policy_external/include/policy/policy_manager_impl.h b/src/components/policy/policy_external/include/policy/policy_manager_impl.h
index f8d226c86a..80ceb06e7e 100644
--- a/src/components/policy/policy_external/include/policy/policy_manager_impl.h
+++ b/src/components/policy/policy_external/include/policy/policy_manager_impl.h
@@ -719,8 +719,9 @@ class PolicyManagerImpl : public PolicyManager {
 
   /**
    * @brief Processes results from policy table update analysis done by
-   * CheckPermissionsChanges() by sending OnPermissionChange and
-   * OnAppPermissionChanged notifications
+   * CheckPermissionsChanges() by filling ApplicationsPoliciesActions struct
+   * with actions which should be done for every application and passes them to
+   * ProcessActionsForAppPolicies()
    * @param results Collection of per-application results
    * @param app_policies Reference to updated application policies section as
    * a data source for generating notifications data
@@ -730,6 +731,19 @@ class PolicyManagerImpl : public PolicyManager {
       const policy_table::ApplicationPolicies& app_policies);
 
   /**
+   * @brief Processes actions filled in ProcessAppPolicyCheckResults() for every
+   * application by sending OnPermissionChange and OnAppPermissionChanged
+   * notifications and by checking consent where it needed
+   * @param actions Reference to map with actions to be done or not for every
+   * application
+   * @param app_policies Reference to updated application policies section as
+   * a data source for generating notifications data
+   */
+  void ProcessActionsForAppPolicies(
+      const ApplicationsPoliciesActions& actions,
+      const policy_table::ApplicationPolicies& app_policies);
+
+  /**
    * @brief Fill structure to be sent with OnPermissionsChanged notification
    *
    * @param Policy table struct, which contains rpc functional groups data
diff --git a/src/components/policy/policy_external/include/policy/policy_types.h b/src/components/policy/policy_external/include/policy/policy_types.h
index 9ba1a1cfff..f57919b6f7 100644
--- a/src/components/policy/policy_external/include/policy/policy_types.h
+++ b/src/components/policy/policy_external/include/policy/policy_types.h
@@ -432,6 +432,28 @@ struct ExternalConsentStatusItemSorter {
 };
 
 /**
+ * @brief The ApplicationPolicyActions struct contains actions which should be
+ * done for some application
+ */
+struct ApplicationPolicyActions {
+  ApplicationPolicyActions()
+      : is_notify_system(false)
+      , is_send_permissions_to_app(false)
+      , is_consent_needed(false) {}
+
+  bool is_notify_system;
+  bool is_send_permissions_to_app;
+  bool is_consent_needed;
+};
+
+/**
+ * @brief ApplicationsPoliciesActions map of actions to be done for every
+ * application
+ */
+typedef std::map<std::string, ApplicationPolicyActions>
+    ApplicationsPoliciesActions;
+
+/**
  * @brief Customer connectivity settings status
  */
 typedef std::set<ExternalConsentStatusItem, ExternalConsentStatusItemSorter>
diff --git a/src/components/policy/policy_external/src/cache_manager.cc b/src/components/policy/policy_external/src/cache_manager.cc
index 22040c88b2..1c81f84a81 100644
--- a/src/components/policy/policy_external/src/cache_manager.cc
+++ b/src/components/policy/policy_external/src/cache_manager.cc
@@ -630,7 +630,7 @@ void CacheManager::ProcessUpdate(
       *(initial_policy_iter->second.RequestType);
 
   const std::string& app_id = initial_policy_iter->first;
-  RequestTypes merged_pt_request_types;
+  bool update_request_types = true;
 
   if (app_id == kDefaultId || app_id == kPreDataConsentId) {
     if (new_request_types.is_omitted()) {
@@ -638,25 +638,28 @@ void CacheManager::ProcessUpdate(
                    "Application " << app_id
                                   << " has omitted RequestTypes."
                                      " Previous values will be kept.");
-      return;
-    }
-    if (new_request_types.empty()) {
+      update_request_types = false;
+    } else if (new_request_types.empty()) {
       if (new_request_types.is_cleaned_up()) {
         LOG4CXX_INFO(logger_,
                      "Application " << app_id
                                     << " has cleaned up all values."
                                        " Previous values will be kept.");
-        return;
+        update_request_types = false;
+      } else {
+        LOG4CXX_INFO(logger_,
+                     "Application " << app_id
+                                    << " has empty RequestTypes."
+                                       " Any parameter will be allowed.");
       }
-      LOG4CXX_INFO(logger_,
-                   "Application " << app_id
-                                  << " has empty RequestTypes."
-                                     " Any parameter will be allowed.");
     }
-    merged_pt_request_types = new_request_types;
-  } else {
-    merged_pt_request_types = new_request_types;
   }
+
+  const RequestTypes merged_pt_request_types =
+      update_request_types
+          ? new_request_types
+          : *(pt_->policy_table.app_policies_section.apps[app_id].RequestType);
+
   pt_->policy_table.app_policies_section.apps[app_id] =
       initial_policy_iter->second;
   *(pt_->policy_table.app_policies_section.apps[app_id].RequestType) =
@@ -682,15 +685,6 @@ bool CacheManager::ApplyUpdate(const policy_table::Table& update_pt) {
       pt_->policy_table.app_policies_section.apps[iter->first].set_to_null();
       pt_->policy_table.app_policies_section.apps[iter->first].set_to_string(
           "");
-    } else if (policy::kDefaultId == (iter->second).get_string()) {
-      policy_table::ApplicationPolicies::const_iterator iter_default =
-          update_pt.policy_table.app_policies_section.apps.find(kDefaultId);
-      if (update_pt.policy_table.app_policies_section.apps.end() ==
-          iter_default) {
-        LOG4CXX_ERROR(logger_, "The default section was not found in PTU");
-        continue;
-      }
-      ProcessUpdate(iter_default);
     } else {
       ProcessUpdate(iter);
     }
diff --git a/src/components/policy/policy_external/src/policy_helper.cc b/src/components/policy/policy_external/src/policy_helper.cc
index 3041323489..5425777833 100644
--- a/src/components/policy/policy_external/src/policy_helper.cc
+++ b/src/components/policy/policy_external/src/policy_helper.cc
@@ -484,6 +484,41 @@ bool CheckAppPolicy::IsRequestTypeChanged(
   return diff.size();
 }
 
+void FillActionsForAppPolicies::operator()(
+    const policy::CheckAppPolicyResults::value_type& value) {
+  const std::string app_id = value.first;
+  const policy_table::ApplicationPolicies::const_iterator app_policy =
+      app_policies_.find(app_id);
+
+  if (app_policies_.end() == app_policy) {
+    return;
+  }
+
+  if (IsPredefinedApp(*app_policy)) {
+    return;
+  }
+
+  switch (value.second) {
+    case RESULT_APP_REVOKED:
+    case RESULT_NICKNAME_MISMATCH:
+      actions_[app_id].is_notify_system = true;
+      return;
+    case RESULT_CONSENT_NEEDED:
+    case RESULT_PERMISSIONS_REVOKED_AND_CONSENT_NEEDED:
+      actions_[app_id].is_consent_needed = true;
+      break;
+    case RESULT_CONSENT_NOT_REQIURED:
+    case RESULT_PERMISSIONS_REVOKED:
+    case RESULT_REQUEST_TYPE_CHANGED:
+      break;
+    case RESULT_NO_CHANGES:
+    default:
+      return;
+  }
+  actions_[app_id].is_notify_system = true;
+  actions_[app_id].is_send_permissions_to_app = true;
+}
+
 FillNotificationData::FillNotificationData(Permissions& data,
                                            GroupConsent group_state,
                                            GroupConsent undefined_group_consent,
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 8a80e7755b..d50779383a 100644
--- a/src/components/policy/policy_external/src/policy_manager_impl.cc
+++ b/src/components/policy/policy_external/src/policy_manager_impl.cc
@@ -385,53 +385,45 @@ CheckAppPolicyResults PolicyManagerImpl::CheckPermissionsChanges(
 void PolicyManagerImpl::ProcessAppPolicyCheckResults(
     const CheckAppPolicyResults& results,
     const policy_table::ApplicationPolicies& app_policies) {
-  CheckAppPolicyResults::const_iterator it_results = results.begin();
+  ApplicationsPoliciesActions actions_for_apps_policies;
+  FillActionsForAppPolicies filler(actions_for_apps_policies, app_policies);
 
-  for (; results.end() != it_results; ++it_results) {
-    const policy_table::ApplicationPolicies::const_iterator app_policy =
-        app_policies.find(it_results->first);
+  std::for_each(results.begin(), results.end(), filler);
 
+  ProcessActionsForAppPolicies(actions_for_apps_policies, app_policies);
+}
+
+void PolicyManagerImpl::ProcessActionsForAppPolicies(
+    const ApplicationsPoliciesActions& actions,
+    const policy_table::ApplicationPolicies& app_policies) {
+  ApplicationsPoliciesActions::const_iterator it_actions = actions.begin();
+  for (; it_actions != actions.end(); ++it_actions) {
+    policy_table::ApplicationPolicies::const_iterator app_policy =
+        app_policies.find(it_actions->first);
     if (app_policies.end() == app_policy) {
       continue;
     }
 
-    if (IsPredefinedApp(*app_policy)) {
-      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(policy_app_id)) {
+        sync_primitives::AutoLock lock(app_permissions_diff_lock_);
 
-    switch (it_results->second) {
-      case RESULT_NO_CHANGES:
-        continue;
-      case RESULT_APP_REVOKED:
-        NotifySystem(*app_policy);
-        continue;
-      case RESULT_NICKNAME_MISMATCH:
-        NotifySystem(*app_policy);
-        continue;
-      case RESULT_CONSENT_NEEDED:
-      case RESULT_PERMISSIONS_REVOKED_AND_CONSENT_NEEDED: {
-        // Post-check after ExternalConsent consent changes
-        const std::string policy_app_id = app_policy->first;
-        if (!IsConsentNeeded(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;
-          }
+        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;
         }
-      } break;
-      case RESULT_CONSENT_NOT_REQIURED:
-      case RESULT_PERMISSIONS_REVOKED:
-      case RESULT_REQUEST_TYPE_CHANGED:
-        break;
-      default:
-        continue;
+      }
+    }
+    if (it_actions->second.is_notify_system) {
+      NotifySystem(*app_policy);
+    }
+    if (it_actions->second.is_send_permissions_to_app) {
+      SendPermissionsToApp(*app_policy);
     }
-    NotifySystem(*app_policy);
-    SendPermissionsToApp(*app_policy);
   }
 }
 
diff --git a/src/components/policy/policy_regular/src/cache_manager.cc b/src/components/policy/policy_regular/src/cache_manager.cc
index 78674c81f3..f4fa573c1b 100644
--- a/src/components/policy/policy_regular/src/cache_manager.cc
+++ b/src/components/policy/policy_regular/src/cache_manager.cc
@@ -238,16 +238,6 @@ bool CacheManager::ApplyUpdate(const policy_table::Table& update_pt) {
       pt_->policy_table.app_policies_section.apps[iter->first].set_to_null();
       pt_->policy_table.app_policies_section.apps[iter->first].set_to_string(
           "");
-    } else if (policy::kDefaultId == (iter->second).get_string()) {
-      policy_table::ApplicationPolicies::const_iterator iter_default =
-          update_pt.policy_table.app_policies_section.apps.find(kDefaultId);
-      if (update_pt.policy_table.app_policies_section.apps.end() ==
-          iter_default) {
-        LOG4CXX_ERROR(logger_, "The default section was not found in PTU");
-        continue;
-      }
-      pt_->policy_table.app_policies_section.apps[iter->first] =
-          iter_default->second;
     } else {
       pt_->policy_table.app_policies_section.apps[iter->first] = iter->second;
     }