Rev 2439: Use less confusing version strings, and define REQUEST_VERSION_TWO/RESPONSE_VERSION_TWO constants for them. in http://bazaar.launchpad.net/~bzr/bzr/hpss-protocol2

Andrew Bennetts andrew.bennetts at canonical.com
Wed Apr 25 05:59:48 BST 2007


At http://bazaar.launchpad.net/~bzr/bzr/hpss-protocol2

------------------------------------------------------------
revno: 2439
revision-id: andrew.bennetts at canonical.com-20070425045619-gfsty2ebwx3c5rka
parent: andrew.bennetts at canonical.com-20070424090150-9buzrpi8lt6qv0tg
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: hpss-protocol2
timestamp: Wed 2007-04-25 14:56:19 +1000
message:
  Use less confusing version strings, and define REQUEST_VERSION_TWO/RESPONSE_VERSION_TWO constants for them.
modified:
  bzrlib/smart/medium.py         medium.py-20061103051856-rgu2huy59fkz902q-1
  bzrlib/smart/protocol.py       protocol.py-20061108035435-ot0lstk2590yqhzr-1
  bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
  bzrlib/tests/test_wsgi.py      test_wsgi.py-20061005091552-rz8pva0olkxv0sd8-1
  bzrlib/transport/http/wsgi.py  wsgi.py-20061005091552-rz8pva0olkxv0sd8-2
=== modified file 'bzrlib/smart/medium.py'
--- a/bzrlib/smart/medium.py	2007-04-24 08:36:32 +0000
+++ b/bzrlib/smart/medium.py	2007-04-25 04:56:19 +0000
@@ -30,6 +30,7 @@
 
 from bzrlib import errors
 from bzrlib.smart.protocol import (
+    REQUEST_VERSION_TWO,
     SmartServerRequestProtocolOne,
     SmartServerRequestProtocolTwo,
     )
@@ -78,10 +79,10 @@
 
     def _build_protocol(self):
         # Identify the protocol version.
-        bytes = self._get_bytes(2)
-        if bytes.startswith('2\n'):
+        bytes = self._get_line()
+        if bytes.startswith(REQUEST_VERSION_TWO):
             protocol_class = SmartServerRequestProtocolTwo
-            bytes = bytes[2:]
+            bytes = bytes[len(REQUEST_VERSION_TWO):]
         else:
             protocol_class = SmartServerRequestProtocolOne
         protocol = protocol_class(self.backing_transport, self._write_out)
@@ -111,6 +112,24 @@
         """
         raise NotImplementedError(self._get_bytes)
 
+    def _get_line(self):
+        """Read bytes from this request's response until a newline byte.
+        
+        This isn't particularly efficient, so should only be used when the
+        expected size of the line is quite short.
+
+        :returns: a string of bytes ending in a newline (byte 0x0A).
+        """
+        # XXX: this duplicates SmartClientRequestProtocolOne._recv_tuple
+        line = ''
+        while not line or line[-1] != '\n':
+            new_char = self._get_bytes(1)
+            line += new_char
+            if new_char == '':
+                # Ran out of bytes before receiving a complete line.
+                break
+        return line
+
 
 class SmartServerSocketStreamMedium(SmartServerStreamMedium):
 
@@ -300,7 +319,7 @@
 
         This method will block and wait for count bytes to be read. It may not
         be invoked until finished_writing() has been called - this is to ensure
-        a message-based approach to requests, for compatability with message
+        a message-based approach to requests, for compatibility with message
         based mediums like HTTP.
         """
         if self._state == "writing":
@@ -318,6 +337,24 @@
         """
         raise NotImplementedError(self._read_bytes)
 
+    def read_line(self):
+        """Read bytes from this request's response until a newline byte.
+        
+        This isn't particularly efficient, so should only be used when the
+        expected size of the line is quite short.
+
+        :returns: a string of bytes ending in a newline (byte 0x0A).
+        """
+        # XXX: this duplicates SmartClientRequestProtocolOne._recv_tuple
+        line = ''
+        while not line or line[-1] != '\n':
+            new_char = self.read_bytes(1)
+            line += new_char
+            if new_char == '':
+                raise errors.SmartProtocolError(
+                    'unexpected end of file reading from server')
+        return line
+
 
 class SmartClientMedium(object):
     """Smart client is a medium for sending smart protocol requests over."""

=== modified file 'bzrlib/smart/protocol.py'
--- a/bzrlib/smart/protocol.py	2007-04-24 08:36:32 +0000
+++ b/bzrlib/smart/protocol.py	2007-04-25 04:56:19 +0000
@@ -25,6 +25,13 @@
 from bzrlib.smart import request
 
 
+# Protocol version strings.  These are sent as prefixes of bzr requests and
+# responses to identify the protocol version being used. (There are no version
+# one strings because that version doesn't send any).
+REQUEST_VERSION_TWO = 'bzr request 2\n'
+RESPONSE_VERSION_TWO = 'bzr response 2\n'
+
+
 def _recv_tuple(from_file):
     req_line = from_file.readline()
     return _decode_tuple(req_line)
@@ -160,15 +167,15 @@
 class SmartServerRequestProtocolTwo(SmartServerRequestProtocolOne):
     r"""Version two of the server side of the smart protocol.
    
-    This prefixes responses with the protocol version: "2\n".
+    This prefixes responses with the value of RESPONSE_VERSION_TWO.
     """
 
     def _write_protocol_version(self):
         r"""Write any prefixes this protocol requires.
         
-        Version two sends "2\n".
+        Version two sends the value of RESPONSE_VERSION_TWO.
         """
-        self._write_func('2\n')
+        self._write_func(RESPONSE_VERSION_TWO)
 
 
 class LengthPrefixedBodyDecoder(object):
@@ -372,27 +379,25 @@
 
 
 class SmartClientRequestProtocolTwo(SmartClientRequestProtocolOne):
-    r"""Version two of the client side of the smart protocol.
+    """Version two of the client side of the smart protocol.
     
-    This prefixes the request with the protocol version: "2\n".
+    This prefixes the request with the value of REQUEST_VERSION_TWO.
     """
 
-    _version_string = '2\n'
-
     def read_response_tuple(self, expect_body=False):
         """Read a response tuple from the wire.
 
         This should only be called once.
         """
-        version = self._request.read_bytes(2)
-        if version != SmartClientRequestProtocolTwo._version_string:
+        version = self._request.read_line()
+        if version != RESPONSE_VERSION_TWO:
             raise errors.SmartProtocolError('bad protocol marker %r' % version)
         return SmartClientRequestProtocolOne.read_response_tuple(self, expect_body)
 
     def _write_protocol_version(self):
         r"""Write any prefixes this protocol requires.
         
-        Version two sends "2\n".
+        Version two sends the value of REQUEST_VERSION_TWO.
         """
-        self._request.accept_bytes(SmartClientRequestProtocolTwo._version_string)
+        self._request.accept_bytes(REQUEST_VERSION_TWO)
 

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2007-04-24 08:50:29 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2007-04-25 04:56:19 +0000
@@ -806,23 +806,23 @@
         self.assertProtocolOne(server_protocol)
 
     def test_pipe_like_build_protocol_non_two(self):
-        # A request that doesn't start with "2\n" is version one.
-        server_protocol = self.build_protocol_pipe_like('2-')
+        # A request that doesn't start with "bzr request 2\n" is version one.
+        server_protocol = self.build_protocol_pipe_like('abc\n')
         self.assertProtocolOne(server_protocol)
 
     def test_socket_build_protocol_non_two(self):
-        # A request that doesn't start with "2\n" is version one.
-        server_protocol = self.build_protocol_socket('2-')
+        # A request that doesn't start with "bzr request 2\n" is version one.
+        server_protocol = self.build_protocol_socket('abc\n')
         self.assertProtocolOne(server_protocol)
 
     def test_pipe_like_build_protocol_two(self):
-        # A request that starts with "2\n" is version two.
-        server_protocol = self.build_protocol_pipe_like('2\n')
+        # A request that starts with "bzr request 2\n" is version two.
+        server_protocol = self.build_protocol_pipe_like('bzr request 2\n')
         self.assertProtocolTwo(server_protocol)
 
     def test_socket_build_protocol_two(self):
-        # A request that starts with "2\n" is version two.
-        server_protocol = self.build_protocol_socket('2\n')
+        # A request that starts with "bzr request 2\n" is version two.
+        server_protocol = self.build_protocol_socket('bzr request 2\n')
         self.assertProtocolTwo(server_protocol)
         
 
@@ -1556,7 +1556,8 @@
         self.assertEqual('abc', smart_protocol.in_buffer)
         smart_protocol.accept_bytes('\n')
         self.assertEqual(
-            "2\nerror\x01Generic bzr smart protocol error: bad request 'abc'\n",
+            protocol.RESPONSE_VERSION_TWO +
+            "error\x01Generic bzr smart protocol error: bad request 'abc'\n",
             out_stream.getvalue())
         self.assertTrue(smart_protocol.has_dispatched)
         self.assertEqual(0, smart_protocol.next_read_size())
@@ -1580,7 +1581,8 @@
                 out_stream.write)
         smart_protocol.accept_bytes('readv\x01foo\n3\n3,3done\n')
         self.assertEqual(0, smart_protocol.next_read_size())
-        self.assertEqual('2\nreadv\n3\ndefdone\n', out_stream.getvalue())
+        self.assertEqual(protocol.RESPONSE_VERSION_TWO + 'readv\n3\ndefdone\n',
+                         out_stream.getvalue())
         self.assertEqual('', smart_protocol.excess_buffer)
         self.assertEqual('', smart_protocol.in_buffer)
 
@@ -1589,31 +1591,38 @@
         smart_protocol = protocol.SmartServerRequestProtocolTwo(
             None, out_stream.write)
         smart_protocol.accept_bytes('hello\nhello\n')
-        self.assertEqual("2\nok\x012\n", out_stream.getvalue())
+        self.assertEqual(protocol.RESPONSE_VERSION_TWO + "ok\x012\n",
+                         out_stream.getvalue())
         self.assertEqual("hello\n", smart_protocol.excess_buffer)
         self.assertEqual("", smart_protocol.in_buffer)
 
     def test_accept_excess_bytes_after_body(self):
         # The excess bytes look like the start of another request.
-        protocol = self.build_protocol_waiting_for_body()
-        protocol.accept_bytes('7\nabcdefgdone\n2\n')
+        server_protocol = self.build_protocol_waiting_for_body()
+        server_protocol.accept_bytes(
+            '7\nabcdefgdone\n' + protocol.RESPONSE_VERSION_TWO)
         self.assertTrue(self.end_received)
-        self.assertEqual("2\n", protocol.excess_buffer)
-        self.assertEqual("", protocol.in_buffer)
-        protocol.accept_bytes('Y')
-        self.assertEqual("2\nY", protocol.excess_buffer)
-        self.assertEqual("", protocol.in_buffer)
+        self.assertEqual(protocol.RESPONSE_VERSION_TWO,
+                         server_protocol.excess_buffer)
+        self.assertEqual("", server_protocol.in_buffer)
+        server_protocol.accept_bytes('Y')
+        self.assertEqual(protocol.RESPONSE_VERSION_TWO + "Y",
+                         server_protocol.excess_buffer)
+        self.assertEqual("", server_protocol.in_buffer)
 
     def test_accept_excess_bytes_after_dispatch(self):
         out_stream = StringIO()
         smart_protocol = protocol.SmartServerRequestProtocolTwo(
             None, out_stream.write)
         smart_protocol.accept_bytes('hello\n')
-        self.assertEqual("2\nok\x012\n", out_stream.getvalue())
-        smart_protocol.accept_bytes('2\nhel')
-        self.assertEqual("2\nhel", smart_protocol.excess_buffer)
+        self.assertEqual(protocol.RESPONSE_VERSION_TWO + "ok\x012\n",
+                         out_stream.getvalue())
+        smart_protocol.accept_bytes(protocol.REQUEST_VERSION_TWO + 'hel')
+        self.assertEqual(protocol.REQUEST_VERSION_TWO + "hel",
+                         smart_protocol.excess_buffer)
         smart_protocol.accept_bytes('lo\n')
-        self.assertEqual("2\nhello\n", smart_protocol.excess_buffer)
+        self.assertEqual(protocol.REQUEST_VERSION_TWO + "hello\n",
+                         smart_protocol.excess_buffer)
         self.assertEqual("", smart_protocol.in_buffer)
 
     def test__send_response_sets_finished_reading(self):
@@ -1633,7 +1642,7 @@
         # accept_bytes(tuple_based_encoding_of_hello) and reads and parses the
         # response of tuple-encoded (ok, 1).  Also, seperately we should test
         # the error if the response is a non-understood version.
-        input = StringIO('2\nok\x012\n')
+        input = StringIO(protocol.RESPONSE_VERSION_TWO + 'ok\x012\n')
         output = StringIO()
         client_medium = medium.SmartSimplePipesClientMedium(input, output)
         request = client_medium.get_request()
@@ -1644,18 +1653,20 @@
         # protocol.call() can get back an empty tuple as a response. This occurs
         # when the parsed line is an empty line, and results in a tuple with
         # one element - an empty string.
-        self.assertServerToClientEncoding('2\n\n', ('', ), [(), ('', )])
+        self.assertServerToClientEncoding(
+            protocol.RESPONSE_VERSION_TWO + '\n', ('', ), [(), ('', )])
 
     def test_client_call_three_element_response(self):
         # protocol.call() can get back tuples of other lengths. A three element
         # tuple should be unpacked as three strings.
-        self.assertServerToClientEncoding('2\na\x01b\x0134\n', ('a', 'b', '34'),
+        self.assertServerToClientEncoding(
+            protocol.RESPONSE_VERSION_TWO + 'a\x01b\x0134\n', ('a', 'b', '34'),
             [('a', 'b', '34')])
 
     def test_client_call_with_body_bytes_uploads(self):
         # protocol.call_with_body_bytes should length-prefix the bytes onto the
         # wire.
-        expected_bytes = "2\nfoo\n7\nabcdefgdone\n"
+        expected_bytes = protocol.REQUEST_VERSION_TWO + "foo\n7\nabcdefgdone\n"
         input = StringIO("\n")
         output = StringIO()
         client_medium = medium.SmartSimplePipesClientMedium(input, output)
@@ -1667,7 +1678,7 @@
     def test_client_call_with_body_readv_array(self):
         # protocol.call_with_upload should encode the readv array and then
         # length-prefix the bytes onto the wire.
-        expected_bytes = "2\nfoo\n7\n1,2\n5,6done\n"
+        expected_bytes = protocol.REQUEST_VERSION_TWO+"foo\n7\n1,2\n5,6done\n"
         input = StringIO("\n")
         output = StringIO()
         client_medium = medium.SmartSimplePipesClientMedium(input, output)
@@ -1680,7 +1691,7 @@
         # read_body_bytes should decode the body bytes from the wire into
         # a response.
         expected_bytes = "1234567"
-        server_bytes = "2\nok\n7\n1234567done\n"
+        server_bytes = protocol.RESPONSE_VERSION_TWO + "ok\n7\n1234567done\n"
         input = StringIO(server_bytes)
         output = StringIO()
         client_medium = medium.SmartSimplePipesClientMedium(input, output)
@@ -1697,7 +1708,7 @@
         # LengthPrefixedBodyDecoder that is already well tested - we can skip
         # that.
         expected_bytes = "1234567"
-        server_bytes = "2\nok\n7\n1234567done\n"
+        server_bytes = protocol.RESPONSE_VERSION_TWO + "ok\n7\n1234567done\n"
         input = StringIO(server_bytes)
         output = StringIO()
         client_medium = medium.SmartSimplePipesClientMedium(input, output)
@@ -1714,7 +1725,7 @@
         # cancelling the expected body needs to finish the request, but not
         # read any more bytes.
         expected_bytes = "1234567"
-        server_bytes = "2\nok\n7\n1234567done\n"
+        server_bytes = protocol.RESPONSE_VERSION_TWO + "ok\n7\n1234567done\n"
         input = StringIO(server_bytes)
         output = StringIO()
         client_medium = medium.SmartSimplePipesClientMedium(input, output)
@@ -1723,7 +1734,8 @@
         smart_protocol.call('foo')
         smart_protocol.read_response_tuple(True)
         smart_protocol.cancel_read_body()
-        self.assertEqual(5, input.tell())
+        self.assertEqual(len(protocol.RESPONSE_VERSION_TWO + 'ok\n'),
+                         input.tell())
         self.assertRaises(
             errors.ReadingCompleted, smart_protocol.read_body_bytes)
 

=== modified file 'bzrlib/tests/test_wsgi.py'
--- a/bzrlib/tests/test_wsgi.py	2007-04-24 08:36:32 +0000
+++ b/bzrlib/tests/test_wsgi.py	2007-04-25 04:56:19 +0000
@@ -19,6 +19,7 @@
 from cStringIO import StringIO
 
 from bzrlib import tests
+from bzrlib.smart import protocol
 from bzrlib.transport.http import wsgi
 from bzrlib.transport import chroot, memory
 
@@ -204,8 +205,8 @@
         self.assertEqual('error\x01incomplete request\n', response)
 
     def test_protocol_version_detection_one(self):
-        # SmartWSGIApp detects requests that don't start with '2\n' as version
-        # one.
+        # SmartWSGIApp detects requests that don't start with
+        # REQUEST_VERSION_TWO as version one.
         transport = memory.MemoryTransport()
         wsgi_app = wsgi.SmartWSGIApp(transport)
         fake_input = StringIO('hello\n')
@@ -222,10 +223,11 @@
         self.assertEqual('ok\x012\n', response)
 
     def test_protocol_version_detection_two(self):
-        # SmartWSGIApp detects requests that start with '2\n' as version two.
+        # SmartWSGIApp detects requests that start with REQUEST_VERSION_TWO
+        # as version two.
         transport = memory.MemoryTransport()
         wsgi_app = wsgi.SmartWSGIApp(transport)
-        fake_input = StringIO('2\nhello\n')
+        fake_input = StringIO(protocol.REQUEST_VERSION_TWO + 'hello\n')
         environ = self.build_environ({
             'REQUEST_METHOD': 'POST',
             'CONTENT_LENGTH': len(fake_input.getvalue()),
@@ -236,7 +238,7 @@
         response = self.read_response(iterable)
         self.assertEqual('200 OK', self.status)
         # Expect a version 2-encoded response.
-        self.assertEqual('2\nok\x012\n', response)
+        self.assertEqual(protocol.RESPONSE_VERSION_TWO + 'ok\x012\n', response)
 
 
 class FakeRequest(object):

=== modified file 'bzrlib/transport/http/wsgi.py'
--- a/bzrlib/transport/http/wsgi.py	2007-04-24 08:36:32 +0000
+++ b/bzrlib/transport/http/wsgi.py	2007-04-25 04:56:19 +0000
@@ -134,9 +134,9 @@
     def make_request(self, transport, write_func, request_bytes):
         # XXX: This duplicates the logic in
         # SmartServerStreamMedium._build_protocol.
-        if request_bytes.startswith('2\n'):
+        if request_bytes.startswith(protocol.REQUEST_VERSION_TWO):
             protocol_class = protocol.SmartServerRequestProtocolTwo
-            request_bytes = request_bytes[2:]
+            request_bytes = request_bytes[len(protocol.REQUEST_VERSION_TWO):]
         else:
             protocol_class = protocol.SmartServerRequestProtocolOne
         server_protocol = protocol_class(transport, write_func)




More information about the bazaar-commits mailing list