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

Jakub Narebski wrote:

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

Will do in the next 2 days after some cleanup.

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