Populate versions on individual nodes in sync model and native bookmark model.

chrome/browser/sync/profile_sync_service_bookmark_unittest.cc

 // Copyright (c) 2012 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.
 
 // TODO(akalin): This file is basically just a unit test for
 // BookmarkChangeProcessor.  Write unit tests for
 // BookmarkModelAssociator separately.
 
+#include <map>
+#include <queue>
 #include <stack>
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
@@ skipping 533 matching lines @@
   }
 
  private:
   // Used by both |ui_thread_| and |file_thread_|.
   MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
   // Needed by |model_|.
   content::TestBrowserThread file_thread_;
 
   TestingProfile profile_;
-  scoped_ptr<TestBookmarkModelAssociator> model_associator_;
 
  protected:
   BookmarkModel* model_;
   syncer::TestUserShare test_user_share_;
   scoped_ptr<BookmarkChangeProcessor> change_processor_;
+  scoped_ptr<TestBookmarkModelAssociator> model_associator_;
   StrictMock<DataTypeErrorHandlerMock> mock_error_handler_;
 };
 
 TEST_F(ProfileSyncServiceBookmarkTest, InitialState) {
   LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
   StartSync();
 
   EXPECT_TRUE(other_bookmarks_id());
   EXPECT_TRUE(bookmark_bar_id());
   EXPECT_TRUE(mobile_bookmarks_id());
@@ skipping 391 matching lines @@
 
   EXPECT_EQ(2, model_->other_node()->child_count());
   ExpectModelMatch();
 }
 
 struct TestData {
   const wchar_t* title;
   const char* url;
 };
 
+// Map from bookmark node ID to its version.
+typedef std::map<int64, int64> BookmarkNodeVersionMap;
+
 // TODO(ncarter): Integrate the existing TestNode/PopulateNodeFromString code
 // in the bookmark model unittest, to make it simpler to set up test data
 // here (and reduce the amount of duplication among tests), and to reduce the
 // duplication.
 class ProfileSyncServiceBookmarkTestWithData
     : public ProfileSyncServiceBookmarkTest {
  public:
   ProfileSyncServiceBookmarkTestWithData();
 
  protected:
   // Populates or compares children of the given bookmark node from/with the
   // given test data array with the given size. |running_count| is updated as
   // urls are added. It is used to set the creation date (or test the creation
   // date for CompareWithTestData()).
   void PopulateFromTestData(const BookmarkNode* node,
                             const TestData* data,
                             int size,
                             int* running_count);
   void CompareWithTestData(const BookmarkNode* node,
                            const TestData* data,
                            int size,
                            int* running_count);
 
   void ExpectBookmarkModelMatchesTestData();
   void WriteTestDataToBookmarkModel();
 
+  // Verify transaction versions of bookmark nodes and sync nodes are equal
+  // recursively. If node is in |version_expected|, versions should match
+  // there, too.
+  void ExpectTransactionVersionMatch(
+      const BookmarkNode* node,
+      const BookmarkNodeVersionMap& version_expected);
+
  private:
   const base::Time start_time_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileSyncServiceBookmarkTestWithData);
 };
 
 namespace {
 
 // Constants for bookmark model that looks like:
 // |-- Bookmark bar
@@ skipping 112 matching lines @@
   DCHECK(node);
   DCHECK(data);
   DCHECK(node->is_folder());
   for (int i = 0; i < size; ++i) {
     const TestData& item = data[i];
     if (item.url) {
       const base::Time add_time =
           start_time_ + base::TimeDelta::FromMinutes(*running_count);
       model_->AddURLWithCreationTime(node, i, WideToUTF16Hack(item.title),
                                      GURL(item.url), add_time);
-      (*running_count)++;
     } else {
       model_->AddFolder(node, i, WideToUTF16Hack(item.title));
     }
+    (*running_count)++;
   }
 }
 
 void ProfileSyncServiceBookmarkTestWithData::CompareWithTestData(
     const BookmarkNode* node,
     const TestData* data,
     int size,
     int* running_count) {
   DCHECK(node);
   DCHECK(data);
   DCHECK(node->is_folder());
   ASSERT_EQ(size, node->child_count());
   for (int i = 0; i < size; ++i) {
     const BookmarkNode* child_node = node->GetChild(i);
     const TestData& item = data[i];
     GURL url = GURL(item.url == NULL ? "" : item.url);
     BookmarkNode test_node(url);
     test_node.SetTitle(WideToUTF16Hack(item.title));
     EXPECT_EQ(child_node->GetTitle(), test_node.GetTitle());
     if (item.url) {
       EXPECT_FALSE(child_node->is_folder());
       EXPECT_TRUE(child_node->is_url());
       EXPECT_EQ(child_node->url(), test_node.url());
       const base::Time expected_time =
           start_time_ + base::TimeDelta::FromMinutes(*running_count);
       EXPECT_EQ(expected_time.ToInternalValue(),
                 child_node->date_added().ToInternalValue());
-      (*running_count)++;
     } else {
       EXPECT_TRUE(child_node->is_folder());
       EXPECT_FALSE(child_node->is_url());
     }
+    (*running_count)++;
   }
 }
 
 // TODO(munjal): We should implement some way of generating random data and can
 // use the same seed to generate the same sequence.
 void ProfileSyncServiceBookmarkTestWithData::WriteTestDataToBookmarkModel() {
   const BookmarkNode* bookmarks_bar_node = model_->bookmark_bar_node();
   int count = 0;
   PopulateFromTestData(bookmarks_bar_node,
                        kBookmarkBarChildren,
@@ skipping 405 matching lines @@
   updates.ApplyPendingChanges(change_processor_.get());
 
   const BookmarkNode* node = model_->bookmark_bar_node()->GetChild(0);
   ASSERT_TRUE(node);
   EXPECT_TRUE(node->is_url());
   EXPECT_EQ(WideToUTF16Hack(kTitle), node->GetTitle());
   EXPECT_EQ(kUrl, node->url().possibly_invalid_spec());
   EXPECT_EQ(node->date_added(), base::Time::FromInternalValue(30));
 }
 
+// Output transaction versions of |node| and nodes under it to |node_versions|.
+void GetTransactionVersions(
+    const BookmarkNode* root,
+    BookmarkNodeVersionMap *node_versions) {

[Nicolas Zea, 2012/11/08 19:05:08]

fix asterisk

[haitaol1, 2012/11/08 19:18:47]

Done.

+  node_versions->clear();
+  std::queue<const BookmarkNode*> nodes;
+  nodes.push(root);
+  while (!nodes.empty()) {
+    const BookmarkNode* n = nodes.front();
+    nodes.pop();
+
+    std::string version_str;
+    int64 version;
+    EXPECT_TRUE(n->GetMetaInfo(kBookmarkTransactionVersionKey, &version_str));
+    EXPECT_TRUE(base::StringToInt64(version_str, &version));
+
+    (*node_versions)[n->id()] = version;
+    for (int i = 0; i < n->child_count(); ++i)
+      nodes.push(n->GetChild(i));
+  }
+}
+
+void ProfileSyncServiceBookmarkTestWithData::ExpectTransactionVersionMatch(
+    const BookmarkNode* node,
+    const BookmarkNodeVersionMap& version_expected) {
+  syncer::ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+
+  BookmarkNodeVersionMap bnodes_versions;
+  GetTransactionVersions(node, &bnodes_versions);
+  for (BookmarkNodeVersionMap::const_iterator it = bnodes_versions.begin();
+      it != bnodes_versions.end(); ++it) {
+    syncer::ReadNode sync_node(&trans);
+    ASSERT_TRUE(model_associator_->InitSyncNodeFromChromeId(it->first,
+                                                            &sync_node));
+    EXPECT_EQ(sync_node.GetEntry()->Get(syncer::syncable::TRANSACTION_VERSION),
+              it->second);
+    BookmarkNodeVersionMap::const_iterator expected_ver_it =
+        version_expected.find(it->first);
+    if (expected_ver_it != version_expected.end())
+      EXPECT_EQ(expected_ver_it->second, it->second);
+  }
+}
+
+// Test transaction versions of model and nodes are incremented after changes
+// are applied.
+TEST_F(ProfileSyncServiceBookmarkTestWithData, UpdateTransactionVersion) {
+  LoadBookmarkModel(DELETE_EXISTING_STORAGE, DONT_SAVE_TO_STORAGE);
+  StartSync();
+  WriteTestDataToBookmarkModel();
+  MessageLoop::current()->RunUntilIdle();
+
+  BookmarkNodeVersionMap initial_versions;
+
+  // Verify transaction versions in sync model and bookmark model are equal
+  // after WriteTestDataToBookmarkModel() created bookmarks.
+  {
+    syncer::ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    EXPECT_GT(trans.GetModelVersion(syncer::BOOKMARKS), 0);
+    GetTransactionVersions(model_->root_node(), &initial_versions);
+    EXPECT_EQ(trans.GetModelVersion(syncer::BOOKMARKS),
+              initial_versions[model_->root_node()->id()]);
+  }
+  ExpectTransactionVersionMatch(model_->bookmark_bar_node(),
+                                BookmarkNodeVersionMap());
+  ExpectTransactionVersionMatch(model_->other_node(),
+                                BookmarkNodeVersionMap());
+  ExpectTransactionVersionMatch(model_->mobile_node(),
+                                BookmarkNodeVersionMap());
+
+  // Verify model version is incremented and bookmark node versions remain
+  // the same.
+  const BookmarkNode* bookmark_bar = model_->bookmark_bar_node();
+  model_->Remove(bookmark_bar, 0);

[Nicolas Zea, 2012/11/08 19:05:08]

Is removing a child expected to update the parent version?

Also, I'm a bit surprised this works. I wouldn't have expected the api to let
you remove a permanent node. That's likely to change, so it'd probably be better
to remove a different node.

[haitaol1, 2012/11/08 19:18:47]

No. The relationship is one-way from child to parent in directory.
It's the first child of bookmark bar being removed.

+  MessageLoop::current()->RunUntilIdle();
+  BookmarkNodeVersionMap new_versions;
+  GetTransactionVersions(model_->root_node(), &new_versions);
+  EXPECT_EQ(initial_versions[model_->root_node()->id()] + 1,
+            new_versions[model_->root_node()->id()]);
+  // HACK(haitaol): siblings of removed node are actually updated in sync model
+  //                because of NEXT_ID/PREV_ID. After switching to ordinal,
+  //                siblings will not get updated and the hack below can be
+  //                removed.
+  model_->SetNodeMetaInfo(bookmark_bar->GetChild(0),
+                          kBookmarkTransactionVersionKey, "43");
+  initial_versions[bookmark_bar->GetChild(0)->id()] = 43;
+  ExpectTransactionVersionMatch(model_->bookmark_bar_node(), initial_versions);
+  ExpectTransactionVersionMatch(model_->other_node(), initial_versions);
+  ExpectTransactionVersionMatch(model_->mobile_node(), initial_versions);
+
+  // Verify model version and version of changed bookmark are incremented and
+  // versions of others remain same.
+  const BookmarkNode* changed_bookmark =
+      model_->bookmark_bar_node()->GetChild(0);
+  model_->SetTitle(changed_bookmark, WideToUTF16Hack(L"test"));
+  MessageLoop::current()->RunUntilIdle();
+  GetTransactionVersions(model_->root_node(), &new_versions);
+  EXPECT_EQ(initial_versions[model_->root_node()->id()] + 2,

[Nicolas Zea, 2012/11/08 19:05:08]

why has the root node version gone up here? Does modifying a child updated the
parent version?

[haitaol1, 2012/11/08 19:18:47]

The version on root node is for the whole model. So it's incremented when a
change happens.

[Nicolas Zea, 2012/11/08 19:26:00]

Ah, I see. Maybe modify the test comment to make that clearer?

[haitaol1, 2012/11/08 19:44:25]

Done.

+            new_versions[model_->root_node()->id()]);
+  EXPECT_EQ(initial_versions[changed_bookmark->id()] + 1,
+            new_versions[changed_bookmark->id()]);
+  initial_versions.erase(changed_bookmark->id());
+  ExpectTransactionVersionMatch(model_->bookmark_bar_node(), initial_versions);
+  ExpectTransactionVersionMatch(model_->other_node(), initial_versions);
+  ExpectTransactionVersionMatch(model_->mobile_node(), initial_versions);
+}
+
 }  // namespace
 
 }  // namespace browser_sync