Client-Server 6

I am questioning whether I need all this rigmarole, and whether I owe you some further testing of this “legacy” overly complicated Message Class. Just thinking today, code browsing, no changes.

We have learned, over the past few days, that the messages the tutorial sends and receives have three components:

• A two-byte field giving the length of the next item;
• A dictionary containing ‘byte_order’, ‘content_type’, ‘content_encoding’, and ‘content_length’;
• Another dictionary containing ‘content_type’, ‘content_encoding’, and ‘content_bytes’;

The content_bytes are the json-encoded query or answer to the query, depending on which side we’re on. The query is a dictionary that looks like this:

        return dict(
            type="text/json",
            encoding="utf-8",
            content=dict(action=action, value=value),
        )

And the answer looks like this:

{"result": answer}

So it’s dictionaries all the way down. But really, Alice, all we really care about when we get a question is the action and value, and we only care about the action because the example allows a binary query as well as a json text one. Similarly, when we get the answer, all we care about is that final string.

In the tutorial, the code shows how we could allow for different encodings. I think it would even support different encodings in the header and in the actual query and response. It shows how, with a header dictionary, we could support more than one content type, and how we can encode the variable length of the response.

It seems to me that this is too much for our purposes with the bot game. It seems to me that if we were to remove the weird binary query capability from the tutorial we could eliminate the protocol header dictionary altogether and just encode the length of the message in the header bytes.

What are some ways I might be wrong? There’s the byte order that we pass in the protocol header. However, I don’t see in the code where that’s actually being used. The tutorial goes on at some length about the topic, and also says that you may not need byteorder.

I think if we’re using JSON we are impervious to byte order. Let’s test that.

    def test_json_numbers(self):
        d = dict(value=123456789)
        json_d = json.dumps(d)
        assert json_d == '{"value": 123456789}'

Right. It’s a string. The parser on the other end can turn that back into a number any way it pleases.

Note

I was already sure of this, reasoning “JSON returns a string and I’ve read them and the numbers are just text, not binary”. I wrote the test to provide a different kind of “sure”. There’s a difference between reasoning and seeing.

What Next?

I can imagine proceeding in on of at least two directions now:

1. Continue to get this code under test, as a sort of exercise to show myself and the reader how we might get some gnarly legacy code under test, or

2. Take the knowledge we’ve gained and begin to build the client-server objects for the robots world game. We would probably start with some small experiments using a much simpler message format, custom-designed for our purposes, rather than this overly general one.

3.¹ Something else more interesting, like building smarter robots.

My Coefficients of Boredom² for those ideas are roughly this:

I think that in fairness to … honestly, I don’t know to whom … in fairness to someone, I should do a bit more work on bringing this overly complicated code under test. Maybe just today.

But I’m not very interested in client-server at all, so I can imagine that I might abandon this whole thread. It’s not like there are users just waiting until they can talk to my server. I do have some concern that unless I complete it, Bryan will look askance at me, but I have been looked at askance by the best and I can probably take it. Anyway, we can decide that later.

Let’s look for something that wants to be tested. There are bits of code like this:

    def read(self):
        self._read()

        if self._jsonheader_len is None:
            self.process_protoheader()

        if self._jsonheader_len is not None:
            if self.jsonheader is None:
                self.process_jsonheader()

        if self.jsonheader:
            if self.response is None:
                self.process_response()

    def process_protoheader(self):
        hdrlen = 2
        if len(self._recv_buffer) >= hdrlen:
            self._jsonheader_len = struct.unpack(
                ">H", self._recv_buffer[:hdrlen]
            )[0]
            self._recv_buffer = self._recv_buffer[hdrlen:]

    def process_jsonheader(self):
        hdrlen = self._jsonheader_len
        if len(self._recv_buffer) >= hdrlen:
            self.jsonheader = self._json_decode(
                self._recv_buffer[:hdrlen], "utf-8"
            )
            self._recv_buffer = self._recv_buffer[hdrlen:]
            for reqhdr in (
                "byteorder",
                "content-length",
                "content-type",
                "content-encoding",
            ):
                if reqhdr not in self.jsonheader:
                    raise ValueError(f"Missing required header '{reqhdr}'.")

    def process_response(self):
        content_len = self.jsonheader["content-length"]
        if not len(self._recv_buffer) >= content_len:
            return
        data = self._recv_buffer[:content_len]
        self._recv_buffer = self._recv_buffer[content_len:]
        if self.jsonheader["content-type"] == "text/json":
            encoding = self.jsonheader["content-encoding"]
            self.response = self._json_decode(data, encoding)
            print(f"Received response {self.response!r} from {self.addr}")
            self._process_response_json_content()
        else:
            # Binary or unknown content-type
            self.response = data
            print(
                f"Received {self.jsonheader['content-type']} "
                f"response from {self.addr}"
            )
            self._process_response_binary_content()
        # Close when response has been processed
        self.close()

This code might want to be tested. The idea of the read is that it’ll first add the new data from the socket to the receive buffer and then try to build up the three items we need, the protocol header, json header and actual response. Certainly things could go wrong in this process. Let’s list some, to see what we might test:

1. Do we correctly accumulate bytes into the receive buffer?
2. When we consume those bytes, do we correctly remove them?
3. When we read the two byte “protocol header”, do we get the value sent in?
4. When we read the protocol header do we actually set the _jsonheader_len variable, so that the next call will occur?
5. Will the code correctly drop through on a single read that has all the necessary data?
6. Does the code properly handle all the gaps that might occur due to incomplete reads?
7. Does process_response correctly decide between json and other?
8. Does whatever it calls deal properly with an unexpected format?

I think there are probably even more things that could go wrong with this code.

OK, you’ve convinced me. Tomorrow, if the crick don’t rise, we’ll begin to get this code under test.

I can see some issues that we’ll need to deal with. As things stand, the code expects a well-structured instance of Message. So far, our tests have not really needed the instance. We’d like to avoid using a test double, but it may turn out to be the best idea.

We’ll find out. For today, we’ve identified some code that needs testing, and we’ll explore actually doing that … next time!