Merge pull request #8 from mathroc/refactoring

refactoring

Author: mathroc

README.md

@@ -44,9 +44,9 @@ $lock->release();
 
 use TH\Lock\FileLock;
 
-$lock = new FileLock('/path/to/file');
+$lock = new FileLock('/path/to/file', FileLock::SHARED);
 
-$lock->acquire(FileLock::SHARED);
+$lock->acquire();
 
 // other processes that try to acquire an exclusive lock on the file will fail,
 // processes that try to acquire an shared lock on the file will succeed
@@ -58,10 +58,6 @@ $lock->acquire();
 // other processes can now acquire an exclusive lock on the file if no other shared lock remains.
 ```
 
-### Upgrading or downgrading a lock
-
-`$lock->acquire()` can be called multiple times to either upgrade a shared lock to an exclusive lock or downgrade an exclusive lock to a shared one. You can use it when you're done editing the file.
-
 ### Auto release
 
 `$lock->release()` is called automatically when the lock is destroyed so you don't need to manually release it at the end of a script or if it got out of scope.
@@ -121,13 +117,13 @@ $lock = $factory->create('resource identifier');
 
 There are two methods you can use on a `FileLock`:
 
-* `\TH\Lock\FileLock::acquire($exclusive = FileLock::EXCLUSIVE, $blocking = FileLock::NON_BLOCKING)` used to acquire a lock on the file
+* `\TH\Lock\FileLock::acquire()` used to acquire a lock on the file
 * `\TH\Lock\FileLock::release()` used to release a lock on the file
 
-And one on a `FileFactory':
+And one on a `FileFactory`:
 
-* `\TH\Lock\FileFactory::create($resource, $owner = null)` used to create a `FileLock` for $resource
+* `\TH\Lock\FileFactory::create($resource, $owner = null, $exclusive = FileLock::EXCLUSIVE, $blocking = FileLock::NON_BLOCKING)` used to create a `FileLock` for `$resource`
 
 ## Notes
 
-Only lock file are currently supported. It means distributed processes can't be lock this way. It should be easy to implements the `TH\Lock\Lock` and `TH\Lock\LockFactory` interface to use a distributed lock mechanism (maybe with [Redis](http://redis.io/topics/distlock)).
+Only lock file are currently supported. It means distributed processes can't be lock this way. It should be easy to implements the `TH\Lock\Lock`  interface to use a distributed lock mechanism (maybe with [Redis](http://redis.io/topics/distlock)).

spec/TH/Lock/FileFactorySpec.php

@@ -22,7 +22,6 @@ public function let()
     public function it_is_initializable()
     {
         $this->shouldHaveType('TH\Lock\FileFactory');
-        $this->shouldImplement('TH\Lock\LockFactory');
     }
 
     public function it_should_create_a_file_lock()

spec/TH/Lock/FileLockSpec.php

@@ -42,9 +42,11 @@ public function it_should_acquire_an_exclusive_lock()
         }
     }
 
-    public function it_should_acquire_an_shared_lock()
+    public function it_should_acquire_a_shared_lock()
     {
-        $this->acquire(FileLock::SHARED);
+        $this->beConstructedWith($this->lock_file, FileLock::SHARED);
+
+        $this->acquire();
 
         if (!file_exists($this->lock_file)) {
             throw new Exception('Lock file was not created');
@@ -61,7 +63,7 @@ public function it_should_acquire_an_shared_lock()
 
     public function it_should_release_a_lock()
     {
-        $this->acquire(FileLock::SHARED);
+        $this->acquire();
         $this->release();
 
         if (!flock(fopen($this->lock_file, 'r'), LOCK_EX|LOCK_NB)) {
@@ -79,7 +81,7 @@ public function it_should_throw_if_it_cannot_lock()
 
     public function it_remove_its_lock_file_if_not_locked()
     {
-        $this->beConstructedWith($this->lock_file, null, null, true);
+        $this->beConstructedWith($this->lock_file, FileLock::EXCLUSIVE, FileLock::NON_BLOCKING, null, null, true);
 
         $this->acquire();
         $this->release();
@@ -91,12 +93,12 @@ public function it_remove_its_lock_file_if_not_locked()
 
     public function it_does_not_remove_its_lock_file_if_still_locked()
     {
-        $this->beConstructedWith($this->lock_file, null, null, true);
+        $this->beConstructedWith($this->lock_file, FileLock::SHARED, FileLock::NON_BLOCKING, null, null, true);
 
         touch($this->lock_file);
         flock(fopen($this->lock_file, 'r'), LOCK_SH|LOCK_NB);
 
-        $this->acquire(FileLock::SHARED);
+        $this->acquire();
         $this->release();
 
         if (!file_exists($this->lock_file)) {
@@ -106,7 +108,7 @@ public function it_does_not_remove_its_lock_file_if_still_locked()
 
     public function it_can_acquire_then_release_and_acquire_again()
     {
-        $this->beConstructedWith($this->lock_file, null, null, true);
+        $this->beConstructedWith($this->lock_file, FileLock::EXCLUSIVE, FileLock::NON_BLOCKING, null, null, true);
 
         $this->acquire();
         $this->release();

src/FileFactory.php

@@ -5,13 +5,20 @@
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 
-class FileFactory implements LockFactory
+class FileFactory
 {
     private $lock_dir;
     private $hash_algo;
 
     private $logger;
 
+    /**
+     * Create a new FileFactory
+     * @param string               $lock_dir  directory where lock files will be created
+     * @param string               $hash_algo hashing algotithms used to generate lock file name from identifier
+     *                                        see http://php.net/manual/en/function.hash-algos.php
+     * @param LoggerInterface|null $logger
+     */
     public function __construct($lock_dir, $hash_algo = 'sha256', LoggerInterface $logger = null)
     {
         $this->lock_dir  = $lock_dir;
@@ -20,15 +27,27 @@ public function __construct($lock_dir, $hash_algo = 'sha256', LoggerInterface $l
         $this->logger = $logger ?: new NullLogger;
     }
 
-    public function create($resource, $owner = null)
-    {
+    /**
+     * Create a FileLock for $resource
+     * @param string      $resource  resource identifier
+     * @param string|null $owner     owner name (for logging)
+     * @param boolean     $exclusive true for an exclusive lock, false for shared one
+     * @param boolean     $blocking  true to wait for lock to be available, false to throw exception instead of waiting
+     * @return FileLock
+     */
+    public function create(
+        $resource,
+        $owner = null,
+        $exclusive = FileLock::EXCLUSIVE,
+        $blocking = FileLock::NON_BLOCKING
+    ) {
         if (!is_dir($this->lock_dir)) {
             mkdir($this->lock_dir, 0777, true);
         }
 
         $path = $this->lock_dir.'/'.hash($this->hash_algo, serialize($resource)).'.lock';
 
-        $lock = new FileLock($path, $resource, $owner, true, $this->logger);
+        $lock = new FileLock($path, $exclusive, $blocking, $resource, $owner, true, $this->logger);
 
         return $lock;
     }

src/FileLock.php

@@ -15,6 +15,8 @@ class FileLock implements Lock
     const NON_BLOCKING = false;
 
     private $lock_file;
+    private $exclusive;
+    private $blocking;
     private $identifier;
     private $owner;
     private $fh;
@@ -24,19 +26,26 @@ class FileLock implements Lock
 
     /**
      * @param string          $lock_file         path to file
+     * @param boolean         $exclusive         true for an exclusive lock, false for shared one
+     * @param boolean         $blocking          true to wait for lock to be available,
+     *                                           false to throw exception instead of waiting
      * @param string|null     $identifier        resource identifier (default to $lock_file) for logging
      * @param string|null     $owner             owner name for logging
      * @param boolean         $remove_on_release remove file on release if no other lock remains
      * @param LoggerInterface $logger
      */
     public function __construct(
         $lock_file,
+        $exclusive = FileLock::EXCLUSIVE,
+        $blocking = FileLock::NON_BLOCKING,
         $identifier = null,
         $owner = null,
         $remove_on_release = false,
         LoggerInterface $logger = null
     ) {
         $this->lock_file         = $lock_file;
+        $this->exclusive         = $exclusive;
+        $this->blocking          = $blocking;
         $this->identifier        = $identifier?:$lock_file;
         $this->owner             = $owner === null ? '' : $owner.': ';
         $this->remove_on_release = $remove_on_release;
@@ -45,21 +54,19 @@ public function __construct(
     }
 
     /**
-     * @param  boolean $exclusive true for an exclusive lock, false for shared one
-     * @param  boolean $blocking  true to wait for lock to be available, false to throw exception instead of waiting
      * @inherit
      */
-    public function acquire($exclusive = FileLock::EXCLUSIVE, $blocking = FileLock::NON_BLOCKING)
+    public function acquire()
     {
-        if ($exclusive === FileLock::EXCLUSIVE) {
+        if ($this->exclusive === FileLock::EXCLUSIVE) {
             $lock_type = 'exclusive';
             $operation = LOCK_EX;
         } else {
             $lock_type = 'shared';
             $operation = LOCK_SH;
         }
 
-        if ($blocking === FileLock::NON_BLOCKING) {
+        if ($this->blocking === FileLock::NON_BLOCKING) {
             $operation |= LOCK_NB;
         }