From 2f18ed322f453a8e1ba5d31f75301a44af7bdf80 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Mon, 4 May 2020 14:45:06 +0200 Subject: [PATCH] Adds pagination to publisher list Resolves https://github.com/brave/brave-browser/issues/9607 --- .../browser/rewards_service_browsertest.cc | 4 +- vendor/bat-native-ledger/BUILD.gn | 2 + .../include/bat/ledger/mojom_structs.h | 2 + .../bat/ledger/public/interfaces/ledger.mojom | 5 +- .../ledger/internal/publisher/publisher.cc | 2 +- .../bat/ledger/internal/publisher/publisher.h | 6 +- .../publisher/publisher_server_list.cc | 158 ++++++++++-------- .../publisher/publisher_server_list.h | 26 +-- .../internal/request/request_publisher.cc | 21 +++ .../internal/request/request_publisher.h | 19 +++ .../ledger/internal/request/request_util.cc | 23 +-- .../ledger/internal/request/request_util.h | 3 +- .../src/bat/ledger/internal/static_values.h | 14 +- 13 files changed, 158 insertions(+), 127 deletions(-) create mode 100644 vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.cc create mode 100644 vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.h diff --git a/components/brave_rewards/browser/rewards_service_browsertest.cc b/components/brave_rewards/browser/rewards_service_browsertest.cc index 300f99827952..ccf92190b288 100644 --- a/components/brave_rewards/browser/rewards_service_browsertest.cc +++ b/components/brave_rewards/browser/rewards_service_browsertest.cc @@ -354,8 +354,8 @@ class BraveRewardsBrowserTest } else if (URLMatches(url, "/captchas", PREFIX_V1, ServerTypes::kPromotion)) { *response = brave_test_resp::captcha_; - } else if (URLMatches(url, GET_PUBLISHERS_LIST, "", - ServerTypes::PUBLISHER_DISTRO)) { + } else if (URLMatches(url, "/api/v3/public/channels", "", + ServerTypes::kPublisher)) { if (alter_publisher_list_) { *response = "[" diff --git a/vendor/bat-native-ledger/BUILD.gn b/vendor/bat-native-ledger/BUILD.gn index d0c057f17f9c..7fbade013f6a 100644 --- a/vendor/bat-native-ledger/BUILD.gn +++ b/vendor/bat-native-ledger/BUILD.gn @@ -241,6 +241,8 @@ source_set("ledger") { "src/bat/ledger/internal/request/request_attestation.h", "src/bat/ledger/internal/request/request_promotion.cc", "src/bat/ledger/internal/request/request_promotion.h", + "src/bat/ledger/internal/request/request_publisher.cc", + "src/bat/ledger/internal/request/request_publisher.h", "src/bat/ledger/internal/request/request_util.cc", "src/bat/ledger/internal/request/request_util.h", "src/bat/ledger/internal/state_keys.h", diff --git a/vendor/bat-native-ledger/include/bat/ledger/mojom_structs.h b/vendor/bat-native-ledger/include/bat/ledger/mojom_structs.h index 65f5b7b13a59..171052236a67 100644 --- a/vendor/bat-native-ledger/include/bat/ledger/mojom_structs.h +++ b/vendor/bat-native-ledger/include/bat/ledger/mojom_structs.h @@ -138,6 +138,8 @@ using ServerPublisherInfoPtr = mojom::ServerPublisherInfoPtr; using ServerPublisherPartial = mojom::ServerPublisherPartial; using ServerPublisherPartialPtr = mojom::ServerPublisherPartialPtr; +using ServerPublisherPartialList = + std::vector; using TransferFee = mojom::TransferFee; using TransferFeePtr = mojom::TransferFeePtr; diff --git a/vendor/bat-native-ledger/include/bat/ledger/public/interfaces/ledger.mojom b/vendor/bat-native-ledger/include/bat/ledger/public/interfaces/ledger.mojom index 225d548dc55d..6ba49343f6e8 100644 --- a/vendor/bat-native-ledger/include/bat/ledger/public/interfaces/ledger.mojom +++ b/vendor/bat-native-ledger/include/bat/ledger/public/interfaces/ledger.mojom @@ -241,7 +241,10 @@ enum Result { ALREADY_EXISTS = 26, SAFETYNET_ATTESTATION_FAILED = 27, DATABASE_INIT_FAILED = 28, - RETRY = 29 + RETRY = 29, + RETRY_SHORT = 30, + RETRY_LONG = 31, + CONTINUE = 32 }; enum PublisherStatus { diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.cc index 11d1d1f0402c..a3fbaf8247d7 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.cc +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.cc @@ -56,7 +56,7 @@ void Publisher::OnTimer(uint32_t timer_id) { void Publisher::RefreshPublisher( const std::string& publisher_key, ledger::OnRefreshPublisherCallback callback) { - server_list_->Download(std::bind(&Publisher::OnRefreshPublisher, + server_list_->Start(std::bind(&Publisher::OnRefreshPublisher, this, _1, publisher_key, diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.h b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.h index 8102b5091295..0b57dfba7b53 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.h +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher.h @@ -28,10 +28,6 @@ namespace braveledger_publisher { class PublisherServerList; -using ParsePublisherListCallback = std::function; -using DownloadServerPublisherListCallback = - std::function; - class Publisher { public: explicit Publisher(bat_ledger::LedgerImpl* ledger); @@ -93,7 +89,7 @@ class Publisher { void ParsePublisherList( const std::string& data, - ParsePublisherListCallback callback); + ledger::ResultCallback callback); void getPublisherActivityFromUrl( uint64_t windowId, diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.cc index 05f948c58eb1..155963122aad 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.cc +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.cc @@ -11,6 +11,7 @@ #include "bat/ledger/internal/ledger_impl.h" #include "bat/ledger/internal/publisher/publisher_server_list.h" #include "bat/ledger/internal/state_keys.h" +#include "bat/ledger/internal/request/request_publisher.h" #include "bat/ledger/internal/request/request_util.h" #include "bat/ledger/internal/static_values.h" #include "bat/ledger/option_keys.h" @@ -21,6 +22,12 @@ using std::placeholders::_1; using std::placeholders::_2; using std::placeholders::_3; +namespace { + +const int kHardLimit = 100; + +} // namespace + namespace braveledger_publisher { PublisherServerList::PublisherServerList(bat_ledger::LedgerImpl* ledger) : @@ -34,19 +41,29 @@ PublisherServerList::~PublisherServerList() { void PublisherServerList::OnTimer(uint32_t timer_id) { if (timer_id == server_list_timer_id_) { server_list_timer_id_ = 0; - Download([](const ledger::Result _){}); + Start([](const ledger::Result _){}); + } +} + +void PublisherServerList::Start(ledger::ResultCallback callback) { + if (in_progress_) { + BLOG(ledger_, ledger::LogLevel::LOG_INFO) << "Publisher list in progress"; + callback(ledger::Result::LEDGER_OK); + return; } + + in_progress_ = true; + current_page_ = 1; + + Download(callback); } -void PublisherServerList::Download( - DownloadServerPublisherListCallback callback) { +void PublisherServerList::Download(ledger::ResultCallback callback) { std::vector headers; headers.push_back("Accept-Encoding: gzip"); - const std::string url = braveledger_request_util::BuildUrl( - GET_PUBLISHERS_LIST, - "", - braveledger_request_util::ServerTypes::PUBLISHER_DISTRO); + const std::string url = + braveledger_request_util::GetPublisherListUrl(current_page_); const ledger::LoadURLCallback download_callback = std::bind( &PublisherServerList::OnDownload, @@ -69,13 +86,20 @@ void PublisherServerList::OnDownload( int response_status_code, const std::string& response, const std::map& headers, - DownloadServerPublisherListCallback callback) { + ledger::ResultCallback callback) { ledger_->LogResponse( __func__, response_status_code, "Publisher list", headers); + // we iterated through all pages + if (response_status_code == net::HTTP_NO_CONTENT) { + in_progress_ = false; + OnParsePublisherList(ledger::Result::LEDGER_OK, callback); + return; + } + if (response_status_code == net::HTTP_OK && !response.empty()) { const auto parse_callback = std::bind(&PublisherServerList::OnParsePublisherList, this, _1, callback); @@ -90,15 +114,22 @@ void PublisherServerList::OnDownload( void PublisherServerList::OnParsePublisherList( const ledger::Result result, - DownloadServerPublisherListCallback callback) { + ledger::ResultCallback callback) { + if (result == ledger::Result::CONTINUE && current_page_ < kHardLimit) { + current_page_++; + Download(callback); + return; + } + uint64_t new_time = 0ull; - if (result == ledger::Result::LEDGER_OK) { + if (result != ledger::Result::LEDGER_ERROR) { ledger_->ContributeUnverifiedPublishers(); new_time = braveledger_time_util::GetCurrentTimeStamp(); } ledger_->SetUint64State(ledger::kStateServerPublisherListStamp, new_time); + in_progress_ = false; bool retry_after_error = result != ledger::Result::LEDGER_OK; SetTimer(retry_after_error); @@ -153,7 +184,7 @@ uint64_t PublisherServerList::GetTimerTime( ? 0ull : now_seconds - last_download; - uint64_t interval = + const uint64_t interval = ledger_->GetUint64Option(ledger::kOptionPublisherListRefreshInterval); if (now_seconds == last_download) { @@ -183,7 +214,7 @@ ledger::PublisherStatus PublisherServerList::ParsePublisherStatus( void PublisherServerList::ParsePublisherList( const std::string& data, - ParsePublisherListCallback callback) { + ledger::ResultCallback callback) { auto list_publisher = std::make_shared>(); auto list_banner = std::make_shared>(); @@ -234,18 +265,29 @@ void PublisherServerList::ParsePublisherList( } if (list_publisher->empty()) { + BLOG(ledger_, ledger::LogLevel::LOG_ERROR) << "Publisher list is empty"; callback(ledger::Result::LEDGER_ERROR); return; } - auto clear_callback = std::bind(&PublisherServerList::SaveParsedData, + // we need to clear table when we process first page, but only once + if (current_page_ == 1) { + auto clear_callback = std::bind(&PublisherServerList::SaveParsedData, this, _1, list_publisher, list_banner, callback); - ledger_->ClearServerPublisherList(clear_callback); + ledger_->ClearServerPublisherList(clear_callback); + return; + } + + SaveParsedData( + ledger::Result::LEDGER_OK, + list_publisher, + list_banner, + callback); } void PublisherServerList::ParsePublisherBanner( @@ -299,91 +341,63 @@ void PublisherServerList::SaveParsedData( const ledger::Result result, const SharedServerPublisherPartial& list_publisher, const SharedPublisherBanner& list_banner, - ParsePublisherListCallback callback) { + ledger::ResultCallback callback) { if (result != ledger::Result::LEDGER_OK) { + BLOG(ledger_, ledger::LogLevel::LOG_ERROR) << "DB was not cleared"; callback(result); return; } - if (list_publisher && !list_publisher->empty()) { - SavePublishers(list_publisher, list_banner, callback); + if (!list_publisher || list_publisher->empty()) { + BLOG(ledger_, ledger::LogLevel::LOG_ERROR) << "Publisher list is null"; + callback(ledger::Result::LEDGER_ERROR); return; } - if (list_banner && !list_banner->empty()) { - SaveBanners(list_banner, callback); - return; - } + auto save_callback = std::bind(&PublisherServerList::SaveBanners, + this, + _1, + list_banner, + callback); - callback(ledger::Result::LEDGER_OK); + ledger_->InsertServerPublisherList(*list_publisher, save_callback); } -void PublisherServerList::SavePublishers( - const SharedServerPublisherPartial& list_publisher, +void PublisherServerList::SaveBanners( + const ledger::Result result, const SharedPublisherBanner& list_banner, - ParsePublisherListCallback callback) { - if (!list_publisher) { - callback(ledger::Result::LEDGER_OK); + ledger::ResultCallback callback) { + if (!list_banner || result != ledger::Result::LEDGER_OK) { + BLOG(ledger_, ledger::LogLevel::LOG_ERROR) << + "Publisher list was not saved"; + callback(ledger::Result::LEDGER_ERROR); return; } - const int max_insert_records_ = 100000; - - int32_t interval = max_insert_records_; - const auto list_size = list_publisher->size(); - if (list_size < max_insert_records_) { - interval = list_size; + if (list_banner->empty()) { + callback(ledger::Result::CONTINUE); + return; } - std::vector save_list( - list_publisher->begin(), - list_publisher->begin() + interval); - auto new_list_publisher = - std::make_shared>( - list_publisher->begin() + interval, - list_publisher->end()); - - auto save_callback = std::bind(&PublisherServerList::SaveParsedData, + auto save_callback = std::bind(&PublisherServerList::BannerSaved, this, _1, - new_list_publisher, - list_banner, callback); - ledger_->InsertServerPublisherList(save_list, save_callback); + ledger_->InsertPublisherBannerList(*list_banner, save_callback); } -void PublisherServerList::SaveBanners( - const SharedPublisherBanner& list_banner, - ParsePublisherListCallback callback) { - if (!list_banner) { - callback(ledger::Result::LEDGER_OK); +void PublisherServerList::BannerSaved( + const ledger::Result result, + ledger::ResultCallback callback) { + if (result == ledger::Result::LEDGER_OK) { + callback(ledger::Result::CONTINUE); return; } - const int max_insert_records_ = 80000; - - int32_t interval = max_insert_records_; - const auto list_size = list_banner->size(); - if (list_size < max_insert_records_) { - interval = list_size; - } - std::vector save_list( - list_banner->begin(), - list_banner->begin() + interval); - auto new_list_banner = std::make_shared>( - list_banner->begin() + interval, - list_banner->end()); - - auto save_callback = std::bind(&PublisherServerList::SaveParsedData, - this, - _1, - nullptr, - new_list_banner, - callback); - - ledger_->InsertPublisherBannerList(save_list, save_callback); + BLOG(ledger_, ledger::LogLevel::LOG_ERROR) << "Banners were not saved"; + callback(result); } void PublisherServerList::ClearTimer() { diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.h b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.h index 201946997e55..db49edfada33 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.h +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/publisher/publisher_server_list.h @@ -33,7 +33,7 @@ class PublisherServerList { explicit PublisherServerList(bat_ledger::LedgerImpl* ledger); ~PublisherServerList(); - void Download(DownloadServerPublisherListCallback callback); + void Start(ledger::ResultCallback callback); // Called when timer is triggered void OnTimer(uint32_t timer_id); @@ -43,15 +43,17 @@ class PublisherServerList { void ClearTimer(); private: + void Download(ledger::ResultCallback callback); + void OnDownload( int response_status_code, const std::string& response, const std::map& headers, - DownloadServerPublisherListCallback callback); + ledger::ResultCallback callback); void OnParsePublisherList( const ledger::Result result, - DownloadServerPublisherListCallback callback); + ledger::ResultCallback callback); uint64_t GetTimerTime( bool retry_after_error, @@ -61,7 +63,7 @@ class PublisherServerList { void ParsePublisherList( const std::string& data, - ParsePublisherListCallback callback); + ledger::ResultCallback callback); void ParsePublisherBanner( ledger::PublisherBanner* banner, @@ -71,19 +73,21 @@ class PublisherServerList { const ledger::Result result, const SharedServerPublisherPartial& list_publisher, const SharedPublisherBanner& list_banner, - ParsePublisherListCallback callback); - - void SavePublishers( - const SharedServerPublisherPartial& list_publisher, - const SharedPublisherBanner& list_banner, - ParsePublisherListCallback callback); + ledger::ResultCallback callback); void SaveBanners( + const ledger::Result result, const SharedPublisherBanner& list_banner, - ParsePublisherListCallback callback); + ledger::ResultCallback callback); + + void BannerSaved( + const ledger::Result result, + ledger::ResultCallback callback); bat_ledger::LedgerImpl* ledger_; // NOT OWNED uint32_t server_list_timer_id_; + bool in_progress_ = false; + uint32_t current_page_ = 1; }; } // namespace braveledger_publisher diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.cc new file mode 100644 index 000000000000..58e21bc2fa8d --- /dev/null +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.cc @@ -0,0 +1,21 @@ +/* Copyright (c) 2020 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/. */ + +#include "base/strings/stringprintf.h" +#include "bat/ledger/internal/static_values.h" +#include "bat/ledger/internal/request/request_publisher.h" +#include "bat/ledger/internal/request/request_util.h" + +namespace braveledger_request_util { + +std::string GetPublisherListUrl(const uint32_t page) { + const std::string path = base::StringPrintf( + "/api/v3/public/channels?page=%d", + page); + + return BuildUrl(path, "", ServerTypes::kPublisher); +} + +} // namespace braveledger_request_util diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.h b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.h new file mode 100644 index 000000000000..55504e2d2a7b --- /dev/null +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_publisher.h @@ -0,0 +1,19 @@ +/* Copyright (c) 2020 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 BRAVELEDGER_COMMON_REQUEST_PUBLISHER_H_ +#define BRAVELEDGER_COMMON_REQUEST_PUBLISHER_H_ + +#include + +#include "bat/ledger/mojom_structs.h" + +namespace braveledger_request_util { + +std::string GetPublisherListUrl(const uint32_t page); + +} // namespace braveledger_request_util + +#endif // BRAVELEDGER_COMMON_REQUEST_PUBLISHER_H_ diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.cc index f1736585212d..06d4314f9261 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.cc +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.cc @@ -47,23 +47,6 @@ std::string BuildPublisherUrl() { return url; } -std::string BuildPublisherDistroUrl() { - std::string url; - switch (ledger::_environment) { - case ledger::Environment::STAGING: - url = PUBLISHER_DISTRO_STAGING_SERVER; - break; - case ledger::Environment::PRODUCTION: - url = PUBLISHER_DISTRO_PRODUCTION_SERVER; - break; - case ledger::Environment::DEVELOPMENT: - url = PUBLISHER_DISTRO_DEVELOPMENT_SERVER; - break; - } - - return url; -} - std::string BuildLedgerUrl() { std::string url; switch (ledger::_environment) { @@ -115,14 +98,10 @@ std::string BuildUrl( url = BuildBalanceUrl(); break; } - case ServerTypes::PUBLISHER: { + case ServerTypes::kPublisher: { url = BuildPublisherUrl(); break; } - case ServerTypes::PUBLISHER_DISTRO: { - url = BuildPublisherDistroUrl(); - break; - } case ServerTypes::LEDGER: { url = BuildLedgerUrl(); break; diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.h b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.h index e786e2198caf..c024983e4e81 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.h +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/request/request_util.h @@ -14,8 +14,7 @@ namespace braveledger_request_util { enum class ServerTypes { LEDGER, BALANCE, - PUBLISHER, - PUBLISHER_DISTRO, + kPublisher, kPromotion }; diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/static_values.h b/vendor/bat-native-ledger/src/bat/ledger/internal/static_values.h index 491cf1c4555d..3a1bd88b0ca1 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/static_values.h +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/static_values.h @@ -24,18 +24,11 @@ #define BALANCE_DEVELOPMENT_SERVER \ "https://balance.rewards.brave.software" -#define PUBLISHER_STAGING_SERVER \ -"https://publishers-staging.basicattentiontoken.org" -#define PUBLISHER_PRODUCTION_SERVER \ -"https://publishers.basicattentiontoken.org" -#define PUBLISHER_DEVELOPMENT_SERVER \ -"https://creators.brave.software" - -#define PUBLISHER_DISTRO_STAGING_SERVER \ +#define PUBLISHER_STAGING_SERVER \ "https://publishers-staging-distro.basicattentiontoken.org" -#define PUBLISHER_DISTRO_PRODUCTION_SERVER \ +#define PUBLISHER_PRODUCTION_SERVER \ "https://publishers-distro.basicattentiontoken.org" -#define PUBLISHER_DISTRO_DEVELOPMENT_SERVER \ +#define PUBLISHER_DEVELOPMENT_SERVER \ "https://creators-distro.brave.software" #define PROMOTION_STAGING_SERVER \ @@ -63,7 +56,6 @@ "/v2/publisher/ruleset?limit=512&excludedOnly=false" #define RECOVER_WALLET_PUBLIC_KEY "/wallet?publicKey=" #define GET_PROMOTION_CAPTCHA "/captchas/" -#define GET_PUBLISHERS_LIST "/api/v3/public/channels" #define GET_PROMOTION_ATTESTATION "/attestations/" #define REGISTRARVK_FIELDNAME "registrarVK"