symfony/symfony — pull request #5381 by msonnabaum (2012-08-30 04:21:48+00:00)
Title: Added lockExists to Store interface, fixed locking bugs, added tests.

While working on Drupal's HttpCache implementation, I discovered that the base HttpCache class does an is_file to check for a lock, which assumes a file-based cache is being used. This seems like a mistake since the rest of the Store interface is easily swappable. I added a lockExists method so that this is properly abstracted.

I also noticed there were no tests for the change I made, so I added some very basic locking tests. While adding those I found that the existing lock method is a bit broken. This line here:

```php
getPath($this->getCacheKey($request).'.lck'), 'x')) {
```

will return false if the file couldn't be written for any reason, but the rest of the method assumes that if $lock == false, the lock exists already. So if the file couldnt be written due to the parent directory not existing, $path will be returned as if it exists, which is clearly not the desired behavior.

I changed this to return false if the file couldnt be written and doesn't exist, $path if it exists, and true if the lock was created. It still doesn't feel great to have bool|string return values, but that's the best I could come up with atm. I also added a check for the parent directory that creates it if it doesn't exist. The new tests fail without it.

I also broke out that code a bit as it was very difficult to read.