Commit 947bd6b7 authored by Justin DeWitt's avatar Justin DeWitt Committed by Commit Bot

[EoS] Fetcher fixes

When testing end-to-end, found some problems with the Fetcher:
* POST is not accepted by the server, converted to GET (using query
  params)
* URL was not set in the request
* URL utils had a bug causing "/v1/" to be missing from the path.

Bug: 867488
Change-Id: Ie60010bc45ac67923d211bce2a84248186f2dbc0
Reviewed-on: https://chromium-review.googlesource.com/1247021Reviewed-by: default avatarCathy Li <chili@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594484}
parent 21b66f47
......@@ -41,7 +41,7 @@ namespace {
// Content type needed in order to communicate with the server in binary
// proto format.
const char kRequestContentType[] = "application/x-protobuf";
const char kRequestMethod[] = "POST";
const char kRequestMethod[] = "GET";
constexpr net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("explore_sites", R"(
......@@ -102,26 +102,26 @@ ExploreSitesFetcher::ExploreSitesFetcher(
version.components()[2], // Build
version.components()[3], // Patch
channel_name.c_str());
GURL final_url =
net::AppendOrReplaceQueryParameter(url, "country_code", country_code);
final_url = net::AppendOrReplaceQueryParameter(final_url, "version_token",
catalog_version);
auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->url = final_url;
resource_request->method = kRequestMethod;
bool is_stable_channel =
chrome::GetChannel() == version_info::Channel::STABLE;
std::string api_key = is_stable_channel ? google_apis::GetAPIKey()
: google_apis::GetNonStableAPIKey();
resource_request->headers.SetHeader("x-google-api-key", api_key);
resource_request->headers.SetHeader("x-goog-api-key", api_key);
resource_request->headers.SetHeader("X-Client-Version", client_version);
resource_request->headers.SetHeader("Content-Type", kRequestContentType);
// TODO(freedjm): Implement Accept-Language support.
url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
traffic_annotation);
GetCatalogRequest request;
request.set_version_token(catalog_version);
request.set_country_code(country_code);
std::string request_message;
request.SerializeToString(&request_message);
url_loader_->AttachStringForUpload(request_message, kRequestContentType);
url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
base::BindOnce(&ExploreSitesFetcher::OnSimpleLoaderComplete,
......
......@@ -38,6 +38,8 @@ class ExploreSitesFetcherTest : public testing::Test {
ExploreSitesRequestStatus RunFetcherWithData(const std::string& response_data,
std::string* data_received);
network::ResourceRequest last_resource_request;
private:
ExploreSitesFetcher::Callback StoreResult();
network::TestURLLoaderFactory::PendingRequest* GetPendingRequest(
......@@ -52,7 +54,6 @@ class ExploreSitesFetcherTest : public testing::Test {
network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::SharedURLLoaderFactory>
test_shared_url_loader_factory_;
network::ResourceRequest last_resource_request_;
ExploreSitesRequestStatus last_status_;
std::unique_ptr<std::string> last_data_;
......@@ -79,7 +80,8 @@ ExploreSitesFetcherTest::ExploreSitesFetcherTest()
void ExploreSitesFetcherTest::SetUp() {
test_url_loader_factory_.SetInterceptor(
base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
last_resource_request_ = request;
EXPECT_TRUE(request.url.is_valid() && !request.url.is_empty());
last_resource_request = request;
}));
}
......@@ -153,7 +155,7 @@ ExploreSitesRequestStatus ExploreSitesFetcherTest::RunFetcher(
base::OnceCallback<void(void)> respond_callback,
std::string* data_received) {
std::unique_ptr<ExploreSitesFetcher> fetcher =
ExploreSitesFetcher::CreateForGetCatalog(StoreResult(), "", "KE",
ExploreSitesFetcher::CreateForGetCatalog(StoreResult(), "123", "KE",
test_shared_url_loader_factory_);
std::move(respond_callback).Run();
......@@ -215,6 +217,10 @@ TEST_F(ExploreSitesFetcherTest, Success) {
RunFetcherWithData("Any data.", &data));
EXPECT_FALSE(data.empty());
EXPECT_EQ(data, "Any data.");
EXPECT_EQ(last_resource_request.url.spec(),
"https://exploresites-pa.googleapis.com/v1/"
"getcatalog?country_code=KE&version_token=123");
}
} // namespace explore_sites
......@@ -14,7 +14,7 @@ namespace explore_sites {
GURL GetBaseURL() {
const char kBaseURLOption[] = "base_url";
const char kDefaultBaseUrl[] = "https://exploresites-pa.googleapis.com/v1";
const char kDefaultBaseUrl[] = "https://exploresites-pa.googleapis.com";
std::string field_trial_param = base::GetFieldTrialParamValueByFeature(
chrome::android::kExploreSites, kBaseURLOption);
if (field_trial_param.empty())
......@@ -23,7 +23,7 @@ GURL GetBaseURL() {
}
GURL GetCatalogURL() {
const char kGetCatalogPath[] = "/getcatalog";
const char kGetCatalogPath[] = "/v1/getcatalog";
std::string path(kGetCatalogPath);
GURL base_url(GetBaseURL());
......@@ -33,7 +33,7 @@ GURL GetCatalogURL() {
}
GURL GetCategoriesURL() {
const char kNtpJsonPath[] = "/getcategories";
const char kNtpJsonPath[] = "/v1/getcategories";
std::string path(kNtpJsonPath);
GURL base_url(GetBaseURL());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment