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 <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 417 matching lines @@
                             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.
+  void ExpectTransactionVersionMatch(const BookmarkNode* node);
+
  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));
 }
 
+int64 GetTransactionVersion(const BookmarkNode* node) {
+  std::string version_str;
+  int64 version;
+  EXPECT_TRUE(node->GetMetaInfo(kBookmarkTransactionVersionKey, &version_str));
+  EXPECT_TRUE(base::StringToInt64(version_str, &version));
+  return version;
+}
+
+void ProfileSyncServiceBookmarkTestWithData::ExpectTransactionVersionMatch(
+    const BookmarkNode* node) {
+  std::queue<const BookmarkNode*> nodes;
+  nodes.push(node);
+  while (!nodes.empty()) {
+    const BookmarkNode* n = nodes.front();
+    nodes.pop();
+
+    syncer::ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    syncer::ReadNode sync_node(&trans);
+    ASSERT_TRUE(model_associator_->InitSyncNodeFromChromeId(n->id(),
+                                                            &sync_node));
+    EXPECT_EQ(sync_node.GetEntry()->Get(syncer::syncable::TRANSACTION_VERSION),
+              GetTransactionVersion(n));
+    for (int i = 0; i < n->child_count(); ++i)
+      nodes.push(n->GetChild(i));
+  }
+}
+
+// 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();
+
+  // Verify model versions are equal.
+  {
+    syncer::ReadTransaction trans(FROM_HERE, test_user_share_.user_share());
+    EXPECT_EQ(trans.GetModelVersion(syncer::BOOKMARKS),
+              GetTransactionVersion(model_->root_node()));
+  }
+
+  // Verify node versions are equal.
+  ExpectTransactionVersionMatch(model_->bookmark_bar_node());
+  ExpectTransactionVersionMatch(model_->other_node());
+  ExpectTransactionVersionMatch(model_->mobile_node());
+
+  // verify version is incremented when bookmark is removed.
+  int64 old_model_version = GetTransactionVersion(model_->root_node());
+  model_->Remove(model_->bookmark_bar_node(), 0);
+  MessageLoop::current()->RunUntilIdle();
+  EXPECT_EQ(old_model_version + 1, GetTransactionVersion(model_->root_node()));
+
+  // Verify version is updated when bookmark is changed.
+  old_model_version = GetTransactionVersion(model_->root_node());
+  model_->SetTitle(model_->bookmark_bar_node()->GetChild(0),
+                   WideToUTF16Hack(L"test"));
+  MessageLoop::current()->RunUntilIdle();
+  EXPECT_EQ(old_model_version + 1, GetTransactionVersion(model_->root_node()));
+  EXPECT_EQ(old_model_version + 1,
+            GetTransactionVersion(model_->bookmark_bar_node()->GetChild(0)));
+  ExpectTransactionVersionMatch(model_->bookmark_bar_node()->GetChild(0));
+}
+

[Nicolas Zea, 2012/11/07 00:20:31]

do children get updated when a parent is modified? Perhaps include a expectation
of that here?

Also, verify that the version does not change if the node itself doesn't change
(for example by setting specifics to the same value)? Or does the version always
change?

[haitaol1, 2012/11/07 22:20:25]

Parent is not updated but siblings do because of NEXT_ID/PREV_ID under current
implementation, but not with ordinal in the future. Added expectations to verify
versions of other nodes are unchanged with hack for sibling.
The version always change in test because test uses NullDirectoryChangeDelegate,
which I changed to simply return IDs of mutations always without diffing.

 }  // namespace
 
 }  // namespace browser_sync