Todo List

Class Utils::AbstractValue< AbstractType, NullObjectType, HierarchyRoot >
http://carcino.gen.nz/tech/cpp/multiple_inheritance_this.php The safest way would be to use dynamic_cast in operator* and operator-> even though static_cast is faster and will work in most of the cases. Should we switch to dynamic_cast?

Class Protocols::Generics::BadPacket
How to reduce the code duplication with BadPacket for each protocol implementing its own PacketBase specialization? Make this class a template? But then what if different specializations require further pure virtual methods to be implemented and specific values to be passed to the PacketBase ctor?

Class Utils::Encodings::BinaryReader
Drop the Data and make its fields members of BinaryReader.

Extract an interface class? Rename to BinaryReader base and make the dtor protected to avoid deleting using base-class pointer? Same for BinaryWriter.

Member Protocols::Gnutella::Packets::Testing::BinaryReaderTest::scenarioReadResultSet (const ResultData &resultData, const QByteArray &bytes, int number)
This test effectively tests only that ResultData extensions are read correctly. It might be nice to test also:
  • reading an empty file name (maybe a new scenario for that)
  • reading result sets without extensions (this scenario could be used)

Member Protocols::Gnutella::Packets::Testing::BinaryReaderTest::testReadQueryHitsDataNoQHD ()
how about no QHD? - then vendorCode == VendorCode(), same for

Class Utils::Encodings::BinaryWriterTest
Each of the tests actually contains more than one test. Each "artificial" block could become a test for itself. The current test functions are essentially fixtures - the setup is done once and then multiple tests are performed. Maybe create multiple fixtures and make more smaller individual tests.

Class Protocols::Gnutella::Packets::Testing::BinaryWriterTest
Using the BinaryReader to test the writer is smart and we should have such test but I'm not sure whether these should not be integration tests. Keep the BinaryWriter test like this for now but also keep this note so that we could come back and eventually refactor the tests once we've become wiser!

Member Protocols::Gnutella::Packets::Testing::BinaryWriterTest::scenarioWriteQueryHitsData (QueryHitsData &hitsData)
The test fails now because probably some trash gets read into hitsData.privateData. To be checked!

Class Protocols::BitTorrent::Bencoding::BItem
Add VALUE_OBJECT macro to derived classes?

Class Protocols::BitTorrent::Bencoding::BItem
Add comparison operators? We don't need them now, but maybe they could be handy in the future...

Class Protocols::BitTorrent::Packets::BitField
Use a bitfield class instead of QByteArray for the field bitField.

Class Http::BodyReader
What about MIME?

Read the body bytes and write them in q QIODevice, or better create a Body class and write data into it. That would be better to support multipart bodies. For example when reading multipart/ranges from a file for download. Alternatively, we might use a different reader for handling multipart messages.

Class Protocols::Http::BodyReader
implement IdentityBodyReader

implement ChunkBodyReader

implement DeflateBodyReader

implement BodyReaderChain

implement UnknownBodyReader

Class Protocols::Http::BodyWriter
implement IdentityBodyWriter

implement ChunkBodyWriter

implement DeflateBodyWriter

implement BodyWriterChain

Class Http::BodyWriter
Rename to BodyReader

Class Utils::Testing::CallableTest
test comparison

test assignement

test other specializations

Class Utils::CalitkoMocks::Testing::CallDriverTest
Should we test the Msg function variants?

Test willEmit()

Class Protocols::BitTorrent::Transfers::Choker
UploadSlotsCount should be configurable/dynamic. Maybe use an interface to something like a SlotsManager. It could decide based on the currently available traffic or static user configuration how many upload slots can be used. It might also take care of splitting the Calitko-wide number of slots to the individual protocols.

Member Protocols::BitTorrent::Transfers::Choker::doOptimisticUnchokingIfTimeHasCome ()
What if we have UploadSlotsCount changing dynamically at runtime and the new number of slots is less than before. Additionally, we have a number of snubbed connections which for each of which make an additional optimistic unchoke. Then we could end up with more optimistic unchokes than number of slots. The ones that are too much should not be optimisticly unchoked.

Member Protocols::BitTorrent::Transfers::Choker::reallocateUploadSlotsAndChokeUnchokeSessions ()
If the peer is a seeder it does not need to download anything from us. Should we always keep it choked, or we can treat it like any other peer? I makes sense that we always choke it otherwise we might assign it an uploading slot or select it to be an optimistic choke which makes no sense.

Member Protocols::BitTorrent::Transfers::Choker::sortSessionsForSelection ()
If we are uploading only, then sessions should be sorted by upload rather then download speed.

Class Protocols::Http::ClientHttpSessionFactory
It currently creates only CompositeSingleHostClientHttpSession objects. Should we rename the class to CompositeSingleHostClientHttpSessonFactory and extract an interface ClientHttpSessionFactory?

Member Protocols::Http::ClientHttpSessionFactory::~ClientHttpSessionFactory ()
Should we somehow notify the TransportFactory that we are going down, or is it safe to assume that TransportFactory is also going down? That may be required when we have requested Transport objects to be created and established.

Class Protocols::Http::ClientHttpSessionFactoryStatus
...Failed() to also pass a QString or an Error object identifying the error?

Class Http::ClientSession
Add support for polling! Queue requests and write them when possible. Required for better BitTorrent support.

Class Protocols::Http::Testing::CompositeSingleHostClientHttpSessionTest
test at least a single GET request

test at least two of the body encodings

Member Gnutella::Bootstrapping::ConnectionKeeper::doUhcQuery ()
What if all caches were queried and we have to wait before trying again?

Class Protocols::Generics::DataBase
isEqualTo() is needed by the AbstractValue wrapper. We could implement it in terms of toRawBytes() and thus not require every derived class to implement it.

Class Utils::CalitkoMocks::Testing::DelayedCallDriverTest
Test willEmit()

Class Gnutella::PacketProcessing::DynamicSearching::DynamicSearch
What about dynamic quering with leaf guidance? What about apllication filters? We should search until the searcher says they got enough results.

Class Utils::CalitkoMocks::ExpectationsList< Function >
To explicitly show that ExpectationsList owns the Function objects have add() take an auto_ptr. Doing so would require all EXPECTED_FUNCTION_FACTORYX macros to be updated.

Class Utils::CalitkoMocks::ExpectedFunctionBase
Add tests for numberOfTimes() - it is now only tested in "real" tests.

Class Utils::CalitkoMocks::Testing::ExpectedFunctionTest
What if we need to pass to the function a non-const reference, which should be modifed before the stubbed value is returned.

What is with const pointer to the host class and with const functions? It is now solved ugly by adding a new ctor to ExpectedFunction.

Class Gnutella::Packets::Extensions::Extension
Add support for the possible packet extensions: GGEP, HUGE, EQHD, what else?

An unknown extension will have data and dataSize fields.

Class Gnutella::Packets::Extensions::ExtensionBlock
This class was NEVER tested!!!

Class Protocols::Http::File
Rename to SeekableBuffer? A simple Buffer would do if we only request a single range. In that case we could assume that the user will have seeked the buffer correctly. However, if we want to support multiple ranges, then it will be better to be able to do the seeking. It could be possible to have two overloads - one taking a Buffer and one taking a SeekableBuffer. A third possibility would be to have the implementation try a cast to SeekableBuffer in case a partial entity is returned.

Class Protocols::Generics::GenericSession
Do test the example and make sure it compiles and works!

Class Protocols::Generics::GenericSession
Another approach (that would also be easier to test and thus probably better!) would be to have a GenericSession object as a member and just delegate the Session functions to GenericSession. Update the example and description to it eventually!

Member Protocols::Generics::GenericSession::GenericSession (Transport *, DataSerializer *, DataQueue *, SessionStatus *)
The underscore coding convention! Having underscore suffix in member variable names will make the implementation of the class ugly. Having the underscore suffix in the parameter names would make the docs ugly. If the "param" names are different than the actual parameter names Doxygen will print a warning. What to do!?

Member Protocols::Generics::GenericSession::abort ()
check whether Transport::abort() sends any status notifications. Make the behaviour of abort consistent for all layers (e.g. Session, ClientHttpSession, etc.)

Class Protocols::Generics::Testing::GenericSessionTest
test close() while inside the read notification

test abort() while inside the read notification

Member Protocols::Generics::Testing::GenericSessionTest::testOpenSessionStartsReading ()
Should we require Transport to always signal readyRead() after connection is established?

Class Gnutella::Packets::Extensions::Ggep
Add reference to the specs.

Class Gnutella::Packets::Extensions::Ggep
Test if encoding/decoding works.

Class Protocols::Gnutella::Packets::Extensions::GgepBlock
Use QSharedData and QSharedDataPointer to easily implement implicit sharing.

Member Protocols::Gnutella::Packets::Extensions::GgepBlock::extensions () const
This function is potentially dangerous! Think about wrapping the const Ggep * in an object (GgepRef??) and implement implicit sharing to be able to treat it like a value object and thus to save us the bad stuff that could result by passing pointers around.

Member Protocols::Gnutella::Packets::Extensions::GgepBlock::extensions () const
When the above is accomplished, add a function: template <typename concreteggep>=""> QList <GgepRef <ConcreteGgep> > extensions(); which will allow us to extract only the extensions of certain type.

Class Gnutella::Packets::Extensions::GgepBlock
Make copying an object chap by using reference counting. See Qt for reference.

Member Gnutella::Handshaking::Handshaker::compatibleNodeTypes (const NodeInfo &otherNodeInfo)
Promotion/demotion should maybe be implemented here:

Member Gnutella::Handshaking::Handshaker::hasRequiredFeatures (const NodeInfo &nodeInfo)
To be moved in a class SlotManager

Class Gnutella::Handshaking::HandshakeSession
Maybe rename just to Session if we can handle the problem with the duplicate filenames caused by qmake

Member Gnutella::Handshaking::HandshakeSession::completed ()
If Listen-IP is not null, but different than the one we connected to?

NodeInfo is copied a lot. Make it a class and use ref-counting.

Member Gnutella::Handshaking::HandshakeSession::setHandshakeHeaders (Header &handshakeHeaders, const NodeInfo &myNodeInfo)
Should we return only ultrapeers with leaf/ultrapeers slots?

Class Http::Header
Adapt the original Qt documentation to the current source:

Class Protocols::BitTorrent::Trackers::HttpRequestSession
This interface is not complete and covers only necessary functions that StandardTrackerRequestSession needs/supports.

Class Protocols::BitTorrent::Trackers::HttpRequestSession
If we want to pass some specific headers to get() we could use an overload like: get (const Uri &requestUri, const Header &extraHeaders); (see http://www.calitko.org/source-talk/793).

Class Protocols::BitTorrent::Trackers::HttpRequestSession
Move this interface to another place?

Class Protocols::Gnutella::Packets::Extensions::HugeGemBlock
use Uri for the HUGE URNs.

Class Protocols::Gnutella::Packets::Extensions::HugeGemBlock
Maybe rename xmlExtension() and urnExtension() to fromXmlExtension() and fromUrnExtension() - that's the naming style used in Qt. Better yet, create a HugeGemBlockBuilder class that supports method chaining.

Class Protocols::BitTorrent::Transfers::IntegerResetStopwatch
Could derive from a more generic Stopwatch.

Class Protocols::Generics::IpResolvingTransportFactory
Now we only try the first QHostAddress that we get back. Extend the class to try the following addresses if a Transport to the first could not be established.

The same as above is two hostnames get resolved to the same IP

Class Protocols::Generics::IpResolvingTransportFactory
What about timeouts? How long should lookup + connection establishment take in total? Where to check the timeouts?

Class Protocols::Generics::IpResolvingTransportFactory
Think about what happens if two Transports for the same hostname are requested at the same time/shortly one after the other. What happens if the same URI is requested in a loop? Only the last one will be checked at any given time.

Class MemoryFile
That is quite a mess now but after a bunch of refactorings we'll get things tidied up a little. We should figure how to do the acceptance testing... it would be similar to a unit test except that the "unit" will be the complete application.

Class Protocols::BitTorrent::Trackers::MultiTrackerRequestSession
Add implementation and docs.

Class Protocols::BitTorrent::Trackers::Testing::MultiTrackerRequestSessionTest
Won't there be a problem that the announce list is passed to the ctor? This will force us to test only one announce list...

Class Protocols::Transports::NodeAddress
Host name and address are exclusive now. It might make sence to allow both to be set at the same time for example when you have a connected connection one might want to query both the name and the address.

Class Protocols::Transports::NodeAddress
Maybe a solution to the above would be to encapsulate the lookup function in this class and allow multiple addresses for one name. For reverse lookups we could have alliases for a single IP.

Class Protocols::Transports::NodeAddress
Maybe make a generic class out of this one by adding a protocol field?

Member Protocols::Transports::NodeAddress::NodeAddress (const QString &hostNameAndPort)
Add support for parsing IPv6:port addreses (they use : and .)

Member Protocols::Transports::NodeAddress::NodeAddress (const QString &hostNameAndPort)
Check for ':' for the port, but have in mind it's ok

Member Gnutella::Bootstrapping::NodeCache::getNodes (int count, NodeAvailability availability, bool freshNodes, const Predicate &pred)
It still happens to have two connections to the SAME host! More absurdly, this host was we ourselves!!! How did it happen that the host cache returned two times the same host??

Member NodeModel::ModelRefreshInterval
move to the view.

Member NodeModel::Type
Do we really need an enum for the type field? What is when we add support to eDonkey for example? We should extend the Type enum. Why don't we simply make this field a QString and let each protocol provide the strings itself. This way the NodeModel wouldn't have to know the different protocols. The same thoughts would be valid for the Status enum.

Member NodeModel::refreshModel ()
Move to the view!

Member NodeModel::setNodeStatus (ConnectionId id, Status status)
Ensure that the connections are sorted by their lifetime, i.e. not have a connection established half a second after the next in the list (due to different timings in the handshaking).

Member NodeModel::updateTimer
Move to view.

Class NodeModel::NodeData
Add a field for the node's capabilities (which proposals are supported etc.).

Class Utils::ObjectDispatcher< DispatchBase, Sender >
Any idea how to reduce the code duplication in all Handler derived classes? That would possibly also make it possible to get rid of the four register-handler overloads per handler type.

Class Gnutella::Packets::Packet
Move the implementation completely in the .cpp. This will make the header nicer to read, but will make the use of the class more constly due to the out of line code expansion. When the implementation is stable, the functions may be declared as inline, and their implementation can be moved back to the header. This would genrally be made for the accessor functions, even if private data is used. It can be moved to the header as well. We would generally prefer using private data to separate the interface from its implementation. This way many headers need only be included in the .cpp but not in the .h, which would decrease the dependancies. Of course if the private data stores only pointers, then the headers need again only be included in the .cpp.

Class Gnutella::Packets::Packet
Replace rawHeader() and rawPayload() with rawPacket(). This would make it intuitive for the packet writer to write all data at once making it UdpConnection compatible. There is no need for both functions anyway. Moreover, change fromRawData to take just const QByteArray &rawPacket.

Member Gnutella::Packets::Packet::fromRawData (const QByteArray &rawHeader, const QByteArray &rawPayload)
That would become part of the PacketFactory for Gnutella.

Member Gnutella::Packets::Packet::name () const =0
remove this?

Member Gnutella::Packets::Packet::rawHeader () const
The above works like this only in the case when we forward a packet received from a peer. We do not change the payload and it is never rebuilt, thus all copies of the Packet share the same QByteArray for the rawPayload. In the case when we generate the packet, we first copy it in each PacketConnection and then call rawHeader() and rawPayload(). The raw data is built for each of the copies separately. Reference counting the Packet's Private data should solve the problem.

Member Gnutella::Packets::Packet::rawHeader () const
What happens if the payload changes (thus the payload size) and the header is rebuilt before the new payload change is recalculated? It's clear what happens, but check whether it really does!

Member Gnutella::Packets::Packet::rawPayload () const
Implement locking in case the private data is shared among some objects to ensure only one rebuilds the payload.

Member Gnutella::Packets::Packet::readHeader (QDataStream &stream)
The descriptor ID is said to be a 16 byte string, which implies big endianness if we consder it as a 4 + 2*2 + 8*1 bytes. Need to change byte order?

Member Gnutella::Packets::Packet::writeHeader (QDataStream &stream) const
The descriptor ID is said to be a 16 byte string, which implies big endianness if we consder it as a 4 + 2*2 + 8*1 bytes. Need to change byte order?

Class Protocols::Kad::Packets::Packet
Find the way to support both protocol types (compressed and plain).

Class Protocols::Generics::PacketBase
In the rare occasions when a packet would have a trailer, the trailer could be considered part of the payload. The PacketBase specialization of such protocol could even handle the trailer.

Class Protocols::Generics::PacketBase
Consider moving all packet-related stuff to the Packets sub-package.

Member Protocols::Generics::PacketBase::name () const =0
Better use PacketId (which possibly contains the ProtocolId)?

Member Protocols::Generics::PacketBase::protocol () const =0
Better use ProtocolId?

Class Protocols::Generics::PacketFactory
See how this one fits with PacketProtocol!

Class Gnutella::PacketProcessing::PacketProcessor
Maybe hide the packet routing policies inside the PacketProcessor? Do not implement it as a filter that is explicetly added. Instead, use the PacketRouter internally accrding to the basic Gnutella routing rules.

Member Gnutella::PacketProcessing::PacketProcessor::addConnection (Connection *connection, NodeInfo nodeInfo)
Can't we directly connect the Handshaker with the PacketProcessor? This would mean connect handshakeCompleted with addConnection. Similarly, connect PacketSession::connectionClosed to removeConnection (rename PacketProcessor::connectionClosed)

Class Gnutella::PacketProcessing::PacketRouter
Move member definitions to cpp.

There is lots of code duplication! Remove it, maybe using templates.

Member Gnutella::PacketProcessing::PacketRouter::~PacketRouter ()
Free memory! Make a helper function clearPaths (paths, expiredOnly)

Class Protocols::BitTorrent::Transfers::PacketSession
Calculate the max packet size. The transport buffer should be able to fit the biggest packet (either piece or bitfield).

Class Protocols::BitTorrent::Transfers::PacketSession
When a choke is received, all pending requests not sent to the OS TCP buffer should be dropped (suggested by official specs). What should out PacketBuffer be like? Should we be able to drop packets from the PacketBuffer. In reality we cannot guarantee that we send no more requests after a choke is received, simply because we could send right before we read the choke, which means that due to network latency, the remore party will receive requests even though it has sent a choke. To support pipelining, we the Transfers will remember requests for which pieces have been sent and new request will be sent only after a piece packet matching any of the requests is received.

Member Protocols::Generics::Testing::PacketTest::testParseRawDataCached ()
If we use a test driver we could also verify that the virtual functions writeHeader() and writePayload() are NOT called. Alternatively, we could set the fields, then unset the rewrite flags and verify that the cached bytes are returned.

Class Protocols::BitTorrent::Trackers::PeerInfo
Move this module out of the Trackers package? It will be used also in transfers so maybe we should move it somewhere else.

Member Protocols::Kad::Packets::PeerInfoPacket::readPayload (BinaryReader &)
How should I read a 128 bits number? I guess it should be inside Binary readers and writers... but that wouldn't be very clean.

Member Gnutella::Packets::Pong::ggepBlock () const
Decide whether to return a reference or a copy. Return a copy only if using reference counting.

Class Gnutella::PacketProcessing::PongCache
Move the member definitons to the cpp.

Use EMA for Daily Uptimes: An exponential moving average (EMA) is actually a specific type of weighted moving average. It uses a constant (a smoothing factor) between 0 and 1 in the following manner: the current closing price (C) multiplied by the smoothing constant (S) added to the product of the previous day's exponential moving average value (PEMA) and 1 minus the smoothing factor, or: Today's EMA = S*C + (1 - S)*PEMA

Member Gnutella::PacketProcessing::PongCache::~PongCache ()
Clear cache.

Member Gnutella::PacketProcessing::PongCache::cachePong (const Pong &, PacketSession *)
Split the function into parts.

Maybe implement the direct ping/pong as over the filter mechanism?

Member Gnutella::PacketProcessing::PongCache::sendCachedPongs (const Ping &, PacketSession *session)
Split in smaller functions!

Member Gnutella::PacketProcessing::PongCache::sendCachedPongs (const Ping &, PacketSession *session)
iterate over the cached pongs and directly move them

Class Gnutella::Packets::Push
Create accessors for the payload fields.

Class Gnutella::PacketProcessing::QueryRouting::QrtExchanger
Send our table over new peer and ultrapeer connections.

Schedule the table updates so that they possibly do not overlap in time.

Member Gnutella::PacketProcessing::QueryRouting::QrtExchanger::qrtRead (PacketSession *)
Wait a little before sending the table to the next peer to avoid traffic bursts. Maybe even only update tables once every few minutes and schedule non time-overlapping updates.

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::completeTableUpdate ()
Close the connection due to invlid routing table update.

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::completeTableUpdate ()
Close connection -> Inconsistent QRT update!

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::processPatch (const QueryRoutingPatch &)
Start a timer to wait for the update to complete?

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::processPatch (const QueryRoutingPatch &)
Verify that entryBits, tableSize and infinity have acceptible values!

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::processPatch (const QueryRoutingPatch &)
Do not let the table buffer become of exceeding size!

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::processReset (const QueryRoutingReset &)
Close connection if tableSize is excessive!

Member Gnutella::PacketProcessing::QueryRouting::QrtReader::processReset (const QueryRoutingReset &)
The query routing specs state that after a reset message is received, the host is enouraged to discard the current table and forward all queries until the update is completed. That is not what we do now - instead we continue to filter the queries using the old table. Normally, it would not take long for the update to be completed.

Class Protocols::Generics::Testing::QtNameResolverTest
How to test it? Move it in a separate OS functionality test app?

Class Gnutella::Packets::Query
Implement the query data extension.

Class Gnutella::Packets::QueryHits
Add functions to modify the speed flags.

Implement the query hits data extension.

Substitute Result::fileName with QFileInfo. It provides functions to get the extension, path, etc.

Add setter functions.

Forbid changing the result entries. Payload would need to be invalidated.

Class Gnutella::PacketProcessing::QueryRouting::QueryRoutingTable
Maybe extract the hashing stuff in a separate class?

Member Gnutella::PacketProcessing::QueryRouting::QueryRoutingTable::applyPatch (QByteArray &patchBuffer, quint8 entryBits)
Maybe could use a lookup table for the cases entryBits equal 4 and 8?

Member Gnutella::PacketProcessing::QueryRouting::QueryRoutingTable::makePatchTo (const QueryRoutingTable &newTable, quint8 entryBits) const
Maybe could use a lookup table for the cases entryBits equal 4 and 8?

Member Gnutella::PacketProcessing::QueryRouting::QueryRoutingTable::remove (QString x)
This may effectively remove another string y with same index.

Class Gnutella::Handshaking::RequestHeader
Use a uniform notation for member variables. Decide on whether to use PrivateObject derived classes to store private data.

Class Http::RequestHeader
Revise the this documentation is necessary:

Class Http::RequestHeader
Properly implement implicit sharing of private data (passing the pointer to the private data to the ctor of the base class).

Class Gnutella::Handshaking::ResponseHeader
Use a uniform notation for member variables. Decide on whether to use PrivateObject derived classes to store private data.

Member Gnutella::Handshaking::ResponseHeader::ResponseHeader ()
This constructor we need bacause of HandshakeConnection storing the the headers as members and not just pointers which will be initialized when required. Should we remove the default constructor and switch to pointers? The same considerations are valid also for HandshakeRequestHeader. The only difference is that we have a default constructor due to the default parameters.

Class Http::ResponseHeader
Revise this documentation:

Class Http::ResponseHeader
Properly implement implicit sharing of private data (passing the pointer to the private data to the ctor of the base class).

Member UIs::Searching::SearchingWidget::closeTabButtonPressed ()
do we need to close "Search Results" tab, when no tabs remain?

Member UIs::Searching::SearchResult::SearchResult (const QString &fn, const QString &ext, const quint32 &sz, quint32 sp, QString ht, QString clt)
Why these funny default values?

Class Protocols::Generics::Session
Again as in DataSerializer, should we make the binding with Transport explicit? Maybe changing open() to open (Transport *). The session would then attach itself to Transport's TransportStatus?

Class Protocols::Generics::Session
Switch to passing Transport * instead of Transport & (as a convention for differenciating between Value Objects and Reference Objects). That would require updating a few classes.

Class Protocols::Generics::Session
Should each Interface interface has a method setStatus (InterfaceStatus *), which would allow us to change the listener at runtime? For example, a HandshakingSession could register as a TransportStatus first and once handshaking is finished, a PacketSession would register itself using setStatus (this).

Class Protocols::Generics::Session
Currently we only have unicast status notifications. What about multicast notifications? We could use an adapter for such scenarios. Such an adaptor will have to be manually implemented. It seems like clearer design to prefer unicast notifications. See for example GenericSession. It listens to TransportStatus and reacts on its signals. GenericSession is a kind of controller that then delegates functionality to other helper objects (e.g. DataSerializer). I have a feeling that using multicast notifications and having both GenericSession and DataSerializer reacting on the notifications would make the design more complex (e.g. DataSerializer would probably need a status interface too and it is not clear whi is "in control").

Class Protocols::BitTorrent::Transfers::Session
Use the state design patter the avoid the switch-es.

Member Protocols::BitTorrent::Transfers::Session::readHandshake ()
if handshake is incorrect, go to CorrupredSession state and disconnect the transports?

Class Protocols::BitTorrent::Transfers::SessionManager
start() might need at least peerId in order to have a SessionAcceptor forward it all session negotiated for this peerId.

Class Protocols::Http::SingleHostClientHttpSession
Allow queuing multiple requests - now we can only send one request.

Class Protocols::Http::SingleHostClientHttpSession
We can also have a ProxiedClientHttpSession. The only difference will be that we shall not write the "Host:" header and that we shall request the complete URI together with the host part. Maybe we should provide a switch in the ctor that determines which kind of request to send?

Class Protocols::Http::SingleHostClientHttpSession
We can also implement a MultiHostClientHttpSession class. It could be implemented in terms of SingleHostClientHttpSession. It can be configured to use multiple SingleHostClientHttpSession objects per host and also talk to numerous hosts at the same time. The HttpDownloadManager could then use the MultiHostClientHttpSession for the HTTP tranfers and the manager would only need to take care of storing the download list, starting/stoping transfers, associating transfers with local files.

Member Protocols::Http::SingleHostClientHttpSession::open ()
test it, check state invariants

Class Protocols::Http::Testing::SingleHostClientHttpSessionTest
Together with a RangeBodyReader/MultibyteRangexBodyReader we should make sure their Data objects are handled accordingly

Class Gnutella::Handshaking::SlotAllocator
Move out of Handshaking?

Class Protocols::Generics::SocketBuffer
We have redundancy with interface Transport? Maybe we should extract an interface Buffer which both SocketBuffer and Transport would need to extend?

Member Protocols::Generics::SocketTransport::connectToNode (const Uri &)
Should we require that the scheme of nodeAddress determines what kind of socket to use (UDP or TCP)? In this case we could assert that socket is of the correct type.

Member Protocols::Generics::Testing::SocketTransportTest::testAbortConnectionAttempt ()
check the todo in SocketTransport::abort()

Class Protocols::Generics::TcpSocket
do not send socketRead() or socketWritten() from within read() or write(). These should be sent only when the buffers have been empty/full and that has changed.

Class Protocols::Generics::TcpSocketBuffer
What about peek() and variants?

Class Protocols::Generics::TcpSocketBuffer
Add '_' postfix to all private data variables, like readBuffer_.

Member Protocols::Generics::TcpSocketBuffer::setReadBufferSize (int size)
use a QByteArray or an auto_ptr instead of raw pointers?

only called by the ctor - use ctor initializer lists instead?

Member Protocols::Generics::TcpSocketBuffer::setWriteBufferSize (int size)
use a QByteArray or an auto_ptr instead of raw pointers?

only called by the ctor - use ctor initializer lists instead?

Class Protocols::Generics::TcpTransport
The raw QTcpSocket object probably needs to be passed from the outside - for the cases when we accept incoming connections.

Class Protocols::Generics::TcpTransportFactory
Different protocols based on TCP will need different buffer sizes. For example, HTTP migth be fine with just 4K, however, Gnutella would need at least 64K. There should be a way to pass in the requested buffer size! Maybe we could use the query string of the TCP address URI, for example, tcp://host:port?r=4096&w=4096

Class Protocols::Generics::TcpTransportFactory
The implementation of this class so far does not care exactly what kind of Transport we are creating (well, TcpTransport is hardcoded now), so we could implement a registration mechanism like: registerTransportFactory (scheme, Callable <auto_ptr <Transport> (uri)>) Name resolution can be done in advance and name resolvers can be registered too.

Class Protocols::Generics::Testing::TcpTransportFactoryTest
test multiple createTransport in parallel

try to delay acceptConnection() and see what happens

Class Protocols::Generics::Testing::TcpTransportFactoryTest
Should we move this test to another tester-app dedicated on OS-integration and acceptance tests?

Class Utils::CalitkoMocks::TesterApplication
Move to its own module?

Class Protocols::BitTorrent::Trackers::Timer
What is the Utils::Timer class? It seems that one timer is already there, but I haven't studied it...

Class Protocols::BitTorrent::Trackers::Timer
Replace this interface for the "real" one from the Peter's dev branch. The first parameter of the start() function is only a temporary "solution".

Class Protocols::BitTorrent::Torrents::Torrent
Use something like Sha1HashDigest instead of FixedSizeByteArray <20> as Piece data type.

Class Protocols::BitTorrent::Torrents::Torrent
Remove announce() and keep only announceList()? Single torrent trackers will have announceList() return a single list with one element.

Class Protocols::BitTorrent::Torrents::Torrent
Do BitTorrent client really query only one of the trackers in the announceList()? I think BitComet at least queries all the trackers.

Member Protocols::BitTorrent::Torrents::Torrent::InfoHash
Use something like Sha1HashDigest instead.

Class Protocols::BitTorrent::Torrents::Torrent::FileInfo
Use something like Md5HashDigest for storing file checksum.

Class Protocols::BitTorrent::Torrents::TorrentParser
Implement getting and computing the hashed value of the info key from the torrent metainfo file. This will definitely require some addings to some of the classes from the Bencoding package and also some class that provides hashing.

Class Protocols::BitTorrent::Transfers::TrackerManager
Pass the complete torrent? Also at least info_hash is required!

Class Protocols::BitTorrent::Transfers::TrackerManager
What is with the uploaded/downloaded info? We could use a PacketProcessor to count the total number of bytes and we could give TrackerManager access to it? Maybe we could define an interface TransferState which could be used to query current upload/download rate and total bytes.

Class Protocols::BitTorrent::Transfers::TrackerManager
start() should take a QList <Uri> or an AnnounceList.

Class Protocols::BitTorrent::Trackers::TrackerManagerImpl
Implement real tracker request creation (from the selected torrent object). See createTrackerRequest().

Class Protocols::BitTorrent::Trackers::TrackerManagerImpl
Implement session re-establishment after it gets closed? Currently the tracker manager gets stopped after the session gets closed, so maybe we should try to "reconnect". This would require adding some more member functions to the TrackerManagerStatus like sessionEstablished(), sessionClosed() etc. to keep the user informed.

Class Protocols::BitTorrent::Trackers::TrackerManagerImpl
What is the meaning of the 'min interval' parameter in the TrackerResponse? I don't see the difference between the 'interval' and the 'min interval' parameter. Maybe it means that after a session gets closed the tracker manager must wait for this ammount of time before sending the first request?

Member Protocols::BitTorrent::Trackers::TrackerManagerImpl::TrackerManagerImpl (TrackerRequestSessionFactory *, Timer *, TrackerManagerStatus *)
How to assert the precondition since there is no Timer::isStarted() member function?

Member Protocols::BitTorrent::Trackers::TrackerManagerImpl::createTrackerRequest () const
Implement real tracker request creation (from the selected torrent object). Now it's returned only a hardcoded value.

Class Protocols::BitTorrent::Trackers::TrackerRequest
Remove the first parameter (announceUrl) from the ctor? We added it since we needed to pass it anyway with the request, but the situation has changed because with the new TrackerRequestSession interface (and considering the TrackerRequestSessionFactory/TrackerManager classes)
  • it won't be necessary to have it there. And because the TrackerManager creates its own requests, but it doesn't know to which tracker the session is connected, it may be even impossible to have this parameter there!

Class Protocols::BitTorrent::Trackers::TrackerRequestSessionFactoryImpl
What with the holdedSessionsCount() member function? It's not needed by users of this class, but it's needed in the tests (without it I can't test the destroySession() function behavior properly).

Class Protocols::BitTorrent::Trackers::TrackerRequestSessionFactoryImpl
Implement real HttpRequestSession and TrackerRequestSessionStatus instance creation in the createSession() member function. Now it's impossible to call any member function of the created session!

Class Protocols::BitTorrent::Trackers::TrackerRequestSessionFactoryImpl
After the DhtTrackerRequestSession is implemented we'll need to modify the TrackerRequestSessionData type (among others) because the session will definitely not need a HttpRequestSession instance in the ctor.

Class Protocols::BitTorrent::Transfers::Transfer
Use TransferSession and add it a few more state members (e.g. weAreChoked, peerIsChoked, weAreInterested, peerIsInterested), or keep such state on per PacketProcessor basis only in which case a Session * will be enough?

Class Protocols::BitTorrent::Transfers::Transfer
start() should probably also get a FileSystem or FileProvider object which will be used to read and write file pieces from and to.

Class Protocols::BitTorrent::Transfers::Transfer
Implement TransferSessionStatus rather then SessionStatus - we'll need directly TransferSession * in the notifications.

Class Protocols::BitTorrent::Transfers::TransferManagerState
Only that? Needed to query whether we are downloading or only seeding.

Class Protocols::BitTorrent::Transfers::TransferSession
Add something like overallAverageUploadRate() and movingAverageUploadRate() that will say what the average for the last x seconds/minutes is.

Class Protocols::BitTorrent::Transfers::TransferSession
Store the BitField piece availability map here or in one of the PacketProcessors only?

Class Protocols::BitTorrent::Transfers::TransferSession
Rename sendPacket() to send() ??

Class Protocols::BitTorrent::Transfers::TransferSessionImpl
Rename to StandardTransferSession??

Member Protocols::BitTorrent::Transfers::TransferSessionImpl::TransferSessionImpl (Session *, const Handshake &, IntegerResetStopwatch *downloadStopwatch, IntegerResetStopwatch *uploadStopwatch, TransferSessionStatus *)
Register handlers fro the Fast Extension packets only if Fast Extension was negotiated. Done for HaveAll and HaveNone - register and implement also SuggestPiece and CancelReqeust.

Member Protocols::BitTorrent::Transfers::TransferSessionImpl::updateDownloadSpeed (int pieceLength) const
When do we restart the stopwatch? Be carefull with the integers!

Member Protocols::BitTorrent::Transfers::TransferSessionImpl::updateUploadSpeed (int pieceLength) const
When do we restart the stopwatch? Be carefull with the integers!

Class Protocols::BitTorrent::Transfers::Testing::TransferSessionImplTest
Test with hasFastExtension disabled.

u/d speeds, snubbed flags

Class Protocols::BitTorrent::Transfers::TransferSessionStatus
Write docs!

Class Protocols::Generics::Transport
Most network operations have some time constraints and associated timeout intervals (connect/disconnect/read/write). To simplify implementation of application protocols, we could probably have a decorator or a proxy take care of these timeouts.

Class Protocols::Generics::Transport
Sometimes it is necessary to limit the upload and download bandwidth of a connections. That could be realized using a ThrottledTransport proxying to the real Transport. The proxy would limit the amount of data that can be read or written per second and if the channel is saturated it will emit the signals readyRead() and bytesWritten() once a second to keep up the transfer rate.

Class Protocols::Generics::TransportFactoryStatus
...Failed() to also pass a QString or an Error object identifying the error?

Class Protocols::Generics::TransportStatus
Apply the policy that each status interface provides a pointer to the interface it is notifying about (Transport *) as a first argument?

Class Protocols::Generics::TransportStatus
Document the individual methods in the cpp file.

Class Protocols::Generics::Testing::TransportStub
It may be interesting to record also calls to connectToNode(), disconnectFromNode() and abort() so that we could check whether they were indeed called afterwards. Another option would be to come up with a combination of a mock and a stub.

Class Protocols::Generics::Testing::TransportStub
Write tests for the stub?

Class Protocols::Transports::UdpConnection
The description in the note below is not consistent with the current implementation of dataRead() ! If we change the implementation according to the specification we will also need to change Gnutella::PacketProcessor::PacketReader! Maybe it is a good idea to add a property isStream() or isDatagram() and have each reader/writer use a suitable strategy for the current Connection. If we do like this, then a maxsize packet will always be read from a datagram connection and then the header be parsed, whereas for stream connection the header will be read first, parsed and then the payload read. Making this design decision is not important at this time, so let's keep the the currect system as is and consider these concerns at a later point when necessary for a new feature.

Class Protocols::Transports::UdpConnection
Allow to set buffer size? What to do if buffers get full?

Class Protocols::Transports::UdpConnection
Create generic PacketReader and PacketWriter which could be used as base classes by protocols like Gnutella?

Class Gnutella::Bootstrapping::UdpHostCache
Check how much time we should wait before querying the same cache again.

Class Gnutella::Packets::Extensions::Ggeps::Ultrapeer
make it possible to change data

Class Gnutella::Packets::Extensions::UnknownExtension
Move to a separate file.

Class Utils::Uri
Host must be in UTF-8 encodig for consistency?? Refer to RFC 3986:

Member Utils::Uri::queryItemValue (const QByteArray &key) const
Return concatenation of all associated values instead? I haven't found any note about it in the RFC 3986.

Member Utils::Testing::UriTest::testGettingSubsetsOfEncodedRepresentation ()
If scheme is missing, then there is no ":" after it. Similarly with userInfo, port, query, fragment...

Class Gnutella::Packets::VendorCode
maybe move it to namespace Gnutella::Utils

Class Gnutella::Packets::Extensions::Ggeps::VendorCode
reference counting

VendorCode_

Member Gnutella::Packets::Extensions::Ggeps::VendorCode::writeData (QDataStream &stream) const
Verify that the code is 4 chars. Maybe implement a class VendorCode which supports mappings between code and user-firendly string.

Class Protocols::Gnutella::Packets::Extensions::VendorCode
maybe move it to namespace Gnutella::Utils

Member Protocols::Gnutella::Packets::Extensions::VendorCode::toString () const
Make a map with all known codes - vendor names and return a string from there.

Class Utils::Version
In Linux it is typical to use a string for build (e.g. 4.0.1.ubuntu1.2.3)

Member Gnutella::Packets::PacketConstants
Make this enum a member of Packet

Maybe move all such global constants in a Settings class and make it possible to be changed without rebuilding the complete source.

Member Gnutella::Packets::PayloadDescriptor
Add reference to the Gnutella specs.

Member Protocols::Generics::Packet
There was the idea to define Packet for each protocol. The problem was that that would require each protocol to also define a BadPacket or NullPacket class. If the one from Generics (i.e. here) is used, then there might be a problem at runtime - casting BadPacket which is derived from Generics::PacketBase to DataBase (that's fine) and then casing is to e.g. BitTorrent::Packets::PacketBase, which is unrelated to BadPacket! How to solve that?

File Mocks.h
Write a script that generates the macros EXPECTED_FUNCTION_CHECKERX and the like to make modifications and adding new macros easier and less error-prone.

File Mocks.h
A test driver should implement all virtual functions and create expectations similarly like for virtual functions of mocked classes. The difference would be that the return values will not be stubbed and after the parameter values have been checked, the return value of the base implementation should be returned. If the function is pure virtual then the call is completely mocked, including the stubbed return value.

Member compressBound
Bind the latest version of zlib, Qt is not exporting compressBound().

Member Utils::Encodings::Deflate::unCompress (const QByteArray &compressedData)
Since we cannot know the length of the uncompressed buffer, it would be better to not call uncompress() but rather call the sequence deflateInit(), a number of times daflate() and deflateEnd().

Member Utils::Encodings::Deflate::unCompress (const QByteArray &compressedData)
What happens when the compressedData is corrupted?

Namespace Gnutella
We must add support for Gnutella file transfers. We'll most likely need a new sub-package called Transfers.

No Calitko abstractions (classes) for a search and search result developed yet. Maybe the best would be to use XML! We could decide on that after work on the Services::Searching is started.

Namespace Gnutella::Handshaking
Verify that handshaking works correctly in all modes combinations between Calitkos and between Calitko and other Gnutella servents.

Try using Doxygen's grouping functionality to group RequestHeader and ResponseHeader into "Handshaking Presentation Layer" group for example. Then add a reference to this group in the picture above.

Does SlotAllocator fit nice here? Maybe it, together with Bootstrapping::ConnectionKeeper should either be moved in a separate package directly in Gnutella.

Namespace Gnutella::PacketProcessing
Move QrtExchanger, QrtReader, QrtWriter, QueryRoutingTable to a new sub-package QueryRouting

Create a new sub-package DynamicSearching and move the classes Gnutella::Searching::DynamicSearcher and Gnutella::Searching::DynamicSearch to the new sub-package.

Implement flow control in PacketSession.

Namespace Gnutella::PacketProcessing::DynamicSearching
v0.6.0 Implement leaf guidance to dynamic querying.

Namespace Gnutella::PacketProcessing::QueryRouting
v0.6.0 QrtExchanger must be modified to store per-connection QRP state. That means to add a QMap <PacketSession, SessionInfo> member. SessionInfo would be a struct containing a QrtReader, a QrtWriter and probably some status or last changed marker (if necessary). All QRP stuff from Gnutella::PacketProcessor::PacketSession must be removed.

Namespace Gnutella::PacketProcessing::QueryRouting
v0.6.0 QrtExchanger must be added a new function that can be used by PacketProcessor to check whether a query packet should be forwarded over a specific PacketSession. PacketProcessor would call the function only for ttl=1 queries. Integrate the new function in PacketProcessor.

Namespace Gnutella::PacketProcessing::QueryRouting
v0.6.0 It must be tested whether QrtExchanger correctly merges the QRTs of its leaves. It should further be verified that QRT updates are issued in a good rate (check with specs) and that query packets a correctly filtered. It would be nice to develop a package test app, which can facilitate the process.

Namespace Gnutella::Packets
Add support for Open vendor and standard vendor messages.

The searching-related extensions are not fully implemented and tested yet (XML metadata, EQHD).

Namespace Gnutella::Packets
Not all GGEP exptensions are implemented yet. Implement them as need arises.

Namespace Gnutella::Searching
LocalSearch is not used anywhere and its code is a bit old now. It needs to be refactored. FileIndexer and FileInfo belong to it.

DynamicSearch and DynamicSearcher should be moved to PacketProcessing::DynamicSearching.

Find and add the links to the specs about metadata searching in Gnutella.

Namespace Http
v0.6.0 Implement ClientHttpSession which can be used to request a file and write the result in a QIODevice.

Namespace Http
v0.6.0 Implement ServerHttpSession which can be used to read a HTTP request and transmit the file content back.