Allow server handlers to specify capabilities required by clients.
authorDave Borowitz <dborowitz@google.com>
Fri, 19 Feb 2010 23:50:08 +0000 (15:50 -0800)
committerDave Borowitz <dborowitz@google.com>
Thu, 4 Mar 2010 17:50:05 +0000 (09:50 -0800)
Ideally, the server should be able to handle the absence (as well as
presence) of any capabilities it advertises. However, currently, the
server does not know how to *not* support some capabilities, such
as side-band-64k and delete-refs, which makes it incompatible with
clients that do not request these capabilities. This change adds a
set of "required" capabilites to the server handlers, and causes the
server to fail fast if the client does not request them.

Capabilities should be removed from the required capabilities lists as
their inverses are implemented.

Note that the protocol capabilities spec[1] makes no requirements
about servers supporting the inverse of capabilities.

[1] http://repo.or.cz/w/git.git/blob/HEAD:/Documentation/technical/protocol-capabilities.txt

Change-Id: I6ad2dc3dba67b78b2e195ec0b77eace09a36b0f5

dulwich/server.py
dulwich/tests/test_server.py

index 8df6be6663237ee3ffc8eaaa2da9b9f5466e54da..afe87b35d53508d8fa288016401d4cd743e7aa65 100644 (file)
@@ -160,18 +160,13 @@ class Handler(object):
     def capabilities(self):
         raise NotImplementedError(self.capabilities)
 
-<<<<<<< HEAD
-    def set_client_capabilities(self, caps):
-        my_caps = self.capabilities()
-        for cap in caps:
-            if cap not in my_caps:
-                raise GitProtocolError('Client asked for capability %s that '
-                                       'was not advertised.' % cap)
-        self._client_capabilities = set(caps)
-=======
     def innocuous_capabilities(self):
         return ("include-tag", "thin-pack", "no-progress", "ofs-delta")
 
+    def required_capabilities(self):
+        """Return a list of capabilities that we require the client to have."""
+        return []
+
     def set_client_capabilities(self, caps):
         allowable_caps = set(self.innocuous_capabilities())
         allowable_caps.update(self.capabilities())
@@ -179,8 +174,11 @@ class Handler(object):
             if cap not in allowable_caps:
                 raise GitProtocolError('Client asked for capability %s that '
                                        'was not advertised.' % cap)
-        self._client_capabilities = caps
->>>>>>> Refactor server capability code into base Handler.
+        for cap in self.required_capabilities():
+            if cap not in caps:
+                raise GitProtocolError('Client does not support required '
+                                       'capability %s.' % cap)
+        self._client_capabilities = set(caps)
 
     def has_capability(self, cap):
         if self._client_capabilities is None:
@@ -203,14 +201,14 @@ class UploadPackHandler(Handler):
         return ("multi_ack_detailed", "multi_ack", "side-band-64k", "thin-pack",
                 "ofs-delta", "no-progress")
 
-<<<<<<< HEAD
+    def required_capabilities(self):
+        return ("side-band-64k", "thin-pack", "ofs-delta")
+
     def progress(self, message):
         if self.has_capability("no-progress"):
             return
         self.proto.write_sideband(2, message)
 
-=======
->>>>>>> Refactor server capability code into base Handler.
     def handle(self):
         write = lambda x: self.proto.write_sideband(1, x)
 
@@ -532,6 +530,9 @@ class ReceivePackHandler(Handler):
     def capabilities(self):
         return ("report-status", "delete-refs")
 
+    def required_capabilities(self):
+        return ("delete-refs",)
+
     def handle(self):
         refs = self.backend.get_refs().items()
 
index 7a47909a75aaba716b1f81a371a3ea35045991e1..dff22dc1015dd5a127c285960ba5b6dd447bb2b5 100644 (file)
@@ -81,31 +81,34 @@ class HandlerTestCase(TestCase):
     def setUp(self):
         self._handler = Handler(None, None, None)
         self._handler.capabilities = lambda: ('cap1', 'cap2', 'cap3')
+        self._handler.required_capabilities = lambda: ('cap2',)
 
     def assertSucceeds(self, func, *args, **kwargs):
         try:
             func(*args, **kwargs)
-        except GitProtocolError:
-            self.fail()
+        except GitProtocolError, e:
+            self.fail(e)
 
     def test_capability_line(self):
         self.assertEquals('cap1 cap2 cap3', self._handler.capability_line())
 
     def test_set_client_capabilities(self):
         set_caps = self._handler.set_client_capabilities
-
-        self.assertSucceeds(set_caps, [])
         self.assertSucceeds(set_caps, ['cap2'])
         self.assertSucceeds(set_caps, ['cap1', 'cap2'])
+
         # different order
         self.assertSucceeds(set_caps, ['cap3', 'cap1', 'cap2'])
-        self.assertRaises(GitProtocolError, set_caps, ['capxxx', 'cap1'])
+
+        # error cases
+        self.assertRaises(GitProtocolError, set_caps, ['capxxx', 'cap2'])
+        self.assertRaises(GitProtocolError, set_caps, ['cap1', 'cap3'])
 
         # ignore innocuous but unknown capabilities
-        self.assertRaises(GitProtocolError, set_caps, ['ignoreme'])
+        self.assertRaises(GitProtocolError, set_caps, ['cap2', 'ignoreme'])
         self.assertFalse('ignoreme' in self._handler.capabilities())
         self._handler.innocuous_capabilities = lambda: ('ignoreme',)
-        self.assertSucceeds(set_caps, ['ignoreme'])
+        self.assertSucceeds(set_caps, ['cap2', 'ignoreme'])
 
     def test_has_capability(self):
         self.assertRaises(GitProtocolError, self._handler.has_capability, 'cap')
@@ -122,7 +125,8 @@ class UploadPackHandlerTestCase(TestCase):
         self._handler.proto = TestProto()
 
     def test_progress(self):
-        self._handler.set_client_capabilities([])
+        caps = self._handler.required_capabilities()
+        self._handler.set_client_capabilities(caps)
         self._handler.progress('first message')
         self._handler.progress('second message')
         self.assertEqual('first message',
@@ -132,7 +136,8 @@ class UploadPackHandlerTestCase(TestCase):
         self.assertEqual(None, self._handler.proto.get_received_line(2))
 
     def test_no_progress(self):
-        self._handler.set_client_capabilities(['no-progress'])
+        caps = list(self._handler.required_capabilities()) + ['no-progress']
+        self._handler.set_client_capabilities(caps)
         self._handler.progress('first message')
         self._handler.progress('second message')
         self.assertEqual(None, self._handler.proto.get_received_line(2))