Add types to private and internal properties

symfony · Jul 25, 2023 · 836e1d1 · 836e1d1
1 parent 67419c6
commit 836e1d1
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 45 deletions.
diff --git a/Lock.php b/Lock.php
@@ -13,7 +13,6 @@
 
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
-use Psr\Log\NullLogger;
 use Symfony\Component\Lock\Exception\InvalidArgumentException;
 use Symfony\Component\Lock\Exception\LockAcquiringException;
 use Symfony\Component\Lock\Exception\LockConflictedException;
@@ -29,24 +28,18 @@ final class Lock implements SharedLockInterface, LoggerAwareInterface
 {
     use LoggerAwareTrait;
 
-    private PersistingStoreInterface $store;
-    private Key $key;
-    private ?float $ttl;
-    private bool $autoRelease;
     private bool $dirty = false;
 
     /**
      * @param float|null $ttl         Maximum expected lock duration in seconds
      * @param bool       $autoRelease Whether to automatically release the lock or not when the lock instance is destroyed
      */
-    public function __construct(Key $key, PersistingStoreInterface $store, float $ttl = null, bool $autoRelease = true)
-    {
-        $this->store = $store;
-        $this->key = $key;
-        $this->ttl = $ttl;
-        $this->autoRelease = $autoRelease;
-
-        $this->logger = new NullLogger();
+    public function __construct(
+        private Key $key,
+        private PersistingStoreInterface $store,
+        private ?float $ttl = null,
+        private bool $autoRelease = true,
+    ) {
     }
 
     public function __sleep(): array
@@ -93,7 +86,7 @@ public function acquire(bool $blocking = false): bool
             }
 
             $this->dirty = true;
-            $this->logger->debug('Successfully acquired the "{resource}" lock.', ['resource' => $this->key]);
+            $this->logger?->debug('Successfully acquired the "{resource}" lock.', ['resource' => $this->key]);
 
             if ($this->ttl) {
                 $this->refresh();
@@ -111,15 +104,15 @@ public function acquire(bool $blocking = false): bool
             return true;
         } catch (LockConflictedException $e) {
             $this->dirty = false;
-            $this->logger->info('Failed to acquire the "{resource}" lock. Someone else already acquired the lock.', ['resource' => $this->key]);
+            $this->logger?->info('Failed to acquire the "{resource}" lock. Someone else already acquired the lock.', ['resource' => $this->key]);
 
             if ($blocking) {
                 throw $e;
             }
 
             return false;
         } catch (\Exception $e) {
-            $this->logger->notice('Failed to acquire the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]);
+            $this->logger?->notice('Failed to acquire the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]);
             throw new LockAcquiringException(sprintf('Failed to acquire the "%s" lock.', $this->key), 0, $e);
         }
     }
@@ -129,7 +122,7 @@ public function acquireRead(bool $blocking = false): bool
         $this->key->resetLifetime();
         try {
             if (!$this->store instanceof SharedLockStoreInterface) {
-                $this->logger->debug('Store does not support ReadLocks, fallback to WriteLock.', ['resource' => $this->key]);
+                $this->logger?->debug('Store does not support ReadLocks, fallback to WriteLock.', ['resource' => $this->key]);
 
                 return $this->acquire($blocking);
             }
@@ -151,7 +144,7 @@ public function acquireRead(bool $blocking = false): bool
             }
 
             $this->dirty = true;
-            $this->logger->debug('Successfully acquired the "{resource}" lock for reading.', ['resource' => $this->key]);
+            $this->logger?->debug('Successfully acquired the "{resource}" lock for reading.', ['resource' => $this->key]);
 
             if ($this->ttl) {
                 $this->refresh();
@@ -169,15 +162,15 @@ public function acquireRead(bool $blocking = false): bool
             return true;
         } catch (LockConflictedException $e) {
             $this->dirty = false;
-            $this->logger->info('Failed to acquire the "{resource}" lock. Someone else already acquired the lock.', ['resource' => $this->key]);
+            $this->logger?->info('Failed to acquire the "{resource}" lock. Someone else already acquired the lock.', ['resource' => $this->key]);
 
             if ($blocking) {
                 throw $e;
             }
 
             return false;
         } catch (\Exception $e) {
-            $this->logger->notice('Failed to acquire the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]);
+            $this->logger?->notice('Failed to acquire the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]);
             throw new LockAcquiringException(sprintf('Failed to acquire the "%s" lock.', $this->key), 0, $e);
         }
     }
@@ -202,13 +195,13 @@ public function refresh(float $ttl = null): void
                 throw new LockExpiredException(sprintf('Failed to put off the expiration of the "%s" lock within the specified time.', $this->key));
             }
 
-            $this->logger->debug('Expiration defined for "{resource}" lock for "{ttl}" seconds.', ['resource' => $this->key, 'ttl' => $ttl]);
+            $this->logger?->debug('Expiration defined for "{resource}" lock for "{ttl}" seconds.', ['resource' => $this->key, 'ttl' => $ttl]);
         } catch (LockConflictedException $e) {
             $this->dirty = false;
-            $this->logger->notice('Failed to define an expiration for the "{resource}" lock, someone else acquired the lock.', ['resource' => $this->key]);
+            $this->logger?->notice('Failed to define an expiration for the "{resource}" lock, someone else acquired the lock.', ['resource' => $this->key]);
             throw $e;
         } catch (\Exception $e) {
-            $this->logger->notice('Failed to define an expiration for the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]);
+            $this->logger?->notice('Failed to define an expiration for the "{resource}" lock.', ['resource' => $this->key, 'exception' => $e]);
             throw new LockAcquiringException(sprintf('Failed to define an expiration for the "%s" lock.', $this->key), 0, $e);
         }
     }
@@ -234,9 +227,9 @@ public function release(): void
                 throw new LockReleasingException(sprintf('Failed to release the "%s" lock, the resource is still locked.', $this->key));
             }
 
-            $this->logger->debug('Successfully released the "{resource}" lock.', ['resource' => $this->key]);
+            $this->logger?->debug('Successfully released the "{resource}" lock.', ['resource' => $this->key]);
         } catch (LockReleasingException $e) {
-            $this->logger->notice('Failed to release the "{resource}" lock.', ['resource' => $this->key]);
+            $this->logger?->notice('Failed to release the "{resource}" lock.', ['resource' => $this->key]);
             throw $e;
         }
     }

diff --git a/LockFactory.php b/LockFactory.php
@@ -25,13 +25,9 @@ class LockFactory implements LoggerAwareInterface
 {
     use LoggerAwareTrait;
 
-    private PersistingStoreInterface $store;
-
-    public function __construct(PersistingStoreInterface $store)
-    {
-        $this->store = $store;
-
-        $this->logger = new NullLogger();
+    public function __construct(
+        private PersistingStoreInterface $store,
+    ) {
     }
 
     /**
@@ -60,7 +56,9 @@ public function createLock(string $resource, ?float $ttl = 300.0, bool $autoRele
     public function createLockFromKey(Key $key, ?float $ttl = 300.0, bool $autoRelease = true): LockInterface
     {
         $lock = new Lock($key, $this->store, $ttl, $autoRelease);
-        $lock->setLogger($this->logger);
+        if ($this->logger) {
+            $lock->setLogger($this->logger);
+        }
 
         return $lock;
     }

diff --git a/Store/CombinedStore.php b/Store/CombinedStore.php
@@ -13,7 +13,6 @@
 
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
-use Psr\Log\NullLogger;
 use Symfony\Component\Lock\Exception\InvalidArgumentException;
 use Symfony\Component\Lock\Exception\LockConflictedException;
 use Symfony\Component\Lock\Key;
@@ -50,7 +49,6 @@ public function __construct(array $stores, StrategyInterface $strategy)
 
         $this->stores = $stores;
         $this->strategy = $strategy;
-        $this->logger = new NullLogger();
     }
 
     /**
@@ -67,7 +65,7 @@ public function save(Key $key)
                 $store->save($key);
                 ++$successCount;
             } catch (\Exception $e) {
-                $this->logger->debug('One store failed to save the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
+                $this->logger?->debug('One store failed to save the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
                 ++$failureCount;
             }
 
@@ -82,7 +80,7 @@ public function save(Key $key)
             return;
         }
 
-        $this->logger->info('Failed to store the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]);
+        $this->logger?->info('Failed to store the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]);
 
         // clean up potential locks
         $this->delete($key);
@@ -108,7 +106,7 @@ public function saveRead(Key $key)
                 }
                 ++$successCount;
             } catch (\Exception $e) {
-                $this->logger->debug('One store failed to save the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
+                $this->logger?->debug('One store failed to save the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
                 ++$failureCount;
             }
 
@@ -123,7 +121,7 @@ public function saveRead(Key $key)
             return;
         }
 
-        $this->logger->info('Failed to store the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]);
+        $this->logger?->info('Failed to store the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]);
 
         // clean up potential locks
         $this->delete($key);
@@ -144,15 +142,15 @@ public function putOffExpiration(Key $key, float $ttl)
         foreach ($this->stores as $store) {
             try {
                 if (0.0 >= $adjustedTtl = $expireAt - microtime(true)) {
-                    $this->logger->debug('Stores took to long to put off the expiration of the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'ttl' => $ttl]);
+                    $this->logger?->debug('Stores took to long to put off the expiration of the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'ttl' => $ttl]);
                     $key->reduceLifetime(0);
                     break;
                 }
 
                 $store->putOffExpiration($key, $adjustedTtl);
                 ++$successCount;
             } catch (\Exception $e) {
-                $this->logger->debug('One store failed to put off the expiration of the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
+                $this->logger?->debug('One store failed to put off the expiration of the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
                 ++$failureCount;
             }
 
@@ -167,7 +165,7 @@ public function putOffExpiration(Key $key, float $ttl)
             return;
         }
 
-        $this->logger->notice('Failed to define the expiration for the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]);
+        $this->logger?->notice('Failed to define the expiration for the "{resource}" lock. Quorum has not been met.', ['resource' => $key, 'success' => $successCount, 'failure' => $failureCount]);
 
         // clean up potential locks
         $this->delete($key);
@@ -184,7 +182,7 @@ public function delete(Key $key)
             try {
                 $store->delete($key);
             } catch (\Exception $e) {
-                $this->logger->notice('One store failed to delete the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
+                $this->logger?->notice('One store failed to delete the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
             }
         }
     }
@@ -203,7 +201,7 @@ public function exists(Key $key): bool
                     ++$failureCount;
                 }
             } catch (\Exception $e) {
-                $this->logger->debug('One store failed to check the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
+                $this->logger?->debug('One store failed to check the "{resource}" lock.', ['resource' => $key, 'store' => $store, 'exception' => $e]);
                 ++$failureCount;
             }
 

diff --git a/Store/DoctrineDbalPostgreSqlStore.php b/Store/DoctrineDbalPostgreSqlStore.php
@@ -33,7 +33,7 @@
 class DoctrineDbalPostgreSqlStore implements BlockingSharedLockStoreInterface, BlockingStoreInterface
 {
     private Connection $conn;
-    private static $storeRegistry = [];
+    private static array $storeRegistry = [];
 
     /**
      * You can either pass an existing database connection a Doctrine DBAL Connection