Commit cb5d7fc0 authored by fhorschig's avatar fhorschig Committed by Commit bot

NTP: Extract JSON requests from Fetcher.

This CL does not change any behavior.

Moving internal classes into an internal namespace.
This simplfies testing these classes (JsonRequest and its Builder) and
prevents the use of internal enums (e.g. FetchResults) that were
publicly exposed but not intended for public use.

BUG=672422

Review-Url: https://codereview.chromium.org/2578173002
Cr-Commit-Position: refs/heads/master@{#439780}
parent 27420bcb
...@@ -296,19 +296,10 @@ void SnippetsInternalsMessageHandler::SendAllContent() { ...@@ -296,19 +296,10 @@ void SnippetsInternalsMessageHandler::SendAllContent() {
SendLastRemoteSuggestionsBackgroundFetchTime(); SendLastRemoteSuggestionsBackgroundFetchTime();
if (remote_suggestions_provider_) { if (remote_suggestions_provider_) {
switch ( // TODO(fhorschig): Read this string from variations directly.
remote_suggestions_provider_->snippets_fetcher()->personalization()) { SendString("switch-personalized",
case ntp_snippets::NTPSnippetsFetcher::Personalization::kPersonal: remote_suggestions_provider_->snippets_fetcher()
SendString("switch-personalized", "Only personalized"); ->PersonalizationModeString());
break;
case ntp_snippets::NTPSnippetsFetcher::Personalization::kBoth:
SendString("switch-personalized",
"Both personalized and non-personalized");
break;
case ntp_snippets::NTPSnippetsFetcher::Personalization::kNonPersonal:
SendString("switch-personalized", "Only non-personalized");
break;
}
SendString( SendString(
"switch-fetch-url", "switch-fetch-url",
......
...@@ -49,6 +49,10 @@ static_library("ntp_snippets") { ...@@ -49,6 +49,10 @@ static_library("ntp_snippets") {
"remote/ntp_snippet.h", "remote/ntp_snippet.h",
"remote/ntp_snippets_fetcher.cc", "remote/ntp_snippets_fetcher.cc",
"remote/ntp_snippets_fetcher.h", "remote/ntp_snippets_fetcher.h",
"remote/ntp_snippets_json_request.cc",
"remote/ntp_snippets_json_request.h",
"remote/ntp_snippets_request_params.cc",
"remote/ntp_snippets_request_params.h",
"remote/ntp_snippets_scheduler.h", "remote/ntp_snippets_scheduler.h",
"remote/remote_suggestions_database.cc", "remote/remote_suggestions_database.cc",
"remote/remote_suggestions_database.h", "remote/remote_suggestions_database.h",
...@@ -124,6 +128,7 @@ source_set("unit_tests") { ...@@ -124,6 +128,7 @@ source_set("unit_tests") {
"physical_web_pages/physical_web_page_suggestions_provider_unittest.cc", "physical_web_pages/physical_web_page_suggestions_provider_unittest.cc",
"remote/ntp_snippet_unittest.cc", "remote/ntp_snippet_unittest.cc",
"remote/ntp_snippets_fetcher_unittest.cc", "remote/ntp_snippets_fetcher_unittest.cc",
"remote/ntp_snippets_json_request_unittest.cc",
"remote/remote_suggestions_database_unittest.cc", "remote/remote_suggestions_database_unittest.cc",
"remote/remote_suggestions_provider_unittest.cc", "remote/remote_suggestions_provider_unittest.cc",
"remote/remote_suggestions_status_service_unittest.cc", "remote/remote_suggestions_status_service_unittest.cc",
......
This diff is collapsed.
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_JSON_REQUEST_H_
#define COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_JSON_REQUEST_H_
#include <memory>
#include <string>
#include <utility>
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "components/ntp_snippets/remote/ntp_snippets_request_params.h"
#include "components/ntp_snippets/status.h"
#include "components/translate/core/browser/language_model.h"
#include "google_apis/gaia/oauth2_token_service.h"
#include "net/http/http_request_headers.h"
namespace base {
class Value;
class TickClock;
} // namespace base
class FetchAPI;
class Personalization;
namespace ntp_snippets {
class UserClassifier;
namespace internal {
// Enumeration listing all possible outcomes for fetch attempts. Used for UMA
// histograms, so do not change existing values. Insert new values at the end,
// and update the histogram definition.
enum class FetchResult {
SUCCESS,
DEPRECATED_EMPTY_HOSTS,
URL_REQUEST_STATUS_ERROR,
HTTP_ERROR,
JSON_PARSE_ERROR,
INVALID_SNIPPET_CONTENT_ERROR,
OAUTH_TOKEN_ERROR,
INTERACTIVE_QUOTA_ERROR,
NON_INTERACTIVE_QUOTA_ERROR,
RESULT_MAX
};
enum FetchAPI {
CHROME_READER_API,
CHROME_CONTENT_SUGGESTIONS_API,
};
// A single request to query snippets.
class NTPSnippetsJsonRequest : public net::URLFetcherDelegate {
public:
// A client can expect error_details only, if there was any error during the
// fetching or parsing. In successful cases, it will be an empty string.
using CompletedCallback =
base::OnceCallback<void(std::unique_ptr<base::Value> result,
FetchResult result_code,
const std::string& error_details)>;
// Builds authenticated and non-authenticated NTPSnippetsJsonRequests.
class Builder {
public:
Builder();
Builder(Builder&&);
~Builder();
// Builds a Request object that contains all data to fetch new snippets.
std::unique_ptr<NTPSnippetsJsonRequest> Build() const;
Builder& SetAuthentication(const std::string& account_id,
const std::string& auth_header);
Builder& SetCreationTime(base::TimeTicks creation_time);
Builder& SetFetchAPI(FetchAPI fetch_api);
// The language_model borrowed from the fetcher needs to stay alive until
// the request body is built.
Builder& SetLanguageModel(const translate::LanguageModel* language_model);
Builder& SetParams(const NTPSnippetsRequestParams& params);
Builder& SetParseJsonCallback(ParseJSONCallback callback);
Builder& SetPersonalization(Personalization personalization);
// The tick_clock borrowed from the fetcher will be injected into the
// request. It will be used at build time and after the fetch returned.
// It has to be alive until the request is destroyed.
Builder& SetTickClock(base::TickClock* tick_clock);
Builder& SetUrl(const GURL& url);
Builder& SetUrlRequestContextGetter(
const scoped_refptr<net::URLRequestContextGetter>& context_getter);
Builder& SetUserClassifier(const UserClassifier& user_classifier);
// These preview methods allow to inspect the Request without exposing it
// publicly.
// TODO(fhorschig): Remove these when moving the Builder to
// snippets::internal and trigger the request to intercept the request.
std::string PreviewRequestBodyForTesting() { return BuildBody(); }
std::string PreviewRequestHeadersForTesting() { return BuildHeaders(); }
Builder& SetUserClassForTesting(const std::string& user_class) {
user_class_ = user_class;
return *this;
}
private:
std::string BuildHeaders() const;
std::string BuildBody() const;
std::unique_ptr<net::URLFetcher> BuildURLFetcher(
net::URLFetcherDelegate* request,
const std::string& headers,
const std::string& body) const;
bool ReturnOnlyPersonalizedResults() const {
return !obfuscated_gaia_id_.empty() &&
personalization_ == Personalization::kPersonal;
}
void PrepareLanguages(
translate::LanguageModel::LanguageInfo* ui_language,
translate::LanguageModel::LanguageInfo* other_top_language) const;
// Only required, if the request needs to be sent.
std::string auth_header_;
base::TickClock* tick_clock_;
FetchAPI fetch_api_;
NTPSnippetsRequestParams params_;
ParseJSONCallback parse_json_callback_;
Personalization personalization_;
GURL url_;
scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
// Optional properties.
std::string obfuscated_gaia_id_;
std::string user_class_;
const translate::LanguageModel* language_model_;
DISALLOW_COPY_AND_ASSIGN(Builder);
};
NTPSnippetsJsonRequest(base::Optional<Category> exclusive_category,
base::TickClock* tick_clock,
const ParseJSONCallback& callback);
NTPSnippetsJsonRequest(NTPSnippetsJsonRequest&&);
~NTPSnippetsJsonRequest() override;
void Start(CompletedCallback callback);
const base::Optional<Category>& exclusive_category() const {
return exclusive_category_;
}
base::TimeDelta GetFetchDuration() const;
std::string GetResponseString() const;
private:
// URLFetcherDelegate implementation.
void OnURLFetchComplete(const net::URLFetcher* source) override;
void ParseJsonResponse();
void OnJsonParsed(std::unique_ptr<base::Value> result);
void OnJsonError(const std::string& error);
// The fetcher for downloading the snippets. Only non-null if a fetch is
// currently ongoing.
std::unique_ptr<net::URLFetcher> url_fetcher_;
// If set, only return results for this category.
base::Optional<Category> exclusive_category_;
// Use the TickClock from the Fetcher to measure the fetch time. It will be
// used on creation and after the fetch returned. It has to be alive until the
// request is destroyed.
base::TickClock* tick_clock_;
base::TimeTicks creation_time_;
// This callback is called to parse a json string. It contains callbacks for
// error and success cases.
ParseJSONCallback parse_json_callback_;
// The callback to notify when URLFetcher finished and results are available.
CompletedCallback request_completed_callback_;
base::WeakPtrFactory<NTPSnippetsJsonRequest> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(NTPSnippetsJsonRequest);
};
} // namespace internal
} // namespace ntp_snippets
#endif // COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_JSON_REQUEST_H_
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/ntp_snippets/remote/ntp_snippets_request_params.h"
namespace ntp_snippets {
NTPSnippetsRequestParams::NTPSnippetsRequestParams() = default;
NTPSnippetsRequestParams::NTPSnippetsRequestParams(
const NTPSnippetsRequestParams&) = default;
NTPSnippetsRequestParams::~NTPSnippetsRequestParams() = default;
} // namespace ntp_snippets
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_REQUEST_PARAMS_H_
#define COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_REQUEST_PARAMS_H_
#include <memory>
#include <set>
#include <string>
#include "base/callback.h"
#include "base/optional.h"
#include "base/values.h"
#include "components/ntp_snippets/category.h"
namespace ntp_snippets {
// Enumeration listing all possible variants of dealing with personalization.
enum class Personalization { kPersonal, kNonPersonal, kBoth };
// Contains all parameters for fetching NTPSnippets.
struct NTPSnippetsRequestParams {
NTPSnippetsRequestParams();
NTPSnippetsRequestParams(const NTPSnippetsRequestParams&);
~NTPSnippetsRequestParams();
// BCP 47 language code specifying the user's UI language.
std::string language_code;
// A set of suggestion IDs that should not be returned again.
std::set<std::string> excluded_ids;
// Maximum number of snippets to fetch.
int count_to_fetch = 0;
// Whether this is an interactive request, i.e. triggered by an explicit
// user action. Typically, non-interactive requests are subject to a daily
// quota.
bool interactive_request = false;
// If set, only return results for this category.
base::Optional<Category> exclusive_category;
};
// Callbacks for JSON parsing to allow injecting platform-dependent code.
using SuccessCallback =
base::Callback<void(std::unique_ptr<base::Value> result)>;
using ErrorCallback = base::Callback<void(const std::string& error)>;
using ParseJSONCallback =
base::Callback<void(const std::string& raw_json_string,
const SuccessCallback& success_callback,
const ErrorCallback& error_callback)>;
} // namespace ntp_snippets
#endif // COMPONENTS_NTP_SNIPPETS_REMOTE_NTP_SNIPPETS_REQUEST_PARAMS_H_
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
#include "components/ntp_snippets/category_rankers/category_ranker.h" #include "components/ntp_snippets/category_rankers/category_ranker.h"
#include "components/ntp_snippets/features.h" #include "components/ntp_snippets/features.h"
#include "components/ntp_snippets/pref_names.h" #include "components/ntp_snippets/pref_names.h"
#include "components/ntp_snippets/remote/ntp_snippets_request_params.h"
#include "components/ntp_snippets/remote/remote_suggestions_database.h" #include "components/ntp_snippets/remote/remote_suggestions_database.h"
#include "components/ntp_snippets/switches.h" #include "components/ntp_snippets/switches.h"
#include "components/ntp_snippets/user_classifier.h" #include "components/ntp_snippets/user_classifier.h"
...@@ -416,7 +417,7 @@ void RemoteSuggestionsProvider::FetchSnippets( ...@@ -416,7 +417,7 @@ void RemoteSuggestionsProvider::FetchSnippets(
MarkEmptyCategoriesAsLoading(); MarkEmptyCategoriesAsLoading();
NTPSnippetsFetcher::Params params = BuildFetchParams(); NTPSnippetsRequestParams params = BuildFetchParams();
params.interactive_request = interactive_request; params.interactive_request = interactive_request;
snippets_fetcher_->FetchSnippets( snippets_fetcher_->FetchSnippets(
params, base::BindOnce(&RemoteSuggestionsProvider::OnFetchFinished, params, base::BindOnce(&RemoteSuggestionsProvider::OnFetchFinished,
...@@ -433,7 +434,7 @@ void RemoteSuggestionsProvider::Fetch( ...@@ -433,7 +434,7 @@ void RemoteSuggestionsProvider::Fetch(
"RemoteSuggestionsProvider is not ready!")); "RemoteSuggestionsProvider is not ready!"));
return; return;
} }
NTPSnippetsFetcher::Params params = BuildFetchParams(); NTPSnippetsRequestParams params = BuildFetchParams();
params.excluded_ids.insert(known_suggestion_ids.begin(), params.excluded_ids.insert(known_suggestion_ids.begin(),
known_suggestion_ids.end()); known_suggestion_ids.end());
params.interactive_request = true; params.interactive_request = true;
...@@ -445,8 +446,8 @@ void RemoteSuggestionsProvider::Fetch( ...@@ -445,8 +446,8 @@ void RemoteSuggestionsProvider::Fetch(
} }
// Builds default fetcher params. // Builds default fetcher params.
NTPSnippetsFetcher::Params RemoteSuggestionsProvider::BuildFetchParams() const { NTPSnippetsRequestParams RemoteSuggestionsProvider::BuildFetchParams() const {
NTPSnippetsFetcher::Params result; NTPSnippetsRequestParams result;
result.language_code = application_language_code_; result.language_code = application_language_code_;
result.count_to_fetch = kMaxSnippetCount; result.count_to_fetch = kMaxSnippetCount;
for (const auto& map_entry : category_contents_) { for (const auto& map_entry : category_contents_) {
......
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "components/ntp_snippets/content_suggestions_provider.h" #include "components/ntp_snippets/content_suggestions_provider.h"
#include "components/ntp_snippets/remote/ntp_snippet.h" #include "components/ntp_snippets/remote/ntp_snippet.h"
#include "components/ntp_snippets/remote/ntp_snippets_fetcher.h" #include "components/ntp_snippets/remote/ntp_snippets_fetcher.h"
#include "components/ntp_snippets/remote/ntp_snippets_request_params.h"
#include "components/ntp_snippets/remote/ntp_snippets_scheduler.h" #include "components/ntp_snippets/remote/ntp_snippets_scheduler.h"
#include "components/ntp_snippets/remote/remote_suggestions_status_service.h" #include "components/ntp_snippets/remote/remote_suggestions_status_service.h"
#include "components/ntp_snippets/remote/request_throttler.h" #include "components/ntp_snippets/remote/request_throttler.h"
...@@ -148,6 +149,9 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider { ...@@ -148,6 +149,9 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// and request lower latency processing. // and request lower latency processing.
void FetchSnippetsForAllCategories(); void FetchSnippetsForAllCategories();
// Only used in tests and for debugging in snippets-internal/.
// TODO(fhorschig): Remove this getter when there is an interface for the
// fetcher that allows better mocks.
const NTPSnippetsFetcher* snippets_fetcher() const { const NTPSnippetsFetcher* snippets_fetcher() const {
return snippets_fetcher_.get(); return snippets_fetcher_.get();
} }
...@@ -393,7 +397,7 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider { ...@@ -393,7 +397,7 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
void RestoreCategoriesFromPrefs(); void RestoreCategoriesFromPrefs();
void StoreCategoriesToPrefs(); void StoreCategoriesToPrefs();
NTPSnippetsFetcher::Params BuildFetchParams() const; NTPSnippetsRequestParams BuildFetchParams() const;
void MarkEmptyCategoriesAsLoading(); void MarkEmptyCategoriesAsLoading();
......
...@@ -305,10 +305,9 @@ gfx::Image FetchImage(RemoteSuggestionsProvider* service, ...@@ -305,10 +305,9 @@ gfx::Image FetchImage(RemoteSuggestionsProvider* service,
return result; return result;
} }
void ParseJson( void ParseJson(const std::string& json,
const std::string& json, const SuccessCallback& success_callback,
const ntp_snippets::NTPSnippetsFetcher::SuccessCallback& success_callback, const ErrorCallback& error_callback) {
const ntp_snippets::NTPSnippetsFetcher::ErrorCallback& error_callback) {
base::JSONReader json_reader; base::JSONReader json_reader;
std::unique_ptr<base::Value> value = json_reader.ReadToValue(json); std::unique_ptr<base::Value> value = json_reader.ReadToValue(json);
if (value) { if (value) {
......
...@@ -59,8 +59,8 @@ using suggestions::ImageFetcherImpl; ...@@ -59,8 +59,8 @@ using suggestions::ImageFetcherImpl;
namespace { namespace {
void ParseJson(const std::string& json, void ParseJson(const std::string& json,
const NTPSnippetsFetcher::SuccessCallback& success_callback, const ntp_snippets::SuccessCallback& success_callback,
const NTPSnippetsFetcher::ErrorCallback& error_callback) { const ntp_snippets::ErrorCallback& error_callback) {
base::JSONReader json_reader; base::JSONReader json_reader;
std::unique_ptr<base::Value> value = json_reader.ReadToValue(json); std::unique_ptr<base::Value> value = json_reader.ReadToValue(json);
if (value) { if (value) {
......
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