Implement changes_remaining GetUpdates response; this will reduce

the number of GetUpdates that the client actually does, since
"changes_remaining == 0" is treated as a termination condition.

Also, add an indication of client identity to the logger.

BUG=55809
TEST=sync_integration_tests, chromiumsync_test.py

Review URL: http://codereview.chromium.org/3462006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@60229 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 82 insertions, 43 deletions

diff --git a/net/tools/testserver/chromiumsync.py b/net/tools/testserver/chromiumsync.py
index c36054c..843761a 100755
--- a/net/tools/testserver/chromiumsync.py
+++ b/net/tools/testserver/chromiumsync.py
@@ -9,9 +9,11 @@ The details of the protocol are described mostly by comments in the protocol
 buffer definition at chrome/browser/sync/protocol/sync.proto.
 """
 
+import cgi
 import copy
 import operator
 import random
+import sys
 import threading
 
 import app_specifics_pb2
@@ -395,9 +397,11 @@ class SyncDataModel(object):
         more recently than this value will be retrieved; older items will
         be filtered out.
     Returns:
-      A tuple of (version, entries).  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.
+      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 timestamp == 0:
       self._CreatePermanentItems(requested_types)
@@ -409,7 +413,7 @@ class SyncDataModel(object):
     batch = new_changes[:self._BATCH_SIZE]
     if not batch:
       # Client is up to date.
-      return (timestamp, [])
+      return (timestamp, [], 0)
 
     # Restrict batch to requested types.  Tombstones are untyped
     # and will always get included.
@@ -418,7 +422,7 @@ class SyncDataModel(object):
 
     # The new client timestamp is the timestamp of the last item in the
     # batch, even if that item was filtered out.
-    return (batch[-1].version, filtered)
+    return (batch[-1].version, filtered, len(new_changes) - len(batch))
 
   def _CheckVersionForCommit(self, entry):
     """Perform an optimistic concurrency check on the version number.
@@ -607,6 +611,7 @@ class SyncDataModel(object):
     self._SaveEntry(entry)
     return entry
 
+
 class TestServer(object):
   """An object to handle requests for one (and only one) Chrome Sync account.
 
@@ -619,8 +624,23 @@ class TestServer(object):
     # The implementation supports exactly one account; its state is here.
     self.account = SyncDataModel()
     self.account_lock = threading.Lock()
-
-  def HandleCommand(self, raw_request):
+    # Clients that have talked to us: a map from the full client ID
+    # to its nickname.
+    self.clients = {}
+    self.client_name_generator = ('+' * times + chr(c)
+        for times in xrange(0, sys.maxint) for c in xrange(ord('A'),ord('Z')))
+
+  def GetShortClientName(self, query):
+    parsed = cgi.parse_qs(query[query.find('?')+1:])
+    client_id = parsed.get('client_id')
+    if not client_id:
+      return '?'
+    client_id = client_id[0]
+    if client_id not in self.clients:
+      self.clients[client_id] = self.client_name_generator.next()
+    return self.clients[client_id]
+
+  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
@@ -643,19 +663,21 @@ class TestServer(object):
       response = sync_pb2.ClientToServerResponse()
       response.error_code = sync_pb2.ClientToServerResponse.SUCCESS
       response.store_birthday = self.account.store_birthday
+      log_context = "[Client %s -> %s.py]" % (self.GetShortClientName(query),
+                                              __name__)
 
       if contents == sync_pb2.ClientToServerMessage.AUTHENTICATE:
-        print 'Authenticate'
+        print '%s Authenticate' % log_context
         # We accept any authentication token, and support only one account.
         # TODO(nick): Mock out the GAIA authentication as well; hook up here.
         response.authenticate.user.email = 'syncjuser@chromium'
         response.authenticate.user.display_name = 'Sync J User'
       elif contents == sync_pb2.ClientToServerMessage.COMMIT:
-        print 'Commit'
+        print '%s Commit' % log_context
         self.HandleCommit(request.commit, response.commit)
       elif contents == sync_pb2.ClientToServerMessage.GET_UPDATES:
-        print ('GetUpdates from timestamp %d' %
-               request.get_updates.from_timestamp)
+        print ('%s GetUpdates from timestamp %d' %
+               (log_context, request.get_updates.from_timestamp))
         self.HandleGetUpdates(request.get_updates, response.get_updates)
       return (200, response.SerializeToString())
     finally:
@@ -716,9 +738,10 @@ class TestServer(object):
     """
     update_response.SetInParent()
     requested_types = GetRequestedTypes(update_request)
-    new_timestamp, entries = self.account.GetChangesFromTimestamp(
+    new_timestamp, entries, remaining = self.account.GetChangesFromTimestamp(
         requested_types, update_request.from_timestamp)
 
+    update_response.changes_remaining = remaining
     # If the client is up to date, we are careful not to set the
     # new_timestamp field.
     if new_timestamp != update_request.from_timestamp:
diff --git a/net/tools/testserver/chromiumsync_test.py b/net/tools/testserver/chromiumsync_test.py
index 6d2beef..f753c3b 100755
--- a/net/tools/testserver/chromiumsync_test.py
+++ b/net/tools/testserver/chromiumsync_test.py
@@ -116,7 +116,8 @@ class SyncDataModelTest(unittest.TestCase):
       self.model = chromiumsync.SyncDataModel()
       request_types = [sync_type, chromiumsync.TOP_LEVEL]
 
-      version, changes = self.model.GetChangesFromTimestamp(request_types, 0)
+      version, changes, remaining = (
+          self.model.GetChangesFromTimestamp(request_types, 0))
 
       expected_count = self.ExpectedPermanentItemCount(sync_type)
       self.assertEqual(expected_count, version)
@@ -128,19 +129,24 @@ class SyncDataModelTest(unittest.TestCase):
         self.assertTrue(change.version <= version)
 
       # Test idempotence: another GetUpdates from ts=0 shouldn't recreate.
-      version, changes = self.model.GetChangesFromTimestamp(request_types, 0)
+      version, changes, remaining = (
+          self.model.GetChangesFromTimestamp(request_types, 0))
       self.assertEqual(expected_count, version)
       self.assertEqual(expected_count, len(changes))
+      self.assertEqual(0, remaining)
 
       # Doing a wider GetUpdates from timestamp zero shouldn't recreate either.
-      new_version, changes = self.model.GetChangesFromTimestamp(
-          chromiumsync.ALL_TYPES, 0)
+      new_version, changes, remaining = (
+          self.model.GetChangesFromTimestamp(chromiumsync.ALL_TYPES, 0))
       self.assertEqual(len(chromiumsync.SyncDataModel._PERMANENT_ITEM_SPECS),
                        new_version)
       self.assertEqual(new_version, len(changes))
-      version, changes = self.model.GetChangesFromTimestamp(request_types, 0)
+      self.assertEqual(0, remaining)
+      version, changes, remaining = (
+          self.model.GetChangesFromTimestamp(request_types, 0))
       self.assertEqual(new_version, version)
       self.assertEqual(expected_count, len(changes))
+      self.assertEqual(0, remaining)
 
   def testBatchSize(self):
     for sync_type in chromiumsync.ALL_TYPES[1:]:
@@ -153,19 +159,23 @@ class SyncDataModelTest(unittest.TestCase):
         entry.id_string = 'batch test %d' % i
         entry.specifics.CopyFrom(specifics)
         self.model._SaveEntry(entry)
-      version, changes = self.model.GetChangesFromTimestamp(request_types, 0)
+      last_bit = self.ExpectedPermanentItemCount(sync_type)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, 0))
       self.assertEqual(self.model._BATCH_SIZE, version)
-      version, changes = self.model.GetChangesFromTimestamp(request_types,
-                                                            version)
+      self.assertEqual(self.model._BATCH_SIZE*2 + last_bit, changes_remaining)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, version))
       self.assertEqual(self.model._BATCH_SIZE*2, version)
-      version, changes = self.model.GetChangesFromTimestamp(request_types,
-                                                            version)
+      self.assertEqual(self.model._BATCH_SIZE + last_bit, changes_remaining)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, version))
       self.assertEqual(self.model._BATCH_SIZE*3, version)
-      expected_dingleberry = self.ExpectedPermanentItemCount(sync_type)
-      version, changes = self.model.GetChangesFromTimestamp(request_types,
-                                                            version)
-      self.assertEqual(self.model._BATCH_SIZE*3 + expected_dingleberry,
-                       version)
+      self.assertEqual(last_bit, changes_remaining)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, version))
+      self.assertEqual(self.model._BATCH_SIZE*3 + last_bit, version)
+      self.assertEqual(0, changes_remaining)
 
       # Now delete a third of the items.
       for i in xrange(self.model._BATCH_SIZE*3 - 1, 0, -3):
@@ -175,19 +185,23 @@ class SyncDataModelTest(unittest.TestCase):
         self.model._SaveEntry(entry)
 
       # The batch counts shouldn't change.
-      version, changes = self.model.GetChangesFromTimestamp(request_types, 0)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, 0))
       self.assertEqual(self.model._BATCH_SIZE, len(changes))
-      version, changes = self.model.GetChangesFromTimestamp(request_types,
-                                                            version)
+      self.assertEqual(self.model._BATCH_SIZE*2 + last_bit, changes_remaining)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, version))
       self.assertEqual(self.model._BATCH_SIZE, len(changes))
-      version, changes = self.model.GetChangesFromTimestamp(request_types,
-                                                            version)
+      self.assertEqual(self.model._BATCH_SIZE + last_bit, changes_remaining)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, version))
       self.assertEqual(self.model._BATCH_SIZE, len(changes))
-      expected_dingleberry = self.ExpectedPermanentItemCount(sync_type)
-      version, changes = self.model.GetChangesFromTimestamp(request_types,
-                                                            version)
-      self.assertEqual(expected_dingleberry, len(changes))
-      self.assertEqual(self.model._BATCH_SIZE*4 + expected_dingleberry, version)
+      self.assertEqual(last_bit, changes_remaining)
+      version, changes, changes_remaining = (
+          self.model.GetChangesFromTimestamp(request_types, version))
+      self.assertEqual(last_bit, len(changes))
+      self.assertEqual(self.model._BATCH_SIZE*4 + last_bit, version)
+      self.assertEqual(0, changes_remaining)
 
   def testCommitEachDataType(self):
     for sync_type in chromiumsync.ALL_TYPES[1:]:
@@ -198,7 +212,7 @@ class SyncDataModelTest(unittest.TestCase):
       commit_session = {}
 
       # Start with a GetUpdates from timestamp 0, to populate permanent items.
-      original_version, original_changes = (
+      original_version, original_changes, changes_remaining = (
           self.model.GetChangesFromTimestamp([sync_type], 0))
 
       def DoCommit(original=None, id_string='', name=None, parent=None,
@@ -251,9 +265,10 @@ class SyncDataModelTest(unittest.TestCase):
         self.assertTrue(r.version > original_version)
       self.assertEqual(result1.parent_id_string, proto1.parent_id_string)
       self.assertEqual(result2.parent_id_string, result1.id_string)
-      version, changes = self.model.GetChangesFromTimestamp([sync_type],
-                                                            original_version)
+      version, changes, remaining = (
+          self.model.GetChangesFromTimestamp([sync_type], original_version))
       self.assertEqual(3, len(changes))
+      self.assertEqual(0, remaining)
       self.assertEqual(original_version + 3, version)
       self.assertEqual([result1, result2, result3], changes)
       for c in changes:
@@ -298,9 +313,10 @@ class SyncDataModelTest(unittest.TestCase):
         self.assertTrue(p is not self.model._entries[r.id_string],
                         "Commit didn't make a defensive copy.")
         self.assertTrue(r.version > p.version)
-      version, changes = self.model.GetChangesFromTimestamp([sync_type],
-                                                            original_version)
+      version, changes, remaining = (
+          self.model.GetChangesFromTimestamp([sync_type], original_version))
       self.assertEqual(5, len(changes))
+      self.assertEqual(0, remaining)
       self.assertEqual(original_version + 7, version)
       self.assertEqual([result3, result2b, result4, result1b, result5], changes)
       for c in changes:
diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py
index ee2476c..9f99d71 100644
--- a/net/tools/testserver/testserver.py
+++ b/net/tools/testserver/testserver.py
@@ -1018,7 +1018,7 @@ class TestPageHandler(BaseHTTPServer.BaseHTTPRequestHandler):
       import chromiumsync
       self.server._sync_handler = chromiumsync.TestServer()
     http_response, raw_reply = self.server._sync_handler.HandleCommand(
-        raw_request)
+        self.path, raw_request)
     self.send_response(http_response)
     self.end_headers()
     self.wfile.write(raw_reply)