cleanup

src-run · Sep 15, 2016 · 893772e · 893772e
1 parent 04aa36f
commit 893772e
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 45 deletions.
diff --git a/lib/FileLock.php b/lib/FileLock.php
@@ -31,19 +31,19 @@ class FileLock implements FileLockInterface
     private $file;
 
     /**
-     * @var int
+     * @var resource
      */
-    private $options;
+    private $fileResource;
 
     /**
-     * @var resource
+     * @var int
      */
-    private $handle;
+    private $options;
 
     /**
      * @var bool
      */
-    private $acquired;
+    private $acquired = false;
 
     /**
      * FileLockInterface constructor.
@@ -54,10 +54,16 @@ class FileLock implements FileLockInterface
      */
     final public function __construct($file, $options = null, LoggerInterface $logger = null)
     {
-        $this->file = $file;
-        $this->acquired = false;
+        if (self::LOCK_SHARED & $options && self::LOCK_EXCLUSIVE & $options) {
+            throw new InvalidOptionsException('Lock cannot be both shared and exclusive.');
+        }
 
-        $this->setOptions($options);
+        if (self::LOCK_NON_BLOCKING & $options && self::LOCK_BLOCKING & $options) {
+            throw new InvalidOptionsException('Lock cannot be both non-blocking and blocking.');
+        }
+
+        $this->options = $options === null ? self::LOCK_SHARED | self::LOCK_NON_BLOCKING : $options;
+        $this->file = $file;
 
         if ($logger) {
             $this->setLogger($logger);
@@ -114,14 +120,24 @@ final public function isNonBlocking()
         return (bool) (self::LOCK_NON_BLOCKING & $this->options);
     }
 
+    /**
+     * Returns if file handle is held.
+     *
+     * @return bool
+     */
+    final public function hasResource()
+    {
+        return is_resource($this->fileResource);
+    }
+
     /**
      * Returns the file handle.
      *
      * @return resource
      */
-    final public function getHandle()
+    final public function getResource()
     {
-        return $this->handle;
+        return $this->fileResource;
     }
 
     /**
@@ -144,7 +160,7 @@ final public function acquire()
             $desc .= ', blocking';
         }
 
-        if (!$this->flockOperation($type)) {
+        if (!$this->fileLock($type)) {
             $this->logDebug('Could not acquire {desc} lock on file {file}.', [
                 'desc' => $desc,
                 'file' => $this->file,
@@ -168,7 +184,7 @@ final public function acquire()
      */
     final public function release()
     {
-        if (!is_resource($this->handle) || !$this->flockOperation(LOCK_UN) || !fclose($this->handle)) {
+        if (!$this->hasResource() || !$this->fileLock(LOCK_UN) || !fclose($this->fileResource)) {
             $this->logDebug('Could not release lock on file {file}.', [
                 'file' => $this->file,
             ]);
@@ -185,46 +201,24 @@ final public function release()
         return true;
     }
 
-    /**
-     * @param int|null $options
-     *
-     * @throws InvalidOptionsException
-     */
-    private function setOptions($options)
-    {
-        if ($options === null) {
-            $options = self::LOCK_SHARED | self::LOCK_NON_BLOCKING;
-        }
-
-        if (self::LOCK_SHARED & $options && self::LOCK_EXCLUSIVE & $options) {
-            throw new InvalidOptionsException('Lock for "%s" cannot be both shared and exclusive.', $this->file);
-        }
-
-        if (self::LOCK_NON_BLOCKING & $options && self::LOCK_BLOCKING & $options) {
-            throw new InvalidOptionsException('Lock for "%s" cannot be both non blocking and blocking.', $this->file);
-        }
-
-        $this->options = $options;
-    }
-
     /**
      * @param int $operation
      *
      * @throws FileResourceException
      *
      * @return bool
      */
-    private function flockOperation($operation)
+    private function fileLock($operation)
     {
-        if (!$this->handle) {
-            $this->handle = @fopen($this->file, 'c');
+        if (!$this->hasResource()) {
+            $this->fileResource = @fopen($this->file, 'c+');
         }
 
-        if (!is_resource($this->handle)) {
+        if (!$this->hasResource()) {
             return false;
         }
 
-        return flock($this->handle, $operation);
+        return flock($this->fileResource, $operation);
     }
 }
 

diff --git a/lib/FileLockInterface.php b/lib/FileLockInterface.php
@@ -12,15 +12,31 @@
 namespace SR\File\Lock;
 
 use Psr\Log\LoggerInterface;
+use SR\Log\LoggerAwareInterface;
 
 /**
  * Interface that a file lock.
  */
-interface FileLockInterface
+interface FileLockInterface extends LoggerAwareInterface
 {
+    /**
+     * Option for shared lock.
+     */
     const LOCK_SHARED = 1;
+
+    /**
+     * Option for exclusive lock.
+     */
     const LOCK_EXCLUSIVE = 2;
+
+    /**
+     * Option for non-blocking lock acquisition.
+     */
     const LOCK_NON_BLOCKING = 4;
+
+    /**
+     * Option for blocking lock acquisition.
+     */
     const LOCK_BLOCKING = 8;
 
     /**
@@ -67,12 +83,19 @@ public function isBlocking();
      */
     public function isNonBlocking();
 
+    /**
+     * Returns if file handle is held.
+     *
+     * @return bool
+     */
+    public function hasResource();
+
     /**
      * Returns the file handle.
      *
      * @return resource
      */
-    public function getHandle();
+    public function getResource();
 
     /**
      * Try to acquire a file lock.

diff --git a/tests/FileLockTest.php b/tests/FileLockTest.php
@@ -42,7 +42,8 @@ public function testDefaultOptions()
         $this->assertFalse($lock->isExclusive());
         $this->assertFalse($lock->isBlocking());
         $this->assertFalse($lock->isAcquired());
-        $this->assertNull($lock->getHandle());
+        $this->assertNull($lock->getResource());
+        $this->assertFalse($lock->hasResource());
     }
 
     /**
@@ -67,11 +68,11 @@ public function testAcquireLock()
 
         $lock->acquire();
         $this->assertTrue($lock->isAcquired());
-        $this->assertTrue(is_resource($lock->getHandle()));
+        $this->assertTrue(is_resource($lock->getResource()));
 
         $lock->release();
         $this->assertFalse($lock->isAcquired());
-        $this->assertFalse(is_resource($lock->getHandle()));
+        $this->assertFalse(is_resource($lock->getResource()));
     }
 
     /**
@@ -105,6 +106,7 @@ public function testOptions()
         $this->assertTrue($lock->isExclusive());
         $this->assertTrue($lock->isBlocking());
         $this->assertTrue($lock->isAcquired());
+        $this->assertTrue($lock->hasResource());
 
         $lock->release();
 
@@ -185,7 +187,7 @@ public function testAcquireLogger()
             ->method('debug')
             ->with('Could not acquire {desc} lock on file {file}.', [
                 'file' => __FILE__.DIRECTORY_SEPARATOR.'does-not-exist.ext',
-                'desc' => 'shared'
+                'desc' => 'shared',
             ])
             ->willReturn(null);
 
@@ -205,6 +207,20 @@ public function testExclusiveLock()
         $lock2 = new FileLock(__FILE__, FileLock::LOCK_EXCLUSIVE | FileLock::LOCK_NON_BLOCKING);
         $lock2->acquire();
     }
+
+    public function testCreateFileOnLock()
+    {
+        $file = tempnam(sys_get_temp_dir(), 'lock-test');
+        unlink($file);
+        $this->assertFalse(file_exists($file));
+
+        $lock = new FileLock($file);
+        $lock->acquire();
+        $this->assertTrue(file_exists($file));
+
+        $lock->release();
+        unlink($file);
+    }
 }
 
 /* EOF */