Hi Petr,

1. Unifying URL storage.
I’ve noticed that now we’re using a QString for an announce URL storage in the Torrent[Parser], Http uses QUrl, TrackerRequestWriter uses QByteArray. Maybe it would be good to merge your Uri class (if it’s implemented everything for basic usage - I think it is) to the main branch so I can unify at least the Torrent[Parser] and the TrackerRequestWriter. Or should now just leave these classes and use QUrl in the TrackerRequestSession for now and we will unify it later?

Unifying these seems like a good idea! Uri should be fine for basic usage - merge my developers/peter/calitko-petr branch into to get the code. I see that Http::ClientSession uses QUrl. I don’t think ClientSession has been tested much - it should better be ported to Protocols/Http. The good thing with Mock Objects is that you can test a class that is supposed to use ClientSession even if ClientSession does not exits. More to that in a while.

2. [snip]
My idea is to implement the communication with a tracker by using ClientSession class in the Http package (is this class done/working/useful?). You wrote that it would be optimal to build it upon an HttpRequestSession, but I don’t know whether you ment to use a new class for that or the existing ClientSession class. This class needs a Connection-derived class instance in its constructor, so I was thinking about the TcpConnection class for that.

In your TrackerRequestSession.h you could declare an interface (abstract class) for HttpRequestSession. You would declare it with CALITKO_MOCKABLE. Then you could discover what this interface would look like as you write tests and implementation for TrackerRequestSession. For example, sendRequest() could create a RequestHeader object and would pass it to the HttpRequestSession:

call (session->sendRequest (announceUrl, trackerRequest))
.willCall (httpRequestSession->get (httpRequest))
.returns()
.returns();

so the task of sendRequest() would be generally to convert the BitTorrent request to an HTTP request that is sent to the tracker using the GET method.

Now assuming that the ResponseHeader comes via the status interface which TrackerRequestSession implements, the test would look like this:

call (session->handleResponseHeader (responseHeader))
.willCall (status->receivedResponse (trackerResponse)
.returns()
.returns()

so just converting the HTTP response to a BitTorrent response object.

This process of establishing a session actually has a number of steps: connect the transport, possibly negotiate a session (some kind of handshaking), then attach the transport to the “user” session. Furthermore, as in this example, sessions could be built on top each other. For our purposes in TrackerRequestSession I think we could assume that the HttpRequestSession has already been established and a reference to it is passed to the ctor.

The BitTorrent Transfer class would manage the list of trackers and I imagine it would do something like this:

httpProtocol->establishHttpRequestSession (announceUrl, this);

if the session could be established, the function httpRequestSessionEstablished() of Transfer is called with arguments announceUrl and a pointer (auto_ptr maybe) to the session object. If the session could not be established, the function httpRequestSessionNotEstablished() would be called, passing only the announceUrl as a parameter.

So the tracker communication scenario would be like this: First I’ll connect to the selected tracker with the help of a NameLookup class to get an address of that tracker (e.g. creat a Connection class instance). Then I’ll create a ClientSession instance using that connection. Then I’ll send a request (using a RequestHeader and ClientSession::get()) and wait for a response which will be parsed using a TrackerResponseParser class.

For TrackerRequestSesson just assume an HttpRequestSession already exists. I believe this way the class will be more cohesive. The stuff with NameLookup should be done separately, though I’m not quite clear about how exactly.

Is it possible to have static functions in the classes with CALITKO_TESTABLE marker? I haven’t studied it deeply (so it might be only my mistake) but when I had a static function in the TrackerRequestSession I got the following error during compilation:

Currently no - I never thought of that :-). I’ve created a ticket for this and I think it could be fixed.

Second problem is described in the TrackerRequestSession() dtor. I don’t want to have a public function disconnect() because connecting is done in the sendRequest() function, so maybe I just shouldn’t call the status object methods in the closeSessionWithTracker() and disconnectFromTracker() functions.

I’ve had the same problem with dtors… I asked a friend of mine who’s using EasyMocks in Java how they’ve solved the problem there and he told me “There’re no destructors in Java.” and I thought “Wow!” :-)

I think that it should be safe to call the dtor from any situation - especially as part of stack unwinding when an exception is thrown. Thus, it might be good not to call any extra functions. So instead of the dtor of TrackerRequestSession calling abort() on HttpRequestSession, the dtor of the latter would call its private abort() equivalent (no status notifications though!) to clean up stuff before the object is actually deleted.

I hope you find my replies helpful!

Regards,

Peter