Set version field when changes are made to enhanced bookmarks.

components/enhanced_bookmarks/enhanced_bookmark_model.cc

Index: components/enhanced_bookmarks/enhanced_bookmark_model.cc
diff --git a/components/enhanced_bookmarks/metadata_accessor.cc b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
similarity index 38%
rename from components/enhanced_bookmarks/metadata_accessor.cc
rename to components/enhanced_bookmarks/enhanced_bookmark_model.cc
index cfcc9be5efb2bd2c59f596e159e7e63225e7194b..3c81fc7b3d5ece07a3cce223b95136dbbde68a21 100644
--- a/components/enhanced_bookmarks/metadata_accessor.cc
+++ b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
@@ -2,54 +2,44 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/enhanced_bookmarks/metadata_accessor.h"
+#include "components/enhanced_bookmarks/enhanced_bookmark_model.h"
 
 #include <iomanip>
+#include <sstream>
 
 #include "base/base64.h"
+#include "base/logging.h"
 #include "base/rand_util.h"
 #include "components/bookmarks/browser/bookmark_model.h"
+#include "components/bookmarks/browser/bookmark_node.h"
 #include "components/enhanced_bookmarks/proto/metadata.pb.h"
-#include "ui/base/models/tree_node_iterator.h"
-
-using namespace image::collections;
+#include "url/gurl.h"
 
 namespace {
+const char* kBookmarkBarId = "f_bookmarks_bar";
+
+const char* kIdKey = "stars.id";
+const char* kImageDataKey = "stars.imageData";
+const char* kNoteKey = "stars.note";
+const char* kPageDataKey = "stars.pageData";
+const char* kVersionKey = "stars.version";
 
 // Helper method for working with bookmark metainfo.
 std::string DataForMetaInfoField(const BookmarkNode* node,
                                  const std::string& field) {
-  const BookmarkNode::MetaInfoMap* map = node->GetMetaInfoMap();
-  if (!map)
-    return "";
-
-  BookmarkNode::MetaInfoMap::const_iterator it = map->find(field);
-  if (it == map->end())
-    return "";
+  std::string value;
+  if (!node->GetMetaInfo(field, &value))
+    return std::string();
 
   std::string decoded;
-  bool result = base::Base64Decode((*it).second, &decoded);
-  if (!result)
-    return "";
+  if (!base::Base64Decode(value, &decoded))
+    return std::string();
 
   return decoded;
 }
 
-// Sets a new remote id on a bookmark.
-std::string SetRemoteIdOnBookmark(BookmarkModel* bookmark_model,
-                                  const BookmarkNode* node) {
-  // Generate 16 digit hex string random id.
-  std::stringstream random_id;
-  random_id << std::hex << std::setfill('0') << std::setw(16);
-  random_id << base::RandUint64() << base::RandUint64();
-  std::string random_id_str = random_id.str();
-  bookmark_model->SetNodeMetaInfo(
-      node, enhanced_bookmarks::kIdDataKey, random_id_str);
-  return random_id_str;
-}
-
 // Helper method for working with ImageData_ImageInfo.
-bool PopulateImageData(const ImageData_ImageInfo& info,
+bool PopulateImageData(const image::collections::ImageData_ImageInfo& info,
                        GURL* out_url,
                        int* width,
                        int* height) {
@@ -65,59 +55,86 @@ bool PopulateImageData(const ImageData_ImageInfo& info,
   *height = info.height();
   return true;
 }
-
 }  // namespace
 
 namespace enhanced_bookmarks {
 
-const char* kPageDataKey = "stars.pageData";
-const char* kImageDataKey = "stars.imageData";
-const char* kIdDataKey = "stars.id";
-const char* kNoteKey = "stars.note";
+EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model)
+    : bookmark_model_(bookmark_model), initialized_(false) {
+}
+
+EnhancedBookmarkModel::~EnhancedBookmarkModel() {
+}
 
-std::string RemoteIdFromBookmark(BookmarkModel* bookmark_model,
-                                 const BookmarkNode* node) {
-  const BookmarkNode::MetaInfoMap* map = node->GetMetaInfoMap();
-  if (!map)
-    return SetRemoteIdOnBookmark(bookmark_model, node);
+void EnhancedBookmarkModel::Initialize(const std::string& version) {
+  version_ = version;
+  initialized_ = true;
+}
+
+std::string EnhancedBookmarkModel::GetRemoteIdForNode(
+    const BookmarkNode* node) {
+  DCHECK(initialized_);
 
-  BookmarkNode::MetaInfoMap::const_iterator it = map->find(kIdDataKey);
-  if (it == map->end())
-    return SetRemoteIdOnBookmark(bookmark_model, node);
+  if (node == bookmark_model_->bookmark_bar_node())
+    return kBookmarkBarId;
 
-  DCHECK(it->second.length());
-  return it->second;
+  // Permanent nodes other than the bookmarks bar don't have ids.
+  DCHECK(!bookmark_model_->is_permanent_node(node));
+
+  std::string id;
+  if (!node->GetMetaInfo(kIdKey, &id) || id.empty())
+    return SetRemoteIdForNode(node);
+  return id;
 }
 
-void SetDescriptionForBookmark(BookmarkModel* bookmark_model,
-                               const BookmarkNode* node,
-                               const std::string& description) {
-  bookmark_model->SetNodeMetaInfo(node, kNoteKey, description);
+std::string EnhancedBookmarkModel::SetRemoteIdForNode(
+    const BookmarkNode* node) {
+  DCHECK(initialized_);
+
+  std::stringstream random_id;
+  // Add prefix depending on whether the node is a folder or not.
+  if (node->is_url())
+    random_id << "ebc_";
+  else
+    random_id << "ebf_";
+
+  // Generate 32 digit hex string random suffix.
+  random_id << std::hex << std::setfill('0') << std::setw(16);
+  random_id << base::RandUint64() << base::RandUint64();
+  std::string random_id_str = random_id.str();
+
+  SetNodeMetaInfo(node, kIdKey, random_id_str);
+  return random_id_str;
 }
 
-std::string DescriptionFromBookmark(const BookmarkNode* node) {
-  const BookmarkNode::MetaInfoMap* map = node->GetMetaInfoMap();
-  if (!map)
-    return "";
+void EnhancedBookmarkModel::SetDescriptionForNode(
+    const BookmarkNode* node,
+    const std::string& description) {
+  DCHECK(initialized_);
+  SetNodeMetaInfo(node, kNoteKey, description);
+}
 
+std::string EnhancedBookmarkModel::GetDescriptionForNode(
+    const BookmarkNode* node) {
+  DCHECK(initialized_);
   // First, look for a custom note set by the user.
-  BookmarkNode::MetaInfoMap::const_iterator it = map->find(kNoteKey);
-  if (it != map->end() && it->second != "")
-    return it->second;
+  std::string description;
+  if (node->GetMetaInfo(kNoteKey, &description) && !description.empty())
+    return description;
 
   // If none are present, return the snippet.
-  return SnippetFromBookmark(node);
+  return GetSnippetForNode(node);
 }
 
-bool SetOriginalImageForBookmark(BookmarkModel* bookmark_model,
-                                 const BookmarkNode* node,
-                                 const GURL& url,
-                                 int width,
-                                 int height) {
+bool EnhancedBookmarkModel::SetOriginalImageForNode(const BookmarkNode* node,
+                                                    const GURL& url,
+                                                    int width,
+                                                    int height) {
+  DCHECK(initialized_);
   DCHECK(url.is_valid());
 
   std::string decoded(DataForMetaInfoField(node, kImageDataKey));
-  ImageData data;
+  image::collections::ImageData data;
 
   // Try to populate the imageData with the existing data.
   if (decoded != "") {
@@ -127,7 +144,8 @@ bool SetOriginalImageForBookmark(BookmarkModel* bookmark_model,
       return false;
   }
 
-  scoped_ptr<ImageData_ImageInfo> info(new ImageData_ImageInfo);
+  scoped_ptr<image::collections::ImageData_ImageInfo> info(
+      new image::collections::ImageData_ImageInfo);
   info->set_url(url.spec());
   info->set_width(width);
   info->set_height(height);
@@ -140,21 +158,22 @@ bool SetOriginalImageForBookmark(BookmarkModel* bookmark_model,
 
   std::string encoded;
   base::Base64Encode(output, &encoded);
-  bookmark_model->SetNodeMetaInfo(node, kImageDataKey, encoded);
+  SetNodeMetaInfo(node, kImageDataKey, encoded);
   // Ensure that the bookmark has a stars.id, to trigger the server processing.
-  RemoteIdFromBookmark(bookmark_model, node);
+  GetRemoteIdForNode(node);
   return true;
 }
 
-bool OriginalImageFromBookmark(const BookmarkNode* node,
-                               GURL* url,
-                               int* width,
-                               int* height) {
+bool EnhancedBookmarkModel::GetOriginalImageForNode(const BookmarkNode* node,
+                                                    GURL* url,
+                                                    int* width,
+                                                    int* height) {
+  DCHECK(initialized_);
   std::string decoded(DataForMetaInfoField(node, kImageDataKey));
   if (decoded == "")
     return false;
 
-  ImageData data;
+  image::collections::ImageData data;
   bool result = data.ParseFromString(decoded);
   if (!result)
     return false;
@@ -165,15 +184,16 @@ bool OriginalImageFromBookmark(const BookmarkNode* node,
   return PopulateImageData(data.original_info(), url, width, height);
 }
 
-bool ThumbnailImageFromBookmark(const BookmarkNode* node,
-                                GURL* url,
-                                int* width,
-                                int* height) {
+bool EnhancedBookmarkModel::GetThumbnailImageForNode(const BookmarkNode* node,
+                                                     GURL* url,
+                                                     int* width,
+                                                     int* height) {
+  DCHECK(initialized_);
   std::string decoded(DataForMetaInfoField(node, kImageDataKey));
   if (decoded == "")
     return false;
 
-  ImageData data;
+  image::collections::ImageData data;
   bool result = data.ParseFromString(decoded);
   if (!result)
     return false;
@@ -184,74 +204,44 @@ bool ThumbnailImageFromBookmark(const BookmarkNode* node,
   return PopulateImageData(data.thumbnail_info(), url, width, height);
 }
 
-std::string SnippetFromBookmark(const BookmarkNode* node) {
+std::string EnhancedBookmarkModel::GetSnippetForNode(const BookmarkNode* node) {
+  DCHECK(initialized_);
   std::string decoded(DataForMetaInfoField(node, kPageDataKey));
-  if (decoded == "")
+  if (decoded.empty())
     return decoded;
 
-  PageData data;
+  image::collections::PageData data;
   bool result = data.ParseFromString(decoded);
   if (!result)
-    return "";
+    return std::string();
 
   return data.snippet();
 }
 
-bool SetAllImagesForBookmark(BookmarkModel* bookmark_model,

[Yaron, 2014/08/27 23:32:36]

Where did this code? Presumably it's used by the mobile client.

[Rune Fevang, 2014/08/27 23:52:22]

I removed it because the comment said it was used for tests and it was only used
in one test, that did the same thing a different test did without using it
anyway. If it's actually used in mobile test code I could put it back, though it
probably belongs in a test-only utils file in that case?

[Yaron, 2014/08/28 00:11:49]

From a quick grep, it appears to be used from internal test code.

[Rune Fevang, 2014/08/29 01:10:10]

OK, re-added it as a static function.

-                             const BookmarkNode* node,
-                             const GURL& image_url,
-                             int image_width,
-                             int image_height,
-                             const GURL& thumbnail_url,
-                             int thumbnail_width,
-                             int thumbnail_height) {
-  DCHECK(image_url.is_valid() || image_url.is_empty());
-  DCHECK(thumbnail_url.is_valid() || thumbnail_url.is_empty());
-  std::string decoded(DataForMetaInfoField(node, kImageDataKey));
-  ImageData data;
-
-  // Try to populate the imageData with the existing data.
-  if (decoded != "") {
-    // If the parsing fails, something is wrong. Immediately fail.
-    bool result = data.ParseFromString(decoded);
-    if (!result)
-      return false;
-  }
-
-  if (image_url.is_empty()) {
-    data.release_original_info();
-  } else {
-    // Regardless of whether an image info exists, we make a new one.
-    // Intentially make a raw pointer.
-    ImageData_ImageInfo* info = new ImageData_ImageInfo;
-    info->set_url(image_url.spec());
-    info->set_width(image_width);
-    info->set_height(image_height);
-    // This method consumes the raw pointer.
-    data.set_allocated_original_info(info);
-  }
-
-  if (thumbnail_url.is_empty()) {
-    data.release_thumbnail_info();
-  } else {
-    // Regardless of whether an image info exists, we make a new one.
-    // Intentially make a raw pointer.
-    ImageData_ImageInfo* info = new ImageData_ImageInfo;
-    info->set_url(thumbnail_url.spec());
-    info->set_width(thumbnail_width);
-    info->set_height(thumbnail_height);
-    // This method consumes the raw pointer.
-    data.set_allocated_thumbnail_info(info);
-  }
-  std::string output;
-  bool result = data.SerializePartialToString(&output);
-  if (!result)
-    return false;
+std::string EnhancedBookmarkModel::GetVersionForNode(const BookmarkNode* node) {
+  DCHECK(initialized_);
+  std::string version;
+  if (!node->GetMetaInfo(kVersionKey, &version))
+    return std::string();
+  return version;
+}
 
-  std::string encoded;
-  base::Base64Encode(output, &encoded);
-  bookmark_model->SetNodeMetaInfo(node, kImageDataKey, encoded);
-  return true;
+void EnhancedBookmarkModel::SetNodeMetaInfo(const BookmarkNode* node,
+                                            const std::string& field,
+                                            const std::string& value) {
+  BookmarkNode::MetaInfoMap meta_info;
+  const BookmarkNode::MetaInfoMap* old_meta_info = node->GetMetaInfoMap();
+  if (old_meta_info)
+    meta_info.insert(old_meta_info->begin(), old_meta_info->end());
+
+  // Don't update anything if the value to set is already there.
+  BookmarkNode::MetaInfoMap::iterator it = meta_info.find(field);
+  if (it != meta_info.end() && it->second == value)
+    return;
+
+  meta_info[field] = value;
+  meta_info[kVersionKey] = version_;
+  bookmark_model_->SetNodeMetaInfoMap(node, meta_info);
 }
 
 }  // namespace enhanced_bookmarks