Add favicon and publisher name to snippet cards

components/ntp_snippets/ntp_snippets_service_unittest.cc

 // Copyright 2015 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/ntp_snippets_service.h"
 
 #include <memory>
+#include <vector>
 
 #include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
@@ skipping 16 matching lines @@
 base::Time GetDefaultCreationTime() {
   return base::Time::FromUTCExploded(kDefaultCreationTime);
 }
 
 std::string GetTestJson(const std::string& content_creation_time_str,
                         const std::string& expiry_time_str) {
   char json_str_format[] =
       "{ \"recos\": [ "
       "{ \"contentInfo\": {"
       "\"url\" : \"http://localhost/foobar\","
-      "\"site_title\" : \"Site Title\","
-      "\"favicon_url\" : \"http://localhost/favicon\","
       "\"title\" : \"Title\","
       "\"snippet\" : \"Snippet\","
       "\"thumbnailUrl\" : \"http://localhost/salient_image\","
       "\"creationTimestampSec\" : \"%s\","
       "\"expiryTimestampSec\" : \"%s\","
       "\"sourceCorpusInfo\" : [ "
-      "{\"ampUrl\" : \"http://localhost/amp\"},"
-      "{\"corpusId\" : \"id\"}]"
+      "{\"ampUrl\" : \"http://localhost/amp\","
+      "\"corpusId\" : \"http://localhost/foobar\","
+      "\"publisherData\": { \"sourceName\" : \"Foo News\"}}]"
       "}}"
       "]}";
 
   return base::StringPrintf(json_str_format, content_creation_time_str.c_str(),
                             expiry_time_str.c_str());
 }
 
+std::string GetTestJsonWithSources(const std::string& content_creation_time_str,
+                                   const std::string& expiry_time_str,
+                                   const std::vector<std::string>& source_urls,
+                                   const std::vector<std::string>& publishers,
+                                   const std::vector<std::string>& amp_urls) {
+  char json_str_format[] =
+      "{ \"recos\": [ "
+      "{ \"contentInfo\": {"
+      "\"url\" : \"http://localhost/foobar\","
+      "\"title\" : \"Title\","
+      "\"snippet\" : \"Snippet\","
+      "\"thumbnailUrl\" : \"http://localhost/salient_image\","
+      "\"creationTimestampSec\" : \"%s\","
+      "\"expiryTimestampSec\" : \"%s\","
+      "\"sourceCorpusInfo\" : [%s]"
+      "}}"
+      "]}";
+
+  char source_corpus_info_format[] =
+      "{\"corpusId\": \"%s\","
+      "\"publisherData\": {"
+      "\"sourceName\": \"%s\""
+      "},"
+      "\"ampUrl\": \"%s\"}";
+
+  std::string source_corpus_info_list_str;
+  for (size_t i = 0; i < source_urls.size(); ++i) {
+    std::string source_corpus_info_str =
+        base::StringPrintf(source_corpus_info_format,
+                           source_urls[i].empty() ? "" : source_urls[i].c_str(),
+                           publishers[i].empty() ? "" : publishers[i].c_str(),
+                           amp_urls[i].empty() ? "" : amp_urls[i].c_str());
+    source_corpus_info_list_str.append(source_corpus_info_str);
+    source_corpus_info_list_str.append(",");
+  }
+  // Remove the last comma
+  source_corpus_info_list_str.pop_back();
+  return base::StringPrintf(json_str_format, content_creation_time_str.c_str(),
+                            expiry_time_str.c_str(),
+                            source_corpus_info_list_str.c_str());
+}
+
+std::string GetTestJsonWithSources(const std::vector<std::string>& source_urls,
+                                   const std::vector<std::string>& publishers,
+                                   const std::vector<std::string>& amp_urls) {
+  base::Time expiry_time = base::Time::Now() + base::TimeDelta::FromHours(1);
+  return GetTestJsonWithSources(
+      NTPSnippet::TimeToJsonString(GetDefaultCreationTime()),
+      NTPSnippet::TimeToJsonString(expiry_time), source_urls, publishers,
+      amp_urls);
+}
+
 std::string GetTestJson(const std::string& content_creation_time_str) {
   base::Time expiry_time = base::Time::Now() + base::TimeDelta::FromHours(1);
   return GetTestJson(content_creation_time_str,
                      NTPSnippet::TimeToJsonString(expiry_time));
 }
 
 std::string GetTestJson() {
   return GetTestJson(NTPSnippet::TimeToJsonString(GetDefaultCreationTime()));
 }
 
@@ skipping 141 matching lines @@
 TEST_F(NTPSnippetsServiceTest, Full) {
   std::string json_str(GetTestJson());
 
   LoadFromJSONString(json_str);
   EXPECT_EQ(service()->size(), 1u);
 
   // The same for loop without the '&' should not compile.
   for (auto& snippet : *service()) {
     // Snippet here is a const.
     EXPECT_EQ(snippet.url(), GURL("http://localhost/foobar"));
-    EXPECT_EQ(snippet.site_title(), "Site Title");
-    EXPECT_EQ(snippet.favicon_url(), GURL("http://localhost/favicon"));
+    EXPECT_EQ(snippet.site_title(), "Foo News");
     EXPECT_EQ(snippet.title(), "Title");
     EXPECT_EQ(snippet.snippet(), "Snippet");
     EXPECT_EQ(snippet.salient_image_url(),
               GURL("http://localhost/salient_image"));
     EXPECT_EQ(GetDefaultCreationTime(), snippet.publish_date());
-    EXPECT_EQ(snippet.amp_url(), GURL("http://localhost/amp"));
+    EXPECT_EQ(snippet.amp_url().spec(), GURL("http://localhost/amp").spec());
   }
 }
 
 TEST_F(NTPSnippetsServiceTest, Clear) {
   std::string json_str(GetTestJson());
 
   LoadFromJSONString(json_str);
   EXPECT_EQ(service()->size(), 1u);
 
   service()->ClearSnippets();
   EXPECT_EQ(service()->size(), 0u);
 }
 
 TEST_F(NTPSnippetsServiceTest, InsertAtFront) {
+  std::vector<std::string> source_urls;

[Bernhard Bauer, 2016/04/27 13:22:05]

I think you could use an initializer list here to construct the vector in one
go?

[Bernhard Bauer, 2016/04/28 14:19:20]

Ping :)

[May, 2016/04/28 18:01:52]

Done.

+  source_urls.push_back(std::string("http://first"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string("Source 1"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string(""));

[Bernhard Bauer, 2016/04/27 13:22:05]

The empty string literal is unnecessary.

[Bernhard Bauer, 2016/04/28 14:19:20]

Ping :)

[May, 2016/04/28 18:01:52]

Done.

   std::string json_str(
-      "{ \"recos\": [ "
-      "  { \"contentInfo\": { \"url\" : \"http://first\" }}"
-      "]}");
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
   LoadFromJSONString(json_str);
+
   ASSERT_EQ(service()->size(), 1u);
 
-  std::string json_str2(
-      "{ \"recos\": [ "
-      "  { \"contentInfo\": { \"url\" : \"http://second\" }}"
-      "]}");
-  LoadFromJSONString(json_str2);
+  source_urls.clear();
+  source_urls.push_back(std::string("http://second"));
+  publishers.clear();
+  publishers.push_back(std::string("Source 1"));
+  amp_urls.clear();
+  amp_urls.push_back(std::string(""));
+  json_str = GetTestJsonWithSources(source_urls, publishers, amp_urls);
+
+  LoadFromJSONString(json_str);
   ASSERT_EQ(service()->size(), 2u);
 
   // The snippet loaded last should be at the first position in the list now.
   const NTPSnippet& first_snippet = *service()->begin();
   EXPECT_EQ(first_snippet.url(), GURL("http://second"));
 }
 
 TEST_F(NTPSnippetsServiceTest, LimitNumSnippets) {
   int max_snippet_count = NTPSnippetsService::GetMaxSnippetCountForTesting();
   int snippets_per_load = max_snippet_count / 2 + 1;
@@ skipping 41 matching lines @@
 TEST_F(NTPSnippetsServiceTest, LoadIncompleteJsonWithExistingSnippets) {
   LoadFromJSONString(GetTestJson());
   ASSERT_EQ(service()->size(), 1u);
 
   LoadFromJSONString(GetIncompleteJson());
   // This should not have changed the existing snippets.
   EXPECT_EQ(service()->size(), 1u);
 }
 
 TEST_F(NTPSnippetsServiceTest, Discard) {
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("http://site.com"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string("Source 1"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string(""));
   std::string json_str(
-      "{ \"recos\": [ { \"contentInfo\": { \"url\" : \"http://site.com\" }}]}");
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
   LoadFromJSONString(json_str);
 
   ASSERT_EQ(1u, service()->size());
 
   // Discarding a non-existent snippet shouldn't do anything.
   EXPECT_FALSE(service()->DiscardSnippet(GURL("http://othersite.com")));
   EXPECT_EQ(1u, service()->size());
 
   // Discard the snippet.
   EXPECT_TRUE(service()->DiscardSnippet(GURL("http://site.com")));
   EXPECT_EQ(0u, service()->size());
 
   // Make sure that fetching the same snippet again does not re-add it.
   LoadFromJSONString(json_str);
   EXPECT_EQ(0u, service()->size());
 
   // The snippet should stay discarded even after re-creating the service.
   CreateSnippetsService();
   LoadFromJSONString(json_str);
   EXPECT_EQ(0u, service()->size());
 
   // The snippet can be added again after clearing discarded snippets.
   service()->ClearDiscardedSnippets();
   EXPECT_EQ(0u, service()->size());
   LoadFromJSONString(json_str);
   EXPECT_EQ(1u, service()->size());
 }
 
 TEST_F(NTPSnippetsServiceTest, GetDiscarded) {
-  std::string json_str(
-      "{ \"recos\": [ { \"contentInfo\": { \"url\" : \"http://site.com\" }}]}");
-  LoadFromJSONString(json_str);
+  LoadFromJSONString(GetTestJson());
 
   // For the test, we need the snippet to get discarded.
-  ASSERT_TRUE(service()->DiscardSnippet(GURL("http://site.com")));
+  ASSERT_TRUE(service()->DiscardSnippet(GURL("http://localhost/foobar")));
   const NTPSnippetsService::NTPSnippetStorage& snippets =
       service()->discarded_snippets();
   EXPECT_EQ(1u, snippets.size());
   for (auto& snippet : snippets) {
-    EXPECT_EQ(GURL("http://site.com"), snippet->url());
+    EXPECT_EQ(GURL("http://localhost/foobar"), snippet->url());
   }
 
   // There should be no discarded snippet after clearing the list.
   service()->ClearDiscardedSnippets();
   EXPECT_EQ(0u, service()->discarded_snippets().size());
 }
 
 TEST_F(NTPSnippetsServiceTest, CreationTimestampParseFail) {
   std::string json_str(GetTestJson("aaa1448459205"));
 
@@ skipping 10 matching lines @@
   }
 }
 
 TEST_F(NTPSnippetsServiceTest, RemoveExpiredContent) {
   std::string json_str(GetTestExpiredJson());
 
   LoadFromJSONString(json_str);
   EXPECT_EQ(service()->size(), 0u);
 }
 
+TEST_F(NTPSnippetsServiceTest, TestSingleSource) {
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("http://source1.com"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string("Source 1"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string("http://source1.amp.com"));
+  std::string json_str(
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.url(), GURL("http://source1.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 1"));
+    EXPECT_EQ(snippet.amp_url(), GURL("http://source1.amp.com"));
+    EXPECT_EQ(snippet.get_sources().size(), 1u);
+  }
+}
+
+TEST_F(NTPSnippetsServiceTest, TestSingleSourceWithMalformedUrl) {
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("aaaa"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string("Source 1"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string("http://source1.amp.com"));
+  std::string json_str(
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
+  LoadFromJSONString(json_str);
+// If we're running a release build, we only add snippets that have a well-
+// formed source url
+#if !DCHECK_IS_ON()
+  EXPECT_EQ(service()->size(), 0u);
+#else
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.site_title(), std::string());
+    EXPECT_EQ(snippet.get_sources().size(), 0u);
+  }
+#endif
+}
+
+TEST_F(NTPSnippetsServiceTest, TestSingleSourceWithMissingData) {
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("http://source1.com"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string(""));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string(""));
+  std::string json_str(
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
+  LoadFromJSONString(json_str);
+// If we're running a release build, we only add snippets have a source url
+// and publisher
+#if !DCHECK_IS_ON()
+  EXPECT_EQ(service()->size(), 0u);
+#else
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.url(), GURL("http://source1.com"));
+    EXPECT_EQ(snippet.site_title(), std::string());
+    EXPECT_EQ(snippet.amp_url(), GURL());
+    EXPECT_EQ(snippet.get_sources().size(), 1u);
+  }
+#endif
+}
+
+TEST_F(NTPSnippetsServiceTest, TestMultipleSources) {
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string("Source 1"));
+  publishers.push_back(std::string("Source 2"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string("http://source1.amp.com"));
+  amp_urls.push_back(std::string("http://source2.amp.com"));
+  std::string json_str(
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  // Expect the first source to be chosen
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 2u);
+    EXPECT_EQ(snippet.url(), GURL("http://source1.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 1"));
+    EXPECT_EQ(snippet.amp_url(), GURL("http://source1.amp.com"));
+  }
+}
+
+TEST_F(NTPSnippetsServiceTest, TestMultipleIncompleteSources) {
+  // Set Source 2 to have no AMP url, and Source 1 to have no publisher name
+  // Source 2 should win since we favor publisher name over amp url
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string(""));
+  publishers.push_back(std::string("Source 2"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string("http://source1.amp.com"));
+  amp_urls.push_back(std::string(""));
+  std::string json_str(
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 2u);
+    EXPECT_EQ(snippet.url(), GURL("http://source2.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 2"));
+    EXPECT_EQ(snippet.amp_url(), GURL());
+  }
+
+  service()->ClearSnippets();
+  // Set Source 1 to have no AMP url, and Source 2 to have no publisher name
+  // Source 1 should win in this case since we prefer publisher name to AMP url
+  source_urls.clear();
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  publishers.clear();
+  publishers.push_back(std::string("Source 1"));
+  publishers.push_back(std::string(""));
+  amp_urls.clear();
+  amp_urls.push_back(std::string(""));
+  amp_urls.push_back(std::string("http://source2.amp.com"));
+  json_str = GetTestJsonWithSources(source_urls, publishers, amp_urls);
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 2u);
+    EXPECT_EQ(snippet.url(), GURL("http://source1.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 1"));
+    EXPECT_EQ(snippet.amp_url(), GURL());
+  }
+
+  service()->ClearSnippets();
+  // Set source 1 to have no AMP url and no source. Source 1 should win since
+  // all sources without publisher name are ranked equally
+  source_urls.clear();
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  publishers.clear();
+  publishers.push_back(std::string(""));
+  publishers.push_back(std::string(""));
+  amp_urls.clear();
+  amp_urls.push_back(std::string(""));
+  amp_urls.push_back(std::string("http://source2.amp.com"));
+  json_str = GetTestJsonWithSources(source_urls, publishers, amp_urls);
+
+  LoadFromJSONString(json_str);
+#if !DCHECK_IS_ON()
+  EXPECT_EQ(service()->size(), 0u);
+#else
+  EXPECT_EQ(service()->size(), 1u);
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 2u);
+    EXPECT_EQ(snippet.url(), GURL("http://source1.com"));
+    EXPECT_EQ(snippet.site_title(), std::string());
+    EXPECT_EQ(snippet.amp_url(), GURL());
+  }
+#endif
+}
+
+TEST_F(NTPSnippetsServiceTest, TestMultipleCompleteSources) {
+  // Test 2 complete sources, we should choose the first complete source
+  std::vector<std::string> source_urls;
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  source_urls.push_back(std::string("http://source3.com"));
+  std::vector<std::string> publishers;
+  publishers.push_back(std::string("Source 1"));
+  publishers.push_back(std::string(""));
+  publishers.push_back(std::string("Source 3"));
+  std::vector<std::string> amp_urls;
+  amp_urls.push_back(std::string("http://source1.amp.com"));
+  amp_urls.push_back(std::string("http://source2.amp.com"));
+  amp_urls.push_back(std::string("http://source3.amp.com"));
+  std::string json_str(
+      GetTestJsonWithSources(source_urls, publishers, amp_urls));
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 3u);
+    EXPECT_EQ(snippet.url(), GURL("http://source1.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 1"));
+    EXPECT_EQ(snippet.amp_url(), GURL("http://source1.amp.com"));
+  }
+
+  // Test 2 complete sources, we should choose the first complete source
+  service()->ClearSnippets();
+  source_urls.clear();
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  source_urls.push_back(std::string("http://source3.com"));
+  publishers.clear();
+  publishers.push_back(std::string(""));
+  publishers.push_back(std::string("Source 2"));
+  publishers.push_back(std::string("Source 3"));
+  amp_urls.clear();
+  amp_urls.push_back(std::string("http://source1.amp.com"));
+  amp_urls.push_back(std::string("http://source2.amp.com"));
+  amp_urls.push_back(std::string("http://source3.amp.com"));
+  json_str = GetTestJsonWithSources(source_urls, publishers, amp_urls);
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 3u);
+    EXPECT_EQ(snippet.url(), GURL("http://source2.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 2"));
+    EXPECT_EQ(snippet.amp_url(), GURL("http://source2.amp.com"));
+  }
+
+  // Test 3 complete sources, we should choose the first complete source
+  service()->ClearSnippets();
+  source_urls.clear();
+  source_urls.push_back(std::string("http://source1.com"));
+  source_urls.push_back(std::string("http://source2.com"));
+  source_urls.push_back(std::string("http://source3.com"));
+  publishers.clear();
+  publishers.push_back(std::string("Source 1"));
+  publishers.push_back(std::string("Source 2"));
+  publishers.push_back(std::string("Source 3"));
+  amp_urls.clear();
+  amp_urls.push_back(std::string(""));
+  amp_urls.push_back(std::string("http://source2.amp.com"));
+  amp_urls.push_back(std::string("http://source3.amp.com"));
+  json_str = GetTestJsonWithSources(source_urls, publishers, amp_urls);
+
+  LoadFromJSONString(json_str);
+  EXPECT_EQ(service()->size(), 1u);
+
+  for (auto& snippet : *service()) {
+    EXPECT_EQ(snippet.get_sources().size(), 3u);
+    EXPECT_EQ(snippet.url(), GURL("http://source2.com"));
+    EXPECT_EQ(snippet.site_title(), std::string("Source 2"));
+    EXPECT_EQ(snippet.amp_url(), GURL("http://source2.amp.com"));
+  }
+}
 }  // namespace ntp_snippets