[NTP::SectionOrder] Replace CategoryFactory with a category ranker.

components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc

Index: components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc b/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
index 378ff2fef7602655ad364333e7bfe08198afc4c1..1a916336ce11a6ac4fdbe73101b29fb16b9e2630 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_unittest.cc
@@ -26,8 +26,10 @@
 #include "components/image_fetcher/image_decoder.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "components/image_fetcher/image_fetcher_delegate.h"
-#include "components/ntp_snippets/category_factory.h"
+#include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_info.h"
+#include "components/ntp_snippets/category_rankers/category_ranker.h"
+#include "components/ntp_snippets/category_rankers/constant_category_ranker.h"
 #include "components/ntp_snippets/ntp_snippets_constants.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/ntp_snippet.h"
@@ -116,44 +118,64 @@ base::Time GetDefaultExpirationTime() {
   return base::Time::Now() + base::TimeDelta::FromHours(1);
 }
 
-std::string GetTestJson(const std::vector<std::string>& snippets,
-                        const std::string& category_title) {
+std::string GetCategoryJson(const std::vector<std::string>& snippets,
+                            int remote_category_id,
+                            const std::string& category_title) {
   return base::StringPrintf(
-      "{\n"
-      "  \"categories\": [{\n"
-      "    \"id\": 1,\n"
+      "  {\n"
+      "    \"id\": %d,\n"
       "    \"localizedTitle\": \"%s\",\n"
       "    \"suggestions\": [%s]\n"
-      "  }]\n"
-      "}\n",
-      category_title.c_str(), base::JoinString(snippets, ", ").c_str());
+      "  }\n",
+      remote_category_id, category_title.c_str(),
+      base::JoinString(snippets, ", ").c_str());
 }
 
-std::string GetTestJson(const std::vector<std::string>& snippets) {
-  return GetTestJson(snippets, kTestJsonDefaultCategoryTitle);
+class MutliCategoryJsonBuilder {
+ public:
+  MutliCategoryJsonBuilder() {}
+
+  MutliCategoryJsonBuilder& AddCategoryWithCustomTitle(
+      const std::vector<std::string>& snippets,
+      int remote_category_id,
+      const std::string& category_title) {
+    category_json.push_back(
+        GetCategoryJson(snippets, remote_category_id, category_title));
+    return *this;
+  }
+
+  MutliCategoryJsonBuilder& AddCategory(
+      const std::vector<std::string>& snippets,
+      int remote_category_id) {
+    return AddCategoryWithCustomTitle(
+        snippets, remote_category_id,
+        "Title" + base::IntToString(remote_category_id));
+  }
+
+  std::string Build() {
+    return base::StringPrintf(
+        "{\n"
+        "  \"categories\": [\n"
+        "%s\n"
+        "  ]\n"
+        "}\n",
+        base::JoinString(category_json, "  ,\n").c_str());
+  }
+
+ private:
+  std::vector<std::string> category_json;
+};
+
+std::string GetTestJson(const std::vector<std::string>& snippets,
+                        const std::string& category_title) {
+  return MutliCategoryJsonBuilder()
+      .AddCategoryWithCustomTitle(snippets, /*remote_category_id=*/1,
+                                  category_title)
+      .Build();
 }
 
-// TODO(tschumann): Remove the default parameter other_id. It makes the tests
-// less explicit and hard to read. Also get rid of the convenience
-// other_category() and unknown_category() helpers -- tests can just define
-// their own.
-std::string GetMultiCategoryJson(const std::vector<std::string>& articles,
-                                 const std::vector<std::string>& others,
-                                 int other_id = 2) {
-  return base::StringPrintf(
-      "{\n"
-      "  \"categories\": [{\n"
-      "    \"id\": 1,\n"
-      "    \"localizedTitle\": \"Articles for You\",\n"
-      "    \"suggestions\": [%s]\n"
-      "  }, {\n"
-      "    \"id\": %i,\n"
-      "    \"localizedTitle\": \"Other Things\",\n"
-      "    \"suggestions\": [%s]\n"
-      "  }]\n"
-      "}\n",
-      base::JoinString(articles, ", ").c_str(), other_id,
-      base::JoinString(others, ", ").c_str());
+std::string GetTestJson(const std::vector<std::string>& snippets) {
+  return GetTestJson(snippets, kTestJsonDefaultCategoryTitle);
 }
 
 std::string FormatTime(const base::Time& t) {
@@ -401,6 +423,7 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
         fake_url_fetcher_factory_(
             /*default_factory=*/&failing_url_fetcher_factory_),
         test_url_(kTestContentSuggestionsServerWithAPIKey),
+        category_ranker_(base::MakeUnique<ConstantCategoryRanker>()),
         user_classifier_(/*pref_service=*/nullptr),
         image_fetcher_(nullptr),
         image_decoder_(nullptr) {
@@ -436,9 +459,8 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
     std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher =
         base::MakeUnique<NTPSnippetsFetcher>(
             utils_.fake_signin_manager(), fake_token_service_.get(),
-            std::move(request_context_getter), utils_.pref_service(),
-            &category_factory_, nullptr, base::Bind(&ParseJson), kAPIKey,
-            &user_classifier_);
+            std::move(request_context_getter), utils_.pref_service(), nullptr,
+            base::Bind(&ParseJson), kAPIKey, &user_classifier_);
 
     utils_.fake_signin_manager()->SignIn("foo@bar.com");
 
@@ -451,7 +473,7 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
     EXPECT_FALSE(observer_);
     observer_ = base::MakeUnique<FakeContentSuggestionsProviderObserver>();
     return base::MakeUnique<RemoteSuggestionsProvider>(
-        observer_.get(), &category_factory_, utils_.pref_service(), "fr",
+        observer_.get(), utils_.pref_service(), "fr", category_ranker_.get(),
         &user_classifier_, &scheduler_, std::move(snippets_fetcher),
         std::move(image_fetcher), std::move(image_decoder),
         base::MakeUnique<RemoteSuggestionsDatabase>(database_dir_.GetPath(),
@@ -478,26 +500,37 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
   void ResetSnippetsService(
       std::unique_ptr<RemoteSuggestionsProvider>* service) {
     service->reset();
+    category_ranker_ = base::MakeUnique<ConstantCategoryRanker>();
     observer_.reset();
     *service = MakeSnippetsService();
   }
 
+  std::vector<Category> SortCategories(std::vector<Category> categories) {
+    std::sort(categories.begin(), categories.end(),
+              [this](Category left, Category right) {
+                return category_ranker_->Compare(left, right);
+              });
+    return categories;
+  }
+
   ContentSuggestion::ID MakeArticleID(const std::string& id_within_category) {
     return ContentSuggestion::ID(articles_category(), id_within_category);
   }
 
   Category articles_category() {
-    return category_factory_.FromKnownCategory(KnownCategories::ARTICLES);
+    return Category::FromKnownCategory(KnownCategories::ARTICLES);
   }
 
   ContentSuggestion::ID MakeOtherID(const std::string& id_within_category) {
     return ContentSuggestion::ID(other_category(), id_within_category);
   }
 
-  Category other_category() { return category_factory_.FromRemoteCategory(2); }
+  // TODO(tschumann): Get rid of the convenience other_category() and
+  // unknown_category() helpers -- tests can just define their own.
+  Category other_category() { return Category::FromRemoteCategory(2); }
 
   Category unknown_category() {
-    return category_factory_.FromRemoteCategory(kUnknownRemoteCategoryId);
+    return Category::FromRemoteCategory(kUnknownRemoteCategoryId);
   }
 
  protected:
@@ -542,10 +575,10 @@ class RemoteSuggestionsProviderTest : public ::testing::Test {
   net::FakeURLFetcherFactory fake_url_fetcher_factory_;
   const GURL test_url_;
   std::unique_ptr<OAuth2TokenService> fake_token_service_;
+  std::unique_ptr<CategoryRanker> category_ranker_;
   UserClassifier user_classifier_;
   NiceMock<MockScheduler> scheduler_;
   std::unique_ptr<FakeContentSuggestionsProviderObserver> observer_;
-  CategoryFactory category_factory_;
   NiceMock<MockImageFetcher>* image_fetcher_;
   FakeImageDecoder* image_decoder_;
 
@@ -730,11 +763,12 @@ TEST_F(RemoteSuggestionsProviderTest, CategoryTitle) {
 }
 
 TEST_F(RemoteSuggestionsProviderTest, MultipleCategories) {
-  std::string json_str(
-      GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)}));
-
   auto service = MakeSnippetsService();
-
+  std::string json_str =
+      MutliCategoryJsonBuilder()
+          .AddCategory({GetSnippetN(0)}, /*remote_category_id=*/1)
+          .AddCategory({GetSnippetN(1)}, /*remote_category_id=*/2)
+          .Build();
   LoadFromJSONString(service.get(), json_str);
 
   ASSERT_THAT(observer().statuses(),
@@ -787,12 +821,15 @@ TEST_F(RemoteSuggestionsProviderTest, ArticleCategoryInfo) {
 
 TEST_F(RemoteSuggestionsProviderTest, ExperimentalCategoryInfo) {
   auto service = MakeSnippetsService();
-
+  std::string json_str =
+      MutliCategoryJsonBuilder()
+          .AddCategory({GetSnippetN(0)}, /*remote_category_id=*/1)
+          .AddCategory({GetSnippetN(1)}, kUnknownRemoteCategoryId)
+          .Build();
   // Load data with multiple categories so that a new experimental category gets
   // registered.
-  LoadFromJSONString(service.get(),
-                     GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)},
-                                          kUnknownRemoteCategoryId));
+  LoadFromJSONString(service.get(), json_str);
+
   CategoryInfo info = service->GetCategoryInfo(unknown_category());
   EXPECT_THAT(info.has_more_action(), Eq(false));
   EXPECT_THAT(info.has_reload_action(), Eq(false));
@@ -802,10 +839,15 @@ TEST_F(RemoteSuggestionsProviderTest, ExperimentalCategoryInfo) {
 
 TEST_F(RemoteSuggestionsProviderTest, PersistCategoryInfos) {
   auto service = MakeSnippetsService();
-
-  LoadFromJSONString(service.get(),
-                     GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)},
-                                          kUnknownRemoteCategoryId));
+  // TODO(vitaliii): Use |articles_category()| instead of constant ID below.
+  std::string json_str =
+      MutliCategoryJsonBuilder()
+          .AddCategoryWithCustomTitle(
+              {GetSnippetN(0)}, /*remote_category_id=*/1, "Articles for You")
+          .AddCategoryWithCustomTitle({GetSnippetN(1)},
+                                      kUnknownRemoteCategoryId, "Other Things")
+          .Build();
+  LoadFromJSONString(service.get(), json_str);
 
   ASSERT_EQ(observer().StatusForCategory(articles_category()),
             CategoryStatus::AVAILABLE);
@@ -840,11 +882,40 @@ TEST_F(RemoteSuggestionsProviderTest, PersistCategoryInfos) {
   EXPECT_EQ(info_unknown_before.title(), info_unknown_after.title());
 }
 
-TEST_F(RemoteSuggestionsProviderTest, PersistSuggestions) {
+TEST_F(RemoteSuggestionsProviderTest, PersistRemoteCategoryOrder) {
   auto service = MakeSnippetsService();
+  std::string json_str =
+      MutliCategoryJsonBuilder()
+          .AddCategory({GetSnippetN(0)}, /*remote_category_id=*/1)
+          .AddCategory({GetSnippetN(1)}, /*remote_category_id=*/3)
+          .AddCategory({GetSnippetN(2)}, /*remote_category_id=*/2)
+          .Build();
+  LoadFromJSONString(service.get(), json_str);
 
-  LoadFromJSONString(service.get(),
-                     GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)}));
+  std::vector<Category> unordered_categories = {
+      Category::FromRemoteCategory(1), Category::FromRemoteCategory(2),
+      Category::FromRemoteCategory(3)};
+  std::vector<Category> ordered_categories = {Category::FromRemoteCategory(1),
+                                              Category::FromRemoteCategory(3),
+                                              Category::FromRemoteCategory(2)};
+  ASSERT_EQ(ordered_categories, SortCategories(unordered_categories));
+
+  // Recreate the service to simulate a Chrome restart. The ranker does not
+  // persist the order.
+  ResetSnippetsService(&service);
+
+  // The categories should have been restored in the same order.
+  EXPECT_EQ(ordered_categories, SortCategories(unordered_categories));
+}
+
+TEST_F(RemoteSuggestionsProviderTest, PersistSuggestions) {
+  auto service = MakeSnippetsService();
+  std::string json_str =
+      MutliCategoryJsonBuilder()
+          .AddCategory({GetSnippetN(0)}, /*remote_category_id=*/1)
+          .AddCategory({GetSnippetN(2)}, /*remote_category_id=*/2)
+          .Build();
+  LoadFromJSONString(service.get(), json_str);
 
   ASSERT_THAT(observer().SuggestionsForCategory(articles_category()),
               SizeIs(1));
@@ -862,8 +933,14 @@ TEST_F(RemoteSuggestionsProviderTest, PersistSuggestions) {
 TEST_F(RemoteSuggestionsProviderTest, DontNotifyIfNotAvailable) {
   // Get some suggestions into the database.
   auto service = MakeSnippetsService();
-  LoadFromJSONString(service.get(),
-                     GetMultiCategoryJson({GetSnippetN(0)}, {GetSnippetN(1)}));
+  std::string json_str =
+      MutliCategoryJsonBuilder()
+          .AddCategory({GetSnippetN(0)},
+                       /*remote_category_id=*/1)
+          .AddCategory({GetSnippetN(1)}, /*remote_category_id=*/2)
+          .Build();
+  LoadFromJSONString(service.get(), json_str);
+
   ASSERT_THAT(observer().SuggestionsForCategory(articles_category()),
               SizeIs(1));
   ASSERT_THAT(observer().SuggestionsForCategory(other_category()), SizeIs(1));