From d9a54115ee4ddcd8027cebd9d0ad370033abc640 Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Tue, 25 Mar 2025 13:51:56 -0700 Subject: [PATCH] Change scheduler to require internet connectivity for jobs that use require_connectivity flag. PiperOrigin-RevId: 740473218 --- sharing/scheduling/format.cc | 5 --- sharing/scheduling/format.h | 1 - .../scheduling/nearby_share_scheduler_base.cc | 42 +++++++++---------- .../scheduling/nearby_share_scheduler_base.h | 6 +-- .../nearby_share_scheduler_base_test.cc | 15 ++++++- 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/sharing/scheduling/format.cc b/sharing/scheduling/format.cc index dca91d71..71bb35bf 100644 --- a/sharing/scheduling/format.cc +++ b/sharing/scheduling/format.cc @@ -16,7 +16,6 @@ #include -#include "absl/strings/str_format.h" #include "absl/time/time.h" namespace nearby { @@ -27,9 +26,5 @@ std::string TimeFormatShortDateAndTimeWithTimeZone(absl::Time time) { return absl::FormatTime("%Y/%m/%d %H:%M:%S %z", time, tz); } -std::string TimeDurationFormatWithSeconds(absl::Duration duration) { - return absl::StrFormat("%ds", duration / absl::Seconds(1)); -} - } // namespace utils } // namespace nearby diff --git a/sharing/scheduling/format.h b/sharing/scheduling/format.h index 052a42f3..4946b925 100644 --- a/sharing/scheduling/format.h +++ b/sharing/scheduling/format.h @@ -23,7 +23,6 @@ namespace nearby { namespace utils { std::string TimeFormatShortDateAndTimeWithTimeZone(absl::Time time); -std::string TimeDurationFormatWithSeconds(absl::Duration duration); } // namespace utils } // namespace nearby diff --git a/sharing/scheduling/nearby_share_scheduler_base.cc b/sharing/scheduling/nearby_share_scheduler_base.cc index a813e561..7487db42 100644 --- a/sharing/scheduling/nearby_share_scheduler_base.cc +++ b/sharing/scheduling/nearby_share_scheduler_base.cc @@ -82,7 +82,7 @@ NearbyShareSchedulerBase::NearbyShareSchedulerBase( connection_listener_name_, [this](nearby::ConnectivityManager::ConnectionType connection_type, bool is_lan_connected, bool is_internet_connected) { - OnConnectionChanged(connection_type); + OnInternetConnectivityChanged(is_internet_connected); }); } } @@ -116,7 +116,6 @@ void NearbyShareSchedulerBase::HandleResult(bool success) { SetIsWaitingForResult(false); Reschedule(); - PrintSchedulerState(); } void NearbyShareSchedulerBase::Reschedule() { @@ -125,10 +124,15 @@ void NearbyShareSchedulerBase::Reschedule() { timer_->Stop(); std::optional delay = GetTimeUntilNextRequest(); - if (!delay.has_value()) return; - - int64_t delay_milliseconds = absl::ToInt64Milliseconds(*delay); - timer_->Start(delay_milliseconds, /*period=*/0, [this]() { OnTimerFired(); }); + if (!delay.has_value()) { + LOG(INFO) << "Task \"" << pref_name_ << "\"" << " not scheduled"; + } else { + int64_t delay_milliseconds = absl::ToInt64Milliseconds(*delay); + LOG(INFO) << "Task \"" << pref_name_ << "\"" << " scheduled in " << *delay; + timer_->Start(delay_milliseconds, /*period=*/0, + [this]() { OnTimerFired(); }); + } + PrintSchedulerState(delay); } std::optional NearbyShareSchedulerBase::GetLastSuccessTime() const { @@ -182,17 +186,16 @@ size_t NearbyShareSchedulerBase::GetNumConsecutiveFailures() const { void NearbyShareSchedulerBase::OnStart() { Reschedule(); - LOG(INFO) << "Starting Nearby Share scheduler \"" << pref_name_ << "\""; - PrintSchedulerState(); } void NearbyShareSchedulerBase::OnStop() { timer_->Stop(); } -void NearbyShareSchedulerBase::OnConnectionChanged( - nearby::ConnectivityManager::ConnectionType connection_type) { - if (connection_type == nearby::ConnectivityManager::ConnectionType::kNone) +void NearbyShareSchedulerBase::OnInternetConnectivityChanged( + bool is_internet_connected) { + if (!is_internet_connected) { return; - + } + LOG(INFO) << "Internet connectivity restored for scheduler: " << pref_name_; Reschedule(); } @@ -276,9 +279,9 @@ void NearbyShareSchedulerBase::OnTimerFired() { LOG(DFATAL) << "Timer fired after stop for scheduler: " << pref_name_; return; } - if (require_connectivity_ && - (connectivity_manager_->GetConnectionType() == - nearby::ConnectivityManager::ConnectionType::kNone)) { + if (require_connectivity_ && !connectivity_manager_->IsInternetConnected()) { + LOG(INFO) << "Task \"" << pref_name_ + << "\" ignored, no internet connection"; return; } @@ -287,14 +290,13 @@ void NearbyShareSchedulerBase::OnTimerFired() { NotifyOfRequest(); } -void NearbyShareSchedulerBase::PrintSchedulerState() const { +void NearbyShareSchedulerBase::PrintSchedulerState( + std::optional time_until_next_request) const { if (!VLOG_IS_ON(1)) { return; } std::optional last_attempt_time = GetLastAttemptTime(); std::optional last_success_time = GetLastSuccessTime(); - std::optional time_until_next_request = - GetTimeUntilNextRequest(); std::stringstream ss; ss << "State of Nearby Share scheduler \"" << pref_name_ << "\":" @@ -316,9 +318,7 @@ void NearbyShareSchedulerBase::PrintSchedulerState() const { ss << "\n Time until next request: "; if (time_until_next_request) { - std::u16string next_request_delay; - ss << nearby::utils::TimeDurationFormatWithSeconds( - *time_until_next_request); + ss << *time_until_next_request; } else { ss << "Never"; } diff --git a/sharing/scheduling/nearby_share_scheduler_base.h b/sharing/scheduling/nearby_share_scheduler_base.h index b0e301aa..bedf273b 100644 --- a/sharing/scheduling/nearby_share_scheduler_base.h +++ b/sharing/scheduling/nearby_share_scheduler_base.h @@ -83,8 +83,7 @@ class NearbyShareSchedulerBase : public NearbyShareScheduler { void OnStop() override; private: - void OnConnectionChanged( - nearby::ConnectivityManager::ConnectionType connection_type); + void OnInternetConnectivityChanged(bool is_internet_connected); std::optional GetLastAttemptTime() const; bool HasPendingImmediateRequest() const; @@ -106,7 +105,8 @@ class NearbyShareSchedulerBase : public NearbyShareScheduler { // connectivity is restored. void OnTimerFired(); - void PrintSchedulerState() const; + void PrintSchedulerState( + std::optional time_until_next_request) const; nearby::ConnectivityManager* const connectivity_manager_; nearby::sharing::api::PreferenceManager& preference_manager_; diff --git a/sharing/scheduling/nearby_share_scheduler_base_test.cc b/sharing/scheduling/nearby_share_scheduler_base_test.cc index c5f50cfb..c3c34fb8 100644 --- a/sharing/scheduling/nearby_share_scheduler_base_test.cc +++ b/sharing/scheduling/nearby_share_scheduler_base_test.cc @@ -133,7 +133,7 @@ class NearbyShareSchedulerBaseTest : public ::testing::Test { size_t on_request_call_count() const { return on_request_call_count_; } NearbyShareScheduler* scheduler() { return scheduler_.get(); } - private: + protected: nearby::FakePreferenceManager preference_manager_; nearby::FakeContext fake_context_; size_t on_request_call_count_ = 0; @@ -404,6 +404,19 @@ TEST_F(NearbyShareSchedulerBaseTest, RestoreSchedulingData) { EXPECT_EQ(scheduler()->GetNumConsecutiveFailures(), 1u); } + +TEST_F(NearbyShareSchedulerBaseTest, InternetConnectivityChange) { + fake_context_.fake_connectivity_manager()->SetInternetConnected(false); + CreateScheduler(/*retry_failures=*/true, /*require_connectivity=*/true); + StartScheduling(); + scheduler()->MakeImmediateRequest(); + ASSERT_NO_FATAL_FAILURE(RunPendingRequest()); + EXPECT_EQ(on_request_call_count(), 0); + + fake_context_.fake_connectivity_manager()->SetInternetConnected(true); + ASSERT_NO_FATAL_FAILURE(RunPendingRequest()); + EXPECT_EQ(on_request_call_count(), 1); +} } // namespace } // namespace sharing } // namespace nearby