I’ve reviewed the full app server example from RealPython. Rather than just run it here and work through the details of its operation, I want to get it under test. This promises to be … interesting.

Note
In this article, I’ll scan some of the files, a bit for understanding but mostly with an eye to getting the code under test. In the following article, I’ll create a testing foothold with some nearly-trivial tests.

The final example in the excellent RealPython tutorial includes four files, not the two files per example of the first two examples. The files are ‘app-client.py’, ‘libclient.py’, ‘app-server.py’, and ‘libclient.py’. The basic idea of the breakout is that the ‘app’ ones are the generic client or server code, with the select and the infinite loop that we’ve seen before, while the ‘lib’ ones each define a Message class that embodies the “domain” logic of the client-server relationship, The example is a simple query system of no particular consequence.

For the first two examples in the tutorial, I jumped right in, without much reading, and just started studying the code with you, here in preceding articles. With this larger RealPython example, I have already studied the code a bit and read most of the tutorial, in a sort of random order, depending on the part of the code I was looking at. And I’ve popped over to python.org a time or two to look at some of their documentation. So I’m a bit less ignorant today than I have been in the preceding articles. Still ignorant enough for our purposes, I’m sure.

Because the base-lever server and client are much the same, we’ll take a quick look at one key bit and then move on to the real problem, the Message class. Here’s how the client starts up:

if len(sys.argv) != 5:
    print(f"Usage: {sys.argv[0]} <host> <port> <action> <value>")
    sys.exit(1)

host, port = sys.argv[1], int(sys.argv[2])
action, value = sys.argv[3], sys.argv[4]
request = create_request(action, value)
start_connection(host, port, request)

And:

def start_connection(host, port, request):
    addr = (host, port)
    print(f"Starting connection to {addr}")
    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    sock.setblocking(False)
    sock.connect_ex(addr)
    events = selectors.EVENT_READ | selectors.EVENT_WRITE
    message = libclient.Message(sel, sock, addr, request)
    sel.register(sock, events, data=message)

So we see the big issue here is that we register our selector with an instance of libclient.Message, our new and soon to be explored class.

A second question is what’s up with action and value, and that’s here:

def create_request(action, value):
    if action == "search":
        return dict(
            type="text/json",
            encoding="utf-8",
            content=dict(action=action, value=value),
        )
    else:
        return dict(
            type="binary/custom-client-binary-type",
            encoding="binary",
            content=bytes(action + value, encoding="utf-8"),
        )

It turns out that the example includes two different styles of interaction, a JSON text format and a binary format. I’m not sure that I’d ever write a real client-server operation that allowed both but it makes sense in a tutorial to show how one might do it.

The server side is similar, also creating an instance of Message in the data member of the selector.

All this wiring allows the main loop to look like this:

try:
    while True:
        events = sel.select(timeout=1)
        for key, mask in events:
            message = key.data
            try:
                message.process_events(mask)
            except Exception:
                print(
                    f"Main: Error: Exception for {message.addr}:\n"
                    f"{traceback.format_exc()}"
                )
                message.close()
        # Check for a socket being monitored to continue.
        if not sel.get_map():
            break
except KeyboardInterrupt:
    print("Caught keyboard interrupt, exiting")
finally:
    sel.close()

The main thing here is that in our try, we simply fetch the Message instance from key.data and asking to process the events. We’ll see in a moment how it does that.

The sel.get_map is new to me. This is the client-side and we want to exit the program if we have no more selectors alive.

As I look at this I realize that I don’t quite know how we are supposed to run this thing. RealPython would have me running it in Terminal, and we’d pass in the query, as we saw in the startup, so we just do one query per execution. Fine for what they’re doing, I suppose, and maybe fine for some apps, though I generally think of a real application as a bit more fancy than running something in Terminal.

We’ll revisit this below, I think, but before we try to run this thing, I’d like to start looking at the Message. We’ll do the client one, I guess. I’ll spare us the entire file dump and try something a bit more focused. Here’s the class init :

class Message:
    def __init__(self, selector, sock, addr, request):
        self.selector = selector
        self.sock = sock
        self.addr = addr
        self.request = request
        self._recv_buffer = b""
        self._send_buffer = b""
        self._request_queued = False
        self._jsonheader_len = None
        self.jsonheader = None
        self.response = None

Everything else follows from these. The first thing I notice is ten member variables, which is a lot. the first four are provided from the ‘app-client.py’. We pass the selector (DefaultSelector), the socket, address pair (host,port), and the request, which is a little dictionary whose format we’ll doubtless discover.

Then we init two private buffers, some kind of private status flag, and _jsonheader_len. That’s going to be something about the length of the json header, I imagine. Recall that sockets don’t deliver messages in one chunk, they are just a stream of bytes, and we need to have some way of knowing when we have the whole message. This length will play in there, I’m sure. Then we have two non-private members, the json header and response.

I do not understand the breakout between public and private, and maybe they’re using _ more in the sense of temporary variables or something. None of these members looks to be public in any ordinary sense, as far as I can imagine now.

Looking at these member variables, I’m starting to question the design of this object. Mind you, it is intended as a tutorial example, and in such examples most authors keep more code in one chunk than I would do in production code. So I am not, by any means, criticizing what RealPython has provided as a tutorial. I’m assessing the style I would like to see in production code, or in code I need to understand.

I am prepared to bet that this object is not cohesive: that it embodies more than one idea, perhaps closely related but not just one single identifiable notion. And, when I’m at my best, I think that objects should really deal with one idea, not several.

Just looking at the member variables, we can begin to guess that this one class, Message, probably deals with:

  • Selectors
  • A socket
  • Reading and writing the socket
  • Managing buffer filling and emptying
  • The socket’s address
  • A request
  • Message header creation
  • Length determinations
  • Creating an actual response
  • Formatting the response as JSON

That’s a lot, and While we might not want ten or more objects, I think we might like to have more than one. Just an impression at this point. We’ll see what we see.

Now let’s look at the entry point to the object. The network code calls this method on each event:

    def process_events(self, mask):
        if mask & selectors.EVENT_READ:
            self.read()
        if mask & selectors.EVENT_WRITE:
            self.write()

Now this is the query side, so first it will write and then it will read to get the answer, I reckon.

    def write(self):
        if not self._request_queued:
            self.queue_request()

        self._write()

        if self._request_queued:
            if not self._send_buffer:
                # Set selector to listen for read events, we're done writing.
                self._set_selector_events_mask("r")

We see that there is this odd conditional logic going on here. It’s driven, at least in part, by the fact that it might take multiple calls to write to empty the output buffers, because sockets.

So when we first arrive, there is no request_queued (we init it to False), so presumably we create and queue one in gueue_request. Then we do _write, which presumably starts writing to the socket. If we don’t finish up the write, we’ll come back here in a while and do it again.

Then, if we have queued a request and the send buffer is empty, we change our selector from ‘rw’ to just ‘r’, because we are done sending and there is no sense wasting calls to write.

Next, I think I’d look at queue_request.

    def queue_request(self):
        content = self.request["content"]
        content_type = self.request["type"]
        content_encoding = self.request["encoding"]
        if content_type == "text/json":
            req = {
                "content_bytes": self._json_encode(content, content_encoding),
                "content_type": content_type,
                "content_encoding": content_encoding,
            }
        else:
            req = {
                "content_bytes": content,
                "content_type": content_type,
                "content_encoding": content_encoding,
            }
        message = self._create_message(**req)
        self._send_buffer += message
        self._request_queued = True

Recall that we set up the request over in the app server, depending on the action and value in the args. The difference seems to be that the non-JSON one has the content ready to go and the JSON one needs to be encoded. I could imagine a pair of small objects here, one for binary and one for JSON, each returning the right stuff. The binary one would be trivial and the JSON one would do the encoding. Let’s explore that next.

    def _json_encode(self, obj, encoding):
        return json.dumps(obj, ensure_ascii=False).encode(encoding)

OK, that was simple enough. I’m not sure why we chose that ensure-ascii value, but we probably don’t need to understand so much as treat it as boilerplate.

Anyway, having created a dictionary with our request in it, we do create message and add it to the send buffer.

    def _create_message(
        self, *, content_bytes, content_type, content_encoding
    ):
        jsonheader = {
            "byteorder": sys.byteorder,
            "content-type": content_type,
            "content-encoding": content_encoding,
            "content-length": len(content_bytes),
        }
        jsonheader_bytes = self._json_encode(jsonheader, "utf-8")
        message_hdr = struct.pack(">H", len(jsonheader_bytes))
        message = message_hdr + jsonheader_bytes + content_bytes
        return message

So here we see the fancy length handling. The header tells us some key stuff, including sys.byteorder, which I think we do not use in this code, but it’s the indicator of whether the processor is big-endian or little-endian, that is, whether numbers are stored starting with the byte of highest value or lowest. I would guess that our JSON encode and decode operations might be sensitive to byte order, but I don’t know. I think it’ll remain under the covers and we may not have to worry about it.

We’ve been given the content bytes … we’ll look at where those came from in a moment, and we create the header describing them, including the length. Then we get the length of the header … and pack it into a halfword with format ‘>H’ which means big-ending halfword, so that length 10 will be packed as a zero byte and a byte containing 10, in that order.

So our message will be a byte record like this:

<header_length, content header including content length, content>

So there we have all the info we need to know when the message is complete. We can use the header length to read the header, and the content length to read the content.

We’ll see more of that in the read. Let’s look there now.

    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()

Presumably _read adds one or more bytes to the receive buffer. We do another cute bit of conditional logic:

If we don’t have the header length yet, we try to get it. ‘protoheader’ is an interesting name. Term of art? I don’t know, but we do know it’s likely those two bytes like we put on our request. (We don’t really know that, because we have not looked at what the server does. But it does the same thing.)

And if we have the header length and no header we process that, and if we have that we process the response. process_response is large:

    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()

Here we check to see if there is enough data in the receive buffer to allow us to read all the content. If not, we just return. Presumably there will be, sooner or later.

So we read that many bytes, and trim those bytes off the buffer, in case there are more incoming. (I don’t think there will be, but in principle there might.)

Then if the answer is json we decode it and print. If it’s binary we just announce that we have it. In either case we put the response into our response member.

Then, finally, we call the appropriate _process_reponse method, which prints the result:

    def _process_response_json_content(self):
        content = self.response
        result = content.get("result")
        print(f"Got result: {result}")

    def _process_response_binary_content(self):
        content = self.response
        print(f"Got response: {content!r}")

Wow. Just reviewing this code has taken up a lot of time and a lot of words. Let me sum up, in the sense of reporting where I find myself.

Summary

This object, Message, deals with matters at quite a few levels, from dealing with events, through buffering, input parsing and output encoding, accepting binary or text/json input, and more. If this were my object, or I wanted to change it, I would want it under test. And quite a few tests at that, including but not limited to:

  • Tests of buffer sizes that are too small, too large or just right;
  • Test buffer sizes as above for the header length, the header, and the content;
  • Test that the sequences sent are correct based on the input;
  • Possibly test that the printed output is correct, but perhaps not;
  • Test the flow of those drop-through if statements for working as anticipated;

And surely there’s more. If we were to do a perfect job, there would be at least a pair of tests for each if, although often we can satisfy ourselves with fewer.

When faced with this code in an existing program that we had to enhance, almost certainly the right thing to do is to leave these files alone unless they actually require functional changes. No matter how good we are, there is always a chance of putting a new error into a file like these, and if there is no reason to change them, they’re better left alone.

If we did need to change them, we might face a rather tedious task of slowly bringing the code under test, refactoring as needed to make it so. We would almost certainly have to provide some test doubles that we could set up with scenarios we wanted to test. As we refactored, we would be working to create new methods and classes that could be called directly from tests without the need for doubles, fakes, or mocks.

And, of course, there is the option, which I’ve taken many times, which is simply to change the existing code as carefully as we can, testing manually as carefully as we can, then set it loose with a prayer. I have gotten away with this may times … and I’ve been bitten by it many times. The bites can be quite unpleasant, particularly if one were to take down the entire corporate product line with a flawed service.

Yet another option that might apply here, and that sometimes works out, would be to recreate the Message object from scratch, using TDD, and taking the existing code as a working example of the sort of thing we need to do. In this case, we only have one real entry point, process_events, and only a couple of hundred lines of code, so recreating the thing could be a decent way to go.

It might even be a good exercise prior to refactoring, as it could give us a good sense of what objects we might want to break out. Then we could go back and refactor the existing code to bring it more and more into line with what we need.

Quite possibly, the changes we needed might just touch one aspect of the thing. Perhaps a second json format, or a more complex query, which we might be able to process with a separate object, without really changing the existing one. We might just add an else clause to that dispatch between json and binary, and do the new case in new tested code.

Of these, I know down in my most rational mind that having the code under test is by far preferable, and that having some of it under test is better than having none of it under test. I know that editing it “carefully” is very risky, and that rewriting it quite likely will take a lot longer than a less all-at-once approach.

And in whatever mind actually operates me, I feel that I am smart enough to just modify the code carefully. I find it very difficult to get that first test running.

And I have found that getting that first test running is often not as difficult as I thought, and that once I have one, the next ones generally come pretty readily, resulting in more tests, better tests, and better code as well.

It’s that first step that’s difficult.

Next time, I’ll pick one side or the other, server or client, and try to write that first test.

Sudden surge of commitment

In fact, I’ll try it now, but only until I get frustrated. That will be in the next article, created right on the tail of this one. See you there!