[NTP Snippets] Persist non-article remote suggestions in the DB

components/ntp_snippets/remote/ntp_snippet.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/remote/ntp_snippet.h"
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
-
+#include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/remote/proto/ntp_snippets.pb.h"
 
+namespace ntp_snippets {
+
 namespace {
 
+const int kArticlesRemoteId = 1;

[Bernhard Bauer, 2016/10/10 13:54:54]

I'm starting to get the feeling that just using a pair<bool, int> might have
been easier :)

[Marc Treib, 2016/10/10 15:54:47]

...I don't quite follow? You mean essentially store an "Optional<int>
remote_id_" in NTPSnippet?

[Bernhard Bauer, 2016/10/10 16:03:12]

No, I meant that maybe we should have stored category IDs as pairs of
(local/remote x ID) instead of encoding both in a single int value.

[Marc Treib, 2016/10/10 16:04:14]

Aah - yes, that might have made sense...

+static_assert(int(KnownCategories::ARTICLES) -

[Bernhard Bauer, 2016/10/10 13:54:54]

Should this be a static_cast<int>()?

[Marc Treib, 2016/10/10 15:54:47]

Sure, done.

+                      int(KnownCategories::REMOTE_CATEGORIES_OFFSET) ==
+                  kArticlesRemoteId,
+              "kArticlesRemoteId has a wrong value?!");
+
 // dict.Get() specialization for base::Time values
 bool GetTimeValue(const base::DictionaryValue& dict,
                   const std::string& key,
                   base::Time* time) {
   std::string time_value;
   return dict.GetString(key, &time_value) &&
          base::Time::FromString(time_value.c_str(), time);
 }
 
 // dict.Get() specialization for GURL values
 bool GetURLValue(const base::DictionaryValue& dict,
                  const std::string& key,
                  GURL* url) {
   std::string spec;
   if (!dict.GetString(key, &spec)) {
     return false;
   }
   *url = GURL(spec);
   return url->is_valid();
 }
 
 }  // namespace
 
-namespace ntp_snippets {
-
-NTPSnippet::NTPSnippet(const std::string& id)
-    : id_(id), score_(0), is_dismissed_(false), best_source_index_(0) {}
+NTPSnippet::NTPSnippet(const std::string& id, int remote_category_id)
+    : id_(id),
+      score_(0),
+      is_dismissed_(false),
+      remote_category_id_(remote_category_id),
+      best_source_index_(0) {}
 
 NTPSnippet::~NTPSnippet() = default;
 
 // static
 std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromChromeReaderDictionary(
     const base::DictionaryValue& dict) {
   const base::DictionaryValue* content = nullptr;
   if (!dict.GetDictionary("contentInfo", &content))
     return nullptr;
 
   // Need at least the id.
   std::string id;
   if (!content->GetString("url", &id) || id.empty())
     return nullptr;
 
-  std::unique_ptr<NTPSnippet> snippet(new NTPSnippet(id));
+  std::unique_ptr<NTPSnippet> snippet(new NTPSnippet(id, kArticlesRemoteId));
 
   std::string title;
   if (content->GetString("title", &title))
     snippet->set_title(title);
   std::string salient_image_url;
   if (content->GetString("thumbnailUrl", &salient_image_url))
     snippet->set_salient_image_url(GURL(salient_image_url));
   std::string snippet_str;
   if (content->GetString("snippet", &snippet_str))
     snippet->set_snippet(snippet_str);
@@ skipping 62 matching lines @@
 
   double score;
   if (dict.GetDouble("score", &score))
     snippet->set_score(score);
 
   return snippet;
 }
 
 // static
 std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromContentSuggestionsDictionary(
-    const base::DictionaryValue& dict) {
+    const base::DictionaryValue& dict,
+    int remote_category_id) {
   const base::ListValue* ids;
   std::string id;
   if (!(dict.GetList("ids", &ids) &&
         ids->GetString(0, &id))) {  // TODO(sfiera): multiple IDs
     return nullptr;
   }
 
-  auto snippet = base::MakeUnique<NTPSnippet>(id);
+  auto snippet = base::MakeUnique<NTPSnippet>(id, remote_category_id);
   snippet->sources_.emplace_back(GURL(), std::string(), GURL());
   auto* source = &snippet->sources_.back();
   snippet->best_source_index_ = 0;
 
   if (!(dict.GetString("title", &snippet->title_) &&
         dict.GetString("snippet", &snippet->snippet_) &&
         GetTimeValue(dict, "creationTime", &snippet->publish_date_) &&
         GetTimeValue(dict, "expirationTime", &snippet->expiry_date_) &&
         GetURLValue(dict, "imageUrl", &snippet->salient_image_url_) &&
         dict.GetString("attribution", &source->publisher_name) &&
         GetURLValue(dict, "fullPageUrl", &source->url))) {
     return nullptr;
   }
   GetURLValue(dict, "ampUrl", &source->amp_url);  // May fail; OK.
   // TODO(sfiera): also favicon URL.
 
   snippet->score_ = 0.0;  // TODO(sfiera): put score in protocol.
 
   return snippet;
 }
 
 // static
 std::unique_ptr<NTPSnippet> NTPSnippet::CreateFromProto(
     const SnippetProto& proto) {
   // Need at least the id.
   if (!proto.has_id() || proto.id().empty())
     return nullptr;
 
-  std::unique_ptr<NTPSnippet> snippet(new NTPSnippet(proto.id()));
+  int remote_category_id = kArticlesRemoteId;
+  if (proto.has_remote_category_id())
+    remote_category_id = proto.remote_category_id();

[Bernhard Bauer, 2016/10/10 13:54:54]

Nit: Maybe use a ternary operator so you only assign the variable once?

[Marc Treib, 2016/10/10 15:54:47]

Done.

+
+  std::unique_ptr<NTPSnippet> snippet(
+      new NTPSnippet(proto.id(), remote_category_id));
 
   snippet->set_title(proto.title());
   snippet->set_snippet(proto.snippet());
   snippet->set_salient_image_url(GURL(proto.salient_image_url()));
   snippet->set_publish_date(
       base::Time::FromInternalValue(proto.publish_date()));
   snippet->set_expiry_date(base::Time::FromInternalValue(proto.expiry_date()));
   snippet->set_score(proto.score());
   snippet->set_dismissed(proto.dismissed());
 
@@ skipping 35 matching lines @@
   if (!snippet_.empty())
     result.set_snippet(snippet_);
   if (salient_image_url_.is_valid())
     result.set_salient_image_url(salient_image_url_.spec());
   if (!publish_date_.is_null())
     result.set_publish_date(publish_date_.ToInternalValue());
   if (!expiry_date_.is_null())
     result.set_expiry_date(expiry_date_.ToInternalValue());
   result.set_score(score_);
   result.set_dismissed(is_dismissed_);
+  result.set_remote_category_id(remote_category_id_);
 
   for (const SnippetSource& source : sources_) {
     SnippetSourceProto* source_proto = result.add_sources();
     source_proto->set_url(source.url.spec());
     if (!source.publisher_name.empty())
       source_proto->set_publisher_name(source.publisher_name);
     if (source.amp_url.is_valid())
       source_proto->set_amp_url(source.amp_url.spec());
   }
 
@@ skipping 32 matching lines @@
       best_source_index_ = i;
       if (!source.amp_url.is_empty()) {
         // This is the best possible source, stop looking.
         break;
       }
     }
   }
 }
 
 }  // namespace ntp_snippets