Merge pull request #384 from LuxoftSDL/hotfix/SDL_does_not_send_OnSystemRequest_during_retry_Policy_sequence

Add retry sequence in policy for Genevi

9 files changed, 111 insertions, 67 deletions

diff --git a/src/components/application_manager/include/application_manager/policies/policy_handler.h b/src/components/application_manager/include/application_manager/policies/policy_handler.h
index deda63c0a..74843e038 100644
--- a/src/components/application_manager/include/application_manager/policies/policy_handler.h
+++ b/src/components/application_manager/include/application_manager/policies/policy_handler.h
@@ -1,5 +1,5 @@
 /*
- Copyright (c) 2013, Ford Motor Company
+ Copyright (c) 2016, Ford Motor Company
  All rights reserved.
 
  Redistribution and use in source and binary forms, with or without
@@ -37,6 +37,7 @@
 #include <map>
 #include <set>
 #include <vector>
+#include <cstdint>
 #include "policy/policy_manager.h"
 #include "application_manager/policies/policy_event_observer.h"
 #include "application_manager/policies/delegates/statistics_delegate.h"
@@ -84,9 +85,7 @@ class PolicyHandler
   virtual void OnPermissionsUpdated(const std::string& policy_app_id,
                                     const Permissions& permissions);
 
-  virtual void OnSnapshotCreated(const BinaryMessage& pt_string,
-                                 const std::vector<int>& retry_delay_seconds,
-                                 int timeout_exchange);
+  void OnSnapshotCreated(const BinaryMessage& pt_string) OVERRIDE;
 
   bool GetPriority(const std::string& policy_app_id, std::string* priority);
   void CheckPermissions(const PTString& app_id,
@@ -107,7 +106,7 @@ class PolicyHandler
 
   std::string GetLockScreenIconUrl() const;
   void ResetRetrySequence();
-  int NextRetryTimeout();
+  uint32_t NextRetryTimeout();
   int TimeoutExchange();
   void OnExceededTimeout();
   void OnSystemReady();
@@ -244,7 +243,7 @@ class PolicyHandler
   /**
    * @brief Send request to HMI to get update on system parameters
    */
-  virtual void OnSystemInfoUpdateRequired();
+  void OnSystemInfoUpdateRequired() OVERRIDE;
 
   /**
    * @brief Sends GetVehicleData request in case when Vechicle info is ready.
@@ -283,19 +282,19 @@ class PolicyHandler
 
   custom_str::CustomString GetAppName(const std::string& policy_app_id);
 
-  virtual void OnUpdateHMIAppType(
-      std::map<std::string, StringArray> app_hmi_types);
+  void OnUpdateHMIAppType(
+      std::map<std::string, StringArray> app_hmi_types) OVERRIDE;
 
-  virtual void OnCertificateUpdated(const std::string& certificate_data);
+  void OnCertificateUpdated(const std::string& certificate_data) OVERRIDE;
 
-  virtual bool CanUpdate();
+  bool CanUpdate() OVERRIDE;
 
-  virtual void OnDeviceConsentChanged(const std::string& device_id,
-                                      bool is_allowed);
+  void OnDeviceConsentChanged(const std::string& device_id,
+                              bool is_allowed) OVERRIDE;
 
   virtual void OnPTExchangeNeeded();
 
-  virtual void GetAvailableApps(std::queue<std::string>& apps);
+  void GetAvailableApps(std::queue<std::string>& apps) OVERRIDE;
 
   /**
    * @brief Allows to add new or update existed application during
diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc
index 26f984a99..6d39b5d9f 100644
--- a/src/components/application_manager/src/policies/policy_handler.cc
+++ b/src/components/application_manager/src/policies/policy_handler.cc
@@ -1,5 +1,5 @@
 /*
- Copyright (c) 2014, Ford Motor Company
+ Copyright (c) 2016, Ford Motor Company
  All rights reserved.
 
  Redistribution and use in source and binary forms, with or without
@@ -1036,10 +1036,7 @@ bool PolicyHandler::SaveSnapshot(const BinaryMessage& pt_string,
   return result;
 }
 
-void PolicyHandler::OnSnapshotCreated(
-    const BinaryMessage& pt_string,
-    const std::vector<int>& retry_delay_seconds,
-    int timeout_exchange) {
+void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) {
   EndpointUrls urls;
   policy_manager_->GetServiceUrls("0x07", urls);
 
@@ -1106,7 +1103,7 @@ void PolicyHandler::ResetRetrySequence() {
   policy_manager_->ResetRetrySequence();
 }
 
-int PolicyHandler::NextRetryTimeout() {
+uint32_t PolicyHandler::NextRetryTimeout() {
   POLICY_LIB_CHECK(0);
   return policy_manager_->NextRetryTimeout();
 }
diff --git a/src/components/policy/src/policy/include/policy/policy_listener.h b/src/components/policy/src/policy/include/policy/policy_listener.h
index 2565bc353..197dfd5c8 100644
--- a/src/components/policy/src/policy/include/policy/policy_listener.h
+++ b/src/components/policy/src/policy/include/policy/policy_listener.h
@@ -1,5 +1,5 @@
 /*
- Copyright (c) 2013, Ford Motor Company
+ Copyright (c) 2016, Ford Motor Company
  All rights reserved.
 
  Redistribution and use in source and binary forms, with or without
@@ -75,13 +75,8 @@ class PolicyListener {
    *
    * @param pt_string the snapshot
    *
-   * @param retry_seconds retry sequence timeouts.
-   *
-   * @param timeout_exceed timeout.
    */
-  virtual void OnSnapshotCreated(const BinaryMessage& pt_string,
-                                 const std::vector<int>& retry_seconds,
-                                 int timeout_exceed) = 0;
+  virtual void OnSnapshotCreated(const BinaryMessage& pt_string) = 0;
 
   /**
    * @brief Make appropriate changes for related applications permissions and
diff --git a/src/components/policy/src/policy/include/policy/policy_manager.h b/src/components/policy/src/policy/include/policy/policy_manager.h
index 8c41c0d3b..40f6bd53d 100644
--- a/src/components/policy/src/policy/include/policy/policy_manager.h
+++ b/src/components/policy/src/policy/include/policy/policy_manager.h
@@ -1,5 +1,5 @@
 /*
- Copyright (c) 2013, Ford Motor Company
+ Copyright (c) 2016, Ford Motor Company
  All rights reserved.
 
  Redistribution and use in source and binary forms, with or without
@@ -34,6 +34,7 @@
 #define SRC_COMPONENTS_POLICY_INCLUDE_POLICY_POLICY_MANAGER_H_
 
 #include <vector>
+#include <cstdint>
 
 #include "policy/policy_types.h"
 #include "policy/policy_listener.h"
@@ -90,7 +91,7 @@ class PolicyManager : public usage_statistics::StatisticsManager {
     /**
      * @brief PTU is needed, for this PTS has to be formed and sent.
      */
-    virtual void RequestPTUpdate() = 0;
+    virtual bool RequestPTUpdate() = 0;
 
     /**
      * @brief Check if specified RPC for specified application
@@ -144,10 +145,10 @@ class PolicyManager : public usage_statistics::StatisticsManager {
 
     /**
      * Gets timeout to wait before next retry updating PT
-     * If timeout is less or equal to zero then the retry sequence is not need.
+     * If timeout is equal to zero then the retry sequence is not need.
      * @return timeout in seconds
      */
-    virtual int NextRetryTimeout() = 0;
+    virtual uint32_t NextRetryTimeout() = 0;
 
     /**
      * Gets timeout to wait until receive response
diff --git a/src/components/policy/src/policy/include/policy/policy_manager_impl.h b/src/components/policy/src/policy/include/policy/policy_manager_impl.h
index 1c489e840..efdd0e4a1 100644
--- a/src/components/policy/src/policy/include/policy/policy_manager_impl.h
+++ b/src/components/policy/src/policy/include/policy/policy_manager_impl.h
@@ -1,5 +1,5 @@
 /*
- Copyright (c) 2013, Ford Motor Company
+ Copyright (c) 2016, Ford Motor Company
  All rights reserved.
 
  Redistribution and use in source and binary forms, with or without
@@ -35,6 +35,7 @@
 
 #include <string>
 #include <list>
+#include <cstdint>
 
 #include "utils/shared_ptr.h"
 #include "utils/lock.h"
@@ -45,6 +46,7 @@
 #include "functions.h"
 #include "usage_statistics/statistics_manager.h"
 #include "policy/policy_helper.h"
+#include "utils/timer.h"
 
 namespace policy_table = rpc::policy_table_interface_base;
 
@@ -66,7 +68,7 @@ class PolicyManagerImpl : public PolicyManager {
                                 EndpointUrls& end_points);
 
     virtual std::string GetLockScreenIconUrl() const;
-    virtual void RequestPTUpdate();
+    virtual bool RequestPTUpdate();
     virtual void CheckPermissions(const PTString& app_id,
         const PTString& hmi_level,
         const PTString& rpc,
@@ -78,7 +80,7 @@ class PolicyManagerImpl : public PolicyManager {
     virtual std::string ForcePTExchange();
     virtual std::string GetPolicyTableStatus() const;
     virtual void ResetRetrySequence();
-    virtual int NextRetryTimeout();
+    virtual uint32_t NextRetryTimeout();
     virtual int TimeoutExchange();
     virtual const std::vector<int> RetrySequenceDelaysSeconds();
     virtual void OnExceededTimeout();
@@ -284,6 +286,8 @@ class PolicyManagerImpl : public PolicyManager {
     bool IsPTValid(utils::SharedPtr<policy_table::Table> policy_table,
                    policy_table::PolicyTableType type) const;
 
+    void RetrySequence();
+
 private:
     PolicyListener* listener_;
 
@@ -296,7 +300,7 @@ private:
     /**
      * Timeout to wait response with UpdatePT
      */
-    int retry_sequence_timeout_;
+    uint32_t retry_sequence_timeout_;
 
     /**
      * Seconds between retries to update PT
@@ -314,6 +318,11 @@ private:
     sync_primitives::Lock retry_sequence_lock_;
 
     /**
+      * Timer to retry UpdatePT
+      */
+    timer::Timer timer_retry_sequence_;
+
+    /**
      * @brief Device id, which is used during PTU handling for specific
      * application
      */
diff --git a/src/components/policy/src/policy/src/policy_manager_impl.cc b/src/components/policy/src/policy/src/policy_manager_impl.cc
index 910ec764c..8cc8870a3 100644
--- a/src/components/policy/src/policy/src/policy_manager_impl.cc
+++ b/src/components/policy/src/policy/src/policy_manager_impl.cc
@@ -1,5 +1,5 @@
 /*
- Copyright (c) 2013, Ford Motor Company
+ Copyright (c) 2016, Ford Motor Company
  All rights reserved.
 
  Redistribution and use in source and binary forms, with or without
@@ -44,26 +44,34 @@
 #include "utils/file_system.h"
 #include "utils/logger.h"
 #include "utils/date_time.h"
+#include "utils/make_shared.h"
 #include "policy/cache_manager.h"
 #include "policy/update_status_manager.h"
 #include "config_profile/profile.h"
+#include "utils/timer_task_impl.h"
 
 policy::PolicyManager* CreateManager() {
   return new policy::PolicyManagerImpl();
 }
 
+namespace {
+const uint32_t kDefaultRetryTimeoutInSec = 60u;
+}  // namespace
+
 namespace policy {
 
 CREATE_LOGGERPTR_GLOBAL(logger_, "Policy")
 
 PolicyManagerImpl::PolicyManagerImpl()
-  : PolicyManager(),
-    listener_(NULL),
-    cache_(new CacheManager),
-    retry_sequence_timeout_(60),
-    retry_sequence_index_(0),
-    ignition_check(true) {
-}
+    : PolicyManager()
+    , listener_(NULL)
+    , cache_(new CacheManager)
+    , retry_sequence_timeout_(kDefaultRetryTimeoutInSec)
+    , retry_sequence_index_(0)
+    , timer_retry_sequence_("Retry sequence timer",
+                            new timer::TimerTaskImpl<PolicyManagerImpl>(
+                                this, &PolicyManagerImpl::RetrySequence))
+    , ignition_check(true) {}
 
 void PolicyManagerImpl::set_listener(PolicyListener* listener) {
   listener_ = listener;
@@ -152,6 +160,12 @@ bool PolicyManagerImpl::LoadPT(const std::string& file,
   update_status_manager_.OnValidUpdateReceived();
   cache_->SaveUpdateRequired(false);
 
+  // Update finished, no need retry
+  if (timer_retry_sequence_.IsRunning()) {
+    LOG4CXX_INFO(logger_, "Stop retry sequence");
+    timer_retry_sequence_.Stop();
+  }
+
   {
     sync_primitives::AutoLock lock(apps_registration_lock_);
 
@@ -232,13 +246,13 @@ void PolicyManagerImpl::GetServiceUrls(const std::string& service_type,
   cache_->GetServiceUrls(service_type, end_points);
 }
 
-void PolicyManagerImpl::RequestPTUpdate() {
+bool PolicyManagerImpl::RequestPTUpdate() {
   LOG4CXX_AUTO_TRACE(logger_);
   utils::SharedPtr<policy_table::Table> policy_table_snapshot =
       cache_->GenerateSnapshot();
   if (!policy_table_snapshot) {
     LOG4CXX_ERROR(logger_, "Failed to create snapshot of policy table");
-    return;
+    return false;
   }
 
   IsPTValid(policy_table_snapshot, policy_table::PT_SNAPSHOT);
@@ -251,14 +265,13 @@ void PolicyManagerImpl::RequestPTUpdate() {
 
   BinaryMessage update(message_string.begin(), message_string.end());
 
-
-  listener_->OnSnapshotCreated(update,
-                               RetrySequenceDelaysSeconds(),
-                               TimeoutExchange());
+  listener_->OnSnapshotCreated(update);
 
   // Need to reset update schedule since all currenly registered applications
   // were already added to the snapshot so no update for them required.
   update_status_manager_.ResetUpdateSchedule();
+
+  return true;
 }
 
 std::string PolicyManagerImpl::GetLockScreenIconUrl() const {
@@ -289,7 +302,10 @@ void PolicyManagerImpl::StartPTExchange() {
     }
 
     if (update_status_manager_.IsUpdateRequired()) {
-      RequestPTUpdate();
+      if (RequestPTUpdate() && !timer_retry_sequence_.IsRunning()) {
+        // Start retry sequency
+        timer_retry_sequence_.Start(NextRetryTimeout(), true);
+      }
     }
   }
 }
@@ -726,16 +742,25 @@ std::string PolicyManagerImpl::GetPolicyTableStatus() const {
   return update_status_manager_.StringifiedUpdateStatus();
 }
 
-int PolicyManagerImpl::NextRetryTimeout() {
+uint32_t PolicyManagerImpl::NextRetryTimeout() {
   sync_primitives::AutoLock auto_lock(retry_sequence_lock_);
   LOG4CXX_DEBUG(logger_, "Index: " << retry_sequence_index_);
-  int next = 0;
-  if (!retry_sequence_seconds_.empty()
-      && retry_sequence_index_ < retry_sequence_seconds_.size()) {
-    next = retry_sequence_seconds_[retry_sequence_index_];
-    ++retry_sequence_index_;
+  uint32_t next = 0u;
+  if (retry_sequence_seconds_.empty() ||
+      retry_sequence_index_ >= retry_sequence_seconds_.size()) {
+    return next;
+  }
+
+  ++retry_sequence_index_;
+
+  for (uint32_t i = 0u; i < retry_sequence_index_; ++i) {
+    next += retry_sequence_seconds_[i];
+    // According to requirement APPLINK-18244
+    next += retry_sequence_timeout_;
   }
-  return next;
+
+  // Return miliseconds
+  return next * date_time::DateTime::MILLISECONDS_IN_SECOND;
 }
 
 void PolicyManagerImpl::RefreshRetrySequence() {
@@ -960,5 +985,18 @@ void PolicyManagerImpl::set_cache_manager(
   cache_ = cache_manager;
 }
 
-}  //  namespace policy
+void PolicyManagerImpl::RetrySequence() {
+  LOG4CXX_INFO(logger_, "Start new retry sequence");
+  RequestPTUpdate();
+
+  uint32_t timeout = NextRetryTimeout();
 
+  if (!timeout && timer_retry_sequence_.IsRunning()) {
+    timer_retry_sequence_.Stop();
+    return;
+  }
+
+  timer_retry_sequence_.Start(timeout, true);
+}
+
+}  //  namespace policy
diff --git a/src/components/policy/test/include/mock_policy_listener.h b/src/components/policy/test/include/mock_policy_listener.h
index 6a5aa92d5..8a50ad62f 100644
--- a/src/components/policy/test/include/mock_policy_listener.h
+++ b/src/components/policy/test/include/mock_policy_listener.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2014, Ford Motor Company
+/* Copyright (c) 2016, Ford Motor Company
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -69,10 +69,7 @@ class MockPolicyListener : public PolicyListener {
                void(const std::string& device_id, bool is_allowed));
   MOCK_METHOD1(OnUpdateHMIAppType, void(std::map<std::string, StringArray>));
   MOCK_METHOD1(GetAvailableApps, void(std::queue<std::string>&));
-  MOCK_METHOD3(OnSnapshotCreated,
-               void(const BinaryMessage& pt_string,
-                    const std::vector<int>& retry_seconds,
-                    int timeout_exceed));
+  MOCK_METHOD1(OnSnapshotCreated, void(const BinaryMessage& pt_string));
   MOCK_METHOD0(CanUpdate, bool());
   MOCK_METHOD1(OnCertificateUpdated, void(const std::string&));
   MOCK_CONST_METHOD2(SendOnAppPermissionsChanged,
diff --git a/src/components/policy/test/include/mock_policy_manager.h b/src/components/policy/test/include/mock_policy_manager.h
index 72be5d41d..c2f9a4b6e 100644
--- a/src/components/policy/test/include/mock_policy_manager.h
+++ b/src/components/policy/test/include/mock_policy_manager.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Ford Motor Company
+ * Copyright (c) 2016, Ford Motor Company
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -58,7 +58,7 @@ class MockPolicyManager : public PolicyManager {
   MOCK_METHOD1(ResetPT, bool(const std::string& file_name));
   MOCK_CONST_METHOD1(GetUpdateUrl, std::string(int service_type));
   MOCK_METHOD2(GetUpdateUrls, void(int service_type, EndpointUrls& end_points));
-  MOCK_METHOD0(RequestPTUpdate, void());
+  MOCK_METHOD0(RequestPTUpdate, bool());
   MOCK_METHOD5(CheckPermissions,
                void(const PTString& app_id, const PTString& hmi_level,
                     const PTString& rpc, const RPCParams& rpc_params,
diff --git a/src/components/policy/test/policy_manager_impl_test.cc b/src/components/policy/test/policy_manager_impl_test.cc
index 73a8a0079..ad28ebe2a 100644
--- a/src/components/policy/test/policy_manager_impl_test.cc
+++ b/src/components/policy/test/policy_manager_impl_test.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Ford Motor Company
+ * Copyright (c) 2016, Ford Motor Company
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -651,8 +651,16 @@ TEST_F(PolicyManagerImplTest2, NextRetryTimeout_ExpectTimeoutsFromPT) {
         root["policy_table"]["module_config"]["seconds_between_retries"];
     uint32_t size = seconds_between_retries.size();
     CreateLocalPT("sdl_preloaded_pt.json");
-    for (uint32_t i = 0; i < size; ++i) {
-      EXPECT_EQ(seconds_between_retries[i], manager->NextRetryTimeout());
+
+    uint32_t waiting_timeout = 0u;
+
+    for (uint32_t retry_number = 0u; retry_number < size; ++retry_number) {
+      waiting_timeout += seconds_between_retries[retry_number].asInt();
+      waiting_timeout += manager->TimeoutExchange();
+
+      // it's in miliseconds
+      EXPECT_EQ(waiting_timeout * date_time::DateTime::MILLISECONDS_IN_SECOND,
+                manager->NextRetryTimeout());
     }
   }
 }