Add check for already processed resumptions (#3525)

3 files changed, 61 insertions, 142 deletions

diff --git a/src/components/application_manager/include/application_manager/resumption/resumption_data_processor_impl.h b/src/components/application_manager/include/application_manager/resumption/resumption_data_processor_impl.h
index 12d25e2ac5..c2040852ca 100644
--- a/src/components/application_manager/include/application_manager/resumption/resumption_data_processor_impl.h
+++ b/src/components/application_manager/include/application_manager/resumption/resumption_data_processor_impl.h
@@ -125,14 +125,22 @@ class ResumptionDataProcessorImpl
                                const ResumptionRequest& found_request);
 
   /**
-   * @brief IsResumptionFinished checks whether some responses are still waiting
+   * @brief EraseProcessedRequest erases processed request from list of pending
+   * requests
    * @param app_id ID of application, related to event
    * @param found_request reference to found request
+   */
+  void EraseProcessedRequest(const uint32_t app_id,
+                             const ResumptionRequest& found_request);
+
+  /**
+   * @brief IsResumptionFinished checks whether some responses are still
+   * waiting
+   * @param app_id ID of application, related to event
    * @return true, if resumption for this application is finished, or false, if
-   * some requests aren't processed  yet
+   * some requests aren't processed yet
    */
-  bool IsResumptionFinished(const uint32_t app_id,
-                            const ResumptionRequest& found_request);
+  bool IsResumptionFinished(const uint32_t app_id) const;
 
   /**
    * @brief IsResumptionSuccessful checks whether overall resumption status
@@ -156,12 +164,8 @@ class ResumptionDataProcessorImpl
    * @brief EraseAppResumptionData erases data, needed for resumption, for
    * given application
    * @param app_id ID of application, related to event
-   * @param function_id Function ID
-   * @param corr_id Correlation ID
    */
-  void EraseAppResumptionData(const uint32_t app_id,
-                              const hmi_apis::FunctionID::eType function_id,
-                              const int32_t corr_id);
+  void EraseAppResumptionData(const uint32_t app_id);
 
   /**
    * @brief Processes response message from HMI
@@ -174,6 +178,14 @@ class ResumptionDataProcessorImpl
                               const int32_t corr_id);
 
   /**
+   * @brief Checks whether resumption is successful and finalizes resumption
+   * corresponding to result of this check
+   * @param callback Function to be called when data resumption will be finished
+   * @param app_id ID of application, related to event
+   */
+  void FinalizeResumption(const ResumeCtrl::ResumptionCallBack& callback,
+                          const uint32_t app_id);
+  /**
    * @brief Revert the data to the state before Resumption
    * @param shared ptr to application
    */
@@ -355,34 +367,25 @@ class ResumptionDataProcessorImpl
       const smart_objects::SmartObject& request,
       const smart_objects::SmartObject& response) const;
 
-  /**
-   * @brief Determines whether application has saved data, including
-   * submenus, commands, choice sets, global properties, subscriptions to
-   * restore.
-   * @param saved_app smart object containing saved app data
-   * @return bool value stating whether app has mentioned data to restore
-   */
-  bool HasDataToRestore(const smart_objects::SmartObject& saved_app) const;
-
   app_mngr::ApplicationManager& application_manager_;
 
   /**
    * @brief A map of the IDs and Application Resumption Status for these ID
    **/
   std::map<std::int32_t, ApplicationResumptionStatus> resumption_status_;
-  sync_primitives::RWLock resumption_status_lock_;
+  mutable sync_primitives::RWLock resumption_status_lock_;
 
   /**
    * @brief A map of callbacks used when resumption is finished
    */
   std::map<std::int32_t, ResumeCtrl::ResumptionCallBack> register_callbacks_;
-  sync_primitives::RWLock register_callbacks_lock_;
+  mutable sync_primitives::RWLock register_callbacks_lock_;
 
   /**
    * @brief A map of sent requests and corresponding app_id
    */
   std::map<ResumptionRequestID, std::uint32_t> request_app_ids_;
-  sync_primitives::RWLock request_app_ids_lock_;
+  mutable sync_primitives::RWLock request_app_ids_lock_;
 };
 
 }  // namespace resumption
diff --git a/src/components/application_manager/src/resumption/resumption_data_processor_impl.cc b/src/components/application_manager/src/resumption/resumption_data_processor_impl.cc
index d3ac9c24e1..792e7f97ea 100644
--- a/src/components/application_manager/src/resumption/resumption_data_processor_impl.cc
+++ b/src/components/application_manager/src/resumption/resumption_data_processor_impl.cc
@@ -71,12 +71,6 @@ void ResumptionDataProcessorImpl::Restore(
     ResumeCtrl::ResumptionCallBack callback) {
   SDL_LOG_AUTO_TRACE();
 
-  if (!HasDataToRestore(saved_app)) {
-    SDL_LOG_DEBUG("No data to restore, resumption is successful");
-    callback(mobile_apis::Result::SUCCESS, "Data resumption succesful");
-    return;
-  }
-
   AddFiles(application, saved_app);
   AddSubmenus(application, saved_app);
   AddCommands(application, saved_app);
@@ -85,99 +79,15 @@ void ResumptionDataProcessorImpl::Restore(
   AddSubscriptions(application, saved_app);
   AddWindows(application, saved_app);
 
-  resumption_status_lock_.AcquireForReading();
   const auto app_id = application->app_id();
-  bool is_requests_list_empty = true;
-  if (resumption_status_.find(app_id) != resumption_status_.end()) {
-    is_requests_list_empty =
-        resumption_status_[app_id].list_of_sent_requests.empty();
-  }
-  resumption_status_lock_.Release();
-
-  if (!is_requests_list_empty) {
+  if (!IsResumptionFinished(app_id)) {
     sync_primitives::AutoWriteLock lock(register_callbacks_lock_);
     register_callbacks_[app_id] = callback;
   } else {
-    SDL_LOG_DEBUG("No requests to HMI for " << app_id
-                                            << " , resumption is successful");
-    callback(mobile_apis::Result::SUCCESS, "Data resumption successful");
+    FinalizeResumption(callback, app_id);
   }
 }
 
-bool ResumptionDataProcessorImpl::HasDataToRestore(
-    const smart_objects::SmartObject& saved_app) const {
-  SDL_LOG_AUTO_TRACE();
-
-  auto has_data_to_restore = [&saved_app]() -> bool {
-    return !saved_app[strings::application_files].empty() ||
-           !saved_app[strings::application_submenus].empty() ||
-           !saved_app[strings::application_commands].empty() ||
-           !saved_app[strings::application_choice_sets].empty() ||
-           !saved_app[strings::windows_info].empty();
-  };
-
-  auto has_gp_to_restore = [&saved_app]() -> bool {
-    const smart_objects::SmartObject& global_properties =
-        saved_app[strings::application_global_properties];
-
-    return !global_properties[strings::help_prompt].empty() ||
-           !global_properties[strings::keyboard_properties].empty() ||
-           !global_properties[strings::menu_icon].empty() ||
-           !global_properties[strings::menu_title].empty() ||
-           !global_properties[strings::timeout_prompt].empty() ||
-           !global_properties[strings::vr_help].empty() ||
-           !global_properties[strings::vr_help_title].empty();
-  };
-
-  auto has_subscriptions_to_restore = [&saved_app]() -> bool {
-    const smart_objects::SmartObject& subscriptions =
-        saved_app[strings::application_subscriptions];
-
-    const bool has_ivi_subscriptions =
-        !subscriptions[strings::application_vehicle_info].empty();
-
-    const bool has_button_subscriptions =
-        !subscriptions[strings::application_buttons].empty() &&
-        !(subscriptions[strings::application_buttons].length() == 1 &&
-          static_cast<hmi_apis::Common_ButtonName::eType>(
-              subscriptions[strings::application_buttons][0].asInt()) ==
-              hmi_apis::Common_ButtonName::CUSTOM_BUTTON);
-
-    const bool has_waypoints_subscriptions =
-        subscriptions[strings::subscribed_for_way_points].asBool();
-
-    const bool has_appservice_subscriptions =
-        subscriptions.keyExists(app_mngr::hmi_interface::app_service) &&
-        !subscriptions[app_mngr::hmi_interface::app_service].empty();
-
-    const bool has_system_capability_subscriptions =
-        subscriptions.keyExists(strings::system_capability) &&
-        !subscriptions[strings::system_capability].empty();
-
-    return has_ivi_subscriptions || has_button_subscriptions ||
-           has_waypoints_subscriptions || has_appservice_subscriptions ||
-           has_system_capability_subscriptions;
-  };
-
-  if (has_data_to_restore()) {
-    SDL_LOG_DEBUG("Application has data to restore");
-    return true;
-  }
-
-  if (has_gp_to_restore()) {
-    SDL_LOG_DEBUG("Application has global properties to restore");
-    return true;
-  }
-
-  if (has_subscriptions_to_restore()) {
-    SDL_LOG_DEBUG("Application has subscriptions to restore");
-    return true;
-  }
-
-  SDL_LOG_DEBUG("Application does not have any data to restore");
-  return false;
-}
-
 utils::Optional<uint32_t>
 ResumptionDataProcessorImpl::GetAppIdWaitingForResponse(
     const hmi_apis::FunctionID::eType function_id, const int32_t corr_id) {
@@ -260,7 +170,7 @@ void ResumptionDataProcessorImpl::ProcessResumptionStatus(
   }
 }
 
-bool ResumptionDataProcessorImpl::IsResumptionFinished(
+void ResumptionDataProcessorImpl::EraseProcessedRequest(
     const uint32_t app_id, const ResumptionRequest& found_request) {
   SDL_LOG_AUTO_TRACE();
 
@@ -277,8 +187,19 @@ bool ResumptionDataProcessorImpl::IsResumptionFinished(
                                 found_request.request_id.function_id;
                    });
   list_of_sent_requests.erase(request_iter);
+}
+
+bool ResumptionDataProcessorImpl::IsResumptionFinished(
+    const uint32_t app_id) const {
+  SDL_LOG_AUTO_TRACE();
 
-  return list_of_sent_requests.empty();
+  sync_primitives::AutoReadLock lock(resumption_status_lock_);
+  bool is_requests_list_empty = true;
+  const auto app_status = resumption_status_.find(app_id);
+  if (app_status != resumption_status_.end()) {
+    is_requests_list_empty = app_status->second.list_of_sent_requests.empty();
+  }
+  return is_requests_list_empty;
 }
 
 utils::Optional<ResumeCtrl::ResumptionCallBack>
@@ -299,7 +220,7 @@ bool ResumptionDataProcessorImpl::IsResumptionSuccessful(
   sync_primitives::AutoReadLock lock(resumption_status_lock_);
   auto it = resumption_status_.find(app_id);
   if (resumption_status_.end() == it) {
-    return false;
+    return true;
   }
 
   const ApplicationResumptionStatus& status = it->second;
@@ -309,17 +230,27 @@ bool ResumptionDataProcessorImpl::IsResumptionSuccessful(
 }
 
 void ResumptionDataProcessorImpl::EraseAppResumptionData(
-    const uint32_t app_id,
-    const hmi_apis::FunctionID::eType function_id,
-    const int32_t corr_id) {
+    const uint32_t app_id) {
   SDL_LOG_AUTO_TRACE();
 
+  std::vector<ResumptionRequest> all_requests;
+
   resumption_status_lock_.AcquireForWriting();
+  all_requests.insert(all_requests.end(),
+                      resumption_status_[app_id].successful_requests.begin(),
+                      resumption_status_[app_id].successful_requests.end());
+  all_requests.insert(all_requests.end(),
+                      resumption_status_[app_id].error_requests.begin(),
+                      resumption_status_[app_id].error_requests.end());
+
   resumption_status_.erase(app_id);
   resumption_status_lock_.Release();
 
   request_app_ids_lock_.AcquireForWriting();
-  request_app_ids_.erase({function_id, corr_id});
+  for (auto request : all_requests) {
+    request_app_ids_.erase(
+        {request.request_id.function_id, request.request_id.correlation_id});
+  }
   request_app_ids_lock_.Release();
 
   register_callbacks_lock_.AcquireForWriting();
@@ -354,8 +285,9 @@ void ResumptionDataProcessorImpl::ProcessResponseFromHMI(
   auto request = *found_request;
 
   ProcessResumptionStatus(app_id, response, request);
+  EraseProcessedRequest(app_id, request);
 
-  if (!IsResumptionFinished(app_id, request)) {
+  if (!IsResumptionFinished(app_id)) {
     SDL_LOG_DEBUG("Resumption app "
                   << app_id << " not finished. Some requests are still waited");
     return;
@@ -367,7 +299,11 @@ void ResumptionDataProcessorImpl::ProcessResponseFromHMI(
     return;
   }
   auto callback = *found_callback;
+  FinalizeResumption(callback, app_id);
+}
 
+void ResumptionDataProcessorImpl::FinalizeResumption(
+    const ResumeCtrl::ResumptionCallBack& callback, const uint32_t app_id) {
   if (IsResumptionSuccessful(app_id)) {
     SDL_LOG_DEBUG("Resumption for app " << app_id << " successful");
     callback(mobile_apis::Result::SUCCESS, "Data resumption successful");
@@ -378,8 +314,7 @@ void ResumptionDataProcessorImpl::ProcessResponseFromHMI(
     RevertRestoredData(application_manager_.application(app_id));
     application_manager_.state_controller().DropPostponedWindows(app_id);
   }
-
-  EraseAppResumptionData(app_id, function_id, corr_id);
+  EraseAppResumptionData(app_id);
 }
 
 void ResumptionDataProcessorImpl::HandleOnTimeOut(
diff --git a/src/components/application_manager/test/resumption/resume_ctrl_test.cc b/src/components/application_manager/test/resumption/resume_ctrl_test.cc
index ded6aa956e..3953e7e2dc 100644
--- a/src/components/application_manager/test/resumption/resume_ctrl_test.cc
+++ b/src/components/application_manager/test/resumption/resume_ctrl_test.cc
@@ -251,25 +251,6 @@ class ResumeCtrlTest : public ::testing::Test {
  * @brief  Group of tests which check starting resumption with different data
  */
 
-TEST_F(ResumeCtrlTest, StartResumption_AppWithGrammarId) {
-  smart_objects::SmartObject saved_app;
-  saved_app[application_manager::strings::hash_id] = kHash_;
-  saved_app[application_manager::strings::grammar_id] = kTestGrammarId_;
-
-  // Check RestoreApplicationData
-  GetInfoFromApp();
-  ON_CALL(mock_app_mngr_, GetDefaultHmiLevel(const_app_))
-      .WillByDefault(Return(kDefaultTestLevel_));
-  ON_CALL(*mock_storage_,
-          GetSavedApplication(kTestPolicyAppId_, kMacAddress_, _))
-      .WillByDefault(DoAll(SetArgReferee<2>(saved_app), Return(true)));
-
-  EXPECT_CALL(*mock_app_, set_grammar_id(kTestGrammarId_));
-
-  const bool res = res_ctrl_->StartResumption(mock_app_, kHash_, callback_);
-  EXPECT_TRUE(res);
-}
-
 MATCHER_P4(CheckAppFile, is_persistent, is_download, file_name, file_type, "") {
   application_manager::AppFile app_file = arg;
   return app_file.is_persistent == is_persistent &&