Fix OnSystemRequest cycling urls refactoring

Required refactoring of the functionality that is responsible
for sending different application URL on each OnSystemRequest.
The logic was placed in OnSnapshotCreated method. Now it is
separated on two methods - GetNextUpdateUrl - returns pair of
policy application id and url from the Endpoints vector that
potentially will be sent, IsUrlAppIdValid - checks if given
policy application id is assigned to a registered application
or it is the default id.

12 files changed, 130 insertions, 27 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 7aca49a96a..31b3487a6f 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
@@ -570,6 +570,15 @@ class PolicyHandler : public PolicyHandlerInterface,
   application_manager::ApplicationManager& application_manager_;
   friend class AppPermissionDelegate;
 
+  /**
+   * @brief Checks if the application with the given policy
+   * application id is registered or it is default id
+   * @param policy_app_id Application policy id
+   * @return TRUE if the application with given id is registered or
+   * it is default id, otherwise FALSE
+   */
+  bool IsUrlAppIdValid(const std::string& policy_app_id) const;
+
   DISALLOW_COPY_AND_ASSIGN(PolicyHandler);
 };
 
diff --git a/src/components/application_manager/src/policies/policy_handler.cc b/src/components/application_manager/src/policies/policy_handler.cc
index 9336c6199e..3a5303945a 100644
--- a/src/components/application_manager/src/policies/policy_handler.cc
+++ b/src/components/application_manager/src/policies/policy_handler.cc
@@ -1366,32 +1366,12 @@ void PolicyHandler::OnSnapshotCreated(const BinaryMessage& pt_string) {
     return;
   }
 
-  static size_t current_app = 0;
-  static size_t current_url = 0;
-  if (current_url >= urls.at(current_app).url.size()) {
-    ApplicationSharedPtr app;
-    current_url = 0;
-
-    bool is_default = false;
-    bool is_registered = false;
-    bool has_urls = false;
-    bool valid_app_found = false;
-    do {
-      if (++current_app >= urls.size()) {
-        current_app = 0;
-      }
-      const std::string& app_id = urls.at(current_app).app_id;
-      app = application_manager_.application_by_policy_id(app_id);
-
-      is_default = (app_id == policy::kDefaultId);
-      is_registered = (app && app->IsRegistered());
-      has_urls = !urls.at(current_app).url.empty();
-      valid_app_found = (is_default || (is_registered && has_urls));
-    } while (!valid_app_found);
+  AppIdURL app_url;
+  app_url = policy_manager_->GetNextUpdateUrl(urls);
+  while (!IsUrlAppIdValid(app_url.first)) {
+    app_url = policy_manager_->GetNextUpdateUrl(urls);
   }
-
-  SendMessageToSDK(pt_string, urls.at(current_app).url.at(current_url));
-  current_url++;
+  SendMessageToSDK(pt_string, app_url.second);
 #endif  // PROPRIETARY_MODE
   // reset update required false
   OnUpdateRequestSentToMobile();
@@ -1819,4 +1799,16 @@ void PolicyHandler::Add(const std::string& app_id,
   policy_manager_->Add(app_id, type, timespan_seconds);
 }
 
+bool PolicyHandler::IsUrlAppIdValid(const std::string& policy_app_id) const {
+  bool is_registered = false;
+  bool is_default = false;
+
+  ApplicationSharedPtr app =
+      application_manager_.application_by_policy_id(policy_app_id);
+
+  is_registered = (app && (app->IsRegistered()));
+  is_default = (policy_app_id == policy::kDefaultId);
+
+  return (is_registered || is_default);
+}
 }  //  namespace policy
diff --git a/src/components/application_manager/test/policy_handler_test.cc b/src/components/application_manager/test/policy_handler_test.cc
index 4f80f2e464..8bedebee69 100644
--- a/src/components/application_manager/test/policy_handler_test.cc
+++ b/src/components/application_manager/test/policy_handler_test.cc
@@ -1351,12 +1351,18 @@ TEST_F(PolicyHandlerTest, OnSnapshotCreated_UrlAdded) {
   EndpointUrls test_data;
   EndpointData data("some_data");
   test_data.push_back(data);
+  AppIdURL next_app_url = std::make_pair("default", "some data");
+  ApplicationSharedPtr mock_app;
 
 #ifdef PROPRIETARY_MODE
   ExtendedPolicyExpectations();
 #else
   EXPECT_CALL(*mock_policy_manager_, GetUpdateUrls("0x07", _))
       .WillRepeatedly(SetArgReferee<1>(test_data));
+  EXPECT_CALL(*mock_policy_manager_, GetNextUpdateUrl(_))
+      .WillOnce(Return(next_app_url));
+  EXPECT_CALL(app_manager_, application_by_policy_id(_))
+      .WillOnce(Return(mock_app));
   EXPECT_CALL(app_manager_, connection_handler())
       .WillOnce(ReturnRef(conn_handler));
   EXPECT_CALL(conn_handler, get_session_observer())
@@ -1372,6 +1378,7 @@ TEST_F(PolicyHandlerTest, OnSnapshotCreated_UrlAdded) {
   EXPECT_CALL(*mock_app_, policy_app_id()).WillOnce(Return(kPolicyAppId_));
 #endif  // PROPRIETARY_MODE
 
+  EXPECT_CALL(*mock_policy_manager_, OnUpdateStarted());
   policy_handler_.OnSnapshotCreated(msg);
 }
 #endif  // EXTERNAL_PROPRIETARY_MODE
diff --git a/src/components/include/policy/policy_external/policy/policy_manager.h b/src/components/include/policy/policy_external/policy/policy_manager.h
index 7d5bbb5e52..181e4c364e 100644
--- a/src/components/include/policy/policy_external/policy/policy_manager.h
+++ b/src/components/include/policy/policy_external/policy/policy_manager.h
@@ -481,6 +481,12 @@ class PolicyManager : public usage_statistics::StatisticsManager {
 
   virtual const PolicySettings& get_settings() const = 0;
 
+  /**
+   * @brief Finds the next URL that must be sent on OnSystemRequest retry
+   * @param urls vector of vectors that contain urls for each application
+   * @return Pair of policy application id and application url
+   */
+  virtual AppIdURL GetNextUpdateUrl(const EndpointUrls& urls) = 0;
  protected:
   /**
    * Checks is PT exceeded IgnitionCycles
diff --git a/src/components/include/policy/policy_regular/policy/policy_types.h b/src/components/include/policy/policy_regular/policy/policy_types.h
index b3843f8889..70930f7048 100644
--- a/src/components/include/policy/policy_regular/policy/policy_types.h
+++ b/src/components/include/policy/policy_regular/policy/policy_types.h
@@ -313,6 +313,19 @@ struct MetaInfo {
   std::string language;
 };
 
+struct RetrySequenceURL {
+  int app;
+  int url;
+  RetrySequenceURL(int app_idx, int url_idx) {
+    app = app_idx;
+    url = url_idx;
+  }
+};
+
+typedef struct RetrySequenceURL RetrySequenceURL;
+
+typedef std::pair<std::string, std::string> AppIdURL;
+
 }  //  namespace policy
 
 #endif  // SRC_COMPONENTS_INCLUDE_POLICY_POLICY_TYPES_H_
diff --git a/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h b/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h
index 0140efddf5..8c5a51a55d 100644
--- a/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h
+++ b/src/components/include/test/policy/policy_external/policy/mock_policy_manager.h
@@ -182,6 +182,7 @@ class MockPolicyManager : public PolicyManager {
                     int32_t timespan_seconds));
   MOCK_CONST_METHOD0(get_settings, const PolicySettings&());
   MOCK_METHOD1(set_settings, void(const PolicySettings* get_settings));
+  MOCK_METHOD1(GetNextUpdateUrl, AppIdURL(const EndpointUrls& urls));
 };
 }  // namespace policy_manager_test
 }  // namespace components
diff --git a/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h b/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h
index 638f33399c..c6c02fac55 100644
--- a/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h
+++ b/src/components/include/test/policy/policy_regular/policy/mock_policy_manager.h
@@ -184,6 +184,7 @@ class MockPolicyManager : public PolicyManager {
   MOCK_CONST_METHOD0(get_settings, const PolicySettings&());
   MOCK_METHOD1(set_settings, void(const PolicySettings* get_settings));
   MOCK_CONST_METHOD0(GetLockScreenIconUrl, std::string());
+  MOCK_METHOD1(GetNextUpdateUrl, AppIdURL(const EndpointUrls& urls));
 };
 
 }  // namespace policy_manager_test
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 55b5ab1212..aa83eb88dc 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
@@ -335,6 +335,13 @@ class PolicyManagerImpl : public PolicyManager {
 
   const PolicySettings* settings_;
   friend struct CheckAppPolicy;
+
+  /**
+   * @brief Pair of app index and url index from Endpoints vector
+   * that contains all application URLs
+   */
+  RetrySequenceURL retry_sequence_url_;
+  friend struct ProccessAppGroups;
 };
 
 }  // namespace policy
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 e0f7d89ae9..558fc91727 100644
--- a/src/components/policy/policy_external/include/policy/policy_types.h
+++ b/src/components/policy/policy_external/include/policy/policy_types.h
@@ -364,6 +364,19 @@ struct MetaInfo {
   std::string language;
 };
 
+struct RetrySequenceURL {
+  int app;
+  int url;
+  RetrySequenceURL(int app_idx, int url_idx) {
+    app = app_idx;
+    url = url_idx;
+  }
+}
+
+typedef struct RetrySequenceURL RetrySequenceURL;
+
+typedef std::pair<std::string, std::string> AppIdURL;
+
 }  //  namespace policy
 
 #endif  // SRC_COMPONENTS_POLICY_POLICY_EXTERNAL_INCLUDE_POLICY_POLICY_TYPES_H_
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 88d810ecc2..fa27c95607 100644
--- a/src/components/policy/policy_external/src/policy_manager_impl.cc
+++ b/src/components/policy/policy_external/src/policy_manager_impl.cc
@@ -66,7 +66,9 @@ PolicyManagerImpl::PolicyManagerImpl()
     , cache_(new CacheManager)
     , retry_sequence_timeout_(60)
     , retry_sequence_index_(0)
-    , ignition_check(true) {}
+    , ignition_check(true)
+    , retry_sequence_url_(0, 0) {
+}
 
 PolicyManagerImpl::PolicyManagerImpl(bool in_memory)
     : PolicyManager()
@@ -1190,6 +1192,27 @@ void PolicyManagerImpl::SetDecryptedCertificate(
   cache_->SetDecryptedCertificate(certificate);
 }
 
+AppIdURL PolicyManagerImpl::GetNextUpdateUrl(const EndpointUrls& urls) {
+  LOG4CXX_AUTO_TRACE(logger_);
+
+  AppIdURL next_app_url;
+  uint32_t url = retry_sequence_url_.url;
+  uint32_t app = retry_sequence_url_.app;
+
+  if (url >= urls[app].url.size()) {
+    url = 0;
+    if (++app >= urls.size()) {
+      app = 0;
+    }
+  }
+
+  next_app_url = std::make_pair(urls[app].app_id, urls[app].url[url]);
+  retry_sequence_url_.url = url + 1;
+  retry_sequence_url_.app = app;
+
+  return next_app_url;
+}
+
 /**
  * @brief The CallStatusChange class notify update manager aboun new application
  */
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 c7773d7728..b1fc476516 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
@@ -213,6 +213,8 @@ class PolicyManagerImpl : public PolicyManager {
 
   const PolicySettings& get_settings() const OVERRIDE;
 
+  virtual AppIdURL GetNextUpdateUrl(const EndpointUrls& urls);
+
  private:
   void CheckTriggers();
   /*
@@ -347,6 +349,13 @@ class PolicyManagerImpl : public PolicyManager {
 
   const PolicySettings* settings_;
   friend struct CheckAppPolicy;
+  friend struct ProccessAppGroups;
+
+  /**
+   * @brief Pair of app index and url index from Endpoints vector
+   * that contains all application URLs
+   */
+  RetrySequenceURL retry_sequence_url_;
 };
 
 }  // namespace policy
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 3a4e9d6acb..2183af228b 100644
--- a/src/components/policy/policy_regular/src/policy_manager_impl.cc
+++ b/src/components/policy/policy_regular/src/policy_manager_impl.cc
@@ -76,7 +76,9 @@ PolicyManagerImpl::PolicyManagerImpl()
     , timer_retry_sequence_("Retry sequence timer",
                             new timer::TimerTaskImpl<PolicyManagerImpl>(
                                 this, &PolicyManagerImpl::RetrySequence))
-    , ignition_check(true) {}
+    , ignition_check(true)
+    , retry_sequence_url_(0, 0) {
+}
 
 void PolicyManagerImpl::set_listener(PolicyListener* listener) {
   listener_ = listener;
@@ -898,6 +900,26 @@ std::string PolicyManagerImpl::RetrieveCertificate() const {
   return cache_->GetCertificate();
 }
 
+AppIdURL PolicyManagerImpl::GetNextUpdateUrl(const EndpointUrls& urls) {
+  LOG4CXX_AUTO_TRACE(logger_);
+
+  AppIdURL next_app_url;
+  uint32_t url = retry_sequence_url_.url;
+  uint32_t app = retry_sequence_url_.app;
+
+  if (url >= urls[app].url.size()) {
+    url = 0;
+    if (++app >= urls.size()) {
+      app = 0;
+    }
+  }
+
+  next_app_url = std::make_pair(urls[app].app_id, urls[app].url[url]);
+  retry_sequence_url_.url = url + 1;
+  retry_sequence_url_.app = app;
+
+  return next_app_url;
+
 class CallStatusChange : public utils::Callable {
  public:
   CallStatusChange(UpdateStatusManager& upd_manager,