From 658dad19900606fed9652ba47e0fb0a3c1a0478f Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Mon, 27 Sep 2021 11:22:22 +0900 Subject: [PATCH] Fetch and cache hostnames list for selected region fix https://github.com/brave/brave-browser/issues/18330 --- components/brave_vpn/BUILD.gn | 1 + components/brave_vpn/brave_vpn_data_types.h | 22 +++++++ .../brave_vpn/brave_vpn_service_desktop.cc | 60 +++++++++++++++++++ .../brave_vpn/brave_vpn_service_desktop.h | 10 ++++ components/brave_vpn/brave_vpn_unittest.cc | 45 ++++++++++++++ 5 files changed, 138 insertions(+) create mode 100644 components/brave_vpn/brave_vpn_data_types.h diff --git a/components/brave_vpn/BUILD.gn b/components/brave_vpn/BUILD.gn index 4351841c8e2d..53093df621e2 100644 --- a/components/brave_vpn/BUILD.gn +++ b/components/brave_vpn/BUILD.gn @@ -40,6 +40,7 @@ static_library("brave_vpn") { sources += [ "brave_vpn_connection_info.cc", "brave_vpn_connection_info.h", + "brave_vpn_data_types.h", "brave_vpn_os_connection_api.cc", "brave_vpn_os_connection_api.h", "brave_vpn_os_connection_api_sim.cc", diff --git a/components/brave_vpn/brave_vpn_data_types.h b/components/brave_vpn/brave_vpn_data_types.h new file mode 100644 index 000000000000..531a64a26ae3 --- /dev/null +++ b/components/brave_vpn/brave_vpn_data_types.h @@ -0,0 +1,22 @@ +/* Copyright (c) 2021 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_BRAVE_VPN_BRAVE_VPN_DATA_TYPES_H_ +#define BRAVE_COMPONENTS_BRAVE_VPN_BRAVE_VPN_DATA_TYPES_H_ + +#include + +namespace brave_vpn { + +struct Hostname { + std::string hostname; + std::string display_name; + bool is_offline; + int capacity_score; +}; + +} // namespace brave_vpn + +#endif // BRAVE_COMPONENTS_BRAVE_VPN_BRAVE_VPN_DATA_TYPES_H_ diff --git a/components/brave_vpn/brave_vpn_service_desktop.cc b/components/brave_vpn/brave_vpn_service_desktop.cc index 052379e43948..40ffc7f59e6d 100644 --- a/components/brave_vpn/brave_vpn_service_desktop.cc +++ b/components/brave_vpn/brave_vpn_service_desktop.cc @@ -414,4 +414,64 @@ void BraveVpnServiceDesktop::SetSelectedRegion( dict->SetStringKey(kRegionContinentKey, region_ptr->continent); dict->SetStringKey(kRegionNameKey, region_ptr->name); dict->SetStringKey(kRegionNamePrettyKey, region_ptr->name_pretty); + + // Start hostname fetching for selected region. + FetchHostnamesForRegion(region_ptr->name); +} + +void BraveVpnServiceDesktop::FetchHostnamesForRegion(const std::string& name) { + // Unretained is safe here becasue this class owns request helper. + GetHostnamesForRegion( + base::BindOnce(&BraveVpnServiceDesktop::OnFetchHostnames, + base::Unretained(this), name), + name); +} + +void BraveVpnServiceDesktop::OnFetchHostnames(const std::string& region, + const std::string& hostnames, + bool success) { + if (!success) { + // TODO(simonhong): Retry? + return; + } + + absl::optional value = base::JSONReader::Read(hostnames); + if (value && value->is_list()) { + ParseAndCacheHostnames(region, std::move(*value)); + return; + } + + // TODO(simonhong): Retry? +} + +void BraveVpnServiceDesktop::ParseAndCacheHostnames( + const std::string& region, + base::Value hostnames_value) { + DCHECK(hostnames_value.is_list()); + if (!hostnames_value.is_list()) + return; + + constexpr char kHostnameKey[] = "hostname"; + constexpr char kDisplayNameKey[] = "display-name"; + constexpr char kOfflineKey[] = "offline"; + constexpr char kCapacityScoreKey[] = "capacity-score"; + std::vector hostnames; + for (const auto& value : hostnames_value.GetList()) { + DCHECK(value.is_dict()); + if (!value.is_dict()) + continue; + + const std::string* hostname_str = value.FindStringKey(kHostnameKey); + const std::string* display_name_str = value.FindStringKey(kDisplayNameKey); + absl::optional offline = value.FindBoolKey(kOfflineKey); + absl::optional capacity_score = value.FindIntKey(kCapacityScoreKey); + + if (!hostname_str || !display_name_str || !offline || !capacity_score) + continue; + + hostnames.push_back(brave_vpn::Hostname{*hostname_str, *display_name_str, + *offline, *capacity_score}); + } + + hostnames_[region] = std::move(hostnames); } diff --git a/components/brave_vpn/brave_vpn_service_desktop.h b/components/brave_vpn/brave_vpn_service_desktop.h index 32c17b3e4480..5a2a4dcc171d 100644 --- a/components/brave_vpn/brave_vpn_service_desktop.h +++ b/components/brave_vpn/brave_vpn_service_desktop.h @@ -9,9 +9,11 @@ #include #include +#include "base/containers/flat_map.h" #include "base/scoped_observation.h" #include "brave/components/brave_vpn/brave_vpn.mojom.h" #include "brave/components/brave_vpn/brave_vpn_connection_info.h" +#include "brave/components/brave_vpn/brave_vpn_data_types.h" #include "brave/components/brave_vpn/brave_vpn_os_connection_api.h" #include "brave/components/brave_vpn/brave_vpn_service.h" #include "mojo/public/cpp/bindings/pending_remote.h" @@ -67,6 +69,7 @@ class BraveVpnServiceDesktop friend class BraveAppMenuBrowserTest; friend class BraveBrowserCommandControllerTest; FRIEND_TEST_ALL_PREFIXES(BraveVPNTest, RegionDataTest); + FRIEND_TEST_ALL_PREFIXES(BraveVPNTest, HostnamesTest); // BraveVpnService overrides: void Shutdown() override; @@ -86,6 +89,12 @@ class BraveVpnServiceDesktop void ParseAndCacheRegionList(base::Value region_value); void OnFetchTimezones(const std::string& timezones_list, bool success); void ParseAndCacheDefaultRegionName(base::Value timezons_value); + void FetchHostnamesForRegion(const std::string& name); + void OnFetchHostnames(const std::string& region, + const std::string& hostnames, + bool success); + void ParseAndCacheHostnames(const std::string& region, + base::Value hostnames_value); void SetDeviceRegion(const std::string& name); void SetFallbackDeviceRegion(); std::string GetCurrentTimeZone(); @@ -98,6 +107,7 @@ class BraveVpnServiceDesktop } PrefService* prefs_ = nullptr; + base::flat_map> hostnames_; std::vector regions_; brave_vpn::mojom::Region device_region_; ConnectionState state_ = ConnectionState::DISCONNECTED; diff --git a/components/brave_vpn/brave_vpn_unittest.cc b/components/brave_vpn/brave_vpn_unittest.cc index e1e4bd7f8ad3..3576cda51aab 100644 --- a/components/brave_vpn/brave_vpn_unittest.cc +++ b/components/brave_vpn/brave_vpn_unittest.cc @@ -135,6 +135,41 @@ class BraveVPNTest : public testing::Test { ])"; } + std::string GetHostnamesData() { + return R"([ + { + "hostname": "host-1.brave.com", + "display-name": "host-1", + "offline": false, + "capacity-score": 0 + }, + { + "hostname": "host-2.brave.com", + "display-name": "host-2", + "offline": false, + "capacity-score": 1 + }, + { + "hostname": "host-3.brave.com", + "display-name": "Singapore", + "offline": false, + "capacity-score": 0 + }, + { + "hostname": "host-4.brave.com", + "display-name": "host-4", + "offline": false, + "capacity-score": 0 + }, + { + "hostname": "host-5.brave.com", + "display-name": "host-5", + "offline": false, + "capacity-score": 1 + } + ])"; + } + content::BrowserTaskEnvironment task_environment_; TestingPrefServiceSimple pref_service_; std::unique_ptr service_; @@ -171,3 +206,13 @@ TEST_F(BraveVPNTest, RegionDataTest) { service_->OnFetchTimezones(GetTimeZonesData(), true); EXPECT_EQ(service_->regions_[0], service_->device_region_); } + +TEST_F(BraveVPNTest, HostnamesTest) { + // Set valid hostnames list + service_->OnFetchHostnames("region-a", GetHostnamesData(), true); + EXPECT_EQ(5UL, service_->hostnames_["region-a"].size()); + + // Set invalid hostnames list + service_->OnFetchHostnames("invalid-region-b", "", false); + EXPECT_EQ(0UL, service_->hostnames_["invalid-region-b"].size()); +}