Re: [PATCH] gitweb: return correct HTTP status codes

[Lea Wiemann <lewiemann@gmail.com>]

Jakub Narebski wrote:
> But what are arguments for "check params; run command" vs "run command;
> check params if error" proposed by Junio?  Why do you want to check
> parameters upfront?

It's actually not checking, it's resolving.  Instead of ...

get_commit($symbol)

... (with $symbol = 'HEAD' for instance), you do (pseudocode):

$hash = get_hash($symbol, 'commit'); # 'commit' to resolve tags
check that $hash is defined
get_commit($hash)

(And get_commit won't even accept anything but 40-byte hashes.)  This is 
for two reasons:

1. Caching: Resolving symbols first gives you some (very few) cache 
entries that need to be expired (namely, get_hash results for symbols 
that are not SHA1 hashes already), but most cache entries (like the 
get_commit) are infinitely valid.

2. Besides being a little more straightforward to implement, it ensures 
that you have well-defined failure points.  IOW, first you resolve the 
symbols you're getting from the user and check them for existence (and 
correct type).  From then on, any hashes you pass around are guaranteed 
to be valid, so failures indicate that something serious went wrong. 
Apart from making things easier for the developer by reminding them of 
the points where they *need* to check for errors, it means that you can 
very easily check the error-handling code for completeness (by only 
reading the first few lines that resolve symbols).

> By the way, would you be sending your current WIP for review?

Will do in the next 2 days after some cleanup.

> [is] adding object oriented interface (wrapper) to git repositories
> really  needed for implementing gitweb caching?

It makes things better to read for sure, and it means that you can have 
a plain API without caching, and implement the caching layer as a 
subclass (which overrides the methods with cacheable results).

-- Lea