Feature Proposals » A collection of ideas for TWiki refactoring, all of them tedious and maybe wrong as well

Motivation

Whenever I walk through TWiki code while working on a Support Item or a bug, there are things I find disturbing, or annoying. And there are things which are very disturbing and very annoying, too. Mind you, these are not bugs, the code works. But these things slow TWiki down, and what may be worse, they slow TWiki development down (at least for me, who has't worked with the code for years).

In some cases this is just a matter of bringing a brush and a shovel, but in some cases the root cause is deeply buried under several years of creeping featurism.

So these are the issues:

Testing SVN via webserver is too tedious

I find testing SVN TWiki with a webserver is a nuisance and very difficult to automate. One of the annoyances is file permissions: the SVN tree belongs to me, but stuff created by the webserver belongs to whomever the Linux distribution thinks it should belong. Yes, I can open up permissions, I can create Linux groups containing me and the webserver user, but I still have to sudo when I want to change the ownership of a file.

Remedy:

• Make TWiki runnable and browsable under my own user id. There was some work to that end in TWikiStandAlone, but guess what: The script ./tools/twiki_http.pl mentioned in the section HTTP Engine isn't shipped with TWiki and it is not in SVN trunk. I have no idea how much work would be necessary to get this running again, but I doubt that it is worth it. In these days, the way to go with Perl is Wikipedia:PSGI. I hear you cough, and I understand that. Removing all the CGI.pm stuff would really be a enormous task.

Benefits:

• Tests could be automated as firing up the standalone PSGI server plackup, running the test with a LWP::UserAgent, then killing the server. For a test suite, or for an individual test. Can even be done in parallel by listening on different ports, without affecting any productive web server.
• Running the standalone server would create files with permissions I can handle.
• Code sections asking whether we run under mod_perl vanish into thin air.
• Oh, and apart from testing: There are web hosters which run PSGI applications for you.

The TWiki object structure is messy (or: Names matter)

In TWiki, we have the $twiki object, and the $session object, and most of the times they refer to the same thing. Both of these names confer the wrong context, at least to me. A lot of performance hits occur due to the unclear purpose of these objects. There are cases where the same parameter is passed in the $twiki object, as a parameter to a function call, and (shudder) occasionally in an environment variable, too. And because somewhere in the deeply nested procedures some function use the parameter from the object and some from @_ we are doomed to supply both (because we've found out that some unit tests fail if we don't).

In the ancient CGI this didn't matter so much: There was a HTTP request which was transformed by magic to a HTTP response, and then the server was entitled to forget what he was doing.

Then came persistent interpreters. TWiki is expensive to compile due to its amount of code, so let's do it just once. And... possibly we could also make some data structures persistent? But which ones? And where to store them? There has, as far as I can say, never been a design decision to cut the objects in a way that reflects their lifetime. An object called $twiki sounds like it would hold data which persist as long as the application is running, and only those. But we know that this isn't true, and after every request there's a lot of cleanup to undef data. The code we have today is, in my opinion, way too cluttered with BEGIN blocks. They have their purpose in Perl, but they are difficult or impossible to catch in a debugger.

Then came sessions. An user's actions are remembered between his browser and the server. Whenever I see $session in TWiki code, my skin starts to itch, because in most cases it is not the user session which the variable refers to. Over time, TWiki has responded to user sessions by spawning the sub-objects user and users and bolting on a lot of mapping, checking and crossreferencing code which contains comments like TODO: why is this not used internally? When is it called, and why, added in 2007. By now I know when it is called and why, but honestly, I refuse to add yet another comment to this section.

Remedy:

• Refactor TWiki's object structure so that it reflects the object's life cycle.
  • A $twiki object should be for data with an application scope, and it shouldn't be necessary to build sections of it for every request.
  • The name $session should only be used if we refer to a session in the sense used today (user session). In today's TWiki the user session is associated with the session cookie and the data stored on the server side, which is quite fine.
  • We need to create a new root object for all data which are valid only for the current request. When done, then let this object go out of scope and that's it. Though I am carrying this idea with me for quite some time now, I lack a good name for this object. Request would come handy, but is already taken for HTTP::Request which is a well-defined object describing all input data for the current task.

Benefits:

• Performance. Persistent interpreters would be faster because they know what to persist. Lots of re- and de-initialisations are no longer necessary.
• Code clarity. We know reliably which object should hold the data and reduce side effects. I guess that many of today's circular references can vanish as well.

TWiki's source code conventions have been lost over time

Due to the high productivity of developers, especially around TWiki 4, the source code conventions have more or less been lost. Yes, times have moved on, and the original conventions may not have been the state of the art any more, but replacing them by nothing doesn't help.

Remedy:

• Establish best practice for TWiki coding. Perl best practices. Perl best practices as in Damian Conway's book. Ooops. The topic PerlBestPractices already exists. I wrote it. Back in 2007.

Benefits:

• It is easier to read other people's code.
• The book also contains ready-made emacs and vi configurations, so conforming to the conventions is pretty easy.
• There's a toolset for Perl::Critic which allows to check for PBP violations

TWiki isn't linuxish (or windowsish) enough

This is more an issue for productive installations, which tend to have a segregation of duties between sysadmins and application admins. In my current installation, TWiki code is under /opt/twiki, data are under /var/opt/twiki, configuration stuff under /etc/opt/twiki, and so on. I am too lazy to move the TWiki web to the documentation directories, but this is how it should be. This may be just a personal preference, but with this setup I find it easier to fit it into my system operations (backup, upgrade, local changes).

I think that this is a similar task to previous attempts to create Debian packages for TWiki, so I don't go into details on that one. I think that it is possible to ship TWiki with a post-installation script that does the trick. For Windows installations, conventions are different, but they do exist (stored in system variables like e.g. APPDATA).

TWiki isn't Perlish enough

The way how Perl code is developed has changed a lot since TWiki 1.0. I am an old dog and only reluctantly learn new tricks, but I find it annoying that TWiki does make so little use of the Perl development environment. Here are some examples:

• Perl test infrastucture - not only the Test::* modules, but also things as simple as /usr/bin/prove. TWiki has its own infrastructure, which has great merits, but I always need to reconfigure my brain when switching into the TWiki world. And we know that right now the test suite has issues.
• Perl package builders: Perl now offers things like Module::Starter and Dist::Zilla, with slightly different scope. They would make TWiki look like the other Perl stuff I write and fit with Perl's test infrastructure, but also with CPAN (e.g. dependency checking).
• Moose: Ok, this may be a tad to radical and I hear you shouting "the performance hit!" Yes, for a non-persistent TWiki interpreter, Moose might eat to much. But in any persistent environment (did I mention PSGI?) it doesn't matter at all. But even if it isn't used in the application, Moose can be useful, for example to write test classes.

TWiki triggers the olfactory system too often

When reading TWiki code, it doesn't take long until you find a SMELL, a TODO or a FIXME. A grep finds about 300 in TWiki core. Some of them indicate that the writer of the comment simply hasn't understood the code, some suggest that the code is inefficient, and some might actually point to a serious problem lurking in the shadows. This hinders development because people don't dare to touch such code without gloves, and with gloves typing is seriously slowed down.

Remedy: Apply a brush and a shovel. I feel that TWiki's functionality is mature enough, tending to over-featured. With a reduced pressure for new features, there should be time to reduce the smell.

Benefit:

• Performance: I'd assume that TWiki could lose some hundreds line of bolt-on double and triple check code without losing any features.
• Developer trust: A code without smell (not just with the SMELL comment removed) is more inviting to improve.

Impact

Implementation

Maybe one of the issues annoys other people, too. Perhaps there's enough interest to start a branch?

-- Contributors: Harald Jörg - 2015-01-23

Discussion

Fresh pair of eyes, very good points, Harald! I like especially the $twiki vs $session object, important to fix for performance.

-- Peter Thoeny - 2015-01-24