From 9a6b1ba3b9ca5ccbfcecc693ba2000787eff024b Mon Sep 17 00:00:00 2001 From: deeppandya Date: Mon, 12 Aug 2024 23:10:01 +0530 Subject: [PATCH] Add region precision in the api --- .../VpnServerSelectionActivity.java | 2 ++ .../browser/api/brave_vpn_api_request.cc | 6 +++-- .../browser/api/brave_vpn_api_request.h | 3 ++- .../brave_vpn/browser/brave_vpn_service.cc | 8 ++++--- .../brave_vpn/browser/brave_vpn_service.h | 4 ++-- .../browser/brave_vpn_service_helper.cc | 18 +++++++++------ .../brave_vpn_region_data_manager.cc | 23 ++++--------------- .../brave_vpn/common/brave_vpn_constants.h | 2 +- .../brave_vpn/common/mojom/brave_vpn.mojom | 7 +++++- 9 files changed, 37 insertions(+), 36 deletions(-) diff --git a/android/java/org/chromium/chrome/browser/vpn/activities/VpnServerSelectionActivity.java b/android/java/org/chromium/chrome/browser/vpn/activities/VpnServerSelectionActivity.java index b47df98f7e6..691967dfc3d 100644 --- a/android/java/org/chromium/chrome/browser/vpn/activities/VpnServerSelectionActivity.java +++ b/android/java/org/chromium/chrome/browser/vpn/activities/VpnServerSelectionActivity.java @@ -19,6 +19,7 @@ import androidx.recyclerview.widget.RecyclerView; import com.google.android.material.materialswitch.MaterialSwitch; +import org.chromium.brave_vpn.mojom.BraveVpnConstants; import org.chromium.brave_vpn.mojom.Region; import org.chromium.brave_vpn.mojom.ServiceHandler; import org.chromium.chrome.R; @@ -132,6 +133,7 @@ public class VpnServerSelectionActivity extends BraveVpnParentActivity initVpnService(); showProgress(); mServiceHandler.getAllRegions( + BraveVpnConstants.REGION_PRECISION_CITY_BY_COUNTRY, regions -> { if (regions.length > 0) { mBraveVpnServerSelectionAdapter = diff --git a/components/brave_vpn/browser/api/brave_vpn_api_request.cc b/components/brave_vpn/browser/api/brave_vpn_api_request.cc index e4bad8be498..43504071577 100644 --- a/components/brave_vpn/browser/api/brave_vpn_api_request.cc +++ b/components/brave_vpn/browser/api/brave_vpn_api_request.cc @@ -62,11 +62,13 @@ BraveVpnAPIRequest::BraveVpnAPIRequest( BraveVpnAPIRequest::~BraveVpnAPIRequest() = default; -void BraveVpnAPIRequest::GetServerRegionsWithCities(ResponseCallback callback) { +void BraveVpnAPIRequest::GetServerRegions(ResponseCallback callback, + const std::string& region_precision) { auto internal_callback = base::BindOnce(&BraveVpnAPIRequest::OnGetResponse, weak_ptr_factory_.GetWeakPtr(), std::move(callback)); - GURL base_url = GetURLWithPath(kVpnHost, kServerRegionsWithCities); + GURL base_url = + GetURLWithPath(kVpnHost, kServerRegionsWithCities + region_precision); OAuthRequest(base_url, "GET", "", std::move(internal_callback)); } diff --git a/components/brave_vpn/browser/api/brave_vpn_api_request.h b/components/brave_vpn/browser/api/brave_vpn_api_request.h index d9d3c00fe43..e0001775826 100644 --- a/components/brave_vpn/browser/api/brave_vpn_api_request.h +++ b/components/brave_vpn/browser/api/brave_vpn_api_request.h @@ -33,7 +33,8 @@ class BraveVpnAPIRequest { base::OnceCallback; using APIRequestResult = api_request_helper::APIRequestResult; - void GetServerRegionsWithCities(ResponseCallback callback); + void GetServerRegions(ResponseCallback callback, + const std::string& region_precision); void GetTimezonesForRegions(ResponseCallback callback); void GetHostnamesForRegion(ResponseCallback callback, const std::string& region); diff --git a/components/brave_vpn/browser/brave_vpn_service.cc b/components/brave_vpn/browser/brave_vpn_service.cc index b0685c61446..9d726ee8a95 100644 --- a/components/brave_vpn/browser/brave_vpn_service.cc +++ b/components/brave_vpn/browser/brave_vpn_service.cc @@ -378,12 +378,14 @@ void BraveVpnService::UpdatePurchasedStateForSessionExpired( } #endif // !BUILDFLAG(IS_ANDROID) -void BraveVpnService::GetAllRegions(GetAllRegionsCallback callback) { +void BraveVpnService::GetAllRegions(const std::string& region_precision, + GetAllRegionsCallback callback) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); #if BUILDFLAG(IS_ANDROID) - api_request_->GetServerRegionsWithCities( + api_request_->GetServerRegions( base::BindOnce(&BraveVpnService::OnFetchRegionList, - base::Unretained(this), std::move(callback))); + base::Unretained(this), std::move(callback)), + region_precision); #else std::vector regions; for (const auto& region : diff --git a/components/brave_vpn/browser/brave_vpn_service.h b/components/brave_vpn/browser/brave_vpn_service.h index 09c1b9e3747..db127eeaae1 100644 --- a/components/brave_vpn/browser/brave_vpn_service.h +++ b/components/brave_vpn/browser/brave_vpn_service.h @@ -127,9 +127,9 @@ class BraveVpnService : void GetPurchasedState(GetPurchasedStateCallback callback) override; void LoadPurchasedState(const std::string& domain) override; - void GetAllRegions(GetAllRegionsCallback callback) override; + void GetAllRegions(const std::string& region_precision, + GetAllRegionsCallback callback) override; - void GetServerRegionsWithCities(ResponseCallback callback); void GetTimezonesForRegions(ResponseCallback callback); void GetHostnamesForRegion(ResponseCallback callback, const std::string& region); diff --git a/components/brave_vpn/browser/brave_vpn_service_helper.cc b/components/brave_vpn/browser/brave_vpn_service_helper.cc index 451c71cace8..fb3bda2c87c 100644 --- a/components/brave_vpn/browser/brave_vpn_service_helper.cc +++ b/components/brave_vpn/browser/brave_vpn_service_helper.cc @@ -50,11 +50,13 @@ base::Value::Dict GetValueFromRegionWithoutCity( base::Value::Dict GetValueFromRegion(const mojom::RegionPtr& region) { base::Value::Dict region_dict = GetValueFromRegionWithoutCity(region); - base::Value::List cities; - for (const auto& city : region->cities) { - cities.Append(GetValueFromRegionWithoutCity(city)); + if (!region->cities.empty()) { + base::Value::List cities; + for (const auto& city : region->cities) { + cities.Append(GetValueFromRegionWithoutCity(city)); + } + region_dict.Set(kRegionCitiesKey, std::move(cities)); } - region_dict.Set(kRegionCitiesKey, std::move(cities)); return region_dict; } @@ -108,9 +110,11 @@ mojom::RegionPtr GetRegionFromValueWithoutCity(const base::Value::Dict& value) { mojom::RegionPtr GetRegionFromValue(const base::Value::Dict& value) { mojom::RegionPtr region = GetRegionFromValueWithoutCity(value); - const auto* cities = value.FindList(kRegionCitiesKey); - for (const auto& city : *cities) { - region->cities.push_back(GetRegionFromValueWithoutCity(city.GetDict())); + if (value.FindList(kRegionCitiesKey)) { + const auto* cities = value.FindList(kRegionCitiesKey); + for (const auto& city : *cities) { + region->cities.push_back(GetRegionFromValueWithoutCity(city.GetDict())); + } } return region; diff --git a/components/brave_vpn/browser/connection/brave_vpn_region_data_manager.cc b/components/brave_vpn/browser/connection/brave_vpn_region_data_manager.cc index 5c2d10ab82d..1cfe3b2d21b 100644 --- a/components/brave_vpn/browser/connection/brave_vpn_region_data_manager.cc +++ b/components/brave_vpn/browser/connection/brave_vpn_region_data_manager.cc @@ -190,16 +190,13 @@ void BraveVPNRegionDataManager::FetchRegionDataIfNeeded() { } void BraveVPNRegionDataManager::FetchRegions() { - LOG(ERROR) << "brave_vpn : " << "FetchRegions 1"; - if (!url_loader_factory_) { - LOG(ERROR) << "brave_vpn : " << "FetchRegions 1.5"; - } api_request_ = std::make_unique(url_loader_factory_); VLOG(2) << __func__ << " : Start fetching region data"; - LOG(ERROR) << "brave_vpn : " << "FetchRegions 2"; // Unretained is safe here becasue this class owns |api_request_|. - api_request_->GetServerRegionsWithCities(base::BindOnce( - &BraveVPNRegionDataManager::OnFetchRegionList, base::Unretained(this))); + api_request_->GetServerRegions( + base::BindOnce(&BraveVPNRegionDataManager::OnFetchRegionList, + base::Unretained(this)), + mojom::kRegionPrecisionCityByCountry); } void BraveVPNRegionDataManager::OnFetchRegionList( @@ -210,17 +207,6 @@ void BraveVPNRegionDataManager::OnFetchRegionList( } api_request_.reset(); std::optional value = base::JSONReader::Read(region_list); - - LOG(ERROR) << "brave_vpn : " << "OnFetchRegionList 1"; -#if BUILDFLAG(IS_ANDROID) - if (value && value->is_list()) { - auto new_regions = ParseRegionList(value->GetList()); - LOG(ERROR) << "brave_vpn : " << "OnFetchRegionList 2"; - for (const auto& region : new_regions) { - LOG(ERROR) << "brave_vpn : " << region->name; - } - } -#else if (value && value->is_list() && ParseAndCacheRegionList(value->GetList(), true)) { VLOG(2) << "Got valid region list"; @@ -236,7 +222,6 @@ void BraveVPNRegionDataManager::OnFetchRegionList( VLOG(2) << "Got invalid region list"; NotifyRegionDataReady(); -#endif } bool BraveVPNRegionDataManager::ParseAndCacheRegionList( diff --git a/components/brave_vpn/common/brave_vpn_constants.h b/components/brave_vpn/common/brave_vpn_constants.h index 7a54fd230df..64e9f2299b9 100644 --- a/components/brave_vpn/common/brave_vpn_constants.h +++ b/components/brave_vpn/common/brave_vpn_constants.h @@ -41,7 +41,7 @@ inline constexpr char kSupportTicketTimezoneKey[] = "timezone"; inline constexpr char kVpnHost[] = "connect-api.guardianapp.com"; inline constexpr char kAllServerRegions[] = "api/v1/servers/all-server-regions"; inline constexpr char kServerRegionsWithCities[] = - "api/v1.3/servers/all-server-regions/city-by-country"; + "api/v1.3/servers/all-server-regions/"; inline constexpr char kTimezonesForRegions[] = "api/v1.1/servers/timezones-for-regions"; inline constexpr char kHostnameForRegion[] = diff --git a/components/brave_vpn/common/mojom/brave_vpn.mojom b/components/brave_vpn/common/mojom/brave_vpn.mojom index 0706f7a8fb6..c486efbeed7 100644 --- a/components/brave_vpn/common/mojom/brave_vpn.mojom +++ b/components/brave_vpn/common/mojom/brave_vpn.mojom @@ -51,7 +51,7 @@ interface ServiceHandler { Disconnect(); // Gets all region from internal cache which is fetched from Guardian API - GetAllRegions() => (array regions); + GetAllRegions(string region_precision) => (array regions); [EnableIfNot=is_android] GetSelectedRegion() => (Region current_region); @@ -132,3 +132,8 @@ struct Region { double longitude; int32 server_count; }; + +const string kRegionPrecisionDefault = "default"; +const string kRegionPrecisionCity = "city"; +const string kRegionPrecisionCountry = "country"; +const string kRegionPrecisionCityByCountry = "city-by-country";