Fix #135 - remove suppress the errors by @

lib/OpenPayU/Oauth/Cache/OauthCacheFile.php

@@ -2,39 +2,54 @@
 
 class OauthCacheFile implements OauthCacheInterface
 {
-    private $directory;
+    private ?string $directory;
 
     /**
-     * @param string $directory
      * @throws OpenPayU_Exception_Configuration
      */
-    public function __construct($directory = null)
+    public function __construct(string $directory = null)
     {
         if ($directory === null) {
-            $directory = dirname(__FILE__).'/../../../Cache';
+            $directory = __DIR__ . '/../../../Cache';
         }
 
-        if (!is_dir($directory) || !is_writable($directory)) {
-            throw new OpenPayU_Exception_Configuration('Cache directory [' . $directory . '] not exist or not writable.');
+        if ( ! is_dir($directory) || ! is_writable($directory)) {
+            throw new OpenPayU_Exception_Configuration(
+                'Cache directory [' . $directory . '] not exist or not writable.'
+            );
         }
 
-        $this->directory = $directory . (substr($directory, -1) != '/' ? '/' : '');
+        $this->directory = $directory . (substr($directory, -1) !== '/' ? '/' : '');
     }
 
-    public function get($key)
+    public function get(string $key): ?OauthResultClientCredentials
     {
-        $cache = @file_get_contents($this->directory . md5($key));
-        return $cache === false ? null : unserialize($cache);
+        $cacheFile = $this->getFilePath($key);
+
+        try {
+            return file_exists($cacheFile) ? unserialize(
+                file_get_contents($cacheFile),
+                ['allowed_classes' => [OauthResultClientCredentials::class]]
+            ) : null;
+        } catch (\Throwable $e) {
+            return null;
+        }
     }
 
-    public function set($key, $value)
+    public function set(string $key, OauthResultClientCredentials $value): bool
     {
-        return @file_put_contents($this->directory . md5($key), serialize($value));
+        return file_put_contents($this->directory . md5($key), serialize($value)) !== false;
     }
 
-    public function invalidate($key)
+    public function invalidate(string $key): bool
     {
-        return @unlink($this->directory . md5($key));
+        $cacheFile = $this->getFilePath($key);
+
+        return !file_exists($cacheFile) || unlink($cacheFile);
     }
 
-}
+    private function getFilePath(string $key): string
+    {
+        return $this->directory . md5($key);
+    }
+}

lib/OpenPayU/Oauth/Cache/OauthCacheInterface.php

@@ -1,26 +1,11 @@
 <?php
 
-
 interface OauthCacheInterface
 {
 
-    /**
-     * @param string $key
-     * @return null | object
-     */
-    public function get($key);
-
-    /**
-     * @param string $key
-     * @param object $value
-     * @return bool
-     */
-    public function set($key, $value);
+    public function get(string $key): ?OauthResultClientCredentials;
 
-    /**
-     * @param string $key
-     * @return bool
-     */
-    public function invalidate($key);
+    public function set(string $key, OauthResultClientCredentials $value): bool;
 
-}
+    public function invalidate(string  $key): bool;
+}

lib/OpenPayU/Oauth/Cache/OauthCacheMemcached.php

@@ -2,15 +2,12 @@
 
 class OauthCacheMemcached implements OauthCacheInterface
 {
-    private $memcached;
+    private \Memcached $memcached;
 
     /**
-     * @param string $host
-     * @param int $port
-     * @param int $weight
      * @throws OpenPayU_Exception_Configuration
      */
-    public function __construct($host = 'localhost', $port = 11211, $weight = 0)
+    public function __construct(string $host = 'localhost', int $port = 11211, int $weight = 0)
     {
         if (!class_exists('Memcached')) {
             throw new OpenPayU_Exception_Configuration('PHP Memcached extension not installed.');
@@ -19,25 +16,31 @@ public function __construct($host = 'localhost', $port = 11211, $weight = 0)
         $this->memcached = new Memcached();
         $this->memcached->addServer($host, $port, $weight);
         $stats = $this->memcached->getStats();
-        if ($stats[$host . ':' . $port]['pid'] == -1) {
+        if ($stats[$host . ':' . $port]['pid'] === -1) {
             throw new OpenPayU_Exception_Configuration('Problem with connection to memcached server [host=' . $host . '] [port=' . $port . '] [weight=' . $weight . ']');
         }
     }
 
-    public function get($key)
+    public function get(string $key): ?OauthResultClientCredentials
     {
         $cache = $this->memcached->get($key);
-        return $cache === false ? null : unserialize($cache);
+        try {
+            return $cache === false ? null : unserialize(
+                $cache,
+                ['allowed_classes' => [OauthResultClientCredentials::class]]
+            );
+        } catch (\Error $e) {
+            return null;
+        }
     }
 
-    public function set($key, $value)
+    public function set(string $key, OauthResultClientCredentials $value): bool
     {
         return $this->memcached->set($key, serialize($value));
     }
 
-    public function invalidate($key)
+    public function invalidate(string $key): bool
     {
         return $this->memcached->delete($key);
     }
-
 }

lib/OpenPayU/Oauth/Oauth.php

@@ -27,7 +27,7 @@ public static function getAccessToken($clientId = null, $clientSecret = null)
 
         $tokenCache = self::$oauthTokenCache->get($cacheKey);
 
-        if ($tokenCache instanceof OauthResultClientCredentials && !$tokenCache->hasExpire()) {
+        if (isset($tokenCache) && !$tokenCache->hasExpire()) {
             return $tokenCache;
         }
 

tests/unit/OpenPayU/Oauth/Cache/OauthCacheFileTest.php

@@ -0,0 +1,191 @@
+<?php
+
+use PHPUnit\Framework\TestCase;
+
+require_once realpath(__DIR__) . '/../../../../TestHelper.php';
+
+class OauthCacheFileTest extends TestCase
+{
+    private string $tempDir;
+
+    protected function setUp(): void
+    {
+        $this->tempDir = sys_get_temp_dir() . '/oauthcache_test_' . uniqid('', true);
+        mkdir($this->tempDir, 0777, true);
+    }
+
+    protected function tearDown(): void
+    {
+        foreach (glob($this->tempDir . '/*') as $file) {
+            unlink($file);
+        }
+        rmdir($this->tempDir);
+    }
+
+    private function buildCredentials(string $token = 'test-token', int $expiresIn = 3600): OauthResultClientCredentials
+    {
+        $creds = new OauthResultClientCredentials();
+        $creds->setAccessToken($token);
+        $creds->setTokenType('bearer');
+        $creds->setExpiresIn($expiresIn);
+        $creds->setGrantType('client_credentials');
+        $creds->calculateExpireDate(new DateTime());
+        return $creds;
+    }
+
+    /** @test */
+    public function shouldThrowExceptionWhenDirectoryDoesNotExist(): void
+    {
+        $this->expectException(OpenPayU_Exception_Configuration::class);
+        $this->expectExceptionMessageMatches('/not exist or not writable/');
+
+        new OauthCacheFile('/nonexistent/path/that/does/not/exist');
+    }
+
+    /** @test */
+    public function shouldThrowExceptionWhenDirectoryIsNotWritable(): void
+    {
+        $readonlyDir = $this->tempDir . '/readonly';
+        mkdir($readonlyDir, 0444);
+
+        try {
+            $this->expectException(OpenPayU_Exception_Configuration::class);
+            $this->expectExceptionMessageMatches('/not exist or not writable/');
+
+            new OauthCacheFile($readonlyDir);
+        } finally {
+            chmod($readonlyDir, 0777);
+            rmdir($readonlyDir);
+        }
+    }
+
+    /** @test */
+    public function shouldInstantiateWithValidDirectory(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $this->assertInstanceOf(OauthCacheFile::class, $cache);
+    }
+
+    /** @test */
+    public function shouldInstantiateWithDefaultDirectory(): void
+    {
+        $cache = new OauthCacheFile();
+        $this->assertInstanceOf(OauthCacheFile::class, $cache);
+    }
+
+    /** @test */
+    public function shouldReturnNullWhenKeyNotFound(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+
+        $result = $cache->get('nonexistent-key');
+
+        $this->assertNull($result);
+    }
+
+    /** @test */
+    public function shouldStoreAndRetrieveCredentials(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $creds = $this->buildCredentials('my-access-token');
+
+        $cache->set('my-key', $creds);
+        $result = $cache->get('my-key');
+
+        $this->assertInstanceOf(OauthResultClientCredentials::class, $result);
+        $this->assertSame('my-access-token', $result->getAccessToken());
+        $this->assertSame('bearer', $result->getTokenType());
+        $this->assertSame('client_credentials', $result->getGrantType());
+    }
+
+    /** @test */
+    public function shouldReturnTrueOnSuccessfulSet(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $creds = $this->buildCredentials();
+
+        $result = $cache->set('some-key', $creds);
+
+        $this->assertTrue($result);
+    }
+
+    /** @test */
+    public function shouldIsolateDifferentKeys(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $cache->set('key-a', $this->buildCredentials('token-a'));
+        $cache->set('key-b', $this->buildCredentials('token-b'));
+
+        $this->assertSame('token-a', $cache->get('key-a')->getAccessToken());
+        $this->assertSame('token-b', $cache->get('key-b')->getAccessToken());
+    }
+
+    /** @test */
+    public function shouldOverwriteExistingCacheEntry(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $cache->set('key', $this->buildCredentials('old-token'));
+        $cache->set('key', $this->buildCredentials('new-token'));
+
+        $result = $cache->get('key');
+        $this->assertSame('new-token', $result->getAccessToken());
+    }
+
+    /** @test */
+    public function shouldReturnNullForCorruptedCacheFile(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $corruptedFile = $this->tempDir . '/' . md5('corrupted-key');
+        file_put_contents($corruptedFile, 'this is not valid serialized data }{{{');
+
+        $result = $cache->get('corrupted-key');
+
+        $this->assertNull($result);
+    }
+
+    /** @test */
+    public function shouldReturnNullForNonOauthResultClientCredentialsInCacheFile(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $nonOauthResultClientCredentialsFile = $this->tempDir . '/' . md5('non-oauth-client-credentials');
+        file_put_contents($nonOauthResultClientCredentialsFile, serialize(new stdClass()));
+
+        $result = $cache->get('non-oauth-client-credentials');
+
+        $this->assertNull($result);
+    }
+
+    /** @test */
+    public function shouldInvalidateExistingCacheEntry(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $cache->set('key', $this->buildCredentials());
+
+        $result = $cache->invalidate('key');
+
+        $this->assertTrue($result);
+        $this->assertNull($cache->get('key'));
+    }
+
+    /** @test */
+    public function shouldReturnTrueWhenInvalidatingNonExistentKey(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+
+        $result = $cache->invalidate('does-not-exist');
+
+        $this->assertTrue($result);
+    }
+
+    /** @test */
+    public function shouldRemoveCacheFileOnInvalidate(): void
+    {
+        $cache = new OauthCacheFile($this->tempDir);
+        $key = 'key-to-remove';
+        $cache->set($key, $this->buildCredentials());
+
+        $cache->invalidate($key);
+
+        $this->assertFileDoesNotExist($this->tempDir . '/' . md5($key));
+    }
+}