[Sync] Add support for associating a new Synced Bookmarks node.

net/tools/testserver/chromiumsync.py

 # 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.
 
 """An implementation of the server side of the Chromium sync protocol.
 
 The details of the protocol are described mostly by comments in the protocol
 buffer definition at chrome/browser/sync/protocol/sync.proto.
 """
 
@@ skipping 246 matching lines @@
   Attributes:
     tag: A known-to-the-client value that uniquely identifies a server-created
       permanent item.
     name: The human-readable display name for this item.
     parent_tag: The tag of the permanent item's parent.  If ROOT_ID, indicates
       a top-level item.  Otherwise, this must be the tag value of some other
       server-created permanent item.
     sync_type: A value from ALL_TYPES, giving the datatype of this permanent
       item.  This controls which types of client GetUpdates requests will
       cause the permanent item to be created and returned.
+    create_by_default: Whether the permanent item is created at startup or not.
+      This value is set to True in the default case. Non-default permanent items
+      are those that are created only when a client explicitly tells the server
+      to do so.
   """
 
-  def __init__(self, tag, name, parent_tag, sync_type):
+  def __init__(self, tag, name, parent_tag, sync_type, create_by_default=True):
     self.tag = tag
     self.name = name
     self.parent_tag = parent_tag
     self.sync_type = sync_type
+    self.create_by_default = create_by_default
 
 
 class MigrationHistory(object):
   """A record of the migration events associated with an account.
 
   Each migration event invalidates one or more datatypes on all clients
   that had synced the datatype before the event.  Such clients will continue
   to receive MigrationDone errors until they throw away their progress and
   re-sync that datatype from the beginning.
   """
@@ skipping 118 matching lines @@
   # Specify all the permanent items that a model might need.
   _PERMANENT_ITEM_SPECS = [
       PermanentItem('google_chrome', name='Google Chrome',
                     parent_tag=ROOT_ID, sync_type=TOP_LEVEL),
       PermanentItem('google_chrome_bookmarks', name='Bookmarks',
                     parent_tag='google_chrome', sync_type=BOOKMARK),
       PermanentItem('bookmark_bar', name='Bookmark Bar',
                     parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK),
       PermanentItem('other_bookmarks', name='Other Bookmarks',
                     parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK),
+      PermanentItem('synced_bookmarks', name='Synced Bookmarks',
+                    parent_tag='google_chrome_bookmarks', sync_type=BOOKMARK,
+                    create_by_default=False),
       PermanentItem('google_chrome_preferences', name='Preferences',
                     parent_tag='google_chrome', sync_type=PREFERENCE),
       PermanentItem('google_chrome_autofill', name='Autofill',
                     parent_tag='google_chrome', sync_type=AUTOFILL),
       PermanentItem('google_chrome_autofill_profiles', name='Autofill Profiles',
                     parent_tag='google_chrome', sync_type=AUTOFILL_PROFILE),
       PermanentItem('google_chrome_app_settings',
                     name='App Settings',
                     parent_tag='google_chrome', sync_type=APP_SETTINGS),
       PermanentItem('google_chrome_extension_settings',
@@ skipping 166 matching lines @@
     entry.id_string = id_string
     entry.non_unique_name = spec.name
     entry.name = spec.name
     entry.server_defined_unique_tag = spec.tag
     entry.folder = True
     entry.deleted = False
     entry.specifics.CopyFrom(GetDefaultEntitySpecifics(spec.sync_type))
     self._WritePosition(entry, self._ServerTagToId(spec.parent_tag))
     self._SaveEntry(entry)
 
-  def _CreatePermanentItems(self, requested_types):
-    """Ensure creation of all permanent items for a given set of sync types.
+  def _CreateDefaultPermanentItems(self, requested_types):
+    """Ensure creation of all default permanent items for a given set of types.
 
     Args:
       requested_types: A list of sync data types from ALL_TYPES.
-        Permanent items of only these types will be created.
+        All default permanent items of only these types will be created.
     """
     for spec in self._PERMANENT_ITEM_SPECS:
-      if spec.sync_type in requested_types:
+      if spec.sync_type in requested_types and spec.create_by_default:
         self._CreatePermanentItem(spec)
 
   def ResetStoreBirthday(self):
     """Resets the store birthday to a random value."""
     # TODO(nick): uuid.uuid1() is better, but python 2.5 only.
     self.store_birthday = '%0.30f' % random.random()
 
   def StoreBirthday(self):
     """Gets the store birthday."""
     return self.store_birthday
@@ skipping 10 matching lines @@
     Returns:
       A tuple of (version, entries, changes_remaining).  Version is a new
       timestamp value, which should be used as the starting point for the
       next query.  Entries is the batch of entries meeting the current
       timestamp query.  Changes_remaining indicates the number of changes
       left on the server after this batch.
     """
     if not sieve.HasAnyTimestamp():
       return (0, [], 0)
     min_timestamp = sieve.GetMinTimestamp()
-    self._CreatePermanentItems(sieve.GetFirstTimeTypes())
+    self._CreateDefaultPermanentItems(sieve.GetFirstTimeTypes())
     change_log = sorted(self._entries.values(),
                         key=operator.attrgetter('version'))
     new_changes = [x for x in change_log if x.version > min_timestamp]
     # Pick batch_size new changes, and then filter them.  This matches
     # the RPC behavior of the production sync server.
     batch = new_changes[:self._BATCH_SIZE]
     if not batch:
       # Client is up to date.
       return (min_timestamp, [], 0)
 
@@ skipping 270 matching lines @@
     """
 
     nigori_tag = "google_chrome_nigori"
     nigori_original = self._entries.get(self._ServerTagToId(nigori_tag))
     if (nigori_original.specifics.nigori.sync_tabs):
       return
     nigori_new = copy.deepcopy(nigori_original)
     nigori_new.specifics.nigori.sync_tabs = True
     self._SaveEntry(nigori_new)
 
+  def TriggerCreateSyncedBookmarks(self):
+    """Create the Synced Bookmarks folder under the Bookmarks permanent item.
+
+    Clients will then receive the Synced Bookmarks folder on future
+    GetUpdates, and new bookmarks can be added within the Synced Bookmarks
+    folder.
+    """
+
+    synced_bookmarks_spec = (spec for spec in self._PERMANENT_ITEM_SPECS if
+                             spec.tag == "synced_bookmarks").next()

[ncarter (slow), 2012/03/15 22:44:52]

This is a pretty unidiomatic use of a generator expression.

The first improvement I'd suggest would be to use a list comprehension and not a
generator expression, unless there's a clear benefit to suspending the
evaluation. When you're doing a filtering operation like this, there's no harm
in having the loop evaluate completely.  But the style guide isn't explicit on
this, it's more of a personal preference. Here, it's your call.

Secondly, calling .next() on the result of a generator expression is a code
smell: comprehensions are syntactic sugar that are supposed to let your code
read naturally, and .next() breaks that sweet illusion. In essence you're
forcing me to remember that "the semantics of a generator expression are
equivalent to creating an anonymous generator function and calling it".  Don't
make me remember this.

Fortunately there's a good python idiom for unpacking lists.  You are probably
familiar with this:

 a, b = [1, 2]  # afterwards, a = 1 and b = 2

And this:
 a = [1]  # afterwards, a = [1]

But did you know about this?
 a, = [1]  # afterwards, a = 1

Thirdly, the convention I've seen for formatting multi-line generator
expressions is to prefer breaking the line before the keywords ("for" / "in" /
"if").  Reference:
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Generators

+    self._CreatePermanentItem(synced_bookmarks_spec)
+
   def SetInducedError(self, error, error_frequency,
                       sync_count_before_errors):
     self.induced_error = error
     self.induced_error_frequency = error_frequency
     self.sync_count_before_errors = sync_count_before_errors
 
   def GetInducedError(self):
     return self.induced_error
 
 
@@ skipping 129 matching lines @@
         200,
         '<html><title>Transient error</title><H1>Transient error</H1></html>')
 
   def HandleSetSyncTabs(self):
     """Set the 'sync_tab' field of the nigori node for this account."""
     self.account.TriggerSyncTabs()
     return (
         200,
         '<html><title>Sync Tabs</title><H1>Sync Tabs</H1></html>')
 
+  def HandleCreateSyncedBookmarks(self):
+    """Create the Synced Bookmarks folder under Bookmarks."""
+    self.account.TriggerCreateSyncedBookmarks()
+    return (
+        200,
+        '<html><title>Synced Bookmarks</title><H1>Synced Bookmarks</H1></html>')
+
   def HandleCommand(self, query, raw_request):
     """Decode and handle a sync command from a raw input of bytes.
 
     This is the main entry point for this class.  It is safe to call this
     method from multiple threads.
 
     Args:
       raw_request: An iterable byte sequence to be interpreted as a sync
         protocol command.
     Returns:
@@ skipping 139 matching lines @@
 
     update_sieve.CheckMigrationState()
 
     new_timestamp, entries, remaining = self.account.GetChanges(update_sieve)
 
     update_response.changes_remaining = remaining
     for entry in entries:
       reply = update_response.entries.add()
       reply.CopyFrom(entry)
     update_sieve.SaveProgress(new_timestamp, update_response)