Re: Naming clashes post src/clients, src/servers restructuring

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 27/08/2014 9:00 a.m., Alex Rousskov wrote:
> 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(##).

Agreed.

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

ConnStateData is a non-issue for now. see #3.

>
> 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.
>

If we are to do #3 below there should be no confusion when talking
about clients and servers (non-class name) as external to Squid.
Developers with basic understanding of client-server networking will
not be confused that a client and FooServer interact, or a FooClient
and server. Those who are we can reference to wikipedia or a tech
dictionary.

> 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.

Sounds like a good plan. It may be worth referencing RFC 7230 for
terminology definition of the naming for client, server, gateway,
relay, tunnel classes.

Or better, once Server is renamed referencing the existig class names
should also be fine. The "FooStateData" names for example are not in
themselves part of the "client"/"server" actor-vs-side confusion.

>
> 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/.

Agreed. IMO this should fall nicely inline sometime after the Parer-NG
updates have detached/extracted the majority of parsing code.

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

Sounds good.

Amos
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)

iQEcBAEBAgAGBQJT/ykVAAoJELJo5wb/XPRjsKgH/iBKuicBf72C0DuPy5hCth7p
mwhx7V7dLmAHv6zUAoSaWFL/+4CCxOGptUsqATcZuzAvgCjLmub0I7ZkGd1mPAH5
ocCpdo8PRHWn7HzXHOC/Kf7bHz0xrkz8UbotLXaIPpI7wBxD2Pl3xrCzh5FeQDzP
1cIiubPHFF1sTs5LwvdNs585odTeGcZw2BNqD8aayOq4+AeoY0mUxXFsI9zCGx8M
6d+xl+tFZ4V6sM/uH0uEh8Vm+kgeVRBXltRhmRSxiAI0lka3QU+jOPeFVy2wa684
ICilYNW3StNalT1EjBRsembyjx78QiSzlzRysfTb7eSBjMFJfS131rV9xiCifew=
=GDMw
-----END PGP SIGNATURE-----
Received on Thu Aug 28 2014 - 13:05:41 MDT