Only set remote id during url node creation.

components/enhanced_bookmarks/enhanced_bookmark_model_unittest.cc

 // Copyright 2014 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/enhanced_bookmarks/enhanced_bookmark_model.h"
 
 #include "base/base64.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/bookmarks/test/test_bookmark_client.h"
 #include "components/enhanced_bookmarks/proto/metadata.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 namespace {
 
+using enhanced_bookmarks::EnhancedBookmarkModel;
+
 const std::string BOOKMARK_URL("http://example.com/index.html");
 
 class EnhancedBookmarkModelTest : public testing::Test {
  public:
   EnhancedBookmarkModelTest() {}
   virtual ~EnhancedBookmarkModelTest() {}
 
   virtual void SetUp() OVERRIDE {
     bookmarks::TestBookmarkClient bookmark_client;
     bookmark_model_.reset(bookmark_client.CreateModel().release());
-    model_.reset(new enhanced_bookmarks::EnhancedBookmarkModel(
-        bookmark_model_.get(), "v1.0"));
+    model_.reset(new EnhancedBookmarkModel(bookmark_model_.get(), "v1.0"));
   }
 
   virtual void TearDown() OVERRIDE {
+    model_->ShutDown();
     model_.reset();
     bookmark_model_.reset();
   }
 
  protected:
   const BookmarkNode* AddBookmark() {
     return AddBookmark("Some title", bookmark_model_->other_node());
   }
 
   const BookmarkNode* AddFolder() {
     return AddFolder("Some title", bookmark_model_->other_node());
   }
 
   const BookmarkNode* AddBookmark(const std::string& name,
                                   const BookmarkNode* parent) {
-    return bookmark_model_->AddURL(parent,
-                                   0,  // index.
-                                   base::ASCIIToUTF16(name),
-                                   GURL(BOOKMARK_URL));
+    return model_->AddURL(parent,
+                          0,  // index.
+                          base::ASCIIToUTF16(name),
+                          GURL(BOOKMARK_URL),
+                          base::Time::Now());
   }
 
   const BookmarkNode* AddFolder(const std::string& name,
                                 const BookmarkNode* parent) {
-    return bookmark_model_->AddFolder(parent, 0, base::ASCIIToUTF16(name));
+    return model_->AddFolder(parent, 0, base::ASCIIToUTF16(name));
   }
 
-  std::string GetVersionForNode(const BookmarkNode* node) {
-    std::string version;
-    if (!node->GetMetaInfo("stars.version", &version))
+  std::string GetVersion(const BookmarkNode* node) {
+    return GetMetaInfoField(node, "stars.version");
+  }
+
+  std::string GetId(const BookmarkNode* node) {
+    return GetMetaInfoField(node, "stars.id");
+  }
+
+  std::string GetOldId(const BookmarkNode* node) {
+    return GetMetaInfoField(node, "stars.oldId");
+  }
+
+  std::string GetMetaInfoField(const BookmarkNode* node,
+                               const std::string& name) {
+    std::string value;
+    if (!node->GetMetaInfo(name, &value))
       return std::string();
-    return version;
+    return value;
   }
 
   scoped_ptr<BookmarkModel> bookmark_model_;
-  scoped_ptr<enhanced_bookmarks::EnhancedBookmarkModel> model_;
+  scoped_ptr<EnhancedBookmarkModel> model_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(EnhancedBookmarkModelTest);
 };
 
 TEST_F(EnhancedBookmarkModelTest, TestEmptySnippet) {
   const BookmarkNode* node = AddBookmark();
 
   std::string snippet(model_->GetSnippet(node));
   EXPECT_EQ(snippet, "");
@@ skipping 165 matching lines @@
   ASSERT_TRUE(result);
 
   GURL url;
   int width;
   int height;
   result = model_->GetOriginalImage(node, &url, &width, &height);
   ASSERT_TRUE(result);
   EXPECT_EQ(url, GURL("http://example.com/i.jpg"));
   EXPECT_EQ(width, 22);
   EXPECT_EQ(height, 33);
-  EXPECT_EQ("v1.0", GetVersionForNode(node));
+  EXPECT_EQ("v1.0", GetVersion(node));
 }
 
 TEST_F(EnhancedBookmarkModelTest, TestDoubleEncodeDecode) {
   const BookmarkNode* node = AddBookmark();
 
   // Encode some information.
   bool result =
       model_->SetOriginalImage(node, GURL("http://example.com/i.jpg"), 22, 33);
   ASSERT_TRUE(result);
   // Encode some different information.
   result =
       model_->SetOriginalImage(node, GURL("http://example.com/i.jpg"), 33, 44);
   ASSERT_TRUE(result);
 
   GURL url;
   int width;
   int height;
   result = model_->GetOriginalImage(node, &url, &width, &height);
   ASSERT_TRUE(result);
   EXPECT_EQ(url, GURL("http://example.com/i.jpg"));
   EXPECT_EQ(width, 33);
   EXPECT_EQ(height, 44);
-  EXPECT_EQ("v1.0", GetVersionForNode(node));
+  EXPECT_EQ("v1.0", GetVersion(node));
 }
 
 TEST_F(EnhancedBookmarkModelTest, TestRemoteId) {
   const BookmarkNode* node = AddBookmark();
+  // Verify that the remote id starts with the correct prefix.
+  EXPECT_TRUE(StartsWithASCII(model_->GetRemoteId(node), "ebc_", true));
+
+  // Getting the remote id for nodes that don't have them should return the
+  // empty string.
+  const BookmarkNode* existing_node =
+      bookmark_model_->AddURL(bookmark_model_->other_node(),
+                              0,
+                              base::ASCIIToUTF16("Title"),
+                              GURL(GURL(BOOKMARK_URL)));
+  EXPECT_TRUE(model_->GetRemoteId(existing_node).empty());
+
+  // Folder nodes should not have a remote id set on creation.
   const BookmarkNode* folder_node = AddFolder();
-
-  std::string remote_id = model_->GetRemoteId(node);
-  // First call creates the UUID, second call should return the same.
-  EXPECT_EQ(remote_id, model_->GetRemoteId(node));
-
-  // Verify that the remote id starts with the correct prefix.
-  EXPECT_TRUE(StartsWithASCII(remote_id, "ebc_", true));
-  std::string folder_remote_id = model_->GetRemoteId(folder_node);
-  EXPECT_TRUE(StartsWithASCII(folder_remote_id, "ebf_", true));
-
-  // Verifiy version field was set.
-  EXPECT_EQ("v1.0", GetVersionForNode(node));
-  EXPECT_EQ("v1.0", GetVersionForNode(folder_node));
+  EXPECT_TRUE(model_->GetRemoteId(folder_node).empty());
 }
 
 TEST_F(EnhancedBookmarkModelTest, TestEmptyDescription) {
   const BookmarkNode* node = AddBookmark();
 
   std::string description(model_->GetDescription(node));
   EXPECT_EQ(description, "");
 }
 
 TEST_F(EnhancedBookmarkModelTest, TestDescription) {
   const BookmarkNode* node = AddBookmark();
   const std::string description("This is the most useful description of all.");
 
   // Set the description.
   model_->SetDescription(node, description);
 
   // Check the description is the one that was set.
   EXPECT_EQ(model_->GetDescription(node), description);
-  EXPECT_EQ("v1.0", GetVersionForNode(node));
+  EXPECT_EQ("v1.0", GetVersion(node));
 }
 
 // If there is no notes field, the description should fall back on the snippet.
 TEST_F(EnhancedBookmarkModelTest, TestDescriptionFallback) {
   const BookmarkNode* node = AddBookmark();
 
   // Binary serialize the protobuf.
   image::collections::PageData data;
   data.set_snippet("Joe Bar Team");
   ASSERT_TRUE(data.IsInitialized());
@@ skipping 15 matching lines @@
   model_->SetDescription(node, description);
 
   // Check the description is the one that was set.
   EXPECT_EQ(model_->GetDescription(node), description);
 }
 
 // Makes sure that the stars.version field is set every time
 // EnhancedBookmarkModel makes a change to a node.
 TEST_F(EnhancedBookmarkModelTest, TestVersionField) {
   const BookmarkNode* node = AddBookmark();
-  EXPECT_EQ("", GetVersionForNode(node));
+  EXPECT_EQ("", GetVersion(node));
 
   model_->SetDescription(node, "foo");
-  EXPECT_EQ("v1.0", GetVersionForNode(node));
+  EXPECT_EQ("v1.0", GetVersion(node));
 
   // Add a suffix to the version to set.
   model_->SetVersionSuffix("alpha");
 
   model_->SetDescription(node, "foo");
   // Since the description didn't actually change, the version field should
   // not either.
-  EXPECT_EQ("v1.0", GetVersionForNode(node));
+  EXPECT_EQ("v1.0", GetVersion(node));
 
   model_->SetDescription(node, "bar");
-  EXPECT_EQ("v1.0/alpha", GetVersionForNode(node));
+  EXPECT_EQ("v1.0/alpha", GetVersion(node));
 }
 
-// Verifies that the stars.userEdit field is set appropriately when editing a
-// node.
-TEST_F(EnhancedBookmarkModelTest, TestUserEdit) {
-  const BookmarkNode* node = AddBookmark();
+// Verifies that duplicate nodes are reset when the model is created.
+TEST_F(EnhancedBookmarkModelTest, ResetDuplicateNodesOnInitialization) {
+  model_->ShutDown();
 
-  model_->SetDescription(node, "foo");
-  std::string user_edit;
-  ASSERT_TRUE(node->GetMetaInfo("stars.userEdit", &user_edit));
-  EXPECT_EQ("true", user_edit);
+  const BookmarkNode* parent = bookmark_model_->other_node();
+  const BookmarkNode* node1 = bookmark_model_->AddURL(
+      parent, 0, base::ASCIIToUTF16("Some title"), GURL(BOOKMARK_URL));
+  const BookmarkNode* node2 = bookmark_model_->AddURL(
+      parent, 0, base::ASCIIToUTF16("Some title"), GURL(BOOKMARK_URL));
+  const BookmarkNode* node3 = bookmark_model_->AddURL(
+      parent, 0, base::ASCIIToUTF16("Some title"), GURL(BOOKMARK_URL));
+  const BookmarkNode* node4 = bookmark_model_->AddURL(
+      parent, 0, base::ASCIIToUTF16("Some title"), GURL(BOOKMARK_URL));
+
+  bookmark_model_->SetNodeMetaInfo(node1, "stars.id", "c_1");
+  bookmark_model_->SetNodeMetaInfo(node2, "stars.id", "c_2");
+  bookmark_model_->SetNodeMetaInfo(node3, "stars.id", "c_1");
+  bookmark_model_->SetNodeMetaInfo(node4, "stars.id", "c_1");
+  EXPECT_EQ("c_1", GetId(node1));
+  EXPECT_EQ("c_2", GetId(node2));
+  EXPECT_EQ("c_1", GetId(node3));
+  EXPECT_EQ("c_1", GetId(node4));
+
+  model_.reset(new EnhancedBookmarkModel(bookmark_model_.get(), "v2.0"));
+
+  EXPECT_EQ("c_2", GetId(node2));
+  EXPECT_EQ("", GetId(node1));
+  EXPECT_EQ("", GetId(node3));
+  EXPECT_EQ("", GetId(node4));
+  EXPECT_EQ("c_1", GetOldId(node1));
+  EXPECT_EQ("c_1", GetOldId(node3));
+  EXPECT_EQ("c_1", GetOldId(node4));
+  EXPECT_EQ("v2.0", GetVersion(node1));
+  EXPECT_EQ("v2.0", GetVersion(node3));
+  EXPECT_EQ("v2.0", GetVersion(node4));
+}
+
+// Verifies that duplicate nodes are reset if one is created.
+TEST_F(EnhancedBookmarkModelTest, ResetDuplicateAddedNodes) {
+  BookmarkNode::MetaInfoMap meta_info;
+  meta_info["stars.id"] = "c_1";
+  const BookmarkNode* parent = bookmark_model_->other_node();
+
+  const BookmarkNode* node1 =
+      bookmark_model_->AddURLWithCreationTimeAndMetaInfo(
+          parent,
+          0,
+          base::ASCIIToUTF16("Some title"),
+          GURL(BOOKMARK_URL),
+          base::Time::Now(),
+          &meta_info);
+  EXPECT_EQ("c_1", GetId(node1));
+
+  const BookmarkNode* node2 =
+      bookmark_model_->AddURLWithCreationTimeAndMetaInfo(
+          parent,
+          0,
+          base::ASCIIToUTF16("Some title"),
+          GURL(BOOKMARK_URL),
+          base::Time::Now(),
+          &meta_info);
+  EXPECT_EQ("", GetId(node1));
+  EXPECT_EQ("", GetId(node2));
+  EXPECT_EQ("c_1", GetOldId(node1));
+  EXPECT_EQ("c_1", GetOldId(node2));
+  EXPECT_EQ("v1.0", GetVersion(node1));
+  EXPECT_EQ("v1.0", GetVersion(node2));
+}
+
+// Verifies that duplicate nodes are reset if an id is changed to a duplicate
+// value.
+TEST_F(EnhancedBookmarkModelTest, ResetDuplicateChangedNodes) {
+  const BookmarkNode* node1 = AddBookmark();
+  const BookmarkNode* node2 = AddBookmark();
+
+  bookmark_model_->SetNodeMetaInfo(node1, "stars.id", "c_1");
+  EXPECT_EQ("c_1", GetId(node1));
+
+  bookmark_model_->SetNodeMetaInfo(node2, "stars.id", "c_1");
+  EXPECT_EQ("", GetId(node1));
+  EXPECT_EQ("", GetId(node2));
+  EXPECT_EQ("c_1", GetOldId(node1));
+  EXPECT_EQ("c_1", GetOldId(node2));
+  EXPECT_EQ("v1.0", GetVersion(node1));
+  EXPECT_EQ("v1.0", GetVersion(node2));
 }
 

[noyau (Ping after 24h), 2014/09/18 08:43:22]

Can you add tests to make sure the observer methods are all called properly?
Thanks.

[Rune Fevang, 2014/09/19 01:18:07]

Done.

 }  // namespace