On 02/09/2016 09:59 AM, Amos Jeffries wrote: > "Download" activity is a ::Client class type of activity, which is about > managing the Squid end of a Squid->server connection.
I am afraid you got it backwards: Downloader is a request source (and a response sink). Downloader does not actually care about the Squid-to-server connection! It is up to Squid to manage Downloader request and deliver the response back to Downloader (with or without the connection to the origin server or peer). > I am aware there is a need for the store logics to be involved. But > surely StoreEntry and/or clientStreams does not require the whole > ::Server 'side' of Squid to be involved. Besides StoreEntry and/or clientStreams, download requests should go through the regular doCallout() processing sequence including StoreID, ICAP, and other complex things. Thus, I am sure Downloader needs to be in the same group of classes as ConnStateData. > Please no more ConnStateData derived classes. ::Server is the head of > that hierarchy and the servers/ classes as the per-protocol children. As for the "Server" parent specifically, I cannot yet say whether your request is valid. As I said earlier in another thread, I need more time to study what that unaudited class is and what it should be. Based on the current "used to manage connections from clients" description alone, it is obviously the wrong parent for Downloader, but I do not claim that the description itself is correct. FWIW, the introduction of that Server class is the primary reason we have not posted the long-polished "Fetch missing certificates" patch for review... > Looking at the patch I also see you have added a thing called > BinaryTokenizer, but the Tokenizer we already have operates on binary > octets. All that seems needed is some new accessor(s) for fetching the > fixed-width binary fields alongside the current int64() ASCII->integer > decoder. * Tokenizer API is about sequences of characters -- it is built around CharacterSet. When it retrieves a number, it may retrieve "012345" characters. * BinaryTokenizer is about sequences of bits. When it retrieves a number, it may retrieve 64 bits. While it all boils down to "binary octets" at some level, it would be a design mistake to mix the two concepts in one class IMO; just like it would be a mistake to add "some accessors" to HTTP parser to parse FTP messages, even though both HTTP and FTP deal with the same kind of input token streams. It is possible to create a common Tokenizer parent for TextTokenizer and BinaryTokenizer, but I think we should wait for BinaryTokenizer to mature first. It is possible that it (or another *Tokenizer class using it) will start dealing with not-byte-aligned bits. If that happens, building a common parent for all tokenizers would be awkward and expensive performance-wise. Alex. _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
