Naming clashes post src/clients, src/servers restructuring

On 08/07/2014 Alex Rousskov wrote on [PATCH] Native FTP Relay thread:

> Changes to the general code used by the Native FTP Relay code:
>
>> > * The user- and origin-facing code restructured as agreed previously on
>> > this mailing list. I will email some thoughts about the results separately,
>> > but here is the executive summary:
>> >
>> > src/servers/FtpServer.* # new FTP server, relaying FTP
>> > src/servers/HttpServer.* # old ConnStateData parts conflicting with FtpServer
>> > src/clients/FtpClient.* # code shared by old and new FTP clients
>> > src/clients/FtpGateway.* # old FTP client, translating back to HTTP
>> > src/clients/FtpRelay.* # new FTP client, relaying FTP
>> > src/ftp/* # FTP stuff shared by clients and servers

Hello,

    After native FTP Relay changes were merged we got even more naming
clashes and confusion in Squid code than before. Specifically, the
following terms and related terminology conflict with each other and/or
with the newly agreed upon scheme:

* server-side(50) means the opposite of src/servers/
* client-side(60) means the opposite of src/clients/
* src/Server(150) class should be src/clients/Client
* ConnStateData(100) class should be src/servers/{HttpServer,Server}
* "bar server" means a server we connect to, not src/servers/bar
* "foo client" means a client connected to us, not src/clients/foo

The numbers in (parenthesis) represent a very approximate number of
occurrences of the corresponding terms in trunk src/.

For example, the phrase "HTTP server" may mean either
src/servers/HttpServer (Squid class implementing an HTTP server) or an
HTTP origin server that our src/clients/HttpClient is talking to. Very
confusing!

Current developers should be able to successfully navigate this mess in
most cases because we can usually distinguish new code from the old one
and, hence, correctly guess the terminology flavor used. New developers
will be really puzzled. And even for current developers, deciding
whether to be consistent with the surrounding code or with the
current/new terminology is going to be exceedingly tricky!

I cannot propose a good solution that we can afford. I assume nobody
would volunteer to go through sources to fix all the inconsistencies and
violations in one sweep. If I am wrong, please speak up now so that we
can go through this procedure before v3.5 is branched!

I can only suggest the following imperfect measures to reduce the number
of clashes (using HTTP as an example/placeholder for other protocols):

0. Move src/Server.* to client/Client.* now(##). Unlike ConnStateData,
the existing Server class was scoped and named correctly (but based on
the old terminology!) so it clashes with the new terminology more than
any other class and it is easier to rename/move (we do not need to split
HTTP code out of it -- that has been done already).

1. If you want to talk about an agent connecting to Squid using HTTP,
say "HTTP user". Yes, "user" clashes with a human "user" that may
control that connecting HTTP agent, but that distinction is usually not
important. Use "human HTTP user" and "HTTP user agent" to differentiate
where needed. Yes, "user" also often implies "end user" rather than just
the previous HTTP hop. As I said, this suggestion is far from perfect.

2. If you want to talk about an agent Squid connects to using HTTP, say
"HTTP peer" (in general) and "HTTP origin" when talking about the end
BAR server specifically. Yes, this suffers from problems similar to #1
above.

3. Use future class names in new comments, even if those classes do not
exist yet, especially when working outside the corresponding class code.
For example, when documenting something about ConnStateData inside
FwdState, call it Server instead of ConnStateData. This will reduce
future changes as the classes get renamed.

4. Keep ConnStateData as is for now, until somebody volunteers to
extract HTTP code from it and place the extracted HTTP code into
servers/HttpServer, moving the remaining generic code (from several
different client_side* files IIRC!) into src/servers/Server and src/http/.

(##) In this context, "now" means after the Peek and Splice changes are
committed to avoid updating that already approved patch) and before v3.5
is branched (to minimize future porting pains). If there are any other
large changes that should go in first, please speak up!

Any comments, objections, or better ideas?

Thank you,

Alex.
Received on Tue Aug 26 2014 - 21:00:16 MDT